Copilot commented on code in PR #3863:
URL: https://github.com/apache/celeborn/pull/3863#discussion_r4244295661
##########
service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala:
##########
@@ -157,10 +157,23 @@ class AuthenticationFilter(conf: CelebornConf,
serviceName: String) extends Filt
try {
if (matchedHandler == null) {
logDebug(s"No auth scheme matched for url:
${httpRequest.getRequestURL}")
- httpResponse.setStatus(HttpServletResponse.SC_UNAUTHORIZED)
- httpResponse.sendError(
- HttpServletResponse.SC_UNAUTHORIZED,
- s"No auth scheme matched for $authorization")
+ if (authorization == null || authorization.isEmpty) {
+ // First request with no Authorization header. Send WWW-Authenticate
+ // challenge(s) to initiate auth handshake (e.g. SPNEGO/Kerberos
+ // requires a 401+WWW-Authenticate to start the handshake).
+ authSchemeHandlers.keys.foreach { scheme =>
+ httpResponse.addHeader("WWW-Authenticate", scheme.toString)
Review Comment:
Please add an automated test for the new unauthenticated challenge path. The
existing authentication suite configures multiple schemes but never sends a
request without `Authorization`; it should verify the 401 response contains one
`WWW-Authenticate` value for every registered scheme, while the
nonmatching-header path retains its existing behavior. This handshake bootstrap
is the core regression being fixed and is otherwise covered only by the manual
curl check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
