Skip to content

Commit 620f4c4

Browse files
adrianhoelzl-sapadrianhoelzl-sap
authored
Refactor ExternalLoginAuthenticationManager (#3607)
* Use Lombok Getter and Setter annotation * Remove static import of Optional.ofNullable * Clean up * Introduce constant for fallback e-mail domain * Refactor generateEmailIfNullOrEmpty * Make class ExternalLoginAuthenticationManager abstract * Make method ExternalLoginAuthenticationManager#isAddNewShadowUser abstract * Make method ExternalLoginAuthenticationManager#getExternalAuthenticationDetails abstract * Make methods ExternalLoginAuthenticationManager#getOrigin and ExternalLoginAuthenticationManager#setOrigin abstract * Make method ExternalLoginAuthenticationManager#setApplicationEventPublisher final * Make method ExternalLoginAuthenticationManager#publish final * Make method ExternalLoginAuthenticationManager#userAuthenticated abstract * Make method ExternalLoginAuthenticationManager#mapAuthorities final * Make method ExternalLoginAuthenticationManager#haveUserAttributesChanged final * Make method ExternalLoginAuthenticationManager#generateEmailIfNullOrEmpty final * Make method ExternalLoginAuthenticationManager#getExternalUserAuthorities abstract * Address warnings in LdapLoginAuthenticationManager * Address warnings in ExternalLoginAuthenticationManager * Fix comment in LdapLoginAuthenticationManager
1 parent cf0aea7 commit 620f4c4

File tree

5 files changed

+112
-85
lines changed

5 files changed

+112
-85
lines changed

‎server/src/main/java/org/cloudfoundry/identity/uaa/SpringServletXmlBeansConfiguration.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.cloudfoundry.identity.uaa.client.ClientAdminEndpointsValidator;
1010
import org.cloudfoundry.identity.uaa.client.event.ClientAdminEventPublisher;
1111
import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
12-
import org.cloudfoundry.identity.uaa.constants.OriginKeys;
1312
import org.cloudfoundry.identity.uaa.impl.config.IdentityProviderBootstrap;
1413
import org.cloudfoundry.identity.uaa.impl.config.IdentityZoneConfigurationBootstrap;
1514
import org.cloudfoundry.identity.uaa.impl.config.UaaConfiguration;
@@ -340,7 +339,6 @@ LdapLoginAuthenticationManager ldapLoginAuthenticationMgr(
340339
) {
341340
LdapLoginAuthenticationManager bean = new LdapLoginAuthenticationManager(provisioning);
342341
bean.setUserDatabase(userDatabase);
343-
bean.setOrigin(OriginKeys.LDAP);
344342
return bean;
345343
}
346344

‎server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java‎

Lines changed: 51 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.cloudfoundry.identity.uaa.authentication.manager;
22

3+
import lombok.Getter;
4+
import lombok.Setter;
35
import org.apache.commons.lang3.StringUtils;
46
import org.cloudfoundry.identity.uaa.authentication.AccountNotPreCreatedException;
57
import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
@@ -23,6 +25,7 @@
2325
import org.cloudfoundry.identity.uaa.user.VerifiableUser;
2426
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
2527
import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
28+
import org.jspecify.annotations.NonNull;
2629
import org.slf4j.Logger;
2730
import org.slf4j.LoggerFactory;
2831
import org.springframework.beans.factory.BeanNameAware;
@@ -47,71 +50,45 @@
4750
import java.util.HashSet;
4851
import java.util.LinkedList;
4952
import java.util.List;
53+
import java.util.Optional;
5054

5155
import static java.util.Collections.emptySet;
52-
import static java.util.Optional.ofNullable;
5356

54-
public class ExternalLoginAuthenticationManager<ExternalAuthenticationDetails> implements AuthenticationManager, ApplicationEventPublisherAware, BeanNameAware {
57+
public abstract class ExternalLoginAuthenticationManager<ExternalAuthenticationDetails> implements AuthenticationManager, ApplicationEventPublisherAware, BeanNameAware {
5558

5659
public static final String USER_ATTRIBUTE_PREFIX = "user.attribute.";
60+
private static final String FALLBACK_EMAIL_DOMAIN_TEMPLATE = "user.from.%s.cf";
61+
5762
protected final Logger logger = LoggerFactory.getLogger(getClass());
5863

5964
private ApplicationEventPublisher eventPublisher;
6065

66+
@Getter
67+
@Setter
6168
private UaaUserDatabase userDatabase;
6269

6370
private String name;
6471

65-
private String origin = "unknown";
66-
72+
@Getter
73+
@Setter
6774
private IdentityProviderProvisioning providerProvisioning;
6875

76+
@Getter
77+
@Setter
6978
private ScimGroupExternalMembershipManager externalMembershipManager;
7079

71-
7280
public ExternalLoginAuthenticationManager(IdentityProviderProvisioning providerProvisioning) {
7381
this.providerProvisioning = providerProvisioning;
7482
}
7583

76-
public IdentityProviderProvisioning getProviderProvisioning() {
77-
return providerProvisioning;
78-
}
79-
80-
public void setProviderProvisioning(IdentityProviderProvisioning providerProvisioning) {
81-
this.providerProvisioning = providerProvisioning;
82-
}
83-
84-
public ScimGroupExternalMembershipManager getExternalMembershipManager() {
85-
return externalMembershipManager;
86-
}
87-
88-
public void setExternalMembershipManager(ScimGroupExternalMembershipManager externalMembershipManager) {
89-
this.externalMembershipManager = externalMembershipManager;
90-
}
91-
92-
public String getOrigin() {
93-
return origin;
94-
}
95-
96-
public void setOrigin(String origin) {
97-
this.origin = origin;
98-
}
99-
10084
@Override
101-
public void setApplicationEventPublisher(ApplicationEventPublisher eventPublisher) {
85+
public final void setApplicationEventPublisher(@NonNull ApplicationEventPublisher eventPublisher) {
10286
this.eventPublisher = eventPublisher;
10387
}
10488

105-
/**
106-
* @param userDatabase the userDatabase to set
107-
*/
108-
public void setUserDatabase(UaaUserDatabase userDatabase) {
109-
this.userDatabase = userDatabase;
110-
}
89+
public abstract String getOrigin();
11190

112-
public UaaUserDatabase getUserDatabase() {
113-
return this.userDatabase;
114-
}
91+
public abstract void setOrigin(String origin);
11592

11693
@Override
11794
public Authentication authenticate(Authentication request) throws AuthenticationException {
@@ -171,14 +148,17 @@ protected void populateAuthenticationAttributes(UaaAuthentication authentication
171148
if (authentication.getAuthenticationMethods() == null) {
172149
authentication.setAuthenticationMethods(new HashSet<>());
173150
}
151+
174152
authentication.getAuthenticationMethods().add("ext");
153+
154+
// persist the user attributes and external groups in the user info table if configured in the IdP
175155
if ((hasUserAttributes(authentication) || hasExternalGroups(authentication)) && getProviderProvisioning() != null) {
176156
IdentityProvider<ExternalIdentityProviderDefinition> provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId());
177157
if (provider.getConfig() != null && provider.getConfig().isStoreCustomAttributes()) {
178158
logger.debug("Storing custom attributes for user_id:{}", authentication.getPrincipal().getId());
179159
UserInfo userInfo = new UserInfo()
180160
.setUserAttributes(authentication.getUserAttributes())
181-
.setRoles(new LinkedList<>(ofNullable(authentication.getExternalGroups()).orElse(emptySet())));
161+
.setRoles(new LinkedList<>(Optional.ofNullable(authentication.getExternalGroups()).orElse(emptySet())));
182162
getUserDatabase().storeUserInfo(authentication.getPrincipal().getId(), userInfo);
183163
}
184164
}
@@ -192,31 +172,23 @@ private boolean hasUserAttributes(UaaAuthentication authentication) {
192172
return authentication.getUserAttributes() != null && !authentication.getUserAttributes().isEmpty();
193173
}
194174

195-
protected ExternalAuthenticationDetails getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException {
196-
return null;
197-
}
175+
protected abstract ExternalAuthenticationDetails getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException;
198176

199-
protected boolean isAddNewShadowUser() {
200-
return true;
201-
}
177+
protected abstract boolean isAddNewShadowUser();
202178

203179
protected MultiValueMap<String, String> getUserAttributes(UserDetails request) {
204180
return new LinkedMultiValueMap<>();
205181
}
206182

207-
protected List<String> getExternalUserAuthorities(UserDetails request) {
208-
return new LinkedList<>();
209-
}
183+
protected abstract List<String> getExternalUserAuthorities(UserDetails request);
210184

211-
protected void publish(ApplicationEvent event) {
185+
protected final void publish(ApplicationEvent event) {
212186
if (eventPublisher != null) {
213187
eventPublisher.publishEvent(event);
214188
}
215189
}
216190

217-
protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequest, UaaUser userFromDb) {
218-
return userFromDb;
219-
}
191+
protected abstract UaaUser userAuthenticated(Authentication request, UaaUser userFromRequest, UaaUser userFromDb);
220192

221193
protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails authDetails) {
222194
UserDetails userDetails;
@@ -259,7 +231,7 @@ protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails
259231

260232
String phoneNumber = userDetails instanceof DialableByPhone dbp ? dbp.getPhoneNumber() : null;
261233
String externalId = userDetails instanceof ExternallyIdentifiable ei ? ei.getExternalId() : name;
262-
boolean verified = userDetails instanceof VerifiableUser vu ? vu.isVerified() : false;
234+
boolean verified = userDetails instanceof VerifiableUser vu && vu.isVerified();
263235
UaaUserPrototype userPrototype = new UaaUserPrototype()
264236
.withVerified(verified)
265237
.withUsername(name)
@@ -278,27 +250,33 @@ protected UaaUser getUser(Authentication request, ExternalAuthenticationDetails
278250
return new UaaUser(userPrototype);
279251
}
280252

281-
protected String generateEmailIfNullOrEmpty(String name) {
282-
String email;
283-
if (name != null) {
284-
if (name.contains("@")) {
285-
if (name.split("@").length == 2 && !name.startsWith("@") && !name.endsWith("@")) {
286-
email = name;
287-
} else {
288-
email = name.replace("@", "") + "@user.from." + getOrigin() + ".cf";
289-
}
290-
} else {
291-
email = name + "@user.from." + getOrigin() + ".cf";
292-
}
293-
} else {
253+
protected final String generateEmailIfNullOrEmpty(String name) {
254+
if (name == null) {
294255
throw new BadCredentialsException("Cannot determine username from credentials supplied");
295256
}
296-
return email;
257+
258+
final String fallbackEmailDomain = FALLBACK_EMAIL_DOMAIN_TEMPLATE.formatted(getOrigin());
259+
260+
// use fallback domain if no '@' is present
261+
if (!name.contains("@")) {
262+
return name + "@" + fallbackEmailDomain;
263+
}
264+
265+
// use as-is if it represents a valid e-mail address
266+
if (name.split("@").length == 2 && !name.startsWith("@") && !name.endsWith("@")) {
267+
return name;
268+
}
269+
270+
// otherwise, remove any '@' characters and use fallback domain
271+
return name.replace("@", "") + "@" + fallbackEmailDomain;
297272
}
298273

299-
protected boolean haveUserAttributesChanged(UaaUser existingUser, UaaUser user) {
300-
return !StringUtils.equals(existingUser.getGivenName(), user.getGivenName()) || !StringUtils.equals(existingUser.getFamilyName(), user.getFamilyName()) ||
301-
!StringUtils.equals(existingUser.getPhoneNumber(), user.getPhoneNumber()) || !StringUtils.equals(existingUser.getEmail(), user.getEmail()) || !StringUtils.equals(existingUser.getExternalId(), user.getExternalId());
274+
protected final boolean haveUserAttributesChanged(UaaUser existingUser, UaaUser user) {
275+
return !StringUtils.equals(existingUser.getGivenName(), user.getGivenName())
276+
|| !StringUtils.equals(existingUser.getFamilyName(), user.getFamilyName())
277+
|| !StringUtils.equals(existingUser.getPhoneNumber(), user.getPhoneNumber())
278+
|| !StringUtils.equals(existingUser.getEmail(), user.getEmail())
279+
|| !StringUtils.equals(existingUser.getExternalId(), user.getExternalId());
302280
}
303281

304282
/**
@@ -312,7 +290,7 @@ protected boolean haveUserAttributesChanged(UaaUser existingUser, UaaUser user)
312290
* 'external_groups' attribute mapping of the IdP
313291
* @return the internal groups
314292
*/
315-
protected List<SimpleGrantedAuthority> evaluateExternalGroupMappings(String origin, Collection<? extends GrantedAuthority> externalGroups) {
293+
protected final List<SimpleGrantedAuthority> evaluateExternalGroupMappings(String origin, Collection<? extends GrantedAuthority> externalGroups) {
316294
List<SimpleGrantedAuthority> result = new LinkedList<>();
317295
for (GrantedAuthority authority : externalGroups) {
318296
String externalGroup = authority.getAuthority();
@@ -329,7 +307,7 @@ protected List<SimpleGrantedAuthority> evaluateExternalGroupMappings(String orig
329307
}
330308

331309
@Override
332-
public void setBeanName(String name) {
310+
public void setBeanName(@NonNull String name) {
333311
this.name = name;
334312
}
335313
}

‎server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.cloudfoundry.identity.uaa.authentication.manager;
1717

1818
import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
19+
import org.cloudfoundry.identity.uaa.constants.OriginKeys;
1920
import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
2021
import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
2122
import org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition;
@@ -28,6 +29,7 @@
2829
import org.slf4j.LoggerFactory;
2930
import org.springframework.beans.factory.annotation.Qualifier;
3031
import org.springframework.security.core.Authentication;
32+
import org.springframework.security.core.AuthenticationException;
3133
import org.springframework.security.core.GrantedAuthority;
3234
import org.springframework.security.core.userdetails.UserDetails;
3335
import org.springframework.util.MultiValueMap;
@@ -44,20 +46,38 @@
4446
import static java.util.Collections.emptyList;
4547
import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.retainAllMatches;
4648

47-
public class LdapLoginAuthenticationManager extends ExternalLoginAuthenticationManager {
49+
public class LdapLoginAuthenticationManager extends ExternalLoginAuthenticationManager<Object> {
4850

4951
protected static Logger logger = LoggerFactory.getLogger(LdapLoginAuthenticationManager.class);
5052

5153
public LdapLoginAuthenticationManager(final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning providerProvisioning) {
5254
super(providerProvisioning);
5355
}
5456

57+
private String origin = OriginKeys.LDAP;
58+
59+
@Override
60+
public String getOrigin() {
61+
return origin;
62+
}
63+
64+
@Override
65+
public void setOrigin(String origin) {
66+
// only used in LdapLoginAuthenticationManagerTests
67+
this.origin = origin;
68+
}
69+
5570
@Override
5671
protected void populateAuthenticationAttributes(UaaAuthentication authentication, Authentication request, Object authenticationData) {
5772
super.populateAuthenticationAttributes(authentication, request, authenticationData);
5873
authentication.getAuthenticationMethods().add("pwd");
5974
}
6075

76+
@Override
77+
protected Object getExternalAuthenticationDetails(Authentication authentication) throws AuthenticationException {
78+
return null;
79+
}
80+
6181
@Override
6282
protected MultiValueMap<String, String> getUserAttributes(UserDetails request) {
6383
MultiValueMap<String, String> result = super.getUserAttributes(request);
@@ -73,7 +93,7 @@ protected MultiValueMap<String, String> getUserAttributes(UserDetails request) {
7393
String[] values = ldapDetails.getAttribute((String) entry.getValue(), false);
7494
if (values != null && values.length > 0) {
7595
result.put(key, Arrays.asList(values));
76-
logger.debug("Mappcustom attribute key:{} and value:{}", key, result.get(key));
96+
logger.debug("Map custom attribute key:{} and value:{}", key, result.get(key));
7797
}
7898
}
7999
}
@@ -86,17 +106,17 @@ protected MultiValueMap<String, String> getUserAttributes(UserDetails request) {
86106

87107
@Override
88108
protected List<String> getExternalUserAuthorities(UserDetails request) {
89-
List<String> result = super.getExternalUserAuthorities(request);
109+
List<String> result = new LinkedList<>();
90110
if (getProviderProvisioning() != null) {
91111
IdentityProvider provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), IdentityZoneHolder.get().getId());
92112
LdapIdentityProviderDefinition ldapIdentityProviderDefinition = ObjectUtils.castInstance(provider.getConfig(), LdapIdentityProviderDefinition.class);
93113
List<String> externalWhiteList = ldapIdentityProviderDefinition.getExternalGroupsWhitelist();
94-
result = new ArrayList<>(retainAllMatches(getAuthoritesAsNames(request.getAuthorities()), externalWhiteList));
114+
result = new ArrayList<>(retainAllMatches(getAuthoritiesAsNames(request.getAuthorities()), externalWhiteList));
95115
}
96116
return result;
97117
}
98118

99-
protected Set<String> getAuthoritesAsNames(Collection<? extends GrantedAuthority> authorities) {
119+
protected Set<String> getAuthoritiesAsNames(Collection<? extends GrantedAuthority> authorities) {
100120
Set<String> result = new HashSet<>();
101121
authorities = new LinkedList<>(authorities != null ? authorities : emptyList());
102122
for (GrantedAuthority a : authorities) {

‎server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ protected void populateAuthenticationAttributes(UaaAuthentication authentication
436436

437437
@Override
438438
protected List<String> getExternalUserAuthorities(UserDetails request) {
439-
return super.getExternalUserAuthorities(request);
439+
return new LinkedList<>();
440440
}
441441

442442
@Override
@@ -598,9 +598,6 @@ private boolean isRegisteredIdpAuthentication(Authentication request) {
598598

599599
@Override
600600
protected boolean isAddNewShadowUser() {
601-
if (!super.isAddNewShadowUser()) {
602-
return false;
603-
}
604601
IdentityProvider<AbstractExternalOAuthIdentityProviderDefinition> provider = getProviderProvisioning().retrieveByOrigin(getOrigin(), identityZoneManager.getCurrentIdentityZoneId());
605602
return provider.getConfig().isAddShadowUserOnLogin();
606603
}

0 commit comments

Comments
 (0)