From 28b61f32b4d8d5ee9324c9ae801504593c2ab561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 4 Jan 2022 13:02:55 +0100 Subject: [PATCH] slim down and fetch on demand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../DelegatedSignInIdentity.test.ts | 84 ++----- .../DelegatedSignInIdentity.ts | 208 +++++++++--------- .../DelegatedSignInPage.tsx | 3 - 3 files changed, 120 insertions(+), 175 deletions(-) diff --git a/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.test.ts b/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.test.ts index 11fdaf257e..16ce71980a 100644 --- a/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.test.ts +++ b/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.test.ts @@ -21,90 +21,35 @@ import { setupServer } from 'msw/node'; import { DEFAULTS, DelegatedSignInIdentity, - sleep, - tokenToExpiryMillis, + tokenToExpiry, } from './DelegatedSignInIdentity'; -const flushPromises = () => new Promise(setImmediate); - const validBackstageTokenExpClaim = 1641216199; const validBackstageToken = 'eyJhbGciOiJFUzI1NiIsImtpZCI6ImMxNTMzNDRiLWZjYzktNGIwOS1iN2ZhLTU3ZmM5MDhjMjBiNiJ9.eyJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjcwMDcvYXBpL2F1dGgiLCJzdWIiOiJmcmViZW4iLCJhdWQiOiJiYWNrc3RhZ2UiLCJpYXQiOjE2NDEyMTI1OTksImV4cCI6MTY0MTIxNjE5OSwiZW50IjpbInVzZXI6ZGVmYXVsdC9mcmViZW4iXX0.4nOTmPHPwhzaKTzikgUsHcszfcP-JamcojMnRfyfsKhyHCCEywe6uLFlvvmK5NbaX5Z7IIji-kg7bxKU58kwoQ'; describe('DelegatedSignInIdentity', () => { - describe('tokenToExpiryMillis', () => { + describe('tokenToExpiry', () => { beforeEach(() => jest.useFakeTimers('modern')); afterEach(() => jest.useRealTimers()); it('handles undefined', async () => { - expect(tokenToExpiryMillis(undefined)).toEqual( - DEFAULTS.defaultTokenExpiryMillis, + expect(tokenToExpiry(undefined)).toEqual( + new Date(Date.now() + DEFAULTS.defaultTokenExpiryMillis), ); }); it('handles a valid token', async () => { - jest.setSystemTime( - validBackstageTokenExpClaim * 1000 - - (3600 + DEFAULTS.tokenExpiryMarginMillis), + expect(tokenToExpiry(validBackstageToken)).toEqual( + new Date( + validBackstageTokenExpClaim * 1000 - DEFAULTS.tokenExpiryMarginMillis, + ), ); - expect(tokenToExpiryMillis(validBackstageToken)).toEqual(3600); - }); - }); - - describe('sleep', () => { - beforeEach(() => jest.useFakeTimers()); - afterEach(() => jest.useRealTimers()); - - it('normally sleeps', async () => { - const fn = jest.fn(); - sleep(100, new AbortController().signal).then(fn); - - await flushPromises(); - expect(fn).toBeCalledTimes(0); - - jest.advanceTimersByTime(99); - await flushPromises(); - expect(fn).toBeCalledTimes(0); - - jest.advanceTimersByTime(2); - await flushPromises(); - expect(fn).toBeCalledTimes(1); - }); - - it('can be aborted', async () => { - const controller = new AbortController(); - const fn = jest.fn(); - sleep(100, controller.signal).then(fn); - - jest.advanceTimersByTime(50); - await flushPromises(); - expect(fn).toBeCalledTimes(0); - - controller.abort(); - await flushPromises(); - expect(fn).toBeCalledTimes(1); - }); - - it('reuses the same controller nicely', async () => { - const controller = new AbortController(); - - for (let i = 0; i < 50; ++i) { - const fn = jest.fn(); - sleep(100, controller.signal).then(fn); - - jest.advanceTimersByTime(99); - await flushPromises(); - expect(fn).toBeCalledTimes(0); - - jest.advanceTimersByTime(2); - await flushPromises(); - expect(fn).toBeCalledTimes(1); - } }); }); describe('DelegatedSignInIdentity', () => { - beforeEach(() => jest.useFakeTimers()); + beforeEach(() => jest.useFakeTimers('modern')); afterEach(() => jest.useRealTimers()); const worker = setupServer(); @@ -112,7 +57,6 @@ describe('DelegatedSignInIdentity', () => { it('runs the happy path', async () => { const getBaseUrl = jest.fn(); - const postError = jest.fn(); const serverCalled = jest.fn(); function makeToken() { @@ -166,17 +110,17 @@ describe('DelegatedSignInIdentity', () => { const identity = new DelegatedSignInIdentity({ provider: 'foo', discoveryApi: { getBaseUrl }, - errorApi: { post: postError } as any, fetchApi: { fetch }, }); getBaseUrl.mockResolvedValue('http://example.com/api/auth'); await identity.start(); // should not throw - expect(getBaseUrl).toBeCalledTimes(1); expect(getBaseUrl).lastCalledWith('auth'); - expect(postError).toBeCalledTimes(0); + expect(serverCalled).toBeCalledTimes(1); + + await identity.getSessionAsync(); // no need to fetch again just yet expect(serverCalled).toBeCalledTimes(1); // Use a fairly large margin (1000) since the iat and exp are clamped to @@ -184,11 +128,11 @@ describe('DelegatedSignInIdentity', () => { jest.advanceTimersByTime( 100 * 1000 - DEFAULTS.tokenExpiryMarginMillis - 1000, ); - await flushPromises(); + await identity.getSessionAsync(); // still no need to fetch again expect(serverCalled).toBeCalledTimes(1); jest.advanceTimersByTime(1001); - await flushPromises(); + await identity.getSessionAsync(); // now the expiry has passed expect(serverCalled).toBeCalledTimes(2); }); }); diff --git a/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.ts b/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.ts index 92c8b34c50..750f9ef027 100644 --- a/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.ts +++ b/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInIdentity.ts @@ -17,7 +17,6 @@ import { BackstageUserIdentity, discoveryApiRef, - errorApiRef, fetchApiRef, IdentityApi, ProfileInfo, @@ -26,8 +25,6 @@ import { ResponseError } from '@backstage/errors'; import { DelegatedSession, delegatedSessionSchema } from './types'; export const DEFAULTS = { - // How often we retry a failed auth refresh call - retryRefreshFrequencyMillis: 5_000, // The amount of time between token refreshes, if we fail to get an actual // value out of the exp claim defaultTokenExpiryMillis: 60_000, @@ -36,117 +33,99 @@ export const DEFAULTS = { tokenExpiryMarginMillis: 30_000, } as const; -// The amount of time to wait until trying to refresh the auth session -export function tokenToExpiryMillis(jwtToken: string | undefined): number { - if (typeof jwtToken !== 'string') { - return DEFAULTS.defaultTokenExpiryMillis; +// When the token expires, with some margin +export function tokenToExpiry(jwtToken: string | undefined): Date { + if (!jwtToken) { + return new Date(Date.now() + DEFAULTS.defaultTokenExpiryMillis); } const [_header, rawPayload, _signature] = jwtToken.split('.'); const payload = JSON.parse(atob(rawPayload)); - const expiresInMillis = payload.exp * 1000 - Date.now(); - return Math.max(expiresInMillis - DEFAULTS.tokenExpiryMarginMillis, 0); -} - -// Sleeps for a given duration, unless interrupted by signal -export async function sleep(durationMillis: number, signal: AbortSignal) { - if (!signal.aborted) { - await new Promise(resolve => { - const timeoutHandle = setTimeout(done, durationMillis); - signal.addEventListener('abort', done); - function done() { - clearTimeout(timeoutHandle); - signal.removeEventListener('abort', done); - resolve(); - } - }); - } + return new Date(payload.exp * 1000 - DEFAULTS.tokenExpiryMarginMillis); } type DelegatedSignInIdentityOptions = { provider: string; discoveryApi: typeof discoveryApiRef.T; fetchApi: typeof fetchApiRef.T; - errorApi: typeof errorApiRef.T; }; +type State = + | { + type: 'empty'; + } + | { + type: 'fetching'; + promise: Promise; + previous: DelegatedSession | undefined; + } + | { + type: 'active'; + session: DelegatedSession; + expiresAt: Date; + } + | { + type: 'failed'; + error: Error; + }; + /** - * An identity API that keeps itself up to date solely based on getting session - * information from a `/refresh` endpoint. + * An identity API that gets the user auth information solely based on a + * provider's `/refresh` endpoint. */ export class DelegatedSignInIdentity implements IdentityApi { private readonly options: DelegatedSignInIdentityOptions; private readonly abortController: AbortController; - private session: DelegatedSession | undefined; + private state: State; constructor(options: DelegatedSignInIdentityOptions) { this.options = options; this.abortController = new AbortController(); - this.session = undefined; + this.state = { type: 'empty' }; } async start() { - let signalErrors = true; - let pause = 0; - - const refreshOnce = async () => { - if (!this.abortController.signal.aborted) { - try { - await this.refresh(signalErrors); - signalErrors = true; - pause = tokenToExpiryMillis(this.session?.backstageIdentity.token); - if (pause <= 0) { - this.session = undefined; - } - } catch { - signalErrors = false; // Only signal first failure in a row - pause = DEFAULTS.retryRefreshFrequencyMillis; - } - } - }; - - const refreshLoop = async () => { - while (!this.abortController.signal.aborted) { - await sleep(pause, this.abortController.signal); - await refreshOnce(); - } - }; - - // Try first refresh and bubble up any errors to the caller, then get the - // loop going behind the scenes and return - await refreshOnce(); - refreshLoop(); + // Try to make a first fetch, bubble up any errors to the caller + await this.getSessionAsync(); } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.getUserId} */ getUserId(): string { - return this.getSession().backstageIdentity.id; + const session = this.getSessionSync(); + return session.backstageIdentity.id; } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.getIdToken} */ async getIdToken(): Promise { - return this.getSession().backstageIdentity.token; + const session = await this.getSessionAsync(); + return session.backstageIdentity.token; } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.getProfile} */ getProfile(): ProfileInfo { - return this.getSession().profile; + const session = this.getSessionSync(); + return session.profile; } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.getProfileInfo} */ async getProfileInfo(): Promise { - return this.getSession().profile; + const session = await this.getSessionAsync(); + return session.profile; } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.getBackstageIdentity} */ async getBackstageIdentity(): Promise { - return this.getSession().backstageIdentity.identity; + const session = await this.getSessionAsync(); + return session.backstageIdentity.identity; } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.getCredentials} */ async getCredentials(): Promise<{ token?: string | undefined }> { - return { token: this.getSession().backstageIdentity.token }; + const session = await this.getSessionAsync(); + return { + token: session.backstageIdentity.token, + }; } /** {@inheritdoc @backstage/core-plugin-api#IdentityApi.signOut} */ @@ -154,46 +133,71 @@ export class DelegatedSignInIdentity implements IdentityApi { this.abortController.abort(); } - getSession(): DelegatedSession { - if (!this.session) { - throw new Error('No session available. Try reloading your browser page.'); + getSessionSync(): DelegatedSession { + if (this.state.type === 'active') { + return this.state.session; + } else if (this.state.type === 'fetching' && this.state.previous) { + return this.state.previous; } - return this.session; + throw new Error('No session available. Try reloading your browser page.'); } - async refresh(notifyErrors: boolean) { - try { - const baseUrl = await this.options.discoveryApi.getBaseUrl('auth'); - - const response = await this.options.fetchApi.fetch( - `${baseUrl}/${this.options.provider}/refresh`, - { - signal: this.abortController.signal, - headers: { 'x-requested-with': 'XMLHttpRequest' }, - credentials: 'include', - }, - ); - - if (!response.ok) { - throw await ResponseError.fromResponse(response); - } - - const session = delegatedSessionSchema.parse(await response.json()); - this.session = session; - } catch (e) { - if (this.abortController.signal.aborted) { - return; - } - - if (notifyErrors) { - this.options.errorApi.post( - new Error( - `Failed to refresh browser session, ${e}. Try reloading your browser page.`, - ), - ); - } - - throw e; + async getSessionAsync(): Promise { + if (this.state.type === 'fetching') { + return this.state.promise; + } else if ( + this.state.type === 'active' && + new Date() < this.state.expiresAt + ) { + return this.state.session; } + + const previous = + this.state.type === 'active' ? this.state.session : undefined; + + const promise = this.fetchSession().then( + session => { + this.state = { + type: 'active', + session, + expiresAt: tokenToExpiry(session.backstageIdentity.token), + }; + return session; + }, + error => { + this.state = { + type: 'failed', + error, + }; + throw error; + }, + ); + + this.state = { + type: 'fetching', + promise, + previous, + }; + + return promise; + } + + async fetchSession(): Promise { + const baseUrl = await this.options.discoveryApi.getBaseUrl('auth'); + + const response = await this.options.fetchApi.fetch( + `${baseUrl}/${this.options.provider}/refresh`, + { + signal: this.abortController.signal, + headers: { 'x-requested-with': 'XMLHttpRequest' }, + credentials: 'include', + }, + ); + + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + return delegatedSessionSchema.parse(await response.json()); } } diff --git a/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInPage.tsx b/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInPage.tsx index 54cc0495fd..0337e3122d 100644 --- a/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInPage.tsx +++ b/packages/core-components/src/layout/DelegatedSignInPage/DelegatedSignInPage.tsx @@ -16,7 +16,6 @@ import { discoveryApiRef, - errorApiRef, fetchApiRef, SignInPageProps, useApi, @@ -58,12 +57,10 @@ export type DelegatedSignInPageProps = SignInPageProps & { export const DelegatedSignInPage = (props: DelegatedSignInPageProps) => { const discoveryApi = useApi(discoveryApiRef); const fetchApi = useApi(fetchApiRef); - const errorApi = useApi(errorApiRef); const { loading, error } = useAsync(async () => { const identity = new DelegatedSignInIdentity({ provider: props.provider, - errorApi, discoveryApi, fetchApi, });