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')], [