From 574dd36c17c20ee61f88f5438be18802fd9358c8 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 18 Jul 2023 10:49:05 +1200 Subject: [PATCH 01/31] very messy poc --- src/http-api/fetch.ts | 33 +++++++++++++++-- src/oidc/authorize.ts | 6 ++-- src/oidc/tokenRefresher.ts | 74 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 src/oidc/tokenRefresher.ts diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 9408c94cefe..0f82d7f7a25 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -47,6 +47,7 @@ export class FetchHttpApi { checkObjectHasKeys(opts, ["baseUrl", "prefix"]); opts.onlyData = !!opts.onlyData; opts.useAuthorizationHeader = opts.useAuthorizationHeader ?? true; + } public abort(): void { @@ -132,10 +133,12 @@ export class FetchHttpApi { path: string, queryParams?: QueryDict, body?: Body, - opts: IRequestOpts = {}, + paramOpts: IRequestOpts = {}, ): Promise> { if (!queryParams) queryParams = {}; + let opts = { ...paramOpts }; + if (this.opts.accessToken) { if (this.opts.useAuthorizationHeader) { if (!opts.headers) { @@ -154,7 +157,7 @@ export class FetchHttpApi { const requestPromise = this.request(method, path, queryParams, body, opts); - requestPromise.catch((err: MatrixError) => { + requestPromise.catch(async (err: MatrixError) => { if (err.errcode == "M_UNKNOWN_TOKEN" && !opts?.inhibitLogoutEmit) { this.eventEmitter.emit(HttpApiEvent.SessionLoggedOut, err); } else if (err.errcode == "M_CONSENT_NOT_GIVEN") { @@ -201,6 +204,7 @@ export class FetchHttpApi { opts?: IRequestOpts, ): Promise> { const fullUri = this.getUrl(path, queryParams, opts?.prefix, opts?.baseUrl); + return this.requestOtherUrl(method, fullUri, body, opts); } @@ -227,6 +231,8 @@ export class FetchHttpApi { const urlForLogs = this.clearUrlParamsForLogs(url); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); + console.log('hhhh REQUESTOTHERURL', opts); + const headers = Object.assign({}, opts.headers || {}); const json = opts.json ?? true; // We can't use getPrototypeOf here as objects made in other contexts e.g. over postMessage won't have same ref @@ -280,6 +286,7 @@ export class FetchHttpApi { logger.debug(`FetchHttpApi: <-- ${method} ${urlForLogs} [${Date.now() - start}ms ${res.status}]`); } catch (e) { + console.log('hhhh requestOtherUrl error', e); logger.debug(`FetchHttpApi: <-- ${method} ${urlForLogs} [${Date.now() - start}ms ${e}]`); if ((e).name === "AbortError") { throw e; @@ -290,7 +297,27 @@ export class FetchHttpApi { } if (!res.ok) { - throw parseErrorResponse(res, await res.text()); + const err = parseErrorResponse(res, await res.text()); + + console.log('HHHHHHHHHH CATCH', err.errcode, err, opts, this.opts.tokenRefresher, opts.retryWithRefreshedToken); + if (err.errcode === "M_UNKNOWN_TOKEN" && !!this.opts.tokenRefresher && !opts.retryWithRefreshedToken) { + const setToken = (newToken) => { + console.log('hhhh refreshToken.setting token', newToken, this); + this.opts.accessToken = newToken; + } + await this.opts.tokenRefresher.doRefreshAccessToken(setToken.bind(this)); + // we set a new token successfully + console.log('hhh', 'retrying request after refreshing token', this.opts.accessToken); + // @TODO(kerrya) don't mutate params (opts, qp) + // this function modifies these params :/ so the retry call may be with different params + return this.requestOtherUrl(method, url, body, { + ...opts, retryWithRefreshedToken: true, + headers: { + ...opts.headers, + Authorization: "Bearer " + this.opts.accessToken + } + }); + } } if (this.opts.onlyData) { diff --git a/src/oidc/authorize.ts b/src/oidc/authorize.ts index df802aa0a4f..fbcfeaae0d1 100644 --- a/src/oidc/authorize.ts +++ b/src/oidc/authorize.ts @@ -51,9 +51,9 @@ export type AuthorizationParams = { * Generate the scope used in authorization request with OIDC OP * @returns scope */ -const generateScope = (): string => { - const deviceId = randomString(10); - return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; +export const generateScope = (deviceId?: string): string => { + const safeDeviceId = deviceId || randomString(10); + return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${safeDeviceId}`; }; // https://www.rfc-editor.org/rfc/rfc7636 diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts new file mode 100644 index 00000000000..4f00bc6d6e1 --- /dev/null +++ b/src/oidc/tokenRefresher.ts @@ -0,0 +1,74 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { OidcClient, UseRefreshTokenArgs, WebStorageStateStore } from "oidc-client-ts"; +import { IDelegatedAuthConfig } from "../client"; +import { MatrixClient } from "../client"; + +import { generateScope } from "./authorize"; +import { discoverAndValidateAuthenticationConfig } from "./discovery"; + +export class OidcTokenRefresher { + private oidcClient!: OidcClient; + constructor( + private refreshToken: string, + authConfig: IDelegatedAuthConfig, + clientId: string, + redirectUri: string, + deviceId: string, + ) { + this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri); + } + + private async initialiseOidcClient(authConfig: IDelegatedAuthConfig, clientId: string, deviceId: string, redirectUri: string): Promise { + const config = await discoverAndValidateAuthenticationConfig(authConfig); + + const scope = await generateScope(deviceId); + + this.oidcClient = new OidcClient({ + ...config.metadata, + client_id: clientId, + scope, + redirect_uri: redirectUri, + authority: config.metadata.issuer, + // @TODO(kerrya) need this? + stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }), + }); + } + + public async doRefreshAccessToken (setAccessToken: MatrixClient['setAccessToken']) { + if (!this.oidcClient) { + throw new Error("No client TODO") + } + + const refreshTokenState = { + refresh_token: this.refreshToken, + session_state: 'test', + data: undefined, + } + const response = await this.oidcClient.useRefreshToken({ + state: refreshTokenState, timeoutInSeconds: 300 }); + + this.refreshToken = response.refresh_token; + this.expiresAt = response.expires_at; + + // TODO persist tokens in storage + + setAccessToken(response.access_token); + + console.log('hhhh doRefreshAccessToken', response); + } +} \ No newline at end of file From 3cc1bcdac37714314d67f9ee5b9ec2d0b9d26f07 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 19 Jul 2023 15:45:31 +1200 Subject: [PATCH 02/31] iterate --- src/http-api/fetch.ts | 72 +++++++++++++++++++++++--------------- src/oidc/tokenRefresher.ts | 15 +++++--- 2 files changed, 53 insertions(+), 34 deletions(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 073cc4252bd..50cec5c8c97 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -128,7 +128,7 @@ export class FetchHttpApi { * @returns Rejects with an error if a problem occurred. * This includes network problems and Matrix-specific error JSON. */ - public authedRequest( + public async authedRequest( method: Method, path: string, queryParams?: QueryDict, @@ -155,19 +155,53 @@ export class FetchHttpApi { } } - const requestPromise = this.request(method, path, queryParams, body, opts); + try { + const response = await this.request(method, path, queryParams, body, opts); + return response; + } catch (error) { + const err = error as MatrixError; + + console.log('HHHHHHHHHH CATCH', err.errcode, err, opts, this.opts.tokenRefresher, opts.retryWithRefreshedToken); + if (err.errcode === "M_UNKNOWN_TOKEN" && !opts.retryWithRefreshedToken) { + const shouldRetry = await this.tryRefreshToken(); + // if we got a new token retry the request + if (shouldRetry) { + // we set a new token successfully + console.log('hhh', 'retrying request after refreshing token', this.opts.accessToken); + // @TODO(kerrya) don't mutate params (opts, qp) + // this function modifies these params :/ so the retry call may be with different params + return this.authedRequest(method, path, queryParams, body, { + ...paramOpts, + retryWithRefreshedToken: true + }); + } + } + // otherwise continue with error handling - requestPromise.catch(async (err: MatrixError) => { 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; + } + } + + private async tryRefreshToken(): Promise { + if (!this.opts.tokenRefresher) { + return false; + } + + try { + const newToken = await this.opts.tokenRefresher.doRefreshAccessToken(); + this.opts.accessToken = newToken; + // successfully got new token + return true; + } catch (error) { + logger.warn("Failed to refresh token", error); + return false; + } } /** @@ -231,7 +265,7 @@ export class FetchHttpApi { const urlForLogs = this.sanitizeUrlForLogs(url); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); - console.log('hhhh REQUESTOTHERURL', opts); + console.log('hhhh REQUESTOTHERURL', {opts, body, url }); const headers = Object.assign({}, opts.headers || {}); const json = opts.json ?? true; @@ -297,27 +331,7 @@ export class FetchHttpApi { } if (!res.ok) { - const err = parseErrorResponse(res, await res.text()); - - console.log('HHHHHHHHHH CATCH', err.errcode, err, opts, this.opts.tokenRefresher, opts.retryWithRefreshedToken); - if (err.errcode === "M_UNKNOWN_TOKEN" && !!this.opts.tokenRefresher && !opts.retryWithRefreshedToken) { - const setToken = (newToken) => { - console.log('hhhh refreshToken.setting token', newToken, this); - this.opts.accessToken = newToken; - } - await this.opts.tokenRefresher.doRefreshAccessToken(setToken.bind(this)); - // we set a new token successfully - console.log('hhh', 'retrying request after refreshing token', this.opts.accessToken); - // @TODO(kerrya) don't mutate params (opts, qp) - // this function modifies these params :/ so the retry call may be with different params - return this.requestOtherUrl(method, url, body, { - ...opts, retryWithRefreshedToken: true, - headers: { - ...opts.headers, - Authorization: "Bearer " + this.opts.accessToken - } - }); - } + throw parseErrorResponse(res, await res.text()); } if (this.opts.onlyData) { diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index 4f00bc6d6e1..fce55acf985 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { OidcClient, UseRefreshTokenArgs, WebStorageStateStore } from "oidc-client-ts"; +import { OidcClient, SigninResponse, UseRefreshTokenArgs, WebStorageStateStore } from "oidc-client-ts"; import { IDelegatedAuthConfig } from "../client"; import { MatrixClient } from "../client"; @@ -23,6 +23,7 @@ import { discoverAndValidateAuthenticationConfig } from "./discovery"; export class OidcTokenRefresher { private oidcClient!: OidcClient; + constructor( private refreshToken: string, authConfig: IDelegatedAuthConfig, @@ -49,7 +50,12 @@ export class OidcTokenRefresher { }); } - public async doRefreshAccessToken (setAccessToken: MatrixClient['setAccessToken']) { + public async doRefreshAccessToken (): Promise { + // @TODO something here with only one inflight refresh attempt + return this.getNewToken(); + } + + private async getNewToken(): Promise { if (!this.oidcClient) { throw new Error("No client TODO") } @@ -66,9 +72,8 @@ export class OidcTokenRefresher { this.expiresAt = response.expires_at; // TODO persist tokens in storage - - setAccessToken(response.access_token); - console.log('hhhh doRefreshAccessToken', response); + + return response.access_token; } } \ No newline at end of file From 8fe0ea759b551f87c6c741fb867dc7070d64ed04 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Jul 2023 16:27:55 +1200 Subject: [PATCH 03/31] more types and use tokenRefreshFunction --- src/client.ts | 11 +++++++++++ src/http-api/fetch.ts | 16 ++++++++-------- src/http-api/interface.ts | 5 +++++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/client.ts b/src/client.ts index 6fc34249295..7ccfc880959 100644 --- a/src/client.ts +++ b/src/client.ts @@ -72,6 +72,7 @@ import { HTTPError, IRequestOpts, Body, + TokenRefreshFunction } from "./http-api"; import { Crypto, @@ -293,6 +294,14 @@ export interface ICreateClientOpts { deviceId?: string; accessToken?: string; + refreshToken?: string; + + /** + * Function used to attempt refreshing the access token + * 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 @@ -1291,6 +1300,8 @@ export class MatrixClient extends TypedEventEmitter { } catch (error) { const err = error as MatrixError; - console.log('HHHHHHHHHH CATCH', err.errcode, err, opts, this.opts.tokenRefresher, opts.retryWithRefreshedToken); + console.log('HHHHHHHHHH CATCH', err.errcode, err, opts, opts.retryWithRefreshedToken); if (err.errcode === "M_UNKNOWN_TOKEN" && !opts.retryWithRefreshedToken) { const shouldRetry = await this.tryRefreshToken(); // if we got a new token retry the request @@ -189,14 +189,17 @@ export class FetchHttpApi { } private async tryRefreshToken(): Promise { - if (!this.opts.tokenRefresher) { + if (!this.opts.refreshToken || !this.opts.tokenRefreshFunction) { return false; } try { - const newToken = await this.opts.tokenRefresher.doRefreshAccessToken(); - this.opts.accessToken = newToken; - // successfully got new token + 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) { logger.warn("Failed to refresh token", error); @@ -265,8 +268,6 @@ export class FetchHttpApi { const urlForLogs = this.sanitizeUrlForLogs(url); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); - console.log('hhhh REQUESTOTHERURL', {opts, body, url }); - const headers = Object.assign({}, opts.headers || {}); const json = opts.json ?? true; // We can't use getPrototypeOf here as objects made in other contexts e.g. over postMessage won't have same ref @@ -321,7 +322,6 @@ export class FetchHttpApi { logger.debug(`FetchHttpApi: <-- ${method} ${urlForLogs} [${Date.now() - start}ms ${res.status}]`); } catch (e) { - console.log('hhhh requestOtherUrl error', e); logger.debug(`FetchHttpApi: <-- ${method} ${urlForLogs} [${Date.now() - start}ms ${e}]`); if ((e).name === "AbortError") { throw e; diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index 57e8a18e851..3dfcc5cb466 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -18,6 +18,9 @@ import { MatrixError } from "./errors"; export type Body = Record | BodyInit; +export type TokenRefreshFunction = (refreshToken: string) => Promise<{ + accessToken: string; refreshToken: string; +}> export interface IHttpOpts { fetchFn?: typeof global.fetch; @@ -27,6 +30,8 @@ export interface IHttpOpts { extraParams?: Record; accessToken?: string; + refreshToken?: string; + tokenRefreshFunction?: TokenRefreshFunction; useAuthorizationHeader?: boolean; // defaults to true onlyData?: boolean; From 3d7afcf4f2b7ff7a69c608071212be315256ae39 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 11:17:10 +1300 Subject: [PATCH 04/31] working refresh without persistence --- src/http-api/fetch.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index e866c2675b1..816721fd70b 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -332,7 +332,29 @@ export class FetchHttpApi { } if (!res.ok) { - throw parseErrorResponse(res, await res.text()); + const err = parseErrorResponse(res, await res.text()); + + if (err.errcode === "M_UNKNOWN_TOKEN" && !!this.opts.tokenRefresher && !opts.retryWithRefreshedToken) { + const setToken = (newToken) => { + console.log('hhhh refreshToken.setting token', newToken, this); + this.opts.accessToken = newToken; + } + const newToken = await this.opts.tokenRefresher.doRefreshAccessToken(); + // we set a new token successfully + console.log('hhh', 'retrying request after refreshing token', this.opts.accessToken); + + this.opts.accessToken = newToken; + + return this.requestOtherUrl(method, url, body, { + ...opts, retryWithRefreshedToken: true, + headers: { + ...opts.headers, + Authorization: "Bearer " + this.opts.accessToken + } + }); + } else { + throw err; + } } if (this.opts.onlyData) { From 17bd9b4b1685f289fe1aa21d6b91d66a2246ab4a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 13:52:46 +1300 Subject: [PATCH 05/31] tidy --- src/http-api/fetch.ts | 38 +++++--------------------------------- src/http-api/interface.ts | 2 +- src/oidc/tokenRefresher.ts | 36 ++++++++++++++++++++++++------------ 3 files changed, 30 insertions(+), 46 deletions(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 816721fd70b..61491de2e48 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -133,7 +133,7 @@ export class FetchHttpApi { path: string, queryParams?: QueryDict, body?: Body, - paramOpts: IRequestOpts = {}, + paramOpts: IRequestOpts & { doNotAttemptTokenRefresh?: boolean } = {}, ): Promise> { if (!queryParams) queryParams = {}; @@ -161,23 +161,17 @@ export class FetchHttpApi { } catch (error) { const err = error as MatrixError; - console.log('HHHHHHHHHH CATCH', err.errcode, err, opts, opts.retryWithRefreshedToken); - if (err.errcode === "M_UNKNOWN_TOKEN" && !opts.retryWithRefreshedToken) { + if (err.errcode === "M_UNKNOWN_TOKEN" && !opts.doNotAttemptTokenRefresh) { const shouldRetry = await this.tryRefreshToken(); // if we got a new token retry the request if (shouldRetry) { - // we set a new token successfully - console.log('hhh', 'retrying request after refreshing token', this.opts.accessToken); - // @TODO(kerrya) don't mutate params (opts, qp) - // this function modifies these params :/ so the retry call may be with different params return this.authedRequest(method, path, queryParams, body, { ...paramOpts, - retryWithRefreshedToken: true + 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") { @@ -263,7 +257,7 @@ export class FetchHttpApi { method: Method, url: URL | string, body?: Body, - opts: Pick = {}, + opts: Pick & {doNotAttemptTokenRefresh?: boolean} = {}, ): Promise> { const urlForLogs = this.sanitizeUrlForLogs(url); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); @@ -332,29 +326,7 @@ export class FetchHttpApi { } if (!res.ok) { - const err = parseErrorResponse(res, await res.text()); - - if (err.errcode === "M_UNKNOWN_TOKEN" && !!this.opts.tokenRefresher && !opts.retryWithRefreshedToken) { - const setToken = (newToken) => { - console.log('hhhh refreshToken.setting token', newToken, this); - this.opts.accessToken = newToken; - } - const newToken = await this.opts.tokenRefresher.doRefreshAccessToken(); - // we set a new token successfully - console.log('hhh', 'retrying request after refreshing token', this.opts.accessToken); - - this.opts.accessToken = newToken; - - return this.requestOtherUrl(method, url, body, { - ...opts, retryWithRefreshedToken: true, - headers: { - ...opts.headers, - Authorization: "Bearer " + this.opts.accessToken - } - }); - } else { - throw err; - } + throw parseErrorResponse(res, await res.text()); } if (this.opts.onlyData) { diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index 3dfcc5cb466..ddb9f48bb1b 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -19,7 +19,7 @@ import { MatrixError } from "./errors"; export type Body = Record | BodyInit; export type TokenRefreshFunction = (refreshToken: string) => Promise<{ - accessToken: string; refreshToken: string; + accessToken: string; refreshToken?: string; }> export interface IHttpOpts { fetchFn?: typeof global.fetch; diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index fce55acf985..c8e862b5556 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -14,18 +14,17 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { OidcClient, SigninResponse, UseRefreshTokenArgs, WebStorageStateStore } from "oidc-client-ts"; +import { OidcClient, WebStorageStateStore } from "oidc-client-ts"; +import { TokenRefreshFunction } from ".."; import { IDelegatedAuthConfig } from "../client"; -import { MatrixClient } from "../client"; import { generateScope } from "./authorize"; import { discoverAndValidateAuthenticationConfig } from "./discovery"; -export class OidcTokenRefresher { +export abstract class OidcTokenRefresher { private oidcClient!: OidcClient; constructor( - private refreshToken: string, authConfig: IDelegatedAuthConfig, clientId: string, redirectUri: string, @@ -50,30 +49,43 @@ export class OidcTokenRefresher { }); } - public async doRefreshAccessToken (): Promise { + public async doRefreshAccessToken (refreshToken: string): ReturnType { // @TODO something here with only one inflight refresh attempt - return this.getNewToken(); + const tokens = await this.getNewToken(refreshToken); + + // await this.persistTokens(tokens); + + return tokens; } - private async getNewToken(): Promise { + /** + * Persist the new tokens after successfully refreshing + * @param accessToken new access token + * @param refreshToken OPTIONAL new refresh token + */ + public abstract persistTokens({ accessToken, refreshToken }: { + accessToken: string, refreshToken?: string + }): Promise; + + private async getNewToken(refreshToken: string): ReturnType { if (!this.oidcClient) { throw new Error("No client TODO") } const refreshTokenState = { - refresh_token: this.refreshToken, + refresh_token: refreshToken, session_state: 'test', data: undefined, } const response = await this.oidcClient.useRefreshToken({ state: refreshTokenState, timeoutInSeconds: 300 }); - this.refreshToken = response.refresh_token; - this.expiresAt = response.expires_at; - // TODO persist tokens in storage console.log('hhhh doRefreshAccessToken', response); - return response.access_token; + return { + accessToken: response.access_token, + refreshToken: response.refresh_token, + } } } \ No newline at end of file From 0299230cb84a8e58cb8cf76473991c13c38785db Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 16:34:43 +1300 Subject: [PATCH 06/31] add claims to completeauhtorizationcodegrant response --- spec/unit/oidc/authorize.spec.ts | 2 ++ src/oidc/authorize.ts | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/unit/oidc/authorize.spec.ts b/spec/unit/oidc/authorize.spec.ts index ffd22148dd1..fdab18d0950 100644 --- a/spec/unit/oidc/authorize.spec.ts +++ b/spec/unit/oidc/authorize.spec.ts @@ -284,6 +284,7 @@ describe("oidc authorization", () => { expires_at: result.tokenResponse.expires_at, scope, }, + idTokenClaims: result.idTokenClaims, }); }); @@ -325,6 +326,7 @@ describe("oidc authorization", () => { expires_at: result.tokenResponse.expires_at, scope, }, + idTokenClaims: result.idTokenClaims, }); expect(result.tokenResponse.token_type).toEqual("Bearer"); diff --git a/src/oidc/authorize.ts b/src/oidc/authorize.ts index fbcfeaae0d1..c8f11476833 100644 --- a/src/oidc/authorize.ts +++ b/src/oidc/authorize.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Log, OidcClient, SigninResponse, SigninState, WebStorageStateStore } from "oidc-client-ts"; +import { IdTokenClaims, Log, OidcClient, SigninResponse, SigninState, WebStorageStateStore } from "oidc-client-ts"; import { IDelegatedAuthConfig } from "../client"; import { subtleCrypto, TextEncoder } from "../crypto/crypto"; @@ -199,6 +199,7 @@ export const completeAuthorizationCodeGrant = async ( oidcClientSettings: IDelegatedAuthConfig & { clientId: string }; tokenResponse: BearerTokenResponse; homeserverUrl: string; + idTokenClaims: IdTokenClaims; identityServerUrl?: string; }> => { /** @@ -250,6 +251,7 @@ export const completeAuthorizationCodeGrant = async ( tokenResponse: normalizedTokenResponse, homeserverUrl: userState.homeserverUrl, identityServerUrl: userState.identityServerUrl, + idTokenClaims: signinResponse.profile, }; } catch (error) { logger.error("Oidc login failed", error); From 05611aafe1ef48743c0c58d9a356023cbd440675 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 16:35:44 +1300 Subject: [PATCH 07/31] export tokenrefresher from matrix --- src/matrix.ts | 1 + src/oidc/index.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 src/oidc/index.ts diff --git a/src/matrix.ts b/src/matrix.ts index 26ca8c36b18..20f894a2dc1 100644 --- a/src/matrix.ts +++ b/src/matrix.ts @@ -42,6 +42,7 @@ export * from "./models/typed-event-emitter"; export * from "./models/user"; export * from "./models/device"; export * from "./models/search-result"; +export * from "./oidc"; export * from "./scheduler"; export * from "./filter"; export * from "./timeline-window"; diff --git a/src/oidc/index.ts b/src/oidc/index.ts new file mode 100644 index 00000000000..81ae1833b94 --- /dev/null +++ b/src/oidc/index.ts @@ -0,0 +1,17 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +export * from "./tokenRefresher"; From f534fe2a54702aa30d35180b63a33db9d8d991da Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 16:37:02 +1300 Subject: [PATCH 08/31] add idtokenclaims --- src/http-api/fetch.ts | 13 +++++---- src/http-api/interface.ts | 5 ++-- src/oidc/tokenRefresher.ts | 55 +++++++++++++++++++++++--------------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 61491de2e48..d7abb32ba85 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -47,7 +47,6 @@ export class FetchHttpApi { checkObjectHasKeys(opts, ["baseUrl", "prefix"]); opts.onlyData = !!opts.onlyData; opts.useAuthorizationHeader = opts.useAuthorizationHeader ?? true; - } public abort(): void { @@ -137,7 +136,7 @@ export class FetchHttpApi { ): Promise> { if (!queryParams) queryParams = {}; - let opts = { ...paramOpts }; + const opts = { ...paramOpts }; if (this.opts.accessToken) { if (this.opts.useAuthorizationHeader) { @@ -167,7 +166,7 @@ export class FetchHttpApi { if (shouldRetry) { return this.authedRequest(method, path, queryParams, body, { ...paramOpts, - doNotAttemptTokenRefresh: true + doNotAttemptTokenRefresh: true, }); } } @@ -188,9 +187,7 @@ export class FetchHttpApi { } try { - const { - accessToken, refreshToken, - } = await this.opts.tokenRefreshFunction(this.opts.refreshToken); + const { accessToken, refreshToken } = await this.opts.tokenRefreshFunction(this.opts.refreshToken); this.opts.accessToken = accessToken; this.opts.refreshToken = refreshToken; // successfully got new tokens @@ -257,7 +254,9 @@ export class FetchHttpApi { method: Method, url: URL | string, body?: Body, - opts: Pick & {doNotAttemptTokenRefresh?: boolean} = {}, + opts: Pick & { + doNotAttemptTokenRefresh?: boolean; + } = {}, ): Promise> { const urlForLogs = this.sanitizeUrlForLogs(url); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index ddb9f48bb1b..dc3637d15c4 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -19,8 +19,9 @@ import { MatrixError } from "./errors"; export type Body = Record | BodyInit; export type TokenRefreshFunction = (refreshToken: string) => Promise<{ - accessToken: string; refreshToken?: string; -}> + accessToken: string; + refreshToken?: string; +}>; export interface IHttpOpts { fetchFn?: typeof global.fetch; diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index c8e862b5556..50e67e7ab6c 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -14,26 +14,34 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { OidcClient, WebStorageStateStore } from "oidc-client-ts"; -import { TokenRefreshFunction } from ".."; -import { IDelegatedAuthConfig } from "../client"; +import { IdTokenClaims, OidcClient, WebStorageStateStore } from "oidc-client-ts"; +import { TokenRefreshFunction } from "../http-api"; +import { IDelegatedAuthConfig } from "../client"; import { generateScope } from "./authorize"; import { discoverAndValidateAuthenticationConfig } from "./discovery"; export abstract class OidcTokenRefresher { private oidcClient!: OidcClient; + private idTokenClaims!: IdTokenClaims; - constructor( + public constructor( authConfig: IDelegatedAuthConfig, clientId: string, redirectUri: string, deviceId: string, + idTokenClaims: IdTokenClaims, ) { this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri); + this.idTokenClaims = idTokenClaims; } - private async initialiseOidcClient(authConfig: IDelegatedAuthConfig, clientId: string, deviceId: string, redirectUri: string): Promise { + private async initialiseOidcClient( + authConfig: IDelegatedAuthConfig, + clientId: string, + deviceId: string, + redirectUri: string, + ): Promise { const config = await discoverAndValidateAuthenticationConfig(authConfig); const scope = await generateScope(deviceId); @@ -44,48 +52,51 @@ export abstract class OidcTokenRefresher { scope, redirect_uri: redirectUri, authority: config.metadata.issuer, - // @TODO(kerrya) need this? stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }), }); } - public async doRefreshAccessToken (refreshToken: string): ReturnType { - // @TODO something here with only one inflight refresh attempt + public async doRefreshAccessToken(refreshToken: string): ReturnType { + // @TODO(kerrya) something here with only one inflight refresh attempt const tokens = await this.getNewToken(refreshToken); - // await this.persistTokens(tokens); + await this.persistTokens(tokens); return tokens; } /** * Persist the new tokens after successfully refreshing - * @param accessToken new access token - * @param refreshToken OPTIONAL new refresh token + * @param accessToken - new access token + * @param refreshToken - OPTIONAL new refresh token */ - public abstract persistTokens({ accessToken, refreshToken }: { - accessToken: string, refreshToken?: string + public abstract persistTokens({ + accessToken, + refreshToken, + }: { + accessToken: string; + refreshToken?: string; }): Promise; private async getNewToken(refreshToken: string): ReturnType { if (!this.oidcClient) { - throw new Error("No client TODO") + throw new Error("No client TODO"); } const refreshTokenState = { refresh_token: refreshToken, - session_state: 'test', + session_state: "test", data: undefined, - } + profile: this.idTokenClaims, + }; const response = await this.oidcClient.useRefreshToken({ - state: refreshTokenState, timeoutInSeconds: 300 }); - - // TODO persist tokens in storage - console.log('hhhh doRefreshAccessToken', response); + state: refreshTokenState, + timeoutInSeconds: 300, + }); return { accessToken: response.access_token, refreshToken: response.refresh_token, - } + }; } -} \ No newline at end of file +} From 2a09a57b91adab41a40fbbedc37235ca537ee217 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 16:34:43 +1300 Subject: [PATCH 09/31] add claims to completeauhtorizationcodegrant response --- spec/unit/oidc/authorize.spec.ts | 2 ++ src/oidc/authorize.ts | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/unit/oidc/authorize.spec.ts b/spec/unit/oidc/authorize.spec.ts index ffd22148dd1..fdab18d0950 100644 --- a/spec/unit/oidc/authorize.spec.ts +++ b/spec/unit/oidc/authorize.spec.ts @@ -284,6 +284,7 @@ describe("oidc authorization", () => { expires_at: result.tokenResponse.expires_at, scope, }, + idTokenClaims: result.idTokenClaims, }); }); @@ -325,6 +326,7 @@ describe("oidc authorization", () => { expires_at: result.tokenResponse.expires_at, scope, }, + idTokenClaims: result.idTokenClaims, }); expect(result.tokenResponse.token_type).toEqual("Bearer"); diff --git a/src/oidc/authorize.ts b/src/oidc/authorize.ts index df802aa0a4f..d6ca942e6f2 100644 --- a/src/oidc/authorize.ts +++ b/src/oidc/authorize.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Log, OidcClient, SigninResponse, SigninState, WebStorageStateStore } from "oidc-client-ts"; +import { IdTokenClaims, Log, OidcClient, SigninResponse, SigninState, WebStorageStateStore } from "oidc-client-ts"; import { IDelegatedAuthConfig } from "../client"; import { subtleCrypto, TextEncoder } from "../crypto/crypto"; @@ -199,6 +199,7 @@ export const completeAuthorizationCodeGrant = async ( oidcClientSettings: IDelegatedAuthConfig & { clientId: string }; tokenResponse: BearerTokenResponse; homeserverUrl: string; + idTokenClaims: IdTokenClaims; identityServerUrl?: string; }> => { /** @@ -250,6 +251,7 @@ export const completeAuthorizationCodeGrant = async ( tokenResponse: normalizedTokenResponse, homeserverUrl: userState.homeserverUrl, identityServerUrl: userState.identityServerUrl, + idTokenClaims: signinResponse.profile, }; } catch (error) { logger.error("Oidc login failed", error); From b4dec8080df82e6962062e8a534c9687568508a1 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 11:40:43 +1300 Subject: [PATCH 10/31] only one token refresh attempt at a time --- src/oidc/tokenRefresher.ts | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index 50e67e7ab6c..53fae166a55 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -20,10 +20,16 @@ import { TokenRefreshFunction } from "../http-api"; import { IDelegatedAuthConfig } from "../client"; import { generateScope } from "./authorize"; import { discoverAndValidateAuthenticationConfig } from "./discovery"; +import { logger } from "../logger"; +/** + * @experimental + */ export abstract class OidcTokenRefresher { + public readonly oidcClientReady!: Promise; private oidcClient!: OidcClient; private idTokenClaims!: IdTokenClaims; + private inflightRefreshRequest?: ReturnType; public constructor( authConfig: IDelegatedAuthConfig, @@ -32,7 +38,7 @@ export abstract class OidcTokenRefresher { deviceId: string, idTokenClaims: IdTokenClaims, ) { - this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri); + this.oidcClientReady = this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri); this.idTokenClaims = idTokenClaims; } @@ -44,7 +50,7 @@ export abstract class OidcTokenRefresher { ): Promise { const config = await discoverAndValidateAuthenticationConfig(authConfig); - const scope = await generateScope(deviceId); + const scope = generateScope(deviceId); this.oidcClient = new OidcClient({ ...config.metadata, @@ -57,12 +63,19 @@ export abstract class OidcTokenRefresher { } public async doRefreshAccessToken(refreshToken: string): ReturnType { - // @TODO(kerrya) something here with only one inflight refresh attempt - const tokens = await this.getNewToken(refreshToken); - - await this.persistTokens(tokens); - - return tokens; + if (!this.inflightRefreshRequest) { + this.inflightRefreshRequest = this.getNewToken(refreshToken); + } + try { + const tokens = await this.inflightRefreshRequest; + await this.persistTokens(tokens); + return tokens; + } catch (error) { + logger.error("Failed to refresh access token", error); + throw error; + } finally { + this.inflightRefreshRequest = undefined; + } } /** @@ -80,7 +93,7 @@ export abstract class OidcTokenRefresher { private async getNewToken(refreshToken: string): ReturnType { if (!this.oidcClient) { - throw new Error("No client TODO"); + throw new Error("Cannot get new token before OIDC client is initialised."); } const refreshTokenState = { @@ -89,6 +102,7 @@ export abstract class OidcTokenRefresher { data: undefined, profile: this.idTokenClaims, }; + const response = await this.oidcClient.useRefreshToken({ state: refreshTokenState, timeoutInSeconds: 300, From a3bb5fec83f884a1574ca9080bb24a3d23e9c7f4 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 11:40:48 +1300 Subject: [PATCH 11/31] tests --- spec/unit/oidc/tokenRefresher.spec.ts | 266 ++++++++++++++++++++++++++ 1 file changed, 266 insertions(+) create mode 100644 spec/unit/oidc/tokenRefresher.spec.ts diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts new file mode 100644 index 00000000000..6eb2a0fda72 --- /dev/null +++ b/spec/unit/oidc/tokenRefresher.spec.ts @@ -0,0 +1,266 @@ +/** + * @jest-environment jsdom + */ + +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import fetchMock from "fetch-mock-jest"; +import { IdTokenClaims } from "oidc-client-ts"; + +import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src"; +import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; + +describe("TokenRefresher()", () => { + const authConfig = { + issuer: "https://issuer.org/", + }; + const clientId = "test-client-id"; + const redirectUri = "https://test.org"; + const deviceId = "abc123"; + const idTokenClaims = { + exp: Date.now() / 1000 + 100000, + aud: clientId, + iss: authConfig.issuer, + sub: "123", + iat: 123, + }; + const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; + class TestClass extends OidcTokenRefresher { + public constructor( + authConfig: IDelegatedAuthConfig, + clientId: string, + redirectUri: string, + deviceId: string, + idTokenClaims: IdTokenClaims, + ) { + super(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + } + + public async persistTokens(_tokens: { accessToken: string; refreshToken?: string | undefined }): Promise { + // NOOP + } + } + + const config = makeDelegatedAuthConfig(authConfig.issuer); + + const makeTokenResponse = (accessToken: string, refreshToken?: string) => ({ + access_token: accessToken, + refresh_token: refreshToken, + token_type: "Bearer", + expires_in: 300, + scope: scope, + }); + + beforeEach(() => { + 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: [], + }); + + fetchMock.post(config.metadata.token_endpoint, { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("new-access-token", "new-refresh-token"), + }); + }); + + afterEach(() => { + fetchMock.resetBehavior(); + }); + + it("initialises oidc client", async () => { + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + + // @ts-ignore peek at private property to see we initialised the client correctly + expect(refresher.oidcClient.settings).toEqual( + expect.objectContaining({ + client_id: clientId, + redirect_uri: redirectUri, + authority: authConfig.issuer, + scope, + }), + ); + }); + + describe("doRefreshAccessToken()", () => { + it("should throw when oidcClient has not been initialised", async () => { + const refresher = new TestClass(authConfig, 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 TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + + const refreshToken = "refresh-token"; + const result = await refresher.doRefreshAccessToken(refreshToken); + + expect(fetchMock).toHaveFetched(config.metadata.token_endpoint, { + method: "POST", + }); + + expect(result).toEqual({ + accessToken: "new-access-token", + refreshToken: "new-refresh-token", + }); + }); + + it("should persist the new tokens", async () => { + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + // spy on our stub + jest.spyOn(refresher, "persistTokens"); + + const refreshToken = "refresh-token"; + await refresher.doRefreshAccessToken(refreshToken); + + expect(refresher.persistTokens).toHaveBeenCalledWith({ + accessToken: "new-access-token", + refreshToken: "new-refresh-token", + }); + }); + + it("should only have one inflight refresh request at once", async () => { + fetchMock + .postOnce( + config.metadata.token_endpoint, + { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("first-new-access-token", "first-new-refresh-token"), + }, + { overwriteRoutes: true }, + ) + .postOnce( + config.metadata.token_endpoint, + { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("second-new-access-token", "second-new-refresh-token"), + }, + { overwriteRoutes: false }, + ); + + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + // reset call counts + fetchMock.resetHistory(); + + const refreshToken = "refresh-token"; + const first = refresher.doRefreshAccessToken(refreshToken); + const second = refresher.doRefreshAccessToken(refreshToken); + + const result1 = await second; + const result2 = await first; + + // only one call to token endpoint + expect(fetchMock).toHaveFetchedTimes(1, config.metadata.token_endpoint); + expect(result1).toEqual({ + accessToken: "first-new-access-token", + refreshToken: "first-new-refresh-token", + }); + // same response + expect(result1).toEqual(result2); + + // call again after first request resolves + const third = await refresher.doRefreshAccessToken("first-new-refresh-token"); + + // called token endpoint, got new tokens + expect(third).toEqual({ + accessToken: "second-new-access-token", + refreshToken: "second-new-refresh-token", + }); + }); + + it("should log and rethrow when token refresh fails", async () => { + fetchMock.post( + config.metadata.token_endpoint, + { + status: 503, + headers: { + "Content-Type": "application/json", + }, + }, + { overwriteRoutes: true }, + ); + + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + + const refreshToken = "refreshToken"; + await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow(); + }); + + it("should make fresh request after a failed request", async () => { + // make sure inflight request is cleared after a failure + fetchMock + .postOnce( + config.metadata.token_endpoint, + { + status: 503, + headers: { + "Content-Type": "application/json", + }, + }, + { overwriteRoutes: true }, + ) + .postOnce( + config.metadata.token_endpoint, + { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("second-new-access-token", "second-new-refresh-token"), + }, + { overwriteRoutes: false }, + ); + + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + // reset call counts + fetchMock.resetHistory(); + + const refreshToken = "refresh-token"; + + // first call fails + await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow(); + + // call again after first request resolves + const result = await refresher.doRefreshAccessToken("first-new-refresh-token"); + + // called token endpoint, got new tokens + expect(result).toEqual({ + accessToken: "second-new-access-token", + refreshToken: "second-new-refresh-token", + }); + }); + }); +}); From 2c6eafa3a5b7647f19d4245a4b480acab6da9f0f Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 11:52:44 +1300 Subject: [PATCH 12/31] comments --- src/oidc/tokenRefresher.ts | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index 53fae166a55..4ca30c1da4b 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -24,22 +24,34 @@ import { logger } from "../logger"; /** * @experimental + * Class responsible to refreshing OIDC access tokens */ export abstract class OidcTokenRefresher { public readonly oidcClientReady!: Promise; private oidcClient!: OidcClient; - private idTokenClaims!: IdTokenClaims; private inflightRefreshRequest?: ReturnType; public constructor( + /** + * Delegated auth config as found in matrix client .well-known + */ authConfig: IDelegatedAuthConfig, + /** + * id of this client as registered with the OP + */ clientId: string, + /** + * redirectUri as registered with OP + */ redirectUri: string, deviceId: string, - idTokenClaims: IdTokenClaims, + /** + * idTokenClaims as returned from authorization grant + * used to validate tokens + */ + private readonly idTokenClaims: IdTokenClaims, ) { this.oidcClientReady = this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri); - this.idTokenClaims = idTokenClaims; } private async initialiseOidcClient( @@ -62,6 +74,13 @@ export abstract class OidcTokenRefresher { }); } + /** + * Attempt token refresh using given access token + * When successful, persist new tokens + * @param refreshToken - refresh token to use in request with token issuer + * @returns tokens - Promise that resolves with new access and refresh tokens + * @throws when token refresh fails + */ public async doRefreshAccessToken(refreshToken: string): ReturnType { if (!this.inflightRefreshRequest) { this.inflightRefreshRequest = this.getNewToken(refreshToken); From 97c0f717d60624d70cfd1a5831562e1cd98c8291 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 12:20:34 +1300 Subject: [PATCH 13/31] add tokenRefresher class --- spec/unit/oidc/tokenRefresher.spec.ts | 266 ++++++++++++++++++++++++++ src/http-api/interface.ts | 4 + src/oidc/tokenRefresher.ts | 135 +++++++++++++ 3 files changed, 405 insertions(+) create mode 100644 spec/unit/oidc/tokenRefresher.spec.ts create mode 100644 src/oidc/tokenRefresher.ts diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts new file mode 100644 index 00000000000..6eb2a0fda72 --- /dev/null +++ b/spec/unit/oidc/tokenRefresher.spec.ts @@ -0,0 +1,266 @@ +/** + * @jest-environment jsdom + */ + +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import fetchMock from "fetch-mock-jest"; +import { IdTokenClaims } from "oidc-client-ts"; + +import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src"; +import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; + +describe("TokenRefresher()", () => { + const authConfig = { + issuer: "https://issuer.org/", + }; + const clientId = "test-client-id"; + const redirectUri = "https://test.org"; + const deviceId = "abc123"; + const idTokenClaims = { + exp: Date.now() / 1000 + 100000, + aud: clientId, + iss: authConfig.issuer, + sub: "123", + iat: 123, + }; + const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; + class TestClass extends OidcTokenRefresher { + public constructor( + authConfig: IDelegatedAuthConfig, + clientId: string, + redirectUri: string, + deviceId: string, + idTokenClaims: IdTokenClaims, + ) { + super(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + } + + public async persistTokens(_tokens: { accessToken: string; refreshToken?: string | undefined }): Promise { + // NOOP + } + } + + const config = makeDelegatedAuthConfig(authConfig.issuer); + + const makeTokenResponse = (accessToken: string, refreshToken?: string) => ({ + access_token: accessToken, + refresh_token: refreshToken, + token_type: "Bearer", + expires_in: 300, + scope: scope, + }); + + beforeEach(() => { + 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: [], + }); + + fetchMock.post(config.metadata.token_endpoint, { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("new-access-token", "new-refresh-token"), + }); + }); + + afterEach(() => { + fetchMock.resetBehavior(); + }); + + it("initialises oidc client", async () => { + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + + // @ts-ignore peek at private property to see we initialised the client correctly + expect(refresher.oidcClient.settings).toEqual( + expect.objectContaining({ + client_id: clientId, + redirect_uri: redirectUri, + authority: authConfig.issuer, + scope, + }), + ); + }); + + describe("doRefreshAccessToken()", () => { + it("should throw when oidcClient has not been initialised", async () => { + const refresher = new TestClass(authConfig, 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 TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + + const refreshToken = "refresh-token"; + const result = await refresher.doRefreshAccessToken(refreshToken); + + expect(fetchMock).toHaveFetched(config.metadata.token_endpoint, { + method: "POST", + }); + + expect(result).toEqual({ + accessToken: "new-access-token", + refreshToken: "new-refresh-token", + }); + }); + + it("should persist the new tokens", async () => { + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + // spy on our stub + jest.spyOn(refresher, "persistTokens"); + + const refreshToken = "refresh-token"; + await refresher.doRefreshAccessToken(refreshToken); + + expect(refresher.persistTokens).toHaveBeenCalledWith({ + accessToken: "new-access-token", + refreshToken: "new-refresh-token", + }); + }); + + it("should only have one inflight refresh request at once", async () => { + fetchMock + .postOnce( + config.metadata.token_endpoint, + { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("first-new-access-token", "first-new-refresh-token"), + }, + { overwriteRoutes: true }, + ) + .postOnce( + config.metadata.token_endpoint, + { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("second-new-access-token", "second-new-refresh-token"), + }, + { overwriteRoutes: false }, + ); + + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + // reset call counts + fetchMock.resetHistory(); + + const refreshToken = "refresh-token"; + const first = refresher.doRefreshAccessToken(refreshToken); + const second = refresher.doRefreshAccessToken(refreshToken); + + const result1 = await second; + const result2 = await first; + + // only one call to token endpoint + expect(fetchMock).toHaveFetchedTimes(1, config.metadata.token_endpoint); + expect(result1).toEqual({ + accessToken: "first-new-access-token", + refreshToken: "first-new-refresh-token", + }); + // same response + expect(result1).toEqual(result2); + + // call again after first request resolves + const third = await refresher.doRefreshAccessToken("first-new-refresh-token"); + + // called token endpoint, got new tokens + expect(third).toEqual({ + accessToken: "second-new-access-token", + refreshToken: "second-new-refresh-token", + }); + }); + + it("should log and rethrow when token refresh fails", async () => { + fetchMock.post( + config.metadata.token_endpoint, + { + status: 503, + headers: { + "Content-Type": "application/json", + }, + }, + { overwriteRoutes: true }, + ); + + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + + const refreshToken = "refreshToken"; + await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow(); + }); + + it("should make fresh request after a failed request", async () => { + // make sure inflight request is cleared after a failure + fetchMock + .postOnce( + config.metadata.token_endpoint, + { + status: 503, + headers: { + "Content-Type": "application/json", + }, + }, + { overwriteRoutes: true }, + ) + .postOnce( + config.metadata.token_endpoint, + { + status: 200, + headers: { + "Content-Type": "application/json", + }, + ...makeTokenResponse("second-new-access-token", "second-new-refresh-token"), + }, + { overwriteRoutes: false }, + ); + + const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await refresher.oidcClientReady; + // reset call counts + fetchMock.resetHistory(); + + const refreshToken = "refresh-token"; + + // first call fails + await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow(); + + // call again after first request resolves + const result = await refresher.doRefreshAccessToken("first-new-refresh-token"); + + // called token endpoint, got new tokens + expect(result).toEqual({ + accessToken: "second-new-access-token", + refreshToken: "second-new-refresh-token", + }); + }); + }); +}); diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index 57e8a18e851..f927f368756 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -18,6 +18,10 @@ import { MatrixError } from "./errors"; export type Body = Record | BodyInit; +export type TokenRefreshFunction = (refreshToken: string) => Promise<{ + accessToken: string; + refreshToken?: string; +}>; export interface IHttpOpts { fetchFn?: typeof global.fetch; diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts new file mode 100644 index 00000000000..4ca30c1da4b --- /dev/null +++ b/src/oidc/tokenRefresher.ts @@ -0,0 +1,135 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { IdTokenClaims, OidcClient, WebStorageStateStore } from "oidc-client-ts"; + +import { TokenRefreshFunction } from "../http-api"; +import { IDelegatedAuthConfig } from "../client"; +import { generateScope } from "./authorize"; +import { discoverAndValidateAuthenticationConfig } from "./discovery"; +import { logger } from "../logger"; + +/** + * @experimental + * Class responsible to refreshing OIDC access tokens + */ +export abstract class OidcTokenRefresher { + public readonly oidcClientReady!: Promise; + private oidcClient!: OidcClient; + private inflightRefreshRequest?: ReturnType; + + public constructor( + /** + * Delegated auth config as found in matrix client .well-known + */ + authConfig: IDelegatedAuthConfig, + /** + * id of this client as registered with the OP + */ + clientId: string, + /** + * redirectUri as registered with OP + */ + redirectUri: string, + deviceId: string, + /** + * idTokenClaims as returned from authorization grant + * used to validate tokens + */ + private readonly idTokenClaims: IdTokenClaims, + ) { + this.oidcClientReady = this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri); + } + + private async initialiseOidcClient( + authConfig: IDelegatedAuthConfig, + clientId: string, + deviceId: string, + redirectUri: string, + ): Promise { + const config = await discoverAndValidateAuthenticationConfig(authConfig); + + const scope = generateScope(deviceId); + + this.oidcClient = new OidcClient({ + ...config.metadata, + client_id: clientId, + scope, + redirect_uri: redirectUri, + authority: config.metadata.issuer, + stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }), + }); + } + + /** + * Attempt token refresh using given access token + * When successful, persist new tokens + * @param refreshToken - refresh token to use in request with token issuer + * @returns tokens - Promise that resolves with new access and refresh tokens + * @throws when token refresh fails + */ + public async doRefreshAccessToken(refreshToken: string): ReturnType { + if (!this.inflightRefreshRequest) { + this.inflightRefreshRequest = this.getNewToken(refreshToken); + } + try { + const tokens = await this.inflightRefreshRequest; + await this.persistTokens(tokens); + return tokens; + } catch (error) { + logger.error("Failed to refresh access token", error); + throw error; + } finally { + this.inflightRefreshRequest = undefined; + } + } + + /** + * Persist the new tokens after successfully refreshing + * @param accessToken - new access token + * @param refreshToken - OPTIONAL new refresh token + */ + public abstract persistTokens({ + accessToken, + refreshToken, + }: { + accessToken: string; + refreshToken?: string; + }): Promise; + + private async getNewToken(refreshToken: string): ReturnType { + if (!this.oidcClient) { + throw new Error("Cannot get new token before OIDC client is initialised."); + } + + const refreshTokenState = { + refresh_token: refreshToken, + session_state: "test", + data: undefined, + profile: this.idTokenClaims, + }; + + const response = await this.oidcClient.useRefreshToken({ + state: refreshTokenState, + timeoutInSeconds: 300, + }); + + return { + accessToken: response.access_token, + refreshToken: response.refresh_token, + }; + } +} From f499c9edfd5022d55c66c74145ebe93c8842666b Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 12:20:50 +1300 Subject: [PATCH 14/31] export generateScope --- src/oidc/authorize.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/oidc/authorize.ts b/src/oidc/authorize.ts index df802aa0a4f..0b49da21cab 100644 --- a/src/oidc/authorize.ts +++ b/src/oidc/authorize.ts @@ -51,9 +51,9 @@ export type AuthorizationParams = { * Generate the scope used in authorization request with OIDC OP * @returns scope */ -const generateScope = (): string => { - const deviceId = randomString(10); - return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; +export const generateScope = (deviceId?: string): string => { + const safeDeviceId = deviceId ?? randomString(10); + return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${safeDeviceId}`; }; // https://www.rfc-editor.org/rfc/rfc7636 From 4879f941528af514ca57352718edc0bb633b641c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 12:20:59 +1300 Subject: [PATCH 15/31] export oidc from matrix --- src/matrix.ts | 1 + src/oidc/index.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 src/oidc/index.ts diff --git a/src/matrix.ts b/src/matrix.ts index 26ca8c36b18..20f894a2dc1 100644 --- a/src/matrix.ts +++ b/src/matrix.ts @@ -42,6 +42,7 @@ export * from "./models/typed-event-emitter"; export * from "./models/user"; export * from "./models/device"; export * from "./models/search-result"; +export * from "./oidc"; export * from "./scheduler"; export * from "./filter"; export * from "./timeline-window"; diff --git a/src/oidc/index.ts b/src/oidc/index.ts new file mode 100644 index 00000000000..81ae1833b94 --- /dev/null +++ b/src/oidc/index.ts @@ -0,0 +1,17 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +export * from "./tokenRefresher"; From 90c19a1e609e4ee50adbcabb38b9d72ae88be5f2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 15:15:58 +1300 Subject: [PATCH 16/31] test refreshtoken --- spec/unit/http-api/fetch.spec.ts | 141 ++++++++++++++++++++++++++++++- 1 file changed, 140 insertions(+), 1 deletion(-) diff --git a/spec/unit/http-api/fetch.spec.ts b/spec/unit/http-api/fetch.spec.ts index 434d507dafc..62b2a0bcf0e 100644 --- a/spec/unit/http-api/fetch.spec.ts +++ b/spec/unit/http-api/fetch.spec.ts @@ -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"; @@ -238,6 +246,137 @@ describe("FetchHttpApi", () => { 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(); + 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(); + 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(); + 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(); + 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()", () => { From f5168ee39b2c9cd335d556d736f2452aec59ff7a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 4 Oct 2023 10:50:49 +1300 Subject: [PATCH 17/31] mark experimental --- src/http-api/interface.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index f927f368756..7d19cac6cdb 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -18,6 +18,9 @@ import { MatrixError } from "./errors"; export type Body = Record | BodyInit; +/** + * @experimental + */ export type TokenRefreshFunction = (refreshToken: string) => Promise<{ accessToken: string; refreshToken?: string; From 8d146ae7b3e5d58ad593e44ed5c597bebe810a27 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 4 Oct 2023 11:14:54 +1300 Subject: [PATCH 18/31] add getRefreshToken to client --- src/client.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/client.ts b/src/client.ts index ac32553348b..d631757f898 100644 --- a/src/client.ts +++ b/src/client.ts @@ -7706,6 +7706,14 @@ export class MatrixClient extends TypedEventEmitter Date: Thu, 5 Oct 2023 09:57:56 +1300 Subject: [PATCH 19/31] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- spec/unit/oidc/tokenRefresher.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts index 6eb2a0fda72..265eccc87c7 100644 --- a/spec/unit/oidc/tokenRefresher.spec.ts +++ b/spec/unit/oidc/tokenRefresher.spec.ts @@ -24,7 +24,7 @@ import { IdTokenClaims } from "oidc-client-ts"; import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src"; import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; -describe("TokenRefresher()", () => { +describe("OidcTokenRefresher", () => { const authConfig = { issuer: "https://issuer.org/", }; From c0ec8875252ad45d2b94f69087a9d26819cb3d1d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 5 Oct 2023 10:03:56 +1300 Subject: [PATCH 20/31] remove some vars in test --- spec/unit/oidc/tokenRefresher.spec.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts index 265eccc87c7..498e08af8f4 100644 --- a/spec/unit/oidc/tokenRefresher.spec.ts +++ b/spec/unit/oidc/tokenRefresher.spec.ts @@ -115,8 +115,7 @@ describe("OidcTokenRefresher", () => { const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; - const refreshToken = "refresh-token"; - const result = await refresher.doRefreshAccessToken(refreshToken); + const result = await refresher.doRefreshAccessToken("refresh-token"); expect(fetchMock).toHaveFetched(config.metadata.token_endpoint, { method: "POST", @@ -134,8 +133,7 @@ describe("OidcTokenRefresher", () => { // spy on our stub jest.spyOn(refresher, "persistTokens"); - const refreshToken = "refresh-token"; - await refresher.doRefreshAccessToken(refreshToken); + await refresher.doRefreshAccessToken("refresh-token"); expect(refresher.persistTokens).toHaveBeenCalledWith({ accessToken: "new-access-token", @@ -214,8 +212,7 @@ describe("OidcTokenRefresher", () => { const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; - const refreshToken = "refreshToken"; - await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow(); + await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow(); }); it("should make fresh request after a failed request", async () => { @@ -248,10 +245,8 @@ describe("OidcTokenRefresher", () => { // reset call counts fetchMock.resetHistory(); - const refreshToken = "refresh-token"; - // first call fails - await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow(); + await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow(); // call again after first request resolves const result = await refresher.doRefreshAccessToken("first-new-refresh-token"); From cc62fac31f9de3fdda126144dfb46e6c71685727 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 5 Oct 2023 10:49:20 +1300 Subject: [PATCH 21/31] make TokenRefresher un-abstract, comments and improvements --- spec/unit/oidc/tokenRefresher.spec.ts | 54 +++++++++-------- src/http-api/interface.ts | 8 ++- src/oidc/tokenRefresher.ts | 84 ++++++++++++++++----------- 3 files changed, 85 insertions(+), 61 deletions(-) diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts index 498e08af8f4..85737f1b679 100644 --- a/spec/unit/oidc/tokenRefresher.spec.ts +++ b/spec/unit/oidc/tokenRefresher.spec.ts @@ -19,9 +19,9 @@ limitations under the License. */ import fetchMock from "fetch-mock-jest"; -import { IdTokenClaims } from "oidc-client-ts"; -import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src"; +import { OidcTokenRefresher } from "../../../src"; +import { logger } from "../../../src/logger"; import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; describe("OidcTokenRefresher", () => { @@ -38,22 +38,8 @@ describe("OidcTokenRefresher", () => { sub: "123", iat: 123, }; + // used to mock a valid token response, as consumed by OidcClient library const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; - class TestClass extends OidcTokenRefresher { - public constructor( - authConfig: IDelegatedAuthConfig, - clientId: string, - redirectUri: string, - deviceId: string, - idTokenClaims: IdTokenClaims, - ) { - super(authConfig, clientId, redirectUri, deviceId, idTokenClaims); - } - - public async persistTokens(_tokens: { accessToken: string; refreshToken?: string | undefined }): Promise { - // NOOP - } - } const config = makeDelegatedAuthConfig(authConfig.issuer); @@ -85,11 +71,31 @@ describe("OidcTokenRefresher", () => { }); afterEach(() => { + jest.restoreAllMocks(); fetchMock.resetBehavior(); }); + it("throws when oidc client cannot be initialised", async () => { + jest.spyOn(logger, "error"); + fetchMock.get( + `${config.metadata.issuer}.well-known/openid-configuration`, + { + ok: false, + status: 404, + }, + { overwriteRoutes: true }, + ); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + await expect(refresher.oidcClientReady).rejects.toThrow(); + expect(logger.error).toHaveBeenCalledWith( + "Failed to initialise OIDC client.", + // error from OidcClient + expect.any(Error), + ); + }); + it("initialises oidc client", async () => { - const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; // @ts-ignore peek at private property to see we initialised the client correctly @@ -105,14 +111,14 @@ describe("OidcTokenRefresher", () => { describe("doRefreshAccessToken()", () => { it("should throw when oidcClient has not been initialised", async () => { - const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, 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 TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; const result = await refresher.doRefreshAccessToken("refresh-token"); @@ -128,7 +134,7 @@ describe("OidcTokenRefresher", () => { }); it("should persist the new tokens", async () => { - const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; // spy on our stub jest.spyOn(refresher, "persistTokens"); @@ -166,7 +172,7 @@ describe("OidcTokenRefresher", () => { { overwriteRoutes: false }, ); - const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; // reset call counts fetchMock.resetHistory(); @@ -209,7 +215,7 @@ describe("OidcTokenRefresher", () => { { overwriteRoutes: true }, ); - const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow(); @@ -240,7 +246,7 @@ describe("OidcTokenRefresher", () => { { overwriteRoutes: false }, ); - const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims); + const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims); await refresher.oidcClientReady; // reset call counts fetchMock.resetHistory(); diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index 7d19cac6cdb..e068894f7b1 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -21,10 +21,14 @@ export type Body = Record | BodyInit; /** * @experimental */ -export type TokenRefreshFunction = (refreshToken: string) => Promise<{ +export type AccessTokens = { accessToken: string; refreshToken?: string; -}>; +}; +/** + * @experimental + */ +export type TokenRefreshFunction = (refreshToken: string) => Promise; export interface IHttpOpts { fetchFn?: typeof global.fetch; diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index 4ca30c1da4b..50cb5653694 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -16,7 +16,7 @@ limitations under the License. import { IdTokenClaims, OidcClient, WebStorageStateStore } from "oidc-client-ts"; -import { TokenRefreshFunction } from "../http-api"; +import { AccessTokens } from "../http-api"; import { IDelegatedAuthConfig } from "../client"; import { generateScope } from "./authorize"; import { discoverAndValidateAuthenticationConfig } from "./discovery"; @@ -24,12 +24,21 @@ import { logger } from "../logger"; /** * @experimental - * Class responsible to refreshing OIDC access tokens + * Class responsible for refreshing OIDC access tokens + * + * Client implementations will likely want to override {@link persistTokens} to persist tokens after successful refresh + * + * You can then use {@link doRefreshAccessToken} in {@link ICreateClientOpts.tokenRefreshFunction}. */ -export abstract class OidcTokenRefresher { +export class OidcTokenRefresher { + /** + * Used to determine the OidcClient has been initialised + * and is ready to start refreshing tokens + * throws when client initialisation fails + */ public readonly oidcClientReady!: Promise; private oidcClient!: OidcClient; - private inflightRefreshRequest?: ReturnType; + private inflightRefreshRequest?: Promise; public constructor( /** @@ -44,6 +53,9 @@ export abstract class OidcTokenRefresher { * redirectUri as registered with OP */ redirectUri: string, + /** + * Device ID of current session + */ deviceId: string, /** * idTokenClaims as returned from authorization grant @@ -60,57 +72,55 @@ export abstract class OidcTokenRefresher { deviceId: string, redirectUri: string, ): Promise { - const config = await discoverAndValidateAuthenticationConfig(authConfig); - - const scope = generateScope(deviceId); - - this.oidcClient = new OidcClient({ - ...config.metadata, - client_id: clientId, - scope, - redirect_uri: redirectUri, - authority: config.metadata.issuer, - stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }), - }); + try { + const config = await discoverAndValidateAuthenticationConfig(authConfig); + + const scope = generateScope(deviceId); + + this.oidcClient = new OidcClient({ + ...config.metadata, + client_id: clientId, + scope, + redirect_uri: redirectUri, + authority: config.metadata.issuer, + stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }), + }); + } catch (error) { + logger.error("Failed to initialise OIDC client.", error); + throw new Error("Failed to initialise OIDC client."); + } } /** - * Attempt token refresh using given access token - * When successful, persist new tokens + * Attempt token refresh using given refresh token * @param refreshToken - refresh token to use in request with token issuer * @returns tokens - Promise that resolves with new access and refresh tokens * @throws when token refresh fails */ - public async doRefreshAccessToken(refreshToken: string): ReturnType { + public async doRefreshAccessToken(refreshToken: string): Promise { if (!this.inflightRefreshRequest) { - this.inflightRefreshRequest = this.getNewToken(refreshToken); + this.inflightRefreshRequest = this.getNewTokens(refreshToken); } try { const tokens = await this.inflightRefreshRequest; - await this.persistTokens(tokens); return tokens; - } catch (error) { - logger.error("Failed to refresh access token", error); - throw error; } finally { this.inflightRefreshRequest = undefined; } } /** - * Persist the new tokens after successfully refreshing + * Persist the new tokens + * Called after tokens are successfully refreshed + * This function is intended to be overriden by the consumer when persistence is necessary * @param accessToken - new access token * @param refreshToken - OPTIONAL new refresh token */ - public abstract persistTokens({ - accessToken, - refreshToken, - }: { - accessToken: string; - refreshToken?: string; - }): Promise; - - private async getNewToken(refreshToken: string): ReturnType { + public async persistTokens(_tokens: { accessToken: string; refreshToken?: string }): Promise { + // NOOP + } + + private async getNewTokens(refreshToken: string): Promise { if (!this.oidcClient) { throw new Error("Cannot get new token before OIDC client is initialised."); } @@ -127,9 +137,13 @@ export abstract class OidcTokenRefresher { timeoutInSeconds: 300, }); - return { + const tokens = { accessToken: response.access_token, refreshToken: response.refresh_token, }; + + await this.persistTokens(tokens); + + return tokens; } } From 43070f9e43542fb0058c1e2acf85a294533778a5 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 5 Oct 2023 13:51:30 +1300 Subject: [PATCH 22/31] remove invalid jsdoc --- src/oidc/tokenRefresher.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index 50cb5653694..e1087818dbd 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -28,7 +28,6 @@ import { logger } from "../logger"; * * Client implementations will likely want to override {@link persistTokens} to persist tokens after successful refresh * - * You can then use {@link doRefreshAccessToken} in {@link ICreateClientOpts.tokenRefreshFunction}. */ export class OidcTokenRefresher { /** From 4b1b5ef8a6d223eacc6f6812d6acc7026767b7e3 Mon Sep 17 00:00:00 2001 From: Kerry Date: Fri, 6 Oct 2023 10:53:07 +1300 Subject: [PATCH 23/31] Update src/oidc/tokenRefresher.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/oidc/tokenRefresher.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index e1087818dbd..70c62d49df4 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -31,9 +31,10 @@ import { logger } from "../logger"; */ export class OidcTokenRefresher { /** - * Used to determine the OidcClient has been initialised - * and is ready to start refreshing tokens - * throws when client initialisation fails + * Promise which will complete once the OidcClient has been initialised + * and is ready to start refreshing tokens. + * + * Will reject if the client initialisation fails. */ public readonly oidcClientReady!: Promise; private oidcClient!: OidcClient; From ac1a44bae8c18bacf9dac02aa68171e61ec2d499 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 6 Oct 2023 11:00:52 +1300 Subject: [PATCH 24/31] Code review improvements --- spec/unit/oidc/tokenRefresher.spec.ts | 3 +++ src/oidc/tokenRefresher.ts | 7 ++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts index 85737f1b679..803a63d9aac 100644 --- a/spec/unit/oidc/tokenRefresher.spec.ts +++ b/spec/unit/oidc/tokenRefresher.spec.ts @@ -25,6 +25,8 @@ import { logger } from "../../../src/logger"; import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; describe("OidcTokenRefresher", () => { + // OidcTokenRefresher props + // see class declaration for info const authConfig = { issuer: "https://issuer.org/", }; @@ -41,6 +43,7 @@ describe("OidcTokenRefresher", () => { // used to mock a valid token response, as consumed by OidcClient library const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; + // auth config used in mocked calls to OP .well-known const config = makeDelegatedAuthConfig(authConfig.issuer); const makeTokenResponse = (accessToken: string, refreshToken?: string) => ({ diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts index 70c62d49df4..10c9bc48ea3 100644 --- a/src/oidc/tokenRefresher.ts +++ b/src/oidc/tokenRefresher.ts @@ -110,9 +110,10 @@ export class OidcTokenRefresher { } /** - * Persist the new tokens - * Called after tokens are successfully refreshed - * This function is intended to be overriden by the consumer when persistence is necessary + * Persist the new tokens, called after tokens are successfully refreshed. + * + * This function is intended to be overriden by the consumer when persistence is necessary. + * * @param accessToken - new access token * @param refreshToken - OPTIONAL new refresh token */ From 5a70f655ff9aaa2834d2cb3b3639a8e768a2f966 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 6 Oct 2023 12:20:55 +1300 Subject: [PATCH 25/31] fix verification integ tests --- spec/integ/crypto/verification.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts index 918ed7cab67..e71b9ac5934 100644 --- a/spec/integ/crypto/verification.spec.ts +++ b/spec/integ/crypto/verification.spec.ts @@ -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. From 25be4bb256df0f8132863ccaebd50a1fa684b0ff Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 6 Oct 2023 12:58:40 +1300 Subject: [PATCH 26/31] remove unused type from props --- src/http-api/fetch.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index d7abb32ba85..7e425ab289d 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -254,9 +254,7 @@ export class FetchHttpApi { method: Method, url: URL | string, body?: Body, - opts: Pick & { - doNotAttemptTokenRefresh?: boolean; - } = {}, + opts: Pick = {}, ): Promise> { const urlForLogs = this.sanitizeUrlForLogs(url); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); From 445315d793105a336bdfa8647b5d3bfb1fb0f7ea Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 6 Oct 2023 12:58:50 +1300 Subject: [PATCH 27/31] fix incomplete mock fn in fetch.spec --- spec/unit/http-api/fetch.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/unit/http-api/fetch.spec.ts b/spec/unit/http-api/fetch.spec.ts index 62b2a0bcf0e..fd0c21b6753 100644 --- a/spec/unit/http-api/fetch.spec.ts +++ b/spec/unit/http-api/fetch.spec.ts @@ -239,11 +239,11 @@ 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(); 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(); }); @@ -272,6 +272,7 @@ describe("FetchHttpApi", () => { ok: true, status: 200, }; + describe("without a tokenRefreshFunction", () => { it("should emit logout and throw", async () => { const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse); From 2c2f0fb97c26962233cc292126e6e02941514431 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 9 Oct 2023 10:10:06 +1300 Subject: [PATCH 28/31] document TokenRefreshFunction --- src/http-api/interface.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index e068894f7b1..a9bf7712741 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -20,13 +20,17 @@ export type Body = Record | BodyInit; /** * @experimental + * Unencrypted access and (optional) refresh token */ 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. */ export type TokenRefreshFunction = (refreshToken: string) => Promise; export interface IHttpOpts { From 482e77a5eb511fd786394a834bfc6dcdf50c9714 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 10:16:29 +1300 Subject: [PATCH 29/31] comments --- src/client.ts | 4 ++-- src/http-api/interface.ts | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/client.ts b/src/client.ts index 161fa5cec36..7b31ee63bc2 100644 --- a/src/client.ts +++ b/src/client.ts @@ -298,7 +298,7 @@ export interface ICreateClientOpts { refreshToken?: string; /** - * Function used to attempt refreshing the access token + * 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 */ @@ -7715,7 +7715,7 @@ export class MatrixClient extends TypedEventEmitter Promise; export interface IHttpOpts { @@ -42,7 +43,14 @@ export interface IHttpOpts { extraParams?: Record; 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 From dc3cf018feb0bd18606bdcaad53a772fd15d2e9f Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 10:49:57 +1300 Subject: [PATCH 30/31] tidying --- src/http-api/fetch.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 7e425ab289d..0883d90e9e1 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -112,8 +112,9 @@ export class FetchHttpApi { * * @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 * ``` @@ -136,6 +137,7 @@ export class FetchHttpApi { ): Promise> { if (!queryParams) queryParams = {}; + // avoid mutating paramOpts so they can be used on retry const opts = { ...paramOpts }; if (this.opts.accessToken) { @@ -181,6 +183,11 @@ export class FetchHttpApi { } } + /** + * 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 { if (!this.opts.refreshToken || !this.opts.tokenRefreshFunction) { return false; @@ -232,7 +239,6 @@ export class FetchHttpApi { opts?: IRequestOpts, ): Promise> { const fullUri = this.getUrl(path, queryParams, opts?.prefix, opts?.baseUrl); - return this.requestOtherUrl(method, fullUri, body, opts); } From 0a17770697cc0832e14bc2d908865a5336364e4a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 11 Oct 2023 16:16:59 +1300 Subject: [PATCH 31/31] update for injected logger --- src/http-api/fetch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index a4d2ea237a3..0c6f9a5bfb8 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -199,7 +199,7 @@ export class FetchHttpApi { // successfully got new tokens return true; } catch (error) { - logger.warn("Failed to refresh token", error); + this.opts.logger?.warn("Failed to refresh token", error); return false; } }