simplify code and test cases. clean up rendering.

Signed-off-by: headphonejames <generalfuzz@gmail.com>

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

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

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,

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({

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,

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,

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 () => {

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(),

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;
 };

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 (
-      <Component
-        onSignInSuccess={setIdentityApi}
-        onSignInStarted={handleSignInStarted}
-        onSignInFailure={handleSignInFailure}
-      />
-    );
+    return <Component onSignInSuccess={setIdentityApi} />;
   }
 
   appIdentityProxy.setTarget(identityApi, {

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;
 };
 
 /**

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<AuthSession> = {
   /**
    * DiscoveryApi instance used to locate the auth backend endpoint.
@@ -52,7 +54,7 @@ type Options<AuthSession> = {
   /**
    * ConfigApi instance used to configure authentication flow of pop-up or redirect.
    */
-  configApi: ConfigApi;
+  configApi?: ConfigApi;
 };
 
 function defaultJoinScopes(scopes: Set<string>) {
@@ -68,12 +70,12 @@ export class DefaultAuthConnector<AuthSession>
   implements AuthConnector<AuthSession>
 {
   private readonly discoveryApi: DiscoveryApi;
-  private readonly configApi: ConfigApi;
   private readonly environment: string;
   private readonly provider: AuthProviderInfo;
   private readonly joinScopesFunc: (scopes: Set<string>) => string;
   private readonly authRequester: OAuthRequester<AuthSession>;
   private readonly sessionTransform: (response: any) => Promise<AuthSession>;
+  private readonly enableExperimentalRedirectFlow: boolean;
   constructor(options: Options<AuthSession>) {
     const {
       configApi,
@@ -85,20 +87,28 @@ export class DefaultAuthConnector<AuthSession>
       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<AuthSession>
   }
 
   async createSession(options: CreateSessionOptions): Promise<AuthSession> {
-    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);

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: {}) {
           <Typography className={classes.titleHeading} variant="h1">
             Login Required
           </Typography>
-          <Typography>{redirectMessage}</Typography>
+          {authRedirect ? (
+            <Typography>
+              This will trigger a http redirect to OAuth Login.
+            </Typography>
+          ) : null}
         </DialogTitle>
 
         <DialogContent dividers classes={{ root: classes.contentList }}>

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 = (

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

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<void>;
+
   /**
    * Observe the current state of the auth session. Emits the current state on subscription.
    */

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

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