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

Commit

Permalink
Update MSC2965 OIDC Discovery implementation (#12245)
Browse files Browse the repository at this point in the history
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
  • Loading branch information
t3chguy and richvdh authored Feb 23, 2024
1 parent 729eca4 commit 7b1e8e3
Show file tree
Hide file tree
Showing 19 changed files with 349 additions and 299 deletions.
2 changes: 1 addition & 1 deletion src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis
throw new Error("Expected deviceId in user credentials.");
}
const tokenRefresher = new TokenRefresher(
{ issuer: tokenIssuer },
tokenIssuer,
clientId,
redirectUri,
deviceId,
Expand Down
8 changes: 4 additions & 4 deletions src/components/views/dialogs/ServerPickerDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ export default class ServerPickerDialog extends React.PureComponent<IProps, ISta
this.setState({ otherHomeserver: ev.target.value });
};

// TODO: Do we want to support .well-known lookups here?
// If for some reason someone enters "matrix.org" for a URL, we could do a lookup to
// find their homeserver without demanding they use "https://matrix.org"
private validate = withValidation<this, { error?: string }>({
deriveData: async ({ value }): Promise<{ error?: string }> => {
let hsUrl = (value ?? "").trim(); // trim to account for random whitespace
Expand All @@ -91,7 +88,10 @@ export default class ServerPickerDialog extends React.PureComponent<IProps, ISta
if (!hsUrl.includes("://")) {
try {
const discoveryResult = await AutoDiscovery.findClientConfig(hsUrl);
this.validatedConf = AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(hsUrl, discoveryResult);
this.validatedConf = await AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(
hsUrl,
discoveryResult,
);
return {}; // we have a validated config, we don't need to try the other paths
} catch (e) {
logger.error(`Attempted ${hsUrl} as a server_name but it failed`, e);
Expand Down
36 changes: 22 additions & 14 deletions src/stores/oidc/OidcClientStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,23 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { IDelegatedAuthConfig, MatrixClient, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
import { discoverAndValidateAuthenticationConfig } from "matrix-js-sdk/src/oidc/discovery";
import { MatrixClient, discoverAndValidateOIDCIssuerWellKnown } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import { OidcClient } from "oidc-client-ts";

import { getStoredOidcTokenIssuer, getStoredOidcClientId } from "../../utils/oidc/persistOidcSettings";
import { getDelegatedAuthAccountUrl } from "../../utils/oidc/getDelegatedAuthAccountUrl";
import PlatformPeg from "../../PlatformPeg";

/**
* @experimental
* Stores information about configured OIDC provider
*
* In OIDC Native mode the client is registered with OIDC directly and maintains an OIDC token.
*
* In OIDC Aware mode, the client is aware that the Server is using OIDC, but is using the standard Matrix APIs for most things.
* (Notable exceptions are account management, where a link to the account management endpoint will be provided instead.)
*
* Otherwise, the store is not operating. Auth is then in Legacy mode and everything uses normal Matrix APIs.
*/
export class OidcClientStore {
private oidcClient?: OidcClient;
Expand All @@ -47,8 +52,16 @@ export class OidcClientStore {
if (this.authenticatedIssuer) {
await this.getOidcClient();
} else {
const wellKnown = await this.matrixClient.waitForClientWellKnown();
this._accountManagementEndpoint = getDelegatedAuthAccountUrl(wellKnown);
// We are not in OIDC Native mode, as we have no locally stored issuer. Check if the server delegates auth to OIDC.
try {
const authIssuer = await this.matrixClient.getAuthIssuer();
const { accountManagementEndpoint, metadata } = await discoverAndValidateOIDCIssuerWellKnown(
authIssuer.issuer,
);
this._accountManagementEndpoint = accountManagementEndpoint ?? metadata.issuer;
} catch (e) {
console.log("Auth issuer not found", e);
}
}
}

Expand Down Expand Up @@ -127,23 +140,18 @@ export class OidcClientStore {
* @returns promise that resolves when initialising OidcClient succeeds or fails
*/
private async initOidcClient(): Promise<void> {
const wellKnown = await this.matrixClient.waitForClientWellKnown();
if (!wellKnown && !this.authenticatedIssuer) {
if (!this.authenticatedIssuer) {
logger.error("Cannot initialise OIDC client without issuer.");
return;
}
const delegatedAuthConfig =
(wellKnown && M_AUTHENTICATION.findIn<IDelegatedAuthConfig>(wellKnown)) ?? undefined;

try {
const clientId = getStoredOidcClientId();
const { account, metadata, signingKeys } = await discoverAndValidateAuthenticationConfig(
// if HS has valid delegated auth config in .well-known, use it
// otherwise fallback to the known issuer
delegatedAuthConfig ?? { issuer: this.authenticatedIssuer! },
const { accountManagementEndpoint, metadata, signingKeys } = await discoverAndValidateOIDCIssuerWellKnown(
this.authenticatedIssuer,
);
// if no account endpoint is configured default to the issuer
this._accountManagementEndpoint = account ?? metadata.issuer;
this._accountManagementEndpoint = accountManagementEndpoint ?? metadata.issuer;
this.oidcClient = new OidcClient({
...metadata,
authority: metadata.issuer,
Expand Down
44 changes: 20 additions & 24 deletions src/utils/AutoDiscoveryUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import {
AutoDiscovery,
AutoDiscoveryError,
ClientConfig,
OidcClientConfig,
M_AUTHENTICATION,
discoverAndValidateOIDCIssuerWellKnown,
IClientWellKnown,
MatrixClient,
MatrixError,
OidcClientConfig,
} from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";

Expand Down Expand Up @@ -217,12 +219,12 @@ export default class AutoDiscoveryUtils {
* @param {boolean} isSynthetic If true, then the discoveryResult was synthesised locally.
* @returns {Promise<ValidatedServerConfig>} Resolves to the validated configuration.
*/
public static buildValidatedConfigFromDiscovery(
public static async buildValidatedConfigFromDiscovery(
serverName?: string,
discoveryResult?: ClientConfig,
syntaxOnly = false,
isSynthetic = false,
): ValidatedServerConfig {
): Promise<ValidatedServerConfig> {
if (!discoveryResult?.["m.homeserver"]) {
// This shouldn't happen without major misconfiguration, so we'll log a bit of information
// in the log so we can find this bit of code but otherwise tell the user "it broke".
Expand Down Expand Up @@ -293,26 +295,20 @@ export default class AutoDiscoveryUtils {
throw new UserFriendlyError("auth|autodiscovery_unexpected_error_hs");
}

// This isn't inherently auto-discovery but used to be in an earlier incarnation of the MSC,
// and shuttling the data together makes a lot of sense
let delegatedAuthentication: OidcClientConfig | undefined;
if (discoveryResult[M_AUTHENTICATION.stable!]?.state === AutoDiscovery.SUCCESS) {
const {
authorizationEndpoint,
registrationEndpoint,
tokenEndpoint,
account,
issuer,
metadata,
signingKeys,
} = discoveryResult[M_AUTHENTICATION.stable!] as OidcClientConfig;
delegatedAuthentication = Object.freeze({
authorizationEndpoint,
registrationEndpoint,
tokenEndpoint,
account,
issuer,
metadata,
signingKeys,
});
let delegatedAuthenticationError: Error | undefined;
try {
const tempClient = new MatrixClient({ baseUrl: preferredHomeserverUrl });
const { issuer } = await tempClient.getAuthIssuer();
delegatedAuthentication = await discoverAndValidateOIDCIssuerWellKnown(issuer);
} catch (e) {
if (e instanceof MatrixError && e.httpStatus === 404 && e.errcode === "M_UNRECOGNIZED") {
// 404 M_UNRECOGNIZED means the server does not support OIDC
} else {
delegatedAuthenticationError = e as Error;
}
}

return {
Expand All @@ -321,7 +317,7 @@ export default class AutoDiscoveryUtils {
hsNameIsDifferent: url.hostname !== preferredHomeserverName,
isUrl: preferredIdentityUrl,
isDefault: false,
warning: hsResult.error,
warning: hsResult.error ?? delegatedAuthenticationError ?? null,
isNameResolvable: !isSynthetic,
delegatedAuthentication,
} as ValidatedServerConfig;
Expand Down
11 changes: 4 additions & 7 deletions src/utils/ValidatedServerConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { OidcClientConfig, IDelegatedAuthConfig } from "matrix-js-sdk/src/matrix";
import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate";

export type ValidatedDelegatedAuthConfig = IDelegatedAuthConfig & ValidatedIssuerConfig;
import { OidcClientConfig } from "matrix-js-sdk/src/matrix";

export interface ValidatedServerConfig {
hsUrl: string;
Expand All @@ -34,9 +31,9 @@ export interface ValidatedServerConfig {

/**
* Config related to delegated authentication
* Included when delegated auth is configured and valid, otherwise undefined
* From homeserver .well-known m.authentication, and issuer's .well-known/openid-configuration
* Used for OIDC native flow authentication
* Included when delegated auth is configured and valid, otherwise undefined.
* From issuer's .well-known/openid-configuration.
* Used for OIDC native flow authentication.
*/
delegatedAuthentication?: OidcClientConfig;
}
6 changes: 3 additions & 3 deletions src/utils/oidc/TokenRefresher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { IDelegatedAuthConfig, OidcTokenRefresher, AccessTokens } from "matrix-js-sdk/src/matrix";
import { OidcTokenRefresher, AccessTokens } from "matrix-js-sdk/src/matrix";
import { IdTokenClaims } from "oidc-client-ts";

import PlatformPeg from "../../PlatformPeg";
Expand All @@ -28,14 +28,14 @@ export class TokenRefresher extends OidcTokenRefresher {
private readonly deviceId!: string;

public constructor(
authConfig: IDelegatedAuthConfig,
issuer: string,
clientId: string,
redirectUri: string,
deviceId: string,
idTokenClaims: IdTokenClaims,
private readonly userId: string,
) {
super(authConfig, clientId, deviceId, redirectUri, idTokenClaims);
super(issuer, clientId, deviceId, redirectUri, idTokenClaims);
this.deviceId = deviceId;
}

Expand Down
27 changes: 0 additions & 27 deletions src/utils/oidc/getDelegatedAuthAccountUrl.ts

This file was deleted.

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

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

import { IConfigOptions } from "../../IConfigOptions";
import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig";
import PlatformPeg from "../../PlatformPeg";

/**
Expand Down Expand Up @@ -46,12 +45,12 @@ const getStaticOidcClientId = (
* @throws if no clientId is found
*/
export const getOidcClientId = async (
delegatedAuthConfig: ValidatedDelegatedAuthConfig,
delegatedAuthConfig: OidcClientConfig,
staticOidcClients?: IConfigOptions["oidc_static_clients"],
): Promise<string> => {
const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients);
const staticClientId = getStaticOidcClientId(delegatedAuthConfig.metadata.issuer, staticOidcClients);
if (staticClientId) {
logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`);
logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.metadata.issuer}`);
return staticClientId;
}
return await registerOidcClient(delegatedAuthConfig, await PlatformPeg.get()!.getOidcClientMetadata());
Expand Down
22 changes: 11 additions & 11 deletions test/Lifecycle-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,10 @@ describe("Lifecycle", () => {

beforeAll(() => {
fetchMock.get(
`${delegatedAuthConfig.issuer}.well-known/openid-configuration`,
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
delegatedAuthConfig.metadata,
);
fetchMock.get(`${delegatedAuthConfig.issuer}jwks`, {
fetchMock.get(`${delegatedAuthConfig.metadata.issuer}jwks`, {
status: 200,
headers: {
"Content-Type": "application/json",
Expand All @@ -696,12 +696,6 @@ describe("Lifecycle", () => {
});

beforeEach(() => {
// mock oidc config for oidc client initialisation
mockClient.waitForClientWellKnown.mockResolvedValue({
"m.authentication": {
issuer: issuer,
},
});
initSessionStorageMock();
// set values in session storage as they would be after a successful oidc authentication
persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims);
Expand All @@ -711,7 +705,9 @@ describe("Lifecycle", () => {
await setLoggedIn(credentials);

// didn't try to initialise token refresher
expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`);
expect(fetchMock).not.toHaveFetched(
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
);
});

it("should not try to create a token refresher without a deviceId", async () => {
Expand All @@ -722,7 +718,9 @@ describe("Lifecycle", () => {
});

// didn't try to initialise token refresher
expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`);
expect(fetchMock).not.toHaveFetched(
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
);
});

it("should not try to create a token refresher without an issuer in session storage", async () => {
Expand All @@ -738,7 +736,9 @@ describe("Lifecycle", () => {
});

// didn't try to initialise token refresher
expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`);
expect(fetchMock).not.toHaveFetched(
`${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`,
);
});

it("should create a client with a tokenRefreshFunction", async () => {
Expand Down
5 changes: 5 additions & 0 deletions test/components/structures/MatrixChat-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,11 @@ describe("<MatrixChat />", () => {
unstable_features: {},
versions: SERVER_SUPPORTED_MATRIX_VERSIONS,
});
fetchMock.catch({
status: 404,
body: '{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}',
headers: { "content-type": "application/json" },
});

jest.spyOn(StorageManager, "idbLoad").mockReset();
jest.spyOn(StorageManager, "idbSave").mockResolvedValue(undefined);
Expand Down
Loading

0 comments on commit 7b1e8e3

Please sign in to comment.