diff --git a/src/main/java/com/uid2/admin/managers/KeysetManager.java b/src/main/java/com/uid2/admin/managers/KeysetManager.java index 8aa9739c4..17b035765 100644 --- a/src/main/java/com/uid2/admin/managers/KeysetManager.java +++ b/src/main/java/com/uid2/admin/managers/KeysetManager.java @@ -133,7 +133,7 @@ public int getNextKeysetId() { return KeysetManager.getMaxKeyset(this.keysetProvider.getSnapshot().getAllKeysets()) + 1; } - public AdminKeyset createAndAddDefaultKeyset(Integer siteId) throws Exception{ + private AdminKeyset createAndAddDefaultKeyset(Integer siteId) throws Exception{ if(!enableKeysets) return null; this.keysetProvider.loadContent(); diff --git a/src/main/java/com/uid2/admin/vertx/service/EncryptionKeyService.java b/src/main/java/com/uid2/admin/vertx/service/EncryptionKeyService.java index 95bf22098..37c4840a9 100644 --- a/src/main/java/com/uid2/admin/vertx/service/EncryptionKeyService.java +++ b/src/main/java/com/uid2/admin/vertx/service/EncryptionKeyService.java @@ -554,7 +554,7 @@ else if(siteId == Const.Data.AdvertisingTokenSiteId) { return keyset.getKeysetId(); } - private int getSiteId(int keysetId) throws Exception { + private int getSiteId(int keysetId) { Map currentKeysets = keysetProvider.getSnapshot().getAllKeysets(); return currentKeysets.get(keysetId).getSiteId(); } @@ -563,14 +563,13 @@ private List addKeysetKeys(Iterable keysetIds, Duration acti throws Exception { final Instant now = clock.now(); - final List keys = this.keysetKeyProvider.getSnapshot().getAllKeysetKeys().stream() + final List keysetKeys = this.keysetKeyProvider.getSnapshot().getAllKeysetKeys(); + final List keys = keysetKeys.stream() .sorted(Comparator.comparingInt(KeysetKey::getId)) .filter(k -> isWithinCutOffTime(k, now, isDuringRotation)) .collect(Collectors.toList()); - - int maxKeyId = MaxKeyUtil.getMaxKeysetKeyId(this.keysetKeyProvider.getSnapshot().getAllKeysetKeys(), - this.keysetKeyProvider.getMetadata().getInteger("max_key_id")); + int maxKeyId = MaxKeyUtil.getMaxKeysetKeyId(keysetKeys, this.keysetKeyProvider.getMetadata().getInteger("max_key_id")); final List addedKeys = new ArrayList<>(); @@ -578,7 +577,10 @@ private List addKeysetKeys(Iterable keysetIds, Duration acti ++maxKeyId; final byte[] secret = keyGenerator.generateRandomKey(32); final Instant created = now; - final Instant activates = created.plusSeconds(activatesIn.getSeconds()); + + final boolean isAddingFirstKeyForKeyset = (!isDuringRotation && keysetKeys.stream().noneMatch(key -> key.getKeysetId() == keysetId)); + + final Instant activates = isAddingFirstKeyForKeyset ? created : created.plusSeconds(activatesIn.getSeconds()); final Instant expires = activates.plusSeconds(expiresAfter.getSeconds()); final KeysetKey key = new KeysetKey(maxKeyId, secret, created, activates, expires, keysetId); keys.add(key); @@ -655,8 +657,7 @@ private boolean isWithinCutOffTime(KeysetKey key, Instant now, boolean duringRot if (!(filterKeyOverCutOffTime && duringRotation)) { return true; } - Duration cutoffTime = siteKeyRotationCutOffTime; - return now.compareTo(key.getExpires().plus(cutoffTime.toDays(), ChronoUnit.DAYS)) < 0; + return now.compareTo(key.getExpires().plus(siteKeyRotationCutOffTime.toDays(), ChronoUnit.DAYS)) < 0; } private void loadAllContent() throws Exception { diff --git a/src/test/java/com/uid2/admin/vertx/EncryptionKeyServiceTest.java b/src/test/java/com/uid2/admin/vertx/EncryptionKeyServiceTest.java index 1b062b32c..6a839cdb0 100644 --- a/src/test/java/com/uid2/admin/vertx/EncryptionKeyServiceTest.java +++ b/src/test/java/com/uid2/admin/vertx/EncryptionKeyServiceTest.java @@ -40,7 +40,7 @@ public class EncryptionKeyServiceTest extends ServiceTestBase { private static final long A_HUNDRED_DAYS_IN_SECONDS = 8640000L; private static final int MAX_KEY_ID = 777; private static final boolean FILTER_KEY_OVER_CUT_OFF_DAYS = true; - private Clock clock = mock(Clock.class); + private final Clock clock = mock(Clock.class); private EncryptionKeyService keyService = null; @BeforeEach @@ -80,11 +80,6 @@ private void assertSiteKeyActivation(EncryptionKey key, Instant generatedTime) { key.getCreated(), key.getActivates(), key.getExpires()); } - private void assertSiteKeyActivation(KeysetKey key, Instant generatedTime) { - assertKeyActivation(generatedTime, SITE_KEY_ACTIVATES_IN_SECONDS, SITE_KEY_EXPIRES_AFTER_SECONDS, - key.getCreated(), key.getActivates(), key.getExpires()); - } - private void checkEncryptionKeyResponse(EncryptionKey[] expectedKeys, Object[] actualKeys) { assertEquals(expectedKeys.length, actualKeys.length); for (int i = 0; i < expectedKeys.length; ++i) { @@ -147,7 +142,7 @@ void addSiteKey() throws Exception { @Test void addSiteKeyAddsKeysetAndKey() throws Exception { - Map keysets = new HashMap() {{ + Map keysets = new HashMap<>() {{ put(1, new AdminKeyset(1, 2, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); }}; setAdminKeysets(keysets); @@ -157,13 +152,13 @@ void addSiteKeyAddsKeysetAndKey() throws Exception { AdminKeyset expected = new AdminKeyset(4, 5, "", null, Instant.now().getEpochSecond(), true, true, new HashSet<>()); assertNotNull(keysets.get(4)); - assertTrue(keysets.get(4).equals(expected)); + assertEquals(expected, keysets.get(4)); verify(keysetKeyStoreWriter).upload(collectionOfSize(1), eq(124)); } @Test void addSiteKeyUsesKeysetAndAddsKey() throws Exception { - Map keysets = new HashMap() {{ + Map keysets = new HashMap<>() {{ put(1, new AdminKeyset(1, 5, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); }}; setAdminKeysets(keysets); @@ -177,21 +172,41 @@ void addSiteKeyUsesKeysetAndAddsKey() throws Exception { } @Test - void addKeysetKey() throws Exception { + void whenAddFirstKeyForKeysetItIsImmediatelyActiveAndUploaded() throws Exception { setKeysetKeys(123); - Map keysets = new HashMap() {{ + Map keysets = new HashMap<>() {{ put(1, new AdminKeyset(1, 5, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); }}; setAdminKeysets(keysets); final KeysetKey key = keyService.addKeysetKey(1); verify(keysetKeyStoreWriter).upload(collectionOfSize(1), eq(124)); - assertSiteKeyActivation(key, clock.now()); + + final int siteKeyActivatesInSeconds = 0; //since this is the first key for a keyset, test that it is immediately active + assertKeyActivation(clock.now(), siteKeyActivatesInSeconds, SITE_KEY_EXPIRES_AFTER_SECONDS, key.getCreated(), key.getActivates(), key.getExpires()); } + @Test + void whenAddSecondKeyForKeysetItIsNotImmediatelyActive() throws Exception { + final int keysetId = 567; + Map keysets = new HashMap<>() {{ + put(keysetId, new AdminKeyset(keysetId, 5, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); + }}; + setAdminKeysets(keysets); + + final KeysetKey[] keys = { + new KeysetKey(11, null, Instant.ofEpochMilli(KEY_CREATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_ACTIVATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_EXPIRE_TIME_IN_MILLI), keysetId), + }; + setKeysetKeys(MAX_KEY_ID, keys); + + final KeysetKey key = keyService.addKeysetKey(keysetId); + assertKeyActivation(clock.now(), SITE_KEY_ACTIVATES_IN_SECONDS, SITE_KEY_EXPIRES_AFTER_SECONDS, key.getCreated(), key.getActivates(), key.getExpires()); + } + + @Test void addKeysetKeyAddsSiteKey() throws Exception { setKeysetKeys(123); - Map keysets = new HashMap() {{ + Map keysets = new HashMap<>() {{ put(1, new AdminKeyset(1, 5, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); }}; setAdminKeysets(keysets); @@ -406,7 +421,7 @@ void rotateKeysetKey(Vertx vertx, VertxTestContext testContext) throws Exception new KeysetKey(12, null, Instant.ofEpochMilli(KEY_CREATE_TIME_IN_MILLI+1), Instant.ofEpochMilli(KEY_ACTIVATE_TIME_IN_MILLI+1), Instant.ofEpochMilli(KEY_EXPIRE_TIME_IN_MILLI+1), 5) }; setKeysetKeys(MAX_KEY_ID, keys); - Map keysets = new HashMap() {{ + Map keysets = new HashMap<>() {{ put(4, new AdminKeyset(4, 2, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); put(5, new AdminKeyset(5, 3, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); }}; @@ -673,8 +688,7 @@ void createKeysetKeysFirstRun() throws Exception { new EncryptionKey(17, null, Instant.ofEpochMilli(KEY_CREATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_ACTIVATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_EXPIRE_TIME_IN_MILLI), 2), }; setEncryptionKeys(MAX_KEY_ID, keys); - Map keysets = new HashMap() {{ - }}; + Map keysets = new HashMap<>(); setAdminKeysets(keysets); final KeysetKey[] keysetKeys = {}; setKeysetKeys(0, keysetKeys); @@ -704,8 +718,7 @@ void createKeysetKeysNoKeysNeed() throws Exception { new EncryptionKey(14, null, Instant.ofEpochMilli(KEY_CREATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_ACTIVATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_EXPIRE_TIME_IN_MILLI), 7), }; setEncryptionKeys(MAX_KEY_ID, keys); - Map keysets = new HashMap() {{ - }}; + Map keysets = new HashMap<>(); setAdminKeysets(keysets); final KeysetKey[] keysetKeys = { new KeysetKey(11, null, Instant.ofEpochMilli(KEY_CREATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_ACTIVATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_EXPIRE_TIME_IN_MILLI), 1), @@ -731,7 +744,7 @@ void createKeysetKeysMissingKey() throws Exception { new EncryptionKey(16, null, Instant.ofEpochMilli(KEY_CREATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_ACTIVATE_TIME_IN_MILLI), Instant.ofEpochMilli(KEY_EXPIRE_TIME_IN_MILLI), 8), }; setEncryptionKeys(MAX_KEY_ID, keys); - Map keysets = new HashMap() {{ + Map keysets = new HashMap<>() {{ put(1, new AdminKeyset(1, 7, "test", Set.of(4,6,7), Instant.now().getEpochSecond(),true, true, new HashSet<>())); }}; setAdminKeysets(keysets);