proxy: add support for basic authorization#445
Conversation
Fixes #444 where the UI with api keys did not work. The choice to use http basic authorization is for simple, automatic browser support. No changes to the UI were necessary. Just use an API key as the password, no user name is required.
WalkthroughThe pull request adds HTTP Basic authentication support to the proxy's API key authentication mechanism. Basic credentials are decoded from base64 to extract the password as the API key. Basic auth is prioritized over Bearer and x-api-key headers. Authentication failures now include a WWW-Authenticate header response. Comprehensive test coverage validates these scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
proxy/proxymanager_test.go (1)
1297-1307: Consider adding a priority conflict test.This test validates the happy path where both headers have valid keys. To fully verify the documented priority (Basic > Bearer > x-api-key), consider adding a test where Basic auth has a valid key but x-api-key has an invalid key, confirming Basic takes precedence.
🔎 Suggested additional test case
t.Run("Basic Auth takes priority over invalid x-api-key", func(t *testing.T) { reqBody := `{"model":"model1"}` req := httptest.NewRequest("POST", "/v1/chat/completions", bytes.NewBufferString(reqBody)) req.Header.Set("x-api-key", "invalid-key") credentials := base64.StdEncoding.EncodeToString([]byte("user:valid-key-1")) req.Header.Set("Authorization", "Basic "+credentials) w := CreateTestResponseRecorder() proxy.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) // Basic auth valid key takes priority })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
proxy/proxymanager.goproxy/proxymanager_test.go
🧰 Additional context used
📓 Path-based instructions (1)
proxy/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Run
make test-devwhen making iterative changes to code under theproxy/directory - this runsgo testandstaticcheck, and all static checking errors must be fixed
Files:
proxy/proxymanager_test.goproxy/proxymanager.go
🧠 Learnings (1)
📚 Learning: 2025-11-25T05:40:06.444Z
Learnt from: CR
Repo: mostlygeek/llama-swap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:40:06.444Z
Learning: Applies to proxy/**/*.go : Run `make test-dev` when making iterative changes to code under the `proxy/` directory - this runs `go test` and `staticcheck`, and all static checking errors must be fixed
Applied to files:
proxy/proxymanager_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run-tests
- GitHub Check: run-tests
🔇 Additional comments (9)
proxy/proxymanager.go (3)
6-6: LGTM!Standard library import for base64 decoding added correctly.
829-853: LGTM! Basic auth implementation is correct.The implementation correctly handles edge cases:
SplitN(_, ":", 2)preserves passwords containing colons- Invalid base64 silently falls through to other auth methods
- Key priority (Basic > Bearer > x-api-key) aligns with PR description
864-869: LGTM! WWW-Authenticate header enables browser auth prompt.Adding the
WWW-Authenticate: Basic realm="llama-swap"header on 401 responses correctly triggers the browser's native authentication dialog, which fulfills the PR objective of making the Web UI accessible when API keys are enabled.proxy/proxymanager_test.go (6)
6-6: LGTM!Standard library import added for encoding Basic auth credentials in tests.
1253-1262: LGTM!Test correctly validates that an invalid API key returns 401 with an "unauthorized" message.
1264-1271: LGTM!Test correctly validates that missing authentication returns 401 Unauthorized.
1273-1283: LGTM!Test correctly validates Basic auth with the API key as password. Using "anyuser" demonstrates that only the password (API key) is validated, which matches the implementation.
1285-1295: LGTM!Test correctly validates that an invalid API key in Basic auth returns 401 Unauthorized.
1309-1317: LGTM!Test correctly validates that 401 responses include the
WWW-Authenticate: Basic realm="llama-swap"header, which is essential for browser authentication prompts.
Fixes mostlygeek#444 where the UI with api keys did not work. The choice to use http basic authorization is for simple, automatic browser support. No changes to the UI were necessary. Just use an API key as the password, no user name is required.
1 participant
Fixes #444 where the UI with api keys did not work. The choice to use http basic authorization is for simple, automatic browser support. No changes to the UI were necessary. Just use an API key as the password, no user name is required.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.