Add negative-auth canary tests for the internal loopback backend#17023
Open
lukebakken wants to merge 1 commit into
Open
Add negative-auth canary tests for the internal loopback backend#17023lukebakken wants to merge 1 commit into
lukebakken wants to merge 1 commit into
Conversation
The internal loopback authN backend's user_login_authentication/2 has several fail-closed branches, but the existing suite only exercised the positive login paths and the is_loopback=false gate. The two "refused" cases refused on the address gate before ever reaching password verification, so the salted-hash comparison, blank-password rejection, and the fail-closed default when no socket or address is provided had no coverage. Add four canaries in the localhost_connection group, each asserting the specific refusal message so a regression that short-circuits on the wrong branch is caught: - wrong password is refused with invalid credentials - blank password is refused - unknown user is refused with invalid credentials - missing socket/address info fails closed These are test-only additions and pass against the current implementation.
|
Tick the box to add this pull request to the merge queue (same as
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The internal loopback authN backend's
user_login_authentication/2has several fail-closed branches, but the existing suite only exercised the positive login paths and theis_loopback=falsegate. Both pre-existing "refused" cases refused on the address gate before ever reaching password verification, so the salted-hash comparison, the blank-password rejection, and the fail-closed default when no socket or address is provided had no coverage.This change adds four negative-auth canaries to the
localhost_connectiongroup, each asserting the specific refusal message so a regression that short-circuits on the wrong branch is caught (the pre-existing cases used a wildcard for the message):Why
These branches are the security-critical part of the backend: they are what keeps a loopback connection from authenticating without valid credentials. A future change that made any of them fail open would be silent today because nothing asserts on them. Asserting on the exact refusal reason (rather than just "some refusal happened") ensures each test proves the branch it is meant to guard actually fired.
Scope
Test-only. No production code changes. The new cases pass against the current implementation.
Testing
Ran the suite locally:
Result: 8 Ok, 0 Failed, 0 Skipped (4 pre-existing plus the 4 new canaries).