Skip to content

Commit 236c5cd

Browse files
committed
Changed as per PR comments
1 parent 3ca12dd commit 236c5cd

File tree

5 files changed

+42
-22
lines changed

5 files changed

+42
-22
lines changed

core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ public class Saml2Settings {
3030
*/
3131
private static final Logger LOGGER = LoggerFactory.getLogger(Saml2Settings.class);
3232

33-
public static final String DEFAULT_UNIQUE_ID_PREFIX = "ONELOGIN_";
34-
3533
// Toolkit settings
3634
private boolean strict = false;
3735
private boolean debug = false;
@@ -75,7 +73,7 @@ public class Saml2Settings {
7573
private String signatureAlgorithm = Constants.RSA_SHA1;
7674
private String digestAlgorithm = Constants.SHA1;
7775
private boolean rejectUnsolicitedResponsesWithInResponseTo = false;
78-
private String uniqueIDPrefix = DEFAULT_UNIQUE_ID_PREFIX;
76+
private String uniqueIDPrefix = null;
7977

8078
// Compress
8179
private boolean compressRequest = true;

core/src/main/java/com/onelogin/saml2/settings/SettingsBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ private String loadUniqueIDPrefix() {
390390
if (StringUtils.isNotEmpty(uniqueIDPrefix)) {
391391
return uniqueIDPrefix;
392392
} else {
393-
return Saml2Settings.DEFAULT_UNIQUE_ID_PREFIX;
393+
return Util.UNIQUE_ID_PREFIX;
394394
}
395395
}
396396

core/src/main/java/com/onelogin/saml2/util/Util.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ public final class Util {
105105

106106
private static final DateTimeFormatter DATE_TIME_FORMAT = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss'Z'").withZone(DateTimeZone.UTC);
107107
private static final DateTimeFormatter DATE_TIME_FORMAT_MILLS = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'").withZone(DateTimeZone.UTC);
108+
public static final String UNIQUE_ID_PREFIX = "ONELOGIN_";
108109
public static final String RESPONSE_SIGNATURE_XPATH = "/samlp:Response/ds:Signature";
109110
public static final String ASSERTION_SIGNATURE_XPATH = "/samlp:Response/saml:Assertion/ds:Signature";
110111
/** Indicates if JAXP 1.5 support has been detected. */
@@ -1537,11 +1538,19 @@ private static SecretKey generateSymmetricKey() throws Exception {
15371538
* @return A unique string
15381539
*/
15391540
public static String generateUniqueID(String prefix) {
1540-
if (StringUtils.isNotEmpty(prefix)) {
1541-
return prefix + UUID.randomUUID();
1542-
} else {
1543-
throw new IllegalArgumentException("Prefix cannot be null or empty.");
1541+
if (prefix == null || StringUtils.isEmpty(prefix)) {
1542+
prefix = Util.UNIQUE_ID_PREFIX;
15441543
}
1544+
return prefix + UUID.randomUUID();
1545+
}
1546+
1547+
/**
1548+
* Generates a unique string (used for example as ID of assertions)
1549+
*
1550+
* @return A unique string
1551+
*/
1552+
public static String generateUniqueID() {
1553+
return generateUniqueID(null);
15451554
}
15461555

15471556
/**

core/src/test/java/com/onelogin/saml2/test/util/UtilsTest.java

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1876,19 +1876,30 @@ public void testGenerateNameIdWithoutFormat() throws URISyntaxException, IOExcep
18761876
assertEquals(expectedNameId, nameId);
18771877
}
18781878

1879+
/**
1880+
* Tests the generateUniqueID method
1881+
*
1882+
* * @see com.onelogin.saml2.util.Util#generateUniqueID
1883+
*/
1884+
@Test
1885+
public void testGenerateUniqueID() {
1886+
String s1 = Util.generateUniqueID();
1887+
assertThat(s1, startsWith(Util.UNIQUE_ID_PREFIX));
1888+
}
1889+
18791890
/**
18801891
* Tests the generateUniqueID method
18811892
*
18821893
* @see com.onelogin.saml2.util.Util#generateUniqueID
18831894
*/
18841895
@Test
1885-
public void testGenerateUniqueID() {
1886-
String s1 = Util.generateUniqueID(Saml2Settings.DEFAULT_UNIQUE_ID_PREFIX);
1896+
public void testGenerateUniqueID_withCustomPrefix() {
1897+
String s1 = Util.generateUniqueID(Util.UNIQUE_ID_PREFIX);
18871898

1888-
assertThat(s1, startsWith(Saml2Settings.DEFAULT_UNIQUE_ID_PREFIX));
1899+
assertThat(s1, startsWith(Util.UNIQUE_ID_PREFIX));
18891900
assertTrue(s1.length() > 40);
18901901

1891-
String s2 = Util.generateUniqueID(Saml2Settings.DEFAULT_UNIQUE_ID_PREFIX);
1902+
String s2 = Util.generateUniqueID(Util.UNIQUE_ID_PREFIX);
18921903
String s3 = Util.generateUniqueID("_");
18931904
assertThat(s3, startsWith("_"));
18941905

@@ -1898,19 +1909,21 @@ public void testGenerateUniqueID() {
18981909
}
18991910

19001911
/**
1901-
* Tests that generateUniqueID method throws when given null
1912+
* Tests that generateUniqueID method uses default prefix when given null
19021913
*/
1903-
@Test(expected=IllegalArgumentException.class)
1904-
public void testGenerateUniqueID_throwsOnNullPrefix() {
1905-
Util.generateUniqueID(null);
1914+
@Test
1915+
public void testGenerateUniqueID_usesDefaultOnNull() {
1916+
String s1 = Util.generateUniqueID(null);
1917+
assertThat(s1, startsWith(Util.UNIQUE_ID_PREFIX));
19061918
}
19071919

19081920
/**
1909-
* Tests that generateUniqueID method throws when given empty String
1921+
* Tests that generateUniqueID method uses default prefix when given empty String
19101922
*/
1911-
@Test(expected=IllegalArgumentException.class)
1912-
public void testGenerateUniqueID_throwsOnEmptyPrefix() {
1913-
Util.generateUniqueID("");
1923+
@Test
1924+
public void testGenerateUniqueID_usesDefaultOnEmpty() {
1925+
String s1 = Util.generateUniqueID("");
1926+
assertThat(s1, startsWith(Util.UNIQUE_ID_PREFIX));
19141927
}
19151928

19161929
/**

toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,7 @@ public void testLogin() throws IOException, SettingsException, URISyntaxExceptio
10651065
Auth auth = new Auth(settings, request, response);
10661066
auth.login();
10671067
verify(response).sendRedirect(matches("https:\\/\\/pitbulk.no-ip.org\\/simplesaml\\/saml2\\/idp\\/SSOService.php\\?SAMLRequest=(.)*&RelayState=http%3A%2F%2Flocalhost%3A8080%2Finitial.jsp"));
1068-
assertThat(auth.getLastRequestId(), startsWith(Saml2Settings.DEFAULT_UNIQUE_ID_PREFIX));
1068+
assertThat(auth.getLastRequestId(), startsWith(Util.UNIQUE_ID_PREFIX));
10691069
}
10701070

10711071
/**
@@ -1251,7 +1251,7 @@ public void testLogout() throws IOException, SettingsException, XMLEntityExcepti
12511251
auth.logout();
12521252

12531253
verify(response).sendRedirect(matches("https:\\/\\/pitbulk.no-ip.org\\/simplesaml\\/saml2\\/idp\\/SingleLogoutService.php\\?SAMLRequest=(.)*&RelayState=http%3A%2F%2Flocalhost%3A8080%2Finitial.jsp"));
1254-
assertThat(auth.getLastRequestId(), startsWith(Saml2Settings.DEFAULT_UNIQUE_ID_PREFIX));
1254+
assertThat(auth.getLastRequestId(), startsWith(Util.UNIQUE_ID_PREFIX));
12551255
}
12561256

12571257
/**

0 commit comments

Comments
 (0)