feat(auth): Send client_id, service, firstAuthorization to attached services#20766
Open
LZoog wants to merge 1 commit into
Open
feat(auth): Send client_id, service, firstAuthorization to attached services#20766LZoog wants to merge 1 commit into
LZoog wants to merge 1 commit into
Conversation
…ervices Because: * Braze needs the client_id/service on signup, and to know the first time a user authorizes a new service or RP This commit: * Adds client_id to the 'verified' signup event * Adds firstAuthorization boolean to the 'login' event, derived from the accountAuthorizations table * Sets the 'login' event's 'service' to the browser service and falls back to the client_id (existing functionality) for web RPs * Removes some event-broker README docs in favor of ecosystem-platform docs closes FXA-13784
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates FxA “attached services” events so downstream consumers (e.g., Braze) can reliably identify the OAuth clientId, the effective service for browser/native flows, and whether a login corresponds to a user’s first authorization of a given service/RP. It also centralizes event format documentation by linking to ecosystem-platform docs.
Changes:
- Add
clientIdto theverifiedsignup event payload when available from request tags. - Add
firstAuthorizationto theloginevent, derived from existingaccountAuthorizationsrows read before consent writes. - Adjust
login.serviceto prefer resolved OAuthNative service (browser service) and document event formats via external docs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-event-broker/README.md | Replaces embedded event format docs with links to ecosystem-platform documentation. |
| packages/fxa-auth-server/lib/routes/utils/signup.spec.ts | Adds tests asserting verified event includes/omits clientId appropriately. |
| packages/fxa-auth-server/lib/routes/utils/signup.js | Includes clientId in verified notifications when request is tagged with a client id. |
| packages/fxa-auth-server/lib/routes/utils/account.ts | Adds clientId to verified event for flows that bypass AccountHandler.createAccount. |
| packages/fxa-auth-server/lib/routes/oauth/authorization.spec.ts | Extends consent-write tests to cover firstAuthorization read/flag behavior. |
| packages/fxa-auth-server/lib/routes/oauth/authorization.js | Implements best-effort first-authorization detection; updates login payload service and adds firstAuthorization. |
| packages/fxa-auth-server/lib/routes/account.ts | Adds clientId to verified event on account creation when email is already verified. |
| packages/fxa-auth-server/lib/oauth/first-authorization.ts | Adds helper to derive whether an authorization is the first for a service/RP. |
| packages/fxa-auth-server/lib/oauth/first-authorization.spec.ts | Adds unit tests for deriveFirstAuthorization across native/web/ambiguous cases. |
| packages/fxa-auth-server/lib/log.spec.ts | Adds regression test ensuring explicit clientId + firstAuthorization pass through when service is an OAuthNative service string. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+211
to
+214
| // Expose the resolved service so the `login` event reports `service` at the | ||
| // same grain as `firstAuthorization` (serviceTag alone misses scope-only | ||
| // flows, e.g. VPN cached sign-in that sends scope but no service=). | ||
| req.app.oauthService = serviceValue; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because:
This commit:
closes FXA-13784
--
For the
verifiedsignup event, we already sendservicefrom the query parameters over, interestingly enough. I addedclient_idthere.For
login,serviceis currently sent as theclient_id, but we also explicitly passclient_idtoo. So, here, I've changedserviceto be from the query params if available, and it still falls back toclient_id.Documentation added in mozilla/ecosystem-platform#784.