From 5f33f96ca8c4b888df128d0d4ebb92397a846959 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Tue, 19 Dec 2023 08:57:24 -0600 Subject: [PATCH 1/2] Sub domains should not match --- .../oauth/security/OrcidOauthRedirectResolver.java | 2 +- .../security/OrcidOauthRedirectResolverTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/orcid-core/src/main/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolver.java b/orcid-core/src/main/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolver.java index 63b4fbcc37..4c790a3ad9 100644 --- a/orcid-core/src/main/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolver.java +++ b/orcid-core/src/main/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolver.java @@ -91,6 +91,6 @@ private boolean isEqual(String str1, String str2) { @Override protected boolean hostMatches(String registered, String requested) { - return isEqual(registered, requested) || (requested != null && requested.endsWith("." + registered)); + return isEqual(registered, requested); } } diff --git a/orcid-core/src/test/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolverTest.java b/orcid-core/src/test/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolverTest.java index d11533c2b9..3684a5643f 100644 --- a/orcid-core/src/test/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolverTest.java +++ b/orcid-core/src/test/java/org/orcid/core/oauth/security/OrcidOauthRedirectResolverTest.java @@ -218,14 +218,14 @@ private void redirectUriGeneralTest() { @Test public void redirectMatches_AllowMatchingSubdomainsTest() { - // Temp: Subdomain should match if the togglz is OFF - assertTrue(resolver.redirectMatches("https://www.orcid.org", "https://orcid.org")); - assertTrue(resolver.redirectMatches("https://qa.orcid.org", "https://orcid.org")); + // Subdomain should not match + assertFalse(resolver.redirectMatches("https://www.orcid.org", "https://orcid.org")); + assertFalse(resolver.redirectMatches("https://qa.orcid.org", "https://orcid.org")); - // Acceptance criteria checks: These should pass when the togglz is OFF - assertTrue(resolver.redirectMatches("https://subdomain.example.com/", "https://example.com")); - assertTrue(resolver.redirectMatches("https://subdomain.example.com/subdirectory", "https://example.com")); - assertTrue(resolver.redirectMatches("https://www.example.com", "https://example.com")); + // Acceptance criteria checks: subdomains should be rejected + assertFalse(resolver.redirectMatches("https://subdomain.example.com/", "https://example.com")); + assertFalse(resolver.redirectMatches("https://subdomain.example.com/subdirectory", "https://example.com")); + assertFalse(resolver.redirectMatches("https://www.example.com", "https://example.com")); } } From 6cf9d2907f367a53da2cbc882d999a4084f40529 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Tue, 19 Dec 2023 10:27:03 -0600 Subject: [PATCH 2/2] Use random number generator instead of time millis --- .../OrcidRefreshTokenTokenGranterTest.java | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/orcid-core/src/test/java/org/orcid/core/oauth/OrcidRefreshTokenTokenGranterTest.java b/orcid-core/src/test/java/org/orcid/core/oauth/OrcidRefreshTokenTokenGranterTest.java index 6ed76a3fc1..7b643c77a5 100644 --- a/orcid-core/src/test/java/org/orcid/core/oauth/OrcidRefreshTokenTokenGranterTest.java +++ b/orcid-core/src/test/java/org/orcid/core/oauth/OrcidRefreshTokenTokenGranterTest.java @@ -11,6 +11,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Random; import java.util.Set; import javax.annotation.Resource; @@ -42,6 +43,8 @@ public class OrcidRefreshTokenTokenGranterTest extends DBUnitTest { private static final String CLIENT_ID_2 = "APP-5555555555555556"; private static final String USER_ORCID = "0000-0000-0000-0001"; + private Random random = new Random(System.currentTimeMillis()); + @Resource private OrcidOauth2TokenDetailService orcidOauth2TokenDetailService; @@ -119,8 +122,8 @@ public void createRefreshTokenTest() { // should be equal long time = System.currentTimeMillis(); String scope = "/activities/update"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = null; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -154,8 +157,8 @@ public void createRefreshTokenWithNarrowerScopesTest() { long time = System.currentTimeMillis(); String parentScope = "/activities/update"; String refreshScope = "/orcid-works/create"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -188,8 +191,8 @@ public void createRefreshTokenWithoutRevokeParent() { // should be enabled, refresh should be enabled long time = System.currentTimeMillis(); String parentScope = "/activities/update /read-limited"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = false; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -228,8 +231,8 @@ public void createRefreshTokenWithoutRevokeParentAndWithNarrowerScopes() { long time = System.currentTimeMillis(); String parentScope = "/person/read-limited"; String refreshScope = "/orcid-bio/read-limited"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = false; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -262,8 +265,8 @@ public void createRefreshTokenWithExpirationOf10Secs() { long time = System.currentTimeMillis(); String parentScope = "/person/read-limited"; String refreshScope = "/orcid-bio/read-limited"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = false; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = 5L; @@ -299,8 +302,8 @@ public void tryToCreateRefreshTokenWithInvalidScopesTest() { long time = System.currentTimeMillis(); String parentScope = "/person/update"; String refreshScope = "/orcid-works/read-limited"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -322,8 +325,8 @@ public void tryToCreateRefreshTokenWithThatExpireAfterParentTokenTest() { // token, fail long time = System.currentTimeMillis(); String parentScope = "/person/update"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = time + (15000); @@ -345,8 +348,8 @@ public void tryToCreateRefreshTokenWithInvalidClientTest() { // client # 2, fail long time = System.currentTimeMillis(); String parentScope = "/person/update"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -367,8 +370,8 @@ public void tryToCreateRefreshTokenWithInvalidClientTest() { public void tryToRefreshAnExpiredTokenTest() { long time = System.currentTimeMillis(); String parentScope = "/person/update"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time - 10000); Long expireIn = null; @@ -390,8 +393,8 @@ public void tryToCreateRefreshTokenWithInvalidRefreshTokenTest() { // fail long time = System.currentTimeMillis(); String parentScope = "/person/update"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time + 10000); Long expireIn = null; @@ -415,8 +418,8 @@ public void tryToCreateRefreshTokenWithInvalidParentTokenValueTest() { // value, fail long time = System.currentTimeMillis(); String parentScope = "/person/update"; - String tokenValue = "parent-token-" + time; - String refreshTokenValue = "refresh-token-" + time; + String tokenValue = "parent-token-" + random.nextLong(); + String refreshTokenValue = "refresh-token-" + random.nextLong(); Boolean revokeOld = true; Date parentTokenExpiration = new Date(time + 15000); Long expireIn = null; @@ -428,7 +431,7 @@ public void tryToCreateRefreshTokenWithInvalidParentTokenValueTest() { // We shouldn't care about the access token, it's not required and // shouldn't really be there. If the refresh token and client // credentials are good, we can generate the refresh token. - assertNotNull(refreshedToken); + assertNotNull(refreshedToken); } }