feat: authorization response parameter checking based on response_type

passing checks.response_type will now validate that parameters that are normatively REQUIRED are present.
panva · Aug 11, 2018 · 6e0ac57 · 6e0ac57
1 parent 29df0a2
commit 6e0ac57
Show file tree

Hide file tree

Showing 5 changed files with 260 additions and 69 deletions.
diff --git a/README.md b/README.md
@@ -156,19 +156,18 @@ client.authorizationCallback('https://client.example.com/callback', request.quer
   });
 ```
 
-### Processing callback with state, nonce or max_age check
+### Processing callback with required params, state, nonce or max_age checks (recommended)
 ```js
-const state = session.state;
-const nonce = session.nonce;
+const { state, nonce, max_age, response_type } = session[authorizationRequestState];
 
-client.authorizationCallback('https://client.example.com/callback', request.query, { state, nonce, max_age }) // => Promise
+client.authorizationCallback('https://client.example.com/callback', request.query, { state, nonce, max_age, response_type }) // => Promise
   .then(function (tokenSet) {
     console.log('received and validated tokens %j', tokenSet);
     console.log('validated id_token claims %j', tokenSet.claims);
   });
 ```
 
-### IdP Errors - OpenIdConnectError
+### OP Errors - OpenIdConnectError
 When the OpenID Provider returns an OIDC formatted error from either authorization callbacks or
 any of the JSON responses the library will reject a given Promise with `OpenIdConnectError` instance.
 

diff --git a/example/app.js b/example/app.js
@@ -1,4 +1,4 @@
-/* eslint-disable import/no-extraneous-dependencies */
+/* eslint-disable import/no-extraneous-dependencies, camelcase */
 
 const crypto = require('crypto');
 const url = require('url');
@@ -166,20 +166,23 @@ module.exports = (issuer) => {
   });
 
   router.get('/login', async (ctx, next) => {
-    ctx.session.state = crypto.randomBytes(16).toString('hex');
-    ctx.session.nonce = crypto.randomBytes(16).toString('hex');
+    const state = crypto.randomBytes(16).toString('hex');
+    const nonce = crypto.randomBytes(16).toString('hex');
+    ctx.session.auth_request = { state, nonce };
 
     const { client } = ctx.session;
 
-    const authorizationRequest = Object.assign({
+    const authorization_request = Object.assign({
       redirect_uri: url.resolve(ctx.href, 'cb'),
       scope: 'openid profile email address phone',
-      state: ctx.session.state,
-      nonce: ctx.session.nonce,
+      state,
+      nonce,
       response_type: client.response_types[0],
     }, ctx.session.authorization_params);
 
-    ctx.redirect(client.authorizationUrl(authorizationRequest));
+    ctx.session.auth_request.response_type = authorization_request.response_type;
+
+    ctx.redirect(client.authorizationUrl(authorization_request));
     return next();
   });
 
@@ -210,25 +213,23 @@ module.exports = (issuer) => {
       return ctx.render('repost', { layout: false });
     }
 
-    const { state, nonce } = ctx.session;
-    delete ctx.session.state;
-    delete ctx.session.nonce;
+    const { state, nonce, response_type } = ctx.session.auth_request;
+    delete ctx.session.auth_request;
 
-    ctx.session.tokenset = await client.authorizationCallback(url.resolve(ctx.href, 'cb'), params, { nonce, state });
+    ctx.session.tokenset = await client.authorizationCallback(url.resolve(ctx.href, 'cb'), params, { nonce, state, response_type });
 
     ctx.redirect('/user');
 
     return next();
   });
 
   router.post('/cb', body({ patchNode: true }), async (ctx, next) => {
-    const { state, nonce } = ctx.session;
-    delete ctx.session.state;
-    delete ctx.session.nonce;
+    const { state, nonce, response_type } = ctx.session.auth_request;
+    delete ctx.session.auth_request;
     const { client } = ctx.session;
     const params = client.callbackParams(ctx.request.req);
 
-    ctx.session.tokenset = await client.authorizationCallback(url.resolve(ctx.href, 'cb'), params, { nonce, state });
+    ctx.session.tokenset = await client.authorizationCallback(url.resolve(ctx.href, 'cb'), params, { nonce, state, response_type });
 
     ctx.redirect('/user');
 

diff --git a/lib/client.js b/lib/client.js
@@ -320,14 +320,44 @@ class Client {
 
     if (this.default_max_age && !checks.max_age) checks.max_age = this.default_max_age;
 
-    if (checks.state !== parameters.state) {
+    if (!params.state && checks.state) {
+      return Promise.reject(new Error('state missing from response'));
+    }
+
+    if (params.state && !checks.state) {
+      return Promise.reject(new Error('checks.state missing'));
+    }
+
+    if (checks.state !== params.state) {
       return Promise.reject(new Error('state mismatch'));
     }
 
     if (params.error) {
       return Promise.reject(new OpenIdConnectError(params));
     }
 
+    const RESPONSE_TYPE_REQUIRED_PARAMS = {
+      code: ['code'],
+      id_token: ['id_token'],
+      token: ['access_token', 'token_type'],
+    };
+
+    if (checks.response_type) {
+      for (const type of checks.response_type.split(' ')) { // eslint-disable-line no-restricted-syntax
+        if (type === 'none') {
+          if (params.code || params.id_token || params.access_token) {
+            return Promise.reject(new Error('unexpected params encountered for "none" response'));
+          }
+        } else {
+          for (const param of RESPONSE_TYPE_REQUIRED_PARAMS[type]) { // eslint-disable-line no-restricted-syntax, max-len
+            if (!params[param]) {
+              return Promise.reject(new Error(`${param} missing from response`));
+            }
+          }
+        }
+      }
+    }
+
     let promise;
 
     if (params.id_token) {
@@ -368,14 +398,45 @@ class Client {
   oauthCallback(redirectUri, parameters, checks = {}) {
     const params = _.pick(parameters, CALLBACK_PROPERTIES);
 
-    if (checks.state !== parameters.state) {
+    if (!params.state && checks.state) {
+      return Promise.reject(new Error('state missing from response'));
+    }
+
+    if (params.state && !checks.state) {
+      return Promise.reject(new Error('checks.state missing'));
+    }
+
+    if (checks.state !== params.state) {
       return Promise.reject(new Error('state mismatch'));
     }
 
     if (params.error) {
       return Promise.reject(new OpenIdConnectError(params));
     }
 
+    const RESPONSE_TYPE_REQUIRED_PARAMS = {
+      code: ['code'],
+      token: ['access_token', 'token_type'],
+    };
+
+    if (checks.response_type) {
+      for (const type of checks.response_type.split(' ')) { // eslint-disable-line no-restricted-syntax
+        if (type === 'none') {
+          if (params.code || params.id_token || params.access_token) {
+            return Promise.reject(new Error('unexpected params encountered for "none" response'));
+          }
+        }
+
+        if (RESPONSE_TYPE_REQUIRED_PARAMS[type]) {
+          for (const param of RESPONSE_TYPE_REQUIRED_PARAMS[type]) { // eslint-disable-line no-restricted-syntax, max-len
+            if (!params[param]) {
+              return Promise.reject(new Error(`${param} missing from response`));
+            }
+          }
+        }
+      }
+    }
+
     if (params.code) {
       return this.grant({
         grant_type: 'authorization_code',

diff --git a/lib/helpers/consts.js b/lib/helpers/consts.js
@@ -30,6 +30,7 @@ const CALLBACK_PROPERTIES = [
   'code',
   'error',
   'error_description',
+  'error_uri',
   'expires_in',
   'id_token',
   'state',