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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
