Update default redirect url matching to be more secure

scripts/boot/uaa.yml

@@ -375,3 +375,8 @@ oauth:
       - roles
       - user_attributes
       - uaa.offline_token
+
+uaa:
+  oauth:
+    redirect_uri:
+      allow_unsafe_matching: true

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/beans/RedirectResolverFactoryBean.java

@@ -12,7 +12,7 @@ public class RedirectResolverFactoryBean implements FactoryBean<RedirectResolver
     private final boolean allowUnsafeMatching;
 
     public RedirectResolverFactoryBean(
-            @Value("${uaa.oauth.redirect_uri.allow_unsafe_matching:true}") boolean allowUnsafeMatching
+            @Value("${uaa.oauth.redirect_uri.allow_unsafe_matching:false}") boolean allowUnsafeMatching
     ) {
         this.allowUnsafeMatching = allowUnsafeMatching;
     }

server/src/test/java/org/cloudfoundry/identity/uaa/oauth/beans/RedirectResolverFactoryBeanTest.java

@@ -1,6 +1,5 @@
 package org.cloudfoundry.identity.uaa.oauth.beans;
 
-import org.cloudfoundry.identity.uaa.oauth.provider.endpoint.DefaultRedirectResolver;
 import org.cloudfoundry.identity.uaa.oauth.provider.endpoint.RedirectResolver;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.FactoryBean;
@@ -18,12 +17,11 @@ void allowUnsafeMatching_shouldReturnLegacyRedirectResolver() throws Exception {
     }
 
     @Test
-    void disallowUnsafeMatching_shouldReturnSpringSecurityOauth2RedirectResolver_withDontMatchSubdomain() throws Exception {
+    void disallowUnsafeMatching_shouldReturnRedirectResolver_withDontMatchSubdomain() throws Exception {
         FactoryBean<RedirectResolver> factory = new RedirectResolverFactoryBean(false);
 
         RedirectResolver redirectResolver = factory.getObject();
-        assertThat(redirectResolver).isInstanceOf(DefaultRedirectResolver.class);
+        assertThat(redirectResolver).isInstanceOf(NormalizedRedirectResolver.class);
         assertThat(ReflectionTestUtils.getField(redirectResolver, "matchSubdomains")).isEqualTo(false);
     }
-
 }

uaa/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsEndpointMockMvcTests.java

@@ -28,6 +28,7 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.test.context.TestPropertySource;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -139,9 +140,15 @@ void inviteUserWithUserCredentials() throws Exception {
 
     @Nested
     @DefaultTestContext
+    @TestPropertySource(
+            properties = "uaa.oauth.redirect_uri.allow_unsafe_matching=true"
+    )
     @ExtendWith(ZoneSeederExtension.class)
     class WithOtherIdentityZone {
 
+        @Autowired // New mockMvc tied to the new web app context created by @TestPropertySource
+        protected MockMvc mockMvc;
+
         private ZoneSeeder zoneSeeder;
 
         @BeforeEach

uaa/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsEndpointMockMvcZonePathTests.java

@@ -23,7 +23,6 @@
 import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
-import org.cloudfoundry.identity.uaa.oauth.provider.ClientDetails;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning;
 import org.cloudfoundry.identity.uaa.zone.MultitenantJdbcClientDetailsService;
 import org.flywaydb.core.internal.util.StringUtils;
@@ -39,6 +38,7 @@
 import org.springframework.http.HttpMethod;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
 import org.springframework.web.context.WebApplicationContext;
@@ -254,9 +254,15 @@ void inviteUserWithUserCredentialsWithinZone(ZoneResolutionMode mode) throws Exc
 
     @Nested
     @DefaultTestContext
+    @TestPropertySource(
+            properties = "uaa.oauth.redirect_uri.allow_unsafe_matching=true"
+    )
     @ExtendWith(ZoneSeederExtension.class)
     class WithOtherIdentityZone {
 
+        @Autowired // New mockMvc tied to the new web app context created by @TestPropertySource
+        protected MockMvc mockMvc;
+
         private ZoneSeeder zoneSeeder;
 
         @BeforeEach
@@ -761,12 +767,6 @@ private static InvitationsResponse sendRequestWithTokenAndReturnResponse(WebAppl
                 mockMvc, token, subdomain, clientId, redirectUri, emails);
     }
 
-    private static void sendRequestWithToken(WebApplicationContext webApplicationContext, MockMvc mockMvc, String token, String clientId, String... emails) throws Exception {
-        InvitationsResponse response = sendRequestWithTokenAndReturnResponse(webApplicationContext, mockMvc, token, null, clientId, "example.com", emails);
-        assertThat(response.getNewInvites()).hasSameSizeAs(emails);
-        assertThat(response.getFailedInvites()).isEmpty();
-    }
-
     private static void assertResponseAndCodeCorrect(ExpiringCodeStore expiringCodeStore, String[] emails, String redirectUrl, IdentityZone zone, InvitationsResponse response, ClientDetails clientDetails) {
         String expectedLinkPrefix = (zone != null && StringUtils.hasText(zone.getSubdomain()))
                 ? "http://" + zone.getSubdomain() + ".localhost/invitations/accept"

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/oauth/AuthorizationPromptNoneEntryPointMockMvcTests.java

@@ -11,8 +11,8 @@
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.mock.web.MockHttpSession;
-import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.web.context.WebApplicationContext;
 
@@ -30,6 +30,7 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+@TestPropertySource(properties = "uaa.oauth.redirect_uri.allow_unsafe_matching=true")
 @DefaultTestContext
 class AuthorizationPromptNoneEntryPointMockMvcTests {
 
@@ -115,6 +116,7 @@ void silentAuthenticationIncludesSessionState() throws Exception {
 
             //we need to know session id when we are calculating session_state
             MockHttpSession session = new MockHttpSession(null, "12345") {
+                @Override
                 public String changeSessionId() {
                     return "12345";
                 }

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java

@@ -155,10 +155,14 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.request;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
-@TestPropertySource(properties = {"uaa.url=https://localhost:8080/uaa", "jwt.token.refresh.format=jwt"})
+@TestPropertySource(properties = {
+        "uaa.url=https://localhost:8080/uaa",
+        "jwt.token.refresh.format=jwt",
+        "uaa.oauth.redirect_uri.allow_unsafe_matching=true"
+})
 @DefaultTestContext
-// public for LimitedModeTokenMockMvcTests
-public class TokenMvcMockTests extends AbstractTokenMockMvcTests {
+public // NOSONAR public for LimitedModeTokenMockMvcTests
+class TokenMvcMockTests extends AbstractTokenMockMvcTests {
     private static final String BAD_SECRET = "badsecret";
     protected AlphanumericRandomValueStringGenerator generator = new AlphanumericRandomValueStringGenerator();
 

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockZonePathTests.java

@@ -160,11 +160,15 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.view;
 
-@TestPropertySource(properties = {"uaa.url=https://localhost:8080/uaa", "jwt.token.refresh.format=jwt"})
-// public for LimitedModeTokenMockMvcTests
+@TestPropertySource(properties = {
+        "uaa.url=https://localhost:8080/uaa",
+        "jwt.token.refresh.format=jwt",
+        "uaa.oauth.redirect_uri.allow_unsafe_matching=true"
+})
 @EnabledIfZonePathsEnabled
 @DefaultTestContext
-public class TokenMvcMockZonePathTests extends AbstractTokenMockMvcTests {
+public // NOSONAR public for LimitedModeTokenMockMvcTests
+class TokenMvcMockZonePathTests extends AbstractTokenMockMvcTests {
     private static final String BAD_SECRET = "badsecret";
     protected AlphanumericRandomValueStringGenerator generator = new AlphanumericRandomValueStringGenerator();
 

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/util/MockMvcUtils.java

@@ -351,8 +351,8 @@ public static InvitationsResponse sendRequestWithTokenAndReturnResponse(Applicat
         return JsonUtils.readValue(result.getResponse().getContentAsString(), InvitationsResponse.class);
     }
 
-    public static URL inviteUser(ApplicationContext context, MockMvc mockMvc, String email, String userInviteToken, String subdomain, String clientId, String expectedOrigin, String REDIRECT_URI) throws Exception {
-        InvitationsResponse response = sendRequestWithTokenAndReturnResponse(context, mockMvc, userInviteToken, subdomain, clientId, REDIRECT_URI, email);
+    public static URL inviteUser(ApplicationContext context, MockMvc mockMvc, String email, String userInviteToken, String subdomain, String clientId, String expectedOrigin, String redirectUri) throws Exception {
+        InvitationsResponse response = sendRequestWithTokenAndReturnResponse(context, mockMvc, userInviteToken, subdomain, clientId, redirectUri, email);
         assertThat(response.getNewInvites()).hasSize(1);
         assertThat(context.getBean(JdbcTemplate.class).queryForObject("SELECT origin FROM users WHERE username='" + email + "'", String.class)).isEqualTo(expectedOrigin);
         return response.getNewInvites().getFirst().getInviteLink();
@@ -1036,7 +1036,7 @@ public static String getUserOAuthAccessTokenAuthCode(MockMvc mockMvc, String cli
                 .param(TokenConstants.REQUEST_TOKEN_FORMAT, tokenFormat.getStringValue())
                 .param(OAuth2Utils.STATE, state)
                 .param(OAuth2Utils.CLIENT_ID, clientId)
-                .param(OAuth2Utils.REDIRECT_URI, "http://localhost/test");
+                .param(OAuth2Utils.REDIRECT_URI, "http://localhost:8080/test");
         if (StringUtils.hasText(scope)) {
             authRequest.param(OAuth2Utils.SCOPE, scope);
         }
@@ -1052,7 +1052,7 @@ public static String getUserOAuthAccessTokenAuthCode(MockMvc mockMvc, String cli
                 .param(OAuth2Utils.GRANT_TYPE, GRANT_TYPE_AUTHORIZATION_CODE)
                 .param("code", code)
                 .param(OAuth2Utils.CLIENT_ID, clientId)
-                .param(OAuth2Utils.REDIRECT_URI, "http://localhost/test");
+                .param(OAuth2Utils.REDIRECT_URI, "http://localhost:8080/test");
         if (StringUtils.hasText(scope)) {
             authRequest.param(OAuth2Utils.SCOPE, scope);
         }

uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationEndpointMockMvcTest.java

@@ -62,8 +62,14 @@ void setUp(ZoneSeeder zoneSeeder) {
 
     @Nested
     @DefaultTestContext
+    @TestPropertySource(
+            properties = "uaa.oauth.redirect_uri.allow_unsafe_matching=true"
+    )
     class WhenRedirectUriAllowUnsafeMatchingIsEnabled {
 
+        @Autowired // New mockMvc tied to the new web app context created by @TestPropertySource
+        protected MockMvc mockMvc;
+
         @Nested
         @DefaultTestContext
         class WhenConfiguredRedirectUriHasWildcards {
@@ -82,17 +88,6 @@ void shouldRedirect_whenItReliesOnLegacyWildcardBehavior() throws Exception {
                         .andExpect(status().isFound())
                         .andExpect(header().string("Location", startsWith("http://sample.com/a/b?code=")));
             }
-
-            @Test
-            void shouldRedirect_whenTheRequestRedirectUriIsAnExactMatch() throws Exception {
-                mockMvc.perform(implicitGrantAuthorizeRequest("http://sample.com/a/b"))
-                        .andExpect(status().isFound())
-                        .andExpect(header().string("Location", startsWith("http://sample.com/a/b#token_type=bearer&access_token=")));
-
-                mockMvc.perform(authCodeAuthorizeRequest("http://sample.com/a/b"))
-                        .andExpect(status().isFound())
-                        .andExpect(header().string("Location", startsWith("http://sample.com/a/b?code=")));
-            }
         }
 
         @Nested

uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationEndpointMockMvcZonePathTest.java

@@ -70,8 +70,14 @@ void setUp(ZoneSeeder zoneSeeder) {
 
     @Nested
     @DefaultTestContext
+    @TestPropertySource(
+            properties = "uaa.oauth.redirect_uri.allow_unsafe_matching=true"
+    )
     class WhenRedirectUriAllowUnsafeMatchingIsEnabled {
 
+        @Autowired // Need a new mockMvc which is tied to the new web app context created by @TestPropertySource
+        protected MockMvc mockMvc;
+
         @Nested
         @DefaultTestContext
         class WhenConfiguredRedirectUriHasWildcards {
@@ -90,17 +96,6 @@ void shouldRedirect_whenItReliesOnLegacyWildcardBehavior() throws Exception {
                         .andExpect(status().isFound())
                         .andExpect(header().string("Location", startsWith("http://sample.com/a/b?code=")));
             }
-
-            @Test
-            void shouldRedirect_whenTheRequestRedirectUriIsAnExactMatch() throws Exception {
-                mockMvc.perform(implicitGrantAuthorizeRequest("http://sample.com/a/b"))
-                        .andExpect(status().isFound())
-                        .andExpect(header().string("Location", startsWith("http://sample.com/a/b#token_type=bearer&access_token=")));
-
-                mockMvc.perform(authCodeAuthorizeRequest("http://sample.com/a/b"))
-                        .andExpect(status().isFound())
-                        .andExpect(header().string("Location", startsWith("http://sample.com/a/b?code=")));
-            }
         }
 
         @Nested
@@ -142,7 +137,7 @@ void shouldRedirect_whenTheRequestRedirectUriIsAnExactMatch() throws Exception {
     )
     class WhenRedirectUriAllowUnsafeMatchingIsDisabled {  // "spec-compliant" mode
 
-        @Autowired // Need a new mockMvc which is tied to the new web app context created by @TestPropertySource
+        @Autowired // New mockMvc tied to the new web app context created by @TestPropertySource
         protected MockMvc mockMvc;
 
         @Nested

uaa/src/test/resources/mockmvc_unittest_properties.yml

@@ -418,6 +418,9 @@ oauth:
       - uaa.offline_token
 
 uaa:
+  oauth:
+    redirect_uri:
+      allow_unsafe_matching: true
   # The hostname of the UAA that this login server will connect to
   url: http://localhost:8080/uaa
   token: