Support stored authentication headers prior to version 6.7 #92221
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
Officially Elasticsearch is compatible with last major at data level. Therefore v8 is not compatible with v6. However we don't have a guided migration path for stored authentication headers, e.g. upgrade assistant does not do anything for them. Therefore it is more helpful and user friendly for v8 to support v6 stored authentication headers. This PR adds back the version conditional logic removed in elastic#41185 along with tests.
ywangd
added
>bug
:Security/Authentication
Collaborator
|
Pinging @elastic/es-security (Team:Security) |
Collaborator
|
Hi @ywangd, I've created a changelog YAML for you. |
droberts195
pushed a commit
to droberts195/elasticsearch
that referenced
this pull request
Dec 12, 2022
Due to elastic#41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to elastic#87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
ywangd
commented
Dec 12, 2022
Comment on lines
+130
to
+136
| if (version.onOrAfter(VERSION_AUTHENTICATION_TYPE)) { | ||
| type = AuthenticationType.values()[in.readVInt()]; | ||
| metadata = in.readMap(); | ||
| } else { | ||
| type = AuthenticationType.REALM; | ||
| metadata = Map.of(); | ||
| } |
Member
Author
There was a problem hiding this comment.
We should have corresponding logic on the writeTo side. But I could not decide whether it should be:
- Simply throwing an error if version is 6.x
- Drop type and metadata if version is 6.x
- A combination of the two, ie. drop type and metadata if type is
REALMand metadata is empty, otherwise throw.
I think the answer might be 3?
Member
Author
There was a problem hiding this comment.
I pushed a commit for option 2. Because option 1 is not viable as it prevents authentication to be sent across wire. Option 3 is likely overkill and hard to test (need to hack an invalid 6.x header). Please consider it as a strawman implementation. It is easier to discuss with something concrete.
ywangd
force-pushed
the
support-v6-stored-authc-header
branch
from
4eb9cdd to
9b3a82c
Compare
ywangd
added
auto-merge-without-approval
Member
Author
|
@elasticmachine run elasticsearch-ci/docs |
ywangd
added a commit
to ywangd/elasticsearch
that referenced
this pull request
Dec 12, 2022
…2221) Officially Elasticsearch is compatible with last major at data level. Therefore v8 is not compatible with v6. However we don't have a guided migration path for stored authentication headers, e.g. upgrade assistant does not do anything for them. Therefore it is more helpful and user friendly for v8 to support v6 stored authentication headers. This PR adds back the version conditional logic removed in elastic#41185 along with tests.
elasticsearchmachine
pushed a commit
that referenced
this pull request
Dec 12, 2022
…92295) Officially Elasticsearch is compatible with last major at data level. Therefore v8 is not compatible with v6. However we don't have a guided migration path for stored authentication headers, e.g. upgrade assistant does not do anything for them. Therefore it is more helpful and user friendly for v8 to support v6 stored authentication headers. This PR adds back the version conditional logic removed in #41185 along with tests.
droberts195
pushed a commit
that referenced
this pull request
Dec 13, 2022
…2274) Due to #41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to #87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by #92168 and fixed by #92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
droberts195
pushed a commit
to droberts195/elasticsearch
that referenced
this pull request
Dec 13, 2022
…astic#92274) Due to elastic#41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to elastic#87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
elasticsearchmachine
pushed a commit
that referenced
this pull request
Dec 13, 2022
…2274) (#92318) Due to #41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to #87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by #92168 and fixed by #92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
droberts195
pushed a commit
to droberts195/elasticsearch
that referenced
this pull request
Dec 13, 2022
Due to elastic#41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to elastic#87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive. Backport of elastic#92274
droberts195
pushed a commit
to droberts195/elasticsearch
that referenced
this pull request
Dec 13, 2022
Due to elastic#41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to elastic#87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive. Backport of elastic#92274
elasticsearchmachine
pushed a commit
that referenced
this pull request
Dec 13, 2022
…em (#92323) Due to #41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to #87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by #92168 and fixed by #92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive. Backport of #92274
droberts195
pushed a commit
that referenced
this pull request
Dec 13, 2022
…em (#92324) Due to #41185 datafeeds created prior to 7.0 and not updated since then have unparseable authorization headers in 8.x. In 8.0-8.3 this could easily be a non-issue, as such datafeeds were likely forgotten leftovers and never run. Even if it was a problem, only the datafeed of interest would need updating with any urgency. Due to #87884 datafeeds with authorization headers older than 7.0 prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn breaks the anomaly detection job management section of the ML UI. The problem is alleviated by #92168 and fixed by #92221, but we should warn users that the problem exists in 8.4.0-8.5.3 inclusive. Backport of #92274
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Officially Elasticsearch is compatible with last major at data level. Therefore v8 is not compatible with v6. However we don't have a guided migration path for stored authentication headers, e.g. upgrade assistant does not do anything for them. Therefore it is more helpful and user friendly for v8 to support v6 stored authentication headers.
This PR adds back the version conditional logic removed in #41185 along with tests.