Skip to content

Conversation

robvanoostenrijk
Copy link
Contributor

nginx auth_request requires either a 2xx (authenticated) or 401/403 (authentication required) response.

This updates saml-auth-proxy to respond appropriately to nginx auth_requests.

The authentication check is identical to the middleware.RequireAccount function.
middleware.RequireAccount cannot be used directly because it starts the authentication flow if not authentication, which nginx auth_request does not expect.

@itzg
Copy link
Owner

itzg commented Jul 21, 2025

It actually used to be the way you have it before #106. I'm going to also comment there, because it now looks like this behavior needs to be configurable.

@itzg
Copy link
Owner

itzg commented Jul 22, 2025

Sorry for pushing a commit

4057215

It turned out by the time I adjusted the code locally to suggest a change, I had a commitable change ready to go.

@robvanoostenrijk
Copy link
Contributor Author

Noted others require the exact opposite behavior. Configurable option seems to be right solution here.

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

Ready for merge? Perhaps want to re-test with my config changes? Should by default be the "old"/your PR's behavior.

@kennethklee
Copy link
Contributor

just wanted to bump this.

@itzg
Copy link
Owner

itzg commented Jul 31, 2025

Thanks @kennethklee ...I'll assume it's ready for merge due to no further commits.

@itzg itzg merged commit ec738fa into itzg:master Jul 31, 2025
2 checks passed
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.

3 participants