Skip to content

Commit a02bd0f

Browse files
committed
improve resilence and fix potential NPE in LDAP synchronization
1 parent 7ac861c commit a02bd0f

File tree

6 files changed

+301
-131
lines changed

6 files changed

+301
-131
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package name.nkonev.aaa.entity.ldap;
2+
3+
import name.nkonev.aaa.config.properties.LdapAttributes;
4+
import name.nkonev.aaa.utils.NullUtils;
5+
6+
import javax.naming.directory.Attributes;
7+
import java.util.Set;
8+
9+
import static name.nkonev.aaa.utils.ConvertUtils.*;
10+
11+
// all props are nullable
12+
public record LdapEntity(
13+
String id,
14+
String username,
15+
String email,
16+
Set<String> roles,
17+
Boolean locked,
18+
Boolean enabled
19+
) {
20+
21+
public LdapEntity(LdapAttributes attributeNames, Attributes ldapEntry) {
22+
this(
23+
extractId(attributeNames, ldapEntry),
24+
extractUsername(attributeNames, ldapEntry),
25+
extractEmail(attributeNames, ldapEntry),
26+
extractRoles(attributeNames, ldapEntry),
27+
extractLocked(attributeNames, ldapEntry),
28+
extractEnabled(attributeNames, ldapEntry)
29+
);
30+
}
31+
}

aaa/src/main/java/name/nkonev/aaa/repository/jdbc/UserAccountRepository.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.springframework.stereotype.Repository;
99

1010
import java.time.LocalDateTime;
11+
import java.util.Collection;
1112
import java.util.List;
1213
import java.util.Optional;
1314
import java.util.Set;
@@ -38,4 +39,6 @@ public interface UserAccountRepository extends ListCrudRepository<UserAccount, L
3839
// here we intentionally set that deleted user exists
3940
@Query("select u.id from user_account u where u.id in (:userIds)")
4041
Set<Long> findUserIds(List<Long> userIds);
42+
43+
List<UserAccount> findByLdapIdInOrderById(Collection<String> strings);
4144
}

aaa/src/main/java/name/nkonev/aaa/repository/spring/jdbc/UserListViewRepository.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -227,16 +227,6 @@ public List<UserAccount> findPage(int pageSize, int offset) {
227227
userAccountRowMapper);
228228
}
229229

230-
public List<UserAccount> findPageWithLdapId(int pageSize, int offset) {
231-
return jdbcTemplate.query(
232-
PREFIX + " and ldap_id is not null " + PAGINATION_SUFFIX,
233-
Map.of(
234-
"limit", pageSize,
235-
"offset", offset
236-
),
237-
userAccountRowMapper);
238-
}
239-
240230
private static final String AND_USERNAME_ILIKE = " and " + USERNAME_SEARCH;
241231

242232
public List<UserAccount> findByUsernameContainsIgnoreCase(int pageSize, long offset, String searchString) {

aaa/src/main/java/name/nkonev/aaa/security/LdapAuthenticationProvider.java

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import name.nkonev.aaa.converter.UserAccountConverter;
55
import name.nkonev.aaa.dto.UserAccountDetailsDTO;
66
import name.nkonev.aaa.entity.jdbc.UserAccount;
7+
import name.nkonev.aaa.entity.ldap.LdapEntity;
78
import name.nkonev.aaa.exception.UserAlreadyPresentException;
89
import name.nkonev.aaa.repository.jdbc.UserAccountRepository;
910
import name.nkonev.aaa.services.CheckService;
@@ -27,13 +28,6 @@
2728
import java.util.Set;
2829
import java.util.concurrent.atomic.AtomicBoolean;
2930

30-
import static name.nkonev.aaa.converter.UserAccountConverter.normalizeEmail;
31-
import static name.nkonev.aaa.utils.ConvertUtils.convertToStrings;
32-
33-
import name.nkonev.aaa.utils.NullUtils;
34-
35-
import javax.naming.NamingException;
36-
3731
// https://spring.io/guides/gs/authenticating-ldap
3832
@Component
3933
public class LdapAuthenticationProvider implements AuthenticationProvider {
@@ -76,37 +70,38 @@ public Authentication authenticate(Authentication authentication) throws Authent
7670
var userAccount = transactionTemplate.execute(status -> {
7771
var lq = LdapQueryBuilder.query().base(aaaProperties.ldap().auth().base()).filter(aaaProperties.ldap().auth().filter(), userName);
7872
ldapOperations.authenticate(lq, encodedPassword);
79-
var ldapEntry = ldapOperations.searchForContext(lq).getAttributes();
8073

81-
var ldapUserId = NullUtils.getOrNullWrapException(() -> ldapEntry.get(aaaProperties.ldap().attributeNames().id()).get().toString());
74+
var ldapAttributes = ldapOperations.searchForContext(lq).getAttributes();
75+
var ldapEntry = new LdapEntity(aaaProperties.ldap().attributeNames(), ldapAttributes);
76+
77+
var ldapUserId = ldapEntry.id();
78+
if (ldapUserId == null) {
79+
LOGGER.warn("Got null ldap id for username={}", userName);
80+
return null;
81+
}
8282

8383
UserAccount byLdapId = userAccountRepository
8484
.findByLdapId(ldapUserId)
8585
.orElseGet(() -> {
8686
// create a new
87+
88+
// check conflict by username
8789
userAccountRepository.findByUsername(userName).ifPresent(ua -> {
8890
throw new UserAlreadyPresentException("User with login '" + userName + "' is already present");
8991
});
9092

9193
String email = null;
9294
if (StringUtils.hasLength(aaaProperties.ldap().attributeNames().email())) {
93-
var ldapEmail = NullUtils.getOrNullWrapException(() -> ldapEntry.get(aaaProperties.ldap().attributeNames().email()).get().toString());
94-
email = normalizeEmail(ldapEmail);
95+
email = ldapEntry.email();
9596
}
9697

97-
final Set<String> rawRoles = new HashSet<>();
98+
Set<String> rawRoles = new HashSet<>();
9899
if (StringUtils.hasLength(aaaProperties.ldap().attributeNames().role())) {
99-
try {
100-
var groups = ldapEntry.get(aaaProperties.ldap().attributeNames().role()).getAll();
101-
if (groups != null) {
102-
rawRoles.addAll(convertToStrings(groups));
103-
}
104-
} catch (NamingException e) {
105-
LOGGER.error(e.getMessage(), e);
106-
}
100+
rawRoles = ldapEntry.roles();
107101
}
108102
var mappedRoles = RoleMapper.map(aaaProperties.roleMappings().ldap(), rawRoles);
109103

104+
// check conflict by email
110105
if (StringUtils.hasLength(email)) {
111106
if (!userService.checkEmailIsFree(email)){
112107
throw new UserAlreadyPresentException("User with email '" + email + "' is already present");

0 commit comments

Comments
 (0)