RockteMQ-AI commented on code in PR #2162:
URL: 
https://github.com/apache/rocketmq-dashboard/pull/2162#discussion_r3783672947


##########
server/src/main/java/org/apache/rocketmq/studio/auth/PasswordHasher.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.apache.rocketmq.studio.auth;
+
+import org.springframework.stereotype.Component;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+import java.security.MessageDigest;
+import java.security.SecureRandom;
+import java.util.Base64;
+
+@Component
+public class PasswordHasher {
+
+    private static final String ALGORITHM = "PBKDF2WithHmacSHA256";
+    private static final int ITERATIONS = 210_000;
+    private static final int KEY_LENGTH = 256;
+    private static final int SALT_LENGTH = 16;
+    private static final int MIN_ITERATIONS = 100_000;

Review Comment:
   **[Info]** Good security practice: PBKDF2WithHmacSHA256 with 210k 
iterations, proper salt generation, and constant-time comparison via 
`MessageDigest.isEqual()`. The iteration bounds check (100k-1M) prevents 
downgrade attacks.



##########
server/src/main/java/org/apache/rocketmq/studio/auth/AuthService.java:
##########
@@ -17,153 +17,393 @@
 
 package org.apache.rocketmq.studio.auth;
 
+import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
+import lombok.extern.slf4j.Slf4j;
 import org.apache.rocketmq.studio.common.exception.BusinessException;
+import org.apache.rocketmq.studio.persistence.entity.RmqStudioSession;
+import org.apache.rocketmq.studio.persistence.entity.RmqStudioUser;
+import org.apache.rocketmq.studio.persistence.mapper.RmqStudioSessionMapper;
+import org.apache.rocketmq.studio.persistence.mapper.RmqStudioUserMapper;
 import org.apache.rocketmq.studio.settings.GeneralSettingsVO;
 import org.apache.rocketmq.studio.settings.SettingsRepository;
-import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.dao.DuplicateKeyException;
 import org.springframework.scheduling.annotation.Scheduled;
 import org.springframework.stereotype.Service;
 
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.SecureRandom;
 import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
+import java.util.Base64;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 
+/**
+ * Authenticates Studio users and owns bearer session lifecycle.
+ *
+ * <p>The configuration users remain a bootstrap mechanism for a fresh 
database only. Once a
+ * Studio user has been created, the database is the source of truth for 
credentials and account
+ * status.</p>
+ */
 @Slf4j
 @Service
 public class AuthService {
 
     private static final int DEFAULT_SESSION_TIMEOUT_MINUTES = 30;
     private static final int MIN_SESSION_TIMEOUT_MINUTES = 5;
     private static final int MAX_SESSION_TIMEOUT_MINUTES = 1440;
+    private static final Duration LAST_SEEN_UPDATE_INTERVAL = 
Duration.ofMinutes(5);
     private static final String TOKEN_PREFIX = "Bearer ";
+    private static final SecureRandom TOKEN_RANDOM = new SecureRandom();
 
     private final AuthProperties authProperties;
     private final SettingsRepository settingsRepository;
     private final Clock clock;
+    private final RmqStudioUserMapper userMapper;
+    private final RmqStudioSessionMapper sessionMapper;
+    private final PasswordHasher passwordHasher;
+
+    // Retained only for narrow unit tests that construct the legacy service 
directly.
     private final Map<String, AuthSession> activeTokens = new 
ConcurrentHashMap<>();
 
     @Autowired
+    public AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository,
+                       RmqStudioUserMapper userMapper, RmqStudioSessionMapper 
sessionMapper,
+                       PasswordHasher passwordHasher) {
+        this(authProperties, settingsRepository, Clock.systemUTC(), 
userMapper, sessionMapper,
+                passwordHasher);
+    }
+
     public AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository) {
         this(authProperties, settingsRepository, Clock.systemUTC());
     }
 
     AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository, Clock clock) {
+        this(authProperties, settingsRepository, clock, null, null, new 
PasswordHasher());
+    }
+
+    AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository, Clock clock,
+                RmqStudioUserMapper userMapper, RmqStudioSessionMapper 
sessionMapper,
+                PasswordHasher passwordHasher) {
         this.authProperties = authProperties;
         this.settingsRepository = settingsRepository;
         this.clock = clock;
+        this.userMapper = userMapper;
+        this.sessionMapper = sessionMapper;
+        this.passwordHasher = passwordHasher;
     }
 
     public LoginVO login(LoginDTO request) {
-        if (request == null) {
-            throw new BusinessException(400, "Login request is required");
+        validateLogin(request);
+        return databaseBacked() ? loginDatabaseUser(request) : 
loginConfiguredUser(request);
+    }
+
+    public boolean isAuthenticated(String authorization) {
+        return getAuthenticatedUser(authorization).isPresent();
+    }
+
+    public Optional<LoginVO.UserInfo> getAuthenticatedUser(String 
authorization) {
+        Optional<String> token = tokenFromAuthorization(authorization);
+        if (token.isEmpty()) {
+            return Optional.empty();
         }
+        return databaseBacked() ? databaseUserForToken(token.get()) : 
inMemoryUserForToken(token.get());
+    }
 
-        log.info("Login attempt for user: {}", request.getUsername());
+    public boolean isAdmin(String authorization) {
+        return 
getAuthenticatedUser(authorization).map(LoginVO.UserInfo::isAdmin).orElse(false);
+    }
 
-        if (request.getUsername() == null || request.getUsername().isBlank()) {
-            throw new BusinessException(400, "Username is required");
+    public void logout(String authorization) {
+        tokenFromAuthorization(authorization).ifPresent(token -> {
+            if (databaseBacked()) {
+                sessionMapper.update(null, new 
UpdateWrapper<RmqStudioSession>()
+                        .eq("token_hash", tokenHash(token))
+                        .isNull("revoked_at")
+                        .set("revoked_at", now()));
+            } else {
+                activeTokens.remove(token);
+            }
+        });
+    }
+
+    public List<RmqStudioUser> listUsers() {
+        requireDatabaseBacked();
+        return userMapper.selectList(new 
QueryWrapper<RmqStudioUser>().orderByAsc("username"));
+    }
+
+    public RmqStudioUser createUser(String username, String password, boolean 
admin) {
+        requireDatabaseBacked();
+        validateUsername(username);
+        validatePassword(password);
+        if (findUserByUsername(username).isPresent()) {
+            throw new BusinessException(409, "Username is already in use");
         }
-        if (request.getPassword() == null || request.getPassword().isBlank()) {
-            throw new BusinessException(400, "Password is required");
+        RmqStudioUser user = new RmqStudioUser();
+        user.setId(UUID.randomUUID().toString());
+        user.setUsername(username.trim());
+        user.setPasswordHash(passwordHasher.hash(password));
+        user.setAdmin(admin);
+        user.setEnabled(true);
+        user.setPasswordChangedAt(now());
+        userMapper.insert(user);
+        return user;
+    }
+
+    public RmqStudioUser setUserEnabled(String userId, boolean enabled) {
+        requireDatabaseBacked();
+        RmqStudioUser user = getUser(userId);
+        if (!enabled && Boolean.TRUE.equals(user.getAdmin()) && 
enabledAdminCount() <= 1) {
+            throw new BusinessException(409, "The last enabled administrator 
cannot be disabled");
+        }
+        userMapper.updateById(userWithEnabled(user, enabled));
+        if (!enabled) {
+            revokeUserSessions(user.getId());
         }
+        user.setEnabled(enabled);
+        return user;
+    }
 
-        LoginVO.UserInfo user = authenticate(request);
-        long now = clock.millis();
-        purgeExpiredSessions(now);
-        int tokenTtlSeconds = sessionTimeoutSeconds();
-        String token = "studio-jwt-" + UUID.randomUUID();
-        activeTokens.put(token, new AuthSession(user, now + tokenTtlSeconds * 
1000L));
+    public void changePassword(String userId, String currentPassword, String 
newPassword,
+                               boolean requireCurrentPassword) {
+        requireDatabaseBacked();

Review Comment:
   **[Warning]** The `loginDatabaseUser` method throws generic "Invalid 
username or password" for both wrong username and wrong password, which is good 
for security. However, ensure this error message is not logged with the 
username to avoid username enumeration via logs.



##########
server/src/main/java/org/apache/rocketmq/studio/auth/AuthService.java:
##########
@@ -17,153 +17,393 @@
 
 package org.apache.rocketmq.studio.auth;
 
+import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper;
+import lombok.extern.slf4j.Slf4j;
 import org.apache.rocketmq.studio.common.exception.BusinessException;
+import org.apache.rocketmq.studio.persistence.entity.RmqStudioSession;
+import org.apache.rocketmq.studio.persistence.entity.RmqStudioUser;
+import org.apache.rocketmq.studio.persistence.mapper.RmqStudioSessionMapper;
+import org.apache.rocketmq.studio.persistence.mapper.RmqStudioUserMapper;
 import org.apache.rocketmq.studio.settings.GeneralSettingsVO;
 import org.apache.rocketmq.studio.settings.SettingsRepository;
-import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.dao.DuplicateKeyException;
 import org.springframework.scheduling.annotation.Scheduled;
 import org.springframework.stereotype.Service;
 
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.SecureRandom;
 import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
+import java.util.Base64;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 
+/**
+ * Authenticates Studio users and owns bearer session lifecycle.
+ *
+ * <p>The configuration users remain a bootstrap mechanism for a fresh 
database only. Once a
+ * Studio user has been created, the database is the source of truth for 
credentials and account
+ * status.</p>
+ */
 @Slf4j
 @Service
 public class AuthService {
 
     private static final int DEFAULT_SESSION_TIMEOUT_MINUTES = 30;
     private static final int MIN_SESSION_TIMEOUT_MINUTES = 5;
     private static final int MAX_SESSION_TIMEOUT_MINUTES = 1440;
+    private static final Duration LAST_SEEN_UPDATE_INTERVAL = 
Duration.ofMinutes(5);
     private static final String TOKEN_PREFIX = "Bearer ";
+    private static final SecureRandom TOKEN_RANDOM = new SecureRandom();
 
     private final AuthProperties authProperties;
     private final SettingsRepository settingsRepository;
     private final Clock clock;
+    private final RmqStudioUserMapper userMapper;
+    private final RmqStudioSessionMapper sessionMapper;
+    private final PasswordHasher passwordHasher;
+
+    // Retained only for narrow unit tests that construct the legacy service 
directly.
     private final Map<String, AuthSession> activeTokens = new 
ConcurrentHashMap<>();
 
     @Autowired
+    public AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository,
+                       RmqStudioUserMapper userMapper, RmqStudioSessionMapper 
sessionMapper,
+                       PasswordHasher passwordHasher) {
+        this(authProperties, settingsRepository, Clock.systemUTC(), 
userMapper, sessionMapper,
+                passwordHasher);
+    }
+
     public AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository) {
         this(authProperties, settingsRepository, Clock.systemUTC());
     }
 
     AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository, Clock clock) {
+        this(authProperties, settingsRepository, clock, null, null, new 
PasswordHasher());
+    }
+
+    AuthService(AuthProperties authProperties, SettingsRepository 
settingsRepository, Clock clock,
+                RmqStudioUserMapper userMapper, RmqStudioSessionMapper 
sessionMapper,
+                PasswordHasher passwordHasher) {
         this.authProperties = authProperties;
         this.settingsRepository = settingsRepository;
         this.clock = clock;
+        this.userMapper = userMapper;
+        this.sessionMapper = sessionMapper;
+        this.passwordHasher = passwordHasher;
     }
 
     public LoginVO login(LoginDTO request) {
-        if (request == null) {
-            throw new BusinessException(400, "Login request is required");
+        validateLogin(request);
+        return databaseBacked() ? loginDatabaseUser(request) : 
loginConfiguredUser(request);
+    }
+
+    public boolean isAuthenticated(String authorization) {
+        return getAuthenticatedUser(authorization).isPresent();
+    }
+
+    public Optional<LoginVO.UserInfo> getAuthenticatedUser(String 
authorization) {
+        Optional<String> token = tokenFromAuthorization(authorization);
+        if (token.isEmpty()) {
+            return Optional.empty();
         }
+        return databaseBacked() ? databaseUserForToken(token.get()) : 
inMemoryUserForToken(token.get());
+    }
 
-        log.info("Login attempt for user: {}", request.getUsername());
+    public boolean isAdmin(String authorization) {
+        return 
getAuthenticatedUser(authorization).map(LoginVO.UserInfo::isAdmin).orElse(false);
+    }
 
-        if (request.getUsername() == null || request.getUsername().isBlank()) {
-            throw new BusinessException(400, "Username is required");
+    public void logout(String authorization) {
+        tokenFromAuthorization(authorization).ifPresent(token -> {
+            if (databaseBacked()) {
+                sessionMapper.update(null, new 
UpdateWrapper<RmqStudioSession>()
+                        .eq("token_hash", tokenHash(token))
+                        .isNull("revoked_at")
+                        .set("revoked_at", now()));
+            } else {
+                activeTokens.remove(token);
+            }
+        });
+    }
+
+    public List<RmqStudioUser> listUsers() {
+        requireDatabaseBacked();
+        return userMapper.selectList(new 
QueryWrapper<RmqStudioUser>().orderByAsc("username"));
+    }
+
+    public RmqStudioUser createUser(String username, String password, boolean 
admin) {
+        requireDatabaseBacked();
+        validateUsername(username);
+        validatePassword(password);
+        if (findUserByUsername(username).isPresent()) {
+            throw new BusinessException(409, "Username is already in use");
         }
-        if (request.getPassword() == null || request.getPassword().isBlank()) {
-            throw new BusinessException(400, "Password is required");
+        RmqStudioUser user = new RmqStudioUser();
+        user.setId(UUID.randomUUID().toString());
+        user.setUsername(username.trim());
+        user.setPasswordHash(passwordHasher.hash(password));
+        user.setAdmin(admin);
+        user.setEnabled(true);
+        user.setPasswordChangedAt(now());
+        userMapper.insert(user);
+        return user;
+    }
+
+    public RmqStudioUser setUserEnabled(String userId, boolean enabled) {
+        requireDatabaseBacked();
+        RmqStudioUser user = getUser(userId);
+        if (!enabled && Boolean.TRUE.equals(user.getAdmin()) && 
enabledAdminCount() <= 1) {
+            throw new BusinessException(409, "The last enabled administrator 
cannot be disabled");
+        }
+        userMapper.updateById(userWithEnabled(user, enabled));
+        if (!enabled) {
+            revokeUserSessions(user.getId());
         }
+        user.setEnabled(enabled);
+        return user;
+    }
 
-        LoginVO.UserInfo user = authenticate(request);
-        long now = clock.millis();
-        purgeExpiredSessions(now);
-        int tokenTtlSeconds = sessionTimeoutSeconds();
-        String token = "studio-jwt-" + UUID.randomUUID();
-        activeTokens.put(token, new AuthSession(user, now + tokenTtlSeconds * 
1000L));
+    public void changePassword(String userId, String currentPassword, String 
newPassword,
+                               boolean requireCurrentPassword) {
+        requireDatabaseBacked();
+        RmqStudioUser user = getUser(userId);
+        if (requireCurrentPassword && !passwordHasher.matches(currentPassword, 
user.getPasswordHash())) {
+            throw new BusinessException(401, "Current password is incorrect");
+        }
+        validatePassword(newPassword);
+        userMapper.update(null, new UpdateWrapper<RmqStudioUser>()
+                .eq("id", user.getId())
+                .set("password_hash", passwordHasher.hash(newPassword))
+                .set("password_changed_at", now()));
+        revokeUserSessions(user.getId());
+    }
 
-        LoginVO response = LoginVO.builder()
-                .token(token)
-                .expiresIn(tokenTtlSeconds)
-                .user(user)
-                .build();
+    @Scheduled(fixedDelayString = 
"${studio.auth.session-cleanup-interval:PT5M}")
+    public void purgeExpiredSessions() {
+        if (databaseBacked()) {
+            sessionMapper.delete(new 
QueryWrapper<RmqStudioSession>().lt("expires_at", now()));
+        } else {
+            purgeExpiredSessions(clock.millis());
+        }
+    }
 
-        log.info("User {} logged in successfully, admin={}", 
user.getUsername(), user.isAdmin());
-        return response;
+    private LoginVO loginDatabaseUser(LoginDTO request) {
+        ensureBootstrapUsers();
+        RmqStudioUser user = findUserByUsername(request.getUsername())
+                .orElseThrow(() -> new BusinessException(401, "Invalid 
username or password"));
+        if (!Boolean.TRUE.equals(user.getEnabled())) {
+            throw new BusinessException(403, "User account is disabled");
+        }
+        if (!passwordHasher.matches(request.getPassword(), 
user.getPasswordHash())) {
+            throw new BusinessException(401, "Invalid username or password");
+        }
+        int tokenTtlSeconds = sessionTimeoutSeconds();
+        String token = newBearerToken();
+        LocalDateTime current = now();
+        RmqStudioSession session = new RmqStudioSession();
+        session.setId(UUID.randomUUID().toString());
+        session.setUserId(user.getId());
+        session.setTokenHash(tokenHash(token));
+        session.setCreatedAt(current);
+        session.setLastSeenAt(current);

Review Comment:
   **[Info]** Good: Sessions are stored with hashed tokens (SHA-256) rather 
than plaintext, and sessions are properly revoked on password change and user 
disable.



-- 
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]

Reply via email to