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.

Sign up for GitHub

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

Jump to bottom

Fixes #5845 - Use UTF-8 encoding for client basic auth if requested. #5847

Merged
merged 4 commits into from Jan 12, 2021
Merged

Fixes #5845 - Use UTF-8 encoding for client basic auth if requested. #5847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4caf07
Fixes #5845 - Use UTF-8 encoding for client basic auth if requested.
sbordet Dec 29, 2020
655c2e3
Fixes #5845 - Use UTF-8 encoding for client basic auth if requested.
sbordet Dec 30, 2020
6535f2d
Fixes #5845 - Use UTF-8 encoding for client basic auth if requested.
sbordet Dec 30, 2020
b050bdb
Merged branch 'jetty-9.4.x' into 'jetty-9.4.x-5845-basic_auth_charset'.
sbordet Dec 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java
View file
Open in desktop
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

Expand Down
42 changes: 31 additions & 11 deletions jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -141,6 +151,16 @@ 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());
// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
testAuthentication(scenario, new BasicAuthentication(uri, realm, "basic_utf8", "\u20AC"));
}

@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testDigestAuthentication(Scenario scenario) throws Exception
Expand All @@ -159,11 +179,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<CountDownLatch> requests = new AtomicReference<>(new CountDownLatch(1));
AtomicReference<CountDownLatch> requests = new AtomicReference<>(new CountDownLatch(1));
Request.Listener.Adapter requestListener = new Request.Listener.Adapter()
{
@Override
Expand Down Expand Up @@ -247,7 +267,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
Expand Down Expand Up @@ -286,7 +306,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
Expand Down Expand Up @@ -314,7 +334,7 @@ public void testBasicAuthenticationWithAuthenticationRemoved(Scenario scenario)
{
startBasic(scenario, new EmptyServerHandler());

final AtomicReference<CountDownLatch> requests = new AtomicReference<>(new CountDownLatch(2));
AtomicReference<CountDownLatch> requests = new AtomicReference<>(new CountDownLatch(2));
Request.Listener.Adapter requestListener = new Request.Listener.Adapter()
{
@Override
Expand Down Expand Up @@ -386,7 +406,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
Expand All @@ -402,7 +422,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")
Expand Down
1 change: 1 addition & 0 deletions jetty-client/src/test/resources/realm.properties
View file
Open in desktop
@@ -1,4 +1,5 @@
# Format is <user>:<password>,<roles>
basic:basic
basic_utf8:\u20AC
digest:digest
spnego_client:,admin
32 changes: 18 additions & 14 deletions ...-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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
{
Expand All @@ -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)
{
Expand All @@ -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);
}
}
}
}
Expand All @@ -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);
Comment on lines +100 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe pre compute this string rather than recreate on every request?

response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
return Authentication.SEND_CONTINUE;
}
Expand Down