From 9a9170047b628c59f1c7737e7c650465fbb35778 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Wed, 18 Jan 2023 14:32:00 -0800 Subject: [PATCH] Implementation for oauth2 authentication using a redirect flow (without window popup) from frontend to backend API, followed by a redirect back to the fronted. A localstorage provider token is added before the redirect to the backend auth API. During the subsequent front end refresh an asynchronous backstage session refresh is triggered when the provider token is put in localstorage. The session refresh will return a backstage authentication token if authentication succeeded during the previous backend auth API execution. Addresses https://github.com/backstage/backstage/issues/9582 Signed-off-by: headphonejames --- app-config.yaml | 1 + packages/app-defaults/src/defaults/apis.ts | 8 + packages/core-app-api/api-report.md | 2 + packages/core-app-api/config.d.ts | 6 + .../auth/atlassian/AtlassianAuth.ts | 2 + .../auth/bitbucket/BitbucketAuth.ts | 3 + .../implementations/auth/github/GithubAuth.ts | 3 + .../implementations/auth/gitlab/GitlabAuth.ts | 3 + .../implementations/auth/google/GoogleAuth.ts | 3 + .../auth/microsoft/MicrosoftAuth.ts | 3 + .../implementations/auth/oauth2/OAuth2.ts | 2 + .../implementations/auth/okta/OktaAuth.ts | 3 + .../auth/onelogin/OneLoginAuth.ts | 4 + .../src/apis/implementations/auth/types.ts | 1 + .../DefaultAuthConnector.test.ts | 35 ++ .../lib/AuthConnector/DefaultAuthConnector.ts | 31 +- .../src/lib/AuthConnector/types.ts | 1 + .../RefreshingAuthSessionManager.ts | 2 +- .../OAuthRequestDialog/OAuthRequestDialog.tsx | 13 +- .../src/layout/SignInPage/commonProvider.tsx | 9 +- packages/core-plugin-api/api-report.md | 1 + .../src/apis/definitions/auth.ts | 6 +- plugins/auth-backend/api-report.md | 12 +- .../src/lib/flow/authFlowHelpers.test.ts | 17 + .../src/lib/flow/authFlowHelpers.ts | 8 + plugins/auth-backend/src/lib/flow/index.ts | 6 +- .../src/lib/oauth/OAuthAdapter.test.ts | 537 +++++++----------- .../src/lib/oauth/OAuthAdapter.ts | 31 +- plugins/auth-backend/src/lib/oauth/types.ts | 1 + plugins/auth-backend/src/providers/types.ts | 2 + plugins/auth-backend/src/service/router.ts | 4 + 31 files changed, 426 insertions(+), 334 deletions(-) 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,