From 1d86c9c3d128f64afa51460e3ed7900963755cfd Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 27 Jul 2017 16:08:29 +0200 Subject: [PATCH] Use Credentials object instead of 2 attributes for Basic Authentication This commit changes the usage of two separate attributes (username and password) into one: a single `Credentials` object. Additionally, the attributes key under which the credentials are stored is changed to be specific to Basic Authentication, in order to allow for other sorts of authentication later. Issue: SPR-15764 --- .../client/ExchangeFilterFunctions.java | 114 +++++++++++++----- .../client/ExchangeFilterFunctionsTests.java | 13 +- 2 files changed, 94 insertions(+), 33 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctions.java index 9053fdb9c9..5820b8b383 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctions.java @@ -16,10 +16,12 @@ package org.springframework.web.reactive.function.client; -import java.nio.charset.Charset; +import java.nio.charset.CharsetEncoder; import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.Map; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; @@ -40,52 +42,45 @@ import org.springframework.util.Assert; public abstract class ExchangeFilterFunctions { /** - * Name of the {@link ClientRequest} attribute that contains the username, as used by + * Name of the {@link ClientRequest} attribute that contains the {@link Credentials}, as used by * {@link #basicAuthentication()} */ - public static final String USERNAME_ATTRIBUTE = ExchangeFilterFunctions.class.getName() + ".username"; - - /** - * Name of the {@link ClientRequest} attribute that contains the password, as used by - * {@link #basicAuthentication()} - */ - public static final String PASSWORD_ATTRIBUTE = ExchangeFilterFunctions.class.getName() + ".password"; + public static final String BASIC_AUTHENTICATION_CREDENTIALS_ATTRIBUTE = ExchangeFilterFunctions.class.getName() + ".basicAuthenticationCredentials"; /** * Return a filter that adds an Authorization header for HTTP Basic Authentication, based on * the given username and password. + *

Note that Basic Authentication only supports characters in the + * {@link StandardCharsets#ISO_8859_1 ISO-8859-1} character set. * @param username the username to use * @param password the password to use * @return the {@link ExchangeFilterFunction} that adds the Authorization header + * @throws IllegalArgumentException if either {@code username} or {@code password} contain + * characters that cannot be encoded to ISO-8859-1 */ public static ExchangeFilterFunction basicAuthentication(String username, String password) { Assert.notNull(username, "'username' must not be null"); Assert.notNull(password, "'password' must not be null"); + checkIllegalCharacters(username, password); return basicAuthenticationInternal(r -> Optional.of(new Credentials(username, password))); } /** * Return a filter that adds an Authorization header for HTTP Basic Authentication, based on - * the username and password provided in the - * {@linkplain ClientRequest#attributes() request attributes}. If the attributes are not found, - * no authorization header + * the {@link Credentials} provided in the + * {@linkplain ClientRequest#attributes() request attributes}. If the attribute is not found, + * no authorization header is added. + *

Note that Basic Authentication only supports characters in the + * {@link StandardCharsets#ISO_8859_1 ISO-8859-1} character set. * @return the {@link ExchangeFilterFunction} that adds the Authorization header - * @see #USERNAME_ATTRIBUTE - * @see #PASSWORD_ATTRIBUTE + * @see #BASIC_AUTHENTICATION_CREDENTIALS_ATTRIBUTE + * @see Credentials#basicAuthenticationCredentials(String, String) */ public static ExchangeFilterFunction basicAuthentication() { return basicAuthenticationInternal( - request -> { - Optional username = request.attribute(USERNAME_ATTRIBUTE).map(o -> (String)o); - Optional password = request.attribute(PASSWORD_ATTRIBUTE).map(o -> (String)o); - if (username.isPresent() && password.isPresent()) { - return Optional.of(new Credentials(username.get(), password.get())); - } else { - return Optional.empty(); - } - }); + request -> request.attribute(BASIC_AUTHENTICATION_CREDENTIALS_ATTRIBUTE).map(o -> (Credentials)o)); } private static ExchangeFilterFunction basicAuthenticationInternal( @@ -106,12 +101,28 @@ public abstract class ExchangeFilterFunctions { } private static String authorization(Credentials credentials) { - byte[] credentialBytes = credentials.toByteArray(StandardCharsets.ISO_8859_1); + String credentialsString = credentials.username + ":" + credentials.password; + byte[] credentialBytes = credentialsString.getBytes(StandardCharsets.ISO_8859_1); byte[] encodedBytes = Base64.getEncoder().encode(credentialBytes); String encodedCredentials = new String(encodedBytes, StandardCharsets.ISO_8859_1); return "Basic " + encodedCredentials; } + /* + * Basic authentication only supports ISO 8859-1, see + * https://stackoverflow.com/questions/702629/utf-8-characters-mangled-in-http-basic-auth-username#703341 + */ + private static void checkIllegalCharacters(String username, String password) { + CharsetEncoder encoder = StandardCharsets.ISO_8859_1.newEncoder(); + if (!encoder.canEncode(username) || !encoder.canEncode(password)) { + throw new IllegalArgumentException( + "Username or password contains characters that cannot be encoded to ISO-8859-1"); + } + + } + + + /** * Return a filter that returns a given {@link Throwable} as response if the given * {@link HttpStatus} predicate matches. @@ -140,20 +151,63 @@ public abstract class ExchangeFilterFunctions { } - private static final class Credentials { + /** + * Represents a combination of username and password, as used by {@link #basicAuthentication()}. + * @see #basicAuthenticationCredentials(String, String) + */ + public static final class Credentials { - private String username; + private final String username; - private String password; + private final String password; + /** + * Create a new {@code Credentials} instance with the given username and password. + * @param username the username + * @param password the password + */ public Credentials(String username, String password) { + Assert.notNull(username, "'username' must not be null"); + Assert.notNull(password, "'password' must not be null"); + this.username = username; this.password = password; } - public byte[] toByteArray(Charset charset) { - String credentials = this.username + ":" + this.password; - return credentials.getBytes(charset); + /** + * Return a consumer that stores the given username and password in the + * {@linkplain ClientRequest.Builder#attributes(java.util.function.Consumer) request + * attributes} as a {@code Credentials} object. + * @param username the username + * @param password the password + * @return a consumer that adds the given credentials to the attribute map + * @see ClientRequest.Builder#attributes(java.util.function.Consumer) + * @see #BASIC_AUTHENTICATION_CREDENTIALS_ATTRIBUTE + */ + public static Consumer> basicAuthenticationCredentials(String username, String password) { + Credentials credentials = new Credentials(username, password); + checkIllegalCharacters(username, password); + + return attributes -> attributes.put(BASIC_AUTHENTICATION_CREDENTIALS_ATTRIBUTE, credentials); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o instanceof Credentials) { + Credentials other = (Credentials) o; + return this.username.equals(other.username) && + this.password.equals(other.password); + + } + return false; + } + + @Override + public int hashCode() { + return 31 * this.username.hashCode() + this.password.hashCode(); } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctionsTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctionsTests.java index 5e4266207f..dd658e6ce9 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctionsTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/ExchangeFilterFunctionsTests.java @@ -28,6 +28,7 @@ import org.springframework.http.HttpStatus; import static org.junit.Assert.*; import static org.mockito.Mockito.*; import static org.springframework.http.HttpMethod.GET; +import static org.springframework.web.reactive.function.client.ExchangeFilterFunctions.Credentials.basicAuthenticationCredentials; /** * @author Arjen Poutsma @@ -84,7 +85,7 @@ public class ExchangeFilterFunctionsTests { } @Test - public void basicAuthentication() throws Exception { + public void basicAuthenticationUsernamePassword() throws Exception { ClientRequest request = ClientRequest.method(GET, URI.create("http://example.com")).build(); ClientResponse response = mock(ClientResponse.class); @@ -100,11 +101,17 @@ public class ExchangeFilterFunctionsTests { assertEquals(response, result); } + @Test(expected = IllegalArgumentException.class) + public void basicAuthenticationInvalidCharacters() throws Exception { + + ExchangeFilterFunctions.basicAuthentication("foo", "\ud83d\udca9"); + } + @Test public void basicAuthenticationAttributes() throws Exception { ClientRequest request = ClientRequest.method(GET, URI.create("http://example.com")) - .attribute(ExchangeFilterFunctions.USERNAME_ATTRIBUTE, "foo") - .attribute(ExchangeFilterFunctions.PASSWORD_ATTRIBUTE, "bar").build(); + .attributes(basicAuthenticationCredentials("foo", "bar")) + .build(); ClientResponse response = mock(ClientResponse.class); ExchangeFunction exchange = r -> {