Skip to content

fix(oauth2): Require JWT subject token type - #1575

Open
ZebangLI wants to merge 2 commits into
openstack-experimental:mainfrom
millionsouls:zebang_branch
Open

ZebangLI wants to merge 2 commits into
openstack-experimental:mainfrom
millionsouls:zebang_branch

Conversation

@ZebangLI

Copy link
Copy Markdown

Summary

Require subject_token_type for the existing RFC 8693 token-exchange handler.

The existing grant dispatcher, client authentication, token verification, and delegation checks are retained. This change validates the declared token type; it does not add external JWT signature verification.

This implements the course requirement, not the full conformance requirements of upstream issue #1354. Alignment with upstream token-type dispatch remains pending.

Test plan

Ran on the feature branch:

cargo test -p openstack-keystone test_token_exchange -j 1

Result: 11 passed, 0 failed.

Coverage includes:

  • Successful application-credential token exchange.
  • Missing subject token.
  • Missing or unsupported subject-token type.
  • Client without the token-exchange grant.
  • Rejected trust delegation and non-delegated subjects.
  • Audience defaults, narrowing, invalid values, and count limits.

Additional integration validation on demo-2: 17 token-exchange tests and the external JWT verification demo passed, after a separate follow-up updated the expiry and external JWT test requests. Those integration changes are not included in this PR.

The full project test suite was not run.

Security review checklist

Author notes for reviewer assessment:

  • No scope, role, delegation, or OPA policy logic is changed.
  • A supported type proceeds through the existing validation flow; the type declaration alone does not authenticate the token.
  • New negative tests verify rejection of missing and unsupported types.
  • The new tests exercise the HTTP handler with mocked providers; they do not add end-to-end coverage through ValidatedSecurityContext::new_for_scope().

The checklist below is left unchecked for reviewer assessment.

See doc/src/contributor/security-model.md §7 for the full context behind each item.

  • Does any delegation/authorization decision read the scope (project_id, ScopeInfo) where it should read the chain (authentication_context(), delegation object)? (I1/I2)
  • New delegation-sensitive policy rule? Is the fact it needs projected onto Credentials from the chain, and does the rule anchor on delegated_project_id + carry the scope-drift tripwire? (I2/I3)
  • New scope shape or redemption path for a delegated auth? Are effective roles still bounded by the delegation? Added a test that a restricted delegation cannot exceed its roles via the new path? (I4)
  • New ScopeInfo variant or auth method? Updated validate_scope_boundaries(), calculate_effective_roles(), fully_resolved(), and Credentials::try_from — all without adding a wildcard _ => arm? (I5, Gate J)
  • New lookup by a client-derivable key (like sha256(access))? Does it re-assert type/shape after fetch? (I6)
  • Does any new data reaching OPA include secrets/decrypted blobs? (I7, Gate I)
  • New list/collection endpoint? Does it re-check each item with the per-item read policy? (I8)
  • Same endpoint: can an unprivileged caller request the whole collection and leave the per-item pass to sort it out? Is the value the collection policy gates on the same one the driver actually filters by? (I8a)
  • Does the change let a narrow auth method be broadened by a request-supplied scope? (I5)
  • Are there negative tests proving the escape is blocked, not just positive tests proving the happy path works?
  • Does the test drive ValidatedSecurityContext::new_for_scope() end-to-end, not just the inner helper (calculate_effective_roles, validate_scope_boundaries) in isolation?

Mark101001 and others added 2 commits October 20, 2026 16:16
Signed-off-by: Zebang Li <99422132+Mark101001@users.noreply.github.com>

This branch has not been deployed

No deployments
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