feat: add per-request http options helper function configuration

This allows every request's [got](https://github.com/sindresorhus/got/tree/v9.6.0) options to be changed on a per-request basis. BREAKING CHANGE: logoutPendingSource no longer receives a `timeout` argument BREAKING CHANGE: `provider.defaultHttpOptions` setter was removed, use the new `httpOptions` configuration helper function instead BREAKING CHANGE: provider now asserts that client's `backchannel_logout_uri` returns a 200 OK response as per specification.
panva · May 17, 2019 · 4aee414 · 4aee414
1 parent 91f341c
commit 4aee414
Show file tree

Hide file tree

Showing 21 changed files with 176 additions and 257 deletions.
diff --git a/docs/README.md b/docs/README.md
@@ -26,8 +26,6 @@ If you or your business use oidc-provider, please consider becoming a [Patron][s
 - [Accounts](#accounts)
 - [User flows](#user-flows)
 - [Custom Grant Types](#custom-grant-types)
-- [HTTP Request Library / Proxy settings](#http-request-library--proxy-settings)
-- [Changing HTTP Request Defaults](#changing-http-request-defaults)
 - [Registering module middlewares (helmet, ip-filters, rate-limiters, etc)](#registering-module-middlewares-helmet-ip-filters-rate-limiters-etc)
 - [Pre- and post-middlewares](#pre--and-post-middlewares)
 - [Mounting oidc-provider](#mounting-oidc-provider)
@@ -334,44 +332,6 @@ provider.registerGrantType(grantType, tokenExchangeHandler, parameters, allowedD
 ```
 
 
-## HTTP Request Library / Proxy settings
-oidc-provider uses the [got][got-library] module. Because of its lightweight nature the provider
-will not use environment-defined http(s) proxies. In order to have them used you'll need to follow
-got's [README](https://github.com/sindresorhus/got#proxies) and use e.g.
-[`global-tunnel`](https://github.com/np-maintain/global-tunnel)
-
-
-## Changing HTTP Request Defaults
-On four occasions the OIDC Provider needs to venture out to the world wide webs to fetch or post
-to external resources, those are
-
-- fetching an authorization request by request_uri reference
-- fetching and refreshing client's referenced asymmetric keys (jwks_uri client metadata)
-- validating pairwise client's relation to a sector (sector_identifier_uri client metadata)
-- posting to client's backchannel_logout_uri
-
-oidc-provider uses these default options for http requests
-```js
-const DEFAULT_HTTP_OPTIONS = {
-  followRedirect: false,
-  headers: { 'User-Agent': `${pkg.name}/${pkg.version} (${this.issuer})` },
-  retry: 0,
-  timeout: 2500,
-};
-```
-
-Setting `defaultHttpOptions` on `Provider` instance merges your passed options with these defaults,
-for example you can add your own headers, change the user-agent used or change the timeout setting
-```js
-provider.defaultHttpOptions = { timeout: 2500, headers: { 'X-Your-Header': '<whatever>' } };
-```
-
-Confirm your httpOptions by
-```js
-console.log('httpOptions %j', provider.defaultHttpOptions);
-```
-
-
 ## Registering module middlewares (helmet, ip-filters, rate-limiters, etc)
 When using `provider.app` or `provider.callback` as a mounted application in your own koa or express
 stack just follow the respective module's documentation. However, when using the `provider.app` Koa
@@ -1076,7 +1036,7 @@ HTML source rendered when there are pending front-channel logout iframes to be c
 
 _**default value**_:
 ```js
-async logoutPendingSource(ctx, frames, postLogoutRedirectUri, timeout) {
+async logoutPendingSource(ctx, frames, postLogoutRedirectUri) {
   ctx.body = `<!DOCTYPE html>
 ead>
 <title>Logout</title>
@@ -1098,7 +1058,7 @@ ${frames.join('')}
   Array.prototype.slice.call(document.querySelectorAll('iframe')).forEach(function (element) {
     element.onload = frameOnLoad;
   });
-  setTimeout(redirect, ${timeout});
+  setTimeout(redirect, 2500);
 </script>
 <noscript>
   Your browser does not support JavaScript or you've disabled it.<br/>
@@ -1986,6 +1946,40 @@ async extraJwtAccessTokenClaims(ctx, token) {
 ```
 </details>
 
+### httpOptions
+
+Helper called whenever the provider calls an external HTTP(S) resource. Use to change the [got](https://github.com/sindresorhus/got/tree/v9.6.0) library's request options as they happen. This can be used to e.g. Change the request timeout option or to configure the global agent to use HTTP_PROXY and HTTPS_PROXY environment variables.   
+
+
+
+_**default value**_:
+```js
+httpOptions(options) {
+  options.followRedirect = false;
+  options.headers['User-Agent'] = 'oidc-provider/${VERSION} (${ISSUER_IDENTIFIER})';
+  options.retry = 0;
+  options.throwHttpErrors = false;
+  options.timeout = 2500;
+  return options;
+}
+```
+<details>
+  <summary>(Click to expand) To change the request's timeout</summary>
+  <br>
+
+
+To change all request's timeout configure the httpOptions as a function like so: ``js
+ {
+   httpOptions(options) {
+     options.timeout = 5000;
+     return options;
+   }
+ }
+
+
+```
+</details>
+
 ### interactionUrl
 
 Helper used by the OP to determine where to redirect User-Agent for necessary interaction, can return both absolute and relative urls  

diff --git a/docs/update-configuration.js b/docs/update-configuration.js
@@ -79,7 +79,7 @@ const props = [
       line = Buffer.from(strLine);
 
 
-      if (strLine.startsWith('/*')) {
+      if (strLine.startsWith('/*') && !strLine.includes('eslint') && !strLine.includes('istanbul')) {
         inBlock = true;
         nextIsOption = true;
         return;
@@ -252,6 +252,12 @@ const props = [
             if (line.includes('mustChange')) return undefined;
             if (line.startsWith(' ')) line = line.slice(fixIndent);
             line = line.replace(/ \/\/ eslint-disable.+/, '');
+            if (line.includes('/* eslint-disable')) {
+              return undefined;
+            }
+            if (line.includes('/* eslint-enable')) {
+              return undefined;
+            }
             line = line.replace(/ \/\/ TODO.+/, '');
             line = line.replace(/ class="[ \-\w]+ ?"/, '');
             if (line.includes('<meta ')) {

diff --git a/example/express.js b/example/express.js
@@ -13,7 +13,7 @@ const Account = require('./support/account');
 const configuration = require('./support/configuration');
 const routes = require('./routes/express');
 
-const { PORT = 3000, ISSUER = `http://localhost:${PORT}`, TIMEOUT } = process.env;
+const { PORT = 3000, ISSUER = `http://localhost:${PORT}` } = process.env;
 configuration.findAccount = Account.findAccount;
 
 const app = express();
@@ -32,10 +32,6 @@ let server;
 
   const provider = new Provider(ISSUER, { adapter, ...configuration });
 
-  if (TIMEOUT) {
-    provider.defaultHttpOptions = { timeout: parseInt(TIMEOUT, 10) };
-  }
-
   provider.use(helmet());
 
   if (process.env.NODE_ENV === 'production') {

diff --git a/example/koa.js b/example/koa.js
@@ -14,7 +14,7 @@ const Account = require('./support/account');
 const configuration = require('./support/configuration');
 const routes = require('./routes/koa');
 
-const { PORT = 3000, ISSUER = `http://localhost:${PORT}`, TIMEOUT } = process.env;
+const { PORT = 3000, ISSUER = `http://localhost:${PORT}` } = process.env;
 configuration.findAccount = Account.findAccount;
 
 const app = new Koa();
@@ -57,10 +57,6 @@ let server;
 
   const provider = new Provider(ISSUER, { adapter, ...configuration });
 
-  if (TIMEOUT) {
-    provider.defaultHttpOptions = { timeout: parseInt(TIMEOUT, 10) };
-  }
-
   provider.use(helmet());
 
   app.use(routes(provider).routes());

diff --git a/example/standalone.js b/example/standalone.js
@@ -12,7 +12,7 @@ const Account = require('./support/account');
 const configuration = require('./support/configuration');
 const routes = require('./routes/koa');
 
-const { PORT = 3000, ISSUER = `http://localhost:${PORT}`, TIMEOUT } = process.env;
+const { PORT = 3000, ISSUER = `http://localhost:${PORT}` } = process.env;
 configuration.findAccount = Account.findAccount;
 
 let server;
@@ -26,10 +26,6 @@ let server;
 
   const provider = new Provider(ISSUER, { adapter, ...configuration });
 
-  if (TIMEOUT) {
-    provider.defaultHttpOptions = { timeout: parseInt(TIMEOUT, 10) };
-  }
-
   provider.use(helmet());
 
   if (process.env.NODE_ENV === 'production') {

diff --git a/example/support/configuration.js b/example/support/configuration.js
@@ -9,6 +9,13 @@ try {
   HEROKU_EXAMPLE_CONFIGURATION = {};
 }
 
+if (process.env.TIMEOUT) {
+  HEROKU_EXAMPLE_CONFIGURATION.httpOptions = (gotOptions) => {
+    gotOptions.timeout = parseInt(process.env.TIMEOUT, 10); // eslint-disable-line no-param-reassign
+    return gotOptions;
+  };
+}
+
 module.exports = Object.assign({
   acrValues: ['urn:mace:incommon:iap:bronze'],
   clients: [

diff --git a/lib/actions/end_session.js b/lib/actions/end_session.js
@@ -222,9 +222,7 @@ module.exports = {
 
       if (front.length) {
         const frames = front.map(frameFor);
-        await frontchannelLogout.logoutPendingSource(
-          ctx, frames, uri, ctx.oidc.provider.httpOptions().timeout,
-        );
+        await frontchannelLogout.logoutPendingSource(ctx, frames, uri);
       } else {
         ctx.redirect(uri);
       }

diff --git a/lib/consts/default_http_options.js b/lib/consts/default_http_options.js
diff --git a/lib/consts/index.js b/lib/consts/index.js
@@ -1,11 +1,9 @@
 const PARAM_LIST = require('./param_list');
 const DEV_KEYSTORE = require('./dev_keystore');
-const DEFAULT_HTTP_OPTIONS = require('./default_http_options');
 const CLIENT_ATTRIBUTES = require('./client_attributes');
 const JWA = require('./jwa');
 
 module.exports = {
-  DEFAULT_HTTP_OPTIONS,
   DEV_KEYSTORE,
   PARAM_LIST,
   CLIENT_ATTRIBUTES,

diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js
@@ -693,7 +693,7 @@ const DEFAULTS = {
        *   called to trigger RP logouts. It should handle waiting for the frames to be loaded as well
        *   as have a timeout mechanism in it.
        */
-      async logoutPendingSource(ctx, frames, postLogoutRedirectUri, timeout) {
+      async logoutPendingSource(ctx, frames, postLogoutRedirectUri) {
         shouldChange('features.frontchannelLogout.logoutPendingSource', 'customize the front-channel logout pending page');
         ctx.body = `<!DOCTYPE html>
     <head>
@@ -721,7 +721,7 @@ const DEFAULTS = {
         Array.prototype.slice.call(document.querySelectorAll('iframe')).forEach(function (element) {
           element.onload = frameOnLoad;
         });
-        setTimeout(redirect, ${timeout});
+        setTimeout(redirect, 2500);
       </script>
       <noscript>
         Your browser does not support JavaScript or you've disabled it.<br/>
@@ -1142,6 +1142,38 @@ const DEFAULTS = {
     ClientCredentials: undefined,
   },
 
+  /*
+   * httpOptions
+   *
+   * description: Helper called whenever the provider calls an external HTTP(S) resource. Use to
+   * change the [got](https://github.com/sindresorhus/got/tree/v9.6.0) library's request options
+   * as they happen. This can be used to e.g. change the request timeout option or to configure
+   * the global agent to use HTTP_PROXY and HTTPS_PROXY environment variables.
+   *
+   * example: To change the request's timeout
+   * To change all request's timeout configure the httpOptions as a function like so:
+   *
+   *```js
+   * {
+   *   httpOptions(options) {
+   *     options.timeout = 5000;
+   *     return options;
+   *   }
+   * }
+   *```
+   */
+  /* istanbul ignore next */
+  httpOptions(options) {
+    /* eslint-disable no-param-reassign */
+    options.followRedirect = false;
+    options.headers['User-Agent'] = 'oidc-provider/${VERSION} (${ISSUER_IDENTIFIER})'; // eslint-disable-line no-template-curly-in-string
+    options.retry = 0;
+    options.throwHttpErrors = false;
+    options.timeout = 2500;
+    /* eslint-enable no-param-reassign */
+    return options;
+  },
+
   /*
    * expiresWithSession
    * description: Helper used by the OP to decide whether the given authorization code/ device code

diff --git a/lib/helpers/http/got.js b/lib/helpers/http/got.js
diff --git a/lib/helpers/http/index.js b/lib/helpers/http/index.js
diff --git a/lib/helpers/request.js b/lib/helpers/request.js
@@ -0,0 +1,28 @@
+const got = require('got');
+const { defaultsDeep } = require('lodash');
+
+const pkg = require('../../package.json');
+
+const { httpOptions } = require('./defaults');
+const instance = require('./weak_cache');
+
+const USER_AGENT = `${pkg.name}/${pkg.version} (${pkg.homepage})`;
+
+const DEFAULT_HTTP_OPTIONS = {
+  followRedirect: false,
+  headers: { 'User-Agent': USER_AGENT },
+  retry: 0,
+  throwHttpErrors: false,
+  timeout: 2500,
+};
+
+module.exports = async function request(options) {
+  const optsFn = instance(this).configuration('httpOptions');
+
+  let opts = defaultsDeep(options, DEFAULT_HTTP_OPTIONS);
+  if (optsFn !== httpOptions) {
+    opts = optsFn(opts);
+  }
+
+  return got(opts);
+};
diff --git a/lib/helpers/request_uri_cache.js b/lib/helpers/request_uri_cache.js
@@ -1,10 +1,11 @@
 const crypto = require('crypto');
 const assert = require('assert');
+const { STATUS_CODES } = require('http');
 
 const LRU = require('lru-cache');
 
-const httpRequest = require('./http');
 const epochTime = require('./epoch_time');
+const request = require('./request');
 
 class RequestUriCache {
   constructor(provider) {
@@ -25,14 +26,17 @@ class RequestUriCache {
       return cached;
     }
 
-    const { statusCode, headers, body } = await httpRequest.get(
-      requestUri,
-      this.provider.httpOptions(),
-    );
+    const { statusCode, headers, body } = await request.call(this.provider, {
+      method: 'GET',
+      url: requestUri,
+      headers: {
+        Accept: 'application/jwt',
+      },
+    });
 
     assert.deepEqual(
       statusCode, 200,
-      `unexpected request_uri statusCode, expected 200, got ${statusCode}`,
+      `unexpected request_uri response status code, expected 200 OK, got ${statusCode} ${STATUS_CODES[statusCode]}`,
     );
 
     let cacheDuration;