[security] Support alternate auth providers for login #26979
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Login is no longer coupled directly to our basic auth provider, so alternative auth providers can now be used with our standard login flow. The LoginAttempt request service is the mechanism for auth providers to integrate with the login flow.
|
Transient CI failure due to #26228 |
|
retest |
This comment has been minimized.
This comment has been minimized.
epixa
commented
Dec 11, 2018
| /** | ||
| * Utility class that knows how to decorate request with proper Basic authentication headers. | ||
| */ | ||
| export class BasicCredentials { |
There was a problem hiding this comment.
Nothing in BasicCredentials changed, I just had to move it to the top of the file so I could use it within _authenticateViaLoginAttempt without the linter complaining.
epixa
commented
Dec 11, 2018
| @@ -123,7 +193,7 @@ export class BasicAuthenticationProvider { | |||
| this._options.log(['debug', 'security', 'basic'], 'Request has been authenticated via header.'); | |||
|
|
|||
| return { | |||
| authenticationResult: AuthenticationResult.succeeded(user, { authorization }) | |||
| authenticationResult: AuthenticationResult.succeeded(user) | |||
There was a problem hiding this comment.
No more cookie for basic auth headers. It's worth mentioning that this change isn't essential for the addition of LoginAttempt nor the token provider, but it seems like the right behavior, and I can't think of a reason why the old behavior should be supported.
There was a problem hiding this comment.
and I can't think of a reason why the old behavior should be supported.
We didn't have this behavior before "auth providers", I'm really glad you removed that nasty thing! I can sleep well again now 🙂
epixa
added
non-issue
|
Pinging @elastic/kibana-security |
epixa
added
the
Feature:Security/Authentication
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kobelb
reviewed
Dec 11, 2018
x-pack/plugins/security/server/lib/authentication/__tests__/authenticator.js
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/server/lib/authentication/__tests__/authenticator.js
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
retest |
💚 Build Succeeded |
epixa
added a commit
to epixa/kibana
that referenced
this pull request
Dec 13, 2018
Login is no longer coupled directly to our basic auth provider, so alternative auth providers can now be used with our standard login flow. The LoginAttempt request service is the mechanism for auth providers to integrate with the login flow.
epixa
added a commit
that referenced
this pull request
Dec 13, 2018
Login is no longer coupled directly to our basic auth provider, so alternative auth providers can now be used with our standard login flow. The LoginAttempt request service is the mechanism for auth providers to integrate with the login flow.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Login is no longer coupled directly to our basic auth provider, so
alternative auth providers can now be used with our standard login flow.
The LoginAttempt request service is the mechanism for auth providers to
integrate with the login flow.
The only noticeable change here from a product perspective is that
requests authenticated via basic headers should no longer respond with
a session cookie. Beyond that, basic authentication should still be possible
for curl commands and login should otherwise appear to work as it always
has.