feat(oauth): Support multiple client redirect urls

packages/db-migrations/databases/fxa_oauth/patches/patch-031-32.sql

@@ -0,0 +1,5 @@
+-- To support multiple redirect uris, we are changing the column form varchar 256 to 2048
+-- should enough to store 20 redirect uris. 
+ALTER TABLE `clients` CHANGE COLUMN `redirectUri` `redirectUri` VARCHAR(2048) NULL DEFAULT NULL;
+
+UPDATE dbMetadata SET value = '32' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa_oauth/patches/patch-032-031.sql

@@ -0,0 +1,3 @@
+-- ALTER TABLE `clients` CHANGE COLUMN `redirectUri` `redirectUri` VARCHAR(256) NULL DEFAULT NULL;
+
+-- UPDATE dbMetadata SET value = '31' WHERE name = 'schema-patch-level';

packages/db-migrations/databases/fxa_oauth/target-patch.json

@@ -1,3 +1,3 @@
 {
-  "level": 31
+  "level": 32
 }

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

@@ -126,14 +126,26 @@ module.exports = ({ log, oauthDB, config }) => {
   }
 
   function validateClientDetails(client, payload) {
-    // Clients must use a single specific redirect_uri,
-    // but they're allowed to not provide one and have us fill it in automatically.
-    payload.redirect_uri = payload.redirect_uri || client.redirectUri;
-    if (payload.redirect_uri !== client.redirectUri) {
-      log.debug('redirect.mismatch', {
-        param: payload.redirect_uri,
-        registered: client.redirectUri,
-      });
+    if (!payload.redirect_uri && !client.redirectUri) {
+      throw OauthError.incorrectRedirect();
+    }
+
+    // Starting in train-248, FxA added the ability for an OAuth client to support
+    // multiple redirect uris (comma separated list). The authorization flow redirect uri
+    // must match one of these exactly. Pattern matching is not supported.
+    const redirectUris = client.redirectUri.split(',');
+
+    // Authorization flow must use a single specific redirect_uri,
+    // but allowed to not provide one and have us fill it in automatically.
+    payload.redirect_uri = payload.redirect_uri || redirectUris[0];
+
+    const validUri = redirectUris.some((uri) => {
+      if (uri === payload.redirect_uri) {
+        return true;
+      }
+    });
+
+    if (!validUri) {
       if (
         config.oauthServer.localRedirects &&
         isLocalHost(payload.redirect_uri)
@@ -142,6 +154,11 @@ module.exports = ({ log, oauthDB, config }) => {
       } else {
         throw OauthError.incorrectRedirect(payload.redirect_uri);
       }
+    } else {
+      log.debug('redirect.mismatch', {
+        param: payload.redirect_uri,
+        registered: client.redirectUri,
+      });
     }
   }
 

packages/fxa-content-server/app/scripts/lib/validate.js

@@ -234,6 +234,21 @@ var Validate = {
     return JWT_STRING.test(value);
   },
 
+  /**
+   * Check whether `redirectUri` string contains only valid uris.
+   * Clients can specify a comma separated list and this checks to see
+   * if each is a valid uri.
+   *
+   * @param {String} redirectUrisStr
+   * @returns {Boolean}
+   */
+  isRedirectUriValid(redirectUrisStr) {
+    const redirectUris = redirectUrisStr.split(',');
+    return redirectUris.every((value) => {
+      return urlRegEx.test(value);
+    });
+  },
+
   /**
    * Check whether `newsletters` contains only valid newsletter slugs.
    *

packages/fxa-content-server/app/scripts/lib/vat.js

@@ -44,6 +44,10 @@ Vat.register(
   'verificationRedirect',
   Vat.string().test(Validate.isVerificationRedirectValid)
 );
+Vat.register(
+  'redirectUri',
+  Vat.string().required().test(Validate.isRedirectUriValid)
+);
 
 // depends on hex, must come afterwards
 Vat.register('clientId', Vat.hex());

packages/fxa-content-server/app/scripts/models/reliers/oauth.js

@@ -21,7 +21,9 @@ const CLIENT_INFO_SCHEMA = {
   id: Vat.hex().required().renameTo('clientId'),
   image_uri: Vat.url().allow('').renameTo('imageUri'),
   name: Vat.string().required().min(1).renameTo('serviceName'),
-  redirect_uri: Vat.url().required().renameTo('redirectUri'),
+
+  // This can be a single uri or comma separated list
+  redirect_uri: Vat.redirectUri().renameTo('redirectUri'),
   trusted: Vat.boolean().required(),
 };
 
@@ -246,8 +248,8 @@ var OAuthRelier = Relier.extend({
          *
          * Verification (email) flows do not have a redirect uri, nothing to validate
          */
-        if (!isCorrectRedirect(this.get('redirectUri'), result.redirectUri)) {
-          // if provided redirect uri doesn't match with client info then throw
+        if (!isCorrectRedirect(this.get('redirectUri'), result)) {
+          // if provided redirect uri doesn't match with any client redirectUri then throw
           throw OAuthErrors.toError('INCORRECT_REDIRECT');
         }
 
@@ -270,12 +272,28 @@ var OAuthRelier = Relier.extend({
       }
     );
 
-    function isCorrectRedirect(queryRedirectUri, resultRedirectUri) {
+    function isCorrectRedirect(queryRedirectUri, client) {
+      // If RP doesn't specify redirectUri, we default to the first redirectUri
+      // for the client
+      const redirectUris = client.redirectUri.split(',');
       if (!queryRedirectUri) {
+        client.redirectUri = redirectUris[0];
         return true;
-      } else if (queryRedirectUri === resultRedirectUri) {
+      }
+
+      const hasRedirectUri = redirectUris.some((uri) => {
+        if (queryRedirectUri === uri) {
+          return true;
+        }
+      });
+      if (hasRedirectUri) {
+        client.redirectUri = queryRedirectUri;
         return true;
-      } else if (
+      }
+
+      // Pairing has a special redirectUri that deep links into the specific
+      // mobile app
+      if (
         queryRedirectUri === Constants.DEVICE_PAIRING_AUTHORITY_REDIRECT_URI
       ) {
         return true;

packages/fxa-content-server/app/tests/spec/lib/validate.js

@@ -297,4 +297,26 @@ describe('lib/validate', function () {
       assert.isTrue(Validate.isUtmValid('marketing-snippet'));
     });
   });
+
+  describe('isRedirectUriValid', () => {
+    it('returns false if any redirect uri is not valid', () => {
+      const testString = 'https://localhost,http://';
+      assert.isFalse(Validate.isRedirectUriValid(testString));
+    });
+
+    it('returns false for invalid uri', () => {
+      const testString = 'c';
+      assert.isFalse(Validate.isRedirectUriValid(testString));
+    });
+
+    it('returns true for valid uris', () => {
+      const testString = 'https://localhost,http://mozilla.org';
+      assert.isTrue(Validate.isRedirectUriValid(testString));
+    });
+
+    it('returns true for single valid uri', () => {
+      const testString = 'https://localhost';
+      assert.isTrue(Validate.isRedirectUriValid(testString));
+    });
+  });
 });

packages/fxa-content-server/app/tests/spec/models/reliers/oauth.js

@@ -44,6 +44,8 @@ describe('models/reliers/oauth', () => {
   var SCOPE_WITH_EXTRAS = 'profile:email profile:uid profile:non_whitelisted';
   var SCOPE_WITH_OPENID = 'profile:email profile:uid openid';
   var SERVER_REDIRECT_URI = 'http://localhost:8080/api/oauth';
+  var SERVER_REDIRECT_URIS =
+    'http://localhost:8080/api/oauth,https://www.mozilla.org';
   var SERVICE = 'service';
   var SERVICE_NAME = '123Done';
   var STATE = 'fakestatetoken';
@@ -181,6 +183,22 @@ describe('models/reliers/oauth', () => {
           assert.equal(err.param, 'code_challenge_method');
         });
       });
+
+      it('throws if incorrect `redirectUri` is specified', () => {
+        windowMock.location.search = toSearchString({
+          access_type: ACCESS_TYPE,
+          action: ACTION,
+          client_id: CLIENT_ID,
+          prompt: PROMPT,
+          redirect_uri: 'http://www.notvalidclienturl.com',
+          scope: SCOPE,
+          state: STATE,
+        });
+
+        return relier.fetch().then(assert.fail, (err) => {
+          assert.isTrue(OAuthErrors.is(err, 'INCORRECT_REDIRECT'));
+        });
+      });
     });
 
     describe('verification flow', () => {
@@ -403,7 +421,12 @@ describe('models/reliers/oauth', () => {
         const invalidValues = ['', ' ', 'invalid'];
         testInvalidQueryParams('prompt', invalidValues);
 
-        const validValues = [undefined, OAuthPrompt.CONSENT, OAuthPrompt.NONE];
+        const validValues = [
+          undefined,
+          OAuthPrompt.CONSENT,
+          OAuthPrompt.NONE,
+          OAuthPrompt.LOGIN,
+        ];
         testValidQueryParams('prompt', validValues, 'prompt', validValues);
       });
 
@@ -575,7 +598,7 @@ describe('models/reliers/oauth', () => {
           testMissingClientInfoValue('redirect_uri');
         });
 
-        var invalidClientInfoValues = ['', ' '];
+        var invalidClientInfoValues = ['', ' ', ',', 'http://moz.org,'];
         testInvalidClientInfoValues('redirect_uri', invalidClientInfoValues);
       });
 
@@ -1242,7 +1265,7 @@ describe('models/reliers/oauth', () => {
       var clientInfo = {
         id: CLIENT_ID,
         name: SERVICE_NAME,
-        redirect_uri: SERVER_REDIRECT_URI,
+        redirect_uri: SERVER_REDIRECT_URIS,
         trusted: isTrusted,
       };
 
@@ -1260,7 +1283,6 @@ describe('models/reliers/oauth', () => {
 
   function fetchExpectError(params) {
     windowMock.location.search = toSearchString(params);
-
     return relier.fetch().then(assert.fail, function (_err) {
       err = _err;
     });