Skip to content

Commit c66850e

Browse files
author
Kerry
authored
OIDC: Validate m.authentication configuration (#3419)
* validate m.authentication, fetch issuer wellknown * move validation functions into separate file * test validateWellKnownAuthentication * test validateOIDCIssuerWellKnown * add authentication cases to autodiscovery tests * test invalid authentication config on wk * improve types * test case for account:false * use hasOwnProperty in validateWellKnownAuthentication * comments * make registration_endpoint optional
1 parent 2766146 commit c66850e

File tree

5 files changed

+458
-10
lines changed

5 files changed

+458
-10
lines changed

spec/unit/autodiscovery.spec.ts

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ limitations under the License.
1818
import MockHttpBackend from "matrix-mock-request";
1919

2020
import { AutoDiscovery } from "../../src/autodiscovery";
21+
import { OidcDiscoveryError } from "../../src/oidc/validate";
2122

2223
describe("AutoDiscovery", function () {
2324
const getHttpBackend = (): MockHttpBackend => {
@@ -368,7 +369,7 @@ describe("AutoDiscovery", function () {
368369
},
369370
);
370371

371-
it("should return SUCCESS when .well-known has a verifiably accurate base_url for " + "m.homeserver", function () {
372+
it("should return SUCCESS when .well-known has a verifiably accurate base_url for m.homeserver", function () {
372373
const httpBackend = getHttpBackend();
373374
httpBackend
374375
.when("GET", "/_matrix/client/versions")
@@ -397,6 +398,10 @@ describe("AutoDiscovery", function () {
397398
error: null,
398399
base_url: null,
399400
},
401+
"m.authentication": {
402+
state: "IGNORE",
403+
error: OidcDiscoveryError.NotSupported,
404+
},
400405
};
401406

402407
expect(conf).toEqual(expected);
@@ -434,6 +439,54 @@ describe("AutoDiscovery", function () {
434439
error: null,
435440
base_url: null,
436441
},
442+
"m.authentication": {
443+
state: "IGNORE",
444+
error: OidcDiscoveryError.NotSupported,
445+
},
446+
};
447+
448+
expect(conf).toEqual(expected);
449+
}),
450+
]);
451+
});
452+
453+
it("should return SUCCESS with authentication error when authentication config is invalid", function () {
454+
const httpBackend = getHttpBackend();
455+
httpBackend
456+
.when("GET", "/_matrix/client/versions")
457+
.check((req) => {
458+
expect(req.path).toEqual("https://chat.example.org/_matrix/client/versions");
459+
})
460+
.respond(200, {
461+
versions: ["r0.0.1"],
462+
});
463+
httpBackend.when("GET", "/.well-known/matrix/client").respond(200, {
464+
"m.homeserver": {
465+
// Note: we also expect this test to trim the trailing slash
466+
base_url: "https://chat.example.org/",
467+
},
468+
"m.authentication": {
469+
invalid: true,
470+
},
471+
});
472+
return Promise.all([
473+
httpBackend.flushAllExpected(),
474+
AutoDiscovery.findClientConfig("example.org").then((conf) => {
475+
const expected = {
476+
"m.homeserver": {
477+
state: "SUCCESS",
478+
error: null,
479+
base_url: "https://chat.example.org",
480+
},
481+
"m.identity_server": {
482+
state: "PROMPT",
483+
error: null,
484+
base_url: null,
485+
},
486+
"m.authentication": {
487+
state: "FAIL_ERROR",
488+
error: OidcDiscoveryError.Misconfigured,
489+
},
437490
};
438491

439492
expect(conf).toEqual(expected);
@@ -625,7 +678,7 @@ describe("AutoDiscovery", function () {
625678
},
626679
);
627680

628-
it("should return SUCCESS when the identity server configuration is " + "verifiably accurate", function () {
681+
it("should return SUCCESS when the identity server configuration is verifiably accurate", function () {
629682
const httpBackend = getHttpBackend();
630683
httpBackend
631684
.when("GET", "/_matrix/client/versions")
@@ -664,14 +717,18 @@ describe("AutoDiscovery", function () {
664717
error: null,
665718
base_url: "https://identity.example.org",
666719
},
720+
"m.authentication": {
721+
state: "IGNORE",
722+
error: OidcDiscoveryError.NotSupported,
723+
},
667724
};
668725

669726
expect(conf).toEqual(expected);
670727
}),
671728
]);
672729
});
673730

674-
it("should return SUCCESS and preserve non-standard keys from the " + ".well-known response", function () {
731+
it("should return SUCCESS and preserve non-standard keys from the .well-known response", function () {
675732
const httpBackend = getHttpBackend();
676733
httpBackend
677734
.when("GET", "/_matrix/client/versions")
@@ -716,6 +773,10 @@ describe("AutoDiscovery", function () {
716773
"org.example.custom.property": {
717774
cupcakes: "yes",
718775
},
776+
"m.authentication": {
777+
state: "IGNORE",
778+
error: OidcDiscoveryError.NotSupported,
779+
},
719780
};
720781

721782
expect(conf).toEqual(expected);

spec/unit/oidc/validate.spec.ts

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
/*
2+
Copyright 2023 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import { M_AUTHENTICATION } from "../../../src";
18+
import { logger } from "../../../src/logger";
19+
import {
20+
OidcDiscoveryError,
21+
validateOIDCIssuerWellKnown,
22+
validateWellKnownAuthentication,
23+
} from "../../../src/oidc/validate";
24+
25+
describe("validateWellKnownAuthentication()", () => {
26+
const baseWk = {
27+
"m.homeserver": {
28+
base_url: "https://hs.org",
29+
},
30+
};
31+
it("should throw not supported error when wellKnown has no m.authentication section", () => {
32+
expect(() => validateWellKnownAuthentication(baseWk)).toThrow(OidcDiscoveryError.NotSupported);
33+
});
34+
35+
it("should throw misconfigured error when authentication issuer is not a string", () => {
36+
const wk = {
37+
...baseWk,
38+
[M_AUTHENTICATION.stable!]: {
39+
issuer: { url: "test.com" },
40+
},
41+
};
42+
expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcDiscoveryError.Misconfigured);
43+
});
44+
45+
it("should throw misconfigured error when authentication account is not a string", () => {
46+
const wk = {
47+
...baseWk,
48+
[M_AUTHENTICATION.stable!]: {
49+
issuer: "test.com",
50+
account: { url: "test" },
51+
},
52+
};
53+
expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcDiscoveryError.Misconfigured);
54+
});
55+
56+
it("should throw misconfigured error when authentication account is false", () => {
57+
const wk = {
58+
...baseWk,
59+
[M_AUTHENTICATION.stable!]: {
60+
issuer: "test.com",
61+
account: false,
62+
},
63+
};
64+
expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcDiscoveryError.Misconfigured);
65+
});
66+
67+
it("should return valid config when wk uses stable m.authentication", () => {
68+
const wk = {
69+
...baseWk,
70+
[M_AUTHENTICATION.stable!]: {
71+
issuer: "test.com",
72+
account: "account.com",
73+
},
74+
};
75+
expect(validateWellKnownAuthentication(wk)).toEqual({
76+
issuer: "test.com",
77+
account: "account.com",
78+
});
79+
});
80+
81+
it("should return valid config when m.authentication account is missing", () => {
82+
const wk = {
83+
...baseWk,
84+
[M_AUTHENTICATION.stable!]: {
85+
issuer: "test.com",
86+
},
87+
};
88+
expect(validateWellKnownAuthentication(wk)).toEqual({
89+
issuer: "test.com",
90+
});
91+
});
92+
93+
it("should remove unexpected properties", () => {
94+
const wk = {
95+
...baseWk,
96+
[M_AUTHENTICATION.stable!]: {
97+
issuer: "test.com",
98+
somethingElse: "test",
99+
},
100+
};
101+
expect(validateWellKnownAuthentication(wk)).toEqual({
102+
issuer: "test.com",
103+
});
104+
});
105+
106+
it("should return valid config when wk uses unstable prefix for m.authentication", () => {
107+
const wk = {
108+
...baseWk,
109+
[M_AUTHENTICATION.unstable!]: {
110+
issuer: "test.com",
111+
account: "account.com",
112+
},
113+
};
114+
expect(validateWellKnownAuthentication(wk)).toEqual({
115+
issuer: "test.com",
116+
account: "account.com",
117+
});
118+
});
119+
});
120+
121+
describe("validateOIDCIssuerWellKnown", () => {
122+
const validWk: any = {
123+
authorization_endpoint: "https://test.org/authorize",
124+
token_endpoint: "https://authorize.org/token",
125+
registration_endpoint: "https://authorize.org/regsiter",
126+
response_types_supported: ["code"],
127+
grant_types_supported: ["authorization_code"],
128+
code_challenge_methods_supported: ["S256"],
129+
};
130+
beforeEach(() => {
131+
// stub to avoid console litter
132+
jest.spyOn(logger, "error")
133+
.mockClear()
134+
.mockImplementation(() => {});
135+
});
136+
137+
it("should throw OP support error when wellKnown is not an object", () => {
138+
expect(() => {
139+
validateOIDCIssuerWellKnown([]);
140+
}).toThrow(OidcDiscoveryError.OpSupport);
141+
expect(logger.error).toHaveBeenCalledWith("Issuer configuration not found or malformed");
142+
});
143+
144+
it("should log all errors before throwing", () => {
145+
expect(() => {
146+
validateOIDCIssuerWellKnown({
147+
...validWk,
148+
authorization_endpoint: undefined,
149+
response_types_supported: [],
150+
});
151+
}).toThrow(OidcDiscoveryError.OpSupport);
152+
expect(logger.error).toHaveBeenCalledWith("OIDC issuer configuration: authorization_endpoint is invalid");
153+
expect(logger.error).toHaveBeenCalledWith(
154+
"OIDC issuer configuration: response_types_supported is invalid. code is required.",
155+
);
156+
});
157+
158+
it("should return validated issuer config", () => {
159+
expect(validateOIDCIssuerWellKnown(validWk)).toEqual({
160+
authorizationEndpoint: validWk.authorization_endpoint,
161+
tokenEndpoint: validWk.token_endpoint,
162+
registrationEndpoint: validWk.registration_endpoint,
163+
});
164+
});
165+
166+
it("should return validated issuer config without registrationendpoint", () => {
167+
const wk = { ...validWk };
168+
delete wk.registration_endpoint;
169+
expect(validateOIDCIssuerWellKnown(wk)).toEqual({
170+
authorizationEndpoint: validWk.authorization_endpoint,
171+
tokenEndpoint: validWk.token_endpoint,
172+
registrationEndpoint: undefined,
173+
});
174+
});
175+
176+
type TestCase = [string, any];
177+
it.each<TestCase>([
178+
["authorization_endpoint", undefined],
179+
["authorization_endpoint", { not: "a string" }],
180+
["token_endpoint", undefined],
181+
["token_endpoint", { not: "a string" }],
182+
["registration_endpoint", { not: "a string" }],
183+
["response_types_supported", undefined],
184+
["response_types_supported", "not an array"],
185+
["response_types_supported", ["doesnt include code"]],
186+
["grant_types_supported", undefined],
187+
["grant_types_supported", "not an array"],
188+
["grant_types_supported", ["doesnt include authorization_code"]],
189+
["code_challenge_methods_supported", undefined],
190+
["code_challenge_methods_supported", "not an array"],
191+
["code_challenge_methods_supported", ["doesnt include S256"]],
192+
])("should throw OP support error when %s is %s", (key, value) => {
193+
const wk = {
194+
...validWk,
195+
[key]: value,
196+
};
197+
expect(() => validateOIDCIssuerWellKnown(wk)).toThrow(OidcDiscoveryError.OpSupport);
198+
});
199+
});

0 commit comments

Comments
 (0)