Skip to content

Issue #13398 - Improve LoginAuthenticators to handle Proxy-Authenticate#13452

Merged
sbordet merged 4 commits intojetty:jetty-12.1.xfrom
znight1020:jetty-12.1.x-13398-handleProxyAuthenticate
Aug 24, 2025
Merged

Issue #13398 - Improve LoginAuthenticators to handle Proxy-Authenticate#13452
sbordet merged 4 commits intojetty:jetty-12.1.xfrom
znight1020:jetty-12.1.x-13398-handleProxyAuthenticate

Conversation

@znight1020
Copy link
Contributor

@znight1020 znight1020 commented Aug 12, 2025

This pull request resolves issue #13398 by enhancing server-side authenticators to support proxy authentication.

Summary of Changes

  1. Authenticator Enhancements:

    • Server-side authenticators like BasicAuthenticator, DigestAuthenticator and SPNEGOAuthenticator no longer handle only hardcoded WWW-Authenticate headers.
    • A new setProxyMode(boolean) method has been added to the base LoginAuthenticator in both jetty-security and jetty-ee9-security.
    • When proxy mode is enabled, these authenticators now dynamically issue 407 challenges with Proxy-Authenticate headers and process credentials from the Proxy-Authorization header, using new internal helper methods.
  2. Test Refactoring:

    • The parent HttpClientAuthenticationTest has been refactored to use a new protected boolean isProxyMode() method (which defaults to false), allowing it to test both authentication modes.
    • A new HttpClientProxyAuthenticationTest subclass is introduced, which simply overrides isProxyMode() to return true, reusing the entire parent test suite for comprehensive validation of the new proxy mode.

Resolves #13398

@znight1020 znight1020 force-pushed the jetty-12.1.x-13398-handleProxyAuthenticate branch from e9fc606 to 472f55e Compare August 12, 2025 20:26
@sbordet sbordet self-requested a review August 13, 2025 07:11
@sbordet sbordet moved this to 👀 In review in Jetty 12.1.1 - FROZEN Aug 13, 2025
@sbordet sbordet linked an issue Aug 13, 2025 that may be closed by this pull request
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

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

@znight1020 excellent work!
Please follow the few renamings that I suggested, and then we should be good to merge.

@znight1020
Copy link
Contributor Author

Thanks for the feedback, @sbordet!

I've implemented the requested renamings and Javadoc improvements, but before I push the changes, I had a quick question regarding testInfiniteAuthentication, which I've detailed in the unresolved conversation above.

@sbordet sbordet self-requested a review August 13, 2025 16:28
@sbordet
Copy link
Contributor

sbordet commented Aug 13, 2025

@znight1020 I don't see the changes after review.
Did you push them?

@znight1020 znight1020 force-pushed the jetty-12.1.x-13398-handleProxyAuthenticate branch from efefd6e to 35066ed Compare August 13, 2025 17:20
@znight1020
Copy link
Contributor Author

znight1020 commented Aug 13, 2025

Sorry for that, @sbordet. I've just pushed the requested changes now!

To get the tests passing, I've temporarily updated testInfiniteAuthentication() to use startBasic(), which resolves the NullPointerException. However, my original question from the thread still stands: #13452 (comment)

Could you please take a look at the reasoning in that thread when you have a moment?

@znight1020 znight1020 force-pushed the jetty-12.1.x-13398-handleProxyAuthenticate branch from 35066ed to 7690cfc Compare August 14, 2025 06:19
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

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

Just a typo in the javadocs, and fix testInfiniteAuthentication() and we should be good.

@znight1020
Copy link
Contributor Author

znight1020 commented Aug 14, 2025

I've corrected the typo in the Javadocs and updated testInfiniteAuthentication.
Thanks again for all the feedback and guidance through the process!

@znight1020 znight1020 requested a review from sbordet August 14, 2025 06:56
@sbordet
Copy link
Contributor

sbordet commented Aug 14, 2025

@znight1020 excellent work!
This PR is approved, we'll wait on the CI build and then merge it.
Thanks!

@znight1020 znight1020 force-pushed the jetty-12.1.x-13398-handleProxyAuthenticate branch from c96377e to daf4d0d Compare August 22, 2025 02:56
@sbordet sbordet merged commit 088ba25 into jetty:jetty-12.1.x Aug 24, 2025
4 of 7 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Jetty 12.1.1 - FROZEN Aug 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Improve LoginAuthenticators to handle Proxy-Authenticate

2 participants