From ff50a98ac490cb3119676d4809cc0ba71a0c17cc Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Sun, 31 May 2020 14:50:59 +0200 Subject: [PATCH 1/3] Add PassportStrategyHelper tests. Move util fns to OAuthProvider class. --- .../src/providers/OAuthProvider.test.ts | 5 ++ .../providers/PassportStrategyHelper.test.ts | 65 +++++++------------ .../src/providers/PassportStrategyHelper.ts | 2 +- 3 files changed, 31 insertions(+), 41 deletions(-) create mode 100644 plugins/auth-backend/src/providers/OAuthProvider.test.ts diff --git a/plugins/auth-backend/src/providers/OAuthProvider.test.ts b/plugins/auth-backend/src/providers/OAuthProvider.test.ts new file mode 100644 index 0000000000..b06d5571e8 --- /dev/null +++ b/plugins/auth-backend/src/providers/OAuthProvider.test.ts @@ -0,0 +1,5 @@ +describe('OAuthProvider', () => { + it('unbreak test runner', () => { + expect(true).toBeTruthy(); + }); +}); diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts index e977ee877d..9e87af4569 100644 --- a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts +++ b/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts @@ -1,19 +1,3 @@ -/* - * 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 express from 'express'; import passport from 'passport'; import { @@ -21,18 +5,19 @@ import { executeFrameHandlerStrategy, executeRefreshTokenStrategy, } from './PassportStrategyHelper'; +import { RedirectInfo } from './types'; const mockRequest = ({} as unknown) as express.Request; describe('PassportStrategyHelper', () => { class MyCustomRedirectStrategy extends passport.Strategy { - authenticate() { + authenticate(_req: express.Request, options: any) { this.redirect('a', 302); } } describe('executeRedirectStrategy', () => { - it('should call authenticate and resolve with RedirectInfo', async () => { + it('should call authenticate and resolve with RedirectInfo', () => { const mockStrategy = new MyCustomRedirectStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const redirectStrategyPromise = executeRedirectStrategy( @@ -41,7 +26,7 @@ describe('PassportStrategyHelper', () => { {}, ); expect(spyAuthenticate).toBeCalledTimes(1); - await expect(redirectStrategyPromise).resolves.toStrictEqual( + expect(redirectStrategyPromise).resolves.toStrictEqual( expect.objectContaining({ url: 'a', status: 302 }), ); }); @@ -49,7 +34,7 @@ describe('PassportStrategyHelper', () => { describe('executeFrameHandlerStrategy', () => { class MyCustomAuthSuccessStrategy extends passport.Strategy { - authenticate() { + authenticate(_req: express.Request, options: any) { this.success( { accessToken: 'ACCESS_TOKEN' }, { refreshToken: 'REFRESH_TOKEN' }, @@ -57,22 +42,22 @@ describe('PassportStrategyHelper', () => { } } class MyCustomAuthErrorStrategy extends passport.Strategy { - authenticate() { + authenticate(_req: express.Request, options: any) { this.error(new Error('MyCustomAuth error')); } } class MyCustomAuthRedirectStrategy extends passport.Strategy { - authenticate() { + authenticate(_req: express.Request, options: any) { this.redirect('URL', 302); } } class MyCustomAuthFailStrategy extends passport.Strategy { - authenticate() { + authenticate(_req: express.Request, options: any) { this.fail('challenge', 302); } } - it('should resolve with user and info on success', async () => { + it('should resolve with user and info on success', () => { const mockStrategy = new MyCustomAuthSuccessStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -80,7 +65,7 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - await expect(frameHandlerStrategyPromise).resolves.toStrictEqual( + expect(frameHandlerStrategyPromise).resolves.toStrictEqual( expect.objectContaining({ user: { accessToken: 'ACCESS_TOKEN' }, info: { refreshToken: 'REFRESH_TOKEN' }, @@ -88,7 +73,7 @@ describe('PassportStrategyHelper', () => { ); }); - it('should reject on error', async () => { + it('should reject on error', () => { const mockStrategy = new MyCustomAuthErrorStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -96,12 +81,12 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - await expect(frameHandlerStrategyPromise).rejects.toThrow( + expect(frameHandlerStrategyPromise).rejects.toThrowError( 'Authentication failed, Error: MyCustomAuth error', ); }); - it('should reject on redirect', async () => { + it('should reject on redirect', () => { const mockStrategy = new MyCustomAuthRedirectStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -109,12 +94,12 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - await expect(frameHandlerStrategyPromise).rejects.toThrow( + expect(frameHandlerStrategyPromise).rejects.toThrowError( 'Unexpected redirect', ); }); - it('should reject on fail', async () => { + it('should reject on fail', () => { const mockStrategy = new MyCustomAuthFailStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -122,13 +107,14 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - await expect(frameHandlerStrategyPromise).rejects.toThrow(); + expect(frameHandlerStrategyPromise).rejects.toThrow(); }); }); describe('executeRefreshTokenStrategy', () => { - it('should resolve with a new access token, scope and expiry', async () => { + it('should resolve with a new access token, scope and expiry', () => { class MyCustomOAuth2Success { + constructor() {} getOAuthAccessToken( _refreshToken: string, _options: any, @@ -141,7 +127,6 @@ describe('PassportStrategyHelper', () => { } } class MyCustomRefreshTokenSuccess extends passport.Strategy { - // @ts-ignore private _oauth2 = new MyCustomOAuth2Success(); } @@ -151,7 +136,7 @@ describe('PassportStrategyHelper', () => { 'REFRESH_TOKEN', 'a', ); - await expect(refreshTokenPromise).resolves.toStrictEqual( + expect(refreshTokenPromise).resolves.toStrictEqual( expect.objectContaining({ accessToken: 'ACCESS_TOKEN', params: expect.objectContaining({ scope: 'a', expires_in: 10 }), @@ -159,8 +144,9 @@ describe('PassportStrategyHelper', () => { ); }); - it('should reject with an error if refresh failed', async () => { + it('should reject with an error if refresh failed', () => { class MyCustomOAuth2Error { + constructor() {} getOAuthAccessToken( _refreshToken: string, _options: any, @@ -170,7 +156,6 @@ describe('PassportStrategyHelper', () => { } } class MyCustomRefreshTokenSuccess extends passport.Strategy { - // @ts-ignore private _oauth2 = new MyCustomOAuth2Error(); } @@ -180,13 +165,14 @@ describe('PassportStrategyHelper', () => { 'REFRESH_TOKEN', 'a', ); - await expect(refreshTokenPromise).rejects.toThrow( + expect(refreshTokenPromise).rejects.toThrow( 'Failed to refresh access token Error: Unknown error', ); }); - it('should reject with an error if access token missing in refresh callback', async () => { + it('should reject with an error if access token missing in refresh callback', () => { class MyCustomOAuth2AccessTokenMissing { + constructor() {} getOAuthAccessToken( _refreshToken: string, _options: any, @@ -196,7 +182,6 @@ describe('PassportStrategyHelper', () => { } } class MyCustomRefreshTokenSuccess extends passport.Strategy { - // @ts-ignore private _oauth2 = new MyCustomOAuth2AccessTokenMissing(); } @@ -206,7 +191,7 @@ describe('PassportStrategyHelper', () => { 'REFRESH_TOKEN', 'a', ); - await expect(refreshTokenPromise).rejects.toThrow( + expect(refreshTokenPromise).rejects.toThrow( 'Failed to refresh access token, no access token received', ); }); diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts b/plugins/auth-backend/src/providers/PassportStrategyHelper.ts index 5c02930c2c..684e357e47 100644 --- a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/providers/PassportStrategyHelper.ts @@ -23,7 +23,7 @@ export const executeRedirectStrategy = async ( providerStrategy: passport.Strategy, options: any, ): Promise => { - return new Promise(resolve => { + return new Promise((resolve, reject) => { const strategy = Object.create(providerStrategy); strategy.redirect = (url: string, status?: number) => { resolve({ url, status: status ?? undefined }); From a9680936d0acf35dab857a9336dc8639e2bff652 Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Sun, 31 May 2020 22:06:49 +0200 Subject: [PATCH 2/3] Add tests for OAuthProvider Utils test --- .../src/providers/OAuthProvider.test.ts | 199 +++++++++++++++++- 1 file changed, 196 insertions(+), 3 deletions(-) diff --git a/plugins/auth-backend/src/providers/OAuthProvider.test.ts b/plugins/auth-backend/src/providers/OAuthProvider.test.ts index b06d5571e8..62d2f5a32d 100644 --- a/plugins/auth-backend/src/providers/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/providers/OAuthProvider.test.ts @@ -1,5 +1,198 @@ -describe('OAuthProvider', () => { - it('unbreak test runner', () => { - expect(true).toBeTruthy(); +/* + * 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 express from 'express'; +import { + ensuresXRequestedWith, + postMessageResponse, + removeRefreshTokenCookie, + setRefreshTokenCookie, + THOUSAND_DAYS_MS, + setNonceCookie, + TEN_MINUTES_MS, + verifyNonce, +} from './OAuthProvider'; +import { AuthResponse } from './types'; + +describe('OAuthProvider Utils', () => { + describe('verifyNonce', () => { + it('should throw error if cookie nonce missing', () => { + const mockRequest = ({ + cookies: {}, + query: { + state: 'NONCE', + }, + } as unknown) as express.Request; + expect(() => { + verifyNonce(mockRequest, 'providera'); + }).toThrowError('Missing nonce'); + }); + it('should throw error if state nonce missing', () => { + const mockRequest = ({ + cookies: { + 'providera-nonce': 'NONCE', + }, + query: {}, + } as unknown) as express.Request; + expect(() => { + verifyNonce(mockRequest, 'providera'); + }).toThrowError('Missing nonce'); + }); + it('should throw error if nonce mismatch', () => { + const mockRequest = ({ + cookies: { + 'providera-nonce': 'NONCEA', + }, + query: { + state: 'NONCEB', + }, + } as unknown) as express.Request; + expect(() => { + verifyNonce(mockRequest, 'providera'); + }).toThrowError('Invalid nonce'); + }); + it('should not throw any error if nonce matches', () => { + const mockRequest = ({ + cookies: { + 'providera-nonce': 'NONCE', + }, + query: { + state: 'NONCE', + }, + } as unknown) as express.Request; + expect(() => { + verifyNonce(mockRequest, 'providera'); + }).not.toThrow(); + }); + }); + + describe('setNonceCookie', () => { + it('should set nonce cookie', () => { + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + setNonceCookie(mockResponse, 'providera'); + expect(mockResponse.cookie).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledWith( + 'providera-nonce', + expect.any(String), + expect.objectContaining({ maxAge: TEN_MINUTES_MS }), + ); + }); + }); + + describe('setRefreshTokenCookie', () => { + it('should set refresh token cookie', () => { + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + setRefreshTokenCookie(mockResponse, 'providera', 'REFRESH_TOKEN'); + expect(mockResponse.cookie).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledWith( + 'providera-refresh-token', + 'REFRESH_TOKEN', + expect.objectContaining({ maxAge: THOUSAND_DAYS_MS }), + ); + }); + }); + + describe('removeRefreshTokenCookie', () => { + it('should remove refresh token cookie', () => { + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + removeRefreshTokenCookie(mockResponse, 'providera'); + expect(mockResponse.cookie).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledWith( + 'providera-refresh-token', + '', + expect.objectContaining({ maxAge: 0 }), + ); + }); + }); + + describe('postMessageResponse', () => { + it('should post a message back with payload success', () => { + const mockResponse = ({ + end: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + const data: AuthResponse = { + type: 'auth-result', + payload: { + accessToken: 'ACCESS_TOKEN', + idToken: 'ID_TOKEN', + expiresInSeconds: 10, + scope: 'email', + }, + }; + const jsonData = JSON.stringify(data); + const base64Data = Buffer.from(jsonData, 'utf8').toString('base64'); + + postMessageResponse(mockResponse, data); + expect(mockResponse.setHeader).toBeCalledTimes(2); + expect(mockResponse.end).toBeCalledTimes(1); + expect(mockResponse.end).toBeCalledWith( + expect.stringContaining(base64Data), + ); + }); + + it('should post a message back with payload error', () => { + const mockResponse = ({ + end: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + const data: AuthResponse = { + type: 'auth-result', + error: new Error('Unknown error occured'), + }; + const jsonData = JSON.stringify(data); + const base64Data = Buffer.from(jsonData, 'utf8').toString('base64'); + + postMessageResponse(mockResponse, data); + expect(mockResponse.setHeader).toBeCalledTimes(2); + expect(mockResponse.end).toBeCalledTimes(1); + expect(mockResponse.end).toBeCalledWith( + expect.stringContaining(base64Data), + ); + }); + }); + + describe('ensuresXRequestedWith', () => { + it('should return false if no header present', () => { + const mockRequest = ({ + header: () => jest.fn(), + } as unknown) as express.Request; + expect(ensuresXRequestedWith(mockRequest)).toBe(false); + }); + + it('should return false if header present with incorrect value', () => { + const mockRequest = ({ + header: () => 'INVALID', + } as unknown) as express.Request; + expect(ensuresXRequestedWith(mockRequest)).toBe(false); + }); + + it('should return true if header present with correct value', () => { + const mockRequest = ({ + header: () => 'XMLHttpRequest', + } as unknown) as express.Request; + expect(ensuresXRequestedWith(mockRequest)).toBe(true); + }); }); }); From a53977997c46500998fcabe95a52eb39714b7427 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Wed, 3 Jun 2020 10:15:12 +0200 Subject: [PATCH 3/3] Add tests for OAuthProvider class --- .../src/providers/OAuthProvider.test.ts | 168 +++++++++++++++++- .../providers/PassportStrategyHelper.test.ts | 65 ++++--- .../src/providers/PassportStrategyHelper.ts | 2 +- 3 files changed, 208 insertions(+), 27 deletions(-) diff --git a/plugins/auth-backend/src/providers/OAuthProvider.test.ts b/plugins/auth-backend/src/providers/OAuthProvider.test.ts index 62d2f5a32d..362653b5f7 100644 --- a/plugins/auth-backend/src/providers/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/providers/OAuthProvider.test.ts @@ -24,8 +24,9 @@ import { setNonceCookie, TEN_MINUTES_MS, verifyNonce, + OAuthProvider, } from './OAuthProvider'; -import { AuthResponse } from './types'; +import { AuthResponse, OAuthProviderHandlers } from './types'; describe('OAuthProvider Utils', () => { describe('verifyNonce', () => { @@ -196,3 +197,168 @@ describe('OAuthProvider Utils', () => { }); }); }); + +describe('OAuthProvider', () => { + class MyAuthProvider implements OAuthProviderHandlers { + async start() { + return { + url: '/url', + status: 301, + }; + } + async handler() { + return { + user: {}, + info: { + refreshToken: 'token', + }, + }; + } + async refresh() { + return { + accessToken: 'token', + }; + } + } + const providerInstance = new MyAuthProvider(); + const providerId = 'test-provider'; + + it('sets the correct headers in start', async () => { + const oauthProvider = new OAuthProvider(providerInstance, providerId); + const mockRequest = ({ + query: { + scope: 'user', + }, + } as unknown) as express.Request; + + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + statusCode: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + await oauthProvider.start(mockRequest, mockResponse); + expect(mockResponse.setHeader).toHaveBeenCalledTimes(2); + expect(mockResponse.setHeader).toHaveBeenCalledWith('Location', '/url'); + expect(mockResponse.setHeader).toHaveBeenCalledWith('Content-Length', '0'); + expect(mockResponse.statusCode).toEqual(301); + expect(mockResponse.end).toHaveBeenCalledTimes(1); + }); + + it('sets the refresh cookie if refresh is enabled', async () => { + const oauthProvider = new OAuthProvider(providerInstance, providerId); + + const mockRequest = ({ + cookies: { + 'test-provider-nonce': 'nonce', + }, + query: { + state: 'nonce', + }, + } as unknown) as express.Request; + + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockResponse.cookie).toHaveBeenCalledTimes(1); + expect(mockResponse.cookie).toHaveBeenCalledWith( + expect.stringContaining('test-provider-refresh-token'), + expect.stringContaining('token'), + expect.objectContaining({ path: '/auth/test-provider' }), + ); + }); + + it('does no set the refresh cookie if refresh is disabled', async () => { + const oauthProvider = new OAuthProvider(providerInstance, providerId, true); + + const mockRequest = ({ + cookies: { + 'test-provider-nonce': 'nonce', + }, + query: { + state: 'nonce', + }, + } as unknown) as express.Request; + + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockResponse.cookie).toHaveBeenCalledTimes(0); + }); + + it('removes refresh cookie when logging out', async () => { + const oauthProvider = new OAuthProvider(providerInstance, providerId); + + const mockRequest = ({ + header: () => 'XMLHttpRequest', + } as unknown) as express.Request; + + const mockResponse = ({ + cookie: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + await oauthProvider.logout(mockRequest, mockResponse); + expect(mockResponse.cookie).toHaveBeenCalledTimes(1); + expect(mockResponse.cookie).toHaveBeenCalledWith( + expect.stringContaining('test-provider-refresh-token'), + '', + expect.objectContaining({ path: '/auth/test-provider' }), + ); + }); + + it('gets new access-token when refreshing', async () => { + const oauthProvider = new OAuthProvider(providerInstance, providerId); + + const mockRequest = ({ + header: () => 'XMLHttpRequest', + cookies: { + 'test-provider-refresh-token': 'token', + }, + query: {}, + } as unknown) as express.Request; + + const mockResponse = ({ + send: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + await oauthProvider.refresh(mockRequest, mockResponse); + expect(mockResponse.send).toHaveBeenCalledTimes(1); + expect(mockResponse.send).toHaveBeenCalledWith( + expect.objectContaining({ + accessToken: 'token', + }), + ); + }); + + it('handles refresh without capabilities', async () => { + const oauthProvider = new OAuthProvider(providerInstance, providerId, true); + + const mockRequest = ({ + header: () => 'XMLHttpRequest', + cookies: { + 'test-provider-refresh-token': 'token', + }, + query: {}, + } as unknown) as express.Request; + + const mockResponse = ({ + send: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + await oauthProvider.refresh(mockRequest, mockResponse); + expect(mockResponse.send).toHaveBeenCalledTimes(1); + expect(mockResponse.send).toHaveBeenCalledWith( + 'Refresh token not supported for provider: test-provider', + ); + }); +}); diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts index 9e87af4569..e977ee877d 100644 --- a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts +++ b/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts @@ -1,3 +1,19 @@ +/* + * 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 express from 'express'; import passport from 'passport'; import { @@ -5,19 +21,18 @@ import { executeFrameHandlerStrategy, executeRefreshTokenStrategy, } from './PassportStrategyHelper'; -import { RedirectInfo } from './types'; const mockRequest = ({} as unknown) as express.Request; describe('PassportStrategyHelper', () => { class MyCustomRedirectStrategy extends passport.Strategy { - authenticate(_req: express.Request, options: any) { + authenticate() { this.redirect('a', 302); } } describe('executeRedirectStrategy', () => { - it('should call authenticate and resolve with RedirectInfo', () => { + it('should call authenticate and resolve with RedirectInfo', async () => { const mockStrategy = new MyCustomRedirectStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const redirectStrategyPromise = executeRedirectStrategy( @@ -26,7 +41,7 @@ describe('PassportStrategyHelper', () => { {}, ); expect(spyAuthenticate).toBeCalledTimes(1); - expect(redirectStrategyPromise).resolves.toStrictEqual( + await expect(redirectStrategyPromise).resolves.toStrictEqual( expect.objectContaining({ url: 'a', status: 302 }), ); }); @@ -34,7 +49,7 @@ describe('PassportStrategyHelper', () => { describe('executeFrameHandlerStrategy', () => { class MyCustomAuthSuccessStrategy extends passport.Strategy { - authenticate(_req: express.Request, options: any) { + authenticate() { this.success( { accessToken: 'ACCESS_TOKEN' }, { refreshToken: 'REFRESH_TOKEN' }, @@ -42,22 +57,22 @@ describe('PassportStrategyHelper', () => { } } class MyCustomAuthErrorStrategy extends passport.Strategy { - authenticate(_req: express.Request, options: any) { + authenticate() { this.error(new Error('MyCustomAuth error')); } } class MyCustomAuthRedirectStrategy extends passport.Strategy { - authenticate(_req: express.Request, options: any) { + authenticate() { this.redirect('URL', 302); } } class MyCustomAuthFailStrategy extends passport.Strategy { - authenticate(_req: express.Request, options: any) { + authenticate() { this.fail('challenge', 302); } } - it('should resolve with user and info on success', () => { + it('should resolve with user and info on success', async () => { const mockStrategy = new MyCustomAuthSuccessStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -65,7 +80,7 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - expect(frameHandlerStrategyPromise).resolves.toStrictEqual( + await expect(frameHandlerStrategyPromise).resolves.toStrictEqual( expect.objectContaining({ user: { accessToken: 'ACCESS_TOKEN' }, info: { refreshToken: 'REFRESH_TOKEN' }, @@ -73,7 +88,7 @@ describe('PassportStrategyHelper', () => { ); }); - it('should reject on error', () => { + it('should reject on error', async () => { const mockStrategy = new MyCustomAuthErrorStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -81,12 +96,12 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - expect(frameHandlerStrategyPromise).rejects.toThrowError( + await expect(frameHandlerStrategyPromise).rejects.toThrow( 'Authentication failed, Error: MyCustomAuth error', ); }); - it('should reject on redirect', () => { + it('should reject on redirect', async () => { const mockStrategy = new MyCustomAuthRedirectStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -94,12 +109,12 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - expect(frameHandlerStrategyPromise).rejects.toThrowError( + await expect(frameHandlerStrategyPromise).rejects.toThrow( 'Unexpected redirect', ); }); - it('should reject on fail', () => { + it('should reject on fail', async () => { const mockStrategy = new MyCustomAuthFailStrategy(); const spyAuthenticate = jest.spyOn(mockStrategy, 'authenticate'); const frameHandlerStrategyPromise = executeFrameHandlerStrategy( @@ -107,14 +122,13 @@ describe('PassportStrategyHelper', () => { mockStrategy, ); expect(spyAuthenticate).toBeCalledTimes(1); - expect(frameHandlerStrategyPromise).rejects.toThrow(); + await expect(frameHandlerStrategyPromise).rejects.toThrow(); }); }); describe('executeRefreshTokenStrategy', () => { - it('should resolve with a new access token, scope and expiry', () => { + it('should resolve with a new access token, scope and expiry', async () => { class MyCustomOAuth2Success { - constructor() {} getOAuthAccessToken( _refreshToken: string, _options: any, @@ -127,6 +141,7 @@ describe('PassportStrategyHelper', () => { } } class MyCustomRefreshTokenSuccess extends passport.Strategy { + // @ts-ignore private _oauth2 = new MyCustomOAuth2Success(); } @@ -136,7 +151,7 @@ describe('PassportStrategyHelper', () => { 'REFRESH_TOKEN', 'a', ); - expect(refreshTokenPromise).resolves.toStrictEqual( + await expect(refreshTokenPromise).resolves.toStrictEqual( expect.objectContaining({ accessToken: 'ACCESS_TOKEN', params: expect.objectContaining({ scope: 'a', expires_in: 10 }), @@ -144,9 +159,8 @@ describe('PassportStrategyHelper', () => { ); }); - it('should reject with an error if refresh failed', () => { + it('should reject with an error if refresh failed', async () => { class MyCustomOAuth2Error { - constructor() {} getOAuthAccessToken( _refreshToken: string, _options: any, @@ -156,6 +170,7 @@ describe('PassportStrategyHelper', () => { } } class MyCustomRefreshTokenSuccess extends passport.Strategy { + // @ts-ignore private _oauth2 = new MyCustomOAuth2Error(); } @@ -165,14 +180,13 @@ describe('PassportStrategyHelper', () => { 'REFRESH_TOKEN', 'a', ); - expect(refreshTokenPromise).rejects.toThrow( + await expect(refreshTokenPromise).rejects.toThrow( 'Failed to refresh access token Error: Unknown error', ); }); - it('should reject with an error if access token missing in refresh callback', () => { + it('should reject with an error if access token missing in refresh callback', async () => { class MyCustomOAuth2AccessTokenMissing { - constructor() {} getOAuthAccessToken( _refreshToken: string, _options: any, @@ -182,6 +196,7 @@ describe('PassportStrategyHelper', () => { } } class MyCustomRefreshTokenSuccess extends passport.Strategy { + // @ts-ignore private _oauth2 = new MyCustomOAuth2AccessTokenMissing(); } @@ -191,7 +206,7 @@ describe('PassportStrategyHelper', () => { 'REFRESH_TOKEN', 'a', ); - expect(refreshTokenPromise).rejects.toThrow( + await expect(refreshTokenPromise).rejects.toThrow( 'Failed to refresh access token, no access token received', ); }); diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts b/plugins/auth-backend/src/providers/PassportStrategyHelper.ts index 684e357e47..5c02930c2c 100644 --- a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/providers/PassportStrategyHelper.ts @@ -23,7 +23,7 @@ export const executeRedirectStrategy = async ( providerStrategy: passport.Strategy, options: any, ): Promise => { - return new Promise((resolve, reject) => { + return new Promise(resolve => { const strategy = Object.create(providerStrategy); strategy.redirect = (url: string, status?: number) => { resolve({ url, status: status ?? undefined });