From 9a9170047b628c59f1c7737e7c650465fbb35778 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Wed, 18 Jan 2023 14:32:00 -0800 Subject: [PATCH 01/15] 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, From ae4d826fb215e7a6b2038cccf90970027a073bf6 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Fri, 3 Feb 2023 12:03:01 -0800 Subject: [PATCH 02/15] Updates to redirect implementation after initial PR review. Signed-off-by: headphonejames --- app-config.yaml | 2 +- packages/app-defaults/src/defaults/apis.ts | 16 +++--- packages/core-app-api/config.d.ts | 6 +- .../OAuthRequestApi/OAuthRequestManager.ts | 6 ++ .../auth/atlassian/AtlassianAuth.ts | 4 +- .../auth/bitbucket/BitbucketAuth.ts | 5 +- .../implementations/auth/github/GithubAuth.ts | 5 +- .../implementations/auth/gitlab/GitlabAuth.ts | 5 +- .../implementations/auth/google/GoogleAuth.ts | 5 +- .../auth/microsoft/MicrosoftAuth.ts | 5 +- .../implementations/auth/oauth2/OAuth2.ts | 4 +- .../implementations/auth/okta/OktaAuth.ts | 5 +- .../auth/onelogin/OneLoginAuth.ts | 7 +-- .../src/apis/implementations/auth/types.ts | 2 +- .../DefaultAuthConnector.test.ts | 40 +------------- .../lib/AuthConnector/DefaultAuthConnector.ts | 55 +++++++++---------- .../src/lib/AuthConnector/types.ts | 1 - .../OAuthRequestDialog/OAuthRequestDialog.tsx | 15 ++--- .../src/layout/SignInPage/commonProvider.tsx | 15 +++-- .../src/layout/SignInPage/providers.tsx | 7 +-- .../src/apis/definitions/OAuthRequestApi.ts | 10 ++++ .../src/apis/definitions/auth.ts | 5 -- .../src/lib/flow/authFlowHelpers.test.ts | 17 ------ .../src/lib/flow/authFlowHelpers.ts | 8 --- .../src/lib/oauth/OAuthAdapter.test.ts | 16 +++--- .../src/lib/oauth/OAuthAdapter.ts | 13 ++--- plugins/auth-backend/src/lib/oauth/types.ts | 1 + plugins/auth-backend/src/providers/types.ts | 2 - plugins/auth-backend/src/service/router.ts | 3 - 29 files changed, 107 insertions(+), 178 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 1d3bf31083..350b9068e3 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -293,7 +293,7 @@ auth: # path: my-sessions environment: development - usePopup: false + authFlow: redirect ### 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 9e71855714..feb4cd6cc8 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -131,7 +131,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -146,7 +146,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -162,7 +162,7 @@ export const apis = [ oauthRequestApi, defaultScopes: ['read:user'], environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -177,7 +177,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -192,7 +192,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -207,7 +207,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -223,7 +223,7 @@ export const apis = [ oauthRequestApi, defaultScopes: ['team'], environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -238,7 +238,7 @@ export const apis = [ discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - usePopup: configApi.getOptionalBoolean('auth.usePopup'), + authFlow: configApi.getOptionalString('auth.authFlow'), }); }, }), diff --git a/packages/core-app-api/config.d.ts b/packages/core-app-api/config.d.ts index 0dd73369f6..c499dfa7c6 100644 --- a/packages/core-app-api/config.d.ts +++ b/packages/core-app-api/config.d.ts @@ -115,10 +115,10 @@ export interface Config { */ environment?: string; /** - $ The authentication flow type - either using a popup to authenticate or perform a http redirect - * default value: 'true' + $ The authentication flow type - currently supports 'popup' and 'redirect' + * default value: 'popup' * @visibility frontend */ - usePopup?: boolean; + authFlow?: string; }; } diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts index 5d467be7b4..751f2bf24c 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts @@ -37,8 +37,10 @@ export class OAuthRequestManager implements OAuthRequestApi { private readonly subject = new BehaviorSubject([]); private currentRequests: PendingOAuthRequest[] = []; private handlerCount = 0; + private authFlowStr: string = 'popup'; createAuthRequester(options: OAuthRequesterOptions): OAuthRequester { + this.authFlowStr = options.authFlow; const handler = new OAuthPendingRequests(); const index = this.handlerCount; @@ -91,4 +93,8 @@ export class OAuthRequestManager implements OAuthRequestApi { authRequest$(): Observable { return this.subject; } + + authFlow(): string { + return this.authFlowStr; + } } 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 5582a29c00..df714e2290 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,7 +34,7 @@ export default class AtlassianAuth { const { discoveryApi, environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, } = options; @@ -44,7 +44,7 @@ export default class AtlassianAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow, }); } } 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 1d27eb1aa3..3bdb09b010 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,7 +36,6 @@ export type BitbucketAuthResponse = { const DEFAULT_PROVIDER = { id: 'bitbucket', title: 'Bitbucket', - provider_id: 'bitbucket-auth-provider', icon: () => null, }; @@ -50,7 +49,7 @@ export default class BitbucketAuth { const { discoveryApi, environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['team'], @@ -61,7 +60,7 @@ export default class BitbucketAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow, 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 11af9995a1..5acdd20719 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,7 +21,6 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'github', title: 'GitHub', - provider_id: 'github-auth-provider', icon: () => null, }; @@ -38,7 +37,7 @@ export default class GithubAuth { provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['read:user'], - usePopup = true, + authFlow = 'popup', } = options; return OAuth2.create({ @@ -47,7 +46,7 @@ export default class GithubAuth { provider, environment, defaultScopes, - usePopup, + authFlow, }); } } 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 965d0431ac..919d67dd8d 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,7 +21,6 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'gitlab', title: 'GitLab', - provider_id: 'gitlab-auth-provider', icon: () => null, }; @@ -35,7 +34,7 @@ export default class GitlabAuth { const { discoveryApi, environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['read_user'], @@ -46,7 +45,7 @@ export default class GitlabAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow, 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 514b257ada..e8adefb08f 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,7 +21,6 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'google', title: 'Google', - provider_id: 'google-auth-provider', icon: () => null, }; @@ -38,7 +37,7 @@ export default class GoogleAuth { discoveryApi, oauthRequestApi, environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, defaultScopes = [ 'openid', @@ -52,7 +51,7 @@ export default class GoogleAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow, 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 e988a79268..c28e115950 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,7 +21,6 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'microsoft', title: 'Microsoft', - provider_id: 'microsoft-auth-provider', icon: () => null, }; @@ -34,7 +33,7 @@ export default class MicrosoftAuth { static create(options: OAuthApiCreateOptions): typeof microsoftAuthApiRef.T { const { environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, discoveryApi, @@ -52,7 +51,7 @@ export default class MicrosoftAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow, 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 a1d0c7d3a9..7a133154f2 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,7 +77,7 @@ export default class OAuth2 provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = [], - usePopup = true, + authFlow = 'popup', scopeTransform = x => x, } = options; @@ -86,7 +86,7 @@ export default class OAuth2 environment, provider, oauthRequestApi: oauthRequestApi, - usePopup, + authFlow, 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 db574e7fb2..9075d8cb82 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,7 +21,6 @@ import { OAuthApiCreateOptions } from '../types'; const DEFAULT_PROVIDER = { id: 'okta', title: 'Okta', - provider_id: 'okta-auth-provider', icon: () => null, }; @@ -47,7 +46,7 @@ export default class OktaAuth { const { discoveryApi, environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['openid', 'email', 'profile', 'offline_access'], @@ -58,7 +57,7 @@ export default class OktaAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow, 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 d767cb6b55..edfcbc4463 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,14 +30,13 @@ export type OneLoginAuthCreateOptions = { discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; - usePopup?: boolean; + authFlow?: string; provider?: AuthProviderInfo; }; const DEFAULT_PROVIDER = { id: 'onelogin', title: 'onelogin', - provider_id: 'onelogin-auth-provider', icon: () => null, }; @@ -65,7 +64,7 @@ export default class OneLoginAuth { const { discoveryApi, environment = 'development', - usePopup = true, + authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, } = options; @@ -75,7 +74,7 @@ export default class OneLoginAuth { oauthRequestApi, provider, environment, - usePopup, + authFlow: authFlow, 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 e29fe0c7d2..fdb2bbc7f8 100644 --- a/packages/core-app-api/src/apis/implementations/auth/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/types.ts @@ -37,5 +37,5 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; - usePopup?: boolean; + authFlow?: string; }; 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 113ee95bd0..fd2a2833a8 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -34,16 +34,15 @@ 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, scopes: new Set(res.scopes.split(' ')), expiresAt: new Date(Date.now() + expiresInSeconds * 1000), }), + authFlow: 'popup', }; describe('DefaultAuthConnector', () => { @@ -133,7 +132,7 @@ describe('DefaultAuthConnector', () => { expect(popupSpy).toHaveBeenCalledTimes(1); expect(popupSpy.mock.calls[0][0]).toMatchObject({ - url: 'http://my-host/api/auth/my-provider/start?scope=a%20b&origin=http%3A%2F%2Flocalhost&env=production', + url: 'http://my-host/api/auth/my-provider/start?scope=a%20b&origin=http%3A%2F%2Flocalhost&authFlow=popup&env=production', }); await expect(sessionPromise).resolves.toEqual({ @@ -181,40 +180,7 @@ describe('DefaultAuthConnector', () => { expect(popupSpy).toHaveBeenCalledTimes(1); expect(popupSpy.mock.calls[0][0]).toMatchObject({ - url: 'http://my-host/api/auth/my-provider/start?scope=-ab-&origin=http%3A%2F%2Flocalhost&env=production', + url: 'http://my-host/api/auth/my-provider/start?scope=-ab-&origin=http%3A%2F%2Flocalhost&authFlow=popup&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 acc6b97b2c..7cada8b0f9 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -32,10 +32,6 @@ 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'. @@ -53,6 +49,10 @@ type Options = { * Function used to transform an auth response into the session type. */ sessionTransform?(response: any): AuthSession | Promise; + /** + * The UI authentication flow with backend authentication api. Supports either 'popup' or 'redirect'. + */ + authFlow: string; }; function defaultJoinScopes(scopes: Set) { @@ -73,6 +73,7 @@ export class DefaultAuthConnector private readonly joinScopesFunc: (scopes: Set) => string; private readonly authRequester: OAuthRequester; private readonly sessionTransform: (response: any) => Promise; + private readonly authFlow: string; constructor(options: Options) { const { @@ -81,39 +82,22 @@ export class DefaultAuthConnector provider, joinScopes = defaultJoinScopes, oauthRequestApi, - usePopup, + authFlow, sessionTransform = id => id, } = options; this.authRequester = oauthRequestApi.createAuthRequester({ provider, 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); + if (authFlow === 'popup') { + return this.showPopup(scopes); } - return this.showPopup(scopes); + return this.executeRedirect(scopes); }, + authFlow, }); + this.authFlow = authFlow; this.discoveryApi = discoveryApi; this.environment = environment; this.provider = provider; @@ -122,7 +106,7 @@ export class DefaultAuthConnector } async createSession(options: CreateSessionOptions): Promise { - if (options.instantPopup) { + if (options.instantPopup && this.authFlow === 'popup') { return this.showPopup(options.scopes); } return this.authRequester(options.scopes); @@ -184,6 +168,7 @@ export class DefaultAuthConnector const popupUrl = await this.buildUrl('/start', { scope, origin: window.location.origin, + authFlow: 'popup', }); const payload = await showLoginPopup({ @@ -197,6 +182,20 @@ export class DefaultAuthConnector return await this.sessionTransform(payload); } + private async executeRedirect(scopes: Set): Promise { + const scope = this.joinScopesFunc(scopes); + const redirectUrl = await this.buildUrl('/start', { + scope, + origin: window.location.origin, + redirectUrl: window.location.href, + authFlow: 'redirect', + }); + // redirect to auth api + window.location.href = redirectUrl; + // return a promise that never resolves + return new Promise(() => {}); + } + private async buildUrl( path: string, query?: { [key: string]: string | boolean | undefined }, diff --git a/packages/core-app-api/src/lib/AuthConnector/types.ts b/packages/core-app-api/src/lib/AuthConnector/types.ts index 7c4f19184e..464a2ed627 100644 --- a/packages/core-app-api/src/lib/AuthConnector/types.ts +++ b/packages/core-app-api/src/lib/AuthConnector/types.ts @@ -17,7 +17,6 @@ export type CreateSessionOptions = { scopes: Set; instantPopup?: boolean; - redirectUrl?: string; }; /** diff --git a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx index 1884e6e358..e68120298a 100644 --- a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx +++ b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx @@ -24,11 +24,7 @@ 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, - configApiRef, -} from '@backstage/core-plugin-api'; +import { useApi, oauthRequestApiRef } from '@backstage/core-plugin-api'; import Typography from '@material-ui/core/Typography'; export type OAuthRequestDialogClassKey = @@ -62,11 +58,10 @@ 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 redirectMessage = + oauthRequestApi.authFlow() === 'popup' + ? '' + : 'This will trigger a http redirect to OAuth Login.'; const requests = useObservable( useMemo(() => oauthRequestApi.authRequest$(), [oauthRequestApi]), diff --git a/packages/core-components/src/layout/SignInPage/commonProvider.tsx b/packages/core-components/src/layout/SignInPage/commonProvider.tsx index aa458692dd..6796e3f22d 100644 --- a/packages/core-components/src/layout/SignInPage/commonProvider.tsx +++ b/packages/core-components/src/layout/SignInPage/commonProvider.tsx @@ -24,27 +24,25 @@ import { SignInProvider, SignInProviderConfig, } from './types'; -import { useApi, errorApiRef, configApiRef } from '@backstage/core-plugin-api'; +import { useApi, errorApiRef } from '@backstage/core-plugin-api'; import { GridItem } from './styles'; import { ForwardedError } from '@backstage/errors'; import { UserIdentity } from './UserIdentity'; +import { PROVIDER_STORAGE_KEY } from './providers'; const Component: ProviderComponent = ({ config, onSignInSuccess }) => { - const { apiRef, title, message } = config as SignInProviderConfig; + const { apiRef, title, message, id } = 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 { + localStorage.setItem(PROVIDER_STORAGE_KEY, id); const identityResponse = await authApi.getBackstageIdentity({ - instantPopup: usePopup, + instantPopup: true, }); if (!identityResponse) { - if (!usePopup) { - return; - } + localStorage.removeItem(PROVIDER_STORAGE_KEY); throw new Error( `The ${title} provider is not configured to support sign-in`, ); @@ -60,6 +58,7 @@ const Component: ProviderComponent = ({ config, onSignInSuccess }) => { }), ); } catch (error) { + localStorage.removeItem(PROVIDER_STORAGE_KEY); errorApi.post(new ForwardedError('Login failed', error)); } }; diff --git a/packages/core-components/src/layout/SignInPage/providers.tsx b/packages/core-components/src/layout/SignInPage/providers.tsx index 23d6c95b3c..716befda17 100644 --- a/packages/core-components/src/layout/SignInPage/providers.tsx +++ b/packages/core-components/src/layout/SignInPage/providers.tsx @@ -32,7 +32,7 @@ import { guestProvider } from './guestProvider'; import { customProvider } from './customProvider'; import { IdentityApiSignOutProxy } from './IdentityApiSignOutProxy'; -const PROVIDER_STORAGE_KEY = '@backstage/core:SignInPage:provider'; +export const PROVIDER_STORAGE_KEY = '@backstage/core:SignInPage:provider'; export type SignInProviderType = { [key: string]: { @@ -134,6 +134,7 @@ export const useSignInProviders = ( .loader(apiHolder, provider.config?.apiRef!) .then(result => { if (didCancel) { + localStorage.removeItem(PROVIDER_STORAGE_KEY); return; } if (result) { @@ -143,10 +144,10 @@ export const useSignInProviders = ( } }) .catch(error => { + localStorage.removeItem(PROVIDER_STORAGE_KEY); if (didCancel) { return; } - localStorage.removeItem(PROVIDER_STORAGE_KEY); errorApi.post(error); setLoading(false); }); @@ -172,8 +173,6 @@ export const useSignInProviders = ( const { Component } = provider.components; const handleSignInSuccess = (result: IdentityApi) => { - localStorage.setItem(PROVIDER_STORAGE_KEY, provider.id); - handleWrappedResult(result); }; diff --git a/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts b/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts index 75bf3a3864..2027852de7 100644 --- a/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts @@ -35,6 +35,11 @@ export type OAuthRequesterOptions = { * trigger() is called on an auth requests. */ onAuthRequest(scopes: Set): Promise; + + /** + * The authentication flow type + */ + authFlow: string; }; /** @@ -119,6 +124,11 @@ export type OAuthRequestApi = { * AuthRequester calls will resolve to the value returned by the onAuthRequest call. */ authRequest$(): Observable; + + /** + * The authentication flow type + */ + authFlow(): string; }; /** diff --git a/packages/core-plugin-api/src/apis/definitions/auth.ts b/packages/core-plugin-api/src/apis/definitions/auth.ts index 6ae19d53a6..3ab8c98fee 100644 --- a/packages/core-plugin-api/src/apis/definitions/auth.ts +++ b/packages/core-plugin-api/src/apis/definitions/auth.ts @@ -54,11 +54,6 @@ 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; }; /** diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts index 868477506a..a0c437815d 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts @@ -19,7 +19,6 @@ import { safelyEncodeURIComponent, ensuresXRequestedWith, postMessageResponse, - redirectMessageResponse, } from './authFlowHelpers'; import { WebMessageResponse } from './types'; @@ -179,22 +178,6 @@ 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 08449fd923..94ac4fba15 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts @@ -69,14 +69,6 @@ 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/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index 31d7e5aff5..dd45ab5441 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -74,7 +74,6 @@ describe('OAuthAdapter', () => { providerId: 'test-provider', appOrigin: 'http://localhost:3000', baseUrl: 'http://domain.org/auth', - isPopupAuthenticationRequest: true, cookieConfigurer: mockCookieConfigurer, tokenIssuer: { issueToken: async () => 'my-id-token', @@ -174,10 +173,14 @@ describe('OAuthAdapter', () => { const oauthProvider = new OAuthAdapter(providerInstance, { ...oAuthProviderOptions, isOriginAllowed: () => false, - isPopupAuthenticationRequest: false, }); - const mockRequest = createEncodedQueryMockRequest(defaultState); + const state = { + ...defaultState, + redirectUrl: 'http://localhost:3000', + authFlow: 'redirect', + }; + const mockRequest = createEncodedQueryMockRequest(state); await oauthProvider.frameHandler(mockRequest, mockResponse); expect(mockResponse.redirect).toHaveBeenCalledTimes(1); @@ -343,7 +346,6 @@ describe('OAuthAdapter', () => { baseUrl: 'http://domain.org/auth', appUrl: 'http://domain.org', isOriginAllowed: () => false, - isPopupAuthenticationRequest: true, }; const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { @@ -365,7 +367,6 @@ describe('OAuthAdapter', () => { baseUrl: 'http://domain.org/auth', appUrl: 'http://domain.org', isOriginAllowed: () => false, - isPopupAuthenticationRequest: true, }; const mockStartRequestWithOrigin = { @@ -505,7 +506,7 @@ describe('OAuthAdapter', () => { expect(mockResponse.redirect).not.toHaveBeenCalled(); }); - it('executed a response redirect when isPopupAuthenticationRequest is false', async () => { + it('executed a response redirect when authFlow query string is set to "redirect"', async () => { const handlers = { start: jest.fn(async (_req: { state: OAuthState }) => ({ url: '/url', @@ -516,7 +517,6 @@ describe('OAuthAdapter', () => { }; const configWithNoPopupEnabled = { ...configOriginAllowed, - isPopupAuthenticationRequest: false, }; const oauthProvider = OAuthAdapter.fromConfig( configWithNoPopupEnabled, @@ -531,6 +531,7 @@ describe('OAuthAdapter', () => { ...defaultState, origin: 'http://other.domain', redirectUrl: 'http://domain.org', + authFlow: 'redirect', }; const mockRequest = { @@ -540,7 +541,6 @@ describe('OAuthAdapter', () => { 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); diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 239e660298..ac55fa8ad1 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -35,7 +35,6 @@ import { import { defaultCookieConfigurer, readState, verifyNonce } from './helpers'; import { postMessageResponse, - redirectMessageResponse, ensuresXRequestedWith, WebMessageResponse, } from '../flow'; @@ -61,7 +60,6 @@ export type OAuthAdapterOptions = { cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; - isPopupAuthenticationRequest: boolean; }; /** @public */ @@ -74,8 +72,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { 'providerId' | 'persistScopes' | 'callbackUrl' | 'redirectUrl' >, ): OAuthAdapter { - const { appUrl, baseUrl, isOriginAllowed, isPopupAuthenticationRequest } = - config; + const { appUrl, baseUrl, isOriginAllowed } = config; const { origin: appOrigin } = new URL(appUrl); const cookieConfigurer = config.cookieConfigurer ?? defaultCookieConfigurer; @@ -86,7 +83,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { baseUrl, cookieConfigurer, isOriginAllowed, - isPopupAuthenticationRequest: isPopupAuthenticationRequest, }); } @@ -108,6 +104,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const env = req.query.env?.toString(); const origin = req.query.origin?.toString(); const redirectUrl = req.query.redirectUrl?.toString(); + const authFlow = req.query.authFlow?.toString(); if (!env) { throw new InputError('No env provided in request query parameters'); } @@ -118,7 +115,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, redirectUrl }; + const state: OAuthState = { nonce, env, origin, redirectUrl, authFlow }; // If scopes are persisted then we pass them through the state so that we // can set the cookie on successful auth @@ -184,8 +181,8 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { response: { ...response, backstageIdentity: identity }, }; - if (!this.options.isPopupAuthenticationRequest) { - return redirectMessageResponse(res, redirectUrl); + if (state.authFlow === 'redirect') { + res.redirect(redirectUrl); } // post message back to popup if successful return postMessageResponse(res, appOrigin, responseObj); diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index 2b06e1f78b..4cb7a94ffd 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -91,6 +91,7 @@ export type OAuthState = { origin?: string; scope?: string; redirectUrl?: string; + authFlow?: string; }; /** @public */ diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index f70b639de9..1459cff484 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -131,8 +131,6 @@ 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 f19eb54a72..9440a2c4ed 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -107,8 +107,6 @@ export async function createRouter( ...providerFactories, }; const providersConfig = config.getConfig('auth.providers'); - const isPopupAuthenticationRequest = - config.getOptionalBoolean('auth.usePopup') ?? true; const configuredProviders = providersConfig.keys(); @@ -126,7 +124,6 @@ export async function createRouter( baseUrl: authUrl, appUrl, isOriginAllowed, - isPopupAuthenticationRequest: isPopupAuthenticationRequest, }, config: providersConfig.getConfig(providerId), logger, From 167f457803168ab6d675d4e13aa3a0e52c9a906f Mon Sep 17 00:00:00 2001 From: headphonejames Date: Fri, 3 Feb 2023 12:18:50 -0800 Subject: [PATCH 03/15] fix missing code fixes from update after PR review. Signed-off-by: headphonejames --- .../implementations/OAuthRequestApi/MockOAuthApi.test.ts | 4 ++++ .../apis/implementations/OAuthRequestApi/MockOAuthApi.ts | 4 ++++ .../OAuthRequestApi/OAuthRequestManager.test.ts | 1 + plugins/auth-backend/src/lib/flow/index.ts | 6 +----- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts index a64fadeb3e..a22fc24626 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts @@ -25,12 +25,14 @@ describe('MockOAuthApi', () => { const requester1 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler1, + authFlow: 'popup', }); const authHandler2 = jest.fn().mockResolvedValue('other'); const requester2 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler2, + authFlow: 'popup', }); const promises = [ @@ -68,12 +70,14 @@ describe('MockOAuthApi', () => { const requester1 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler1, + authFlow: 'popup', }); const authHandler2 = jest.fn(); const requester2 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler2, + authFlow: 'popup', }); const promises = [ diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts index 193c474361..4ca0ffc645 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts @@ -55,4 +55,8 @@ export default class MockOAuthApi implements OAuthRequestApi { }); }); } + + authFlow() { + return 'popup'; + } } diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts index 4dd6af715e..54cfc8e41b 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts @@ -30,6 +30,7 @@ describe('OAuthRequestManager', () => { icon: () => null, }, onAuthRequest: async () => 'hello', + authFlow: 'popup', }); expect(reqSpy).toHaveBeenCalledTimes(0); diff --git a/plugins/auth-backend/src/lib/flow/index.ts b/plugins/auth-backend/src/lib/flow/index.ts index ebc3fe2ebe..f7b4491edb 100644 --- a/plugins/auth-backend/src/lib/flow/index.ts +++ b/plugins/auth-backend/src/lib/flow/index.ts @@ -14,10 +14,6 @@ * limitations under the License. */ -export { - ensuresXRequestedWith, - postMessageResponse, - redirectMessageResponse, -} from './authFlowHelpers'; +export { ensuresXRequestedWith, postMessageResponse } from './authFlowHelpers'; export type { WebMessageResponse } from './types'; From 564de00947ec31bcd95c74a17ce4dd4aad56f748 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Fri, 3 Feb 2023 13:35:28 -0800 Subject: [PATCH 04/15] update api reports Signed-off-by: headphonejames --- packages/core-app-api/api-report.md | 6 ++++-- packages/core-plugin-api/api-report.md | 3 ++- plugins/auth-backend/api-report.md | 9 +-------- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index aaaccb17de..d0890bfca8 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -263,7 +263,7 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; - usePopup?: boolean; + authFlow?: string; }; // @public @@ -492,6 +492,8 @@ export type OAuthApiCreateOptions = AuthApiCreateOptions & { // @public export class OAuthRequestManager implements OAuthRequestApi { + // (undocumented) + authFlow(): string; // (undocumented) authRequest$(): Observable; // (undocumented) @@ -517,7 +519,7 @@ export type OneLoginAuthCreateOptions = { discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; - usePopup?: boolean; + authFlow?: string; provider?: AuthProviderInfo; }; diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index ecf615dd01..d5f9bcbc60 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -193,7 +193,6 @@ export type AuthProviderInfo = { id: string; title: string; icon: IconComponent; - provider_id?: string; }; // @public @@ -549,6 +548,7 @@ export type OAuthRequestApi = { options: OAuthRequesterOptions, ): OAuthRequester; authRequest$(): Observable; + authFlow(): string; }; // @public @@ -563,6 +563,7 @@ export type OAuthRequester = ( export type OAuthRequesterOptions = { provider: AuthProviderInfo; onAuthRequest(scopes: Set): Promise; + authFlow: string; }; // @public diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index b272f81c2e..68852626a9 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -41,7 +41,6 @@ export type AuthProviderConfig = { appUrl: string; isOriginAllowed: (origin: string) => boolean; cookieConfigurer?: CookieConfigurer; - isPopupAuthenticationRequest: boolean; }; // @public (undocumented) @@ -290,7 +289,6 @@ export type OAuthAdapterOptions = { cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; - isPopupAuthenticationRequest: boolean; }; // @public (undocumented) @@ -389,6 +387,7 @@ export type OAuthState = { origin?: string; scope?: string; redirectUrl?: string; + authFlow?: string; }; // @public @@ -671,12 +670,6 @@ 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) From ad15aaa285d9f2308f8a43eddd011dc8e29eeca3 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Tue, 21 Feb 2023 18:45:50 -0800 Subject: [PATCH 05/15] Add onSignInStarted() and onSignInFailure() hooks. Clean up and renaming. Signed-off-by: headphonejames --- packages/core-app-api/src/app/types.ts | 10 +++++++++- .../src/layout/SignInPage/commonProvider.tsx | 16 ++++++++++------ .../src/layout/SignInPage/customProvider.tsx | 3 ++- .../src/layout/SignInPage/guestProvider.tsx | 7 +++++-- .../src/layout/SignInPage/providers.tsx | 10 ++++++++++ .../src/lib/oauth/OAuthAdapter.test.ts | 6 +++--- .../auth-backend/src/lib/oauth/OAuthAdapter.ts | 17 ++++++++++------- plugins/auth-backend/src/lib/oauth/types.ts | 2 +- plugins/auth-backend/src/service/router.ts | 1 - 9 files changed, 50 insertions(+), 22 deletions(-) diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 75b8fb2a34..8f5e1f5b27 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -45,9 +45,17 @@ export type BootErrorPageProps = { */ export type SignInPageProps = { /** - * Set the IdentityApi on successful sign in. This should only be called once. + * Invoked when the sign-in process has started. + */ + onSignInStarted(): void; + /** + * Set the IdentityApi on successful sign-in. This should only be called once. */ onSignInSuccess(identityApi: IdentityApi): void; + /** + * Invoked when the sign-in process has failed. + */ + onSignInFailure(): void; }; /** diff --git a/packages/core-components/src/layout/SignInPage/commonProvider.tsx b/packages/core-components/src/layout/SignInPage/commonProvider.tsx index 6796e3f22d..1827981e6f 100644 --- a/packages/core-components/src/layout/SignInPage/commonProvider.tsx +++ b/packages/core-components/src/layout/SignInPage/commonProvider.tsx @@ -28,21 +28,25 @@ import { useApi, errorApiRef } from '@backstage/core-plugin-api'; import { GridItem } from './styles'; import { ForwardedError } from '@backstage/errors'; import { UserIdentity } from './UserIdentity'; -import { PROVIDER_STORAGE_KEY } from './providers'; -const Component: ProviderComponent = ({ config, onSignInSuccess }) => { - const { apiRef, title, message, id } = config as SignInProviderConfig; +const Component: ProviderComponent = ({ + config, + onSignInStarted, + onSignInSuccess, + onSignInFailure, +}) => { + const { apiRef, title, message } = config as SignInProviderConfig; const authApi = useApi(apiRef); const errorApi = useApi(errorApiRef); const handleLogin = async () => { try { - localStorage.setItem(PROVIDER_STORAGE_KEY, id); + onSignInStarted(); const identityResponse = await authApi.getBackstageIdentity({ instantPopup: true, }); if (!identityResponse) { - localStorage.removeItem(PROVIDER_STORAGE_KEY); + onSignInFailure(); throw new Error( `The ${title} provider is not configured to support sign-in`, ); @@ -58,7 +62,7 @@ const Component: ProviderComponent = ({ config, onSignInSuccess }) => { }), ); } catch (error) { - localStorage.removeItem(PROVIDER_STORAGE_KEY); + onSignInFailure(); errorApi.post(new ForwardedError('Login failed', error)); } }; diff --git a/packages/core-components/src/layout/SignInPage/customProvider.tsx b/packages/core-components/src/layout/SignInPage/customProvider.tsx index 101c74d3cd..ad02715447 100644 --- a/packages/core-components/src/layout/SignInPage/customProvider.tsx +++ b/packages/core-components/src/layout/SignInPage/customProvider.tsx @@ -61,7 +61,7 @@ const asInputRef = (renderResult: UseFormRegisterReturn) => { }; }; -const Component: ProviderComponent = ({ onSignInSuccess }) => { +const Component: ProviderComponent = ({ onSignInStarted, onSignInSuccess }) => { const classes = useFormStyles(); const { register, handleSubmit, formState } = useForm({ mode: 'onChange', @@ -70,6 +70,7 @@ const Component: ProviderComponent = ({ onSignInSuccess }) => { const { errors } = formState; const handleResult = ({ userId, idToken }: Data) => { + onSignInStarted(); onSignInSuccess( UserIdentity.fromLegacy({ userId, diff --git a/packages/core-components/src/layout/SignInPage/guestProvider.tsx b/packages/core-components/src/layout/SignInPage/guestProvider.tsx index b2370a98d5..5393c1ea89 100644 --- a/packages/core-components/src/layout/SignInPage/guestProvider.tsx +++ b/packages/core-components/src/layout/SignInPage/guestProvider.tsx @@ -22,7 +22,7 @@ import { GridItem } from './styles'; import { ProviderComponent, ProviderLoader, SignInProvider } from './types'; import { GuestUserIdentity } from './GuestUserIdentity'; -const Component: ProviderComponent = ({ onSignInSuccess }) => ( +const Component: ProviderComponent = ({ onSignInStarted, onSignInSuccess }) => ( ( diff --git a/packages/core-components/src/layout/SignInPage/providers.tsx b/packages/core-components/src/layout/SignInPage/providers.tsx index 716befda17..1f3db4da36 100644 --- a/packages/core-components/src/layout/SignInPage/providers.tsx +++ b/packages/core-components/src/layout/SignInPage/providers.tsx @@ -176,11 +176,21 @@ export const useSignInProviders = ( handleWrappedResult(result); }; + const handleSignInStarted = () => { + localStorage.setItem(PROVIDER_STORAGE_KEY, provider.config!.id); + }; + + const handleSignInFailure = () => { + localStorage.removeItem(PROVIDER_STORAGE_KEY); + }; + return ( ); }), diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index dd45ab5441..1b6653d97e 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -178,7 +178,7 @@ describe('OAuthAdapter', () => { const state = { ...defaultState, redirectUrl: 'http://localhost:3000', - authFlow: 'redirect', + flow: 'redirect', }; const mockRequest = createEncodedQueryMockRequest(state); @@ -506,7 +506,7 @@ describe('OAuthAdapter', () => { expect(mockResponse.redirect).not.toHaveBeenCalled(); }); - it('executed a response redirect when authFlow query string is set to "redirect"', async () => { + it('executed a response redirect when flow query string is set to "redirect"', async () => { const handlers = { start: jest.fn(async (_req: { state: OAuthState }) => ({ url: '/url', @@ -531,7 +531,7 @@ describe('OAuthAdapter', () => { ...defaultState, origin: 'http://other.domain', redirectUrl: 'http://domain.org', - authFlow: 'redirect', + flow: 'redirect', }; const mockRequest = { diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index ac55fa8ad1..78365cda0f 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -55,7 +55,6 @@ export type OAuthAdapterOptions = { providerId: string; persistScopes?: boolean; appOrigin: string; - redirectUrl?: string; baseUrl: string; cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; @@ -69,7 +68,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { handlers: OAuthHandlers, options: Pick< OAuthAdapterOptions, - 'providerId' | 'persistScopes' | 'callbackUrl' | 'redirectUrl' + 'providerId' | 'persistScopes' | 'callbackUrl' >, ): OAuthAdapter { const { appUrl, baseUrl, isOriginAllowed } = config; @@ -104,7 +103,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const env = req.query.env?.toString(); const origin = req.query.origin?.toString(); const redirectUrl = req.query.redirectUrl?.toString(); - const authFlow = req.query.authFlow?.toString(); + const flow = req.query.authFlow?.toString(); if (!env) { throw new InputError('No env provided in request query parameters'); } @@ -115,7 +114,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, redirectUrl, authFlow }; + const state: OAuthState = { nonce, env, origin, redirectUrl, flow: flow }; // If scopes are persisted then we pass them through the state so that we // can set the cookie on successful auth @@ -142,7 +141,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { try { const state: OAuthState = readState(req.query.state?.toString() ?? ''); - const redirectUrl = state.redirectUrl ?? ''; if (state.origin) { try { @@ -181,8 +179,13 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { response: { ...response, backstageIdentity: identity }, }; - if (state.authFlow === 'redirect') { - res.redirect(redirectUrl); + if (state.flow === 'redirect') { + if (!state.redirectUrl) { + throw new InputError( + 'No redirectUrl provided in request query parameters', + ); + } + res.redirect(state.redirectUrl); } // post message back to popup if successful return postMessageResponse(res, appOrigin, responseObj); diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index 4cb7a94ffd..e960af2988 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -91,7 +91,7 @@ export type OAuthState = { origin?: string; scope?: string; redirectUrl?: string; - authFlow?: string; + flow?: string; }; /** @public */ diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 9440a2c4ed..8013777415 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -107,7 +107,6 @@ export async function createRouter( ...providerFactories, }; const providersConfig = config.getConfig('auth.providers'); - const configuredProviders = providersConfig.keys(); const isOriginAllowed = createOriginFilter(config); From 0354c9fb0d43eff49f9908d0b90d56ea556fc135 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Tue, 21 Feb 2023 19:35:37 -0800 Subject: [PATCH 06/15] fix SignInPageWrapper with new sign in hooks Signed-off-by: headphonejames --- packages/core-app-api/src/app/AppRouter.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/core-app-api/src/app/AppRouter.tsx b/packages/core-app-api/src/app/AppRouter.tsx index b799983a3b..33de9d479c 100644 --- a/packages/core-app-api/src/app/AppRouter.tsx +++ b/packages/core-app-api/src/app/AppRouter.tsx @@ -72,8 +72,22 @@ function SignInPageWrapper({ const configApi = useApi(configApiRef); const basePath = getBasePath(configApi); + const handleSignInStarted = () => { + // no-op + }; + + const handleSignInFailure = () => { + // no-op + }; + if (!identityApi) { - return ; + return ( + + ); } appIdentityProxy.setTarget(identityApi, { From 3ae55c4a8d9a057233b7f8cc1e87dd9123697595 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Tue, 21 Feb 2023 19:48:35 -0800 Subject: [PATCH 07/15] api reports Signed-off-by: headphonejames --- packages/core-app-api/api-report.md | 2 ++ packages/core-plugin-api/api-report.md | 2 ++ plugins/auth-backend/api-report.md | 5 ++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index d0890bfca8..f8d5a99cd6 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -545,7 +545,9 @@ export class SamlAuth // @public export type SignInPageProps = { + onSignInStarted(): void; onSignInSuccess(identityApi: IdentityApi): void; + onSignInFailure(): void; }; // @public diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index d5f9bcbc60..17278d4cc5 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -695,7 +695,9 @@ export enum SessionState { // @public export type SignInPageProps = { + onSignInStarted(): void; onSignInSuccess(identityApi: IdentityApi_2): void; + onSignInFailure(): void; }; // @public diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index 68852626a9..5caea64b33 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -268,7 +268,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { handlers: OAuthHandlers, options: Pick< OAuthAdapterOptions, - 'providerId' | 'persistScopes' | 'callbackUrl' | 'redirectUrl' + 'providerId' | 'persistScopes' | 'callbackUrl' >, ): OAuthAdapter; // (undocumented) @@ -284,7 +284,6 @@ export type OAuthAdapterOptions = { providerId: string; persistScopes?: boolean; appOrigin: string; - redirectUrl?: string; baseUrl: string; cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; @@ -387,7 +386,7 @@ export type OAuthState = { origin?: string; scope?: string; redirectUrl?: string; - authFlow?: string; + flow?: string; }; // @public From 5acc047748415e48206a5526bb9c10e0ad264e71 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Mon, 27 Feb 2023 16:34:30 -0800 Subject: [PATCH 08/15] Move configuration of redirect authentication flow to a global parameter named "enableExperimentalRedirectFlow". Pass the configApi instance into authentication providers to read new configuration parameter. Signed-off-by: headphonejames --- app-config.yaml | 1 - packages/app-defaults/src/defaults/apis.ts | 16 ++++----- packages/core-app-api/api-report.md | 7 ++-- packages/core-app-api/config.d.ts | 13 +++---- .../OAuthRequestApi/MockOAuthApi.test.ts | 4 --- .../OAuthRequestManager.test.ts | 1 - .../OAuthRequestApi/OAuthRequestManager.ts | 6 ---- .../auth/atlassian/AtlassianAuth.ts | 4 +-- .../auth/bitbucket/BitbucketAuth.test.ts | 7 ++++ .../auth/bitbucket/BitbucketAuth.ts | 4 +-- .../auth/github/GithubAuth.test.ts | 7 ++++ .../implementations/auth/github/GithubAuth.ts | 4 +-- .../auth/gitlab/GitlabAuth.test.ts | 7 ++++ .../implementations/auth/gitlab/GitlabAuth.ts | 4 +-- .../auth/google/GoogleAuth.test.ts | 7 ++++ .../implementations/auth/google/GoogleAuth.ts | 4 +-- .../auth/microsoft/MicrosoftAuth.ts | 4 +-- .../auth/oauth2/OAuth2.test.ts | 13 +++++++ .../implementations/auth/oauth2/OAuth2.ts | 4 +-- .../auth/okta/OktaAuth.test.ts | 6 ++++ .../implementations/auth/okta/OktaAuth.ts | 4 +-- .../auth/onelogin/OneLoginAuth.ts | 7 ++-- .../src/apis/implementations/auth/types.ts | 4 +-- .../DefaultAuthConnector.test.ts | 8 ++++- .../lib/AuthConnector/DefaultAuthConnector.ts | 36 ++++++++++--------- .../OAuthRequestDialog/OAuthRequestDialog.tsx | 18 ++++++---- packages/core-plugin-api/api-report.md | 2 -- .../src/apis/definitions/OAuthRequestApi.ts | 10 ------ .../test-utils/src/testUtils/defaultApis.ts | 8 +++++ .../src/lib/oauth/OAuthAdapter.ts | 2 +- plugins/gitops-profiles/package.json | 1 + .../ProfileCatalog/ProfileCatalog.test.tsx | 6 ++++ yarn.lock | 1 + 33 files changed, 143 insertions(+), 87 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 350b9068e3..dd72051189 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -293,7 +293,6 @@ auth: # path: my-sessions environment: development - authFlow: redirect ### 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 feb4cd6cc8..b3a3d55736 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -128,10 +128,10 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GoogleAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -143,10 +143,10 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => MicrosoftAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -158,11 +158,11 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GithubAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['read:user'], environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -174,10 +174,10 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => OktaAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -189,10 +189,10 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GitlabAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -204,10 +204,10 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => OneLoginAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -219,11 +219,11 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => BitbucketAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['team'], environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }), }), createApiFactory({ @@ -235,10 +235,10 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => { return AtlassianAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), - authFlow: configApi.getOptionalString('auth.authFlow'), }); }, }), diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index f8d5a99cd6..f7e2155db0 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -24,6 +24,7 @@ import { BackstagePlugin } from '@backstage/core-plugin-api'; import { bitbucketAuthApiRef } from '@backstage/core-plugin-api'; import { ComponentType } from 'react'; import { Config } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; import { ConfigReader } from '@backstage/config'; import { DiscoveryApi } from '@backstage/core-plugin-api'; import { ErrorApi } from '@backstage/core-plugin-api'; @@ -263,7 +264,7 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; - authFlow?: string; + configApi: ConfigApi; }; // @public @@ -492,8 +493,6 @@ export type OAuthApiCreateOptions = AuthApiCreateOptions & { // @public export class OAuthRequestManager implements OAuthRequestApi { - // (undocumented) - authFlow(): string; // (undocumented) authRequest$(): Observable; // (undocumented) @@ -516,10 +515,10 @@ export class OneLoginAuth { // @public export type OneLoginAuthCreateOptions = { + configApi: ConfigApi; discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; - authFlow?: string; provider?: AuthProviderInfo; }; diff --git a/packages/core-app-api/config.d.ts b/packages/core-app-api/config.d.ts index c499dfa7c6..2de1d96114 100644 --- a/packages/core-app-api/config.d.ts +++ b/packages/core-app-api/config.d.ts @@ -114,11 +114,12 @@ export interface Config { * @visibility frontend */ environment?: string; - /** - $ The authentication flow type - currently supports 'popup' and 'redirect' - * default value: 'popup' - * @visibility frontend - */ - authFlow?: string; }; + + /** + $ Enable redirect authentication flow type, instead of a popup for authentication + * default value: 'false' + * @visibility frontend + */ + enableExperimentalRedirectFlow?: boolean; } diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts index a22fc24626..a64fadeb3e 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts @@ -25,14 +25,12 @@ describe('MockOAuthApi', () => { const requester1 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler1, - authFlow: 'popup', }); const authHandler2 = jest.fn().mockResolvedValue('other'); const requester2 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler2, - authFlow: 'popup', }); const promises = [ @@ -70,14 +68,12 @@ describe('MockOAuthApi', () => { const requester1 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler1, - authFlow: 'popup', }); const authHandler2 = jest.fn(); const requester2 = mock.createAuthRequester({ provider: { icon: () => null, title: 'Test', id: 'test-provider' }, onAuthRequest: authHandler2, - authFlow: 'popup', }); const promises = [ diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts index 54cfc8e41b..4dd6af715e 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts @@ -30,7 +30,6 @@ describe('OAuthRequestManager', () => { icon: () => null, }, onAuthRequest: async () => 'hello', - authFlow: 'popup', }); expect(reqSpy).toHaveBeenCalledTimes(0); diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts index 751f2bf24c..5d467be7b4 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts @@ -37,10 +37,8 @@ export class OAuthRequestManager implements OAuthRequestApi { private readonly subject = new BehaviorSubject([]); private currentRequests: PendingOAuthRequest[] = []; private handlerCount = 0; - private authFlowStr: string = 'popup'; createAuthRequester(options: OAuthRequesterOptions): OAuthRequester { - this.authFlowStr = options.authFlow; const handler = new OAuthPendingRequests(); const index = this.handlerCount; @@ -93,8 +91,4 @@ export class OAuthRequestManager implements OAuthRequestApi { authRequest$(): Observable { return this.subject; } - - authFlow(): string { - return this.authFlowStr; - } } 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 df714e2290..3477eec361 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 @@ -32,19 +32,19 @@ const DEFAULT_PROVIDER = { export default class AtlassianAuth { static create(options: OAuthApiCreateOptions): typeof atlassianAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow, }); } } diff --git a/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts index f4bd7cbe85..29cd696a29 100644 --- a/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts @@ -17,6 +17,8 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import BitbucketAuth from './BitbucketAuth'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const getSession = jest.fn(); @@ -32,6 +34,10 @@ describe('BitbucketAuth', () => { jest.resetAllMocks(); }); + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); + it.each([ ['team api write_repository', ['team', 'api', 'write_repository']], ['read_repository sudo', ['read_repository', 'sudo']], @@ -39,6 +45,7 @@ describe('BitbucketAuth', () => { const gitlabAuth = BitbucketAuth.create({ oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), + configApi: configApi, }); gitlabAuth.getAccessToken(scope); 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 3bdb09b010..7a9f1bc094 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 @@ -47,20 +47,20 @@ const DEFAULT_PROVIDER = { export default class BitbucketAuth { static create(options: OAuthApiCreateOptions): typeof bitbucketAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['team'], } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow, defaultScopes, }); } diff --git a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts index ea7b4ac688..e34c3cea87 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts @@ -17,6 +17,8 @@ import { UrlPatternDiscovery } from '../../DiscoveryApi'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import GithubAuth from './GithubAuth'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const getSession = jest.fn(); @@ -32,8 +34,13 @@ describe('GithubAuth', () => { jest.resetAllMocks(); }); + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); + it('should forward access token request to session manager', async () => { const githubAuth = GithubAuth.create({ + configApi: configApi, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), }); 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 5acdd20719..943dfa7da4 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 @@ -32,21 +32,21 @@ const DEFAULT_PROVIDER = { export default class GithubAuth { static create(options: OAuthApiCreateOptions): typeof githubAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['read:user'], - authFlow = 'popup', } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, defaultScopes, - authFlow, }); } } diff --git a/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts index 3346af9b7c..71368c173e 100644 --- a/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts @@ -17,6 +17,8 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import GitlabAuth from './GitlabAuth'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const getSession = jest.fn(); @@ -39,7 +41,12 @@ describe('GitlabAuth', () => { ], ['read_repository sudo', ['read_repository', 'sudo']], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); + const gitlabAuth = GitlabAuth.create({ + configApi: configApi, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), }); 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 919d67dd8d..c39f5a6e23 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 @@ -32,20 +32,20 @@ const DEFAULT_PROVIDER = { export default class GitlabAuth { static create(options: OAuthApiCreateOptions): typeof gitlabAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['read_user'], } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow, defaultScopes, }); } diff --git a/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts index f8a1c3a1f5..44a3fed30f 100644 --- a/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts @@ -17,6 +17,8 @@ import GoogleAuth from './GoogleAuth'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const PREFIX = 'https://www.googleapis.com/auth/'; @@ -58,7 +60,12 @@ describe('GoogleAuth', () => { [`${PREFIX}profile`, [`${PREFIX}profile`]], [`${PREFIX}openid`, [`${PREFIX}openid`]], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); + const googleAuth = GoogleAuth.create({ + configApi: configApi, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), }); 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 e8adefb08f..f4674ab64f 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 @@ -34,10 +34,10 @@ const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; export default class GoogleAuth { static create(options: OAuthApiCreateOptions): typeof googleAuthApiRef.T { const { + configApi, discoveryApi, oauthRequestApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, defaultScopes = [ 'openid', @@ -47,11 +47,11 @@ export default class GoogleAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow, 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 c28e115950..30cfde7c6e 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 @@ -32,8 +32,8 @@ const DEFAULT_PROVIDER = { export default class MicrosoftAuth { static create(options: OAuthApiCreateOptions): typeof microsoftAuthApiRef.T { const { + configApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, discoveryApi, @@ -47,11 +47,11 @@ export default class MicrosoftAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow, defaultScopes, }); } diff --git a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts index 0bb40c23fc..671d80c2be 100644 --- a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts @@ -17,6 +17,8 @@ import OAuth2 from './OAuth2'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const theFuture = new Date(Date.now() + 3600000); const thePast = new Date(Date.now() - 10); @@ -34,12 +36,17 @@ jest.mock('../../../../lib/AuthSessionManager', () => ({ }, })); +const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, +}); + describe('OAuth2', () => { it('should get refreshed access token', async () => { getSession = jest.fn().mockResolvedValue({ providerInfo: { accessToken: 'access-token', expiresAt: theFuture }, }); const oauth2 = OAuth2.create({ + configApi: configApi, scopeTransform: scopeTransform, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), @@ -59,6 +66,7 @@ describe('OAuth2', () => { providerInfo: { accessToken: 'access-token', expiresAt: theFuture }, }); const oauth2 = OAuth2.create({ + configApi: configApi, scopeTransform: scopes => scopes.map(scope => `my-prefix/${scope}`), oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), @@ -75,7 +83,9 @@ describe('OAuth2', () => { getSession = jest.fn().mockResolvedValue({ providerInfo: { idToken: 'id-token', expiresAt: theFuture }, }); + const oauth2 = OAuth2.create({ + configApi: configApi, scopeTransform: scopeTransform, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), @@ -90,6 +100,7 @@ describe('OAuth2', () => { providerInfo: { idToken: 'id-token', expiresAt: theFuture }, }); const oauth2 = OAuth2.create({ + configApi: configApi, scopeTransform: scopes => scopes.map(scope => `my-prefix/${scope}`), oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), @@ -113,6 +124,7 @@ describe('OAuth2', () => { }) .mockRejectedValue(error); const oauth2 = OAuth2.create({ + configApi: configApi, scopeTransform: scopes => scopes.map(scope => `my-prefix/${scope}`), oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), @@ -147,6 +159,7 @@ describe('OAuth2', () => { }, }); const oauth2 = OAuth2.create({ + configApi: configApi, scopeTransform: scopes => scopes.map(scope => `my-prefix/${scope}`), oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), 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 7a133154f2..e3ff395fd0 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 @@ -72,21 +72,21 @@ export default class OAuth2 { static create(options: OAuth2CreateOptions) { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = [], - authFlow = 'popup', scopeTransform = x => x, } = options; const connector = new DefaultAuthConnector({ + configApi, discoveryApi, environment, provider, oauthRequestApi: oauthRequestApi, - authFlow, sessionTransform(res: OAuth2Response): OAuth2Session { return { ...res, diff --git a/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts index 6489b326ff..1918e51a34 100644 --- a/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts @@ -17,6 +17,8 @@ import OktaAuth from './OktaAuth'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const PREFIX = 'okta.'; @@ -50,7 +52,11 @@ describe('OktaAuth', () => { [`${PREFIX}profile`, [`${PREFIX}profile`]], [`${PREFIX}openid`, [`${PREFIX}openid`]], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); const auth = OktaAuth.create({ + configApi: configApi, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), }); 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 9075d8cb82..c1921a8f93 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 @@ -44,20 +44,20 @@ const OKTA_SCOPE_PREFIX: string = 'okta.'; export default class OktaAuth { static create(options: OAuthApiCreateOptions): typeof oktaAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, defaultScopes = ['openid', 'email', 'profile', 'offline_access'], } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow, 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 edfcbc4463..8beac05755 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 @@ -18,6 +18,7 @@ import { oneloginAuthApiRef, OAuthRequestApi, AuthProviderInfo, + ConfigApi, DiscoveryApi, } from '@backstage/core-plugin-api'; import { OAuth2 } from '../oauth2'; @@ -27,10 +28,10 @@ import { OAuth2 } from '../oauth2'; * @public */ export type OneLoginAuthCreateOptions = { + configApi: ConfigApi; discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; - authFlow?: string; provider?: AuthProviderInfo; }; @@ -62,19 +63,19 @@ export default class OneLoginAuth { options: OneLoginAuthCreateOptions, ): typeof oneloginAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', - authFlow = 'popup', provider = DEFAULT_PROVIDER, oauthRequestApi, } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, environment, - authFlow: authFlow, 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 fdb2bbc7f8..c2614a54ff 100644 --- a/packages/core-app-api/src/apis/implementations/auth/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/types.ts @@ -13,9 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { AuthProviderInfo, + ConfigApi, DiscoveryApi, OAuthRequestApi, } from '@backstage/core-plugin-api'; @@ -37,5 +37,5 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; - authFlow?: string; + configApi: ConfigApi; }; 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 fd2a2833a8..8234f3cc51 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -21,6 +21,8 @@ import { UrlPatternDiscovery } from '../../apis'; import { setupRequestMockHandlers } from '@backstage/test-utils'; import { setupServer } from 'msw/node'; import { rest } from 'msw'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; jest.mock('../loginPopup', () => { return { @@ -28,6 +30,10 @@ jest.mock('../loginPopup', () => { }; }); +const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, +}); + const defaultOptions = { discoveryApi: UrlPatternDiscovery.compile('http://my-host/api/{{pluginId}}'), environment: 'production', @@ -42,7 +48,7 @@ const defaultOptions = { scopes: new Set(res.scopes.split(' ')), expiresAt: new Date(Date.now() + expiresInSeconds * 1000), }), - authFlow: 'popup', + configApi: configApi, }; describe('DefaultAuthConnector', () => { diff --git a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts index 7cada8b0f9..3ec997d80a 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -13,12 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { - OAuthRequester, - OAuthRequestApi, AuthProviderInfo, + ConfigApi, DiscoveryApi, + OAuthRequestApi, + OAuthRequester, } from '@backstage/core-plugin-api'; import { showLoginPopup } from '../loginPopup'; import { AuthConnector, CreateSessionOptions } from './types'; @@ -50,9 +50,9 @@ type Options = { */ sessionTransform?(response: any): AuthSession | Promise; /** - * The UI authentication flow with backend authentication api. Supports either 'popup' or 'redirect'. + * ConfigApi instance used to configure authentication flow of pop-up or redirect. */ - authFlow: string; + configApi: ConfigApi; }; function defaultJoinScopes(scopes: Set) { @@ -68,36 +68,37 @@ export class DefaultAuthConnector implements AuthConnector { private readonly discoveryApi: DiscoveryApi; + private readonly configApi: ConfigApi; private readonly environment: string; private readonly provider: AuthProviderInfo; private readonly joinScopesFunc: (scopes: Set) => string; private readonly authRequester: OAuthRequester; private readonly sessionTransform: (response: any) => Promise; - private readonly authFlow: string; - constructor(options: Options) { const { + configApi, discoveryApi, environment, provider, joinScopes = defaultJoinScopes, oauthRequestApi, - authFlow, sessionTransform = id => id, } = options; this.authRequester = oauthRequestApi.createAuthRequester({ provider, onAuthRequest: async scopes => { - if (authFlow === 'popup') { + const enableExperimentalRedirectFlow = + this.configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? + false; + if (!enableExperimentalRedirectFlow) { return this.showPopup(scopes); } return this.executeRedirect(scopes); }, - authFlow, }); - this.authFlow = authFlow; + this.configApi = configApi; this.discoveryApi = discoveryApi; this.environment = environment; this.provider = provider; @@ -106,7 +107,11 @@ export class DefaultAuthConnector } async createSession(options: CreateSessionOptions): Promise { - if (options.instantPopup && this.authFlow === 'popup') { + const enableExperimentalRedirectFlow = + this.configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? + false; + + if (options.instantPopup && !enableExperimentalRedirectFlow) { return this.showPopup(options.scopes); } return this.authRequester(options.scopes); @@ -184,14 +189,13 @@ export class DefaultAuthConnector private async executeRedirect(scopes: Set): Promise { const scope = this.joinScopesFunc(scopes); - const redirectUrl = await this.buildUrl('/start', { + // redirect to auth api + window.location.href = await this.buildUrl('/start', { scope, origin: window.location.origin, redirectUrl: window.location.href, - authFlow: 'redirect', + flow: 'redirect', }); - // redirect to auth api - window.location.href = redirectUrl; // return a promise that never resolves return new Promise(() => {}); } diff --git a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx index e68120298a..c7a8e49faa 100644 --- a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx +++ b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { makeStyles, Theme } from '@material-ui/core/styles'; import Dialog from '@material-ui/core/Dialog'; import DialogActions from '@material-ui/core/DialogActions'; @@ -24,7 +23,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, + configApiRef, + oauthRequestApiRef, +} from '@backstage/core-plugin-api'; import Typography from '@material-ui/core/Typography'; export type OAuthRequestDialogClassKey = @@ -58,10 +61,13 @@ export function OAuthRequestDialog(_props: {}) { const classes = useStyles(); const [busy, setBusy] = useState(false); const oauthRequestApi = useApi(oauthRequestApiRef); - const redirectMessage = - oauthRequestApi.authFlow() === 'popup' - ? '' - : 'This will trigger a http redirect to OAuth Login.'; + const configApi = useApi(configApiRef); + + const authRedirect = + configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? false; + const redirectMessage = authRedirect + ? 'This will trigger a http redirect to OAuth Login.' + : ''; const requests = useObservable( useMemo(() => oauthRequestApi.authRequest$(), [oauthRequestApi]), diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 17278d4cc5..6bba47a3f4 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -548,7 +548,6 @@ export type OAuthRequestApi = { options: OAuthRequesterOptions, ): OAuthRequester; authRequest$(): Observable; - authFlow(): string; }; // @public @@ -563,7 +562,6 @@ export type OAuthRequester = ( export type OAuthRequesterOptions = { provider: AuthProviderInfo; onAuthRequest(scopes: Set): Promise; - authFlow: string; }; // @public diff --git a/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts b/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts index 2027852de7..75bf3a3864 100644 --- a/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts @@ -35,11 +35,6 @@ export type OAuthRequesterOptions = { * trigger() is called on an auth requests. */ onAuthRequest(scopes: Set): Promise; - - /** - * The authentication flow type - */ - authFlow: string; }; /** @@ -124,11 +119,6 @@ export type OAuthRequestApi = { * AuthRequester calls will resolve to the value returned by the onAuthRequest call. */ authRequest$(): Observable; - - /** - * The authentication flow type - */ - authFlow(): string; }; /** diff --git a/packages/test-utils/src/testUtils/defaultApis.ts b/packages/test-utils/src/testUtils/defaultApis.ts index 1ff10bfe6f..2231d28cd0 100644 --- a/packages/test-utils/src/testUtils/defaultApis.ts +++ b/packages/test-utils/src/testUtils/defaultApis.ts @@ -89,6 +89,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GoogleAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -103,6 +104,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => MicrosoftAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -117,6 +119,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GithubAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['read:user'], @@ -132,6 +135,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => OktaAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -146,6 +150,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GitlabAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -160,6 +165,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => OneLoginAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -174,6 +180,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => BitbucketAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['team'], @@ -189,6 +196,7 @@ export const defaultApis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => { return AtlassianAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 78365cda0f..3d2a312a81 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -103,7 +103,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const env = req.query.env?.toString(); const origin = req.query.origin?.toString(); const redirectUrl = req.query.redirectUrl?.toString(); - const flow = req.query.authFlow?.toString(); + const flow = req.query.flow?.toString(); if (!env) { throw new InputError('No env provided in request query parameters'); } diff --git a/plugins/gitops-profiles/package.json b/plugins/gitops-profiles/package.json index b485aea867..9c8f623c12 100644 --- a/plugins/gitops-profiles/package.json +++ b/plugins/gitops-profiles/package.json @@ -33,6 +33,7 @@ "clean": "backstage-cli package clean" }, "dependencies": { + "@backstage/config": "workspace:^", "@backstage/core-components": "workspace:^", "@backstage/core-plugin-api": "workspace:^", "@backstage/theme": "workspace:^", diff --git a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx index 5aa59a1cae..03d3d84c36 100644 --- a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx +++ b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx @@ -18,6 +18,8 @@ import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils'; import React from 'react'; import { gitOpsApiRef, GitOpsRestApi } from '../../api'; import ProfileCatalog from './ProfileCatalog'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; import { ApiProvider, @@ -31,6 +33,9 @@ import { githubAuthApiRef } from '@backstage/core-plugin-api'; describe('ProfileCatalog', () => { it('should render', async () => { const oauthRequestApi = new OAuthRequestManager(); + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); const apis = TestApiRegistry.from( [gitOpsApiRef, new GitOpsRestApi('http://localhost:3008')], [ @@ -40,6 +45,7 @@ describe('ProfileCatalog', () => { 'http://example.com/{{pluginId}}', ), oauthRequestApi, + configApi: configApi, }), ], ); diff --git a/yarn.lock b/yarn.lock index 7cc0dbd8b5..974db36e32 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6195,6 +6195,7 @@ __metadata: resolution: "@backstage/plugin-gitops-profiles@workspace:plugins/gitops-profiles" dependencies: "@backstage/cli": "workspace:^" + "@backstage/config": "workspace:^" "@backstage/core-app-api": "workspace:^" "@backstage/core-components": "workspace:^" "@backstage/core-plugin-api": "workspace:^" From fa9e9e36ab093b72669bb8f10caba2e69c5d8ca8 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Mon, 27 Feb 2023 16:39:13 -0800 Subject: [PATCH 09/15] fix query string name Signed-off-by: headphonejames --- .../src/lib/AuthConnector/DefaultAuthConnector.test.ts | 4 ++-- .../src/lib/AuthConnector/DefaultAuthConnector.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 8234f3cc51..51da9cbfb5 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -138,7 +138,7 @@ describe('DefaultAuthConnector', () => { expect(popupSpy).toHaveBeenCalledTimes(1); expect(popupSpy.mock.calls[0][0]).toMatchObject({ - url: 'http://my-host/api/auth/my-provider/start?scope=a%20b&origin=http%3A%2F%2Flocalhost&authFlow=popup&env=production', + url: 'http://my-host/api/auth/my-provider/start?scope=a%20b&origin=http%3A%2F%2Flocalhost&flow=popup&env=production', }); await expect(sessionPromise).resolves.toEqual({ @@ -186,7 +186,7 @@ describe('DefaultAuthConnector', () => { expect(popupSpy).toHaveBeenCalledTimes(1); expect(popupSpy.mock.calls[0][0]).toMatchObject({ - url: 'http://my-host/api/auth/my-provider/start?scope=-ab-&origin=http%3A%2F%2Flocalhost&authFlow=popup&env=production', + url: 'http://my-host/api/auth/my-provider/start?scope=-ab-&origin=http%3A%2F%2Flocalhost&flow=popup&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 3ec997d80a..633ccb8ae1 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -173,7 +173,7 @@ export class DefaultAuthConnector const popupUrl = await this.buildUrl('/start', { scope, origin: window.location.origin, - authFlow: 'popup', + flow: 'popup', }); const payload = await showLoginPopup({ From 63a6a3d44ba2e61094778a7ff4dfcd5e78141a6c Mon Sep 17 00:00:00 2001 From: headphonejames Date: Mon, 27 Feb 2023 17:32:00 -0800 Subject: [PATCH 10/15] rebase and fix bitbucket server Signed-off-by: headphonejames --- packages/app-defaults/src/defaults/apis.ts | 4 +++- .../auth/bitbucketServer/BitbucketServerAuth.test.ts | 7 +++++++ .../auth/bitbucketServer/BitbucketServerAuth.ts | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index f8464adaff..3a552c25c7 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -233,9 +233,11 @@ export const apis = [ deps: { discoveryApi: discoveryApiRef, oauthRequestApi: oauthRequestApiRef, + configApi: configApiRef, }, - factory: ({ discoveryApi, oauthRequestApi }) => + factory: ({ discoveryApi, oauthRequestApi, configApi }) => BitbucketServerAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['REPO_READ'], diff --git a/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts index 32e7755a00..e1625c7687 100644 --- a/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts @@ -17,6 +17,8 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import BitbucketServerAuth from './BitbucketServerAuth'; +import { ConfigReader } from '@backstage/config'; +import { ConfigApi } from '@backstage/core-plugin-api'; const getSession = jest.fn(); @@ -40,7 +42,12 @@ describe('BitbucketServerAuth', () => { ['PROJECT_ADMIN', 'REPO_READ', 'ACCOUNT_WRITE'], ], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi: ConfigApi = new ConfigReader({ + enableExperimentalRedirectFlow: false, + }); + const bitbucketServerAuth = BitbucketServerAuth.create({ + configApi: configApi, oauthRequestApi: new MockOAuthApi(), discoveryApi: UrlPatternDiscovery.compile('http://example.com'), }); diff --git a/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.ts b/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.ts index dc50eff6ab..b25deffb93 100644 --- a/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.ts @@ -48,6 +48,7 @@ export default class BitbucketServerAuth { options: OAuthApiCreateOptions, ): typeof bitbucketServerAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -56,6 +57,7 @@ export default class BitbucketServerAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, From d7e856ad55b002b6ffe1e986fcdceed8e4f3a88d Mon Sep 17 00:00:00 2001 From: headphonejames Date: Tue, 28 Feb 2023 17:05:11 -0800 Subject: [PATCH 11/15] simplify code and test cases. clean up rendering. Signed-off-by: headphonejames --- packages/core-app-api/api-report.md | 4 +-- .../auth/bitbucket/BitbucketAuth.test.ts | 7 ++-- .../BitbucketServerAuth.test.ts | 7 ++-- .../auth/github/GithubAuth.test.ts | 7 ++-- .../auth/gitlab/GitlabAuth.test.ts | 7 ++-- .../auth/google/GoogleAuth.test.ts | 7 ++-- .../auth/oauth2/OAuth2.test.ts | 7 ++-- .../auth/okta/OktaAuth.test.ts | 7 ++-- .../src/apis/implementations/auth/types.ts | 2 +- packages/core-app-api/src/app/AppRouter.tsx | 16 +-------- packages/core-app-api/src/app/types.ts | 8 ----- .../lib/AuthConnector/DefaultAuthConnector.ts | 33 ++++++++++++------- .../OAuthRequestDialog/OAuthRequestDialog.tsx | 9 ++--- .../src/layout/SignInPage/types.ts | 13 +++++++- packages/core-plugin-api/api-report.md | 2 -- .../src/apis/definitions/auth.ts | 1 + .../src/lib/oauth/OAuthAdapter.ts | 2 +- .../ProfileCatalog/ProfileCatalog.test.tsx | 12 +++---- 18 files changed, 63 insertions(+), 88 deletions(-) diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 405c9ed7b8..a2a34171b1 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -265,7 +265,7 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; - configApi: ConfigApi; + configApi?: ConfigApi; }; // @public @@ -564,9 +564,7 @@ export class SamlAuth // @public export type SignInPageProps = { - onSignInStarted(): void; onSignInSuccess(identityApi: IdentityApi): void; - onSignInFailure(): void; }; // @public diff --git a/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts index 29cd696a29..1335808280 100644 --- a/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/bitbucket/BitbucketAuth.test.ts @@ -17,8 +17,7 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import BitbucketAuth from './BitbucketAuth'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -34,9 +33,7 @@ describe('BitbucketAuth', () => { jest.resetAllMocks(); }); - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); it.each([ ['team api write_repository', ['team', 'api', 'write_repository']], diff --git a/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts index e1625c7687..5631a9a886 100644 --- a/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/bitbucketServer/BitbucketServerAuth.test.ts @@ -17,8 +17,7 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import BitbucketServerAuth from './BitbucketServerAuth'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -42,9 +41,7 @@ describe('BitbucketServerAuth', () => { ['PROJECT_ADMIN', 'REPO_READ', 'ACCOUNT_WRITE'], ], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); const bitbucketServerAuth = BitbucketServerAuth.create({ configApi: configApi, diff --git a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts index e34c3cea87..fc77754f6f 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts @@ -17,8 +17,7 @@ import { UrlPatternDiscovery } from '../../DiscoveryApi'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import GithubAuth from './GithubAuth'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -34,9 +33,7 @@ describe('GithubAuth', () => { jest.resetAllMocks(); }); - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); it('should forward access token request to session manager', async () => { const githubAuth = GithubAuth.create({ diff --git a/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts index 71368c173e..6dde0c0334 100644 --- a/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/gitlab/GitlabAuth.test.ts @@ -17,8 +17,7 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import GitlabAuth from './GitlabAuth'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -41,9 +40,7 @@ describe('GitlabAuth', () => { ], ['read_repository sudo', ['read_repository', 'sudo']], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); const gitlabAuth = GitlabAuth.create({ configApi: configApi, diff --git a/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts index 44a3fed30f..7897eef42e 100644 --- a/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/google/GoogleAuth.test.ts @@ -17,8 +17,7 @@ import GoogleAuth from './GoogleAuth'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const PREFIX = 'https://www.googleapis.com/auth/'; @@ -60,9 +59,7 @@ describe('GoogleAuth', () => { [`${PREFIX}profile`, [`${PREFIX}profile`]], [`${PREFIX}openid`, [`${PREFIX}openid`]], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); const googleAuth = GoogleAuth.create({ configApi: configApi, diff --git a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts index 671d80c2be..c6d92d2dad 100644 --- a/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/oauth2/OAuth2.test.ts @@ -17,8 +17,7 @@ import OAuth2 from './OAuth2'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const theFuture = new Date(Date.now() + 3600000); const thePast = new Date(Date.now() - 10); @@ -36,9 +35,7 @@ jest.mock('../../../../lib/AuthSessionManager', () => ({ }, })); -const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, -}); +const configApi = new MockConfigApi({}); describe('OAuth2', () => { it('should get refreshed access token', async () => { diff --git a/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts index 1918e51a34..5e7370a947 100644 --- a/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/okta/OktaAuth.test.ts @@ -17,8 +17,7 @@ import OktaAuth from './OktaAuth'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; const PREFIX = 'okta.'; @@ -52,9 +51,7 @@ describe('OktaAuth', () => { [`${PREFIX}profile`, [`${PREFIX}profile`]], [`${PREFIX}openid`, [`${PREFIX}openid`]], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); const auth = OktaAuth.create({ configApi: configApi, oauthRequestApi: new MockOAuthApi(), 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 c2614a54ff..08ddc5e659 100644 --- a/packages/core-app-api/src/apis/implementations/auth/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/types.ts @@ -37,5 +37,5 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; - configApi: ConfigApi; + configApi?: ConfigApi; }; diff --git a/packages/core-app-api/src/app/AppRouter.tsx b/packages/core-app-api/src/app/AppRouter.tsx index 33de9d479c..b799983a3b 100644 --- a/packages/core-app-api/src/app/AppRouter.tsx +++ b/packages/core-app-api/src/app/AppRouter.tsx @@ -72,22 +72,8 @@ function SignInPageWrapper({ const configApi = useApi(configApiRef); const basePath = getBasePath(configApi); - const handleSignInStarted = () => { - // no-op - }; - - const handleSignInFailure = () => { - // no-op - }; - if (!identityApi) { - return ( - - ); + return ; } appIdentityProxy.setTarget(identityApi, { diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 8f5e1f5b27..3d9005b0c0 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -44,18 +44,10 @@ export type BootErrorPageProps = { * @public */ export type SignInPageProps = { - /** - * Invoked when the sign-in process has started. - */ - onSignInStarted(): void; /** * Set the IdentityApi on successful sign-in. This should only be called once. */ onSignInSuccess(identityApi: IdentityApi): void; - /** - * Invoked when the sign-in process has failed. - */ - onSignInFailure(): void; }; /** diff --git a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts index 633ccb8ae1..f2c35b0ba3 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -23,6 +23,8 @@ import { import { showLoginPopup } from '../loginPopup'; import { AuthConnector, CreateSessionOptions } from './types'; +let warned = false; + type Options = { /** * DiscoveryApi instance used to locate the auth backend endpoint. @@ -52,7 +54,7 @@ type Options = { /** * ConfigApi instance used to configure authentication flow of pop-up or redirect. */ - configApi: ConfigApi; + configApi?: ConfigApi; }; function defaultJoinScopes(scopes: Set) { @@ -68,12 +70,12 @@ export class DefaultAuthConnector implements AuthConnector { private readonly discoveryApi: DiscoveryApi; - private readonly configApi: ConfigApi; private readonly environment: string; private readonly provider: AuthProviderInfo; private readonly joinScopesFunc: (scopes: Set) => string; private readonly authRequester: OAuthRequester; private readonly sessionTransform: (response: any) => Promise; + private readonly enableExperimentalRedirectFlow: boolean; constructor(options: Options) { const { configApi, @@ -85,20 +87,28 @@ export class DefaultAuthConnector sessionTransform = id => id, } = options; + if (!warned && !configApi) { + // eslint-disable-next-line no-console + console.warn( + 'DEPRECATION WARNING: Authentication providers require a configApi instance to configure the authentication flow. Please provide one to the authentication provider constructor.', + ); + warned = true; + } + + this.enableExperimentalRedirectFlow = configApi + ? configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? false + : false; + this.authRequester = oauthRequestApi.createAuthRequester({ provider, onAuthRequest: async scopes => { - const enableExperimentalRedirectFlow = - this.configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? - false; - if (!enableExperimentalRedirectFlow) { + if (!this.enableExperimentalRedirectFlow) { return this.showPopup(scopes); } return this.executeRedirect(scopes); }, }); - this.configApi = configApi; this.discoveryApi = discoveryApi; this.environment = environment; this.provider = provider; @@ -107,11 +117,10 @@ export class DefaultAuthConnector } async createSession(options: CreateSessionOptions): Promise { - const enableExperimentalRedirectFlow = - this.configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? - false; - - if (options.instantPopup && !enableExperimentalRedirectFlow) { + if (options.instantPopup) { + if (this.enableExperimentalRedirectFlow) { + return this.executeRedirect(options.scopes); + } return this.showPopup(options.scopes); } return this.authRequester(options.scopes); diff --git a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx index c7a8e49faa..3e6c685401 100644 --- a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx +++ b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx @@ -65,9 +65,6 @@ export function OAuthRequestDialog(_props: {}) { const authRedirect = configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? false; - const redirectMessage = authRedirect - ? 'This will trigger a http redirect to OAuth Login.' - : ''; const requests = useObservable( useMemo(() => oauthRequestApi.authRequest$(), [oauthRequestApi]), @@ -94,7 +91,11 @@ export function OAuthRequestDialog(_props: {}) { Login Required - {redirectMessage} + {authRedirect ? ( + + This will trigger a http redirect to OAuth Login. + + ) : null} diff --git a/packages/core-components/src/layout/SignInPage/types.ts b/packages/core-components/src/layout/SignInPage/types.ts index 8a0430a27d..6213c03432 100644 --- a/packages/core-components/src/layout/SignInPage/types.ts +++ b/packages/core-components/src/layout/SignInPage/types.ts @@ -35,8 +35,19 @@ export type SignInProviderConfig = { /** @public */ export type IdentityProviders = ('guest' | 'custom' | SignInProviderConfig)[]; +/** + * Invoked when the sign-in process has failed. + */ +export type onSignInFailure = () => void; +/** + * Invoked when the sign-in process has started. + */ +export type onSignInStarted = () => void; + export type ProviderComponent = ComponentType< - SignInPageProps & { config: SignInProviderConfig } + { onSignInStarted: onSignInStarted } & { + onSignInFailure: onSignInFailure; + } & SignInPageProps & { config: SignInProviderConfig } >; export type ProviderLoader = ( diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 035b837729..4887f0b7f2 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -687,9 +687,7 @@ export enum SessionState { // @public export type SignInPageProps = { - onSignInStarted(): void; onSignInSuccess(identityApi: IdentityApi_2): void; - onSignInFailure(): void; }; // @public diff --git a/packages/core-plugin-api/src/apis/definitions/auth.ts b/packages/core-plugin-api/src/apis/definitions/auth.ts index 21717e19e0..43597639b6 100644 --- a/packages/core-plugin-api/src/apis/definitions/auth.ts +++ b/packages/core-plugin-api/src/apis/definitions/auth.ts @@ -285,6 +285,7 @@ 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/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 3d2a312a81..b7aa5390fb 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -114,7 +114,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, redirectUrl, flow: flow }; + const state: OAuthState = { nonce, env, origin, redirectUrl, flow }; // If scopes are persisted then we pass them through the state so that we // can set the cookie on successful auth diff --git a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx index 03d3d84c36..eb58e03580 100644 --- a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx +++ b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx @@ -14,12 +14,14 @@ * limitations under the License. */ -import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils'; +import { + MockConfigApi, + renderInTestApp, + TestApiRegistry, +} from '@backstage/test-utils'; import React from 'react'; import { gitOpsApiRef, GitOpsRestApi } from '../../api'; import ProfileCatalog from './ProfileCatalog'; -import { ConfigReader } from '@backstage/config'; -import { ConfigApi } from '@backstage/core-plugin-api'; import { ApiProvider, @@ -33,9 +35,7 @@ import { githubAuthApiRef } from '@backstage/core-plugin-api'; describe('ProfileCatalog', () => { it('should render', async () => { const oauthRequestApi = new OAuthRequestManager(); - const configApi: ConfigApi = new ConfigReader({ - enableExperimentalRedirectFlow: false, - }); + const configApi = new MockConfigApi({}); const apis = TestApiRegistry.from( [gitOpsApiRef, new GitOpsRestApi('http://localhost:3008')], [ From 57ec7e225036924af3167a9a8d99c344c1b6e8d6 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Tue, 28 Feb 2023 17:20:23 -0800 Subject: [PATCH 12/15] minor clean up Signed-off-by: headphonejames --- .../src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts | 4 ---- packages/core-app-api/src/apis/implementations/auth/types.ts | 1 + .../src/components/OAuthRequestDialog/OAuthRequestDialog.tsx | 1 + packages/core-components/src/layout/SignInPage/providers.tsx | 2 +- plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts | 1 + 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts index 4ca0ffc645..193c474361 100644 --- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts +++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.ts @@ -55,8 +55,4 @@ export default class MockOAuthApi implements OAuthRequestApi { }); }); } - - authFlow() { - return 'popup'; - } } 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 08ddc5e659..7a74fd3a3b 100644 --- a/packages/core-app-api/src/apis/implementations/auth/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/types.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { AuthProviderInfo, ConfigApi, diff --git a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx index 3e6c685401..0159d8f503 100644 --- a/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx +++ b/packages/core-components/src/components/OAuthRequestDialog/OAuthRequestDialog.tsx @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { makeStyles, Theme } from '@material-ui/core/styles'; import Dialog from '@material-ui/core/Dialog'; import DialogActions from '@material-ui/core/DialogActions'; diff --git a/packages/core-components/src/layout/SignInPage/providers.tsx b/packages/core-components/src/layout/SignInPage/providers.tsx index 1f3db4da36..5e7ab7b7be 100644 --- a/packages/core-components/src/layout/SignInPage/providers.tsx +++ b/packages/core-components/src/layout/SignInPage/providers.tsx @@ -32,7 +32,7 @@ import { guestProvider } from './guestProvider'; import { customProvider } from './customProvider'; import { IdentityApiSignOutProxy } from './IdentityApiSignOutProxy'; -export const PROVIDER_STORAGE_KEY = '@backstage/core:SignInPage:provider'; +const PROVIDER_STORAGE_KEY = '@backstage/core:SignInPage:provider'; export type SignInProviderType = { [key: string]: { diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index b7aa5390fb..5f51b2d301 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -104,6 +104,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const origin = req.query.origin?.toString(); const redirectUrl = req.query.redirectUrl?.toString(); const flow = req.query.flow?.toString(); + if (!env) { throw new InputError('No env provided in request query parameters'); } From 7908d72e033fb6af4d95a41d62d82ef1afe42511 Mon Sep 17 00:00:00 2001 From: headphonejames Date: Tue, 28 Feb 2023 21:56:38 -0800 Subject: [PATCH 13/15] add change set Signed-off-by: headphonejames --- .changeset/long-gorillas-remain.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/long-gorillas-remain.md diff --git a/.changeset/long-gorillas-remain.md b/.changeset/long-gorillas-remain.md new file mode 100644 index 0000000000..f74dc1b1a7 --- /dev/null +++ b/.changeset/long-gorillas-remain.md @@ -0,0 +1,11 @@ +--- +'@backstage/core-components': minor +'@backstage/core-plugin-api': minor +'@backstage/plugin-gitops-profiles': minor +'@backstage/app-defaults': minor +'@backstage/core-app-api': minor +'@backstage/plugin-auth-backend': minor +'@backstage/test-utils': minor +--- + +Introduce a new global config parameter, "enableExperimentalRedirectFlow" When enabled, instead of having a popup window where the authentication takes place, backstage will redirect to the authentication backend plugin, followed by a redirect back to the backstage frontend after authentication takes place. From 8f72438559c22c6447dacf94f1173c859b6041b6 Mon Sep 17 00:00:00 2001 From: Headphones James Date: Mon, 6 Mar 2023 10:10:04 -0800 Subject: [PATCH 14/15] Update .changeset/long-gorillas-remain.md Co-authored-by: Patrik Oldsberg Signed-off-by: Headphones James --- .changeset/long-gorillas-remain.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.changeset/long-gorillas-remain.md b/.changeset/long-gorillas-remain.md index f74dc1b1a7..60ad6fa092 100644 --- a/.changeset/long-gorillas-remain.md +++ b/.changeset/long-gorillas-remain.md @@ -1,11 +1,9 @@ --- -'@backstage/core-components': minor -'@backstage/core-plugin-api': minor -'@backstage/plugin-gitops-profiles': minor +'@backstage/core-components': patch '@backstage/app-defaults': minor '@backstage/core-app-api': minor -'@backstage/plugin-auth-backend': minor +'@backstage/plugin-auth-backend': patch '@backstage/test-utils': minor --- -Introduce a new global config parameter, "enableExperimentalRedirectFlow" When enabled, instead of having a popup window where the authentication takes place, backstage will redirect to the authentication backend plugin, followed by a redirect back to the backstage frontend after authentication takes place. +Introduce a new global config parameter, `auth.enableExperimentalRedirectFlow`. When enabled, auth will happen with an in-window redirect flow rather than through a popup window. From ef729bc30cb3c3726fded9b5d7c144b9eaaad3cf Mon Sep 17 00:00:00 2001 From: headphonejames Date: Mon, 6 Mar 2023 10:16:21 -0800 Subject: [PATCH 15/15] clean up, add missing optional configApi to OneLoginAuthCreateOptions. Signed-off-by: headphonejames --- packages/core-app-api/api-report.md | 2 +- .../apis/implementations/auth/onelogin/OneLoginAuth.ts | 2 +- packages/core-components/src/layout/SignInPage/types.ts | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index a2a34171b1..d7e4a22873 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -535,7 +535,7 @@ export class OneLoginAuth { // @public export type OneLoginAuthCreateOptions = { - configApi: ConfigApi; + configApi?: ConfigApi; discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; 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 8beac05755..d8f54e7e50 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 @@ -28,7 +28,7 @@ import { OAuth2 } from '../oauth2'; * @public */ export type OneLoginAuthCreateOptions = { - configApi: ConfigApi; + configApi?: ConfigApi; discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; diff --git a/packages/core-components/src/layout/SignInPage/types.ts b/packages/core-components/src/layout/SignInPage/types.ts index 6213c03432..a7e1ca62c2 100644 --- a/packages/core-components/src/layout/SignInPage/types.ts +++ b/packages/core-components/src/layout/SignInPage/types.ts @@ -45,9 +45,11 @@ export type onSignInFailure = () => void; export type onSignInStarted = () => void; export type ProviderComponent = ComponentType< - { onSignInStarted: onSignInStarted } & { - onSignInFailure: onSignInFailure; - } & SignInPageProps & { config: SignInProviderConfig } + SignInPageProps & { + config: SignInProviderConfig; + onSignInStarted(): void; + onSignInFailure(): void; + } >; export type ProviderLoader = (