🔒 fix: Add ban check and fix domain allowlist on admin OAuth refresh

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.
This commit is contained in:
Dustin Healy 2026-06-22 10:10:59 -07:00
parent 4b4cecb48f
commit 0f14dcce62
2 changed files with 30 additions and 14 deletions

View file

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

View file

@ -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');
}