diff --git a/.changeset/long-gorillas-remain.md b/.changeset/long-gorillas-remain.md new file mode 100644 index 0000000000..60ad6fa092 --- /dev/null +++ b/.changeset/long-gorillas-remain.md @@ -0,0 +1,9 @@ +--- +'@backstage/core-components': patch +'@backstage/app-defaults': minor +'@backstage/core-app-api': minor +'@backstage/plugin-auth-backend': patch +'@backstage/test-utils': minor +--- + +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. diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index d3ebddfb84..3a552c25c7 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -130,6 +130,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GoogleAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -144,6 +145,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => MicrosoftAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -158,6 +160,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GithubAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['read:user'], @@ -173,6 +176,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => OktaAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -187,6 +191,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => GitlabAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -201,6 +206,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => OneLoginAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), @@ -215,6 +221,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => BitbucketAuth.create({ + configApi, discoveryApi, oauthRequestApi, defaultScopes: ['team'], @@ -226,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'], @@ -243,6 +252,7 @@ export const apis = [ }, factory: ({ discoveryApi, oauthRequestApi, configApi }) => { return AtlassianAuth.create({ + configApi, discoveryApi, oauthRequestApi, environment: configApi.getOptionalString('auth.environment'), diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 37f7a98de0..d7e4a22873 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -25,6 +25,7 @@ import { bitbucketAuthApiRef } from '@backstage/core-plugin-api'; import { bitbucketServerAuthApiRef } 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'; @@ -264,6 +265,7 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; + configApi?: ConfigApi; }; // @public @@ -533,6 +535,7 @@ export class OneLoginAuth { // @public export type OneLoginAuthCreateOptions = { + configApi?: ConfigApi; discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; diff --git a/packages/core-app-api/config.d.ts b/packages/core-app-api/config.d.ts index ae876f7fe0..2de1d96114 100644 --- a/packages/core-app-api/config.d.ts +++ b/packages/core-app-api/config.d.ts @@ -115,4 +115,11 @@ export interface Config { */ environment?: 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/auth/atlassian/AtlassianAuth.ts b/packages/core-app-api/src/apis/implementations/auth/atlassian/AtlassianAuth.ts index 07ebefca28..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,6 +32,7 @@ const DEFAULT_PROVIDER = { export default class AtlassianAuth { static create(options: OAuthApiCreateOptions): typeof atlassianAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -39,6 +40,7 @@ export default class AtlassianAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..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,6 +17,7 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import BitbucketAuth from './BitbucketAuth'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -32,6 +33,8 @@ describe('BitbucketAuth', () => { jest.resetAllMocks(); }); + const configApi = new MockConfigApi({}); + it.each([ ['team api write_repository', ['team', 'api', 'write_repository']], ['read_repository sudo', ['read_repository', 'sudo']], @@ -39,6 +42,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 7c5d4f3fc0..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,6 +47,7 @@ const DEFAULT_PROVIDER = { export default class BitbucketAuth { static create(options: OAuthApiCreateOptions): typeof bitbucketAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -55,6 +56,7 @@ export default class BitbucketAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..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,6 +17,7 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import BitbucketServerAuth from './BitbucketServerAuth'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -40,7 +41,10 @@ describe('BitbucketServerAuth', () => { ['PROJECT_ADMIN', 'REPO_READ', 'ACCOUNT_WRITE'], ], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi = new MockConfigApi({}); + 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, 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..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,6 +17,7 @@ import { UrlPatternDiscovery } from '../../DiscoveryApi'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import GithubAuth from './GithubAuth'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -32,8 +33,11 @@ describe('GithubAuth', () => { jest.resetAllMocks(); }); + const configApi = new MockConfigApi({}); + 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 7efe4e95c6..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,6 +32,7 @@ const DEFAULT_PROVIDER = { export default class GithubAuth { static create(options: OAuthApiCreateOptions): typeof githubAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -40,6 +41,7 @@ export default class GithubAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..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,6 +17,7 @@ import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; import GitlabAuth from './GitlabAuth'; +import { MockConfigApi } from '@backstage/test-utils'; const getSession = jest.fn(); @@ -39,7 +40,10 @@ describe('GitlabAuth', () => { ], ['read_repository sudo', ['read_repository', 'sudo']], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi = new MockConfigApi({}); + 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 00085de8ac..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,6 +32,7 @@ const DEFAULT_PROVIDER = { export default class GitlabAuth { static create(options: OAuthApiCreateOptions): typeof gitlabAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -40,6 +41,7 @@ export default class GitlabAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..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,6 +17,7 @@ import GoogleAuth from './GoogleAuth'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import { MockConfigApi } from '@backstage/test-utils'; const PREFIX = 'https://www.googleapis.com/auth/'; @@ -58,7 +59,10 @@ describe('GoogleAuth', () => { [`${PREFIX}profile`, [`${PREFIX}profile`]], [`${PREFIX}openid`, [`${PREFIX}openid`]], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi = new MockConfigApi({}); + 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 8a528f4511..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,6 +34,7 @@ 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', @@ -46,6 +47,7 @@ export default class GoogleAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..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,6 +32,7 @@ const DEFAULT_PROVIDER = { export default class MicrosoftAuth { static create(options: OAuthApiCreateOptions): typeof microsoftAuthApiRef.T { const { + configApi, environment = 'development', provider = DEFAULT_PROVIDER, oauthRequestApi, @@ -46,6 +47,7 @@ export default class MicrosoftAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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 63070120ef..6f941379a7 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,7 @@ import OAuth2 from './OAuth2'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import { MockConfigApi } from '@backstage/test-utils'; const theFuture = new Date(Date.now() + 3600000); const thePast = new Date(Date.now() - 10); @@ -34,12 +35,15 @@ jest.mock('../../../../lib/AuthSessionManager', () => ({ }, })); +const configApi = new MockConfigApi({}); + 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'), @@ -58,6 +62,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 +80,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'), @@ -94,6 +101,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'), @@ -121,6 +129,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'), @@ -155,6 +164,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 6f88178415..44bd5269ea 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,6 +72,7 @@ export default class OAuth2 { static create(options: OAuth2CreateOptions) { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -81,6 +82,7 @@ export default class OAuth2 } = options; const connector = new DefaultAuthConnector({ + configApi, discoveryApi, environment, provider, 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..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,6 +17,7 @@ import OktaAuth from './OktaAuth'; import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import { MockConfigApi } from '@backstage/test-utils'; const PREFIX = 'okta.'; @@ -50,7 +51,9 @@ describe('OktaAuth', () => { [`${PREFIX}profile`, [`${PREFIX}profile`]], [`${PREFIX}openid`, [`${PREFIX}openid`]], ])(`should normalize scopes correctly - %p`, (scope, scopes) => { + const configApi = new MockConfigApi({}); 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 42c0ddd0ed..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,6 +44,7 @@ const OKTA_SCOPE_PREFIX: string = 'okta.'; export default class OktaAuth { static create(options: OAuthApiCreateOptions): typeof oktaAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -52,6 +53,7 @@ export default class OktaAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..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 @@ -18,6 +18,7 @@ import { oneloginAuthApiRef, OAuthRequestApi, AuthProviderInfo, + ConfigApi, DiscoveryApi, } from '@backstage/core-plugin-api'; import { OAuth2 } from '../oauth2'; @@ -27,6 +28,7 @@ import { OAuth2 } from '../oauth2'; * @public */ export type OneLoginAuthCreateOptions = { + configApi?: ConfigApi; discoveryApi: DiscoveryApi; oauthRequestApi: OAuthRequestApi; environment?: string; @@ -61,6 +63,7 @@ export default class OneLoginAuth { options: OneLoginAuthCreateOptions, ): typeof oneloginAuthApiRef.T { const { + configApi, discoveryApi, environment = 'development', provider = DEFAULT_PROVIDER, @@ -68,6 +71,7 @@ export default class OneLoginAuth { } = options; return OAuth2.create({ + configApi, discoveryApi, oauthRequestApi, provider, 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..7a74fd3a3b 100644 --- a/packages/core-app-api/src/apis/implementations/auth/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/types.ts @@ -16,6 +16,7 @@ import { AuthProviderInfo, + ConfigApi, DiscoveryApi, OAuthRequestApi, } from '@backstage/core-plugin-api'; @@ -37,4 +38,5 @@ export type AuthApiCreateOptions = { discoveryApi: DiscoveryApi; environment?: string; provider?: AuthProviderInfo; + configApi?: ConfigApi; }; diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 75b8fb2a34..3d9005b0c0 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -45,7 +45,7 @@ export type BootErrorPageProps = { */ export type SignInPageProps = { /** - * Set the IdentityApi on successful sign in. This should only be called once. + * Set the IdentityApi on successful sign-in. This should only be called once. */ onSignInSuccess(identityApi: IdentityApi): void; }; 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..51da9cbfb5 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,6 +48,7 @@ const defaultOptions = { scopes: new Set(res.scopes.split(' ')), expiresAt: new Date(Date.now() + expiresInSeconds * 1000), }), + configApi: configApi, }; describe('DefaultAuthConnector', () => { @@ -131,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&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({ @@ -179,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&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 988c9f17b1..f2c35b0ba3 100644 --- a/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core-app-api/src/lib/AuthConnector/DefaultAuthConnector.ts @@ -13,16 +13,18 @@ * 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'; +let warned = false; + type Options = { /** * DiscoveryApi instance used to locate the auth backend endpoint. @@ -49,6 +51,10 @@ type Options = { * Function used to transform an auth response into the session type. */ sessionTransform?(response: any): AuthSession | Promise; + /** + * ConfigApi instance used to configure authentication flow of pop-up or redirect. + */ + configApi?: ConfigApi; }; function defaultJoinScopes(scopes: Set) { @@ -69,9 +75,10 @@ export class DefaultAuthConnector 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, discoveryApi, environment, provider, @@ -80,9 +87,26 @@ 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: scopes => this.showPopup(scopes), + onAuthRequest: async scopes => { + if (!this.enableExperimentalRedirectFlow) { + return this.showPopup(scopes); + } + return this.executeRedirect(scopes); + }, }); this.discoveryApi = discoveryApi; @@ -94,6 +118,9 @@ export class DefaultAuthConnector async createSession(options: CreateSessionOptions): Promise { if (options.instantPopup) { + if (this.enableExperimentalRedirectFlow) { + return this.executeRedirect(options.scopes); + } return this.showPopup(options.scopes); } return this.authRequester(options.scopes); @@ -155,6 +182,7 @@ export class DefaultAuthConnector const popupUrl = await this.buildUrl('/start', { scope, origin: window.location.origin, + flow: 'popup', }); const payload = await showLoginPopup({ @@ -168,6 +196,19 @@ export class DefaultAuthConnector return await this.sessionTransform(payload); } + private async executeRedirect(scopes: Set): Promise { + const scope = this.joinScopesFunc(scopes); + // redirect to auth api + window.location.href = await this.buildUrl('/start', { + scope, + origin: window.location.origin, + redirectUrl: window.location.href, + flow: 'redirect', + }); + // 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/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..0159d8f503 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, + configApiRef, + oauthRequestApiRef, +} from '@backstage/core-plugin-api'; import Typography from '@material-ui/core/Typography'; export type OAuthRequestDialogClassKey = @@ -58,6 +62,11 @@ export function OAuthRequestDialog(_props: {}) { const classes = useStyles(); const [busy, setBusy] = useState(false); const oauthRequestApi = useApi(oauthRequestApiRef); + const configApi = useApi(configApiRef); + + const authRedirect = + configApi.getOptionalBoolean('enableExperimentalRedirectFlow') ?? false; + const requests = useObservable( useMemo(() => oauthRequestApi.authRequest$(), [oauthRequestApi]), [], @@ -83,6 +92,11 @@ export function OAuthRequestDialog(_props: {}) { Login Required + {authRedirect ? ( + + This will trigger a http redirect to OAuth Login. + + ) : null} diff --git a/packages/core-components/src/layout/SignInPage/commonProvider.tsx b/packages/core-components/src/layout/SignInPage/commonProvider.tsx index 8f426a3907..1827981e6f 100644 --- a/packages/core-components/src/layout/SignInPage/commonProvider.tsx +++ b/packages/core-components/src/layout/SignInPage/commonProvider.tsx @@ -29,17 +29,24 @@ import { GridItem } from './styles'; import { ForwardedError } from '@backstage/errors'; import { UserIdentity } from './UserIdentity'; -const Component: ProviderComponent = ({ config, onSignInSuccess }) => { +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 { + onSignInStarted(); const identityResponse = await authApi.getBackstageIdentity({ instantPopup: true, }); if (!identityResponse) { + onSignInFailure(); throw new Error( `The ${title} provider is not configured to support sign-in`, ); @@ -55,6 +62,7 @@ const Component: ProviderComponent = ({ config, onSignInSuccess }) => { }), ); } catch (error) { + 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 23d6c95b3c..5e7ab7b7be 100644 --- a/packages/core-components/src/layout/SignInPage/providers.tsx +++ b/packages/core-components/src/layout/SignInPage/providers.tsx @@ -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,16 +173,24 @@ export const useSignInProviders = ( const { Component } = provider.components; const handleSignInSuccess = (result: IdentityApi) => { - localStorage.setItem(PROVIDER_STORAGE_KEY, provider.id); - handleWrappedResult(result); }; + const handleSignInStarted = () => { + localStorage.setItem(PROVIDER_STORAGE_KEY, provider.config!.id); + }; + + const handleSignInFailure = () => { + localStorage.removeItem(PROVIDER_STORAGE_KEY); + }; + return ( ); }), diff --git a/packages/core-components/src/layout/SignInPage/types.ts b/packages/core-components/src/layout/SignInPage/types.ts index 8a0430a27d..a7e1ca62c2 100644 --- a/packages/core-components/src/layout/SignInPage/types.ts +++ b/packages/core-components/src/layout/SignInPage/types.ts @@ -35,8 +35,21 @@ 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 } + SignInPageProps & { + config: SignInProviderConfig; + onSignInStarted(): void; + onSignInFailure(): void; + } >; export type ProviderLoader = ( 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/api-report.md b/plugins/auth-backend/api-report.md index 63aa060875..7515083c09 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -398,6 +398,8 @@ export type OAuthState = { env: string; origin?: string; scope?: string; + redirectUrl?: string; + flow?: string; }; // @public diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index e0d4f0e051..1b6653d97e 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -83,56 +83,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 +94,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 +164,26 @@ 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, + }); + + const state = { + ...defaultState, + redirectUrl: 'http://localhost:3000', + flow: 'redirect', + }; + const mockRequest = createEncodedQueryMockRequest(state); + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockResponse.redirect).toHaveBeenCalledTimes(1); }); it('persists scope through cookie if enabled', async () => { @@ -179,32 +201,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 +228,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 +236,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 +252,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 +262,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 +281,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 +291,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 +300,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 +313,7 @@ describe('OAuthAdapter', () => { }, }, }); + expect(mockResponse.redirect).not.toHaveBeenCalled(); }); it('sets new access-token when old cookie exists', async () => { @@ -337,20 +323,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 +336,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 () => { @@ -380,162 +352,89 @@ describe('OAuthAdapter', () => { ...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, + }; + 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 +442,108 @@ 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 flow query string is set to "redirect"', 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, + }; + 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', + flow: 'redirect', + }; + + 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(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..5f51b2d301 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -33,7 +33,11 @@ import { NotAllowedError, } from '@backstage/errors'; import { defaultCookieConfigurer, readState, verifyNonce } from './helpers'; -import { postMessageResponse, ensuresXRequestedWith } from '../flow'; +import { + postMessageResponse, + ensuresXRequestedWith, + WebMessageResponse, +} from '../flow'; import { OAuthHandlers, OAuthStartRequest, @@ -98,6 +102,8 @@ 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(); + const flow = req.query.flow?.toString(); if (!env) { throw new InputError('No env provided in request query parameters'); @@ -109,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 }; + 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 @@ -169,11 +175,21 @@ 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 (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); } 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..e960af2988 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -90,6 +90,8 @@ export type OAuthState = { env: string; origin?: string; scope?: string; + redirectUrl?: string; + flow?: string; }; /** @public */ diff --git a/plugins/gitops-profiles/package.json b/plugins/gitops-profiles/package.json index 4156a20f00..4474e05300 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..eb58e03580 100644 --- a/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx +++ b/plugins/gitops-profiles/src/components/ProfileCatalog/ProfileCatalog.test.tsx @@ -14,7 +14,11 @@ * 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'; @@ -31,6 +35,7 @@ import { githubAuthApiRef } from '@backstage/core-plugin-api'; describe('ProfileCatalog', () => { it('should render', async () => { const oauthRequestApi = new OAuthRequestManager(); + const configApi = new MockConfigApi({}); 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 436474b158..d8a50af792 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6420,6 +6420,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:^"