apache / apache/rocketmq-dashboard
[Studio][Bug] Login reveals disabled accounts and skips rate limiting before the password is verified
- Dominant language
- Java
- Stars
- 1.4k
- Forks
- 683
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 58
Description
## 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: #4160.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in AuthService.java around loginDatabaseUser and the login rate-limiter handling at the cited lines. Run the two named AuthServiceDatabaseTest regression tests first. Done means wrong passwords for disabled accounts return the generic 401 and count toward lockout, while a correct password still returns the existing 403.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- authentication, backend, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100