-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zdu UID2-1776 Enforce Opt-Out Policy Check for New Participants added after 9/1/2023 #207
Changes from 6 commits
51441a2
6cfbbe5
78a5075
eae64d9
8d1b9cc
4c38fd3
68f36ae
6f5a8dc
78fae45
c0d61a0
ba941b5
3c3404a
84c6309
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ | |
import com.uid2.shared.store.IClientKeyProvider; | ||
import com.uid2.shared.store.IClientSideKeypairStore; | ||
import com.uid2.shared.store.ISaltProvider; | ||
import com.uid2.shared.store.reader.RotatingClientKeyProvider; | ||
import com.uid2.shared.vertx.RequestCapturingHandler; | ||
import io.micrometer.core.instrument.Counter; | ||
import io.micrometer.core.instrument.DistributionSummary; | ||
|
@@ -94,6 +95,7 @@ public class UIDOperatorVerticle extends AbstractVerticle { | |
private final ITokenEncoder encoder; | ||
private final ISaltProvider saltProvider; | ||
private final IOptOutStore optOutStore; | ||
private final IClientKeyProvider clientKeyProvider; | ||
private final Clock clock; | ||
protected IUIDOperatorService idService; | ||
private final Map<String, DistributionSummary> _identityMapMetricSummaries = new HashMap<>(); | ||
|
@@ -111,9 +113,9 @@ public class UIDOperatorVerticle extends AbstractVerticle { | |
private final IStatsCollectorQueue _statsCollectorQueue; | ||
private final KeyManager keyManager; | ||
private final SecureLinkValidatorService secureLinkValidatorService; | ||
|
||
private final boolean cstgDoDomainNameCheck; | ||
public final static int MASTER_KEYSET_ID_FOR_SDKS = 9999999; //this is because SDKs have an issue where they assume keyset ids are always positive; that will be fixed. | ||
public final static long OPT_OUT_CHECK_CUTOFF_DATE = Instant.parse("2023-09-01T00:00:00.00Z").getEpochSecond(); | ||
|
||
public UIDOperatorVerticle(JsonObject config, | ||
boolean clientSideTokenGenerate, | ||
|
@@ -149,6 +151,7 @@ public UIDOperatorVerticle(JsonObject config, | |
this.tcfVendorId = config.getInteger("tcf_vendor_id", 21); | ||
this.cstgDoDomainNameCheck = config.getBoolean("client_side_token_generate_domain_name_check_enabled", true); | ||
this._statsCollectorQueue = statsCollectorQueue; | ||
this.clientKeyProvider = clientKeyProvider; | ||
} | ||
|
||
@Override | ||
|
@@ -768,6 +771,13 @@ private void handleTokenGenerateV2(RoutingContext rc) { | |
} | ||
} | ||
|
||
boolean respsctOptOut = req.containsKey(TOKEN_GENERATE_POLICY_PARAM) | ||
&& TokenGeneratePolicy.fromValue(req.getInteger(TOKEN_GENERATE_POLICY_PARAM)) == TokenGeneratePolicy.respectOptOut(); | ||
if (!respsctOptOut && isOptOutCheckRequired(rc, respsctOptOut)) { | ||
ResponseUtil.ClientError(rc, "Required opt-out policy argument is missing or not set to 1"); | ||
return; | ||
} | ||
|
||
final TokenGeneratePolicy tokenGeneratePolicy = readTokenGeneratePolicy(req); | ||
final IdentityTokens t = this.idService.generateIdentity( | ||
new IdentityRequest( | ||
|
@@ -1274,6 +1284,13 @@ private void handleIdentityMapV2(RoutingContext rc) { | |
return; | ||
} | ||
|
||
boolean respectOptOut = requestJsonObject.containsKey(IDENTITY_MAP_POLICY_PARAM) | ||
&& IdentityMapPolicy.fromValue(requestJsonObject.getInteger(IDENTITY_MAP_POLICY_PARAM)) == IdentityMapPolicy.respectOptOut(); | ||
if (!respectOptOut && isOptOutCheckRequired(rc, respectOptOut)) { | ||
ResponseUtil.ClientError(rc, "Required opt-out policy argument is missing or not set to 1"); | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make a function for the duplicate code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored the code as suggested. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The two blocks still look largely similar. How can more stuff be pushed into the common function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored the similar code into a single method. |
||
|
||
IdentityMapPolicy identityMapPolicy = readIdentityMapPolicy(requestJsonObject); | ||
recordIdentityMapPolicy(getApiContact(rc), identityMapPolicy); | ||
|
||
|
@@ -1616,6 +1633,19 @@ private TokenGeneratePolicy readTokenGeneratePolicy(JsonObject req) { | |
TokenGeneratePolicy.defaultPolicy(); | ||
} | ||
|
||
private boolean isOptOutCheckRequired(RoutingContext rc, boolean respectOptOut) { | ||
final ClientKey clientKey = (ClientKey) AuthMiddleware.getAuthClient(rc); | ||
final ClientKey oldestClientKey = this.clientKeyProvider.getOldestClientKey(clientKey.getSiteId()); | ||
boolean newClient = oldestClientKey.getCreated() >= OPT_OUT_CHECK_CUTOFF_DATE; | ||
|
||
if (newClient && !respectOptOut) { | ||
// log policy violation | ||
LOGGER.warn(String.format("Failed to respect opt-out policy: siteId=%d, clientKeyName=%s, clientKeyCreated=%d", | ||
oldestClientKey.getSiteId(), oldestClientKey.getName(), oldestClientKey.getCreated())); | ||
} | ||
return newClient; | ||
} | ||
|
||
private static final String IDENTITY_MAP_POLICY_PARAM = "policy"; | ||
|
||
private IdentityMapPolicy readIdentityMapPolicy(JsonObject req) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,7 @@ | |
import static com.uid2.operator.ClientSideTokenGenerateTestUtil.decrypt; | ||
import static com.uid2.operator.service.EncodingUtils.getSha256; | ||
import static com.uid2.operator.service.V2RequestUtil.V2_REQUEST_TIMESTAMP_DRIFT_THRESHOLD_IN_MINUTES; | ||
import static com.uid2.operator.vertx.UIDOperatorVerticle.OPT_OUT_CHECK_CUTOFF_DATE; | ||
import static com.uid2.shared.Const.Data.*; | ||
import static org.junit.jupiter.api.Assertions.*; | ||
import static org.mockito.ArgumentMatchers.any; | ||
|
@@ -68,6 +69,8 @@ | |
@ExtendWith(VertxExtension.class) | ||
public class UIDOperatorVerticleTest { | ||
private final Instant now = Instant.now().truncatedTo(ChronoUnit.MILLIS); | ||
private static final Instant legacyClientCreationDateTime = Instant.ofEpochSecond(OPT_OUT_CHECK_CUTOFF_DATE).minus(1, ChronoUnit.SECONDS); | ||
private static final Instant newClientCreationDateTime = Instant.ofEpochSecond(OPT_OUT_CHECK_CUTOFF_DATE).plus(1, ChronoUnit.SECONDS); | ||
private static final String firstLevelSalt = "first-level-salt"; | ||
private static final SaltEntry rotatingSalt123 = new SaltEntry(123, "hashed123", 0, "salt123"); | ||
private static final Duration identityExpiresAfter = Duration.ofMinutes(10); | ||
|
@@ -148,18 +151,22 @@ private static byte[] makeAesKey(String prefix) { | |
} | ||
|
||
protected void fakeAuth(int siteId, Role... roles) { | ||
fakeAuth(siteId, legacyClientCreationDateTime, roles); | ||
} | ||
protected void fakeAuth(int siteId, Instant created, Role... roles) { | ||
ClientKey clientKey = new ClientKey( | ||
"test-key", | ||
"UID2-C-L-999-fCXrMM.fsR3mDqAXELtWWMS+xG1s7RdgRTMqdOH2qaAo=", | ||
"fsSGnDxa/V9eJZ9Tas+dowwyO/X1UsC68RN9qM2xUu9ZOaKEOv9EVd7pkt3As/nE5B6TRu0PzK+IDzSQhD1+rw==", | ||
Utils.toBase64String(clientSecret), | ||
"test-contact", | ||
now, | ||
created, | ||
Set.of(roles), | ||
siteId | ||
); | ||
when(clientKeyProvider.get(any())).thenReturn(clientKey); | ||
when(clientKeyProvider.getClientKey(any())).thenReturn(clientKey); | ||
when(clientKeyProvider.getOldestClientKey(anyInt())).thenReturn(clientKey); | ||
} | ||
|
||
private void clearAuth() { | ||
|
@@ -741,6 +748,68 @@ public static void validateAdvertisingToken(String advertisingTokenString, Token | |
} | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = {"NoOutoutPolicySpecified", "WrongOutOutPolicySpecified"}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you pass the policy values instead of strings? You should avoid switch (and if) statements in the unit tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
void identityMapOptOutPolicyCheckForNewClient(String testRun, Vertx vertx, VertxTestContext testContext) { | ||
final int clientSiteId = 201; | ||
fakeAuth(clientSiteId, newClientCreationDateTime, Role.MAPPER); | ||
setupSalts(); | ||
setupKeys(); | ||
|
||
JsonObject req = new JsonObject(); | ||
JsonArray emails = new JsonArray(); | ||
emails.add("[email protected]"); | ||
req.put("email", emails); | ||
switch (testRun) { | ||
case "NoOutoutPolicySpecified": | ||
break; | ||
case "WrongOutOutPolicySpecified": | ||
req.put("policy", 0); | ||
break; | ||
default: | ||
req.put("policy", 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Judging by value source above this case would never get hit? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed. |
||
break; | ||
} | ||
|
||
send("v2", vertx, "v2/identity/map", false, null, req, 400, respJson -> { | ||
assertFalse(respJson.containsKey("body")); | ||
assertEquals("client_error", respJson.getString("status")); | ||
assertEquals("Required opt-out policy argument is missing or not set to 1", respJson.getString("message")); | ||
testContext.completeNow(); | ||
}); | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = {"NoPolicySpecified", "WrongPolicySpecified"}) | ||
void tokenGenerateOptOutPolicyCheckForNewClient(String testRun, Vertx vertx, VertxTestContext testContext) { | ||
final int clientSiteId = 201; | ||
fakeAuth(clientSiteId, newClientCreationDateTime, Role.GENERATOR); | ||
setupSalts(); | ||
setupKeys(); | ||
|
||
JsonObject v2Payload = new JsonObject(); | ||
v2Payload.put("email", "[email protected]"); | ||
switch (testRun) { | ||
case "NoPolicySpecified": | ||
break; | ||
case "WrongPolicySpecified": | ||
v2Payload.put("policy", 0); | ||
break; | ||
default: | ||
v2Payload.put("policy", 1); | ||
break; | ||
} | ||
|
||
sendTokenGenerate("v2", vertx, | ||
"", v2Payload, 400, | ||
json -> { | ||
assertFalse(json.containsKey("body")); | ||
assertEquals("client_error", json.getString("status")); | ||
assertEquals("Required opt-out policy argument is missing or not set to 1", json.getString("message")); | ||
testContext.completeNow(); | ||
}); | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = {"v1", "v2"}) | ||
void tokenGenerateForEmail(String apiVersion, Vertx vertx, VertxTestContext testContext) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in variable name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops...