Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/com/uid2/admin/AdminConst.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ public class AdminConst {
public static final String ROLE_OKTA_GROUP_MAP_PRIVILEGED = "role_okta_group_map_privileged";
public static final String ROLE_OKTA_GROUP_MAP_SUPER_USER = "role_okta_group_map_super_user";
public static final String ENABLE_SALT_ROTATION_REFRESH_FROM = "enable_salt_rotation_refresh_from";
public static final String ENABLE_SALT_ROTATION_CUSTOM_AGE_THRESHOLDS = "enable_salt_rotation_custom_age_thresholds";
}
6 changes: 6 additions & 0 deletions src/main/java/com/uid2/admin/salt/SaltRotation.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,17 @@ public class SaltRotation {

private final IKeyGenerator keyGenerator;
private final boolean isRefreshFromEnabled;
private final boolean isCustomAgeThresholdEnabled;
private static final Logger LOGGER = LoggerFactory.getLogger(SaltRotation.class);

public SaltRotation(JsonObject config, IKeyGenerator keyGenerator) {
this.keyGenerator = keyGenerator;
this.isRefreshFromEnabled = config.getBoolean(AdminConst.ENABLE_SALT_ROTATION_REFRESH_FROM, false);
this.isCustomAgeThresholdEnabled = config.getBoolean(AdminConst.ENABLE_SALT_ROTATION_CUSTOM_AGE_THRESHOLDS, false);
}

public boolean isCustomAgeThresholdEnabled() {
return this.isCustomAgeThresholdEnabled;
}

public Result rotateSalts(
Expand Down
28 changes: 21 additions & 7 deletions src/main/java/com/uid2/admin/vertx/service/SaltService.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@

import java.time.*;
import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.*;

import static com.uid2.admin.vertx.Endpoints.*;

Expand All @@ -36,6 +33,7 @@ public class SaltService implements IService {
private final SaltStoreWriter storageManager;
private final RotatingSaltProvider saltProvider;
private final SaltRotation saltRotation;
private final Duration[] defaultSaltRotationAgeThresholds;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can directly just set this variable to be a static final

e.g.

private static final Duration[] SALT_ROTATION_AGE_THRESHOLDS = generateThresholds(30, 390, 30);

or

private static final Duration[] SALT_ROTATION_AGE_THRESHOLDS = new Duration[] { ... };

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do the former, remember to make generateThresholds() a static function


public SaltService(AdminAuthMiddleware auth,
WriteLock writeLock,
Expand All @@ -47,6 +45,7 @@ public SaltService(AdminAuthMiddleware auth,
this.storageManager = storageManager;
this.saltProvider = saltProvider;
this.saltRotation = saltRotation;
this.defaultSaltRotationAgeThresholds = generateThresholds(30, 390, 30);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I think having this calculated may be an overkill, we can just have the array hardcoded here or in generateThresholds. It would be both clearer, easier to change and require less tests.

}

@Override
Expand Down Expand Up @@ -117,8 +116,15 @@ private void handleSaltRotate(RoutingContext rc) {
try {
final Optional<Double> fraction = RequestUtil.getDouble(rc, "fraction");
if (fraction.isEmpty()) return;
final Duration[] minAges = RequestUtil.getDurations(rc, "min_ages_in_seconds");
if (minAges == null) return;

final Duration[] ageThresholds;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's log what thresholds we ended up using

if (saltRotation.isCustomAgeThresholdEnabled()) {
ageThresholds = RequestUtil.getDurations(rc, "min_ages_in_seconds");
if (ageThresholds == null) return;
} else {
ageThresholds = defaultSaltRotationAgeThresholds;
}


final TargetDate targetDate =
RequestUtil.getDate(rc, "target_date", DateTimeFormatter.ISO_LOCAL_DATE)
Expand All @@ -134,7 +140,7 @@ private void handleSaltRotate(RoutingContext rc) {
final List<RotatingSaltProvider.SaltSnapshot> snapshots = saltProvider.getSnapshots();
final RotatingSaltProvider.SaltSnapshot lastSnapshot = snapshots.getLast();

final SaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, minAges, fraction.get(), targetDate);
final SaltRotation.Result result = saltRotation.rotateSalts(lastSnapshot, ageThresholds, fraction.get(), targetDate);
if (!result.hasSnapshot()) {
ResponseUtil.error(rc, 200, result.getReason());
return;
Expand All @@ -151,6 +157,14 @@ private void handleSaltRotate(RoutingContext rc) {
}
}

private Duration[] generateThresholds(int minAge, int maxAge, int interval) {
List<Duration> thresholds = new ArrayList<>();
for (int i = minAge; i <= maxAge; i += interval) {
thresholds.add(Duration.ofDays(i));
}
return thresholds.toArray(new Duration[0]);
}

private JsonObject toJson(RotatingSaltProvider.SaltSnapshot snapshot) {
JsonObject jo = new JsonObject();
jo.put("effective", snapshot.getEffective().toEpochMilli());
Expand Down
70 changes: 68 additions & 2 deletions src/test/java/com/uid2/admin/salt/SaltServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import io.vertx.junit5.VertxTestContext;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;

import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;

Expand Down Expand Up @@ -111,7 +111,7 @@ void rotateSaltsNoNewSnapshot(Vertx vertx, VertxTestContext testContext) throws
}

@Test
void rotateSaltsWitnSpecificTargetDate(Vertx vertx, VertxTestContext testContext) throws Exception {
void rotateSaltsWithSpecificTargetDate(Vertx vertx, VertxTestContext testContext) throws Exception {
fakeAuth(Role.SUPER_USER);
final SaltSnapshotBuilder[] snapshots = {
SaltSnapshotBuilder.start().effective(daysEarlier(5)).expires(daysEarlier(4)).entries(10, daysEarlier(5)),
Expand All @@ -133,6 +133,72 @@ void rotateSaltsWitnSpecificTargetDate(Vertx vertx, VertxTestContext testContext
});
}

@Test
void rotateSaltsWithCustomAgeThresholdsEnabled(Vertx vertx, VertxTestContext testContext) throws Exception {
fakeAuth(Role.SUPER_USER);

when(saltRotation.isCustomAgeThresholdEnabled()).thenReturn(true);

final SaltSnapshotBuilder lastSnapshot = SaltSnapshotBuilder.start().effective(daysEarlier(1)).expires(daysLater(6)).entries(1, daysEarlier(1));
setSnapshots(lastSnapshot);

var result = SaltRotation.Result.fromSnapshot(SaltSnapshotBuilder.start().effective(targetDate()).expires(daysEarlier(7)).entries(1, targetDate()).build());

Duration[] expectedCustomAgeThresholds = new Duration[]{
Duration.ofSeconds(50),
Duration.ofSeconds(60),
Duration.ofSeconds(70)
};

when(saltRotation.rotateSalts(any(), eq(expectedCustomAgeThresholds), eq(0.2), eq(utcTomorrow))).thenReturn(result);

post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> {
assertEquals(200, response.statusCode());
testContext.completeNow();
});
}

@Test
void rotateSaltsWithDefaultAgeThresholds(Vertx vertx, VertxTestContext testContext) throws Exception {
fakeAuth(Role.SUPER_USER);

when(saltRotation.isCustomAgeThresholdEnabled()).thenReturn(false);

final SaltSnapshotBuilder lastSnapshot = SaltSnapshotBuilder.start().effective(daysEarlier(1)).expires(daysLater(6)).entries(1, daysEarlier(1));
setSnapshots(lastSnapshot);

var result = SaltRotation.Result.fromSnapshot(SaltSnapshotBuilder.start().effective(targetDate()).expires(daysEarlier(7)).entries(1, targetDate()).build());

Duration[] expectedDefaultAgeThresholds = new Duration[]{
Duration.ofDays(30), Duration.ofDays(60), Duration.ofDays(90), Duration.ofDays(120),
Duration.ofDays(150), Duration.ofDays(180), Duration.ofDays(210), Duration.ofDays(240),
Duration.ofDays(270), Duration.ofDays(300), Duration.ofDays(330), Duration.ofDays(360),
Duration.ofDays(390)
};

when(saltRotation.rotateSalts(any(), eq(expectedDefaultAgeThresholds), eq(0.2), eq(utcTomorrow))).thenReturn(result);

post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> {
assertEquals(200, response.statusCode());
testContext.completeNow();
});
}

@Test
void rotateSaltsWithCustomAgeThresholdsEnabledButMissingParameter(Vertx vertx, VertxTestContext testContext) {
fakeAuth(Role.SUPER_USER);

when(saltRotation.isCustomAgeThresholdEnabled()).thenReturn(true);

final SaltSnapshotBuilder lastSnapshot = SaltSnapshotBuilder.start().effective(daysEarlier(1)).expires(daysLater(6)).entries(1, daysEarlier(1));
setSnapshots(lastSnapshot);

post(vertx, testContext, "api/salt/rotate?fraction=0.2", "", response -> {
assertEquals(400, response.statusCode());
testContext.completeNow();
});
}

private void checkSnapshotsResponse(SaltSnapshotBuilder[] expectedSnapshots, Object[] actualSnapshots) {
assertEquals(expectedSnapshots.length, actualSnapshots.length);
for (int i = 0; i < expectedSnapshots.length; ++i) {
Expand Down