Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OIDC: validate id token #3531

Merged
merged 5 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
62 changes: 59 additions & 3 deletions spec/unit/oidc/authorize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -147,6 +172,7 @@ describe("oidc authorization", () => {
codeVerifier,
redirectUri,
delegatedAuthConfig,
nonce,
});

expect(result).toEqual(validBearerTokenResponse);
Expand All @@ -171,6 +197,7 @@ describe("oidc authorization", () => {
codeVerifier,
redirectUri,
delegatedAuthConfig,
nonce,
});

// results in token that uses 'Bearer' token type
Expand All @@ -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));
});

Expand All @@ -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));
});
});
});
104 changes: 103 additions & 1 deletion spec/unit/oidc/validate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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();
});
});
6 changes: 5 additions & 1 deletion src/oidc/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}

Expand Down
3 changes: 2 additions & 1 deletion src/oidc/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
Loading