From c15e0cedbe1981ccbe29bba83524d30b5357f74f Mon Sep 17 00:00:00 2001 From: Jamie Klassen Date: Mon, 13 Feb 2023 11:50:04 -0500 Subject: [PATCH] can get non-microsoft graph tokens via refresh Next up will be to make it possible to _request_ refreshable tokens via `getAccessToken`, accounting for the fact that the `offline_access` scope does not appear in the auth-backend response when it is requested -- instead the response has a set-cookie header. Signed-off-by: Jamie Klassen --- .changeset/sixty-insects-visit.md | 9 +++ .../auth/microsoft/MicrosoftAuth.test.ts | 66 +++++-------------- .../DefaultAuthConnector.test.ts | 34 +++++----- .../lib/AuthConnector/DefaultAuthConnector.ts | 7 +- .../src/lib/AuthConnector/types.ts | 2 +- .../RefreshingAuthSessionManager.test.ts | 6 +- .../RefreshingAuthSessionManager.ts | 10 +-- 7 files changed, 57 insertions(+), 77 deletions(-) create mode 100644 .changeset/sixty-insects-visit.md diff --git a/.changeset/sixty-insects-visit.md b/.changeset/sixty-insects-visit.md new file mode 100644 index 0000000000..adbe27e0ac --- /dev/null +++ b/.changeset/sixty-insects-visit.md @@ -0,0 +1,9 @@ +--- +'@backstage/core-app-api': minor +--- + +The `AuthConnector` interface now supports specifying a set of scopes when +refreshing a session. The `DefaultAuthConnector` implementation passes the +`scope` query parameter to the auth-backend plugin appropriately. The +`RefreshingAuthSessionManager` passes any scopes in its `GetSessionRequest` +appropriately. diff --git a/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.test.ts index e2aedbc9b4..61a0a5b64a 100644 --- a/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.test.ts @@ -34,59 +34,20 @@ describe('MicrosoftAuth', () => { ), }); - const toHaveJWTClaims = function toHaveJWTClaims( - this: jest.MatcherContext, - received: string, - expected: Record, - ): jest.CustomMatcherResult { - let parsedClaims: Record; - try { - parsedClaims = JSON.parse( - Buffer.from(received.split('.')[1], 'base64').toString(), - ); - } catch (e) { - return { - pass: false, - message: () => - `Expected JWT with claims: ${this.utils.printExpected( - expected, - )}\nReceived invalid JWT ${this.utils.printReceived( - received, - )}\nError: ${e}`, - }; - } - const expectedResult = expect.objectContaining(expected); - return { - pass: this.equals(parsedClaims, expectedResult), - message: () => - `Expected JWT with claims: ${this.utils.printExpected( - expected, - )}\nReceived JWT with claims: ${this.utils.printReceived( - parsedClaims, - )}\n\n${this.utils.diff(expectedResult, parsedClaims)}`, - }; - }; - expect.extend({ toHaveJWTClaims }); - describe('with a refresh token', () => { beforeEach(() => { server.use( rest.get( 'http://backstage.test/api/auth/microsoft/refresh', - (req, res, ctx) => { - const scope = - req.url.searchParams.get('scope') || - 'openid profile email User.Read'; + async (req, res, ctx) => { + const scopeParam = req.url.searchParams.get('scope'); return res( ctx.json({ providerInfo: { - accessToken: `header.${Buffer.from( - JSON.stringify({ - aud: '00000003-0000-0000-c000-000000000000', - scp: 'openid profile email User.Read', - }), - ).toString('base64')}.signature`, - scope, + accessToken: scopeParam + ? 'tokenForOtherResource' + : 'tokenForGrantScopes', + scope: scopeParam || 'grant-resource/scope', }, }), ); @@ -95,13 +56,18 @@ describe('MicrosoftAuth', () => { ); }); - it('gets access token for Microsoft Graph', async () => { + it('gets access token with requested scopes for grant', async () => { const accessToken = await microsoftAuth.getAccessToken(); - expect(accessToken).toHaveJWTClaims({ - aud: '00000003-0000-0000-c000-000000000000', - scp: 'openid profile email User.Read', - }); + expect(accessToken).toEqual('tokenForGrantScopes'); + }); + + it('gets access token for other consented scopes besides those directly granted', async () => { + const accessToken = await microsoftAuth.getAccessToken( + 'azure-resource/scope', + ); + + expect(accessToken).toEqual('tokenForOtherResource'); }); }); diff --git a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts index 51da9cbfb5..c46e87581f 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -59,22 +59,22 @@ describe('DefaultAuthConnector', () => { jest.resetAllMocks(); }); - it('should refresh a session', async () => { + it('should refresh a session with scope', async () => { server.use( - rest.get('*', (_req, res, ctx) => + rest.get('*', (req, res, ctx) => res( ctx.json({ idToken: 'mock-id-token', accessToken: 'mock-access-token', - scopes: 'a b c', + scopes: req.url.searchParams.get('scope') || 'default-scope', expiresInSeconds: '60', }), ), ), ); - const helper = new DefaultAuthConnector(defaultOptions); - const session = await helper.refreshSession(); + const connector = new DefaultAuthConnector(defaultOptions); + const session = await connector.refreshSession(new Set(['a', 'b', 'c'])); expect(session.idToken).toBe('mock-id-token'); expect(session.accessToken).toBe('mock-access-token'); expect(session.scopes).toEqual(new Set(['a', 'b', 'c'])); @@ -89,8 +89,8 @@ describe('DefaultAuthConnector', () => { ), ); - const helper = new DefaultAuthConnector(defaultOptions); - await expect(helper.refreshSession()).rejects.toThrow( + const connector = new DefaultAuthConnector(defaultOptions); + await expect(connector.refreshSession()).rejects.toThrow( 'Auth refresh request failed, Error: Network NOPE', ); }); @@ -98,19 +98,19 @@ describe('DefaultAuthConnector', () => { it('should handle failure response when refreshing session', async () => { server.use(rest.get('*', (_req, res, ctx) => res(ctx.status(401, 'NOPE')))); - const helper = new DefaultAuthConnector(defaultOptions); - await expect(helper.refreshSession()).rejects.toThrow( + const connector = new DefaultAuthConnector(defaultOptions); + await expect(connector.refreshSession()).rejects.toThrow( 'Auth refresh request failed, NOPE', ); }); it('should fail if popup was rejected', async () => { const mockOauth = new MockOAuthApi(); - const helper = new DefaultAuthConnector({ + const connector = new DefaultAuthConnector({ ...defaultOptions, oauthRequestApi: mockOauth, }); - const promise = helper.createSession({ scopes: new Set(['a', 'b']) }); + const promise = connector.createSession({ scopes: new Set(['a', 'b']) }); await mockOauth.rejectAll(); await expect(promise).rejects.toMatchObject({ name: 'RejectedError' }); }); @@ -125,12 +125,12 @@ describe('DefaultAuthConnector', () => { scopes: 'a b', expiresInSeconds: 3600, }); - const helper = new DefaultAuthConnector({ + const connector = new DefaultAuthConnector({ ...defaultOptions, oauthRequestApi: mockOauth, }); - const sessionPromise = helper.createSession({ + const sessionPromise = connector.createSession({ scopes: new Set(['a', 'b']), }); @@ -153,13 +153,13 @@ describe('DefaultAuthConnector', () => { const popupSpy = jest .spyOn(loginPopup, 'showLoginPopup') .mockResolvedValue('my-session'); - const helper = new DefaultAuthConnector({ + const connector = new DefaultAuthConnector({ ...defaultOptions, oauthRequestApi: new MockOAuthApi(), sessionTransform: str => str, }); - const sessionPromise = helper.createSession({ + const sessionPromise = connector.createSession({ scopes: new Set(), instantPopup: true, }); @@ -174,13 +174,13 @@ describe('DefaultAuthConnector', () => { const popupSpy = jest .spyOn(loginPopup, 'showLoginPopup') .mockResolvedValue({ scopes: '' }); - const helper = new DefaultAuthConnector({ + const connector = new DefaultAuthConnector({ ...defaultOptions, joinScopes: scopes => `-${[...scopes].join('')}-`, oauthRequestApi: mockOauth, }); - helper.createSession({ scopes: new Set(['a', 'b']) }); + connector.createSession({ scopes: new Set(['a', 'b']) }); await mockOauth.triggerAll(); diff --git a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts index f2c35b0ba3..728ed3878c 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -126,9 +126,12 @@ export class DefaultAuthConnector return this.authRequester(options.scopes); } - async refreshSession(): Promise { + async refreshSession(scopes?: Set): Promise { const res = await fetch( - await this.buildUrl('/refresh', { optional: true }), + await this.buildUrl('/refresh', { + optional: true, + ...(scopes && { scope: this.joinScopesFunc(scopes) }), + }), { headers: { 'x-requested-with': 'XMLHttpRequest', diff --git a/packages/core-app-api/src/lib/AuthConnector/types.ts b/packages/core-app-api/src/lib/AuthConnector/types.ts index 464a2ed627..c8e83e1ccc 100644 --- a/packages/core-app-api/src/lib/AuthConnector/types.ts +++ b/packages/core-app-api/src/lib/AuthConnector/types.ts @@ -25,6 +25,6 @@ export type CreateSessionOptions = { */ export type AuthConnector = { createSession(options: CreateSessionOptions): Promise; - refreshSession(): Promise; + refreshSession(scopes?: Set): Promise; removeSession(): Promise; }; diff --git a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts index a94aca85f8..23432856cb 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts @@ -47,7 +47,7 @@ describe('RefreshingAuthSessionManager', () => { await manager.getSession({}); expect(createSession).toHaveBeenCalledTimes(1); - expect(refreshSession).toHaveBeenCalledTimes(1); + expect(refreshSession).toHaveBeenCalledWith(undefined); expect(stateSubscriber.mock.calls).toEqual([ [SessionState.SignedOut], [SessionState.SignedIn], @@ -103,7 +103,7 @@ describe('RefreshingAuthSessionManager', () => { await manager.getSession({ scopes: new Set(['a']) }); expect(createSession).toHaveBeenCalledTimes(1); - expect(refreshSession).toHaveBeenCalledTimes(1); + expect(refreshSession).toHaveBeenCalledWith(new Set(['a'])); await manager.getSession({ scopes: new Set(['a']) }); expect(createSession).toHaveBeenCalledTimes(1); @@ -134,7 +134,7 @@ describe('RefreshingAuthSessionManager', () => { expect(await manager.getSession({ optional: true })).toBe(undefined); expect(createSession).toHaveBeenCalledTimes(0); - expect(refreshSession).toHaveBeenCalledTimes(1); + expect(refreshSession).toHaveBeenCalledWith(undefined); }); it('should forward option to instantly show auth popup and not attempt refresh', async () => { diff --git a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index 67d83bb36a..c04eb637de 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -73,7 +73,9 @@ export class RefreshingAuthSessionManager implements SessionManager { } try { - const refreshedSession = await this.collapsedSessionRefresh(); + const refreshedSession = await this.collapsedSessionRefresh( + options.scopes, + ); const currentScopes = this.sessionScopesFunc(this.currentSession!); const refreshedScopes = this.sessionScopesFunc(refreshedSession); if (hasScopes(refreshedScopes, currentScopes)) { @@ -97,7 +99,7 @@ export class RefreshingAuthSessionManager implements SessionManager { // already had an existing session. if (!this.currentSession && !options.instantPopup) { try { - const newSession = await this.collapsedSessionRefresh(); + const newSession = await this.collapsedSessionRefresh(options.scopes); this.currentSession = newSession; // The session might not have the scopes requested so go back and check again return this.getSession(options); @@ -130,12 +132,12 @@ export class RefreshingAuthSessionManager implements SessionManager { return this.stateTracker.sessionState$(); } - private async collapsedSessionRefresh(): Promise { + private async collapsedSessionRefresh(scopes?: Set): Promise { if (this.refreshPromise) { return this.refreshPromise; } - this.refreshPromise = this.connector.refreshSession(); + this.refreshPromise = this.connector.refreshSession(scopes); try { const session = await this.refreshPromise;