From 44318c0e0e7f67dfa2a27d2f23da72462d735ec5 Mon Sep 17 00:00:00 2001 From: oharsta Date: Fri, 13 Nov 2015 08:44:29 +0100 Subject: [PATCH] Bugfix for multiple client redirects in UUIDPairwiseIdentiferService When a client has multiple redirect URIs, no configured sectorIdentifierUri and the subject type is PAIRWISE then the UUIDPairwiseIdentiferService#getIdentifier should not fail, but instead use the first redirect URI for calculating the sector identifier. Fixes#969 --- .gitignore | 1 + .../service/impl/UUIDPairwiseIdentiferService.java | 9 +++++---- .../service/impl/TestUUIDPairwiseIdentiferService.java | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index cc7301fd52..8d0653295f 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ bin /target .springBeans nb-configuration.xml +.java-version diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/UUIDPairwiseIdentiferService.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/UUIDPairwiseIdentiferService.java index 6a6796199b..5f9099cad6 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/UUIDPairwiseIdentiferService.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/service/impl/UUIDPairwiseIdentiferService.java @@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.util.CollectionUtils; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; @@ -56,13 +57,13 @@ public class UUIDPairwiseIdentiferService implements PairwiseIdentiferService { public String getIdentifier(UserInfo userInfo, ClientDetailsEntity client) { String sectorIdentifier = null; + Set redirectUris = client.getRedirectUris(); if (!Strings.isNullOrEmpty(client.getSectorIdentifierUri())) { UriComponents uri = UriComponentsBuilder.fromUriString(client.getSectorIdentifierUri()).build(); sectorIdentifier = uri.getHost(); // calculate based on the host component only - } else { - Set redirectUris = client.getRedirectUris(); - UriComponents uri = UriComponentsBuilder.fromUriString(Iterables.getOnlyElement(redirectUris)).build(); + } else if (!CollectionUtils.isEmpty(redirectUris)) { + UriComponents uri = UriComponentsBuilder.fromUriString(redirectUris.iterator().next()).build(); sectorIdentifier = uri.getHost(); // calculate based on the host of the only redirect URI } @@ -83,7 +84,7 @@ public String getIdentifier(UserInfo userInfo, ClientDetailsEntity client) { return pairwise.getIdentifier(); } else { - + logger.warn("Could not calculate a pairwise subject identifier for userInfo {} and client {}", userInfo.getPreferredUsername(), client.getClientId()); return null; } } diff --git a/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestUUIDPairwiseIdentiferService.java b/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestUUIDPairwiseIdentiferService.java index 46fa42c9a2..a0d088a729 100644 --- a/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestUUIDPairwiseIdentiferService.java +++ b/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestUUIDPairwiseIdentiferService.java @@ -41,6 +41,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; /** * @author jricher @@ -192,9 +193,10 @@ public void testGetIdentifer_unique() { } - @Test(expected = IllegalArgumentException.class) - public void testGetIdentifier_multipleRedirectError() { + @Test + public void testGetIdentifier_multipleRedirects() { String pairwise5 = service.getIdentifier(userInfoRegular, pairwiseClient5); + UUID.fromString(pairwise5); } }