-
Notifications
You must be signed in to change notification settings - Fork 0
fix(auth): mask break-glass allowlist membership behind INVALID_CREDENTIALS #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,15 +68,6 @@ export const emailPasswordRoute = new Hono<HonoAuthContext>() | |
|
|
||
| const { email, password, totpCode, backupCode, csrfToken, captchaToken } = c.req.valid('json'); | ||
|
|
||
| // Break-glass: when password signin is disabled suite-wide, allowlisted | ||
| // admin emails (see NEXT_PRIVATE_BREAK_GLASS_EMAILS) may still sign in | ||
| // via /signin?direct=1 while the OIDC provider is unreachable. | ||
| if (!isSigninEnabledForProvider('email') && !isBreakGlassEmail(email)) { | ||
| throw new AppError(AuthenticationErrorCode.SigninDisabled, { | ||
| statusCode: 400, | ||
| }); | ||
| } | ||
|
|
||
| const loginLimitResult = await loginRateLimit.check({ | ||
| ip: requestMetadata.ipAddress ?? 'unknown', | ||
| identifier: email, | ||
|
|
@@ -104,6 +95,18 @@ export const emailPasswordRoute = new Hono<HonoAuthContext>() | |
| ipAddress: requestMetadata.ipAddress, | ||
| }); | ||
|
|
||
| // Break-glass: when password signin is disabled suite-wide, allowlisted | ||
| // admin emails (see NEXT_PRIVATE_BREAK_GLASS_EMAILS) may still sign in | ||
| // via /signin?direct=1 while the OIDC provider is unreachable. The gate | ||
| // sits after the rate limit and CSRF/captcha checks and rejects with the | ||
| // same INVALID_CREDENTIALS error as a wrong password, so probing the | ||
| // endpoint cannot reveal which emails are on the allowlist. | ||
| if (!isSigninEnabledForProvider('email') && !isBreakGlassEmail(email)) { | ||
| throw new AppError(AuthenticationErrorCode.InvalidCredentials, { | ||
| message: 'Invalid email or password', | ||
| }); | ||
| } | ||
|
Comment on lines
+98
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this change successfully masks the error code behind The Issue
An attacker can easily measure the response times of the The SolutionTo eliminate this timing difference, the break-glass check should be performed after the password comparison. This ensures that both break-glass and non-break-glass emails undergo the exact same database lookup and bcrypt comparison flow, making their response times indistinguishable. Move the check to run immediately after the password comparison (around line 141): const isPasswordsSame = await compare(password, user.password);
if (!isPasswordsSame) {
// ... existing audit log and error throwing ...
}
// Perform the break-glass check here
if (!isSigninEnabledForProvider('email') && !isBreakGlassEmail(email)) {
throw new AppError(AuthenticationErrorCode.InvalidCredentials, {
message: 'Invalid email or password',
});
} |
||
|
|
||
| if (email.toLowerCase() === legacyServiceAccountEmail() || email.toLowerCase() === deletedServiceAccountEmail()) { | ||
| return c.text('FORBIDDEN', 403); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🛡️ Detected with Advanced Tier | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: DOS/Crove-Sign
Length of output: 21299
Information Disclosure
Reachability: External
Exploitability: Moderate
CWE: CWE-208
Equalize the full rejected-request path before applying the sign-in gate.
The blocked path skips the user lookup, password comparison, and failed-login audit work that an allowlisted wrong-password request performs. A dummy comparison alone does not remove the timing signal. Make both paths execute equivalent work before returning
InvalidCredentials, and add a focused regression test for blocked and allowlisted wrong-password requests.🤖 Prompt for AI Agents
Source: Learnings