fix: clean up CoderAuthInputForm

Parkreiner · Parkreiner · commit 90695aee63a3 · 2025-10-14T00:48:51.000-04:00
diff --git a/packages/app/src/components/catalog/EntityPage.tsx b/packages/app/src/components/catalog/EntityPage.tsx
@@ -136,11 +136,6 @@ const coderAppConfig: CoderAppConfig = {
     accessUrl: 'https://dev.coder.com',
   },
 
-  oauth: {
-    clientId: '09cd00cf-9517-401c-9601-3712f187b53c',
-    backendUrl: 'http://localhost:7007',
-  },
-
   workspaces: {
     defaultTemplateName: 'devcontainers',
     defaultMode: 'manual',
diff --git a/plugins/backstage-plugin-coder/dev/DevPage.tsx b/plugins/backstage-plugin-coder/dev/DevPage.tsx
@@ -23,11 +23,6 @@ const appConfig: CoderAppConfig = {
     accessUrl: 'https://dev.coder.com',
   },
 
-  oauth: {
-    clientId: '09cd00cf-9517-401c-9601-3712f187b53c',
-    backendUrl: 'http://localhost:7007',
-  },
-
   workspaces: {
     defaultTemplateName: 'devcontainers',
     defaultMode: 'manual',
diff --git a/plugins/backstage-plugin-coder/src/components/CoderAuthForm/CoderAuthInputForm.tsx b/plugins/backstage-plugin-coder/src/components/CoderAuthForm/CoderAuthInputForm.tsx
@@ -1,18 +1,19 @@
-import React, { type FormEvent, useState, useEffect } from 'react';
+import React, { useState, useEffect } from 'react';
 import { useId } from '../../hooks/hookPolyfills';
 import {
   type CoderAuthStatus,
   useCoderAppConfig,
   useInternalCoderAuth,
 } from '../CoderProvider';
-
 import { CoderLogo } from '../CoderLogo';
 import { Link, LinkButton } from '@backstage/core-components';
 import { VisuallyHidden } from '../VisuallyHidden';
 import { makeStyles } from '@material-ui/core';
 import TextField from '@material-ui/core/TextField';
 import ErrorIcon from '@material-ui/icons/ErrorOutline';
 import SyncIcon from '@material-ui/icons/Sync';
+import { configApiRef, errorApiRef, useApi } from '@backstage/core-plugin-api';
+import { useUrlSync } from '../../hooks/useUrlSync';
 
 const useStyles = makeStyles(theme => ({
   formContainer: {
@@ -45,88 +46,128 @@ const useStyles = makeStyles(theme => ({
   },
 
   oauthSection: {
-    marginTop: theme.spacing(3),
-    marginBottom: theme.spacing(2),
+    paddingTop: theme.spacing(0.5),
+    paddingBottom: theme.spacing(0.5),
   },
 
   oauthButton: {
     display: 'block',
+    // Deliberately making this button bigger than the token button, because we
+    // want to start pushing users to use oauth as the default. The old token
+    // approach may end up getting deprecated
     width: '100%',
-    maxWidth: '100%',
   },
 
   divider: {
     display: 'flex',
     alignItems: 'center',
     textAlign: 'center',
-    marginTop: theme.spacing(2),
-    marginBottom: theme.spacing(2),
+    paddingTop: theme.spacing(2),
+    paddingBottom: theme.spacing(0.5),
 
     '&::before, &::after': {
       content: '""',
-      flex: 1,
+      flexGrow: 1,
       borderBottom: `1px solid ${theme.palette.divider}`,
     },
   },
 
   dividerText: {
+    textTransform: 'uppercase',
     padding: `0 ${theme.spacing(1)}px`,
     color: theme.palette.text.secondary,
-    fontSize: '0.875rem',
+    fontSize: '0.75rem',
+    fontWeight: 500,
+  },
+
+  tokenSection: {
+    paddingTop: `${theme.spacing(1.5)}px`,
+  },
+
+  tokenInstructions: {
+    margin: 0,
+    marginBottom: `-${theme.spacing(0.5)}px`,
   },
 }));
 
 export const CoderAuthInputForm = () => {
   const hookId = useId();
   const styles = useStyles();
   const appConfig = useCoderAppConfig();
+  const urlSync = useUrlSync();
+  const configApi = useApi(configApiRef);
+  const errorApi = useApi(errorApiRef);
   const { status, registerNewToken } = useInternalCoderAuth();
 
+  const backendUrl = urlSync.state.baseUrl;
   useEffect(() => {
-    const handleOAuthMessage = (event: MessageEvent) => {
-      // Verify the message is from our OAuth callback backend
-      const backendUrl = appConfig.oauth?.backendUrl;
-
-      // If backendUrl is configured, verify the origin matches
-      if (backendUrl) {
-        const backendOrigin = new URL(backendUrl).origin;
-        if (event.origin !== backendOrigin) {
-          return;
-        }
+    if (!backendUrl) {
+      return undefined;
+    }
+
+    const onOauthMessage = (event: MessageEvent<unknown>): void => {
+      // Even though we're going to add the event listener to the window object
+      // directly, we still want to make sure that the event originated on the
+      // window, and wasn't received from a DOM node via event bubbling
+      if (event.target !== window) {
+        return;
       }
 
-      if (event.data?.type === 'coder-oauth-success' && event.data?.token) {
-        registerNewToken(event.data.token);
+      const backendOrigin = new URL(backendUrl).origin;
+      const originMismatch = event.origin !== backendOrigin;
+      if (originMismatch) {
+        return;
       }
-    };
 
-    window.addEventListener('message', handleOAuthMessage);
-    return () => window.removeEventListener('message', handleOAuthMessage);
-  }, [registerNewToken, appConfig.oauth?.backendUrl]);
-
-  const onSubmit = (event: FormEvent<HTMLFormElement>) => {
-    event.preventDefault();
-    const formData = Object.fromEntries(new FormData(event.currentTarget));
-    const newToken =
-      typeof formData.authToken === 'string' ? formData.authToken : '';
+      const { data } = event;
+      const messageIsOauthPayload =
+        typeof data === 'object' && data !== null && 'token' in data;
+      if (!messageIsOauthPayload) {
+        return;
+      }
+      // For some reason, TypeScript won't narrow properly if you move this
+      // check to the messageIsOauthPayload boolean
+      if (typeof data.token === 'string') {
+        registerNewToken(data.token);
+      }
+    };
 
-    registerNewToken(newToken);
-  };
+    window.addEventListener('message', onOauthMessage);
+    return () => window.removeEventListener('message', onOauthMessage);
+  }, [registerNewToken, backendUrl]);
 
   const handleOAuthLogin = () => {
-    const authUrl = `${appConfig.deployment.accessUrl}/oauth2/authorize`;
-    const clientId = appConfig.oauth?.clientId || 'backstage';
-    const backendUrl =
-      appConfig.oauth?.backendUrl ||
-      `${window.location.protocol}//${window.location.hostname}:7007`;
-    const redirectUri = `${backendUrl}/api/auth/coder/oauth/callback`;
-    const state = btoa(JSON.stringify({ returnTo: window.location.pathname }));
-
-    const oauthUrl = `${authUrl}?client_id=${clientId}&redirect_uri=${encodeURIComponent(
-      redirectUri,
-    )}&response_type=code&state=${state}`;
-
-    // Open OAuth flow in popup window
+    const clientId = configApi.getOptionalString('coder.oauth.clientId');
+    if (!clientId) {
+      errorApi.post(
+        {
+          name: 'Coder oauth clientId is missing',
+          message:
+            'Please see plugin documentation for how to add clientId to your Backstage deployment',
+        },
+        { hidden: false },
+      );
+      return;
+    }
+
+    const params = new URLSearchParams({
+      /**
+       * @todo See what we can do to move the state calculations to the backend.
+       * The state should actually be cryptographically generated and should
+       * have a high number of bits of entropy, too.
+       */
+      state: btoa(JSON.stringify({ returnTo: window.location.pathname })),
+      response_type: 'code',
+      client_id: clientId,
+      redirect_uri: encodeURIComponent(
+        `${backendUrl}/api/auth/coder/oauth/callback`,
+      ),
+    });
+
+    const oauthUrl = `${
+      appConfig.deployment.accessUrl
+    }/oauth2/authorize?${params.toString()}`;
+
     const width = 600;
     const height = 700;
     const left = window.screen.width / 2 - width / 2;
@@ -148,7 +189,14 @@ export const CoderAuthInputForm = () => {
     <form
       aria-labelledby={formHeaderId}
       className={styles.formContainer}
-      onSubmit={onSubmit}
+      onSubmit={event => {
+        event.preventDefault();
+        const formData = Object.fromEntries(new FormData(event.currentTarget));
+        const newToken =
+          typeof formData.authToken === 'string' ? formData.authToken : '';
+
+        registerNewToken(newToken);
+      }}
     >
       <h3 hidden id={formHeaderId}>
         Authenticate with Coder
@@ -170,56 +218,64 @@ export const CoderAuthInputForm = () => {
           className={styles.oauthButton}
           onClick={handleOAuthLogin}
         >
-          Sign in with Coder
+          Sign in with Coder OAuth
         </LinkButton>
       </div>
 
       <div className={styles.divider}>
-        <span className={styles.dividerText}>OR</span>
+        <span className={styles.dividerText}>or</span>
       </div>
 
-      <p>
-        Alternatively, enter a token from your{' '}
-        <Link to={`${appConfig.deployment.accessUrl}/cli-auth`} target="_blank">
-          Coder deployment's token page
-          <VisuallyHidden> (link opens in new tab)</VisuallyHidden>
-        </Link>
-        .
-      </p>
-
-      <fieldset className={styles.authInputFieldset} aria-labelledby={legendId}>
-        <legend hidden id={legendId}>
-          Auth input
-        </legend>
-
-        <TextField
-          // Adding the label prop directly to the TextField will place a label
-          // in the HTML, so sighted users are fine. But for some reason, it
-          // won't connect the label and input together, which breaks
-          // accessibility for screen readers. Need to wire up extra IDs, sadly.
-          label="Auth token"
-          InputLabelProps={{ htmlFor: authTokenInputId }}
-          InputProps={{ id: authTokenInputId }}
-          required
-          name="authToken"
-          type="password"
-          defaultValue=""
-          aria-errormessage={warningBannerId}
-          style={{ width: '100%' }}
-        />
-
-        <LinkButton
-          disableRipple
-          to=""
-          component="button"
-          type="submit"
-          color="primary"
-          variant="contained"
-          className={styles.authButton}
+      <div className={styles.tokenSection}>
+        <p className={styles.tokenInstructions}>
+          Enter a token from your{' '}
+          <Link
+            to={`${appConfig.deployment.accessUrl}/cli-auth`}
+            target="_blank"
+          >
+            Coder deployment's token page
+            <VisuallyHidden> (link opens in new tab)</VisuallyHidden>
+          </Link>
+          .
+        </p>
+
+        <fieldset
+          className={styles.authInputFieldset}
+          aria-labelledby={legendId}
         >
-          Authenticate
-        </LinkButton>
-      </fieldset>
+          <legend hidden id={legendId}>
+            Auth input
+          </legend>
+
+          <TextField
+            // Adding the label prop directly to the TextField will place a label
+            // in the HTML, so sighted users are fine. But for some reason, it
+            // won't connect the label and input together, which breaks
+            // accessibility for screen readers. Need to wire up extra IDs, sadly.
+            label="Auth token"
+            InputLabelProps={{ htmlFor: authTokenInputId }}
+            InputProps={{ id: authTokenInputId }}
+            required
+            name="authToken"
+            type="password"
+            defaultValue=""
+            aria-errormessage={warningBannerId}
+            style={{ width: '100%' }}
+          />
+
+          <LinkButton
+            disableRipple
+            to=""
+            component="button"
+            type="submit"
+            color="primary"
+            variant="contained"
+            className={styles.authButton}
+          >
+            Use token
+          </LinkButton>
+        </fieldset>
+      </div>
 
       {(status === 'invalid' || status === 'authenticating') && (
         <InvalidStatusNotifier authStatus={status} bannerId={warningBannerId} />
diff --git a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAppConfigProvider.tsx b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAppConfigProvider.tsx
@@ -10,11 +10,6 @@ export type CoderAppConfig = Readonly<{
     accessUrl: string;
   }>;
 
-  oauth?: Readonly<{
-    clientId?: string;
-    backendUrl?: string;
-  }>;
-
   // Type is meant to be used with YamlConfig from useCoderWorkspacesConfig;
   // not using a mapped type because there's just enough differences that
   // maintaining a relationship that way would be a nightmare of ternaries
