Skip to content

Commit 2495629

Browse files
strehleadrianhoelzl-sapdependabot[bot]
authored
fix federated credential administration (#3377)
* fix federated credential administration * Update model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtCredential.java Co-authored-by: Adrian Hölzl <[email protected]> * build(deps): bump joda-time:joda-time from 2.13.1 to 2.14.0 (#3378) Bumps [joda-time:joda-time](https://github.com/JodaOrg/joda-time) from 2.13.1 to 2.14.0. - [Release notes](https://github.com/JodaOrg/joda-time/releases) - [Changelog](https://github.com/JodaOrg/joda-time/blob/main/RELEASE-NOTES.txt) - [Commits](JodaOrg/joda-time@v2.13.1...v2.14.0) --- updated-dependencies: - dependency-name: joda-time:joda-time dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * review * review --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Adrian Hölzl <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
1 parent 2c746f3 commit 2495629

File tree

6 files changed

+78
-18
lines changed

6 files changed

+78
-18
lines changed

‎model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtChangeRequest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import static org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest.ChangeMode.ADD;
99
import static org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest.ChangeMode.DELETE;
10-
import static org.cloudfoundry.identity.uaa.oauth.client.ClientJwtChangeRequest.ChangeMode.UPDATE;
1110

1211
@JsonInclude(JsonInclude.Include.NON_NULL)
1312
@JsonIgnoreProperties(ignoreUnknown = true)
@@ -124,8 +123,8 @@ public String getChangeValue() {
124123

125124
@JsonIgnore
126125
public boolean isFederated() {
127-
return ((changeMode == ADD || changeMode == UPDATE) && issuer != null && subject != null) ||
128-
(changeMode == DELETE && (issuer != null || subject != null));
126+
// private_key_jwt according to RFC 7523. audience is addition supported, but optional
127+
return issuer != null && subject != null;
129128
}
130129

131130
@JsonIgnore

‎model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtCredential.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.springframework.util.StringUtils;
1111

1212
import java.util.List;
13+
import java.util.Objects;
1314

1415
@JsonInclude(JsonInclude.Include.NON_EMPTY)
1516
@JsonIgnoreProperties(ignoreUnknown = true)
@@ -45,4 +46,21 @@ public static List<ClientJwtCredential> parse(String clientJwtCredentials) {
4546
}
4647
}
4748

49+
@Override
50+
public boolean equals(Object object) {
51+
if (this == object) return true;
52+
if (object == null || getClass() != object.getClass()) return false;
53+
ClientJwtCredential that = (ClientJwtCredential) object;
54+
return subject.equals(that.subject) &&
55+
issuer.equals(that.issuer) &&
56+
Objects.equals(audience, that.audience);
57+
}
58+
59+
@Override
60+
public int hashCode() {
61+
int result = subject.hashCode();
62+
result = 31 * result + issuer.hashCode();
63+
result = 31 * result + (audience != null ? audience.hashCode() : 0);
64+
return result;
65+
}
4866
}

‎model/src/test/java/org/cloudfoundry/identity/uaa/oauth/client/ClientJwtCredentialTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,16 @@ void testDeserializerParserException() {
4040
assertThatThrownBy(() -> ClientJwtCredential.parse("[\"iss\":\"issuer\"]"))
4141
.isInstanceOf(IllegalArgumentException.class).hasMessage("Client jwt configuration cannot be parsed");
4242
}
43+
44+
@Test
45+
void testHashCode() {
46+
assertThat(new ClientJwtCredential("subject", "issuer", "audience")).hasSameHashCodeAs(new ClientJwtCredential("subject", "issuer", "audience"));
47+
assertThat(new ClientJwtCredential("subject", "issuer", "audience")).doesNotHaveSameHashCodeAs(new ClientJwtCredential("subject", "issuer", null));
48+
}
49+
50+
@Test
51+
void testEquals() {
52+
assertThat(new ClientJwtCredential("subject", "issuer", "audience")).isEqualTo(new ClientJwtCredential("subject", "issuer", "audience"));
53+
assertThat(new ClientJwtCredential("subject", "issuer", "audience")).isNotEqualTo(new ClientJwtCredential("subject", "issuer", null));
54+
}
4355
}

‎server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody
562562
ActionResult result;
563563
switch (change.getChangeMode()) {
564564
case ADD :
565-
if (change.getChangeValue() != null) {
565+
if (change.getChangeValue() != null && !change.isFederated()) {
566566
clientRegistrationService.addClientJwtConfig(client_id, change.getChangeValue(), IdentityZoneHolder.get().getId(), false);
567567
result = new ActionResult("ok", "Client jwt configuration is added");
568568
} else {
@@ -577,7 +577,7 @@ public ActionResult changeClientJwt(@PathVariable String client_id, @RequestBody
577577
break;
578578

579579
case DELETE :
580-
if (ClientJwtConfiguration.readValue(uaaUaaClientDetails) != null && change.getChangeValue() != null) {
580+
if (ClientJwtConfiguration.readValue(uaaUaaClientDetails) != null && change.getChangeValue() != null && !change.isFederated()) {
581581
clientRegistrationService.deleteClientJwtConfig(client_id, change.getChangeValue(), IdentityZoneHolder.get().getId());
582582
result = new ActionResult("ok", "Client jwt configuration is deleted");
583583
} else {

‎server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfiguration.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,7 @@ public static ClientJwtConfiguration merge(ClientJwtConfiguration existingConfig
311311
result = new ClientJwtConfiguration(newConfig.jwksUri, null);
312312
} else {
313313
result = existingConfig;
314+
result.jwksUri = existingConfig.jwksUri != null ? existingConfig.jwksUri : newConfig.jwksUri;
314315
}
315316
}
316317
if (newConfig.jwkSet != null) {
@@ -319,6 +320,7 @@ public static ClientJwtConfiguration merge(ClientJwtConfiguration existingConfig
319320
result = new ClientJwtConfiguration(null, newConfig.jwkSet);
320321
} else {
321322
result = existingConfig;
323+
result.jwkSet = newConfig.jwkSet;
322324
}
323325
} else {
324326
JsonWebKeySet<JsonWebKey> existingKeySet = existingConfig.jwkSet;
@@ -357,39 +359,35 @@ public static ClientJwtConfiguration delete(ClientJwtConfiguration existingConfi
357359
if (tobeDeleted == null) {
358360
return existingConfig;
359361
}
360-
ClientJwtConfiguration result = null;
362+
ClientJwtConfiguration result = existingConfig;
361363
if (existingConfig.jwkSet != null && tobeDeleted.jwksUri != null) {
362364
JsonWebKeySet<JsonWebKey> existingKeySet = existingConfig.jwkSet;
363365
List<JsonWebKey> keys = existingKeySet.getKeys().stream().filter(k -> !tobeDeleted.jwksUri.equals(k.getKid())).toList();
364366
if (keys.isEmpty()) {
365-
result = null;
367+
result.jwkSet = null;
366368
} else {
367369
result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(keys));
368370
}
369371
} else if (existingConfig.jwkSet != null && tobeDeleted.jwkSet != null) {
370372
List<JsonWebKey> existingKeys = new ArrayList<>(existingConfig.getJwkSet().getKeys());
371373
existingKeys.removeAll(tobeDeleted.jwkSet.getKeys());
372374
if (existingKeys.isEmpty()) {
373-
result = null;
375+
result.jwkSet = null;
374376
} else {
375377
result = new ClientJwtConfiguration(null, new JsonWebKeySet<>(existingKeys));
376378
}
377379
} else if (existingConfig.jwksUri != null && tobeDeleted.jwksUri != null) {
378380
if ("*".equals(tobeDeleted.jwksUri) || existingConfig.jwksUri.equals(tobeDeleted.jwksUri)) {
379-
result = null;
380-
} else {
381-
result = existingConfig;
381+
result.jwksUri = null;
382382
}
383383
} else if (existingConfig.clientJwtCredentials != null && tobeDeleted.clientJwtCredentials != null) {
384384
existingConfig.clientJwtCredentials = existingConfig.clientJwtCredentials.stream()
385385
.filter (c -> tobeDeleted.clientJwtCredentials.stream()
386-
.filter(e -> e.getSubject().equals(c.getSubject()) && e.getIssuer().equals(c.getIssuer())).isParallel()).toList();
387-
result = existingConfig;
388-
if (ObjectUtils.isEmpty(result.clientJwtCredentials)) {
386+
.noneMatch(e -> e.getSubject().equals(c.getSubject()) && e.getIssuer().equals(c.getIssuer()))).toList();
387+
if (ObjectUtils.isEmpty(result.clientJwtCredentials) || tobeDeleted.clientJwtCredentials.equals(List.of(new ClientJwtCredential("*", "*", null))) || tobeDeleted.clientJwtCredentials.equals(List.of(new ClientJwtCredential("*", "*", "*")))) {
389388
result.clientJwtCredentials = null;
390389
}
391-
result = ObjectUtils.isEmpty(result.jwkSet) && ObjectUtils.isEmpty(result.jwksUri) ? null : result;
392390
}
393-
return result;
391+
return ObjectUtils.isEmpty(result.jwkSet) && ObjectUtils.isEmpty(result.jwksUri) && ObjectUtils.isEmpty(result.clientJwtCredentials) ? null : result;
394392
}
395393
}

‎server/src/test/java/org/cloudfoundry/identity/uaa/client/ClientJwtConfigurationTest.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void configMergeDifferentType() {
161161
assertThat(configuration.getJwksUri()).isNull();
162162
configuration = ClientJwtConfiguration.merge(configuration, ClientJwtConfiguration.parse("https://any/jwks-uri"), false);
163163
assertThat(configuration.getJwkSet().getKeys()).hasSize(1);
164-
assertThat(configuration.getJwksUri()).isNull();
164+
assertThat(configuration.getJwksUri()).isEqualTo("https://any/jwks-uri");
165165

166166
configuration = ClientJwtConfiguration.merge(configuration, ClientJwtConfiguration.parse("https://any/jwks-uri"), true);
167167
assertThat(configuration.getJwkSet()).isNull();
@@ -176,7 +176,8 @@ void configMergeDifferentType() {
176176
assertThat(configuration.getJwksUri()).isEqualTo("https://new/jwks-uri");
177177

178178
configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse(jsonJwkSet), false);
179-
assertThat(configuration.getJwkSet()).isNull();
179+
assertThat(configuration.getJwkSet()).isNotNull();
180+
assertThat(configuration.getJwkSet().getKeys()).hasSize(1);
180181
assertThat(configuration.getJwksUri()).isEqualTo("https://any/jwks-uri");
181182

182183
configuration = ClientJwtConfiguration.merge(ClientJwtConfiguration.parse("https://any/jwks-uri"), ClientJwtConfiguration.parse(jsonJwkSet), true);
@@ -225,6 +226,37 @@ void configDelete() {
225226
assertThat(configuration).isNotNull();
226227
}
227228

229+
@Test
230+
void configDeleteFederated() {
231+
ClientJwtCredential existingKey1 = new ClientJwtCredential("subject1", "issuer1", "audience1");
232+
ClientJwtCredential existingKey2 = new ClientJwtCredential("subject2", "issuer2", "audience2");
233+
ClientJwtConfiguration existingConfiguration = new ClientJwtConfiguration(List.of(existingKey1, existingKey2));
234+
assertThat(ClientJwtConfiguration.delete(existingConfiguration, new ClientJwtConfiguration(List.of(new ClientJwtCredential("not-existing", "not-existing", null))))).isEqualTo(existingConfiguration);
235+
assertThat(ClientJwtConfiguration.delete(existingConfiguration, new ClientJwtConfiguration(List.of(existingKey1)))).isEqualTo(new ClientJwtConfiguration(List.of(existingKey2)));
236+
existingConfiguration = new ClientJwtConfiguration(List.of(existingKey1, existingKey2));
237+
assertThat(ClientJwtConfiguration.delete(existingConfiguration, existingConfiguration)).isNull();
238+
assertThat(ClientJwtConfiguration.delete(new ClientJwtConfiguration(List.of(existingKey1, existingKey2)), new ClientJwtConfiguration(List.of(new ClientJwtCredential("*", "*", null))))).isNull();
239+
assertThat(ClientJwtConfiguration.delete(new ClientJwtConfiguration(List.of(existingKey1, existingKey2)), new ClientJwtConfiguration(List.of(new ClientJwtCredential("*", "*", "*"))))).isNull();
240+
}
241+
242+
@Test
243+
void configDeleteMixedJwtConfiguration() {
244+
ClientJwtCredential existingKey1 = new ClientJwtCredential("subject1", "issuer1", "audience1");
245+
ClientJwtCredential existingKey2 = new ClientJwtCredential("subject2", "issuer2", "audience2");
246+
ClientJwtConfiguration existingConfiguration = ClientJwtConfiguration.parse("https://other/jwks-uri");
247+
existingConfiguration.setClientJwtCredentials(List.of(existingKey1, existingKey2));
248+
assertThat(ClientJwtConfiguration.delete(existingConfiguration, new ClientJwtConfiguration(List.of(new ClientJwtCredential("not-existing", "not-existing", null))))).isEqualTo(existingConfiguration);
249+
assertThat(ClientJwtConfiguration.delete(existingConfiguration, new ClientJwtConfiguration(List.of(existingKey1))).getClientJwtCredentials()).hasSize(1);
250+
assertThat(ClientJwtConfiguration.delete(existingConfiguration, new ClientJwtConfiguration(List.of(existingKey1, existingKey2))).getClientJwtCredentials()).isNull();
251+
// complex deletion - given
252+
existingConfiguration = new ClientJwtConfiguration(List.of(existingKey1, existingKey2));
253+
existingConfiguration = ClientJwtConfiguration.merge(existingConfiguration, ClientJwtConfiguration.parse("https://other/jwks-uri"), false);
254+
// When
255+
existingConfiguration = ClientJwtConfiguration.delete(existingConfiguration, ClientJwtConfiguration.parse("https://other/jwks-uri"));
256+
// Then
257+
assertThat(existingConfiguration).isEqualTo(new ClientJwtConfiguration(List.of(existingKey1, existingKey2)));
258+
}
259+
228260
@Test
229261
void configDeleteNull() {
230262
assertThat(ClientJwtConfiguration.delete(null, ClientJwtConfiguration.parse("https://other/jwks-uri"))).isNull();
@@ -238,6 +270,7 @@ void testHashCode() {
238270
assertThat(key2.hashCode()).isNotEqualTo(key1.hashCode());
239271
assertThat(key1).hasSameHashCodeAs(key1);
240272
assertThat(key2).hasSameHashCodeAs(key2);
273+
assertThat(new ClientJwtCredential("subject", "issuer", "audience")).hasSameHashCodeAs(new ClientJwtCredential("subject", "issuer", "audience"));
241274
}
242275

243276
@Test

0 commit comments

Comments
 (0)