fix(auth): For token exchange grant, use client_id from subject_token

Because:
* With this grant, we are currently calling 'authenticateClient' with the request query parameters. This type of token exchange does not need to look up the client by query param because we discern it from the 'subject_token' passed to us and validate against our allowlist in config instead.

This commit:
* Updates tokenHandler for the new grant type accordingly

follow on to FXA-12925

Author: LZoog

packages/fxa-auth-server/lib/routes/oauth/token.js

@@ -10,6 +10,7 @@
 //   * `grant_type=authorization_code` for vanilla exchange-a-code-for-a-token OAuth
 //   * `grant_type=refresh_token` for refreshing a previously-granted token
 //   * `grant_type=fxa-credentials` for directly granting via an FxA identity assertion
+//   * `grant_type=urn:ietf:params:oauth:grant-type:token-exchange` for token exchange, e.g. refresh token for a new refresh token
 //
 // And because of the different types of token that can be requested:
 //
@@ -239,15 +240,18 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
         requestedGrant = await validateAssertionGrant(client, params);
         break;
       case GRANT_TOKEN_EXCHANGE:
-        requestedGrant = await validateTokenExchangeGrant(client, params);
+        requestedGrant = await validateTokenExchangeGrant(params);
         break;
       default:
         // Joi validation means this should never happen.
         throw Error('unreachable');
     }
-    requestedGrant.name = client.name;
-    requestedGrant.canGrant = client.canGrant;
-    requestedGrant.publicClient = client.publicClient;
+    // Token exchange gets client info from the subject_token, not from client auth
+    if (client) {
+      requestedGrant.name = client.name;
+      requestedGrant.canGrant = client.canGrant;
+      requestedGrant.publicClient = client.publicClient;
+    }
     requestedGrant.grantType = params.grant_type;
     requestedGrant.ppidSeed = params.ppid_seed;
     requestedGrant.resource = params.resource;
@@ -404,7 +408,7 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
    * This check happens on their side, and for now we will grant the request.
    * See FXA-12925
    */
-  async function validateTokenExchangeGrant(client, params) {
+  async function validateTokenExchangeGrant(params) {
     const subjectToken = await oauthDB.getRefreshToken(
       encrypt.hash(params.subject_token)
     );
@@ -459,20 +463,26 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
 
   async function tokenHandler(req) {
     var params = req.payload;
-    const client = await authenticateClient(req.headers, params);
 
-    // Refuse to generate new access tokens for disabled clients that are already
-    // connected to the account.  We allow disabled clients to claim existing authorization
-    // codes, because otherwise we risk erroring out halfway through an app login flow
-    // and presenting a very confusing user experience.  The /authorization endpoint refuses
-    // to create new codes for disabled clients.
-    if (
-      DISABLED_CLIENTS.has(hex(client.id)) &&
-      params.grant_type !== GRANT_AUTHORIZATION_CODE
-    ) {
-      throw OauthError.disabledClient(hex(client.id));
+    // Token exchange doesn't require client authentication since the
+    // subject_token is already bound to an allowed client.
+    let client = null;
+    if (params.grant_type !== GRANT_TOKEN_EXCHANGE) {
+      client = await authenticateClient(req.headers, params);
+      // Refuse to generate new access tokens for disabled clients that are already
+      // connected to the account. We allow disabled clients to claim existing authorization
+      // codes, because otherwise we risk erroring out halfway through an app login flow
+      // and presenting a very confusing user experience. The /authorization endpoint refuses
+      // to create new codes for disabled clients.
+      if (
+        DISABLED_CLIENTS.has(hex(client.id)) &&
+        params.grant_type !== GRANT_AUTHORIZATION_CODE
+      ) {
+        throw OauthError.disabledClient(hex(client.id));
+      }
     }
     const grant = await validateGrantParameters(client, params);
+
     const tokens = await generateTokens(grant);
 
     // For token exchange, revoke the original refresh token after successful generation
@@ -508,10 +518,9 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
     glean.oauth.tokenCreated(req, {
       uid,
       oauthClientId,
-      reason: req.payload?.grant_type || '',
+      reason: params.grant_type,
     });
 
-    // the client receiving keys at the end of the scoped keys flow
     if (tokens.keys_jwe) {
       statsd.increment('oauth.rp.keys-jwe', { clientId: oauthClientId });
     }
@@ -641,8 +650,6 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
                 .valid(SUBJECT_TOKEN_TYPE_REFRESH)
                 .required(),
               scope: validators.scope.required(),
-              ttl: Joi.number().positive().optional(),
-              resource: validators.resourceUrl.optional(),
             })
           ),
         },
@@ -690,6 +697,7 @@ module.exports = ({ log, oauthDB, db, mailer, devices, statsd, glean }) => {
               expires_in: Joi.number().required(),
             }),
             // token exchange
+            // Does not require client_id because the client is identified from the subject_token
             Joi.object({
               access_token: validators.accessToken.required(),
               refresh_token: validators.refreshToken.required(),

packages/fxa-auth-server/test/oauth/routes/token.js

@@ -473,7 +473,6 @@ describe('token exchange grant_type', function () {
       const request = {
         headers: {},
         payload: {
-          client_id: FIREFOX_IOS_CLIENT_ID,
           grant_type: GRANT_TOKEN_EXCHANGE,
           subject_token: REFRESH_TOKEN,
           subject_token_type: SUBJECT_TOKEN_TYPE_REFRESH,
@@ -511,7 +510,6 @@ describe('token exchange grant_type', function () {
       const request = {
         headers: {},
         payload: {
-          client_id: NON_FIREFOX_CLIENT_ID,
           grant_type: GRANT_TOKEN_EXCHANGE,
           subject_token: REFRESH_TOKEN,
           subject_token_type: SUBJECT_TOKEN_TYPE_REFRESH,
@@ -551,7 +549,6 @@ describe('token exchange grant_type', function () {
       const request = {
         headers: {},
         payload: {
-          client_id: FIREFOX_IOS_CLIENT_ID,
           grant_type: GRANT_TOKEN_EXCHANGE,
           subject_token: REFRESH_TOKEN,
           subject_token_type: SUBJECT_TOKEN_TYPE_REFRESH,
@@ -613,7 +610,6 @@ describe('token exchange grant_type', function () {
       const request = {
         headers: {},
         payload: {
-          client_id: FIREFOX_IOS_CLIENT_ID,
           grant_type: GRANT_TOKEN_EXCHANGE,
           subject_token: REFRESH_TOKEN,
           subject_token_type: SUBJECT_TOKEN_TYPE_REFRESH,
@@ -628,65 +624,8 @@ describe('token exchange grant_type', function () {
       assert.equal(result.refresh_token, 'new_refresh_token');
       assert.include(result.scope, SYNC_SCOPE);
       assert.include(result.scope, RELAY_SCOPE);
-      // Verify original token was revoked
-      assert.isNotNull(removedTokenId);
-    });
-
-    it('revokes original token after successful exchange', async () => {
-      let removedTokenId = null;
-      const originalTokenId = '1234567890abcdef';
-
-      const routes = proxyquire(tokenRoutePath, {
-        ...tokenRoutesDepMocks,
-        '../../oauth/grant': {
-          generateTokens: (grant) => ({
-            access_token: 'new_access_token',
-            token_type: 'bearer',
-            scope: grant.scope.toString(),
-            expires_in: 3600,
-            refresh_token: 'new_refresh_token',
-          }),
-          validateRequestedGrant: () => ({ offline: true, scope: 'testo' }),
-        },
-      })({
-        ...tokenRoutesArgMocks,
-        log: {
-          debug: () => {},
-          warn: () => {},
-          info: () => {},
-        },
-        oauthDB: {
-          ...tokenRoutesArgMocks.oauthDB,
-          async getRefreshToken() {
-            return {
-              userId: buf(UID),
-              clientId: buf(FIREFOX_IOS_CLIENT_ID),
-              tokenId: buf(originalTokenId),
-              scope: ScopeSet.fromString(SYNC_SCOPE),
-              profileChangedAt: Date.now(),
-            };
-          },
-          async removeRefreshToken({ tokenId }) {
-            removedTokenId = hex(tokenId);
-          },
-        },
-      });
-
-      const request = {
-        headers: {},
-        payload: {
-          client_id: FIREFOX_IOS_CLIENT_ID,
-          grant_type: GRANT_TOKEN_EXCHANGE,
-          subject_token: REFRESH_TOKEN,
-          subject_token_type: SUBJECT_TOKEN_TYPE_REFRESH,
-          scope: RELAY_SCOPE,
-        },
-        emitMetricsEvent: () => {},
-      };
-
-      await routes[0].config.handler(request);
-
-      assert.equal(removedTokenId, originalTokenId);
+      // Verify original token was revoked with correct ID
+      assert.equal(hex(removedTokenId), '1234567890abcdef');
     });
   });
 });
@@ -795,7 +734,6 @@ describe('/oauth/token POST', function () {
         auth: { credentials: null },
         headers: {},
         payload: {
-          client_id: FIREFOX_IOS_CLIENT_ID,
           grant_type: GRANT_TOKEN_EXCHANGE,
           subject_token: REFRESH_TOKEN,
           subject_token_type: SUBJECT_TOKEN_TYPE_REFRESH,