-
-
Notifications
You must be signed in to change notification settings - Fork 330
fix: disabled-key 429 lockout, slow availability page, public-status Redis leak #1187
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
8651627
b5d7449
d314561
26d3b9f
c983b64
5cd7594
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 |
|---|---|---|
|
|
@@ -2,11 +2,51 @@ import { extractApiKeyFromHeaders as sharedExtractApiKeyFromHeaders } from "@/li | |
| import { getClientIpWithFreshSettings } from "@/lib/ip"; | ||
| import { logger } from "@/lib/logger"; | ||
| import { LoginAbusePolicy } from "@/lib/security/login-abuse-policy"; | ||
| import { validateApiKeyAndGetUser } from "@/repository/key"; | ||
| import { ERROR_CODES, getErrorMessageServer } from "@/lib/utils/error-messages"; | ||
| import { resolveApiKeyAuthOutcome } from "@/repository/key"; | ||
| import { markUserExpired } from "@/repository/user"; | ||
| import { GEMINI_PROTOCOL } from "../gemini/protocol"; | ||
| import { ProxyResponses } from "./responses"; | ||
| import type { AuthState, ProxySession } from "./session"; | ||
| import type { AuthFailureKind, AuthState, ProxySession } from "./session"; | ||
|
|
||
| async function getRequestLocale(): Promise<string> { | ||
| // Match the rate-limit-guard pattern: read next-intl's request locale | ||
| // (Accept-Language or cookie) lazily inside the guard so the proxy hot | ||
| // path does not pay the import cost on every cold start. | ||
| const { getLocale } = await import("next-intl/server"); | ||
| return await getLocale(); | ||
| } | ||
|
|
||
| /** | ||
| * Build an auth failure AuthState. The factory exists so every call site is | ||
| * forced (by the function signature) to supply both `failureKind` and an | ||
| * `errorResponse` — preventing the footgun where a new failure branch could | ||
| * forget to tag itself and silently get classified as a credentials failure | ||
| * by the brute-force rate limiter. | ||
| */ | ||
| function buildAuthFailure(params: { | ||
| failureKind: AuthFailureKind; | ||
| errorResponse: Response; | ||
| apiKey?: string | null; | ||
| }): AuthState { | ||
| return { | ||
| user: null, | ||
| key: null, | ||
| apiKey: params.apiKey ?? null, | ||
| success: false, | ||
| failureKind: params.failureKind, | ||
| errorResponse: params.errorResponse, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Exhaustiveness helper. Calling this from a "should be unreachable" branch | ||
| * gives a compile-time error when a new union member is added without an | ||
| * explicit handler. | ||
| */ | ||
| function assertNever(value: never, context: string): never { | ||
| throw new Error(`Unhandled discriminant in ${context}: ${JSON.stringify(value)}`); | ||
| } | ||
|
|
||
| /** | ||
| * Pre-auth rate limiter: throttles repeated authentication failures per IP | ||
|
|
@@ -73,8 +113,13 @@ export class ProxyAuthenticator { | |
| return null; | ||
| } | ||
|
|
||
| // Record failure for rate limiting | ||
| proxyAuthPolicy.recordFailure(clientIp, authState.apiKey ?? candidateApiKey ?? undefined); | ||
| // Only `credentials` failures should feed the brute-force rate limiter. | ||
| // `account_state` failures (key/user disabled or expired) match a real | ||
| // record, so recording them would let an admin lock themselves out by | ||
| // simply disabling a key and watching the owner retry. | ||
| if (authState.failureKind !== "account_state") { | ||
| proxyAuthPolicy.recordFailure(clientIp, authState.apiKey ?? candidateApiKey ?? undefined); | ||
| } | ||
|
|
||
| // 返回详细的错误信息,帮助用户快速定位问题 | ||
| return authState.errorResponse ?? ProxyResponses.buildError(401, "认证失败"); | ||
|
|
@@ -128,17 +173,14 @@ export class ProxyAuthenticator { | |
| hasGeminiApiKeyHeader: !!headers.geminiApiKeyHeader, | ||
| hasGeminiApiKeyQuery: !!headers.geminiApiKeyQuery, | ||
| }); | ||
| return { | ||
| user: null, | ||
| key: null, | ||
| apiKey: null, | ||
| success: false, | ||
| return buildAuthFailure({ | ||
| failureKind: "credentials", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| "未提供认证凭据。请在 Authorization 头部、x-api-key 头部或 x-goog-api-key 头部中包含 API 密钥。", | ||
| "authentication_error" | ||
| ), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| const [firstKey] = providedKeys; | ||
|
|
@@ -148,61 +190,94 @@ export class ProxyAuthenticator { | |
| logger.warn("[ProxyAuthenticator] Multiple conflicting API keys provided", { | ||
| keyCount: providedKeys.length, | ||
| }); | ||
| return { | ||
| user: null, | ||
| key: null, | ||
| apiKey: null, | ||
| success: false, | ||
| return buildAuthFailure({ | ||
| failureKind: "credentials", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| "提供了多个冲突的 API 密钥。请仅使用一种认证方式。", | ||
| "authentication_error" | ||
| ), | ||
|
Contributor
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. [MEDIUM] [STANDARD-VIOLATION] New auth error messages hardcode user-facing text (bypasses i18n) Why this is a problem: Guideline: Suggested fix: // src/app/v1/_lib/proxy/auth-guard.ts
const { getLocale, getTranslations } = await import("next-intl/server");
const locale = await getLocale();
const t = await getTranslations({ locale, namespace: "auth" });
return {
user: null,
key: null,
apiKey,
success: false,
failureKind: "credentials",
errorResponse: ProxyResponses.buildError(
401,
t("errors.apiKeyNotFoundOrDeleted"),
"invalid_api_key"
),
};Add the new keys to |
||
| }; | ||
| }); | ||
| } | ||
|
|
||
| const apiKey = firstKey; | ||
| const authResult = await validateApiKeyAndGetUser(apiKey); | ||
| const outcome = await resolveApiKeyAuthOutcome(apiKey); | ||
|
|
||
| if (!authResult) { | ||
| logger.debug("[ProxyAuthenticator] API key validation failed", { | ||
| apiKeyLength: apiKey.length, | ||
| fromHeader: !!headers.authHeader || !!headers.apiKeyHeader || !!headers.geminiApiKeyHeader, | ||
| fromQuery: !!headers.geminiApiKeyQuery, | ||
| }); | ||
| return { | ||
| user: null, | ||
| key: null, | ||
| apiKey, | ||
| success: false, | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| "API 密钥无效。提供的密钥不存在、已被删除、已被禁用或已过期。", | ||
| "invalid_api_key" | ||
| ), | ||
| }; | ||
| if (!outcome.ok) { | ||
| // Exhaustive switch: adding a new ApiKeyAuthFailureReason in the | ||
| // repository layer will trigger a compile error in assertNever below | ||
| // until this guard is updated, preventing a new variant from silently | ||
| // falling into the wrong branch. | ||
| const locale = await getRequestLocale(); | ||
| switch (outcome.reason) { | ||
| case "not_found": | ||
| logger.debug("[ProxyAuthenticator] API key validation failed: not found", { | ||
| apiKeyLength: apiKey.length, | ||
| fromHeader: | ||
| !!headers.authHeader || !!headers.apiKeyHeader || !!headers.geminiApiKeyHeader, | ||
| fromQuery: !!headers.geminiApiKeyQuery, | ||
| }); | ||
| return buildAuthFailure({ | ||
| apiKey, | ||
| failureKind: "credentials", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| await getErrorMessageServer(locale, ERROR_CODES.PROXY_INVALID_API_KEY), | ||
| "invalid_api_key" | ||
| ), | ||
| }); | ||
|
|
||
| case "key_disabled": | ||
| logger.warn("[ProxyAuthenticator] API key is disabled", { | ||
| apiKeyLength: apiKey.length, | ||
| }); | ||
| return buildAuthFailure({ | ||
| apiKey, | ||
| failureKind: "account_state", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| await getErrorMessageServer(locale, ERROR_CODES.PROXY_API_KEY_DISABLED), | ||
| "key_disabled" | ||
| ), | ||
| }); | ||
|
|
||
| case "key_expired": | ||
| logger.warn("[ProxyAuthenticator] API key has expired", { | ||
| apiKeyLength: apiKey.length, | ||
| }); | ||
| return buildAuthFailure({ | ||
| apiKey, | ||
| failureKind: "account_state", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| await getErrorMessageServer(locale, ERROR_CODES.PROXY_API_KEY_EXPIRED), | ||
| "key_expired" | ||
| ), | ||
| }); | ||
|
|
||
| default: | ||
| assertNever(outcome.reason, "ProxyAuthenticator.validate outcome.reason"); | ||
| } | ||
| } | ||
|
|
||
| // Check user status and expiration | ||
| const { user } = authResult; | ||
| const { user } = outcome; | ||
|
|
||
| // 1. Check if user is disabled | ||
| if (!user.isEnabled) { | ||
| logger.warn("[ProxyAuthenticator] User is disabled", { | ||
| userId: user.id, | ||
| userName: user.name, | ||
| }); | ||
| return { | ||
| user: null, | ||
| key: null, | ||
| return buildAuthFailure({ | ||
| apiKey, | ||
| success: false, | ||
| failureKind: "account_state", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| "用户账户已被禁用。请联系管理员。", | ||
| "user_disabled" | ||
| ), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // 2. Check if user is expired (lazy expiration check) | ||
|
|
@@ -219,26 +294,24 @@ export class ProxyAuthenticator { | |
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| }); | ||
| return { | ||
| user: null, | ||
| key: null, | ||
| return buildAuthFailure({ | ||
| apiKey, | ||
| success: false, | ||
| failureKind: "account_state", | ||
| errorResponse: ProxyResponses.buildError( | ||
| 401, | ||
| `用户账户已于 ${user.expiresAt.toISOString().split("T")[0]} 过期。请续费订阅。`, | ||
| "user_expired" | ||
| ), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| logger.debug("[ProxyAuthenticator] Authentication successful", { | ||
| userId: authResult.user.id, | ||
| userName: authResult.user.name, | ||
| keyName: authResult.key.name, | ||
| userId: outcome.user.id, | ||
| userName: outcome.user.name, | ||
| keyName: outcome.key.name, | ||
| }); | ||
|
|
||
| return { user: authResult.user, key: authResult.key, apiKey, success: true }; | ||
| return { user: outcome.user, key: outcome.key, apiKey, success: true }; | ||
| } | ||
|
|
||
| private static extractKeyFromAuthorization(authHeader?: string): string | null { | ||
|
|
||
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.
[HIGH] [TEST-MISSING-CRITICAL] /v1/models
key_disabled/key_expiredbranches have no unit coverageWhy this is a problem: This PR adds new auth behavior in
authenticateRequest(distinct 401key_disabledvskey_expired). There is currently no unit test that asserts these new branches, so a regression back to a genericinvalid_api_key(or other behavior) would go undetected. Guideline:2. **Test Coverage** - All new features must have unit test coverage of at least 80%.Suggested fix: