feat: add PKCE support for OAuth2 Authorization Code flows (#5361)

* Add PKCE support.

* Fix tests

* Update oauth2.md

* Rename usePkce

* Fix the BrokenComponent error

* Update oauth2.md

* Remove isCode variable. Remove uuid4 dependency.

* Remove utils functions

* Import crypto

* Fix tests

* Fix the tests

* Cleanup

* Fix code_challenge generation

* Move code challenge and verifier to utils for mocks. Update tests.

* Mock the PKCE methods in the utils file properly.

* Add missing expect

* use target-method spies

* Add comments to explain test values.

* Get rid of jsrsasign.

Author: poveilleux

dev-helpers/index.html

@@ -62,7 +62,8 @@
         realm: "your-realms",
         appName: "your-app-name",
         scopeSeparator: " ",
-        additionalQueryStringParams: {}
+        additionalQueryStringParams: {},
+        usePkceWithAuthorizationCodeGrant: false
       })
     }
   </script>

docker/configurator/oauth.js

@@ -1,7 +1,7 @@
 const translator = require("./translator")
 const indent = require("./helpers").indent
 
-const  oauthBlockSchema = {
+const oauthBlockSchema = {
   OAUTH_CLIENT_ID: {
     type: "string",
     name: "clientId"
@@ -26,6 +26,10 @@ const  oauthBlockSchema = {
   OAUTH_ADDITIONAL_PARAMS: {
     type: "object",
     name: "additionalQueryStringParams"
+  },
+  OAUTH_USE_PKCE: {
+    type: "boolean",
+    name: "usePkceWithAuthorizationCodeGrant"
   }
 }
 

docs/usage/oauth2.md

@@ -10,6 +10,7 @@ appName | `OAUTH_APP_NAME` |application name, displayed in authorization popup.
 scopeSeparator | `OAUTH_SCOPE_SEPARATOR` |scope separator for passing scopes, encoded before calling, default value is a space (encoded value `%20`). MUST be a string
 additionalQueryStringParams | `OAUTH_ADDITIONAL_PARAMS` |Additional query parameters added to `authorizationUrl` and `tokenUrl`. MUST be an object
 useBasicAuthenticationWithAccessCodeGrant | _Unavailable_ |Only activated for the `accessCode` flow.  During the `authorization_code` request to the `tokenUrl`, pass the [Client Password](https://tools.ietf.org/html/rfc6749#section-2.3.1) using the HTTP Basic Authentication scheme (`Authorization` header with `Basic base64encode(client_id + client_secret)`).  The default is `false`
+usePkceWithAuthorizationCodeGrant | `OAUTH_USE_PKCE` | Only applies to `authorizatonCode` flows. [Proof Key for Code Exchange](https://tools.ietf.org/html/rfc7636) brings enhanced security for OAuth public clients. The default is `false`
 
 ```javascript
 const ui = SwaggerUI({...})
@@ -21,6 +22,7 @@ ui.initOAuth({
     realm: "your-realms",
     appName: "your-app-name",
     scopeSeparator: " ",
-    additionalQueryStringParams: {test: "hello"}
+    additionalQueryStringParams: {test: "hello"},
+    usePkceWithAuthorizationCodeGrant: true
   })
 ```

package-lock.json

@@ -1,5 +1,5 @@
 import win from "core/window"
-import { btoa, sanitizeUrl } from "core/utils"
+import { btoa, sanitizeUrl, generateCodeVerifier, createCodeChallenge } from "core/utils"
 
 export default function authorize ( { auth, authActions, errActions, configs, authConfigs={} } ) {
   let { schema, scopes, name, clientId } = auth
@@ -66,6 +66,18 @@ export default function authorize ( { auth, authActions, errActions, configs, au
     query.push("realm=" + encodeURIComponent(authConfigs.realm))
   }
 
+  if (flow === "authorizationCode" && authConfigs.usePkceWithAuthorizationCodeGrant) {
+      const codeVerifier = generateCodeVerifier()
+      const codeChallenge = createCodeChallenge(codeVerifier)
+
+      query.push("code_challenge=" + codeChallenge)
+      query.push("code_challenge_method=S256")
+
+      // storing the Code Verifier so it can be sent to the token endpoint
+      // when exchanging the Authorization Code for an Access Token
+      auth.codeVerifier = codeVerifier
+  }
+
   let { additionalQueryStringParams } = authConfigs
 
   for (let key in additionalQueryStringParams) {

src/core/oauth2-authorize.js

@@ -120,13 +120,14 @@ export const authorizeApplication = ( auth ) => ( { authActions } ) => {
 }
 
 export const authorizeAccessCodeWithFormParams = ( { auth, redirectUrl } ) => ( { authActions } ) => {
-  let { schema, name, clientId, clientSecret } = auth
+  let { schema, name, clientId, clientSecret, codeVerifier } = auth
   let form = {
     grant_type: "authorization_code",
     code: auth.code,
     client_id: clientId,
     client_secret: clientSecret,
-    redirect_uri: redirectUrl
+    redirect_uri: redirectUrl,
+    code_verifier: codeVerifier
   }
 
   return authActions.authorizeRequest({body: buildFormData(form), name, url: schema.get("tokenUrl"), auth})

src/core/plugins/auth/actions.js

@@ -22,6 +22,7 @@ import { memoizedSampleFromSchema, memoizedCreateXMLExample } from "core/plugins
 import win from "./window"
 import cssEscape from "css.escape"
 import getParameterSchema from "../helpers/get-parameter-schema"
+import crypto from "crypto"
 
 const DEFAULT_RESPONSE_KEY = "default"
 
@@ -859,3 +860,26 @@ export function paramToValue(param, paramValues) {
 
   return values[0]
 }
+
+// adapted from https://auth0.com/docs/flows/guides/auth-code-pkce/includes/create-code-verifier
+export function generateCodeVerifier() {
+  return toBase64UrlEncoded(
+    crypto.randomBytes(32)
+    .toString("base64")
+  )
+}
+
+export function createCodeChallenge(codeVerifier) {
+  return toBase64UrlEncoded(
+    crypto.createHash("sha256")
+    .update(codeVerifier, "ascii")
+    .digest("base64")
+  )
+}
+
+function toBase64UrlEncoded(str) {
+  return str
+    .replace(/\+/g, "-")
+    .replace(/\//g, "_")
+    .replace(/=/g, "")
+}

src/core/utils.js

@@ -1,8 +1,8 @@
 /* eslint-env mocha */
-import expect, { createSpy } from "expect"
-import { fromJS } from "immutable"
+import expect, { spyOn } from "expect"
 import win from "core/window"
 import oauth2Authorize from "core/oauth2-authorize"
+import * as utils from "core/utils"
 
 describe("oauth2", function () {
 
@@ -20,20 +20,55 @@ describe("oauth2", function () {
   }
 
   describe("authorize redirect", function () {
-
     it("should build authorize url", function() {
-      win.open = createSpy()
+      const windowOpenSpy = spyOn(win, "open")
       oauth2Authorize(authConfig)
-      expect(win.open.calls.length).toEqual(1)
-      expect(win.open.calls[0].arguments[0]).toMatch("https://testAuthorizationUrl?response_type=code&redirect_uri=&state=")
+      expect(windowOpenSpy.calls.length).toEqual(1)
+      expect(windowOpenSpy.calls[0].arguments[0]).toMatch("https://testAuthorizationUrl?response_type=code&redirect_uri=&state=")
+
+      windowOpenSpy.restore()
     })
 
     it("should append query parameters to authorizeUrl with query parameters", function() {
-      win.open = createSpy()
+      const windowOpenSpy = spyOn(win, "open")
       mockSchema.authorizationUrl = "https://testAuthorizationUrl?param=1"
       oauth2Authorize(authConfig)
-      expect(win.open.calls.length).toEqual(1)
-      expect(win.open.calls[0].arguments[0]).toMatch("https://testAuthorizationUrl?param=1&response_type=code&redirect_uri=&state=")
+      expect(windowOpenSpy.calls.length).toEqual(1)
+      expect(windowOpenSpy.calls[0].arguments[0]).toMatch("https://testAuthorizationUrl?param=1&response_type=code&redirect_uri=&state=")
+      
+      windowOpenSpy.restore()
     })
+
+    it("should send code_challenge when using authorizationCode flow with usePkceWithAuthorizationCodeGrant enabled", function () {
+      const windowOpenSpy = spyOn(win, "open")
+      mockSchema.flow = "authorizationCode"
+
+      const expectedCodeVerifier = "mock_code_verifier"
+      const expectedCodeChallenge = "mock_code_challenge"
+      
+      const generateCodeVerifierSpy = spyOn(utils, "generateCodeVerifier").andReturn(expectedCodeVerifier)
+      const createCodeChallengeSpy = spyOn(utils, "createCodeChallenge").andReturn(expectedCodeChallenge)
+
+      authConfig.authConfigs.usePkceWithAuthorizationCodeGrant = true
+
+      oauth2Authorize(authConfig)
+      expect(win.open.calls.length).toEqual(1)
+
+      const actualUrl = new URLSearchParams(win.open.calls[0].arguments[0])
+      expect(actualUrl.get("code_challenge")).toBe(expectedCodeChallenge)
+      expect(actualUrl.get("code_challenge_method")).toBe("S256")
+
+      expect(createCodeChallengeSpy.calls.length).toEqual(1)
+      expect(createCodeChallengeSpy.calls[0].arguments[0]).toBe(expectedCodeVerifier)
+
+      // The code_verifier should be stored to be able to send in
+      // on the TokenUrl call
+      expect(authConfig.auth.codeVerifier).toBe(expectedCodeVerifier)
+
+      // Restore spies
+      windowOpenSpy.restore()
+      generateCodeVerifierSpy.restore()
+      createCodeChallengeSpy.restore()
+    })    
   })
 })

test/mocha/core/oauth2-authorize.js

@@ -1,6 +1,9 @@
 /* eslint-env mocha */
 import expect, { createSpy } from "expect"
-import { authorizeRequest } from "corePlugins/auth/actions"
+import {
+  authorizeRequest,
+  authorizeAccessCodeWithFormParams,
+} from "corePlugins/auth/actions"
 
 describe("auth plugin - actions", () => {
 
@@ -144,4 +147,29 @@ describe("auth plugin - actions", () => {
         .toEqual("http://google.com/authorize?q=1&myCustomParam=abc123")
     })
   })
+
+  describe("tokenRequest", function() {
+    it("should send the code verifier when set", () => {
+      const data = { 
+        auth: {
+          schema: {
+            get: () => "http://tokenUrl"
+          },
+          codeVerifier: "mock_code_verifier"
+        }, 
+        redirectUrl: "http://google.com"
+      }
+
+      const authActions = {
+        authorizeRequest: createSpy()
+      }
+
+      authorizeAccessCodeWithFormParams(data)({ authActions })
+
+      expect(authActions.authorizeRequest.calls.length).toEqual(1)
+      const actualArgument = authActions.authorizeRequest.calls[0].arguments[0]
+      expect(actualArgument.body).toContain("code_verifier=" + data.auth.codeVerifier)
+      expect(actualArgument.body).toContain("grant_type=authorization_code")
+    })
+  })
 })

test/mocha/core/plugins/auth/actions.js

@@ -916,8 +916,8 @@ describe("bound system", function(){
     describe("components", function() {
       it("should catch errors thrown inside of React Component Class render methods", function() {
         // Given
-        // eslint-disable-next-line react/require-render-return
         class BrokenComponent extends React.Component {
+          // eslint-disable-next-line react/require-render-return
           render() {
             throw new Error("This component is broken")
           }
@@ -943,8 +943,8 @@ describe("bound system", function(){
 
       it("should catch errors thrown inside of pure component render methods", function() {
         // Given
-        // eslint-disable-next-line react/require-render-return
         class BrokenComponent extends PureComponent {
+          // eslint-disable-next-line react/require-render-return
           render() {
             throw new Error("This component is broken")
           }
@@ -994,8 +994,8 @@ describe("bound system", function(){
 
       it("should catch errors thrown inside of container components", function() {
         // Given
-        // eslint-disable-next-line react/require-render-return
         class BrokenComponent extends React.Component {
+          // eslint-disable-next-line react/require-render-return
           render() {
             throw new Error("This component is broken")
           }

test/mocha/core/system/system.jsx

@@ -28,6 +28,8 @@ import {
   getSampleSchema,
   paramToIdentifier,
   paramToValue,
+  generateCodeVerifier,
+  createCodeChallenge,
 } from "core/utils"
 import win from "core/window"
 
@@ -1402,4 +1404,27 @@ describe("utils", function() {
       expect(res).toEqual("asdf")
     })
   })
+
+  describe("generateCodeVerifier", function() {
+    it("should generate a value of at least 43 characters", () => {
+      const codeVerifier = generateCodeVerifier()
+
+      // Source: https://tools.ietf.org/html/rfc7636#section-4.1
+      expect(codeVerifier.length).toBeGreaterThanOrEqualTo(43)
+    })
+  })
+
+  describe("createCodeChallenge", function() {
+    it("should hash the input using SHA256 and output the base64 url encoded value", () => {
+      // The `codeVerifier` has been randomly generated
+      const codeVerifier = "cY8OJ9MKvZ7hxQeIyRYD7KFmKA5znSFJ2rELysvy2UI"
+
+      // This value is the `codeVerifier` hashed using SHA256, which has been
+      // encoded using base64 url format.
+      // Source: https://tools.ietf.org/html/rfc7636#section-4.2
+      const expectedCodeChallenge = "LD9lx2p2PbvGkojuJy7-Elex7RnckzmqR7oIXjd4u84"
+
+      expect(createCodeChallenge(codeVerifier)).toBe(expectedCodeChallenge)
+    })
+  })
 })

test/mocha/core/utils.js

@@ -23,6 +23,7 @@ describe("docker: env translator - oauth block", function() {
       OAUTH_APP_NAME: ``,
       OAUTH_SCOPE_SEPARATOR: "",
       OAUTH_ADDITIONAL_PARAMS: ``,
+      OAUTH_USE_PKCE: false
     }
 
     expect(oauthBlockBuilder(input)).toEqual(dedent(`
@@ -33,8 +34,10 @@ describe("docker: env translator - oauth block", function() {
       appName: "",
       scopeSeparator: "",
       additionalQueryStringParams: undefined,
+      usePkceWithAuthorizationCodeGrant: false,
     })`))
   })
+
   it("should generate a full block", function() {
     const input = {
       OAUTH_CLIENT_ID: `myId`,
@@ -43,6 +46,7 @@ describe("docker: env translator - oauth block", function() {
       OAUTH_APP_NAME: `myAppName`,
       OAUTH_SCOPE_SEPARATOR: "%21",
       OAUTH_ADDITIONAL_PARAMS: `{ "a": 1234, "b": "stuff" }`,
+      OAUTH_USE_PKCE: true
     }
 
     expect(oauthBlockBuilder(input)).toEqual(dedent(`
@@ -53,6 +57,7 @@ describe("docker: env translator - oauth block", function() {
       appName: "myAppName",
       scopeSeparator: "%21",
       additionalQueryStringParams: { "a": 1234, "b": "stuff" },
+      usePkceWithAuthorizationCodeGrant: true,
     })`))
   })
 })