Skip to content

Commit

Permalink
OIDC: refresh tokens (#3764)
Browse files Browse the repository at this point in the history 
* very messy poc

* iterate

* more types and use tokenRefreshFunction

* working refresh without persistence

* tidy

* add claims to completeauhtorizationcodegrant response

* export tokenrefresher from matrix

* add idtokenclaims

* add claims to completeauhtorizationcodegrant response

* only one token refresh attempt at a time

* tests

* comments

* add tokenRefresher class

* export generateScope

* export oidc from matrix

* test refreshtoken

* mark experimental

* add getRefreshToken to client

* Apply suggestions from code review

Co-authored-by: Richard van der Hoff <[email protected]>

* remove some vars in test

* make TokenRefresher un-abstract, comments and improvements

* remove invalid jsdoc

* Update src/oidc/tokenRefresher.ts

Co-authored-by: Richard van der Hoff <[email protected]>

* Code review improvements

* fix verification integ tests

* remove unused type from props

* fix incomplete mock fn in fetch.spec

* document TokenRefreshFunction

* comments

* tidying

* update for injected logger

---------

Co-authored-by: Richard van der Hoff <[email protected]>
  • Loading branch information
@richvdh
Kerry and richvdh authored Oct 11, 2023
1 parent 1de6de0 commit 0f4fa5a
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 16 deletions.
6 changes: 6 additions & 0 deletions spec/integ/crypto/verification.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
beforeEach(async () => {
// pretend that we have another device, which we will verify
e2eKeyResponder.addDeviceKeys(SIGNED_TEST_DEVICE_DATA);

fetchMock.put(
new RegExp(`/_matrix/client/(r0|v3)/sendToDevice/${escapeRegExp("m.secret.request")}`),
{ ok: false, status: 404 },
{ overwriteRoutes: true },
);
});

// test with (1) the default verification method list, (2) a custom verification method list.
Expand Down
148 changes: 144 additions & 4 deletions spec/unit/http-api/fetch.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,15 @@ import { Mocked } from "jest-mock";

import { FetchHttpApi } from "../../../src/http-api/fetch";
import { TypedEventEmitter } from "../../../src/models/typed-event-emitter";
import { ClientPrefix, HttpApiEvent, HttpApiEventHandlerMap, IdentityPrefix, IHttpOpts, Method } from "../../../src";
import {
ClientPrefix,
HttpApiEvent,
HttpApiEventHandlerMap,
IdentityPrefix,
IHttpOpts,
MatrixError,
Method,
} from "../../../src";
import { emitPromise } from "../../test-utils/test-utils";
import { defer, QueryDict } from "../../../src/utils";
import { Logger } from "../../../src/logger";
Expand Down Expand Up @@ -231,13 +239,145 @@ describe("FetchHttpApi", () => {
});

describe("authedRequest", () => {
it("should not include token if unset", () => {
const fetchFn = jest.fn();
it("should not include token if unset", async () => {
const fetchFn = jest.fn().mockResolvedValue({ ok: true });
const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn });
api.authedRequest(Method.Post, "/account/password");
await api.authedRequest(Method.Post, "/account/password");
expect(fetchFn.mock.calls[0][1].headers.Authorization).toBeUndefined();
});

describe("with refresh token", () => {
const accessToken = "test-access-token";
const refreshToken = "test-refresh-token";

describe("when an unknown token error is encountered", () => {
const unknownTokenErrBody = {
errcode: "M_UNKNOWN_TOKEN",
error: "Token is not active",
soft_logout: false,
};
const unknownTokenErr = new MatrixError(unknownTokenErrBody, 401);
const unknownTokenResponse = {
ok: false,
status: 401,
headers: {
get(name: string): string | null {
return name === "Content-Type" ? "application/json" : null;
},
},
text: jest.fn().mockResolvedValue(JSON.stringify(unknownTokenErrBody)),
};
const okayResponse = {
ok: true,
status: 200,
};

describe("without a tokenRefreshFunction", () => {
it("should emit logout and throw", async () => {
const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse);
const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
jest.spyOn(emitter, "emit");
const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn, accessToken, refreshToken });
await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow(
unknownTokenErr,
);
expect(emitter.emit).toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
});
});

describe("with a tokenRefreshFunction", () => {
it("should emit logout and throw when token refresh fails", async () => {
const error = new Error("uh oh");
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).toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
});

it("should refresh token and retry request", async () => {
const newAccessToken = "new-access-token";
const newRefreshToken = "new-refresh-token";
const tokenRefreshFunction = jest.fn().mockResolvedValue({
accessToken: newAccessToken,
refreshToken: newRefreshToken,
});
const fetchFn = jest
.fn()
.mockResolvedValueOnce(unknownTokenResponse)
.mockResolvedValueOnce(okayResponse);
const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
jest.spyOn(emitter, "emit");
const api = new FetchHttpApi(emitter, {
baseUrl,
prefix,
fetchFn,
tokenRefreshFunction,
accessToken,
refreshToken,
});
const result = await api.authedRequest(Method.Post, "/account/password");
expect(result).toEqual(okayResponse);
expect(tokenRefreshFunction).toHaveBeenCalledWith(refreshToken);

expect(fetchFn).toHaveBeenCalledTimes(2);
// uses new access token
expect(fetchFn.mock.calls[1][1].headers.Authorization).toEqual("Bearer new-access-token");
expect(emitter.emit).not.toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
});

it("should only try to refresh the token once", async () => {
const newAccessToken = "new-access-token";
const newRefreshToken = "new-refresh-token";
const tokenRefreshFunction = jest.fn().mockResolvedValue({
accessToken: newAccessToken,
refreshToken: newRefreshToken,
});

// fetch doesn't like our new or old tokens
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,
);

// tried to refresh the token once
expect(tokenRefreshFunction).toHaveBeenCalledWith(refreshToken);
expect(tokenRefreshFunction).toHaveBeenCalledTimes(1);

expect(fetchFn).toHaveBeenCalledTimes(2);
// uses new access token on retry
expect(fetchFn.mock.calls[1][1].headers.Authorization).toEqual("Bearer new-access-token");

// logged out after refreshed access token is rejected
expect(emitter.emit).toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
});
});
});
});
});

describe("getUrl()", () => {
Expand Down
19 changes: 19 additions & 0 deletions src/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
IdentityPrefix,
IHttpOpts,
IRequestOpts,
TokenRefreshFunction,
MatrixError,
MatrixHttpApi,
MediaPrefix,
Expand Down Expand Up @@ -294,6 +295,14 @@ export interface ICreateClientOpts {
deviceId?: string;

accessToken?: string;
refreshToken?: string;

/**
* Function used to attempt refreshing access and refresh tokens
* Called by http-api when a possibly expired token is encountered
* and a refreshToken is found
*/
tokenRefreshFunction?: TokenRefreshFunction;

/**
* Identity server provider to retrieve the user's access token when accessing
Expand Down Expand Up @@ -1344,6 +1353,8 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
baseUrl: opts.baseUrl,
idBaseUrl: opts.idBaseUrl,
accessToken: opts.accessToken,
refreshToken: opts.refreshToken,
tokenRefreshFunction: opts.tokenRefreshFunction,
prefix: ClientPrefix.V3,
onlyData: true,
extraParams: opts.queryParams,
Expand Down Expand Up @@ -7716,6 +7727,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
return this.http.opts.accessToken || null;
}

/**
* Get the refresh token associated with this account.
* @returns The refresh_token or null
*/
public getRefreshToken(): string | null {
return this.http.opts.refreshToken ?? null;
}

/**
* Set the access token associated with this account.
* @param token - The new access token.
Expand Down
60 changes: 49 additions & 11 deletions src/http-api/fetch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ export class FetchHttpApi<O extends IHttpOpts> {
*
* @param body - The HTTP JSON body.
*
* @param opts - additional options. If a number is specified,
* this is treated as `opts.localTimeoutMs`.
* @param opts - additional options.
* When `opts.doNotAttemptTokenRefresh` is true, token refresh will not be attempted
* when an expired token is encountered. Used to only attempt token refresh once.
*
* @returns Promise which resolves to
* ```
Expand All @@ -126,15 +127,18 @@ export class FetchHttpApi<O extends IHttpOpts> {
* @returns Rejects with an error if a problem occurred.
* This includes network problems and Matrix-specific error JSON.
*/
public authedRequest<T>(
public async authedRequest<T>(
method: Method,
path: string,
queryParams?: QueryDict,
body?: Body,
opts: IRequestOpts = {},
paramOpts: IRequestOpts & { doNotAttemptTokenRefresh?: boolean } = {},
): Promise<ResponseType<T, O>> {
if (!queryParams) queryParams = {};

// avoid mutating paramOpts so they can be used on retry
const opts = { ...paramOpts };

if (this.opts.accessToken) {
if (this.opts.useAuthorizationHeader) {
if (!opts.headers) {
Expand All @@ -151,19 +155,53 @@ export class FetchHttpApi<O extends IHttpOpts> {
}
}

const requestPromise = this.request<T>(method, path, queryParams, body, opts);

requestPromise.catch((err: MatrixError) => {
try {
const response = await this.request<T>(method, path, queryParams, body, opts);
return response;
} catch (error) {
const err = error as MatrixError;

if (err.errcode === "M_UNKNOWN_TOKEN" && !opts.doNotAttemptTokenRefresh) {
const shouldRetry = await this.tryRefreshToken();
// if we got a new token retry the request
if (shouldRetry) {
return this.authedRequest(method, path, queryParams, body, {
...paramOpts,
doNotAttemptTokenRefresh: true,
});
}
}
// 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);
}
});

// return the original promise, otherwise tests break due to it having to
// go around the event loop one more time to process the result of the request
return requestPromise;
throw err;
}
}

/**
* Attempt to refresh access tokens.
* On success, sets new access and refresh tokens in opts.
* @returns Promise that resolves to a boolean - true when token was refreshed successfully
*/
private async tryRefreshToken(): Promise<boolean> {
if (!this.opts.refreshToken || !this.opts.tokenRefreshFunction) {
return false;
}

try {
const { accessToken, refreshToken } = await this.opts.tokenRefreshFunction(this.opts.refreshToken);
this.opts.accessToken = accessToken;
this.opts.refreshToken = refreshToken;
// successfully got new tokens
return true;
} catch (error) {
this.opts.logger?.warn("Failed to refresh token", error);
return false;
}
}

/**
Expand Down
12 changes: 11 additions & 1 deletion src/http-api/interface.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ export type AccessTokens = {
accessToken: string;
refreshToken?: string;
};
// @TODO(kerrya) add link to IHttpOpts and CreateClientOpts when token refresh is added there
/**
* @experimental
* Function that performs token refresh using the given refreshToken.
* Returns a promise that resolves to the refreshed access and (optional) refresh tokens.
*
* Can be passed to HttpApi instance as {@link IHttpOpts.tokenRefreshFunction} during client creation {@link ICreateClientOpts}
*/
export type TokenRefreshFunction = (refreshToken: string) => Promise<AccessTokens>;
export interface IHttpOpts {
Expand All @@ -43,6 +44,15 @@ export interface IHttpOpts {
extraParams?: Record<string, string>;

accessToken?: string;
/**
* Used in conjunction with tokenRefreshFunction to attempt token refresh
*/
refreshToken?: string;
/**
* Function to attempt token refresh when a possibly expired token is encountered
* Optional, only called when a refreshToken is present
*/
tokenRefreshFunction?: TokenRefreshFunction;
useAuthorizationHeader?: boolean; // defaults to true

onlyData?: boolean;
Expand Down

0 comments on commit 0f4fa5a

Please sign in to comment.