feat(sts): implement AssumeRoot, GetDelegatedAccessToken, GetWebIdentityToken#698
Merged
vieiralucas merged 4 commits intomainfrom Apr 23, 2026
Merged
feat(sts): implement AssumeRoot, GetDelegatedAccessToken, GetWebIdentityToken#698vieiralucas merged 4 commits intomainfrom
vieiralucas merged 4 commits intomainfrom
Conversation
…ityToken - AssumeRoot mints short-lived (max 900s) credentials targeting a member account root; persists them so subsequent calls under the new key resolve to the target account's root identity. - GetDelegatedAccessToken trades any non-empty token for a fresh STS credential bound to the caller's principal (or account root when unauthenticated). - GetWebIdentityToken issues an unsigned JWT with iss/sub/aud/iat/exp claims so client decoders see standard structure; alg defaults to RS256/ES384 per the model. Closes the STS conformance gap to 11/11 (100%).
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-iam/src/sts_service.rs">
<violation number="1" location="crates/fakecloud-iam/src/sts_service.rs:100">
P2: `GetWebIdentityToken` is read-only (it acquires `self.state.read()` and never inserts credentials) but is listed in `is_mutating_action`, causing a needless `save_iam_snapshot` on every successful call. Remove it from the mutating list.</violation>
<violation number="2" location="crates/fakecloud-iam/src/sts_service.rs:1141">
P2: Non-numeric `DurationSeconds` is silently swallowed (`.ok()` + `unwrap_or(300)`) instead of returning a `ValidationError`. The other STS methods (`assume_root`, `get_session_token`) reject invalid integers explicitly. Parse and validate before defaulting.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Cubic flagged two issues on PR #698: - GetWebIdentityToken is read-only: it reads state via .read() and never inserts credentials. Listing it under is_mutating_action triggered a needless snapshot save on every call. Removed. - DurationSeconds was silently swallowed when non-numeric (.ok() + unwrap_or(300)). Match the pattern used by assume_root / get_session_token: parse explicitly and return ValidationError on bad input.
…ntity_token Adds unit tests around: AssumeRoot via account-id and ARN forms + SourceIdentity rendering, missing TaskPolicyArn, malformed principal, duration > 900; GetDelegatedAccessToken happy + missing token; GetWebIdentityToken happy path, missing audience / signing, unknown algorithm, non-numeric and out-of-range duration. Lifts patch coverage on the new STS handlers.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-iam/src/sts_service.rs">
<violation number="1" location="crates/fakecloud-iam/src/sts_service.rs:1793">
P3: The JWT structure assertion is ineffective: checking for any `.` in the full XML body can pass even when `WebIdentityToken` is not a valid JWT.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| let body = String::from_utf8(resp.body.expect_bytes().to_vec()).unwrap(); | ||
| assert!(body.contains("WebIdentityToken"), "{body}"); | ||
| // JWT structure: header.payload.signature (signature blank). | ||
| assert!(body.contains("."), "{body}"); |
There was a problem hiding this comment.
P3: The JWT structure assertion is ineffective: checking for any . in the full XML body can pass even when WebIdentityToken is not a valid JWT.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-iam/src/sts_service.rs, line 1793:
<comment>The JWT structure assertion is ineffective: checking for any `.` in the full XML body can pass even when `WebIdentityToken` is not a valid JWT.</comment>
<file context>
@@ -1669,4 +1669,185 @@ mod tests {
+ let body = String::from_utf8(resp.body.expect_bytes().to_vec()).unwrap();
+ assert!(body.contains("WebIdentityToken"), "{body}");
+ // JWT structure: header.payload.signature (signature blank).
+ assert!(body.contains("."), "{body}");
+ }
+
</file context>
Suggested change
| assert!(body.contains("."), "{body}"); | |
| let token = body | |
| .split("<WebIdentityToken>") | |
| .nth(1) | |
| .and_then(|s| s.split("</WebIdentityToken>").next()) | |
| .unwrap_or(""); | |
| assert_eq!(token.matches('.').count(), 2, "{body}"); |
…-3ops # Conflicts: # conformance-baseline.json
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
AssumeRootmints short-lived (<=900s) member-account-root credentials, persists them in the IAM state so subsequent SigV4 lookups resolve to the new principal.GetDelegatedAccessTokentrades any non-emptyTradeInTokenfor fresh STS credentials tied to the caller (or account root when unauthenticated).GetWebIdentityTokenreturns an unsigned JWT (iss/sub/aud/iat/expclaims) so client decoders see standard structure;SigningAlgorithmvalidated againstRS256/ES384.Test plan
cargo test -p fakecloud-conformance --test sts-> 11 passcargo run -p fakecloud-conformance -- run --services sts-> 11/11 OKcargo run -p fakecloud-conformance -- audit/check-> PASScargo clippy --workspace --all-targets -- -D warningscargo fmtSummary by cubic
Adds three STS actions—AssumeRoot, GetDelegatedAccessToken, and GetWebIdentityToken—to mint short‑lived credentials and issue a web identity token. Completes STS conformance (11/11, 438/438), updates the conformance baseline, and adds unit tests for happy paths and validation errors.
New Features
Bug Fixes
Written for commit b59a43c. Summary will update on new commits.