feat(auth-server): replace accountAuthorizations with accountConsents #20590

LZoog · 2026-05-14T22:30:47Z

Are you sure we need all this? I thought when we tag a new release to stage, since the table doesn't actually exist now and got dropped successfully, that the patcher wouldn't try to go backwards?

Yea we do, the patcher looks at the current db level which is 35, so it will only go forward. These backwards patches are pulled from the train branch so that everything is in sync.

vbudhram · 2026-05-14T18:25:17Z

This should not delete on password reset

Ha, I was going to flag this after running the fxa-review skill. Should the comment be updated then?

LZoog · 2026-05-14T22:32:01Z

Could we call the table accountAuthorizations still? Or change the column names for these to be lastConsentedTosAt? I prefer authorize, but could live with consents if Nick/Wil like it too.

I would second this - just being consistent between table name and columns would be ideal imo, though I do like the consent suffix as it is explicit that it tracks user consent to applications

Going to stick with the consent name and update table column to be consistent.

vbudhram · 2026-05-14T18:26:26Z

Adding this config here saves us from having to add a database to do the service -> scope mapping. This mapping needs to live somewhere and this seemed the most straightforward way. We can always add a table later if there are more services.

LZoog · 2026-05-14T22:46:08Z

Since we have the allowlist that we have to specify services for, do we need this at all? Is it so we don't accidentally add Sync to the other list? 🤔

LZoog · 2026-05-14T22:37:11Z

Let's go ahead and add vpn here. As soon as we ship to stage, they can test.

I don't think I understand Services granted at token-exchange without consulting accountConsents - the entire point is that it checks the account consent table and sees that the user has consented, then denies or grants the request?

I wonder also if we should find a way to reference "Project" in these docs somehow referring to the "project umbrella" that Google uses for their cross-platform identity model.

edit: yep this replaces that hardcoded allowlist so this is nicer and just needs a doc update 👍 https://github.com/mozilla/fxa/pull/20590/changes#r3249064091

We should go ahead and add VPN to the other allowlist.

I wonder also if we should find a way to reference "Project" in these docs somehow referring to the "project umbrella" that Google uses for their cross-platform identity model.

Its a goal and the eventual state we want idk if needs to be in docs yet.

LZoog · 2026-05-14T22:52:23Z

Should these be an enum? Would be great to have that with a JSDoc comment so we know what each does. Reading just this chunk, I don't think I get what 'bypass' is for. It literally checks that the service is in the allow list, but it doesn't check that the user has ever consented? (I haven't tested this out, just trying to read the code)

LZoog · 2026-05-14T22:40:53Z

Did Claude make a typo with iff? 😀

Nah, just means if and only if

-Original file line number
+Diff line change
@@ -0,0 +1,12 @@
+    -- Recreates the accountAuthorizations table from the reverted level-34
+    -- attempt so the forward chain is complete on environments still at 33.
+    -- Patch 34->35 drops the table; nothing in current code reads or writes it.
+    CREATE TABLE IF NOT EXISTS `accountAuthorizations` (
+      `uid` BINARY(16) NOT NULL,
+      `scope` VARCHAR(512) NOT NULL,
+      `service` VARCHAR(64) NOT NULL,
+      `authorizedAt` BIGINT UNSIGNED NOT NULL,
+      PRIMARY KEY (`uid`, `scope`, `service`)
+    ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+    UPDATE dbMetadata SET value = '34' WHERE name = 'schema-patch-level';

Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		-- Reverse of patch-033-034. Reverse patching is disabled in the runner.

		-- UPDATE dbMetadata SET value = '33' WHERE name = 'schema-patch-level';

-Original file line number
+Diff line change
@@ -0,0 +1,7 @@
+    -- Drop the accountAuthorizations table left behind on environments that
+    -- ran the (since-reverted) level-34 migration. IF EXISTS keeps this safe
+    -- on environments where the table was never created.
+    DROP TABLE IF EXISTS `accountAuthorizations`;
+    UPDATE dbMetadata SET value = '35' WHERE name = 'schema-patch-level';

-Original file line number
+Diff line change
@@ -0,0 +1,11 @@
+    -- Reverse of patch-034-035. Reverse patching is disabled in the runner.
+    -- CREATE TABLE IF NOT EXISTS `accountAuthorizations` (
+    --   `uid` BINARY(16) NOT NULL,
+    --   `scope` VARCHAR(512) NOT NULL,
+    --   `service` VARCHAR(64) NOT NULL,
+    --   `authorizedAt` BIGINT UNSIGNED NOT NULL,
+    --   PRIMARY KEY (`uid`, `scope`, `service`)
+    -- ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+    -- UPDATE dbMetadata SET value = '34' WHERE name = 'schema-patch-level';

-Original file line number
+Diff line change
@@ -0,0 +1,26 @@
+    -- accountConsents: per-user OAuth consent ledger. One row per
+    -- (uid, scope, service, clientId). Written by the /authorization path on
+    -- consent acceptance (upserts, bumping lastAuthorizedTosAt). Read by the
+    -- /authorization pre-prompt check and by token-exchange. Cleared by
+    -- OauthDB.removeTokensAndCodes, which runs on account deletion and on
+    -- password reset.
+    --
+    -- The scope-to-service mapping that drives token-exchange resolution
+    -- lives in auth-server Convict config (oauthServer.exchange.serviceScopes),
+    -- not on a column here. Keeping it out of the DB makes adding a new
+    -- browser service a config + deploy, with no schema change.
+    CREATE TABLE IF NOT EXISTS `accountConsents` (
+      `uid` BINARY(16) NOT NULL,
+      `scope` VARCHAR(256) NOT NULL DEFAULT '',
+      `service` VARCHAR(64) NOT NULL DEFAULT '',
+      `clientId` BINARY(8) NOT NULL,
+      `firstAuthorizedTosAt` BIGINT UNSIGNED NOT NULL,
+      `lastAuthorizedTosAt` BIGINT UNSIGNED NOT NULL,
+      PRIMARY KEY (`uid`, `scope`, `service`, `clientId`),
+      -- idx_uid_service supports the token-exchange lookup
+      -- (WHERE uid=? AND service=?). The PK's leading-uid prefix would
+      -- otherwise force a uid-wide scan filtered in memory by service.
+      INDEX `idx_uid_service` (`uid`, `service`)
+    ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+    UPDATE dbMetadata SET value = '36' WHERE name = 'schema-patch-level';

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(auth-server): replace accountAuthorizations with accountConsents #20590

Uh oh!

Diff view

Diff view

There are no files selected for viewing

LZoog May 14, 2026

Uh oh!

vbudhram May 15, 2026

Uh oh!

vbudhram May 14, 2026

Uh oh!

nshirley May 15, 2026

Uh oh!

LZoog May 14, 2026

Uh oh!

nshirley May 15, 2026

Uh oh!

vbudhram May 15, 2026

Uh oh!

vbudhram May 14, 2026

Uh oh!

LZoog May 14, 2026

Uh oh!

LZoog May 14, 2026 •

edited

Loading

Uh oh!

vbudhram May 15, 2026 •

edited

Loading

Uh oh!

LZoog May 14, 2026

Uh oh!

LZoog May 14, 2026

Uh oh!

vbudhram May 15, 2026

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -1,3 +1,3 @@
     {
-      "level": 33
+      "level": 36
     }

-Original file line number
+Diff line change
@@ Expand Up / @@ -1443,6 +1443,31 @@ const convictConf = convict({ @@
             env: 'OAUTH_TOKEN_EXCHANGE_ALLOWED_SCOPES',
           },
         },
+        exchange: {
+          serviceScopes: {
+            doc: 'Map from browser-service name to its authoritative scope URL. Drives token-exchange scope resolution, /authorization service= validation, and the per-RP ToS audit. Adding a new browser service is a config change plus deploy, with no schema migration required.',
+            format: Object,
+            default: {
+              sync: 'https://identity.mozilla.com/apps/oldsync',
+              relay: 'https://identity.mozilla.com/apps/relay',
+              smartwindow: 'https://identity.mozilla.com/apps/smartwindow',
+              vpn: 'https://identity.mozilla.com/apps/vpn',
+            },
+            env: 'OAUTH_EXCHANGE_SERVICE_SCOPES',
+          },
+          denySilentForServices: {
+            doc: 'Services for which silent token-exchange is always rejected, irrespective of accountConsents rows. Wins over bypass. Today: sync, the Sync refresh token is the keys-bearing master credential.',
+            format: Array,
+            default: ['sync'],
+            env: 'OAUTH_EXCHANGE_DENY_SILENT_FOR_SERVICES',
+          },
+          bypassConsentForServices: {
+            doc: 'Services granted at token-exchange without consulting accountConsents. Today: relay, pending application-services landing a 4xx handler in iOS/Android Relay code. To be cleared once that ships.',
+            format: Array,
+            default: ['relay'],
+            env: 'OAUTH_EXCHANGE_BYPASS_CONSENT_FOR_SERVICES',
+          },
+        },
         git: {
           commit: {
             doc: 'Commit SHA when in stage/production',
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -132,6 +132,7 @@ describe('AccountDeleteManager', () => { @@
         mockOAuthDb = {
           removeTokensAndCodes: jest.fn().mockResolvedValue(undefined),
           removePublicAndCanGrantTokens: jest.fn().mockResolvedValue(undefined),
+          deleteAllConsentsForUser: jest.fn().mockResolvedValue(undefined),
         };
         Container.set(StripeHelper, mockStripeHelper);
@@ Expand Down Expand Up / @@ -210,6 +211,8 @@ describe('AccountDeleteManager', () => { @@
           expect(mockPushbox.deleteAccount).toHaveBeenCalledWith(uid);
           expect(mockOAuthDb.removeTokensAndCodes).toHaveBeenCalledTimes(1);
           expect(mockOAuthDb.removeTokensAndCodes).toHaveBeenCalledWith(uid);
+          expect(mockOAuthDb.deleteAllConsentsForUser).toHaveBeenCalledTimes(1);
+          expect(mockOAuthDb.deleteAllConsentsForUser).toHaveBeenCalledWith(uid);
           expect(mockLog.activityEvent).toHaveBeenCalledTimes(1);
           expect(mockLog.activityEvent).toHaveBeenCalledWith({
             uid,
@@ Expand Down Expand Up / @@ -309,6 +312,8 @@ describe('AccountDeleteManager', () => { @@
           );
           expect(mockOAuthDb.removeTokensAndCodes).toHaveBeenCalledTimes(1);
           expect(mockOAuthDb.removeTokensAndCodes).toHaveBeenCalledWith(uid);
+          expect(mockOAuthDb.deleteAllConsentsForUser).toHaveBeenCalledTimes(1);
+          expect(mockOAuthDb.deleteAllConsentsForUser).toHaveBeenCalledWith(uid);
         });
         it('should error if its not user requested', async () => {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -33,7 +33,10 @@ import { @@
       requestForGlean,
     } from './inactive-accounts';
-    type OAuthDbDeleteAccount = Pick<typeof OAuthDb, 'removeTokensAndCodes'> &
+    type OAuthDbDeleteAccount = Pick<
+      typeof OAuthDb,
+      'removeTokensAndCodes' | 'deleteAllConsentsForUser'
+    > &
       InactiveStatusOAuthDb;
     type PushboxDeleteAccount = Pick<
       ReturnType<typeof pushboxApi>,
@@ Expand Down Expand Up / @@ -150,7 +153,7 @@ export class AccountDeleteManager { @@
         }
         await this.deleteAccountFromDb(uid);
-        await this.deleteOAuthTokens(uid);
+        await this.deleteOAuthAccountData(uid);
         // data eng rely on this to delete the account data from BQ.
         // user self-deletes are logged when the client request was handled
@@ Expand Down Expand Up / @@ -185,7 +188,7 @@ export class AccountDeleteManager { @@
         try {
           await this.deleteAccountFromDb(uid);
-          await this.deleteOAuthTokens(uid);
+          await this.deleteOAuthAccountData(uid);
           this.statsd.increment('account.destroy.quick-delete');
         } catch (error) {
           // If the account wasn't fully deleted, we should log the error and
@@ Expand Down Expand Up / @@ -236,12 +239,11 @@ export class AccountDeleteManager { @@
         return true;
       }
-      /**
-       * Delete the account from the OAuth database. This will remove all tokens and
-       * codes associated with the account.
-       */
-      private async deleteOAuthTokens(uid: string) {
+      // Sweep tokens, codes, and consent. Consent is only swept on the
+      // account-delete path, never on password reset.
+      private async deleteOAuthAccountData(uid: string) {
         await this.oauthDb.removeTokensAndCodes(uid);
+        await this.oauthDb.deleteAllConsentsForUser(uid);
       }
       /**
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -24,6 +24,26 @@ const REFRESH_LAST_USED_AT_UPDATE_AFTER_MS = config.get( @@
       'oauthServer.refreshToken.updateAfter'
     );
+    // Service map and policy flags for token-exchange consent gating. Read
+    // once at module load; updates require a process restart.
+    const EXCHANGE_SERVICE_SCOPES = config.get('oauthServer.exchange.serviceScopes');
+    const EXCHANGE_KNOWN_SERVICES = new Set(Object.keys(EXCHANGE_SERVICE_SCOPES));
+    // Inverse map: canonical scope URL -> service name. Used by the
+    // exchange path to resolve the requested scope to its owning service
+    // without touching the DB.
+    const EXCHANGE_SCOPE_TO_SERVICE = new Map(
+      Object.entries(EXCHANGE_SERVICE_SCOPES).map(([service, scope]) => [
+        scope,
+        service,
+      ])
+    );
+    const EXCHANGE_DENY_SILENT_FOR_SERVICES = new Set(
+      config.get('oauthServer.exchange.denySilentForServices')
+    );
+    const EXCHANGE_BYPASS_CONSENT_FOR_SERVICES = new Set(
+      config.get('oauthServer.exchange.bypassConsentForServices')
+    );
     class OauthDB extends ConnectedServicesDb {
       get mysql() {
         return this.db;
@@ Expand Down Expand Up / @@ -229,6 +249,8 @@ class OauthDB extends ConnectedServicesDb { @@
         return ok;
       }
+      // Called from both account deletion AND password reset, so this must
+      // not touch the consent ledger — consent survives credential rotation.
       async removeTokensAndCodes(uid) {
         await this.ready();
         await this.redis.removeAccessTokensForUser(uid);
@@ Expand All / @@ -241,6 +263,78 @@ class OauthDB extends ConnectedServicesDb { @@
           ttlInMs || config.get('oauthServer.expiration.code')
         );
       }
+      // Upserts a consent row for the (uid, scope, service, clientId)
+      // tuple. First write seeds both timestamps to `now`; subsequent
+      // writes preserve firstAuthorizedTosAt and bump lastAuthorizedTosAt.
+      async recordSignInConsent({ uid, scope, service, clientId, now }) {
+        await this.ready();
+        return this.mysql._upsertAccountConsent(
+          uid,
+          scope,
+          service,
+          clientId,
+          now || Date.now()
+        );
+      }
+      // True when a consent row exists for the exact (uid, scope, service).
+      // Used by the /authorization pre-prompt check.
+      async hasConsentForSignIn(uid, scope, service) {
+        await this.ready();
+        const row = await this.mysql._findAccountConsentForSignIn(
+          uid,
+          scope,
+          service
+        );
+        return !!row;
+      }
+      // Applies the token-exchange decision matrix. Returns one of:
+      //   { result: 'allowed',       service }
+      //   { result: 'bypass',        service }
+      //   { result: 'denied',        service, reason: 'silent-disallowed' | 'no-consent' }
+      //   { result: 'fall-through' }
+      // Scope -> service resolution comes from the oauthServer.exchange.serviceScopes
+      // config map; unmapped scopes fall through to clients.allowedScopes.
+      async hasConsentForExchange(uid, scope) {
+        const service = EXCHANGE_SCOPE_TO_SERVICE.get(scope);
+        if (!service) {
+          return { result: 'fall-through' };
+        }
+        if (EXCHANGE_DENY_SILENT_FOR_SERVICES.has(service)) {
+          return { result: 'denied', service, reason: 'silent-disallowed' };
+        }
+        if (EXCHANGE_BYPASS_CONSENT_FOR_SERVICES.has(service)) {
+          return { result: 'bypass', service };
+        }
+        await this.ready();
+        const hasConsent = await this.mysql._hasConsentForService(uid, service);
+        if (hasConsent) {
+          return { result: 'allowed', service };
+        }
+        return { result: 'denied', service, reason: 'no-consent' };
+      }
+      async deleteAllConsentsForUser(uid) {
+        await this.ready();
+        return this.mysql._deleteAllAccountConsentsForUser(uid);
+      }
+      async listAccountConsentsByUid(uid) {
+        await this.ready();
+        return this.mysql._listAccountConsentsByUid(uid);
+      }
+      // True iff serviceName appears in the oauthServer.exchange.serviceScopes
+      // config map. Used by the /authorization writer to validate the URL's
+      // service= param before persisting it; unknown values are dropped to ''.
+      isKnownService(serviceName) {
+        if (!serviceName) {
+          return false;
+        }
+        return EXCHANGE_KNOWN_SERVICES.has(serviceName);
+      }
     }
     // Helper functions
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -176,6 +176,29 @@ const DELETE_REFRESH_TOKEN_WITH_CLIENT_AND_UID = @@
     const PRUNE_AUTHZ_CODES =
       'DELETE FROM codes WHERE TIMESTAMPDIFF(SECOND, createdAt, NOW()) > ? LIMIT 10000';
+    // First insert sets both timestamps to now. Subsequent /authorization
+    // completions for the same PK preserve firstAuthorizedTosAt and bump
+    // lastAuthorizedTosAt only, using GREATEST to guard against clock skew
+    // or reordered writes producing a backwards-moving lastAuthorizedTosAt.
+    const QUERY_ACCOUNT_CONSENT_UPSERT =
+      'INSERT INTO accountConsents ' +
+      '(uid, scope, service, clientId, firstAuthorizedTosAt, lastAuthorizedTosAt) ' +
+      'VALUES (?, ?, ?, ?, ?, ?) ' +
+      'ON DUPLICATE KEY UPDATE ' +
+      'lastAuthorizedTosAt = GREATEST(lastAuthorizedTosAt, VALUES(lastAuthorizedTosAt))';
+    const QUERY_ACCOUNT_CONSENT_FIND_SIGNIN =
+      'SELECT uid, scope, service, clientId, firstAuthorizedTosAt, lastAuthorizedTosAt ' +
+      'FROM accountConsents WHERE uid=? AND scope=? AND service=?';
+    // Direct lookup for the token-exchange gate after the caller has
+    // resolved scope -> service via config. Uses idx_uid_service.
+    const QUERY_HAS_CONSENT_FOR_SERVICE =
+      'SELECT 1 FROM accountConsents WHERE uid=? AND service=? LIMIT 1';
+    const QUERY_ACCOUNT_CONSENT_DELETE_BY_UID =
+      'DELETE FROM accountConsents WHERE uid=?';
+    const QUERY_ACCOUNT_CONSENT_LIST_BY_UID =
+      'SELECT uid, scope, service, clientId, firstAuthorizedTosAt, lastAuthorizedTosAt ' +
+      'FROM accountConsents WHERE uid=?';
     // Scope queries
     const QUERY_SCOPE_FIND = 'SELECT * ' + 'FROM scopes ' + 'WHERE scopes.scope=?;';
     const QUERY_SCOPES_INSERT =
@@ Expand Down Expand Up / @@ -607,6 +630,45 @@ class MysqlStore extends MysqlOAuthShared { @@
         return this._write(QUERY_REFRESH_TOKEN_DELETE, [buf(id)]);
       }
+      _upsertAccountConsent(uid, scope, service, clientId, now) {
+        return this._write(QUERY_ACCOUNT_CONSENT_UPSERT, [
+          buf(uid),
+          scope,
+          service,
+          buf(clientId),
+          now,
+          now,
+        ]);
+      }
+      async _findAccountConsentForSignIn(uid, scope, service) {
+        const rows = await this._read(QUERY_ACCOUNT_CONSENT_FIND_SIGNIN, [
+          buf(uid),
+          scope,
+          service,
+        ]);
+        return firstRow(rows) || null;
+      }
+      // True iff at least one consent row exists for (uid, service). Used
+      // by the exchange gate after the caller has resolved scope to service
+      // via config.
+      async _hasConsentForService(uid, service) {
+        const rows = await this._read(QUERY_HAS_CONSENT_FOR_SERVICE, [
+          buf(uid),
+          service,
+        ]);
+        return rows.length > 0;
+      }
+      _deleteAllAccountConsentsForUser(uid) {
+        return this._write(QUERY_ACCOUNT_CONSENT_DELETE_BY_UID, [buf(uid)]);
+      }
+      _listAccountConsentsByUid(uid) {
+        return this._read(QUERY_ACCOUNT_CONSENT_LIST_BY_UID, [buf(uid)]);
+      }
       getEncodingInfo() {
         var info = {};
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -91,3 +91,4 @@ exports.refreshToken = authServerValidators.refreshToken; @@
     exports.authorizationCode = authServerValidators.authorizationCode;
     exports.url = authServerValidators.url;
     exports.hexString = authServerValidators.hexString;
+    exports.service = authServerValidators.service;

feat(auth-server): replace accountAuthorizations with accountConsents #20590

Are you sure you want to change the base?

Uh oh!

feat(auth-server): replace accountAuthorizations with accountConsents #20590

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

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 14, 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!

vbudhram May 15, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LZoog May 14, 2026 •

edited

Loading

vbudhram May 15, 2026 •

edited

Loading