Skip to content

Conversation

@Saniya777-byte
Copy link

Description:

  • Restrict authentication bypass under /.well-known to only the alive endpoint.
  • Previously, all /.well-known/* endpoints skipped auth via isWellKnown, which made /.well-known/health publicly accessible.
  • This PR changes the auth middleware to bypass auth only for service.AlivePath (/.well-known/alive) and keeps service.HealthPath (/.well-known/health) protected.
  • Updates TestAuthMiddleware to:
    • Assert that /.well-known/alive is accessible without authentication.
    • Assert that /.well-known/health requires authentication (success and failure cases covered).

(Fixes the issue #2562.)

Breaking Changes (if applicable):

  • None.
  • This change only affects auth behavior for /.well-known/health by enforcing authentication where it was previously missing.

Additional Information:

-Checklist:

  • I have formatted my code using goimport and golangci-lint.
  • All new code is covered by unit tests (TestAuthMiddleware updated for the new behavior).
  • This PR does not decrease the overall code coverage.
  • I have reviewed the code comments and documentation for clarity.

@Saniya777-byte
Copy link
Author

@coolwednesday Hi, I’ve fixed this issue. Here’s the solution implemented in this PR.
Kindly review and run the workflows when possible. Thank you!

Copy link
Member

@aryanmehrotra aryanmehrotra left a comment

Choose a reason for hiding this comment

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

It will be a breaking change for users who have health in there readiness endpoint, this change needs to be done carefully and users need to be informed otherwise there service would break on deployment.
@Umang01-hash @coolwednesday how to proceed with this change?

Copy link
Member

@coolwednesday coolwednesday left a comment

Choose a reason for hiding this comment

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

Are we using the isWellKnown() function anywhere else ? if not I think it can be removed.

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.

4 participants