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

Don't rely just on presence of the cookie to detect active user session #50634

Closed
azasypkin opened this issue Nov 14, 2019 · 4 comments
Closed
Labels
blocker Feature:NP Migration Feature:Security/Authentication Platform Security - Authentication Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@azasypkin
Copy link
Member

Currently we have a code in the login view route that decides whether it should render login view or not (and redirect to the Kibana root) solely based on the presence of the session cookie.

That worked well in the legacy world where we had only basic authentication provider, but now it has the following issues:

  • when both saml\oidc and basic providers are used presence of the session cookie may have 3 different meanings:
    1. user is authenticated with the basic
    2. user is authenticated with the saml\oidc
    3. user is not authenticated yet, but in the process of SAML\OpenID Connect handshake with an intermediate cookie

And if user wishes to open login view in cases 2 or 3 it'd make sense to allow them to do that (and effectively fix #25257)

  • NP KibanaRequest won't give us direct access to the cookie anymore (justifies blocker status for 8.0)

It seems currently we have all the necessary pieces to finally fix that issue: Authenticator is able to extract session from KibanaRequest using internal sessionStorageFactory even if the route that handles this request explicitly opted out from mandatory authentication. So essentially we can have a new method on Authenticator that would answer whether or not request is associated with the active session (not intermediate) and which provider owns this session using the code similar to one we have in the authenticate method.

There are two questions we should answer first though:

  • do we want to treat requests with valid Authorization header (or any other header we may support in the future) the same as requests with valid cookie-based session (like we do for normal authentication)? If the answer is yes, then we'll have to iterate through all providers trying to figure out which provider can authenticate the request and return user information.

    My current preference is to answer no to precisely replicate current behavior and keep code simpler. We should stop supporting this scenario in the authentication providers eventually anyway and use a dedicated proxy-headers authentication provider instead (once Reporting starts using API keys instead of request headers "snapshots" or we can add this special authentication provider automatically as the first one).

  • are we okay with any side-effects that may happen when we call provider's authenticate just to know if the session is active (e.g. access token will be automatically refreshed if expired, maybe there are more side-effects)?

cc @restrry

Supersedes: #41959

@azasypkin azasypkin added blocker Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/Authentication Platform Security - Authentication Feature:NP Migration labels Nov 14, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@kobelb
Copy link
Contributor

kobelb commented Nov 14, 2019

are we okay with any side-effects that may happen when we call provider's authenticate just to know if the session is active (e.g. access token will be automatically refreshed if expired, maybe there are more side-effects)?

If we did end up refreshing the access token, is it safe to assume that it'd be sent back to the client with a Set-Cookie response header? If so, it feels like we're almost treating the login route as an optionally authenticated route, which seems fine.

@azasypkin
Copy link
Member Author

If we did end up refreshing the access token, is it safe to assume that it's be sent back to the client with a Set-Cookie response header?

Yeah, we can/should do that.

If so, it feels like we're almost treating the login route as an optionally authenticated route, which seems fine.

Right, it's similar to Hapi's try auth mode. Hmm, in fact we can just call our existing authenticate method instead of another dedicated one I was describing above, but then we'll treat requests with Authorization header differently than we do currently (may not be a big deal though, we'll see).

@azasypkin
Copy link
Member Author

Fixed in #53010 and will be available since 7.7.0 (we still need to fix this for logged_out, but it will be done in the scope of another issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Feature:NP Migration Feature:Security/Authentication Platform Security - Authentication Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

No branches or pull requests

3 participants