PKCE implementation (#205)

* wip * cleanup, fix * cleanup / reduce * simple test app, performs PKCE flow * lint * fix tests * npm start * save / load code verifier * polyfill webcrypto, TextEncoder to test computeChallenge * Add options to util: - generateIDtoken - bypassCrypto (will "ignoreSignature") - option for timeout on test - responseVars for XHR templates * test exchangeFortoken, fix minor bug * cleanup, fix tests * address review feedback * changes needed for fetch to handle token post * validate response types (allow 'code' in array) * Pkce2 (#210) * streamline pkce * fix/disable tests * PKCE flow configured by "grantType: authorization_code" on getWithRedirect * Update README.md * Add unit test for fetch request. Explicitly mock cross-fetch where needed. * enable tests (defaults to fragment mode for code now) * address review feedback * Add karma test for crypto/pkce which needs webcrypto Removes "peculiar" polyfill * browser tests for complete login flow, implicit & pkce * fix karma test * update README from review feedback * Adds tests for "validateOptions" * remove package-lock.json from test/app * use --prefix (easier to understand) * do not export pkce interface * fix test app * validate code_challenge_method against well-known configuration * verify functionality of getWithPopup() for PKCE and implicit * fix tests * support PKCE directly in getToken (for popup, frame, etc.) * Use crypto to generate super random string for verifier * remove breaking change: responseMode * Add tests for renew token * add test for error/iframe offline_access * nits * throw if using undefined storage name key * add method to test for PKCE support (+tests for features) * Add PKCE paragraph to README * withCredentials for reqwest / jquery httpRequestClients * remove features from server test * Throw error if trying to use PKCE on unsupported browser * Update README.md * PKCE supported: only throw in constructor * lint nits * Accept more query params in test app * nit * small fix for testApp * set values for scopes, responseType in test * udpate test app readme * disallow "code" responseType * add tests for base64 utils * review feedback * expect responseType=code when grantType=authorization_code * nits * getToken*: throw if PKCE not supported
okta · Jun 3, 2019 · 0a8a4e1 · 0a8a4e1
1 parent cac7ee7
commit 0a8a4e1
Show file tree

Hide file tree

Showing 65 changed files with 14,921 additions and 779 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -4,6 +4,6 @@
 /buildtools
 /dist
 /target
-/node_modules
+node_modules
 *.config.js
 /lib/config.js
diff --git a/README.md b/README.md
@@ -162,8 +162,10 @@ tokenManager: {
 | `issuer`       | Specify a custom issuer to perform the OIDC flow. Defaults to the base url parameter if not provided. |
 | `clientId`     | Client Id pre-registered with Okta for the OIDC authentication flow. |
 | `redirectUri`  | The url that is redirected to when using `token.getWithRedirect`. This must be pre-registered as part of client registration. If no `redirectUri` is provided, defaults to the current origin. |
+| `grantType`  | Specify `grantType` for this Application. Supported types are `implicit` and `authorization_code`. Defaults to `implicit` |
 | `authorizeUrl` | Specify a custom authorizeUrl to perform the OIDC flow. Defaults to the issuer plus "/v1/authorize". |
 | `userinfoUrl`  | Specify a custom userinfoUrl. Defaults to the issuer plus "/v1/userinfo". |
+| `tokenUrl`  | Specify a custom tokenUrl. Defaults to the issuer plus "/v1/token". |
 | `ignoreSignature` | ID token signatures are validated by default when `token.getWithoutPrompt`, `token.getWithPopup`,  `token.getWithRedirect`, and `token.verify` are called. To disable ID token signature validation for these methods, set this value to `true`. |
 | | This option should be used only for browser support and testing purposes. |
 
@@ -191,6 +193,28 @@ var config = {
 var authClient = new OktaAuth(config);
 ```
 
+##### PKCE OAuth 2.0 flow
+
+By default the `implicit` OAuth flow will be used. It is widely supported by most browsers. PKCE is a newer flow which is more secure, but does require certain capabilities from the browser.
+
+To use PKCE flow, set `grantType` to `authorization_code` in your config.
+
+```javascript
+
+var config = {
+  grantType:  'authorization_code',
+
+  // other config
+  issuer: 'https://{yourOktaDomain}/oauth2/default',
+};
+
+var authClient = new OktaAuth(config);
+```
+
+If the user's browser does not support PKCE, an exception will be thrown. You can test if a browser supports PKCE before construction with this static method:
+
+`OktaAuth.features.isPKCESupported()`
+
 ### Optional configuration options
 
 ### `httpRequestClient`
@@ -212,7 +236,8 @@ var config = {
     //   headers: {
     //     headerName: headerValue
     //   },
-    //   data: postBodyData
+    //   data: postBodyData,
+    //   withCredentials: true|false,
     // }
     return Promise.resolve(/* a raw XMLHttpRequest response */);
   }
@@ -1367,14 +1392,15 @@ The following configuration options can **only** be included in `token.getWithou
 | :-------: | ----------|
 | `sessionToken` | Specify an Okta sessionToken to skip reauthentication when the user already authenticated using the Authentication Flow. |
 | `responseMode` | Specify how the authorization response should be returned. You will generally not need to set this unless you want to override the default values for `token.getWithRedirect`. See [Parameter Details](https://developer.okta.com/docs/api/resources/oidc#parameter-details) for a list of available modes. |
-| `responseType` | Specify the [response type](https://developer.okta.com/docs/api/resources/oidc#request-parameters) for OIDC authentication. Defaults to `id_token`. |
+| `responseType` | Specify the [response type](https://developer.okta.com/docs/api/resources/oidc#request-parameters) for OIDC authentication. The default value is based on the configured `grantType`. If `grantType` is `implicit` (the default setting), `responseType` will have a default value of `id_token`. If `grantType` is `authorization_code`, the default value will be `code`. |
 | | Use an array if specifying multiple response types - in this case, the response will contain both an ID Token and an Access Token. `responseType: ['id_token', 'token']` |
 | `scopes` | Specify what information to make available in the returned `id_token` or `access_token`. For OIDC, you must include `openid` as one of the scopes. Defaults to `['openid', 'email']`. For a list of available scopes, see [Scopes and Claims](https://developer.okta.com/docs/api/resources/oidc#access-token-scopes-and-claims). |
 | `state` | Specify a state that will be validated in an OAuth response. This is usually only provided during redirect flows to obtain an authorization code. Defaults to a random string. |
 | `nonce` | Specify a nonce that will be validated in an `id_token`. This is usually only provided during redirect flows to obtain an authorization code that will be exchanged for an `id_token`. Defaults to a random string. |
 
 For a list of all available parameters that can be passed to the `/authorize` endpoint, see Okta's [Authorize Request API](https://developer.okta.com/docs/api/resources/oidc#request-parameters).
 
+
 ##### Example
 
 ```javascript
@@ -1445,7 +1471,12 @@ authClient.token.getWithRedirect(oauthOptions);
 
 #### `token.parseFromUrl(options)`
 
-Parses the access or ID Tokens from the url after a successful authentication redirect. If an ID token is present, it will be [verified and validated](https://github.com/okta/okta-auth-js/blob/master/lib/token.js#L186-L190) before available for use.
+Parses the authorization code, access, or ID Tokens from the URL after a successful authentication redirect. 
+
+If an authorization code is present, it will be exchanged for token(s) by posting to the `tokenUrl` endpoint. 
+
+The ID token will be [verified and validated](https://github.com/okta/okta-auth-js/blob/master/lib/token.js#L186-L190) before available for use.
+
 
 ```javascript
 authClient.token.parseFromUrl()
@@ -1718,8 +1749,13 @@ yarn install
 | Command               | Description                    |
 | --------------------- | ------------------------------ |
 | `yarn build`          | Build the SDK with a sourcemap |
-| `yarn test`           | Run unit tests using Jest      |
+| `yarn test`           | Run unit tests     |
 | `yarn lint`           | Run eslint linting             |
+| `yarn start`          | Start internal test app        |
+
+#### Test App
+
+Implements a simple SPA application to demonstrate functionality and provide for manual testing. [See here for more information](test/app/README.md).
 
 ## Contributing
 

diff --git a/fetch/fetchRequest.js b/fetch/fetchRequest.js
@@ -12,12 +12,22 @@
 
 var fetch = require('cross-fetch');
 
+/* eslint-disable complexity */
 function fetchRequest(method, url, args) {
+  var body = args.data;
+  var headers = args.headers || {};
+  var contentType = (headers['Content-Type'] || headers['content-type'] || '');
+
+  // JSON encode body (if appropriate)
+  if (contentType === 'application/json' && body && typeof body !== 'string') {
+    body = JSON.stringify(body);
+  }
+
   var fetchPromise = fetch(url, {
     method: method,
     headers: args.headers,
-    body: JSON.stringify(args.data),
-    credentials: 'include'
+    body: body,
+    credentials: args.withCredentials === false ? 'omit' : 'include'
   })
   .then(function(response) {
     var error = !response.ok;

diff --git a/jest.server.js b/jest.server.js
@@ -16,7 +16,9 @@ module.exports = {
     './test/spec/oauthUtil.js',
     './test/spec/token.js',
     './test/spec/tokenManager.js',
-    './test/spec/webfinger.js'
+    './test/spec/webfinger.js',
+    './test/spec/pkce.js',
+    './test/spec/features.js'
   ],
   'reporters': [
     'default',

diff --git a/jquery/jqueryRequest.js b/jquery/jqueryRequest.js
@@ -13,14 +13,15 @@
 var $ = require('jquery');
 
 function jqueryRequest(method, url, args) {
+  // TODO: support content-type
   var deferred = $.Deferred();
   $.ajax({
     type: method,
     url: url,
     headers: args.headers,
     data: JSON.stringify(args.data),
     xhrFields: {
-      withCredentials: true
+      withCredentials: args.withCredentials
     }
   })
   .then(function(data, textStatus, jqXHR) {

diff --git a/karma.conf.js b/karma.conf.js
@@ -0,0 +1,88 @@
+/*!
+ * Copyright (c) 2019-Present, Okta, Inc. and/or its affiliates. All rights reserved.
+ * The Okta software accompanied by this notice is provided pursuant to the Apache License, Version 2.0 (the "License.")
+ *
+ * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0.
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *
+ * See the License for the specific language governing permissions and limitations under the License.
+ */
+
+// Karma configuration file, see link for more information
+// http://karma-runner.github.io/3.0/config/configuration-file.html
+
+/* global __dirname */
+var path = require('path');
+var REPORTS_DIR = path.join(__dirname, 'build2', 'reports', 'karma');
+
+var webpackConf = {
+  devtool: 'inline-source-map',
+  resolve: {
+    alias: {
+      '@okta/okta-auth-js': path.join(__dirname, 'lib/browser/browserIndex.js')
+    }
+  },
+  module: {
+    rules: [
+      {
+        test: /\.js$/,
+        use: { loader: 'istanbul-instrumenter-loader' },
+        enforce: 'post',
+        include: [
+          path.resolve(__dirname, 'lib')
+        ]
+      }
+    ]
+  }
+};
+
+module.exports = function (config) {
+  config.set({
+    basePath: '',
+    frameworks: ['jasmine', 'jquery-3.3.1'],
+    plugins: [
+      'karma-jasmine',
+      'karma-chrome-launcher',
+      'karma-coverage-istanbul-reporter',
+      'karma-webpack',
+      'karma-jquery',
+      'karma-sourcemap-loader'
+    ],
+    files: [
+      { pattern: './test/karma/main.js', watched: false }
+    ],
+    preprocessors: {
+      'test/karma/main.js': ['webpack', 'sourcemap']
+    },
+    webpack: webpackConf,
+    webpackMiddleware: {
+      stats: 'normal',
+    },
+    client: {
+      // Passing specific test to run
+      // but this works only with `karma start`, not `karma run`.
+      test: config.test,
+      clearContext: false // leave Jasmine Spec Runner output visible in browser
+    },
+    coverageIstanbulReporter: {
+      dir: REPORTS_DIR,
+      reports: [ 'html', 'lcovonly' ],
+      fixWebpackSourcePaths: true
+    },
+    reporters: ['progress', 'coverage-istanbul'],
+    port: 9876,
+    colors: true,
+    logLevel: config.LOG_INFO,
+    autoWatch: true,
+    browsers: ['ChromeHeadlessNoSandbox'],
+    customLaunchers: {
+      ChromeHeadlessNoSandbox: {
+        base: 'ChromeHeadless',
+        flags: ['--no-sandbox']
+      }
+    },
+    singleRun: false
+  });
+};
diff --git a/lib/TokenManager.js b/lib/TokenManager.js
@@ -140,18 +140,30 @@ function renew(sdk, tokenMgmtRef, storage, key) {
   // Store the renew promise state, to avoid renewing again
   if (!tokenMgmtRef.renewPromise[key]) {
     tokenMgmtRef.renewPromise[key] = sdk.token.renew(token)
-    .then(function(freshToken) {
-      if (!get(storage, key)) {
-        // It is possible to enter a state where the tokens have been cleared
-        // after a renewal request was triggered. To ensure we do not store a
-        // renewed token, we verify the promise key doesn't exist and return.
-        return;
+    .then(function(freshTokens) {
+      // We may receive more tokens than we requested
+      var map = {};
+      if (freshTokens instanceof Array === false) {
+        freshTokens = [freshTokens];
       }
-      add(sdk, tokenMgmtRef, storage, key, freshToken);
-      tokenMgmtRef.emitter.emit('renewed', key, freshToken, token);
+      freshTokens.forEach(function(freshToken) {
+        var inferredKey = freshToken.idToken ? 'idToken' : freshToken.accessToken ? 'accessToken' : key;
+        map[inferredKey] = freshToken;
+
+        var oldToken = get(storage, inferredKey);
+        if (!oldToken) {
+          // It is possible to enter a state where the tokens have been cleared
+          // after a renewal request was triggered. To ensure we do not store a
+          // renewed token, we verify the promise key doesn't exist and return.
+          return;
+        }
+        add(sdk, tokenMgmtRef, storage, inferredKey, freshToken);
+        tokenMgmtRef.emitter.emit('renewed', inferredKey, freshToken, oldToken);
+      });
+
       // Remove existing promise key
       delete tokenMgmtRef.renewPromise[key];
-      return freshToken;
+      return map[key]; // return the specific token requested
     })
     .fail(function(err) {
       if (err.name === 'OAuthError') {

diff --git a/lib/browser/browser.js b/lib/browser/browser.js
@@ -37,13 +37,19 @@ function OktaAuthBuilder(args) {
     issuer: util.removeTrailingSlash(args.issuer),
     authorizeUrl: util.removeTrailingSlash(args.authorizeUrl),
     userinfoUrl: util.removeTrailingSlash(args.userinfoUrl),
+    tokenUrl: util.removeTrailingSlash(args.tokenUrl),
+    grantType: args.grantType,
     redirectUri: args.redirectUri,
     httpRequestClient: args.httpRequestClient,
     storageUtil: args.storageUtil,
     transformErrorXHR: args.transformErrorXHR,
     headers: args.headers
   };
 
+  if (this.options.grantType === 'authorization_code' && !sdk.features.isPKCESupported()) {
+    throw new AuthSdkError('This browser doesn\'t support PKCE');
+  }
+
   this.userAgent = 'okta-auth-js-' + config.SDK_VERSION;
 
   // Digital clocks will drift over time, so the server
@@ -151,6 +157,10 @@ proto.features.isTokenVerifySupported = function() {
   return typeof crypto !== 'undefined' && crypto.subtle && typeof Uint8Array !== 'undefined';
 };
 
+proto.features.isPKCESupported = function() {
+  return proto.features.isTokenVerifySupported();
+};
+
 // { username, password, (relayState), (context) }
 proto.signIn = function (opts) {
   var sdk = this;

diff --git a/lib/browser/browserStorage.js b/lib/browser/browserStorage.js
@@ -38,6 +38,16 @@ storageUtil.browserHasSessionStorage = function() {
   }
 };
 
+storageUtil.getPKCEStorage = function() {
+  if (storageUtil.browserHasLocalStorage()) {
+    return storageBuilder(storageUtil.getLocalStorage(), config.PKCE_STORAGE_NAME);
+  } else if (storageUtil.browserHasSessionStorage()) {
+    return storageBuilder(storageUtil.getSessionStorage(), config.PKCE_STORAGE_NAME);
+  } else {
+    return storageBuilder(storageUtil.getCookieStorage(), config.PKCE_STORAGE_NAME);
+  }
+};
+
 storageUtil.getHttpCache = function() {
   if (storageUtil.browserHasLocalStorage()) {
     return storageBuilder(storageUtil.getLocalStorage(), config.CACHE_STORAGE_NAME);

diff --git a/lib/builderUtil.js b/lib/builderUtil.js
@@ -80,6 +80,9 @@ function buildOktaAuth(OktaAuthBuilder) {
     OktaAuth.prototype = OktaAuthBuilder.prototype;
     OktaAuth.prototype.constructor = OktaAuth;
 
+    // Hoist feature detection functions to static type
+    OktaAuth.features = OktaAuthBuilder.prototype.features;
+
     return OktaAuth;
   };
 }

diff --git a/lib/http.js b/lib/http.js
@@ -24,6 +24,7 @@ function httpRequest(sdk, options) {
       args = options.args,
       saveAuthnState = options.saveAuthnState,
       accessToken = options.accessToken,
+      withCredentials = options.withCredentials !== false, // default value is true
       storageUtil = sdk.options.storageUtil,
       storage = storageUtil.storage,
       httpCache = storageUtil.getHttpCache();
@@ -49,7 +50,8 @@ function httpRequest(sdk, options) {
 
   var ajaxOptions = {
     headers: headers,
-    data: args || undefined
+    data: args || undefined,
+    withCredentials: withCredentials
   };
 
   var err, res;