OIDC: validate id token (#3531)

* validate id token

* comments

* tidy comments

package.json

@@ -59,6 +59,7 @@
         "another-json": "^0.2.0",
         "bs58": "^5.0.0",
         "content-type": "^1.0.4",
+        "jwt-decode": "^3.1.2",
         "loglevel": "^1.7.1",
         "matrix-events-sdk": "0.0.1",
         "matrix-widget-api": "^1.3.1",

spec/unit/oidc/authorize.spec.ts

@@ -15,6 +15,8 @@ limitations under the License.
 */
 
 import fetchMock from "fetch-mock-jest";
+import { mocked } from "jest-mock";
+import jwtDecode from "jwt-decode";
 
 import { Method } from "../../../src";
 import * as crypto from "../../../src/crypto/crypto";
@@ -26,6 +28,8 @@ import {
 } from "../../../src/oidc/authorize";
 import { OidcError } from "../../../src/oidc/error";
 
+jest.mock("jwt-decode");
+
 // save for resetting mocks
 const realSubtleCrypto = crypto.subtleCrypto;
 
@@ -112,15 +116,28 @@ describe("oidc authorization", () => {
 
     describe("completeAuthorizationCodeGrant", () => {
         const codeVerifier = "abc123";
+        const nonce = "test-nonce";
         const redirectUri = baseUrl;
         const code = "auth_code_xyz";
         const validBearerTokenResponse = {
             token_type: "Bearer",
             access_token: "test_access_token",
             refresh_token: "test_refresh_token",
+            id_token: "valid.id.token",
             expires_in: 12345,
         };
 
+        const validDecodedIdToken = {
+            // nonce matches
+            nonce,
+            // not expired
+            exp: Date.now() / 1000 + 100000,
+            // audience is this client
+            aud: clientId,
+            // issuer matches
+            iss: delegatedAuthConfig.issuer,
+        };
+
         beforeEach(() => {
             fetchMock.mockClear();
             fetchMock.resetBehavior();
@@ -129,10 +146,18 @@ describe("oidc authorization", () => {
                 status: 200,
                 body: JSON.stringify(validBearerTokenResponse),
             });
+
+            mocked(jwtDecode).mockReturnValue(validDecodedIdToken);
         });
 
         it("should make correct request to the token endpoint", async () => {
-            await completeAuthorizationCodeGrant(code, { clientId, codeVerifier, redirectUri, delegatedAuthConfig });
+            await completeAuthorizationCodeGrant(code, {
+                clientId,
+                codeVerifier,
+                redirectUri,
+                delegatedAuthConfig,
+                nonce,
+            });
 
             expect(fetchMock).toHaveBeenCalledWith(tokenEndpoint, {
                 method: Method.Post,
@@ -147,6 +172,7 @@ describe("oidc authorization", () => {
                 codeVerifier,
                 redirectUri,
                 delegatedAuthConfig,
+                nonce,
             });
 
             expect(result).toEqual(validBearerTokenResponse);
@@ -171,6 +197,7 @@ describe("oidc authorization", () => {
                 codeVerifier,
                 redirectUri,
                 delegatedAuthConfig,
+                nonce,
             });
 
             // results in token that uses 'Bearer' token type
@@ -187,7 +214,13 @@ describe("oidc authorization", () => {
                 { overwriteRoutes: true },
             );
             await expect(() =>
-                completeAuthorizationCodeGrant(code, { clientId, codeVerifier, redirectUri, delegatedAuthConfig }),
+                completeAuthorizationCodeGrant(code, {
+                    clientId,
+                    codeVerifier,
+                    redirectUri,
+                    delegatedAuthConfig,
+                    nonce,
+                }),
             ).rejects.toThrow(new Error(OidcError.CodeExchangeFailed));
         });
 
@@ -202,8 +235,31 @@ describe("oidc authorization", () => {
                 { overwriteRoutes: true },
             );
             await expect(() =>
-                completeAuthorizationCodeGrant(code, { clientId, codeVerifier, redirectUri, delegatedAuthConfig }),
+                completeAuthorizationCodeGrant(code, {
+                    clientId,
+                    codeVerifier,
+                    redirectUri,
+                    delegatedAuthConfig,
+                    nonce,
+                }),
             ).rejects.toThrow(new Error(OidcError.InvalidBearerTokenResponse));
         });
+
+        it("should throw invalid id token error when id_token is invalid", async () => {
+            mocked(jwtDecode).mockReturnValue({
+                ...validDecodedIdToken,
+                // invalid audience
+                aud: "something-else",
+            });
+            await expect(() =>
+                completeAuthorizationCodeGrant(code, {
+                    clientId,
+                    codeVerifier,
+                    redirectUri,
+                    delegatedAuthConfig,
+                    nonce,
+                }),
+            ).rejects.toThrow(new Error(OidcError.InvalidIdToken));
+        });
     });
 });

spec/unit/oidc/validate.spec.ts

@@ -14,11 +14,20 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { mocked } from "jest-mock";
+import jwtDecode from "jwt-decode";
+
 import { M_AUTHENTICATION } from "../../../src";
 import { logger } from "../../../src/logger";
-import { validateOIDCIssuerWellKnown, validateWellKnownAuthentication } from "../../../src/oidc/validate";
+import {
+    validateIdToken,
+    validateOIDCIssuerWellKnown,
+    validateWellKnownAuthentication,
+} from "../../../src/oidc/validate";
 import { OidcError } from "../../../src/oidc/error";
 
+jest.mock("jwt-decode");
+
 describe("validateWellKnownAuthentication()", () => {
     const baseWk = {
         "m.homeserver": {
@@ -194,3 +203,96 @@ describe("validateOIDCIssuerWellKnown", () => {
         expect(() => validateOIDCIssuerWellKnown(wk)).toThrow(OidcError.OpSupport);
     });
 });
+
+describe("validateIdToken()", () => {
+    const nonce = "test-nonce";
+    const issuer = "https://auth.org/issuer";
+    const clientId = "test-client-id";
+    const idToken = "test-id-token";
+
+    const validDecodedIdToken = {
+        // nonce matches
+        nonce,
+        // not expired
+        exp: Date.now() / 1000 + 5555,
+        // audience is this client
+        aud: clientId,
+        // issuer matches
+        iss: issuer,
+    };
+    beforeEach(() => {
+        mocked(jwtDecode).mockClear().mockReturnValue(validDecodedIdToken);
+
+        jest.spyOn(logger, "error").mockClear();
+    });
+
+    it("should throw when idToken is falsy", () => {
+        expect(() => validateIdToken(undefined, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+    });
+
+    it("should throw when idToken cannot be decoded", () => {
+        mocked(jwtDecode).mockImplementation(() => {
+            throw new Error("oh no!");
+        });
+        expect(() => validateIdToken(undefined, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+    });
+
+    it("should throw when issuer does not match", () => {
+        mocked(jwtDecode).mockReturnValue({
+            ...validDecodedIdToken,
+            iss: "https://badissuer.com",
+        });
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+        expect(logger.error).toHaveBeenCalledWith("Invalid ID token", new Error("Invalid issuer"));
+    });
+
+    it("should throw when audience does not include clientId", () => {
+        mocked(jwtDecode).mockReturnValue({
+            ...validDecodedIdToken,
+            aud: "qwerty,uiop,asdf",
+        });
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+        expect(logger.error).toHaveBeenCalledWith("Invalid ID token", new Error("Invalid audience"));
+    });
+
+    it("should throw when audience includes clientId and other audiences", () => {
+        mocked(jwtDecode).mockReturnValue({
+            ...validDecodedIdToken,
+            aud: `${clientId},uiop,asdf`,
+        });
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+        expect(logger.error).toHaveBeenCalledWith("Invalid ID token", new Error("Invalid audience"));
+    });
+
+    it("should throw when nonce does not match", () => {
+        mocked(jwtDecode).mockReturnValue({
+            ...validDecodedIdToken,
+            nonce: "something else",
+        });
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+        expect(logger.error).toHaveBeenCalledWith("Invalid ID token", new Error("Invalid nonce"));
+    });
+
+    it("should throw when token does not have an expiry", () => {
+        mocked(jwtDecode).mockReturnValue({
+            ...validDecodedIdToken,
+            exp: undefined,
+        });
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+        expect(logger.error).toHaveBeenCalledWith("Invalid ID token", new Error("Invalid expiry"));
+    });
+
+    it("should throw when token is expired", () => {
+        mocked(jwtDecode).mockReturnValue({
+            ...validDecodedIdToken,
+            // expired in the past
+            exp: Date.now() / 1000 - 777,
+        });
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).toThrow(new Error(OidcError.InvalidIdToken));
+        expect(logger.error).toHaveBeenCalledWith("Invalid ID token", new Error("Invalid expiry"));
+    });
+
+    it("should not throw for a valid id token", () => {
+        expect(() => validateIdToken(idToken, issuer, clientId, nonce)).not.toThrow();
+    });
+});

src/oidc/authorize.ts

@@ -20,7 +20,7 @@ import { subtleCrypto, TextEncoder } from "../crypto/crypto";
 import { logger } from "../logger";
 import { randomString } from "../randomstring";
 import { OidcError } from "./error";
-import { ValidatedIssuerConfig } from "./validate";
+import { validateIdToken, ValidatedIssuerConfig } from "./validate";
 
 /**
  * Authorization parameters which are used in the authentication request of an OIDC auth code flow.
@@ -173,11 +173,13 @@ export const completeAuthorizationCodeGrant = async (
         codeVerifier,
         redirectUri,
         delegatedAuthConfig,
+        nonce,
     }: {
         clientId: string;
         codeVerifier: string;
         redirectUri: string;
         delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig;
+        nonce: string;
     },
 ): Promise<BearerTokenResponse> => {
     const params = new URLSearchParams();
@@ -203,6 +205,8 @@ export const completeAuthorizationCodeGrant = async (
     const token = await response.json();
 
     if (isValidBearerTokenResponse(token)) {
+        // throws when token is invalid
+        validateIdToken(token.id_token, delegatedAuthConfig.issuer, clientId, nonce);
         return normalizeBearerTokenResponseTokenType(token);
     }
 

src/oidc/error.ts

@@ -23,5 +23,6 @@ export enum OidcError {
     DynamicRegistrationFailed = "Dynamic registration failed",
     DynamicRegistrationInvalid = "Dynamic registration invalid response",
     CodeExchangeFailed = "Failed to exchange code for token",
-    InvalidBearerTokenResponse = "Invalid bearer token",
+    InvalidBearerTokenResponse = "Invalid bearer token response",
+    InvalidIdToken = "Invalid ID token",
 }