Skip to content

Conversation

@s0up4200
Copy link
Collaborator

@s0up4200 s0up4200 commented Jan 27, 2026

Summary

  • normalize base URL joins for API mount and OIDC dashboard redirect
  • scope apikey query param support to cross-seed apply/webhook with explicit middleware ordering
  • switch unauthenticated responses to 401 and use typed context keys for username
  • add/adjust tests for CORS base URL and apikey middleware behavior

Testing

  • make test
  • make lint (fails in web/ due to pre-existing frontend lint errors)

Summary by CodeRabbit

  • New Features

    • Added support for passing API keys via query parameters in addition to headers.
  • Bug Fixes

    • Fixed OIDC redirect URL handling to properly handle custom base URLs without duplicate slashes.
    • Corrected authentication error responses to use 401 Unauthorized status code instead of 403 Forbidden.
  • Tests

    • Added comprehensive test coverage for API key and authentication flows.
    • Added CORS preflight validation test with custom base URL support.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Walkthrough

Refactors authentication flow by introducing a typed context key for username, adding a new middleware to promote API keys from query parameters to headers, updating handler signatures to accept middleware parameters explicitly, and shifting error responses from Forbidden to Unauthorized status. Modifies routing to dynamically handle custom base URLs and includes test coverage for these changes.

Changes

Cohort / File(s) Summary
Typed Context Key
internal/api/ctxkeys/ctxkeys.go
New file defining a Key type alias (int) and Username constant for strongly-typed context key storage, preventing string-based collision risks.
Middleware
internal/api/middleware/apikey_query.go, internal/api/middleware/apikey_query_test.go
New APIKeyFromQuery() middleware factory that extracts API key from query parameter and promotes it to X-API-Key header when header is absent. Includes unit test with database setup and auth service.
Authentication Middleware
internal/api/middleware/auth.go, internal/api/middleware/auth_test.go
Refactored auth middleware: removes query parameter fallback for API keys, switches to typed context key (ctxkeys.Username), changes unauthorized status from 403 Forbidden to 401 Unauthorized. Test refactored from query-param focus to header-based auth verification.
Handler Updates
internal/api/handlers/crossseed.go, internal/api/handlers/licenses.go, internal/api/handlers/oidc.go
CrossSeedHandler.Routes now accepts explicit authMiddleware and apiKeyQueryMiddleware parameters and applies them to route registrations. Licenses handler updated to use typed context key for username retrieval. OIDC handler trims trailing slash from BaseURL before appending "/dashboard".
Server Routing
internal/api/server.go
Middleware instances created and passed to handler route setup. API router mount path computed dynamically based on BaseURL to handle custom base paths correctly.
Test Coverage
internal/api/server_cors_test.go
New test TestCORSPreflightWithCustomBaseURL verifies CORS preflight behavior with custom BaseURL configuration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

bugfix, auth, backend, tests

Suggested reviewers

  • Audionut

Poem

🐰 With typed keys so strong and true,
No more string collisions to pursue!
Headers now guide the API's way,
While 401s greet the unauthenticated day.
Routes dressed in middleware with care,
The server hops forward with flair! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main changes: CORS/auth routing improvements and base URL join normalization, which are the primary objectives of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/cors-auth-routing

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants