feat(auth): resolve principal up front and attach to AwsRequest#391
Merged
vieiralucas merged 3 commits intomainfrom Apr 14, 2026
Merged
feat(auth): resolve principal up front and attach to AwsRequest#391vieiralucas merged 3 commits intomainfrom
vieiralucas merged 3 commits intomainfrom
Conversation
Introduces the Principal type and plumbs the resolved caller identity through AwsRequest so service handlers can make identity decisions without re-parsing the Authorization header. - New Principal struct + PrincipalType enum in fakecloud-core::auth (User / AssumedRole / FederatedUser / Root) with an is_root helper and PrincipalType::from_arn classifier. - ResolvedCredential now carries a Principal value instead of flat arn/user_id/account_id fields; accessor methods preserve batch 3's call sites. - IamCredentialResolver classifies the principal type from the stored ARN shape so STS temporary credentials surface as AssumedRole / FederatedUser automatically. - AwsRequest.principal: Option<Principal> populated by dispatch via a single up-front resolver call. SigV4 verification reuses the same lookup instead of calling resolve() twice. - STS GetCallerIdentity refactored to use req.principal first, falling back to the account-root identity only when the caller is the root bypass or didn't present credentials. Removes ~30 lines of duplicated AKID-walking logic. - Account id is always sourced from the credential itself (#381 note), priming multi-account isolation without implementing it. Tests: - 3 new auth:: unit tests (PrincipalType::from_arn classifier, Principal::is_root, ResolvedCredential accessor forwarding). - 1 new credential_resolver:: test classifying STS assumed-role temp credentials. - All existing STS e2e tests green via the refactored handler. - All existing SigV4 verification e2e tests green. - 22 unchanged service crates had their test helpers bulk-updated to supply principal: None / req.principal.clone().
There was a problem hiding this comment.
1 issue found across 25 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-core/src/auth.rs">
<violation number="1" location="crates/fakecloud-core/src/auth.rs:56">
P2: Fallback to `PrincipalType::Root` for unrecognized ARNs makes malformed or unexpected ARNs act as root, which can bypass IAM evaluation. Root should only be returned for explicit `...:root` ARNs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Identified by cubic on PR #391: PrincipalType::from_arn used Root as the catch-all fallback, but Principal::is_root short-circuits IAM enforcement. A malformed or unexpected ARN would silently grant root permissions during policy evaluation. - Added PrincipalType::Unknown for ARNs that don't match any well-known shape. - from_arn now requires an explicit ':root' suffix to return Root. Garbage / empty / unrecognized ARNs return Unknown, which is not treated as root by is_root() and so flows through the policy evaluator like any other non-root principal. - New regression test asserting empty / garbage / weird-IAM ARNs resolve to Unknown and that an Unknown Principal is not is_root().
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
Batch 4 of 9. Introduces the
Principaltype and plumbs the resolved caller identity throughAwsRequestso service handlers can make identity decisions without re-parsing the Authorization header. Prereq for batch 5/6 where the IAM evaluator needs to know the caller.Principalstruct +PrincipalTypeenum infakecloud-core::auth(User/AssumedRole/FederatedUser/Root) with anis_roothelper and an ARN classifier.ResolvedCredentialnow carries aPrincipalvalue instead of flatarn/user_id/account_idfields; accessor methods preserve batch 3's call sites.IamCredentialResolverclassifies the principal type from the stored ARN shape so STS temporary credentials fromAssumeRole/AssumeRoleWithSAML/AssumeRoleWithWebIdentitysurface asAssumedRole, andGetFederationTokencredentials surface asFederatedUser.AwsRequest.principal: Option<Principal>populated by dispatch via a single up-front resolver call. SigV4 verification reuses the same lookup instead of callingresolve()twice.StsService::get_caller_identityrefactored to usereq.principalfirst, removing ~30 lines of duplicated AKID-walking logic.account_idalways sourced from the credential itself (Feature proposal: multi-account isolation within a single fakecloud instance #381 note) — multi-account isolation becomes a state-partitioning change rather than a cross-cutting rewrite.Decisions
principal_arn()/user_id()/account_id()) onResolvedCredentialrather than duplicating the fields, keeping the struct honest and batch 3 call sites unchanged.PrincipalType::from_arnis best-effort and falls back toRootfor unparseable ARNs. This matches how AWS treats unauthenticated callers — they resolve to the account root.AwsRequestproxies bulk-updated to supplyprincipal: None/req.principal.clone(). No behavior change for those services.Test plan
cargo test -p fakecloud-core— 46 unit tests including 3 newauth::cases (PrincipalType::from_arn,Principal::is_root,ResolvedCredentialaccessor forwarding)cargo test -p fakecloud-iam credential_resolver— 3 tests including a newclassifies_sts_assumed_role_principalcargo test -p fakecloud-e2e --test iam— 57 tests,GetCallerIdentitystill round-trips IAM users and STS temp credentials via the newreq.principalpathcargo test -p fakecloud-e2e --test sigv4_verification— 6 tests, SigV4 path uses the shared resolver lookupcargo test -p fakecloud-conformance— no regressionscargo clippy --workspace --all-targets -- -D warningscleancargo fmt --checkcleanSummary by cubic
Resolve the caller’s identity once per request and attach a
PrincipaltoAwsRequest, so handlers can make identity-based decisions without re-parsing Authorization. Harden ARN classification so unknown shapes are never treated as root.New Features
PrincipalandPrincipalTypetofakecloud-core::authwithis_rootand an ARN classifier; introducedUnknownso unrecognized ARNs aren’t treated as root.ResolvedCredentialnow carries aPrincipal; addedprincipal_arn(),user_id(), andaccount_id()accessors.AwsRequest.principal: Option<Principal>is set by dispatch; account ID is sourced from the credential.IamCredentialResolverclassifies principals (User/AssumedRole/FederatedUser/Root) and returnsUnknownfor unrecognized shapes;Rootrequires an explicit:rootARN.Refactors
StsService::get_caller_identityreadsreq.principaland falls back to account root only when needed.principal: None; no behavior changes.rustfmt; no behavior changes.Written for commit 4e84e52. Summary will update on new commits.