diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java index 26103d3df38..6def53fc23e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java @@ -1,5 +1,8 @@ package org.cloudfoundry.identity.uaa.authentication.manager; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; import lombok.Getter; import lombok.Setter; import org.apache.commons.lang3.StringUtils; @@ -54,7 +57,7 @@ import static java.util.Collections.emptySet; -public abstract class ExternalLoginAuthenticationManager implements AuthenticationManager, ApplicationEventPublisherAware, BeanNameAware { +public abstract class ExternalLoginAuthenticationManager implements AuthenticationManager, ApplicationEventPublisherAware, BeanNameAware { public static final String USER_ATTRIBUTE_PREFIX = "user.attribute."; private static final String FALLBACK_EMAIL_DOMAIN_TEMPLATE = "user.from.%s.cf"; @@ -86,16 +89,18 @@ public final void setApplicationEventPublisher(@NonNull ApplicationEventPublishe this.eventPublisher = eventPublisher; } - public abstract String getOrigin(); - - public abstract void setOrigin(String origin); - @Override public Authentication authenticate(Authentication request) throws AuthenticationException { if (logger.isDebugEnabled()) { logger.debug("Starting external authentication for:{}", UaaStringUtils.getCleanedUserControlString(request.toString())); } - ExternalAuthenticationDetails authenticationData = getExternalAuthenticationDetails(request); + + EAD authenticationData = getExternalAuthenticationDetails(request); + if (authenticationData == null) { + return null; + } + final String origin = authenticationData.getOrigin(); + UaaUser userFromRequest = getUser(request, authenticationData); if (userFromRequest == null) { return null; @@ -104,28 +109,28 @@ public Authentication authenticate(Authentication request) throws Authentication UaaUser userFromDb; try { - logger.debug("Searching for user by (username:{} , origin:{})", userFromRequest.getUsername(), getOrigin()); - userFromDb = userDatabase.retrieveUserByName(userFromRequest.getUsername(), getOrigin()); + logger.debug("Searching for user by (username:{} , origin:{})", userFromRequest.getUsername(), origin); + userFromDb = userDatabase.retrieveUserByName(userFromRequest.getUsername(), origin); } catch (UsernameNotFoundException e) { - logger.debug("Searching for user by (email:{} , origin:{})", userFromRequest.getEmail(), getOrigin()); - userFromDb = userDatabase.retrieveUserByEmail(userFromRequest.getEmail(), getOrigin()); + logger.debug("Searching for user by (email:{} , origin:{})", userFromRequest.getEmail(), origin); + userFromDb = userDatabase.retrieveUserByEmail(userFromRequest.getEmail(), origin); } // Register new users automatically if (userFromDb == null) { - if (!isAddNewShadowUser()) { + if (!isAddNewShadowUser(origin)) { throw new AccountNotPreCreatedException("The user account must be pre-created. Please contact your system administrator."); } publish(new NewUserAuthenticatedEvent(userFromRequest.authorities(List.of()))); try { - userFromDb = userDatabase.retrieveUserByName(userFromRequest.getUsername(), getOrigin()); + userFromDb = userDatabase.retrieveUserByName(userFromRequest.getUsername(), origin); } catch (UsernameNotFoundException ex) { throw new BadCredentialsException("Unable to register user in internal UAA store."); } } //user is authenticated and exists in UAA - UaaUser user = userAuthenticated(request, userFromRequest, userFromDb); + UaaUser user = userAuthenticated(request, userFromRequest, userFromDb, authenticationData); UaaAuthenticationDetails uaaAuthenticationDetails; if (request.getDetails() instanceof UaaAuthenticationDetails) { @@ -139,10 +144,10 @@ public Authentication authenticate(Authentication request) throws Authentication return success; } - protected void populateAuthenticationAttributes(UaaAuthentication authentication, Authentication request, ExternalAuthenticationDetails authenticationData) { + protected void populateAuthenticationAttributes(UaaAuthentication authentication, Authentication request, EAD authenticationData) { if (request.getPrincipal() instanceof UserDetails userDetails) { - authentication.setUserAttributes(getUserAttributes(userDetails)); - authentication.setExternalGroups(new HashSet<>(getExternalUserAuthorities(userDetails))); + authentication.setUserAttributes(getUserAttributes(userDetails, authenticationData)); + authentication.setExternalGroups(new HashSet<>(getExternalUserAuthorities(userDetails, authenticationData))); } if (authentication.getAuthenticationMethods() == null) { @@ -153,7 +158,7 @@ protected void populateAuthenticationAttributes(UaaAuthentication authentication // persist the user attributes and external groups in the user info table if configured in the IdP if ((hasUserAttributes(authentication) || hasExternalGroups(authentication)) && getProviderProvisioning() != null) { - IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId()); + IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(authenticationData.getOrigin(), IdentityZoneHolder.get().getId()); if (provider.getConfig() != null && provider.getConfig().isStoreCustomAttributes()) { logger.debug("Storing custom attributes for user_id:{}", authentication.getPrincipal().getId()); UserInfo userInfo = new UserInfo() @@ -172,15 +177,15 @@ private boolean hasUserAttributes(UaaAuthentication authentication) { return authentication.getUserAttributes() != null && !authentication.getUserAttributes().isEmpty(); } - protected abstract ExternalAuthenticationDetails getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException; + protected abstract EAD getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException; - protected abstract boolean isAddNewShadowUser(); + protected abstract boolean isAddNewShadowUser(final String origin); - protected MultiValueMap getUserAttributes(UserDetails request) { + protected MultiValueMap getUserAttributes(UserDetails request, EAD authenticationData) { return new LinkedMultiValueMap<>(); } - protected abstract List getExternalUserAuthorities(UserDetails request); + protected abstract List getExternalUserAuthorities(UserDetails request, EAD authenticationData); protected final void publish(ApplicationEvent event) { if (eventPublisher != null) { @@ -188,9 +193,9 @@ protected final void publish(ApplicationEvent event) { } } - protected abstract UaaUser userAuthenticated(Authentication request, UaaUser userFromRequest, UaaUser userFromDb); + protected abstract UaaUser userAuthenticated(Authentication request, UaaUser userFromRequest, UaaUser userFromDb, EAD authenticationData); - protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails authDetails) { + protected UaaUser getUser(Authentication request, EAD authDetails) { UserDetails userDetails; if (request.getPrincipal() instanceof UserDetails) { userDetails = (UserDetails) request.getPrincipal(); @@ -219,7 +224,7 @@ protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails } if (UaaStringUtils.isEmpty(email)) { - email = generateEmailIfNullOrEmpty(name); + email = generateEmailIfNullOrEmpty(name, authDetails.getOrigin()); } String givenName = null; @@ -242,7 +247,7 @@ protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails .withFamilyName(familyName) .withCreated(new Date()) .withModified(new Date()) - .withOrigin(getOrigin()) + .withOrigin(authDetails.getOrigin()) .withExternalId(externalId) .withZoneId(IdentityZoneHolder.get().getId()) .withPhoneNumber(phoneNumber); @@ -250,12 +255,12 @@ protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails return new UaaUser(userPrototype); } - protected final String generateEmailIfNullOrEmpty(String name) { + protected static String generateEmailIfNullOrEmpty(final String name, final String origin) { if (name == null) { throw new BadCredentialsException("Cannot determine username from credentials supplied"); } - final String fallbackEmailDomain = FALLBACK_EMAIL_DOMAIN_TEMPLATE.formatted(getOrigin()); + final String fallbackEmailDomain = FALLBACK_EMAIL_DOMAIN_TEMPLATE.formatted(origin); // use fallback domain if no '@' is present if (!name.contains("@")) { @@ -310,4 +315,23 @@ protected final List evaluateExternalGroupMappings(Strin public void setBeanName(@NonNull String name) { this.name = name; } + + @Data + @Builder + @AllArgsConstructor + public static class ExternalAuthenticationDetails { + private String origin; + + public ExternalAuthenticationDetails() { + this.origin = "unknown"; + } + + public final String getOrigin() { + return origin; + } + + public final void setOrigin(final String origin) { + this.origin = origin; + } + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java index 1be6a778dfe..c485f418504 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java @@ -15,7 +15,9 @@ package org.cloudfoundry.identity.uaa.authentication.manager; +import com.google.common.annotations.VisibleForTesting; import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication; +import org.cloudfoundry.identity.uaa.authentication.manager.ExternalLoginAuthenticationManager.ExternalAuthenticationDetails; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; @@ -46,44 +48,28 @@ import static java.util.Collections.emptyList; import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.retainAllMatches; -public class LdapLoginAuthenticationManager extends ExternalLoginAuthenticationManager { +public class LdapLoginAuthenticationManager extends ExternalLoginAuthenticationManager { protected static Logger logger = LoggerFactory.getLogger(LdapLoginAuthenticationManager.class); - public LdapLoginAuthenticationManager(final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning providerProvisioning) { - super(providerProvisioning); - } - private String origin = OriginKeys.LDAP; - @Override - public String getOrigin() { - return origin; - } - - @Override - public void setOrigin(String origin) { - // only used in LdapLoginAuthenticationManagerTests - this.origin = origin; + public LdapLoginAuthenticationManager(final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning providerProvisioning) { + super(providerProvisioning); } @Override - protected void populateAuthenticationAttributes(UaaAuthentication authentication, Authentication request, Object authenticationData) { + protected void populateAuthenticationAttributes(UaaAuthentication authentication, Authentication request, ExternalAuthenticationDetails authenticationData) { super.populateAuthenticationAttributes(authentication, request, authenticationData); authentication.getAuthenticationMethods().add("pwd"); } @Override - protected Object getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException { - return null; - } - - @Override - protected MultiValueMap getUserAttributes(UserDetails request) { - MultiValueMap result = super.getUserAttributes(request); - logger.debug("Mapping custom attributes for origin:{} and zone:{}", getOrigin(), IdentityZoneHolder.get().getId()); + protected MultiValueMap getUserAttributes(UserDetails request, ExternalAuthenticationDetails authenticationData) { + MultiValueMap result = super.getUserAttributes(request, authenticationData); + logger.debug("Mapping custom attributes for origin:{} and zone:{}", authenticationData.getOrigin(), IdentityZoneHolder.get().getId()); if (getProviderProvisioning() != null) { - IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId()); + IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(authenticationData.getOrigin(), IdentityZoneHolder.get().getId()); if (request instanceof ExtendedLdapUserDetails ldapDetails) { LdapIdentityProviderDefinition ldapIdentityProviderDefinition = ObjectUtils.castInstance(provider.getConfig(), LdapIdentityProviderDefinition.class); Map providerMappings = ldapIdentityProviderDefinition.getAttributeMappings(); @@ -99,16 +85,16 @@ protected MultiValueMap getUserAttributes(UserDetails request) { } } } else { - logger.debug("Did not find custom attribute configuration for origin:{} and zone:{}", getOrigin(), IdentityZoneHolder.get().getId()); + logger.debug("Did not find custom attribute configuration for origin:{} and zone:{}", authenticationData.getOrigin(), IdentityZoneHolder.get().getId()); } return result; } @Override - protected List getExternalUserAuthorities(UserDetails request) { + protected List getExternalUserAuthorities(UserDetails request, ExternalAuthenticationDetails authenticationData) { List result = new LinkedList<>(); if (getProviderProvisioning() != null) { - IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId()); + IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(authenticationData.getOrigin(), IdentityZoneHolder.get().getId()); LdapIdentityProviderDefinition ldapIdentityProviderDefinition = ObjectUtils.castInstance(provider.getConfig(), LdapIdentityProviderDefinition.class); List externalWhiteList = ldapIdentityProviderDefinition.getExternalGroupsWhitelist(); result = new ArrayList<>(retainAllMatches(getAuthoritiesAsNames(request.getAuthorities()), externalWhiteList)); @@ -131,7 +117,7 @@ protected Set getAuthoritiesAsNames(Collection origin = ThreadLocal.withInitial(() -> "unknown"); - public ExternalOAuthAuthenticationManager( IdentityProviderProvisioning providerProvisioning, IdentityZoneManager identityZoneManager, @@ -168,27 +166,6 @@ public ExternalOAuthAuthenticationManager( this.oidcMetadataFetcher = oidcMetadataFetcher; } - @Override - public String getOrigin() { - //origin is per thread during execution - return origin.get(); - } - - @Override - public void setOrigin(String origin) { - this.origin.set(origin); - } - - @Override - public Authentication authenticate(final Authentication request) throws AuthenticationException { - try { - return super.authenticate(request); - } finally { - // clear ThreadLocal holding the origin key - origin.remove(); - } - } - /** * Resolve the IdP trust for the received ID token: *
    @@ -265,10 +242,11 @@ protected AuthenticationData getExternalAuthenticationDetails(final Authenticati codeToken.setOrigin(provider.getOriginKey()); } - setOrigin(codeToken.getOrigin()); + final String origin = codeToken.getOrigin(); + if (provider == null) { try { - provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), identityZoneManager.getCurrentIdentityZoneId()); + provider = getProviderProvisioning().retrieveByOrigin(origin, identityZoneManager.getCurrentIdentityZoneId()); } catch (EmptyResultDataAccessException e) { logger.info("No provider found for given origin"); throw new InsufficientAuthenticationException("Could not resolve identity provider with given origin."); @@ -277,6 +255,7 @@ protected AuthenticationData getExternalAuthenticationDetails(final Authenticati if (provider != null && provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition config) { final AuthenticationData authenticationData = new AuthenticationData(); + authenticationData.setOrigin(origin); final Map claims = getClaimsFromToken(codeToken, provider); @@ -322,7 +301,7 @@ protected AuthenticationData getExternalAuthenticationDetails(final Authenticati return authenticationData; } - logger.debug("No identity provider found for origin:{} and zone:{}", getOrigin(), identityZoneManager.getCurrentIdentityZoneId()); + logger.debug("No identity provider found for origin:{} and zone:{}", origin, identityZoneManager.getCurrentIdentityZoneId()); return null; } @@ -435,7 +414,7 @@ protected void populateAuthenticationAttributes(UaaAuthentication authentication } @Override - protected List getExternalUserAuthorities(UserDetails request) { + protected List getExternalUserAuthorities(UserDetails request, AuthenticationData authenticationData) { return new LinkedList<>(); } @@ -460,7 +439,7 @@ protected UaaUser getUser(Authentication request, AuthenticationData authenticat boolean verified = verifiedObj instanceof Boolean b ? b : false; if (!StringUtils.hasText(email)) { - email = generateEmailIfNullOrEmpty(username); + email = generateEmailIfNullOrEmpty(username, authenticationData.getOrigin()); } log.debug("Returning user data for username:{}, email:{}", username, email); @@ -476,7 +455,7 @@ protected UaaUser getUser(Authentication request, AuthenticationData authenticat .withPassword("") .withAuthorities(authenticationData.getAuthorities()) .withCreated(new Date()) - .withOrigin(getOrigin()) + .withOrigin(authenticationData.getOrigin()) .withExternalId((String) authenticationData.getClaims().get(SUB)) .withVerified(verified) .withZoneId(identityZoneManager.getCurrentIdentityZoneId()) @@ -537,7 +516,7 @@ private List extractExternalOAuthUserAuthorities(Map provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), identityZoneManager.getCurrentIdentityZoneId()); + protected boolean isAddNewShadowUser(final String origin) { + IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(origin, identityZoneManager.getCurrentIdentityZoneId()); return provider.getConfig().isAddShadowUserOnLogin(); } @@ -1034,7 +1013,8 @@ public String oauthTokenRequest(UaaAuthenticationDetails details, final Identity } @Data - protected static class AuthenticationData { + @EqualsAndHashCode(callSuper = true) + protected static class AuthenticationData extends ExternalAuthenticationDetails { private Map claims; private String username; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java index 8b19c36a5b1..8d758848790 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java @@ -5,6 +5,7 @@ import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; import org.cloudfoundry.identity.uaa.authentication.event.IdentityProviderAuthenticationSuccessEvent; +import org.cloudfoundry.identity.uaa.authentication.manager.ExternalLoginAuthenticationManager.ExternalAuthenticationDetails; import org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; @@ -60,8 +61,8 @@ class ExternalLoginAuthenticationManagerTest { private ApplicationEventPublisher applicationEventPublisher; private UaaUserDatabase uaaUserDatabase; private Authentication inputAuth; - private ExternalLoginAuthenticationManager manager; - private final String origin = "test"; + private ExternalLoginAuthenticationManager manager; + private String origin = "test"; private UserDetails userDetails; private final String userName = "testUserName"; private final String password = ""; @@ -103,36 +104,24 @@ private void mockUaaWithUser() { inputAuth = mock(Authentication.class); when(inputAuth.getPrincipal()).thenReturn(userDetails); - manager = new ExternalLoginAuthenticationManager(null) { - private String origin = "unknown"; - - @Override - public String getOrigin() { - return origin; - } - + manager = new ExternalLoginAuthenticationManager<>(null) { @Override - public void setOrigin(String origin) { - this.origin = origin; + protected ExternalAuthenticationDetails getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException { + return ExternalAuthenticationDetails.builder().origin(origin).build(); } @Override - protected Object getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException { - return null; - } - - @Override - protected boolean isAddNewShadowUser() { + protected boolean isAddNewShadowUser(String origin) { return true; } @Override - protected List getExternalUserAuthorities(UserDetails request) { + protected List getExternalUserAuthorities(UserDetails request, ExternalAuthenticationDetails authenticationDetails) { return new LinkedList<>(); } @Override - protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequest, UaaUser userFromDb) { + protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequest, UaaUser userFromDb, ExternalAuthenticationDetails authenticationDetails) { return userFromDb; } }; @@ -152,7 +141,6 @@ private UaaUser addUserToDb(String userName, String userId, String origin, Strin } private void setupManager() { - manager.setOrigin(origin); String beanName = "ExternalLoginAuthenticationManagerTestBean"; manager.setBeanName(beanName); manager.setApplicationEventPublisher(applicationEventPublisher); @@ -353,7 +341,6 @@ void authenticateLdapUserDetailsPrincipal() { manager = new LdapLoginAuthenticationManager(null); setupManager(); manager.setProviderProvisioning(null); - manager.setOrigin(origin); when(user.getOrigin()).thenReturn(origin); when(uaaUserDatabase.retrieveUserByName(eq(userName), eq(origin))).thenReturn(user); when(inputAuth.getPrincipal()).thenReturn(ldapUserDetails); @@ -376,13 +363,12 @@ void shadowUserCreationDisabled() { when(ldapUserDetails.getDn()).thenReturn(dn); manager = new LdapLoginAuthenticationManager(null) { @Override - protected boolean isAddNewShadowUser() { + protected boolean isAddNewShadowUser(final String origin) { return false; } }; setupManager(); - manager.setOrigin(origin); when(uaaUserDatabase.retrieveUserByName(eq(userName), eq(origin))).thenReturn(null); when(inputAuth.getPrincipal()).thenReturn(ldapUserDetails); @@ -414,7 +400,6 @@ void authenticateCreateUserWithLdapUserDetailsPrincipal() { manager = new LdapLoginAuthenticationManager(null); setupManager(); manager.setProviderProvisioning(null); - manager.setOrigin(origin); when(user.getEmail()).thenReturn(email); when(user.getOrigin()).thenReturn(origin); when(user.getExternalId()).thenReturn(dn); @@ -445,7 +430,6 @@ void authenticateCreateUserWithUserDetailsPrincipal() { manager = new LdapLoginAuthenticationManager(null); setupManager(); - manager.setOrigin(origin); manager.setProviderProvisioning(null); when(user.getOrigin()).thenReturn(origin); @@ -495,7 +479,6 @@ void authenticateInvitedUserWithoutAcceptance() { manager = new LdapLoginAuthenticationManager(null); setupManager(); manager.setProviderProvisioning(null); - manager.setOrigin(origin); when(uaaUserDatabase.retrieveUserByName(eq(username), eq(origin))) .thenThrow(new UsernameNotFoundException("")); @@ -519,7 +502,6 @@ void authenticateInvitedUserWithoutAcceptance() { void populateAttributesStoresCustomAttributesAndRoles() { manager = new LdapLoginAuthenticationManager(null); setupManager(); - manager.setOrigin(origin); IdentityProvider provider = mock(IdentityProvider.class); ExternalIdentityProviderDefinition providerDefinition = new ExternalIdentityProviderDefinition(); when(provider.getConfig()).thenReturn(providerDefinition); @@ -532,13 +514,16 @@ void populateAttributesStoresCustomAttributesAndRoles() { HashSet externalGroupsOnAuthentication = new HashSet<>(externalGroups); when(uaaAuthentication.getExternalGroups()).thenReturn(externalGroupsOnAuthentication); + final ExternalAuthenticationDetails authenticationDetails = mock(ExternalAuthenticationDetails.class); + when(authenticationDetails.getOrigin()).thenReturn(origin); + providerDefinition.setStoreCustomAttributes(false); - manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), null); + manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), authenticationDetails); verify(manager.getUserDatabase(), never()).storeUserInfo(anyString(), any()); // when there are both attributes and groups, store them providerDefinition.setStoreCustomAttributes(true); - manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), null); + manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), authenticationDetails); UserInfo userInfo = new UserInfo() .setUserAttributes(userAttributes) .setRoles(externalGroups); @@ -547,7 +532,7 @@ void populateAttributesStoresCustomAttributesAndRoles() { // when provider is null do not store anything reset(manager.getUserDatabase()); manager.setProviderProvisioning(null); - manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), null); + manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), authenticationDetails); verify(manager.getUserDatabase(), never()).storeUserInfo(anyString(), any()); manager.setProviderProvisioning(providerProvisioning); @@ -555,7 +540,7 @@ void populateAttributesStoresCustomAttributesAndRoles() { // when attributes is empty but roles have contents, store it reset(manager.getUserDatabase()); userAttributes.clear(); - manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), null); + manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), authenticationDetails); userInfo = new UserInfo() .setUserAttributes(userAttributes) .setRoles(externalGroups); @@ -565,7 +550,7 @@ void populateAttributesStoresCustomAttributesAndRoles() { reset(manager.getUserDatabase()); userAttributes.clear(); externalGroupsOnAuthentication.clear(); - manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), null); + manager.populateAuthenticationAttributes(uaaAuthentication, mock(Authentication.class), authenticationDetails); verify(manager.getUserDatabase(), never()).storeUserInfo(anyString(), any()); } @@ -582,8 +567,7 @@ void authenticateUserExists() { @Test void authenticateUserDoesNotExists() { - String origin = "external"; - manager.setOrigin(origin); + origin = "external"; when(uaaUserDatabase.retrieveUserByName(eq(userName), eq(origin))) .thenReturn(null) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManagerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManagerTests.java index 04ae27aeff9..8cf69a9ef9a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManagerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManagerTests.java @@ -1,6 +1,7 @@ package org.cloudfoundry.identity.uaa.authentication.manager; import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication; +import org.cloudfoundry.identity.uaa.authentication.manager.ExternalLoginAuthenticationManager.ExternalAuthenticationDetails; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; @@ -138,7 +139,8 @@ void setUp() { @Test void getUserWithExtendedLdapInfo() { - UaaUser user = am.getUser(auth, null); + final ExternalAuthenticationDetails authenticationData = ExternalAuthenticationDetails.builder().origin(origin).build(); + UaaUser user = am.getUser(auth, authenticationData); assertThat(user.getExternalId()).isEqualTo(DN); assertThat(user.getEmail()).isEqualTo(LDAP_EMAIL); assertThat(user.getOrigin()).isEqualTo(origin); @@ -150,7 +152,8 @@ void getUserWithNonLdapInfo() { UserDetails mockNonLdapUserDetails = mockNonLdapUserDetails(); when(mockNonLdapUserDetails.getUsername()).thenReturn(TEST_EMAIL); when(auth.getPrincipal()).thenReturn(mockNonLdapUserDetails); - UaaUser user = am.getUser(auth, null); + final ExternalAuthenticationDetails authenticationData = ExternalAuthenticationDetails.builder().origin(origin).build(); + UaaUser user = am.getUser(auth, authenticationData); assertThat(user.getExternalId()).isEqualTo(TEST_EMAIL); assertThat(user.getEmail()).isEqualTo(TEST_EMAIL); assertThat(user.getOrigin()).isEqualTo(origin); @@ -159,14 +162,15 @@ void getUserWithNonLdapInfo() { @Test void userAuthenticated() { UaaUser user = getUaaUser(); - UaaUser userFromRequest = am.getUser(auth, null); + final ExternalAuthenticationDetails authenticationData = ExternalAuthenticationDetails.builder().origin(origin).build(); + UaaUser userFromRequest = am.getUser(auth, authenticationData); definition.setAutoAddGroups(true); - UaaUser result = am.userAuthenticated(auth, user, userFromRequest); + UaaUser result = am.userAuthenticated(auth, user, userFromRequest, authenticationData); assertThat(result).isSameAs(dbUser); verify(publisher, times(1)).publishEvent(ArgumentMatchers.any()); definition.setAutoAddGroups(false); - result = am.userAuthenticated(auth, userFromRequest, user); + result = am.userAuthenticated(auth, userFromRequest, user, authenticationData); assertThat(result).isSameAs(dbUser); verify(publisher, times(2)).publishEvent(ArgumentMatchers.any()); } @@ -174,7 +178,7 @@ void userAuthenticated() { @Test void shadowUserCreationDisabledWillNotAddShadowUser() { definition.setAddShadowUserOnLogin(false); - assertThat(am.isAddNewShadowUser()).isFalse(); + assertThat(am.isAddNewShadowUser(origin)).isFalse(); } @Test @@ -183,8 +187,9 @@ void update_existingUser_if_attributes_different() { when(auth.getPrincipal()).thenReturn(authDetails); UaaUser user = getUaaUser(); - UaaUser userFromRequest = am.getUser(auth, null); - am.userAuthenticated(auth, userFromRequest, user); + final ExternalAuthenticationDetails authenticationData = ExternalAuthenticationDetails.builder().origin(origin).build(); + UaaUser userFromRequest = am.getUser(auth, authenticationData); + am.userAuthenticated(auth, userFromRequest, user, authenticationData); ArgumentCaptor captor = ArgumentCaptor.forClass(ExternalGroupAuthorizationEvent.class); verify(publisher, times(1)).publishEvent(captor.capture()); @@ -199,8 +204,9 @@ void dontUpdate_existingUser_if_attributes_same() { ExtendedLdapUserImpl authDetails = getAuthDetails(user.getEmail(), user.getGivenName(), user.getFamilyName(), user.getPhoneNumber()); when(auth.getPrincipal()).thenReturn(authDetails); - UaaUser userFromRequest = am.getUser(auth, null); - am.userAuthenticated(auth, userFromRequest, user); + final ExternalAuthenticationDetails authenticationData = ExternalAuthenticationDetails.builder().origin(origin).build(); + UaaUser userFromRequest = am.getUser(auth, authenticationData); + am.userAuthenticated(auth, userFromRequest, user, authenticationData); ArgumentCaptor captor = ArgumentCaptor.forClass(ExternalGroupAuthorizationEvent.class); verify(publisher, times(1)).publishEvent(captor.capture()); @@ -241,27 +247,29 @@ void group_white_list_with_wildcard() { ) ); + final ExternalAuthenticationDetails authenticationData = ExternalAuthenticationDetails.builder().origin(origin).build(); + definition.setExternalGroupsWhitelist(emptyList()); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder(); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder(); definition.setExternalGroupsWhitelist(null); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder(); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder(); definition.setExternalGroupsWhitelist(Collections.singletonList("ldap.role.1.a")); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder("ldap.role.1.a"); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder("ldap.role.1.a"); definition.setExternalGroupsWhitelist(Arrays.asList("ldap.role.1.a", "ldap.role.2.*")); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.2.a", "ldap.role.2.b"); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.2.a", "ldap.role.2.b"); definition.setExternalGroupsWhitelist(Collections.singletonList("ldap.role.*.*")); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.1.b", "ldap.role.2.a", "ldap.role.2.b"); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.1.b", "ldap.role.2.a", "ldap.role.2.b"); definition.setExternalGroupsWhitelist(Arrays.asList("ldap.role.*.*", "ldap.role.*")); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.1.b", "ldap.role.1", "ldap.role.2.a", "ldap.role.2.b", "ldap.role.2"); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.1.b", "ldap.role.1", "ldap.role.2.a", "ldap.role.2.b", "ldap.role.2"); definition.setExternalGroupsWhitelist(Collections.singletonList("ldap*")); - assertThat(am.getExternalUserAuthorities(authDetails)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.1.b", "ldap.role.1", "ldap.role.2.a", "ldap.role.2.b", "ldap.role.2"); + assertThat(am.getExternalUserAuthorities(authDetails, authenticationData)).containsExactlyInAnyOrder("ldap.role.1.a", "ldap.role.1.b", "ldap.role.1", "ldap.role.2.a", "ldap.role.2.b", "ldap.role.2"); } void test_authentication_attributes(boolean storeUserInfo) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java index ee911071a4d..824f933b85a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java @@ -32,6 +32,7 @@ import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.RawExternalOAuthIdentityProviderDefinition; +import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthAuthenticationManager.AuthenticationData; import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMember; import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager; import org.cloudfoundry.identity.uaa.user.InMemoryUaaUserDatabase; @@ -87,7 +88,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import java.util.concurrent.CountDownLatch; import java.util.stream.Stream; import static java.util.Collections.emptyList; @@ -313,68 +313,6 @@ void verify_hmac_256_signature() throws Exception { assertThat(new String(Base64.encodeBase64URLSafe(hmacData))).isEqualTo(externalOAuthAuthenticationManager.hmacSignAndEncode(data, key)); } - @Test - void authManager_origin_is_thread_safe() throws Exception { - CountDownLatch countDownLatchA = new CountDownLatch(1); - CountDownLatch countDownLatchB = new CountDownLatch(1); - - final String[] thread1Origin = new String[1]; - final String[] thread2Origin = new String[1]; - Thread thread1 = new Thread() { - @Override - public void run() { - externalOAuthAuthenticationManager.setOrigin("a"); - resumeThread2(); - pauseThread1(); - thread1Origin[0] = externalOAuthAuthenticationManager.getOrigin(); - } - - private void resumeThread2() { - countDownLatchB.countDown(); - } - - private void pauseThread1() { - try { - countDownLatchA.await(); - } catch (InterruptedException e) { - e.printStackTrace(); - } - } - }; - - Thread thread2 = new Thread() { - @Override - public void run() { - pauseThread2(); - externalOAuthAuthenticationManager.setOrigin("b"); - resumeThread1(); - - thread2Origin[0] = externalOAuthAuthenticationManager.getOrigin(); - } - - private void resumeThread1() { - countDownLatchA.countDown(); - } - - private void pauseThread2() { - try { - countDownLatchB.await(); - } catch (InterruptedException e) { - e.printStackTrace(); - } - } - }; - - thread2.start(); - thread1.start(); - - thread1.join(); - thread2.join(); - - assertThat(thread1Origin[0]).isEqualTo("a"); - assertThat(thread2Origin[0]).isEqualTo("b"); - } - @Test void when_a_null_id_token_is_provided_resolveOriginProvider_should_throw_a_jwt_validation_exception() { assertThatThrownBy(() -> externalOAuthAuthenticationManager.resolveOriginProvider(null)) @@ -446,12 +384,12 @@ void when_exchanging_an_id_token_retrieved_from_the_internal_uaa_idp_for_an_acce xCodeToken.setIdToken(idToken); xCodeToken.setOrigin(null); - ExternalOAuthAuthenticationManager.AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager + AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager .getExternalAuthenticationDetails(xCodeToken); assertThat(username).isEqualTo(externalAuthenticationDetails.getUsername()); assertThat(externalAuthenticationDetails.getClaims()).containsEntry(ClaimConstants.ORIGIN, UAA_ORIGIN); - assertThat(externalOAuthAuthenticationManager.getOrigin()).isEqualTo(UAA_ORIGIN); + assertThat(externalAuthenticationDetails.getOrigin()).isEqualTo(UAA_ORIGIN); } @ParameterizedTest @@ -491,12 +429,12 @@ void when_exchanging_an_id_token_retrieved_by_uaa_via_an_oidc_idp_for_an_access_ xCodeToken.setIdToken(idToken); xCodeToken.setOrigin(null); - ExternalOAuthAuthenticationManager.AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager + AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager .getExternalAuthenticationDetails(xCodeToken); assertThat(username).isEqualTo(externalAuthenticationDetails.getUsername()); assertThat(externalAuthenticationDetails.getClaims()).containsEntry(ClaimConstants.ORIGIN, idpProvider.getOriginKey()); - assertThat(externalOAuthAuthenticationManager.getOrigin()).isEqualTo(idpProvider.getOriginKey()); + assertThat(externalAuthenticationDetails.getOrigin()).isEqualTo(idpProvider.getOriginKey()); } @Test @@ -516,12 +454,12 @@ void when_exchanging_an_id_token_retrieved_by_uaa_via_an_registered_oidc_idp_for xCodeToken.setIdToken(idToken); xCodeToken.setOrigin(null); - ExternalOAuthAuthenticationManager.AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager + AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager .getExternalAuthenticationDetails(xCodeToken); assertThat(username).isEqualTo(externalAuthenticationDetails.getUsername()); assertThat(externalAuthenticationDetails.getClaims()).containsEntry(ClaimConstants.ORIGIN, OriginKeys.UAA); - assertThat(externalOAuthAuthenticationManager.getOrigin()).isEqualTo(idpProvider.getOriginKey()); + assertThat(externalAuthenticationDetails.getOrigin()).isEqualTo(idpProvider.getOriginKey()); } @Test @@ -539,7 +477,7 @@ void when_exchanging_an_id_token_retrieved_by_an_external_oidc_idp_for_an_access xCodeToken.setIdToken(idToken); xCodeToken.setOrigin(null); - ExternalOAuthAuthenticationManager.AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager + AuthenticationData externalAuthenticationDetails = externalOAuthAuthenticationManager .getExternalAuthenticationDetails(xCodeToken); assertThat(username).isEqualTo(externalAuthenticationDetails.getUsername()); @@ -1019,12 +957,28 @@ void getUserWithNullEmail() { @Test void getUserSetsTheRightOrigin() { - externalOAuthAuthenticationManager.getUser(xCodeToken, externalOAuthAuthenticationManager.getExternalAuthenticationDetails(xCodeToken)); - assertThat(externalOAuthAuthenticationManager.getOrigin()).isEqualTo(ORIGIN); + final IdentityProvider> idp = MultitenancyFixture.identityProvider("the_origin", "uaa"); + idp.setConfig(config); + when(provisioning.retrieveByOrigin(eq(ORIGIN), anyString())).thenReturn(idp); + + final IdentityProvider> otherIdp = MultitenancyFixture.identityProvider("other_origin", "uaa"); + otherIdp.setConfig(config); + when(provisioning.retrieveByOrigin(eq("other_origin"), anyString())).thenReturn(otherIdp); + + mockToken(); + AuthenticationData authenticationData = externalOAuthAuthenticationManager.getExternalAuthenticationDetails(xCodeToken); + assertThat(authenticationData).isNotNull(); + externalOAuthAuthenticationManager.getUser(xCodeToken, authenticationData); + assertThat(authenticationData.getOrigin()).isEqualTo(ORIGIN); + mockUaaServer.verify(); + mockUaaServer.reset(); + + mockToken(); ExternalOAuthCodeToken otherToken = new ExternalOAuthCodeToken(CODE, "other_origin", "http://localhost/callback/the_origin"); - externalOAuthAuthenticationManager.getUser(otherToken, externalOAuthAuthenticationManager.getExternalAuthenticationDetails(otherToken)); - assertThat(externalOAuthAuthenticationManager.getOrigin()).isEqualTo("other_origin"); + AuthenticationData authenticationDataOtherToken = externalOAuthAuthenticationManager.getExternalAuthenticationDetails(otherToken); + externalOAuthAuthenticationManager.getUser(otherToken, authenticationDataOtherToken); + assertThat(authenticationDataOtherToken.getOrigin()).isEqualTo("other_origin"); } @Test diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenExchangeOverrideAuthManagerMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenExchangeOverrideAuthManagerMockMvcTests.java index 05dd0a3d544..4e2a85c7485 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenExchangeOverrideAuthManagerMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenExchangeOverrideAuthManagerMockMvcTests.java @@ -64,7 +64,7 @@ ExternalOAuthAuthenticationManager tokenExchangeAuthenticationManager( @Override public AuthenticationData getExternalAuthenticationDetails(Authentication authentication) { AuthenticationData result = super.getExternalAuthenticationDetails(authentication); - this.setOrigin("override-origin"); + result.setOrigin("override-origin"); return result; }