Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

OIDC: attempt dynamic client registration #11074

Merged
merged 31 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c2a78d1
add delegatedauthentication to validated server config
Jun 7, 2023
f946b85
dynamic client registration functions
Jun 9, 2023
223d3ff
Merge branch 'develop' into kerry/25466/oidc-validate-wk-mauthenticat…
Jun 11, 2023
f83499e
Merge branch 'kerry/25466/oidc-validate-wk-mauthentication-2' into ke…
Jun 11, 2023
4201725
test OP registration functions
Jun 12, 2023
314105d
add stubbed nativeOidc flow setup in Login
Jun 12, 2023
6fe2d35
cover more error cases in Login
Jun 12, 2023
c473f11
Merge branch 'kerry/25468/test-login' into kerry/25468/oidc-client-re…
Jun 12, 2023
19dc425
tidy
Jun 12, 2023
94d5c36
test dynamic client registration in Login
Jun 12, 2023
67f6c2e
comment oidc_static_clients
Jun 12, 2023
8df0929
register oidc inside Login.getFlows
Jun 12, 2023
0d4de13
Merge branch 'develop' into kerry/25466/oidc-validate-wk-mauthenticat…
Jun 12, 2023
1a53ad0
strict fixes
Jun 12, 2023
f9aaa28
remove unused code
Jun 12, 2023
9faa9c8
and imports
Jun 12, 2023
0f76ffa
Merge branch 'kerry/25466/oidc-validate-wk-mauthentication-2' into ke…
Jun 13, 2023
4dfa18d
Merge branch 'develop' into kerry/25468/oidc-client-registration
Jun 13, 2023
e920229
comments
Jun 13, 2023
5389d5c
comments 2
Jun 13, 2023
af63a90
util functions to get static client id
Jun 13, 2023
9c89f41
check static client ids in login flow
Jun 13, 2023
056a713
Merge branch 'kerry/25468/oidc-client-static-reg' into kerry/25468/oi…
Jun 13, 2023
422823e
remove dead code
Jun 14, 2023
8483b2f
Merge branch 'kerry/25468/oidc-client-static-reg' into kerry/25468/oi…
Jun 14, 2023
a76cde9
OidcRegistrationClientMetadata type
Jun 14, 2023
e1e22f3
Merge branch 'develop' into kerry/25468/oidc-client-static-reg
Jun 14, 2023
f4a2ff2
Merge branch 'kerry/25468/oidc-client-static-reg' into kerry/25468/oi…
Jun 14, 2023
71c4b59
use registerClient from js-sdk
Jun 16, 2023
40bfc87
use OidcError from js-sdk
Jun 16, 2023
c3143f6
Merge branch 'develop' into kerry/25468/oidc-client-registration
Jun 22, 2023
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: 0 additions & 1 deletion src/components/structures/auth/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ _td("Invalid identity server discovery response");
_td("Invalid base_url for m.identity_server");
_td("Identity server URL does not appear to be a valid identity server");
_td("General failure");

interface IProps {
serverConfig: ValidatedServerConfig;
// If true, the component will consider itself busy.
Expand Down
24 changes: 0 additions & 24 deletions src/utils/oidc/error.ts

This file was deleted.

12 changes: 5 additions & 7 deletions src/utils/oidc/registerClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ limitations under the License.
*/

import { logger } from "matrix-js-sdk/src/logger";
import { registerOidcClient } from "matrix-js-sdk/src/oidc/register";

import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig";
import { OidcClientError } from "./error";

/**
* Get the statically configured clientId for the issuer
Expand All @@ -34,6 +34,7 @@ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record<string
/**
* Get the clientId for an OIDC OP
* Checks statically configured clientIds first
* Then attempts dynamic registration with the OP
* @param delegatedAuthConfig Auth config from ValidatedServerConfig
* @param clientName Client name to register with the OP, eg 'Element'
* @param baseUrl URL of the home page of the Client, eg 'https://app.element.io/'
Expand All @@ -44,17 +45,14 @@ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record<string
export const getOidcClientId = async (
delegatedAuthConfig: ValidatedDelegatedAuthConfig,
// these are used in the following PR
_clientName: string,
_baseUrl: string,
clientName: string,
baseUrl: string,
staticOidcClients?: Record<string, string>,
): Promise<string> => {
const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients);
if (staticClientId) {
logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`);
return staticClientId;
}

// TODO attempt dynamic registration
logger.error("Dynamic registration not yet implemented.");
throw new Error(OidcClientError.DynamicRegistrationNotSupported);
return await registerOidcClient(delegatedAuthConfig, clientName, baseUrl);
};
23 changes: 15 additions & 8 deletions test/components/structures/auth/Login-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import fetchMock from "fetch-mock-jest";
import { DELEGATED_OIDC_COMPATIBILITY, IdentityProviderBrand } from "matrix-js-sdk/src/@types/auth";
import { logger } from "matrix-js-sdk/src/logger";
import { createClient, MatrixClient } from "matrix-js-sdk/src/matrix";
import { OidcError } from "matrix-js-sdk/src/oidc/error";

import SdkConfig from "../../../../src/SdkConfig";
import { mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../test-utils";
Expand All @@ -30,7 +31,6 @@ import SettingsStore from "../../../../src/settings/SettingsStore";
import { Features } from "../../../../src/settings/Settings";
import { ValidatedDelegatedAuthConfig } from "../../../../src/utils/ValidatedServerConfig";
import * as registerClientUtils from "../../../../src/utils/oidc/registerClient";
import { OidcClientError } from "../../../../src/utils/oidc/error";

jest.mock("matrix-js-sdk/src/matrix");

Expand Down Expand Up @@ -365,6 +365,8 @@ describe("Login", function () {

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

// didn't try to register
expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint);
// continued with normal setup
expect(mockClient.loginFlows).toHaveBeenCalled();
// normal password login rendered
Expand All @@ -374,10 +376,13 @@ describe("Login", function () {
it("should attempt to register oidc client", async () => {
// dont mock, spy so we can check config values were correctly passed
jest.spyOn(registerClientUtils, "getOidcClientId");
fetchMock.post(delegatedAuth.registrationEndpoint, { status: 500 });
getComponent(hsUrl, isUrl, delegatedAuth);

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

// tried to register
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object));
// called with values from config
expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith(
delegatedAuth,
Expand All @@ -387,12 +392,15 @@ describe("Login", function () {
);
});

it("should fallback to normal login when client does not have static clientId", async () => {
it("should fallback to normal login when client registration fails", async () => {
fetchMock.post(delegatedAuth.registrationEndpoint, { status: 500 });
getComponent(hsUrl, isUrl, delegatedAuth);

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

expect(logger.error).toHaveBeenCalledWith(new Error(OidcClientError.DynamicRegistrationNotSupported));
// tried to register
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object));
expect(logger.error).toHaveBeenCalledWith(new Error(OidcError.DynamicRegistrationFailed));

// continued with normal setup
expect(mockClient.loginFlows).toHaveBeenCalled();
Expand All @@ -402,11 +410,8 @@ describe("Login", function () {

// short term during active development, UI will be added in next PRs
it("should show error when oidc native flow is correctly configured but not supported by UI", async () => {
const delegatedAuthWithStaticClientId = {
...delegatedAuth,
issuer: "https://staticallyregisteredissuer.org/",
};
getComponent(hsUrl, isUrl, delegatedAuthWithStaticClientId);
fetchMock.post(delegatedAuth.registrationEndpoint, { client_id: "abc123" });
getComponent(hsUrl, isUrl, delegatedAuth);

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

Expand Down Expand Up @@ -439,6 +444,8 @@ describe("Login", function () {

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

// didn't try to register
expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint);
// continued with normal setup
expect(mockClient.loginFlows).toHaveBeenCalled();
// oidc-aware 'continue' button displayed
Expand Down
47 changes: 41 additions & 6 deletions test/utils/oidc/registerClient-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ limitations under the License.
*/

import fetchMockJest from "fetch-mock-jest";
import { OidcError } from "matrix-js-sdk/src/oidc/error";

import { OidcClientError } from "../../../src/utils/oidc/error";
import { getOidcClientId } from "../../../src/utils/oidc/registerClient";

describe("getOidcClientId()", () => {
Expand Down Expand Up @@ -56,7 +56,7 @@ describe("getOidcClientId()", () => {
};
expect(
async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl, staticOidcClients),
).rejects.toThrow(OidcClientError.DynamicRegistrationNotSupported);
).rejects.toThrow(OidcError.DynamicRegistrationNotSupported);
// didn't try to register
expect(fetchMockJest).toHaveFetchedTimes(0);
});
Expand All @@ -67,20 +67,55 @@ describe("getOidcClientId()", () => {
registrationEndpoint: undefined,
};
expect(async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl)).rejects.toThrow(
OidcClientError.DynamicRegistrationNotSupported,
OidcError.DynamicRegistrationNotSupported,
);
// didn't try to register
expect(fetchMockJest).toHaveFetchedTimes(0);
});

it("should throw while dynamic registration is not implemented", async () => {
it("should make correct request to register client", async () => {
fetchMockJest.post(registrationEndpoint, {
status: 200,
body: JSON.stringify({ client_id: dynamicClientId }),
});
expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).toEqual(dynamicClientId);
// didn't try to register
expect(fetchMockJest).toHaveBeenCalledWith(registrationEndpoint, {
headers: {
"Accept": "application/json",
"Content-Type": "application/json",
},
method: "POST",
body: JSON.stringify({
client_name: clientName,
client_uri: baseUrl,
response_types: ["code"],
grant_types: ["authorization_code", "refresh_token"],
redirect_uris: [baseUrl],
id_token_signed_response_alg: "RS256",
token_endpoint_auth_method: "none",
application_type: "web",
}),
});
});

expect(async () => await getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
OidcClientError.DynamicRegistrationNotSupported,
it("should throw when registration request fails", async () => {
fetchMockJest.post(registrationEndpoint, {
status: 500,
});
expect(() => getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
OidcError.DynamicRegistrationFailed,
);
});

it("should throw when registration response is invalid", async () => {
fetchMockJest.post(registrationEndpoint, {
status: 200,
// no clientId in response
body: "{}",
});
expect(() => getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
OidcError.DynamicRegistrationInvalid,
);
});
});
Loading