zhoujinsong commented on code in PR #4118:
URL: https://github.com/apache/amoro/pull/4118#discussion_r2993236560
##########
amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/LoginController.java:
##########
@@ -59,15 +73,37 @@ public void login(Context ctx) {
PreconditionUtils.checkNotNullOrEmpty(user, "user");
PreconditionUtils.checkNotNullOrEmpty(pwd, "password");
DefaultPasswordCredential credential = new DefaultPasswordCredential(user,
pwd);
+
+ // Step 1: Authenticate the user
+ Principal principal;
try {
- this.loginAuthProvider.authenticate(credential);
- ctx.sessionAttribute("user", new SessionInfo(user,
System.currentTimeMillis() + ""));
- ctx.json(OkResponse.of("success"));
+ principal = this.loginAuthProvider.authenticate(credential);
} catch (Exception e) {
LOG.error("authenticate user {} failed", user, e);
String causeMessage = e.getMessage() != null ? e.getMessage() : "unknown
error";
throw new RuntimeException("invalid user " + user + " or password!
Cause: " + causeMessage);
}
+
+ // Step 2: Resolve user role (LDAP group lookup)
+ String authenticatedUser = principal.getName();
Review Comment:
**[Minor] LDAP group lookup happens on every login — consider caching**
`roleResolver.resolve(authenticatedUser)` queries all configured LDAP groups
on every single login. For environments with many groups or a slow LDAP server
this adds noticeable latency to the login flow. A short-lived cache (e.g. Guava
`LoadingCache` with a 5-minute TTL) keyed by username would avoid repeated
round trips without meaningfully delaying role updates.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
