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

Add an optional authentication mode for HTTP resources #41959

Closed
mshustov opened this issue Jul 25, 2019 · 10 comments · Fixed by #58589
Closed

Add an optional authentication mode for HTTP resources #41959

mshustov opened this issue Jul 25, 2019 · 10 comments · Fixed by #58589
Assignees
Labels
blocker Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@mshustov
Copy link
Contributor

mshustov commented Jul 25, 2019

Create the final requirement and discuss implementation details with @elastic/kibana-security team. Context #39446 (comment)

We need a separate mode for HTTP service, providing access to user credentials and current authentication status to understand whether the Kibana server can perform any operations on behalf of the current user.

It somethings similar to Hapi auth: optional or auth: try mode.

@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Jul 25, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@mshustov mshustov changed the title Add mode to get user credentials without performing authentication Add a mode to get user credentials without performing authentication Jul 25, 2019
@joshdover
Copy link
Contributor

@restrry does the authRequired: false flag not satisfy this?

@mshustov
Copy link
Contributor Author

no, it something similar to { auth: 'try' } from Hapi. When user can access a resource if authentication fails.

@azasypkin
Copy link
Member

Superseded by #50634

@mshustov
Copy link
Contributor Author

mshustov commented Feb 25, 2020

I need feedback from the @elastic/kibana-security before starting working on it.
Right now Security plugin terminates the user flow in case of:

  • failed authentication
  • invalid credentials
  • 3rd party provider redirect

That doesn't play well with an optional auth that we want to introduce since a user should have access to a resource if not authenticated. I'd suggest introducing an explicit unauthenticated outcome for the auth hook that indicates that the user cannot be authenticated. It means that:

  • for optional authentication mode, not handed credentials won't terminate the user flow
  • for required authentication mode, the Kibana server responds with unauthorized

There are still 2 authentication outcomes in the gray zone.
I err on keeping failed authentication in a current manner when the server responds with Elasticsearch error since the security plugin cannot validate user credentials.

const statusCode = getErrorStatusCode(error);
if (typeof statusCode === 'number') {
return response.customError({
body: error,
statusCode,
headers: authenticationResult.authResponseHeaders,
});

But I still not sure how to handle redirection to 3rd party IdP - probably is not expected for optional authentication. Any thoughts?

Another question: Does the Security plugin need access to the current auth mode?

@azasypkin
Copy link
Member

for optional authentication mode, invalid credentials won't terminate the user flow

I assume you meant allowing requests without any credentials. Otherwise it'd be more like try Hapi auth mode then. With optional mode we still should reject invalid credentials exactly like in required mode (e.g. request includes Authorization: Basic xxxx with wrong username and password).

But I still not sure how to handle redirection to 3rd party IdP - probably is not expected for optional authentication. Any thoughts?

Don't have a strong and formed opinion on that yet and would like to hear more thoughts from the Team, but I'm leaning towards redirect === unauthenticated (not failed and not succeeded). Basically when we're redirecting we say: Hey, you didn't provide any credentials, go somewhere else and provide your credentials there and then return back. E.g. it seems that it would allow us to set optional for the login page route and don't rely on cookie/session presence in is user already authenticated? check anymore.

Another question: Does the Security plugin need access to the current auth mode?

Even though my initial reaction was that it may be useful to have it in auth hook, I don't have a compelling use case in mind yet. So if none of us can think of one maybe we can expose it only when we need it.

@mshustov mshustov changed the title Add a mode to get user credentials without performing authentication Add an optional authentication mode for HTTP resources Feb 26, 2020
@legrego
Copy link
Member

legrego commented Mar 4, 2020

But I still not sure how to handle redirection to 3rd party IdP - probably is not expected for optional authentication. Any thoughts?

Don't have a strong and formed opinion on that yet and would like to hear more thoughts from the Team, but I'm leaning towards redirect === unauthenticated (not failed and not succeeded). Basically when we're redirecting we say: Hey, you didn't provide any credentials, go somewhere else and provide your credentials there and then return back. E.g. it seems that it would allow us to set optional for the login page route and don't rely on cookie/session presence in is user already authenticated? check anymore.

@azasypkin just to make sure I understand, it sounds like you're agreeing with @restrry here, in that we should not perform IdP redirection for optional routes, right?

If so, I agree with the both of you.

@azasypkin
Copy link
Member

it sounds like you're agreeing with @restrry here, in that we should not perform IdP redirection for optional routes, right?

Yes, that's correct. The second question we were discussing is who (Core or Security) will be making this decision, the consensus currently is to delegate that to the Core (Security will still have all the primitives in place to take control over that decision in case we need it).

@legrego
Copy link
Member

legrego commented Mar 4, 2020

the consensus currently is to delegate that to the Core (Security will still have all the primitives in place to take control over that decision in case we need it).

That makes sense to me

@mshustov
Copy link
Contributor Author

mshustov commented Mar 4, 2020

@legrego then we need to change auth interceptor interface & refactor Security plugin. Steps:

  • introduce authToolkit.failed({status, error, headers}) & authToolkit.redirected ({url}). note: for authRequired: optional the authToolkit.redirected === authToolkit.notHandled
  • remove responseFactory parameter
  • refactor security plugin to use authToolkit instead of responseFactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants