test(api): Harden v3 authorization and authentication coverage - #1166
Open
ShJ-code wants to merge 5 commits into
Open
test(api): Harden v3 authorization and authentication coverage#1166ShJ-code wants to merge 5 commits into
ShJ-code wants to merge 5 commits into
Conversation
gtema
requested changes
Aug 12, 2026
gtema
left a comment
Collaborator
There was a problem hiding this comment.
Correctness (top priority):
[crates/token-driver-jws/src/lib.rs:125](https://github.com/openstack-experimental/keystone/blob/main/crates/token-driver-jws/src/lib.rs#L125) — decode()'s key-rotation candidate loop now uses ? on to_decoding_key() failures instead of the old continue/last_err pattern, so a failure on the primary key aborts the whole loop and never falls back to active.previous. This breaks the documented "multi-generational tolerance during a rotation's grace window" — a corrupt/unparsable primary key would reject otherwise-valid tokens signed with a still-good previous key. Unguarded by any test.
[crates/api-types/src/error_conv.rs:579](https://github.com/openstack-experimental/keystone/blob/main/crates/api-types/src/error_conv.rs#L579) — the blanket From<TokenProviderError> impl wasn't updated for the new InvalidToken/Expired/TokenRevoked variants; the 500→401 fix only lives in a local wrapper in [auth/token/common.rs](https://github.com/openstack-experimental/keystone/blob/main/auth/token/common.rs). Harmless today (all current callers discard the error type first) but a latent trap for future callers using ? directly.
Reuse/simplification:
3. Two test files locally reimplement assert_raw_unauthorized instead of the shared helper this PR itself adds.
4. credential/authorization.rs and auth/token/authorization.rs reimplement provision_fixture_pair's cleanup-on-failure pattern inline (4 copies total).
Minor:
5. Collapsing decode errors into InvalidToken loses diagnostic detail in operator logs (show.rs/delete.rs).
6. The PR title claims to fix "system scope input contracts" but the only Rego change is inside an already-dead, commented-out block — the real unreachable-branch issue documented elsewhere in policy/ is untouched.
ShJ-code
force-pushed
the
fix/api-test-quality-994-review
branch
2 times, most recently
from
August 14, 2026 06:06
be6fdfe to
d5f8ae5
Compare
Add reusable scoped-user and raw-request helpers. Add token, assignment, and auth-plugin fixtures for the matrices. Disable the SDK auth cache so fresh API runs cannot reuse stale tokens. Keep revocation tests isolated from concurrent sessions. Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Add positive and policy-denial matrices for domains and projects. Cover user and credential ownership boundaries. Exercise invalid, missing, and revoked tokens. Keep cleanup guarded on unexpected authorization success. Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Add positive, forbidden, and unauthenticated role matrices. Cover project and system grants across user scope boundaries. Exercise implied-role authorization for every operation. Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Classify malformed Fernet and JWS credentials as invalid tokens. Return 401 for invalid, expired, and revoked token credentials. Preserve operational provider errors and CADF reasons. Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Exercise password, token rescope, EC2, and routed plugin methods. Cover ownership, policy denial, invalid credentials, and revocation. Verify authentication-chain preservation across authorized rescope. Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
ShJ-code
force-pushed
the
fix/api-test-quality-994-review
branch
from
August 14, 2026 15:34
d5f8ae5 to
2a362a6
Compare
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
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.
Summary
Closes #994.
This PR builds on the endpoint coverage introduced by #993 and reuses the API
test infrastructure from #992 to strengthen the v3 authorization and
authentication test suite.
It adds:
implied roles, assignments, and tokens.
coverage.
preservation and unauthorized-scope rejection.
fixtures.
from a previous database from being reused.
The v3 integration binary now contains 251 tests, meeting the 250+ target from
#994.
The review also identified that malformed Fernet and JWS credentials were
surfacing as HTTP 500 responses. They are now classified as invalid
credentials and return HTTP 401. Operational provider failures retain their
existing error status and CADF reason.
OPA policy tests were aligned with the serialized
credentials.systemfield,including explicit rejection of system scopes other than
all.Test plan
Verified with:
The following checks also passed:
The diff was additionally checked for newly introduced
unsafeblocks,unwrap(),expect(), andprintln!; none were found.Security review checklist
Required if this diff touches authentication, scope, delegation, tokens,
credentials, EC2, trusts, application credentials, or OPA policy input.
Delete this section entirely if it doesn't apply. See
doc/src/security.md§7 for the full contextbehind each item.
scope instead of the immutable authentication chain. Token rescope tests
verify that the authentication context is preserved. (I1/I2)
projection,
delegated_project_id, and scope-drift requirements areunchanged. (I2/I3)
bounding remains unchanged. (I4)
ScopeInfovariant or native authentication method was added.Existing scope-boundary and security-context resolution paths are exercised
end-to-end. (I5, Gate J)
Gate I)
authorization requirements remain unchanged. (I8)
user_id; they do not permit an unfiltered collection scan and rely on thesame filter used by the persistence driver. (I8a)
Negative rescope tests verify unauthorized domains are rejected and the
original authentication chain is preserved. (I5)
policy denials, cross-user access, project/system scope isolation, and
unauthorized rescope.
ValidatedSecurityContext::new_for_scope()through the live API rather thantesting only its inner helpers.