Skip to content

Commit ad01218

Browse files
t3chguyt3chguy
authored
Switch OIDC primarily to new /auth_metadata API (#29019)
* Switch OIDC primarily to new `/auth_metadata` API Signed-off-by: Michael Telatynski <[email protected]> * Update tests Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Simplify the world Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]>
1 parent e1e4d26 commit ad01218

17 files changed

+92
-149
lines changed

‎src/components/views/settings/devices/LoginWithQRSection.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ export function shouldShowQr(
3131
): boolean {
3232
const msc4108Supported = !!versions?.unstable_features?.["org.matrix.msc4108"];
3333

34-
const deviceAuthorizationGrantSupported =
35-
oidcClientConfig?.metadata?.grant_types_supported.includes(DEVICE_CODE_SCOPE);
34+
const deviceAuthorizationGrantSupported = oidcClientConfig?.grant_types_supported.includes(DEVICE_CODE_SCOPE);
3635

3736
return (
3837
!!deviceAuthorizationGrantSupported &&

‎src/components/views/settings/tabs/user/SessionManagerTab.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details.
77
*/
88

99
import React, { lazy, Suspense, useCallback, useContext, useEffect, useRef, useState } from "react";
10-
import { discoverAndValidateOIDCIssuerWellKnown, MatrixClient } from "matrix-js-sdk/src/matrix";
10+
import { MatrixClient } from "matrix-js-sdk/src/matrix";
1111
import { logger } from "matrix-js-sdk/src/logger";
1212
import { defer } from "matrix-js-sdk/src/utils";
1313

@@ -163,10 +163,7 @@ const SessionManagerTab: React.FC<{
163163
const clientVersions = useAsyncMemo(() => matrixClient.getVersions(), [matrixClient]);
164164
const oidcClientConfig = useAsyncMemo(async () => {
165165
try {
166-
const authIssuer = await matrixClient?.getAuthIssuer();
167-
if (authIssuer) {
168-
return discoverAndValidateOIDCIssuerWellKnown(authIssuer.issuer);
169-
}
166+
return await matrixClient?.getAuthMetadata();
170167
} catch (e) {
171168
logger.error("Failed to discover OIDC metadata", e);
172169
}

‎src/stores/oidc/OidcClientStore.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,8 @@ export class OidcClientStore {
5050
} else {
5151
// We are not in OIDC Native mode, as we have no locally stored issuer. Check if the server delegates auth to OIDC.
5252
try {
53-
const authIssuer = await this.matrixClient.getAuthIssuer();
54-
const { accountManagementEndpoint, metadata } = await discoverAndValidateOIDCIssuerWellKnown(
55-
authIssuer.issuer,
56-
);
57-
this.setAccountManagementEndpoint(accountManagementEndpoint, metadata.issuer);
53+
const authMetadata = await this.matrixClient.getAuthMetadata();
54+
this.setAccountManagementEndpoint(authMetadata.account_management_uri, authMetadata.issuer);
5855
} catch (e) {
5956
console.log("Auth issuer not found", e);
6057
}
@@ -153,14 +150,11 @@ export class OidcClientStore {
153150

154151
try {
155152
const clientId = getStoredOidcClientId();
156-
const { accountManagementEndpoint, metadata, signingKeys } = await discoverAndValidateOIDCIssuerWellKnown(
157-
this.authenticatedIssuer,
158-
);
159-
this.setAccountManagementEndpoint(accountManagementEndpoint, metadata.issuer);
153+
const authMetadata = await discoverAndValidateOIDCIssuerWellKnown(this.authenticatedIssuer);
154+
this.setAccountManagementEndpoint(authMetadata.account_management_uri, authMetadata.issuer);
160155
this.oidcClient = new OidcClient({
161-
...metadata,
162-
authority: metadata.issuer,
163-
signingKeys,
156+
authority: authMetadata.issuer,
157+
signingKeys: authMetadata.signingKeys ?? undefined,
164158
redirect_uri: PlatformPeg.get()!.getOidcCallbackUrl().href,
165159
client_id: clientId,
166160
});

‎src/utils/AutoDiscoveryUtils.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
AutoDiscovery,
1212
AutoDiscoveryError,
1313
ClientConfig,
14-
discoverAndValidateOIDCIssuerWellKnown,
1514
IClientWellKnown,
1615
MatrixClient,
1716
MatrixError,
@@ -293,8 +292,7 @@ export default class AutoDiscoveryUtils {
293292
let delegatedAuthenticationError: Error | undefined;
294293
try {
295294
const tempClient = new MatrixClient({ baseUrl: preferredHomeserverUrl });
296-
const { issuer } = await tempClient.getAuthIssuer();
297-
delegatedAuthentication = await discoverAndValidateOIDCIssuerWellKnown(issuer);
295+
delegatedAuthentication = await tempClient.getAuthMetadata();
298296
} catch (e) {
299297
if (e instanceof MatrixError && e.httpStatus === 404 && e.errcode === "M_UNRECOGNIZED") {
300298
// 404 M_UNRECOGNIZED means the server does not support OIDC

‎src/utils/oidc/authorize.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export const startOidcLogin = async (
3939
const prompt = isRegistration ? "create" : undefined;
4040

4141
const authorizationUrl = await generateOidcAuthorizationUrl({
42-
metadata: delegatedAuthConfig.metadata,
42+
metadata: delegatedAuthConfig,
4343
redirectUri,
4444
clientId,
4545
homeserverUrl,

‎src/utils/oidc/isUserRegistrationSupported.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ import { OidcClientConfig } from "matrix-js-sdk/src/matrix";
1515
* @returns whether user registration is supported
1616
*/
1717
export const isUserRegistrationSupported = (delegatedAuthConfig: OidcClientConfig): boolean => {
18-
// The OidcMetadata type from oidc-client-ts does not include `prompt_values_supported`
19-
// even though it is part of the OIDC spec, so cheat TS here to access it
20-
const supportedPrompts = (delegatedAuthConfig.metadata as Record<string, unknown>)["prompt_values_supported"];
18+
const supportedPrompts = delegatedAuthConfig.prompt_values_supported;
2119
return Array.isArray(supportedPrompts) && supportedPrompts?.includes("create");
2220
};

‎src/utils/oidc/registerClient.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ export const getOidcClientId = async (
4040
delegatedAuthConfig: OidcClientConfig,
4141
staticOidcClients?: IConfigOptions["oidc_static_clients"],
4242
): Promise<string> => {
43-
const staticClientId = getStaticOidcClientId(delegatedAuthConfig.metadata.issuer, staticOidcClients);
43+
const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients);
4444
if (staticClientId) {
45-
logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.metadata.issuer}`);
45+
logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`);
4646
return staticClientId;
4747
}
4848
return await registerOidcClient(delegatedAuthConfig, await PlatformPeg.get()!.getOidcClientMetadata());

‎test/test-utils/oidc.ts

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,41 +6,4 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import { OidcClientConfig } from "matrix-js-sdk/src/matrix";
10-
import { ValidatedIssuerMetadata } from "matrix-js-sdk/src/oidc/validate";
11-
12-
/**
13-
* Makes a valid OidcClientConfig with minimum valid values
14-
* @param issuer used as the base for all other urls
15-
* @returns OidcClientConfig
16-
*/
17-
export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => {
18-
const metadata = mockOpenIdConfiguration(issuer);
19-
20-
return {
21-
accountManagementEndpoint: issuer + "account",
22-
registrationEndpoint: metadata.registration_endpoint,
23-
authorizationEndpoint: metadata.authorization_endpoint,
24-
tokenEndpoint: metadata.token_endpoint,
25-
metadata,
26-
};
27-
};
28-
29-
/**
30-
* Useful for mocking <issuer>/.well-known/openid-configuration
31-
* @param issuer used as the base for all other urls
32-
* @returns ValidatedIssuerMetadata
33-
*/
34-
export const mockOpenIdConfiguration = (issuer = "https://auth.org/"): ValidatedIssuerMetadata => ({
35-
issuer,
36-
revocation_endpoint: issuer + "revoke",
37-
token_endpoint: issuer + "token",
38-
authorization_endpoint: issuer + "auth",
39-
registration_endpoint: issuer + "registration",
40-
device_authorization_endpoint: issuer + "device",
41-
jwks_uri: issuer + "jwks",
42-
response_types_supported: ["code"],
43-
grant_types_supported: ["authorization_code", "refresh_token"],
44-
code_challenge_methods_supported: ["S256"],
45-
account_management_uri: issuer + "account",
46-
});
9+
export { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "matrix-js-sdk/src/testing";

‎test/unit-tests/Lifecycle-test.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -749,11 +749,8 @@ describe("Lifecycle", () => {
749749
"eyJhbGciOiJSUzI1NiIsImtpZCI6Imh4ZEhXb0Y5bW4ifQ.eyJzdWIiOiIwMUhQUDJGU0JZREU5UDlFTU04REQ3V1pIUiIsImlzcyI6Imh0dHBzOi8vYXV0aC1vaWRjLmxhYi5lbGVtZW50LmRldi8iLCJpYXQiOjE3MTUwNzE5ODUsImF1dGhfdGltZSI6MTcwNzk5MDMxMiwiY19oYXNoIjoidGt5R1RhUjU5aTk3YXoyTU4yMGdidyIsImV4cCI6MTcxNTA3NTU4NSwibm9uY2UiOiJxaXhwM0hFMmVaIiwiYXVkIjoiMDFIWDk0Mlg3QTg3REgxRUs2UDRaNjI4WEciLCJhdF9oYXNoIjoiNFlFUjdPRlVKTmRTeEVHV2hJUDlnZyJ9.HxODneXvSTfWB5Vc4cf7b8GiN2gdwUuTiyVqZuupWske2HkZiJZUt5Lsxg9BW3gz28POkE0Ln17snlkmy02B_AD3DQxKOOxQCzIIARHdfFvZxgGWsMdFcVQZDW7rtXcqgj-SpVaUQ_8acsgxSrz_DF2o0O4tto0PT6wVUiw8KlBmgWTscWPeAWe-39T-8EiQ8Wi16h6oSPcz2NzOQ7eOM_S9fDkOorgcBkRGLl1nrahrPSdWJSGAeruk5mX4YxN714YThFDyEA2t9YmKpjaiSQ2tT-Xkd7tgsZqeirNs2ni9mIiFX3bRX6t2AhUNzA7MaX9ZyizKGa6go3BESO_oDg";
750750

751751
beforeAll(() => {
752-
fetchMock.get(
753-
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
754-
delegatedAuthConfig.metadata,
755-
);
756-
fetchMock.get(`${delegatedAuthConfig.metadata.issuer}jwks`, {
752+
fetchMock.get(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`, delegatedAuthConfig);
753+
fetchMock.get(`${delegatedAuthConfig.issuer}jwks`, {
757754
status: 200,
758755
headers: {
759756
"Content-Type": "application/json",
@@ -772,9 +769,7 @@ describe("Lifecycle", () => {
772769
await setLoggedIn(credentials);
773770

774771
// didn't try to initialise token refresher
775-
expect(fetchMock).not.toHaveFetched(
776-
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
777-
);
772+
expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`);
778773
});
779774

780775
it("should not try to create a token refresher without a deviceId", async () => {
@@ -785,9 +780,7 @@ describe("Lifecycle", () => {
785780
});
786781

787782
// didn't try to initialise token refresher
788-
expect(fetchMock).not.toHaveFetched(
789-
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
790-
);
783+
expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`);
791784
});
792785

793786
it("should not try to create a token refresher without an issuer in session storage", async () => {
@@ -803,9 +796,7 @@ describe("Lifecycle", () => {
803796
});
804797

805798
// didn't try to initialise token refresher
806-
expect(fetchMock).not.toHaveFetched(
807-
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
808-
);
799+
expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`);
809800
});
810801

811802
it("should create a client with a tokenRefreshFunction", async () => {

‎test/unit-tests/components/structures/auth/Login-test.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ describe("Login", function () {
384384
await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
385385

386386
// didn't try to register
387-
expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint);
387+
expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registration_endpoint);
388388
// continued with normal setup
389389
expect(mockClient.loginFlows).toHaveBeenCalled();
390390
// normal password login rendered
@@ -394,25 +394,25 @@ describe("Login", function () {
394394
it("should attempt to register oidc client", async () => {
395395
// dont mock, spy so we can check config values were correctly passed
396396
jest.spyOn(registerClientUtils, "getOidcClientId");
397-
fetchMock.post(delegatedAuth.registrationEndpoint!, { status: 500 });
397+
fetchMock.post(delegatedAuth.registration_endpoint!, { status: 500 });
398398
getComponent(hsUrl, isUrl, delegatedAuth);
399399

400400
await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
401401

402402
// tried to register
403-
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object));
403+
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registration_endpoint, expect.any(Object));
404404
// called with values from config
405405
expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith(delegatedAuth, oidcStaticClientsConfig);
406406
});
407407

408408
it("should fallback to normal login when client registration fails", async () => {
409-
fetchMock.post(delegatedAuth.registrationEndpoint!, { status: 500 });
409+
fetchMock.post(delegatedAuth.registration_endpoint!, { status: 500 });
410410
getComponent(hsUrl, isUrl, delegatedAuth);
411411

412412
await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
413413

414414
// tried to register
415-
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object));
415+
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registration_endpoint, expect.any(Object));
416416
expect(logger.error).toHaveBeenCalledWith(new Error(OidcError.DynamicRegistrationFailed));
417417

418418
// continued with normal setup
@@ -423,7 +423,7 @@ describe("Login", function () {
423423

424424
// short term during active development, UI will be added in next PRs
425425
it("should show continue button when oidc native flow is correctly configured", async () => {
426-
fetchMock.post(delegatedAuth.registrationEndpoint!, { client_id: "abc123" });
426+
fetchMock.post(delegatedAuth.registration_endpoint!, { client_id: "abc123" });
427427
getComponent(hsUrl, isUrl, delegatedAuth);
428428

429429
await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
@@ -455,7 +455,7 @@ describe("Login", function () {
455455
await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
456456

457457
// didn't try to register
458-
expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint);
458+
expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registration_endpoint);
459459
// continued with normal setup
460460
expect(mockClient.loginFlows).toHaveBeenCalled();
461461
// oidc-aware 'continue' button displayed

‎test/unit-tests/components/structures/auth/Registration-test.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,24 +158,26 @@ describe("Registration", function () {
158158
describe("when delegated authentication is configured and enabled", () => {
159159
const authConfig = makeDelegatedAuthConfig();
160160
const clientId = "test-client-id";
161-
// @ts-ignore
162-
authConfig.metadata["prompt_values_supported"] = ["create"];
161+
authConfig.prompt_values_supported = ["create"];
163162

164163
beforeEach(() => {
165164
// mock a statically registered client to avoid dynamic registration
166165
SdkConfig.put({
167166
oidc_static_clients: {
168-
[authConfig.metadata.issuer]: {
167+
[authConfig.issuer]: {
169168
client_id: clientId,
170169
},
171170
},
172171
});
173172

174173
fetchMock.get(`${defaultHsUrl}/_matrix/client/unstable/org.matrix.msc2965/auth_issuer`, {
175-
issuer: authConfig.metadata.issuer,
174+
issuer: authConfig.issuer,
176175
});
177-
fetchMock.get("https://auth.org/.well-known/openid-configuration", authConfig.metadata);
178-
fetchMock.get(authConfig.metadata.jwks_uri!, { keys: [] });
176+
fetchMock.get("https://auth.org/.well-known/openid-configuration", {
177+
...authConfig,
178+
signingKeys: undefined,
179+
});
180+
fetchMock.get(authConfig.jwks_uri!, { keys: [] });
179181
});
180182

181183
it("should display oidc-native continue button", async () => {

‎test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ import SettingsStore from "../../../../../../../src/settings/SettingsStore";
5757
import { getClientInformationEventType } from "../../../../../../../src/utils/device/clientInformation";
5858
import { SDKContext, SdkContextClass } from "../../../../../../../src/contexts/SDKContext";
5959
import { OidcClientStore } from "../../../../../../../src/stores/oidc/OidcClientStore";
60-
import { mockOpenIdConfiguration } from "../../../../../../test-utils/oidc";
60+
import { makeDelegatedAuthConfig } from "../../../../../../test-utils/oidc";
6161
import MatrixClientContext from "../../../../../../../src/contexts/MatrixClientContext";
6262

6363
mockPlatformPeg();
@@ -215,7 +215,7 @@ describe("<SessionManagerTab />", () => {
215215
getPushers: jest.fn(),
216216
setPusher: jest.fn(),
217217
setLocalNotificationSettings: jest.fn(),
218-
getAuthIssuer: jest.fn().mockReturnValue(new Promise(() => {})),
218+
getAuthMetadata: jest.fn().mockRejectedValue(new MatrixError({ errcode: "M_UNRECOGNIZED" }, 404)),
219219
});
220220
jest.clearAllMocks();
221221
jest.spyOn(logger, "error").mockRestore();
@@ -1615,7 +1615,6 @@ describe("<SessionManagerTab />", () => {
16151615
describe("MSC4108 QR code login", () => {
16161616
const settingsValueSpy = jest.spyOn(SettingsStore, "getValue");
16171617
const issuer = "https://issuer.org";
1618-
const openIdConfiguration = mockOpenIdConfiguration(issuer);
16191618

16201619
beforeEach(() => {
16211620
settingsValueSpy.mockClear().mockReturnValue(true);
@@ -1631,16 +1630,16 @@ describe("<SessionManagerTab />", () => {
16311630
enabled: true,
16321631
},
16331632
});
1634-
mockClient.getAuthIssuer.mockResolvedValue({ issuer });
1635-
mockCrypto.exportSecretsBundle = jest.fn();
1636-
fetchMock.mock(`${issuer}/.well-known/openid-configuration`, {
1637-
...openIdConfiguration,
1633+
const delegatedAuthConfig = makeDelegatedAuthConfig(issuer);
1634+
mockClient.getAuthMetadata.mockResolvedValue({
1635+
...delegatedAuthConfig,
16381636
grant_types_supported: [
1639-
...openIdConfiguration.grant_types_supported,
1637+
...delegatedAuthConfig.grant_types_supported,
16401638
"urn:ietf:params:oauth:grant-type:device_code",
16411639
],
16421640
});
1643-
fetchMock.mock(openIdConfiguration.jwks_uri!, {
1641+
mockCrypto.exportSecretsBundle = jest.fn();
1642+
fetchMock.mock(delegatedAuthConfig.jwks_uri!, {
16441643
status: 200,
16451644
headers: {
16461645
"Content-Type": "application/json",

0 commit comments

Comments
 (0)