Skip to content

move fcm token storage #1175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 1, 2025
Merged

move fcm token storage #1175

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1fe5c8d
move fcm token from proving store to setting store for better separat…
aaronmgdr Oct 1, 2025
4854d6f
use analytics on adapter
aaronmgdr Oct 1, 2025
3afb31d
Revert "use analytics on adapter"
aaronmgdr Oct 1, 2025
77b85dc
prettier
aaronmgdr Oct 1, 2025
369d9d3
please be good
aaronmgdr Oct 1, 2025
7478477
ik heb oopsie wooopsie
aaronmgdr Oct 1, 2025
e5522cc
remove
aaronmgdr Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions app/src/providers/selfClientProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { unsafe_getPrivateKey } from '@/providers/authProvider';
import { selfClientDocumentsAdapter } from '@/providers/passportDataProvider';
import { logNFCEvent, logProofEvent } from '@/Sentry';
import { useSettingStore } from '@/stores/settingStore';
import analytics from '@/utils/analytics';

type GlobalCrypto = { crypto?: { subtle?: Crypto['subtle'] } };
Expand Down Expand Up @@ -147,7 +148,7 @@
navigationRef.navigate('UnsupportedDocument', {
countryCode,
documentCategory,
} as any);

Check warning on line 151 in app/src/providers/selfClientProvider.tsx

View workflow job for this annotation

GitHub Actions / build-deps 

Unexpected any. Specify a different type

Check warning on line 151 in app/src/providers/selfClientProvider.tsx

View workflow job for this annotation

GitHub Actions / workspace-lint 

Unexpected any. Specify a different type
}
},
);
Expand All @@ -158,6 +159,35 @@
}
});

addListener(
SdkEvents.PROVING_BEGIN_GENERATION,
async ({ uuid, isMock, context }) => {
const { fcmToken } = useSettingStore.getState();

if (fcmToken) {
try {
adapters.analytics?.trackEvent('DEVICE_TOKEN_REG_STARTED');
logProofEvent('info', 'Device token registration started', context);

const { registerDeviceToken: registerFirebaseDeviceToken } =
await import('@/utils/notifications/notificationService');
await registerFirebaseDeviceToken(uuid, fcmToken, isMock);

adapters.analytics?.trackEvent('DEVICE_TOKEN_REG_SUCCESS');
logProofEvent('info', 'Device token registration success', context);
} catch (error) {
logProofEvent('warn', 'Device token registration failed', context, {
error: error instanceof Error ? error.message : String(error),
});
console.error('Error registering device token:', error);
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential sensitive data exposure in error logging.

The console.error statement logs the raw error object, which could potentially contain sensitive information such as tokens, API responses, or PII depending on what the registerFirebaseDeviceToken function might throw.

Consider sanitizing the error before logging:

-            console.error('Error registering device token:', error);
+            console.error('Error registering device token');

The error details are already captured in the analytics event and logProofEvent call, so the console.error can be simplified to avoid potential data leakage.

As per coding guidelines: Never log sensitive data including credentials, tokens, API keys, or private keys.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.error('Error registering device token:', error);
console.error('Error registering device token');
🤖 Prompt for AI Agents 
In app/src/providers/selfClientProvider.tsx around line 182, the console.error
currently logs the raw error object which may contain sensitive data; replace it
with a non-sensitive message (e.g., a generic "Failed to register device token")
or log only a sanitized error identifier/code, and remove any token/PII from the
output; ensure analytics/logProofEvent still capture full details privately,
while the console log avoids printing sensitive fields.

Copy link
Contributor Author

@aaronmgdr aaronmgdr Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a problem

Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaronmgdr Understood, I'll leave the logging as-is. Thanks for clarifying!

🧠 Learnings used 
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.

adapters.analytics?.trackEvent('DEVICE_TOKEN_REG_FAILED', {
message: error instanceof Error ? error.message : String(error),
});
}
}
},
);
Copy link

@cursor cursor bot Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: FCM Token Registration Race Conditions

The FCM token registration, now an async PROVING_BEGIN_GENERATION event listener, is prone to two race conditions. The proving flow doesn't await its completion, and the fcmToken can be null if the settings store hasn't rehydrated. This may cause users to miss push notifications for proof status updates.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor Author

@aaronmgdr aaronmgdr Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the proving flow doesnt need the notification for completion

Copy link
Contributor Author

@aaronmgdr aaronmgdr Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the token will be there.... trust me


addListener(SdkEvents.PROOF_EVENT, ({ level, context, event, details }) => {
// Log proof events for monitoring/debugging
logProofEvent(level, event, context, details);
Expand All @@ -169,7 +199,7 @@
});

return map;
}, []);

Check warning on line 202 in app/src/providers/selfClientProvider.tsx

View workflow job for this annotation

GitHub Actions / build-deps 

React Hook useMemo has a missing dependency: 'adapters.analytics'. Either include it or remove the dependency array

Check warning on line 202 in app/src/providers/selfClientProvider.tsx

View workflow job for this annotation

GitHub Actions / workspace-lint 

React Hook useMemo has a missing dependency: 'adapters.analytics'. Either include it or remove the dependency array

return (
<SDKSelfClientProvider
Expand Down
5 changes: 3 additions & 2 deletions app/src/screens/prove/ConfirmBelongingScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { Title } from '@/components/typography/Title';
import useHapticNavigation from '@/hooks/useHapticNavigation';
import { ExpandableBottomLayout } from '@/layouts/ExpandableBottomLayout';
import { styles } from '@/screens/prove/ProofRequestStatusScreen';
import { useSettingStore } from '@/stores/settingStore';
import { flushAllAnalytics, trackNfcEvent } from '@/utils/analytics';
import { black, white } from '@/utils/colors';
import { notificationSuccess } from '@/utils/haptic';
Expand All @@ -40,8 +41,8 @@ const ConfirmBelongingScreen: React.FC<ConfirmBelongingScreenProps> = () => {
const [_requestingPermission, setRequestingPermission] = useState(false);
const currentState = useProvingStore(state => state.currentState);
const init = useProvingStore(state => state.init);
const setFcmToken = useProvingStore(state => state.setFcmToken);
const setUserConfirmed = useProvingStore(state => state.setUserConfirmed);
const setFcmToken = useSettingStore(state => state.setFcmToken);
const isReadyToProve = currentState === 'ready_to_prove';
useEffect(() => {
notificationSuccess();
Expand Down Expand Up @@ -74,7 +75,7 @@ const ConfirmBelongingScreen: React.FC<ConfirmBelongingScreenProps> = () => {
if (permissionGranted) {
const token = await getFCMToken();
if (token) {
setFcmToken(token, selfClient);
setFcmToken(token);
trackEvent(ProofEvents.FCM_TOKEN_STORED);
}
}
Expand Down
5 changes: 5 additions & 0 deletions app/src/stores/settingStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ interface PersistedSettingsState {
isDevMode: boolean;
setDevModeOn: () => void;
setDevModeOff: () => void;
fcmToken: string | null;
setFcmToken: (token: string | null) => void;
}

interface NonPersistedSettingsState {
Expand Down Expand Up @@ -69,6 +71,9 @@ export const useSettingStore = create<SettingsState>()(
setDevModeOn: () => set({ isDevMode: true }),
setDevModeOff: () => set({ isDevMode: false }),

fcmToken: null,
setFcmToken: (token: string | null) => set({ fcmToken: token }),

// Non-persisted state (will not be saved to storage)
hideNetworkModal: false,
setHideNetworkModal: (hideNetworkModal: boolean) => {
Expand Down
33 changes: 7 additions & 26 deletions packages/mobile-sdk-alpha/src/proving/provingMachine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,7 @@ export interface ProvingState {
error_code: string | null;
reason: string | null;
endpointType: EndpointType | null;
fcmToken: string | null;
env: 'prod' | 'stg' | null;
setFcmToken: (token: string, selfClient: SelfClient) => void;
init: (
selfClient: SelfClient,
circuitType: 'dsc' | 'disclose' | 'register',
Expand Down Expand Up @@ -487,11 +485,6 @@ export const useProvingStore = create<ProvingState>((set, get) => {
error_code: null,
reason: null,
endpointType: null,
fcmToken: null,
setFcmToken: (token: string, selfClient: SelfClient) => {
set({ fcmToken: token });
selfClient.trackEvent(ProofEvents.FCM_TOKEN_STORED);
},
_handleWebSocketMessage: async (event: MessageEvent, selfClient: SelfClient) => {
if (!actor) {
console.error('Cannot process message: State machine not initialized.');
Expand Down Expand Up @@ -1201,7 +1194,7 @@ export const useProvingStore = create<ProvingState>((set, get) => {
startProving: async (selfClient: SelfClient) => {
_checkActorInitialized(actor);
const startTime = Date.now();
const { wsConnection, sharedKey, passportData, secret, uuid, fcmToken } = get();
const { wsConnection, sharedKey, passportData, secret, uuid } = get();
const context = createProofContext(selfClient, 'startProving', {
sessionId: uuid || get().uuid || 'unknown-session',
});
Expand All @@ -1223,24 +1216,12 @@ export const useProvingStore = create<ProvingState>((set, get) => {
}

try {
if (fcmToken) {
try {
const isMockPassport = passportData?.mock;
selfClient.trackEvent(ProofEvents.DEVICE_TOKEN_REG_STARTED);
selfClient.logProofEvent('info', 'Device token registration started', context);
await selfClient.registerNotificationsToken(uuid, fcmToken, isMockPassport);
selfClient.trackEvent(ProofEvents.DEVICE_TOKEN_REG_SUCCESS);
selfClient.logProofEvent('info', 'Device token registration success', context);
} catch (error) {
selfClient.logProofEvent('warn', 'Device token registration failed', context, {
error: error instanceof Error ? error.message : String(error),
});
console.error('Error registering device token:', error);
selfClient.trackEvent(ProofEvents.DEVICE_TOKEN_REG_FAILED, {
message: error instanceof Error ? error.message : String(error),
});
}
}
// Emit event for FCM token registration
selfClient.emit(SdkEvents.PROVING_BEGIN_GENERATION, {
uuid,
isMock: passportData?.mock ?? false,
context,
});

selfClient.trackEvent(ProofEvents.PAYLOAD_GEN_STARTED);
selfClient.logProofEvent('info', 'Payload generation started', context);
Expand Down
13 changes: 13 additions & 0 deletions packages/mobile-sdk-alpha/src/types/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ export enum SdkEvents {
* and guide them through the recovery process to regain access.
*/
PROVING_ACCOUNT_RECOVERY_REQUIRED = 'PROVING_ACCOUNT_RECOVERY_REQUIRED',

/**
* Emitted when the proving generation process begins.
*
* **Recommended:** Use this to handle notification token registration and other setup tasks
* that need to occur when proof generation starts.
*/
PROVING_BEGIN_GENERATION = 'PROVING_BEGIN_GENERATION',
/**
* Emitted for various proof-related events during the proving process.
*
Expand Down Expand Up @@ -97,6 +105,11 @@ export interface SDKEventMap {
documentCategory: DocumentCategory | null;
};
[SdkEvents.PROVING_ACCOUNT_RECOVERY_REQUIRED]: undefined;
[SdkEvents.PROVING_BEGIN_GENERATION]: {
uuid: string;
isMock: boolean;
context: ProofContext;
};

[SdkEvents.PROGRESS]: Progress;
[SdkEvents.ERROR]: Error;
Expand Down
Loading