diff --git a/app-config.yaml b/app-config.yaml index dd72051189..1d3bf31083 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -293,6 +293,7 @@ auth: # path: my-sessions environment: development + usePopup: false ### Providing an auth.session.secret will enable session support in the auth-backend # session: # secret: custom session secret diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index 3f5cfc1c58..9e71855714 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -131,6 +131,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -145,6 +146,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -160,6 +162,7 @@ export const apis = [ oauthRequestApi, defaultScopes: ['read:user'], environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -174,6 +177,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -188,6 +192,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -202,6 +207,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -217,6 +223,7 @@ export const apis = [ oauthRequestApi, defaultScopes: ['team'], environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }), }), createApiFactory({ @@ -231,6 +238,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), + usePopup: configApi.getOptionalBoolean('auth.usePopup'), }); }, }), diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 133ac22e5b..aaaccb17de 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -263,6 +263,7 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; + usePopup?: boolean; }; // @public @@ -516,6 +517,7 @@ export type OneLoginAuthCreateOptions = { discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; + usePopup?: boolean; provider?: AuthProviderInfo; }; diff --git a/packages/core-app-api/config.d.ts b/packages/core-app-api/config.d.ts index ae876f7fe0..0dd73369f6 100644 --- a/packages/core-app-api/config.d.ts +++ b/packages/core-app-api/config.d.ts @@ -114,5 +114,11 @@ export interface Config { * @visibility frontend */ environment?: string; + /** + $ The authentication flow type - either using a popup to authenticate or perform a http redirect + * default value: 'true' + * @visibility frontend + */ + usePopup?: boolean; }; } diff --git a/packages/core-app-api/src/apis/implementations/auth/atlassian/AtlassianAuth.ts b/packages/core-app-api/src/apis/implementations/auth/atlassian/AtlassianAuth.ts index 07ebefca28..5582a29c00 100644 --- a/packages/core-app-api/src/apis/implementations/auth/atlassian/AtlassianAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/atlassian/AtlassianAuth.ts @@ -34,6 +34,7 @@ export default class AtlassianAuth { const { discoveryApi, environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, oauthRequestApi, } = options; @@ -43,6 +44,7 @@ export default class AtlassianAuth { oauthRequestApi, provider, environment, + usePopup, }); } } diff --git a/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.ts b/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.ts index 7c5d4f3fc0..1d27eb1aa3 100644 --- a/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.ts @@ -36,6 +36,7 @@ export type BitbucketAuthResponse = { const DEFAULT_PROVIDER = { id: 'bitbucket', title: 'Bitbucket', + provider_id: 'bitbucket-auth-provider', icon: () => null, }; @@ -49,6 +50,7 @@ export default class BitbucketAuth { const { discoveryApi, environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['team'], @@ -59,6 +61,7 @@ export default class BitbucketAuth { oauthRequestApi, provider, environment, + usePopup, defaultScopes, }); } diff --git a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts index 7efe4e95c6..11af9995a1 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -21,6 +21,7 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'github', title: 'GitHub', + provider_id: 'github-auth-provider', icon: () => null, }; @@ -37,6 +38,7 @@ export default class GithubAuth { provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['read:user'], + usePopup = true, } = options; return OAuth2.create({ @@ -45,6 +47,7 @@ export default class GithubAuth { provider, environment, defaultScopes, + usePopup, }); } } diff --git a/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.ts b/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.ts index 00085de8ac..965d0431ac 100644 --- a/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.ts @@ -21,6 +21,7 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'gitlab', title: 'GitLab', + provider_id: 'gitlab-auth-provider', icon: () => null, }; @@ -34,6 +35,7 @@ export default class GitlabAuth { const { discoveryApi, environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['read_user'], @@ -44,6 +46,7 @@ export default class GitlabAuth { oauthRequestApi, provider, environment, + usePopup, defaultScopes, }); } diff --git a/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.ts b/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.ts index 8a528f4511..514b257ada 100644 --- a/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.ts @@ -21,6 +21,7 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'google', title: 'Google', + provider_id: 'google-auth-provider', icon: () => null, }; @@ -37,6 +38,7 @@ export default class GoogleAuth { discoveryApi, oauthRequestApi, environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, defaultScopes = [ 'openid', @@ -50,6 +52,7 @@ export default class GoogleAuth { oauthRequestApi, provider, environment, + usePopup, defaultScopes, scopeTransform(scopes: string[]) { return scopes.map(scope => { diff --git a/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.ts b/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.ts index be5776609d..e988a79268 100644 --- a/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/microsoft/MicrosoftAuth.ts @@ -21,6 +21,7 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'microsoft', title: 'Microsoft', + provider_id: 'microsoft-auth-provider', icon: () => null, }; @@ -33,6 +34,7 @@ export default class MicrosoftAuth { static create(options: OAuthApiCreateOptions): typeof microsoftAuthApiRef.T { const { environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, oauthRequestApi, discoveryApi, @@ -50,6 +52,7 @@ export default class MicrosoftAuth { oauthRequestApi, provider, environment, + usePopup, defaultScopes, }); } diff --git a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts index ad2d04cb56..a1d0c7d3a9 100644 --- a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts +++ b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.ts @@ -77,6 +77,7 @@ export default class OAuth2 provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = [], + usePopup = true, scopeTransform = x => x, } = options; @@ -85,6 +86,7 @@ export default class OAuth2 environment, provider, oauthRequestApi: oauthRequestApi, + usePopup, sessionTransform(res: OAuth2Response): OAuth2Session { return { ...res, diff --git a/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.ts b/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.ts index 42c0ddd0ed..db574e7fb2 100644 --- a/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.ts @@ -21,6 +21,7 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'okta', title: 'Okta', + provider_id: 'okta-auth-provider', icon: () => null, }; @@ -46,6 +47,7 @@ export default class OktaAuth { const { discoveryApi, environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['openid', 'email', 'profile', 'offline_access'], @@ -56,6 +58,7 @@ export default class OktaAuth { oauthRequestApi, provider, environment, + usePopup, defaultScopes, scopeTransform(scopes) { return scopes.map(scope => { diff --git a/packages/core-app-api/src/apis/implementations/auth/onelogin/OneLoginAuth.ts b/packages/core-app-api/src/apis/implementations/auth/onelogin/OneLoginAuth.ts index ea9e2cad28..d767cb6b55 100644 --- a/packages/core-app-api/src/apis/implementations/auth/onelogin/OneLoginAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/onelogin/OneLoginAuth.ts @@ -30,12 +30,14 @@ export type OneLoginAuthCreateOptions = { discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; + usePopup?: boolean; provider?: AuthProviderInfo; }; const DEFAULT_PROVIDER = { id: 'onelogin', title: 'onelogin', + provider_id: 'onelogin-auth-provider', icon: () => null, }; @@ -63,6 +65,7 @@ export default class OneLoginAuth { const { discoveryApi, environment = 'development', + usePopup = true, provider = DEFAULT_PROVIDER, oauthRequestApi, } = options; @@ -72,6 +75,7 @@ export default class OneLoginAuth { oauthRequestApi, provider, environment, + usePopup, defaultScopes: ['openid', 'email', 'profile', 'offline_access'], scopeTransform(scopes) { return scopes.map(scope => { diff --git a/packages/core-app-api/src/apis/implementations/auth/types.ts b/packages/core-app-api/src/apis/implementations/auth/types.ts index 55d6c19098..e29fe0c7d2 100644 --- a/packages/core-app-api/src/apis/implementations/auth/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/types.ts @@ -37,4 +37,5 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; + usePopup?: boolean; }; diff --git a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts index 2b132e09cd..113ee95bd0 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -34,8 +34,10 @@ const defaultOptions = { provider: { id: 'my-provider', title: 'My Provider', + provider_id: 'myprovider-auth-provider', icon: () => null, }, + usePopup: true, oauthRequestApi: new MockOAuthApi(), sessionTransform: ({ expiresInSeconds, ...res }: any) => ({ ...res, @@ -182,4 +184,37 @@ describe('DefaultAuthConnector', () => { url: 'http://my-host/api/auth/my-provider/start?scope=-ab-&origin=http%3A%2F%2Flocalhost&env=production', }); }); + + it('should redirect to api server', async () => { + const mockOauth = new MockOAuthApi(); + const mockResponse = jest.fn(); + // replace the window.location object + Object.defineProperty(window, 'location', { + value: { + hash: { + endsWith: mockResponse, + includes: mockResponse, + }, + assign: mockResponse, + }, + writable: true, + }); + const helper = new DefaultAuthConnector({ + ...defaultOptions, + usePopup: false, + oauthRequestApi: mockOauth, + }); + + const sessionPromise = helper.createSession({ + scopes: new Set(['a', 'b']), + }); + + await mockOauth.triggerAll(); + + await expect(sessionPromise).resolves.toEqual({}); + // redirect to the auth api + expect(window.location.href).toMatch( + 'http://my-host/api/auth/my-provider/start?scope=a%20b&authType=redirect&env=production', + ); + }); }); diff --git a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts index 988c9f17b1..acc6b97b2c 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -32,6 +32,10 @@ type Options = { * Environment hint passed on to auth backend, for example 'production' or 'development' */ environment: string; + /** + * use a popup or a redirect for authentication with backend authentication api + */ + usePopup: boolean; /** * Information about the auth provider to be shown to the user. * The ID Must match the backend auth plugin configuration, for example 'google'. @@ -77,12 +81,37 @@ export class DefaultAuthConnector provider, joinScopes = defaultJoinScopes, oauthRequestApi, + usePopup, sessionTransform = id => id, } = options; this.authRequester = oauthRequestApi.createAuthRequester({ provider, - onAuthRequest: scopes => this.showPopup(scopes), + onAuthRequest: async scopes => { + if (!usePopup) { + // modal before redirect + const scope = this.joinScopesFunc(scopes); + const redirectUrl = await this.buildUrl('/start', { + scope, + origin: window.location.origin, + redirectUrl: window.location.href, + authType: 'redirect', + }); + + if (provider.hasOwnProperty('provider_id')) { + // set the sign in provider here + localStorage.setItem( + '@backstage/core:SignInPage:provider', + provider.provider_id!, + ); + } + + window.location.href = redirectUrl; + // we need to return to exit function or else popup occurs + return Promise.resolve({} as AuthSession); + } + return this.showPopup(scopes); + }, }); this.discoveryApi = discoveryApi; diff --git a/packages/core-app-api/src/lib/AuthConnector/types.ts b/packages/core-app-api/src/lib/AuthConnector/types.ts index 464a2ed627..7c4f19184e 100644 --- a/packages/core-app-api/src/lib/AuthConnector/types.ts +++ b/packages/core-app-api/src/lib/AuthConnector/types.ts @@ -17,6 +17,7 @@ export type CreateSessionOptions = { scopes: Set; instantPopup?: boolean; + redirectUrl?: string; }; /** diff --git a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index d31f5e29bf..67d83bb36a 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -93,7 +93,7 @@ export class RefreshingAuthSessionManager implements SessionManager { // // We skip this check if an instant login popup is requested, as we need to // stay in a synchronous call stack from the user interaction. The downside - // is that that the user will sometimes be requested to log in even if they + // is that the user will sometimes be requested to log in even if they // already had an existing session. if (!this.currentSession && !options.instantPopup) { try { diff --git a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx index 02b1afe4a1..1884e6e358 100644 --- a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx +++ b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx @@ -24,7 +24,11 @@ import Button from '@material-ui/core/Button'; import React, { useMemo, useState } from 'react'; import useObservable from 'react-use/lib/useObservable'; import LoginRequestListItem from './LoginRequestListItem'; -import { useApi, oauthRequestApiRef } from '@backstage/core-plugin-api'; +import { + useApi, + oauthRequestApiRef, + configApiRef, +} from '@backstage/core-plugin-api'; import Typography from '@material-ui/core/Typography'; export type OAuthRequestDialogClassKey = @@ -58,6 +62,12 @@ export function OAuthRequestDialog(_props: {}) { const classes = useStyles(); const [busy, setBusy] = useState(false); const oauthRequestApi = useApi(oauthRequestApiRef); + const configApi = useApi(configApiRef); + const usePopup = configApi.getOptionalBoolean('auth.usePopup') ?? true; + const redirectMessage = usePopup + ? '' + : 'This will trigger a http redirect to OAuth Login.'; + const requests = useObservable( useMemo(() => oauthRequestApi.authRequest$(), [oauthRequestApi]), [], @@ -83,6 +93,7 @@ export function OAuthRequestDialog(_props: {}) { Login Required + {redirectMessage} diff --git a/packages/core-components/src/layout/SignInPage/commonProvider.tsx b/packages/core-components/src/layout/SignInPage/commonProvider.tsx index 8f426a3907..aa458692dd 100644 --- a/packages/core-components/src/layout/SignInPage/commonProvider.tsx +++ b/packages/core-components/src/layout/SignInPage/commonProvider.tsx @@ -24,7 +24,7 @@ import { SignInProvider, SignInProviderConfig, } from './types'; -import { useApi, errorApiRef } from '@backstage/core-plugin-api'; +import { useApi, errorApiRef, configApiRef } from '@backstage/core-plugin-api'; import { GridItem } from './styles'; import { ForwardedError } from '@backstage/errors'; import { UserIdentity } from './UserIdentity'; @@ -33,13 +33,18 @@ const Component: ProviderComponent = ({ config, onSignInSuccess }) => { const { apiRef, title, message } = config as SignInProviderConfig; const authApi = useApi(apiRef); const errorApi = useApi(errorApiRef); + const configApi = useApi(configApiRef); + const usePopup = configApi.getOptionalBoolean('auth.usePopup') ?? true; const handleLogin = async () => { try { const identityResponse = await authApi.getBackstageIdentity({ - instantPopup: true, + instantPopup: usePopup, }); if (!identityResponse) { + if (!usePopup) { + return; + } throw new Error( `The ${title} provider is not configured to support sign-in`, ); diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index dc7aa9eb5f..ecf615dd01 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -193,6 +193,7 @@ export type AuthProviderInfo = { id: string; title: string; icon: IconComponent; + provider_id?: string; }; // @public diff --git a/packages/core-plugin-api/src/apis/definitions/auth.ts b/packages/core-plugin-api/src/apis/definitions/auth.ts index 163ae74806..6ae19d53a6 100644 --- a/packages/core-plugin-api/src/apis/definitions/auth.ts +++ b/packages/core-plugin-api/src/apis/definitions/auth.ts @@ -54,6 +54,11 @@ export type AuthProviderInfo = { * Icon for the auth provider. */ icon: IconComponent; + + /** + * The provider id from the list of provider configured in the login page + */ + provider_id?: string; }; /** @@ -285,7 +290,6 @@ export type SessionApi = { * Sign out from the current session. This will reload the page. */ signOut(): Promise; - /** * Observe the current state of the auth session. Emits the current state on subscription. */ diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index 88cb33863e..b272f81c2e 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -41,6 +41,7 @@ export type AuthProviderConfig = { appUrl: string; isOriginAllowed: (origin: string) => boolean; cookieConfigurer?: CookieConfigurer; + isPopupAuthenticationRequest: boolean; }; // @public (undocumented) @@ -268,7 +269,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { handlers: OAuthHandlers, options: Pick< OAuthAdapterOptions, - 'providerId' | 'persistScopes' | 'callbackUrl' + 'providerId' | 'persistScopes' | 'callbackUrl' | 'redirectUrl' >, ): OAuthAdapter; // (undocumented) @@ -284,10 +285,12 @@ export type OAuthAdapterOptions = { providerId: string; persistScopes?: boolean; appOrigin: string; + redirectUrl?: string; baseUrl: string; cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; + isPopupAuthenticationRequest: boolean; }; // @public (undocumented) @@ -385,6 +388,7 @@ export type OAuthState = { env: string; origin?: string; scope?: string; + redirectUrl?: string; }; // @public @@ -667,6 +671,12 @@ export const providers: Readonly<{ // @public (undocumented) export const readState: (stateString: string) => OAuthState; +// @public (undocumented) +export const redirectMessageResponse: ( + res: express.Response, + redirectUrl: string, +) => void; + // @public (undocumented) export interface RouterOptions { // (undocumented) diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts index a0c437815d..868477506a 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts @@ -19,6 +19,7 @@ import { safelyEncodeURIComponent, ensuresXRequestedWith, postMessageResponse, + redirectMessageResponse, } from './authFlowHelpers'; import { WebMessageResponse } from './types'; @@ -178,6 +179,22 @@ describe('oauth helpers', () => { }); }); + describe('redirectMessageResponse', () => { + const redirectUrl = 'http://localhost:3000/catalog'; + it('should perform redirect', () => { + const mockResponse = { + end: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + redirect: jest.fn().mockReturnThis(), + } as unknown as express.Response; + + redirectMessageResponse(mockResponse, redirectUrl); + expect(mockResponse.redirect).toHaveBeenCalledTimes(1); + expect(mockResponse.end).not.toHaveBeenCalled(); + expect(mockResponse.setHeader).not.toHaveBeenCalled(); + }); + }); + describe('ensuresXRequestedWith', () => { it('should return false if no header present', () => { const mockRequest = { diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts index 94ac4fba15..08449fd923 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts @@ -69,6 +69,14 @@ export const postMessageResponse = ( res.end(``); }; +/** @public */ +export const redirectMessageResponse = ( + res: express.Response, + redirectUrl: string, +) => { + res.redirect(redirectUrl); +}; + /** @public */ export const ensuresXRequestedWith = (req: express.Request) => { const requiredHeader = req.header('X-Requested-With'); diff --git a/plugins/auth-backend/src/lib/flow/index.ts b/plugins/auth-backend/src/lib/flow/index.ts index f7b4491edb..ebc3fe2ebe 100644 --- a/plugins/auth-backend/src/lib/flow/index.ts +++ b/plugins/auth-backend/src/lib/flow/index.ts @@ -14,6 +14,10 @@ * limitations under the License. */ -export { ensuresXRequestedWith, postMessageResponse } from './authFlowHelpers'; +export { + ensuresXRequestedWith, + postMessageResponse, + redirectMessageResponse, +} from './authFlowHelpers'; export type { WebMessageResponse } from './types'; diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index e0d4f0e051..31d7e5aff5 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -74,6 +74,7 @@ describe('OAuthAdapter', () => { providerId: 'test-provider', appOrigin: 'http://localhost:3000', baseUrl: 'http://domain.org/auth', + isPopupAuthenticationRequest: true, cookieConfigurer: mockCookieConfigurer, tokenIssuer: { issueToken: async () => 'my-id-token', @@ -83,56 +84,10 @@ describe('OAuthAdapter', () => { callbackUrl: 'http://domain.org/auth/test-provider/handler/frame', }; - it('sets the correct headers in start', async () => { - const oauthProvider = new OAuthAdapter( - providerInstance, - oAuthProviderOptions, - ); - const mockRequest = { - query: { - scope: 'user', - env: 'development', - }, - } as unknown as express.Request; + const defaultState = { nonce: 'nonce', env: 'development' }; - 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); - // nonce cookie checks - expect(mockResponse.cookie).toHaveBeenCalledTimes(1); - expect(mockResponse.cookie).toHaveBeenCalledWith( - `${oAuthProviderOptions.providerId}-nonce`, - expect.any(String), - expect.objectContaining({ - httpOnly: true, - path: '/auth/test-provider/handler', - maxAge: TEN_MINUTES_MS, - domain: 'domain.org', - sameSite: 'lax', - secure: false, - }), - ); - // redirect checks - 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 OAuthAdapter(providerInstance, { - ...oAuthProviderOptions, - isOriginAllowed: () => false, - }); - - const state = { nonce: 'nonce', env: 'development' }; - const mockRequest = { + const createEncodedQueryMockRequest = (state: any) => { + return { cookies: { 'test-provider-nonce': 'nonce', }, @@ -140,12 +95,69 @@ describe('OAuthAdapter', () => { state: encodeState(state), }, } as unknown as express.Request; + }; - const mockResponse = { - cookie: jest.fn().mockReturnThis(), - setHeader: jest.fn().mockReturnThis(), - end: jest.fn().mockReturnThis(), - } as unknown as express.Response; + const mockResponse = { + cookie: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + statusCode: jest.fn().mockReturnThis(), + redirect: jest.fn().mockReturnThis(), + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + } as unknown as express.Response; + + const mockStartRequest = { + query: { + scope: 'user', + env: 'development', + }, + } as unknown as express.Request; + + const expectedStartAuthCookieData = { + httpOnly: true, + path: '/auth/test-provider/handler', + maxAge: TEN_MINUTES_MS, + domain: 'domain.org', + sameSite: 'lax', + secure: false, + }; + + it('sets the correct headers in start', async () => { + const oauthProvider = new OAuthAdapter( + providerInstance, + oAuthProviderOptions, + ); + + await oauthProvider.start(mockStartRequest, mockResponse); + // nonce cookie checks + expect(mockResponse.cookie).toHaveBeenCalledTimes(1); + expect(mockResponse.cookie).toHaveBeenCalledWith( + `${oAuthProviderOptions.providerId}-nonce`, + expect.any(String), + expect.objectContaining(expectedStartAuthCookieData), + ); + 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); + expect(mockResponse.redirect).not.toHaveBeenCalled(); + }); + + const refreshCookieData = { + ...expectedStartAuthCookieData, + path: '/auth/test-provider', + maxAge: THOUSAND_DAYS_MS, + }; + + it('sets the refresh cookie if refresh is enabled', async () => { + const oauthProvider = new OAuthAdapter(providerInstance, { + ...oAuthProviderOptions, + isOriginAllowed: () => false, + }); + + const mockRequest = createEncodedQueryMockRequest(defaultState); await oauthProvider.frameHandler(mockRequest, mockResponse); expect(mockCookieConfigurer).toHaveBeenCalledTimes(1); @@ -153,15 +165,22 @@ describe('OAuthAdapter', () => { expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), expect.stringContaining('token'), - expect.objectContaining({ - httpOnly: true, - path: '/auth/test-provider', - maxAge: THOUSAND_DAYS_MS, - domain: 'domain.org', - secure: false, - sameSite: 'lax', - }), + expect.objectContaining(refreshCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); + }); + + it('sets the refresh cookie if refresh is enabled with redirect', async () => { + const oauthProvider = new OAuthAdapter(providerInstance, { + ...oAuthProviderOptions, + isOriginAllowed: () => false, + isPopupAuthenticationRequest: false, + }); + + const mockRequest = createEncodedQueryMockRequest(defaultState); + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockResponse.redirect).toHaveBeenCalledTimes(1); }); it('persists scope through cookie if enabled', async () => { @@ -179,32 +198,15 @@ describe('OAuthAdapter', () => { }); // First we test the /start request, making sure state is set - const mockStartReq = { - query: { - scope: 'user', - env: 'development', - }, - } as unknown as express.Request; - const mockStartRes = { - cookie: jest.fn().mockReturnThis(), - end: jest.fn().mockReturnThis(), - setHeader: jest.fn().mockReturnThis(), - statusCode: jest.fn().mockReturnThis(), - } as unknown as express.Response; - - await oauthProvider.start(mockStartReq, mockStartRes); + await oauthProvider.start(mockStartRequest, mockResponse); expect(handlers.start).toHaveBeenCalledTimes(1); expect(handlers.start).toHaveBeenCalledWith({ - query: { - scope: 'user', - env: 'development', - }, + ...mockStartRequest, scope: 'user', state: { nonce: expect.any(String), env: 'development', - origin: undefined, scope: 'user', }, }); @@ -223,6 +225,7 @@ describe('OAuthAdapter', () => { cookie: jest.fn().mockReturnThis(), setHeader: jest.fn().mockReturnThis(), end: jest.fn().mockReturnThis(), + redirect: jest.fn().mockReturnThis(), } as unknown as express.Response; await oauthProvider.frameHandler(mockHandleReq, mockHandleRes); @@ -230,17 +233,11 @@ describe('OAuthAdapter', () => { expect(mockHandleRes.cookie).toHaveBeenCalledWith( 'test-provider-granted-scope', 'user', - expect.objectContaining({ - httpOnly: true, - path: '/auth/test-provider', - maxAge: THOUSAND_DAYS_MS, - domain: 'domain.org', - secure: false, - sameSite: 'lax', - }), + expect.objectContaining(refreshCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); - // Them make sure scopes are forwarded correctly during refresh + // Then make sure scopes are forwarded correctly during refresh const mockRefreshReq = { query: { scope: 'ignore-me' }, cookies: { @@ -252,6 +249,7 @@ describe('OAuthAdapter', () => { const mockRefreshRes = { status: jest.fn().mockReturnThis(), json: jest.fn().mockReturnThis(), + redirect: jest.fn().mockReturnThis(), } as unknown as express.Response; await oauthProvider.refresh(mockRefreshReq, mockRefreshRes); expect(handlers.refresh).toHaveBeenCalledTimes(1); @@ -261,8 +259,18 @@ describe('OAuthAdapter', () => { refreshToken: 'refresh-token', }), ); + expect(mockRefreshRes.redirect).not.toHaveBeenCalled(); }); + const mockRequestWithHeader = { + header: () => 'XMLHttpRequest', + cookies: { + 'test-provider-refresh-token': 'token', + }, + query: {}, + get: jest.fn(), + } as unknown as express.Request; + it('removes refresh cookie and calls logout handler when logging out', async () => { const logoutSpy = jest.spyOn(providerInstance, 'logout'); const oauthProvider = new OAuthAdapter(providerInstance, { @@ -270,22 +278,8 @@ describe('OAuthAdapter', () => { isOriginAllowed: () => false, }); - const mockRequest = { - cookies: { - 'test-provider-refresh-token': 'token', - }, - header: () => 'XMLHttpRequest', - get: jest.fn(), - } as unknown as express.Request; - - const mockResponse = { - cookie: jest.fn().mockReturnThis(), - end: jest.fn().mockReturnThis(), - status: jest.fn().mockReturnThis(), - } as unknown as express.Response; - - await oauthProvider.logout(mockRequest, mockResponse); - expect(mockRequest.get).toHaveBeenCalledTimes(1); + await oauthProvider.logout(mockRequestWithHeader, mockResponse); + expect(mockRequestWithHeader.get).toHaveBeenCalledTimes(1); expect(logoutSpy).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( @@ -294,6 +288,7 @@ describe('OAuthAdapter', () => { expect.objectContaining({ path: '/auth/test-provider' }), ); expect(mockResponse.end).toHaveBeenCalledTimes(1); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); it('gets new access-token when refreshing', async () => { @@ -302,20 +297,7 @@ describe('OAuthAdapter', () => { isOriginAllowed: () => false, }); - const mockRequest = { - header: () => 'XMLHttpRequest', - cookies: { - 'test-provider-refresh-token': 'token', - }, - query: {}, - } as unknown as express.Request; - - const mockResponse = { - json: jest.fn().mockReturnThis(), - status: jest.fn().mockReturnThis(), - } as unknown as express.Response; - - await oauthProvider.refresh(mockRequest, mockResponse); + await oauthProvider.refresh(mockRequestWithHeader, mockResponse); expect(mockResponse.json).toHaveBeenCalledTimes(1); expect(mockResponse.json).toHaveBeenCalledWith({ ...mockResponseData, @@ -328,6 +310,7 @@ describe('OAuthAdapter', () => { }, }, }); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); it('sets new access-token when old cookie exists', async () => { @@ -337,20 +320,12 @@ describe('OAuthAdapter', () => { }); const mockRequest = { - header: () => 'XMLHttpRequest', + ...mockRequestWithHeader, cookies: { 'test-provider-refresh-token': 'old-token', }, - query: {}, - get: jest.fn(), } as unknown as express.Request; - const mockResponse = { - json: jest.fn().mockReturnThis(), - status: jest.fn().mockReturnThis(), - cookie: jest.fn().mockReturnThis(), - } as unknown as express.Response; - await oauthProvider.refresh(mockRequest, mockResponse); expect(mockRequest.get).toHaveBeenCalledTimes(1); expect(mockCookieConfigurer).toHaveBeenCalledTimes(1); @@ -358,15 +333,9 @@ describe('OAuthAdapter', () => { expect(mockResponse.cookie).toHaveBeenCalledWith( 'test-provider-refresh-token', 'token', - expect.objectContaining({ - httpOnly: true, - path: '/auth/test-provider', - maxAge: THOUSAND_DAYS_MS, - domain: 'domain.org', - secure: false, - sameSite: 'lax', - }), + expect.objectContaining(refreshCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); it('sets the correct nonce cookie configuration', async () => { @@ -374,168 +343,97 @@ describe('OAuthAdapter', () => { baseUrl: 'http://domain.org/auth', appUrl: 'http://domain.org', isOriginAllowed: () => false, + isPopupAuthenticationRequest: true, }; const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { ...oAuthProviderOptions, }); - const mockRequest = { - query: { - scope: 'user', - env: 'development', - origin: 'http://domain.org', - }, - } 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); + await oauthProvider.start(mockStartRequest, mockResponse); expect(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( `${oAuthProviderOptions.providerId}-nonce`, expect.any(String), - expect.objectContaining({ - httpOnly: true, - domain: 'domain.org', - maxAge: TEN_MINUTES_MS, - path: '/auth/test-provider/handler', - secure: false, - sameSite: 'lax', - }), + expect.objectContaining(expectedStartAuthCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); - it('sets the correct nonce cookie configuration using origin from request', async () => { - const config = { - baseUrl: 'http://domain.org/auth', - appUrl: 'http://domain.org', - isOriginAllowed: () => false, - }; + const config = { + baseUrl: 'http://domain.org/auth', + appUrl: 'http://domain.org', + isOriginAllowed: () => false, + isPopupAuthenticationRequest: true, + }; + const mockStartRequestWithOrigin = { + query: { + scope: 'user', + env: 'development', + origin: 'http://other.domain', + }, + } as unknown as express.Request; + + it('sets the correct nonce cookie configuration using origin from request', async () => { const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { ...oAuthProviderOptions, callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', }); - const mockRequest = { - query: { - scope: 'user', - env: 'development', - origin: 'http://other.domain', - }, - } 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); + await oauthProvider.start(mockStartRequestWithOrigin, mockResponse); expect(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( `${oAuthProviderOptions.providerId}-nonce`, expect.any(String), expect.objectContaining({ - httpOnly: true, - domain: 'domain.org', - maxAge: TEN_MINUTES_MS, - path: '/auth/test-provider/handler', + ...expectedStartAuthCookieData, secure: true, sameSite: 'none', }), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); - it('sets the correct cookie configuration using an secure callbackUrl', async () => { - const config = { - baseUrl: 'https://domain.org/auth', - appUrl: 'http://domain.org', - isOriginAllowed: () => false, - }; + const secureCookieData = { + ...refreshCookieData, + secure: true, + sameSite: 'lax', + maxAge: THOUSAND_DAYS_MS, + }; + it('sets the correct cookie configuration using an secure callbackUrl', async () => { const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { ...oAuthProviderOptions, callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', }); - const state = { - nonce: 'nonce', - env: 'development', - }; - - const mockRequest = { - cookies: { - 'test-provider-nonce': 'nonce', - }, - query: { - state: encodeState(state), - }, - } as unknown as express.Request; - - const mockResponse = { - cookie: jest.fn().mockReturnThis(), - setHeader: jest.fn().mockReturnThis(), - end: jest.fn().mockReturnThis(), - } as unknown as express.Response; - + const mockRequest = createEncodedQueryMockRequest(defaultState); await oauthProvider.frameHandler(mockRequest, mockResponse); expect(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), expect.stringContaining('token'), - expect.objectContaining({ - httpOnly: true, - domain: 'domain.org', - path: '/auth/test-provider', - secure: true, - sameSite: 'lax', - }), + expect.objectContaining(secureCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); - it('sets the correct cookie configuration when on different domains and secure', async () => { - const config = { - baseUrl: 'https://domain.org/auth', - appUrl: 'http://domain.org', - isOriginAllowed: () => false, - }; + const secureSameSiteNoneCookieData = { + ...secureCookieData, + sameSite: 'none', + }; + it('sets the correct cookie configuration when on different domains and secure', async () => { const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { ...oAuthProviderOptions, callbackUrl: 'https://authdomain.org/auth/test-provider/handler/frame', }); - const state = { - nonce: 'nonce', - env: 'development', - }; - - const mockRequest = { - cookies: { - 'test-provider-nonce': 'nonce', - }, - query: { - state: encodeState(state), - }, - } as unknown as express.Request; - - const mockResponse = { - cookie: jest.fn().mockReturnThis(), - setHeader: jest.fn().mockReturnThis(), - end: jest.fn().mockReturnThis(), - } as unknown as express.Response; - + const mockRequest = createEncodedQueryMockRequest(defaultState); await oauthProvider.frameHandler(mockRequest, mockResponse); expect(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); @@ -543,106 +441,109 @@ describe('OAuthAdapter', () => { expect.stringContaining('test-provider-refresh-token'), expect.stringContaining('token'), expect.objectContaining({ - httpOnly: true, + ...secureSameSiteNoneCookieData, domain: 'authdomain.org', - path: '/auth/test-provider', - secure: true, - sameSite: 'none', }), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); + const configOriginAllowed = { + ...config, + isOriginAllowed: () => true, + }; + it('sets the correct cookie configuration using origin from state', async () => { - const config = { - baseUrl: 'https://domain.org/auth', - appUrl: 'http://domain.org', - isOriginAllowed: () => true, - }; + const oauthProvider = OAuthAdapter.fromConfig( + configOriginAllowed, + providerInstance, + { + ...oAuthProviderOptions, + callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', + }, + ); - const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { - ...oAuthProviderOptions, - callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', - }); - - const state = { - nonce: 'nonce', - env: 'development', + const mockRequest = createEncodedQueryMockRequest({ + ...defaultState, origin: 'http://other.domain', - }; - - const mockRequest = { - cookies: { - 'test-provider-nonce': 'nonce', - }, - query: { - state: encodeState(state), - }, - } 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(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), expect.stringContaining('token'), - expect.objectContaining({ - httpOnly: true, - domain: 'domain.org', - path: '/auth/test-provider', - secure: true, - sameSite: 'none', - }), + expect.objectContaining(secureSameSiteNoneCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); - it('sets the correct cookie configuration using origin from header', async () => { - const config = { - baseUrl: 'https://domain.org/auth', - appUrl: 'http://domain.org', - isOriginAllowed: () => false, - }; + const mockRequestWithGetMockReturn = { + header: () => 'XMLHttpRequest', + cookies: { + 'test-provider-refresh-token': 'old-token', + }, + query: {}, + get: jest.fn().mockReturnValue('http://other.domain'), + } as unknown as express.Request; + it('sets the correct cookie configuration using origin from header', async () => { const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { ...oAuthProviderOptions, callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', }); - const mockRequest = { - header: () => 'XMLHttpRequest', - cookies: { - 'test-provider-refresh-token': 'old-token', - }, - query: {}, - get: jest.fn().mockReturnValue('http://other.domain'), - } as unknown as express.Request; - - const mockResponse = { - json: jest.fn().mockReturnThis(), - status: jest.fn().mockReturnThis(), - cookie: jest.fn().mockReturnThis(), - } as unknown as express.Response; - - await oauthProvider.refresh(mockRequest, mockResponse); - expect(mockRequest.get).toHaveBeenCalledTimes(1); + await oauthProvider.refresh(mockRequestWithGetMockReturn, mockResponse); + expect(mockRequestWithGetMockReturn.get).toHaveBeenCalledTimes(1); expect(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( 'test-provider-refresh-token', 'token', - expect.objectContaining({ - httpOnly: true, - path: '/auth/test-provider', - maxAge: THOUSAND_DAYS_MS, - domain: 'domain.org', - secure: true, - sameSite: 'none', - }), + expect.objectContaining(secureSameSiteNoneCookieData), ); + expect(mockResponse.redirect).not.toHaveBeenCalled(); + }); + + it('executed a response redirect when isPopupAuthenticationRequest is false', async () => { + const handlers = { + start: jest.fn(async (_req: { state: OAuthState }) => ({ + url: '/url', + status: 301, + })), + handler: jest.fn(async () => ({ response: mockResponseData })), + refresh: jest.fn(async () => ({ response: mockResponseData })), + }; + const configWithNoPopupEnabled = { + ...configOriginAllowed, + isPopupAuthenticationRequest: false, + }; + const oauthProvider = OAuthAdapter.fromConfig( + configWithNoPopupEnabled, + handlers, + { + ...oAuthProviderOptions, + callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', + }, + ); + + const state = { + ...defaultState, + origin: 'http://other.domain', + redirectUrl: 'http://domain.org', + }; + + const mockRequest = { + ...createEncodedQueryMockRequest(state), + get: jest.fn().mockReturnValue('http://other.domain'), + } as unknown as express.Request; + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockRequest.get).not.toHaveBeenCalled(); + expect(mockResponse.end).not.toHaveBeenCalled(); + expect(mockCookieConfigurer).not.toHaveBeenCalled(); + expect(mockResponse.cookie).not.toHaveBeenCalled(); + expect(mockResponse.redirect).toHaveBeenCalledTimes(1); + expect(mockResponse.redirect).toHaveBeenCalledWith('http://domain.org'); }); }); diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 85104e9608..239e660298 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -33,7 +33,12 @@ import { NotAllowedError, } from '@backstage/errors'; import { defaultCookieConfigurer, readState, verifyNonce } from './helpers'; -import { postMessageResponse, ensuresXRequestedWith } from '../flow'; +import { + postMessageResponse, + redirectMessageResponse, + ensuresXRequestedWith, + WebMessageResponse, +} from '../flow'; import { OAuthHandlers, OAuthStartRequest, @@ -51,10 +56,12 @@ export type OAuthAdapterOptions = { providerId: string; persistScopes?: boolean; appOrigin: string; + redirectUrl?: string; baseUrl: string; cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; + isPopupAuthenticationRequest: boolean; }; /** @public */ @@ -64,10 +71,11 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { handlers: OAuthHandlers, options: Pick< OAuthAdapterOptions, - 'providerId' | 'persistScopes' | 'callbackUrl' + 'providerId' | 'persistScopes' | 'callbackUrl' | 'redirectUrl' >, ): OAuthAdapter { - const { appUrl, baseUrl, isOriginAllowed } = config; + const { appUrl, baseUrl, isOriginAllowed, isPopupAuthenticationRequest } = + config; const { origin: appOrigin } = new URL(appUrl); const cookieConfigurer = config.cookieConfigurer ?? defaultCookieConfigurer; @@ -78,6 +86,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { baseUrl, cookieConfigurer, isOriginAllowed, + isPopupAuthenticationRequest: isPopupAuthenticationRequest, }); } @@ -98,7 +107,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const scope = req.query.scope?.toString() ?? ''; const env = req.query.env?.toString(); const origin = req.query.origin?.toString(); - + const redirectUrl = req.query.redirectUrl?.toString(); if (!env) { throw new InputError('No env provided in request query parameters'); } @@ -109,7 +118,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { // set a nonce cookie before redirecting to oauth provider this.setNonceCookie(res, nonce, cookieConfig); - const state: OAuthState = { nonce, env, origin }; + const state: OAuthState = { nonce, env, origin, redirectUrl }; // If scopes are persisted then we pass them through the state so that we // can set the cookie on successful auth @@ -136,6 +145,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { try { const state: OAuthState = readState(req.query.state?.toString() ?? ''); + const redirectUrl = state.redirectUrl ?? ''; if (state.origin) { try { @@ -169,11 +179,16 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const identity = await this.populateIdentity(response.backstageIdentity); - // post message back to popup if successful - return postMessageResponse(res, appOrigin, { + const responseObj: WebMessageResponse = { type: 'authorization_response', response: { ...response, backstageIdentity: identity }, - }); + }; + + if (!this.options.isPopupAuthenticationRequest) { + return redirectMessageResponse(res, redirectUrl); + } + // post message back to popup if successful + return postMessageResponse(res, appOrigin, responseObj); } catch (error) { const { name, message } = isError(error) ? error diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index 3902c3f026..2b06e1f78b 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -90,6 +90,7 @@ export type OAuthState = { env: string; origin?: string; scope?: string; + redirectUrl?: string; }; /** @public */ diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 1459cff484..f70b639de9 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -131,6 +131,8 @@ export type AuthProviderConfig = { * The function used to resolve cookie configuration based on the auth provider options. */ cookieConfigurer?: CookieConfigurer; + + isPopupAuthenticationRequest: boolean; }; /** @public */ diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 8013777415..f19eb54a72 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -107,6 +107,9 @@ export async function createRouter( ...providerFactories, }; const providersConfig = config.getConfig('auth.providers'); + const isPopupAuthenticationRequest = + config.getOptionalBoolean('auth.usePopup') ?? true; + const configuredProviders = providersConfig.keys(); const isOriginAllowed = createOriginFilter(config); @@ -123,6 +126,7 @@ export async function createRouter( baseUrl: authUrl, appUrl, isOriginAllowed, + isPopupAuthenticationRequest: isPopupAuthenticationRequest, }, config: providersConfig.getConfig(providerId), logger,