fix: allow any HTTP method for /api/auth/envoy

[pushpinderbal]

Re-introduces changes that were reverted in #540. Envoy endpoint must respond on any standard HTTP methods. Added better code comments for clarity.

Summary by CodeRabbit

• Bug Fixes

  • Enforced stricter HTTP method handling for authentication proxies: non-envoy proxies are limited to GET and now return 405 with Allow header for unsupported methods; envoy proxies retain broader method handling and existing redirect behavior.

• Tests

  • Expanded test coverage to validate 405 responses and Allow header for non-envoy proxies, plus envoy POST/DELETE redirect scenarios for unauthenticated requests.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Consolidated proxy routing into a single catch-all route and added runtime method validation: non-envoy proxies are limited to GET (405 + Allow header returned for other methods); envoy proxies bypass the method restriction. Tests expanded to cover the new behaviors.

Changes

Cohort / File(s)	Summary
Route consolidation & method control internal/controller/proxy_controller.go	Replaced per-method route registrations with Any("/:proxy", ...); added runtime check in proxyHandler that enforces GET-only for non-envoy proxies and returns 405 with Allow: GET for violations; added explanatory comments.
Test coverage expansion internal/controller/proxy_controller_test.go	Added tests asserting 405 + Allow for non-envoy non-GET requests; added envoy-related POST and DELETE tests validating 307 redirect behavior for logged-out envoy requests; updated comments.
Module manifest go.mod	No functional changes; listed in manifest.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through routes, combined each little track,
One handler now listens — no more duplicate stack.
Envoy may sprint, others pause at the gate,
Tests clap their paws to confirm every state. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 reflects the main change: allowing any HTTP method for the /api/auth/envoy endpoint, which is the primary objective of this PR.

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 20.34%. Comparing base (3c6bd44) to head (80dee3d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #551      +/-   ##
==========================================
+ Coverage   20.08%   20.34%   +0.25%     
==========================================
  Files          37       37              
  Lines        2166     2173       +7     
==========================================
+ Hits          435      442       +7     
  Misses       1701     1701              
  Partials       30       30              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

internal/controller/proxy_controller.go (1)

72-82: Method validation logic is correct. Consider adding Allow header for HTTP spec compliance.

The implementation correctly enforces method restrictions for non-envoy proxies. The error handling and logging are appropriate.

For improved HTTP specification compliance, consider adding an Allow header to the 405 response indicating which methods are permitted:

🔎 Proposed enhancement

 	if req.Proxy != "envoy" && c.Request.Method != http.MethodGet {
 		log.Warn().Str("method", c.Request.Method).Msg("Invalid method for proxy")
+		c.Header("Allow", "GET")
 		c.JSON(405, gin.H{
 			"status":  405,
 			"message": "Method Not Allowed",
 		})
 		return
 	}

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c6bd44 and d210966.

📒 Files selected for processing (2)

• internal/controller/proxy_controller.go
• internal/controller/proxy_controller_test.go

🔇 Additional comments (4)

internal/controller/proxy_controller_test.go (3)

84-89: LGTM! Good test coverage for method validation.

The test correctly verifies that non-envoy proxies (traefik) reject non-GET methods with a 405 status code, aligning with the new method restriction logic.

---

103-103: LGTM! Comment clarity improvement.

The updated comment better distinguishes this test case from the new DELETE method test, improving test documentation.

---

115-125: LGTM! Comprehensive test coverage for envoy DELETE method.

The test properly validates that envoy accepts DELETE requests and handles unauthenticated users correctly by redirecting to the login page with the appropriate redirect URI.

internal/controller/proxy_controller.go (1)

46-47: LGTM! Route consolidation improves flexibility.

Using Any() for the route registration allows for runtime method-based access control, which is cleaner than registering separate routes per method. The comment appropriately explains the design decision.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d210966 and 80dee3d.

📒 Files selected for processing (2)

• internal/controller/proxy_controller.go
• internal/controller/proxy_controller_test.go

🔇 Additional comments (4)

internal/controller/proxy_controller.go (1)

46-47: LGTM! Clean route consolidation.

The switch to a single Any() route with runtime method validation is a cleaner approach than registering separate per-method routes. The comment clearly explains the deferred validation strategy.

internal/controller/proxy_controller_test.go (3)

84-90: LGTM! Proper test coverage for method validation.

This test correctly verifies that non-envoy proxies reject non-GET methods with 405 and the appropriate Allow header.

---

104-104: Good clarification in test comments.

The updated comment improves readability by explicitly noting the HTTP method being tested.

---

116-126: LGTM! Comprehensive envoy method coverage.

This test effectively verifies that the envoy endpoint accepts multiple HTTP methods (DELETE in this case) and behaves consistently with the POST test. This confirms the "any method" allowance for envoy is working correctly.