Skip to content

Commit

Permalink
OIDC: use oidc-client-ts (#3544)
Browse files Browse the repository at this point in the history 
* use oidc-client-ts during oidc discovery

* export new type for auth config

* deprecate generateAuthorizationUrl in favour of generateOidcAuthorizationUrl

* testing util for oidc configurations

* test generateOidcAuthorizationUrl

* lint

* test discovery

* dont pass whole client wellknown to oidc validation funcs

* add nonce

* use client userState for homeserver
  • Loading branch information
Kerry authored Jul 9, 2023
1 parent b606d1e commit b8fa030
Show file tree
Hide file tree
Showing 10 changed files with 432 additions and 48 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"loglevel": "^1.7.1",
"matrix-events-sdk": "0.0.1",
"matrix-widget-api": "^1.3.1",
"oidc-client-ts": "^2.2.4",
"p-retry": "4",
"sdp-transform": "^2.14.1",
"unhomoglyph": "^1.0.6",
Expand Down
53 changes: 53 additions & 0 deletions spec/test-utils/oidc.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with 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.
*/

import { OidcClientConfig } from "../../src";
import { ValidatedIssuerMetadata } from "../../src/oidc/validate";

/**
* Makes a valid OidcClientConfig with minimum valid values
* @param issuer used as the base for all other urls
* @returns OidcClientConfig
*/
export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => {
const metadata = mockOpenIdConfiguration(issuer);

return {
issuer,
account: issuer + "account",
registrationEndpoint: metadata.registration_endpoint,
authorizationEndpoint: metadata.authorization_endpoint,
tokenEndpoint: metadata.token_endpoint,
metadata,
};
};

/**
* Useful for mocking <issuer>/.well-known/openid-configuration
* @param issuer used as the base for all other urls
* @returns ValidatedIssuerMetadata
*/
export const mockOpenIdConfiguration = (issuer = "https://auth.org/"): ValidatedIssuerMetadata => ({
issuer,
revocation_endpoint: issuer + "revoke",
token_endpoint: issuer + "token",
authorization_endpoint: issuer + "auth",
registration_endpoint: issuer + "registration",
jwks_uri: issuer + "jwks",
response_types_supported: ["code"],
grant_types_supported: ["authorization_code", "refresh_token"],
code_challenge_methods_supported: ["S256"],
});
82 changes: 82 additions & 0 deletions spec/unit/autodiscovery.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,17 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import fetchMock from "fetch-mock-jest";
import MockHttpBackend from "matrix-mock-request";

import { M_AUTHENTICATION } from "../../src";
import { AutoDiscovery } from "../../src/autodiscovery";
import { OidcError } from "../../src/oidc/error";
import { makeDelegatedAuthConfig } from "../test-utils/oidc";

// keep to reset the fetch function after using MockHttpBackend
// @ts-ignore private property
const realAutoDiscoveryFetch: typeof global.fetch = AutoDiscovery.fetchFn;

describe("AutoDiscovery", function () {
const getHttpBackend = (): MockHttpBackend => {
Expand All @@ -27,6 +34,10 @@ describe("AutoDiscovery", function () {
return httpBackend;
};

afterAll(() => {
AutoDiscovery.setFetchFn(realAutoDiscoveryFetch);
});

it("should throw an error when no domain is specified", function () {
getHttpBackend();
return Promise.all([
Expand Down Expand Up @@ -855,4 +866,75 @@ describe("AutoDiscovery", function () {
}),
]);
});

describe("m.authentication", () => {
const homeserverName = "example.org";
const homeserverUrl = "https://chat.example.org/";
const issuer = "https://auth.org/";

beforeAll(() => {
// make these tests independent from fetch mocking above
AutoDiscovery.setFetchFn(realAutoDiscoveryFetch);
});

beforeEach(() => {
fetchMock.resetBehavior();
fetchMock.get(`${homeserverUrl}_matrix/client/versions`, { versions: ["r0.0.1"] });

fetchMock.get("https://example.org/.well-known/matrix/client", {
"m.homeserver": {
// Note: we also expect this test to trim the trailing slash
base_url: "https://chat.example.org/",
},
"m.authentication": {
issuer,
},
});
});

it("should return valid authentication configuration", async () => {
const config = makeDelegatedAuthConfig(issuer);

fetchMock.get(`${config.metadata.issuer}.well-known/openid-configuration`, config.metadata);
fetchMock.get(`${config.metadata.issuer}jwks`, {
status: 200,
headers: {
"Content-Type": "application/json",
},
keys: [],
});

const result = await AutoDiscovery.findClientConfig(homeserverName);

expect(result[M_AUTHENTICATION.stable!]).toEqual({
state: AutoDiscovery.SUCCESS,
...config,
signingKeys: [],
account: undefined,
error: null,
});
});

it("should set state to error for invalid authentication configuration", async () => {
const config = makeDelegatedAuthConfig(issuer);
// authorization_code is required
config.metadata.grant_types_supported = ["openid"];

fetchMock.get(`${config.metadata.issuer}.well-known/openid-configuration`, config.metadata);
fetchMock.get(`${config.metadata.issuer}jwks`, {
status: 200,
headers: {
"Content-Type": "application/json",
},
keys: [],
});

const result = await AutoDiscovery.findClientConfig(homeserverName);

expect(result[M_AUTHENTICATION.stable!]).toEqual({
state: AutoDiscovery.FAIL_ERROR,
error: OidcError.OpSupport,
});
});
});
});
54 changes: 36 additions & 18 deletions spec/unit/oidc/authorize.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
/**
* @jest-environment jsdom
*/

/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Expand Down Expand Up @@ -25,29 +29,27 @@ import {
completeAuthorizationCodeGrant,
generateAuthorizationParams,
generateAuthorizationUrl,
generateOidcAuthorizationUrl,
} from "../../../src/oidc/authorize";
import { OidcError } from "../../../src/oidc/error";
import { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "../../test-utils/oidc";

jest.mock("jwt-decode");

// save for resetting mocks
const realSubtleCrypto = crypto.subtleCrypto;

describe("oidc authorization", () => {
const issuer = "https://auth.com/";
const authorizationEndpoint = "https://auth.com/authorization";
const tokenEndpoint = "https://auth.com/token";
const delegatedAuthConfig = {
issuer,
registrationEndpoint: issuer + "registration",
authorizationEndpoint: issuer + "auth",
tokenEndpoint,
};
const delegatedAuthConfig = makeDelegatedAuthConfig();
const authorizationEndpoint = delegatedAuthConfig.metadata.authorization_endpoint;
const tokenEndpoint = delegatedAuthConfig.metadata.token_endpoint;
const clientId = "xyz789";
const baseUrl = "https://test.com";

beforeAll(() => {
jest.spyOn(logger, "warn");

fetchMock.get(delegatedAuthConfig.issuer + ".well-known/openid-configuration", mockOpenIdConfiguration());
});

afterEach(() => {
Expand Down Expand Up @@ -97,20 +99,36 @@ describe("oidc authorization", () => {
"A secure context is required to generate code challenge. Using plain text code challenge",
);
});
});

describe("generateOidcAuthorizationUrl()", () => {
it("should generate url with correct parameters", async () => {
const nonce = "abc123";

const metadata = delegatedAuthConfig.metadata;

it("uses a s256 code challenge when crypto is available", async () => {
jest.spyOn(crypto.subtleCrypto, "digest");
const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl });
const authUrl = new URL(
await generateAuthorizationUrl(authorizationEndpoint, clientId, authorizationParams),
await generateOidcAuthorizationUrl({
metadata,
homeserverUrl: baseUrl,
clientId,
redirectUri: baseUrl,
nonce,
}),
);

const codeChallenge = authUrl.searchParams.get("code_challenge");
expect(crypto.subtleCrypto.digest).toHaveBeenCalledWith("SHA-256", expect.any(Object));
expect(authUrl.searchParams.get("response_mode")).toEqual("query");
expect(authUrl.searchParams.get("response_type")).toEqual("code");
expect(authUrl.searchParams.get("client_id")).toEqual(clientId);
expect(authUrl.searchParams.get("code_challenge_method")).toEqual("S256");
// scope minus the 10char random device id at the end
expect(authUrl.searchParams.get("scope")!.slice(0, -10)).toEqual(
"openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:",
);
expect(authUrl.searchParams.get("state")).toBeTruthy();
expect(authUrl.searchParams.get("nonce")).toEqual(nonce);

// didn't use plain text code challenge
expect(authorizationParams.codeVerifier).not.toEqual(codeChallenge);
expect(codeChallenge).toBeTruthy();
expect(authUrl.searchParams.get("code_challenge")).toBeTruthy();
});
});

Expand Down
35 changes: 14 additions & 21 deletions spec/unit/oidc/validate.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe("validateWellKnownAuthentication()", () => {
},
};
it("should throw not supported error when wellKnown has no m.authentication section", () => {
expect(() => validateWellKnownAuthentication(baseWk)).toThrow(OidcError.NotSupported);
expect(() => validateWellKnownAuthentication(undefined)).toThrow(OidcError.NotSupported);
});

it("should throw misconfigured error when authentication issuer is not a string", () => {
Expand All @@ -45,7 +45,9 @@ describe("validateWellKnownAuthentication()", () => {
issuer: { url: "test.com" },
},
};
expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured);
expect(() => validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!] as any)).toThrow(
OidcError.Misconfigured,
);
});

it("should throw misconfigured error when authentication account is not a string", () => {
Expand All @@ -56,7 +58,9 @@ describe("validateWellKnownAuthentication()", () => {
account: { url: "test" },
},
};
expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured);
expect(() => validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!] as any)).toThrow(
OidcError.Misconfigured,
);
});

it("should throw misconfigured error when authentication account is false", () => {
Expand All @@ -67,7 +71,9 @@ describe("validateWellKnownAuthentication()", () => {
account: false,
},
};
expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured);
expect(() => validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!] as any)).toThrow(
OidcError.Misconfigured,
);
});

it("should return valid config when wk uses stable m.authentication", () => {
Expand All @@ -78,7 +84,7 @@ describe("validateWellKnownAuthentication()", () => {
account: "account.com",
},
};
expect(validateWellKnownAuthentication(wk)).toEqual({
expect(validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!])).toEqual({
issuer: "test.com",
account: "account.com",
});
Expand All @@ -91,7 +97,7 @@ describe("validateWellKnownAuthentication()", () => {
issuer: "test.com",
},
};
expect(validateWellKnownAuthentication(wk)).toEqual({
expect(validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!])).toEqual({
issuer: "test.com",
});
});
Expand All @@ -104,22 +110,8 @@ describe("validateWellKnownAuthentication()", () => {
somethingElse: "test",
},
};
expect(validateWellKnownAuthentication(wk)).toEqual({
issuer: "test.com",
});
});

it("should return valid config when wk uses unstable prefix for m.authentication", () => {
const wk = {
...baseWk,
[M_AUTHENTICATION.unstable!]: {
issuer: "test.com",
account: "account.com",
},
};
expect(validateWellKnownAuthentication(wk)).toEqual({
expect(validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!])).toEqual({
issuer: "test.com",
account: "account.com",
});
});
});
Expand All @@ -129,6 +121,7 @@ describe("validateOIDCIssuerWellKnown", () => {
authorization_endpoint: "https://test.org/authorize",
token_endpoint: "https://authorize.org/token",
registration_endpoint: "https://authorize.org/regsiter",
revocation_endpoint: "https://authorize.org/regsiter",
response_types_supported: ["code"],
grant_types_supported: ["authorization_code"],
code_challenge_methods_supported: ["S256"],
Expand Down
Loading

0 comments on commit b8fa030

Please sign in to comment.