Skip to content

Commit e8e99eb

Browse files
gmsdelmundogmsdelmundo
authored
Merge pull request #581 from IABTechLab/gdm-UID2-5854-fixes
Salt rotation fixes
2 parents 3530754 + 7f1f05b commit e8e99eb

File tree

5 files changed

+38
-48
lines changed

5 files changed

+38
-48
lines changed

‎src/main/java/com/uid2/admin/auth/AdminAuthMiddleware.java‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,23 @@
11
package com.uid2.admin.auth;
22

3-
43
import com.okta.jwt.*;
54
import com.uid2.admin.AdminConst;
65
import com.uid2.shared.auth.Role;
76
import io.vertx.core.Handler;
87
import io.vertx.core.json.JsonObject;
98
import io.vertx.ext.web.RoutingContext;
10-
import org.slf4j.Logger;
11-
import org.slf4j.LoggerFactory;
129
import com.uid2.shared.audit.AuditParams;
1310
import com.uid2.shared.audit.Audit;
1411

1512
import java.util.*;
1613

1714
public class AdminAuthMiddleware {
18-
private static final Logger LOGGER = LoggerFactory.getLogger(AdminAuthMiddleware.class);
15+
private final Map<Role, List<OktaGroup>> roleToOktaGroups = new EnumMap<>(Role.class);
1916
private final AuthProvider authProvider;
2017
private final String environment;
2118
private final boolean isAuthDisabled;
2219
private final Audit audit;
2320

24-
final Map<Role, List<OktaGroup>> roleToOktaGroups = new EnumMap<>(Role.class);
2521
public AdminAuthMiddleware(AuthProvider authProvider, JsonObject config) {
2622
this.authProvider = authProvider;
2723
this.environment = config.getString("environment", "local");
@@ -59,7 +55,6 @@ public Handler<RoutingContext> handle(Handler<RoutingContext> handler, Role... r
5955
return this.handle(handler, new AuditParams(), roles);
6056
}
6157

62-
6358
private Handler<RoutingContext> logAndHandle(Handler<RoutingContext> handler, AuditParams params) {
6459
return ctx -> {
6560
ctx.addBodyEndHandler(v -> this.audit.log(ctx, params));
@@ -73,6 +68,7 @@ private static class AdminAuthHandler {
7368
private final Set<Role> allowedRoles;
7469
private final Map<Role, List<OktaGroup>> roleToOktaGroups;
7570
private final AuthProvider authProvider;
71+
7672
private AdminAuthHandler(Handler<RoutingContext> handler, AuthProvider authProvider, Set<Role> allowedRoles,
7773
String environment, Map<Role, List<OktaGroup>> roleToOktaGroups) {
7874
this.environment = environment;
@@ -96,6 +92,7 @@ public static String extractBearerToken(String headerValue) {
9692
}
9793
}
9894
}
95+
9996
private boolean isAuthorizedUser(List<String> userAssignedGroups) {
10097
for (Role role : allowedRoles) {
10198
if (roleToOktaGroups.containsKey(role)) {
@@ -109,6 +106,7 @@ private boolean isAuthorizedUser(List<String> userAssignedGroups) {
109106
}
110107
return false;
111108
}
109+
112110
private boolean isAuthorizedService(List<String> scopes) {
113111
for (String scope : scopes) {
114112
if (allowedRoles.contains(OktaCustomScope.fromName(scope).getRole())) {
@@ -117,21 +115,22 @@ private boolean isAuthorizedService(List<String> scopes) {
117115
}
118116
return false;
119117
}
118+
120119
public void handle(RoutingContext rc) {
121120
// human user
122121
String idToken = null;
123-
if(rc.user() != null && rc.user().principal() != null) {
122+
if (rc.user() != null && rc.user().principal() != null) {
124123
idToken = rc.user().principal().getString("id_token");
125124
}
126-
if(idToken != null) {
125+
if (idToken != null) {
127126
validateIdToken(rc, idToken);
128127
return;
129128
}
130129

131130
// machine user
132131
String authHeaderValue = rc.request().getHeader("Authorization");
133132
String accessToken = extractBearerToken(authHeaderValue);
134-
if(accessToken == null) {
133+
if (accessToken == null) {
135134
rc.response().putHeader("REQUIRES_AUTH", "1").setStatusCode(401).end();
136135
return;
137136
}
@@ -146,7 +145,7 @@ private void validateAccessToken(RoutingContext rc, String accessToken) {
146145
rc.response().setStatusCode(401).end();
147146
return;
148147
}
149-
if(jwt.getClaims().get("environment") == null || !jwt.getClaims().get("environment").toString().equals(environment)) {
148+
if (jwt.getClaims().get("environment") == null || !jwt.getClaims().get("environment").toString().equals(environment)) {
150149
rc.response().setStatusCode(401).end();
151150
return;
152151
}
@@ -155,7 +154,7 @@ private void validateAccessToken(RoutingContext rc, String accessToken) {
155154
serviceAccountDetails.put("scope", scopes);
156155
serviceAccountDetails.put("client_id", jwt.getClaims().get("client_id"));
157156
rc.put("user_details", serviceAccountDetails);
158-
if(isAuthorizedService(scopes)) {
157+
if (isAuthorizedService(scopes)) {
159158
innerHandler.handle(rc);
160159
} else {
161160
rc.response().setStatusCode(401).end();
@@ -171,7 +170,7 @@ private void validateIdToken(RoutingContext rc, String idToken) {
171170
rc.response().putHeader("REQUIRES_AUTH", "1").setStatusCode(401).end();
172171
return;
173172
}
174-
if(jwt.getClaims().get("environment") == null || !jwt.getClaims().get("environment").toString().equals(environment)) {
173+
if (jwt.getClaims().get("environment") == null || !jwt.getClaims().get("environment").toString().equals(environment)) {
175174
rc.response().setStatusCode(401).end();
176175
return;
177176
}
@@ -181,7 +180,7 @@ private void validateIdToken(RoutingContext rc, String idToken) {
181180
userDetails.put("email", jwt.getClaims().get("email"));
182181
userDetails.put("sub", jwt.getClaims().get("sub"));
183182
rc.put("user_details", userDetails);
184-
if(isAuthorizedUser(groups)) {
183+
if (isAuthorizedUser(groups)) {
185184
innerHandler.handle(rc);
186185
} else {
187186
rc.response().setStatusCode(401).end();

‎src/main/java/com/uid2/admin/salt/SaltRotation.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,15 @@
1919
public class SaltRotation {
2020
private static final long THIRTY_DAYS_IN_MS = Duration.ofDays(30).toMillis();
2121
private static final double MAX_SALT_PERCENTAGE = 0.8;
22-
private final boolean ENABLE_V4_RAW_UID;
22+
private final boolean enableV4RawUid;
2323

2424
private final IKeyGenerator keyGenerator;
2525

2626
private static final Logger LOGGER = LoggerFactory.getLogger(SaltRotation.class);
2727

2828
public SaltRotation(IKeyGenerator keyGenerator, JsonObject config) {
2929
this.keyGenerator = keyGenerator;
30-
this.ENABLE_V4_RAW_UID = config.getBoolean(AdminConst.ENABLE_V4_RAW_UID, false);
30+
this.enableV4RawUid = config.getBoolean(AdminConst.ENABLE_V4_RAW_UID, false);
3131
}
3232

3333
public Result rotateSalts(
@@ -143,10 +143,9 @@ private long calculateRefreshFrom(SaltEntry bucket, TargetDate targetDate) {
143143

144144
private String calculateCurrentSalt(SaltEntry bucket, boolean shouldRotate) throws Exception {
145145
if (shouldRotate) {
146-
if (ENABLE_V4_RAW_UID) {
146+
if (enableV4RawUid) {
147147
return null;
148-
}
149-
else {
148+
} else {
150149
return this.keyGenerator.generateRandomKeyString(32);
151150
}
152151
}
@@ -165,10 +164,10 @@ private String calculatePreviousSalt(SaltEntry bucket, boolean shouldRotate, Tar
165164

166165
private SaltEntry.KeyMaterial calculateCurrentKeySalt(SaltEntry bucket, boolean shouldRotate, KeyIdGenerator keyIdGenerator) throws Exception {
167166
if (shouldRotate) {
168-
if (ENABLE_V4_RAW_UID) {
167+
if (enableV4RawUid) {
169168
return new SaltEntry.KeyMaterial(
170169
keyIdGenerator.getNextKeyId(),
171-
this.keyGenerator.generateRandomKeyString(32),
170+
this.keyGenerator.generateRandomKeyString(24),
172171
this.keyGenerator.generateRandomKeyString(32)
173172
);
174173
} else {
@@ -253,7 +252,6 @@ private void logSaltAges(String saltCountType, TargetDate targetDate, Collection
253252
}
254253
}
255254

256-
257255
/** Logging to monitor migration of buckets from salts (old format - v2/v3) to encryption keys (new format - v4) **/
258256
private void logBucketFormatCount(TargetDate targetDate, SaltEntry[] postRotationBuckets) {
259257
int totalKeys = 0, totalSalts = 0, totalPreviousKeys = 0, totalPreviousSalts = 0;

‎src/main/java/com/uid2/admin/vertx/service/SaltService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ public void setupRoutes(Router router) {
7373
}
7474
}, new AuditParams(List.of(), Collections.emptyList()), Role.MAINTAINER));
7575

76-
router.post(API_SALT_ROTATE.toString()).blockingHandler(auth.handle((ctx) -> {
76+
router.post(API_SALT_ROTATE.toString()).blockingHandler(auth.handle(ctx -> {
7777
synchronized (writeLock) {
7878
this.handleSaltRotate(ctx);
7979
}
80-
}, new AuditParams(List.of("fraction", "min_ages_in_seconds", "target_date"), Collections.emptyList()), Role.SUPER_USER, Role.SECRET_ROTATION));
80+
}, new AuditParams(List.of("fraction", "target_date"), Collections.emptyList()), Role.SUPER_USER, Role.SECRET_ROTATION));
8181
}
8282

8383
private void handleSaltSnapshots(RoutingContext rc) {

‎src/test/java/com/uid2/admin/salt/SaltServiceTest.java‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,13 @@
2020
import static org.mockito.ArgumentMatchers.*;
2121
import static org.mockito.Mockito.*;
2222

23-
public class SaltServiceTest extends ServiceTestBase {
23+
class SaltServiceTest extends ServiceTestBase {
2424
private final TargetDate utcTomorrow = TargetDate.now().plusDays(1);
25-
@Mock RotatingSaltProvider saltProvider;
26-
@Mock SaltRotation saltRotation;
25+
26+
@Mock
27+
private RotatingSaltProvider saltProvider;
28+
@Mock
29+
private SaltRotation saltRotation;
2730

2831
@Override
2932
protected IService createService() {
@@ -77,7 +80,7 @@ void rotateSalts(Vertx vertx, VertxTestContext testContext) throws Exception {
7780
var result = SaltRotation.Result.fromSnapshot(addedSnapshots[0].build());
7881
when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(utcTomorrow))).thenReturn(result);
7982

80-
post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> {
83+
post(vertx, testContext, "api/salt/rotate?fraction=0.2", "", response -> {
8184
assertEquals(200, response.statusCode());
8285
checkSnapshotsResponse(addedSnapshots, new Object[]{response.bodyAsJsonObject()});
8386
verify(saltStoreWriter).upload(any());
@@ -100,7 +103,7 @@ void rotateSaltsNoNewSnapshot(Vertx vertx, VertxTestContext testContext) throws
100103
var result = SaltRotation.Result.noSnapshot("test");
101104
when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(utcTomorrow))).thenReturn(result);
102105

103-
post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> {
106+
post(vertx, testContext, "api/salt/rotate?fraction=0.2", "", response -> {
104107
assertEquals(200, response.statusCode());
105108
JsonObject jo = response.bodyAsJsonObject();
106109
assertFalse(jo.containsKey("effective"));
@@ -127,31 +130,31 @@ void rotateSaltsWithSpecificTargetDate(Vertx vertx, VertxTestContext testContext
127130
var result = SaltRotation.Result.fromSnapshot(addedSnapshots[0].build());
128131
when(saltRotation.rotateSalts(any(), any(), eq(0.2), eq(targetDate()))).thenReturn(result);
129132

130-
post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2&target_date=2025-01-01", "", response -> {
133+
post(vertx, testContext, "api/salt/rotate?fraction=0.2&target_date=2025-01-01", "", response -> {
131134
assertEquals(200, response.statusCode());
132135
testContext.completeNow();
133136
});
134137
}
135138

136139
@Test
137140
void rotateSaltsWithDefaultAgeThresholds(Vertx vertx, VertxTestContext testContext) throws Exception {
138-
fakeAuth(Role.SUPER_USER);
141+
fakeAuth(Role.SUPER_USER);
139142

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

143146
var result = SaltRotation.Result.fromSnapshot(SaltSnapshotBuilder.start().effective(targetDate()).expires(daysEarlier(7)).entries(1, targetDate()).build());
144-
147+
145148
Duration[] expectedDefaultAgeThresholds = new Duration[]{
146-
Duration.ofDays(30), Duration.ofDays(60), Duration.ofDays(90), Duration.ofDays(120),
147-
Duration.ofDays(150), Duration.ofDays(180), Duration.ofDays(210), Duration.ofDays(240),
148-
Duration.ofDays(270), Duration.ofDays(300), Duration.ofDays(330), Duration.ofDays(360),
149-
Duration.ofDays(390)
149+
Duration.ofDays(30), Duration.ofDays(60), Duration.ofDays(90), Duration.ofDays(120),
150+
Duration.ofDays(150), Duration.ofDays(180), Duration.ofDays(210), Duration.ofDays(240),
151+
Duration.ofDays(270), Duration.ofDays(300), Duration.ofDays(330), Duration.ofDays(360),
152+
Duration.ofDays(390)
150153
};
151154

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

154-
post(vertx, testContext, "api/salt/rotate?min_ages_in_seconds=50,60,70&fraction=0.2", "", response -> {
157+
post(vertx, testContext, "api/salt/rotate?fraction=0.2", "", response -> {
155158
verify(saltRotation).rotateSalts(any(), eq(expectedDefaultAgeThresholds), eq(0.2), eq(utcTomorrow));
156159
assertEquals(200, response.statusCode());
157160
testContext.completeNow();

‎webroot/adm/salt.html‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,6 @@ <h1>UID2 Env - Salt Management</h1>
4141
defaultValue: defaultTargetDate
4242
};
4343

44-
const minAgesMultilineInput = {
45-
name: 'minAges',
46-
label: 'Min Ages (seconds)',
47-
required: true,
48-
defaultValue: '2592000,5184000,7776000,10368000,12960000,15552000,18144000,20736000,23328000,25920000,28512000,31104000,33696000',
49-
type: 'multi-line'
50-
};
51-
5244
const operationConfig = {
5345
read: [
5446
{
@@ -81,16 +73,14 @@ <h1>UID2 Env - Salt Management</h1>
8173
role: 'superuser',
8274
inputs: [
8375
fractionInput,
84-
targetDateInput,
85-
minAgesMultilineInput
76+
targetDateInput
8677
],
8778
apiCall: {
8879
method: 'POST',
8980
getUrl: (inputs) => {
90-
const minAges = encodeURIComponent(inputs.minAges);
9181
const fraction = encodeURIComponent(inputs.fraction);
9282
const targetDate = encodeURIComponent(inputs.targetDate);
93-
return `/api/salt/rotate?min_ages_in_seconds=${minAges}&fraction=${fraction}&target_date=${targetDate}`;
83+
return `/api/salt/rotate?fraction=${fraction}&target_date=${targetDate}`;
9484
}
9585
}
9686
}

0 commit comments

Comments
 (0)