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

Authentication fails on get requests #17 #18

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nkeating-mutualofenumclaw

This fixes the signing header for elastic search requests, as the lowercase "host" header causes header size issues and invalidates the signing.

@nkeating-mutualofenumclaw
Copy link
Author

After diving further into this, I found that the lowercase host works, as long as there's not also an upper case host. I've modified the code to leave the lowercase host, but added a check for the uppercase Host and if present delete it from the object. This allows code that used the uppercase Host to still work

@joshferrell
Copy link

Tried this pr out as I was having similar authentication issues, and this fixed my problem. Thanks!

@compwright compwright self-requested a review July 9, 2020 19:56
@compwright
Copy link
Owner

This should not be the case if the correct version of @elastic/elasticsearch (>= 7.8.0) is being used. However this may help with backward compatibility. I guess I need to come up with a way to test this library with multiple versions of elasticsearch. Thanks for the PR and for your research into this bug!

@compwright compwright linked an issue Jul 9, 2020 that may be closed by this pull request
@nkeating-mutualofenumclaw
Copy link
Author

Is there anything more that's needed for this code to be merged?

@compwright
Copy link
Owner

@nkeating-mutualofenumclaw yes, need tests to confirm that it does indeed fix errors with older versions of elasticsearch, and doesn't break when used with the current version of elasticsearch.

@nkeating-mutualofenumclaw
Copy link
Author

Is this something I need to do? It appears to work for myself and the other person on this thread. Not sure what the next steps should be.

@compwright
Copy link
Owner

@nkeating-mutualofenumclaw yes, please.

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.

Authentication fails on get requests
3 participants