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

HTTPCLIENT-2356: Extend Auth API and authentication Logic to enable m… #614

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

ok2c
Copy link
Member

@ok2c ok2c commented Jan 22, 2025

…utual authentication

@ok2c
Copy link
Member Author

ok2c commented Jan 22, 2025

@stoty Your changes are mostly intact. I folded AuthScheme2 into AuthScheme and changed some javadocs. Please review and approve.

@stoty
Copy link
Contributor

stoty commented Jan 22, 2025

Thank you @ok2c .

Using a default method instead of a new interface is a definite improvement over my version.

Two small issues:

  1. This is only half of the solution for HTTPCLIENT-1625, I think it would be better to use
    HTTPCLIENT-2356 in the commit message.
    (I have not updated the commit message on my original PR to preserve continuity while reviewing, but was planning to)

  2. This has my original mix of "authorization"/"authentication" log messages, you may want do a pass on those.

@stoty
Copy link
Contributor

stoty commented Jan 22, 2025

I was also planning to revert the disabled API check maven plugin before commit.

@ok2c ok2c changed the title HTTPCLIENT-1625: Extend Auth API and authentication Logic to enable m… HTTPCLIENT-2356: Extend Auth API and authentication Logic to enable m… Jan 22, 2025
@ok2c
Copy link
Member Author

ok2c commented Jan 22, 2025

@stoty Commit message and log statements have been corrected. Please do another pass.

Copy link
Contributor

@stoty stoty left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@ok2c
Copy link
Member Author

ok2c commented Jan 22, 2025

I am going to merge the PR later today unless someone objects. @michael-o Please do a post-commit review. If you see anything wrong with the change-set I will revert it.

@michael-o
Copy link
Member

Will review this night

@ok2c
Copy link
Member Author

ok2c commented Jan 22, 2025

@michael-o Corrected. Please do another pass.

@ok2c ok2c requested a review from michael-o January 22, 2025 20:39
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

No objections

Copy link
Member

@arturobernalg arturobernalg left a comment

Choose a reason for hiding this comment

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

LGTM

@ok2c ok2c merged commit 9b93b03 into apache:master Jan 22, 2025
10 checks passed
@ok2c ok2c deleted the HTTPCLIENT-2356 branch January 22, 2025 21:07
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.

4 participants