fix(security/onboarding): add External ID collection + trust-policy snippet to bastion auth mode (#129) - #157
Conversation
…nippet to bastion auth mode
When the AWS account modal is in bastion auth mode, the frontend never
collected an aws_external_id and never rendered a trust-policy snippet
(only role_arn mode did). The user has confirmed this is an oversight,
not a deliberate scope decision: bastion mode also calls sts:AssumeRole
into the customer's role (just via the bastion's STS client instead of
ambient credentials), so the same sts:ExternalId confused-deputy guard
applies.
Frontend (frontend/src/index.html, frontend/src/settings.ts):
- Add a readonly External ID input + copy button + IAM trust-policy
<pre> block + Copy button inside #account-aws-bastion-fields,
mirroring the role_arn block. The two External ID inputs share the
same draft value (resolved via the existing
resolveAwsExternalIdForModal + AWS_DRAFT_EXTERNAL_ID_KEY from
PR #146 — no parallel generator) so toggling auth modes mid-edit
never surfaces diverging values.
- buildAccountRequest's bastion branch now collects aws_external_id
from the new bastion-mode input, undefined when blank.
- New renderAwsBastionTrustPolicy() mirrors renderAwsTrustPolicy but
sets Principal to the *bastion* account root (looked up via
listAccounts → matched on the dropdown selection), with the same
partition awareness (#130c) and race short-circuit (#130a) as the
role_arn renderer. Re-renders on auth-mode change and on bastion
dropdown change.
Backend (internal/api/handler_accounts.go):
- validateAWSAuthMode now also runs validateAWSExternalID for bastion
mode when aws_role_arn is non-empty, parallel to role_arn.
workload_identity_federation stays exempt (OIDC verifies identity
via the token subject claim — stscreds.WebIdentityRoleOptions has
no ExternalID field), and so does access_keys (no AssumeRole at
all). The empty-role-ARN exemption mirrors role_arn's self-account
onboarding shortcut.
- Updated the "required" error message to mention both modes.
- The credential resolver was already correct: resolveBastionProvider
delegates to resolveRoleARNProvider, which has always read
account.AWSExternalID and threaded it into stscreds.AssumeRoleOptions
— so the missing piece was the frontend collection + backend
validation, not the resolver.
Tests:
- frontend/src/__tests__/html.test.ts — assert the bastion section
has the new input, copy button, and trust-policy block.
- frontend/src/__tests__/settings-external-id.test.ts — new suite
covering: bastion shows the same draft as role_arn; selecting a
bastion renders a snippet with the bastion's account root as
Principal; buildAccountRequest sends aws_external_id; toggling
modes preserves the draft.
- frontend/src/__tests__/settings-accounts.test.ts — DOM scaffold
extended with the new bastion-mode fields.
- internal/api/handler_accounts_external_id_test.go — bastion now
enforced in dedicated tests (BastionRequiresExternalID,
BastionHappyPath, BastionNoRoleArnExempt) and removed from the
"no-validation-needed" loop.
Builds on PR #146 (#126/#127/#128/#130) — reuses the secure CSPRNG
generator, sessionStorage draft persistence, validateAWSExternalID
charset/length validator, and partition-aware ARN rendering.
Closes #129.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
When the AWS account modal is in bastion auth mode, the frontend
never collected an
aws_external_idand never rendered a trust-policysnippet (only
role_arnmode did). The user has confirmed this is anoversight, not a deliberate scope decision — bastion mode also
calls
sts:AssumeRoleinto the customer's role (just via the bastion'sSTS client instead of ambient credentials), so the same
sts:ExternalIdconfused-deputy guard applies.
This PR builds on PR #146 (External ID hardening for
role_arnmode — #126/#127/#128/#130) and reuses every primitive it landed:
crypto.getRandomValuesgenerator (no parallel generator)cudly:aws-account-modal:draft-external-idsessionStoragedraftkey (shared across both modes — the value is fungible, so a single
source of truth keeps the two readonly inputs in sync when an operator
toggles auth modes mid-edit)
validateAWSExternalIDbackend charset + length validatoraws/aws-cn/aws-us-gov)#130arace short-circuit when auth mode changes mid-fetchWhat changed
Frontend (
frontend/src/index.html,frontend/src/settings.ts):#account-aws-bastion-fieldsnow hosts a readonly External ID input<pre>block + Copy button, mirroringthe role_arn block.
populateAwsAccountFieldswrites the same draft value to bothaccount-aws-external-idandaccount-aws-external-id-bastion.buildAccountRequest's bastion branch now collectsaws_external_id(undefined when blank).renderAwsBastionTrustPolicy()mirrorsrenderAwsTrustPolicybut sets
Principalto the bastion account root (looked up vialistAccountsmatched on the dropdown selection). Re-renders onauth-mode change and on bastion dropdown change.
Backend (
internal/api/handler_accounts.go):validateAWSAuthModenow also runsvalidateAWSExternalIDforbastion mode when
aws_role_arnis non-empty, parallel to role_arn.workload_identity_federationstays exempt (OIDC verifies identityvia the token subject claim —
stscreds.WebIdentityRoleOptionshasno
ExternalIDfield), and so doesaccess_keys(no AssumeRole atall). The empty-role-ARN exemption mirrors role_arn's self-account
onboarding shortcut.
Resolver (
internal/credentials/resolver.go): no change needed —resolveBastionProvideralready delegates toresolveRoleARNProvider,which has always read
account.AWSExternalIDand threaded it intostscreds.AssumeRoleOptions. The missing piece was frontendcollection + backend validation, not the resolver.
Tests
frontend/src/__tests__/html.test.ts— assert the bastion sectionhas the new input, copy button, and trust-policy block.
frontend/src/__tests__/settings-external-id.test.ts— new suite(4 cases): bastion shows the same draft as role_arn; selecting a
bastion renders a snippet with the bastion's account root as
Principal;buildAccountRequestsendsaws_external_id; togglingmodes preserves the draft across close/reopen.
frontend/src/__tests__/settings-accounts.test.ts— DOM scaffoldextended with the new bastion-mode fields.
internal/api/handler_accounts_external_id_test.go— bastion nowenforced in dedicated cases (
BastionRequiresExternalID,BastionHappyPath,BastionNoRoleArnExempt) and removed from the"no-validation-needed" loop.
Test plan
go build ./...cleango test ./...— all packages passnpx jest— 39 suites / 1325 tests pass (was 1308 on main; +17new cases here)
npx tsc --noEmit && npm run build— typecheck + productionbundle clean
gosec, trivy, frontend tests, frontend build
(https://33pz7pombdqwu3bdlxp4lqxyra0bsriy.lambda-url.us-east-1.on.aws/)
— post-merge.
Out of scope
workload_identity_federation,access_keys) —already correctly omit External ID per the resolver semantics.
Closes #129.