You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Euphonium-1 (via GitHub)" <gi...@apache.org> on 2026/09/21 05:57:57 UTC

[PR] fix(auth): align the login policy reported by /api/auth/status with the interceptor [rocketmq-dashboard]

Euphonium-1 opened a new pull request, #4672:
URL: https://github.com/apache/rocketmq-dashboard/pull/4672

   ## What
   
   `AuthInterceptor` and `AuthController` each computed "is a login required" from the same static property and the same runtime `requireLogin` row, but disagreed on the failure branch:
   
   ```java
   // AuthInterceptor.isLoginRequired() - fails closed
   try {
       GeneralSettingsVO settings = settingsRepository.loadGeneralSettings();
       return settings == null || settings.isRequireLogin();
   } catch (Exception exception) {
       return true;
   }
   ```
   
   ```java
   // AuthController.isLoginRequired() - before this change
   GeneralSettingsVO settings = settingsRepository.loadGeneralSettings();
   return settings != null && settings.isRequireLogin();
   ```
   
   `&&` is the inverse of `||` for the absent-row case, and the controller had no `catch` at all.
   
   ## Why it matters
   
   `web/src/App.tsx` `AuthGate` reads `status.loginRequired` to choose between the login page and the console:
   
   ```tsx
   if (!status.loginRequired) {
     clearAuth();
     setGateState('allowed');
     return;
   }
   ```
   
   `MybatisPlusSettingsRepository.loadGeneralSettings()` (lines 67-92) returns a default VO when the row is missing but **throws** `BusinessException(500, "Persisted general settings are invalid")` when the stored `json` column cannot be deserialized - a reachable state after a partial write or a manual edit. With such a row:
   
   - The interceptor still fails closed and demands a login on every `/api/**` request.
   - `GET /api/auth/status` escaped the exception as a 500, so `AuthGate` fell into `.catch()` and rendered its error screen. The retry re-issues the same failing request, so it can never succeed, and the login page - which the interceptor *would* accept credentials from - is never rendered. The UI is unreachable.
   
   ## The change
   
   `AuthController.isLoginRequired()` now reproduces the interceptor's branches exactly, so the flag advertised to the frontend equals the policy enforced. No authentication is bypassed in either direction: the interceptor still rejects every unauthenticated `/api/**` request, so this is a fail-open *advertisement* with an availability consequence, not an auth bypass.
   
   The stronger follow-up, for maintainers to judge, is extracting the predicate into one shared collaborator so the two layers cannot diverge rather than matching the branches by hand.
   
   ## Tests
   
   - `statusShouldReportLoginRequiredWhenRuntimePolicyIsMissing` - absent policy row.
   - `statusShouldReportLoginRequiredWhenRuntimePolicyCannotBeRead` - the `BusinessException` the real repository throws for a corrupt settings row.
   
   ## ⚠️ Verification disclosure
   
   **I could not build or run this in the environment where it was written** - no Maven or wrapper, no populated `~/.m2`, and only JDK 17 against a Java 21 target. The change was verified by reading both predicates and the frontend consumer, and the tests follow the conventions of the file they sit in. It has **not** been compiled or executed, so please let CI confirm before review. Edits were checked by inspection against `server/style/rmq_checkstyle.xml` (no tabs, no non-ASCII, no unused imports, no line-length or import-order rule).
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org