From 76ca81d9f17eb304008ec2fa702fdaa3e75c0cda Mon Sep 17 00:00:00 2001 From: Ravi Khadiwala Date: Tue, 21 Jul 2026 13:46:54 -0500 Subject: [PATCH] Sink remote config logic down a layer --- .../textsecuregcm/WhisperServerService.java | 4 +- .../controllers/RemoteConfigController.java | 93 +------ .../storage/RemoteConfigsManager.java | 97 ++++++- .../RemoteConfigControllerTest.java | 242 ++++-------------- .../storage/RemoteConfigsManagerTest.java | 165 +++++++++++- 5 files changed, 312 insertions(+), 289 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index ef3c4dc11..16b19c7e1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -790,7 +790,7 @@ public class WhisperServerService extends Application globalConfig; - private static final String GLOBAL_CONFIG_PREFIX = "global."; - private static final Set PLATFORM_PREFIXES = Arrays.stream(ClientPlatform.values()) - .map(p -> p.name().toLowerCase()) - .collect(Collectors.toSet()); - - public RemoteConfigController(RemoteConfigsManager remoteConfigsManager, Map globalConfig) { + public RemoteConfigController(RemoteConfigsManager remoteConfigsManager) { this.remoteConfigsManager = remoteConfigsManager; - this.globalConfig = globalConfig; } @GET @@ -88,69 +63,17 @@ public class RemoteConfigController { @HeaderParam(HttpHeaders.USER_AGENT) String userAgent ) { - final String platformPrefix = platformPrefix(userAgent); - final List remoteConfigs = remoteConfigsManager.getAll(); - try { - MessageDigest digest = MessageDigest.getInstance("SHA-256"); + final Map configs = remoteConfigsManager.getConfigForAccount(auth.accountIdentifier(), userAgent); - final Map configs = Stream.concat( - remoteConfigs.stream() - .filter(config -> { - final String firstNameComponent = config.getName().split("\\.", 2)[0]; - return firstNameComponent.equals(platformPrefix) || !PLATFORM_PREFIXES.contains(firstNameComponent); - }) - .map( - config -> { - final byte[] hashKey = config.getHashKey() != null - ? config.getHashKey().getBytes(StandardCharsets.UTF_8) - : config.getName().getBytes(StandardCharsets.UTF_8); - boolean inBucket = isInBucket(digest, auth.accountIdentifier(), hashKey, config.getPercentage(), config.getUuids()); - final String value = inBucket ? config.getValue() : config.getDefaultValue(); - return Pair.of(config.getName(), value == null ? String.valueOf(inBucket) : value); - }), - globalConfig.entrySet().stream() - .map(e -> Pair.of(GLOBAL_CONFIG_PREFIX + e.getKey(), e.getValue()))) - .collect(Collectors.toMap(Pair::getLeft, Pair::getRight)); - - final EntityTag newETag = new EntityTag(HexFormat.of().toHexDigits(configs.hashCode())); - if (newETag.equals(eTag)) { - return Response.notModified(eTag).build(); - } - - return Response.ok(new RemoteConfigurationResponse(configs)) - .tag(newETag) - .build(); - } catch (NoSuchAlgorithmException e) { - throw new AssertionError(e); - } - } - - private static String platformPrefix(final String userAgent) { - try { - return UserAgentUtil.parseUserAgentString(userAgent).platform().name().toLowerCase(); - } catch (UnrecognizedUserAgentException e) { - return null; - } - } - - @VisibleForTesting - public static boolean isInBucket(MessageDigest digest, UUID uid, byte[] hashKey, int configPercentage, - Set uuidsInBucket) { - if (uuidsInBucket.contains(uid)) { - return true; + final EntityTag newETag = new EntityTag(HexFormat.of().toHexDigits(configs.hashCode())); + if (newETag.equals(eTag)) { + return Response.notModified(eTag).build(); } - ByteBuffer bb = ByteBuffer.allocate(16); - bb.putLong(uid.getMostSignificantBits()); - bb.putLong(uid.getLeastSignificantBits()); - - digest.update(bb.array()); - - byte[] hash = digest.digest(hashKey); - int bucket = (int) (Util.ensureNonNegativeLong(Conversions.byteArrayToLong(hash)) % 100); - - return bucket < configPercentage; + return Response.ok(new RemoteConfigurationResponse(configs)) + .tag(newETag) + .build(); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManager.java index b4c553fc9..fc0fa677d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManager.java @@ -5,22 +5,111 @@ package org.whispersystems.textsecuregcm.storage; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Suppliers; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.apache.commons.lang3.tuple.Pair; +import org.whispersystems.textsecuregcm.util.Conversions; +import org.whispersystems.textsecuregcm.util.Util; +import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; +import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException; +import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil; public class RemoteConfigsManager { private final Supplier> remoteConfigSupplier; + private final Map globalConfig; - public RemoteConfigsManager(RemoteConfigs remoteConfigs) { - remoteConfigSupplier = + private static final String GLOBAL_CONFIG_PREFIX = "global."; + private static final Set PLATFORM_PREFIXES = Arrays.stream(ClientPlatform.values()) + .map(p -> p.name().toLowerCase()) + .collect(Collectors.toSet()); + + public RemoteConfigsManager(final RemoteConfigs remoteConfigs, final Map globalConfig) { + this.remoteConfigSupplier = Suppliers.memoizeWithExpiration(remoteConfigs::getAll, 10, TimeUnit.SECONDS); + this.globalConfig = globalConfig; } - public List getAll() { - return remoteConfigSupplier.get(); + /** + * Returns all remote configuration (client and global) for the specified account and client. + * + * @param accountIdentifier the identifier of the authenticated account + * @param userAgent the requester's user agent, used to select the client platform + * @return a map of namespaced configuration keys to their resolved values + */ + public Map getConfigForAccount(final UUID accountIdentifier, @Nullable final String userAgent) { + final String platformPrefix = platformPrefix(userAgent); + + final MessageDigest digest; + try { + digest = MessageDigest.getInstance("SHA-256"); + } catch (final NoSuchAlgorithmException e) { + throw new AssertionError(e); + } + + final List clientConfig = remoteConfigSupplier.get(); + + return Stream.concat( + clientConfig.stream() + .filter(config -> { + final String firstNameComponent = config.getName().split("\\.", 2)[0]; + return firstNameComponent.equals(platformPrefix) || !PLATFORM_PREFIXES.contains(firstNameComponent); + }) + .map(config -> { + final byte[] hashKey = config.getHashKey() != null + ? config.getHashKey().getBytes(StandardCharsets.UTF_8) + : config.getName().getBytes(StandardCharsets.UTF_8); + boolean inBucket = isInBucket(digest, accountIdentifier, hashKey, config.getPercentage(), + config.getUuids()); + final String value = inBucket ? config.getValue() : config.getDefaultValue(); + return Pair.of(config.getName(), value == null ? String.valueOf(inBucket) : value); + }), + + globalConfig.entrySet().stream() + .map(e -> Pair.of(GLOBAL_CONFIG_PREFIX + e.getKey(), e.getValue()))) + .collect(Collectors.toMap(Pair::getLeft, Pair::getRight)); + } + + @Nullable + private static String platformPrefix(@Nullable final String userAgent) { + try { + return UserAgentUtil.parseUserAgentString(userAgent).platform().name().toLowerCase(); + } catch (UnrecognizedUserAgentException e) { + return null; + } + } + + @VisibleForTesting + static boolean isInBucket(MessageDigest digest, UUID uid, byte[] hashKey, int configPercentage, + Set uuidsInBucket) { + if (uuidsInBucket.contains(uid)) { + return true; + } + + ByteBuffer bb = ByteBuffer.allocate(16); + bb.putLong(uid.getMostSignificantBits()); + bb.putLong(uid.getLeastSignificantBits()); + + digest.update(bb.array()); + + byte[] hash = digest.digest(hashKey); + int bucket = (int) (Util.ensureNonNegativeLong(Conversions.byteArrayToLong(hash)) % 100); + + return bucket < configPercentage; } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigControllerTest.java index 5f140b6f8..54a0b81e6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RemoteConfigControllerTest.java @@ -6,11 +6,8 @@ package org.whispersystems.textsecuregcm.controllers; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.entry; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -19,32 +16,23 @@ import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; import io.dropwizard.testing.junit5.ResourceExtension; import jakarta.ws.rs.core.EntityTag; import jakarta.ws.rs.core.Response; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Random; -import java.util.Set; -import org.assertj.core.data.Offset; +import java.util.TreeMap; +import java.util.stream.Collectors; import org.glassfish.jersey.test.grizzly.GrizzlyWebTestContainerFactory; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; -import org.junit.jupiter.params.provider.ValueSource; -import org.junitpioneer.jupiter.params.IntRangeSource; import org.whispersystems.textsecuregcm.auth.AuthenticatedDevice; -import org.whispersystems.textsecuregcm.entities.RemoteConfigurationResponse; import org.whispersystems.textsecuregcm.mappers.DeviceLimitExceededExceptionMapper; -import org.whispersystems.textsecuregcm.storage.RemoteConfig; import org.whispersystems.textsecuregcm.storage.RemoteConfigsManager; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; -import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; @ExtendWith(DropwizardExtensionsSupport.class) class RemoteConfigControllerTest { @@ -56,122 +44,14 @@ class RemoteConfigControllerTest { .addProvider(new AuthValueFactoryProvider.Binder<>(AuthenticatedDevice.class)) .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) .addProvider(new DeviceLimitExceededExceptionMapper()) - .addResource(new RemoteConfigController(remoteConfigsManager, Map.of("maxGroupSize", "42"))) + .addResource(new RemoteConfigController(remoteConfigsManager)) .build(); - @BeforeEach - void setup() throws Exception { - when(remoteConfigsManager.getAll()).thenReturn( - List.of( - new RemoteConfig("android.stickers", 100, Set.of(), null, null, null), - new RemoteConfig("ios.stickers", 100, Set.of(), null, null, null), - new RemoteConfig("desktop.stickers", 100, Set.of(), null, null, null), - new RemoteConfig("always.true", 100, Set.of(), null, null, null), - new RemoteConfig("only.special", 0, Set.of(AuthHelper.VALID_UUID), null, null, null), - new RemoteConfig("value.always.true", 100, Set.of(), "foo", "bar", null), - new RemoteConfig("value.only.special", 0, Set.of(AuthHelper.VALID_UUID), "abc", "xyz", null), - new RemoteConfig("value.always.false", 0, Set.of(), "red", "green", null), - new RemoteConfig("linked.config.0", 50, Set.of(), null, null, null), - new RemoteConfig("linked.config.1", 50, Set.of(), null, null, "linked.config.0"), - new RemoteConfig("unlinked.config", 50, Set.of(), null, null, null))); - } - - @AfterEach - void teardown() { + void setUp() { reset(remoteConfigsManager); } - @ParameterizedTest - @EnumSource - void testRetrieveConfig(ClientPlatform platform) { - RemoteConfigurationResponse configuration = resources.getJerseyTest() - .target("/v2/config/") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .header("User-Agent", String.format("Signal-%s/7.6.2", platform.name())) - .get(RemoteConfigurationResponse.class); - - verify(remoteConfigsManager, times(1)).getAll(); - - assertThat(configuration.config()).hasSize(10); - assertThat(configuration.config()).containsKeys(platform.name().toLowerCase() + ".stickers", "linked.config.0", "linked.config.1", "unlinked.config"); - assertThat(configuration.config()).contains( - entry("always.true", "true"), - entry("only.special", "true"), - entry("value.always.true", "bar"), - entry("value.only.special", "xyz"), - entry("value.always.false", "red"), - entry("global.maxGroupSize", "42")); - } - - @ParameterizedTest - @EnumSource - void testRetrieveConfigNotSpecial(ClientPlatform platform) { - RemoteConfigurationResponse configuration = resources.getJerseyTest() - .target("/v2/config/") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_TWO, AuthHelper.VALID_PASSWORD_TWO)) - .header("User-Agent", String.format("Signal-%s/7.6.2", platform.name())) - .get(RemoteConfigurationResponse.class); - - verify(remoteConfigsManager, times(1)).getAll(); - - assertThat(configuration.config()).hasSize(10); - assertThat(configuration.config()).containsKeys(platform.name().toLowerCase() + ".stickers", "linked.config.0", "linked.config.1", "unlinked.config"); - assertThat(configuration.config()).contains( - entry("always.true", "true"), - entry("only.special", "false"), - entry("value.always.true", "bar"), - entry("value.only.special", "abc"), - entry("value.always.false", "red"), - entry("global.maxGroupSize", "42")); - } - - @Test - void testRetrieveConfigUnrecognizedPlatform() { - RemoteConfigurationResponse configuration = resources.getJerseyTest() - .target("/v2/config/") - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_TWO, AuthHelper.VALID_PASSWORD_TWO)) - .header("User-Agent", "Third-Party-Signal-Client/1.0.0") - .get(RemoteConfigurationResponse.class); - - verify(remoteConfigsManager, times(1)).getAll(); - - assertThat(configuration.config()).hasSize(9); - assertThat(configuration.config()).containsKeys("linked.config.0", "linked.config.1", "unlinked.config"); - assertThat(configuration.config()).contains( - entry("always.true", "true"), - entry("only.special", "false"), - entry("value.always.true", "bar"), - entry("value.only.special", "abc"), - entry("value.always.false", "red"), - entry("global.maxGroupSize", "42")); - } - - @Test - void testHashKeyLinkedConfigs() { - boolean allUnlinkedConfigsMatched = true; - for (AuthHelper.TestAccount testAccount : AuthHelper.TEST_ACCOUNTS) { - RemoteConfigurationResponse configuration = resources.getJerseyTest() - .target("/v2/config/") - .request() - .header("Authorization", testAccount.getAuthHeader()) - .get(RemoteConfigurationResponse.class); - - assertThat(configuration.config().get("linked.config.0")).isEqualTo(configuration.config().get("linked.config.1")); - allUnlinkedConfigsMatched &= (configuration.config().get("linked.config.0").equals(configuration.config().get("unlinked.config"))); - } - - // with 20 test accounts, 1 in 2^20 chance that this fails when it shouldn't, but - // AuthHelper#generateTestAccounts uses a constant random seed that doesn't fail as of the time - // of this writing; if this starts failing for no apparent reason, it's likely that we've - // changed the order of the sequence of random numbers used during test initialization in such - // a way that we've accidentally picked an unlucky set of accounts here - assertThat(allUnlinkedConfigsMatched).isFalse(); - } - @Test void testRetrieveConfigUnauthorized() { Response response = resources.getJerseyTest() @@ -187,11 +67,15 @@ class RemoteConfigControllerTest { @Test void testRetrieveConfigUnchanged() { + final String userAgent = "user-agent"; + when(remoteConfigsManager.getConfigForAccount(AuthHelper.VALID_UUID, userAgent)) + .thenReturn(Map.of("test.test", "bar", "global.test", "false")); + Response response = resources.getJerseyTest() .target("/v2/config/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .header("User-Agent", "Signal-Android/7.6.2 Android/34 libsignal/0.46.0") + .header("User-Agent", userAgent) .get(); assertThat(response.getStatus()).isEqualTo(200); @@ -203,7 +87,7 @@ class RemoteConfigControllerTest { .target("/v2/config/") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .header("User-Agent", "Signal-Android/7.6.2 Android/34 libsignal/0.46.0") + .header("User-Agent", userAgent) .header("If-None-Match", etag) .get(); @@ -213,6 +97,15 @@ class RemoteConfigControllerTest { @Test void testRetrieveConfigChanged() { + final String userAgent = "Signal-Android/7.6.2 Android/34 libsignal/0.46.0"; + + final Map config1 = Map.of( + "android.stickers", "foo", + "test.test", "bar", + "global.test", "false"); + + when(remoteConfigsManager.getConfigForAccount(AuthHelper.VALID_UUID, userAgent)).thenReturn(config1); + Response response = resources.getJerseyTest() .target("/v2/config/") .request() @@ -225,9 +118,10 @@ class RemoteConfigControllerTest { final EntityTag etag = response.getEntityTag(); assertThat(etag).isNotNull(); - final List configs = new ArrayList<>(remoteConfigsManager.getAll()); - configs.add(new RemoteConfig("android.new.config", 100, Set.of(), null, null, null)); - when(remoteConfigsManager.getAll()).thenReturn(configs); + + final Map config2 = new HashMap<>(config1); + config2.put("android.new.config", "true"); + when(remoteConfigsManager.getConfigForAccount(AuthHelper.VALID_UUID, userAgent)).thenReturn(config2); response = resources.getJerseyTest() .target("/v2/config/") @@ -243,16 +137,18 @@ class RemoteConfigControllerTest { @ParameterizedTest @MethodSource - void testEtag(boolean expect304, String userAgent1, String authHeader1, String userAgent2, String authHeader2) { - // Use a deterministic config; account 1 is special, 2 and 3 are identical - List configs = remoteConfigsManager.getAll().stream().filter(config -> config.getPercentage() == 0 || config.getPercentage() == 100).toList(); - when(remoteConfigsManager.getAll()).thenReturn(configs); + void testEtag(boolean expect304, final Map config1, Map config2) { + final String ua1 = "user-agent-1"; + final String ua2 = "user-agent-2"; + + when(remoteConfigsManager.getConfigForAccount(AuthHelper.VALID_UUID, ua1)).thenReturn(config1); + when(remoteConfigsManager.getConfigForAccount(AuthHelper.VALID_UUID_TWO, ua2)).thenReturn(config2); Response response = resources.getJerseyTest() .target("/v2/config/") .request() - .header("Authorization", authHeader1) - .header("User-Agent", userAgent1) + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .header("User-Agent", ua1) .get(); assertThat(response.getStatus()).isEqualTo(200); @@ -263,8 +159,8 @@ class RemoteConfigControllerTest { response = resources.getJerseyTest() .target("/v2/config/") .request() - .header("Authorization", authHeader2) - .header("User-Agent", userAgent2) + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_TWO, AuthHelper.VALID_PASSWORD_TWO)) + .header("User-Agent", ua2) .header("If-None-Match", etag) .get(); @@ -278,63 +174,23 @@ class RemoteConfigControllerTest { } static List testEtag() { - final String uuid1AuthHeader = AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD); - final String uuid2AuthHeader = AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_TWO, AuthHelper.VALID_PASSWORD_TWO); - final String uuid3AuthHeader = AuthHelper.getAuthHeader(AuthHelper.VALID_UUID_3, AuthHelper.VALID_PASSWORD_3_PRIMARY); + final Map config1 = Map.of( + "android.stickers", "foo", + "test.test", "bar", + "global.test", "false"); + + final Map config2 = Map.of( + "android.stickers", "foo", + "test.test", "bar", + "global.test", "true"); - final String ios762 = "Signal-iOS/7.6.2 iOS/18.5 libsignal/0.46.0"; - final String android762 = "Signal-Android/7.6.2 Android/34 libsignal/0.46.0"; - final String android763 = "Signal-Android/7.6.3 Android/34 libsignal/0.46.0"; - // boolean is expect304 return List.of( - Arguments.argumentSet("User change", false, android762, uuid1AuthHeader, android762, uuid2AuthHeader), - Arguments.argumentSet("Irrelevant user change", true, android762, uuid2AuthHeader, android762, uuid3AuthHeader), - Arguments.argumentSet("User agent change", false, android762, uuid1AuthHeader, ios762, uuid1AuthHeader), - Arguments.argumentSet("Irrelevant user agent change", true, android762, uuid1AuthHeader, android763, uuid1AuthHeader) + Arguments.argumentSet("same", true, config1, config1), + Arguments.argumentSet("change", false, config1, config2), + Arguments.argumentSet("different sort", true, + new TreeMap<>(config1), + config1.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a,_) -> a, () -> new TreeMap(Comparator.reverseOrder())))) ); } - - @ParameterizedTest - @IntRangeSource(from = 1, to = 99) - void testMath(int percentage) throws NoSuchAlgorithmException { - final MessageDigest digest = MessageDigest.getInstance("SHA-256"); - final Random random = new Random(9424242L); // the seed value doesn't matter so much as it's constant to make the test not flaky - final int iterations = 10000; - int enabledCount = 0; - - for (int i = 0; i < iterations; i++) { - if (RemoteConfigController.isInBucket(digest, AuthHelper.getRandomUUID(random), "test".getBytes(), percentage, Set.of())) { - enabledCount++; - } - } - - - // https://en.wikipedia.org/wiki/Binomial_distribution#Expected_value_and_variance - final double expectedCount = iterations * percentage / 100.0; - final double stdev = Math.sqrt(expectedCount * (1 - percentage / 100.0)); - - // 3 standard deviations = 99.73% chance of success for one bucket, 23.5% - // chance of any failure in 99 buckets; if this starts failing after a - // change, run it again with a few different random seeds to make sure it - // fails only about on about one seed in four - assertThat((double) enabledCount).isCloseTo(expectedCount, Offset.offset(3 * stdev)); - } - - @ParameterizedTest - @ValueSource(ints = {0, 100}) - void testMathExactForZeroOrOneHundred(int percentage) throws NoSuchAlgorithmException { - final MessageDigest digest = MessageDigest.getInstance("SHA-256"); - final Random random = new Random(); - final int iterations = 10000; - int enabledCount = 0; - - for (int i = 0; i < iterations; i++) { - if (RemoteConfigController.isInBucket(digest, AuthHelper.getRandomUUID(random), "test".getBytes(), percentage, Set.of())) { - enabledCount++; - } - } - - assertThat(enabledCount).isEqualTo(iterations * percentage / 100); - } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManagerTest.java index 403069ddb..3697a9649 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/RemoteConfigsManagerTest.java @@ -5,30 +5,185 @@ package org.whispersystems.textsecuregcm.storage; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; -import org.junit.jupiter.api.Test; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.Set; +import java.util.UUID; +import org.assertj.core.data.Offset; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.junitpioneer.jupiter.params.IntRangeSource; +import org.whispersystems.textsecuregcm.tests.util.AuthHelper; +import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; class RemoteConfigsManagerTest { + private static final UUID NORMAL_ACI = UUID.randomUUID(); + private static final UUID SPECIAL_ACI = UUID.randomUUID(); private RemoteConfigs remoteConfigs; + private RemoteConfigsManager remoteConfigsManager; + private static final Map GLOBAL = Map.of("maxGroupSize", "42"); + private static final List CLIENT = List.of( + new RemoteConfig("android.stickers", 100, Set.of(), null, null, null), + new RemoteConfig("ios.stickers", 100, Set.of(), null, null, null), + new RemoteConfig("desktop.stickers", 100, Set.of(), null, null, null), + new RemoteConfig("always.true", 100, Set.of(), null, null, null), + new RemoteConfig("only.special", 0, Set.of(SPECIAL_ACI), null, null, null), + new RemoteConfig("value.always.true", 100, Set.of(), "foo", "bar", null), + new RemoteConfig("value.only.special", 0, Set.of(SPECIAL_ACI), "abc", "xyz", null), + new RemoteConfig("value.always.false", 0, Set.of(), "red", "green", null), + new RemoteConfig("linked.config.0", 50, Set.of(), null, null, null), + new RemoteConfig("linked.config.1", 50, Set.of(), null, null, "linked.config.0"), + new RemoteConfig("unlinked.config", 50, Set.of(), null, null, null)); + @BeforeEach void setup() { this.remoteConfigs = mock(RemoteConfigs.class); - this.remoteConfigsManager = new RemoteConfigsManager(remoteConfigs); + when(remoteConfigs.getAll()).thenReturn(CLIENT); + this.remoteConfigsManager = new RemoteConfigsManager(remoteConfigs, GLOBAL); } @Test - void testGetAll() { - remoteConfigsManager.getAll(); - remoteConfigsManager.getAll(); + void testGetConfigForAccount() { + remoteConfigsManager.getConfigForAccount(UUID.randomUUID(), null); + remoteConfigsManager.getConfigForAccount(UUID.randomUUID(), null); // A memoized supplier should prevent multiple calls to the underlying data source verify(remoteConfigs, times(1)).getAll(); } + + @ParameterizedTest + @EnumSource + void testRetrieveConfig(ClientPlatform platform) { + final Map configs = remoteConfigsManager.getConfigForAccount( + SPECIAL_ACI, + String.format("Signal-%s/7.6.2", platform.name())); + + verify(remoteConfigs, times(1)).getAll(); + + assertThat(configs).hasSize(10); + assertThat(configs).containsKeys(platform.name().toLowerCase() + ".stickers", "linked.config.0", "linked.config.1", "unlinked.config"); + assertThat(configs).contains( + entry("always.true", "true"), + entry("only.special", "true"), + entry("value.always.true", "bar"), + entry("value.only.special", "xyz"), + entry("value.always.false", "red"), + entry("global.maxGroupSize", "42")); + } + + @ParameterizedTest + @EnumSource + void testRetrieveConfigNotSpecial(ClientPlatform platform) { + final Map configs = remoteConfigsManager.getConfigForAccount( + NORMAL_ACI, + String.format("Signal-%s/7.6.2", platform.name())); + + verify(remoteConfigs, times(1)).getAll(); + + assertThat(configs).hasSize(10); + assertThat(configs).containsKeys(platform.name().toLowerCase() + ".stickers", "linked.config.0", "linked.config.1", "unlinked.config"); + assertThat(configs).contains( + entry("always.true", "true"), + entry("only.special", "false"), + entry("value.always.true", "bar"), + entry("value.only.special", "abc"), + entry("value.always.false", "red"), + entry("global.maxGroupSize", "42")); + } + + @Test + void testRetrieveConfigUnrecognizedPlatform() { + final Map configs = remoteConfigsManager.getConfigForAccount( + NORMAL_ACI, + "Third-Party-Signal-Client/1.0.0"); + + verify(remoteConfigs, times(1)).getAll(); + + assertThat(configs).hasSize(9); + assertThat(configs).containsKeys("linked.config.0", "linked.config.1", "unlinked.config"); + assertThat(configs).contains( + entry("always.true", "true"), + entry("only.special", "false"), + entry("value.always.true", "bar"), + entry("value.only.special", "abc"), + entry("value.always.false", "red"), + entry("global.maxGroupSize", "42")); + } + + @Test + void testHashKeyLinkedConfigs() { + boolean allUnlinkedConfigsMatched = true; + for (AuthHelper.TestAccount testAccount : AuthHelper.TEST_ACCOUNTS) { + final Map configs = remoteConfigsManager.getConfigForAccount(testAccount.uuid, null); + assertThat(configs.get("linked.config.0")).isEqualTo(configs.get("linked.config.1")); + allUnlinkedConfigsMatched &= (configs.get("linked.config.0").equals(configs.get("unlinked.config"))); + } + + // with 20 test accounts, 1 in 2^20 chance that this fails when it shouldn't, but + // AuthHelper#generateTestAccounts uses a constant random seed that doesn't fail as of the time + // of this writing; if this starts failing for no apparent reason, it's likely that we've + // changed the order of the sequence of random numbers used during test initialization in such + // a way that we've accidentally picked an unlucky set of accounts here + assertThat(allUnlinkedConfigsMatched).isFalse(); + } + + @ParameterizedTest + @IntRangeSource(from = 1, to = 99) + void testMath(int percentage) throws NoSuchAlgorithmException { + final MessageDigest digest = MessageDigest.getInstance("SHA-256"); + final Random random = new Random(9424242L); // the seed value doesn't matter so much as it's constant to make the test not flaky + final int iterations = 10000; + int enabledCount = 0; + + for (int i = 0; i < iterations; i++) { + if (RemoteConfigsManager.isInBucket(digest, AuthHelper.getRandomUUID(random), "test".getBytes(), percentage, Set.of())) { + enabledCount++; + } + } + + + // https://en.wikipedia.org/wiki/Binomial_distribution#Expected_value_and_variance + final double expectedCount = iterations * percentage / 100.0; + final double stdev = Math.sqrt(expectedCount * (1 - percentage / 100.0)); + + // 3 standard deviations = 99.73% chance of success for one bucket, 23.5% + // chance of any failure in 99 buckets; if this starts failing after a + // change, run it again with a few different random seeds to make sure it + // fails only about on about one seed in four + assertThat((double) enabledCount).isCloseTo(expectedCount, Offset.offset(3 * stdev)); + } + + @ParameterizedTest + @ValueSource(ints = {0, 100}) + void testMathExactForZeroOrOneHundred(int percentage) throws NoSuchAlgorithmException { + final MessageDigest digest = MessageDigest.getInstance("SHA-256"); + final Random random = new Random(); + final int iterations = 10000; + int enabledCount = 0; + + for (int i = 0; i < iterations; i++) { + if (RemoteConfigsManager.isInBucket(digest, AuthHelper.getRandomUUID(random), "test".getBytes(), percentage, Set.of())) { + enabledCount++; + } + } + + assertThat(enabledCount).isEqualTo(iterations * percentage / 100); + } + }