Skip to content

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 5 commits into from
Feb 10, 2020

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Feb 7, 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.

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.
@tvernum tvernum added >refactoring :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 v7.7.0 labels Feb 7, 2020
@tvernum tvernum requested a review from jkakavas February 7, 2020 06:49
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Authentication)

@tvernum
Copy link
Contributor Author

tvernum commented Feb 10, 2020

@elasticmachine update branch

elasticmachine and others added 3 commits February 9, 2020 18:23
Update the SetSecurityUserProcessorFactory to defer access to
SecurityContext until after createComponents is called and the
SecurityContext is instantiated.
Copy link
Member

@jkakavas jkakavas left a 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 tvernum merged commit 9fb5c81 into elastic:master Feb 10, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants