-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Extract class to store Authentication in context #52032
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
Merged
Merged
Conversation
This file contains hidden or 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
This change extracts the code that previously existed in the "Authentication" class that was responsible for reading and writing authentication objects to/from the ThreadContext. This is needed to support multiple authentication objects under separate keys. This refactoring highlighted that there were a large number of places where we extracted the Authentication/User objects from the thread context, in a variety of ways. These have been consolidated to rely on the SecurityContext object.
Pinging @elastic/es-security (:Security/Authentication) |
@elasticmachine update branch |
Update the SetSecurityUserProcessorFactory to defer access to SecurityContext until after createComponents is called and the SecurityContext is instantiated.
jkakavas
approved these changes
Feb 10, 2020
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 on green CI
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Feb 11, 2020
This change extracts the code that previously existed in the "Authentication" class that was responsible for reading and writing authentication objects to/from the ThreadContext. This is needed to support multiple authentication objects under separate keys. This refactoring highlighted that there were a large number of places where we extracted the Authentication/User objects from the thread context, in a variety of ways. These have been consolidated to rely on the SecurityContext object. Backport of: elastic#52032
tvernum
added a commit
that referenced
this pull request
Feb 11, 2020
This change extracts the code that previously existed in the "Authentication" class that was responsible for reading and writing authentication objects to/from the ThreadContext. This is needed to support multiple authentication objects under separate keys. This refactoring highlighted that there were a large number of places where we extracted the Authentication/User objects from the thread context, in a variety of ways. These have been consolidated to rely on the SecurityContext object. Backport of: #52032
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Feb 24, 2020
If security was disabled (explicitly), then the SecurityContext would be null, but the set_security_user processor was still registered. Attempting to define a pipeline that used that processor would fail with an (intentional) NPE. This behaviour, introduced in elastic#52032, is a regression from previous releases where the pipeline was allowed, but was no usable. This change restores the previous behaviour (with a new warning).
tvernum
added a commit
that referenced
this pull request
Mar 5, 2020
If security was disabled (explicitly), then the SecurityContext would be null, but the set_security_user processor was still registered. Attempting to define a pipeline that used that processor would fail with an (intentional) NPE. This behaviour, introduced in #52032, is a regression from previous releases where the pipeline was allowed, but was no usable. This change restores the previous behaviour (with a new warning).
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Mar 13, 2020
If security was disabled (explicitly), then the SecurityContext would be null, but the set_security_user processor was still registered. Attempting to define a pipeline that used that processor would fail with an (intentional) NPE. This behaviour, introduced in elastic#52032, is a regression from previous releases where the pipeline was allowed, but was no usable. This change restores the previous behaviour (with a new warning). Backport of: elastic#52691
tvernum
added a commit
that referenced
this pull request
Mar 17, 2020
If security was disabled (explicitly), then the SecurityContext would be null, but the set_security_user processor was still registered. Attempting to define a pipeline that used that processor would fail with an (intentional) NPE. This behaviour, introduced in #52032, is a regression from previous releases where the pipeline was allowed, but was no usable. This change restores the previous behaviour (with a new warning). Backport of: #52691
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
>refactoring
:Security/Authentication
Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc)
v7.7.0
v8.0.0-alpha1
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.
This change extracts the code that previously existed in the
"Authentication" class that was responsible for reading and writing
authentication objects to/from the ThreadContext.
This is needed to support multiple authentication objects under
separate keys.
This refactoring highlighted that there were a large number of places
where we extracted the Authentication/User objects from the thread
context, in a variety of ways. These have been consolidated to rely on
the SecurityContext object.