+ * The actual LDAP connection handling is encapsulated inside {@code LdapAuthService}, which + * uses JNDI {@code DirContext} directly. This avoids maintaining a parallel Spring LDAP + * {@code LdapTemplate}/{@code LdapContextSource} bean graph whose configuration source + * ({@code spring.ldap.*}) would diverge from the application-level {@code skillhub.ldap.*} + * properties consumed by {@link LdapProperties}. + *
+ * {@link LdapProperties} is a standalone {@code @Component} and is always available; the + * {@code LdapAuthService} bean itself is conditionally created only when + * {@code skillhub.ldap.enabled=true}. */ @Configuration -@ConditionalOnProperty(name = "skillhub.ldap.enabled", havingValue = "true") public class LdapAutoConfiguration { - - /** - * Creates an LdapContextSource configured from skillhub.ldap properties. - */ - @Bean - public LdapContextSource ldapContextSource(LdapProperties ldapProperties) { - LdapContextSource contextSource = new LdapContextSource(); - contextSource.setUrl(ldapProperties.getUrl()); - contextSource.setBase(ldapProperties.getBase()); - if (ldapProperties.getUsername() != null && !ldapProperties.getUsername().isEmpty()) { - contextSource.setUserDn(ldapProperties.getUsername()); - contextSource.setPassword(ldapProperties.getPassword()); - } - return contextSource; - } - - /** - * Creates an LdapTemplate for LDAP operations. - */ - @Bean - public LdapTemplate ldapTemplate(LdapContextSource ldapContextSource) { - return new LdapTemplate(ldapContextSource); - } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/LdapProperties.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/LdapProperties.java index e68da824..46cfc80a 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/LdapProperties.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/LdapProperties.java @@ -44,6 +44,40 @@ public class LdapProperties { * Search base for user lookup (relative to base). */ private String userSearchBase = ""; + + /** + * Stable directory identifier attribute used as the LDAP identity subject. + * OpenLDAP uses "entryUUID", Active Directory uses "objectGUID". + */ + private String subjectAttribute = "entryUUID"; + + /** + * LDAP attribute mapped to the local display name. + */ + private String displayNameAttribute = "displayName"; + + /** + * Fallback LDAP attribute for the display name when the primary + * {@link #displayNameAttribute} is absent or empty. Defaults to {@code cn} + * (common name), the conventional fallback for directories that do not + * populate a dedicated display name. + */ + private String displayNameFallbackAttribute = "cn"; + + /** + * LDAP attribute mapped to the local email. + */ + private String emailAttribute = "mail"; + + /** + * LDAP connection timeout in milliseconds. + */ + private int connectTimeoutMillis = 5000; + + /** + * LDAP read timeout in milliseconds. + */ + private int readTimeoutMillis = 10000; public boolean isEnabled() { return enabled; @@ -100,4 +134,52 @@ public class LdapProperties { public void setUserSearchBase(String userSearchBase) { this.userSearchBase = userSearchBase; } -} \ No newline at end of file + + public String getSubjectAttribute() { + return subjectAttribute; + } + + public void setSubjectAttribute(String subjectAttribute) { + this.subjectAttribute = subjectAttribute; + } + + public String getDisplayNameAttribute() { + return displayNameAttribute; + } + + public void setDisplayNameAttribute(String displayNameAttribute) { + this.displayNameAttribute = displayNameAttribute; + } + + public String getDisplayNameFallbackAttribute() { + return displayNameFallbackAttribute; + } + + public void setDisplayNameFallbackAttribute(String displayNameFallbackAttribute) { + this.displayNameFallbackAttribute = displayNameFallbackAttribute; + } + + public String getEmailAttribute() { + return emailAttribute; + } + + public void setEmailAttribute(String emailAttribute) { + this.emailAttribute = emailAttribute; + } + + public int getConnectTimeoutMillis() { + return connectTimeoutMillis; + } + + public void setConnectTimeoutMillis(int connectTimeoutMillis) { + this.connectTimeoutMillis = connectTimeoutMillis; + } + + public int getReadTimeoutMillis() { + return readTimeoutMillis; + } + + public void setReadTimeoutMillis(int readTimeoutMillis) { + this.readTimeoutMillis = readTimeoutMillis; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/ldap/LdapAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/ldap/LdapAuthService.java index bf590475..21ea29bb 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/ldap/LdapAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/ldap/LdapAuthService.java @@ -1,21 +1,27 @@ package com.iflytek.skillhub.auth.ldap; import com.iflytek.skillhub.auth.config.LdapProperties; +import com.iflytek.skillhub.auth.entity.IdentityBinding; import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.rbac.PlatformRoleDefaults; +import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; +import java.security.cert.CertificateException; import java.util.Hashtable; -import java.util.List; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; +import javax.net.ssl.SSLException; +import javax.naming.AuthenticationException; +import javax.naming.CommunicationException; import javax.naming.Context; import javax.naming.NamingException; +import java.util.regex.Pattern; import javax.naming.directory.Attribute; import javax.naming.directory.Attributes; import javax.naming.directory.DirContext; @@ -25,55 +31,70 @@ import javax.naming.directory.SearchResult; import javax.naming.ldap.LdapName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.http.HttpStatus; -import org.springframework.ldap.core.LdapTemplate; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; /** * Handles LDAP authentication for enterprise directory integration. + *
+ * Identity is anchored on a stable directory identifier (entryUUID/objectGUID) via
+ * {@link IdentityBinding} (provider="ldap"), not on the user's email. This prevents
+ * silent account merging when an LDAP user's email collides with an existing
+ * local/OAuth account, and avoids duplicate accounts for email-less users.
*/
@Service
+@ConditionalOnProperty(prefix = "skillhub.ldap", name = "enabled", havingValue = "true")
public class LdapAuthService {
private static final Logger log = LoggerFactory.getLogger(LdapAuthService.class);
+ private static final String LDAP_PROVIDER = "ldap";
+ // Allows alphanumeric, underscore, hyphen, dot, and @ (for UPN formats), 3-64 characters.
+ private static final Pattern USERNAME_PATTERN = Pattern.compile("^[A-Za-z0-9_@.\\-]{3,64}$");
+ // LDAP attribute names only allow ASCII letters, digits, and hyphens.
+ private static final Pattern ATTRIBUTE_NAME_PATTERN = Pattern.compile("^[a-zA-Z][a-zA-Z0-9-]*$");
private final LdapProperties ldapProperties;
- private final LdapTemplate ldapTemplate;
private final UserAccountRepository userAccountRepository;
private final UserRoleBindingRepository userRoleBindingRepository;
private final GlobalNamespaceMembershipService globalNamespaceMembershipService;
+ private final IdentityBindingRepository identityBindingRepository;
public LdapAuthService(LdapProperties ldapProperties,
- LdapTemplate ldapTemplate,
UserAccountRepository userAccountRepository,
UserRoleBindingRepository userRoleBindingRepository,
- GlobalNamespaceMembershipService globalNamespaceMembershipService) {
+ GlobalNamespaceMembershipService globalNamespaceMembershipService,
+ IdentityBindingRepository identityBindingRepository) {
this.ldapProperties = ldapProperties;
- this.ldapTemplate = ldapTemplate;
this.userAccountRepository = userAccountRepository;
this.userRoleBindingRepository = userRoleBindingRepository;
this.globalNamespaceMembershipService = globalNamespaceMembershipService;
+ this.identityBindingRepository = identityBindingRepository;
}
/**
* Authenticates a user against the LDAP server.
* If the user doesn't exist in the local database, creates a new user based on LDAP attributes.
*
- * @param username the username
- * @param password the password
- * @return PlatformPrincipal if authentication succeeds
- * @throws AuthFlowException if authentication fails
- */
+ * @param username the username
+ * @param password the password
+ * @return PlatformPrincipal if authentication succeeds
+ * @throws AuthFlowException if authentication fails
+ */
@Transactional
public PlatformPrincipal login(String username, String password) {
log.info("Starting LDAP authentication for username: {}", username);
-
+
if (!ldapProperties.isEnabled()) {
log.warn("LDAP authentication is not enabled");
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.disabled");
}
+ // Validate all LDAP attribute names that flow into JNDI calls to prevent filter/attribute
+ // injection via operator misconfiguration. These names are operator-controlled, not user input.
+ validateAttributeNames();
+
log.debug("LDAP host: {}, base: {}, searchBase: {}, searchAttr: {}",
safeLogHost(ldapProperties.getUrl()),
ldapProperties.getBase(),
@@ -82,7 +103,7 @@ public class LdapAuthService {
// First, try to find the user in LDAP and authenticate
String userDn = findUserDn(username);
- log.debug("LDAP findUserDn result for {}: {}", username, userDn);
+ log.debug("LDAP findUserDn result for {}: {}", username, userDn != null);
if (userDn == null) {
log.warn("User {} not found in LDAP directory", username);
@@ -104,10 +125,13 @@ public class LdapAuthService {
Attributes userAttributes = getUserAttributes(userDn);
if (userAttributes == null) {
log.error("Failed to fetch user attributes from LDAP for DN: {}", userDn);
- throw new AuthFlowException(HttpStatus.INTERNAL_SERVER_ERROR, "error.auth.ldap.fetchUserFailed");
+ // Bind already succeeded, so the credentials are valid. This is a transient directory
+ // failure; surface a 503 with the directoryUnavailable message instead of masking it
+ // as a 401 "invalid credentials" (which would mislead the user about the password).
+ throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.directoryUnavailable");
}
- // Find or create local user account
+ // Find or create local user account anchored on the stable LDAP subject
log.debug("Finding or creating local user account for username: {}", username);
UserAccount user = findOrCreateLdapUser(username, userAttributes);
@@ -143,12 +167,13 @@ public class LdapAuthService {
log.warn("Invalid username format for LDAP search: {}", username);
return null;
}
+ String searchAttr = ldapProperties.getUserSearchAttribute();
DirContext ctx = null;
javax.naming.NamingEnumeration
+ * Identity is anchored on the stable LDAP subject attribute (entryUUID/objectGUID)
+ * via {@link IdentityBinding}, not on the user's email. This prevents:
+ * These tests exercise the {@code findOrCreateLdapUser} / {@code ensureUserCanLogin} logic via
+ * reflection, with all repositories mocked, so they cover the security-critical behavior called out
+ * in the PR review (email-collision takeover, duplicate provisioning on repeat login, attribute
+ * synchronization, and disabled-account rejection) without requiring a live LDAP directory.
+ */
+class LdapAuthServiceTest {
+
+ private static final String SUBJECT = "entry-uuid-123";
+ private static final String EMAIL = "alice@example.com";
+ private static final String DISPLAY_NAME = "Alice";
+
+ private LdapProperties ldapProperties;
+ private UserAccountRepository userAccountRepository;
+ private UserRoleBindingRepository userRoleBindingRepository;
+ private GlobalNamespaceMembershipService globalNamespaceMembershipService;
+ private IdentityBindingRepository identityBindingRepository;
+ private LdapAuthService ldapAuthService;
+
+ @BeforeEach
+ void setUp() {
+ ldapProperties = new LdapProperties();
+ userAccountRepository = mock(UserAccountRepository.class);
+ userRoleBindingRepository = mock(UserRoleBindingRepository.class);
+ globalNamespaceMembershipService = mock(GlobalNamespaceMembershipService.class);
+ identityBindingRepository = mock(IdentityBindingRepository.class);
+ ldapAuthService = new LdapAuthService(
+ ldapProperties,
+ userAccountRepository,
+ userRoleBindingRepository,
+ globalNamespaceMembershipService,
+ identityBindingRepository);
+ }
+
+ /** Directory attributes: subject=entryUUID, email=mail, displayName=displayName. */
+ private static Attributes directoryAttributes(String subject, String email, String displayName) {
+ BasicAttributes attrs = new BasicAttributes();
+ attrs.put(new BasicAttribute("entryUUID", subject));
+ attrs.put(new BasicAttribute("mail", email));
+ attrs.put(new BasicAttribute("displayName", displayName));
+ return attrs;
+ }
+
+ private UserAccount invokeFindOrCreate(String username, Attributes attrs) throws Exception {
+ Method m = LdapAuthService.class.getDeclaredMethod("findOrCreateLdapUser", String.class, Attributes.class);
+ m.setAccessible(true);
+ return (UserAccount) m.invoke(ldapAuthService, username, attrs);
+ }
+
+ @Test
+ void firstLogin_provisionsNewAccountAndBindsSubject() throws Exception {
+ // Given — no existing binding and no email collision
+ given(identityBindingRepository.findByProviderCodeAndSubject("ldap", SUBJECT))
+ .willReturn(Optional.empty());
+ given(userAccountRepository.findByEmailIgnoreCase(EMAIL)).willReturn(Optional.empty());
+ given(userAccountRepository.save(any(UserAccount.class))).willAnswer(inv -> inv.getArgument(0));
+
+ // When
+ UserAccount created = invokeFindOrCreate("alice", directoryAttributes(SUBJECT, EMAIL, DISPLAY_NAME));
+
+ // Then — new active account bound to the LDAP subject; placeholder never used as identity key
+ assertThat(created.getStatus()).isEqualTo(UserStatus.ACTIVE);
+ assertThat(created.getDisplayName()).isEqualTo(DISPLAY_NAME);
+ assertThat(created.getEmail()).isEqualTo(EMAIL);
+ verify(globalNamespaceMembershipService).ensureMember(created.getId());
+ verify(identityBindingRepository).save(any(IdentityBinding.class));
+ }
+
+ @Test
+ void repeatLogin_hitsExistingBindingBySubject_noDuplicateAccount() throws Exception {
+ // Given — the LDAP subject is already bound to an account (prior login)
+ String existingUserId = "usr_existing";
+ UserAccount existing = new UserAccount(existingUserId, "Old Name", EMAIL, null);
+ existing.setStatus(UserStatus.ACTIVE);
+ IdentityBinding binding = new IdentityBinding(existingUserId, "ldap", SUBJECT, "alice");
+ given(identityBindingRepository.findByProviderCodeAndSubject("ldap", SUBJECT))
+ .willReturn(Optional.of(binding));
+ given(userAccountRepository.findById(existingUserId)).willReturn(Optional.of(existing));
+ given(userAccountRepository.save(any(UserAccount.class))).willAnswer(inv -> inv.getArgument(0));
+
+ // When — same subject logs in again
+ UserAccount result = invokeFindOrCreate("alice", directoryAttributes(SUBJECT, EMAIL, DISPLAY_NAME));
+
+ // Then — returns the same account, never provisions a new one
+ assertThat(result.getId()).isEqualTo(existingUserId);
+ verify(userAccountRepository, never()).save(org.mockito.ArgumentMatchers.argThat(
+ u -> !existingUserId.equals(u.getId())));
+ // Critical: no new binding written on repeat login
+ verify(identityBindingRepository, never()).save(any(IdentityBinding.class));
+ }
+
+ @Test
+ void repeatLogin_refreshesAttributesFromDirectory() throws Exception {
+ // Given — a returning user whose display name and email changed in the directory
+ String userId = "usr_alice";
+ UserAccount existing = new UserAccount(userId, "Old Name", "old@example.com", null);
+ existing.setStatus(UserStatus.ACTIVE);
+ given(identityBindingRepository.findByProviderCodeAndSubject("ldap", SUBJECT))
+ .willReturn(Optional.of(new IdentityBinding(userId, "ldap", SUBJECT, "alice")));
+ given(userAccountRepository.findById(userId)).willReturn(Optional.of(existing));
+ given(userAccountRepository.save(any(UserAccount.class))).willAnswer(inv -> inv.getArgument(0));
+
+ // When — directory now reports a new display name and email
+ UserAccount result = invokeFindOrCreate("alice",
+ directoryAttributes(SUBJECT, "new@example.com", "New Name"));
+
+ // Then — attributes are refreshed on this login (not only at first creation)
+ assertThat(result.getDisplayName()).isEqualTo("New Name");
+ assertThat(result.getEmail()).isEqualTo("new@example.com");
+ }
+
+ @Test
+ void emailCollision_refusesSilentInheritance_throwsConflict() throws Exception {
+ // Given — a different identity provider already owns this email
+ String otherUserId = "usr_oauth";
+ given(identityBindingRepository.findByProviderCodeAndSubject("ldap", SUBJECT))
+ .willReturn(Optional.empty()); // no LDAP binding yet
+ given(userAccountRepository.findByEmailIgnoreCase(EMAIL))
+ .willReturn(Optional.of(new UserAccount(otherUserId, "OAuth User", EMAIL, null)));
+
+ // When — must NOT silently inherit the OAuth account / its roles.
+ // Reflection wraps checked exceptions in InvocationTargetException, so unwrap and assert
+ // the inner AuthFlowException carries a 409 CONFLICT with the emailConflict message key.
+ AuthFlowException thrown = null;
+ try {
+ invokeFindOrCreate("alice", directoryAttributes(SUBJECT, EMAIL, DISPLAY_NAME));
+ } catch (java.lang.reflect.InvocationTargetException ite) {
+ thrown = (AuthFlowException) ite.getCause();
+ }
+ assertThat(thrown).isNotNull();
+ assertThat(thrown.getStatus()).isEqualTo(HttpStatus.CONFLICT);
+ assertThat(thrown.getMessageCode()).isEqualTo("error.auth.ldap.emailConflict");
+
+ // No account created, no binding written
+ verify(userAccountRepository, never()).save(any(UserAccount.class));
+ verify(identityBindingRepository, never()).save(any(IdentityBinding.class));
+ }
+
+ @Test
+ void noEmail_usesPlaceholderAccount_doesNotCollideAcrossLogins() throws Exception {
+ // Given — directory entry has no mail attribute; subject is the only stable key
+ given(identityBindingRepository.findByProviderCodeAndSubject("ldap", SUBJECT))
+ .willReturn(Optional.empty());
+ // No email -> no email-collision lookup happens; placeholder email is generated
+ given(userAccountRepository.save(any(UserAccount.class))).willAnswer(inv -> inv.getArgument(0));
+
+ BasicAttributes attrs = new BasicAttributes();
+ attrs.put(new BasicAttribute("entryUUID", SUBJECT));
+ attrs.put(new BasicAttribute("displayName", DISPLAY_NAME));
+
+ // When
+ UserAccount created = invokeFindOrCreate("bob", attrs);
+
+ // Then — placeholder email follows the ldap:{username}@internal convention
+ assertThat(created.getEmail()).isEqualTo("ldap:bob@internal");
+ verify(userAccountRepository, never()).findByEmailIgnoreCase(any());
+ verify(identityBindingRepository).save(any(IdentityBinding.class));
+ }
+
+ @Test
+ void missingSubjectAttribute_throwsServiceUnavailable() {
+ // Given — bind succeeded but the entry lacks the configured subject attribute
+ BasicAttributes attrs = new BasicAttributes();
+ attrs.put(new BasicAttribute("mail", EMAIL));
+
+ // When & Then — a 503 (not a 401) so the user is not misled into thinking the password is wrong
+ assertThatThrownBy(() -> invokeFindOrCreate("alice", attrs))
+ .hasCauseInstanceOf(AuthFlowException.class);
+ try {
+ invokeFindOrCreate("alice", attrs);
+ } catch (java.lang.reflect.InvocationTargetException ite) {
+ AuthFlowException cause = (AuthFlowException) ite.getCause();
+ assertThat(cause.getStatus()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE);
+ } catch (Throwable t) {
+ throw new AssertionError(t);
+ }
+ }
+
+ @Test
+ void ensureUserCanLogin_rejectsDisabledAccount() throws Exception {
+ // The disabled-account path must surface a FORBIDDEN (propagated, not masked as 401)
+ UserAccount disabled = new UserAccount("usr_x", "X", "x@example.com", null);
+ disabled.setStatus(UserStatus.DISABLED);
+
+ Method m = LdapAuthService.class.getDeclaredMethod("ensureUserCanLogin", UserAccount.class);
+ m.setAccessible(true);
+ try {
+ m.invoke(ldapAuthService, disabled);
+ } catch (java.lang.reflect.InvocationTargetException ite) {
+ AuthFlowException cause = (AuthFlowException) ite.getCause();
+ assertThat(cause.getStatus()).isEqualTo(HttpStatus.FORBIDDEN);
+ assertThat(cause.getMessageCode()).isEqualTo("error.auth.local.accountDisabled");
+ }
+ }
+
+ @Test
+ void displayNameFallsBackToCn_whenDisplayNameAttributeAbsent() throws Exception {
+ // Given — directory has no displayName but has cn; configured fallback defaults to "cn"
+ assertThat(ldapProperties.getDisplayNameFallbackAttribute()).isEqualTo("cn");
+ given(identityBindingRepository.findByProviderCodeAndSubject("ldap", SUBJECT))
+ .willReturn(Optional.empty());
+ given(userAccountRepository.save(any(UserAccount.class))).willAnswer(inv -> inv.getArgument(0));
+
+ BasicAttributes attrs = new BasicAttributes();
+ attrs.put(new BasicAttribute("entryUUID", SUBJECT));
+ attrs.put(new BasicAttribute("cn", "Common Name"));
+
+ // When
+ UserAccount created = invokeFindOrCreate("carol", attrs);
+
+ // Then — display name falls back to the configured cn attribute
+ assertThat(created.getDisplayName()).isEqualTo("Common Name");
+ }
+
+ @Test
+ void isTlsFailure_detectsSslHandshakeInCauseChain() {
+ javax.naming.CommunicationException comm = new javax.naming.CommunicationException("LDAP connect failed");
+ comm.initCause(new javax.net.ssl.SSLHandshakeException("PKIX path building failed"));
+
+ assertThat(LdapAuthService.isTlsFailure(comm)).isTrue();
+ }
+
+ @Test
+ void isTlsFailure_detectsDeepCertificateException() {
+ javax.naming.CommunicationException comm = new javax.naming.CommunicationException("LDAP connect failed");
+ comm.initCause(new java.io.IOException("TLS handshake failed",
+ new java.security.cert.CertificateException("not trusted")));
+
+ assertThat(LdapAuthService.isTlsFailure(comm)).isTrue();
+ }
+
+ @Test
+ void isTlsFailure_ignoresPlainConnectionFailures() {
+ javax.naming.CommunicationException comm = new javax.naming.CommunicationException("LDAP connect failed");
+ comm.initCause(new java.io.IOException("Connection refused"));
+
+ assertThat(LdapAuthService.isTlsFailure(comm)).isFalse();
+ }
+}
diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/ldap/LdapSubjectGuidTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/ldap/LdapSubjectGuidTest.java
new file mode 100644
index 00000000..d64928bc
--- /dev/null
+++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/ldap/LdapSubjectGuidTest.java
@@ -0,0 +1,65 @@
+package com.iflytek.skillhub.auth.ldap;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.lang.reflect.Method;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression coverage for AD objectGUID binary normalization.
+ *
+ * Active Directory stores objectGUID as a 16-byte mixed-endian OctetString. Before the fix,
+ * {@code getAttributeValue} called {@code toString()} on the {@code byte[]} returned by JNDI,
+ * producing an unstable {@code "[B@
+ *
*/
private UserAccount findOrCreateLdapUser(String username, Attributes attributes) {
- String email = getAttributeValue(attributes, "mail");
- String displayName = getAttributeValue(attributes, "displayName");
-
- if (displayName == null || displayName.isEmpty()) {
- displayName = getAttributeValue(attributes, "cn");
- }
+ String subject = getAttributeValue(attributes, ldapProperties.getSubjectAttribute());
+ String email = getAttributeValue(attributes, ldapProperties.getEmailAttribute());
+ String displayName = getAttributeValue(attributes, ldapProperties.getDisplayNameAttribute());
+ if (displayName == null || displayName.isEmpty()) {
+ displayName = getAttributeValue(attributes, ldapProperties.getDisplayNameFallbackAttribute());
+ }
if (displayName == null || displayName.isEmpty()) {
displayName = username;
}
- UserAccount user = null;
+ if (subject == null || subject.isEmpty()) {
+ log.error("LDAP entry for {} has no stable subject attribute '{}'; cannot bind identity",
+ username, ldapProperties.getSubjectAttribute());
+ // Bind already succeeded. The directory entry lacks the configured subject attribute,
+ // which is a configuration/schema issue the user cannot fix. Surface a 503 with the
+ // fetchUserFailed message (no "retry later" wording) instead of a 401 that would look
+ // like a wrong password. Operators can locate the cause via the log line above.
+ throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.fetchUserFailed");
+ }
- // Try to find by email first
+ // Anchor on the stable LDAP subject: an existing binding means this identity is already known.
+ IdentityBinding binding = identityBindingRepository
+ .findByProviderCodeAndSubject(LDAP_PROVIDER, subject)
+ .orElse(null);
+
+ if (binding != null) {
+ // Returning user — refresh attributes from the directory on each login.
+ UserAccount user = userAccountRepository.findById(binding.getUserId())
+ .orElseThrow(() -> new IllegalStateException("User not found for LDAP binding " + subject));
+ updateFromAttributes(user, displayName, email);
+ return userAccountRepository.save(user);
+ }
+
+ // First login for this LDAP identity. Refuse to silently inherit an existing local/OAuth
+ // account that happens to share the same email — that would be a privilege escalation.
if (email != null && !email.isEmpty()) {
- user = userAccountRepository.findByEmailIgnoreCase(email.toLowerCase()).orElse(null);
+ String normalizedEmail = email.toLowerCase();
+ UserAccount existingByEmail = userAccountRepository
+ .findByEmailIgnoreCase(normalizedEmail).orElse(null);
+ if (existingByEmail != null) {
+ // The email already belongs to another account. Refuse to silently create a second
+ // account (two distinct LDAP subjects sharing one email would both map to it, and
+ // user_account.email has no UNIQUE constraint, so this would otherwise happen
+ // silently). This covers both cross-provider collisions and the same-issuer case
+ // (a different LDAP subject under the same email). If an entry's stable subject
+ // legitimately changes (e.g. after an AD objectGUID migration), an administrator
+ // must remove the stale binding before the new subject can log in.
+ log.warn("LDAP user {} email {} collides with an existing account {} (subject differs); refusing to create a duplicate account",
+ username, normalizedEmail, existingByEmail.getId());
+ throw new AuthFlowException(HttpStatus.CONFLICT, "error.auth.ldap.emailConflict");
+ }
}
- // If not found, create a new user
- if (user == null) {
- // For LDAP users without email, use "ldap:{username}@internal" as a unique identifier.
- // This format:
- // 1. Prevents duplicate accounts when email attribute is missing
- // 2. Clearly identifies the account origin (LDAP vs local)
- // 3. Follows email format to satisfy the email NOT NULL constraint
- String normalizedEmail = email != null ? email.toLowerCase() : "ldap:" + username + "@internal";
+ // Create a new user account. The placeholder email is only used to satisfy the NOT NULL
+ // constraint and never serves as an identity key.
+ String normalizedEmail = email != null && !email.isEmpty()
+ ? email.toLowerCase()
+ : LDAP_PROVIDER + ":" + username + "@internal";
- user = new UserAccount(
- "usr_" + UUID.randomUUID(),
- displayName,
- normalizedEmail,
- null
- );
- user.setStatus(UserStatus.ACTIVE);
- userAccountRepository.save(user);
- globalNamespaceMembershipService.ensureMember(user.getId());
- }
+ UserAccount user = new UserAccount(
+ "usr_" + UUID.randomUUID(),
+ displayName,
+ normalizedEmail,
+ null
+ );
+ user.setStatus(UserStatus.ACTIVE);
+ user = userAccountRepository.save(user);
+ globalNamespaceMembershipService.ensureMember(user.getId());
+
+ IdentityBinding newBinding = new IdentityBinding(user.getId(), LDAP_PROVIDER, subject, username);
+ identityBindingRepository.save(newBinding);
return user;
}
+ private void updateFromAttributes(UserAccount user, String displayName, String email) {
+ if (displayName != null && !displayName.isEmpty()) {
+ user.setDisplayName(displayName);
+ }
+ if (email != null && !email.isEmpty()) {
+ user.setEmail(email.toLowerCase());
+ }
+ }
+
/**
* Gets a string attribute value from LDAP attributes.
*/
@@ -341,7 +466,16 @@ public class LdapAuthService {
try {
Attribute attr = attributes.get(attrName);
if (attr != null && attr.get() != null) {
- return attr.get().toString();
+ Object value = attr.get();
+ // Active Directory stores stable identifiers such as objectGUID / objectSid as
+ // binary (OctetString). JNDI returns these as byte[], whose toString() yields an
+ // unstable "[B@