From 0f14dcce62414b2b02c819ce7bc653f09cf86fd3 Mon Sep 17 00:00:00 2001
From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com>
Date: Mon, 22 Jun 2026 10:10:59 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20fix:=20Add=20ban=20check=20and?=
 =?UTF-8?q?=20fix=20domain=20allowlist=20on=20admin=20OAuth=20refresh?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two gaps in the /api/admin/oauth/refresh route:

Add middleware.checkBan to the route chain before preAuthTenantMiddleware,
matching the gate that /login/local and createOAuthHandler already apply.
Without it a banned admin could keep minting JWTs until their IdP refresh
token expired.

Replace getAppConfig({ baseOnly: true }) in the non-tenant isEmailAllowed
closure with getAppConfig({ role: user.role }), which includes DB-layer
overrides from the admin panel. baseOnly returns only YAML-derived config,
so any allowedDomains list maintained entirely through the admin panel was
silently inert on this path. Extract isEmailAllowedForUser as a shared
helper, move it into buildAdminRefreshClosures so both Google and OpenID
refresh paths enforce domain policy consistently, and add isEmailAllowed
to AdminRefreshDeps in the TS package so applyAdminRefresh can invoke it.
---
 api/server/routes/admin/auth.js  | 29 +++++++++++++++--------------
 packages/api/src/auth/refresh.ts | 15 +++++++++++++++
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/api/server/routes/admin/auth.js b/api/server/routes/admin/auth.js
index 6c56e04938..b8a69f8a67 100644
--- a/api/server/routes/admin/auth.js
+++ b/api/server/routes/admin/auth.js
@@ -71,6 +71,19 @@ function resolveRequestOrigin(req) {
   }
 }
 
+async function isEmailAllowedForUser(user) {
+  if (!user?.email) return false;
+  try {
+    const appConfig = user.tenantId
+      ? await resolveAppConfigForUser(getAppConfig, user)
+      : await getAppConfig({ role: user.role ?? '' });
+    return isEmailDomainAllowed(user.email, appConfig?.registration?.allowedDomains);
+  } catch (err) {
+    logger.warn(`[admin/oauth/refresh] domain allowlist check failed, denying: ${err?.message}`);
+    return false;
+  }
+}
+
 function buildAdminRefreshClosures(sessionExpiry) {
   return {
     canAccessAdmin: async (user) => {
@@ -88,6 +101,7 @@ function buildAdminRefreshClosures(sessionExpiry) {
         return false;
       }
     },
+    isEmailAllowed: isEmailAllowedForUser,
     mintToken: async (user) => ({
       token: await generateToken(user, sessionExpiry),
       expiresAt: Date.now() + sessionExpiry,
@@ -100,20 +114,6 @@ function buildGoogleAdminRefreshDeps(sessionExpiry) {
     findUsers,
     getUserById,
     ...buildAdminRefreshClosures(sessionExpiry),
-    isEmailAllowed: async (user) => {
-      if (!user?.email) return false;
-      try {
-        const appConfig = user.tenantId
-          ? await resolveAppConfigForUser(getAppConfig, user)
-          : await getAppConfig({ baseOnly: true });
-        return isEmailDomainAllowed(user.email, appConfig?.registration?.allowedDomains);
-      } catch (err) {
-        logger.warn(
-          `[admin/oauth/refresh] domain allowlist check failed, denying: ${err?.message}`,
-        );
-        return false;
-      }
-    },
   };
 }
 
@@ -564,6 +564,7 @@ router.post('/oauth/exchange', middleware.loginLimiter, async (req, res) => {
 router.post(
   '/oauth/refresh',
   middleware.loginLimiter,
+  middleware.checkBan,
   preAuthTenantMiddleware,
   async (req, res) => {
     try {
diff --git a/packages/api/src/auth/refresh.ts b/packages/api/src/auth/refresh.ts
index 2ae5ca0f0a..ba35c15f4c 100644
--- a/packages/api/src/auth/refresh.ts
+++ b/packages/api/src/auth/refresh.ts
@@ -65,6 +65,13 @@ export interface AdminRefreshDeps {
    * bearers should always inject this.
    */
   canAccessAdmin?: (user: IUser) => Promise<boolean>;
+  /**
+   * Re-runs the deployment's `registration.allowedDomains` check against the
+   * resolved user's email. Returns true to allow refresh, false to reject.
+   * Mirrors the domain check the initial OAuth callback enforces so a domain
+   * removed from the allowlist after issuance can't refresh.
+   */
+  isEmailAllowed?: (user: IUser) => Promise<boolean>;
   /**
    * Optional post-success hook for forks that need to do additional work
    * with the refreshed tokenset and resolved user (e.g. update a server-side
@@ -275,6 +282,14 @@ export async function applyAdminRefresh(
     throw new AdminRefreshError('USER_NOT_FOUND', 401, 'No user found for the refreshed identity');
   }
 
+  if (deps.isEmailAllowed && !(await deps.isEmailAllowed(user))) {
+    throw new AdminRefreshError(
+      'FORBIDDEN',
+      403,
+      'User email domain is not on the deployment allowlist',
+    );
+  }
+
   if (deps.canAccessAdmin && !(await deps.canAccessAdmin(user))) {
     throw new AdminRefreshError('FORBIDDEN', 403, 'User does not have admin access');
   }