fix(auth): 修复 LDAP 身份模型、操作属性获取与错误分类

按 PR #437 review (CHANGES_REQUESTED + 改进方案) 系统修复 LDAP 登录安全与稳定性问题:

身份模型重构:
- 锚定稳定目录标识 (entryUUID/objectGUID) 而非输入侧 username,复用 IdentityBinding 绑定,首次登录持久化、重复登录按 subject 命中,避免重复建号
- 禁止邮箱静默合并/继承:邮箱已被任意账号占用即抛 409 (跨 provider 与同 issuer 双 subject 均拒绝);ldap:{username}@internal 降级为纯占位,不承担身份键
- 规范化 AD objectGUID 二进制属性为稳定 GUID 字符串 (混合字节序),并补回归单测

操作属性获取:
- getUserAttributes 显式请求 "*"+"+" 属性集,OpenLDAP entryUUID / AD objectGUID 等操作属性可正常返回,默认 subject 配置不再导致登录 503;目录拒绝该语法时回退默认属性集

安全与错误分类:
- 属性名白名单校验防 JNDI 注入,isValidUsername 改预编译 Pattern
- 区分 TLS/证书错误与目录不可用 (isTlsFailure 遍历 cause 链),新增 error.auth.ldap.tlsError 中英文消息键
- bind 成功后的目录错误由 500 折叠 401 改为透传 503;LocalAuthService 透传 403/503/409,不再误导为密码错误
- displayName fallback 与超时参数可配置化 (默认 cn / 5s+10s);修正 application.yml LDAP 配置段缩进

架构清理:
- LdapAuthService 改用 JNDI 直连,移除 LdapTemplate 死依赖与失效配置;LdapAuthService 条件化、LocalAuthService 改 ObjectProvider 可选注入

测试补齐 (单测):
- LdapAuthServiceTest 行为单测:邮箱碰撞拒绝、重复登录按 subject 命中不重复建号、属性更新刷新、禁用账号拒绝、subject 缺失 503、cn fallback
- LdapSubjectGuidTest:objectGUID 二进制规范化回归
- LdapAuthServiceTest isTlsFailure 分类单测;LocalAuthServiceTest 错误传播单测

Signed-off-by: jangrui <admin@jangrui.com>
This commit is contained in:
jangrui 2026-08-01 10:51:38 +08:00
parent 9e178711fe
commit fe4784861f
11 changed files with 795 additions and 158 deletions

View file

@ -15,10 +15,6 @@ spring:
basename: messages
application:
name: skillhub
ldap:
# Prevent spring-boot-starter-data-ldap AutoConfiguration from attempting
# to initialize LDAP connections when LDAP is not configured
urls: ""
lifecycle:
timeout-per-shutdown-phase: 30s
jpa:
@ -118,6 +114,13 @@ skillhub:
password: ${SKILLHUB_LDAP_PASSWORD:}
user-search-attribute: ${SKILLHUB_LDAP_USER_SEARCH_ATTRIBUTE:uid}
user-search-base: ${SKILLHUB_LDAP_USER_SEARCH_BASE:}
# Stable directory identifier used as the LDAP identity subject (entryUUID for OpenLDAP, objectGUID for AD)
subject-attribute: ${SKILLHUB_LDAP_SUBJECT_ATTRIBUTE:entryUUID}
display-name-attribute: ${SKILLHUB_LDAP_DISPLAY_NAME_ATTRIBUTE:displayName}
display-name-fallback-attribute: ${SKILLHUB_LDAP_DISPLAY_NAME_FALLBACK_ATTRIBUTE:cn}
email-attribute: ${SKILLHUB_LDAP_EMAIL_ATTRIBUTE:mail}
connect-timeout-millis: ${SKILLHUB_LDAP_CONNECT_TIMEOUT_MILLIS:5000}
read-timeout-millis: ${SKILLHUB_LDAP_READ_TIMEOUT_MILLIS:10000}
public:
base-url: ${SKILLHUB_PUBLIC_BASE_URL:}
access-policy:

View file

@ -46,6 +46,13 @@ error.auth.direct.providerUnsupported=Unsupported direct authentication provider
error.auth.sessionBootstrap.disabled=Session bootstrap is disabled
error.auth.sessionBootstrap.providerUnsupported=Unsupported session bootstrap provider: {0}
error.auth.sessionBootstrap.notAuthenticated=No authenticated external session found
error.auth.ldap.disabled=LDAP authentication is not enabled
error.auth.ldap.userNotFound=Invalid username or password
error.auth.ldap.invalidCredentials=Invalid username or password
error.auth.ldap.fetchUserFailed=Failed to retrieve user information from the directory server
error.auth.ldap.directoryUnavailable=The directory server is temporarily unavailable. Please try again later
error.auth.ldap.tlsError=Failed to establish a secure connection to the directory server. Please check the TLS certificate configuration
error.auth.ldap.emailConflict=This email is already associated with an existing account. Please contact an administrator
error.badRequest=Invalid request
error.forbidden=Forbidden
error.request.timeout=Request timed out

View file

@ -46,6 +46,13 @@ error.auth.direct.providerUnsupported=不支持的直连认证提供方:{0}
error.auth.sessionBootstrap.disabled=会话引导能力未启用
error.auth.sessionBootstrap.providerUnsupported=不支持的会话引导提供方:{0}
error.auth.sessionBootstrap.notAuthenticated=未检测到已认证的外部会话
error.auth.ldap.disabled=LDAP 认证未启用
error.auth.ldap.userNotFound=用户名或密码错误
error.auth.ldap.invalidCredentials=用户名或密码错误
error.auth.ldap.fetchUserFailed=从目录服务器获取用户信息失败
error.auth.ldap.directoryUnavailable=目录服务器暂时不可用,请稍后重试
error.auth.ldap.tlsError=无法与目录服务器建立安全连接,请检查 TLS 证书配置
error.auth.ldap.emailConflict=该邮箱已关联已有账号,请联系管理员处理
error.badRequest=请求参数不合法
error.forbidden=没有权限执行该操作
error.request.timeout=请求超时

View file

@ -44,14 +44,6 @@
<artifactId>spring-boot-configuration-processor</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-ldap</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.ldap</groupId>
<artifactId>spring-ldap-core</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>

View file

@ -1,40 +1,20 @@
package com.iflytek.skillhub.auth.config;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.ldap.core.LdapTemplate;
import org.springframework.ldap.core.support.LdapContextSource;
/**
* LDAP auto-configuration that creates LdapTemplate only when LDAP is enabled.
* This avoids the startup side effects of spring-boot-starter-data-ldap's
* auto-configuration when LDAP is disabled.
* LDAP configuration marker.
* <p>
* 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}.
* <p>
* {@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);
}
}

View file

@ -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;
}
}
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;
}
}

View file

@ -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.
* <p>
* 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<SearchResult> results = null;
try {
ctx = createLdapContext();
String searchFilter = "(" + ldapProperties.getUserSearchAttribute() + "={0})";
String searchFilter = "(" + searchAttr + "={0})";
String searchBase = ldapProperties.getUserSearchBase().isEmpty()
? ldapProperties.getBase()
: ldapProperties.getUserSearchBase() + "," + ldapProperties.getBase();
@ -164,7 +189,18 @@ public class LdapAuthService {
return result.getNameInNamespace();
}
return null;
} catch (Exception e) {
} catch (CommunicationException e) {
if (isTlsFailure(e)) {
log.warn("LDAP TLS/certificate failure while searching for user {}: {}", username, e.getMessage());
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.tlsError");
}
log.warn("LDAP directory unavailable while searching for user {}: {}", username, e.getMessage());
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.directoryUnavailable");
} catch (AuthenticationException e) {
log.warn("LDAP bind authentication failed while searching for user {}: {}", username, e.getMessage());
throw new AuthFlowException(HttpStatus.UNAUTHORIZED, "error.auth.ldap.invalidCredentials");
} catch (NamingException e) {
log.warn("LDAP naming error while searching for user {}: {}", username, e.getMessage());
return null;
} finally {
// Close NamingEnumeration to prevent resource leaks
@ -183,20 +219,31 @@ public class LdapAuthService {
* Creates an LDAP context for searching.
*/
private DirContext createLdapContext() throws NamingException {
// Bind-account context for directory searches/reads; delegates to the shared factory.
return createLdapContext(null, null);
}
/**
* Creates an LDAP context authenticated with the given principal/credentials. When both are
{@code null}, falls back to the configured bind account (or anonymous if none is set). This is
the single place that builds the JNDI environment, so connection/timeout settings stay
consistent across search, bind, and attribute-read operations.
*/
private DirContext createLdapContext(String principal, String credentials) throws NamingException {
Hashtable<String, String> env = new Hashtable<>();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
env.put(Context.PROVIDER_URL, ldapProperties.getUrl());
env.put(Context.SECURITY_AUTHENTICATION, "simple");
// Connection timeout: 5 seconds for connect, 10 seconds for read
env.put("com.sun.jndi.ldap.connect.timeout", "5000");
env.put("com.sun.jndi.ldap.read.timeout", "10000");
if (ldapProperties.getUsername() != null && !ldapProperties.getUsername().isEmpty()) {
env.put(Context.SECURITY_PRINCIPAL, ldapProperties.getUsername());
env.put(Context.SECURITY_CREDENTIALS, ldapProperties.getPassword());
// Connection timeout: configurable (default 5 seconds for connect, 10 for read)
env.put("com.sun.jndi.ldap.connect.timeout", String.valueOf(ldapProperties.getConnectTimeoutMillis()));
env.put("com.sun.jndi.ldap.read.timeout", String.valueOf(ldapProperties.getReadTimeoutMillis()));
// Explicit principal/credentials take precedence; otherwise use the configured bind account.
String bindPrincipal = (principal != null) ? principal : ldapProperties.getUsername();
String bindCredentials = (principal != null) ? credentials : ldapProperties.getPassword();
if (bindPrincipal != null && !bindPrincipal.isEmpty()) {
env.put(Context.SECURITY_PRINCIPAL, bindPrincipal);
env.put(Context.SECURITY_CREDENTIALS, bindCredentials);
}
return new InitialDirContext(env);
}
@ -214,46 +261,67 @@ public class LdapAuthService {
}
/**
* Safely extracts host from LDAP URL for logging, avoiding credential exposure.
* Safely extracts host:port from LDAP URL for logging, avoiding credential exposure.
* Handles formats like: ldap://host:389, ldap://user:pass@host:389, ldaps://host
*/
private String safeLogHost(String url) {
public static String safeLogHost(String url) {
if (url == null || url.isEmpty()) {
return "";
}
try {
// Remove protocol prefix
String withoutProtocol = url.replaceFirst("^ldaps?://", "");
// Extract host:port or just host
int atIndex = withoutProtocol.indexOf('@');
if (atIndex > 0) {
withoutProtocol = withoutProtocol.substring(atIndex + 1);
}
int colonIndex = withoutProtocol.indexOf(':');
return colonIndex > 0 ? withoutProtocol.substring(0, colonIndex) : withoutProtocol;
// IPv6 literal: ldap://[::1]:389 — keep the bracketed address plus port
int bracketEnd = withoutProtocol.indexOf(']');
if (bracketEnd > 0) {
return withoutProtocol.substring(0, Math.min(bracketEnd + 1, withoutProtocol.length()));
}
int slashIndex = withoutProtocol.indexOf('/');
String hostPort = slashIndex > 0 ? withoutProtocol.substring(0, slashIndex) : withoutProtocol;
return hostPort;
} catch (Exception e) {
return "[url-parse-error]";
}
}
/**
* Returns whether the throwable chain indicates a TLS/trust failure (LDAPS handshake or
* certificate validation). JNDI wraps TLS failures in a {@link CommunicationException}, so
* without this check a certificate problem is indistinguishable from an unreachable directory.
*/
static boolean isTlsFailure(Throwable t) {
for (Throwable c = t; c != null; c = c.getCause()) {
if (c instanceof SSLException || c instanceof CertificateException) {
return true;
}
}
return false;
}
/**
* Authenticates a user against the LDAP server using their DN and password.
*/
private boolean authenticateLdap(String userDn, String password) {
DirContext ctx = null;
try {
Hashtable<String, String> env = new Hashtable<>();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
env.put(Context.PROVIDER_URL, ldapProperties.getUrl());
env.put(Context.SECURITY_AUTHENTICATION, "simple");
env.put(Context.SECURITY_PRINCIPAL, userDn);
env.put(Context.SECURITY_CREDENTIALS, password);
env.put("com.sun.jndi.ldap.connect.timeout", "5000");
env.put("com.sun.jndi.ldap.read.timeout", "10000");
ctx = new InitialDirContext(env);
// Bind as the authenticating user to verify credentials (shared factory handles env).
ctx = createLdapContext(userDn, password);
return true;
} catch (AuthenticationException e) {
// Invalid credentials — expected, return false to signal auth failure
return false;
} catch (CommunicationException e) {
if (isTlsFailure(e)) {
log.warn("LDAP TLS/certificate failure while authenticating DN {}: {}", userDn, e.getMessage());
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.tlsError");
}
log.warn("LDAP directory unavailable while authenticating DN {}: {}", userDn, e.getMessage());
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.directoryUnavailable");
} catch (NamingException e) {
log.warn("LDAP naming error while authenticating DN {}: {}", userDn, e.getMessage());
return false;
} finally {
closeContext(ctx);
@ -266,23 +334,30 @@ public class LdapAuthService {
private Attributes getUserAttributes(String userDn) {
DirContext ctx = null;
try {
Hashtable<String, String> env = new Hashtable<>();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
env.put(Context.PROVIDER_URL, ldapProperties.getUrl());
env.put(Context.SECURITY_AUTHENTICATION, "simple");
env.put("com.sun.jndi.ldap.connect.timeout", "5000");
env.put("com.sun.jndi.ldap.read.timeout", "10000");
// Use bind DN if configured, otherwise anonymous bind
if (ldapProperties.getUsername() != null && !ldapProperties.getUsername().isEmpty()) {
env.put(Context.SECURITY_PRINCIPAL, ldapProperties.getUsername());
env.put(Context.SECURITY_CREDENTIALS, ldapProperties.getPassword());
// Read attributes via the bind-account context (shared factory handles env/timeout).
ctx = createLdapContext();
Attributes attrs;
try {
// Request user attributes ("*") and operational attributes ("+") so stable
// directory identifiers (OpenLDAP entryUUID, AD objectGUID) are included in the
// response. Without the explicit request, operational attributes are omitted and
// the subject key would be null on every login.
attrs = ctx.getAttributes(new LdapName(userDn), new String[]{"*", "+"});
} catch (Exception e) {
// Some directories reject the "*"/"+" attribute-request syntax; fall back to the
// default attribute set instead of failing the whole login.
attrs = ctx.getAttributes(new LdapName(userDn));
}
ctx = new InitialDirContext(env);
Attributes attrs = ctx.getAttributes(new LdapName(userDn));
return attrs;
} catch (CommunicationException e) {
if (isTlsFailure(e)) {
log.warn("LDAP TLS/certificate failure while fetching attributes for DN {}: {}", userDn, e.getMessage());
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.tlsError");
}
log.warn("LDAP directory unavailable while fetching attributes for DN {}: {}", userDn, e.getMessage());
throw new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.directoryUnavailable");
} catch (Exception e) {
log.warn("Failed to fetch user attributes from LDAP for DN {}: {}", userDn, e.getMessage());
return null;
} finally {
closeContext(ctx);
@ -291,49 +366,99 @@ public class LdapAuthService {
/**
* Finds an existing LDAP user or creates a new one based on LDAP attributes.
* Note: This method only finds users by email, as the repository doesn't support displayName lookup.
* <p>
* Identity is anchored on the stable LDAP subject attribute (entryUUID/objectGUID)
* via {@link IdentityBinding}, not on the user's email. This prevents:
* <ul>
* <li>Silent account merging when an LDAP email collides with a local/OAuth account</li>
* <li>Duplicate accounts for email-less LDAP users on repeated logins</li>
* </ul>
*/
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@<identityHashCode>" — making every login look like a new identity.
// Convert binary values to a stable hexadecimal representation (mixed-endian GUID
// layout for 16-byte values) so the subject key remains stable across logins.
if (value instanceof byte[] bytes) {
return toStableGuidString(bytes);
}
return value.toString();
}
} catch (Exception e) {
// Ignore and return null
@ -349,6 +483,32 @@ public class LdapAuthService {
return null;
}
/**
* Converts a binary attribute value into a stable string suitable for use as an identity
* subject. Active Directory objectGUID is a 16-byte mixed-endian GUID; rearranging it into
* the canonical 8-4-4-4-12 hex layout yields the same string .NET/AD display, which is stable
* across JVM restarts and connections. Non-16-byte binaries fall back to plain hex so the
* value is still deterministic.
*/
private static String toStableGuidString(byte[] bytes) {
if (bytes.length == 16) {
// AD objectGUID layout: little-endian uint32, little-endian uint16 x2, big-endian rest.
return String.format("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
bytes[3] & 0xff, bytes[2] & 0xff, bytes[1] & 0xff, bytes[0] & 0xff,
bytes[5] & 0xff, bytes[4] & 0xff,
bytes[7] & 0xff, bytes[6] & 0xff,
bytes[8] & 0xff, bytes[9] & 0xff,
bytes[10] & 0xff, bytes[11] & 0xff, bytes[12] & 0xff,
bytes[13] & 0xff, bytes[14] & 0xff, bytes[15] & 0xff);
}
// Non-GUID binary attribute: deterministic plain hex so the value stays stable.
StringBuilder sb = new StringBuilder(bytes.length * 2);
for (byte b : bytes) {
sb.append(String.format("%02x", b & 0xff));
}
return sb.toString();
}
/**
* Builds a PlatformPrincipal from a UserAccount.
*/
@ -369,13 +529,33 @@ public class LdapAuthService {
/**
* Validates username to prevent LDAP injection attacks.
* Allows only alphanumeric characters and underscores, 3-64 characters.
* Allows alphanumeric, underscore, hyphen, dot, and @ (for UPN formats),
* 3-64 characters.
*/
private boolean isValidUsername(String username) {
if (username == null || username.isEmpty()) {
return false;
}
// Allow alphanumeric, underscore, hyphen, dot, and @ for UPN formats
return username.matches("^[A-Za-z0-9_@.\\-]{3,64}$");
return USERNAME_PATTERN.matcher(username).matches();
}
/**
* Validates all operator-configured LDAP attribute names that flow into JNDI calls.
* Rejects names that are null or fail the LDAP attribute-name pattern, preventing
* filter/attribute injection via misconfiguration before any directory call is made.
*/
private void validateAttributeNames() {
String[] attrNames = {
ldapProperties.getUserSearchAttribute(),
ldapProperties.getSubjectAttribute(),
ldapProperties.getDisplayNameAttribute(),
ldapProperties.getEmailAttribute()
};
for (String name : attrNames) {
if (name == null || !ATTRIBUTE_NAME_PATTERN.matcher(name).matches()) {
log.error("Invalid LDAP attribute name configured: {}", name);
throw new AuthFlowException(HttpStatus.INTERNAL_SERVER_ERROR, "error.auth.ldap.fetchUserFailed");
}
}
}
}

View file

@ -23,6 +23,7 @@ import org.slf4j.LoggerFactory;
import org.springframework.http.HttpStatus;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.transaction.annotation.Transactional;
/**
@ -51,7 +52,7 @@ public class LocalAuthService {
private final PasswordEncoder passwordEncoder;
private final Clock clock;
private final LdapProperties ldapProperties;
private final LdapAuthService ldapAuthService;
private final ObjectProvider<LdapAuthService> ldapAuthServiceProvider;
public LocalAuthService(LocalCredentialRepository credentialRepository,
UserAccountRepository userAccountRepository,
@ -61,7 +62,7 @@ public class LocalAuthService {
PasswordEncoder passwordEncoder,
Clock clock,
LdapProperties ldapProperties,
LdapAuthService ldapAuthService) {
ObjectProvider<LdapAuthService> ldapAuthServiceProvider) {
this.credentialRepository = credentialRepository;
this.userAccountRepository = userAccountRepository;
this.userRoleBindingRepository = userRoleBindingRepository;
@ -70,7 +71,7 @@ public class LocalAuthService {
this.passwordEncoder = passwordEncoder;
this.clock = clock;
this.ldapProperties = ldapProperties;
this.ldapAuthService = ldapAuthService;
this.ldapAuthServiceProvider = ldapAuthServiceProvider;
}
/**
@ -133,10 +134,15 @@ public class LocalAuthService {
// Fallback to LDAP authentication if enabled
if (ldapProperties.isEnabled()) {
LdapAuthService ldapAuthService = ldapAuthServiceProvider.getIfAvailable();
if (ldapAuthService == null) {
log.warn("LDAP is enabled but LdapAuthService bean is unavailable; rejecting login for username: {}", username);
throw invalidCredentials();
}
log.info("Local user not found, attempting LDAP authentication for username: {}", username);
log.debug("LDAP enabled: {}, host: {}, base: {}",
ldapProperties.isEnabled(),
safeLogHost(ldapProperties.getUrl()),
LdapAuthService.safeLogHost(ldapProperties.getUrl()),
ldapProperties.getBase());
try {
PlatformPrincipal ldapPrincipal = ldapAuthService.login(username, password);
@ -144,7 +150,17 @@ public class LocalAuthService {
return ldapPrincipal;
} catch (AuthFlowException e) {
log.warn("LDAP authentication failed for username: {}, error: {}", username, e.getMessage());
// LDAP authentication failed, throw invalid credentials
// Propagate account-state, availability, and email-conflict errors instead of
// masking them as invalid credentials, so the frontend can show the right message.
// A 409 emailConflict is only reached after a successful LDAP bind, so the
// credentials are valid; masking it as 401 would mislead the user into thinking
// their password is wrong. Only genuine credential failures (userNotFound /
// invalidCredentials) fall back to the generic response to avoid enumeration.
HttpStatus status = e.getStatus();
if (status == HttpStatus.FORBIDDEN || status == HttpStatus.SERVICE_UNAVAILABLE
|| status == HttpStatus.CONFLICT) {
throw e;
}
throw invalidCredentials();
}
} else {
@ -271,24 +287,4 @@ public class LocalAuthService {
throw new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.local.email.invalid");
}
}
/**
* Safely extracts host from LDAP URL for logging, avoiding credential exposure.
*/
private static String safeLogHost(String url) {
if (url == null || url.isEmpty()) {
return "";
}
try {
String withoutProtocol = url.replaceFirst("^ldaps?://", "");
int atIndex = withoutProtocol.indexOf('@');
if (atIndex > 0) {
withoutProtocol = withoutProtocol.substring(atIndex + 1);
}
int colonIndex = withoutProtocol.indexOf(':');
return colonIndex > 0 ? withoutProtocol.substring(0, colonIndex) : withoutProtocol;
} catch (Exception e) {
return "[url-parse-error]";
}
}
}

View file

@ -0,0 +1,271 @@
package com.iflytek.skillhub.auth.ldap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
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.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 com.iflytek.skillhub.auth.repository.IdentityBindingRepository;
import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository;
import java.lang.reflect.Method;
import java.util.Optional;
import javax.naming.directory.Attributes;
import javax.naming.directory.BasicAttribute;
import javax.naming.directory.BasicAttributes;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
/**
* Behavior-level unit tests for {@link LdapAuthService} identity provisioning.
*
* <p>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();
}
}

View file

@ -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.
*
* <p>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@<identityHashCode>"} that changed on every login and caused
* each login to provision a brand-new account. These tests lock in the stable canonical-GUID
* conversion via reflection on the private {@code toStableGuidString} helper.
*/
class LdapSubjectGuidTest {
private static String invoke(byte[] bytes) throws Exception {
Method m = LdapAuthService.class.getDeclaredMethod("toStableGuidString", byte[].class);
m.setAccessible(true);
return (String) m.invoke(null, bytes);
}
@Test
void objectGuid_byteArray_isStableCanonicalGuid() throws Exception {
// AD objectGUID {0x8b,0xe3,0x9d,0x4c,...} little-endian -> 4c9de38b-...
// The same logical GUID must always serialize to the same string regardless of which
// byte[] instance JNDI handed back, so repeat-login identity matching stays stable.
byte[] guid = new byte[]{
(byte) 0x8b, (byte) 0xe3, (byte) 0x9d, 0x4c, // LE uint32 -> 4c9de38b
(byte) 0xb5, 0x55, // LE uint16 -> 55b5
(byte) 0xe8, 0x42, // LE uint16 -> 42e8
(byte) 0x8e, 0x2f, // big-endian -> 8e2f
(byte) 0x9a, 0x41, (byte) 0xc3, 0x77, (byte) 0xa6, 0x71 // node -> 9a41c377a671
};
String first = invoke(guid);
String second = invoke(guid.clone()); // different instance, same bytes
assertThat(first).isEqualTo("4c9de38b-55b5-42e8-8e2f-9a41c377a671");
assertThat(second).isEqualTo(first); // deterministic across instances
}
@Test
void nonGuidByteArray_fallsBackToDeterministicHex() throws Exception {
byte[] sid = new byte[]{0x01, 0x00, 0x04, (byte) 0x80, 0x14, 0x00, 0x00, 0x00};
String first = invoke(sid);
String second = invoke(sid.clone());
assertThat(first).isEqualTo("0100048014000000");
assertThat(second).isEqualTo(first); // stable even for non-GUID binaries
}
@Test
void sameGuid_differentInstances_produceSameString() throws Exception {
// The core regression: two independent byte[] with identical content must yield identical
// strings, proving identity matching will be stable across LDAP connections.
byte[] a = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
byte[] b = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
assertThat(invoke(a)).isEqualTo(invoke(b));
}
}

View file

@ -61,10 +61,14 @@ class LocalAuthServiceTest {
@Mock
private LdapAuthService ldapAuthService;
@Mock
private org.springframework.beans.factory.ObjectProvider<LdapAuthService> ldapAuthServiceProvider;
private LocalAuthService service;
@BeforeEach
void setUp() {
org.mockito.Mockito.lenient().when(ldapAuthServiceProvider.getIfAvailable()).thenReturn(ldapAuthService);
service = new LocalAuthService(
credentialRepository,
userAccountRepository,
@ -74,7 +78,7 @@ class LocalAuthServiceTest {
passwordEncoder,
CLOCK,
ldapProperties,
ldapAuthService
ldapAuthServiceProvider
);
}
@ -308,6 +312,56 @@ class LocalAuthServiceTest {
verify(ldapAuthService).login("ldapuser", "WrongPassword");
}
@Test
void login_withUnknownUsername_propagatesServiceUnavailable_whenLdapDirectoryDown() {
// Given — LDAP directory unavailable should surface as 503, not be masked as 401
given(credentialRepository.findByUsernameIgnoreCase("ldapuser")).willReturn(Optional.empty());
given(ldapProperties.isEnabled()).willReturn(true);
given(ldapAuthService.login("ldapuser", "password"))
.willThrow(new AuthFlowException(HttpStatus.SERVICE_UNAVAILABLE, "error.auth.ldap.directoryUnavailable"));
// When & Then — 503 must propagate so the frontend can show "try again later"
assertThatThrownBy(() -> service.login("ldapuser", "password"))
.isInstanceOf(AuthFlowException.class)
.extracting("status")
.isEqualTo(HttpStatus.SERVICE_UNAVAILABLE);
}
@Test
void login_withUnknownUsername_propagatesForbidden_whenLdapAccountDisabled() {
// Given — a disabled LDAP account (403) should propagate, not be masked as 401
given(credentialRepository.findByUsernameIgnoreCase("ldapuser")).willReturn(Optional.empty());
given(ldapProperties.isEnabled()).willReturn(true);
given(ldapAuthService.login("ldapuser", "password"))
.willThrow(new AuthFlowException(HttpStatus.FORBIDDEN, "error.auth.ldap.disabled"));
// When & Then — 403 must propagate so the frontend can show "account disabled"
assertThatThrownBy(() -> service.login("ldapuser", "password"))
.isInstanceOf(AuthFlowException.class)
.extracting("status")
.isEqualTo(HttpStatus.FORBIDDEN);
}
@Test
void login_withUnknownUsername_propagatesEmailConflict_whenEmailCollidesWithExistingAccount() {
// Given — an email-conflict (409) is only raised after a successful LDAP bind, so the
// credentials are valid; it must propagate so the frontend can guide the user to an
// explicit account-link flow rather than reporting a misleading "wrong password".
given(credentialRepository.findByUsernameIgnoreCase("ldapuser")).willReturn(Optional.empty());
given(ldapProperties.isEnabled()).willReturn(true);
given(ldapAuthService.login("ldapuser", "password"))
.willThrow(new AuthFlowException(HttpStatus.CONFLICT, "error.auth.ldap.emailConflict"));
// When & Then — 409 is propagated, not masked as 401
assertThatThrownBy(() -> service.login("ldapuser", "password"))
.isInstanceOf(AuthFlowException.class)
.extracting("status")
.isEqualTo(HttpStatus.CONFLICT);
}
@Test
void login_withUnknownUsername_fails_whenLdapDisabled() {
// Given