-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Remove redundant call to _authenticate
API after access token is created.
#82980
Remove redundant call to _authenticate
API after access token is created.
#82980
Conversation
@@ -4,6 +4,9 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
import { AuthenticatedUser } from '../../common/model'; | |||
|
|||
export type Authentication = Omit<AuthenticatedUser, 'authentication_provider'>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: not super happy about the name of this type, but don't have any other ideas what the type name of authentication
response field could be ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, we discussed the naming offline and decided to name this field AuthenticationInfo
.
While being short, Authentication
is slightly meaningless and could mean many things to different people.
According to the ES docs the authentication field represents:
User information such as their username, the roles that are assigned to the user, any assigned metadata, and information about the realms that authenticated and authorized the user
So renaming this type to AuthenticationInfo
makes that a bit clearer.
Pinging @elastic/kibana-security (Team:Security) |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (with my limited knowledge) :)
…eated. (elastic#82980) # Conflicts: # .github/CODEOWNERS # x-pack/plugins/security/server/authentication/providers/saml.test.ts # x-pack/plugins/security/server/authentication/providers/saml.ts
7.x/7.11.0: 660f712 |
Since elastic/elasticsearch#59685 is resolved now (7.11.0+) we can proceed and remove redundant
_authenticate
calls after we create any kind of access tokens (Token/SAML/OIDC/Kerberos/PKI) that would improve login performance and make code simpler overall.Fixes: #80952