From d38031070c2e4fa5a601f9d6b14b0d401e6bb21d Mon Sep 17 00:00:00 2001 From: Aleksandrs Ulme Date: Fri, 9 May 2025 11:18:44 +0800 Subject: [PATCH 1/2] Recording refreshFrom, remove unnecessary interface --- src/main/java/com/uid2/admin/Main.java | 2 +- .../com/uid2/admin/secret/ISaltRotation.java | 34 --- .../com/uid2/admin/secret/SaltRotation.java | 34 ++- .../uid2/admin/vertx/service/SaltService.java | 8 +- .../uid2/admin/secret/SaltRotationTest.java | 196 ++++++++++-------- .../com/uid2/admin/vertx/SaltServiceTest.java | 11 +- 6 files changed, 146 insertions(+), 139 deletions(-) delete mode 100644 src/main/java/com/uid2/admin/secret/ISaltRotation.java diff --git a/src/main/java/com/uid2/admin/Main.java b/src/main/java/com/uid2/admin/Main.java index 8bf8d0f7..0282e740 100644 --- a/src/main/java/com/uid2/admin/Main.java +++ b/src/main/java/com/uid2/admin/Main.java @@ -232,7 +232,7 @@ public void run() { WriteLock writeLock = new WriteLock(); KeyHasher keyHasher = new KeyHasher(); IKeypairGenerator keypairGenerator = new SecureKeypairGenerator(); - ISaltRotation saltRotation = new SaltRotation(keyGenerator); + SaltRotation saltRotation = new SaltRotation(keyGenerator); EncryptionKeyService encryptionKeyService = new EncryptionKeyService( config, auth, writeLock, encryptionKeyStoreWriter, keysetKeyStoreWriter, keyProvider, keysetKeysProvider, adminKeysetProvider, adminKeysetStoreWriter, keyGenerator, clock); KeysetManager keysetManager = new KeysetManager( diff --git a/src/main/java/com/uid2/admin/secret/ISaltRotation.java b/src/main/java/com/uid2/admin/secret/ISaltRotation.java deleted file mode 100644 index f14552f7..00000000 --- a/src/main/java/com/uid2/admin/secret/ISaltRotation.java +++ /dev/null @@ -1,34 +0,0 @@ -package com.uid2.admin.secret; - -import com.uid2.shared.store.salt.RotatingSaltProvider; - -import java.time.Duration; -import java.time.LocalDate; - -public interface ISaltRotation { - Result rotateSalts(RotatingSaltProvider.SaltSnapshot lastSnapshot, - Duration[] minAges, - double fraction, - LocalDate nextEffective) throws Exception; - - class Result { - private RotatingSaltProvider.SaltSnapshot snapshot; // can be null if new snapshot is not needed - private String reason; // why you are not getting a new snapshot - - private Result(RotatingSaltProvider.SaltSnapshot snapshot, String reason) { - this.snapshot = snapshot; - this.reason = reason; - } - - public boolean hasSnapshot() { return snapshot != null; } - public RotatingSaltProvider.SaltSnapshot getSnapshot() { return snapshot; } - public String getReason() { return reason; } - - public static Result fromSnapshot(RotatingSaltProvider.SaltSnapshot snapshot) { - return new Result(snapshot, null); - } - public static Result noSnapshot(String reason) { - return new Result(null, reason); - } - } -} diff --git a/src/main/java/com/uid2/admin/secret/SaltRotation.java b/src/main/java/com/uid2/admin/secret/SaltRotation.java index 35c3b96e..f9d73a6c 100644 --- a/src/main/java/com/uid2/admin/secret/SaltRotation.java +++ b/src/main/java/com/uid2/admin/secret/SaltRotation.java @@ -16,19 +16,17 @@ import static java.util.stream.Collectors.toList; -public class SaltRotation implements ISaltRotation { +public class SaltRotation { private final IKeyGenerator keyGenerator; public SaltRotation(IKeyGenerator keyGenerator) { this.keyGenerator = keyGenerator; } - @Override public Result rotateSalts(RotatingSaltProvider.SaltSnapshot lastSnapshot, Duration[] minAges, double fraction, LocalDate targetDate) throws Exception { - final Instant nextEffective = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC); final Instant nextExpires = nextEffective.plus(7, ChronoUnit.DAYS); if (nextEffective.equals(lastSnapshot.getEffective()) || nextEffective.isBefore(lastSnapshot.getEffective())) { @@ -63,19 +61,27 @@ private SaltEntry[] updateSalts(SaltEntry[] oldSalts, List saltIndexesT private SaltEntry updateSalt(SaltEntry oldSalt, boolean shouldRotate, long nextEffective) throws Exception { var currentSalt = shouldRotate ? this.keyGenerator.generateRandomKeyString(32) : oldSalt.currentSalt(); var lastUpdated = shouldRotate ? nextEffective : oldSalt.lastUpdated(); + var refreshFrom = calculateRefreshFrom(oldSalt.lastUpdated(), nextEffective); return new SaltEntry( oldSalt.id(), oldSalt.hashedId(), lastUpdated, currentSalt, - null, + refreshFrom, null, null, null ); } + private long calculateRefreshFrom(long lastUpdated, long nextEffective) { + long thirtyDaysMillis = 2592000000L; + long age = nextEffective - lastUpdated; + long multiplier = age / thirtyDaysMillis + 1; + return lastUpdated + (multiplier * thirtyDaysMillis); + } + private List pickSaltIndexesToRotate( SaltSnapshot lastSnapshot, Instant nextEffective, @@ -125,4 +131,24 @@ private static boolean isBetween(long t, long minInclusive, long maxExclusive) { return minInclusive <= t && t < maxExclusive; } + public static class Result { + private final RotatingSaltProvider.SaltSnapshot snapshot; // can be null if new snapshot is not needed + private final String reason; // why you are not getting a new snapshot + + private Result(RotatingSaltProvider.SaltSnapshot snapshot, String reason) { + this.snapshot = snapshot; + this.reason = reason; + } + + public boolean hasSnapshot() { return snapshot != null; } + public RotatingSaltProvider.SaltSnapshot getSnapshot() { return snapshot; } + public String getReason() { return reason; } + + public static Result fromSnapshot(RotatingSaltProvider.SaltSnapshot snapshot) { + return new Result(snapshot, null); + } + public static Result noSnapshot(String reason) { + return new Result(null, reason); + } + } } diff --git a/src/main/java/com/uid2/admin/vertx/service/SaltService.java b/src/main/java/com/uid2/admin/vertx/service/SaltService.java index 086bbc26..2f1dc14f 100644 --- a/src/main/java/com/uid2/admin/vertx/service/SaltService.java +++ b/src/main/java/com/uid2/admin/vertx/service/SaltService.java @@ -1,7 +1,7 @@ package com.uid2.admin.vertx.service; import com.uid2.admin.auth.AdminAuthMiddleware; -import com.uid2.admin.secret.ISaltRotation; +import com.uid2.admin.secret.SaltRotation; import com.uid2.admin.store.writer.SaltStoreWriter; import com.uid2.admin.vertx.RequestUtil; import com.uid2.admin.vertx.ResponseUtil; @@ -30,13 +30,13 @@ public class SaltService implements IService { private final WriteLock writeLock; private final SaltStoreWriter storageManager; private final RotatingSaltProvider saltProvider; - private final ISaltRotation saltRotation; + private final SaltRotation saltRotation; public SaltService(AdminAuthMiddleware auth, WriteLock writeLock, SaltStoreWriter storageManager, RotatingSaltProvider saltProvider, - ISaltRotation saltRotation) { + SaltRotation saltRotation) { this.auth = auth; this.writeLock = writeLock; this.storageManager = storageManager; @@ -89,7 +89,7 @@ private void handleSaltRotate(RoutingContext rc) { final List snapshots = this.saltProvider.getSnapshots(); final RotatingSaltProvider.SaltSnapshot lastSnapshot = snapshots.get(snapshots.size() - 1); - final ISaltRotation.Result result = saltRotation.rotateSalts( + final SaltRotation.Result result = saltRotation.rotateSalts( lastSnapshot, minAges, fraction.get(), targetDate); if (!result.hasSnapshot()) { ResponseUtil.error(rc, 200, result.getReason()); diff --git a/src/test/java/com/uid2/admin/secret/SaltRotationTest.java b/src/test/java/com/uid2/admin/secret/SaltRotationTest.java index 2bcf4977..fecdb40d 100644 --- a/src/test/java/com/uid2/admin/secret/SaltRotationTest.java +++ b/src/test/java/com/uid2/admin/secret/SaltRotationTest.java @@ -5,15 +5,16 @@ import com.uid2.shared.store.salt.RotatingSaltProvider; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.time.*; -import java.time.temporal.ChronoUnit; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; +import java.util.*; +import static java.time.temporal.ChronoUnit.*; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -22,8 +23,18 @@ public class SaltRotationTest { @Mock private IKeyGenerator keyGenerator; private SaltRotation saltRotation; - private final LocalDate targetDate = LocalDate.of(2025, 1, 1); - private final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC); + private static final LocalDate targetDate = LocalDate.of(2025, 1, 1); + private static final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC); + + private static final Instant oneDayEarlier = targetDateAsInstant.minus(1, DAYS); + private static final Instant twoDaysEarlier = targetDateAsInstant.minus(2, DAYS); + private static final Instant threeDaysEarlier = targetDateAsInstant.minus(3, DAYS); + private static final Instant fourDaysEarlier = targetDateAsInstant.minus(4, DAYS); + private static final Instant fiveDaysEarlier = targetDateAsInstant.minus(5, DAYS); + private static final Instant sixDaysEarlier = targetDateAsInstant.minus(6, DAYS); + private static final Instant tenDaysEarlier = targetDateAsInstant.minus(10, DAYS); + private static final Instant sixDaysLater = targetDateAsInstant.plus(6, DAYS); + private static final Instant sevenDaysLater = targetDateAsInstant.plus(7, DAYS); @BeforeEach void setup() { @@ -32,8 +43,7 @@ void setup() { saltRotation = new SaltRotation(keyGenerator); } - private static class SnapshotBuilder - { + private static class SnapshotBuilder { private final List entries = new ArrayList<>(); private SnapshotBuilder() {} @@ -47,6 +57,11 @@ public SnapshotBuilder withEntries(int count, Instant lastUpdated) { return this; } + public SnapshotBuilder withEntries(SaltEntry... salts) { + Collections.addAll(this.entries, salts); + return this; + } + public RotatingSaltProvider.SaltSnapshot build(Instant effective, Instant expires) { return new RotatingSaltProvider.SaltSnapshot( effective, expires, entries.toArray(SaltEntry[]::new), "test_first_level_salt"); @@ -57,25 +72,19 @@ private int countEntriesWithLastUpdated(SaltEntry[] entries, Instant lastUpdated return (int)Arrays.stream(entries).filter(e -> e.lastUpdated() == lastUpdated.toEpochMilli()).count(); } - private static void assertEqualsClose(Instant expected, Instant actual, int withinSeconds) { - assertTrue(expected.minusSeconds(withinSeconds).isBefore(actual)); - assertTrue(expected.plusSeconds(withinSeconds).isAfter(actual)); - } - @Test void rotateSaltsLastSnapshotIsUpToDate() throws Exception { final Duration[] minAges = { Duration.ofDays(1), Duration.ofDays(2), }; - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() + var lastSnapshot = SnapshotBuilder.start() .withEntries(10, targetDateAsInstant) - .build(targetDateAsInstant, - targetDateAsInstant.plus(7, ChronoUnit.DAYS)); + .build(targetDateAsInstant, sevenDaysLater); - final ISaltRotation.Result result1 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); + var result1 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertFalse(result1.hasSnapshot()); - final ISaltRotation.Result result2 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate.minusDays(1)); + var result2 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate.minusDays(1)); assertFalse(result2.hasSnapshot()); } @@ -86,12 +95,11 @@ void rotateSaltsAllSaltsUpToDate() throws Exception { Duration.ofDays(2), }; - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() + var lastSnapshot = SnapshotBuilder.start() .withEntries(10, targetDateAsInstant) - .build(targetDateAsInstant.minus(1, ChronoUnit.DAYS), - targetDateAsInstant.plus(6, ChronoUnit.DAYS)); + .build(oneDayEarlier, sixDaysLater); - final ISaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); + var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertFalse(result.hasSnapshot()); verify(keyGenerator, times(0)).generateRandomKeyString(anyInt()); } @@ -103,18 +111,16 @@ void rotateSaltsAllSaltsOld() throws Exception { Duration.ofDays(2), }; - final Instant entryLastUpdated = targetDateAsInstant.minus(10, ChronoUnit.DAYS); - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() - .withEntries(10, entryLastUpdated) - .build(targetDateAsInstant.minus(1, ChronoUnit.DAYS), - targetDateAsInstant.plus(6, ChronoUnit.DAYS)); + var lastSnapshot = SnapshotBuilder.start() + .withEntries(10, tenDaysEarlier) + .build(oneDayEarlier, sixDaysLater); - final ISaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); + var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertTrue(result.hasSnapshot()); assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective())); - assertEquals(8, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), entryLastUpdated)); + assertEquals(8, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), tenDaysEarlier)); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(targetDateAsInstant.plus(7, ChronoUnit.DAYS), result.getSnapshot().getExpires()); + assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); verify(keyGenerator, times(2)).generateRandomKeyString(anyInt()); } @@ -125,24 +131,22 @@ void rotateSaltsRotateSaltsFromOldestBucketOnly() throws Exception { Duration.ofDays(4), }; - final Instant lastUpdated1 = targetDateAsInstant.minus(6, ChronoUnit.DAYS); - final Instant lastUpdated2 = targetDateAsInstant.minus(5, ChronoUnit.DAYS); - final Instant lastUpdated3 = targetDateAsInstant.minus(4, ChronoUnit.DAYS); - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() - .withEntries(3, lastUpdated1) - .withEntries(5, lastUpdated2) - .withEntries(2, lastUpdated3) - .build(targetDateAsInstant.minus(1, ChronoUnit.DAYS), - targetDateAsInstant.plus(6, ChronoUnit.DAYS)); - - final ISaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); + var lastSnapshot = SnapshotBuilder.start() + .withEntries(3, sixDaysEarlier) + .withEntries(5, fiveDaysEarlier) + .withEntries(2, fourDaysEarlier) + .build(targetDateAsInstant.minus(1, DAYS), + targetDateAsInstant.plus(6, DAYS)); + + var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertTrue(result.hasSnapshot()); - assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective())); - assertEquals(1, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated1)); - assertEquals(5, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated2)); - assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated3)); + var salts = result.getSnapshot().getAllRotatingSalts(); + assertEquals(2, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); + assertEquals(1, countEntriesWithLastUpdated(salts, sixDaysEarlier)); + assertEquals(5, countEntriesWithLastUpdated(salts, fiveDaysEarlier)); + assertEquals(2, countEntriesWithLastUpdated(salts, fourDaysEarlier)); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(targetDateAsInstant.plus(7, ChronoUnit.DAYS), result.getSnapshot().getExpires()); + assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); verify(keyGenerator, times(2)).generateRandomKeyString(anyInt()); } @@ -153,21 +157,19 @@ void rotateSaltsRotateSaltsFromNewerBucketOnly() throws Exception { Duration.ofDays(3), }; - final Instant lastUpdated1 = targetDateAsInstant.minus(4, ChronoUnit.DAYS); - final Instant lastUpdated2 = targetDateAsInstant.minus(3, ChronoUnit.DAYS); - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() - .withEntries(3, lastUpdated1) - .withEntries(7, lastUpdated2) - .build(targetDateAsInstant.minus(1, ChronoUnit.DAYS), - targetDateAsInstant.plus(6, ChronoUnit.DAYS)); + var lastSnapshot = SnapshotBuilder.start() + .withEntries(3, fourDaysEarlier) + .withEntries(7, threeDaysEarlier) + .build(oneDayEarlier, sixDaysLater); - final ISaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); + var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertTrue(result.hasSnapshot()); - assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective())); - assertEquals(1, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated1)); - assertEquals(7, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated2)); + var salts = result.getSnapshot().getAllRotatingSalts(); + assertEquals(2, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); + assertEquals(1, countEntriesWithLastUpdated(salts, fourDaysEarlier)); + assertEquals(7, countEntriesWithLastUpdated(salts, threeDaysEarlier)); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(targetDateAsInstant.plus(7, ChronoUnit.DAYS), result.getSnapshot().getExpires()); + assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); verify(keyGenerator, times(2)).generateRandomKeyString(anyInt()); } @@ -178,24 +180,21 @@ void rotateSaltsRotateSaltsFromMultipleBuckets() throws Exception { Duration.ofDays(4), }; - final Instant lastUpdated1 = targetDateAsInstant.minus(6, ChronoUnit.DAYS); - final Instant lastUpdated2 = targetDateAsInstant.minus(5, ChronoUnit.DAYS); - final Instant lastUpdated3 = targetDateAsInstant.minus(4, ChronoUnit.DAYS); - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() - .withEntries(3, lastUpdated1) - .withEntries(5, lastUpdated2) - .withEntries(2, lastUpdated3) - .build(targetDateAsInstant.minus(1, ChronoUnit.DAYS), - targetDateAsInstant.plus(6, ChronoUnit.DAYS)); - - final ISaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate); + var lastSnapshot = SnapshotBuilder.start() + .withEntries(3, sixDaysEarlier) + .withEntries(5, fiveDaysEarlier) + .withEntries(2, fourDaysEarlier) + .build(oneDayEarlier, sixDaysLater); + + var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate); assertTrue(result.hasSnapshot()); - assertEquals(5, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective())); - assertEquals(0, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated1)); - assertEquals(3, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated2)); - assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated3)); + var salts = result.getSnapshot().getAllRotatingSalts(); + assertEquals(5, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); + assertEquals(0, countEntriesWithLastUpdated(salts, sixDaysEarlier)); + assertEquals(3, countEntriesWithLastUpdated(salts, fiveDaysEarlier)); + assertEquals(2, countEntriesWithLastUpdated(salts, fourDaysEarlier)); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(targetDateAsInstant.plus(7, ChronoUnit.DAYS), result.getSnapshot().getExpires()); + assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); verify(keyGenerator, times(5)).generateRandomKeyString(anyInt()); } @@ -206,24 +205,41 @@ void rotateSaltsRotateSaltsInsufficientOutdatedSalts() throws Exception { Duration.ofDays(3), }; - final Instant lastUpdated1 = targetDateAsInstant.minus(5, ChronoUnit.DAYS); - final Instant lastUpdated2 = targetDateAsInstant.minus(4, ChronoUnit.DAYS); - final Instant lastUpdated3 = targetDateAsInstant.minus(2, ChronoUnit.DAYS); - final RotatingSaltProvider.SaltSnapshot lastSnapshot = SnapshotBuilder.start() - .withEntries(1, lastUpdated1) - .withEntries(2, lastUpdated2) - .withEntries(7, lastUpdated3) - .build(targetDateAsInstant.minus(1, ChronoUnit.DAYS), - targetDateAsInstant.plus(6, ChronoUnit.DAYS)); - - final ISaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate); + var lastSnapshot = SnapshotBuilder.start() + .withEntries(1, fiveDaysEarlier) + .withEntries(2, fourDaysEarlier) + .withEntries(7, twoDaysEarlier) + .build(oneDayEarlier, sixDaysLater); + + var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate); assertTrue(result.hasSnapshot()); - assertEquals(3, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective())); - assertEquals(0, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated1)); - assertEquals(0, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated2)); - assertEquals(7, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), lastUpdated3)); + var salts = result.getSnapshot().getAllRotatingSalts(); + assertEquals(3, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); + assertEquals(0, countEntriesWithLastUpdated(salts, fiveDaysEarlier)); + assertEquals(0, countEntriesWithLastUpdated(salts, fourDaysEarlier)); + assertEquals(7, countEntriesWithLastUpdated(salts, twoDaysEarlier)); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(targetDateAsInstant.plus(7, ChronoUnit.DAYS), result.getSnapshot().getExpires()); + assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); verify(keyGenerator, times(3)).generateRandomKeyString(anyInt()); } + + @ParameterizedTest + @CsvSource({ + "5, 30", // Soon after rotation, use 30 days post rotation + "40, 60", // >30 days after rotation use the next increment of 30 days + "60, 90", // Exactly at multiple of 30 days post rotation, use next increment of 30 days + }) + void testRefreshFromCalculation(int lastRotationDaysAgo, int refreshFromDaysFromRotation) throws Exception { + var lastRotation = targetDateAsInstant.minus(lastRotationDaysAgo, DAYS); + var lastSnapshot = SnapshotBuilder.start() + .withEntries(new SaltEntry(1, "1", lastRotation.toEpochMilli(), "salt1", 100L, null, null, null)) + .build(oneDayEarlier, sixDaysLater); + + var result = saltRotation.rotateSalts(lastSnapshot, new Duration[]{ Duration.ofDays(1) }, 0.45, targetDate); + var actual = result.getSnapshot().getAllRotatingSalts()[0]; + + var expected = lastRotation.plus(refreshFromDaysFromRotation, DAYS).toEpochMilli(); + + assertThat(actual.refreshFrom()).isEqualTo(expected); + } } diff --git a/src/test/java/com/uid2/admin/vertx/SaltServiceTest.java b/src/test/java/com/uid2/admin/vertx/SaltServiceTest.java index 087c0dc8..c3952fca 100644 --- a/src/test/java/com/uid2/admin/vertx/SaltServiceTest.java +++ b/src/test/java/com/uid2/admin/vertx/SaltServiceTest.java @@ -1,6 +1,6 @@ package com.uid2.admin.vertx; -import com.uid2.admin.secret.ISaltRotation; +import com.uid2.admin.secret.SaltRotation; import com.uid2.admin.vertx.service.IService; import com.uid2.admin.vertx.service.SaltService; import com.uid2.admin.vertx.test.ServiceTestBase; @@ -15,7 +15,6 @@ import java.time.Instant; import java.time.LocalDate; -import java.time.LocalDateTime; import java.time.ZoneOffset; import java.time.temporal.ChronoUnit; import java.util.Arrays; @@ -26,7 +25,7 @@ public class SaltServiceTest extends ServiceTestBase { @Mock RotatingSaltProvider saltProvider; - @Mock ISaltRotation saltRotation; + @Mock SaltRotation saltRotation; @Override protected IService createService() { @@ -99,7 +98,7 @@ void rotateSalts(Vertx vertx, VertxTestContext testContext) throws Exception { final RotatingSaltProvider.SaltSnapshot[] addedSnapshots = { makeSnapshot(Instant.ofEpochMilli(10004), Instant.ofEpochMilli(20004), 10), }; - when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(LocalDate.now().plusDays(1)))).thenReturn(ISaltRotation.Result.fromSnapshot(addedSnapshots[0])); + when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(LocalDate.now().plusDays(1)))).thenReturn(SaltRotation.Result.fromSnapshot(addedSnapshots[0])); post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> { assertEquals(200, response.statusCode()); @@ -121,7 +120,7 @@ void rotateSaltsNoNewSnapshot(Vertx vertx, VertxTestContext testContext) throws }; setSnapshots(snapshots); - when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(LocalDate.now().plusDays(1)))).thenReturn(ISaltRotation.Result.noSnapshot("test")); + when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(LocalDate.now().plusDays(1)))).thenReturn(SaltRotation.Result.noSnapshot("test")); post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> { assertEquals(200, response.statusCode()); @@ -149,7 +148,7 @@ void rotateSaltsWitnSpecificTargetDate(Vertx vertx, VertxTestContext testContext makeSnapshot(targetDateAsInstant, targetDateAsInstant.plus(1, ChronoUnit.DAYS), 10), }; - when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(targetDate))).thenReturn(ISaltRotation.Result.fromSnapshot(addedSnapshots[0])); + when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(targetDate))).thenReturn(SaltRotation.Result.fromSnapshot(addedSnapshots[0])); post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2&target_date=2025-05-08", "", response -> { assertEquals(200, response.statusCode()); From 59e31883d7b9003ed89718df718b7cd68578c31c Mon Sep 17 00:00:00 2001 From: Aleksandrs Ulme Date: Fri, 9 May 2025 12:27:18 +0800 Subject: [PATCH 2/2] Address feedback --- .../com/uid2/admin/secret/SaltRotation.java | 6 +- .../uid2/admin/secret/SaltRotationTest.java | 99 +++++++++---------- 2 files changed, 51 insertions(+), 54 deletions(-) diff --git a/src/main/java/com/uid2/admin/secret/SaltRotation.java b/src/main/java/com/uid2/admin/secret/SaltRotation.java index f9d73a6c..69685c62 100644 --- a/src/main/java/com/uid2/admin/secret/SaltRotation.java +++ b/src/main/java/com/uid2/admin/secret/SaltRotation.java @@ -18,6 +18,7 @@ public class SaltRotation { private final IKeyGenerator keyGenerator; + private final long THIRTY_DAYS_IN_MS = Duration.ofDays(30).toMillis(); public SaltRotation(IKeyGenerator keyGenerator) { this.keyGenerator = keyGenerator; @@ -76,10 +77,9 @@ private SaltEntry updateSalt(SaltEntry oldSalt, boolean shouldRotate, long nextE } private long calculateRefreshFrom(long lastUpdated, long nextEffective) { - long thirtyDaysMillis = 2592000000L; long age = nextEffective - lastUpdated; - long multiplier = age / thirtyDaysMillis + 1; - return lastUpdated + (multiplier * thirtyDaysMillis); + long multiplier = age / THIRTY_DAYS_IN_MS + 1; + return lastUpdated + (multiplier * THIRTY_DAYS_IN_MS); } private List pickSaltIndexesToRotate( diff --git a/src/test/java/com/uid2/admin/secret/SaltRotationTest.java b/src/test/java/com/uid2/admin/secret/SaltRotationTest.java index fecdb40d..589ede8b 100644 --- a/src/test/java/com/uid2/admin/secret/SaltRotationTest.java +++ b/src/test/java/com/uid2/admin/secret/SaltRotationTest.java @@ -23,18 +23,16 @@ public class SaltRotationTest { @Mock private IKeyGenerator keyGenerator; private SaltRotation saltRotation; - private static final LocalDate targetDate = LocalDate.of(2025, 1, 1); - private static final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC); - - private static final Instant oneDayEarlier = targetDateAsInstant.minus(1, DAYS); - private static final Instant twoDaysEarlier = targetDateAsInstant.minus(2, DAYS); - private static final Instant threeDaysEarlier = targetDateAsInstant.minus(3, DAYS); - private static final Instant fourDaysEarlier = targetDateAsInstant.minus(4, DAYS); - private static final Instant fiveDaysEarlier = targetDateAsInstant.minus(5, DAYS); - private static final Instant sixDaysEarlier = targetDateAsInstant.minus(6, DAYS); - private static final Instant tenDaysEarlier = targetDateAsInstant.minus(10, DAYS); - private static final Instant sixDaysLater = targetDateAsInstant.plus(6, DAYS); - private static final Instant sevenDaysLater = targetDateAsInstant.plus(7, DAYS); + private final LocalDate targetDate = LocalDate.of(2025, 1, 1); + private final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC); + + private Instant daysEarlier(int days) { + return targetDateAsInstant.minus(days, DAYS); + } + + private Instant daysLater(int days) { + return targetDateAsInstant.plus(days, DAYS); + } @BeforeEach void setup() { @@ -80,7 +78,7 @@ void rotateSaltsLastSnapshotIsUpToDate() throws Exception { }; var lastSnapshot = SnapshotBuilder.start() .withEntries(10, targetDateAsInstant) - .build(targetDateAsInstant, sevenDaysLater); + .build(targetDateAsInstant, daysLater(7)); var result1 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertFalse(result1.hasSnapshot()); @@ -97,7 +95,7 @@ void rotateSaltsAllSaltsUpToDate() throws Exception { var lastSnapshot = SnapshotBuilder.start() .withEntries(10, targetDateAsInstant) - .build(oneDayEarlier, sixDaysLater); + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertFalse(result.hasSnapshot()); @@ -112,15 +110,15 @@ void rotateSaltsAllSaltsOld() throws Exception { }; var lastSnapshot = SnapshotBuilder.start() - .withEntries(10, tenDaysEarlier) - .build(oneDayEarlier, sixDaysLater); + .withEntries(10, daysEarlier(10)) + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertTrue(result.hasSnapshot()); assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective())); - assertEquals(8, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), tenDaysEarlier)); + assertEquals(8, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), daysEarlier(10))); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); + assertEquals(daysLater(7), result.getSnapshot().getExpires()); verify(keyGenerator, times(2)).generateRandomKeyString(anyInt()); } @@ -132,21 +130,20 @@ void rotateSaltsRotateSaltsFromOldestBucketOnly() throws Exception { }; var lastSnapshot = SnapshotBuilder.start() - .withEntries(3, sixDaysEarlier) - .withEntries(5, fiveDaysEarlier) - .withEntries(2, fourDaysEarlier) - .build(targetDateAsInstant.minus(1, DAYS), - targetDateAsInstant.plus(6, DAYS)); + .withEntries(3, daysEarlier(6)) + .withEntries(5, daysEarlier(5)) + .withEntries(2, daysEarlier(4)) + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertTrue(result.hasSnapshot()); var salts = result.getSnapshot().getAllRotatingSalts(); assertEquals(2, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); - assertEquals(1, countEntriesWithLastUpdated(salts, sixDaysEarlier)); - assertEquals(5, countEntriesWithLastUpdated(salts, fiveDaysEarlier)); - assertEquals(2, countEntriesWithLastUpdated(salts, fourDaysEarlier)); + assertEquals(1, countEntriesWithLastUpdated(salts, daysEarlier(6))); + assertEquals(5, countEntriesWithLastUpdated(salts, daysEarlier(5))); + assertEquals(2, countEntriesWithLastUpdated(salts, daysEarlier(4))); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); + assertEquals(daysLater(7), result.getSnapshot().getExpires()); verify(keyGenerator, times(2)).generateRandomKeyString(anyInt()); } @@ -158,18 +155,18 @@ void rotateSaltsRotateSaltsFromNewerBucketOnly() throws Exception { }; var lastSnapshot = SnapshotBuilder.start() - .withEntries(3, fourDaysEarlier) - .withEntries(7, threeDaysEarlier) - .build(oneDayEarlier, sixDaysLater); + .withEntries(3, daysEarlier(4)) + .withEntries(7, daysEarlier(3)) + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); assertTrue(result.hasSnapshot()); var salts = result.getSnapshot().getAllRotatingSalts(); assertEquals(2, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); - assertEquals(1, countEntriesWithLastUpdated(salts, fourDaysEarlier)); - assertEquals(7, countEntriesWithLastUpdated(salts, threeDaysEarlier)); + assertEquals(1, countEntriesWithLastUpdated(salts, daysEarlier(4))); + assertEquals(7, countEntriesWithLastUpdated(salts, daysEarlier(3))); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); + assertEquals(daysLater(7), result.getSnapshot().getExpires()); verify(keyGenerator, times(2)).generateRandomKeyString(anyInt()); } @@ -181,20 +178,20 @@ void rotateSaltsRotateSaltsFromMultipleBuckets() throws Exception { }; var lastSnapshot = SnapshotBuilder.start() - .withEntries(3, sixDaysEarlier) - .withEntries(5, fiveDaysEarlier) - .withEntries(2, fourDaysEarlier) - .build(oneDayEarlier, sixDaysLater); + .withEntries(3, daysEarlier(6)) + .withEntries(5, daysEarlier(5)) + .withEntries(2, daysEarlier(4)) + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate); assertTrue(result.hasSnapshot()); var salts = result.getSnapshot().getAllRotatingSalts(); assertEquals(5, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); - assertEquals(0, countEntriesWithLastUpdated(salts, sixDaysEarlier)); - assertEquals(3, countEntriesWithLastUpdated(salts, fiveDaysEarlier)); - assertEquals(2, countEntriesWithLastUpdated(salts, fourDaysEarlier)); + assertEquals(0, countEntriesWithLastUpdated(salts, daysEarlier(6))); + assertEquals(3, countEntriesWithLastUpdated(salts, daysEarlier(5))); + assertEquals(2, countEntriesWithLastUpdated(salts, daysEarlier(4))); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); + assertEquals(daysLater(7), result.getSnapshot().getExpires()); verify(keyGenerator, times(5)).generateRandomKeyString(anyInt()); } @@ -206,20 +203,20 @@ void rotateSaltsRotateSaltsInsufficientOutdatedSalts() throws Exception { }; var lastSnapshot = SnapshotBuilder.start() - .withEntries(1, fiveDaysEarlier) - .withEntries(2, fourDaysEarlier) - .withEntries(7, twoDaysEarlier) - .build(oneDayEarlier, sixDaysLater); + .withEntries(1, daysEarlier(5)) + .withEntries(2, daysEarlier(4)) + .withEntries(7, daysEarlier(2)) + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate); assertTrue(result.hasSnapshot()); var salts = result.getSnapshot().getAllRotatingSalts(); assertEquals(3, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective())); - assertEquals(0, countEntriesWithLastUpdated(salts, fiveDaysEarlier)); - assertEquals(0, countEntriesWithLastUpdated(salts, fourDaysEarlier)); - assertEquals(7, countEntriesWithLastUpdated(salts, twoDaysEarlier)); + assertEquals(0, countEntriesWithLastUpdated(salts, daysEarlier(5))); + assertEquals(0, countEntriesWithLastUpdated(salts, daysEarlier(4))); + assertEquals(7, countEntriesWithLastUpdated(salts, daysEarlier(2))); assertEquals(targetDateAsInstant, result.getSnapshot().getEffective()); - assertEquals(sevenDaysLater, result.getSnapshot().getExpires()); + assertEquals(daysLater(7), result.getSnapshot().getExpires()); verify(keyGenerator, times(3)).generateRandomKeyString(anyInt()); } @@ -230,10 +227,10 @@ void rotateSaltsRotateSaltsInsufficientOutdatedSalts() throws Exception { "60, 90", // Exactly at multiple of 30 days post rotation, use next increment of 30 days }) void testRefreshFromCalculation(int lastRotationDaysAgo, int refreshFromDaysFromRotation) throws Exception { - var lastRotation = targetDateAsInstant.minus(lastRotationDaysAgo, DAYS); + var lastRotation = daysEarlier(lastRotationDaysAgo); var lastSnapshot = SnapshotBuilder.start() .withEntries(new SaltEntry(1, "1", lastRotation.toEpochMilli(), "salt1", 100L, null, null, null)) - .build(oneDayEarlier, sixDaysLater); + .build(daysEarlier(1), daysLater(6)); var result = saltRotation.rotateSalts(lastSnapshot, new Duration[]{ Duration.ofDays(1) }, 0.45, targetDate); var actual = result.getSnapshot().getAllRotatingSalts()[0];