Remove authentication via query parameters for websocket upgrade requests
This commit is contained in:
parent
790b9bbf01
commit
3ceaa8bd20
|
@ -8,9 +8,6 @@ package org.whispersystems.textsecuregcm.websocket;
|
||||||
import static org.whispersystems.textsecuregcm.util.HeaderUtils.basicCredentialsFromAuthHeader;
|
import static org.whispersystems.textsecuregcm.util.HeaderUtils.basicCredentialsFromAuthHeader;
|
||||||
|
|
||||||
import com.google.common.net.HttpHeaders;
|
import com.google.common.net.HttpHeaders;
|
||||||
import io.dropwizard.auth.basic.BasicCredentials;
|
|
||||||
import java.util.List;
|
|
||||||
import java.util.Map;
|
|
||||||
import javax.annotation.Nullable;
|
import javax.annotation.Nullable;
|
||||||
import org.eclipse.jetty.websocket.api.UpgradeRequest;
|
import org.eclipse.jetty.websocket.api.UpgradeRequest;
|
||||||
import org.whispersystems.textsecuregcm.auth.AccountAuthenticator;
|
import org.whispersystems.textsecuregcm.auth.AccountAuthenticator;
|
||||||
|
@ -26,7 +23,6 @@ public class WebSocketAccountAuthenticator implements WebSocketAuthenticator<Aut
|
||||||
private static final ReusableAuth<AuthenticatedDevice> CREDENTIALS_NOT_PRESENTED = ReusableAuth.anonymous();
|
private static final ReusableAuth<AuthenticatedDevice> CREDENTIALS_NOT_PRESENTED = ReusableAuth.anonymous();
|
||||||
|
|
||||||
private static final ReusableAuth<AuthenticatedDevice> INVALID_CREDENTIALS_PRESENTED = ReusableAuth.invalid();
|
private static final ReusableAuth<AuthenticatedDevice> INVALID_CREDENTIALS_PRESENTED = ReusableAuth.invalid();
|
||||||
|
|
||||||
private final AccountAuthenticator accountAuthenticator;
|
private final AccountAuthenticator accountAuthenticator;
|
||||||
private final PrincipalSupplier<AuthenticatedDevice> principalSupplier;
|
private final PrincipalSupplier<AuthenticatedDevice> principalSupplier;
|
||||||
|
|
||||||
|
@ -40,13 +36,7 @@ public class WebSocketAccountAuthenticator implements WebSocketAuthenticator<Aut
|
||||||
public ReusableAuth<AuthenticatedDevice> authenticate(final UpgradeRequest request)
|
public ReusableAuth<AuthenticatedDevice> authenticate(final UpgradeRequest request)
|
||||||
throws AuthenticationException {
|
throws AuthenticationException {
|
||||||
try {
|
try {
|
||||||
// If the `Authorization` header was set for the request it takes priority, and we use the result of the
|
return authenticatedAccountFromHeaderAuth(request.getHeader(HttpHeaders.AUTHORIZATION));
|
||||||
// header-based auth ignoring the result of the query-based auth.
|
|
||||||
final String authHeader = request.getHeader(HttpHeaders.AUTHORIZATION);
|
|
||||||
if (authHeader != null) {
|
|
||||||
return authenticatedAccountFromHeaderAuth(authHeader);
|
|
||||||
}
|
|
||||||
return authenticatedAccountFromQueryParams(request);
|
|
||||||
} catch (final Exception e) {
|
} catch (final Exception e) {
|
||||||
// this will be handled and logged upstream
|
// this will be handled and logged upstream
|
||||||
// the most likely exception is a transient error connecting to account storage
|
// the most likely exception is a transient error connecting to account storage
|
||||||
|
@ -54,23 +44,7 @@ public class WebSocketAccountAuthenticator implements WebSocketAuthenticator<Aut
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private ReusableAuth<AuthenticatedDevice> authenticatedAccountFromQueryParams(final UpgradeRequest request) {
|
private ReusableAuth<AuthenticatedDevice> authenticatedAccountFromHeaderAuth(@Nullable final String authHeader) {
|
||||||
final Map<String, List<String>> parameters = request.getParameterMap();
|
|
||||||
final List<String> usernames = parameters.get("login");
|
|
||||||
final List<String> passwords = parameters.get("password");
|
|
||||||
if (usernames == null || usernames.size() == 0 ||
|
|
||||||
passwords == null || passwords.size() == 0) {
|
|
||||||
return CREDENTIALS_NOT_PRESENTED;
|
|
||||||
}
|
|
||||||
final BasicCredentials credentials = new BasicCredentials(usernames.get(0).replace(" ", "+"),
|
|
||||||
passwords.get(0).replace(" ", "+"));
|
|
||||||
return accountAuthenticator.authenticate(credentials)
|
|
||||||
.map(authenticatedAccount -> ReusableAuth.authenticated(authenticatedAccount, this.principalSupplier))
|
|
||||||
.orElse(INVALID_CREDENTIALS_PRESENTED);
|
|
||||||
}
|
|
||||||
|
|
||||||
private ReusableAuth<AuthenticatedDevice> authenticatedAccountFromHeaderAuth(@Nullable final String authHeader)
|
|
||||||
throws AuthenticationException {
|
|
||||||
if (authHeader == null) {
|
if (authHeader == null) {
|
||||||
return CREDENTIALS_NOT_PRESENTED;
|
return CREDENTIALS_NOT_PRESENTED;
|
||||||
}
|
}
|
||||||
|
|
|
@ -13,8 +13,6 @@ import static org.mockito.Mockito.when;
|
||||||
import com.google.common.net.HttpHeaders;
|
import com.google.common.net.HttpHeaders;
|
||||||
import com.google.i18n.phonenumbers.PhoneNumberUtil;
|
import com.google.i18n.phonenumbers.PhoneNumberUtil;
|
||||||
import io.dropwizard.auth.basic.BasicCredentials;
|
import io.dropwizard.auth.basic.BasicCredentials;
|
||||||
import java.util.List;
|
|
||||||
import java.util.Map;
|
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import java.util.stream.Stream;
|
import java.util.stream.Stream;
|
||||||
import javax.annotation.Nullable;
|
import javax.annotation.Nullable;
|
||||||
|
@ -64,11 +62,9 @@ class WebSocketAccountAuthenticatorTest {
|
||||||
@MethodSource
|
@MethodSource
|
||||||
void testAuthenticate(
|
void testAuthenticate(
|
||||||
@Nullable final String authorizationHeaderValue,
|
@Nullable final String authorizationHeaderValue,
|
||||||
final Map<String, List<String>> upgradeRequestParameters,
|
|
||||||
final boolean expectAccount,
|
final boolean expectAccount,
|
||||||
final boolean expectInvalid) throws Exception {
|
final boolean expectInvalid) throws Exception {
|
||||||
|
|
||||||
when(upgradeRequest.getParameterMap()).thenReturn(upgradeRequestParameters);
|
|
||||||
if (authorizationHeaderValue != null) {
|
if (authorizationHeaderValue != null) {
|
||||||
when(upgradeRequest.getHeader(eq(HttpHeaders.AUTHORIZATION))).thenReturn(authorizationHeaderValue);
|
when(upgradeRequest.getHeader(eq(HttpHeaders.AUTHORIZATION))).thenReturn(authorizationHeaderValue);
|
||||||
}
|
}
|
||||||
|
@ -84,29 +80,16 @@ class WebSocketAccountAuthenticatorTest {
|
||||||
}
|
}
|
||||||
|
|
||||||
private static Stream<Arguments> testAuthenticate() {
|
private static Stream<Arguments> testAuthenticate() {
|
||||||
final Map<String, List<String>> paramsMapWithValidAuth =
|
|
||||||
Map.of("login", List.of(VALID_USER), "password", List.of(VALID_PASSWORD));
|
|
||||||
final Map<String, List<String>> paramsMapWithInvalidAuth =
|
|
||||||
Map.of("login", List.of(INVALID_USER), "password", List.of(INVALID_PASSWORD));
|
|
||||||
final String headerWithValidAuth =
|
final String headerWithValidAuth =
|
||||||
HeaderUtils.basicAuthHeader(VALID_USER, VALID_PASSWORD);
|
HeaderUtils.basicAuthHeader(VALID_USER, VALID_PASSWORD);
|
||||||
final String headerWithInvalidAuth =
|
final String headerWithInvalidAuth =
|
||||||
HeaderUtils.basicAuthHeader(INVALID_USER, INVALID_PASSWORD);
|
HeaderUtils.basicAuthHeader(INVALID_USER, INVALID_PASSWORD);
|
||||||
return Stream.of(
|
return Stream.of(
|
||||||
// if `Authorization` header is present, outcome should not depend on the value of query parameters
|
Arguments.of(headerWithValidAuth, true, false),
|
||||||
Arguments.of(headerWithValidAuth, Map.of(), true, false),
|
Arguments.of(headerWithInvalidAuth, false, true),
|
||||||
Arguments.of(headerWithInvalidAuth, Map.of(), false, true),
|
Arguments.of("invalid header value", false, true),
|
||||||
Arguments.of("invalid header value", Map.of(), false, true),
|
// if `Authorization` header is not set, we expect no account and anonymous credentials
|
||||||
Arguments.of(headerWithValidAuth, paramsMapWithValidAuth, true, false),
|
Arguments.of(null, false, false)
|
||||||
Arguments.of(headerWithInvalidAuth, paramsMapWithValidAuth, false, true),
|
|
||||||
Arguments.of("invalid header value", paramsMapWithValidAuth, false, true),
|
|
||||||
Arguments.of(headerWithValidAuth, paramsMapWithInvalidAuth, true, false),
|
|
||||||
Arguments.of(headerWithInvalidAuth, paramsMapWithInvalidAuth, false, true),
|
|
||||||
Arguments.of("invalid header value", paramsMapWithInvalidAuth, false, true),
|
|
||||||
// if `Authorization` header is not set, outcome should match the query params based auth
|
|
||||||
Arguments.of(null, paramsMapWithValidAuth, true, false),
|
|
||||||
Arguments.of(null, paramsMapWithInvalidAuth, false, true),
|
|
||||||
Arguments.of(null, Map.of(), false, false)
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue