PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Improve client metadata used for OIDC dynamic registration (#12257)

Browse source
This commit is contained in:
Michael Telatynski 2024-02-16 14:43:58 +00:00 committed by GitHub
parent e8ce9cb360
commit cd8679c172
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 80 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/@types/common.ts
View file

@ -16,6 +16,8 @@ limitations under the License.
import { JSXElementConstructor } from "react";
export type { NonEmptyArray } from "matrix-js-sdk/src/matrix";
// Based on https://stackoverflow.com/a/53229857/3532235
export type Without<T, U> = { [P in Exclude<keyof T, keyof U>]?: never };
export type XOR<T, U> = T | U extends object ? (Without<T, U> & U) | (Without<U, T> & T) : T | U;
@ -38,8 +40,6 @@ export type KeysStartingWith<Input extends object, Str extends string> = {
[P in keyof Input]: P extends `${Str}${infer _X}` ? P : never; // we don't use _X
}[keyof Input];
export type NonEmptyArray<T> = [T, ...T[]];
export type Defaultize<P, D> = P extends any
? string extends keyof P
? P

39
src/BasePlatform.ts
View file

@ -17,7 +17,14 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { MatrixClient, MatrixEvent, Room, SSOAction, encodeUnpaddedBase64 } from "matrix-js-sdk/src/matrix";
import {
MatrixClient,
MatrixEvent,
Room,
SSOAction,
encodeUnpaddedBase64,
OidcRegistrationClientMetadata,
} from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import dis from "./dispatcher/dispatcher";
@ -30,6 +37,7 @@ import { MatrixClientPeg } from "./MatrixClientPeg";
import { idbLoad, idbSave, idbDelete } from "./utils/StorageManager";
import { ViewRoomPayload } from "./dispatcher/payloads/ViewRoomPayload";
import { IConfigOptions } from "./IConfigOptions";
import SdkConfig from "./SdkConfig";
export const SSO_HOMESERVER_URL_KEY = "mx_sso_hs_url";
export const SSO_ID_SERVER_URL_KEY = "mx_sso_is_url";
@ -426,7 +434,7 @@ export default abstract class BasePlatform {
/**
* Delete a previously stored pickle key from storage.
* @param {string} userId the user ID for the user that the pickle key is for.
* @param {string} userId the device ID that the pickle key is for.
* @param {string} deviceId the device ID that the pickle key is for.
*/
public async destroyPickleKey(userId: string, deviceId: string): Promise<void> {
try {
@ -443,4 +451,31 @@ export default abstract class BasePlatform {
window.sessionStorage.clear();
window.localStorage.clear();
}
/**
* Base URL to use when generating external links for this client, for platforms e.g. Desktop this will be a different instance
*/
public get baseUrl(): string {
return window.location.origin + window.location.pathname;
}
/**
* Metadata to use for dynamic OIDC client registrations
*/
public async getOidcClientMetadata(): Promise<OidcRegistrationClientMetadata> {
const config = SdkConfig.get();
return {
clientName: config.brand,
clientUri: this.baseUrl,
redirectUris: [this.getSSOCallbackUrl().href],
logoUri: new URL("vector-icons/1024.png", this.baseUrl).href,
applicationType: "web",
// XXX: We break the spec by not consistently supplying these required fields
// contacts: [],
// @ts-ignore
tosUri: config.terms_and_conditions_links?.[0]?.url,
// @ts-ignore
policyUri: config.privacy_policy_url,
};
}
}

7
src/Login.ts
View file

@ -120,7 +120,6 @@ export default class Login {
try {
const oidcFlow = await tryInitOidcNativeFlow(
this.delegatedAuthentication,
SdkConfig.get().brand,
SdkConfig.get().oidc_static_clients,
isRegistration,
);
@ -223,7 +222,6 @@ export interface OidcNativeFlow extends ILoginFlow {
* results.
*
* @param delegatedAuthConfig Auth config from ValidatedServerConfig
* @param clientName Client name to register with the OP, eg 'Element', used during client registration with OP
* @param staticOidcClientIds static client config from config.json, used during client registration with OP
* @param isRegistration true when we are attempting registration
* @returns Promise<OidcNativeFlow> when oidc native authentication flow is supported and correctly configured
@ -231,15 +229,14 @@ export interface OidcNativeFlow extends ILoginFlow {
*/
const tryInitOidcNativeFlow = async (
delegatedAuthConfig: OidcClientConfig,
brand: string,
oidcStaticClients?: IConfigOptions["oidc_static_clients"],
staticOidcClientIds?: IConfigOptions["oidc_static_clients"],
isRegistration?: boolean,
): Promise<OidcNativeFlow> => {
// if registration is not supported, bail before attempting to get the clientId
if (isRegistration && !isUserRegistrationSupported(delegatedAuthConfig)) {
throw new Error("Registration is not supported by OP");
}
const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClients);
const clientId = await getOidcClientId(delegatedAuthConfig, staticOidcClientIds);
const flow = {
type: "oidcNativeFlow",

7
src/utils/oidc/registerClient.ts
View file

@ -19,6 +19,7 @@ import { registerOidcClient } from "matrix-js-sdk/src/oidc/register";
import { IConfigOptions } from "../../IConfigOptions";
import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig";
import PlatformPeg from "../../PlatformPeg";
/**
* Get the statically configured clientId for the issuer
@ -40,16 +41,12 @@ const getStaticOidcClientId = (
* 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/'
* @param staticOidcClients static client config from config.json
* @returns Promise<string> resolves with clientId
* @throws if no clientId is found
*/
export const getOidcClientId = async (
delegatedAuthConfig: ValidatedDelegatedAuthConfig,
clientName: string,
baseUrl: string,
staticOidcClients?: IConfigOptions["oidc_static_clients"],
): Promise<string> => {
const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients);
@ -57,5 +54,5 @@ export const getOidcClientId = async (
logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`);
return staticClientId;
}
return await registerOidcClient(delegatedAuthConfig, clientName, baseUrl);
return await registerOidcClient(delegatedAuthConfig, await PlatformPeg.get()!.getOidcClientMetadata());
};

7
test/components/structures/auth/Login-test.tsx
View file

@ -415,12 +415,7 @@ describe("Login", function () {
// tried to register
expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object));
// called with values from config
expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith(
delegatedAuth,
"test-brand",
"http://localhost",
oidcStaticClientsConfig,
);
expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith(delegatedAuth, oidcStaticClientsConfig);
});
it("should fallback to normal login when client registration fails", async () => {

2
test/test-utils/platform.ts
View file

@ -22,7 +22,7 @@ import PlatformPeg from "../../src/PlatformPeg";
// doesn't implement abstract
// @ts-ignore
class MockPlatform extends BasePlatform {
constructor(platformMocks: Partial<Record<MethodLikeKeys<BasePlatform>, unknown>>) {
constructor(platformMocks: Partial<Record<keyof BasePlatform, unknown>>) {
super();
Object.assign(this, platformMocks);
}

55
test/utils/oidc/registerClient-test.ts
View file

@ -19,6 +19,8 @@ import { OidcError } from "matrix-js-sdk/src/oidc/error";
import { getOidcClientId } from "../../../src/utils/oidc/registerClient";
import { ValidatedDelegatedAuthConfig } from "../../../src/utils/ValidatedServerConfig";
import { mockPlatformPeg } from "../../test-utils";
import PlatformPeg from "../../../src/PlatformPeg";
describe("getOidcClientId()", () => {
const issuer = "https://auth.com/";
@ -41,10 +43,21 @@ describe("getOidcClientId()", () => {
beforeEach(() => {
fetchMockJest.mockClear();
fetchMockJest.resetBehavior();
mockPlatformPeg();
Object.defineProperty(PlatformPeg.get(), "baseUrl", {
get(): string {
return baseUrl;
},
});
Object.defineProperty(PlatformPeg.get(), "getSSOCallbackUrl", {
value: () => ({
href: baseUrl,
}),
});
});
it("should return static clientId when configured", async () => {
expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl, staticOidcClients)).toEqual("abc123");
expect(await getOidcClientId(delegatedAuthConfig, staticOidcClients)).toEqual("abc123");
// didn't try to register
expect(fetchMockJest).toHaveFetchedTimes(0);
});
@ -55,9 +68,9 @@ describe("getOidcClientId()", () => {
issuer: "https://issuerWithoutStaticClientId.org/",
registrationEndpoint: undefined,
};
await expect(
getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl, staticOidcClients),
).rejects.toThrow(OidcError.DynamicRegistrationNotSupported);
await expect(getOidcClientId(authConfigWithoutRegistration, staticOidcClients)).rejects.toThrow(
OidcError.DynamicRegistrationNotSupported,
);
// didn't try to register
expect(fetchMockJest).toHaveFetchedTimes(0);
});
@ -67,7 +80,7 @@ describe("getOidcClientId()", () => {
...delegatedAuthConfig,
registrationEndpoint: undefined,
};
await expect(getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl)).rejects.toThrow(
await expect(getOidcClientId(authConfigWithoutRegistration)).rejects.toThrow(
OidcError.DynamicRegistrationNotSupported,
);
// didn't try to register
@ -79,15 +92,20 @@ describe("getOidcClientId()", () => {
status: 200,
body: JSON.stringify({ client_id: dynamicClientId }),
});
expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).toEqual(dynamicClientId);
expect(await getOidcClientId(delegatedAuthConfig)).toEqual(dynamicClientId);
// didn't try to register
expect(fetchMockJest).toHaveBeenCalledWith(registrationEndpoint, {
headers: {
"Accept": "application/json",
"Content-Type": "application/json",
},
method: "POST",
body: JSON.stringify({
expect(fetchMockJest).toHaveBeenCalledWith(
registrationEndpoint,
expect.objectContaining({
headers: {
"Accept": "application/json",
"Content-Type": "application/json",
},
method: "POST",
}),
);
expect(JSON.parse(fetchMockJest.mock.calls[0][1]!.body as string)).toEqual(
expect.objectContaining({
client_name: clientName,
client_uri: baseUrl,
response_types: ["code"],
@ -96,17 +114,16 @@ describe("getOidcClientId()", () => {
id_token_signed_response_alg: "RS256",
token_endpoint_auth_method: "none",
application_type: "web",
logo_uri: `${baseUrl}/vector-icons/1024.png`,
}),
});
);
});
it("should throw when registration request fails", async () => {
fetchMockJest.post(registrationEndpoint, {
status: 500,
});
await expect(getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
OidcError.DynamicRegistrationFailed,
);
await expect(getOidcClientId(delegatedAuthConfig)).rejects.toThrow(OidcError.DynamicRegistrationFailed);
});
it("should throw when registration response is invalid", async () => {
@ -115,8 +132,6 @@ describe("getOidcClientId()", () => {
// no clientId in response
body: "{}",
});
await expect(getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
OidcError.DynamicRegistrationInvalid,
);
await expect(getOidcClientId(delegatedAuthConfig)).rejects.toThrow(OidcError.DynamicRegistrationInvalid);
});
});