You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by li...@apache.org on 2026/10/09 03:26:25 UTC

(rocketmq-dashboard) branch rocketmq-studio updated: fix(auth): report the login policy the interceptor enforces (#5525)

This is an automated email from the ASF dual-hosted git repository.

lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git


The following commit(s) were added to refs/heads/rocketmq-studio by this push:
     new 5ae73d4c0 fix(auth): report the login policy the interceptor enforces (#5525)
5ae73d4c0 is described below

commit 5ae73d4c08d76b67962b14f109ebc453f134d7b8
Author: Wang1rrr <15...@users.noreply.github.com>
AuthorDate: Fri Oct 9 11:26:19 2026 +0800

    fix(auth): report the login policy the interceptor enforces (#5525)
    
    `AuthController.isLoginRequired` and `AuthInterceptor` kept two divergent copies of the same
    policy: a missing settings row meant "no login required" to the controller but "login required" to
    the interceptor, and an unreadable policy escaped as a 500 that stranded the frontend on its error
    screen with a retry that could never succeed. Fail closed in both branches so the endpoint reports
    exactly what the interceptor does; no third copy of the policy remains in the repository.
---
 .../rocketmq/studio/auth/AuthController.java       | 20 +++++++++++--
 .../rocketmq/studio/auth/AuthControllerTest.java   | 35 ++++++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/org/apache/rocketmq/studio/auth/AuthController.java b/server/src/main/java/org/apache/rocketmq/studio/auth/AuthController.java
index 9432eec09..616339752 100644
--- a/server/src/main/java/org/apache/rocketmq/studio/auth/AuthController.java
+++ b/server/src/main/java/org/apache/rocketmq/studio/auth/AuthController.java
@@ -60,12 +60,28 @@ public class AuthController {
                 .body(Result.ok(status));
     }
 
+    /**
+     * Reports the policy {@link AuthInterceptor} actually enforces, and answers exactly as it does.
+     * Both read the same static property and the same runtime "requireLogin" row, so the failure
+     * branches have to line up too: the frontend reads this flag to choose between the login page
+     * and the console, and a policy that fails differently in the two places strands the console.
+     * The interceptor fails closed (see its own {@code isLoginRequired}), so an unreadable policy
+     * still demands a login here; letting the read failure escape as a 500 instead used to leave the
+     * frontend on its error screen, where the retry could never succeed and the login page that
+     * would in fact work was unreachable.
+     */
     private boolean isLoginRequired() {
         if (authProperties.isLoginRequired()) {
             return true;
         }
-        GeneralSettingsVO settings = settingsRepository.loadGeneralSettings();
-        return settings != null && settings.isRequireLogin();
+        try {
+            GeneralSettingsVO settings = settingsRepository.loadGeneralSettings();
+            // Fail closed, matching the interceptor: an absent policy row means it still demands a
+            // login, so this endpoint must report that rather than the opposite.
+            return settings == null || settings.isRequireLogin();
+        } catch (Exception exception) {
+            return true;
+        }
     }
 
     @PostMapping("/login")
diff --git a/server/src/test/java/org/apache/rocketmq/studio/auth/AuthControllerTest.java b/server/src/test/java/org/apache/rocketmq/studio/auth/AuthControllerTest.java
index 7d7cc7853..c1f503da1 100644
--- a/server/src/test/java/org/apache/rocketmq/studio/auth/AuthControllerTest.java
+++ b/server/src/test/java/org/apache/rocketmq/studio/auth/AuthControllerTest.java
@@ -19,6 +19,7 @@ package org.apache.rocketmq.studio.auth;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.rocketmq.studio.common.config.LegacyJackson2Config;
+import org.apache.rocketmq.studio.common.exception.BusinessException;
 import org.apache.rocketmq.studio.ops.ai.tool.catalog.ToolCatalog;
 import org.apache.rocketmq.studio.settings.GeneralSettingsVO;
 import org.apache.rocketmq.studio.settings.SettingsRepository;
@@ -72,6 +73,12 @@ class AuthControllerTest {
     @Test
     void statusShouldReportDisabledLoginProtection() throws Exception {
         when(authProperties.isLoginRequired()).thenReturn(false);
+        // The policy row exists and says login is not required. It has to be stubbed explicitly:
+        // an unstubbed mock returns null, and an absent policy row means "login required" here just
+        // as it does in the interceptor - see statusShouldReportLoginRequiredWhenRuntimePolicyIsMissing.
+        when(settingsRepository.loadGeneralSettings()).thenReturn(GeneralSettingsVO.builder()
+                .requireLogin(false)
+                .build());
         when(authService.getAuthenticatedUser(null)).thenReturn(Optional.empty());
 
         mockMvc.perform(get("/api/auth/status"))
@@ -96,6 +103,34 @@ class AuthControllerTest {
                 .andExpect(jsonPath("$.data.authenticated").value(false));
     }
 
+    @Test
+    void statusShouldReportLoginRequiredWhenRuntimePolicyIsMissingTest() throws Exception {
+        // AuthInterceptor fails closed when the policy row is absent, so the flag the frontend reads
+        // must not report "login not required" for the same state.
+        when(authProperties.isLoginRequired()).thenReturn(false);
+        when(settingsRepository.loadGeneralSettings()).thenReturn(null);
+        when(authService.getAuthenticatedUser(null)).thenReturn(Optional.empty());
+
+        mockMvc.perform(get("/api/auth/status"))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.data.loginRequired").value(true));
+    }
+
+    @Test
+    void statusShouldReportLoginRequiredWhenRuntimePolicyCannotBeReadTest() throws Exception {
+        // The interceptor swallows this failure and demands a login, so the endpoint that tells the
+        // frontend which screen to render must not answer with an error of its own.
+        when(authProperties.isLoginRequired()).thenReturn(false);
+        when(settingsRepository.loadGeneralSettings())
+                .thenThrow(new BusinessException(500, "Persisted general settings are invalid"));
+        when(authService.getAuthenticatedUser(null)).thenReturn(Optional.empty());
+
+        mockMvc.perform(get("/api/auth/status"))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.data.loginRequired").value(true))
+                .andExpect(jsonPath("$.data.authenticated").value(false));
+    }
+
     @Test
     void statusShouldReportUnauthenticatedWhenTokenIsMissing() throws Exception {
         when(authProperties.isLoginRequired()).thenReturn(true);