Add onSignInStarted() and onSignInFailure() hooks. Clean up and renaming.

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

packages/core-app-api/src/app/types.ts

@@ -45,9 +45,17 @@ export type BootErrorPageProps = {
  */
 export type SignInPageProps = {
   /**
-   * Set the IdentityApi on successful sign in. This should only be called once.
+   * Invoked when the sign-in process has started.
+   */
+  onSignInStarted(): void;
+  /**
+   * Set the IdentityApi on successful sign-in. This should only be called once.
    */
   onSignInSuccess(identityApi: IdentityApi): void;
+  /**
+   * Invoked when the sign-in process has failed.
+   */
+  onSignInFailure(): void;
 };
 
 /**

packages/core-components/src/layout/SignInPage/commonProvider.tsx

@@ -28,21 +28,25 @@ import { useApi, errorApiRef } from '@backstage/core-plugin-api';
 import { GridItem } from './styles';
 import { ForwardedError } from '@backstage/errors';
 import { UserIdentity } from './UserIdentity';
-import { PROVIDER_STORAGE_KEY } from './providers';
 
-const Component: ProviderComponent = ({ config, onSignInSuccess }) => {
-  const { apiRef, title, message, id } = config as SignInProviderConfig;
+const Component: ProviderComponent = ({
+  config,
+  onSignInStarted,
+  onSignInSuccess,
+  onSignInFailure,
+}) => {
+  const { apiRef, title, message } = config as SignInProviderConfig;
   const authApi = useApi(apiRef);
   const errorApi = useApi(errorApiRef);
 
   const handleLogin = async () => {
     try {
-      localStorage.setItem(PROVIDER_STORAGE_KEY, id);
+      onSignInStarted();
       const identityResponse = await authApi.getBackstageIdentity({
         instantPopup: true,
       });
       if (!identityResponse) {
-        localStorage.removeItem(PROVIDER_STORAGE_KEY);
+        onSignInFailure();
         throw new Error(
           `The ${title} provider is not configured to support sign-in`,
         );
@@ -58,7 +62,7 @@ const Component: ProviderComponent = ({ config, onSignInSuccess }) => {
         }),
       );
     } catch (error) {
-      localStorage.removeItem(PROVIDER_STORAGE_KEY);
+      onSignInFailure();
       errorApi.post(new ForwardedError('Login failed', error));
     }
   };

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<Data>({
     mode: 'onChange',
@@ -70,6 +70,7 @@ const Component: ProviderComponent = ({ onSignInSuccess }) => {
   const { errors } = formState;
 
   const handleResult = ({ userId, idToken }: Data) => {
+    onSignInStarted();
     onSignInSuccess(
       UserIdentity.fromLegacy({
         userId,

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 }) => (
   <GridItem>
     <InfoCard
       title="Guest"
@@ -31,7 +31,10 @@ const Component: ProviderComponent = ({ onSignInSuccess }) => (
         <Button
           color="primary"
           variant="outlined"
-          onClick={() => onSignInSuccess(new GuestUserIdentity())}
+          onClick={() => {
+            onSignInStarted();
+            onSignInSuccess(new GuestUserIdentity());
+          }}
         >
           Enter
         </Button>

packages/core-components/src/layout/SignInPage/providers.tsx

@@ -176,11 +176,21 @@ export const useSignInProviders = (
           handleWrappedResult(result);
         };
 
+        const handleSignInStarted = () => {
+          localStorage.setItem(PROVIDER_STORAGE_KEY, provider.config!.id);
+        };
+
+        const handleSignInFailure = () => {
+          localStorage.removeItem(PROVIDER_STORAGE_KEY);
+        };
+
         return (
           <Component
             key={provider.id}
             config={provider.config!}
+            onSignInStarted={handleSignInStarted}
             onSignInSuccess={handleSignInSuccess}
+            onSignInFailure={handleSignInFailure}
           />
         );
       }),

plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts

@@ -178,7 +178,7 @@ describe('OAuthAdapter', () => {
     const state = {
       ...defaultState,
       redirectUrl: 'http://localhost:3000',
-      authFlow: 'redirect',
+      flow: 'redirect',
     };
     const mockRequest = createEncodedQueryMockRequest(state);
 
@@ -506,7 +506,7 @@ describe('OAuthAdapter', () => {
     expect(mockResponse.redirect).not.toHaveBeenCalled();
   });
 
-  it('executed a response redirect when authFlow query string is set to "redirect"', async () => {
+  it('executed a response redirect when flow query string is set to "redirect"', async () => {
     const handlers = {
       start: jest.fn(async (_req: { state: OAuthState }) => ({
         url: '/url',
@@ -531,7 +531,7 @@ describe('OAuthAdapter', () => {
       ...defaultState,
       origin: 'http://other.domain',
       redirectUrl: 'http://domain.org',
-      authFlow: 'redirect',
+      flow: 'redirect',
     };
 
     const mockRequest = {

plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts

@@ -55,7 +55,6 @@ export type OAuthAdapterOptions = {
   providerId: string;
   persistScopes?: boolean;
   appOrigin: string;
-  redirectUrl?: string;
   baseUrl: string;
   cookieConfigurer: CookieConfigurer;
   isOriginAllowed: (origin: string) => boolean;
@@ -69,7 +68,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
     handlers: OAuthHandlers,
     options: Pick<
       OAuthAdapterOptions,
-      'providerId' | 'persistScopes' | 'callbackUrl' | 'redirectUrl'
+      'providerId' | 'persistScopes' | 'callbackUrl'
     >,
   ): OAuthAdapter {
     const { appUrl, baseUrl, isOriginAllowed } = config;
@@ -104,7 +103,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
     const env = req.query.env?.toString();
     const origin = req.query.origin?.toString();
     const redirectUrl = req.query.redirectUrl?.toString();
-    const authFlow = req.query.authFlow?.toString();
+    const flow = req.query.authFlow?.toString();
     if (!env) {
       throw new InputError('No env provided in request query parameters');
     }
@@ -115,7 +114,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
     // set a nonce cookie before redirecting to oauth provider
     this.setNonceCookie(res, nonce, cookieConfig);
 
-    const state: OAuthState = { nonce, env, origin, redirectUrl, authFlow };
+    const state: OAuthState = { nonce, env, origin, redirectUrl, flow: flow };
 
     // If scopes are persisted then we pass them through the state so that we
     // can set the cookie on successful auth
@@ -142,7 +141,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
 
     try {
       const state: OAuthState = readState(req.query.state?.toString() ?? '');
-      const redirectUrl = state.redirectUrl ?? '';
 
       if (state.origin) {
         try {
@@ -181,8 +179,13 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
         response: { ...response, backstageIdentity: identity },
       };
 
-      if (state.authFlow === 'redirect') {
-        res.redirect(redirectUrl);
+      if (state.flow === 'redirect') {
+        if (!state.redirectUrl) {
+          throw new InputError(
+            'No redirectUrl provided in request query parameters',
+          );
+        }
+        res.redirect(state.redirectUrl);
       }
       // post message back to popup if successful
       return postMessageResponse(res, appOrigin, responseObj);

plugins/auth-backend/src/lib/oauth/types.ts

@@ -91,7 +91,7 @@ export type OAuthState = {
   origin?: string;
   scope?: string;
   redirectUrl?: string;
-  authFlow?: string;
+  flow?: string;
 };
 
 /** @public */

plugins/auth-backend/src/service/router.ts

@@ -107,7 +107,6 @@ export async function createRouter(
     ...providerFactories,
   };
   const providersConfig = config.getConfig('auth.providers');
-
   const configuredProviders = providersConfig.keys();
 
   const isOriginAllowed = createOriginFilter(config);