Repository navigation
fix(oauth2): cap exchange lifetime to subject and app cred exp - #1459
Draft
millionsouls wants to merge 6 commits into
Draft
millionsouls wants to merge 6 commits into
millionsouls wants to merge 6 commits into
Conversation
millionsouls
marked this pull request as draft
October 9, 2026 00:05
gtema
reviewed
Oct 9, 2026
millionsouls
force-pushed
the
expiry
branch
2 times, most recently
from
October 14, 2026 04:13
51d1d31 to
59a6f08
Compare
Author
|
I think I messed up on committing my changes with merging the new commits from main. Attempting to fix it, ignore checks for now (apologies). |
millionsouls
force-pushed
the
expiry
branch
3 times, most recently
from
October 14, 2026 22:44
040c714 to
a3b358c
Compare
| MissingSubjectTokenExpiry, | ||
|
|
||
| /// The credential-chain expiry is not later than the new token's issue time. | ||
| /// Subject token/ App Cred has either already expired or expires at/before the new token's issue time |
| MissingSubjectTokenExpiry, | ||
|
|
||
| /// The credential-chain expiry is not later than the new token's issue time. | ||
| /// Subject token/ App Cred has either already expired or expires at/before the new token's issue time |
Prevents an exchanged token from outliving the credential chain it represents. Claim builder now caps the new expiry at the earliest of the config access token expiry, the validated subject token's expiry, and the app cred expiry. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
MissingSubjectTokenExpiry and NonPosititiveLifetime errors log seperately Modified error descriptions to be more specific for these case. Cases will be logged as warnings, while unexpected error(s) will be logged as full errors. Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Three rejections tests through the HTTP endpoint for already expired app cred token, expiring now app cred token, and expiring now subject token Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Modified derive_delegation_context to return app credential expiration along with delegation_context. build_token_exchange_claim does not have to recheck cred expiration again. Adjusted existing tests to account for additional paramter field. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Rename NonPositiveTime to CredentialChainExpired to better reflect the error; subject token or app cred has expired now, or will expire before the new token's expiration. Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
| //! reflects the presented credential's own delegation, per security.md I2 | ||
| //! (the delegation boundary is the credential's own immutable project, not | ||
| //! the request's current scope). | ||
| use openstack_keystone_api_types::webauthn::credential_protection_policy; |
| //! reflects the presented credential's own delegation, per security.md I2 | ||
| //! (the delegation boundary is the credential's own immutable project, not | ||
| //! the request's current scope). | ||
| use openstack_keystone_api_types::webauthn::credential_protection_policy; |
This branch has not been deployed
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
#1353
Caps token exchange lifetime based on subject and app credential lifetime.
Referencing checklist from issue:
crates/core/src/oauth2_client/token_exchange.rs:188:203Exposes expires_at to claim builder.crates/core/src/oauth2_client/token_exchange.rs:205:211Computes min time based on reported cap value in expires_in. Unsure if this should be skipped or rejected if an expiry is absent as I believe open stack does offer non-expiring tokens (?).crates/core/src/oauth2_client/token_exchange.rs:205:211crates/keystone/src/api/v4/oauth2/token/token_exchange.rs:117:122invalid_grant rejection when lifetime <= 0crates/core/src/oauth2_client/token_exchange.rs:484general unit teststests/integration/src/oauth2_token_exchange.rs:201unit tests for short lived app credentialdoc/src/adr/0026-oauth2-oidc-provider.md:1555Test plan
cargo test -p openstack-keystone-core oauth2_client::token_exchange::testsExpiry rulescargo test -p openstack-keystone api::v4::oauth2::token::token_exchange::testsEndpoint and responsecargo check -p openstack-keystone --message-format=shortCompilingcargo fmt --checkFormatAll
pre-commit run --all-filespassedcargo clippy -p openstack-keystone --fix --allow-dirtypassedCould not run
cargo testandcargo nextest runas tss-esapi-sys couldn't run on macOS. Rancargo test -p openstack-keystone-core token_exchangeandcargo test -p openstack-keystone token_exchange: passedSecurity 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/contributor/security-model.md§7 for the full context behind each item.
(
project_id,ScopeInfo) where it should read the chain(
authentication_context(), delegation object)? (I1/I2)onto
Credentialsfrom the chain, and does the rule anchor ondelegated_project_id+ carry the scope-drift tripwire? (I2/I3)roles still bounded by the delegation? Added a test that a restricted
delegation cannot exceed its roles via the new path? (I4)
ScopeInfovariant or auth method? Updatedvalidate_scope_boundaries(),calculate_effective_roles(),fully_resolved(), andCredentials::try_from— all without adding awildcard
_ =>arm? (I5, Gate J)sha256(access))? Does itre-assert
type/shape after fetch? (I6)Gate I)
per-item read policy? (I8)
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)
request-supplied scope? (I5)
positive tests proving the happy path works?
ValidatedSecurityContext::new_for_scope()end-to-end, not just the inner helper (
calculate_effective_roles,validate_scope_boundaries) in isolation?