Update enabled-required authenticator metrics
This commit is contained in:
parent
ba73f757e2
commit
e6ab97dc5a
6
pom.xml
6
pom.xml
|
@ -363,6 +363,12 @@
|
|||
<artifactId>junit-jupiter-api</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.junit-pioneer</groupId>
|
||||
<artifactId>junit-pioneer</artifactId>
|
||||
<version>1.9.1</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
</dependencies>
|
||||
|
||||
|
|
|
@ -27,9 +27,11 @@ import org.whispersystems.textsecuregcm.util.Util;
|
|||
public class BaseAccountAuthenticator {
|
||||
|
||||
private static final String AUTHENTICATION_COUNTER_NAME = name(BaseAccountAuthenticator.class, "authentication");
|
||||
private static final String ENABLED_NOT_REQUIRED_AUTHENTICATION_COUNTER_NAME = name(BaseAccountAuthenticator.class,
|
||||
"enabledNotRequiredAuthentication");
|
||||
private static final String AUTHENTICATION_SUCCEEDED_TAG_NAME = "succeeded";
|
||||
private static final String AUTHENTICATION_FAILURE_REASON_TAG_NAME = "reason";
|
||||
private static final String AUTHENTICATION_ENABLED_REQUIRED_TAG_NAME = "enabledRequired";
|
||||
private static final String ENABLED_TAG_NAME = "enabled";
|
||||
private static final String AUTHENTICATION_HAS_STORY_CAPABILITY = "hasStoryCapability";
|
||||
|
||||
private static final String STORY_ADOPTION_COUNTER_NAME = name(BaseAccountAuthenticator.class, "storyAdoption");
|
||||
|
@ -37,6 +39,9 @@ public class BaseAccountAuthenticator {
|
|||
private static final String DAYS_SINCE_LAST_SEEN_DISTRIBUTION_NAME = name(BaseAccountAuthenticator.class, "daysSinceLastSeen");
|
||||
private static final String IS_PRIMARY_DEVICE_TAG = "isPrimary";
|
||||
|
||||
@VisibleForTesting
|
||||
static final char DEVICE_ID_SEPARATOR = '.';
|
||||
|
||||
private final AccountsManager accountsManager;
|
||||
private final Clock clock;
|
||||
|
||||
|
@ -54,7 +59,7 @@ public class BaseAccountAuthenticator {
|
|||
final String identifier;
|
||||
final long deviceId;
|
||||
|
||||
final int deviceIdSeparatorIndex = basicUsername.indexOf('.');
|
||||
final int deviceIdSeparatorIndex = basicUsername.indexOf(DEVICE_ID_SEPARATOR);
|
||||
|
||||
if (deviceIdSeparatorIndex == -1) {
|
||||
identifier = basicUsername;
|
||||
|
@ -99,15 +104,23 @@ public class BaseAccountAuthenticator {
|
|||
}
|
||||
|
||||
if (enabledRequired) {
|
||||
if (!device.get().isEnabled()) {
|
||||
final boolean deviceDisabled = !device.get().isEnabled();
|
||||
if (deviceDisabled) {
|
||||
failureReason = "deviceDisabled";
|
||||
return Optional.empty();
|
||||
}
|
||||
|
||||
if (!account.get().isEnabled()) {
|
||||
final boolean accountDisabled = !account.get().isEnabled();
|
||||
if (accountDisabled) {
|
||||
failureReason = "accountDisabled";
|
||||
}
|
||||
if (accountDisabled || deviceDisabled) {
|
||||
return Optional.empty();
|
||||
}
|
||||
} else {
|
||||
Metrics.counter(ENABLED_NOT_REQUIRED_AUTHENTICATION_COUNTER_NAME,
|
||||
ENABLED_TAG_NAME, String.valueOf(device.get().isEnabled() && account.get().isEnabled()),
|
||||
IS_PRIMARY_DEVICE_TAG, String.valueOf(device.get().isMaster()))
|
||||
.increment();
|
||||
}
|
||||
|
||||
AuthenticationCredentials deviceAuthenticationCredentials = device.get().getAuthenticationCredentials();
|
||||
|
@ -130,8 +143,7 @@ public class BaseAccountAuthenticator {
|
|||
return Optional.empty();
|
||||
} finally {
|
||||
Tags tags = Tags.of(
|
||||
AUTHENTICATION_SUCCEEDED_TAG_NAME, String.valueOf(succeeded),
|
||||
AUTHENTICATION_ENABLED_REQUIRED_TAG_NAME, String.valueOf(enabledRequired));
|
||||
AUTHENTICATION_SUCCEEDED_TAG_NAME, String.valueOf(succeeded));
|
||||
|
||||
if (StringUtils.isNotBlank(failureReason)) {
|
||||
tags = tags.and(AUTHENTICATION_FAILURE_REASON_TAG_NAME, failureReason);
|
||||
|
|
|
@ -19,7 +19,6 @@ import static org.mockito.Mockito.verify;
|
|||
import static org.mockito.Mockito.when;
|
||||
|
||||
import io.dropwizard.auth.basic.BasicCredentials;
|
||||
import java.time.Clock;
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
|
@ -31,7 +30,7 @@ import org.junit.jupiter.params.ParameterizedTest;
|
|||
import org.junit.jupiter.params.provider.Arguments;
|
||||
import org.junit.jupiter.params.provider.MethodSource;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager;
|
||||
import org.junitpioneer.jupiter.cartesian.CartesianTest;
|
||||
import org.whispersystems.textsecuregcm.storage.Account;
|
||||
import org.whispersystems.textsecuregcm.storage.AccountsManager;
|
||||
import org.whispersystems.textsecuregcm.storage.Device;
|
||||
|
@ -208,37 +207,46 @@ class BaseAccountAuthenticatorTest {
|
|||
verify(accountsManager, never()).updateDeviceAuthentication(any(), any(), any());
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(booleans = {true, false})
|
||||
void testAuthenticateEnabledRequired(final boolean enabledRequired) {
|
||||
@CartesianTest
|
||||
void testAuthenticateEnabledRequired(
|
||||
@CartesianTest.Values(booleans = {true, false}) final boolean enabledRequired,
|
||||
@CartesianTest.Values(booleans = {true, false}) final boolean accountEnabled,
|
||||
@CartesianTest.Values(booleans = {true, false}) final boolean deviceEnabled,
|
||||
@CartesianTest.Values(booleans = {true, false}) final boolean authenticatedDeviceIsPrimary) {
|
||||
final UUID uuid = UUID.randomUUID();
|
||||
final long deviceId = 1;
|
||||
final long deviceId = authenticatedDeviceIsPrimary ? 1 : 2;
|
||||
final String password = "12345";
|
||||
|
||||
final Account account = mock(Account.class);
|
||||
final Device device = mock(Device.class);
|
||||
final Device authenticatedDevice = mock(Device.class);
|
||||
final AuthenticationCredentials credentials = mock(AuthenticationCredentials.class);
|
||||
|
||||
clock.unpin();
|
||||
when(accountsManager.getByAccountIdentifier(uuid)).thenReturn(Optional.of(account));
|
||||
when(account.getUuid()).thenReturn(uuid);
|
||||
when(account.getDevice(deviceId)).thenReturn(Optional.of(device));
|
||||
when(account.isEnabled()).thenReturn(false);
|
||||
when(device.getId()).thenReturn(deviceId);
|
||||
when(device.isEnabled()).thenReturn(false);
|
||||
when(device.getAuthenticationCredentials()).thenReturn(credentials);
|
||||
when(account.getDevice(deviceId)).thenReturn(Optional.of(authenticatedDevice));
|
||||
when(account.isEnabled()).thenReturn(accountEnabled);
|
||||
when(authenticatedDevice.getId()).thenReturn(deviceId);
|
||||
when(authenticatedDevice.isEnabled()).thenReturn(deviceEnabled);
|
||||
when(authenticatedDevice.getAuthenticationCredentials()).thenReturn(credentials);
|
||||
when(credentials.verify(password)).thenReturn(true);
|
||||
when(credentials.getVersion()).thenReturn(AuthenticationCredentials.CURRENT_VERSION);
|
||||
|
||||
final String identifier;
|
||||
if (authenticatedDeviceIsPrimary) {
|
||||
identifier = uuid.toString();
|
||||
} else {
|
||||
identifier = uuid.toString() + BaseAccountAuthenticator.DEVICE_ID_SEPARATOR + deviceId;
|
||||
}
|
||||
final Optional<AuthenticatedAccount> maybeAuthenticatedAccount =
|
||||
baseAccountAuthenticator.authenticate(new BasicCredentials(uuid.toString(), password), enabledRequired);
|
||||
baseAccountAuthenticator.authenticate(new BasicCredentials(identifier, password), enabledRequired);
|
||||
|
||||
if (enabledRequired) {
|
||||
if (enabledRequired && !(accountEnabled && deviceEnabled)) {
|
||||
assertThat(maybeAuthenticatedAccount).isEmpty();
|
||||
} else {
|
||||
assertThat(maybeAuthenticatedAccount).isPresent();
|
||||
assertThat(maybeAuthenticatedAccount.get().getAccount().getUuid()).isEqualTo(uuid);
|
||||
assertThat(maybeAuthenticatedAccount.get().getAuthenticatedDevice()).isEqualTo(device);
|
||||
assertThat(maybeAuthenticatedAccount.get().getAuthenticatedDevice()).isEqualTo(authenticatedDevice);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue