BasicAuthenticator checks only the first WWW-Authenticate header

[ychernysh]

BasicAuthenticator from jersey-client library currently only checks the first value of WWW-Authentication header:

final String authenticate = response.getHeaders().getFirst(HttpHeaders.WWW_AUTHENTICATE);

Which means that if a server returned multiple WWW-Authenticate headers (say, for Basic & Bearer authentication methods), and Basic IS in that list, but IS NOT the first value, which might be the case, then BasicAuthenticator will fail to do its job. This ticket is to make BasicAuthenticator honor the case above by checking all of the WWW-Authenticate values (to pick the one that starts with Basic).

[jansupol]

@ychernysh Hi, would you care to create a PR?

[ychernysh]

@jansupol I haven't signed any contributing agreements yet and I'm not sure which branch to use, but here is the patch I suggest:

diff --git a/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java b/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java
index 96370662a..77ffeeb48 100644
--- a/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java
+++ b/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java
@@ -96,20 +96,21 @@ final class BasicAuthenticator {
      * @throws ResponseAuthenticationException in case that basic credentials missing or are in invalid format
      */
     public boolean filterResponseAndAuthenticate(ClientRequestContext request, ClientResponseContext response) {
-        final String authenticate = response.getHeaders().getFirst(HttpHeaders.WWW_AUTHENTICATE);
-        if (authenticate != null && authenticate.trim().toUpperCase(Locale.ROOT).startsWith("BASIC")) {
-            HttpAuthenticationFilter.Credentials credentials = HttpAuthenticationFilter
-                    .getCredentials(request, defaultCredentials, HttpAuthenticationFilter.Type.BASIC);
-
-            if (credentials == null) {
-                if (response.hasEntity()) {
-                    AuthenticationUtil.discardInputAndClose(response.getEntityStream());
-                }
-                throw new ResponseAuthenticationException(null, LocalizationMessages.AUTHENTICATION_CREDENTIALS_MISSING_BASIC());
-            }
+        if (response.getHeaders().get(HttpHeaders.WWW_AUTHENTICATE).stream()
+                .noneMatch(h -> h != null && h.toUpperCase(Locale.ROOT).startsWith("BASIC"))) {
+            return false;
+        }
+
+        HttpAuthenticationFilter.Credentials credentials = HttpAuthenticationFilter
+                .getCredentials(request, defaultCredentials, HttpAuthenticationFilter.Type.BASIC);
 
-            return HttpAuthenticationFilter.repeatRequest(request, response, calculateAuthentication(credentials));
+        if (credentials == null) {
+            if (response.hasEntity()) {
+                AuthenticationUtil.discardInputAndClose(response.getEntityStream());
+            }
+            throw new ResponseAuthenticationException(null, LocalizationMessages.AUTHENTICATION_CREDENTIALS_MISSING_BASIC());
         }
-        return false;
+
+        return HttpAuthenticationFilter.repeatRequest(request, response, calculateAuthentication(credentials));
     }
 }

It doesn't contain any unit tests though, because as I see it would require adding PowerMock to mock static call HttpAuthenticationFilter#repeatRequest. I've tested the patch only manually.

[senivam]

probably we should check the list of header values not to be NULL as well...

final List<String> authHeaders = response.getHeaders().get(HttpHeaders.WWW_AUTHENTICATE);
        if (authHeaders == null || authHeaders.stream()
                .noneMatch(h -> h != null && h.toUpperCase(Locale.ROOT).startsWith("BASIC"))) {
            return false;
        }

[ychernysh]

@senivam sure, here is updated and tested patch:

diff --git a/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java b/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java
index 96370662a..8e55a3578 100644
--- a/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java
+++ b/core-client/src/main/java/org/glassfish/jersey/client/authentication/BasicAuthenticator.java
@@ -17,6 +17,7 @@
 package org.glassfish.jersey.client.authentication;
 
 import java.util.Base64;
+import java.util.List;
 import java.util.Locale;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -96,20 +97,22 @@ final class BasicAuthenticator {
      * @throws ResponseAuthenticationException in case that basic credentials missing or are in invalid format
      */
     public boolean filterResponseAndAuthenticate(ClientRequestContext request, ClientResponseContext response) {
-        final String authenticate = response.getHeaders().getFirst(HttpHeaders.WWW_AUTHENTICATE);
-        if (authenticate != null && authenticate.trim().toUpperCase(Locale.ROOT).startsWith("BASIC")) {
-            HttpAuthenticationFilter.Credentials credentials = HttpAuthenticationFilter
-                    .getCredentials(request, defaultCredentials, HttpAuthenticationFilter.Type.BASIC);
-
-            if (credentials == null) {
-                if (response.hasEntity()) {
-                    AuthenticationUtil.discardInputAndClose(response.getEntityStream());
-                }
-                throw new ResponseAuthenticationException(null, LocalizationMessages.AUTHENTICATION_CREDENTIALS_MISSING_BASIC());
-            }
+        final List<String> authHeaders = response.getHeaders().get(HttpHeaders.WWW_AUTHENTICATE);
+        if (authHeaders == null || authHeaders.stream()
+                .noneMatch(h -> h != null && h.toUpperCase(Locale.ROOT).startsWith("BASIC"))) {
+            return false;
+        }
+
+        HttpAuthenticationFilter.Credentials credentials = HttpAuthenticationFilter
+                .getCredentials(request, defaultCredentials, HttpAuthenticationFilter.Type.BASIC);
 
-            return HttpAuthenticationFilter.repeatRequest(request, response, calculateAuthentication(credentials));
+        if (credentials == null) {
+            if (response.hasEntity()) {
+                AuthenticationUtil.discardInputAndClose(response.getEntityStream());
+            }
+            throw new ResponseAuthenticationException(null, LocalizationMessages.AUTHENTICATION_CREDENTIALS_MISSING_BASIC());
         }
-        return false;
+
+        return HttpAuthenticationFilter.repeatRequest(request, response, calculateAuthentication(credentials));
     }
 }

[senivam]

@ychernysh, I've created the #5884 PR based on your patch. Feel free to post any notes to it.