Skip to content

Commit d171495

Browse files
MarieseMikelyMarieseMikely
authored
Merge pull request #621 from govuk-one-login/OJ-3391
OJ-3391: Remove Unused Feature Flags
2 parents 24b5008 + 00f2d40 commit d171495

3 files changed

Lines changed: 9 additions & 181 deletions

File tree

‎build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ plugins {
1111
}
1212

1313
// Important: see README on publishing a new version to Maven
14-
def buildVersion = "8.3.5"
14+
def buildVersion = "9.0.0"
1515

1616
defaultTasks 'clean', 'spotlessApply', 'build'
1717

‎src/main/java/uk/gov/di/ipv/cri/common/library/util/VerifiableCredentialClaimsSetBuilder.java‎

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@
1515

1616
public class VerifiableCredentialClaimsSetBuilder {
1717

18-
public static final String ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED =
19-
"ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED";
20-
public static final String ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID =
21-
"ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID";
22-
2318
private final ConfigurationService configurationService;
2419
private final Clock clock;
2520

@@ -28,7 +23,6 @@ public class VerifiableCredentialClaimsSetBuilder {
2823
private String[] contexts;
2924
private String subject;
3025
private Object evidence;
31-
private ChronoUnit ttlUnit;
3226
private long ttl;
3327
private JWTClaimsSet.Builder jwtClaimsSetBuilder;
3428

@@ -47,7 +41,7 @@ public VerifiableCredentialClaimsSetBuilder subject(String subject) {
4741
}
4842

4943
public VerifiableCredentialClaimsSetBuilder timeToLive(long ttl, ChronoUnit ttlUnit) {
50-
this.ttlUnit = Objects.requireNonNull(ttlUnit, "ttlUnit must not be null");
44+
Objects.requireNonNull(ttlUnit, "ttlUnit must not be null");
5145
if (ttl < 1L) {
5246
throw new IllegalArgumentException("ttl must be greater than zero");
5347
}
@@ -108,17 +102,11 @@ public JWTClaimsSet build() {
108102
.issuer(issuer)
109103
.claim(JWTClaimNames.NOT_BEFORE, dateTimeNow.toEpochSecond());
110104

111-
if (!isReleaseFlag(ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED)) {
112-
builder.claim(JWTClaimNames.EXPIRATION_TIME, getExpirationTimestamp(dateTimeNow));
113-
}
114-
115105
Map<String, Object> verifiableCredentialClaims = new LinkedHashMap<>();
116106
verifiableCredentialClaims.put(
117107
"type", new String[] {"VerifiableCredential", this.verifiableCredentialType});
118108

119-
if (isReleaseFlag(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID)) {
120-
builder.claim(JWTClaimNames.JWT_ID, generateUniqueId());
121-
}
109+
builder.claim(JWTClaimNames.JWT_ID, generateUniqueId());
122110

123111
if (Objects.nonNull(this.contexts) && contexts.length > 0) {
124112
verifiableCredentialClaims.put("@context", contexts);
@@ -134,10 +122,7 @@ public JWTClaimsSet build() {
134122
}
135123

136124
public JWTClaimsSet.Builder overrideJti(String jti) {
137-
if (System.getenv(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID).equals("override")) {
138-
return overrideJti().jwtID(jti);
139-
}
140-
return this.jwtClaimsSetBuilder;
125+
return overrideJti().jwtID(jti);
141126
}
142127

143128
private JWTClaimsSet.Builder overrideJti() {
@@ -147,30 +132,6 @@ private JWTClaimsSet.Builder overrideJti() {
147132
return this.jwtClaimsSetBuilder;
148133
}
149134

150-
private boolean isReleaseFlag(String environmentVariable) {
151-
return Boolean.parseBoolean(System.getenv(environmentVariable));
152-
}
153-
154-
private long getExpirationTimestamp(OffsetDateTime dateTimeNow) {
155-
switch (this.ttlUnit) {
156-
case SECONDS:
157-
return dateTimeNow.plusSeconds(this.ttl).toEpochSecond();
158-
case MINUTES:
159-
return dateTimeNow.plusMinutes(this.ttl).toEpochSecond();
160-
case HOURS:
161-
return dateTimeNow.plusHours(this.ttl).toEpochSecond();
162-
case DAYS:
163-
return dateTimeNow.plusDays(this.ttl).toEpochSecond();
164-
case MONTHS:
165-
return dateTimeNow.plusMonths(this.ttl).toEpochSecond();
166-
case YEARS:
167-
return dateTimeNow.plusYears(this.ttl).toEpochSecond();
168-
default:
169-
throw new IllegalStateException(
170-
"Unexpected time-to-live unit encountered: " + ttlUnit);
171-
}
172-
}
173-
174135
private String generateUniqueId() {
175136
return String.format("urn:uuid:%s", UUID.randomUUID());
176137
}

‎src/test/java/uk/gov/di/ipv/cri/common/library/util/VerifiableCredentialClaimsSetBuilderTest.java‎

Lines changed: 5 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,12 @@
55
import org.junit.jupiter.api.Test;
66
import org.junit.jupiter.api.extension.ExtendWith;
77
import org.junit.jupiter.params.ParameterizedTest;
8-
import org.junit.jupiter.params.provider.CsvSource;
98
import org.junit.jupiter.params.provider.NullAndEmptySource;
10-
import org.junit.jupiter.params.provider.NullSource;
119
import org.junit.jupiter.params.provider.ValueSource;
1210
import org.mockito.Mock;
1311
import org.mockito.junit.jupiter.MockitoExtension;
1412
import uk.gov.di.ipv.cri.common.library.domain.personidentity.PersonIdentityDetailed;
1513
import uk.gov.di.ipv.cri.common.library.service.ConfigurationService;
16-
import uk.org.webcompere.systemstubs.environment.EnvironmentVariables;
17-
import uk.org.webcompere.systemstubs.jupiter.SystemStub;
1814
import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension;
1915

2016
import java.text.ParseException;
@@ -26,7 +22,6 @@
2622
import java.util.Map;
2723
import java.util.UUID;
2824

29-
import static com.nimbusds.jwt.JWTClaimNames.EXPIRATION_TIME;
3025
import static com.nimbusds.jwt.JWTClaimNames.NOT_BEFORE;
3126
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
3227
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -37,8 +32,6 @@
3732
import static org.junit.jupiter.api.Assertions.assertTrue;
3833
import static org.mockito.Mockito.mock;
3934
import static org.mockito.Mockito.when;
40-
import static uk.gov.di.ipv.cri.common.library.util.VerifiableCredentialClaimsSetBuilder.ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID;
41-
import static uk.gov.di.ipv.cri.common.library.util.VerifiableCredentialClaimsSetBuilder.ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED;
4235

4336
@ExtendWith(MockitoExtension.class)
4437
@ExtendWith(SystemStubsExtension.class)
@@ -49,21 +42,13 @@ class VerifiableCredentialClaimsSetBuilderTest {
4942
private static final PersonIdentityDetailed TEST_PERSON_IDENTITY =
5043
mock(PersonIdentityDetailed.class);
5144

52-
// Needs to be created here
53-
@SystemStub private EnvironmentVariables environmentVariables = new EnvironmentVariables();
54-
5545
@Mock private ConfigurationService mockConfigurationService;
5646
private Clock clock;
5747
private VerifiableCredentialClaimsSetBuilder builder;
5848

5949
@BeforeEach
6050
void setup() {
6151
clock = Clock.fixed(Instant.parse("2016-11-03T10:15:30Z"), ZoneId.of("UTC"));
62-
63-
// Defaults
64-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED, null);
65-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID, null);
66-
6752
builder = new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock);
6853
}
6954

@@ -92,9 +77,6 @@ void shouldBuildVerifiableCredentialWhenVcExpiryRemovedReleaseFlagIsNotSpecified
9277
assertEquals(TEST_SUBJECT, builtClaimSet.getSubject());
9378
assertEquals(TEST_ISSUER, builtClaimSet.getIssuer());
9479
assertEquals(this.clock.instant().getEpochSecond(), builtClaimSet.getLongClaim(NOT_BEFORE));
95-
assertTrue(
96-
builtClaimSet.getLongClaim(EXPIRATION_TIME)
97-
> this.clock.instant().getEpochSecond());
9880
assertEquals(
9981
TEST_PERSON_IDENTITY,
10082
builtClaimSet.getJSONObjectClaim("vc").get("credentialSubject"));
@@ -106,13 +88,8 @@ void shouldBuildVerifiableCredentialWhenVcExpiryRemovedReleaseFlagIsNotSpecified
10688
}
10789

10890
@ParameterizedTest
109-
@NullSource
11091
@ValueSource(strings = {"", " ", "false", "anything"})
111-
void shouldBuildVerifiableCredential(String expiryRemovedReleasedFlagValue)
112-
throws ParseException {
113-
114-
environmentVariables.set(
115-
ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED, expiryRemovedReleasedFlagValue);
92+
void shouldBuildVerifiableCredential() throws ParseException {
11693
builder = new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock);
11794

11895
String[] testContexts = new String[] {"context1", "context2"};
@@ -137,9 +114,6 @@ void shouldBuildVerifiableCredential(String expiryRemovedReleasedFlagValue)
137114
assertEquals(TEST_SUBJECT, builtClaimSet.getSubject());
138115
assertEquals(TEST_ISSUER, builtClaimSet.getIssuer());
139116
assertEquals(this.clock.instant().getEpochSecond(), builtClaimSet.getLongClaim(NOT_BEFORE));
140-
assertTrue(
141-
builtClaimSet.getLongClaim(EXPIRATION_TIME)
142-
> this.clock.instant().getEpochSecond());
143117
assertEquals(
144118
TEST_PERSON_IDENTITY,
145119
builtClaimSet.getJSONObjectClaim("vc").get("credentialSubject"));
@@ -154,7 +128,6 @@ void shouldBuildVerifiableCredential(String expiryRemovedReleasedFlagValue)
154128
void shouldBuildVerifiableCredentialWithoutAnExpiryTime() throws ParseException {
155129
String[] testContexts = new String[] {"context1", "context2"};
156130

157-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED, true);
158131
builder = new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock);
159132

160133
Map<String, String> evidence =
@@ -185,58 +158,10 @@ void shouldBuildVerifiableCredentialWithoutAnExpiryTime() throws ParseException
185158
(String[]) builtClaimSet.getJSONObjectClaim("vc").get("type"));
186159
assertEquals(testContexts, builtClaimSet.getJSONObjectClaim("vc").get("@context"));
187160
assertEquals(evidence, builtClaimSet.getJSONObjectClaim("vc").get("evidence"));
188-
189-
assertNull(builtClaimSet.getLongClaim(EXPIRATION_TIME));
190-
}
191-
192-
@Test
193-
void shouldOverrideJtiWhenContainUniqueIdIsFalse() {
194-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID, "override");
195-
when(mockConfigurationService.getVerifiableCredentialIssuer()).thenReturn(TEST_ISSUER);
196-
197-
var claimsSetBuilder =
198-
new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock)
199-
.subject(TEST_SUBJECT)
200-
.timeToLive(1L, ChronoUnit.MONTHS)
201-
.verifiableCredentialType(TEST_VC_TYPE)
202-
.verifiableCredentialSubject(TEST_PERSON_IDENTITY);
203-
204-
var originalBuilder = claimsSetBuilder.build();
205-
206-
claimsSetBuilder.overrideJti("dummyJti");
207-
claimsSetBuilder.build();
208-
209-
assertNotEquals(originalBuilder.getJWTID(), claimsSetBuilder.build().getJWTID());
210-
assertEquals(null, originalBuilder.getJWTID());
211-
212-
assertEquals("dummyJti", claimsSetBuilder.build().getJWTID());
213-
}
214-
215-
@Test
216-
void cannotOverrideJtiWhenContainUniqueIdIsFalse() {
217-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID, false);
218-
when(mockConfigurationService.getVerifiableCredentialIssuer()).thenReturn(TEST_ISSUER);
219-
220-
var claimsSetBuilder =
221-
new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock)
222-
.subject(TEST_SUBJECT)
223-
.timeToLive(1L, ChronoUnit.MONTHS)
224-
.verifiableCredentialType(TEST_VC_TYPE)
225-
.verifiableCredentialSubject(TEST_PERSON_IDENTITY);
226-
227-
var originalBuilder = claimsSetBuilder.build();
228-
229-
claimsSetBuilder.overrideJti("dummyJti");
230-
claimsSetBuilder.build();
231-
232-
assertNull(claimsSetBuilder.build().getJWTID());
233-
assertEquals(null, originalBuilder.getJWTID());
234-
assertEquals(null, claimsSetBuilder.build().getJWTID());
235161
}
236162

237163
@Test
238-
void cannotOverrideJtiWhenContainUniqueIdIsTrue() {
239-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID, true);
164+
void cannotOverrideJtiWhenContainUniqueIdIsDefaultedToTrue() {
240165
when(mockConfigurationService.getVerifiableCredentialIssuer()).thenReturn(TEST_ISSUER);
241166

242167
var claimsSetBuilder =
@@ -258,13 +183,10 @@ void cannotOverrideJtiWhenContainUniqueIdIsTrue() {
258183
}
259184

260185
@ParameterizedTest
261-
@NullSource
262186
@ValueSource(strings = {"", " ", "false", "anything"})
263-
void shouldBuildVerifiableCredentialWhenVcContainsUniqueIdReleaseFlagIsNotSpecified(
264-
String expiryRemovedReleasedFlagValue) throws ParseException {
187+
void shouldBuildVerifiableCredentialWhenVcContainsUniqueIdReleaseFlagIsNotSpecified()
188+
throws ParseException {
265189

266-
environmentVariables.set(
267-
ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED, expiryRemovedReleasedFlagValue);
268190
builder = new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock);
269191

270192
String[] testContexts = new String[] {"context1", "context2"};
@@ -286,9 +208,6 @@ void shouldBuildVerifiableCredentialWhenVcContainsUniqueIdReleaseFlagIsNotSpecif
286208
assertEquals(TEST_SUBJECT, builtClaimSet.getSubject());
287209
assertEquals(TEST_ISSUER, builtClaimSet.getIssuer());
288210
assertEquals(this.clock.instant().getEpochSecond(), builtClaimSet.getLongClaim(NOT_BEFORE));
289-
assertTrue(
290-
builtClaimSet.getLongClaim(EXPIRATION_TIME)
291-
> this.clock.instant().getEpochSecond());
292211
assertEquals(
293212
TEST_PERSON_IDENTITY,
294213
builtClaimSet.getJSONObjectClaim("vc").get("credentialSubject"));
@@ -300,14 +219,9 @@ void shouldBuildVerifiableCredentialWhenVcContainsUniqueIdReleaseFlagIsNotSpecif
300219
}
301220

302221
@ParameterizedTest
303-
@NullSource
304222
@ValueSource(strings = {"", " ", "false", "anything"})
305-
void shouldBuildVerifiableCredentialWhenVcContainsUniqueIdReleaseFlagIsSpecified(
306-
String expiryRemovedReleasedFlagValue) throws ParseException {
223+
void shouldBuildVerifiableCredentialWithoutFeatureFlags() throws ParseException {
307224

308-
environmentVariables.set(
309-
ENV_VAR_FEATURE_FLAG_VC_EXPIRY_REMOVED, expiryRemovedReleasedFlagValue);
310-
environmentVariables.set(ENV_VAR_FEATURE_FLAG_VC_CONTAINS_UNIQUE_ID, true);
311225
builder = new VerifiableCredentialClaimsSetBuilder(mockConfigurationService, clock);
312226

313227
String[] testContexts = new String[] {"context1", "context2"};
@@ -328,9 +242,6 @@ void shouldBuildVerifiableCredentialWhenVcContainsUniqueIdReleaseFlagIsSpecified
328242
assertEquals(TEST_SUBJECT, builtClaimSet.getSubject());
329243
assertEquals(TEST_ISSUER, builtClaimSet.getIssuer());
330244
assertEquals(this.clock.instant().getEpochSecond(), builtClaimSet.getLongClaim(NOT_BEFORE));
331-
assertTrue(
332-
builtClaimSet.getLongClaim(EXPIRATION_TIME)
333-
> this.clock.instant().getEpochSecond());
334245
assertEquals(
335246
TEST_PERSON_IDENTITY,
336247
builtClaimSet.getJSONObjectClaim("vc").get("credentialSubject"));
@@ -372,9 +283,6 @@ void shouldBuildVerifiableCredentialWithoutContextAndEvidence() throws ParseExce
372283
assertEquals(TEST_SUBJECT, builtClaimSet.getSubject());
373284
assertEquals(TEST_ISSUER, builtClaimSet.getIssuer());
374285
assertEquals(this.clock.instant().getEpochSecond(), builtClaimSet.getLongClaim(NOT_BEFORE));
375-
assertTrue(
376-
builtClaimSet.getLongClaim(EXPIRATION_TIME)
377-
> this.clock.instant().getEpochSecond());
378286
assertEquals(
379287
TEST_PERSON_IDENTITY,
380288
builtClaimSet.getJSONObjectClaim("vc").get("credentialSubject"));
@@ -386,33 +294,6 @@ void shouldBuildVerifiableCredentialWithoutContextAndEvidence() throws ParseExce
386294
assertNull(builtClaimSet.getJSONObjectClaim("vc").get("evidence"));
387295
}
388296

389-
@ParameterizedTest
390-
@CsvSource({
391-
"66,SECONDS,2016-11-03T10:16:36Z",
392-
"30,MINUTES,2016-11-03T10:45:30Z",
393-
"4,HOURS,2016-11-03T14:15:30Z",
394-
"2,DAYS,2016-11-05T10:15:30Z",
395-
"1,MONTHS,2016-12-03T10:15:30Z",
396-
"2,YEARS,2018-11-03T10:15:30Z"
397-
})
398-
void shouldBuildVerifiableCredentialWithAppropriateExpirationTime(
399-
long ttl, ChronoUnit ttlUnit, String expectedExpirationTime) throws ParseException {
400-
401-
when(mockConfigurationService.getVerifiableCredentialIssuer()).thenReturn(TEST_ISSUER);
402-
403-
JWTClaimsSet builtClaimSet =
404-
this.builder
405-
.subject(TEST_SUBJECT)
406-
.timeToLive(ttl, ttlUnit)
407-
.verifiableCredentialType(TEST_VC_TYPE)
408-
.verifiableCredentialSubject(TEST_PERSON_IDENTITY)
409-
.build();
410-
411-
assertEquals(
412-
builtClaimSet.getLongClaim(EXPIRATION_TIME),
413-
Instant.parse(expectedExpirationTime).getEpochSecond());
414-
}
415-
416297
@Test
417298
void shouldThrowErrorWhenVerifiableCredentialTypeNotSet() {
418299
IllegalStateException thrownException =
@@ -533,18 +414,4 @@ void shouldThrowErrorWhenNullVerifiableCredentialEvidenceSupplied() {
533414
() -> this.builder.verifiableCredentialEvidence(null));
534415
assertEquals("evidence must not be null", thrownException.getMessage());
535416
}
536-
537-
@Test
538-
void shouldThrowErrorWhenInvalidTimeToLiveUnitSupplied() {
539-
when(mockConfigurationService.getVerifiableCredentialIssuer()).thenReturn(TEST_ISSUER);
540-
this.builder
541-
.subject(TEST_SUBJECT)
542-
.timeToLive(5L, ChronoUnit.MILLIS)
543-
.verifiableCredentialType(TEST_VC_TYPE)
544-
.verifiableCredentialSubject(TEST_PERSON_IDENTITY);
545-
IllegalStateException thrownException =
546-
assertThrows(IllegalStateException.class, () -> this.builder.build());
547-
assertEquals(
548-
"Unexpected time-to-live unit encountered: Millis", thrownException.getMessage());
549-
}
550417
}

0 commit comments

Comments
 (0)