Merge remote-tracking branch 'refs/remotes/origin/develop' into fix/2…

…821/addExternId
cloudfoundry · Jun 26, 2024 · bf64ffb · bf64ffb
2 parents 2c38623 + 1308dde
commit bf64ffb
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 7 deletions.
diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml
@@ -12,7 +12,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        database: [ 'postgresql-11', 'postgresql-15', 'mysql', 'mysql-8' ]
+        database: [ 'postgresql-11', 'postgresql-15', 'postgresql-16', 'mysql', 'mysql-8' ]
     container:
       image: cfidentity/uaa-${{ matrix.database }}
       volumes:

diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml
@@ -12,7 +12,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        database: [ 'postgresql-11', 'postgresql-15', 'mysql', 'mysql-8' ]
+        database: [ 'postgresql-11', 'postgresql-15', 'postgresql-16', 'mysql', 'mysql-8' ]
     container:
       image: cfidentity/uaa-${{ matrix.database }}
       volumes:

diff --git a/dependencies.gradle b/dependencies.gradle
@@ -12,7 +12,7 @@ versions.springBootVersion = "2.7.18"
 versions.springFrameworkVersion = "5.3.37"
 versions.springSecurityVersion = "5.8.13"
 versions.springSecuritySamlVersion = "1.0.10.RELEASE"
-versions.tomcatCargoVersion = "9.0.89"
+versions.tomcatCargoVersion = "9.0.90"
 versions.guavaVersion = "33.2.1-jre"
 versions.seleniumVersion = "4.18.1"
 versions.braveVersion = "6.0.3"

diff --git a/...java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java b/...java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java
@@ -480,8 +480,10 @@ protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequ
             userFromDb = new UaaUser(getUserDatabase().retrieveUserPrototypeById(invitedUserId));
         }
 
+        boolean isRegisteredIdpAuthentication = isRegisteredIdpAuthentication(request);
+
         //we must check and see if the email address has changed between authentications
-        if (haveUserAttributesChanged(userFromDb, userFromRequest) && isRegisteredIdpAuthentication(request)) {
+        if (haveUserAttributesChanged(userFromDb, userFromRequest) && isRegisteredIdpAuthentication) {
             logger.debug("User attributed have changed, updating them.");
             userFromDb = userFromDb.modifyAttributes(email,
                                                      userFromRequest.getGivenName(),
@@ -493,8 +495,10 @@ protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequ
             userModified = true;
         }
 
-        ExternalGroupAuthorizationEvent event = new ExternalGroupAuthorizationEvent(userFromDb, userModified, userFromRequest.getAuthorities(), true);
-        publish(event);
+        if (isRegisteredIdpAuthentication) {
+            ExternalGroupAuthorizationEvent event = new ExternalGroupAuthorizationEvent(userFromDb, userModified, userFromRequest.getAuthorities(), true);
+            publish(event);
+        }
         return getUserDatabase().retrieveUserById(userFromDb.getId());
     }
 

diff --git a/...rc/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java b/...rc/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java
@@ -88,7 +88,7 @@ private AbstractExternalOAuthIdentityProviderDefinition getExternalOIDCIdentityP
         oidcIdentityProviderDefinition.setUserInfoUrl(idpDefinitionMap.get("userInfoUrl") == null ? null : new URL((String) idpDefinitionMap.get("userInfoUrl")));
         oidcIdentityProviderDefinition.setPasswordGrantEnabled(
             idpDefinitionMap.get("passwordGrantEnabled") == null ? false : (boolean) idpDefinitionMap.get("passwordGrantEnabled"));
-        oidcIdentityProviderDefinition.setSetForwardHeader(idpDefinitionMap.get("setForwardHeader") == null ? false : (boolean) idpDefinitionMap.get("passwordGrantEnabled"));
+        oidcIdentityProviderDefinition.setSetForwardHeader(idpDefinitionMap.get("setForwardHeader") == null ? false : (boolean) idpDefinitionMap.get("setForwardHeader"));
         oidcIdentityProviderDefinition.setPrompts((List<Prompt>) idpDefinitionMap.get("prompts"));
         setJwtClientAuthentication(idpDefinitionMap, oidcIdentityProviderDefinition);
         oauthIdpDefinitions.put(alias, oidcIdentityProviderDefinition);

diff --git a/...va/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/...va/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java
@@ -7,6 +7,7 @@
 import org.apache.commons.lang3.RandomStringUtils;
 import org.cloudfoundry.identity.uaa.authentication.AccountNotPreCreatedException;
 import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
+import org.cloudfoundry.identity.uaa.authentication.event.IdentityProviderAuthenticationSuccessEvent;
 import org.cloudfoundry.identity.uaa.authentication.manager.ExternalGroupAuthorizationEvent;
 import org.cloudfoundry.identity.uaa.authentication.manager.InvitedUserAuthenticatedEvent;
 import org.cloudfoundry.identity.uaa.authentication.manager.NewUserAuthenticatedEvent;
@@ -917,6 +918,37 @@ void updateShadowUser_IfAlreadyExists() {
         assertEquals(OriginKeys.UAA, uaaUser.getZoneId());
     }
 
+    @Test
+    void publishExternalGroupAuthorizationEvent_skippedIf_notIsRegisteredIdpAuthentication() {
+        claims.put("user_name", "12345");
+        claims.put("origin", "the_origin");
+        claims.put("iss", UAA_ISSUER_URL);
+
+        UaaUser existingShadowUser = new UaaUser(new UaaUserPrototype()
+                .withUsername("12345")
+                .withPassword("")
+                .withEmail("marissa_old@bloggs.com")
+                .withGivenName("Marissa_Old")
+                .withFamilyName("Bloggs_Old")
+                .withId("user-id")
+                .withOrigin("the_origin")
+                .withZoneId("uaa")
+                .withAuthorities(UaaAuthority.USER_AUTHORITIES));
+
+        userDatabase.addUser(existingShadowUser);
+
+        CompositeToken token = getCompositeAccessToken();
+        String idToken = token.getIdTokenValue();
+        xCodeToken = new ExternalOAuthCodeToken(null, null, null, idToken, null, null);
+
+        externalOAuthAuthenticationManager.authenticate(xCodeToken);
+
+        ArgumentCaptor<ApplicationEvent> userArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class);
+        verify(publisher, times(1)).publishEvent(userArgumentCaptor.capture());
+        assertEquals(1, userArgumentCaptor.getAllValues().size());
+        assertTrue(userArgumentCaptor.getAllValues().get(0) instanceof IdentityProviderAuthenticationSuccessEvent);
+    }
+
     @Test
     void invitedUser_becomesVerifiedOnAccept() {
         setUpInvitedUser();

diff --git a/...udfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java b/...udfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java
@@ -17,10 +17,12 @@
 
 import org.cloudfoundry.identity.uaa.constants.ClientAuthentication;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
+import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
 import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -300,4 +302,34 @@ public void testAuthMethodSet() {
         assertNotNull(((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getJwtClientAuthentication());
         assertEquals("none", (((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getAuthMethod()));
     }
+
+    /* The following two test cases check whether different values for 'setForwardHeader' and 'passwordGrantEnabled' are
+     * allowed. Due to a copy/paste issue, the value of 'setForwardHeader' was previously always set to the same value
+     * as 'passwordGrantEnabled'. */
+    @Test
+    public void testSetForwardHeader_ShouldAllowValuesDifferentFromPasswordGrantEnabled_True() {
+        testSetForwardHeader_ShouldAllowValuesDifferentFromPasswordGrantEnabled(true);
+    }
+
+    @Test
+    public void testSetForwardHeader_ShouldAllowValuesDifferentFromPasswordGrantEnabled_False() {
+        testSetForwardHeader_ShouldAllowValuesDifferentFromPasswordGrantEnabled(false);
+    }
+
+    private void testSetForwardHeader_ShouldAllowValuesDifferentFromPasswordGrantEnabled(
+            final boolean setForwardHeader
+    ) {
+        idpDefinitionMap.put("setForwardHeader", setForwardHeader);
+        idpDefinitionMap.put("passwordGrantEnabled", !setForwardHeader);
+        idpDefinitionMap.put("type", OriginKeys.OIDC10);
+
+        factoryBean = new OauthIDPWrapperFactoryBean(Collections.singletonMap("new.idp", idpDefinitionMap));
+        factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition);
+
+        final IdentityProvider provider = factoryBean.getProviders().get(0).getProvider();
+        assertTrue(provider.getConfig() instanceof OIDCIdentityProviderDefinition);
+        final OIDCIdentityProviderDefinition providerConfig = (OIDCIdentityProviderDefinition) provider.getConfig();
+        assertEquals(setForwardHeader, providerConfig.isSetForwardHeader());
+        assertEquals(!setForwardHeader, providerConfig.isPasswordGrantEnabled());
+    }
 }