From 7ba13d0e93557c145662c4cfdb3895fd684950ad Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 11:31:19 +0200 Subject: [PATCH 01/18] packages/core: add MockOAuthRequestApi --- .../OAuthRequestManager/MockOAuthApi.test.ts | 104 ++++++++++++++++++ .../OAuthRequestManager/MockOAuthApi.ts | 66 +++++++++++ 2 files changed, 170 insertions(+) create mode 100644 packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts create mode 100644 packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts new file mode 100644 index 0000000000..d6346c688b --- /dev/null +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts @@ -0,0 +1,104 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 MockOAuthApi from './MockOAuthApi'; +import PowerIcon from '@material-ui/icons/Power'; +import { BasicOAuthScopes } from './BasicOAuthScopes'; + +describe('MockOAuthApi', () => { + it('should trigger all requests', async () => { + const popupResult = { is: 'done' }; + const mock = new MockOAuthApi(popupResult); + + const authHandler1 = jest + .fn() + .mockImplementation(() => mock.showLoginPopup()); + const requester1 = mock.createAuthRequester({ + provider: { icon: PowerIcon, title: 'Test' }, + onAuthRequest: authHandler1, + }); + + const authHandler2 = jest.fn().mockResolvedValue('other'); + const requester2 = mock.createAuthRequester({ + provider: { icon: PowerIcon, title: 'Test' }, + onAuthRequest: authHandler2, + }); + + const promises = [ + requester1(BasicOAuthScopes.from('a')), + requester1(BasicOAuthScopes.from('b')), + requester2(BasicOAuthScopes.from('a b')), + requester2(BasicOAuthScopes.from('b c')), + requester2(BasicOAuthScopes.from('c a')), + ]; + + await expect( + Promise.race([Promise.all(promises), 'waiting']), + ).resolves.toBe('waiting'); + + await mock.triggerAll(); + + await expect(Promise.all(promises)).resolves.toEqual([ + popupResult, + popupResult, + 'other', + 'other', + 'other', + ]); + + expect(authHandler1).toHaveBeenCalledTimes(1); + expect(authHandler1).toHaveBeenCalledWith(BasicOAuthScopes.from('a b')); + expect(authHandler2).toHaveBeenCalledTimes(1); + expect(authHandler2).toHaveBeenCalledWith(BasicOAuthScopes.from('a b c')); + }); + + it('should reject all requests', async () => { + const mock = new MockOAuthApi(); + + const authHandler1 = jest.fn(); + const requester1 = mock.createAuthRequester({ + provider: { icon: PowerIcon, title: 'Test' }, + onAuthRequest: authHandler1, + }); + + const authHandler2 = jest.fn(); + const requester2 = mock.createAuthRequester({ + provider: { icon: PowerIcon, title: 'Test' }, + onAuthRequest: authHandler2, + }); + + const promises = [ + requester1(BasicOAuthScopes.from('a')), + requester1(BasicOAuthScopes.from('b')), + requester2(BasicOAuthScopes.from('a b')), + requester2(BasicOAuthScopes.from('b c')), + requester2(BasicOAuthScopes.from('c a')), + ]; + + await expect( + Promise.race([Promise.all(promises), 'waiting']), + ).resolves.toBe('waiting'); + + await mock.rejectAll(); + + for (const promise of promises) { + await expect(promise).rejects.toMatchObject({ name: 'RejectedError' }); + } + + expect(authHandler1).not.toHaveBeenCalled(); + expect(authHandler2).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts new file mode 100644 index 0000000000..f52c50dbe5 --- /dev/null +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts @@ -0,0 +1,66 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { + OAuthRequestApi, + AuthRequesterOptions, + LoginPopupOptions, +} from '../../definitions'; +import { OAuthRequestManager } from './OAuthRequestManager'; + +export default class MockOAuthApi implements OAuthRequestApi { + private readonly real = new OAuthRequestManager(); + + constructor(private readonly popupResult = {}) {} + + createAuthRequester(options: AuthRequesterOptions) { + return this.real.createAuthRequester(options); + } + + authRequest$() { + return this.real.authRequest$(); + } + + async triggerAll() { + await Promise.resolve(); // Wait a tick to allow new requests to get forwarded + + return new Promise((resolve) => { + const subscription = this.authRequest$().subscribe((requests) => { + subscription.unsubscribe(); + Promise.all(requests.map((request) => request.trigger())).then(() => + resolve(), + ); + }); + }); + } + + async rejectAll() { + await Promise.resolve(); // Wait a tick to allow new requests to get forwarded + + return new Promise((resolve) => { + const subscription = this.authRequest$().subscribe((requests) => { + subscription.unsubscribe(); + requests.map((request) => request.reject()); + resolve(); + }); + }); + } + + async showLoginPopup(options?: LoginPopupOptions): Promise { + // Working around linter complaints, can't remove options since we want correct mock types + return options ? this.popupResult : this.popupResult; + } +} From 3fe63fbab4a4458002f0d8020fba0893389d2fae Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 12:02:43 +0200 Subject: [PATCH 02/18] packages/core: lift out internal goog auth implementation --- packages/core/package.json | 3 +- .../auth/google/GoogleAuth.test.ts | 198 ++++++++++++++++++ .../implementations/auth/google/GoogleAuth.ts | 130 ++++++++++++ .../auth/google/GoogleAuthHelper.test.ts | 116 ++++++++++ .../auth/google/GoogleAuthHelper.ts | 192 +++++++++++++++++ .../auth/google/GoogleScopes.test.ts | 88 ++++++++ .../auth/google/GoogleScopes.ts | 62 ++++++ .../auth/google/MockAuthHelper.test.ts | 36 ++++ .../auth/google/MockAuthHelper.ts | 54 +++++ .../apis/implementations/auth/google/index.ts | 18 ++ .../apis/implementations/auth/google/types.ts | 94 +++++++++ packages/core/src/setupTests.ts | 1 + 12 files changed, 991 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/index.ts create mode 100644 packages/core/src/api/apis/implementations/auth/google/types.ts diff --git a/packages/core/package.json b/packages/core/package.json index e73c7c581f..68b8feb7d7 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -61,7 +61,8 @@ "@backstage/test-utils-core": "^0.1.1-alpha.5", "@testing-library/jest-dom": "^5.7.0", "@testing-library/react": "^9.3.2", - "@testing-library/user-event": "^10.2.4" + "@testing-library/user-event": "^10.2.4", + "jest-fetch-mock": "^3.0.3" }, "files": [ "dist/**/*.{js,d.ts}" diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts new file mode 100644 index 0000000000..a6d2b082b2 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts @@ -0,0 +1,198 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleAuth from './GoogleAuth'; +import GoogleScopes from './GoogleScopes'; + +const theFuture = new Date(Date.now() + 3600000); +const thePast = new Date(Date.now() - 10); + +describe('GoogleAuth', () => { + it('should save result form createSession', async () => { + const createSession = jest.fn().mockResolvedValue({ expiresAt: theFuture }); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); + + await googleAuth.getSession({}); + expect(createSession).toBeCalledTimes(1); + + await googleAuth.getSession({}); + expect(createSession).toBeCalledTimes(1); + + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should ask consent only if scopes have changed', async () => { + const createSession = jest.fn(); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); + + createSession.mockResolvedValue({ + scopes: GoogleScopes.from('a'), + expiresAt: theFuture, + }); + await googleAuth.getSession({ scope: 'a' }); + expect(createSession).toBeCalledTimes(1); + + await googleAuth.getSession({ scope: 'a' }); + expect(createSession).toBeCalledTimes(1); + + await googleAuth.getSession({ scope: 'b' }); + expect(createSession).toBeCalledTimes(2); + }); + + it('should check for session expiry', async () => { + const createSession = jest.fn(); + const refreshSession = jest + .fn() + .mockRejectedValueOnce(new Error('NOPE')) + .mockResolvedValue({ scopes: GoogleScopes.from('a') }); + const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); + + createSession.mockResolvedValue({ + scopes: GoogleScopes.from('a'), + expiresAt: thePast, + }); + + await googleAuth.getSession({ scope: 'a' }); + expect(createSession).toBeCalledTimes(1); + expect(refreshSession).toBeCalledTimes(1); + + await googleAuth.getSession({ scope: 'a' }); + expect(createSession).toBeCalledTimes(1); + expect(refreshSession).toBeCalledTimes(2); + }); + + it('should handle user closed popup', async () => { + const createSession = jest.fn(); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); + + createSession.mockRejectedValueOnce(new Error('some error')); + await expect(googleAuth.getSession({ scope: 'a' })).rejects.toThrow( + 'some error', + ); + }); + + it('should logout and reload', async () => { + // This is a workaround that is used by Facebook and the Jest core team + // It is a limitation with the newest versions of JSDOM, and newer browser standards + // where window.location and all of its properties are read-only. So we re-construct it! + // See https://github.com/facebook/jest/issues/890#issuecomment-209698782 + const location = { ...window.location }; + delete window.location; + window.location = location; + jest.spyOn(window.location, 'reload').mockImplementation(); + + const removeSession = jest.fn(); + const googleAuth = new GoogleAuth({ removeSession } as any); + + await googleAuth.logout(); + expect(window.location.reload).toHaveBeenCalled(); + expect(removeSession).toHaveBeenCalled(); + }); + + it('should get refreshed access token', async () => { + const refreshSession = jest + .fn() + .mockResolvedValue({ accessToken: 'access-token', expiresAt: theFuture }); + const googleAuth = new GoogleAuth({ refreshSession } as any); + + expect(await googleAuth.getAccessToken()).toBe('access-token'); + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should get refreshed id token', async () => { + const refreshSession = jest + .fn() + .mockResolvedValue({ idToken: 'id-token', expiresAt: theFuture }); + const googleAuth = new GoogleAuth({ refreshSession } as any); + + expect(await googleAuth.getIdToken()).toBe('id-token'); + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should get optional id token', async () => { + const refreshSession = jest + .fn() + .mockResolvedValue({ idToken: 'id-token', expiresAt: theFuture }); + const googleAuth = new GoogleAuth({ refreshSession } as any); + + expect(await googleAuth.getIdToken({ optional: true })).toBe('id-token'); + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should not get optional id token', async () => { + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const googleAuth = new GoogleAuth({ refreshSession } as any); + + expect(await googleAuth.getIdToken({ optional: true })).toBe(''); + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should share popup closed errors', async () => { + const error = new Error('NOPE'); + error.name = 'RejectedError'; + const createSession = jest.fn().mockRejectedValue(error); + const refreshSession = jest.fn().mockResolvedValue({ + accessToken: 'access-token', + expiresAt: theFuture, + scopes: GoogleScopes.from('not-enough'), + }); + const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); + + // Make sure we have a session before we do the double request, so that we get past the !this.currentSession check + await expect(googleAuth.getAccessToken()).resolves.toBe('access-token'); + + const promise1 = googleAuth.getAccessToken('more'); + const promise2 = googleAuth.getAccessToken('more'); + await expect(promise1).rejects.toBe(error); + await expect(promise2).rejects.toBe(error); + expect(refreshSession).toBeCalledTimes(1); + expect(createSession).toBeCalledTimes(2); + }); + + it('should wait for all session refreshes', async () => { + const initialSession = { + idToken: 'token1', + expiresAt: theFuture, + scopes: GoogleScopes.empty(), + }; + const refreshSession = jest + .fn() + .mockResolvedValueOnce(initialSession) + .mockResolvedValue({ + idToken: 'token2', + expiresAt: theFuture, + scopes: GoogleScopes.empty(), + }); + const googleAuth = new GoogleAuth({ refreshSession } as any); + + // Grab the expired session first + await expect(googleAuth.getIdToken()).resolves.toBe('token1'); + expect(refreshSession).toBeCalledTimes(1); + + initialSession.expiresAt = thePast; + + const promise1 = googleAuth.getIdToken(); + const promise2 = googleAuth.getIdToken(); + const promise3 = googleAuth.getIdToken(); + await expect(promise1).resolves.toBe('token2'); + await expect(promise2).resolves.toBe('token2'); + await expect(promise3).resolves.toBe('token2'); + expect(refreshSession).toBeCalledTimes(4); // De-duping of session requests happens in client + }); +}); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts new file mode 100644 index 0000000000..d74b159095 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -0,0 +1,130 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { AuthHelper } from './GoogleAuthHelper'; +import GoogleScopes from './GoogleScopes'; +import { GoogleAuthApi, GoogleSession, IdTokenOptions } from './types'; +import { OAuthScopes } from '../../..'; + +class GoogleAuth implements GoogleAuthApi { + private currentSession: GoogleSession | undefined; + + constructor(private readonly helper: AuthHelper) {} + + async getAccessToken(scope?: string | string[]) { + const session = await this.getSession({ optional: false, scope }); + return session.accessToken; + } + + async getIdToken({ optional }: IdTokenOptions = {}) { + const session = await this.getSession({ optional: optional || false }); + if (session) { + return session.idToken; + } + return ''; + } + + async getSession(options: { + optional: false; + scope?: string | string[]; + }): Promise; + async getSession(options: { + optional?: boolean; + scope?: string | string[]; + }): Promise; + async getSession(options: { + optional?: boolean; + scope?: string | string[]; + }): Promise { + if (this.sessionExistsAndHasScope(this.currentSession, options.scope)) { + if (!this.sessionWillExpire(this.currentSession!)) { + return this.currentSession!; + } + + try { + const refreshedSession = await this.helper.refreshSession(); + if (refreshedSession.scopes.hasScopes(this.currentSession!.scopes)) { + this.currentSession = refreshedSession; + } + return refreshedSession; + } catch (error) { + if (options.optional) { + return undefined; + } + throw error; + } + } + + // The user may still have a valid refresh token in their cookies. Attempt to + // initiate a fresh session through the backend using that refresh token. + if (!this.currentSession) { + try { + const newSession = await this.helper.refreshSession(); + this.currentSession = newSession; + // The session might not have the scopes requested so go back and check again + return this.getSession(options); + } catch { + // If the refresh attemp fails we assume we don't have a session, so continue to create one. + } + } + + // If we continue here we will show a popup, so exit if this is an optional session request. + if (options.optional) { + return undefined; + } + + // We can call authRequester multiple times, the returned session will contain all requested scopes. + this.currentSession = await this.helper.createSession( + this.getExtendedScope(options.scope), + ); + return this.currentSession; + } + + async logout() { + await this.helper.removeSession(); + window.location.reload(); + } + + private sessionExistsAndHasScope( + session: GoogleSession | undefined, + scope?: string | string[], + ): boolean { + if (!session) { + return false; + } + if (!scope) { + return true; + } + return session.scopes.hasScopes(scope); + } + + private sessionWillExpire(session: GoogleSession) { + const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; + return expiresInSec < 60 * 5; + } + + private getExtendedScope(scope?: string | string[]) { + let newScope: OAuthScopes = GoogleScopes.default(); + if (this.currentSession) { + newScope = this.currentSession.scopes; + } + if (scope) { + newScope = newScope.extend(scope); + } + return newScope.toString(); + } +} +export default GoogleAuth; diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts new file mode 100644 index 0000000000..fbd45fd779 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts @@ -0,0 +1,116 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleAuthHelper from './GoogleAuthHelper'; +import GoogleScopes from './GoogleScopes'; +import MockOAuthApi from '../../OAuthRequestManager/MockOAuthApi'; + +const anyFetch = fetch as any; + +describe('GoogleAuthHelper', () => { + afterEach(() => { + jest.resetAllMocks(); + anyFetch.resetMocks(); + }); + + it('should refresh a session', async () => { + anyFetch.mockResponseOnce( + JSON.stringify({ + idToken: 'mock-id-token', + accessToken: 'mock-access-token', + scopes: 'a b c', + expiresInSeconds: '60', + }), + ); + + const helper = new GoogleAuthHelper( + { apiOrigin: 'my-origin', dev: false }, + new MockOAuthApi(), + ); + const session = await helper.refreshSession(); + expect(session.idToken).toBe('mock-id-token'); + expect(session.accessToken).toBe('mock-access-token'); + expect(session.scopes.hasScopes('a b c')).toBe(true); + expect(session.expiresAt.getTime()).toBeLessThan(Date.now() + 70000); + expect(session.expiresAt.getTime()).toBeGreaterThan(Date.now() + 50000); + }); + + it('should handle failure to refresh session', async () => { + anyFetch.mockRejectOnce(new Error('Network NOPE')); + + const helper = new GoogleAuthHelper( + { apiOrigin: 'my-origin', dev: true }, + new MockOAuthApi(), + ); + await expect(helper.refreshSession()).rejects.toThrow( + 'Auth refresh request failed, Error: Network NOPE', + ); + }); + + it('should handle failure response when refreshing session', async () => { + anyFetch.mockResponseOnce({}, { status: 401, statusText: 'NOPE' }); + + const helper = new GoogleAuthHelper( + { apiOrigin: 'my-origin', dev: false }, + new MockOAuthApi(), + ); + await expect(helper.refreshSession()).rejects.toThrow( + 'Auth refresh request failed with status NOPE', + ); + }); + + it('should fail if popup was rejected', async () => { + const mockOauth = new MockOAuthApi(); + const helper = new GoogleAuthHelper( + { apiOrigin: 'my-origin', dev: false }, + mockOauth, + ); + const promise = helper.createSession('a b'); + await mockOauth.rejectAll(); + await expect(promise).rejects.toMatchObject({ name: 'RejectedError' }); + }); + + it('should create a session', async () => { + const mockOauth = new MockOAuthApi({ + idToken: 'my-id-token', + accessToken: 'my-access-token', + scopes: 'a b', + expiresInSeconds: 3600, + }); + const popupSpy = jest.spyOn(mockOauth, 'showLoginPopup'); + const helper = new GoogleAuthHelper( + { apiOrigin: 'my-origin', dev: false }, + mockOauth, + ); + + const sessionPromise = helper.createSession('a b'); + + await mockOauth.triggerAll(); + + expect(popupSpy).toBeCalledTimes(1); + expect(popupSpy.mock.calls[0][0]).toMatchObject({ + url: + 'my-origin/api/backend/auth/start?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fa%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fb', + }); + + await expect(sessionPromise).resolves.toEqual({ + idToken: 'my-id-token', + accessToken: 'my-access-token', + scopes: expect.any(GoogleScopes), + expiresAt: expect.any(Date), + }); + }); +}); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts new file mode 100644 index 0000000000..e6e0b8f01e --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts @@ -0,0 +1,192 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleIcon from '@material-ui/icons/AcUnit'; +import GoogleScopes from './GoogleScopes'; +import { GoogleSession } from './types'; +import { AuthRequester, OAuthRequestApi } from '../../..'; + +const API_PATH = '/api/backend/auth'; + +type Options = { + apiOrigin: string; + dev: boolean; +}; + +export type GoogleAuthResponse = { + accessToken: string; + idToken: string; + scopes: string; + expiresInSeconds: number; +}; + +export type AuthHelper = { + refreshSession(): Promise; + removeSession(): Promise; + createSession(scope: string): Promise; +}; + +class GoogleAuthHelper implements AuthHelper { + private readonly authRequester: AuthRequester; + private refreshPromise?: Promise; + + static create(oauthRequest: OAuthRequestApi) { + return new GoogleAuthHelper( + { + apiOrigin: window.location.origin, + dev: process.env.NODE_ENV === 'development', + }, + oauthRequest, + ); + } + + constructor( + private readonly options: Options, + private readonly oauth: OAuthRequestApi, + ) { + this.authRequester = oauth.createAuthRequester({ + provider: { + title: 'Google', + icon: GoogleIcon, + }, + onAuthRequest: (scopes) => this.showPopup(scopes.toString()), + }); + } + + async refreshSession(): Promise { + if (this.refreshPromise) { + return this.refreshPromise; + } + + this.refreshPromise = this.doAuthRefresh(); + + try { + return await this.refreshPromise; + } finally { + delete this.refreshPromise; + } + } + + private async doAuthRefresh(): Promise { + const res = await fetch(this.buildUrl('/token', { optional: true }), { + headers: { + 'x-requested-with': 'XMLHttpRequest', + }, + credentials: 'include', + }).catch((error) => { + throw new Error(`Auth refresh request failed, ${error}`); + }); + + if (!res.ok) { + const error: any = new Error( + `Auth refresh request failed with status ${res.statusText}`, + ); + error.status = res.status; + throw error; + } + + const authInfo = await res.json(); + + if (authInfo.error) { + const error = new Error(authInfo.error.message); + if (authInfo.error.name) { + error.name = authInfo.error.name; + } + throw error; + } + return GoogleAuthHelper.convertAuthInfo(authInfo); + } + + async removeSession(): Promise { + const res = await fetch(this.buildUrl('/logout'), { + method: 'POST', + headers: { + 'x-requested-with': 'XMLHttpRequest', + }, + credentials: 'include', + }); + + if (!res.ok) { + throw new Error(`Logout request failed with status ${res.status}`); + } + } + + async createSession(scope: string): Promise { + return this.authRequester(GoogleScopes.from(scope)); + } + + private async showPopup(scope: string): Promise { + const popupUrl = this.buildUrl('/start', { scope }); + + const payload = await this.oauth.showLoginPopup({ + url: popupUrl, + name: 'google-login', + origin: this.options.apiOrigin, + width: 450, + height: 730, + }); + + return GoogleAuthHelper.convertAuthInfo(payload); + } + + private buildUrl( + path: string, + query?: { [key: string]: string | boolean | undefined }, + ): string { + const queryString = this.buildQueryString({ + ...query, + dev: this.options.dev, + }); + + return `${this.options.apiOrigin}${API_PATH}${path}${queryString}`; + } + + private buildQueryString(query?: { + [key: string]: string | boolean | undefined; + }): string { + if (!query) { + return ''; + } + + const queryString = Object.entries(query) + .map(([key, value]) => { + if (typeof value === 'string') { + return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`; + } else if (value) { + return encodeURIComponent(key); + } + return undefined; + }) + .filter(Boolean) + .join('&'); + + if (!queryString) { + return ''; + } + return `?${queryString}`; + } + + private static convertAuthInfo(authInfo: GoogleAuthResponse): GoogleSession { + return { + idToken: authInfo.idToken, + accessToken: authInfo.accessToken, + scopes: GoogleScopes.from(authInfo.scopes), + expiresAt: new Date(Date.now() + authInfo.expiresInSeconds * 1000), + }; + } +} + +export default GoogleAuthHelper; diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts new file mode 100644 index 0000000000..b4eec497bb --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts @@ -0,0 +1,88 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleScopes from './GoogleScopes'; + +const PREFIX = 'https://www.googleapis.com/auth/'; + +describe('GoogleScopes', () => { + it('should be created from scopes', () => { + const scopes = GoogleScopes.from('a openid b profile'); + expect(scopes.toString()).toBe( + `${PREFIX}a openid ${PREFIX}b ${PREFIX}userinfo.profile`, + ); + }); + + it('should be created with default scopes', () => { + expect(GoogleScopes.default().toString()).toBe( + `openid ${PREFIX}userinfo.email ${PREFIX}userinfo.profile`, + ); + }); + + it('should have or not have scopes', () => { + const scopes = GoogleScopes.from(`a b ${PREFIX}c`); + expect(scopes.hasScopes('a')).toBe(true); + expect(scopes.hasScopes('a b')).toBe(true); + expect(scopes.hasScopes('b')).toBe(true); + expect(scopes.hasScopes('b c')).toBe(true); + expect(scopes.hasScopes('a b c')).toBe(true); + expect(scopes.hasScopes(`a b ${PREFIX}c`)).toBe(true); + expect(scopes.hasScopes(`a ${PREFIX}b c`)).toBe(true); + expect(scopes.hasScopes('a b c d')).toBe(false); + expect(scopes.hasScopes('d')).toBe(false); + expect(scopes.hasScopes('')).toBe(true); + expect(scopes.hasScopes('abc')).toBe(false); + expect(scopes.hasScopes(`${PREFIX}a`)).toBe(true); + }); + + it('should handle scope shorthands correctly', () => { + const scopes = GoogleScopes.default(); + expect(scopes.hasScopes('email')).toBe(true); + expect(scopes.hasScopes('profile')).toBe(true); + expect(scopes.hasScopes('openid')).toBe(true); + expect(scopes.hasScopes('userinfo.email')).toBe(true); + expect(scopes.hasScopes('userinfo.profile')).toBe(true); + expect(scopes.hasScopes('userinfo.openid')).toBe(false); + expect(scopes.hasScopes(`${PREFIX}userinfo.email`)).toBe(true); + expect(scopes.hasScopes(`${PREFIX}userinfo.profile`)).toBe(true); + expect(scopes.hasScopes(`${PREFIX}userinfo.openid`)).toBe(false); + expect(scopes.hasScopes(`${PREFIX}email`)).toBe(false); + expect(scopes.hasScopes(`${PREFIX}profile`)).toBe(false); + expect(scopes.hasScopes(`${PREFIX}openid`)).toBe(false); + }); + + it('should be extended', () => { + const scopes = GoogleScopes.from('a b'); + expect(scopes.extend('')).not.toBe(scopes); + expect(scopes.extend('d').toString()).toBe( + `${PREFIX}a ${PREFIX}b ${PREFIX}d`, + ); + expect(scopes.extend('profile').toString()).toBe( + `${PREFIX}a ${PREFIX}b ${PREFIX}userinfo.profile`, + ); + expect(scopes.extend('d profile').toString()).toBe( + `${PREFIX}a ${PREFIX}b ${PREFIX}d ${PREFIX}userinfo.profile`, + ); + expect(scopes.extend(`${PREFIX}d profile`).toString()).toBe( + `${PREFIX}a ${PREFIX}b ${PREFIX}d ${PREFIX}userinfo.profile`, + ); + expect(scopes.extend('a').toString()).toBe(scopes.toString()); + expect(scopes.extend('').toString()).toBe(scopes.toString()); + expect(scopes.extend('b').toString()).toBe(scopes.toString()); + expect(scopes.extend('b a').toString()).toBe(scopes.toString()); + expect(scopes.extend('b a b a a').toString()).toBe(scopes.toString()); + }); +}); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts new file mode 100644 index 0000000000..a29d392488 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { BasicOAuthScopes } from '../../OAuthRequestManager/BasicOAuthScopes'; +import { OAuthScopeLike } from '../../..'; + +const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; + +export default class GoogleScopes extends BasicOAuthScopes { + static from(scope: OAuthScopeLike): GoogleScopes { + return new GoogleScopes( + new Set(BasicOAuthScopes.asStrings(scope, GoogleScopes.canonicalScope)), + ); + } + + static default(): GoogleScopes { + return new GoogleScopes( + new Set([ + 'openid', + `${SCOPE_PREFIX}userinfo.email`, + `${SCOPE_PREFIX}userinfo.profile`, + ]), + ); + } + + static empty(): GoogleScopes { + return new GoogleScopes(new Set()); + } + + constructor(scopes: Set) { + super(scopes, GoogleScopes.canonicalScope); + } + + private static canonicalScope(scope: string): string { + if (scope === 'openid') { + return scope; + } + + if (scope === 'profile' || scope === 'email') { + return `${SCOPE_PREFIX}userinfo.${scope}`; + } + + if (scope.startsWith(SCOPE_PREFIX)) { + return scope; + } + + return `${SCOPE_PREFIX}${scope}`; + } +} diff --git a/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts b/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts new file mode 100644 index 0000000000..c785ace087 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts @@ -0,0 +1,36 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleScopes from './GoogleScopes'; +import MockAuthHelper, { mockIdToken, mockAccessToken } from './MockAuthHelper'; + +describe('MockAuthHelper', () => { + it('should return mock tokens', async () => { + const helper = new MockAuthHelper(); + await expect(helper.createSession()).resolves.toEqual({ + idToken: mockIdToken, + accessToken: mockAccessToken, + expiresAt: expect.any(Date), + scopes: expect.any(GoogleScopes), + }); + await expect(helper.refreshSession()).resolves.toEqual({ + idToken: mockIdToken, + accessToken: mockAccessToken, + expiresAt: expect.any(Date), + scopes: expect.any(GoogleScopes), + }); + }); +}); diff --git a/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts b/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts new file mode 100644 index 0000000000..4bd5d4e56e --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts @@ -0,0 +1,54 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleScopes from './GoogleScopes'; +import { GoogleSession } from './types'; +import { AuthHelper } from './GoogleAuthHelper'; + +export const mockIdToken = 'mock-id-token'; +export const mockAccessToken = 'mock-access-token'; + +const defaultMockSession: GoogleSession = { + idToken: mockIdToken, + accessToken: mockAccessToken, + expiresAt: new Date(), + scopes: GoogleScopes.default(), +}; + +export default class MockAuthHelper implements AuthHelper { + constructor( + private readonly mockSession: GoogleSession = defaultMockSession, + ) {} + + async refreshSession() { + return this.mockSession; + } + + async removeSession() {} + + async createSession() { + return this.mockSession; + } + + async showPopup(scope: string) { + return { + scopes: GoogleScopes.from(scope), + idToken: 'i', + accessToken: 'a', + expiresAt: new Date(), + }; + } +} diff --git a/packages/core/src/api/apis/implementations/auth/google/index.ts b/packages/core/src/api/apis/implementations/auth/google/index.ts new file mode 100644 index 0000000000..78e8a97c31 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 './types'; +export { default as GoogleAuth } from './GoogleAuth'; diff --git a/packages/core/src/api/apis/implementations/auth/google/types.ts b/packages/core/src/api/apis/implementations/auth/google/types.ts new file mode 100644 index 0000000000..a41c306aa1 --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/google/types.ts @@ -0,0 +1,94 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 GoogleScopes from './GoogleScopes'; + +/** + * This api provides access to Google OAuth credentials. It lets you request access tokens, + * which can be used to act on behalf of the user when talking to Google APIs. It also supplies + * ID Tokens, which can be passed to backend services to prove the user's identity. + * + * The API can be called directly to get access and ID tokens, which will cause a modal dialog + * to show up if the user is not yet signed in. + * + * For more fine grained control of where the sign in prompt is shown, it is possible to use + * the GoogleAuthBarrier components, which ensures that all components rendered inside it + * have synchronous access to both access and ID tokens. + * + * For full examples, see https://backstage.spotify.net/docs/backstage-frontend/apis/#google-auth-api + */ +export type GoogleAuthApi = { + /** + * Requests a Google OAuth ID Token, optionally with a set of scopes. The scopes allow you to access + * google APIs on behalf of the user. A full list of scopes can be found at https://developers.google.com/identity/protocols/googlescopes. + * + * Be sure to include all required scopes when requesting an access token. When testing your implementation + * it is best to log out the Backstage Google session and then visit your plugin page directly, as + * you might already have some required scopes in your existing session. Not requesting the correct + * scopes can lead to 403 or other authorization errors, which can be tricky to debug. + * + * This method is cheap and should be called each time an access token is used. Do not for example + * store the access token in React component state, as that could cause the token to expire. Instead + * fetch a new access token for each request. + * + * If the user has not yet logged in to Google inside Backstage, a dialog window will be shown + * that prompts the user to log in, and the returned promise will not resolve until the user has + * successfully logged in. + * + * The returned promise can be rejected, but only if the user rejects the login request. If the + * login fails because the user fails to log in to their google account, the dialog will simply + * remain and ask them to try again, and the promise will still be pending. + */ + getAccessToken(scope?: string | string[]): Promise; + + /** + * Requests a Google OAuth ID Token. + * + * Note that the ID token payload is only guaranteed to contain the user's numerical Google ID, + * email and expiration information. Do not rely on any other fields, as they might not be present. + * + * This method is cheap and should be called each time an ID token is used. Do not for example + * store the id token in React component state, as that could cause the token to expire. Instead + * fetch a new id token for each request. + * + * If the user has not yet logged in to Google inside Backstage, a dialog window will be shown + * that prompts the user to log in, and the returned promise will not resolve until the user has + * successfully logged in. + * + * The returned promise can be rejected, but only if the user rejects the login request. If the + * login fails because the user fails to log in to their google account, the dialog will simply + * remain and ask them to try again, and the promise will still be pending. + */ + getIdToken(options?: IdTokenOptions): Promise; + + /** + * Logs out the user's Google session. This will reload the page. + */ + logout(): Promise; +}; + +export type GoogleSession = { + idToken: string; + accessToken: string; + scopes: GoogleScopes; + expiresAt: Date; +}; + +export type IdTokenOptions = { + // If this is set to true, the user will not be prompted to log in, + // and an empty id token will be returned if there is no existing session. + optional?: boolean; +}; diff --git a/packages/core/src/setupTests.ts b/packages/core/src/setupTests.ts index 825bcd4115..e34bc46f4b 100644 --- a/packages/core/src/setupTests.ts +++ b/packages/core/src/setupTests.ts @@ -15,3 +15,4 @@ */ import '@testing-library/jest-dom'; +require('jest-fetch-mock').enableMocks(); From dcf2c6e3bbc6000952664abda0b5dc3e3f5950d7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 12:04:14 +0200 Subject: [PATCH 03/18] packages/core: switch google auth to use types from common definitions --- .../implementations/auth/google/GoogleAuth.ts | 9 ++- .../apis/implementations/auth/google/types.ts | 70 ------------------- 2 files changed, 7 insertions(+), 72 deletions(-) diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index d74b159095..f0147c1805 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -16,10 +16,15 @@ import { AuthHelper } from './GoogleAuthHelper'; import GoogleScopes from './GoogleScopes'; -import { GoogleAuthApi, GoogleSession, IdTokenOptions } from './types'; +import { GoogleSession } from './types'; import { OAuthScopes } from '../../..'; +import { + OAuthApi, + OpenIdConnectApi, + IdTokenOptions, +} from '../../../definitions/auth'; -class GoogleAuth implements GoogleAuthApi { +class GoogleAuth implements OAuthApi, OpenIdConnectApi { private currentSession: GoogleSession | undefined; constructor(private readonly helper: AuthHelper) {} diff --git a/packages/core/src/api/apis/implementations/auth/google/types.ts b/packages/core/src/api/apis/implementations/auth/google/types.ts index a41c306aa1..55d9a7ecf2 100644 --- a/packages/core/src/api/apis/implementations/auth/google/types.ts +++ b/packages/core/src/api/apis/implementations/auth/google/types.ts @@ -16,79 +16,9 @@ import GoogleScopes from './GoogleScopes'; -/** - * This api provides access to Google OAuth credentials. It lets you request access tokens, - * which can be used to act on behalf of the user when talking to Google APIs. It also supplies - * ID Tokens, which can be passed to backend services to prove the user's identity. - * - * The API can be called directly to get access and ID tokens, which will cause a modal dialog - * to show up if the user is not yet signed in. - * - * For more fine grained control of where the sign in prompt is shown, it is possible to use - * the GoogleAuthBarrier components, which ensures that all components rendered inside it - * have synchronous access to both access and ID tokens. - * - * For full examples, see https://backstage.spotify.net/docs/backstage-frontend/apis/#google-auth-api - */ -export type GoogleAuthApi = { - /** - * Requests a Google OAuth ID Token, optionally with a set of scopes. The scopes allow you to access - * google APIs on behalf of the user. A full list of scopes can be found at https://developers.google.com/identity/protocols/googlescopes. - * - * Be sure to include all required scopes when requesting an access token. When testing your implementation - * it is best to log out the Backstage Google session and then visit your plugin page directly, as - * you might already have some required scopes in your existing session. Not requesting the correct - * scopes can lead to 403 or other authorization errors, which can be tricky to debug. - * - * This method is cheap and should be called each time an access token is used. Do not for example - * store the access token in React component state, as that could cause the token to expire. Instead - * fetch a new access token for each request. - * - * If the user has not yet logged in to Google inside Backstage, a dialog window will be shown - * that prompts the user to log in, and the returned promise will not resolve until the user has - * successfully logged in. - * - * The returned promise can be rejected, but only if the user rejects the login request. If the - * login fails because the user fails to log in to their google account, the dialog will simply - * remain and ask them to try again, and the promise will still be pending. - */ - getAccessToken(scope?: string | string[]): Promise; - - /** - * Requests a Google OAuth ID Token. - * - * Note that the ID token payload is only guaranteed to contain the user's numerical Google ID, - * email and expiration information. Do not rely on any other fields, as they might not be present. - * - * This method is cheap and should be called each time an ID token is used. Do not for example - * store the id token in React component state, as that could cause the token to expire. Instead - * fetch a new id token for each request. - * - * If the user has not yet logged in to Google inside Backstage, a dialog window will be shown - * that prompts the user to log in, and the returned promise will not resolve until the user has - * successfully logged in. - * - * The returned promise can be rejected, but only if the user rejects the login request. If the - * login fails because the user fails to log in to their google account, the dialog will simply - * remain and ask them to try again, and the promise will still be pending. - */ - getIdToken(options?: IdTokenOptions): Promise; - - /** - * Logs out the user's Google session. This will reload the page. - */ - logout(): Promise; -}; - export type GoogleSession = { idToken: string; accessToken: string; scopes: GoogleScopes; expiresAt: Date; }; - -export type IdTokenOptions = { - // If this is set to true, the user will not be prompted to log in, - // and an empty id token will be returned if there is no existing session. - optional?: boolean; -}; From 775e339b1e258a11fdccc3d1ca0a9fc492d4793c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 13:08:00 +0200 Subject: [PATCH 04/18] packages/core: refactor GoogleAuthHelper into generic helper and move to lib --- .../implementations/auth/google/GoogleAuth.ts | 38 +++++- .../AuthHelper/AuthHelper.test.ts} | 63 +++++---- .../AuthHelper/AuthHelper.ts} | 121 +++++++++--------- .../AuthHelper}/MockAuthHelper.test.ts | 11 +- .../AuthHelper}/MockAuthHelper.ts | 35 ++--- .../implementations/lib/AuthHelper/index.ts | 17 +++ 6 files changed, 165 insertions(+), 120 deletions(-) rename packages/core/src/api/apis/implementations/{auth/google/GoogleAuthHelper.test.ts => lib/AuthHelper/AuthHelper.test.ts} (68%) rename packages/core/src/api/apis/implementations/{auth/google/GoogleAuthHelper.ts => lib/AuthHelper/AuthHelper.ts} (56%) rename packages/core/src/api/apis/implementations/{auth/google => lib/AuthHelper}/MockAuthHelper.test.ts (79%) rename packages/core/src/api/apis/implementations/{auth/google => lib/AuthHelper}/MockAuthHelper.ts (58%) create mode 100644 packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index f0147c1805..efa5ef7079 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { AuthHelper } from './GoogleAuthHelper'; +import GoogleIcon from '@material-ui/icons/AcUnit'; +import { AuthHelper } from '../../lib/AuthHelper'; import GoogleScopes from './GoogleScopes'; import { GoogleSession } from './types'; import { OAuthScopes } from '../../..'; @@ -23,11 +24,42 @@ import { OpenIdConnectApi, IdTokenOptions, } from '../../../definitions/auth'; +import { OAuthRequestApi } from '../../../definitions'; +import { GenericAuthHelper } from '../../lib/AuthHelper/AuthHelper'; + +export type GoogleAuthResponse = { + accessToken: string; + idToken: string; + scopes: string; + expiresInSeconds: number; +}; class GoogleAuth implements OAuthApi, OpenIdConnectApi { private currentSession: GoogleSession | undefined; - constructor(private readonly helper: AuthHelper) {} + static create(oauthRequestApi: OAuthRequestApi) { + const helper = new AuthHelper({ + providerPath: 'google/', + environment: 'dev', + provider: { + title: 'Google', + icon: GoogleIcon, + }, + oauthRequestApi: oauthRequestApi, + sessionTransform(res: GoogleAuthResponse): GoogleSession { + return { + idToken: res.idToken, + accessToken: res.accessToken, + scopes: GoogleScopes.from(res.scopes), + expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), + }; + }, + }); + + return new GoogleAuth(helper); + } + + constructor(private readonly helper: GenericAuthHelper) {} async getAccessToken(scope?: string | string[]) { const session = await this.getSession({ optional: false, scope }); @@ -129,7 +161,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { if (scope) { newScope = newScope.extend(scope); } - return newScope.toString(); + return newScope; } } export default GoogleAuth; diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts similarity index 68% rename from packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts rename to packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts index fbd45fd779..713a5e7d05 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts @@ -14,13 +14,30 @@ * limitations under the License. */ -import GoogleAuthHelper from './GoogleAuthHelper'; -import GoogleScopes from './GoogleScopes'; +import ProviderIcon from '@material-ui/icons/AcUnit'; +import { AuthHelper } from './AuthHelper'; import MockOAuthApi from '../../OAuthRequestManager/MockOAuthApi'; +import { BasicOAuthScopes } from '../../OAuthRequestManager/BasicOAuthScopes'; const anyFetch = fetch as any; -describe('GoogleAuthHelper', () => { +const defaultOptions = { + apiOrigin: 'my-origin', + providerPath: 'my-provider', + environment: 'production', + provider: { + title: 'My Provider', + icon: ProviderIcon, + }, + oauthRequestApi: new MockOAuthApi(), + sessionTransform: ({ expiresInSeconds, ...res }: any) => ({ + ...res, + scopes: BasicOAuthScopes.from(res.scopes), + expiresAt: new Date(Date.now() + expiresInSeconds * 1000), + }), +}; + +describe('AuthHelper', () => { afterEach(() => { jest.resetAllMocks(); anyFetch.resetMocks(); @@ -36,10 +53,7 @@ describe('GoogleAuthHelper', () => { }), ); - const helper = new GoogleAuthHelper( - { apiOrigin: 'my-origin', dev: false }, - new MockOAuthApi(), - ); + const helper = new AuthHelper(defaultOptions); const session = await helper.refreshSession(); expect(session.idToken).toBe('mock-id-token'); expect(session.accessToken).toBe('mock-access-token'); @@ -51,10 +65,7 @@ describe('GoogleAuthHelper', () => { it('should handle failure to refresh session', async () => { anyFetch.mockRejectOnce(new Error('Network NOPE')); - const helper = new GoogleAuthHelper( - { apiOrigin: 'my-origin', dev: true }, - new MockOAuthApi(), - ); + const helper = new AuthHelper(defaultOptions); await expect(helper.refreshSession()).rejects.toThrow( 'Auth refresh request failed, Error: Network NOPE', ); @@ -63,10 +74,7 @@ describe('GoogleAuthHelper', () => { it('should handle failure response when refreshing session', async () => { anyFetch.mockResponseOnce({}, { status: 401, statusText: 'NOPE' }); - const helper = new GoogleAuthHelper( - { apiOrigin: 'my-origin', dev: false }, - new MockOAuthApi(), - ); + const helper = new AuthHelper(defaultOptions); await expect(helper.refreshSession()).rejects.toThrow( 'Auth refresh request failed with status NOPE', ); @@ -74,11 +82,11 @@ describe('GoogleAuthHelper', () => { it('should fail if popup was rejected', async () => { const mockOauth = new MockOAuthApi(); - const helper = new GoogleAuthHelper( - { apiOrigin: 'my-origin', dev: false }, - mockOauth, - ); - const promise = helper.createSession('a b'); + const helper = new AuthHelper({ + ...defaultOptions, + oauthRequestApi: mockOauth, + }); + const promise = helper.createSession(BasicOAuthScopes.from('a b')); await mockOauth.rejectAll(); await expect(promise).rejects.toMatchObject({ name: 'RejectedError' }); }); @@ -91,25 +99,24 @@ describe('GoogleAuthHelper', () => { expiresInSeconds: 3600, }); const popupSpy = jest.spyOn(mockOauth, 'showLoginPopup'); - const helper = new GoogleAuthHelper( - { apiOrigin: 'my-origin', dev: false }, - mockOauth, - ); + const helper = new AuthHelper({ + ...defaultOptions, + oauthRequestApi: mockOauth, + }); - const sessionPromise = helper.createSession('a b'); + const sessionPromise = helper.createSession(BasicOAuthScopes.from('a b')); await mockOauth.triggerAll(); expect(popupSpy).toBeCalledTimes(1); expect(popupSpy.mock.calls[0][0]).toMatchObject({ - url: - 'my-origin/api/backend/auth/start?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fa%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fb', + url: 'my-origin/api/auth/my-provider/start?scope=a%20b&env=production', }); await expect(sessionPromise).resolves.toEqual({ idToken: 'my-id-token', accessToken: 'my-access-token', - scopes: expect.any(GoogleScopes), + scopes: expect.any(BasicOAuthScopes), expiresAt: expect.any(Date), }); }); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts similarity index 56% rename from packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts rename to packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts index e6e0b8f01e..d42055d332 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts @@ -14,59 +14,69 @@ * limitations under the License. */ -import GoogleIcon from '@material-ui/icons/AcUnit'; -import GoogleScopes from './GoogleScopes'; -import { GoogleSession } from './types'; -import { AuthRequester, OAuthRequestApi } from '../../..'; +import { AuthRequester } from '../../..'; +import { + OAuthRequestApi, + AuthProvider, + OAuthScopes, +} from '../../../definitions'; -const API_PATH = '/api/backend/auth'; +const DEFAULT_BASE_PATH = '/api/auth/'; -type Options = { - apiOrigin: string; - dev: boolean; +type Options = { + apiOrigin?: string; + basePath?: string; + providerPath: string; + environment: string; + provider: AuthProvider; + oauthRequestApi: OAuthRequestApi; + sessionTransform?(response: any): AuthSession | Promise; }; -export type GoogleAuthResponse = { - accessToken: string; - idToken: string; - scopes: string; - expiresInSeconds: number; -}; - -export type AuthHelper = { - refreshSession(): Promise; +export type GenericAuthHelper = { + refreshSession(): Promise; removeSession(): Promise; - createSession(scope: string): Promise; + createSession(scope: OAuthScopes): Promise; }; -class GoogleAuthHelper implements AuthHelper { - private readonly authRequester: AuthRequester; - private refreshPromise?: Promise; +export class AuthHelper implements AuthHelper { + private readonly apiOrigin: string; + private readonly basePath: string; + private readonly providerPath: string; + private readonly environment: string; + private readonly provider: AuthProvider; + private readonly oauthRequestApi: OAuthRequestApi; + private readonly authRequester: AuthRequester; + private readonly sessionTransform: (response: any) => Promise; - static create(oauthRequest: OAuthRequestApi) { - return new GoogleAuthHelper( - { - apiOrigin: window.location.origin, - dev: process.env.NODE_ENV === 'development', - }, - oauthRequest, - ); - } + private refreshPromise?: Promise; - constructor( - private readonly options: Options, - private readonly oauth: OAuthRequestApi, - ) { - this.authRequester = oauth.createAuthRequester({ - provider: { - title: 'Google', - icon: GoogleIcon, - }, + constructor(options: Options) { + const { + apiOrigin = window.location.origin, + basePath = DEFAULT_BASE_PATH, + providerPath, + environment, + provider, + oauthRequestApi, + sessionTransform = (id) => id, + } = options; + + this.authRequester = oauthRequestApi.createAuthRequester({ + provider, onAuthRequest: (scopes) => this.showPopup(scopes.toString()), }); + + this.apiOrigin = apiOrigin; + this.basePath = basePath; + this.providerPath = providerPath; + this.environment = environment; + this.provider = provider; + this.oauthRequestApi = oauthRequestApi; + this.sessionTransform = sessionTransform; } - async refreshSession(): Promise { + async refreshSession(): Promise { if (this.refreshPromise) { return this.refreshPromise; } @@ -107,7 +117,7 @@ class GoogleAuthHelper implements AuthHelper { } throw error; } - return GoogleAuthHelper.convertAuthInfo(authInfo); + return await this.sessionTransform(authInfo); } async removeSession(): Promise { @@ -124,22 +134,22 @@ class GoogleAuthHelper implements AuthHelper { } } - async createSession(scope: string): Promise { - return this.authRequester(GoogleScopes.from(scope)); + async createSession(scope: OAuthScopes): Promise { + return this.authRequester(scope); } - private async showPopup(scope: string): Promise { + private async showPopup(scope: string): Promise { const popupUrl = this.buildUrl('/start', { scope }); - const payload = await this.oauth.showLoginPopup({ + const payload = await this.oauthRequestApi.showLoginPopup({ url: popupUrl, - name: 'google-login', - origin: this.options.apiOrigin, + name: `${this.provider.title} Login`, + origin: this.apiOrigin, width: 450, height: 730, }); - return GoogleAuthHelper.convertAuthInfo(payload); + return await this.sessionTransform(payload); } private buildUrl( @@ -148,10 +158,10 @@ class GoogleAuthHelper implements AuthHelper { ): string { const queryString = this.buildQueryString({ ...query, - dev: this.options.dev, + env: this.environment, }); - return `${this.options.apiOrigin}${API_PATH}${path}${queryString}`; + return `${this.apiOrigin}${this.basePath}${this.providerPath}${path}${queryString}`; } private buildQueryString(query?: { @@ -178,15 +188,4 @@ class GoogleAuthHelper implements AuthHelper { } return `?${queryString}`; } - - private static convertAuthInfo(authInfo: GoogleAuthResponse): GoogleSession { - return { - idToken: authInfo.idToken, - accessToken: authInfo.accessToken, - scopes: GoogleScopes.from(authInfo.scopes), - expiresAt: new Date(Date.now() + authInfo.expiresInSeconds * 1000), - }; - } } - -export default GoogleAuthHelper; diff --git a/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.test.ts similarity index 79% rename from packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts rename to packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.test.ts index c785ace087..0cb1e13163 100644 --- a/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.test.ts @@ -14,23 +14,22 @@ * limitations under the License. */ -import GoogleScopes from './GoogleScopes'; -import MockAuthHelper, { mockIdToken, mockAccessToken } from './MockAuthHelper'; +import MockAuthHelper, { mockAccessToken } from './MockAuthHelper'; describe('MockAuthHelper', () => { it('should return mock tokens', async () => { const helper = new MockAuthHelper(); + await expect(helper.createSession()).resolves.toEqual({ - idToken: mockIdToken, accessToken: mockAccessToken, expiresAt: expect.any(Date), - scopes: expect.any(GoogleScopes), + scopes: expect.any(String), }); + await expect(helper.refreshSession()).resolves.toEqual({ - idToken: mockIdToken, accessToken: mockAccessToken, expiresAt: expect.any(Date), - scopes: expect.any(GoogleScopes), + scopes: expect.any(String), }); }); }); diff --git a/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts similarity index 58% rename from packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts rename to packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts index 4bd5d4e56e..c86854bfc2 100644 --- a/packages/core/src/api/apis/implementations/auth/google/MockAuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts @@ -14,24 +14,24 @@ * limitations under the License. */ -import GoogleScopes from './GoogleScopes'; -import { GoogleSession } from './types'; -import { AuthHelper } from './GoogleAuthHelper'; +import { GenericAuthHelper } from './AuthHelper'; -export const mockIdToken = 'mock-id-token'; export const mockAccessToken = 'mock-access-token'; -const defaultMockSession: GoogleSession = { - idToken: mockIdToken, - accessToken: mockAccessToken, - expiresAt: new Date(), - scopes: GoogleScopes.default(), +type MockSession = { + accessToken: string; + expiresAt: Date; + scopes: string; }; -export default class MockAuthHelper implements AuthHelper { - constructor( - private readonly mockSession: GoogleSession = defaultMockSession, - ) {} +const defaultMockSession: MockSession = { + accessToken: mockAccessToken, + expiresAt: new Date(), + scopes: 'profile email', +}; + +export default class MockAuthHelper implements GenericAuthHelper { + constructor(private readonly mockSession: MockSession = defaultMockSession) {} async refreshSession() { return this.mockSession; @@ -42,13 +42,4 @@ export default class MockAuthHelper implements AuthHelper { async createSession() { return this.mockSession; } - - async showPopup(scope: string) { - return { - scopes: GoogleScopes.from(scope), - idToken: 'i', - accessToken: 'a', - expiresAt: new Date(), - }; - } } diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts new file mode 100644 index 0000000000..29066e4a4b --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { AuthHelper } from './AuthHelper'; From 8b6f4a4ffe5a20a9a5b0cc55e48aa4b47f9e5227 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 14:47:56 +0200 Subject: [PATCH 05/18] packages/core: add more tests for OAuthRequestManager --- .../OAuthRequestManager.test.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts index 940c0230f9..115f2b2c95 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts @@ -14,7 +14,50 @@ * limitations under the License. */ +import ProviderIcon from '@material-ui/icons/AcUnit'; import { OAuthRequestManager } from './OAuthRequestManager'; +import { BasicOAuthScopes } from './BasicOAuthScopes'; + +describe('OAuthRequestManager', () => { + it('should forward a requests', async () => { + const manager = new OAuthRequestManager(); + + const reqSpy = jest.fn(); + manager.authRequest$().subscribe(reqSpy); + + const requester = manager.createAuthRequester({ + provider: { + title: 'My Provider', + icon: ProviderIcon, + }, + onAuthRequest: async () => 'hello', + }); + + expect(reqSpy).toHaveBeenCalledTimes(0); + await 'a tick'; + expect(reqSpy).toHaveBeenCalledTimes(2); + expect(reqSpy).toHaveBeenLastCalledWith([]); + + const req = requester(BasicOAuthScopes.from('my-scope')); + + expect(reqSpy).toHaveBeenCalledTimes(3); + expect(reqSpy).toHaveBeenLastCalledWith([ + expect.objectContaining({ + reject: expect.any(Function), + trigger: expect.any(Function), + }), + ]); + + await expect(Promise.race([req, Promise.resolve('not yet')])).resolves.toBe( + 'not yet', + ); + + const [request] = reqSpy.mock.calls[2][0]; + request.trigger(); + + await expect(req).resolves.toBe('hello'); + }); +}); describe('OAuthApi login popup', () => { afterEach(() => { From bf6c807d6d324053dc9d1bc2a9a0faeb58399d3c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 14:56:59 +0200 Subject: [PATCH 06/18] packages/core: move out showLoginPopup from OAuthRequestApi to separate lib module + tweak and document payload format --- .../api/apis/definitions/OAuthRequestApi.ts | 40 ----- .../OAuthRequestManager/MockOAuthApi.test.ts | 12 +- .../OAuthRequestManager/MockOAuthApi.ts | 25 +-- .../OAuthRequestManager.test.ts | 161 ----------------- .../OAuthRequestManager.ts | 61 ------- .../lib/AuthHelper/AuthHelper.test.ts | 17 +- .../lib/AuthHelper/AuthHelper.ts | 5 +- .../implementations/lib/loginPopup.test.ts | 170 ++++++++++++++++++ .../apis/implementations/lib/loginPopup.ts | 127 +++++++++++++ 9 files changed, 321 insertions(+), 297 deletions(-) create mode 100644 packages/core/src/api/apis/implementations/lib/loginPopup.test.ts create mode 100644 packages/core/src/api/apis/implementations/lib/loginPopup.ts diff --git a/packages/core/src/api/apis/definitions/OAuthRequestApi.ts b/packages/core/src/api/apis/definitions/OAuthRequestApi.ts index 09e5f77718..a6441e9d1a 100644 --- a/packages/core/src/api/apis/definitions/OAuthRequestApi.ts +++ b/packages/core/src/api/apis/definitions/OAuthRequestApi.ts @@ -30,36 +30,6 @@ export type OAuthScopeLike = | string[] /** Array of individual scope strings */ | OAuthScopes; -/** - * Options used to open a login popup. - */ -export type LoginPopupOptions = { - /** - * The URL that the auth popup should point to - */ - url: string; - - /** - * The name of the popup, as in second argument to window.open - */ - name: string; - - /** - * The origin of the final popup page that will post a message to this window. - */ - origin: string; - - /** - * The width of the popup in pixels, defaults to 500 - */ - width?: number; - - /** - * The height of the popup in pixels, defaults to 700 - */ - height?: number; -}; - /** * Information about the auth provider that we're requesting a login towards. * @@ -139,16 +109,6 @@ export type PendingAuthRequest = { * Provides helpers for implemented OAuth login flows within Backstage. */ export type OAuthRequestApi = { - /** - * Show a popup pointing to a URL that starts an OAuth flow. - * - * The redirect handler of the flow should use postMessage to communicate back - * to the app window. - * - * The returned promise resolves to the contents of the message that was posted from the auth popup. - */ - showLoginPopup(options: LoginPopupOptions): Promise; - /** * A utility for showing login popups or similar things, and merging together multiple requests for * different scopes into one request that inclues all scopes. diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts index d6346c688b..61e466f200 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts @@ -20,12 +20,10 @@ import { BasicOAuthScopes } from './BasicOAuthScopes'; describe('MockOAuthApi', () => { it('should trigger all requests', async () => { - const popupResult = { is: 'done' }; - const mock = new MockOAuthApi(popupResult); + const authResult = { is: 'done' }; + const mock = new MockOAuthApi(); - const authHandler1 = jest - .fn() - .mockImplementation(() => mock.showLoginPopup()); + const authHandler1 = jest.fn().mockImplementation(() => authResult); const requester1 = mock.createAuthRequester({ provider: { icon: PowerIcon, title: 'Test' }, onAuthRequest: authHandler1, @@ -52,8 +50,8 @@ describe('MockOAuthApi', () => { await mock.triggerAll(); await expect(Promise.all(promises)).resolves.toEqual([ - popupResult, - popupResult, + authResult, + authResult, 'other', 'other', 'other', diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts index f52c50dbe5..5d539dca27 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts @@ -14,18 +14,12 @@ * limitations under the License. */ -import { - OAuthRequestApi, - AuthRequesterOptions, - LoginPopupOptions, -} from '../../definitions'; +import { OAuthRequestApi, AuthRequesterOptions } from '../../definitions'; import { OAuthRequestManager } from './OAuthRequestManager'; export default class MockOAuthApi implements OAuthRequestApi { private readonly real = new OAuthRequestManager(); - constructor(private readonly popupResult = {}) {} - createAuthRequester(options: AuthRequesterOptions) { return this.real.createAuthRequester(options); } @@ -37,10 +31,10 @@ export default class MockOAuthApi implements OAuthRequestApi { async triggerAll() { await Promise.resolve(); // Wait a tick to allow new requests to get forwarded - return new Promise((resolve) => { - const subscription = this.authRequest$().subscribe((requests) => { + return new Promise(resolve => { + const subscription = this.authRequest$().subscribe(requests => { subscription.unsubscribe(); - Promise.all(requests.map((request) => request.trigger())).then(() => + Promise.all(requests.map(request => request.trigger())).then(() => resolve(), ); }); @@ -50,17 +44,12 @@ export default class MockOAuthApi implements OAuthRequestApi { async rejectAll() { await Promise.resolve(); // Wait a tick to allow new requests to get forwarded - return new Promise((resolve) => { - const subscription = this.authRequest$().subscribe((requests) => { + return new Promise(resolve => { + const subscription = this.authRequest$().subscribe(requests => { subscription.unsubscribe(); - requests.map((request) => request.reject()); + requests.map(request => request.reject()); resolve(); }); }); } - - async showLoginPopup(options?: LoginPopupOptions): Promise { - // Working around linter complaints, can't remove options since we want correct mock types - return options ? this.popupResult : this.popupResult; - } } diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts index 115f2b2c95..922cb20a25 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts @@ -58,164 +58,3 @@ describe('OAuthRequestManager', () => { await expect(req).resolves.toBe('hello'); }); }); - -describe('OAuthApi login popup', () => { - afterEach(() => { - jest.resetAllMocks(); - }); - - it('should show an auth popup', async () => { - const oauth = new OAuthRequestManager(); - - const popupMock = { closed: false }; - const openSpy = jest - .spyOn(window, 'open') - .mockReturnValue(popupMock as Window); - const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); - const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); - - const payloadPromise = oauth.showLoginPopup({ - url: - 'my-origin/api/backend/auth/start?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fa%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fb', - name: 'test-popup', - origin: 'my-origin', - }); - - expect(openSpy).toBeCalledTimes(1); - expect(openSpy.mock.calls[0][0]).toBe( - 'my-origin/api/backend/auth/start?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fa%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fb', - ); - expect(openSpy.mock.calls[0][1]).toBe('test-popup'); - expect(addEventListenerSpy).toBeCalledTimes(1); - expect(removeEventListenerSpy).toBeCalledTimes(0); - - const listener = addEventListenerSpy.mock.calls[0][1] as EventListener; - - await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( - 'waiting', - ); - - listener({} as MessageEvent); - - await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( - 'waiting', - ); - - // None of these should be accepted - listener({ source: popupMock } as MessageEvent); - listener({ origin: 'my-origin' } as MessageEvent); - listener({ data: { type: 'oauth-result' } } as MessageEvent); - listener({ - source: popupMock, - origin: 'my-origin', - data: {}, - } as MessageEvent); - listener({ - source: popupMock, - origin: 'my-origin', - data: { type: 'not-oauth-result', payload: {} }, - } as MessageEvent); - - await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( - 'waiting', - ); - - const myPayload = {}; - - // This should be accepted as a valid sessions response - listener({ - source: popupMock, - origin: 'my-origin', - data: { - type: 'oauth-result', - payload: myPayload, - }, - } as MessageEvent); - - await expect(payloadPromise).resolves.toBe(myPayload); - - expect(openSpy).toBeCalledTimes(1); - expect(addEventListenerSpy).toBeCalledTimes(1); - expect(removeEventListenerSpy).toBeCalledTimes(1); - }); - - it('should fail if popup returns error', async () => { - const oauth = new OAuthRequestManager(); - - const popupMock = { closed: false }; - const openSpy = jest - .spyOn(window, 'open') - .mockReturnValue(popupMock as Window); - const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); - const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); - - const payloadPromise = oauth.showLoginPopup({ - url: 'url', - name: 'name', - origin: 'my-origin', - }); - - expect(openSpy).toBeCalledTimes(1); - expect(addEventListenerSpy).toBeCalledTimes(1); - expect(removeEventListenerSpy).toBeCalledTimes(0); - - const listener = addEventListenerSpy.mock.calls[0][1] as EventListener; - - listener({ - source: popupMock, - origin: 'my-origin', - data: { - type: 'oauth-result', - payload: { - error: { - message: 'NOPE', - name: 'NopeError', - }, - }, - }, - } as MessageEvent); - - await expect(payloadPromise).rejects.toThrow({ - name: 'NopeError', - message: 'NOPE', - }); - - expect(openSpy).toBeCalledTimes(1); - expect(addEventListenerSpy).toBeCalledTimes(1); - expect(removeEventListenerSpy).toBeCalledTimes(1); - }); - - it('should fail if popup is closed', async () => { - const oauth = new OAuthRequestManager(); - - const openSpy = jest - .spyOn(window, 'open') - .mockReturnValue({ closed: false } as Window); - const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); - const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); - const popupMock = { closed: false }; - - openSpy.mockReturnValue(popupMock as Window); - - const payloadPromise = oauth.showLoginPopup({ - url: 'url', - name: 'name', - origin: 'origin', - }); - - expect(openSpy).toBeCalledTimes(1); - expect(addEventListenerSpy).toBeCalledTimes(1); - expect(removeEventListenerSpy).toBeCalledTimes(0); - - setTimeout(() => { - popupMock.closed = true; - }, 150); - await expect(payloadPromise).rejects.toThrow( - 'Login failed, popup was closed', - ); - - expect(openSpy).toBeCalledTimes(1); - expect(addEventListenerSpy).toBeCalledTimes(1); - expect(removeEventListenerSpy).toBeCalledTimes(1); - }); -}); diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts index 632ec059eb..8b3b2d94c2 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts @@ -16,7 +16,6 @@ import { OAuthRequestApi, - LoginPopupOptions, PendingAuthRequest, AuthRequester, AuthRequesterOptions, @@ -90,64 +89,4 @@ export class OAuthRequestManager implements OAuthRequestApi { authRequest$(): Observable { return this.subject; } - - async showLoginPopup(options: LoginPopupOptions): Promise { - return new Promise((resolve, reject) => { - const width = options.width || 500; - const height = options.height || 700; - const left = window.screen.width / 2 - width / 2; - const top = window.screen.height / 2 - height / 2; - - const popup = window.open( - options.url, - options.name, - `menubar=no,location=no,resizable=no,scrollbars=no,status=no,width=${width},height=${height},top=${top},left=${left}`, - ); - - if (!popup || typeof popup.closed === 'undefined' || popup.closed) { - reject(new Error('Failed to open auth popup.')); - return; - } - - const messageListener = (event: MessageEvent) => { - if (event.source !== popup) { - return; - } - if (event.origin !== options.origin) { - return; - } - const { data } = event; - if (data.type !== 'oauth-result') { - return; - } - - if (data.payload.error) { - const error = new Error(data.payload.error.message); - error.name = data.payload.error.name; - // TODO: proper error type - // error.extra = data.payload.error.extra; - reject(error); - } else { - resolve(data.payload); - } - done(); - }; - - const intervalId = setInterval(() => { - if (popup.closed) { - const error = new Error('Login failed, popup was closed'); - error.name = 'PopupClosedError'; - reject(error); - done(); - } - }, 100); - - function done() { - window.removeEventListener('message', messageListener); - clearInterval(intervalId); - } - - window.addEventListener('message', messageListener); - }); - } } diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts index 713a5e7d05..5ca95cb600 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts @@ -18,6 +18,7 @@ import ProviderIcon from '@material-ui/icons/AcUnit'; import { AuthHelper } from './AuthHelper'; import MockOAuthApi from '../../OAuthRequestManager/MockOAuthApi'; import { BasicOAuthScopes } from '../../OAuthRequestManager/BasicOAuthScopes'; +import * as loginPopup from '../loginPopup'; const anyFetch = fetch as any; @@ -92,13 +93,15 @@ describe('AuthHelper', () => { }); it('should create a session', async () => { - const mockOauth = new MockOAuthApi({ - idToken: 'my-id-token', - accessToken: 'my-access-token', - scopes: 'a b', - expiresInSeconds: 3600, - }); - const popupSpy = jest.spyOn(mockOauth, 'showLoginPopup'); + const mockOauth = new MockOAuthApi(); + const popupSpy = jest + .spyOn(loginPopup, 'showLoginPopup') + .mockResolvedValue({ + idToken: 'my-id-token', + accessToken: 'my-access-token', + scopes: 'a b', + expiresInSeconds: 3600, + }); const helper = new AuthHelper({ ...defaultOptions, oauthRequestApi: mockOauth, diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts index d42055d332..dc79641e70 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts @@ -20,6 +20,7 @@ import { AuthProvider, OAuthScopes, } from '../../../definitions'; +import { showLoginPopup } from '../loginPopup'; const DEFAULT_BASE_PATH = '/api/auth/'; @@ -45,7 +46,6 @@ export class AuthHelper implements AuthHelper { private readonly providerPath: string; private readonly environment: string; private readonly provider: AuthProvider; - private readonly oauthRequestApi: OAuthRequestApi; private readonly authRequester: AuthRequester; private readonly sessionTransform: (response: any) => Promise; @@ -72,7 +72,6 @@ export class AuthHelper implements AuthHelper { this.providerPath = providerPath; this.environment = environment; this.provider = provider; - this.oauthRequestApi = oauthRequestApi; this.sessionTransform = sessionTransform; } @@ -141,7 +140,7 @@ export class AuthHelper implements AuthHelper { private async showPopup(scope: string): Promise { const popupUrl = this.buildUrl('/start', { scope }); - const payload = await this.oauthRequestApi.showLoginPopup({ + const payload = await showLoginPopup({ url: popupUrl, name: `${this.provider.title} Login`, origin: this.apiOrigin, diff --git a/packages/core/src/api/apis/implementations/lib/loginPopup.test.ts b/packages/core/src/api/apis/implementations/lib/loginPopup.test.ts new file mode 100644 index 0000000000..755438fead --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/loginPopup.test.ts @@ -0,0 +1,170 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { showLoginPopup } from './loginPopup'; + +describe('showLoginPopup', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should show an auth popup', async () => { + const popupMock = { closed: false }; + const openSpy = jest + .spyOn(window, 'open') + .mockReturnValue(popupMock as Window); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); + + const payloadPromise = showLoginPopup({ + url: + 'my-origin/api/backend/auth/start?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fa%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fb', + name: 'test-popup', + origin: 'my-origin', + }); + + expect(openSpy).toBeCalledTimes(1); + expect(openSpy.mock.calls[0][0]).toBe( + 'my-origin/api/backend/auth/start?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fa%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fb', + ); + expect(openSpy.mock.calls[0][1]).toBe('test-popup'); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(0); + + const listener = addEventListenerSpy.mock.calls[0][1] as EventListener; + + await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( + 'waiting', + ); + + listener({} as MessageEvent); + + await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( + 'waiting', + ); + + // None of these should be accepted + listener({ source: popupMock } as MessageEvent); + listener({ origin: 'my-origin' } as MessageEvent); + listener({ data: { type: 'auth-result' } } as MessageEvent); + listener({ + source: popupMock, + origin: 'my-origin', + data: {}, + } as MessageEvent); + listener({ + source: popupMock, + origin: 'my-origin', + data: { type: 'not-auth-result', payload: {} }, + } as MessageEvent); + + await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( + 'waiting', + ); + + const myPayload = {}; + + // This should be accepted as a valid sessions response + listener({ + source: popupMock, + origin: 'my-origin', + data: { + type: 'auth-result', + payload: myPayload, + }, + } as MessageEvent); + + await expect(payloadPromise).resolves.toBe(myPayload); + + expect(openSpy).toBeCalledTimes(1); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(1); + }); + + it('should fail if popup returns error', async () => { + const popupMock = { closed: false }; + const openSpy = jest + .spyOn(window, 'open') + .mockReturnValue(popupMock as Window); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); + + const payloadPromise = showLoginPopup({ + url: 'url', + name: 'name', + origin: 'my-origin', + }); + + expect(openSpy).toBeCalledTimes(1); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(0); + + const listener = addEventListenerSpy.mock.calls[0][1] as EventListener; + + listener({ + source: popupMock, + origin: 'my-origin', + data: { + type: 'auth-result', + error: { + message: 'NOPE', + name: 'NopeError', + }, + }, + } as MessageEvent); + + await expect(payloadPromise).rejects.toThrow({ + name: 'NopeError', + message: 'NOPE', + }); + + expect(openSpy).toBeCalledTimes(1); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(1); + }); + + it('should fail if popup is closed', async () => { + const openSpy = jest + .spyOn(window, 'open') + .mockReturnValue({ closed: false } as Window); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); + const popupMock = { closed: false }; + + openSpy.mockReturnValue(popupMock as Window); + + const payloadPromise = showLoginPopup({ + url: 'url', + name: 'name', + origin: 'origin', + }); + + expect(openSpy).toBeCalledTimes(1); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(0); + + setTimeout(() => { + popupMock.closed = true; + }, 150); + await expect(payloadPromise).rejects.toThrow( + 'Login failed, popup was closed', + ); + + expect(openSpy).toBeCalledTimes(1); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(1); + }); +}); diff --git a/packages/core/src/api/apis/implementations/lib/loginPopup.ts b/packages/core/src/api/apis/implementations/lib/loginPopup.ts new file mode 100644 index 0000000000..b4af94be49 --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/loginPopup.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2020 Spotify AB + * + * 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. + */ + +/** + * Options used to open a login popup. + */ +export type LoginPopupOptions = { + /** + * The URL that the auth popup should point to + */ + url: string; + + /** + * The name of the popup, as in second argument to window.open + */ + name: string; + + /** + * The origin of the final popup page that will post a message to this window. + */ + origin: string; + + /** + * The width of the popup in pixels, defaults to 500 + */ + width?: number; + + /** + * The height of the popup in pixels, defaults to 700 + */ + height?: number; +}; + +type AuthResult = + | { + type: 'auth-result'; + payload: any; + } + | { + type: 'auth-result'; + error: { + name: string; + message: string; + }; + }; + +/** + * Show a popup pointing to a URL that starts an auth flow. + * + * The redirect handler of the flow should use postMessage to communicate back + * to the app window. The message posted to the app must match the AuthResult type. + * + * The returned promise resolves to the contents of the message that was posted from the auth popup. + */ +export function showLoginPopup(options: LoginPopupOptions): Promise { + return new Promise((resolve, reject) => { + const width = options.width || 500; + const height = options.height || 700; + const left = window.screen.width / 2 - width / 2; + const top = window.screen.height / 2 - height / 2; + + const popup = window.open( + options.url, + options.name, + `menubar=no,location=no,resizable=no,scrollbars=no,status=no,width=${width},height=${height},top=${top},left=${left}`, + ); + + if (!popup || typeof popup.closed === 'undefined' || popup.closed) { + reject(new Error('Failed to open auth popup.')); + return; + } + + const messageListener = (event: MessageEvent) => { + if (event.source !== popup) { + return; + } + if (event.origin !== options.origin) { + return; + } + const { data } = event; + if (data.type !== 'auth-result') { + return; + } + const authResult = data as AuthResult; + + if ('error' in authResult) { + const error = new Error(authResult.error.message); + error.name = authResult.error.name; + // TODO: proper error type + // error.extra = authResult.error.extra; + reject(error); + } else { + resolve(authResult.payload); + } + done(); + }; + + const intervalId = setInterval(() => { + if (popup.closed) { + const error = new Error('Login failed, popup was closed'); + error.name = 'PopupClosedError'; + reject(error); + done(); + } + }, 100); + + function done() { + window.removeEventListener('message', messageListener); + clearInterval(intervalId); + } + + window.addEventListener('message', messageListener); + }); +} From 2d65c9a38fb181ad74779aeffe301aee817f9153 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 17:22:05 +0200 Subject: [PATCH 07/18] packages/core: remove custom OAuth scope classes in favor for early transform functions --- .../api/apis/definitions/OAuthRequestApi.ts | 16 +--- .../OAuthRequestManager/BasicOAuthScopes.ts | 77 ---------------- .../OAuthRequestManager/MockOAuthApi.test.ts | 25 +++--- .../OAuthPendingRequests.test.ts | 25 +++--- .../OAuthPendingRequests.ts | 46 ++++++++-- .../OAuthRequestManager.test.ts | 3 +- .../OAuthRequestManager.ts | 4 +- .../auth/google/GoogleAuth.test.ts | 15 ++-- .../implementations/auth/google/GoogleAuth.ts | 64 +++++++++++--- .../auth/google/GoogleScopes.test.ts | 88 ------------------- .../auth/google/GoogleScopes.ts | 62 ------------- .../apis/implementations/auth/google/types.ts | 4 +- .../lib/AuthHelper/AuthHelper.test.ts | 11 ++- .../lib/AuthHelper/AuthHelper.ts | 18 ++-- 14 files changed, 138 insertions(+), 320 deletions(-) delete mode 100644 packages/core/src/api/apis/implementations/OAuthRequestManager/BasicOAuthScopes.ts delete mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts delete mode 100644 packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts diff --git a/packages/core/src/api/apis/definitions/OAuthRequestApi.ts b/packages/core/src/api/apis/definitions/OAuthRequestApi.ts index a6441e9d1a..52d30a763e 100644 --- a/packages/core/src/api/apis/definitions/OAuthRequestApi.ts +++ b/packages/core/src/api/apis/definitions/OAuthRequestApi.ts @@ -18,18 +18,6 @@ import { IconComponent } from '../../../icons'; import { Observable } from '../../types'; import { ApiRef } from '../ApiRef'; -export type OAuthScopes = { - extend(scopes: OAuthScopeLike): OAuthScopes; - hasScopes(scopes: OAuthScopeLike): boolean; - toSet(): Set; - toString(): string; -}; - -export type OAuthScopeLike = - | string /** Space separated scope strings */ - | string[] /** Array of individual scope strings */ - | OAuthScopes; - /** * Information about the auth provider that we're requesting a login towards. * @@ -62,7 +50,7 @@ export type AuthRequesterOptions = { * Implementation of the auth flow, which will be called synchronously when * trigger() is called on an auth requests. */ - onAuthRequest(scope: OAuthScopes): Promise; + onAuthRequest(scopes: Set): Promise; }; /** @@ -76,7 +64,7 @@ export type AuthRequesterOptions = { * union of all requested scopes. */ export type AuthRequester = ( - scope: OAuthScopes, + scopes: Set, ) => Promise; /** diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/BasicOAuthScopes.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/BasicOAuthScopes.ts deleted file mode 100644 index fb022c5385..0000000000 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/BasicOAuthScopes.ts +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 { OAuthScopes, OAuthScopeLike } from '../../definitions'; - -/** - * The BasicOAuthScopes class is an implementation of OAuthScopes that - * works for any simple comma- or space-separated format of scope. - */ -export class BasicOAuthScopes implements OAuthScopes { - static from(scopes: OAuthScopeLike, normalizer?: (scope: string) => string) { - const normalized = BasicOAuthScopes.asStrings(scopes, normalizer); - return new BasicOAuthScopes(new Set(normalized), normalizer); - } - - constructor( - private readonly scopes: Set, - private readonly normalizer?: (scope: string) => string, - ) {} - - extend(requestedScopes: OAuthScopeLike): BasicOAuthScopes { - const newScopes = new Set(this.scopes); - BasicOAuthScopes.asStrings(requestedScopes, this.normalizer).forEach((s) => - newScopes.add(s), - ); - return new BasicOAuthScopes(newScopes, this.normalizer); - } - - hasScopes(scopes: OAuthScopeLike): boolean { - return BasicOAuthScopes.asStrings(scopes, this.normalizer).every((s) => - this.scopes.has(s), - ); - } - - toSet(): Set { - return this.scopes; - } - - toString(): string { - return Array.from(this.scopes).join(' '); - } - - toJSON() { - return Array.from(this.scopes); - } - - static asStrings( - input: OAuthScopeLike, - normalizer?: (scope: string) => string, - ): string[] { - let scopeArray: string[]; - if (typeof input === 'string') { - scopeArray = input.split(/[,\s]/).filter(Boolean); - } else if (Array.isArray(input)) { - scopeArray = input; - } else { - scopeArray = Array.from(input.toSet()); - } - if (normalizer) { - scopeArray = scopeArray.map((x) => normalizer(x)); - } - return scopeArray; - } -} diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts index 61e466f200..32170acc6f 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts @@ -16,7 +16,6 @@ import MockOAuthApi from './MockOAuthApi'; import PowerIcon from '@material-ui/icons/Power'; -import { BasicOAuthScopes } from './BasicOAuthScopes'; describe('MockOAuthApi', () => { it('should trigger all requests', async () => { @@ -36,11 +35,11 @@ describe('MockOAuthApi', () => { }); const promises = [ - requester1(BasicOAuthScopes.from('a')), - requester1(BasicOAuthScopes.from('b')), - requester2(BasicOAuthScopes.from('a b')), - requester2(BasicOAuthScopes.from('b c')), - requester2(BasicOAuthScopes.from('c a')), + requester1(new Set(['a'])), + requester1(new Set(['b'])), + requester2(new Set(['a', 'b'])), + requester2(new Set(['b', 'c'])), + requester2(new Set(['c', 'a'])), ]; await expect( @@ -58,9 +57,9 @@ describe('MockOAuthApi', () => { ]); expect(authHandler1).toHaveBeenCalledTimes(1); - expect(authHandler1).toHaveBeenCalledWith(BasicOAuthScopes.from('a b')); + expect(authHandler1).toHaveBeenCalledWith(new Set(['a', 'b'])); expect(authHandler2).toHaveBeenCalledTimes(1); - expect(authHandler2).toHaveBeenCalledWith(BasicOAuthScopes.from('a b c')); + expect(authHandler2).toHaveBeenCalledWith(new Set(['a', 'b', 'c'])); }); it('should reject all requests', async () => { @@ -79,11 +78,11 @@ describe('MockOAuthApi', () => { }); const promises = [ - requester1(BasicOAuthScopes.from('a')), - requester1(BasicOAuthScopes.from('b')), - requester2(BasicOAuthScopes.from('a b')), - requester2(BasicOAuthScopes.from('b c')), - requester2(BasicOAuthScopes.from('c a')), + requester1(new Set(['a'])), + requester1(new Set(['b'])), + requester2(new Set(['a', 'b'])), + requester2(new Set(['b', 'c'])), + requester2(new Set(['c', 'a'])), ]; await expect( diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.test.ts index be762ca936..36378798d4 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.test.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.test.ts @@ -16,7 +16,6 @@ import { wait } from '@testing-library/react'; import { OAuthPendingRequests } from './OAuthPendingRequests'; -import { BasicOAuthScopes } from './BasicOAuthScopes'; describe('OAuthPendingRequests', () => { it('notifies new observers about current state', async () => { @@ -24,7 +23,7 @@ describe('OAuthPendingRequests', () => { const next = jest.fn(); const error = jest.fn(); - const input = BasicOAuthScopes.from('a b'); + const input = new Set(['a', 'b']); target.pending().subscribe({ next, error }); target.request(input); @@ -39,11 +38,11 @@ describe('OAuthPendingRequests', () => { const next = jest.fn(); const error = jest.fn(); - const request1 = target.request(BasicOAuthScopes.from('a')); - const request2 = target.request(BasicOAuthScopes.from('a')); + const request1 = target.request(new Set(['a'])); + const request2 = target.request(new Set(['a'])); target.pending().subscribe({ next, error }); - target.resolve(BasicOAuthScopes.from('a'), 'session1'); - target.resolve(BasicOAuthScopes.from('a'), 'session2'); + target.resolve(new Set(['a']), 'session1'); + target.resolve(new Set(['a']), 'session2'); await expect(request1).resolves.toBe('session1'); await expect(request2).resolves.toBe('session1'); @@ -53,10 +52,10 @@ describe('OAuthPendingRequests', () => { it('can resolve through the observable', async () => { const target = new OAuthPendingRequests(); - const next = jest.fn((pendingRequest) => pendingRequest.resolve('done')); + const next = jest.fn(pendingRequest => pendingRequest.resolve('done')); const error = jest.fn(); - const request1 = target.request(BasicOAuthScopes.from('a')); + const request1 = target.request(new Set(['a'])); target.pending().subscribe({ next, error }); await expect(request1).resolves.toBe('done'); @@ -70,11 +69,11 @@ describe('OAuthPendingRequests', () => { const error = jest.fn(); const rejection = new Error('eek'); - const request1 = target.request(BasicOAuthScopes.from('a')); - const request2 = target.request(BasicOAuthScopes.from('a')); + const request1 = target.request(new Set(['a'])); + const request2 = target.request(new Set(['a'])); target.pending().subscribe({ next, error }); target.reject(rejection); - target.resolve(BasicOAuthScopes.from('a'), 'session'); + target.resolve(new Set(['a']), 'session'); await expect(request1).rejects.toBe(rejection); await expect(request2).rejects.toBe(rejection); @@ -85,10 +84,10 @@ describe('OAuthPendingRequests', () => { it('can reject through the observable', async () => { const target = new OAuthPendingRequests(); const rejection = new Error('nope'); - const next = jest.fn((pendingRequest) => pendingRequest.reject(rejection)); + const next = jest.fn(pendingRequest => pendingRequest.reject(rejection)); const error = jest.fn(); - const request1 = target.request(BasicOAuthScopes.from('a')); + const request1 = target.request(new Set(['a'])); target.pending().subscribe({ next, error }); await expect(request1).rejects.toBe(rejection); diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts index e8dd0e12ef..6ee1674b39 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts @@ -14,22 +14,50 @@ * limitations under the License. */ -import { OAuthScopes } from '../../definitions'; import { BehaviorSubject } from '../lib'; import { Observable } from '../../../types'; type RequestQueueEntry = { - scopes: OAuthScopes; + scopes: Set; resolve: (value?: ResultType | PromiseLike | undefined) => void; reject: (reason: Error) => void; }; export type PendingRequest = { - scopes: OAuthScopes | undefined; + scopes: Set | undefined; resolve: (value: ResultType) => void; reject: (reason: Error) => void; }; +export function hasScopes( + searched: Set, + searchFor: Set, +): boolean { + for (const scope of searchFor) { + if (!searched.has(scope)) { + return false; + } + } + return true; +} + +export function joinScopes( + scopes: Set, + ...moreScopess: Set[] +): Set { + const result = new Set(scopes); + + for (const moreScopes of moreScopess) { + for (const scope of moreScopes) { + if (!result.has(scope)) { + result.add(scope); + } + } + } + + return result; +} + /** * The OAuthPendingRequests class is a utility for managing and observing * a stream of requests for oauth scopes for a single provider, and resolving @@ -41,7 +69,7 @@ export class OAuthPendingRequests { this.getCurrentPending(), ); - request(scopes: OAuthScopes): Promise { + request(scopes: Set): Promise { return new Promise((resolve, reject) => { this.requests.push({ scopes, resolve, reject }); @@ -49,9 +77,9 @@ export class OAuthPendingRequests { }); } - resolve(scopes: OAuthScopes, result: ResultType): void { - this.requests = this.requests.filter((request) => { - if (scopes.hasScopes(request.scopes)) { + resolve(scopes: Set, result: ResultType): void { + this.requests = this.requests.filter(request => { + if (hasScopes(scopes, request.scopes)) { request.resolve(result); return false; } @@ -62,7 +90,7 @@ export class OAuthPendingRequests { } reject(error: Error) { - this.requests.forEach((request) => request.reject(error)); + this.requests.forEach(request => request.reject(error)); this.requests = []; this.subject.next(this.getCurrentPending()); @@ -79,7 +107,7 @@ export class OAuthPendingRequests { : this.requests .slice(1) .reduce( - (acc, current) => acc.extend(current.scopes), + (acc, current) => joinScopes(acc, current.scopes), this.requests[0].scopes, ); diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts index 922cb20a25..46a5362f35 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts @@ -16,7 +16,6 @@ import ProviderIcon from '@material-ui/icons/AcUnit'; import { OAuthRequestManager } from './OAuthRequestManager'; -import { BasicOAuthScopes } from './BasicOAuthScopes'; describe('OAuthRequestManager', () => { it('should forward a requests', async () => { @@ -38,7 +37,7 @@ describe('OAuthRequestManager', () => { expect(reqSpy).toHaveBeenCalledTimes(2); expect(reqSpy).toHaveBeenLastCalledWith([]); - const req = requester(BasicOAuthScopes.from('my-scope')); + const req = requester(new Set(['my-scope'])); expect(reqSpy).toHaveBeenCalledTimes(3); expect(reqSpy).toHaveBeenLastCalledWith([ diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts index 8b3b2d94c2..1168d6abd4 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts @@ -43,7 +43,7 @@ export class OAuthRequestManager implements OAuthRequestApi { this.handlerCount++; handler.pending().subscribe({ - next: (scopeRequest) => { + next: scopeRequest => { const newRequests = this.currentRequests.slice(); const request = this.makeAuthRequest(scopeRequest, options); if (!request) { @@ -57,7 +57,7 @@ export class OAuthRequestManager implements OAuthRequestApi { }, }); - return (scopes) => { + return scopes => { return handler.request(scopes); }; } diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts index a6d2b082b2..b386806636 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts @@ -15,11 +15,12 @@ */ import GoogleAuth from './GoogleAuth'; -import GoogleScopes from './GoogleScopes'; const theFuture = new Date(Date.now() + 3600000); const thePast = new Date(Date.now() - 10); +const PREFIX = 'https://www.googleapis.com/auth/'; + describe('GoogleAuth', () => { it('should save result form createSession', async () => { const createSession = jest.fn().mockResolvedValue({ expiresAt: theFuture }); @@ -41,7 +42,7 @@ describe('GoogleAuth', () => { const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); createSession.mockResolvedValue({ - scopes: GoogleScopes.from('a'), + scopes: new Set([`${PREFIX}a`]), expiresAt: theFuture, }); await googleAuth.getSession({ scope: 'a' }); @@ -59,11 +60,11 @@ describe('GoogleAuth', () => { const refreshSession = jest .fn() .mockRejectedValueOnce(new Error('NOPE')) - .mockResolvedValue({ scopes: GoogleScopes.from('a') }); + .mockResolvedValue({ scopes: new Set([`${PREFIX}a`]) }); const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); createSession.mockResolvedValue({ - scopes: GoogleScopes.from('a'), + scopes: new Set([`${PREFIX}a`]), expiresAt: thePast, }); @@ -150,7 +151,7 @@ describe('GoogleAuth', () => { const refreshSession = jest.fn().mockResolvedValue({ accessToken: 'access-token', expiresAt: theFuture, - scopes: GoogleScopes.from('not-enough'), + scopes: new Set([`${PREFIX}not-enough`]), }); const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); @@ -169,7 +170,7 @@ describe('GoogleAuth', () => { const initialSession = { idToken: 'token1', expiresAt: theFuture, - scopes: GoogleScopes.empty(), + scopes: new Set(), }; const refreshSession = jest .fn() @@ -177,7 +178,7 @@ describe('GoogleAuth', () => { .mockResolvedValue({ idToken: 'token2', expiresAt: theFuture, - scopes: GoogleScopes.empty(), + scopes: new Set(), }); const googleAuth = new GoogleAuth({ refreshSession } as any); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index efa5ef7079..1ae94ae9c6 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -16,9 +16,7 @@ import GoogleIcon from '@material-ui/icons/AcUnit'; import { AuthHelper } from '../../lib/AuthHelper'; -import GoogleScopes from './GoogleScopes'; import { GoogleSession } from './types'; -import { OAuthScopes } from '../../..'; import { OAuthApi, OpenIdConnectApi, @@ -26,6 +24,7 @@ import { } from '../../../definitions/auth'; import { OAuthRequestApi } from '../../../definitions'; import { GenericAuthHelper } from '../../lib/AuthHelper/AuthHelper'; +import { hasScopes } from '../../OAuthRequestManager/OAuthPendingRequests'; export type GoogleAuthResponse = { accessToken: string; @@ -34,6 +33,13 @@ export type GoogleAuthResponse = { expiresInSeconds: number; }; +const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; +const DEFAULT_SCOPES = [ + 'openid', + `${SCOPE_PREFIX}userinfo.email`, + `${SCOPE_PREFIX}userinfo.profile`, +]; + class GoogleAuth implements OAuthApi, OpenIdConnectApi { private currentSession: GoogleSession | undefined; @@ -50,7 +56,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { return { idToken: res.idToken, accessToken: res.accessToken, - scopes: GoogleScopes.from(res.scopes), + scopes: GoogleAuth.normalizeScopes(res.scopes), expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), }; }, @@ -86,14 +92,16 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { optional?: boolean; scope?: string | string[]; }): Promise { - if (this.sessionExistsAndHasScope(this.currentSession, options.scope)) { + const normalizedScope = GoogleAuth.normalizeScopes(options.scope); + + if (this.sessionExistsAndHasScope(this.currentSession, normalizedScope)) { if (!this.sessionWillExpire(this.currentSession!)) { return this.currentSession!; } try { const refreshedSession = await this.helper.refreshSession(); - if (refreshedSession.scopes.hasScopes(this.currentSession!.scopes)) { + if (hasScopes(refreshedSession.scopes, this.currentSession!.scopes)) { this.currentSession = refreshedSession; } return refreshedSession; @@ -125,7 +133,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { // We can call authRequester multiple times, the returned session will contain all requested scopes. this.currentSession = await this.helper.createSession( - this.getExtendedScope(options.scope), + this.getExtendedScope(normalizedScope), ); return this.currentSession; } @@ -137,7 +145,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { private sessionExistsAndHasScope( session: GoogleSession | undefined, - scope?: string | string[], + scope?: Set, ): boolean { if (!session) { return false; @@ -145,7 +153,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { if (!scope) { return true; } - return session.scopes.hasScopes(scope); + return hasScopes(session.scopes, scope); } private sessionWillExpire(session: GoogleSession) { @@ -153,15 +161,45 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { return expiresInSec < 60 * 5; } - private getExtendedScope(scope?: string | string[]) { - let newScope: OAuthScopes = GoogleScopes.default(); + private getExtendedScope(scopes: Set) { + const newScope = new Set(DEFAULT_SCOPES); if (this.currentSession) { - newScope = this.currentSession.scopes; + for (const scope of this.currentSession.scopes) { + newScope.add(scope); + } } - if (scope) { - newScope = newScope.extend(scope); + for (const scope of scopes) { + newScope.add(scope); } return newScope; } + + private static normalizeScopes(scopes?: string | string[]): Set { + if (!scopes) { + return new Set(); + } + + const scopeList = Array.isArray(scopes) + ? scopes + : scopes.split(' ').filter(Boolean); + + const normalizedScopes = scopeList.map(scope => { + if (scope === 'openid') { + return scope; + } + + if (scope === 'profile' || scope === 'email') { + return `${SCOPE_PREFIX}userinfo.${scope}`; + } + + if (scope.startsWith(SCOPE_PREFIX)) { + return scope; + } + + return `${SCOPE_PREFIX}${scope}`; + }); + + return new Set(normalizedScopes); + } } export default GoogleAuth; diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts deleted file mode 100644 index b4eec497bb..0000000000 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.test.ts +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 GoogleScopes from './GoogleScopes'; - -const PREFIX = 'https://www.googleapis.com/auth/'; - -describe('GoogleScopes', () => { - it('should be created from scopes', () => { - const scopes = GoogleScopes.from('a openid b profile'); - expect(scopes.toString()).toBe( - `${PREFIX}a openid ${PREFIX}b ${PREFIX}userinfo.profile`, - ); - }); - - it('should be created with default scopes', () => { - expect(GoogleScopes.default().toString()).toBe( - `openid ${PREFIX}userinfo.email ${PREFIX}userinfo.profile`, - ); - }); - - it('should have or not have scopes', () => { - const scopes = GoogleScopes.from(`a b ${PREFIX}c`); - expect(scopes.hasScopes('a')).toBe(true); - expect(scopes.hasScopes('a b')).toBe(true); - expect(scopes.hasScopes('b')).toBe(true); - expect(scopes.hasScopes('b c')).toBe(true); - expect(scopes.hasScopes('a b c')).toBe(true); - expect(scopes.hasScopes(`a b ${PREFIX}c`)).toBe(true); - expect(scopes.hasScopes(`a ${PREFIX}b c`)).toBe(true); - expect(scopes.hasScopes('a b c d')).toBe(false); - expect(scopes.hasScopes('d')).toBe(false); - expect(scopes.hasScopes('')).toBe(true); - expect(scopes.hasScopes('abc')).toBe(false); - expect(scopes.hasScopes(`${PREFIX}a`)).toBe(true); - }); - - it('should handle scope shorthands correctly', () => { - const scopes = GoogleScopes.default(); - expect(scopes.hasScopes('email')).toBe(true); - expect(scopes.hasScopes('profile')).toBe(true); - expect(scopes.hasScopes('openid')).toBe(true); - expect(scopes.hasScopes('userinfo.email')).toBe(true); - expect(scopes.hasScopes('userinfo.profile')).toBe(true); - expect(scopes.hasScopes('userinfo.openid')).toBe(false); - expect(scopes.hasScopes(`${PREFIX}userinfo.email`)).toBe(true); - expect(scopes.hasScopes(`${PREFIX}userinfo.profile`)).toBe(true); - expect(scopes.hasScopes(`${PREFIX}userinfo.openid`)).toBe(false); - expect(scopes.hasScopes(`${PREFIX}email`)).toBe(false); - expect(scopes.hasScopes(`${PREFIX}profile`)).toBe(false); - expect(scopes.hasScopes(`${PREFIX}openid`)).toBe(false); - }); - - it('should be extended', () => { - const scopes = GoogleScopes.from('a b'); - expect(scopes.extend('')).not.toBe(scopes); - expect(scopes.extend('d').toString()).toBe( - `${PREFIX}a ${PREFIX}b ${PREFIX}d`, - ); - expect(scopes.extend('profile').toString()).toBe( - `${PREFIX}a ${PREFIX}b ${PREFIX}userinfo.profile`, - ); - expect(scopes.extend('d profile').toString()).toBe( - `${PREFIX}a ${PREFIX}b ${PREFIX}d ${PREFIX}userinfo.profile`, - ); - expect(scopes.extend(`${PREFIX}d profile`).toString()).toBe( - `${PREFIX}a ${PREFIX}b ${PREFIX}d ${PREFIX}userinfo.profile`, - ); - expect(scopes.extend('a').toString()).toBe(scopes.toString()); - expect(scopes.extend('').toString()).toBe(scopes.toString()); - expect(scopes.extend('b').toString()).toBe(scopes.toString()); - expect(scopes.extend('b a').toString()).toBe(scopes.toString()); - expect(scopes.extend('b a b a a').toString()).toBe(scopes.toString()); - }); -}); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts deleted file mode 100644 index a29d392488..0000000000 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleScopes.ts +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 { BasicOAuthScopes } from '../../OAuthRequestManager/BasicOAuthScopes'; -import { OAuthScopeLike } from '../../..'; - -const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; - -export default class GoogleScopes extends BasicOAuthScopes { - static from(scope: OAuthScopeLike): GoogleScopes { - return new GoogleScopes( - new Set(BasicOAuthScopes.asStrings(scope, GoogleScopes.canonicalScope)), - ); - } - - static default(): GoogleScopes { - return new GoogleScopes( - new Set([ - 'openid', - `${SCOPE_PREFIX}userinfo.email`, - `${SCOPE_PREFIX}userinfo.profile`, - ]), - ); - } - - static empty(): GoogleScopes { - return new GoogleScopes(new Set()); - } - - constructor(scopes: Set) { - super(scopes, GoogleScopes.canonicalScope); - } - - private static canonicalScope(scope: string): string { - if (scope === 'openid') { - return scope; - } - - if (scope === 'profile' || scope === 'email') { - return `${SCOPE_PREFIX}userinfo.${scope}`; - } - - if (scope.startsWith(SCOPE_PREFIX)) { - return scope; - } - - return `${SCOPE_PREFIX}${scope}`; - } -} diff --git a/packages/core/src/api/apis/implementations/auth/google/types.ts b/packages/core/src/api/apis/implementations/auth/google/types.ts index 55d9a7ecf2..96c69c5d4f 100644 --- a/packages/core/src/api/apis/implementations/auth/google/types.ts +++ b/packages/core/src/api/apis/implementations/auth/google/types.ts @@ -14,11 +14,9 @@ * limitations under the License. */ -import GoogleScopes from './GoogleScopes'; - export type GoogleSession = { idToken: string; accessToken: string; - scopes: GoogleScopes; + scopes: Set; expiresAt: Date; }; diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts index 5ca95cb600..d62226679d 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts @@ -17,7 +17,6 @@ import ProviderIcon from '@material-ui/icons/AcUnit'; import { AuthHelper } from './AuthHelper'; import MockOAuthApi from '../../OAuthRequestManager/MockOAuthApi'; -import { BasicOAuthScopes } from '../../OAuthRequestManager/BasicOAuthScopes'; import * as loginPopup from '../loginPopup'; const anyFetch = fetch as any; @@ -33,7 +32,7 @@ const defaultOptions = { oauthRequestApi: new MockOAuthApi(), sessionTransform: ({ expiresInSeconds, ...res }: any) => ({ ...res, - scopes: BasicOAuthScopes.from(res.scopes), + scopes: new Set(res.scopes.split(' ')), expiresAt: new Date(Date.now() + expiresInSeconds * 1000), }), }; @@ -58,7 +57,7 @@ describe('AuthHelper', () => { const session = await helper.refreshSession(); expect(session.idToken).toBe('mock-id-token'); expect(session.accessToken).toBe('mock-access-token'); - expect(session.scopes.hasScopes('a b c')).toBe(true); + expect(session.scopes).toEqual(new Set(['a', 'b', 'c'])); expect(session.expiresAt.getTime()).toBeLessThan(Date.now() + 70000); expect(session.expiresAt.getTime()).toBeGreaterThan(Date.now() + 50000); }); @@ -87,7 +86,7 @@ describe('AuthHelper', () => { ...defaultOptions, oauthRequestApi: mockOauth, }); - const promise = helper.createSession(BasicOAuthScopes.from('a b')); + const promise = helper.createSession(new Set(['a', 'b'])); await mockOauth.rejectAll(); await expect(promise).rejects.toMatchObject({ name: 'RejectedError' }); }); @@ -107,7 +106,7 @@ describe('AuthHelper', () => { oauthRequestApi: mockOauth, }); - const sessionPromise = helper.createSession(BasicOAuthScopes.from('a b')); + const sessionPromise = helper.createSession(new Set(['a', 'b'])); await mockOauth.triggerAll(); @@ -119,7 +118,7 @@ describe('AuthHelper', () => { await expect(sessionPromise).resolves.toEqual({ idToken: 'my-id-token', accessToken: 'my-access-token', - scopes: expect.any(BasicOAuthScopes), + scopes: expect.any(Set), expiresAt: expect.any(Date), }); }); diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts index dc79641e70..e1d9d12b31 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts @@ -15,11 +15,7 @@ */ import { AuthRequester } from '../../..'; -import { - OAuthRequestApi, - AuthProvider, - OAuthScopes, -} from '../../../definitions'; +import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { showLoginPopup } from '../loginPopup'; const DEFAULT_BASE_PATH = '/api/auth/'; @@ -37,7 +33,7 @@ type Options = { export type GenericAuthHelper = { refreshSession(): Promise; removeSession(): Promise; - createSession(scope: OAuthScopes): Promise; + createSession(scopes: Set): Promise; }; export class AuthHelper implements AuthHelper { @@ -59,12 +55,12 @@ export class AuthHelper implements AuthHelper { environment, provider, oauthRequestApi, - sessionTransform = (id) => id, + sessionTransform = id => id, } = options; this.authRequester = oauthRequestApi.createAuthRequester({ provider, - onAuthRequest: (scopes) => this.showPopup(scopes.toString()), + onAuthRequest: scopes => this.showPopup([...scopes].join(' ')), }); this.apiOrigin = apiOrigin; @@ -95,7 +91,7 @@ export class AuthHelper implements AuthHelper { 'x-requested-with': 'XMLHttpRequest', }, credentials: 'include', - }).catch((error) => { + }).catch(error => { throw new Error(`Auth refresh request failed, ${error}`); }); @@ -133,8 +129,8 @@ export class AuthHelper implements AuthHelper { } } - async createSession(scope: OAuthScopes): Promise { - return this.authRequester(scope); + async createSession(scopes: Set): Promise { + return this.authRequester(scopes); } private async showPopup(scope: string): Promise { From e6537acc9c53e2e1d9eb8a380449646782f33ab2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 12:03:45 +0200 Subject: [PATCH 08/18] packages/core: move google auth session refresh logic out into separate session manager helper --- .../auth/google/GoogleAuth.test.ts | 137 +++------------- .../implementations/auth/google/GoogleAuth.ts | 126 +++------------ .../lib/AuthHelper/AuthHelper.ts | 24 +-- .../lib/AuthHelper/MockAuthHelper.ts | 2 +- .../implementations/lib/AuthHelper/index.ts | 1 + .../implementations/lib/AuthHelper/types.ts | 26 +++ .../RefreshingAuthSessionManager.test.ts | 128 +++++++++++++++ .../RefreshingAuthSessionManager.ts | 152 ++++++++++++++++++ .../lib/AuthSessionManager/index.ts | 18 +++ .../lib/AuthSessionManager/types.ts | 35 ++++ 10 files changed, 407 insertions(+), 242 deletions(-) create mode 100644 packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts create mode 100644 packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts create mode 100644 packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts create mode 100644 packages/core/src/api/apis/implementations/lib/AuthSessionManager/index.ts create mode 100644 packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts index b386806636..fde1256ea8 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts @@ -22,138 +22,48 @@ const thePast = new Date(Date.now() - 10); const PREFIX = 'https://www.googleapis.com/auth/'; describe('GoogleAuth', () => { - it('should save result form createSession', async () => { - const createSession = jest.fn().mockResolvedValue({ expiresAt: theFuture }); - const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); - const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); - - await googleAuth.getSession({}); - expect(createSession).toBeCalledTimes(1); - - await googleAuth.getSession({}); - expect(createSession).toBeCalledTimes(1); - - expect(refreshSession).toBeCalledTimes(1); - }); - - it('should ask consent only if scopes have changed', async () => { - const createSession = jest.fn(); - const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); - const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); - - createSession.mockResolvedValue({ - scopes: new Set([`${PREFIX}a`]), - expiresAt: theFuture, - }); - await googleAuth.getSession({ scope: 'a' }); - expect(createSession).toBeCalledTimes(1); - - await googleAuth.getSession({ scope: 'a' }); - expect(createSession).toBeCalledTimes(1); - - await googleAuth.getSession({ scope: 'b' }); - expect(createSession).toBeCalledTimes(2); - }); - - it('should check for session expiry', async () => { - const createSession = jest.fn(); - const refreshSession = jest - .fn() - .mockRejectedValueOnce(new Error('NOPE')) - .mockResolvedValue({ scopes: new Set([`${PREFIX}a`]) }); - const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); - - createSession.mockResolvedValue({ - scopes: new Set([`${PREFIX}a`]), - expiresAt: thePast, - }); - - await googleAuth.getSession({ scope: 'a' }); - expect(createSession).toBeCalledTimes(1); - expect(refreshSession).toBeCalledTimes(1); - - await googleAuth.getSession({ scope: 'a' }); - expect(createSession).toBeCalledTimes(1); - expect(refreshSession).toBeCalledTimes(2); - }); - - it('should handle user closed popup', async () => { - const createSession = jest.fn(); - const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); - const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); - - createSession.mockRejectedValueOnce(new Error('some error')); - await expect(googleAuth.getSession({ scope: 'a' })).rejects.toThrow( - 'some error', - ); - }); - - it('should logout and reload', async () => { - // This is a workaround that is used by Facebook and the Jest core team - // It is a limitation with the newest versions of JSDOM, and newer browser standards - // where window.location and all of its properties are read-only. So we re-construct it! - // See https://github.com/facebook/jest/issues/890#issuecomment-209698782 - const location = { ...window.location }; - delete window.location; - window.location = location; - jest.spyOn(window.location, 'reload').mockImplementation(); - - const removeSession = jest.fn(); - const googleAuth = new GoogleAuth({ removeSession } as any); - - await googleAuth.logout(); - expect(window.location.reload).toHaveBeenCalled(); - expect(removeSession).toHaveBeenCalled(); - }); - it('should get refreshed access token', async () => { - const refreshSession = jest + const getSession = jest .fn() .mockResolvedValue({ accessToken: 'access-token', expiresAt: theFuture }); - const googleAuth = new GoogleAuth({ refreshSession } as any); + const googleAuth = new GoogleAuth({ getSession } as any); expect(await googleAuth.getAccessToken()).toBe('access-token'); - expect(refreshSession).toBeCalledTimes(1); + expect(getSession).toBeCalledTimes(1); }); it('should get refreshed id token', async () => { - const refreshSession = jest + const getSession = jest .fn() .mockResolvedValue({ idToken: 'id-token', expiresAt: theFuture }); - const googleAuth = new GoogleAuth({ refreshSession } as any); + const googleAuth = new GoogleAuth({ getSession } as any); expect(await googleAuth.getIdToken()).toBe('id-token'); - expect(refreshSession).toBeCalledTimes(1); + expect(getSession).toBeCalledTimes(1); }); it('should get optional id token', async () => { - const refreshSession = jest + const getSession = jest .fn() .mockResolvedValue({ idToken: 'id-token', expiresAt: theFuture }); - const googleAuth = new GoogleAuth({ refreshSession } as any); + const googleAuth = new GoogleAuth({ getSession } as any); expect(await googleAuth.getIdToken({ optional: true })).toBe('id-token'); - expect(refreshSession).toBeCalledTimes(1); - }); - - it('should not get optional id token', async () => { - const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); - const googleAuth = new GoogleAuth({ refreshSession } as any); - - expect(await googleAuth.getIdToken({ optional: true })).toBe(''); - expect(refreshSession).toBeCalledTimes(1); + expect(getSession).toBeCalledTimes(1); }); it('should share popup closed errors', async () => { const error = new Error('NOPE'); error.name = 'RejectedError'; - const createSession = jest.fn().mockRejectedValue(error); - const refreshSession = jest.fn().mockResolvedValue({ - accessToken: 'access-token', - expiresAt: theFuture, - scopes: new Set([`${PREFIX}not-enough`]), - }); - const googleAuth = new GoogleAuth({ createSession, refreshSession } as any); + const getSession = jest + .fn() + .mockResolvedValueOnce({ + accessToken: 'access-token', + expiresAt: theFuture, + scopes: new Set([`${PREFIX}not-enough`]), + }) + .mockRejectedValue(error); + const googleAuth = new GoogleAuth({ getSession } as any); // Make sure we have a session before we do the double request, so that we get past the !this.currentSession check await expect(googleAuth.getAccessToken()).resolves.toBe('access-token'); @@ -162,8 +72,7 @@ describe('GoogleAuth', () => { const promise2 = googleAuth.getAccessToken('more'); await expect(promise1).rejects.toBe(error); await expect(promise2).rejects.toBe(error); - expect(refreshSession).toBeCalledTimes(1); - expect(createSession).toBeCalledTimes(2); + expect(getSession).toBeCalledTimes(3); }); it('should wait for all session refreshes', async () => { @@ -172,7 +81,7 @@ describe('GoogleAuth', () => { expiresAt: theFuture, scopes: new Set(), }; - const refreshSession = jest + const getSession = jest .fn() .mockResolvedValueOnce(initialSession) .mockResolvedValue({ @@ -180,11 +89,11 @@ describe('GoogleAuth', () => { expiresAt: theFuture, scopes: new Set(), }); - const googleAuth = new GoogleAuth({ refreshSession } as any); + const googleAuth = new GoogleAuth({ getSession } as any); // Grab the expired session first await expect(googleAuth.getIdToken()).resolves.toBe('token1'); - expect(refreshSession).toBeCalledTimes(1); + expect(getSession).toBeCalledTimes(1); initialSession.expiresAt = thePast; @@ -194,6 +103,6 @@ describe('GoogleAuth', () => { await expect(promise1).resolves.toBe('token2'); await expect(promise2).resolves.toBe('token2'); await expect(promise3).resolves.toBe('token2'); - expect(refreshSession).toBeCalledTimes(4); // De-duping of session requests happens in client + expect(getSession).toBeCalledTimes(4); // De-duping of session requests happens in client }); }); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index 1ae94ae9c6..0b32db2023 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -23,8 +23,8 @@ import { IdTokenOptions, } from '../../../definitions/auth'; import { OAuthRequestApi } from '../../../definitions'; -import { GenericAuthHelper } from '../../lib/AuthHelper/AuthHelper'; -import { hasScopes } from '../../OAuthRequestManager/OAuthPendingRequests'; +import { SessionManager } from '../../lib/AuthSessionManager/types'; +import { RefreshingAuthSessionManager } from '../../lib/AuthSessionManager'; export type GoogleAuthResponse = { accessToken: string; @@ -34,15 +34,8 @@ export type GoogleAuthResponse = { }; const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; -const DEFAULT_SCOPES = [ - 'openid', - `${SCOPE_PREFIX}userinfo.email`, - `${SCOPE_PREFIX}userinfo.profile`, -]; class GoogleAuth implements OAuthApi, OpenIdConnectApi { - private currentSession: GoogleSession | undefined; - static create(oauthRequestApi: OAuthRequestApi) { const helper = new AuthHelper({ providerPath: 'google/', @@ -62,116 +55,41 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { }, }); - return new GoogleAuth(helper); + const sessionManager = new RefreshingAuthSessionManager({ + helper, + defaultScopes: new Set([ + 'openid', + `${SCOPE_PREFIX}userinfo.email`, + `${SCOPE_PREFIX}userinfo.profile`, + ]), + }); + + return new GoogleAuth(sessionManager); } - constructor(private readonly helper: GenericAuthHelper) {} + constructor(private readonly sessionManager: SessionManager) {} async getAccessToken(scope?: string | string[]) { - const session = await this.getSession({ optional: false, scope }); + const normalizedScopes = GoogleAuth.normalizeScopes(scope); + const session = await this.sessionManager.getSession({ + optional: false, + scope: normalizedScopes, + }); return session.accessToken; } async getIdToken({ optional }: IdTokenOptions = {}) { - const session = await this.getSession({ optional: optional || false }); + const session = await this.sessionManager.getSession({ + optional: optional || false, + }); if (session) { return session.idToken; } return ''; } - async getSession(options: { - optional: false; - scope?: string | string[]; - }): Promise; - async getSession(options: { - optional?: boolean; - scope?: string | string[]; - }): Promise; - async getSession(options: { - optional?: boolean; - scope?: string | string[]; - }): Promise { - const normalizedScope = GoogleAuth.normalizeScopes(options.scope); - - if (this.sessionExistsAndHasScope(this.currentSession, normalizedScope)) { - if (!this.sessionWillExpire(this.currentSession!)) { - return this.currentSession!; - } - - try { - const refreshedSession = await this.helper.refreshSession(); - if (hasScopes(refreshedSession.scopes, this.currentSession!.scopes)) { - this.currentSession = refreshedSession; - } - return refreshedSession; - } catch (error) { - if (options.optional) { - return undefined; - } - throw error; - } - } - - // The user may still have a valid refresh token in their cookies. Attempt to - // initiate a fresh session through the backend using that refresh token. - if (!this.currentSession) { - try { - const newSession = await this.helper.refreshSession(); - this.currentSession = newSession; - // The session might not have the scopes requested so go back and check again - return this.getSession(options); - } catch { - // If the refresh attemp fails we assume we don't have a session, so continue to create one. - } - } - - // If we continue here we will show a popup, so exit if this is an optional session request. - if (options.optional) { - return undefined; - } - - // We can call authRequester multiple times, the returned session will contain all requested scopes. - this.currentSession = await this.helper.createSession( - this.getExtendedScope(normalizedScope), - ); - return this.currentSession; - } - async logout() { - await this.helper.removeSession(); - window.location.reload(); - } - - private sessionExistsAndHasScope( - session: GoogleSession | undefined, - scope?: Set, - ): boolean { - if (!session) { - return false; - } - if (!scope) { - return true; - } - return hasScopes(session.scopes, scope); - } - - private sessionWillExpire(session: GoogleSession) { - const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; - return expiresInSec < 60 * 5; - } - - private getExtendedScope(scopes: Set) { - const newScope = new Set(DEFAULT_SCOPES); - if (this.currentSession) { - for (const scope of this.currentSession.scopes) { - newScope.add(scope); - } - } - for (const scope of scopes) { - newScope.add(scope); - } - return newScope; + await this.sessionManager.removeSession(); } private static normalizeScopes(scopes?: string | string[]): Set { diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts index e1d9d12b31..d855c17076 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts @@ -30,12 +30,6 @@ type Options = { sessionTransform?(response: any): AuthSession | Promise; }; -export type GenericAuthHelper = { - refreshSession(): Promise; - removeSession(): Promise; - createSession(scopes: Set): Promise; -}; - export class AuthHelper implements AuthHelper { private readonly apiOrigin: string; private readonly basePath: string; @@ -45,8 +39,6 @@ export class AuthHelper implements AuthHelper { private readonly authRequester: AuthRequester; private readonly sessionTransform: (response: any) => Promise; - private refreshPromise?: Promise; - constructor(options: Options) { const { apiOrigin = window.location.origin, @@ -71,21 +63,7 @@ export class AuthHelper implements AuthHelper { this.sessionTransform = sessionTransform; } - async refreshSession(): Promise { - if (this.refreshPromise) { - return this.refreshPromise; - } - - this.refreshPromise = this.doAuthRefresh(); - - try { - return await this.refreshPromise; - } finally { - delete this.refreshPromise; - } - } - - private async doAuthRefresh(): Promise { + async refreshSession(): Promise { const res = await fetch(this.buildUrl('/token', { optional: true }), { headers: { 'x-requested-with': 'XMLHttpRequest', diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts index c86854bfc2..e793ebd047 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { GenericAuthHelper } from './AuthHelper'; +import { GenericAuthHelper } from './types'; export const mockAccessToken = 'mock-access-token'; diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts index 29066e4a4b..e472ca06de 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts @@ -15,3 +15,4 @@ */ export { AuthHelper } from './AuthHelper'; +export * from './types'; diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts b/packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts new file mode 100644 index 0000000000..b9ccae11c8 --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts @@ -0,0 +1,26 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 type BaseAuthSession = { + scopes: Set; + expiresAt: Date; +}; + +export type GenericAuthHelper = { + refreshSession(): Promise; + removeSession(): Promise; + createSession(scopes: Set): Promise; +}; diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts new file mode 100644 index 0000000000..511f860f75 --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { RefreshingAuthSessionManager } from './RefreshingAuthSessionManager'; + +const theFuture = new Date(Date.now() + 3600000); +const thePast = new Date(Date.now() - 10); + +describe('RefreshingAuthSessionManager', () => { + it('should save result form createSession', async () => { + const createSession = jest.fn().mockResolvedValue({ expiresAt: theFuture }); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const manager = new RefreshingAuthSessionManager({ + helper: { createSession, refreshSession }, + } as any); + + await manager.getSession({}); + expect(createSession).toBeCalledTimes(1); + + await manager.getSession({}); + expect(createSession).toBeCalledTimes(1); + + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should ask consent only if scopes have changed', async () => { + const createSession = jest.fn(); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const manager = new RefreshingAuthSessionManager({ + helper: { createSession, refreshSession }, + } as any); + + createSession.mockResolvedValue({ + scopes: new Set(['a']), + expiresAt: theFuture, + }); + await manager.getSession({ scope: new Set(['a']) }); + expect(createSession).toBeCalledTimes(1); + + await manager.getSession({ scope: new Set(['a']) }); + expect(createSession).toBeCalledTimes(1); + + await manager.getSession({ scope: new Set(['b']) }); + expect(createSession).toBeCalledTimes(2); + }); + + it('should check for session expiry', async () => { + const createSession = jest.fn(); + const refreshSession = jest + .fn() + .mockRejectedValueOnce(new Error('NOPE')) + .mockResolvedValue({ scopes: new Set(['a']) }); + const manager = new RefreshingAuthSessionManager({ + helper: { createSession, refreshSession }, + } as any); + + createSession.mockResolvedValue({ + scopes: new Set(['a']), + expiresAt: thePast, + }); + + await manager.getSession({ scope: new Set(['a']) }); + expect(createSession).toBeCalledTimes(1); + expect(refreshSession).toBeCalledTimes(1); + + await manager.getSession({ scope: new Set(['a']) }); + expect(createSession).toBeCalledTimes(1); + expect(refreshSession).toBeCalledTimes(2); + }); + + it('should handle user closed popup', async () => { + const createSession = jest.fn(); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const manager = new RefreshingAuthSessionManager({ + helper: { createSession, refreshSession }, + } as any); + + createSession.mockRejectedValueOnce(new Error('some error')); + await expect(manager.getSession({ scope: new Set(['a']) })).rejects.toThrow( + 'some error', + ); + }); + + it('should not get optional session', async () => { + const createSession = jest.fn(); + const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const manager = new RefreshingAuthSessionManager({ + helper: { createSession, refreshSession }, + } as any); + + expect(await manager.getSession({ optional: true })).toBe(undefined); + expect(createSession).toBeCalledTimes(0); + expect(refreshSession).toBeCalledTimes(1); + }); + + it('should remove session and reload', async () => { + // This is a workaround that is used by Facebook and the Jest core team + // It is a limitation with the newest versions of JSDOM, and newer browser standards + // where window.location and all of its properties are read-only. So we re-construct it! + // See https://github.com/facebook/jest/issues/890#issuecomment-209698782 + const location = { ...window.location }; + delete window.location; + window.location = location; + jest.spyOn(window.location, 'reload').mockImplementation(); + + const removeSession = jest.fn(); + const manager = new RefreshingAuthSessionManager({ + helper: { removeSession }, + } as any); + + await manager.removeSession(); + expect(window.location.reload).toHaveBeenCalled(); + expect(removeSession).toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts new file mode 100644 index 0000000000..5670a0f7df --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -0,0 +1,152 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { hasScopes } from '../../OAuthRequestManager/OAuthPendingRequests'; +import { SessionManager } from './types'; +import { BaseAuthSession, GenericAuthHelper } from '../AuthHelper'; + +type Options = { + helper: GenericAuthHelper; + defaultScopes?: Set; +}; + +/** + * RefreshingAuthSessionManager manages an underlying session that has + * and expiration time and needs to be refreshed periodically. + */ +export class RefreshingAuthSessionManager + implements SessionManager { + private readonly helper: GenericAuthHelper; + private readonly defaultScopes?: Set; + + private refreshPromise?: Promise; + private currentSession: AuthSession | undefined; + + constructor(options: Options) { + const { helper, defaultScopes = new Set() } = options; + + this.helper = helper; + this.defaultScopes = defaultScopes; + } + + async getSession(options: { + optional: false; + scope?: Set; + }): Promise; + async getSession(options: { + optional?: boolean; + scope?: Set; + }): Promise; + async getSession(options: { + optional?: boolean; + scope?: Set; + }): Promise { + if (this.sessionExistsAndHasScope(this.currentSession, options.scope)) { + if (!this.sessionWillExpire(this.currentSession!)) { + return this.currentSession!; + } + + try { + const refreshedSession = await this.collapsedSessionRefresh(); + if (hasScopes(refreshedSession.scopes, this.currentSession!.scopes)) { + this.currentSession = refreshedSession; + } + return refreshedSession; + } catch (error) { + if (options.optional) { + return undefined; + } + throw error; + } + } + + // The user may still have a valid refresh token in their cookies. Attempt to + // initiate a fresh session through the backend using that refresh token. + if (!this.currentSession) { + try { + const newSession = await this.collapsedSessionRefresh(); + this.currentSession = newSession; + // The session might not have the scopes requested so go back and check again + return this.getSession(options); + } catch { + // If the refresh attemp fails we assume we don't have a session, so continue to create one. + } + } + + // If we continue here we will show a popup, so exit if this is an optional session request. + if (options.optional) { + return undefined; + } + + // We can call authRequester multiple times, the returned session will contain all requested scopes. + this.currentSession = await this.helper.createSession( + this.getExtendedScope(options.scope), + ); + return this.currentSession; + } + + async removeSession() { + await this.helper.removeSession(); + window.location.reload(); // TODO(Rugvip): make this work without reload? + } + + private sessionExistsAndHasScope( + session: AuthSession | undefined, + scope?: Set, + ): boolean { + if (!session) { + return false; + } + if (!scope) { + return true; + } + return hasScopes(session.scopes, scope); + } + + private sessionWillExpire(session: AuthSession) { + const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; + return expiresInSec < 60 * 5; + } + + private getExtendedScope(scopes?: Set) { + const newScope = new Set(this.defaultScopes); + if (this.currentSession) { + for (const scope of this.currentSession.scopes) { + newScope.add(scope); + } + } + if (scopes) { + for (const scope of scopes) { + newScope.add(scope); + } + } + return newScope; + } + + private async collapsedSessionRefresh(): Promise { + if (this.refreshPromise) { + return this.refreshPromise; + } + + this.refreshPromise = this.helper.refreshSession(); + + try { + return await this.refreshPromise; + } finally { + delete this.refreshPromise; + } + } +} diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/index.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/index.ts new file mode 100644 index 0000000000..426c514646 --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { RefreshingAuthSessionManager } from './RefreshingAuthSessionManager'; +export * from './types'; diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts new file mode 100644 index 0000000000..6475776505 --- /dev/null +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts @@ -0,0 +1,35 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { BaseAuthSession } from '../AuthHelper/types'; + +/** + * A sessions manager keeps track of the current session and makes sure that + * multiple simultaneous requests for sessions with different scope are handled + * in a correct way. + */ +export type SessionManager = { + getSession(options: { + optional: false; + scope?: Set; + }): Promise; + getSession(options: { + optional?: boolean; + scope?: Set; + }): Promise; + + removeSession(): Promise; +}; From eccb7e1cb8b0d25c835beaa2b111c266e57f218d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 12:12:19 +0200 Subject: [PATCH 09/18] packages/core: rename AuthHelper to AuthConnector --- .../implementations/auth/google/GoogleAuth.ts | 6 +++--- .../DefaultAuthConnector.test.ts} | 14 +++++++------- .../DefaultAuthConnector.ts} | 9 ++++++++- .../MockAuthConnector.test.ts} | 6 +++--- .../MockAuthConnector.ts} | 4 ++-- .../lib/{AuthHelper => AuthConnector}/index.ts | 2 +- .../lib/{AuthHelper => AuthConnector}/types.ts | 6 +++++- .../RefreshingAuthSessionManager.test.ts | 12 ++++++------ .../RefreshingAuthSessionManager.ts | 16 ++++++++-------- 9 files changed, 43 insertions(+), 32 deletions(-) rename packages/core/src/api/apis/implementations/lib/{AuthHelper/AuthHelper.test.ts => AuthConnector/DefaultAuthConnector.test.ts} (90%) rename packages/core/src/api/apis/implementations/lib/{AuthHelper/AuthHelper.ts => AuthConnector/DefaultAuthConnector.ts} (93%) rename packages/core/src/api/apis/implementations/lib/{AuthHelper/MockAuthHelper.test.ts => AuthConnector/MockAuthConnector.test.ts} (86%) rename packages/core/src/api/apis/implementations/lib/{AuthHelper/MockAuthHelper.ts => AuthConnector/MockAuthConnector.ts} (89%) rename packages/core/src/api/apis/implementations/lib/{AuthHelper => AuthConnector}/index.ts (90%) rename packages/core/src/api/apis/implementations/lib/{AuthHelper => AuthConnector}/types.ts (80%) diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index 0b32db2023..599ce789db 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -15,7 +15,7 @@ */ import GoogleIcon from '@material-ui/icons/AcUnit'; -import { AuthHelper } from '../../lib/AuthHelper'; +import { DefaultAuthConnector } from '../../lib/AuthConnector'; import { GoogleSession } from './types'; import { OAuthApi, @@ -37,7 +37,7 @@ const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; class GoogleAuth implements OAuthApi, OpenIdConnectApi { static create(oauthRequestApi: OAuthRequestApi) { - const helper = new AuthHelper({ + const connector = new DefaultAuthConnector({ providerPath: 'google/', environment: 'dev', provider: { @@ -56,7 +56,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { }); const sessionManager = new RefreshingAuthSessionManager({ - helper, + connector, defaultScopes: new Set([ 'openid', `${SCOPE_PREFIX}userinfo.email`, diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts similarity index 90% rename from packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts rename to packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts index d62226679d..4ea3bde015 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -15,7 +15,7 @@ */ import ProviderIcon from '@material-ui/icons/AcUnit'; -import { AuthHelper } from './AuthHelper'; +import { DefaultAuthConnector } from './DefaultAuthConnector'; import MockOAuthApi from '../../OAuthRequestManager/MockOAuthApi'; import * as loginPopup from '../loginPopup'; @@ -37,7 +37,7 @@ const defaultOptions = { }), }; -describe('AuthHelper', () => { +describe('DefaultAuthConnector', () => { afterEach(() => { jest.resetAllMocks(); anyFetch.resetMocks(); @@ -53,7 +53,7 @@ describe('AuthHelper', () => { }), ); - const helper = new AuthHelper(defaultOptions); + const helper = new DefaultAuthConnector(defaultOptions); const session = await helper.refreshSession(); expect(session.idToken).toBe('mock-id-token'); expect(session.accessToken).toBe('mock-access-token'); @@ -65,7 +65,7 @@ describe('AuthHelper', () => { it('should handle failure to refresh session', async () => { anyFetch.mockRejectOnce(new Error('Network NOPE')); - const helper = new AuthHelper(defaultOptions); + const helper = new DefaultAuthConnector(defaultOptions); await expect(helper.refreshSession()).rejects.toThrow( 'Auth refresh request failed, Error: Network NOPE', ); @@ -74,7 +74,7 @@ describe('AuthHelper', () => { it('should handle failure response when refreshing session', async () => { anyFetch.mockResponseOnce({}, { status: 401, statusText: 'NOPE' }); - const helper = new AuthHelper(defaultOptions); + const helper = new DefaultAuthConnector(defaultOptions); await expect(helper.refreshSession()).rejects.toThrow( 'Auth refresh request failed with status NOPE', ); @@ -82,7 +82,7 @@ describe('AuthHelper', () => { it('should fail if popup was rejected', async () => { const mockOauth = new MockOAuthApi(); - const helper = new AuthHelper({ + const helper = new DefaultAuthConnector({ ...defaultOptions, oauthRequestApi: mockOauth, }); @@ -101,7 +101,7 @@ describe('AuthHelper', () => { scopes: 'a b', expiresInSeconds: 3600, }); - const helper = new AuthHelper({ + const helper = new DefaultAuthConnector({ ...defaultOptions, oauthRequestApi: mockOauth, }); diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts similarity index 93% rename from packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts rename to packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts index d855c17076..bad8ac948b 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/AuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts @@ -17,6 +17,7 @@ import { AuthRequester } from '../../..'; import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { showLoginPopup } from '../loginPopup'; +import { AuthConnector } from './types'; const DEFAULT_BASE_PATH = '/api/auth/'; @@ -30,7 +31,13 @@ type Options = { sessionTransform?(response: any): AuthSession | Promise; }; -export class AuthHelper implements AuthHelper { +/** + * DefaultAuthConnector is the default auth connector in Backstage. It talks to the + * backend auth plugin through the standardized API, and requests user permission + * via the OAuthRequestApi. + */ +export class DefaultAuthConnector + implements AuthConnector { private readonly apiOrigin: string; private readonly basePath: string; private readonly providerPath: string; diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.test.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.test.ts similarity index 86% rename from packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.test.ts rename to packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.test.ts index 0cb1e13163..cd7986ffd0 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.test.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import MockAuthHelper, { mockAccessToken } from './MockAuthHelper'; +import { MockAuthConnector, mockAccessToken } from './MockAuthConnector'; -describe('MockAuthHelper', () => { +describe('MockAuthConnector', () => { it('should return mock tokens', async () => { - const helper = new MockAuthHelper(); + const helper = new MockAuthConnector(); await expect(helper.createSession()).resolves.toEqual({ accessToken: mockAccessToken, diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts similarity index 89% rename from packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts rename to packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts index e793ebd047..50f045671a 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/MockAuthHelper.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { GenericAuthHelper } from './types'; +import { AuthConnector } from './types'; export const mockAccessToken = 'mock-access-token'; @@ -30,7 +30,7 @@ const defaultMockSession: MockSession = { scopes: 'profile email', }; -export default class MockAuthHelper implements GenericAuthHelper { +export class MockAuthConnector implements AuthConnector { constructor(private readonly mockSession: MockSession = defaultMockSession) {} async refreshSession() { diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/index.ts similarity index 90% rename from packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts rename to packages/core/src/api/apis/implementations/lib/AuthConnector/index.ts index e472ca06de..db5c582328 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/index.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/index.ts @@ -14,5 +14,5 @@ * limitations under the License. */ -export { AuthHelper } from './AuthHelper'; +export { DefaultAuthConnector } from './DefaultAuthConnector'; export * from './types'; diff --git a/packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts similarity index 80% rename from packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts rename to packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts index b9ccae11c8..86858a90e5 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthHelper/types.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts @@ -19,7 +19,11 @@ export type BaseAuthSession = { expiresAt: Date; }; -export type GenericAuthHelper = { +/** + * An AuthConnector is responsible for realizing auth session actions + * by for example communicating with a backend or interacting with the user. + */ +export type AuthConnector = { refreshSession(): Promise; removeSession(): Promise; createSession(scopes: Set): Promise; diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts index 511f860f75..fb56b9e62d 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts @@ -24,7 +24,7 @@ describe('RefreshingAuthSessionManager', () => { const createSession = jest.fn().mockResolvedValue({ expiresAt: theFuture }); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ - helper: { createSession, refreshSession }, + connector: { createSession, refreshSession }, } as any); await manager.getSession({}); @@ -40,7 +40,7 @@ describe('RefreshingAuthSessionManager', () => { const createSession = jest.fn(); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ - helper: { createSession, refreshSession }, + connector: { createSession, refreshSession }, } as any); createSession.mockResolvedValue({ @@ -64,7 +64,7 @@ describe('RefreshingAuthSessionManager', () => { .mockRejectedValueOnce(new Error('NOPE')) .mockResolvedValue({ scopes: new Set(['a']) }); const manager = new RefreshingAuthSessionManager({ - helper: { createSession, refreshSession }, + connector: { createSession, refreshSession }, } as any); createSession.mockResolvedValue({ @@ -85,7 +85,7 @@ describe('RefreshingAuthSessionManager', () => { const createSession = jest.fn(); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ - helper: { createSession, refreshSession }, + connector: { createSession, refreshSession }, } as any); createSession.mockRejectedValueOnce(new Error('some error')); @@ -98,7 +98,7 @@ describe('RefreshingAuthSessionManager', () => { const createSession = jest.fn(); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ - helper: { createSession, refreshSession }, + connector: { createSession, refreshSession }, } as any); expect(await manager.getSession({ optional: true })).toBe(undefined); @@ -118,7 +118,7 @@ describe('RefreshingAuthSessionManager', () => { const removeSession = jest.fn(); const manager = new RefreshingAuthSessionManager({ - helper: { removeSession }, + connector: { removeSession }, } as any); await manager.removeSession(); diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index 5670a0f7df..fbbb95a1eb 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -16,10 +16,10 @@ import { hasScopes } from '../../OAuthRequestManager/OAuthPendingRequests'; import { SessionManager } from './types'; -import { BaseAuthSession, GenericAuthHelper } from '../AuthHelper'; +import { BaseAuthSession, AuthConnector } from '../AuthConnector'; type Options = { - helper: GenericAuthHelper; + connector: AuthConnector; defaultScopes?: Set; }; @@ -29,16 +29,16 @@ type Options = { */ export class RefreshingAuthSessionManager implements SessionManager { - private readonly helper: GenericAuthHelper; + private readonly connector: AuthConnector; private readonly defaultScopes?: Set; private refreshPromise?: Promise; private currentSession: AuthSession | undefined; constructor(options: Options) { - const { helper, defaultScopes = new Set() } = options; + const { connector, defaultScopes = new Set() } = options; - this.helper = helper; + this.connector = connector; this.defaultScopes = defaultScopes; } @@ -92,14 +92,14 @@ export class RefreshingAuthSessionManager } // We can call authRequester multiple times, the returned session will contain all requested scopes. - this.currentSession = await this.helper.createSession( + this.currentSession = await this.connector.createSession( this.getExtendedScope(options.scope), ); return this.currentSession; } async removeSession() { - await this.helper.removeSession(); + await this.connector.removeSession(); window.location.reload(); // TODO(Rugvip): make this work without reload? } @@ -141,7 +141,7 @@ export class RefreshingAuthSessionManager return this.refreshPromise; } - this.refreshPromise = this.helper.refreshSession(); + this.refreshPromise = this.connector.refreshSession(); try { return await this.refreshPromise; From 42000fbaaf75745b1d6f72e545b7491b94f49ab8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 12:34:42 +0200 Subject: [PATCH 10/18] packages/core: refactor AuthConnector options a bit and add docs --- .../implementations/auth/google/GoogleAuth.ts | 2 +- .../DefaultAuthConnector.test.ts | 2 +- .../lib/AuthConnector/DefaultAuthConnector.ts | 29 ++++++++++++++----- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index 599ce789db..e3c10d71f7 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -38,9 +38,9 @@ const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; class GoogleAuth implements OAuthApi, OpenIdConnectApi { static create(oauthRequestApi: OAuthRequestApi) { const connector = new DefaultAuthConnector({ - providerPath: 'google/', environment: 'dev', provider: { + id: 'google', title: 'Google', icon: GoogleIcon, }, diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts index 4ea3bde015..812ee07add 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -23,9 +23,9 @@ const anyFetch = fetch as any; const defaultOptions = { apiOrigin: 'my-origin', - providerPath: 'my-provider', environment: 'production', provider: { + id: 'my-provider', title: 'My Provider', icon: ProviderIcon, }, diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts index bad8ac948b..e9d3918742 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts @@ -22,12 +22,30 @@ import { AuthConnector } from './types'; const DEFAULT_BASE_PATH = '/api/auth/'; type Options = { + /** + * Origin of auth requests, defaults to location.origin + */ apiOrigin?: string; + /** + * Base path of the auth requests, defaults to /api/auth/ + */ basePath?: string; - providerPath: string; + /** + * Environment hint passed on to auth backend, for example 'production' or 'development' + */ environment: string; - provider: AuthProvider; + /** + * Information about the auth provider to be shown to the user. + * The ID Must match the backend auth plugin configuration, for example 'google'. + */ + provider: AuthProvider & { id: string }; + /** + * API used to instanciate an auth requester. + */ oauthRequestApi: OAuthRequestApi; + /** + * Function used to transform an auth response into the session type. + */ sessionTransform?(response: any): AuthSession | Promise; }; @@ -40,9 +58,8 @@ export class DefaultAuthConnector implements AuthConnector { private readonly apiOrigin: string; private readonly basePath: string; - private readonly providerPath: string; private readonly environment: string; - private readonly provider: AuthProvider; + private readonly provider: AuthProvider & { id: string }; private readonly authRequester: AuthRequester; private readonly sessionTransform: (response: any) => Promise; @@ -50,7 +67,6 @@ export class DefaultAuthConnector const { apiOrigin = window.location.origin, basePath = DEFAULT_BASE_PATH, - providerPath, environment, provider, oauthRequestApi, @@ -64,7 +80,6 @@ export class DefaultAuthConnector this.apiOrigin = apiOrigin; this.basePath = basePath; - this.providerPath = providerPath; this.environment = environment; this.provider = provider; this.sessionTransform = sessionTransform; @@ -141,7 +156,7 @@ export class DefaultAuthConnector env: this.environment, }); - return `${this.apiOrigin}${this.basePath}${this.providerPath}${path}${queryString}`; + return `${this.apiOrigin}${this.basePath}${this.provider.id}${path}${queryString}`; } private buildQueryString(query?: { From 3ae262f4f829156ab4f2833848112ea118732754 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 12:46:29 +0200 Subject: [PATCH 11/18] packages/core: configurable scope join for DefaultAuthConnector --- .../DefaultAuthConnector.test.ts | 21 +++++++++++++++++++ .../lib/AuthConnector/DefaultAuthConnector.ts | 11 +++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts index 812ee07add..f3698b4e15 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -122,4 +122,25 @@ describe('DefaultAuthConnector', () => { expiresAt: expect.any(Date), }); }); + + it('should use join func to join scopes', async () => { + const mockOauth = new MockOAuthApi(); + const popupSpy = jest + .spyOn(loginPopup, 'showLoginPopup') + .mockResolvedValue({ scopes: '' }); + const helper = new DefaultAuthConnector({ + ...defaultOptions, + joinScopes: scopes => `-${[...scopes].join('')}-`, + oauthRequestApi: mockOauth, + }); + + helper.createSession(new Set(['a', 'b'])); + + await mockOauth.triggerAll(); + + expect(popupSpy).toBeCalledTimes(1); + expect(popupSpy.mock.calls[0][0]).toMatchObject({ + url: 'my-origin/api/auth/my-provider/start?scope=-ab-&env=production', + }); + }); }); diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts index e9d3918742..498565e10e 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts @@ -43,12 +43,20 @@ type Options = { * API used to instanciate an auth requester. */ oauthRequestApi: OAuthRequestApi; + /** + * Function used to join together a set of scopes, defaults to joining with whitespace. + */ + joinScopes?: (scopes: Set) => string; /** * Function used to transform an auth response into the session type. */ sessionTransform?(response: any): AuthSession | Promise; }; +function defaultJoinScopes(scopes: Set) { + return [...scopes].join(' '); +} + /** * DefaultAuthConnector is the default auth connector in Backstage. It talks to the * backend auth plugin through the standardized API, and requests user permission @@ -69,13 +77,14 @@ export class DefaultAuthConnector basePath = DEFAULT_BASE_PATH, environment, provider, + joinScopes = defaultJoinScopes, oauthRequestApi, sessionTransform = id => id, } = options; this.authRequester = oauthRequestApi.createAuthRequester({ provider, - onAuthRequest: scopes => this.showPopup([...scopes].join(' ')), + onAuthRequest: scopes => this.showPopup(joinScopes(scopes)), }); this.apiOrigin = apiOrigin; From 930c2dba2b4640a1a21c0e9c27c4ba9890be8bd6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 12:53:39 +0200 Subject: [PATCH 12/18] packages/core: reorder some AuthConnector methods --- .../lib/AuthConnector/DefaultAuthConnector.ts | 8 ++++---- .../lib/AuthConnector/MockAuthConnector.ts | 8 ++++---- .../api/apis/implementations/lib/AuthConnector/types.ts | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts index 498565e10e..12a2118a42 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts @@ -94,6 +94,10 @@ export class DefaultAuthConnector this.sessionTransform = sessionTransform; } + async createSession(scopes: Set): Promise { + return this.authRequester(scopes); + } + async refreshSession(): Promise { const res = await fetch(this.buildUrl('/token', { optional: true }), { headers: { @@ -138,10 +142,6 @@ export class DefaultAuthConnector } } - async createSession(scopes: Set): Promise { - return this.authRequester(scopes); - } - private async showPopup(scope: string): Promise { const popupUrl = this.buildUrl('/start', { scope }); diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts index 50f045671a..9134fd0773 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/MockAuthConnector.ts @@ -33,13 +33,13 @@ const defaultMockSession: MockSession = { export class MockAuthConnector implements AuthConnector { constructor(private readonly mockSession: MockSession = defaultMockSession) {} + async createSession() { + return this.mockSession; + } + async refreshSession() { return this.mockSession; } async removeSession() {} - - async createSession() { - return this.mockSession; - } } diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts index 86858a90e5..ad3188dee4 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts @@ -24,7 +24,7 @@ export type BaseAuthSession = { * by for example communicating with a backend or interacting with the user. */ export type AuthConnector = { + createSession(scopes: Set): Promise; refreshSession(): Promise; removeSession(): Promise; - createSession(scopes: Set): Promise; }; From 31202a1e95a5859748a04a91371f83a5d643ce4d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 16:32:06 +0200 Subject: [PATCH 13/18] packages/core: refactor to remove need to BaseAuthSession --- .../implementations/auth/google/GoogleAuth.ts | 5 ++ .../lib/AuthConnector/types.ts | 5 -- .../RefreshingAuthSessionManager.test.ts | 18 +++++-- .../RefreshingAuthSessionManager.ts | 52 ++++++++++++++----- .../lib/AuthSessionManager/types.ts | 4 +- 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index e3c10d71f7..04b5404c0a 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -62,6 +62,11 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { `${SCOPE_PREFIX}userinfo.email`, `${SCOPE_PREFIX}userinfo.profile`, ]), + sessionScopes: session => session.scopes, + sessionShouldRefresh: session => { + const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; + return expiresInSec < 60 * 5; + }, }); return new GoogleAuth(sessionManager); diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts index ad3188dee4..146c31cbe1 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/types.ts @@ -14,11 +14,6 @@ * limitations under the License. */ -export type BaseAuthSession = { - scopes: Set; - expiresAt: Date; -}; - /** * An AuthConnector is responsible for realizing auth session actions * by for example communicating with a backend or interacting with the user. diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts index fb56b9e62d..6841934357 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts @@ -16,15 +16,18 @@ import { RefreshingAuthSessionManager } from './RefreshingAuthSessionManager'; -const theFuture = new Date(Date.now() + 3600000); -const thePast = new Date(Date.now() - 10); +const defaultOptions = { + sessionScopes: (session: { scopes: Set }) => session.scopes, + sessionShouldRefresh: (session: { expired: boolean }) => session.expired, +}; describe('RefreshingAuthSessionManager', () => { it('should save result form createSession', async () => { - const createSession = jest.fn().mockResolvedValue({ expiresAt: theFuture }); + const createSession = jest.fn().mockResolvedValue({ expired: false }); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ connector: { createSession, refreshSession }, + ...defaultOptions, } as any); await manager.getSession({}); @@ -41,11 +44,12 @@ describe('RefreshingAuthSessionManager', () => { const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ connector: { createSession, refreshSession }, + ...defaultOptions, } as any); createSession.mockResolvedValue({ scopes: new Set(['a']), - expiresAt: theFuture, + expired: false, }); await manager.getSession({ scope: new Set(['a']) }); expect(createSession).toBeCalledTimes(1); @@ -65,11 +69,12 @@ describe('RefreshingAuthSessionManager', () => { .mockResolvedValue({ scopes: new Set(['a']) }); const manager = new RefreshingAuthSessionManager({ connector: { createSession, refreshSession }, + ...defaultOptions, } as any); createSession.mockResolvedValue({ scopes: new Set(['a']), - expiresAt: thePast, + expired: true, }); await manager.getSession({ scope: new Set(['a']) }); @@ -86,6 +91,7 @@ describe('RefreshingAuthSessionManager', () => { const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ connector: { createSession, refreshSession }, + ...defaultOptions, } as any); createSession.mockRejectedValueOnce(new Error('some error')); @@ -99,6 +105,7 @@ describe('RefreshingAuthSessionManager', () => { const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ connector: { createSession, refreshSession }, + ...defaultOptions, } as any); expect(await manager.getSession({ optional: true })).toBe(undefined); @@ -119,6 +126,7 @@ describe('RefreshingAuthSessionManager', () => { const removeSession = jest.fn(); const manager = new RefreshingAuthSessionManager({ connector: { removeSession }, + ...defaultOptions, } as any); await manager.removeSession(); diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index fbbb95a1eb..147d9f4b49 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -16,10 +16,27 @@ import { hasScopes } from '../../OAuthRequestManager/OAuthPendingRequests'; import { SessionManager } from './types'; -import { BaseAuthSession, AuthConnector } from '../AuthConnector'; +import { AuthConnector } from '../AuthConnector'; -type Options = { +type Options = { + /** + * The connector used for acting on the auth session. + */ connector: AuthConnector; + /** + * A function called to determine the scopes of the session. + */ + sessionScopes: (session: AuthSession) => Set; + /** + * A function called to determine whether it's time for a session to refresh. + * + * This should return true before the session expires, for example, if a session + * expires after 60 minutes, you could return true if the session is older than 45 minutes. + */ + sessionShouldRefresh: (session: AuthSession) => boolean; + /** + * The default scopes that should always be present in a session, defaults to none. + */ defaultScopes?: Set; }; @@ -27,19 +44,28 @@ type Options = { * RefreshingAuthSessionManager manages an underlying session that has * and expiration time and needs to be refreshed periodically. */ -export class RefreshingAuthSessionManager +export class RefreshingAuthSessionManager implements SessionManager { private readonly connector: AuthConnector; private readonly defaultScopes?: Set; + private readonly sessionScopesFunc: (session: AuthSession) => Set; + private readonly sessionShouldRefreshFunc: (session: AuthSession) => boolean; private refreshPromise?: Promise; private currentSession: AuthSession | undefined; constructor(options: Options) { - const { connector, defaultScopes = new Set() } = options; + const { + connector, + defaultScopes = new Set(), + sessionScopes, + sessionShouldRefresh, + } = options; this.connector = connector; this.defaultScopes = defaultScopes; + this.sessionScopesFunc = sessionScopes; + this.sessionShouldRefreshFunc = sessionShouldRefresh; } async getSession(options: { @@ -55,13 +81,16 @@ export class RefreshingAuthSessionManager scope?: Set; }): Promise { if (this.sessionExistsAndHasScope(this.currentSession, options.scope)) { - if (!this.sessionWillExpire(this.currentSession!)) { + const shouldRefresh = this.sessionShouldRefreshFunc(this.currentSession!); + if (!shouldRefresh) { return this.currentSession!; } try { const refreshedSession = await this.collapsedSessionRefresh(); - if (hasScopes(refreshedSession.scopes, this.currentSession!.scopes)) { + const currentScopes = this.sessionScopesFunc(this.currentSession!); + const refreshedScopes = this.sessionScopesFunc(refreshedSession); + if (hasScopes(refreshedScopes, currentScopes)) { this.currentSession = refreshedSession; } return refreshedSession; @@ -113,18 +142,15 @@ export class RefreshingAuthSessionManager if (!scope) { return true; } - return hasScopes(session.scopes, scope); - } - - private sessionWillExpire(session: AuthSession) { - const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; - return expiresInSec < 60 * 5; + const sessionScopes = this.sessionScopesFunc(session); + return hasScopes(sessionScopes, scope); } private getExtendedScope(scopes?: Set) { const newScope = new Set(this.defaultScopes); if (this.currentSession) { - for (const scope of this.currentSession.scopes) { + const sessionScopes = this.sessionScopesFunc(this.currentSession); + for (const scope of sessionScopes) { newScope.add(scope); } } diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts index 6475776505..440944cce0 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/types.ts @@ -14,14 +14,12 @@ * limitations under the License. */ -import { BaseAuthSession } from '../AuthHelper/types'; - /** * A sessions manager keeps track of the current session and makes sure that * multiple simultaneous requests for sessions with different scope are handled * in a correct way. */ -export type SessionManager = { +export type SessionManager = { getSession(options: { optional: false; scope?: Set; From a928e952cdda15f628f6bb7aa3e2d0d9ce0e27c1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 May 2020 17:19:04 +0200 Subject: [PATCH 14/18] packages/core: add tests for google auth scope normalization --- .../auth/google/GoogleAuth.test.ts | 27 +++++++++++++++++++ .../implementations/auth/google/GoogleAuth.ts | 4 +-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts index fde1256ea8..1ee3575b89 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.test.ts @@ -105,4 +105,31 @@ describe('GoogleAuth', () => { await expect(promise3).resolves.toBe('token2'); expect(getSession).toBeCalledTimes(4); // De-duping of session requests happens in client }); + + it.each([ + ['email', [`${PREFIX}userinfo.email`]], + ['profile', [`${PREFIX}userinfo.profile`]], + ['openid', ['openid']], + ['userinfo.email', [`${PREFIX}userinfo.email`]], + [ + 'userinfo.profile email', + [`${PREFIX}userinfo.profile`, `${PREFIX}userinfo.email`], + ], + [ + `profile ${PREFIX}userinfo.email`, + [`${PREFIX}userinfo.profile`, `${PREFIX}userinfo.email`], + ], + [`${PREFIX}userinfo.profile`, [`${PREFIX}userinfo.profile`]], + ['a', [`${PREFIX}a`]], + ['a b\tc', [`${PREFIX}a`, `${PREFIX}b`, `${PREFIX}c`]], + [`${PREFIX}a b`, [`${PREFIX}a`, `${PREFIX}b`]], + [`${PREFIX}a`, [`${PREFIX}a`]], + + // Some incorrect scopes that we don't try to fix + [`${PREFIX}email`, [`${PREFIX}email`]], + [`${PREFIX}profile`, [`${PREFIX}profile`]], + [`${PREFIX}openid`, [`${PREFIX}openid`]], + ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + expect(GoogleAuth.normalizeScopes(scope)).toEqual(new Set(scopes)); + }); }); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index 04b5404c0a..d512d2be95 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -97,14 +97,14 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { await this.sessionManager.removeSession(); } - private static normalizeScopes(scopes?: string | string[]): Set { + static normalizeScopes(scopes?: string | string[]): Set { if (!scopes) { return new Set(); } const scopeList = Array.isArray(scopes) ? scopes - : scopes.split(' ').filter(Boolean); + : scopes.split(/[\s]/).filter(Boolean); const normalizedScopes = scopeList.map(scope => { if (scope === 'openid') { From 72feb6055eec5eff5ae10a60664b14a46604609d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 May 2020 13:38:54 +0200 Subject: [PATCH 15/18] package/core: forward more config through GoogleAuth --- .../implementations/auth/google/GoogleAuth.ts | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index d512d2be95..d1d5da1a02 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -22,10 +22,21 @@ import { OpenIdConnectApi, IdTokenOptions, } from '../../../definitions/auth'; -import { OAuthRequestApi } from '../../../definitions'; +import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../lib/AuthSessionManager/types'; import { RefreshingAuthSessionManager } from '../../lib/AuthSessionManager'; +type CreateOptions = { + // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GoogleAuth + apiOrigin: string; + basePath: string; + + oauthRequestApi: OAuthRequestApi; + + environment?: string; + provider?: AuthProvider & { id: string }; +}; + export type GoogleAuthResponse = { accessToken: string; idToken: string; @@ -33,17 +44,27 @@ export type GoogleAuthResponse = { expiresInSeconds: number; }; +const DEFAULT_PROVIDER = { + id: 'google', + title: 'Google', + icon: GoogleIcon, +}; + const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; class GoogleAuth implements OAuthApi, OpenIdConnectApi { - static create(oauthRequestApi: OAuthRequestApi) { + static create({ + apiOrigin, + basePath, + environment = 'dev', + provider = DEFAULT_PROVIDER, + oauthRequestApi, + }: CreateOptions) { const connector = new DefaultAuthConnector({ - environment: 'dev', - provider: { - id: 'google', - title: 'Google', - icon: GoogleIcon, - }, + apiOrigin, + basePath, + environment, + provider, oauthRequestApi: oauthRequestApi, sessionTransform(res: GoogleAuthResponse): GoogleSession { return { From 659e904f55843ed98da9e8e503b04bd7fd23163b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 May 2020 13:39:39 +0200 Subject: [PATCH 16/18] packages/core: export auth APIs --- packages/core/src/api/apis/definitions/index.ts | 2 ++ .../src/api/apis/implementations/auth/index.ts | 17 +++++++++++++++++ .../core/src/api/apis/implementations/index.ts | 1 + 3 files changed, 20 insertions(+) create mode 100644 packages/core/src/api/apis/implementations/auth/index.ts diff --git a/packages/core/src/api/apis/definitions/index.ts b/packages/core/src/api/apis/definitions/index.ts index 2bb365b2ab..2e008965cd 100644 --- a/packages/core/src/api/apis/definitions/index.ts +++ b/packages/core/src/api/apis/definitions/index.ts @@ -20,6 +20,8 @@ // // If you think some API definition is missing, please open an Issue or send a PR! +export * from './auth'; + export * from './AlertApi'; export * from './AppThemeApi'; export * from './ErrorApi'; diff --git a/packages/core/src/api/apis/implementations/auth/index.ts b/packages/core/src/api/apis/implementations/auth/index.ts new file mode 100644 index 0000000000..5fa6644b2a --- /dev/null +++ b/packages/core/src/api/apis/implementations/auth/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 './google'; diff --git a/packages/core/src/api/apis/implementations/index.ts b/packages/core/src/api/apis/implementations/index.ts index 8334b068fc..0bf5cbbb24 100644 --- a/packages/core/src/api/apis/implementations/index.ts +++ b/packages/core/src/api/apis/implementations/index.ts @@ -18,6 +18,7 @@ // // Plugins should rely on these APIs for functionality as much as possible. +export * from './auth'; export * from './AppThemeSelector'; export * from './AlertApiForwarder'; export * from './ErrorApiForwarder'; From c1b76913a07afd9f5b4f276924e45663dfafcffd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 May 2020 13:41:55 +0200 Subject: [PATCH 17/18] packages/app: add oauth request and google auth APIs --- packages/app/src/App.tsx | 3 ++- packages/app/src/apis.ts | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index fc2e876468..e29fac8865 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { createApp } from '@backstage/core'; +import { createApp, OAuthRequestDialog } from '@backstage/core'; import React, { FC } from 'react'; import { BrowserRouter as Router } from 'react-router-dom'; import Root from './components/Root'; @@ -33,6 +33,7 @@ const AppComponent = app.getRootComponent(); const App: FC<{}> = () => ( + diff --git a/packages/app/src/apis.ts b/packages/app/src/apis.ts index c3380215b3..c11ea24aca 100644 --- a/packages/app/src/apis.ts +++ b/packages/app/src/apis.ts @@ -23,6 +23,10 @@ import { ErrorApiForwarder, featureFlagsApiRef, FeatureFlags, + GoogleAuth, + oauthRequestApiRef, + OAuthRequestManager, + googleAuthApiRef, } from '@backstage/core'; import { @@ -46,6 +50,20 @@ builder.add(featureFlagsApiRef, new FeatureFlags()); builder.add(lighthouseApiRef, new LighthouseRestApi('http://localhost:3003')); +const oauthRequestApi = builder.add( + oauthRequestApiRef, + new OAuthRequestManager(), +); + +builder.add( + googleAuthApiRef, + GoogleAuth.create({ + apiOrigin: 'http://localhost:7000', + basePath: '/auth/', + oauthRequestApi, + }), +); + builder.add( techRadarApiRef, new TechRadar({ From a22a9cbfddacfa97c2a8e118b9bed8e1d0f9f987 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 May 2020 15:26:27 +0200 Subject: [PATCH 18/18] packages/core: minor auth text fixes + reduntant has --- .../OAuthRequestManager/OAuthPendingRequests.ts | 4 +--- .../implementations/lib/AuthConnector/DefaultAuthConnector.ts | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts index 6ee1674b39..aa5609b1e9 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts @@ -49,9 +49,7 @@ export function joinScopes( for (const moreScopes of moreScopess) { for (const scope of moreScopes) { - if (!result.has(scope)) { - result.add(scope); - } + result.add(scope); } } diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts index 12a2118a42..161015b1a8 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts @@ -40,11 +40,11 @@ type Options = { */ provider: AuthProvider & { id: string }; /** - * API used to instanciate an auth requester. + * API used to instantiate an auth requester. */ oauthRequestApi: OAuthRequestApi; /** - * Function used to join together a set of scopes, defaults to joining with whitespace. + * Function used to join together a set of scopes, defaults to joining with a space character. */ joinScopes?: (scopes: Set) => string; /**