matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-08-06 12:02:40 +03:00
Code Activity

Handle unexpected token refresh failures gracefully (#4731)

Browse Source 
* Fix idempotency issue around token refresh

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Improve test

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Handle unexpected token refresh failures gracefully

e.g. connection errors, proxy errors differently from token invalidated errors

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Michael Telatynski
2025-02-28 11:25:06 +00:00
committed by GitHub
parent 72b997d1f3
commit 71bffb6c1b
3 changed files with 71 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
spec/unit/http-api/fetch.spec.ts
View File

@@ -20,6 +20,7 @@ import { FetchHttpApi } from "../../../src/http-api/fetch";
import { TypedEventEmitter } from "../../../src/models/typed-event-emitter";
import {
ClientPrefix,
ConnectionError,
HttpApiEvent,
type HttpApiEventHandlerMap,
IdentityPrefix,
@@ -288,7 +289,7 @@ describe("FetchHttpApi", () => {
describe("with a tokenRefreshFunction", () => {
it("should emit logout and throw when token refresh fails", async () => {
const error = new Error("uh oh");
const error = new MatrixError();
const tokenRefreshFunction = jest.fn().mockRejectedValue(error);
const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse);
const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
@@ -308,6 +309,27 @@ describe("FetchHttpApi", () => {
expect(emitter.emit).toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
});
it("should not emit logout but still throw when token refresh fails due to transitive fault", async () => {
const error = new ConnectionError("transitive fault");
const tokenRefreshFunction = jest.fn().mockRejectedValue(error);
const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse);
const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
jest.spyOn(emitter, "emit");
const api = new FetchHttpApi(emitter, {
baseUrl,
prefix,
fetchFn,
tokenRefreshFunction,
accessToken,
refreshToken,
});
await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow(
unknownTokenErr,
);
expect(tokenRefreshFunction).toHaveBeenCalledWith(refreshToken);
expect(emitter.emit).not.toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
});
it("should refresh token and retry request", async () => {
const newAccessToken = "new-access-token";
const newRefreshToken = "new-refresh-token";

15
src/http-api/errors.ts
View File

@@ -197,3 +197,18 @@ export class ConnectionError extends Error {
return "ConnectionError";
}
}
/**
* Construct a TokenRefreshError. This indicates that a request failed due to the token being expired,
* and attempting to refresh said token also failed but in a way which was not indicative of token invalidation.
* Assumed to be a temporary failure.
*/
export class TokenRefreshError extends Error {
public constructor(cause?: Error) {
super(cause?.message ?? "");
}
public get name(): string {
return "TokenRefreshError";
}
}

46
src/http-api/fetch.ts
View File

@@ -18,10 +18,12 @@ limitations under the License.
* This is an internal module. See {@link MatrixHttpApi} for the public class.
*/
import { ErrorResponse as OidcAuthError } from "oidc-client-ts";
import { checkObjectHasKeys, encodeParams } from "../utils.ts";
import { type TypedEventEmitter } from "../models/typed-event-emitter.ts";
import { Method } from "./method.ts";
import { ConnectionError, type MatrixError } from "./errors.ts";
import { ConnectionError, MatrixError, TokenRefreshError } from "./errors.ts";
import {
HttpApiEvent,
type HttpApiEventHandlerMap,
@@ -43,6 +45,12 @@ export type ResponseType<T, O extends IHttpOpts> = O extends undefined
? T
: TypedResponse<T>;
const enum TokenRefreshOutcome {
Success = "success",
Failure = "failure",
Logout = "logout",
}
export class FetchHttpApi<O extends IHttpOpts> {
private abortController = new AbortController();
@@ -174,29 +182,36 @@ export class FetchHttpApi<O extends IHttpOpts> {
const response = await this.request<T>(method, path, queryParams, body, opts);
return response;
} catch (error) {
const err = error as MatrixError;
if (!(error instanceof MatrixError)) {
throw error;
}
if (err.errcode === "M_UNKNOWN_TOKEN" && !opts.doNotAttemptTokenRefresh) {
if (error.errcode === "M_UNKNOWN_TOKEN" && !opts.doNotAttemptTokenRefresh) {
const tokenRefreshPromise = this.tryRefreshToken();
this.tokenRefreshPromise = Promise.allSettled([tokenRefreshPromise]);
const shouldRetry = await tokenRefreshPromise;
const outcome = await tokenRefreshPromise;
if (outcome === TokenRefreshOutcome.Success) {
// if we got a new token retry the request
if (shouldRetry) {
return this.authedRequest(method, path, queryParams, body, {
...paramOpts,
doNotAttemptTokenRefresh: true,
});
}
if (outcome === TokenRefreshOutcome.Failure) {
throw new TokenRefreshError(error);
}
// Fall through to SessionLoggedOut handler below
}
// otherwise continue with error handling
if (err.errcode == "M_UNKNOWN_TOKEN" && !opts?.inhibitLogoutEmit) {
this.eventEmitter.emit(HttpApiEvent.SessionLoggedOut, err);
} else if (err.errcode == "M_CONSENT_NOT_GIVEN") {
this.eventEmitter.emit(HttpApiEvent.NoConsent, err.message, err.data.consent_uri);
if (error.errcode == "M_UNKNOWN_TOKEN" && !opts?.inhibitLogoutEmit) {
this.eventEmitter.emit(HttpApiEvent.SessionLoggedOut, error);
} else if (error.errcode == "M_CONSENT_NOT_GIVEN") {
this.eventEmitter.emit(HttpApiEvent.NoConsent, error.message, error.data.consent_uri);
}
throw err;
throw error;
}
}
@@ -206,9 +221,9 @@ export class FetchHttpApi<O extends IHttpOpts> {
* @returns Promise that resolves to a boolean - true when token was refreshed successfully
*/
@singleAsyncExecution
private async tryRefreshToken(): Promise<boolean> {
private async tryRefreshToken(): Promise<TokenRefreshOutcome> {
if (!this.opts.refreshToken || !this.opts.tokenRefreshFunction) {
return false;
return TokenRefreshOutcome.Logout;
}
try {
@@ -216,10 +231,13 @@ export class FetchHttpApi<O extends IHttpOpts> {
this.opts.accessToken = accessToken;
this.opts.refreshToken = refreshToken;
// successfully got new tokens
return true;
return TokenRefreshOutcome.Success;
} catch (error) {
this.opts.logger?.warn("Failed to refresh token", error);
return false;
if (error instanceof OidcAuthError || error instanceof MatrixError) {
return TokenRefreshOutcome.Logout;
}
return TokenRefreshOutcome.Failure;
}
}