feat(pkce): add PKCE support to the oauth server (#466) r=seanmonstar

Fixes mozilla/fxa-oauth-server#465

config/dev.json

@@ -14,6 +14,16 @@
       "trusted": true,
       "canGrant": false
     },
+    {
+      "id": "38a6b9b3a65a1871",
+      "hashedSecret": "289a885946ee316844d9ffd0d725ee714901548a1e6507f1a40fb3c2ae0c99f1",
+      "name": "123Done PKCE",
+      "imageUri": "https://mozorg.cdn.mozilla.net/media/img/firefox/new/header-firefox.png",
+      "redirectUri": "http://127.0.0.1:8080/?oauth_pkce_redirect=1",
+      "trusted": true,
+      "canGrant": false,
+      "publicClient": true
+    },
     {
       "id": "325b4083e32fe8e7",
       "hashedSecret": "ded3c396f28123f3fe6b152784e8eab7357c6806cb5175805602a2cd67f85080",

config/test.json

@@ -45,6 +45,16 @@
       "redirectUri": "https://example.domain/return?foo=bar",
       "trusted": false,
       "canGrant": false
+    },
+    {
+      "id": "38a6b9b3a65a1871",
+      "hashedSecret": "289a885946ee316844d9ffd0d725ee714901548a1e6507f1a40fb3c2ae0c99f1",
+      "name": "Public Client PKCE",
+      "imageUri": "https://mozorg.cdn.mozilla.net/media/img/firefox/new/header-firefox.png",
+      "redirectUri": "https://example.domain/return?foo=bar",
+      "trusted": true,
+      "canGrant": false,
+      "publicClient": true
     }
   ],
   "logging": {

docs/api.md

@@ -295,10 +295,6 @@ content-server page.
     signed in email address will be used as the default.
   - If `action` is `force_auth`, the user is unable to modify the email
     address and is unable to sign up if the address is not registered.
-- `keys`: Optional. Boolean setting, set this if the relier wants to receive encryption keys.
-- `verification_redirect`: Optional. This option adds a "Proceed" button into the "Account Ready" view. See options for details.
-  - Default. If `verification_redirect` is `no` the account ready view will not show a "Proceed" button that will return to the relier.
-  - If `verification_redirect` is `always` then a "Proceed" button is only displayed if the user verifies in a 2nd browser. If the user verifies in the same browser, they are automatically redirected w/o user interaction.
 
 **Example:**
 
@@ -323,6 +319,9 @@ back to the client. This code will be traded for a token at the
 - `redirect_uri`: Optional. If supplied, a string URL of where to redirect afterwards. Must match URL from registration.
 - `scope`: Optional. A string-separated list of scopes that the user has authorized. This could be pruned by the user at the confirmation dialog.
 - `access_type`: Optional. A value of `offline` will generate a refresh token along with the access token.
+- `code_challenge_method`: Required if using [PKCE](pkce.md). Must be `S256`, no other value is accepted.
+- `code_challenge`: Required if using [PKCE](pkce.md). A minimum length of 43 characters and a maximum length of 128 characters string, encoded as `BASE64URL`.
+
 
 **Example:**
 
@@ -388,6 +387,9 @@ particular user.
   - If `urn:ietf:params:oauth:grant-type:jwt-bearer`:
     - `assertion`: A signed JWT assertion. See [Service
       Clients][] for more.
+  - if client is type `publicClient:true` and `authorization_code`:
+    - `code_verifier`: Required if using [PKCE](pkce.md).
+
 
 
 **Example:**

docs/pkce.md

@@ -0,0 +1,13 @@
+# Firefox Accounts OAuth - PKCE Support
+
+> Proof Key for Code Exchange by OAuth Public Clients
+
+Firefox Accounts OAuth flow supports the [PKCE RFC7636](https://tools.ietf.org/html/rfc7636).
+This feature helps us authenticate clients such as WebExtensions and Native apps.
+Clients that do not have a server component or a secure way to store a `client_secret`.
+
+To better understand this protocol please read the [Proof Key for Code Exchange (RFC 7636) by Authlete Inc.](https://www.authlete.com/documents/article/pkce/index).
+
+Please see the [API](API.md) documentation that explains the support parameters - `code_challenge_method`, `code_challenge` and `code_verifier`.
+
+At this time Firefox Accounts requires you to use the `S256` flow, we do not support the `plain` code challenge method.

lib/db/index.js

@@ -79,6 +79,7 @@ function preClients() {
       // ensure booleans are boolean and not undefined
       c.trusted = !!c.trusted;
       c.canGrant = !!c.canGrant;
+      c.publicClient = !!c.publicClient;
 
       // Modification of the database at startup in production and stage is
       // not preferred. This option will be set to false on those stacks.

lib/db/memory.js

@@ -37,7 +37,9 @@ const MAX_TTL = config.get('expiration.accessToken');
  *       scope: <string>,
  *       authAt: <timestamp>,
  *       createdAt: <timestamp>,
- *       offline: <boolean>
+ *       offline: <boolean>,
+ *       codeChallengeMethod: <string>,
+ *       codeChallenge: <string>,
  *     }
  *   },
  *   developers: {

lib/db/mysql/index.js

@@ -126,8 +126,8 @@ MysqlStore.connect = function mysqlConnect(options) {
 const QUERY_CLIENT_REGISTER =
   'INSERT INTO clients ' +
   '(id, name, imageUri, hashedSecret, hashedSecretPrevious, redirectUri,' +
-  'trusted, canGrant) ' +
-  'VALUES (?, ?, ?, ?, ?, ?, ?, ?);';
+  'trusted, canGrant, publicClient) ' +
+  'VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?);';
 const QUERY_CLIENT_DEVELOPER_INSERT =
   'INSERT INTO clientDevelopers ' +
   '(rowId, developerId, clientId) ' +
@@ -148,7 +148,7 @@ const QUERY_DEVELOPER_INSERT =
   'VALUES (?, ?);';
 const QUERY_CLIENT_GET = 'SELECT * FROM clients WHERE id=?';
 const QUERY_CLIENT_LIST = 'SELECT id, name, redirectUri, imageUri, ' +
-  'canGrant, trusted ' +
+  'canGrant, publicClient, trusted ' +
   'FROM clients, clientDevelopers, developers ' +
   'WHERE clients.id = clientDevelopers.clientId AND ' +
   'developers.developerId = clientDevelopers.developerId AND ' +
@@ -162,8 +162,8 @@ const QUERY_CLIENT_UPDATE = 'UPDATE clients SET ' +
   'WHERE id=?';
 const QUERY_CLIENT_DELETE = 'DELETE FROM clients WHERE id=?';
 const QUERY_CODE_INSERT =
-  'INSERT INTO codes (clientId, userId, email, scope, authAt, offline, code) ' +
-  'VALUES (?, ?, ?, ?, ?, ?, ?)';
+  'INSERT INTO codes (clientId, userId, email, scope, authAt, offline, code, codeChallengeMethod, codeChallenge) ' +
+  'VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)';
 const QUERY_ACCESS_TOKEN_INSERT =
   'INSERT INTO tokens (clientId, userId, email, scope, type, expiresAt, ' +
   'token) VALUES (?, ?, ?, ?, ?, ?, ?)';
@@ -240,7 +240,8 @@ MysqlStore.prototype = {
       client.hashedSecretPrevious ? buf(client.hashedSecretPrevious) : null,
       client.redirectUri,
       !!client.trusted,
-      !!client.canGrant
+      !!client.canGrant,
+      !!client.publicClient
     ]).then(function() {
       logger.debug('registerClient.success', { id: hex(id) });
       client.id = id;
@@ -364,7 +365,9 @@ MysqlStore.prototype = {
       codeObj.scope.join(' '),
       codeObj.authAt,
       !!codeObj.offline,
-      hash
+      hash,
+      codeObj.codeChallengeMethod,
+      codeObj.codeChallenge
     ]).then(function() {
       return code;
     });

lib/db/mysql/patch.js

@@ -6,4 +6,4 @@
 // Update this if you add a new patch, and don't forget to update
 // the documentation for the current schema in ../schema.sql.
 
-module.exports.level = 17;
+module.exports.level = 18;

lib/db/mysql/patches/patch-017-018.sql

@@ -0,0 +1,11 @@
+--  Add `publicClient` column to the `clients` table.
+ALTER TABLE clients ADD COLUMN publicClient BOOLEAN DEFAULT FALSE NOT NULL AFTER canGrant;
+UPDATE clients SET publicClient=false;
+
+--  Add `codeChallengeMethod` and `codeChallenge` column to the `codes` table.
+ALTER TABLE codes
+ADD COLUMN codeChallengeMethod VARCHAR(256) AFTER offline,
+ADD COLUMN codeChallenge VARCHAR(256) AFTER codeChallengeMethod,
+ALGORITHM = INPLACE, LOCK = NONE;
+
+UPDATE dbMetadata SET value = '18' WHERE name = 'schema-patch-level';

lib/db/mysql/patches/patch-018-017.sql

@@ -0,0 +1,14 @@
+--  Drop `publicClient` column from the `clients` table.
+
+-- ALTER TABLE clients DROP COLUMN publicClient,
+-- ALGORITHM = INPLACE, LOCK = NONE;
+
+--  Drop `codeChallengeMethod` and `codeChallenge` column from the `codes` table.
+
+-- ALTER TABLE codes DROP COLUMN codeChallengeMethod,
+-- ALGORITHM = INPLACE, LOCK = NONE;
+
+-- ALTER TABLE codes DROP COLUMN codeChallenge,
+-- ALGORITHM = INPLACE, LOCK = NONE;
+
+-- UPDATE dbMetadata SET value = '17' WHERE name = 'schema-patch-level';

lib/error.js

@@ -232,4 +232,36 @@ AppError.expiredToken = function expiredToken(expiredAt) {
   });
 };
 
+AppError.notPublicClient = function unknownClient(clientId) {
+  return new AppError({
+    code: 400,
+    error: 'Bad Request',
+    errno: 116,
+    message: 'Not a public client'
+  }, {
+    clientId: clientId
+  });
+};
+
+
+AppError.mismatchCodeChallenge = function mismatchCodeChallenge(pkceHashValue) {
+  return new AppError({
+    code: 400,
+    error: 'Bad Request',
+    errno: 117,
+    message: 'Incorrect code_challenge'
+  }, {
+    requestCodeChallenge: pkceHashValue
+  });
+};
+
+AppError.missingPkceParameters = function missingPkceParameters() {
+  return new AppError({
+    code: 400,
+    error: 'PKCE parameters missing',
+    errno: 118,
+    message: 'Public clients require PKCE OAuth parameters'
+  });
+};
+
 module.exports = AppError;

lib/routes/authorization.js

@@ -22,6 +22,9 @@ const TOKEN = 'token';
 const ACCESS_TYPE_ONLINE = 'online';
 const ACCESS_TYPE_OFFLINE = 'offline';
 
+const PKCE_SHA256_CHALLENGE_METHOD = 'S256'; // This server only supports S256 PKCE, no 'plain'
+const PKCE_CODE_CHALLENGE_LENGTH = 43;
+
 const MAX_TTL_S = config.get('expiration.accessToken') / 1000;
 
 const UNTRUSTED_CLIENT_ALLOWED_SCOPES = [
@@ -65,7 +68,9 @@ function generateCode(claims, client, scope, req) {
     email: claims['fxa-verifiedEmail'],
     scope: scope,
     authAt: claims['fxa-lastAuthAt'],
-    offline: req.payload.access_type === ACCESS_TYPE_OFFLINE
+    offline: req.payload.access_type === ACCESS_TYPE_OFFLINE,
+    codeChallengeMethod: req.payload.code_challenge_method,
+    codeChallenge: req.payload.code_challenge,
   }).then(function(code) {
     logger.debug('redirecting', { uri: req.payload.redirect_uri });
 
@@ -142,6 +147,20 @@ module.exports = {
         .valid(ACCESS_TYPE_OFFLINE, ACCESS_TYPE_ONLINE)
         .default(ACCESS_TYPE_ONLINE)
         .optional(),
+      code_challenge_method: Joi.string()
+        .valid(PKCE_SHA256_CHALLENGE_METHOD)
+        .when('response_type', {
+          is: CODE,
+          then: Joi.optional(),
+          otherwise: Joi.forbidden()
+        }),
+      code_challenge: Joi.string()
+        .length(PKCE_CODE_CHALLENGE_LENGTH)
+        .when('response_type', {
+          is: CODE,
+          then: Joi.optional(),
+          otherwise: Joi.forbidden()
+        })
     }
   },
   response: {
@@ -162,6 +181,7 @@ module.exports = {
     ])
   },
   handler: function authorizationEndpoint(req, reply) {
+    /*eslint complexity: [2, 13] */
     logger.debug('response_type', req.payload.response_type);
     var start = Date.now();
     var wantsGrant = req.payload.response_type === TOKEN;
@@ -194,6 +214,19 @@ module.exports = {
           }
         }
 
+        // PKCE client enforcement
+        if (client.publicClient &&
+           (! req.payload.code_challenge_method || ! req.payload.code_challenge)) {
+          // only Public Clients support code_challenge
+          logger.info('client.missingPkceParameters');
+          throw AppError.missingPkceParameters();
+        } else if (! client.publicClient &&
+            (req.payload.code_challenge_method || req.payload.code_challenge)) {
+          // non-Public Clients do not allow code challenge
+          logger.info('client.notPublicClient');
+          throw AppError.notPublicClient({ id: req.payload.client_id });
+        }
+
         var uri = req.payload.redirect_uri || client.redirectUri;
 
         if (uri !== client.redirectUri) {