From f4caf07b172fa198be95d0353c5fd96735978683 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 29 Dec 2020 17:58:43 +0100 Subject: [PATCH 1/3] Fixes #5845 - Use UTF-8 encoding for client basic auth if requested. * Introduced get/setCharset in BasicAuthenticator on server-side. * Looking for the "charset" parameter on the client-side, and if there, use it. * Added test case. * Code cleanups. Signed-off-by: Simone Bordet --- .../client/util/BasicAuthentication.java | 14 ++++++- .../client/HttpClientAuthenticationTest.java | 41 ++++++++++++++----- .../src/test/resources/realm.properties | 1 + .../authentication/BasicAuthenticator.java | 32 ++++++++------- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java index 9f2217c24fb4..a231661a08c4 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.client.util; import java.net.URI; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Base64; @@ -63,7 +64,9 @@ public String getType() @Override public Result authenticate(Request request, ContentResponse response, HeaderInfo headerInfo, Attributes context) { - return new BasicResult(getURI(), headerInfo.getHeader(), user, password); + String charsetParam = headerInfo.getParameter("charset"); + Charset charset = charsetParam == null ? null : Charset.forName(charsetParam); + return new BasicResult(getURI(), headerInfo.getHeader(), user, password, charset); } /** @@ -88,10 +91,17 @@ public BasicResult(URI uri, String user, String password) } public BasicResult(URI uri, HttpHeader header, String user, String password) + { + this(uri, header, user, password, StandardCharsets.ISO_8859_1); + } + + public BasicResult(URI uri, HttpHeader header, String user, String password, Charset charset) { this.uri = uri; this.header = header; - byte[] authBytes = (user + ":" + password).getBytes(StandardCharsets.ISO_8859_1); + if (charset == null) + charset = StandardCharsets.ISO_8859_1; + byte[] authBytes = (user + ":" + password).getBytes(charset); this.value = "Basic " + Base64.getEncoder().encodeToString(authBytes); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java index e85394eb905c..aa28aea3e516 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java @@ -22,6 +22,8 @@ import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; @@ -79,17 +81,25 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest { private String realm = "TestRealm"; - public void startBasic(final Scenario scenario, Handler handler) throws Exception + public void startBasic(Scenario scenario, Handler handler) throws Exception { - start(scenario, new BasicAuthenticator(), handler); + startBasic(scenario, handler, null); } - public void startDigest(final Scenario scenario, Handler handler) throws Exception + public void startBasic(Scenario scenario, Handler handler, Charset charset) throws Exception + { + BasicAuthenticator authenticator = new BasicAuthenticator(); + if (charset != null) + authenticator.setCharset(charset); + start(scenario, authenticator, handler); + } + + public void startDigest(Scenario scenario, Handler handler) throws Exception { start(scenario, new DigestAuthenticator(), handler); } - private void start(final Scenario scenario, Authenticator authenticator, Handler handler) throws Exception + private void start(Scenario scenario, Authenticator authenticator, Handler handler) throws Exception { server = new Server(); File realmFile = MavenTestingUtils.getTestResourceFile("realm.properties"); @@ -141,6 +151,15 @@ public void testBasicAnyRealm(Scenario scenario) throws Exception testAuthentication(scenario, new BasicAuthentication(uri, ANY_REALM, "basic", "basic")); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testBasicWithUTF8Password(Scenario scenario) throws Exception + { + startBasic(scenario, new EmptyServerHandler(), StandardCharsets.UTF_8); + URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort()); + testAuthentication(scenario, new BasicAuthentication(uri, realm, "basic_utf8", "\u20AC")); + } + @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) public void testDigestAuthentication(Scenario scenario) throws Exception @@ -159,11 +178,11 @@ public void testDigestAnyRealm(Scenario scenario) throws Exception testAuthentication(scenario, new DigestAuthentication(uri, ANY_REALM, "digest", "digest")); } - private void testAuthentication(final Scenario scenario, Authentication authentication) throws Exception + private void testAuthentication(Scenario scenario, Authentication authentication) throws Exception { AuthenticationStore authenticationStore = client.getAuthenticationStore(); - final AtomicReference requests = new AtomicReference<>(new CountDownLatch(1)); + AtomicReference requests = new AtomicReference<>(new CountDownLatch(1)); Request.Listener.Adapter requestListener = new Request.Listener.Adapter() { @Override @@ -247,7 +266,7 @@ protected void service(String target, org.eclipse.jetty.server.Request jettyRequ URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort()); client.getAuthenticationStore().addAuthentication(new BasicAuthentication(uri, realm, "basic", "basic")); - final CountDownLatch requests = new CountDownLatch(3); + CountDownLatch requests = new CountDownLatch(3); Request.Listener.Adapter requestListener = new Request.Listener.Adapter() { @Override @@ -286,7 +305,7 @@ protected void service(String target, org.eclipse.jetty.server.Request jettyRequ URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort()); client.getAuthenticationStore().addAuthentication(new BasicAuthentication(uri, realm, "basic", "basic")); - final CountDownLatch requests = new CountDownLatch(3); + CountDownLatch requests = new CountDownLatch(3); Request.Listener.Adapter requestListener = new Request.Listener.Adapter() { @Override @@ -314,7 +333,7 @@ public void testBasicAuthenticationWithAuthenticationRemoved(Scenario scenario) { startBasic(scenario, new EmptyServerHandler()); - final AtomicReference requests = new AtomicReference<>(new CountDownLatch(2)); + AtomicReference requests = new AtomicReference<>(new CountDownLatch(2)); Request.Listener.Adapter requestListener = new Request.Listener.Adapter() { @Override @@ -386,7 +405,7 @@ public void testAuthenticationThrowsException(Scenario scenario) throws Exceptio // Request without Authentication would cause a 401, // but the client will throw an exception trying to // send the credentials to the server. - final String cause = "thrown_explicitly_by_test"; + String cause = "thrown_explicitly_by_test"; client.getAuthenticationStore().addAuthentication(new Authentication() { @Override @@ -402,7 +421,7 @@ public Result authenticate(Request request, ContentResponse response, HeaderInfo } }); - final CountDownLatch latch = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(1); client.newRequest("localhost", connector.getLocalPort()) .scheme(scenario.getScheme()) .path("/secure") diff --git a/jetty-client/src/test/resources/realm.properties b/jetty-client/src/test/resources/realm.properties index 27e300ad53cd..095506f9efa6 100644 --- a/jetty-client/src/test/resources/realm.properties +++ b/jetty-client/src/test/resources/realm.properties @@ -1,4 +1,5 @@ # Format is :, basic:basic +basic_utf8:\u20AC digest:digest spnego_client:,admin diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java index adf902eca15c..eaab6dc828d9 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.security.authentication; import java.io.IOException; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Base64; import javax.servlet.ServletRequest; @@ -34,28 +35,26 @@ import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.util.security.Constraint; -/** - * @version $Rev: 4793 $ $Date: 2009-03-19 00:00:01 +0100 (Thu, 19 Mar 2009) $ - */ public class BasicAuthenticator extends LoginAuthenticator { + private Charset _charset; - public BasicAuthenticator() + public Charset getCharset() { + return _charset; + } + + public void setCharset(Charset charset) + { + this._charset = charset; } - /** - * @see org.eclipse.jetty.security.Authenticator#getAuthMethod() - */ @Override public String getAuthMethod() { return Constraint.__BASIC_AUTH; } - /** - * @see org.eclipse.jetty.security.Authenticator#validateRequest(javax.servlet.ServletRequest, javax.servlet.ServletResponse, boolean) - */ @Override public Authentication validateRequest(ServletRequest req, ServletResponse res, boolean mandatory) throws ServerAuthException { @@ -77,7 +76,10 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b if ("basic".equalsIgnoreCase(method)) { credentials = credentials.substring(space + 1); - credentials = new String(Base64.getDecoder().decode(credentials), StandardCharsets.ISO_8859_1); + Charset charset = getCharset(); + if (charset == null) + charset = StandardCharsets.ISO_8859_1; + credentials = new String(Base64.getDecoder().decode(credentials), charset); int i = credentials.indexOf(':'); if (i > 0) { @@ -86,9 +88,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b UserIdentity user = login(username, password, request); if (user != null) - { return new UserAuthentication(getAuthMethod(), user); - } } } } @@ -97,7 +97,11 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b if (DeferredAuthentication.isDeferred(response)) return Authentication.UNAUTHENTICATED; - response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), "basic realm=\"" + _loginService.getName() + '"'); + String value = "basic realm=\"" + _loginService.getName() + "\""; + Charset charset = getCharset(); + if (charset != null) + value += ", charset=\"" + charset.name() + "\""; + response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), value); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); return Authentication.SEND_CONTINUE; } From 655c2e3eb11390a4f8702406fa739d14be8253b9 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 30 Dec 2020 12:05:29 +0100 Subject: [PATCH 2/3] Fixes #5845 - Use UTF-8 encoding for client basic auth if requested. Don't use unicode sequences to please CheckStyle. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpClientAuthenticationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java index aa28aea3e516..ae1763b516d7 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java @@ -157,7 +157,7 @@ public void testBasicWithUTF8Password(Scenario scenario) throws Exception { startBasic(scenario, new EmptyServerHandler(), StandardCharsets.UTF_8); URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort()); - testAuthentication(scenario, new BasicAuthentication(uri, realm, "basic_utf8", "\u20AC")); + testAuthentication(scenario, new BasicAuthentication(uri, realm, "basic_utf8", "€")); } @ParameterizedTest From 6535f2d6e77b5db2432895fdbc2edd915e85be71 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 30 Dec 2020 12:08:13 +0100 Subject: [PATCH 3/3] Fixes #5845 - Use UTF-8 encoding for client basic auth if requested. Disable CheckStyle check so that the unicode sequence in the test matches that in realm.properties. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpClientAuthenticationTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java index ae1763b516d7..4047d416146b 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java @@ -157,7 +157,8 @@ public void testBasicWithUTF8Password(Scenario scenario) throws Exception { startBasic(scenario, new EmptyServerHandler(), StandardCharsets.UTF_8); URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort()); - testAuthentication(scenario, new BasicAuthentication(uri, realm, "basic_utf8", "€")); + // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck + testAuthentication(scenario, new BasicAuthentication(uri, realm, "basic_utf8", "\u20AC")); } @ParameterizedTest