feat(security): add pwned password validation#2959
feat(security): add pwned password validation#2959AlexProgrammerDE wants to merge 1 commit intoAuthMe:masterfrom
Conversation
|
Sorry @AlexProgrammerDE , completely reworked the plugin :D |
AlexProgrammerDE
force-pushed
the
master
branch
from
b4f0051 to
847f977
Compare
AlexProgrammerDE
changed the title
|
@Xephi PR is ready for merge/review |
There was a problem hiding this comment.
Pull request overview
Adds an optional HaveIBeenPwned “Pwned Passwords” range-API check to AuthMe’s password validation so servers can reject commonly breached passwords during registration / password changes.
Changes:
- Introduces
PwnedPasswordServiceto query the HIBP range API and parse breach counts. - Extends
ValidationService.validatePasswordto optionally reject passwords above a configurable breach-count threshold. - Adds message key + English translation and unit tests covering service + validation behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| authme-core/src/main/java/fr/xephi/authme/service/PwnedPasswordService.java | New service performing HIBP range API request and parsing response into a pwned-count result. |
| authme-core/src/main/java/fr/xephi/authme/service/ValidationService.java | Wires the pwned-password check into password validation behind a settings flag + threshold. |
| authme-core/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java | Adds config toggles for enabling the check and setting the breach-count threshold. |
| authme-core/src/main/java/fr/xephi/authme/message/MessageKey.java | Adds a new message key + placeholder for breach count. |
| authme-core/src/main/resources/messages/messages_en.yml | Adds the English message text for the new validation error. |
| authme-core/src/test/java/fr/xephi/authme/service/PwnedPasswordServiceTest.java | New unit tests for parsing + request prefix behavior + failure behavior. |
| authme-core/src/test/java/fr/xephi/authme/service/ValidationServiceTest.java | Adds tests for threshold behavior and “unavailable API” behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (settings.getProperty(SecuritySettings.ENABLE_PWNED_PASSWORD_CHECK)) { | ||
| OptionalLong pwnedCount = pwnedPasswordService.getPwnedCount(password); | ||
| int threshold = settings.getProperty(SecuritySettings.PWNED_PASSWORD_CHECK_THRESHOLD); | ||
| if (pwnedCount.isPresent() && pwnedCount.getAsLong() > threshold) { | ||
| return new ValidationResult(MessageKey.PASSWORD_PWNED_ERROR, Long.toString(pwnedCount.getAsLong())); |
There was a problem hiding this comment.
validatePassword now performs a synchronous network call to the HIBP API (pwnedPasswordService.getPwnedCount). This method is invoked directly on the command thread in some flows (e.g., RegisterAdminCommand calls validatePassword before any async handoff), and also when useAsyncTasks is disabled, which can block the server thread for up to the configured HTTP timeouts. Consider moving the pwned-password query to an async step (or providing an async validation API) so password validation cannot stall the main thread.
| public OptionalLong getPwnedCount(String password) { | ||
| String hash = HashUtils.sha1(password).toUpperCase(Locale.ROOT); | ||
| String hashPrefix = hash.substring(0, HASH_PREFIX_LENGTH); | ||
| String hashSuffix = hash.substring(HASH_PREFIX_LENGTH); |
There was a problem hiding this comment.
getPwnedCount relies on HashUtils.sha1(password), but HashUtils.hash uses message.getBytes() without an explicit charset, so the SHA-1 can differ across platforms for non-ASCII passwords. For interoperability with the HIBP API and consistent behavior across JVMs/OSes, ensure the SHA-1 is computed over a fixed charset (typically UTF-8), either by updating HashUtils or by hashing with an explicit charset here.
|
Thanks copilot :D |
Successor of #2642 because for some reason it was closed unmerged and I didn't get a reply from the maintainers so far. Will update the code to be mergable this evening.