Skip to content
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

fix: preemptively send basic auth #127

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Conversation

yaauie
Copy link
Member

@yaauie yaauie commented Feb 8, 2024

When Elasticsearch is configured to allow anonymous auth, it does not include a challenge header in response to requests, so we cannot rely on the apache http client's auth handling (which is overkill anyway, since an individual Elasticsearch client is not connected to multiple realms).

Instead, we build the http basic auth header ourselves, and register an interceptor to ensure it is added to all requests, similar to how we handle api key auth headers.

When Elasticsearch is configured to allow anonymous auth, it does not include
a challenge header in response to requests, so we cannot rely on the apache
http client's auth handling (which is overkill anyway, since an individual
Elasticsearch client is not connected to multiple realms).

Instead, we build the http basic auth header ourselves, and register an
interceptor to ensure it is added to all requests, similar to how we handle
api key auth headers.
@yaauie yaauie requested a review from mashhurs February 8, 2024 20:45
mashhurs
mashhurs previously approved these changes Feb 8, 2024
Copy link
Collaborator

@mashhurs mashhurs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Tested locally.
🔴 CI you don't need to care for now.

  • PR should be tested against current release and build pipeline should take care snapshots
  • Or if we move to versions truth of source (we discuss in weekly), this will be resolved.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Mashhur <99575341+mashhurs@users.noreply.github.com>
@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 8, 2024

@yaauie
Copy link
Member Author

yaauie commented Feb 8, 2024

Buildkite failed because it is trying to build against Elasticsearch 8.13's source, but the 8.13 branch has not yet been cut. When building against Elasticsearch 8.11's source (as done in our gradle tasks for building and releasing the artifact), tests succeed.

@yaauie yaauie merged commit 933511b into elastic:main Feb 8, 2024
1 of 2 checks passed
@yaauie yaauie deleted the preemptive-basic-auth branch February 8, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants