Repository navigation
fix: [SDK-5154] stop mixing dashboard sender id with the shared FCM project - #2731
abdulraqeeb33 wants to merge 1 commit into
Conversation
…roject FCM getToken() uses the Firebase project in FirebaseOptions, not gcmSenderId. Pairing a customer dashboard sender with onesignal-shared-public minted a token the customer's FCM v1 credentials cannot send to, which showed up as Invalid Google Project Number (-6) or Never Subscribed. Prefer google-services.json, then complete backend fcm params, and only use the shared project when the dashboard sender is that project's own sender. Co-authored-by: abdulraqeeb33 <abdulraqeeb33@users.noreply.github.com>
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)149/154 touched executable lines covered (96.8% — requires ≥ 80%) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Multi-model review
Claude Opus 5, GPT 5.6 Sol, Cursor Grok 4.6. Diff +1084/−47 (5 Kotlin files). No generated/lock/vendor/snapshots.
The consistent-project goal is right. Treating android_sender_id as a hard gate can recreate the same -6 this PR is fixing.
Act on
- Do not discard a complete customer project because the dashboard sender disagrees (Opus + Grok).
getToken()uses project fields, notgcmSenderId. Ifandroid_sender_idis still754795614042and backend/hostfcmis the customer project,compatibleWithDashboardfails anduseSharedwins — shared token again. Inverse: a customer sender plus a different-project backendfcmis rejected entirely → hard-6. Prefer any complete non-shared host/backend; use shared only when no customer project exists. isSharedis only applied to backend (3/3). A missing dashboard sender plus a host default that isonesignal-shared-publicstill mints a shared token — the case the comments say they refuse.
Consider
- Token fetch is outside
firebaseLock; start/HYDRATE candelete()the named app mid-Tasks.await(3/3). New vs old code. -6is not retryable. A miss on empty persistedfcmParamsbefore HYDRATE can unsubscribe until params arrive (Grok).hostDefaultFirebaseAppstill callsinitializeApp(context)when the host deferred Firebase init (Opus, Grok).- Unparseable
app_idis treated as compatible (Opus, Sol).
Noted / dismissed
- Switching named app → host default rotates tokens (Opus; release-note worthy, not a logic bug).
- Shared API key Base64 is pre-existing; this PR reduces when it is used.
missingProjectMessage/ host-mismatch log can blame the wrong source (nits).
No tests for: dashboard=shared + customer backend; host shared + null dashboard; concurrent getToken + delete().
Sent by Cursor Automation: PR Reviews
| val matchingDefaultApp = | ||
| defaultApp?.takeIf { it.isComplete && compatibleWithDashboard(it, dashboard) } | ||
| val matchingBackend = | ||
| backend?.takeIf { | ||
| it.isComplete && | ||
| !FcmSharedProject.isShared(it) && | ||
| compatibleWithDashboard(it, dashboard) | ||
| } | ||
| val useShared = dashboard == FcmSharedProject.SENDER_ID && sharedDefault.isComplete |
There was a problem hiding this comment.
Act on (Opus + Grok): dashboard sender is a hard gate, but getToken() ignores gcmSenderId.
- Dashboard still
754795614042+ complete customer backend/host →compatibleWithDashboardfails, thenuseSharedis true → shared-project token (the-6this PR fixes). Old code minted from the customerfcmfields. - Customer dashboard + different-project backend
fcm→ both sources rejected → hard-6.
Prefer any complete non-shared host/backend. Use shared only when no customer project exists; log sender mismatches. Also apply isShared to defaultApp — missing dashboard + shared host currently passes (3/3).
| override suspend fun getToken(senderId: String): String { | ||
| initFirebaseApp(senderId) | ||
| return getTokenWithClassFirebaseMessaging() | ||
| val app = initFirebaseApp(senderId) | ||
| return getTokenWithClassFirebaseMessaging(app) | ||
| } |
There was a problem hiding this comment.
Consider (3/3): firebaseLock covers resolve/delete/init only. Tasks.await runs after the lock is released. DeviceRegistrationListener fires retrievePushToken from start(), HYDRATE, and permission change on IO — so one caller can current.delete() the named app another is still awaiting. Old code never deleted. Hold the lock through token fetch, or refcount in-flight users before delete().
Description
One Line Summary
Register Android FCM tokens against one consistent Firebase project so we stop minting shared-project tokens that the dashboard reports as Invalid Google Project Number (
-6) or Never Subscribed.Details
Motivation
FirebaseMessaging.getToken()uses the Firebase project inFirebaseOptions(application id / project id / API key), notgcmSenderId. The SDK was initializing its ownFirebaseAppwith:onesignal-shared-public/754795614042) when backendfcmparams were missing or snapshotted empty in thePushRegistratorFCMconstructor beforeandroid_params.jshydratedThat mixed config still produced a token (the shared project does not care about sender id), so a subscription was created. OneSignal then sends with the customer's FCM v1 credentials, the token's project does not match, and the dashboard shows Invalid Google Project Number (
notification_types = -6) or Never Subscribed.This is the SDK-5154 split of the google-services / consistent-project work from #2725, without the Firebase Installation ID / SDK-4946 path.
Scope
FCM registration now picks one consistent project, in this order:
google-services.json— reuse the defaultFirebaseAppwhen it is complete and compatible with the dashboard sender (or when the dashboard sender is missing).fcmparams fromandroid_params.js, read at registration time (not constructor snapshot). Rejected when they are actually the shared public project, or when the application id's project number does not match the dashboard sender.754795614042. A customer sender is no longer paired with shared credentials.If none of those form a consistent project, registration returns
INVALID_FCM_SENDER_IDinstead of uploading a token from the shared project.Also:
FirebaseApp.getInstance()lookup beforeinitializeApp(context).getToken()calls so hydrated backend params can replace an earlier config (namedFirebaseAppis deleted when credentials change).FirebaseMessaging.register()path (that stays on SDK-4946).No public API changes.
Testing
Unit testing
FcmFirebaseConfigResolverTestscovers host match/mismatch, missing dashboard sender, backend consistency (including mixed application id / shared project classified as backend), and refusing to mix a customer sender with the shared project.PushRegistratorFCMTestscovers host reuse, backend params (including after construction and after a first registration), shared-project refusal for customer senders,INVALID_FCM_SENDER_IDfromregisterForPush, andhostDefaultFirebaseApplookup.:OneSignal:notifications:testDebugUnitTest --tests com.onesignal.notifications.internal.registration.impl.*, Spotless, and Detekt pass locally.Manual testing
Not run on a device in this environment. To verify on a device:
google-services.jsonwhose sender matches the OneSignal dashboard — logs should say registration is using that project, and a push should arrive.google-services.jsonbut with complete Android FCM config on the dashboard — registration should use backendfcmparams afterandroid_params.jshydrates (including first session).fcmparams — should not subscribe viaonesignal-shared-public; status should be Invalid Google Project Number rather than a silent shared-project token.Affected code checklist
Checklist
Overview
Testing
Final pass