unbridled-41 opened a new issue, #4159:
URL: https://github.com/apache/rocketmq-dashboard/issues/4159
## Problem
`AuthService.loginDatabaseUser` checks whether the Studio user is enabled
**before** comparing the password:
```java
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"); //
before password check
}
if (!passwordHasher.matches(request.getPassword(), user.getPasswordHash())) {
throw new BusinessException(401, "Invalid username or password");
}
```
`AuthService.java` (lines 352-359 on the current `rocketmq-studio` head).
## Evidence
Two new regression tests in `AuthServiceDatabaseTest` fail on the unmodified
base branch (`0a596661`):
- `loginShouldNotRevealDisabledAccountsBeforeThePasswordIsVerified` — a
disabled user with a **wrong** password gets `403 "User account is disabled"`
instead of the generic `401 "Invalid username or password"`. Actual red output:
`Expecting message to be: "Invalid username or password" but was: "User account
is disabled"`.
- `disabledAccountLoginsAreRateLimitedLikeWrongPasswords` —
`LoginRateLimiter.recordFailure` is only called for `401` (`AuthService.login`,
lines 138-143), so five failed attempts against a disabled account never reach
`LoginRateLimiter.MAX_FAILED_ATTEMPTS` and no `429` lockout is applied. Actual
red output: expected `429 Too many failed login attempts...` but got the sixth
`403 "User account is disabled"`.
## Impact
- **User enumeration**: any unauthenticated caller can distinguish suspended
usernames from unknown ones (`403` vs `401`) without knowing the password.
- **Brute-force bypass**: unlimited password guessing against disabled
accounts — the failure counter is never incremented, so the existing
`LoginRateLimiter` lockout never triggers for these attempts.
## Expected behavior
The disabled-account state must not be disclosed before the credentials are
verified. The password should be compared first; a disabled account with a
wrong password receives the same generic `401`. After a correct password, the
existing `403 "User account is disabled"` response is preserved. Failed
attempts against disabled accounts then flow through the existing login rate
limiter like any other failed login.
## Related work
- #3045 / PR #3046 fixed the rate limiter's own capacity behavior (lockouts
cleared by repeated failures / overflow bypass) — a different defect; the
bypass here comes from which HTTP codes `AuthService.login` records, not from
the limiter itself.
- PR #2876 (open) trims usernames and tolerates missing payloads — different
defect.
- No open or closed issue covers the disabled-before-password ordering.
## PR
Fix incoming.
--
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]
