Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce scope of trusted header auth #17483

Merged
merged 3 commits into from Dec 11, 2023

Conversation

thll
Copy link
Contributor

@thll thll commented Nov 27, 2023

Description

Only allow trusted header auth for the SessionsResource.

/nocl

Motivation and Context

Trusted header auth is only used until a session has been created for the user. From then on, if a session cookie is set on a request, it takes precedence over trusted headers to authenticate a request.

This was causing issues for proxied requests when a session expired. Our authentication chain would fail to authenticate the session cookie, because it was expired, and would then fall back to the next authentication method in the chain, which was trusted header auth. When trying to forward the request to other servers, we failed to extract an authentication token from the original request and caused the forwarded request to fail because it was unauthenticated.

With this fix, we are only allowing trusted header authentication for requests that target the sessions resource. Requests to other resources will fail fast if they require authentication but only have trusted header set.

For the mentioned scenario of session expiration, this will cause any background REST call to to a proxied endpoint fail with a 401 once the session expires, even if trusted headers are set. This in turn will cause the UI to call the sessions endpoint which will then refresh the session based on the trusted headers.

How Has This Been Tested?

See steps to reproduce here.

We shouldn't get into this situation anymore
because requests which are authenticated only
by trusted headers won't hit a proxied
resource anymore.
@thll thll changed the title Refactor/reduce scope of trusted header auth Reduce scope of trusted header auth Nov 27, 2023
@thll thll marked this pull request as ready for review November 27, 2023 15:39
@bernd bernd self-assigned this Dec 8, 2023
Copy link
Member

@bernd bernd left a comment

Choose a reason for hiding this comment

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

Thanks! Tested with two nodes and Caddy.

@bernd bernd merged commit 55c8836 into master Dec 11, 2023
8 checks passed
@bernd bernd deleted the refactor/reduce-scope-of-trusted-header-auth branch December 11, 2023 10:16
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.

None yet

2 participants