feat(auth-server): account-level authorizations for browser services by vbudhram · Pull Request #20500 · mozilla/fxa

vbudhram · 2026-05-01T16:41:38Z

Because

ADR 0048 specifies a per-account consent record so token-exchange can be gated per service rather than via a global allowlist
Today there is no record of which Firefox browser services (Sync, Relay, VPN, Smart Window) a user has authorized, so per-service revocation and inactive-account signals are not possible
Firefox Desktop's single clientId mints tokens for multiple services, and the wire scope alone cannot tell which service the user is actually authorizing in a given flow

This pull request

Adds accountAuthorizations(uid, scope, service, authorizedAt) in fxa_oauth (patch-033-034.sql + rollback)
Adds the oauthServer.browserServices Convict block (relay/smartwindow/sync/vpn) and lib/oauth/browser-services.ts resolver, writer, and SERVICE_AMBIGUOUS_CLIENT_IDS set for Desktop
Replaces the global allowlist in lib/routes/oauth/token.js token-exchange path with a per-scope check that enforces accountAuthorizations rows, falls through to the legacy allowlist for unconfigured scopes, and bypasses Relay until application-services handles 4xx
Records consent at lib/routes/oauth/authorization.js for offline grants, taking service= as the trustworthy intent signal for Desktop and writing additional rows for any configured browser-service scope on the wire
Skips the /oauth/token writer for Desktop's clientId since /oauth/authorization covers it; non-ambiguous clientIds (mobile, web RPs) keep the scope-iteration path with a clientId-membership guard
Plumbs service through createOAuthCode in fxa-auth-client and through getOAuthData in fxa-settings
Adds StatsD metrics: oauth.token_exchange.resolution{service, outcome}, account_authz.upsert.attempt, account_authz.upsert.error{err_type}

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-12930
Closes: https://mozilla-hub.atlassian.net/browse/FXA-12931

Checklist

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.
I have added necessary documentation (if appropriate).
I have verified that my changes render correctly in RTL (if appropriate).
I have manually reviewed all AI generated code.

Other information

Automated:

nx run db-migrations:migrate to apply patch-033-034.
yarn jest lib/oauth/browser-services.spec.ts lib/routes/oauth/token.spec.ts lib/routes/attached-clients.spec.ts (100 unit tests).
With infrastructure running: yarn test test/remote/account_authorizations.in.spec.ts.

Manual (Firefox Desktop):

Sign into Smart Window via the in-browser flow → expect rows for smartwindow and sync in accountAuthorizations.
Sign into Sync via the avatar-menu flow → expect a single sync row (deduped).
Sign into Relay via the relay-integration flow → expect rows for relay and sync.
Mobile (Fenix/iOS) sign-in → expect sync row written via the existing /oauth/token path.

Note: The SERVICE_AMBIGUOUS_CLIENT_IDS Desktop carve-out is a transitional measure; remove once Desktop completes the application-services migration to refresh tokens.

Copilot

Pull request overview

Adds an account-level accountAuthorizations projection for “browser services” and uses it to gate /oauth/token token-exchange for configured service scopes (with a temporary Relay carve-out), while recording/refresh-touching authorization state from refresh-token activity.

Changes:

Introduces oauthServer.browserServices config and a new accountAuthorizations table (migration level 33).
Adds browser-services resolution + writer/touch helpers, and wires them into /oauth/token and refresh-token reads.
Adds unit + remote integration tests for DB helpers, writers/touches, and token-exchange authorization behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/fxa-auth-server/test/remote/account_authorizations_writers.in.spec.ts	Remote integration coverage for write/touch semantics (login, refresh throttling, deletion).
packages/fxa-auth-server/test/remote/account_authorizations_token_exchange.in.spec.ts	Remote integration coverage for token-exchange authorization behavior (Relay carve-out, configured/unconfigured scopes).
packages/fxa-auth-server/test/remote/account_authorizations_db.in.spec.ts	Remote integration coverage for new DB CRUD + throttled touch behavior.
packages/fxa-auth-server/lib/routes/oauth/token.spec.ts	Unit tests asserting token-exchange behavior re: Relay bypass and accountAuthorizations checks.
packages/fxa-auth-server/lib/routes/oauth/token.js	Enforces accountAuthorizations checks for configured scopes in token-exchange; records authorizations on offline grants.
packages/fxa-auth-server/lib/oauth/db/mysql/index.js	Adds MySQL queries + store methods for `accountAuthorizations` upsert/find/list/delete.
packages/fxa-auth-server/lib/oauth/db/index.js	Exposes OauthDB API for accountAuthorizations; touches authorizations when refresh tokens are read; deletes rows on account deletion.
packages/fxa-auth-server/lib/oauth/browser-services.ts	New helper for config validation, scope/client/serviceParam resolution, Relay bypass, login recording, refresh touch.
packages/fxa-auth-server/lib/oauth/browser-services.spec.ts	Unit tests for config validation, resolution, bypass, login writer, and refresh touch helper behavior.
packages/fxa-auth-server/config/index.ts	Adds `oauthServer.browserServices` configuration (default service entries + metadata).
packages/db-migrations/databases/fxa_oauth/target-patch.json	Bumps schema patch level from 32 → 33.
packages/db-migrations/databases/fxa_oauth/patches/patch-032-033.sql	Creates the `accountAuthorizations` table and advances patch level.
packages/db-migrations/databases/fxa_oauth/patches/patch-033-032.sql	Down-migration reference for dropping `accountAuthorizations` (commented per repo convention).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-05-01T16:47:22Z

+  afterEach(async () => {
+    // Each test uses a fresh uid, but make sure we clean up across reruns.
+  });
+
+  it('round-trips upsert + get + delete', async () => {
+    const id = uid();
+    const now = Date.now();
+    await db.upsertAccountAuthorization(id, RELAY_SCOPE, 'relay', now, now);
+    const row = await db.getAccountAuthorization(id, RELAY_SCOPE, 'relay');
+    expect(row).toBeTruthy();
+    expect(row.scope).toBe(RELAY_SCOPE);
+    expect(row.service).toBe('relay');
+    expect(Number(row.authorizedAt)).toBe(now);
+    expect(Number(row.lastUsedAt)).toBe(now);
+
+    await db.deleteAccountAuthorization(id, RELAY_SCOPE, 'relay');
+    const gone = await db.getAccountAuthorization(id, RELAY_SCOPE, 'relay');
+    expect(gone).toBeNull();
+  });
+
+  it('returns null for an unknown row', async () => {
+    const row = await db.getAccountAuthorization(uid(), RELAY_SCOPE, 'relay');
+    expect(row).toBeNull();
+  });
+
+  it('upsert is idempotent on the primary key', async () => {
+    const id = uid();
+    const t0 = Date.now();
+    await db.upsertAccountAuthorization(id, RELAY_SCOPE, 'relay', t0, t0);
+    // Second upsert with the unthrottled wrapper still preserves authorizedAt
+    // because ON DUPLICATE KEY UPDATE only touches lastUsedAt.
+    await db.upsertAccountAuthorization(
+      id,
+      RELAY_SCOPE,
+      'relay',
+      t0 + 86400000 * 2,
+      t0 + 86400000 * 2
+    );
+    const row = await db.getAccountAuthorization(id, RELAY_SCOPE, 'relay');
+    expect(Number(row.authorizedAt)).toBe(t0);
+    expect(Number(row.lastUsedAt)).toBe(t0 + 86400000 * 2);
+  });


This file leaves some rows behind in the accountAuthorizations table (e.g., the idempotent upsert, touch throttle, list-by-uid, and “same uid + scope + different service” tests don’t delete their inserted rows). Since these remote integration tests run against a DB, this can cause test data accumulation across reruns. Add cleanup (e.g., track generated uids and call deleteAllAccountAuthorizationsForUser in afterEach) or explicitly delete in each test.

Copilot · 2026-05-01T16:47:22Z

+-- Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist and is the
+-- single source of truth for "user X is connected to service Y".


The migration header comment says this table “Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist”, but lib/routes/oauth/token.js still falls back to TOKEN_EXCHANGE_ALLOWED_SCOPES for unconfigured scopes. Consider rewording this comment to reflect that the table replaces the allowlist only for configured browser-service scopes (with legacy allowlist fallback still in place).

Suggested change

-- Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist and is the

-- single source of truth for "user X is connected to service Y".

-- Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist for

-- configured browser-service scopes; unconfigured scopes still fall back to

-- the legacy allowlist. This is the source of truth for "user X is

-- connected to service Y" for configured browser-service scopes.

Copilot · 2026-05-01T16:47:23Z

+    // Resolve from the requested scope only; the bearer's clientId proves
+    // authentication, not which service the user is asking for.
    const requestedScope = params.scope;
-    if (!TOKEN_EXCHANGE_ALLOWED_SCOPES.contains(requestedScope)) {
+    const resolution = getAuthorizationScope(
+      undefined,
+      undefined,
+      requestedScope
+    );
+


Token-exchange authorization checks only the first configured scope found in params.scope via getAuthorizationScope(...). Because params.scope is a ScopeSet, a request can include multiple scopes; if the first one resolves to Relay (bypass) this will skip the table check and still grant additional configured scopes (e.g., Relay + Smartwindow) without verifying authorization for the others. Update this logic to validate each configured scope value in the requested ScopeSet (enforcing allowSilentExchange/accountAuthorizations per-scope), and only apply the Relay bypass to the Relay scope itself (not the whole request).

Copilot · 2026-05-01T16:47:23Z

+      // Bump lastUsedAt on matching accountAuthorizations rows; throttled in SQL.
+      const now = extraMetadata.lastUsedAt.getTime
+        ? extraMetadata.lastUsedAt.getTime()
+        : extraMetadata.lastUsedAt;
+      await touchAuthorizationsForRefresh(this, undefined, {
+        uid: t.userId,
+        scope: t.scope,
+        now,
+      });


touchAuthorizationsForRefresh(...) is invoked on every getRefreshToken call, which means each refresh-token use will issue additional SQL write(s) (one per configured scope value) even when within the throttle window. The SQL-level throttle prevents the column from changing but does not avoid the write/query overhead. Consider gating this call behind the same REFRESH_LAST_USED_AT_UPDATE_AFTER_MS check used for _touchRefreshToken, or otherwise reducing DB write frequency (while still handling the “missing row” case if that behavior is required).

vbudhram · 2026-05-04T17:45:52Z

        env: 'OAUTH_TOKEN_EXCHANGE_ALLOWED_SCOPES',
      },
    },
+    browserServices: {


These values were taken directly from the Firefox and mobile repos.

LZoog · 2026-05-05T15:29:08Z

+  `scope` VARCHAR(512) NOT NULL,
+  `service` VARCHAR(64) NOT NULL,
+  `authorizedAt` BIGINT UNSIGNED NOT NULL,
+  `lastUsedAt` BIGINT UNSIGNED,


What will we use lastUsedAt for if we'll track it / use it in the inactive accounts table?

LZoog · 2026-05-05T15:31:53Z

      },
    },
+    browserServices: {
+      doc: 'FxA-trusted browser services keyed by service name. Drives accountAuthorizations writes/reads, token-exchange auth checks, and Connected Services.',


tiny nit: "and Connected Services" here sounds a little confusing, though we might later display this table in Settings for users to let them choose to revoke all consents. Maybe and partially, Connected Services is more clear?

LZoog · 2026-05-05T15:33:01Z

+            '9ebfe2c2f9ea3c58',
+            '41b4363ae36440a9',
+            '723aa3bce05884d8',
+            // Firefox Desktop and Thunderbird mint Relay tokens directly


We should remove Thunderbird here, they don't create Relay tokens

LZoog · 2026-05-05T15:39:38Z

+            // post-ADR-0048 refresh-token migration.
+            '5882386c6d801776',
+            '8269bacd7bbc7f80',
+            // Mobile Firefox clients already use refresh tokens.


Just a nit, we can probably remove already use refresh tokens 🤷‍♀️

LZoog · 2026-05-05T15:50:19Z

+
+// Relay carve-out: bypass the table check until application-services handles 4xx
+// from token-exchange. Greppable so a future cleanup can't silently break Relay.
+export function shouldBypassAuthCheck(


👍 We can let Mark/Jonathan know about this. Since Mobile ships pretty quick and the Relay MVP was for a small audience, I don't imagine we'll have to maintain it for too long.

LZoog · 2026-05-05T15:50:53Z

+  scopeIdx: Map<string, string>;
+  // A clientId can participate in multiple services (Firefox Desktop mints
+  // Sync and Relay tokens). Scope match wins; clientId fallback uses the
+  // first declared service.


LZoog · 2026-05-05T15:52:47Z

+  const seenServiceParams = new Map<string, string>();
+  const seenScopes = new Map<string, string>();
+
+  for (const [name, c] of Object.entries(cfg)) {


Just a nit, but I think I prefer config over cfg / maybe service or a different name over c?

LZoog · 2026-05-05T16:03:54Z

+      )
+    );
+  } catch (err) {
+    log?.warn?.('accountAuthorizations.upsertFailed', {


Probably log a Sentry event?

LZoog · 2026-05-05T16:05:18Z

+  }
+}
+
+// Bump lastUsedAt for each matching scope on refresh. SQL throttles writes;


Assuming we pull lastUsedAt out of this table, we should save some context/notes/code here for the inactive account table ticket 👀

LZoog · 2026-05-05T16:12:42Z


-    // Validate requested scope is in allowlist
+    // Authorize scope-by-scope. One Firefox clientId mints tokens for
+    // several services, so we resolve off the requested scope, not the


Super nit: One Firefox clientId could be "Firefox desktop" just so it's super clear

Is we resolve off the requested scope accurate or does that mean for non-desktop requests? Would we still be able to record Desktop logins for service=vpn if they provide the VPN scope? It doesn't look like we have to record Desktop VPN logins now btw, but, could be a nice bonus because it'd be one less point of friction for users trying to use VPN on mobile. Same for Relay, but they are not working on their post-MVP yet so I'm less concerned with them.

dschom · 2026-05-05T23:42:06Z

+  `scope` VARCHAR(512) NOT NULL,
+  `service` VARCHAR(64) NOT NULL,
+  `authorizedAt` BIGINT UNSIGNED NOT NULL,
+  PRIMARY KEY (`uid`, `scope`, `service`)


Do we want an fk?

dschom · 2026-05-05T23:47:04Z

    it('rejects unauthorized scopes', async () => {
      const UNAUTHORIZED_SCOPE =
        'https://identity.mozilla.com/apps/unauthorized';
+      mockStatsD.increment.mockClear();


does 'resetAndMockDeps' not cover this?

LZoog

Thank you @vbudhram for all your work here. I did a thorough review yesterday and glanced through it again today and did a lot of testing, but didn't do as thorough of a code review.

Checks I did locally:

Signing into, and signing up, for an RP with a matching scope/client IDs adds a record to the table
Removing the RP refresh token from connected services deletes the row from the table
Signing into, and signing up, for an RP with a matching scope but NOT matching client ID does NOT add a record
Signing into service= Smart Window, VPN, and Relay through cached sign-in, signing up, and password entry, adds the smart window etc. scope/auth to the row
Deleting the account removes all the rows

Notes and caveats for posterity:

Sync is added to the auth table for all browser flows, because through all browser service sign-ins, a refresh token with the Sync scope gets created (but desktop discards it)
- this is something we have discussed and have agreed it's okay since the primary use of the table is for the silent token exchange grant, and we aren't going to silently grant the Sync scope. It will be fixed when Desktop moves to refresh tokens. If we could add special casing for this we'd need special casing on the backfill script too
Removing the session token does not revoke the account-level consent, because we can't associate the service/scope with the session token (Ross says this is OK)
We can remove some of our "record at login" pieces when Desktop switches to a refresh token
authorizedAt does not get updated when re-authorizing a service - TBD / will file a follow up to confirm Product's desire

Because: - ADR 0048 specifies a per-account consent record so token-exchange can be gated per service rather than via a global allowlist. - Today there is no record of which Firefox browser services (Sync, Relay, VPN, Smart Window) a user has authorized, so per-service revocation and inactive-account signals are not possible. - Firefox Desktop uses a single clientId across multiple services, and the wire scope alone cannot tell which service the user is actually authorizing in a given flow. This commit: - Adds accountAuthorizations(uid, scope, service, authorizedAt) in fxa_oauth (patch-033-034 + rollback) and DB read/write methods. - Adds the oauthServer.browserServices Convict block (relay, sync, smartwindow, vpn) and lib/oauth/browser-services.ts with the resolver, login-time writer, and SERVICE_AMBIGUOUS_CLIENT_IDS carve-out for Firefox Desktop. - Replaces the global allowlist in lib/routes/oauth/token.js token-exchange path with a per-scope check that requires an accountAuthorizations row, falls through to the legacy allowlist for unconfigured scopes, and bypasses Relay until application-services handles 4xx. - Records consent at lib/routes/oauth/authorization.js for offline grants, taking service= as the trustworthy intent signal for the ambiguous Desktop clientId and writing additional rows for any configured browser-service scope on the wire. - Plumbs service through createOAuthCode in fxa-auth-client and through getOAuthData in fxa-settings so the param reaches /oauth/authorization. - Cleans up accountAuthorizations rows in attached-clients.js when the last refresh token for a service is destroyed. - Emits per-service statsd metrics for dashboards and alerting: oauth.token_exchange.resolution{service, outcome}, account_authz.upsert.attempt|success|error{service}, account_authz.write.skipped{reason}, and account_authz.cleanup.deleted|failed|snapshot_failed. - Adds unit coverage for the writer, token-exchange gate, and metric paths, plus integration tests for end-to-end flows on Desktop (Smart Window, Sync, Relay) and Fenix.

Copilot AI review requested due to automatic review settings May 1, 2026 16:41

vbudhram requested a review from a team as a code owner May 1, 2026 16:41

Copilot started reviewing on behalf of vbudhram May 1, 2026 16:42 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

vbudhram force-pushed the fxa-12930-orig branch 11 times, most recently from 87956ca to d742bf1 Compare May 4, 2026 17:41

vbudhram commented May 4, 2026

View reviewed changes

vbudhram force-pushed the fxa-12930-orig branch 8 times, most recently from 99eb984 to 49df960 Compare May 4, 2026 20:13

nshirley reviewed May 4, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/db/mysql/index.js Outdated

nshirley reviewed May 4, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/oauth/db/mysql/index.js Outdated

nshirley reviewed May 4, 2026

View reviewed changes

Comment thread packages/fxa-auth-server/lib/routes/attached-clients.js Outdated

nshirley mentioned this pull request May 4, 2026

feat(auth): Backfill accountAuthorizations with refresh tokens #20513

Open

5 tasks

vbudhram force-pushed the fxa-12930-orig branch from 49df960 to a60201a Compare May 5, 2026 00:57

LZoog reviewed May 5, 2026

View reviewed changes

vbudhram force-pushed the fxa-12930-orig branch 3 times, most recently from 51d444c to a4079a2 Compare May 5, 2026 17:03

vbudhram self-assigned this May 5, 2026

dschom reviewed May 5, 2026

View reviewed changes

vbudhram force-pushed the fxa-12930-orig branch 4 times, most recently from 4bab8d7 to f780423 Compare May 6, 2026 17:04

LZoog approved these changes May 6, 2026

View reviewed changes

vbudhram force-pushed the fxa-12930-orig branch from f780423 to 8ada436 Compare May 6, 2026 17:24

vbudhram merged commit b0fff0c into main May 6, 2026
21 checks passed

vbudhram deleted the fxa-12930-orig branch May 6, 2026 18:00

LZoog mentioned this pull request May 7, 2026

feat(admin): expose accountAuthorizations on the admin panel #20534

Open

5 tasks

		-- Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist and is the
		-- single source of truth for "user X is connected to service Y".

--- Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist and is the
--- single source of truth for "user X is connected to service Y".
+-- Replaces the hardcoded TOKEN_EXCHANGE_ALLOWED_SCOPES allowlist for
+-- configured browser-service scopes; unconfigured scopes still fall back to
+-- the legacy allowlist. This is the source of truth for "user X is
+-- connected to service Y" for configured browser-service scopes.

Conversation

vbudhram commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Because

This pull request

Issue that this pull request solves

Checklist

Other information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vbudhram commented May 1, 2026 •

edited

Loading

LZoog May 5, 2026 •

edited

Loading

LZoog left a comment •

edited

Loading