Skip to content

Commit

Permalink
Update MSC2965 OIDC Discovery implementation (#4064)
Browse files Browse the repository at this point in the history
  • Loading branch information
@t3chguy
t3chguy authored Feb 23, 2024
1 parent be3913e commit a26fc46
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 421 deletions.
6 changes: 2 additions & 4 deletions spec/test-utils/oidc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

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

/**
* Makes a valid OidcClientConfig with minimum valid values
Expand All @@ -26,8 +25,7 @@ export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClien
const metadata = mockOpenIdConfiguration(issuer);

return {
issuer,
account: issuer + "account",
accountManagementEndpoint: issuer + "account",
registrationEndpoint: metadata.registration_endpoint,
authorizationEndpoint: metadata.authorization_endpoint,
tokenEndpoint: metadata.token_endpoint,
Expand Down
99 changes: 1 addition & 98 deletions spec/unit/autodiscovery.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@ 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 { AutoDiscoveryAction, M_AUTHENTICATION } from "../../src";
import { AutoDiscoveryAction } 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
Expand Down Expand Up @@ -409,10 +406,6 @@ describe("AutoDiscovery", function () {
error: null,
base_url: null,
},
"m.authentication": {
state: "IGNORE",
error: OidcError.NotSupported,
},
};

expect(conf).toEqual(expected);
Expand Down Expand Up @@ -450,10 +443,6 @@ describe("AutoDiscovery", function () {
error: null,
base_url: null,
},
"m.authentication": {
state: "IGNORE",
error: OidcError.NotSupported,
},
};

expect(conf).toEqual(expected);
Expand All @@ -476,9 +465,6 @@ describe("AutoDiscovery", function () {
// Note: we also expect this test to trim the trailing slash
base_url: "https://chat.example.org/",
},
"m.authentication": {
invalid: true,
},
});
return Promise.all([
httpBackend.flushAllExpected(),
Expand All @@ -494,10 +480,6 @@ describe("AutoDiscovery", function () {
error: null,
base_url: null,
},
"m.authentication": {
state: "FAIL_ERROR",
error: OidcError.Misconfigured,
},
};

expect(conf).toEqual(expected);
Expand Down Expand Up @@ -728,10 +710,6 @@ describe("AutoDiscovery", function () {
error: null,
base_url: "https://identity.example.org",
},
"m.authentication": {
state: "IGNORE",
error: OidcError.NotSupported,
},
};

expect(conf).toEqual(expected);
Expand Down Expand Up @@ -784,10 +762,6 @@ describe("AutoDiscovery", function () {
"org.example.custom.property": {
cupcakes: "yes",
},
"m.authentication": {
state: "IGNORE",
error: OidcError.NotSupported,
},
};

expect(conf).toEqual(expected);
Expand Down Expand Up @@ -897,75 +871,4 @@ 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: ["v1.5"] });

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,
});
});
});
});
9 changes: 6 additions & 3 deletions spec/unit/oidc/authorize.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ const realSubtleCrypto = crypto.subtleCrypto;

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

Expand All @@ -58,7 +58,10 @@ describe("oidc authorization", () => {
jest.spyOn(logger, "warn");
jest.setSystemTime(now);

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

Object.defineProperty(window, "crypto", {
value: {
Expand Down
17 changes: 6 additions & 11 deletions spec/unit/oidc/register.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ import fetchMockJest from "fetch-mock-jest";

import { OidcError } from "../../../src/oidc/error";
import { OidcRegistrationClientMetadata, registerOidcClient } from "../../../src/oidc/register";
import { makeDelegatedAuthConfig } from "../../test-utils/oidc";

describe("registerOidcClient()", () => {
const issuer = "https://auth.com/";
const registrationEndpoint = "https://auth.com/register";
const clientName = "Element";
const baseUrl = "https://just.testing";
const metadata: OidcRegistrationClientMetadata = {
Expand All @@ -35,25 +35,20 @@ describe("registerOidcClient()", () => {
};
const dynamicClientId = "xyz789";

const delegatedAuthConfig = {
issuer,
registrationEndpoint,
authorizationEndpoint: issuer + "auth",
tokenEndpoint: issuer + "token",
};
const delegatedAuthConfig = makeDelegatedAuthConfig(issuer);
beforeEach(() => {
fetchMockJest.mockClear();
fetchMockJest.resetBehavior();
});

it("should make correct request to register client", async () => {
fetchMockJest.post(registrationEndpoint, {
fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, {
status: 200,
body: JSON.stringify({ client_id: dynamicClientId }),
});
expect(await registerOidcClient(delegatedAuthConfig, metadata)).toEqual(dynamicClientId);
expect(fetchMockJest).toHaveBeenCalledWith(
registrationEndpoint,
delegatedAuthConfig.registrationEndpoint!,
expect.objectContaining({
headers: {
"Accept": "application/json",
Expand All @@ -77,7 +72,7 @@ describe("registerOidcClient()", () => {
});

it("should throw when registration request fails", async () => {
fetchMockJest.post(registrationEndpoint, {
fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, {
status: 500,
});
await expect(() => registerOidcClient(delegatedAuthConfig, metadata)).rejects.toThrow(
Expand All @@ -86,7 +81,7 @@ describe("registerOidcClient()", () => {
});

it("should throw when registration response is invalid", async () => {
fetchMockJest.post(registrationEndpoint, {
fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, {
status: 200,
// no clientId in response
body: "{}",
Expand Down
32 changes: 16 additions & 16 deletions spec/unit/oidc/tokenRefresher.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe("OidcTokenRefresher", () => {
keys: [],
});

fetchMock.post(config.metadata.token_endpoint, {
fetchMock.post(config.tokenEndpoint, {
status: 200,
headers: {
"Content-Type": "application/json",
Expand All @@ -88,7 +88,7 @@ describe("OidcTokenRefresher", () => {
},
{ overwriteRoutes: true },
);
const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await expect(refresher.oidcClientReady).rejects.toThrow();
expect(logger.error).toHaveBeenCalledWith(
"Failed to initialise OIDC client.",
Expand All @@ -98,7 +98,7 @@ describe("OidcTokenRefresher", () => {
});

it("initialises oidc client", async () => {
const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await refresher.oidcClientReady;

// @ts-ignore peek at private property to see we initialised the client correctly
Expand All @@ -114,19 +114,19 @@ describe("OidcTokenRefresher", () => {

describe("doRefreshAccessToken()", () => {
it("should throw when oidcClient has not been initialised", async () => {
const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await expect(refresher.doRefreshAccessToken("token")).rejects.toThrow(
"Cannot get new token before OIDC client is initialised.",
);
});

it("should refresh the tokens", async () => {
const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await refresher.oidcClientReady;

const result = await refresher.doRefreshAccessToken("refresh-token");

expect(fetchMock).toHaveFetched(config.metadata.token_endpoint, {
expect(fetchMock).toHaveFetched(config.tokenEndpoint, {
method: "POST",
});

Expand All @@ -137,7 +137,7 @@ describe("OidcTokenRefresher", () => {
});

it("should persist the new tokens", async () => {
const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await refresher.oidcClientReady;
// spy on our stub
jest.spyOn(refresher, "persistTokens");
Expand All @@ -153,7 +153,7 @@ describe("OidcTokenRefresher", () => {
it("should only have one inflight refresh request at once", async () => {
fetchMock
.postOnce(
config.metadata.token_endpoint,
config.tokenEndpoint,
{
status: 200,
headers: {
Expand All @@ -164,7 +164,7 @@ describe("OidcTokenRefresher", () => {
{ overwriteRoutes: true },
)
.postOnce(
config.metadata.token_endpoint,
config.tokenEndpoint,
{
status: 200,
headers: {
Expand All @@ -175,7 +175,7 @@ describe("OidcTokenRefresher", () => {
{ overwriteRoutes: false },
);

const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await refresher.oidcClientReady;
// reset call counts
fetchMock.resetHistory();
Expand All @@ -188,7 +188,7 @@ describe("OidcTokenRefresher", () => {
const result2 = await first;

// only one call to token endpoint
expect(fetchMock).toHaveFetchedTimes(1, config.metadata.token_endpoint);
expect(fetchMock).toHaveFetchedTimes(1, config.tokenEndpoint);
expect(result1).toEqual({
accessToken: "first-new-access-token",
refreshToken: "first-new-refresh-token",
Expand All @@ -208,7 +208,7 @@ describe("OidcTokenRefresher", () => {

it("should log and rethrow when token refresh fails", async () => {
fetchMock.post(
config.metadata.token_endpoint,
config.tokenEndpoint,
{
status: 503,
headers: {
Expand All @@ -218,7 +218,7 @@ describe("OidcTokenRefresher", () => {
{ overwriteRoutes: true },
);

const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await refresher.oidcClientReady;

await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow();
Expand All @@ -228,7 +228,7 @@ describe("OidcTokenRefresher", () => {
// make sure inflight request is cleared after a failure
fetchMock
.postOnce(
config.metadata.token_endpoint,
config.tokenEndpoint,
{
status: 503,
headers: {
Expand All @@ -238,7 +238,7 @@ describe("OidcTokenRefresher", () => {
{ overwriteRoutes: true },
)
.postOnce(
config.metadata.token_endpoint,
config.tokenEndpoint,
{
status: 200,
headers: {
Expand All @@ -249,7 +249,7 @@ describe("OidcTokenRefresher", () => {
{ overwriteRoutes: false },
);

const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
const refresher = new OidcTokenRefresher(authConfig.issuer, clientId, redirectUri, deviceId, idTokenClaims);
await refresher.oidcClientReady;
// reset call counts
fetchMock.resetHistory();
Expand Down
Loading

0 comments on commit a26fc46

Please sign in to comment.