Skip to content

test(api): Expand v3 coverage and enforce EC2 token restrictions - #1084

Open
ShJ-code wants to merge 9 commits into
openstack-experimental:mainfrom
ShJ-code:feat/api-test-coverage-993-v3
Open

test(api): Expand v3 coverage and enforce EC2 token restrictions#1084
ShJ-code wants to merge 9 commits into
openstack-experimental:mainfrom
ShJ-code:feat/api-test-coverage-993-v3

Conversation

@ShJ-code

@ShJ-code ShJ-code commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Part of #993 (Phase 2). This PR delivers the v3 portion; v4 coverage will
follow separately.

  • Add the full 4×3 authorization matrix for all four OS-EC2 credential
    handlers: owner success, cross-user policy denial, and invalid-token
    rejection.
  • Add eight live tests for POST /v3/ec2tokens, covering issuance, caller
    authorization, invalid signatures, stale timestamps, token validation,
    token reauthentication, and the fix(security): Do not allow ec2cred token to do anything #1071 bearer-token restriction.
  • Enforce the ec2credential marker from the immutable Fernet token payload
    and return 401 when an EC2-issued token is used on an ordinary API endpoint.
  • Preserve Trust and Application Credential delegation contexts during EC2
    redemption while encoding the exact EC2 authentication method.
  • Make Fernet authentication-method encoding fail closed if any requested
    method is unsupported.
  • Cover GET /v3/users/{id}/groups with empty and SCIM-populated success
    cases, cross-user 403, and invalid-token 401.
  • Cover GET /v3/role_inferences for admin and system-reader success,
    project-scoped-member 403, and invalid-token 401.
  • Disable persistent SDK authentication caching for API tests and configure
    ec2credential consistently across the repository-owned Rust Keystone
    test environments.

Test plan

  • cargo check --message-format=short -p openstack-keystone-core-types -p openstack-keystone-core -p openstack-keystone-token-driver-fernet -p openstack-keystone -p test_api
    • Passed.
  • cargo test -p openstack-keystone-core-types
    • 126 passed.
  • cargo test -p openstack-keystone-core
    • 611 unit tests and 3 doctests passed.
  • cargo test -p openstack-keystone-token-driver-fernet
    • 42 passed.
  • cargo test -p openstack-keystone -- --test-threads=1
    • 760 library tests and 4 binary tests passed.
  • cargo test -p test_api
    • 25 passed.
  • cargo nextest run --profile api -p test_api --test integration_api_v3 --test integration_api_v4 --test scim_v2
    • 230 passed, 0 failed, 0 skipped:
      • v3: 133
      • existing v4 regression suite: 33
      • SCIM v2: 64
  • cargo clippy --message-format=short -p openstack-keystone-core-types -p openstack-keystone-core -p openstack-keystone-token-driver-fernet -p openstack-keystone -p test_api --lib --tests
    • Completed successfully; only pre-existing warnings remain.
  • pre-commit run --all-files
    • Passed.

Security review checklist

Required because this diff touches authentication, delegation, tokens,
credentials, and EC2.

  • No. Delegation and authorization facts continue to come from
    authentication_context() and the Trust/Application Credential objects;
    no decision was changed to infer delegation from ScopeInfo or
    project_id. (I1/I2)
  • Not applicable. This PR adds no policy rule and does not change the
    projection of delegation facts onto Credentials. (I2/I3)
  • Reviewed. EC2 redemption retains the existing Trust or Application
    Credential context, so effective-role calculation remains bounded by the
    delegation; the existing delegated-role matrices and new EC2 carrier
    round-trip tests pass. (I4)
  • Not applicable. No new ScopeInfo variant or authentication method
    is introduced; ec2credential already existed and is now registered in
    the relevant test configurations. (I5, Gate J)
  • Not applicable. No new lookup by a client-derived key is added by
    this PR; the existing EC2 credential lookup behavior is unchanged. (I6)
  • No. No new secrets or decrypted credential blobs are added to OPA
    policy input. EC2 secrets are used only for signature verification. (I7,
    Gate I)
  • Not applicable. No new list or collection endpoint is introduced;
    this PR tests existing endpoints. (I8)
  • No. Request-supplied scope cannot broaden an EC2-issued token; the
    immutable ec2credential marker is preserved across delegation and token
    reauthentication, and ordinary bearer use is rejected with 401. (I5)
  • Negative tests cover cross-user credential access, unauthorized
    callers, the /v3/ec2tokens member-caller gate, invalid signatures, stale
    timestamps, and EC2-token use on ordinary endpoints.
  • The live-server suites exercise authentication and scope resolution
    end to end, including ValidatedSecurityContext::new_for_scope(), rather
    than testing only inner scope or effective-role helpers.

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stopped reviewing the PR since it makes a forbidden change to the auth logic. Please fix this before we can look at the rest of PR.

Comment thread crates/core-types/src/auth/tests.rs Outdated
}

#[test]
fn test_methods_application_credential_preserves_parent_token_methods() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is very confusing. It simulates ec2cred token but then build AuthContext as if user would renew the auth with application credentials. It is forbidden and unreachable business logic - auth with application credentials always return a fresh token - nothing is preserved in it

Comment thread crates/core-types/src/auth/tests.rs Outdated
}

#[test]
fn test_methods_trust_preserves_parent_token_methods() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - ec2cred auth does not allow to get a trust auth. Trust always require password auth, not a previously obtained token.

/// `Ok(())` when the token was not minted from an EC2 credential, `Err(401
/// Unauthorized)` otherwise.
fn reject_if_ec2(user_auth: &ValidatedSecurityContext) -> Result<(), KeystoneApiError> {
if matches!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this MUST stay - ec2credential token is returned by Keystone, is accepted in the validate token, but is not accepted for anything else - this is a security fix

@ShJ-code
ShJ-code force-pushed the feat/api-test-coverage-993-v3 branch from 2fe020e to 1af031e Compare July 27, 2026 17:49
@ShJ-code

Copy link
Copy Markdown
Collaborator Author

Thanks for the comments. I just applied the changes to the new commit. Please let me know if anything needs to change as I work on v4.

@ShJ-code
ShJ-code requested a review from gtema July 27, 2026 18:39
Comment thread .github/workflows/ci.yml
if: needs.changes.outputs.code == 'true'
run: cargo nextest run -p test_api --profile ci-api --test integration_api_v3 --test integration_api_v4

- name: Invalidate eventual openstack_sdk cache

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing cache invalidation should be only possible once the openstack_sdk releases with the possibility to control caching programmatically

Comment thread tests/api/clouds.yaml Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be done. Pretty soon I will release the openstack_sdk with programmatical cache control. Adding the clouds.yaml only to disable caching is not desired

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't the crud_endpoint! macro to prevent manually creating endpoints?

Comment thread tests/api/src/common.rs Outdated

const AUTH_URL: &str = "http://localhost:8080";

#[test]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we please also get rid of this test?

@ShJ-code

Copy link
Copy Markdown
Collaborator Author

There is currently one structural issue in the test: the populated user-group test is located in the scim_v2 binary rather than integration_api_v3 because there is currently no v3 membership write API; actual memberships can only be constructed via SCIM.

@ShJ-code
ShJ-code force-pushed the feat/api-test-coverage-993-v3 branch from 1af031e to adbdebc Compare July 28, 2026 19:51
@ShJ-code
ShJ-code requested a review from gtema July 28, 2026 20:56

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, very good. Few last points:

  • tests/api/src/auth/ec2.rs: sign_v2_hmac_sha256 sorts pairs via Vec<(&String,&String)>::sort(), i.e. lexicographic by (key, value) tuple — for AWS SigV2 canonicalization the spec requires sorting by key (byte order) only. Since AWSAccessKeyId/SignatureMethod/SignatureVersion/Timestamp keys are all distinct in practice this doesn't currently bite, but it's a latent correctness gap in the "independent" reference signer if a future test ever needs duplicate keys or relies on value as tiebreaker — worth a one-line comment or switching to sort_by_key(|(k, _)| *k) for precision.

  • test_ec2_token_forbidden_member_caller asserts StatusCode::FORBIDDEN, while test_ec2_credential_create_forbidden_cross_user (and others) use the shared assert_forbidden helper. Fine functionally, just a minor consistency nit — could route through the same helper for uniformity.

  • Fixture provisioning isn't guarded against partial failure in ProjectScopedUser::provision: if add_project_grant or get_user_session fails after the project/user were created, both resources leak (no cleanup-on-error, unlike SystemScopedUser::provision, which does clean up the user on session_result failure). Given these are live-server tests hitting a long-lived dev server, an intermittent failure here silently accumulates orphan projects/users. Might be worth aligning ProjectScopedUser::provision with the same clean-up-on-error pattern used in SystemScopedUser::provision.

@ShJ-code
ShJ-code force-pushed the feat/api-test-coverage-993-v3 branch from c4e4004 to 5afdfb7 Compare July 30, 2026 04:23
ShJ-code added 9 commits July 30, 2026 20:11
Add the shared building blocks the remaining openstack-experimental#993 v3 suites rely on:

- `test_api::fixtures::ProjectScopedUser`, a real password user holding
  a bootstrap role on a fresh project, authenticated with a genuinely
  project-scoped token through the live auth path. It is provisioned and
  torn down with the caller's admin session, so cleanup never depends on
  the underprivileged session, and it unwinds its own partial work if a
  later provisioning step fails.

- `warn_on_cleanup_failure`, which logs a failed best-effort cleanup
  instead of shadowing the error that triggered it.

- `test_api::common::raw_request`, for the invalid-auth leg of the
  matrix: the SDK refuses to send a syntactically invalid token, so
  those cases have to bypass it.

Teach `asserts::status_from_error` to unwrap a `reqwest::Error` so the
`raw_request` responses can go through `assert_unauthorized` and report
the full error chain like every other negative assertion.

Migrate the openstack-experimental#992 group suite onto these helpers: its local
`ProjectScopedManager` and `status_with_invalid_token` were the
prototypes the shared versions generalize, and keeping both would let
them drift apart.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Extend crud_endpoint! with an optional parent collection and unwrapped
create bodies so the legacy OS-EC2 sub-resource is generated rather than
hand-written. Add unguarded create and delete flavors for resources
whose teardown session differs from the creating one.

Add an independent AWS SigV2 HMAC-SHA256 signer with a published
golden-vector unit test. Canonicalization sorts strictly by raw
parameter name: the value must never act as a tiebreaker, and encoding
before sorting would reorder names whose first differing byte is
percent-escaped. A unit test pins that ordering.

Enable ec2credential in the live-test and Skaffold Rust configurations.
Keep the upstream startup timeout and failure diagnostics.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Cover all four OS-EC2 credential handlers with owner success, cross-user
policy denial, and invalid-token rejection.

Exercise EC2 token issuance, caller policy, signature and timestamp
validation, subject-token validation, and token-from-token
reauthentication through live API tests.

Route the invalid-token cases through assert_unauthorized so a wrong
status reports the full error chain instead of a bare status compare,
and reduce asserted values to their access key: Ec2Credential's derived
Debug carries the plaintext secret, which the assertion helpers would
print on an unexpected success.

Release the already-provisioned fixtures when a later setup step fails,
so a mid-setup error leaves no orphaned users, projects or credentials
behind on the live server.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Cover an admin reading empty and populated memberships, a project-scoped
member receiving 403 for another user, and an invalid token receiving
401.

Create the populated membership through the only live write path, SCIM,
then verify it through v3 in the SCIM binary to preserve the v3 binary's
Python Keystone compatibility. Explicitly remove and purge every
resource.

Route the invalid-token case through assert_unauthorized, release the
first fixture when the second fails to provision, and name the tests in
the coverage table exactly as they are declared so the table stays
greppable.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Keep the admin positive case and add the policy's system-scoped reader
positive branch, project-scoped member denial, and invalid-token
rejection.

Add a reusable system-scoped user fixture and system role-grant
endpoint, with admin-owned cleanup for both success and provisioning
failures, and route the invalid-token case through assert_unauthorized.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Return 401 when an EC2-issued token is presented as X-Auth-Token,
while preserving subject-token validation and token-to-token reauth.

Use the shared openstack-experimental#992 status assertions and complete fixture cleanup before
panicable assertions in the negative cases.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Re-enforce identity/group/show against every returned group and omit
items denied by policy, as required by security-model invariant I8.

Add reusable caller-configured policy state and focused tests that verify
both per-record policy inputs and denied-item filtering.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Attempt every project-scoped fixture deletion before returning an error,
and reuse the helper across multi-user authorization tests.

Complete cleanup before panicable assertions and retain the reason the
live Rust server enables the ec2credential authentication method.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
Compile FederationProtocol validation tests only when the validate
feature provides the Validate implementation. This restores the default
crate test configuration after rebasing onto upstream/main.

Signed-off-by: ShJ-code <sihao_jiang@outlook.com>
@ShJ-code
ShJ-code force-pushed the feat/api-test-coverage-993-v3 branch from 5afdfb7 to 644534b Compare July 30, 2026 20:11
@ShJ-code

Copy link
Copy Markdown
Collaborator Author

I modified the commits so that there's better and more extensive use of helpers developed from #992 , specifically for EC2 credential endpoints and role-inference tests, as well as for cleanup.

@ShJ-code
ShJ-code requested a review from gtema July 31, 2026 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants