Skip to content
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.

Sign up for GitHub

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

Compatible for getting auth token from client #5904

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −2
Open

Compatible for getting auth token from client #5904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/api/anthropic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ async function request(req: NextRequest) {

let authHeaderName = "x-api-key";
let authValue =
req.headers.get(authHeaderName) ||
req.headers.get("Authorization")?.replaceAll("Bearer ", "").trim() ||
req.headers.get(authHeaderName) ||
serverConfig.anthropicApiKey ||
Comment on lines 64 to 67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Align authentication header precedence with auth.ts

The authentication header precedence here differs from auth.ts, which could lead to inconsistent behavior. Additionally, the Bearer token stripping is only applied to the 'Authorization' header, not to 'x-api-key'.

- let authValue =
-   req.headers.get("Authorization")?.replaceAll("Bearer ", "").trim() ||
-   req.headers.get(authHeaderName) ||
-   serverConfig.anthropicApiKey ||
-   "";
+ const stripBearer = (token: string | null) => 
+   token?.replaceAll("Bearer ", "").trim() ?? null;
+ let authValue =
+   stripBearer(req.headers.get(authHeaderName)) ??
+   stripBearer(req.headers.get("Authorization")) ??
+   serverConfig.anthropicApiKey ??
+   "";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let authValue =
req.headers.get(authHeaderName) ||
req.headers.get("Authorization")?.replaceAll("Bearer ", "").trim() ||
req.headers.get(authHeaderName) ||
serverConfig.anthropicApiKey ||
const stripBearer = (token: string | null) =>
token?.replaceAll("Bearer ", "").trim() ?? null;
let authValue =
stripBearer(req.headers.get(authHeaderName)) ??
stripBearer(req.headers.get("Authorization")) ??
serverConfig.anthropicApiKey ??
"";

"";

Expand Down
3 changes: 2 additions & 1 deletion app/api/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ function parseApiKey(bearToken: string) {
}

export function auth(req: NextRequest, modelProvider: ModelProvider) {
const authToken = req.headers.get("Authorization") ?? "";
const authToken =
req.headers.get("Authorization") ?? req.headers.get("x-api-key") ?? "";
Comment on lines +28 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider reversing the header precedence order

The current implementation prioritizes the 'Authorization' header over 'x-api-key'. While this maintains backward compatibility, it could potentially allow an attacker to bypass intended 'x-api-key' authentication by providing an 'Authorization' header. Consider reversing the order if 'x-api-key' should take precedence for certain clients (e.g., Anthropic services).

-  const authToken =
-    req.headers.get("Authorization") ?? req.headers.get("x-api-key") ?? "";
+  const authToken =
+    req.headers.get("x-api-key") ?? req.headers.get("Authorization") ?? "";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const authToken =
req.headers.get("Authorization") ?? req.headers.get("x-api-key") ?? "";
const authToken =
req.headers.get("x-api-key") ?? req.headers.get("Authorization") ?? "";


// check if it is openai api key or user token
const { accessCode, apiKey } = parseApiKey(authToken);
Expand Down