Skip to content

Commit 88c0896

Browse files
hughnshughns
authored
Validate that an error from OAuth token refresh looks like an OAuth error before treating as logout (#5472)
* Validate that an error from OAuth token refresh looks like an OAuth error before treating as logout * Fix bad jsdoc link
1 parent 53a5245 commit 88c0896

5 files changed

Lines changed: 155 additions & 9 deletions

File tree

‎spec/unit/oauth/tokenRefresher.spec.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,5 +238,65 @@ describe("OidcTokenRefresher", () => {
238238

239239
await expect(refresher.tokenRefreshFunction("refresh-token")).rejects.not.toThrow(TokenRefreshLogoutError);
240240
});
241+
242+
it("should not throw TokenRefreshLogoutError on a 4xx without a JSON body", async () => {
243+
fetchMock.modifyRoute("token-endpoint", {
244+
response: {
245+
status: 403,
246+
headers: {
247+
"Content-Type": "text/html",
248+
},
249+
body: "<html><body>Blocked by your network administrator</body></html>",
250+
},
251+
});
252+
253+
const fn = vi.fn();
254+
const refresher = new TokenRefresher(auth, fn);
255+
256+
await expect(refresher.tokenRefreshFunction("refresh-token")).rejects.not.toThrow(TokenRefreshLogoutError);
257+
});
258+
259+
it("should not throw TokenRefreshLogoutError on a 4xx with a JSON body which is not an OAuth 2.0 error", async () => {
260+
fetchMock.modifyRoute("token-endpoint", {
261+
response: {
262+
status: 429,
263+
headers: {
264+
"Content-Type": "application/json",
265+
},
266+
body: {
267+
some: "field",
268+
},
269+
},
270+
});
271+
272+
const fn = vi.fn();
273+
const refresher = new TokenRefresher(auth, fn);
274+
275+
await expect(refresher.tokenRefreshFunction("refresh-token")).rejects.not.toThrow(TokenRefreshLogoutError);
276+
});
277+
278+
it("should throw TokenRefreshLogoutError on a 4xx Matrix API error code which looks like an OAuth 2.0 error", async () => {
279+
// The C-S API [standard error response](https://spec.matrix.org/v1.18/client-server-api/#standard-error-response)
280+
// is hard to distinguish from an RFC 6749 error response. This test asserts the current logic from
281+
// https://spec.matrix.org/v1.18/client-server-api/#refresh-token-grant where a 4xx response is treated as a
282+
// logout even if the body is a misidentified Matrix API error response.
283+
fetchMock.modifyRoute("token-endpoint", {
284+
response: {
285+
status: 429,
286+
headers: {
287+
"Content-Type": "application/json",
288+
},
289+
body: {
290+
errcode: "M_LIMIT_EXCEEDED",
291+
error: "Rate limited",
292+
},
293+
},
294+
});
295+
296+
const fn = vi.fn();
297+
const refresher = new TokenRefresher(auth, fn);
298+
299+
await expect(refresher.tokenRefreshFunction("refresh-token")).rejects.toThrow(TokenRefreshLogoutError);
300+
});
241301
});
242302
});

‎src/oauth/authorize.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ limitations under the License.
1515
*/
1616

1717
import { secureRandomString } from "../randomstring.ts";
18-
import { OAuth2Error } from "./error.ts";
18+
import { OAuth2Error, type OAuth2ErrorResponse } from "./error.ts";
1919
import { type ValidatedAuthMetadata } from "./discover.ts";
2020
import {
2121
hasOptionalNumberProperty,
@@ -127,10 +127,7 @@ export function isValidDeviceAccessTokenResponse(response: unknown): response is
127127
/**
128128
* Error from the OAuth2 token endpoint when exchanging a token for grant_type device_code.
129129
*/
130-
export interface DeviceAccessTokenError {
131-
error: string;
132-
error_description?: string;
133-
error_uri?: string;
130+
export interface DeviceAccessTokenError extends OAuth2ErrorResponse {
134131
session_state?: string;
135132
}
136133

‎src/oauth/error.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17+
import { hasOptionalStringProperty, hasRequiredStringProperty, isRecord } from "../@types/type-guards.ts";
18+
import { HTTPError } from "../http-api/errors.ts";
19+
1720
/**
1821
* Errors expected to be encountered during OAuth2 discovery, client registration, and authentication.
1922
* Not intended to be displayed directly to the user.
@@ -32,3 +35,69 @@ export enum OAuth2Error {
3235
RevokeTokenFailed = "Failed to revoke token",
3336
DeviceAuthorizationGrantFailed = "Failed to perform device authorization grant",
3437
}
38+
39+
/**
40+
* An error response from an OAuth 2.0 endpoint,
41+
* as specified in https://datatracker.ietf.org/doc/html/rfc6749#section-5.2
42+
*/
43+
export interface OAuth2ErrorResponse {
44+
/** A single ASCII error code, e.g. `invalid_grant`. */
45+
error: string;
46+
/** Human-readable ASCII text providing additional information about the error. */
47+
error_description?: string;
48+
/** A URI identifying a human-readable web page with information about the error. */
49+
error_uri?: string;
50+
}
51+
52+
/**
53+
* Check whether the given (JSON-parsed) response body is an OAuth 2.0 error response
54+
* as specified in https://datatracker.ietf.org/doc/html/rfc6749#section-5.2
55+
* @param response - the parsed response body to check
56+
* @returns whether the response is a valid {@link OAuth2ErrorResponse}
57+
*/
58+
export function isOAuth2ErrorResponse(response: unknown): response is OAuth2ErrorResponse {
59+
return (
60+
isRecord(response) &&
61+
hasRequiredStringProperty(response, "error") &&
62+
hasOptionalStringProperty(response, "error_description") &&
63+
hasOptionalStringProperty(response, "error_uri")
64+
);
65+
}
66+
67+
/**
68+
* An error thrown when a request to an OAuth 2.0 endpoint fails with a body matching the error
69+
* response format specified in [RFC 6749 section 5.2](https://datatracker.ietf.org/doc/html/rfc6749#section-5.2).
70+
*/
71+
export class OAuth2HTTPError extends HTTPError implements OAuth2ErrorResponse {
72+
/**
73+
* RFC 6749 section 5.2 error code, e.g. `invalid_grant`
74+
*
75+
* IANA matains a registry of valid values at
76+
* https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#extensions-error
77+
*/
78+
public error: string;
79+
80+
/**
81+
* RFC 6749 section 5.2 human-readable ASCII text providing additional information about the error.
82+
* This field is optional and may be omitted by the endpoint.
83+
*/
84+
public error_description?: string;
85+
86+
/**
87+
* RFC 6749 section 5.2 URI identifying a human-readable web page with information about the error.
88+
* This field is optional and may be omitted by the endpoint.
89+
*/
90+
public error_uri?: string;
91+
92+
public constructor(
93+
msg: string,
94+
httpStatus: number | undefined,
95+
httpHeaders: Headers | undefined,
96+
{ error, error_description, error_uri }: OAuth2ErrorResponse,
97+
) {
98+
super(msg, httpStatus, httpHeaders);
99+
this.error = error;
100+
this.error_description = error_description;
101+
this.error_uri = error_uri;
102+
}
103+
}

‎src/oauth/index.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import { encodeUnpaddedBase64Url } from "../base64.ts";
3636
import { sha256 } from "../digest.ts";
3737
import { HTTPError, Method } from "../http-api/index.ts";
3838
import { logger } from "../logger.ts";
39-
import { OAuth2Error } from "./error.ts";
39+
import { isOAuth2ErrorResponse, OAuth2Error, type OAuth2ErrorResponse, OAuth2HTTPError } from "./error.ts";
4040
import { secureRandomString } from "../randomstring.ts";
4141
import { type NonEmptyArray } from "../@types/common.ts";
4242

@@ -289,7 +289,20 @@ export class OAuth2 {
289289
});
290290

291291
if (res.status >= 400) {
292-
throw new HTTPError(error, res.status, res.headers);
292+
let errorResponse: OAuth2ErrorResponse | undefined;
293+
try {
294+
const body = await res.json();
295+
if (isOAuth2ErrorResponse(body)) {
296+
errorResponse = body;
297+
}
298+
} catch {
299+
// The endpoint didn't give us a JSON body, so we definitely have no OAuth 2.0 error response to use
300+
}
301+
if (errorResponse) {
302+
throw new OAuth2HTTPError(error, res.status, res.headers, errorResponse);
303+
} else {
304+
throw new HTTPError(error, res.status, res.headers);
305+
}
293306
}
294307

295308
return await res.json();

‎src/oauth/tokenRefresher.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ limitations under the License.
1515
*/
1616

1717
import { type AccessTokens, HTTPError, type TokenRefreshFunction, TokenRefreshLogoutError } from "../http-api/index.ts";
18+
import { OAuth2HTTPError } from "./error.ts";
1819
import { type OAuth2 } from "./index.ts";
1920

2021
/**
@@ -54,8 +55,14 @@ export class TokenRefresher {
5455
};
5556

5657
private shouldLogoutOnError(error: HTTPError): boolean {
57-
// As per https://spec.matrix.org/v1.18/client-server-api/#refresh-token-grant
58-
return typeof error.httpStatus === "number" && error.httpStatus < 500 && error.httpStatus >= 400;
58+
// Treat as logout as per https://spec.matrix.org/v1.18/client-server-api/#refresh-token-grant
59+
// after making sure it is an RFC 6749 section 5.2 error response
60+
return (
61+
error instanceof OAuth2HTTPError &&
62+
typeof error.httpStatus === "number" &&
63+
error.httpStatus < 500 &&
64+
error.httpStatus >= 400
65+
);
5966
}
6067

6168
private async getNewTokens(refreshToken: string): Promise<AccessTokens> {

0 commit comments

Comments
 (0)