diff --git a/crates/fakecloud-cloudformation/src/extras.rs b/crates/fakecloud-cloudformation/src/extras.rs index 528d8f48e..c214e8cb4 100644 --- a/crates/fakecloud-cloudformation/src/extras.rs +++ b/crates/fakecloud-cloudformation/src/extras.rs @@ -2941,7 +2941,9 @@ pub(crate) mod tests { ecr: shared::(), cloudwatch: Arc::new(RwLock::new(fakecloud_cloudwatch::CloudWatchAccounts::new())), elbv2: Arc::new(RwLock::new(fakecloud_elbv2::Elbv2Accounts::new())), - organizations: Arc::new(RwLock::new(None)), + organizations: Arc::new(RwLock::new( + fakecloud_organizations::OrganizationsRegistry::default(), + )), cognito: shared::(), rds: shared::(), ec2: shared::(), diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs index 455cf31ee..31012b50d 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/mod.rs @@ -4117,7 +4117,7 @@ mod tests { )), cloudwatch_state: Arc::new(RwLock::new(fakecloud_cloudwatch::CloudWatchAccounts::new())), elbv2_state: Arc::new(RwLock::new(fakecloud_elbv2::Elbv2Accounts::new())), - organizations_state: Arc::new(RwLock::new(None)), + organizations_state: Arc::new(RwLock::new(fakecloud_organizations::OrganizationsRegistry::default())), cognito_state: Arc::new(RwLock::new( fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )), @@ -4676,7 +4676,7 @@ mod tests { .expect("org provisions"); let root_id = { let g = prov.organizations_state.read(); - g.as_ref().unwrap().root_id.clone() + g.sole().unwrap().root_id.clone() }; let ou = prov @@ -4756,7 +4756,7 @@ mod tests { ); let g = prov.organizations_state.read(); - let org = g.as_ref().unwrap(); + let org = g.sole().unwrap(); assert_eq!(org.ous.get(&ou_id).unwrap().name, "team-renamed"); assert_eq!(org.policies.get(&pol_id).unwrap().content, "{\"v\":2}"); assert!( diff --git a/crates/fakecloud-cloudformation/src/resource_provisioner/organizations.rs b/crates/fakecloud-cloudformation/src/resource_provisioner/organizations.rs index ac7f7502e..65b83595b 100644 --- a/crates/fakecloud-cloudformation/src/resource_provisioner/organizations.rs +++ b/crates/fakecloud-cloudformation/src/resource_provisioner/organizations.rs @@ -17,8 +17,23 @@ impl ResourceProvisioner { .to_string(); let mut org = self.organizations_state.write(); - if org.is_some() { - return Err("Organization already exists; only one per fakecloud process".to_string()); + // Only the stack's own account blocks this. Organizations are + // independent, so another account having one must not stop this + // stack from creating its own (#2543). + if org.account_is_enrolled(&self.account_id) { + return Err(format!( + "Account {} is already a member of an organization", + self.account_id + )); + } + // The management account registers its own synthetic address, so + // that address must be free -- the same rule the API's + // `CreateOrganization` applies. + let management_email = format!("{}@example.com", self.account_id); + if org.email_in_use(&management_email) { + return Err(format!( + "The email address {management_email} is already associated with another account" + )); } let mut state = OrganizationState::bootstrap(&self.account_id); state.feature_set = feature_set; @@ -26,7 +41,7 @@ impl ResourceProvisioner { let org_arn = state.org_arn.clone(); let mgmt_arn = state.management_account_arn.clone(); let root_id = state.root_id.clone(); - *org = Some(state); + org.insert(state); Ok(ProvisionResult::new(org_id.clone()) .with("Id", org_id) @@ -35,9 +50,11 @@ impl ResourceProvisioner { .with("RootId", root_id)) } - pub(crate) fn delete_organization(&self, _physical_id: &str) -> Result<(), String> { + pub(crate) fn delete_organization(&self, physical_id: &str) -> Result<(), String> { + // The physical id IS the organization id, so delete exactly the + // one this stack created rather than every organization. let mut org = self.organizations_state.write(); - *org = None; + org.remove(physical_id); Ok(()) } @@ -60,7 +77,7 @@ impl ResourceProvisioner { let mut org_lock = self.organizations_state.write(); let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; // Accept root id, OU id, or `Ref`-resolved logical id (we map to root). let resolved_parent_id = if parent_id == org.root_id || org.ous.contains_key(&parent_id) { @@ -96,7 +113,7 @@ impl ResourceProvisioner { pub(crate) fn delete_organization_unit(&self, physical_id: &str) -> Result<(), String> { let mut org_lock = self.organizations_state.write(); - if let Some(org) = org_lock.as_mut() { + if let Some(org) = org_lock.org_of_account_mut(&self.account_id) { org.ous.remove(physical_id); org.attachments.remove(physical_id); } @@ -148,14 +165,43 @@ impl ResourceProvisioner { .unwrap_or_default(); let mut org_lock = self.organizations_state.write(); + // Authorize FIRST, as the API paths do: the address checks below + // span the registry, so running them before resolving the stack + // account's own organization would report whether an address is + // registered in an organization this stack has nothing to do with. + if org_lock.org_of_account(&self.account_id).is_none() { + return Err("Organization not yet created".to_string()); + } + // Same address-uniqueness rule the API enforces: resolution by + // address decides who may accept an EMAIL-targeted handshake, so + // two accounts sharing one would make that answer depend on id + // ordering. + if org_lock.email_in_use(&email) { + return Err(format!( + "The email address {email} is already associated with an account" + )); + } + // Mint from the registry so the id cannot collide with an account + // another organization already owns. + let new_account_id = org_lock.next_account_id(); + // ...and a `@example.com` address belongs to the id it + // spells, so this account cannot squat another one's. + if fakecloud_organizations::OrganizationsRegistry::email_reserved_for_other( + &email, + &new_account_id, + ) { + return Err(format!( + "The email address {email} is reserved for another account" + )); + } let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; // CFN provisioning is its own asynchronous flow; we don't need // a second layer of poll-for-completion on top. Begin the // request and immediately drive it to SUCCEEDED so the rest of // this provisioner sees a fully enrolled account. - let pending = org.begin_create_account(&email, &name, None); + let pending = org.begin_create_account(&email, &name, new_account_id, None); let status = org.complete_create_account(&pending.id).unwrap_or(pending); let account_id = status .account_id @@ -216,7 +262,7 @@ impl ResourceProvisioner { /// `close_account` so subsequent reads see it as suspended. pub(crate) fn delete_organization_account(&self, physical_id: &str) -> Result<(), String> { let mut org_lock = self.organizations_state.write(); - if let Some(org) = org_lock.as_mut() { + if let Some(org) = org_lock.org_of_account_mut(&self.account_id) { let _ = org.close_account(physical_id); } Ok(()) @@ -265,7 +311,7 @@ impl ResourceProvisioner { let mut org_lock = self.organizations_state.write(); let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; let id_suffix: String = Uuid::new_v4() .simple() @@ -307,7 +353,7 @@ impl ResourceProvisioner { pub(crate) fn delete_organization_policy(&self, physical_id: &str) -> Result<(), String> { let mut org_lock = self.organizations_state.write(); - if let Some(org) = org_lock.as_mut() { + if let Some(org) = org_lock.org_of_account_mut(&self.account_id) { org.policies.remove(physical_id); for attachments in org.attachments.values_mut() { attachments.remove(physical_id); @@ -334,7 +380,7 @@ impl ResourceProvisioner { let mut org_lock = self.organizations_state.write(); let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; org.resource_policy = Some(content); let arn = format!( @@ -349,7 +395,7 @@ impl ResourceProvisioner { _physical_id: &str, ) -> Result<(), String> { let mut org_lock = self.organizations_state.write(); - if let Some(org) = org_lock.as_mut() { + if let Some(org) = org_lock.org_of_account_mut(&self.account_id) { org.resource_policy = None; } Ok(()) @@ -378,7 +424,7 @@ impl ResourceProvisioner { let mut org_lock = self.organizations_state.write(); let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; let ou = org .ous @@ -430,7 +476,7 @@ impl ResourceProvisioner { let mut org_lock = self.organizations_state.write(); let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; // AccountName/Email are immutable; do NOT mint a new account. Move to a // new parent if ParentIds changed and refresh tags in place. @@ -504,7 +550,7 @@ impl ResourceProvisioner { let mut org_lock = self.organizations_state.write(); let org = org_lock - .as_mut() + .org_of_account_mut(&self.account_id) .ok_or_else(|| "Organization not yet created".to_string())?; let (arn, name) = { let policy = org diff --git a/crates/fakecloud-cloudformation/src/service.rs b/crates/fakecloud-cloudformation/src/service.rs index f7e10ee18..3811802aa 100644 --- a/crates/fakecloud-cloudformation/src/service.rs +++ b/crates/fakecloud-cloudformation/src/service.rs @@ -4233,7 +4233,9 @@ mod tests { )), cloudwatch: Arc::new(RwLock::new(fakecloud_cloudwatch::CloudWatchAccounts::new())), elbv2: Arc::new(RwLock::new(fakecloud_elbv2::Elbv2Accounts::new())), - organizations: Arc::new(RwLock::new(None)), + organizations: Arc::new(RwLock::new( + fakecloud_organizations::OrganizationsRegistry::default(), + )), cognito: Arc::new(RwLock::new( fakecloud_core::multi_account::MultiAccountState::new( "123456789012", diff --git a/crates/fakecloud-cloudformation/src/stack_sets.rs b/crates/fakecloud-cloudformation/src/stack_sets.rs index 9d724bbac..b727a370a 100644 --- a/crates/fakecloud-cloudformation/src/stack_sets.rs +++ b/crates/fakecloud-cloudformation/src/stack_sets.rs @@ -1501,7 +1501,7 @@ impl CloudFormationService { None | Some("SELF") => Ok(caller.to_string()), Some("DELEGATED_ADMIN") => { let orgs = self.deps.organizations.read(); - let org = orgs.as_ref().ok_or_else(|| { + let org = orgs.org_of_account(caller).ok_or_else(|| { validation("AWS Organizations is not enabled for this account") })?; let registered = org @@ -1526,7 +1526,7 @@ impl CloudFormationService { let trusted_in_org = { let orgs = self.deps.organizations.read(); let org = orgs - .as_ref() + .org_of_account(admin) .ok_or_else(|| validation("AWS Organizations is not enabled for this account"))?; if org.management_account_id != admin { return Err(validation( @@ -2051,7 +2051,7 @@ impl CloudFormationService { )?; (targets, explicit_regions.clone()) } else { - let suspended = self.suspended_accounts_for(&updated); + let suspended = self.suspended_accounts_for(&updated, &admin); let targets = updated .instances .iter() @@ -2254,7 +2254,7 @@ impl CloudFormationService { let filter = self.account_filter_type(dt, &filter_accounts)?; let orgs = self.deps.organizations.read(); let org = orgs - .as_ref() + .org_of_account(caller) .ok_or_else(|| validation("AWS Organizations is not enabled for this account"))?; let mut seen = BTreeSet::new(); for ou in &dt.organizational_unit_ids { @@ -2331,8 +2331,10 @@ impl CloudFormationService { } /// Organization member accounts that are not ACTIVE. Existing instances in - /// them are skipped again rather than failed. - fn suspended_accounts_for(&self, set: &StackSet) -> BTreeSet { + /// them are skipped again rather than failed. Scoped to `admin`'s own + /// organization — another organization's suspended accounts are none of + /// this stack set's business. + fn suspended_accounts_for(&self, set: &StackSet, admin: &str) -> BTreeSet { // Only service-managed stack sets deploy through the organization; // a self-managed one targets accounts directly. if set.permission_model != "SERVICE_MANAGED" { @@ -2341,7 +2343,7 @@ impl CloudFormationService { self.deps .organizations .read() - .as_ref() + .org_of_account(admin) .map(|org| { org.accounts .values() @@ -2428,7 +2430,7 @@ impl CloudFormationService { "Only one of Accounts or DeploymentTargets can be specified", )); } - let suspended = self.suspended_accounts_for(set); + let suspended = self.suspended_accounts_for(set, caller); let mut targets = Vec::new(); let mut push = |instance: &StackInstance| { if !targets @@ -2465,7 +2467,7 @@ impl CloudFormationService { .deps .organizations .read() - .as_ref() + .org_of_account(caller) .map(|org| { dt.organizational_unit_ids .iter() @@ -3679,7 +3681,7 @@ impl CloudFormationService { if !ous.is_empty() { let orgs = self.deps.organizations.read(); let org = orgs - .as_ref() + .org_of_account(&admin) .ok_or_else(|| validation("AWS Organizations is not enabled for this account"))?; for ou in &ous { for (account, _) in accounts_under(org, ou) { @@ -4022,10 +4024,9 @@ impl CloudFormationService { } async fn reconcile_auto_deployments_once(&self, deadline: tokio::time::Instant) { - let org = match self.deps.organizations.read().as_ref() { - Some(org) => org.clone(), - None => return, - }; + if self.deps.organizations.read().is_empty() { + return; + } let candidates: Vec<(String, String)> = { let accounts = self.state.read(); accounts @@ -4041,8 +4042,30 @@ impl CloudFormationService { }) .collect() }; + // Each stack set reconciles against ITS OWN administrator's + // organization. Reconciling every stack set in the process against + // one organization would deploy an administrator's stack set to + // accounts belonging to a different organization. + // + // `candidates` is grouped by administrator, and one administrator + // usually owns several stack sets, so the organization is cloned + // once per administrator rather than once per stack set -- the + // clone carries every account, OU and policy. + let mut current: Option<(String, fakecloud_organizations::OrganizationState)> = None; for (admin, set_id) in candidates { - self.reconcile_stack_set_auto_deployment(&org, &admin, &set_id, deadline) + if current.as_ref().is_none_or(|(owner, _)| owner != &admin) { + current = self + .deps + .organizations + .read() + .org_of_account(&admin) + .cloned() + .map(|org| (admin.clone(), org)); + } + let Some((_, org)) = ¤t else { + continue; + }; + self.reconcile_stack_set_auto_deployment(org, &admin, &set_id, deadline) .await; } } @@ -4130,7 +4153,7 @@ impl CloudFormationService { // Without the organization there is nothing to work out who was // left out, but what this call deployed is still known, and the // exclusions on those are cleared below either way. - if let Some(org) = orgs.as_ref() { + if let Some(org) = orgs.org_of_account(admin) { for ou in &dt.organizational_unit_ids { for (account, _) in accounts_under(org, ou) { for region in regions { @@ -5655,7 +5678,7 @@ mod tests { org.enroll_account_if_missing(account); org.move_account(account, &root, dest).unwrap(); } - *svc.deps.organizations.write() = Some(org); + svc.deps.organizations.write().insert(org); (parent.id, child.id) } @@ -5704,7 +5727,7 @@ mod tests { /// Put `account` in the organization under `parent`. fn join_ou(svc: &CloudFormationService, account: &str, parent: &str) { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().expect("organization"); + let org = guard.sole_mut().expect("organization"); let root = org.root_id.clone(); org.enroll_account_if_missing(account); org.move_account(account, &root, parent).unwrap(); @@ -5764,7 +5787,7 @@ mod tests { // ACCT_C moves out of the target OU tree, back to the root. { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().expect("organization"); + let org = guard.sole_mut().expect("organization"); let root = org.root_id.clone(); let parent = org.parent_of(ACCT_C).expect("parent").0; org.move_account(ACCT_C, &parent, &root).unwrap(); @@ -5793,7 +5816,7 @@ mod tests { { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().expect("organization"); + let org = guard.sole_mut().expect("organization"); let root = org.root_id.clone(); let parent = org.parent_of(ACCT_C).expect("parent").0; org.move_account(ACCT_C, &parent, &root).unwrap(); @@ -5843,7 +5866,7 @@ mod tests { // covers the account while it is still there. let (root, parent) = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); let parent = org.parent_of(ACCT_C).unwrap().0; org.move_account(ACCT_C, &parent, &root).unwrap(); @@ -5852,7 +5875,7 @@ mod tests { svc.reconcile_auto_deployments().await; { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account(ACCT_C, &root, &parent).unwrap(); } svc.reconcile_auto_deployments().await; @@ -5923,7 +5946,7 @@ mod tests { // Two sibling OUs, each a target in the same region. let other = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -5951,7 +5974,7 @@ mod tests { // Moving between two target OUs keeps the stack but re-attributes it. { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account(ACCT_D, &other, &prod).unwrap(); } svc.reconcile_auto_deployments().await; @@ -5996,7 +6019,7 @@ mod tests { // It leaves, so the stack set has no instances at all left. { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.move_account(ACCT_C, &prod, &root).unwrap(); } @@ -6041,7 +6064,7 @@ mod tests { .await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -6075,7 +6098,7 @@ mod tests { // instance in the OU it left must not be orphaned. { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account(ACCT_D, &sandbox, &prod).unwrap(); } svc.reconcile_auto_deployments().await; @@ -6431,7 +6454,7 @@ mod tests { // A stack in an account under a *different* OU, adopted into the set. let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -6684,7 +6707,7 @@ mod tests { let (_workloads, _prod) = auto_deployed_set(&svc, "org", false).await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -6727,7 +6750,7 @@ mod tests { auto_deployed_set(&svc, "org", false).await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -6798,7 +6821,7 @@ mod tests { let (_workloads, prod) = auto_deployed_set(&svc, "org", false).await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -6932,7 +6955,7 @@ mod tests { .await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -6954,7 +6977,7 @@ mod tests { std::fs::remove_file(&path).ok(); { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account(ACCT_D, &sandbox, &prod).unwrap(); } @@ -6997,7 +7020,7 @@ mod tests { let (workloads, _prod) = auto_deployed_set(&svc, "org", false).await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -7036,7 +7059,7 @@ mod tests { // any other: the decision was about the OU it left. { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let parent = org.parent_of(ACCT_C).unwrap().0; org.move_account(ACCT_C, &parent, &sandbox).unwrap(); } @@ -7095,7 +7118,7 @@ mod tests { auto_deployed_set(&svc, "org", false).await; let sandbox = { let mut guard = svc.deps.organizations.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); org.create_ou(&root, "sandbox").unwrap().id }; @@ -7353,7 +7376,7 @@ mod tests { seed_org(&svc); { let mut orgs = svc.deps.organizations.write(); - let org = orgs.as_mut().unwrap(); + let org = orgs.sole_mut().unwrap(); org.enable_aws_service_access(STACKSETS_PRINCIPAL); org.register_delegated_administrator(ACCT_B, STACKSETS_PRINCIPAL) .unwrap(); @@ -7666,7 +7689,7 @@ mod tests { seed_org(&svc); { let mut orgs = svc.deps.organizations.write(); - let org = orgs.as_mut().unwrap(); + let org = orgs.sole_mut().unwrap(); org.enable_aws_service_access(STACKSETS_PRINCIPAL); org.register_delegated_administrator(ACCT_B, STACKSETS_PRINCIPAL) .unwrap(); @@ -7778,7 +7801,7 @@ mod tests { svc.deps .organizations .write() - .as_mut() + .sole_mut() .unwrap() .close_account(ACCT_C) .unwrap(); @@ -7891,7 +7914,7 @@ mod tests { seed_org(&svc); { let mut orgs = svc.deps.organizations.write(); - let org = orgs.as_mut().unwrap(); + let org = orgs.sole_mut().unwrap(); org.enable_aws_service_access(STACKSETS_PRINCIPAL); org.register_delegated_administrator(ACCT_B, STACKSETS_PRINCIPAL) .unwrap(); @@ -8227,7 +8250,7 @@ mod tests { svc.deps .organizations .write() - .as_mut() + .sole_mut() .unwrap() .close_account(ACCT_C) .unwrap(); diff --git a/crates/fakecloud-e2e/tests/organizations.rs b/crates/fakecloud-e2e/tests/organizations.rs index 37a85baf8..f266bb651 100644 --- a/crates/fakecloud-e2e/tests/organizations.rs +++ b/crates/fakecloud-e2e/tests/organizations.rs @@ -230,11 +230,9 @@ async fn ou_tree_crud_and_move_account() { /// enrolling it handed the account another organization's SCP ceiling, /// exposed that organization's metadata to it, and made it a stack-set /// auto-deployment target. -/// -/// This is one half of #2543. The other half -- letting a second -/// management account create an organization of its own -- needs the -/// process-wide single-organization state to become a registry, and -/// lands separately. +/// See `two_accounts_each_run_their_own_organization` for the other +/// half of #2543: a second management account running an organization +/// of its own. #[tokio::test] async fn create_admin_leaves_the_account_outside_an_existing_organization() { let server = start().await; @@ -261,6 +259,133 @@ async fn create_admin_leaves_the_account_outside_an_existing_organization() { assert_eq!(ids, [ACCOUNT_A]); } +/// The whole of #2543's repro: two management accounts, each with its +/// own organization and its own child accounts, fully independent. +/// Before multi-organization support the second `CreateOrganization` +/// failed with `AlreadyInOrganizationException` purely because the +/// first organization existed. +#[tokio::test] +async fn two_accounts_each_run_their_own_organization() { + let server = start().await; + + // Distinct from the management ids: reusing an owner's own id as a + // child would re-bootstrap that account's admin user and invalidate + // the credentials this test is already holding. + const CHILDREN_A: [&str; 2] = ["111111110001", "111111110002"]; + const CHILDREN_B: [&str; 2] = ["222222220001", "222222220002"]; + + let mut orgs_by_owner = Vec::new(); + for (owner, children) in [(ACCOUNT_A, CHILDREN_A), (ACCOUNT_B, CHILDREN_B)] { + let (akid, secret) = server.create_admin(owner, "root").await; + let cfg = config_with(&server, &akid, &secret).await; + let orgs = OrgsClient::new(&cfg); + + let org_id = orgs + .create_organization() + .feature_set(aws_sdk_organizations::types::OrganizationFeatureSet::All) + .send() + .await + .unwrap() + .organization() + .unwrap() + .id() + .unwrap() + .to_string(); + + // Two children per organization, enrolled at bootstrap. + for child in children { + server.create_admin_in_org(child, "root", &org_id).await; + } + orgs_by_owner.push((owner, orgs, org_id, children)); + } + + let (_, orgs_a, org_a, children_a) = &orgs_by_owner[0]; + let (_, orgs_b, org_b, children_b) = &orgs_by_owner[1]; + assert_ne!(org_a, org_b, "each account gets its own organization"); + + // Each management account sees only its own organization... + assert_eq!( + orgs_a + .describe_organization() + .send() + .await + .unwrap() + .organization() + .unwrap() + .id() + .unwrap(), + org_a + ); + assert_eq!( + orgs_b + .describe_organization() + .send() + .await + .unwrap() + .organization() + .unwrap() + .id() + .unwrap(), + org_b + ); + + // ...and only its own accounts: management plus its two children, + // with nothing from the other organization leaking in. + for (owner, orgs, _, children) in [ + (ACCOUNT_A, orgs_a, org_a, children_a), + (ACCOUNT_B, orgs_b, org_b, children_b), + ] { + let listed = orgs.list_accounts().send().await.unwrap(); + let mut ids: Vec = listed + .accounts() + .iter() + .filter_map(|a| a.id()) + .map(str::to_string) + .collect(); + ids.sort(); + let mut expected: Vec = children.iter().map(|c| c.to_string()).collect(); + expected.push(owner.to_string()); + expected.sort(); + assert_eq!( + ids, expected, + "organization of {owner} has the wrong members" + ); + } +} + +/// An account can only ever be in one organization: an invitation to an +/// account another organization already holds is rejected. +#[tokio::test] +async fn an_account_cannot_be_invited_into_a_second_organization() { + let server = start().await; + let (a_akid, a_secret) = server.create_admin(ACCOUNT_A, "admin-a").await; + let a_cfg = config_with(&server, &a_akid, &a_secret).await; + let orgs_a = OrgsClient::new(&a_cfg); + orgs_a.create_organization().send().await.unwrap(); + + let (b_akid, b_secret) = server.create_admin(ACCOUNT_B, "admin-b").await; + let b_cfg = config_with(&server, &b_akid, &b_secret).await; + let orgs_b = OrgsClient::new(&b_cfg); + orgs_b.create_organization().send().await.unwrap(); + + let err = orgs_a + .invite_account_to_organization() + .target( + aws_sdk_organizations::types::HandshakeParty::builder() + .id(ACCOUNT_B) + .r#type(aws_sdk_organizations::types::HandshakePartyType::Account) + .build() + .unwrap(), + ) + .send() + .await + .unwrap_err(); + assert!( + format!("{err:?}").contains("HandshakeConstraintViolationException"), + "expected HandshakeConstraintViolationException, got: {err:?}" + ); +} + /// The opt-in half of #2543: naming an organization on the bootstrap /// request enrolls the account into it, the shortcut equivalent of an /// invite/accept handshake. diff --git a/crates/fakecloud-e2e/tests/organizations_introspection.rs b/crates/fakecloud-e2e/tests/organizations_introspection.rs index fd5eafe83..8a5bc2bef 100644 --- a/crates/fakecloud-e2e/tests/organizations_introspection.rs +++ b/crates/fakecloud-e2e/tests/organizations_introspection.rs @@ -187,6 +187,16 @@ async fn responsibility_transfers_and_firehose_streams_introspection() { orgs.create_organization().send().await.unwrap(); + // The transfer targets another ORGANIZATION's management account, so + // stand one up to invite. + let (t_akid, t_secret) = server.create_admin("222222222222", "admin").await; + let target_cfg = config_with(&server, &t_akid, &t_secret).await; + OrgsClient::new(&target_cfg) + .create_organization() + .send() + .await + .unwrap(); + // Invite an outbound BILLING responsibility transfer. orgs.invite_organization_to_transfer_responsibility() .r#type(ResponsibilityTransferType::Billing) diff --git a/crates/fakecloud-organizations/src/introspection.rs b/crates/fakecloud-organizations/src/introspection.rs index febd15e16..44aeae950 100644 --- a/crates/fakecloud-organizations/src/introspection.rs +++ b/crates/fakecloud-organizations/src/introspection.rs @@ -13,12 +13,19 @@ use crate::state::{ResponsibilityTransfer, SharedOrganizationsState}; /// One billing-responsibility transfer flattened for introspection. #[derive(Debug, Clone)] pub struct ResponsibilityTransferRow { + /// The organization that holds the transfer. The listing spans every + /// organization, so without this a reader cannot tell them apart. + pub organization_id: String, pub id: String, pub arn: String, pub name: String, pub transfer_type: String, pub status: String, - /// INBOUND / OUTBOUND. + /// `INBOUND` / `OUTBOUND`, relative to this row's `organization_id`. + /// A transfer is recorded once, in the source organization, so every + /// row reads `OUTBOUND`; the target organization reads the same + /// transfer as inbound through `ListInboundResponsibilityTransfers`, + /// which resolves by party rather than by this field. pub direction: String, pub source_management_account_id: String, pub source_management_account_email: String, @@ -29,8 +36,9 @@ pub struct ResponsibilityTransferRow { pub active_handshake_id: Option, } -fn transfer_to_row(t: &ResponsibilityTransfer) -> ResponsibilityTransferRow { +fn transfer_to_row(org_id: &str, t: &ResponsibilityTransfer) -> ResponsibilityTransferRow { ResponsibilityTransferRow { + organization_id: org_id.to_string(), id: t.id.clone(), arn: t.arn.clone(), name: t.name.clone(), @@ -47,19 +55,19 @@ fn transfer_to_row(t: &ResponsibilityTransfer) -> ResponsibilityTransferRow { } } -/// List every billing-responsibility transfer in the org, sorted by id. -/// Empty when no organization has been created. +/// List every billing-responsibility transfer across every organization, +/// sorted by id. Empty when no organization has been created. pub fn list_all_responsibility_transfers( state: &SharedOrganizationsState, ) -> Vec { let guard = state.read(); - let Some(org) = guard.as_ref() else { - return Vec::new(); - }; - let mut rows: Vec = org - .responsibility_transfers - .values() - .map(transfer_to_row) + let mut rows: Vec = guard + .iter() + .flat_map(|org| { + org.responsibility_transfers + .values() + .map(|t| transfer_to_row(&org.org_id, t)) + }) .collect(); rows.sort_by(|a, b| a.id.cmp(&b.id)); rows @@ -72,7 +80,9 @@ mod tests { #[test] fn empty_when_no_org() { - let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(None)); + let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new( + crate::state::OrganizationsRegistry::default(), + )); assert!(list_all_responsibility_transfers(&state).is_empty()); } @@ -101,7 +111,7 @@ mod tests { }, ); } - let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(Some(org))); + let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(org.into())); let rows = list_all_responsibility_transfers(&state); assert_eq!(rows.len(), 2); assert_eq!(rows[0].id, "rt-a"); diff --git a/crates/fakecloud-organizations/src/lib.rs b/crates/fakecloud-organizations/src/lib.rs index f8b940072..a0379f007 100644 --- a/crates/fakecloud-organizations/src/lib.rs +++ b/crates/fakecloud-organizations/src/lib.rs @@ -5,7 +5,8 @@ pub(crate) mod state; pub use service::{OrgChangeHook, OrgChangeHooks, OrganizationsService}; pub use state::{ - MemberAccount, OrganizationState, OrganizationalUnit, OrganizationsSnapshot, Policy, - ResponsibilityTransfer, SharedOrganizationsState, FEATURE_SET_ALL, - FEATURE_SET_CONSOLIDATED_BILLING, ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, POLICY_TYPE_SCP, + MemberAccount, OrganizationState, OrganizationalUnit, OrganizationsRegistry, + OrganizationsSnapshot, Policy, ResponsibilityTransfer, SharedOrganizationsState, + FEATURE_SET_ALL, FEATURE_SET_CONSOLIDATED_BILLING, ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, + POLICY_TYPE_SCP, }; diff --git a/crates/fakecloud-organizations/src/resolver.rs b/crates/fakecloud-organizations/src/resolver.rs index 939c7b148..a1f863c79 100644 --- a/crates/fakecloud-organizations/src/resolver.rs +++ b/crates/fakecloud-organizations/src/resolver.rs @@ -39,7 +39,10 @@ impl OrganizationsScpResolver { impl ScpResolver for OrganizationsScpResolver { fn scps_for(&self, principal: &Principal) -> Option> { let guard = self.state.read(); - let org = guard.as_ref()?; + // The ceiling that applies is the one from the principal's OWN + // organization. A principal in no organization has no ceiling — + // another organization's SCPs must never reach it. + let org = guard.org_of_account(&principal.account_id)?; // Management account: always exempt. if org.is_management(&principal.account_id) { @@ -205,12 +208,14 @@ impl OrganizationsMembershipResolver { impl OrgMembershipResolver for OrganizationsMembershipResolver { fn can_assume_root_into(&self, caller_account: &str, target_account: &str) -> bool { let guard = self.state.read(); - let Some(org) = guard.as_ref() else { - // No organization exists — there is no centralized root access to - // grant, so cross-account AssumeRoot is never permitted. + // Resolve the CALLER's organization: centralized root access is + // granted within one organization, so a caller outside any + // organization has none to grant. + let Some(org) = guard.org_of_account(caller_account) else { return false; }; - // The target must be enrolled in this organization. + // The target must be enrolled in that same organization — a + // management account has no reach into another organization. if !org.accounts.contains_key(target_account) { return false; } @@ -233,7 +238,7 @@ mod tests { use parking_lot::RwLock; fn shared(org: OrganizationState) -> SharedOrganizationsState { - Arc::new(RwLock::new(Some(org))) + Arc::new(RwLock::new(org.into())) } fn user_principal(account: &str) -> Principal { @@ -249,7 +254,8 @@ mod tests { #[test] fn no_org_returns_none() { - let state: SharedOrganizationsState = Arc::new(RwLock::new(None)); + let state: SharedOrganizationsState = + Arc::new(RwLock::new(crate::state::OrganizationsRegistry::default())); let resolver = OrganizationsScpResolver::new(state); assert!(resolver.scps_for(&user_principal("111111111111")).is_none()); } @@ -411,7 +417,8 @@ mod tests { #[test] fn membership_resolver_denies_when_no_org() { - let state: SharedOrganizationsState = Arc::new(RwLock::new(None)); + let state: SharedOrganizationsState = + Arc::new(RwLock::new(crate::state::OrganizationsRegistry::default())); let resolver = OrganizationsMembershipResolver::new(state); assert!(!resolver.can_assume_root_into("111111111111", "222222222222")); } diff --git a/crates/fakecloud-organizations/src/service/accounts.rs b/crates/fakecloud-organizations/src/service/accounts.rs index e90a399e4..60a874b6a 100644 --- a/crates/fakecloud-organizations/src/service/accounts.rs +++ b/crates/fakecloud-organizations/src/service/accounts.rs @@ -55,8 +55,7 @@ impl OrganizationsService { let source = required_str(&body, "SourceParentId")?; let dest = required_str(&body, "DestinationParentId")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.move_account(account_id, source, dest) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(Value::Null)) @@ -68,9 +67,23 @@ impl OrganizationsService { let name = required_str(&body, "AccountName")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); - let status = org.begin_create_account(&email, &name, None); + // The address check runs in the completion tick, not here: AWS + // reports a duplicate as a FAILED request rather than a + // synchronous error, and running a registry-wide check here would + // also tell any caller whether an address is registered in an + // organization it has nothing to do with. + let org_id = self + .management_org_mut(&mut guard, &req.account_id)? + .org_id + .clone(); + // Mint from the registry: an account id names one account + // process-wide, so a per-organization check could hand out an id + // another organization already owns. + let new_account_id = guard.next_account_id(); + let org = guard + .org_by_id_mut(&org_id) + .expect("management gate resolved this organization"); + let status = org.begin_create_account(&email, &name, new_account_id, None); let request_id = status.id.clone(); // Apply create-time Tags to the reserved account id so // ListTagsForResource reflects them without a follow-up TagResource. @@ -100,13 +113,24 @@ impl OrganizationsService { let name = required_str(&body, "AccountName")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + // Authorize before the registry-wide address check, as above. + let org_id = self + .management_org_mut(&mut guard, &req.account_id)? + .org_id + .clone(); // The GovCloud "paired" id is a 12-digit account id in the // GovCloud partition; we mint one alongside the commercial id - // so callers see both, matching the real AWS response. - let gov_id = org.next_account_id(); - let status = org.begin_create_account(&email, &name, Some(gov_id)); + // so callers see both, matching the real AWS response. Both come + // from the registry so neither can collide with another + // organization's account. + let new_account_id = guard.next_account_id(); + // Exclude the id just minted: it is not recorded anywhere yet, so + // a plain second call could hand back the same one. + let gov_id = guard.next_account_id_besides(&[new_account_id.as_str()]); + let org = guard + .org_by_id_mut(&org_id) + .expect("management gate resolved this organization"); + let status = org.begin_create_account(&email, &name, new_account_id, Some(gov_id)); let request_id = status.id.clone(); // Apply create-time Tags to the reserved (primary) account id, mirroring // CreateAccount; the id is reserved synchronously (bug-hunt). @@ -147,9 +171,39 @@ impl OrganizationsService { }; tokio::spawn(async move { tokio::time::sleep(delay).await; + let mut failed = false; let completed = { let mut guard = state.write(); - match guard.as_mut() { + // An address already in use fails the request rather than + // the call: `CreateAccount` models no synchronous error for + // it, and clients hand back a request id to poll. AWS + // reports it as FAILED with EMAIL_ALREADY_EXISTS. + let taken = guard + .org_of_create_account_request(&request_id) + .and_then(|org| org.create_account_requests.get(&request_id)) + .and_then(|req| req.pending_email.clone()) + .is_some_and(|email| { + // The address must be free, and must not be the + // synthetic form reserved for a different id. + let spelled_for_other = guard + .org_of_create_account_request(&request_id) + .and_then(|org| org.create_account_requests.get(&request_id)) + .and_then(|req| req.account_id.clone()) + .is_some_and(|mine| { + crate::state::OrganizationsRegistry::email_reserved_for_other( + &email, &mine, + ) + }); + spelled_for_other || guard.email_in_use_besides(&email, &request_id) + }); + // Request ids are globally unique, so the owning + // organization is whichever one holds this request. + match guard.org_of_create_account_request_mut(&request_id) { + Some(org) if taken => { + org.fail_create_account(&request_id, "EMAIL_ALREADY_EXISTS"); + failed = true; + false + } Some(org) => { org.complete_create_account(&request_id); true @@ -157,8 +211,14 @@ impl OrganizationsService { None => false, } }; - if completed { + if completed || failed { + // FAILED is durable too: the request was persisted as + // IN_PROGRESS, so a restart would otherwise re-arm it and + // a poller would watch it go FAILED -> IN_PROGRESS -> + // FAILED. super::save_organizations_snapshot(&state, store, &lock).await; + } + if completed { // The account only joins the organization here, so this is // where StackSets auto-deployment gets to see it. hooks.fire_if_membership_changed(&state).await; @@ -275,8 +335,7 @@ impl OrganizationsService { let target = required_str(&body, "AccountId")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.close_account(&target).map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(json!({}))) } @@ -289,8 +348,7 @@ impl OrganizationsService { let target = required_str(&body, "AccountId")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.remove_account(&target).map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(json!({}))) } @@ -298,13 +356,22 @@ impl OrganizationsService { /// `LeaveOrganization` removes the *calling* member account from its /// organization. The management account cannot leave its own org /// (it must `DeleteOrganization` instead), and a caller that isn't a - /// member of any org gets `AccountNotFoundException`. + /// member of any org gets `AWSOrganizationsNotInUseException` — the + /// op takes no AccountId, and with several organizations in the + /// process that is also the non-leaking answer. pub(super) fn leave_organization( &self, req: &AwsRequest, ) -> Result { let mut guard = self.state.write(); - let org = guard.as_mut().ok_or_else(organizations_not_in_use)?; + // Resolve the caller's OWN organization: an account can only leave + // the one it is actually in. The op takes no AccountId, so AWS + // answers a caller that belongs to no organization with + // `AWSOrganizationsNotInUseException` -- also the non-leaking answer + // now that several organizations can coexist. + let org = guard + .org_of_account_mut(&req.account_id) + .ok_or_else(organizations_not_in_use)?; if org.is_management(&req.account_id) { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST, @@ -313,16 +380,6 @@ impl OrganizationsService { delete the organization instead.", )); } - if !org.accounts.contains_key(&req.account_id) { - return Err(AwsServiceError::aws_error( - StatusCode::BAD_REQUEST, - "AccountNotFoundException", - format!( - "The account {} is not a member of an organization.", - req.account_id - ), - )); - } org.remove_account(&req.account_id) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(json!({}))) @@ -340,10 +397,13 @@ impl OrganizationsService { "Target is required", ) })?; + // `HandshakeParty.Type` is modeled required. Defaulting it meant a + // caller that omitted it had its id validated against a type it + // never declared. let kind = target_obj .get("Type") .and_then(|v| v.as_str()) - .unwrap_or("ACCOUNT"); + .ok_or_else(|| invalid_input("Target.Type is required"))?; let id = target_obj .get("Id") .and_then(|v| v.as_str()) @@ -355,6 +415,11 @@ impl OrganizationsService { ) })? .to_string(); + // Validate the id against the declared kind. AWS rejects a + // mismatch with InvalidInputException; accepting one here would + // open a handshake keyed by a string no caller can ever + // authenticate as, which sits OPEN forever. + validate_invite_target(kind, &id)?; let notes = body .get("Notes") .and_then(|v| v.as_str()) @@ -366,13 +431,38 @@ impl OrganizationsService { }; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org_id = self + .management_org_mut(&mut guard, &req.account_id)? + .org_id + .clone(); + // An account belongs to at most one organization, so an invitation + // to an account another organization already holds must be rejected + // here rather than quietly opening a handshake that could never be + // accepted. A target that is already OUR member falls through to + // `invite_account`, which reports it as such. + // Applies to an EMAIL target once it resolves, but resolution here + // is deliberately scoped to the caller's own organization: + // resolving registry-wide would let the caller read a foreign + // account id back out of the error below. An address registered + // in ANOTHER organization therefore slips past this guard and is + // caught at accept time instead, which costs a handshake that + // sits OPEN until it expires -- the price of not answering "whose + // account is this?" to anyone who asks. + if let Some(target) = guard.resolve_target_account(kind, &id, &org_id) { + if guard.claimed_by_other_org(&target, &org_id).is_some() { + return Err(org_error_to_aws( + crate::state::OrgError::AccountInAnotherOrganization(target), + )); + } + } + let org = guard + .org_by_id_mut(&org_id) + .expect("management gate resolved this organization"); let handshake = org - .invite_account(&req.account_id, &id, target_email, notes) + .invite_account(&req.account_id, kind, &id, target_email, notes) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json( - json!({ "Handshake": handshake_payload(&handshake) }), + json!({ "Handshake": handshake_payload(org, &handshake) }), )) } @@ -388,15 +478,35 @@ impl OrganizationsService { // ListHandshakesForAccount is scoped to the calling account, not to // an organization — a caller in no org simply has no handshakes. // (The op doesn't even declare AWSOrganizationsNotInUseException.) - let filtered: Vec = match guard.as_ref() { - Some(org) => org - .list_handshakes(Some(&req.account_id)) - .into_iter() - .filter(|h| handshake_matches_filter(h, &filter)) - .map(|h| handshake_payload(&h)) - .collect(), - None => Vec::new(), - }; + // It spans every organization on purpose: the invitations a + // standalone account most wants to list are the ones held by the + // organizations inviting it, none of which it belongs to yet. + let mut filtered: Vec = guard + .iter() + .flat_map(|org| org.list_handshakes().into_iter().map(move |h| (org, h))) + .filter(|(_, h)| { + // "Associated with the account of the requesting user" + // includes the invitations it SENT, not only those + // addressed to it. Cross-organization isolation is + // unaffected: the source is always a member of the + // organization that owns the handshake. + h.source_account_id == req.account_id + || guard.account_matches_target( + &h.target_kind, + &h.target_account_id, + &req.account_id, + ) + }) + .filter(|(_, h)| handshake_matches_filter(h, &filter)) + .map(|(org, h)| handshake_payload(org, &h)) + .collect(); + // `list_handshakes` sorts within an organization; re-sort so the + // merged result is stable and paginates consistently. + filtered.sort_by(|a, b| { + a.get("Id") + .and_then(Value::as_str) + .cmp(&b.get("Id").and_then(Value::as_str)) + }); let (page, token) = paginate_checked(&filtered, next_token.as_deref(), max_results) .map_err(|_| invalid_input("Invalid NextToken"))?; let mut body = json!({ "Handshakes": page }); @@ -413,9 +523,8 @@ impl OrganizationsService { let body = req.json_body(); let account_id = required_str(&body, "AccountId")?.to_string(); let (max_results, next_token) = parse_list_pagination(&body)?; - let guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_ref().expect("management gate proved Some"); + let guard = self.state.read(); + let org = self.management_or_delegated_org(&guard, &req.account_id)?; let entries: Vec = org .list_delegated_services_for_account(&account_id) .into_iter() @@ -435,3 +544,46 @@ impl OrganizationsService { Ok(AwsResponse::ok_json(body)) } } + +/// Check `Target.Id` against the declared `Target.Type`. AWS accepts +/// `ACCOUNT` and `EMAIL` only. +/// +/// An `ACCOUNT` target must be a 12-digit account id. An `EMAIL` target +/// must be an address, and any address is accepted — inviting the +/// account owner's real address is AWS's primary flow. fakecloud can +/// only resolve one it minted itself (`@example.com`) back +/// to an account, so an external address yields a handshake its source +/// can read and cancel but that no caller can prove it is the target +/// of; it stays OPEN until it expires, mirroring AWS before the owner +/// acts on the emailed link. +pub(super) fn validate_invite_target(kind: &str, id: &str) -> Result<(), AwsServiceError> { + match kind { + "ACCOUNT" => { + if id.len() == 12 && id.chars().all(|c| c.is_ascii_digit()) { + Ok(()) + } else { + Err(invalid_input( + "Target.Id must be a 12-digit account id when Target.Type is ACCOUNT", + )) + } + } + "EMAIL" => { + // AWS's primary invite flow names the account owner's real + // address, so any address is accepted. fakecloud can only + // resolve one it minted itself (`@example.com`) + // back to an account, so an external address yields a handshake + // its source can read and cancel but that no caller can prove + // it is the target of -- same as AWS before the owner acts on + // the emailed link. + if !id.contains('@') { + return Err(invalid_input( + "Target.Id must be an email address when Target.Type is EMAIL", + )); + } + Ok(()) + } + other => Err(invalid_input(&format!( + "Target.Type must be one of [ACCOUNT, EMAIL], got {other}" + ))), + } +} diff --git a/crates/fakecloud-organizations/src/service/delegated.rs b/crates/fakecloud-organizations/src/service/delegated.rs index 955bf2d1a..0caff308a 100644 --- a/crates/fakecloud-organizations/src/service/delegated.rs +++ b/crates/fakecloud-organizations/src/service/delegated.rs @@ -11,8 +11,7 @@ impl OrganizationsService { let account_id = required_str(&body, "AccountId")?.to_string(); let principal = required_str(&body, "ServicePrincipal")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.register_delegated_administrator(&account_id, &principal) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(json!({}))) @@ -26,8 +25,7 @@ impl OrganizationsService { let account_id = required_str(&body, "AccountId")?.to_string(); let principal = required_str(&body, "ServicePrincipal")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.deregister_delegated_administrator(&account_id, &principal) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(json!({}))) @@ -43,9 +41,8 @@ impl OrganizationsService { .and_then(|v| v.as_str()) .map(|s| s.to_string()); let (max_results, next_token) = parse_list_pagination(&body)?; - let guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_ref().expect("management gate proved Some"); + let guard = self.state.read(); + let org = self.management_or_delegated_org(&guard, &req.account_id)?; let entries: Vec = org .list_delegated_administrators(filter.as_deref()) .into_iter() diff --git a/crates/fakecloud-organizations/src/service/handshakes.rs b/crates/fakecloud-organizations/src/service/handshakes.rs index 53d99db7e..4abab4584 100644 --- a/crates/fakecloud-organizations/src/service/handshakes.rs +++ b/crates/fakecloud-organizations/src/service/handshakes.rs @@ -32,12 +32,20 @@ impl OrganizationsService { let body = req.json_body(); let id = required_str(&body, "HandshakeId")?.to_string(); let mut guard = self.state.write(); - // Handshake transitions are handshake-scoped, not org-scoped: with no - // org there are no handshakes, so the id can't be found. These ops - // don't declare AWSOrganizationsNotInUseException. - let org = guard.as_mut().ok_or_else(|| { - org_error_to_aws(crate::state::OrgError::HandshakeNotFound(id.clone())) - })?; + // Handshake transitions are handshake-scoped, not org-scoped, and + // deliberately so: the account answering an invitation is not yet a + // member of the inviting organization, so resolving the caller's own + // organization would never find the handshake. Look it up by id + // across every organization instead, then let the party gate below + // decide who may act on it. With no handshake anywhere the id can't + // be found — these ops don't declare + // AWSOrganizationsNotInUseException. + let not_found = || org_error_to_aws(crate::state::OrgError::HandshakeNotFound(id.clone())); + let (org_id, handshake) = { + let org = guard.org_of_handshake(&id).ok_or_else(not_found)?; + let handshake = org.handshakes.get(&id).ok_or_else(not_found)?.clone(); + (org.org_id.clone(), handshake) + }; // AcceptHandshake / DeclineHandshake belong to the *target* // account; CancelHandshake belongs to the *source* (management) @@ -48,19 +56,25 @@ impl OrganizationsService { // org-wide and any member account can accept/decline their copy; // they don't have a single target_account_id, so we skip the // party gate for them. - let handshake = org.handshakes.get(&id).ok_or_else(|| { - org_error_to_aws(crate::state::OrgError::HandshakeNotFound(id.clone())) - })?; let org_wide_action = matches!( handshake.action.as_str(), "ENABLE_ALL_FEATURES" | "APPROVE_ALL_FEATURES" ); let allowed = if org_wide_action { - // For org-wide handshakes only require membership. - true + // For org-wide handshakes only require membership -- of the + // organization that owns the handshake. With several + // organizations in the process, "any account" would let an + // outsider resolve a handshake it has nothing to do with. + guard + .org_of_account(&req.account_id) + .is_some_and(|org| org.org_id == org_id) } else { match new_state { - "ACCEPTED" | "DECLINED" => req.account_id == handshake.target_account_id, + "ACCEPTED" | "DECLINED" => guard.account_matches_target( + &handshake.target_kind, + &handshake.target_account_id, + &req.account_id, + ), "CANCELED" => req.account_id == handshake.source_account_id, _ => false, } @@ -70,11 +84,120 @@ impl OrganizationsService { crate::state::OrgError::InvalidHandshakeParty(req.account_id.clone()), )); } + // Terminal state next: after the party gate, so a non-party cannot + // read a handshake's state off the error, but BEFORE the + // membership gates below -- those are necessarily satisfied by a + // handshake that was already accepted, so checking them first + // reported a re-accept as a constraint violation instead of the + // modeled transition error, and a client retrying after a timeout + // read a join that had succeeded as a hard failure. + if !matches!(handshake.state.as_str(), "OPEN" | "REQUESTED") { + return Err(org_error_to_aws( + crate::state::OrgError::HandshakeAlreadyResolved(handshake.state.clone()), + )); + } + + // Re-check membership at accept time, not just at invite time: the + // target may have joined another organization while the invitation + // sat open, and an account can only ever be in one. Only an INVITE + // enrolls the target, so only an INVITE can collide — a + // TRANSFER_RESPONSIBILITY handshake targets another organization's + // management account by design. + if new_state == "ACCEPTED" && handshake.action == "INVITE" { + // The caller has just been proved to be the target, so its own + // id is the one that must not already belong elsewhere -- + // including to an in-flight `CreateAccount` that has not + // finished enrolling it yet. + if guard + .claimed_by_other_org(&req.account_id, &org_id) + .is_some() + { + return Err(org_error_to_aws( + crate::state::OrgError::AccountInAnotherOrganization(req.account_id.clone()), + )); + } + // ...and already being a member of the INVITING organization is + // equally a no-op accept. `invite_account` rejects that case at + // invite time; the account may have joined since, through + // `CreateAccount` or the bootstrap shortcut, and the two gates + // must agree. + if guard + .org_by_id(&org_id) + .is_some_and(|org| org.accounts.contains_key(&req.account_id)) + { + return Err(org_error_to_aws( + crate::state::OrgError::AccountAlreadyMember(req.account_id.clone()), + )); + } + } + // An account's address must be unique registry-wide, so pick one + // that is free before enrolling: the address the invitation named + // may have been registered by somebody else while it sat open. + // The invitee did nothing wrong, so fall back to its own synthetic + // address rather than refusing the accept. + let enrolling_email = if new_state == "ACCEPTED" && handshake.action == "INVITE" { + let named = handshake + .target_email + .clone() + .unwrap_or_else(|| format!("{}@example.com", req.account_id)); + let synthetic = format!("{}@example.com", req.account_id); + match (guard.email_in_use(&named), guard.email_in_use(&synthetic)) { + (false, _) => Some(named), + (true, false) => Some(synthetic), + // Both taken. The caller is a member of NO organization -- + // the gates above just proved it -- so reporting "already + // a member" would send it looking for a membership that + // does not exist. The real cause is that no free address + // is left for it. + (true, true) => { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "ConstraintViolationException", + format!( + "No free email address for account {}: both {named} and \ + {synthetic} are already associated with an account.", + req.account_id + ), + )) + } + } + } else { + None + }; + // The same "the world moved while this sat open" re-check the + // INVITE path gets: a transfer target that has since become a + // plain member of some organization has no billing responsibility + // to take over, which is what the invite guard rejects up front. + if new_state == "ACCEPTED" && handshake.action == "TRANSFER_RESPONSIBILITY" { + let became_plain_member = guard + .org_of_account(&req.account_id) + .is_some_and(|org| org.management_account_id != req.account_id); + if became_plain_member { + return Err(AwsServiceError::aws_error_with_fields( + StatusCode::BAD_REQUEST, + "HandshakeConstraintViolationException", + "A responsibility transfer targets another organization's \ + management account.", + vec![( + "Reason".to_string(), + "SOURCE_AND_TARGET_CANNOT_MATCH".to_string(), + )], + )); + } + } + let org = guard + .org_by_id_mut(&org_id) + .expect("handshake lookup resolved this organization"); let updated = org - .resolve_handshake(&id, new_state) + .resolve_handshake( + &id, + new_state, + Some(req.account_id.as_str()), + enrolling_email, + ) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json( - json!({ "Handshake": handshake_payload(&updated) }), + json!({ "Handshake": handshake_payload(org, &updated) }), )) } @@ -87,14 +210,38 @@ impl OrganizationsService { let guard = self.state.read(); // DescribeHandshake is handshake-scoped; with no org the id can't be // found. It doesn't declare AWSOrganizationsNotInUseException. - let handshake = guard - .as_ref() - .and_then(|org| org.handshakes.get(&id)) - .ok_or_else(|| { - org_error_to_aws(crate::state::OrgError::HandshakeNotFound(id.clone())) - })?; + let owner = guard.org_of_handshake(&id).ok_or_else(|| { + org_error_to_aws(crate::state::OrgError::HandshakeNotFound(id.clone())) + })?; + let handshake = owner.handshakes.get(&id).ok_or_else(|| { + org_error_to_aws(crate::state::OrgError::HandshakeNotFound(id.clone())) + })?; + // Only the two parties to a handshake may read it. Without this a + // bystander account could enumerate handshake ids to learn the + // management account and organization id of an organization it has + // nothing to do with. + // An EMAIL target stores the address, so resolve it back to the + // account it names before comparing. Leaving the gate off entirely + // would let any account anywhere read any handshake, learning + // another organization's id and management account. + let is_party = req.account_id == handshake.source_account_id + || guard.account_matches_target( + &handshake.target_kind, + &handshake.target_account_id, + &req.account_id, + ); + // AWS documents DescribeHandshake as callable "from any account in + // the organization", so membership of the organization that owns + // the handshake is enough. It is only another ORGANIZATION's + // handshakes that must stay invisible. + let is_member = owner.accounts.contains_key(&req.account_id); + if !(is_party || is_member) { + return Err(org_error_to_aws(crate::state::OrgError::HandshakeNotFound( + id.clone(), + ))); + } Ok(AwsResponse::ok_json( - json!({ "Handshake": handshake_payload(handshake) }), + json!({ "Handshake": handshake_payload(owner, handshake) }), )) } @@ -106,14 +253,13 @@ impl OrganizationsService { let filter = parse_handshake_filter(&body)?; let (max_results, next_token) = parse_list_pagination(&body)?; - let guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_ref().expect("management gate proved Some"); + let guard = self.state.read(); + let org = self.management_or_delegated_org(&guard, &req.account_id)?; let filtered: Vec = org - .list_handshakes(None) + .list_handshakes() .into_iter() .filter(|h| handshake_matches_filter(h, &filter)) - .map(|h| handshake_payload(&h)) + .map(|h| handshake_payload(org, &h)) .collect(); let (page, token) = paginate_checked(&filtered, next_token.as_deref(), max_results) .map_err(|_| invalid_input("Invalid NextToken"))?; diff --git a/crates/fakecloud-organizations/src/service/mod.rs b/crates/fakecloud-organizations/src/service/mod.rs index 5774b3f3b..cfcb4222b 100644 --- a/crates/fakecloud-organizations/src/service/mod.rs +++ b/crates/fakecloud-organizations/src/service/mod.rs @@ -15,9 +15,9 @@ use fakecloud_core::service::{AwsRequest, AwsResponse, AwsService, AwsServiceErr use fakecloud_persistence::SnapshotStore; use crate::state::{ - MemberAccount, OrgError, OrganizationState, OrganizationalUnit, OrganizationsSnapshot, Policy, - SharedOrganizationsState, FEATURE_SET_ALL, FEATURE_SET_CONSOLIDATED_BILLING, - ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, POLICY_TYPE_SCP, + MemberAccount, OrgError, OrganizationState, OrganizationalUnit, OrganizationsRegistry, + OrganizationsSnapshot, Policy, SharedOrganizationsState, FEATURE_SET_ALL, + FEATURE_SET_CONSOLIDATED_BILLING, ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, POLICY_TYPE_SCP, }; /// Organizations read actions all start with `Describe` or `List`; every other @@ -142,6 +142,22 @@ fn membership_fingerprint(org: &OrganizationState) -> u64 { hasher.finish() } +/// Fingerprint of membership across EVERY organization. An organization +/// appearing or disappearing has to register as a change too, so this +/// hashes the whole registry rather than tracking one value per org. +fn registry_membership_fingerprint(registry: &OrganizationsRegistry) -> Option { + if registry.is_empty() { + return None; + } + use std::hash::Hasher; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + // `iter()` is ordered by organization id, so the digest is stable. + for org in registry.iter() { + hasher.write_u64(membership_fingerprint(org)); + } + Some(hasher.finish()) +} + impl OrgChangeHooks { pub fn new() -> Self { Self::default() @@ -163,7 +179,7 @@ impl OrgChangeHooks { // announced. let (previous, claimed) = { let mut seen = self.seen.lock(); - let fingerprint = state.read().as_ref().map(membership_fingerprint); + let fingerprint = registry_membership_fingerprint(&state.read()); if *seen == fingerprint { return; } @@ -279,7 +295,8 @@ impl OrganizationsService { } pub fn shared() -> (Arc, SharedOrganizationsState) { - let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(None)); + let state: SharedOrganizationsState = + Arc::new(parking_lot::RwLock::new(OrganizationsRegistry::default())); (Arc::new(Self::new(state.clone())), state) } @@ -319,59 +336,103 @@ impl OrganizationsService { pub fn rearm_in_progress_account_creations(&self) { let pending: Vec = { let guard = self.state.read(); - match guard.as_ref() { - Some(org) => org - .create_account_requests - .iter() - .filter(|(_, s)| s.state == "IN_PROGRESS") - .map(|(id, _)| id.clone()) - .collect(), - None => Vec::new(), - } + // Request ids are globally unique (`car-` + 20 random chars), so + // the completion tick can find its own request by scanning every + // organization — no need to thread the owning org id through. + guard + .iter() + .flat_map(|org| org.create_account_requests.iter()) + .filter(|(_, s)| s.state == "IN_PROGRESS") + .map(|(id, _)| id.clone()) + .collect() }; for request_id in pending { self.spawn_create_account_completion(request_id); } } - /// Read-side helper: enforce that an org exists and the caller is a - /// member. Returns the borrowed org on success. + /// Read-side helper: resolve the organization the caller belongs to. + /// A caller in no organization gets the same + /// `AWSOrganizationsNotInUseException` as a caller in a process with + /// no organizations at all, so another organization's existence is + /// never observable from outside it. fn require_member<'a>( &self, - guard: &'a parking_lot::RwLockReadGuard<'_, Option>, + guard: &'a parking_lot::RwLockReadGuard<'_, OrganizationsRegistry>, account_id: &str, ) -> Result<&'a OrganizationState, AwsServiceError> { - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; - if !org.accounts.contains_key(account_id) { - return Err(organizations_not_in_use()); + guard + .org_of_account(account_id) + .ok_or_else(organizations_not_in_use) + } + + /// Write-side helper for mutating ops: resolve the caller's own + /// organization and enforce that the caller is its management + /// account. Returns the organization itself, so a handler never has + /// to re-resolve it out of the registry. + fn management_org_mut<'a>( + &self, + guard: &'a mut parking_lot::RwLockWriteGuard<'_, OrganizationsRegistry>, + account_id: &str, + ) -> Result<&'a mut OrganizationState, AwsServiceError> { + let org = guard + .org_of_account_mut(account_id) + .ok_or_else(organizations_not_in_use)?; + if !org.is_management(account_id) { + return Err(not_management()); } Ok(org) } - /// Write-side helper for mutating ops: caller must be the - /// management account of an existing organization. Returns the - /// management-only error rather than an Option, so the caller can - /// unwrap the guard safely right after. - fn require_member_management( + /// The caller's organization, for the read operations AWS opens to + /// the management account OR to any member registered as a + /// delegated administrator (for any service principal). + /// + /// Delegated administration exists so a member account can run a + /// service's org-wide integration on the management account's + /// behalf, which means reading the organization it administers: + /// `ListHandshakesForOrganization`, `ListAWSServiceAccessForOrganization`, + /// `ListDelegatedAdministrators` and `ListDelegatedServicesForAccount` + /// are all documented as callable by a delegated administrator. + /// Mutating operations stay management-only via + /// [`Self::management_org_mut`], which is why there is no read-side + /// management-only gate left: every management-only op mutates. + fn management_or_delegated_org<'a>( &self, - guard: &parking_lot::RwLockWriteGuard<'_, Option>, + guard: &'a parking_lot::RwLockReadGuard<'_, OrganizationsRegistry>, account_id: &str, - ) -> Result<(), AwsServiceError> { - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; - if !org.accounts.contains_key(account_id) { - return Err(organizations_not_in_use()); - } - if !org.is_management(account_id) { - return Err(AwsServiceError::aws_error( - StatusCode::FORBIDDEN, - "AccessDeniedException", - "This operation can be called only from the organization's management account.", - )); + ) -> Result<&'a OrganizationState, AwsServiceError> { + let org = self.require_member(guard, account_id)?; + if !org.is_management(account_id) && !org.is_delegated_administrator(account_id) { + return Err(not_management()); } - Ok(()) + Ok(org) } } +/// AWS's handshake-time answer for "that account already belongs to an +/// organization", carrying the modeled `Reason` discriminator. +fn already_in_an_organization(message: String) -> AwsServiceError { + AwsServiceError::aws_error_with_fields( + StatusCode::BAD_REQUEST, + "HandshakeConstraintViolationException", + message, + vec![( + "Reason".to_string(), + "ALREADY_IN_AN_ORGANIZATION".to_string(), + )], + ) +} + +/// AWS's error for a management-only operation attempted by a member. +fn not_management() -> AwsServiceError { + AwsServiceError::aws_error( + StatusCode::FORBIDDEN, + "AccessDeniedException", + "This operation can be called only from the organization's management account.", + ) +} + fn parse_tags(value: Option<&Value>) -> Vec<(String, String)> { let arr = match value.and_then(|v| v.as_array()) { Some(a) => a, @@ -531,7 +592,10 @@ pub async fn save_organizations_snapshot( let _guard = lock.lock().await; let snapshot = OrganizationsSnapshot { schema_version: ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, - organization: state.read().clone(), + // v1's single-organization field is read-only now; v2 always + // writes the whole registry. + organization: None, + organizations: state.read().clone(), }; let join = tokio::task::spawn_blocking(move || -> std::io::Result<()> { let bytes = serde_json::to_vec(&snapshot) @@ -777,11 +841,17 @@ fn org_error_to_aws(err: OrgError) -> AwsServiceError { "DuplicateHandshakeException", format!("An OPEN handshake already exists for account {account}."), ), - OrgError::AccountAlreadyMember(account) => AwsServiceError::aws_error( - StatusCode::BAD_REQUEST, - "AccountAlreadyRegisteredException", - format!("Account {account} is already a member of this organization."), - ), + // AWS reports both of these on InviteAccountToOrganization and + // AcceptHandshake as HandshakeConstraintViolationException with + // Reason=ALREADY_IN_AN_ORGANIZATION; those operations do not model + // AccountAlreadyRegisteredException at all, so a typed SDK catching + // the modeled exception would miss it. + OrgError::AccountAlreadyMember(account) => already_in_an_organization(format!( + "Account {account} is already a member of this organization." + )), + OrgError::AccountInAnotherOrganization(account) => already_in_an_organization(format!( + "Account {account} is already a member of an organization." + )), OrgError::AWSServiceAccessNotEnabled(svc) => AwsServiceError::aws_error( StatusCode::BAD_REQUEST, "AWSOrganizationsNotInUseException", @@ -839,6 +909,10 @@ fn parse_handshake_filter(body: &Value) -> Result Result<(usize, Option), AwsSer Ok((max_results, next_token)) } -fn handshake_payload(h: &crate::state::Handshake) -> Value { +fn handshake_payload(org: &OrganizationState, h: &crate::state::Handshake) -> Value { // Real AWS Organizations encodes the inviter as the org itself // (`Type: ORGANIZATION`, `Id` = the org id) and the invitee as the // member account (`Type: ACCOUNT`, `Id` = account id) or its email @@ -934,14 +1008,82 @@ fn handshake_payload(h: &crate::state::Handshake) -> Value { {"Id": h.organization_id, "Type": "ORGANIZATION"}, {"Id": h.source_account_id, "Type": "ACCOUNT"}, { - "Id": h.target_email.clone().unwrap_or_else(|| h.target_account_id.clone()), + // Pick the id by the party's own Type. Preferring + // `target_email` whenever it was recorded rendered an address + // under `Type: ACCOUNT` for any handshake that stored both. + "Id": if h.target_kind == "EMAIL" { + h.target_email.clone().unwrap_or_else(|| h.target_account_id.clone()) + } else { + h.target_account_id.clone() + }, "Type": h.target_kind, }, ]); - let resources = json!([ - {"Type": "ORGANIZATION", "Value": h.organization_id}, - {"Type": "ACCOUNT", "Value": h.target_account_id}, - ]); + // Same rule as the parties above: the value has to match the type it + // is labelled with. `HandshakeResourceType` models EMAIL separately, + // so an email-target invite reports the address as EMAIL rather than + // as an ACCOUNT id. + let target_resource = if h.target_kind == "EMAIL" { + json!({ + "Type": "EMAIL", + "Value": h.target_email.clone().unwrap_or_else(|| h.target_account_id.clone()), + }) + } else { + json!({"Type": "ACCOUNT", "Value": h.target_account_id}) + }; + let mut resources = vec![ + json!({"Type": "ORGANIZATION", "Value": h.organization_id}), + target_resource, + ]; + // A TRANSFER_RESPONSIBILITY handshake carries the transfer it is + // offering, and AWS models exactly that as a nested + // `RESPONSIBILITY_TRANSFER` resource -- which is how an SDK reading + // only the handshake learns what is being handed over and by whom. + // `HandshakeResourceType` defines TRANSFER_TYPE, + // TRANSFER_START_TIMESTAMP and MANAGEMENT_ACCOUNT for no other + // purpose. + if let Some(transfer) = h + .responsibility_transfer_id + .as_deref() + .and_then(|id| org.responsibility_transfers.get(id)) + // A handshake written before the link existed deserializes with + // no transfer id, and nothing backfills it. Fall back to the + // transfer's own `ActiveHandshakeId`, which covers every restored + // handshake still OPEN. A restored handshake that had already + // resolved is beyond recovery -- resolution is what clears that + // field, and nothing else ties the two together -- so this is as + // far back as the stored data reaches. + .or_else(|| { + org.responsibility_transfers + .values() + .find(|t| t.active_handshake_id.as_deref() == Some(h.id.as_str())) + }) + { + resources.push(json!({ + "Type": "RESPONSIBILITY_TRANSFER", + "Value": transfer.id, + "Resources": [ + {"Type": "TRANSFER_TYPE", "Value": transfer.transfer_type}, + { + "Type": "TRANSFER_START_TIMESTAMP", + // A nested resource's Value is a string in the Smithy + // model, so the timestamp goes out ISO-8601 rather + // than as the epoch number the top-level timestamp + // members use. + "Value": transfer.start_timestamp.to_rfc3339_opts(chrono::SecondsFormat::Millis, true), + }, + { + "Type": "MANAGEMENT_ACCOUNT", + "Value": transfer.source_management_account_id, + }, + { + "Type": "MANAGEMENT_EMAIL", + "Value": transfer.source_management_account_email, + }, + ], + })); + } + let resources = Value::Array(resources); let mut obj = json!({ "Id": h.id, "Arn": h.arn, diff --git a/crates/fakecloud-organizations/src/service/org.rs b/crates/fakecloud-organizations/src/service/org.rs index 92f53ca61..7f45cb0e1 100644 --- a/crates/fakecloud-organizations/src/service/org.rs +++ b/crates/fakecloud-organizations/src/service/org.rs @@ -24,13 +24,31 @@ impl OrganizationsService { } let mut guard = self.state.write(); - if guard.is_some() { + // Only the CALLER's own membership blocks this. Organizations are + // independent of each other, so somebody else having created one + // must not stop this account from creating its own (#2543). + if guard.account_is_enrolled(&req.account_id) { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST, "AlreadyInOrganizationException", "The AWS account is already a member of an organization.", )); } + // The management account registers its own synthetic address, so + // that address must not already be taken -- otherwise two live + // accounts share one and resolution by address, which decides who + // may accept an EMAIL-targeted handshake, answers by org ordering. + let management_email = format!("{}@example.com", req.account_id); + if guard.email_in_use(&management_email) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "ConstraintViolationException", + format!( + "The email address {management_email} is already associated \ + with another account." + ), + )); + } let mut org = OrganizationState::bootstrap(&req.account_id); // A CONSOLIDATED_BILLING org has no policy management; reflect the // requested feature set and drop the auto-enabled SCP type. @@ -39,7 +57,7 @@ impl OrganizationsService { org.enabled_policy_types.clear(); } let resp_value = organization_payload(&org); - *guard = Some(org); + guard.insert(org); Ok(AwsResponse::ok_json(json!({ "Organization": resp_value }))) } @@ -48,14 +66,12 @@ impl OrganizationsService { req: &AwsRequest, ) -> Result { let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; // AWS scopes DescribeOrganization to members of the organization. - // Non-members must not learn that an org exists at all — return - // the same `AWSOrganizationsNotInUseException` the no-org path - // returns so org metadata doesn't leak across account boundaries. - if !org.accounts.contains_key(&req.account_id) { - return Err(organizations_not_in_use()); - } + // Non-members must not learn that an org exists at all — the + // resolver returns the same `AWSOrganizationsNotInUseException` + // the no-org path returns, so org metadata doesn't leak across + // account boundaries. + let org = self.require_member(&guard, &req.account_id)?; Ok(AwsResponse::ok_json( json!({ "Organization": organization_payload(org) }), )) @@ -66,13 +82,12 @@ impl OrganizationsService { req: &AwsRequest, ) -> Result { let mut guard = self.state.write(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; // Non-members get the same "not in use" error as callers in a // process with no org at all — they should not be able to tell // the difference. - if !org.accounts.contains_key(&req.account_id) { - return Err(organizations_not_in_use()); - } + let org = guard + .org_of_account(&req.account_id) + .ok_or_else(organizations_not_in_use)?; if !org.is_management(&req.account_id) { return Err(AwsServiceError::aws_error( StatusCode::FORBIDDEN, @@ -96,7 +111,8 @@ impl OrganizationsService { "The organization still has member accounts. Remove them first.", )); } - *guard = None; + let org_id = org.org_id.clone(); + guard.remove(&org_id); Ok(AwsResponse::ok_json(Value::Null)) } } diff --git a/crates/fakecloud-organizations/src/service/ous.rs b/crates/fakecloud-organizations/src/service/ous.rs index 98c55d218..4fc768917 100644 --- a/crates/fakecloud-organizations/src/service/ous.rs +++ b/crates/fakecloud-organizations/src/service/ous.rs @@ -11,8 +11,7 @@ impl OrganizationsService { let parent_id = required_str(&body, "ParentId")?; let name = required_str(&body, "Name")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; let ou = org.create_ou(parent_id, name).map_err(org_error_to_aws)?; // Apply create-time Tags so ListTagsForResource reflects them without a // follow-up TagResource (bug-audit 2026-06-20, 1.24). @@ -33,8 +32,7 @@ impl OrganizationsService { let ou_id = required_str(&body, "OrganizationalUnitId")?; let new_name = required_str(&body, "Name")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; let ou = org.rename_ou(ou_id, new_name).map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json( json!({ "OrganizationalUnit": ou_payload(&ou) }), @@ -48,8 +46,7 @@ impl OrganizationsService { let body = req.json_body(); let ou_id = required_str(&body, "OrganizationalUnitId")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.delete_ou(ou_id).map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(Value::Null)) } diff --git a/crates/fakecloud-organizations/src/service/policies.rs b/crates/fakecloud-organizations/src/service/policies.rs index 2b12eaa55..7566f07d4 100644 --- a/crates/fakecloud-organizations/src/service/policies.rs +++ b/crates/fakecloud-organizations/src/service/policies.rs @@ -32,8 +32,7 @@ impl OrganizationsService { .and_then(|v| v.as_str()) .unwrap_or(""); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; let policy = org .create_policy(name, description, content, policy_type) .map_err(org_error_to_aws)?; @@ -55,8 +54,7 @@ impl OrganizationsService { let description = body.get("Description").and_then(|v| v.as_str()); let content = body.get("Content").and_then(|v| v.as_str()); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; let policy = org .update_policy(policy_id, name, description, content) .map_err(org_error_to_aws)?; @@ -69,8 +67,7 @@ impl OrganizationsService { let body = req.json_body(); let policy_id = required_str(&body, "PolicyId")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.delete_policy(policy_id).map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(Value::Null)) } @@ -115,8 +112,7 @@ impl OrganizationsService { let policy_id = required_str(&body, "PolicyId")?; let target_id = required_str(&body, "TargetId")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.attach_policy(policy_id, target_id) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(Value::Null)) @@ -127,8 +123,7 @@ impl OrganizationsService { let policy_id = required_str(&body, "PolicyId")?; let target_id = required_str(&body, "TargetId")?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().unwrap(); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.detach_policy(policy_id, target_id) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(Value::Null)) @@ -186,8 +181,7 @@ impl OrganizationsService { let body = req.json_body(); let policy_type = required_str(&body, "PolicyType")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.enable_policy_type(&policy_type); let policy_types: Vec = org .list_policy_type_statuses() @@ -212,8 +206,7 @@ impl OrganizationsService { let body = req.json_body(); let policy_type = required_str(&body, "PolicyType")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.disable_policy_type(&policy_type) .map_err(org_error_to_aws)?; let policy_types: Vec = org @@ -244,7 +237,19 @@ impl OrganizationsService { .map(|s| s.to_string()) .unwrap_or_else(|| req.account_id.clone()); let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; + let org = self.require_member(&guard, &req.account_id)?; + // The target must live in the caller's own organization. Without + // this a foreign (or mistyped) id walked no hierarchy at all and + // came back as an empty, successful "no effective policy" instead + // of the modeled not-found. + if target_id != org.root_id + && !org.ous.contains_key(&target_id) + && !org.accounts.contains_key(&target_id) + { + return Err(org_error_to_aws(crate::state::OrgError::TargetNotFound( + target_id, + ))); + } // The effective policy is the union of every policy of `policy_type` // attached up the org hierarchy from `target_id` to root. We // present it as a single Statement[] union so callers can audit. @@ -293,8 +298,7 @@ impl OrganizationsService { ) })?; let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.resource_policy = Some(content); let payload = json!({ "ResourcePolicy": { @@ -316,18 +320,20 @@ impl OrganizationsService { req: &AwsRequest, ) -> Result { let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.resource_policy = None; Ok(AwsResponse::ok_json(json!({}))) } pub(super) fn describe_resource_policy( &self, - _req: &AwsRequest, + req: &AwsRequest, ) -> Result { let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; + // AWS allows the management account OR a member registered as a + // delegated administrator -- unlike `PutResourcePolicy` and + // `DeleteResourcePolicy`, which really are management-only. + let org = self.management_or_delegated_org(&guard, &req.account_id)?; let content = org.resource_policy.clone().ok_or_else(|| { AwsServiceError::aws_error( StatusCode::BAD_REQUEST, diff --git a/crates/fakecloud-organizations/src/service/policy_types.rs b/crates/fakecloud-organizations/src/service/policy_types.rs index 50136aa58..0612db24b 100644 --- a/crates/fakecloud-organizations/src/service/policy_types.rs +++ b/crates/fakecloud-organizations/src/service/policy_types.rs @@ -8,8 +8,7 @@ impl OrganizationsService { req: &AwsRequest, ) -> Result { let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.enable_all_features(); // AWS returns a Handshake here; we synthesize a minimal accepted // shape so SDKs can deserialize the response. diff --git a/crates/fakecloud-organizations/src/service/responsibility.rs b/crates/fakecloud-organizations/src/service/responsibility.rs index 975eb9ebb..a68e8bf12 100644 --- a/crates/fakecloud-organizations/src/service/responsibility.rs +++ b/crates/fakecloud-organizations/src/service/responsibility.rs @@ -34,7 +34,79 @@ fn require_transfer_type(body: &Value) -> Result { Ok(t.to_string()) } -fn transfer_payload(t: &ResponsibilityTransfer) -> Value { +/// Is `account_id` the target of `t`? +/// +/// Delegates to the one shared predicate so this agrees with the +/// handshake gate: an account that can accept an invitation must also +/// be able to read and act on the transfer it accepted. +fn is_transfer_target( + registry: &crate::state::OrganizationsRegistry, + t: &ResponsibilityTransfer, + account_id: &str, +) -> bool { + let stored = &t.target_management_account_id; + if *stored == account_id { + return true; + } + // Only an EMAIL-form target resolves further. Reading a 12-digit id + // as an address let an account registered with the literal string + // "222222222222" pass as the target of a transfer addressed to + // ACCOUNT 222222222222 -- `CreateAccount` does not validate that + // `Email` is address-shaped. + let is_account_id = stored.len() == 12 && stored.chars().all(|c| c.is_ascii_digit()); + !is_account_id && registry.account_matches_target("EMAIL", stored, account_id) +} + +fn is_transfer_party( + registry: &crate::state::OrganizationsRegistry, + t: &ResponsibilityTransfer, + account_id: &str, +) -> bool { + t.source_management_account_id == account_id || is_transfer_target(registry, t, account_id) +} + +/// `TransferParticipant.ManagementAccountId` is modeled as an +/// `AccountId` (exactly 12 digits), so an email-targeted transfer -- whose +/// target is recorded as the address the source named -- carries only +/// the email until the address resolves to an account id. +fn target_participant( + registry: &crate::state::OrganizationsRegistry, + t: &ResponsibilityTransfer, + caller: &str, +) -> Value { + let mut party = json!({ + "ManagementAccountEmail": t.target_management_account_email, + }); + let stored = &t.target_management_account_id; + let is_account_id = stored.len() == 12 && stored.chars().all(|c| c.is_ascii_digit()); + let resolved = if is_account_id { + Some(stored.clone()) + } else { + // The synthetic form decodes only an id the caller already spelled + // out, so it leaks nothing. A registered address otherwise + // resolves ONLY to the caller itself: the party gate already used + // that resolution to let this caller in, so telling it its own id + // reveals nothing, while a third party still learns nothing about + // whose account an address belongs to. An address that names + // somebody else therefore reports no `ManagementAccountId`, which + // the Smithy shape allows. + crate::state::target_account_id("EMAIL", stored).or_else(|| { + registry + .account_matches_target("EMAIL", stored, caller) + .then(|| caller.to_string()) + }) + }; + if let Some(id) = resolved { + party["ManagementAccountId"] = json!(id); + } + party +} + +fn transfer_payload( + registry: &crate::state::OrganizationsRegistry, + t: &ResponsibilityTransfer, + caller: &str, +) -> Value { let mut obj = json!({ "Arn": t.arn, "Name": t.name, @@ -45,10 +117,7 @@ fn transfer_payload(t: &ResponsibilityTransfer) -> Value { "ManagementAccountId": t.source_management_account_id, "ManagementAccountEmail": t.source_management_account_email, }, - "Target": { - "ManagementAccountId": t.target_management_account_id, - "ManagementAccountEmail": t.target_management_account_email, - }, + "Target": target_participant(registry, t, caller), "StartTimestamp": t.start_timestamp.timestamp() as f64, }); if let Some(end) = t.end_timestamp { @@ -60,6 +129,63 @@ fn transfer_payload(t: &ResponsibilityTransfer) -> Value { obj } +impl OrganizationsService { + /// Resolve a transfer for a MUTATING call and return the id of the + /// organization that stores it. + /// + /// A transfer is an arrangement between two management accounts, and + /// both can read it. `source_only` says whether this particular + /// mutation is the source's alone: renaming is, because the source + /// chose the name. Ending is the source's too while the offer is + /// still open -- `WITHDRAWN` means the inviter pulled it, and the + /// target's answer to an open offer is `DeclineHandshake`. Once the + /// transfer is ACCEPTED the riding handshake is gone, so the target + /// may end it as well, or it would have no way out of an + /// arrangement it is actively carrying. + /// + /// Resolving through the caller's own organization instead reported + /// "not found" to the target, which is indistinguishable from a bad + /// id. + fn party_org_of_transfer( + &self, + guard: &parking_lot::RwLockWriteGuard<'_, crate::state::OrganizationsRegistry>, + id: &str, + caller: &str, + source_only: bool, + ) -> Result { + let org = guard + .org_of_responsibility_transfer(id) + .ok_or_else(|| transfer_not_found(id))?; + let transfer = org + .responsibility_transfers + .get(id) + .ok_or_else(|| transfer_not_found(id))?; + // A stranger learns nothing beyond "no such transfer". + if !is_transfer_party(guard, transfer, caller) { + return Err(transfer_not_found(id)); + } + // Only an offer still awaiting an answer is the source's alone to + // withdraw. A terminal status falls through, so the caller gets the + // real "already in that status" answer rather than advice to + // decline a handshake that no longer exists. + if transfer.source_management_account_id != caller + && (source_only || transfer.status == "REQUESTED") + { + return Err(AwsServiceError::aws_error( + StatusCode::FORBIDDEN, + "AccessDeniedException", + if source_only { + "Only the source management account can rename a responsibility transfer." + } else { + "Only the source management account can withdraw a transfer that has not \ + been accepted; decline the handshake instead." + }, + )); + } + Ok(org.org_id.clone()) + } +} + impl OrganizationsService { pub(super) fn invite_organization_to_transfer_responsibility( &self, @@ -82,30 +208,143 @@ impl OrganizationsService { .and_then(|v| v.as_str()) .ok_or_else(|| invalid_input("Target.Id is required"))? .to_string(); + // `HandshakeParty.Type` is modeled required here too. let target_kind = target_obj .get("Type") .and_then(|v| v.as_str()) - .unwrap_or("ACCOUNT"); + .ok_or_else(|| invalid_input("Target.Type is required"))?; + // Same shape validation the account-invite path applies: without + // it a mismatched target (an address under Type=ACCOUNT) is stored + // as the target account id, and the handshake sits OPEN forever + // because no caller can authenticate as that string. + super::accounts::validate_invite_target(target_kind, &target_id)?; let notes = body .get("Notes") .and_then(|v| v.as_str()) .map(|s| s.to_string()); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + // Authorize FIRST. Resolving the target before the management gate + // turned "is this address registered?" into an oracle any caller + // could read off the difference between InvalidInputException and + // AWSOrganizationsNotInUseException. + let source_org_id = self + .management_org_mut(&mut guard, &req.account_id)? + .org_id + .clone(); + let registry = &*guard; - // The invited party is identified by account id or email; record - // whichever the caller supplied as the target management account. + // Record the target EXACTLY as the caller named it, as AWS does: + // resolving an address against other organizations' management + // accounts would answer "does this address exist, and what is its + // account id?" for organizations the caller has nothing to do + // with. The party gate resolves the other way instead -- it asks + // the CALLER what its own address is (see `is_transfer_target`). let (target_account_id, target_email) = if target_kind == "EMAIL" { (target_id.clone(), target_id.clone()) } else { (target_id.clone(), format!("{target_id}@example.com")) }; + // Naming your own organization is the one case there is nothing to + // leak about. Comparing only against the management account let a + // plain member of the SAME organization be named, which opened -- + // and let that member accept -- a "cross-organization" transfer + // whose two ends were one organization. + // Resolve once, the same way every party gate does. Asking + // `org_of_account` about the raw string answered `None` for every + // EMAIL target, so the non-management check below never fired and + // an address spelling a plain member of another organization got + // through. This scan drives a rejection only -- it never hands an + // id back -- so it is not the oracle the id-returning resolvers + // are scoped to avoid. + let resolved_target = registry + .iter() + .flat_map(|org| org.accounts.keys()) + .find(|id| registry.account_matches_target(target_kind, &target_id, id)) + .cloned(); + let target_org = resolved_target + .as_deref() + .and_then(|id| registry.org_of_account(id)); + let target_is_own_org = target_org.is_some_and(|org| org.org_id == source_org_id); + // Two rejections, and deliberately only two: + // + // - the caller's OWN organization, which cannot hand billing to + // itself; and + // - a plain member of another organization, which has no billing + // responsibility to take over. + // + // A target that belongs to NO organization is allowed. AWS's + // invite takes an `EMAIL` party precisely so it can address an + // owner it has not seen yet, and requiring the target to already + // run an organization would make the op unusable until it did. + // That is why the inbound reads below are party-scoped rather + // than membership-gated: the target must be able to find the + // transfer it was invited to before it has an organization. + let target_is_non_management_member = match (&resolved_target, target_org) { + (Some(id), Some(org)) => org.management_account_id != *id, + _ => false, + }; + if target_is_own_org || target_is_non_management_member { + return Err(AwsServiceError::aws_error_with_fields( + StatusCode::BAD_REQUEST, + "HandshakeConstraintViolationException", + "A responsibility transfer targets another organization's \ + management account.", + vec![( + "Reason".to_string(), + "SOURCE_AND_TARGET_CANNOT_MATCH".to_string(), + )], + )); + } + // One live invitation per target, the same rule + // `InviteAccountToOrganization` enforces and with the same modeled + // error. Without it a caller could stack OPEN transfers on one + // target, and accepting any of them would move billing while the + // rest stayed OPEN against an organization that no longer owns it. + // The two handshake actions stay independent: an INVITE to the + // same account is a different offer and does not collide. + // + // Compare the RESOLVED target, so the same account named two ways + // still collides. An enrolled account resolves through the + // registry; an account that exists nowhere yet -- the common case + // here, since the invite exists to address an owner AWS has not + // seen -- resolves only through the synthetic address form. + let canonical_target = |kind: &str, id: &str| -> String { + registry + .iter() + .flat_map(|org| org.accounts.keys()) + .find(|account| registry.account_matches_target(kind, id, account)) + .cloned() + .or_else(|| crate::state::target_account_id(kind, id)) + .unwrap_or_else(|| id.to_string()) + }; + let want = canonical_target(target_kind, &target_id); + let duplicate = registry + .org_by_id(&source_org_id) + .expect("management gate resolved this organization") + .handshakes + .values() + .filter(|h| { + h.action == "TRANSFER_RESPONSIBILITY" + && matches!(h.state.as_str(), "REQUESTED" | "OPEN") + }) + .any(|h| canonical_target(&h.target_kind, &h.target_account_id) == want); + if duplicate { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "DuplicateHandshakeException", + format!("An OPEN responsibility transfer already targets {target_id}."), + )); + } + + let org = guard + .org_by_id_mut(&source_org_id) + .expect("management gate resolved this organization"); let now = Utc::now(); // The transfer rides on a handshake the invited org accepts. let handshake_id = format!("h-{}", random_id(32)); + let transfer_id = format!("rt-{}", random_id(32)); let handshake_arn = format!( "arn:aws:organizations::{}:handshake/{}/transfer/{}", org.management_account_id, org.org_id, handshake_id @@ -123,11 +362,11 @@ impl OrganizationsService { target_kind: target_kind.to_string(), notes, organization_id: org.org_id.clone(), + responsibility_transfer_id: Some(transfer_id.clone()), }; org.handshakes .insert(handshake_id.clone(), handshake.clone()); - let transfer_id = format!("rt-{}", random_id(32)); let transfer_arn = format!( "arn:aws:organizations::{}:responsibilitytransfer/{}/{}", org.management_account_id, org.org_id, transfer_id @@ -151,7 +390,7 @@ impl OrganizationsService { .insert(transfer_id, transfer.clone()); Ok(AwsResponse::ok_json( - json!({ "Handshake": handshake_payload(&handshake) }), + json!({ "Handshake": handshake_payload(org, &handshake) }), )) } @@ -162,13 +401,20 @@ impl OrganizationsService { let body = req.json_body(); let id = required_str(&body, "Id")?.to_string(); let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; - let transfer = org - .responsibility_transfers - .get(&id) + // Party-scoped, not membership-gated, for the same reason as the + // inbound listing: the target of a transfer may not have an + // organization of its own yet. + // A transfer is stored once, in the SOURCE organization, but it has + // two parties: resolving it through the caller's own organization + // would hide every inbound transfer from the account being invited + // to take over billing. + let transfer = guard + .org_of_responsibility_transfer(&id) + .and_then(|org| org.responsibility_transfers.get(&id)) + .filter(|t| is_transfer_party(&guard, t, &req.account_id)) .ok_or_else(|| transfer_not_found(&id))?; Ok(AwsResponse::ok_json( - json!({ "ResponsibilityTransfer": transfer_payload(transfer) }), + json!({ "ResponsibilityTransfer": transfer_payload(&guard, transfer, &req.account_id) }), )) } @@ -180,15 +426,15 @@ impl OrganizationsService { let id = required_str(&body, "Id")?.to_string(); let name = required_str(&body, "Name")?.to_string(); let mut guard = self.state.write(); - let org = guard.as_mut().ok_or_else(organizations_not_in_use)?; - let transfer = org - .responsibility_transfers - .get_mut(&id) - .ok_or_else(|| transfer_not_found(&id))?; + let org_id = self.party_org_of_transfer(&guard, &id, &req.account_id, true)?; + let transfer = guard + .org_by_id_mut(&org_id) + .and_then(|org| org.responsibility_transfers.get_mut(&id)) + .expect("resolved just above"); transfer.name = name; let snapshot = transfer.clone(); Ok(AwsResponse::ok_json( - json!({ "ResponsibilityTransfer": transfer_payload(&snapshot) }), + json!({ "ResponsibilityTransfer": transfer_payload(&guard, &snapshot, &req.account_id) }), )) } @@ -203,12 +449,13 @@ impl OrganizationsService { .and_then(json_to_datetime) .unwrap_or_else(Utc::now); let mut guard = self.state.write(); - let org = guard.as_mut().ok_or_else(organizations_not_in_use)?; + // Either party can end the arrangement. + let org_id = self.party_org_of_transfer(&guard, &id, &req.account_id, false)?; + let org = guard.org_by_id_mut(&org_id).expect("resolved just above"); let transfer = org .responsibility_transfers .get_mut(&id) - .ok_or_else(|| transfer_not_found(&id))?; - // Only a still-pending transfer can be terminated. + .expect("resolved just above"); if transfer.status == "WITHDRAWN" { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST, @@ -216,7 +463,10 @@ impl OrganizationsService { "The responsibility transfer is already withdrawn.", )); } - if transfer.status != "REQUESTED" { + // AWS's op "ends a transfer", so it applies to one still awaiting an + // answer AND to one already accepted and running. Only a transfer + // that has already reached a terminal state cannot be ended. + if !matches!(transfer.status.as_str(), "REQUESTED" | "ACCEPTED") { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST, "InvalidResponsibilityTransferTransitionException", @@ -228,16 +478,20 @@ impl OrganizationsService { } transfer.status = "WITHDRAWN".to_string(); transfer.end_timestamp = Some(end); - transfer.active_handshake_id = None; + // Take the riding handshake id BEFORE clearing the field: reading it + // back off the post-clear snapshot always saw `None`, so the + // handshake stayed OPEN and the target could still accept a + // withdrawn transfer. + let riding_handshake = transfer.active_handshake_id.take(); let snapshot = transfer.clone(); // Cancel the riding handshake too. - if let Some(hid) = &snapshot.active_handshake_id { + if let Some(hid) = &riding_handshake { if let Some(h) = org.handshakes.get_mut(hid) { h.state = "CANCELED".to_string(); } } Ok(AwsResponse::ok_json( - json!({ "ResponsibilityTransfer": transfer_payload(&snapshot) }), + json!({ "ResponsibilityTransfer": transfer_payload(&guard, &snapshot, &req.account_id) }), )) } @@ -255,6 +509,11 @@ impl OrganizationsService { self.list_responsibility_transfers(req, "OUTBOUND") } + /// AWS's `ResponsibilityTransfer` shape has no `Direction` member -- + /// direction is expressed by which operation you call, so the stored + /// `direction` field is fakecloud-internal provenance surfaced only + /// through introspection. Which list a transfer belongs to is decided + /// by whether the caller is its source or its target. fn list_responsibility_transfers( &self, req: &AwsRequest, @@ -263,14 +522,49 @@ impl OrganizationsService { let body = req.json_body(); // `Type` is required on both list ops. let transfer_type = require_transfer_type(&body)?; + // Only the INBOUND request models an optional `Id` to fetch a + // single transfer. + let only_id = (direction == "INBOUND") + .then(|| body.get("Id").and_then(|v| v.as_str()).map(str::to_string)) + .flatten(); let (max_results, next_token) = parse_list_pagination(&body)?; let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; - let filtered: Vec = org - .responsibility_transfers - .values() - .filter(|t| t.direction == direction && t.transfer_type == transfer_type) - .map(transfer_payload) + // OUTBOUND is membership-gated: its caller is a source management + // account by construction, so `AWSOrganizationsNotInUseException` + // is the right modeled answer. INBOUND is NOT: the target may be + // an account that has not created an organization yet -- see the + // invite guard above -- and it has to be able to find the + // transfer addressed to it. Party scoping already keeps it from + // seeing anything else. + if direction == "OUTBOUND" { + self.require_member(&guard, &req.account_id)?; + } + let mut rows: Vec<&ResponsibilityTransfer> = guard + .iter() + .flat_map(|org| org.responsibility_transfers.values()) + .filter(|t| { + only_id.as_deref().is_none_or(|id| t.id == id) + && t.transfer_type == transfer_type + && match direction { + "OUTBOUND" => t.source_management_account_id == req.account_id, + _ => is_transfer_target(&guard, t, &req.account_id), + } + }) + .collect(); + // Merged across organizations, so impose a stable order for + // pagination rather than relying on per-organization map order. + // `ListInboundResponsibilityTransfers` models a not-found error, so + // a named id the caller is not a party to is reported rather than + // silently returned as an empty page. + if let Some(id) = &only_id { + if rows.is_empty() { + return Err(transfer_not_found(id)); + } + } + rows.sort_by(|a, b| a.id.cmp(&b.id)); + let filtered: Vec = rows + .into_iter() + .map(|t| transfer_payload(&guard, t, &req.account_id)) .collect(); let (page, token) = paginate_checked(&filtered, next_token.as_deref(), max_results) .map_err(|_| invalid_input("Invalid NextToken"))?; diff --git a/crates/fakecloud-organizations/src/service/roots.rs b/crates/fakecloud-organizations/src/service/roots.rs index e532f8f9a..bac065a27 100644 --- a/crates/fakecloud-organizations/src/service/roots.rs +++ b/crates/fakecloud-organizations/src/service/roots.rs @@ -25,7 +25,7 @@ impl OrganizationsService { let body = req.json_body(); let child_id = required_str(&body, "ChildId")?.to_string(); let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; + let org = self.require_member(&guard, &req.account_id)?; let parents = match org.parent_of(&child_id) { Some((id, kind)) => vec![json!({"Id": id, "Type": kind})], None => Vec::new(), @@ -38,7 +38,7 @@ impl OrganizationsService { let parent_id = required_str(&body, "ParentId")?.to_string(); let child_type = required_str(&body, "ChildType")?.to_string(); let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; + let org = self.require_member(&guard, &req.account_id)?; let children: Vec = org .list_children(&parent_id, &child_type) .into_iter() diff --git a/crates/fakecloud-organizations/src/service/service_access.rs b/crates/fakecloud-organizations/src/service/service_access.rs index 28f902a4d..4b149b04e 100644 --- a/crates/fakecloud-organizations/src/service/service_access.rs +++ b/crates/fakecloud-organizations/src/service/service_access.rs @@ -10,8 +10,7 @@ impl OrganizationsService { let body = req.json_body(); let principal = required_str(&body, "ServicePrincipal")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.enable_aws_service_access(&principal); Ok(AwsResponse::ok_json(json!({}))) } @@ -23,8 +22,7 @@ impl OrganizationsService { let body = req.json_body(); let principal = required_str(&body, "ServicePrincipal")?.to_string(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.disable_aws_service_access(&principal) .map_err(org_error_to_aws)?; Ok(AwsResponse::ok_json(json!({}))) @@ -36,9 +34,8 @@ impl OrganizationsService { ) -> Result { let body = req.json_body(); let (max_results, next_token) = parse_list_pagination(&body)?; - let guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_ref().expect("management gate proved Some"); + let guard = self.state.read(); + let org = self.management_or_delegated_org(&guard, &req.account_id)?; let entries: Vec = org .list_trusted_services() .into_iter() diff --git a/crates/fakecloud-organizations/src/service/tags.rs b/crates/fakecloud-organizations/src/service/tags.rs index 478a5a700..307483dbf 100644 --- a/crates/fakecloud-organizations/src/service/tags.rs +++ b/crates/fakecloud-organizations/src/service/tags.rs @@ -8,8 +8,7 @@ impl OrganizationsService { let resource_id = required_str(&body, "ResourceId")?.to_string(); let tags = parse_tags(body.get("Tags")); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.set_resource_tags(&resource_id, &tags); Ok(AwsResponse::ok_json(json!({}))) } @@ -27,8 +26,7 @@ impl OrganizationsService { }) .unwrap_or_default(); let mut guard = self.state.write(); - self.require_member_management(&guard, &req.account_id)?; - let org = guard.as_mut().expect("management gate proved Some"); + let org = self.management_org_mut(&mut guard, &req.account_id)?; org.untag_resource(&resource_id, &tag_keys); Ok(AwsResponse::ok_json(json!({}))) } @@ -40,7 +38,7 @@ impl OrganizationsService { let body = req.json_body(); let resource_id = required_str(&body, "ResourceId")?.to_string(); let guard = self.state.read(); - let org = guard.as_ref().ok_or_else(organizations_not_in_use)?; + let org = self.require_member(&guard, &req.account_id)?; let tags: Vec = org .list_resource_tags(&resource_id) .into_iter() diff --git a/crates/fakecloud-organizations/src/service/tests.rs b/crates/fakecloud-organizations/src/service/tests.rs index 00f06f307..780adc537 100644 --- a/crates/fakecloud-organizations/src/service/tests.rs +++ b/crates/fakecloud-organizations/src/service/tests.rs @@ -47,20 +47,1298 @@ async fn create_organization_succeeds_once() { assert_eq!(resp.status, StatusCode::OK); let v = body_json(&resp); assert_eq!(v["Organization"]["MasterAccountId"], "111111111111"); - assert!(state.read().is_some()); + assert!(!state.read().is_empty()); } #[tokio::test] -async fn create_organization_twice_errors() { +async fn create_organization_twice_from_the_same_account_errors() { let (svc, _state) = OrganizationsService::shared(); svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) .await .unwrap(); + let err = expect_err( + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await, + ); + assert_eq!(err.code(), "AlreadyInOrganizationException"); +} + +/// #2543: organizations are independent. One account creating an +/// organization must not stop an unrelated account from creating its +/// own, and the two must not see each other. +#[tokio::test] +async fn a_second_account_can_create_its_own_organization() { + let (svc, state) = OrganizationsService::shared(); + let first = body_json( + &svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(), + ); + let second = body_json( + &svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(), + ); + + let first_id = first["Organization"]["Id"].as_str().unwrap(); + let second_id = second["Organization"]["Id"].as_str().unwrap(); + assert_ne!(first_id, second_id); + assert_eq!(state.read().len(), 2); + + // Each management account describes only its own organization. + let described = body_json( + &svc.handle(req_with("222222222222", "DescribeOrganization", json!({}))) + .await + .unwrap(), + ); + assert_eq!(described["Organization"]["Id"], second_id); + assert_eq!(described["Organization"]["MasterAccountId"], "222222222222"); + + // ...and lists only its own accounts. + let listed = body_json( + &svc.handle(req_with("222222222222", "ListAccounts", json!({}))) + .await + .unwrap(), + ); + let ids: Vec<&str> = listed["Accounts"] + .as_array() + .unwrap() + .iter() + .map(|a| a["Id"].as_str().unwrap()) + .collect(); + assert_eq!(ids, ["222222222222"]); +} + +/// An account already in an organization cannot create another one, and +/// the error is the same whichever organization it belongs to. +#[tokio::test] +async fn a_member_of_another_organization_cannot_create_one() { + let (svc, state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + let err = expect_err( svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) .await, ); - assert_eq!(err.code(), "AlreadyInOrganizationException"); + assert_eq!(err.code(), "AlreadyInOrganizationException"); +} + +/// An account can only ever be in one organization, so an invitation to +/// an account another organization already holds is rejected up front +/// rather than opening a handshake that could never be accepted. +#[tokio::test] +async fn inviting_an_account_from_another_organization_errors() { + let (svc, _state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + + let err = expect_err( + svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); +} + +/// Reads that used to be satisfied by "an organization exists" now +/// resolve the caller's own organization. A bystander account must not +/// be able to read another organization's tree, tags or resource policy +/// by guessing ids. +#[tokio::test] +async fn a_bystander_cannot_read_another_organizations_state() { + let (svc, state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + let root_id = state.read().sole().unwrap().root_id.clone(); + + svc.handle(req_with( + "111111111111", + "PutResourcePolicy", + json!({ "Content": "{\"Version\":\"2012-10-17\",\"Statement\":[]}" }), + )) + .await + .unwrap(); + + for (action, body) in [ + ("ListParents", json!({ "ChildId": "111111111111" })), + ( + "ListChildren", + json!({ "ParentId": root_id, "ChildType": "ACCOUNT" }), + ), + ("ListTagsForResource", json!({ "ResourceId": root_id })), + ("DescribeResourcePolicy", json!({})), + ( + "DescribeEffectivePolicy", + json!({ "PolicyType": "SERVICE_CONTROL_POLICY" }), + ), + ] { + let err = expect_err(svc.handle(req_with("999999999999", action, body)).await); + assert_eq!( + err.code(), + "AWSOrganizationsNotInUseException", + "{action} leaked another organization's state to a non-member" + ); + } +} + +/// A handshake is readable only by its two parties. Otherwise a +/// bystander could enumerate handshake ids to learn another +/// organization's id and management account. +#[tokio::test] +async fn describe_handshake_is_limited_to_the_parties() { + let (svc, _state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + + // The target can read it... + svc.handle(req_with( + "222222222222", + "DescribeHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .unwrap(); + + // ...a bystander cannot. + let err = expect_err( + svc.handle(req_with( + "999999999999", + "DescribeHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeNotFoundException"); +} + +/// Membership is re-checked when the handshake is accepted, not only +/// when it is opened: the target may have joined another organization +/// while the invitation sat open. +#[tokio::test] +async fn accepting_an_invite_fails_once_the_target_joined_another_organization() { + let (svc, _state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + + // The target creates its own organization before answering. + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + + let err = expect_err( + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); +} + +/// An EMAIL-target invite records the address, not the account id. The +/// account it names must still be able to read and accept it — matching +/// the raw field would compare an address against a 12-digit id and +/// never hit. +#[tokio::test] +async fn an_email_target_invite_is_readable_and_acceptable_by_the_account_it_names() { + let (svc, state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "EMAIL", "Id": "222222222222@example.com" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + + svc.handle(req_with( + "222222222222", + "DescribeHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .expect("the named account is a party to the invitation"); + + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .expect("the named account can accept"); + + assert!(state + .read() + .org_of_account("222222222222") + .is_some_and(|org| org.is_management("111111111111"))); +} + +/// The cross-organization guard applies to an EMAIL target too, once it +/// resolves — otherwise the invite opens a handshake nobody can accept. +#[tokio::test] +async fn an_email_invite_to_an_account_in_another_organization_errors() { + let (svc, _state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + + let err = expect_err( + svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "EMAIL", "Id": "222222222222@example.com" } }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); +} + +/// `Target.Id` must match the declared `Target.Type`. Accepting a +/// mismatch opened a handshake keyed by a string no caller can ever +/// authenticate as, which then sat OPEN forever with no error anywhere +/// the caller could see it. +#[tokio::test] +async fn invite_rejects_a_target_id_that_does_not_match_its_type() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + + for target in [ + json!({ "Type": "ACCOUNT", "Id": "bob@corp.com" }), + json!({ "Type": "ACCOUNT", "Id": "12345" }), + json!({ "Type": "EMAIL", "Id": "222222222222" }), + json!({ "Type": "SOMETHING", "Id": "222222222222" }), + ] { + let err = expect_err( + svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": target }), + )) + .await, + ); + assert_eq!( + err.code(), + "InvalidInputException", + "target {target} should have been rejected" + ); + } +} + +/// `TerminateResponsibilityTransfer` ends a transfer, which includes one +/// already ACCEPTED and running -- that is the case the operation exists +/// for. Syncing the transfer's status with its handshake must not lock +/// the accepted transfer out of ever being ended. +#[tokio::test] +async fn an_accepted_responsibility_transfer_can_still_be_terminated() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + let invite = body_value( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222", "Type": "ACCOUNT"}, + }), + )) + .await + .unwrap(), + ); + let handshake_id = invite["Handshake"]["Id"].as_str().unwrap().to_string(); + let listed = body_value( + svc.handle(req_with( + "111111111111", + "ListOutboundResponsibilityTransfers", + json!({ "Type": "BILLING" }), + )) + .await + .unwrap(), + ); + let transfer_id = listed["ResponsibilityTransfers"][0]["Id"] + .as_str() + .unwrap() + .to_string(); + + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .unwrap(); + + // An accepted transfer is starting, not over. + let accepted = body_value( + svc.handle(req_with( + "111111111111", + "DescribeResponsibilityTransfer", + json!({ "Id": transfer_id }), + )) + .await + .unwrap(), + ); + assert_eq!(accepted["ResponsibilityTransfer"]["Status"], "ACCEPTED"); + assert!( + accepted["ResponsibilityTransfer"]["EndTimestamp"].is_null(), + "an accepted transfer has not ended" + ); + + // Once accepted the riding handshake is gone, so the TARGET -- which + // is actively carrying the responsibility -- can end it too. + let ended = body_value( + svc.handle(req_with( + "222222222222", + "TerminateResponsibilityTransfer", + json!({ "Id": transfer_id }), + )) + .await + .expect("an accepted transfer can be ended by either party"), + ); + assert_eq!(ended["ResponsibilityTransfer"]["Status"], "WITHDRAWN"); + assert!(!ended["ResponsibilityTransfer"]["EndTimestamp"].is_null()); +} + +/// AWS's primary invite flow names the account owner's real address, so +/// an external address is accepted -- fakecloud just cannot resolve it +/// to an account, exactly as AWS cannot until the owner acts on the +/// emailed link. +#[tokio::test] +async fn invite_accepts_an_external_email_target() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "EMAIL", "Id": "owner@acme.com" } }), + )) + .await + .expect("a real address is a valid invite target"), + ); + assert_eq!(invited["Handshake"]["State"], "OPEN"); +} + +/// AWS documents `DescribeHandshake` as callable from any account in the +/// organization, not just the handshake's two parties -- while an +/// account in a DIFFERENT organization still sees nothing. +#[tokio::test] +async fn describe_handshake_is_readable_by_any_member_of_the_owning_org() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("333333333333"); + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + + // A plain member of the inviting organization can read it. + svc.handle(req_with( + "333333333333", + "DescribeHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .expect("any member of the owning organization may read it"); + + // An account outside the organization still cannot. + let err = expect_err( + svc.handle(req_with( + "999999999999", + "DescribeHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeNotFoundException"); +} + +/// An EMAIL-targeted responsibility transfer must be answerable by the +/// account the address names. Recording the resolved id while still +/// labelling the target "EMAIL" made it unresolvable again, so the +/// target could neither accept nor describe its own handshake. +#[tokio::test] +async fn an_email_targeted_responsibility_transfer_is_answerable() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + + let invite = body_value( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222@example.com", "Type": "EMAIL"}, + }), + )) + .await + .unwrap(), + ); + let handshake_id = invite["Handshake"]["Id"].as_str().unwrap().to_string(); + + svc.handle(req_with( + "222222222222", + "DescribeHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .expect("the account the address names is a party to it"); + + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .expect("and can accept it"); +} + +/// A responsibility transfer cannot name the caller's own organization, +/// nor a plain member of another one -- neither has billing +/// responsibility to hand over. An account in no organization IS a +/// valid target, the way AWS's invite addresses an owner it has not +/// seen yet. Every rejection reads the same, so the error says nothing +/// about what exists elsewhere. +#[tokio::test] +async fn a_responsibility_transfer_cannot_target_its_own_organization() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let own_email = state + .read() + .sole() + .unwrap() + .management_account_email + .clone(); + + // A plain member of the caller's own organization, by id and by both + // spellings of its address, is just as much "itself". + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("333333333333"); + for target in [ + json!({"Id": "111111111111", "Type": "ACCOUNT"}), + json!({"Id": own_email, "Type": "EMAIL"}), + json!({"Id": "333333333333", "Type": "ACCOUNT"}), + json!({"Id": "333333333333@example.com", "Type": "EMAIL"}), + ] { + let err = expect_err( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": target, + }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); + } + + // Nor a member of ANOTHER organization that is not its management + // account -- it cannot take over billing for one. + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + state + .write() + .org_of_account_mut("222222222222") + .unwrap() + .enroll_account_if_missing("222222220001"); + // Both spellings of that member are refused: resolving only the + // ACCOUNT form left the EMAIL form as a way around the check. + for target in [ + json!({"Id": "222222220001", "Type": "ACCOUNT"}), + json!({"Id": "222222220001@example.com", "Type": "EMAIL"}), + ] { + let err = expect_err( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": target, + }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); + } + + // An account in NO organization is allowed: AWS's invite addresses an + // owner it has not seen yet, and the inbound reads are party-scoped + // so the target can still find it. + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "ops@acme.com", "Type": "EMAIL"}, + }), + )) + .await + .expect("an owner fakecloud has not seen is a valid target"); + + // As is another organization's management account. + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222", "Type": "ACCOUNT"}, + }), + )) + .await + .expect("another organization's management account is a valid target"); +} + +/// Whoever can accept an invitation must also be able to find it and +/// act on what it created. Two matchers that disagreed let an account +/// accept a transfer it could then neither read nor end. +#[tokio::test] +async fn the_target_of_an_email_invite_can_find_accept_and_act_on_it() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + + let invite = body_value( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222@example.com", "Type": "EMAIL"}, + }), + )) + .await + .unwrap(), + ); + let handshake_id = invite["Handshake"]["Id"].as_str().unwrap().to_string(); + + // Findable... + let listed = body_value( + svc.handle(req_with( + "222222222222", + "ListHandshakesForAccount", + json!({}), + )) + .await + .unwrap(), + ); + assert_eq!(listed["Handshakes"][0]["Id"], handshake_id.as_str()); + + // ...acceptable... + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .expect("the named account can accept"); + + // ...and the transfer it accepted is readable and endable by it. + let inbound = body_value( + svc.handle(req_with( + "222222222222", + "ListInboundResponsibilityTransfers", + json!({ "Type": "BILLING" }), + )) + .await + .unwrap(), + ); + let transfer_id = inbound["ResponsibilityTransfers"][0]["Id"] + .as_str() + .expect("the accepted transfer is visible to its target") + .to_string(); + svc.handle(req_with( + "222222222222", + "TerminateResponsibilityTransfer", + json!({ "Id": transfer_id }), + )) + .await + .expect("and can be ended by it"); +} + +/// `CreateAccount` hands back the new id immediately and enrolls it a +/// moment later. During that window the id is already spoken for: it +/// must not be able to create an organization of its own, be invited +/// elsewhere, or accept an invitation, or the completion tick would +/// leave it in two organizations at once. +#[tokio::test] +async fn an_account_id_reserved_by_create_account_is_already_claimed() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let created = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "dev@example.com", "AccountName": "dev" }), + )) + .await + .unwrap(), + ); + let reserved = created["CreateAccountStatus"]["AccountId"] + .as_str() + .expect("the id is handed back before enrollment") + .to_string(); + // Still only the management account is enrolled. + assert!(state.read().org_of_account(&reserved).is_none()); + + // ...but it cannot start an organization of its own. + let err = expect_err( + svc.handle(req_with(&reserved, "CreateOrganization", json!({}))) + .await, + ); + assert_eq!(err.code(), "AlreadyInOrganizationException"); + + // ...nor be invited into another one. + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + let err = expect_err( + svc.handle(req_with( + "222222222222", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": reserved } }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); +} + +/// `DescribeEffectivePolicy` must not answer for a target in another +/// organization. Walking a hierarchy that does not contain the target +/// found no ancestors and returned an empty, successful "no effective +/// policy" -- the worst answer for a caller auditing one. +#[tokio::test] +async fn describe_effective_policy_rejects_a_target_in_another_organization() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + let other_root = state + .read() + .org_of_account("222222222222") + .unwrap() + .root_id + .clone(); + + for target in [other_root.as_str(), "222222222222"] { + let err = expect_err( + svc.handle(req_with( + "111111111111", + "DescribeEffectivePolicy", + json!({ "PolicyType": "SERVICE_CONTROL_POLICY", "TargetId": target }), + )) + .await, + ); + assert_eq!(err.code(), "TargetNotFoundException"); + } + + // The caller's own account still resolves. + svc.handle(req_with( + "111111111111", + "DescribeEffectivePolicy", + json!({ "PolicyType": "SERVICE_CONTROL_POLICY" }), + )) + .await + .expect("the caller's own account is a valid target"); +} + +/// An address names exactly one account. `CreateAccount` stores the +/// caller's own email, so an account can be registered with an address +/// that *looks* like the synthetic form of a different id -- and then +/// both resolutions were accepted, letting the account the address only +/// spells read and accept an invitation meant for its real owner. +#[tokio::test] +async fn a_registered_address_names_its_own_account_and_no_other() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + // A member registered with an address that spells another id. + let created = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "222222222222@example.com", "AccountName": "decoy" }), + )) + .await + .unwrap(), + ); + let owner = created["CreateAccountStatus"]["AccountId"] + .as_str() + .unwrap() + .to_string(); + assert_ne!(owner, "222222222222"); + state + .write() + .org_of_account_mut("111111111111") + .unwrap() + .complete_create_account(created["CreateAccountStatus"]["Id"].as_str().unwrap()); + + // Inviting that address now names a member already enrolled. + let err = expect_err( + svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "EMAIL", "Id": "222222222222@example.com" } }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); + + // And the account the address merely spells is not its owner. + assert!(state + .read() + .account_matches_target("EMAIL", "222222222222@example.com", &owner)); + assert!(!state.read().account_matches_target( + "EMAIL", + "222222222222@example.com", + "222222222222" + )); +} + +/// AWS requires an account's address to be unused, and reports a +/// duplicate ASYNCHRONOUSLY: `CreateAccount` models no synchronous error +/// for it, so a polling client (Terraform's `aws_organizations_account`) +/// must still get a request id back. The request then lands in FAILED +/// with `EMAIL_ALREADY_EXISTS`. +#[tokio::test] +async fn create_account_fails_asynchronously_on_a_duplicate_address() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let first = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "ops@corp.com", "AccountName": "ops" }), + )) + .await + .unwrap(), + ); + state + .write() + .sole_mut() + .unwrap() + .complete_create_account(first["CreateAccountStatus"]["Id"].as_str().unwrap()); + + // The duplicate is accepted, with a request id to poll. + let second = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "ops@corp.com", "AccountName": "dupe" }), + )) + .await + .expect("a duplicate address is not a synchronous error"), + ); + assert_eq!(second["CreateAccountStatus"]["State"], "IN_PROGRESS"); + let request_id = second["CreateAccountStatus"]["Id"].as_str().unwrap(); + + // ...and resolves to FAILED rather than a second account on one + // address, which would make resolution depend on id ordering. + let described = body_value( + svc.handle(req_with( + "111111111111", + "DescribeCreateAccountStatus", + json!({ "CreateAccountRequestId": request_id }), + )) + .await + .unwrap(), + ); + assert_eq!(described["CreateAccountStatus"]["State"], "IN_PROGRESS"); + + // Drive the tick the server would run in the background. + let failed = state + .write() + .sole_mut() + .unwrap() + .fail_create_account(request_id, "EMAIL_ALREADY_EXISTS") + .unwrap(); + assert_eq!(failed.state, "FAILED"); + assert_eq!( + failed.failure_reason.as_deref(), + Some("EMAIL_ALREADY_EXISTS") + ); +} + +/// Accepting an invitation you have since satisfied another way is not a +/// silent no-op. `invite_account` rejects an existing member at invite +/// time; the account may have joined between invite and accept, and the +/// two gates must give the same answer. +#[tokio::test] +async fn accepting_after_joining_the_inviting_organization_errors() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + + // It joins the same organization by another route while the + // invitation sits open. + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + + let err = expect_err( + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + assert_eq!(err.code(), "HandshakeConstraintViolationException"); +} + +/// Closing an account releases its address everywhere. `CloseAccount` +/// only suspends, so a resolver that still matched the closed record +/// would let the address be re-used by `CreateAccount` while making it +/// permanently un-invitable. +#[tokio::test] +async fn a_closed_account_releases_its_address() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let created = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "alice@corp.com", "AccountName": "alice" }), + )) + .await + .unwrap(), + ); + let account_id = { + let mut guard = state.write(); + let org = guard.sole_mut().unwrap(); + org.complete_create_account(created["CreateAccountStatus"]["Id"].as_str().unwrap()); + let id = created["CreateAccountStatus"]["AccountId"] + .as_str() + .unwrap() + .to_string(); + org.close_account(&id).unwrap(); + id + }; + + // The address is free again... + assert!(!state.read().email_in_use("alice@corp.com")); + // ...and no longer resolves to the closed account, so inviting it is + // not refused as "already a member". + assert!(!state + .read() + .account_matches_target("EMAIL", "alice@corp.com", &account_id)); + svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "EMAIL", "Id": "alice@corp.com" } }), + )) + .await + .expect("a closed account's address can be invited again"); +} + +/// A `@example.com` address belongs to the id it spells. +/// fakecloud mints those for the accounts it creates, so letting an +/// unrelated account register one put two live accounts on one address +/// -- and resolution by address decides who may accept an +/// EMAIL-targeted handshake. +#[tokio::test] +async fn a_synthetic_address_is_reserved_for_the_account_it_spells() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + + // `CreateAccount` cannot take another id's synthetic address. + let created = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "222222222222@example.com", "AccountName": "decoy" }), + )) + .await + .expect("accepted, then failed asynchronously"), + ); + let request_id = created["CreateAccountStatus"]["Id"].as_str().unwrap(); + let minted = created["CreateAccountStatus"]["AccountId"] + .as_str() + .unwrap() + .to_string(); + assert!( + crate::state::OrganizationsRegistry::email_reserved_for_other( + "222222222222@example.com", + &minted + ), + "the address spells an id other than the one being created" + ); + state + .write() + .sole_mut() + .unwrap() + .fail_create_account(request_id, "EMAIL_ALREADY_EXISTS"); + + // ...and the account it spells keeps it when it creates its own + // organization. + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .expect("its own synthetic address is free"); + assert!(state.read().account_matches_target( + "EMAIL", + "222222222222@example.com", + "222222222222" + )); +} + +/// AWS lets the management account OR a delegated administrator read +/// the resource policy, unlike `Put`/`Delete`, which are +/// management-only. A plain member still cannot. +#[tokio::test] +async fn describe_resource_policy_allows_a_delegated_administrator() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + svc.handle(req_with( + "111111111111", + "PutResourcePolicy", + json!({ "Content": "{\"Version\":\"2012-10-17\",\"Statement\":[]}" }), + )) + .await + .unwrap(); + + // A plain member cannot read it. + let err = expect_err( + svc.handle(req_with( + "222222222222", + "DescribeResourcePolicy", + json!({}), + )) + .await, + ); + assert_eq!(err.code(), "AccessDeniedException"); + + // Registered as a delegated administrator, it can. + svc.handle(req_with( + "111111111111", + "EnableAWSServiceAccess", + json!({ "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + svc.handle(req_with( + "111111111111", + "RegisterDelegatedAdministrator", + json!({ "AccountId": "222222222222", "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + svc.handle(req_with( + "222222222222", + "DescribeResourcePolicy", + json!({}), + )) + .await + .expect("a delegated administrator may read the resource policy"); +} + +/// Re-accepting an already-accepted handshake is a terminal-transition +/// error, not a membership one. The membership gates are necessarily +/// satisfied once the accept succeeded, so checking them first made a +/// client retrying after a timeout read a join that had worked as a +/// hard constraint failure -- and disagreed with Decline/Cancel, which +/// answered correctly. +#[tokio::test] +async fn re_accepting_a_handshake_reports_the_transition_error() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .unwrap(); + + for action in ["AcceptHandshake", "DeclineHandshake"] { + let err = expect_err( + svc.handle(req_with( + "222222222222", + action, + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + assert_eq!( + err.code(), + "InvalidHandshakeTransitionException", + "{action} on a terminal handshake" + ); + } +} + +/// A `CreateAccount` that ends FAILED leaves no tags behind on the id it +/// reserved: create-time tags are applied straight away, but that id +/// never becomes an account, and AWS answers `TargetNotFoundException` +/// for an id that is not a real resource. +#[tokio::test] +async fn a_failed_create_account_drops_the_tags_it_reserved() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let created = body_value( + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ + "Email": "222222222222@example.com", + "AccountName": "doomed", + "Tags": [{ "Key": "env", "Value": "prod" }], + }), + )) + .await + .unwrap(), + ); + let account_id = created["CreateAccountStatus"]["AccountId"] + .as_str() + .unwrap() + .to_string(); + // Tagged synchronously, against the reserved id. + assert!(!state + .read() + .sole() + .unwrap() + .list_resource_tags(&account_id) + .is_empty()); + + state.write().sole_mut().unwrap().fail_create_account( + created["CreateAccountStatus"]["Id"].as_str().unwrap(), + "EMAIL_ALREADY_EXISTS", + ); + + assert!( + state + .read() + .sole() + .unwrap() + .list_resource_tags(&account_id) + .is_empty(), + "a failed request's reserved id keeps no tags" + ); +} + +/// A non-party learns nothing from a handshake id it guessed -- not +/// even whether it exists or what state it is in. The terminal-state +/// answer is for the handshake's own parties. +#[tokio::test] +async fn a_bystander_cannot_read_handshake_state_off_an_error() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .unwrap(); + + // Resolved, but a bystander is told only "not a party". + let err = expect_err( + svc.handle(req_with( + "999999999999", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + // `InvalidHandshakeParty` renders as AccessDenied -- the point is + // that it says nothing about the handshake's state. + assert_eq!(err.code(), "AccessDeniedException"); + + // ...while a party still gets the transition error it needs. + let err = expect_err( + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await, + ); + assert_eq!(err.code(), "InvalidHandshakeTransitionException"); +} + +/// `ListHandshakesForAccount` lists the handshakes associated with the +/// calling account, which includes the invitations it SENT -- not only +/// those addressed to it. +#[tokio::test] +async fn list_handshakes_for_account_includes_the_ones_it_sent() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let invited = body_json( + &svc.handle(req_with( + "111111111111", + "InviteAccountToOrganization", + json!({ "Target": { "Type": "ACCOUNT", "Id": "222222222222" } }), + )) + .await + .unwrap(), + ); + let handshake_id = invited["Handshake"]["Id"].as_str().unwrap().to_string(); + + for caller in ["111111111111", "222222222222"] { + let listed = body_value( + svc.handle(req_with(caller, "ListHandshakesForAccount", json!({}))) + .await + .unwrap(), + ); + assert_eq!( + listed["Handshakes"][0]["Id"], + handshake_id.as_str(), + "{caller} should see the handshake it is a party to" + ); + } + + // A bystander sees none of it. + let listed = body_value( + svc.handle(req_with( + "999999999999", + "ListHandshakesForAccount", + json!({}), + )) + .await + .unwrap(), + ); + assert!(listed["Handshakes"].as_array().unwrap().is_empty()); +} + +/// Deleting one organization leaves every other one standing. +#[tokio::test] +async fn deleting_one_organization_leaves_the_others() { + let (svc, state) = OrganizationsService::shared(); + svc.handle(req_with("111111111111", "CreateOrganization", json!({}))) + .await + .unwrap(); + let second = body_json( + &svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(), + ); + let second_id = second["Organization"]["Id"].as_str().unwrap().to_string(); + + svc.handle(req_with("111111111111", "DeleteOrganization", json!({}))) + .await + .unwrap(); + + let guard = state.read(); + assert_eq!(guard.len(), 1); + assert!(guard.org_by_id(&second_id).is_some()); + assert!(guard.org_of_account("111111111111").is_none()); } #[tokio::test] @@ -124,7 +1402,7 @@ async fn member_non_management_delete_returns_access_denied() { // directly in state (auto-enrollment lands in Batch 2). { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let account_id = "222222222222".to_string(); let parent_id = org.root_id.clone(); let org_id = org.org_id.clone(); @@ -162,7 +1440,7 @@ async fn delete_clears_state() { svc.handle(req_with("111111111111", "DeleteOrganization", json!({}))) .await .unwrap(); - assert!(state.read().is_none()); + assert!(state.read().is_empty()); } #[tokio::test] @@ -400,7 +1678,7 @@ async fn create_ou_non_management_rejected() { { let mut guard = state.write(); guard - .as_mut() + .sole_mut() .unwrap() .enroll_account_if_missing("222222222222"); } @@ -510,7 +1788,7 @@ async fn delete_ou_rejects_when_not_empty() { .to_string(); { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.enroll_account_if_missing("222222222222"); let root = org.root_id.clone(); org.move_account("222222222222", &root, &ou_id).unwrap(); @@ -627,7 +1905,7 @@ async fn list_accounts_returns_all_members() { { let mut guard = state.write(); guard - .as_mut() + .sole_mut() .unwrap() .enroll_account_if_missing("222222222222"); } @@ -657,7 +1935,7 @@ async fn list_accounts_for_parent_scopes_to_parent() { .to_string(); { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.enroll_account_if_missing("222222222222"); org.move_account("222222222222", &org.root_id.clone(), &ou_id) .unwrap(); @@ -734,7 +2012,7 @@ async fn move_account_happy_path() { { let mut guard = state.write(); guard - .as_mut() + .sole_mut() .unwrap() .enroll_account_if_missing("222222222222"); } @@ -750,7 +2028,7 @@ async fn move_account_happy_path() { .await .unwrap(); let guard = state.read(); - let org = guard.as_ref().unwrap(); + let org = guard.sole().unwrap(); assert_eq!(org.accounts.get("222222222222").unwrap().parent_id, ou_id); } @@ -792,7 +2070,7 @@ async fn move_account_wrong_source_parent() { { let mut guard = state.write(); guard - .as_mut() + .sole_mut() .unwrap() .enroll_account_if_missing("222222222222"); } @@ -959,7 +2237,7 @@ async fn create_policy_non_management_rejected() { { let mut guard = state.write(); guard - .as_mut() + .sole_mut() .unwrap() .enroll_account_if_missing("222222222222"); } @@ -1716,7 +2994,12 @@ async fn leave_organization_non_member_errors() { svc.handle(req_with("999999999999", "LeaveOrganization", json!({}))) .await, ); - assert_eq!(err.code(), "AccountNotFoundException"); + // `LeaveOrganization` takes no AccountId, so AWS's answer for a caller + // that belongs to no organization is `AWSOrganizationsNotInUseException`, + // not `AccountNotFoundException`. It is also the non-leaking answer: with + // several organizations in the process, a non-member must not be able to + // tell whether any exist. + assert_eq!(err.code(), "AWSOrganizationsNotInUseException"); } #[tokio::test] @@ -1779,6 +3062,11 @@ async fn list_effective_policy_validation_errors_is_empty() { async fn responsibility_transfer_lifecycle() { let (svc, _state) = OrganizationsService::shared(); create_org_with_root(&svc).await; + // The op invites an ORGANIZATION, so the target must be another + // organization's management account. + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); // Invite an outbound BILLING transfer. let invite = svc .handle(req_with( @@ -1864,24 +3152,218 @@ async fn responsibility_transfer_lifecycle() { .handle(req_with( "111111111111", "TerminateResponsibilityTransfer", - json!({"Id": transfer_id}), + json!({"Id": transfer_id}), + )) + .await + .unwrap(); + let term_body = body_value(term); + assert_eq!(term_body["ResponsibilityTransfer"]["Status"], "WITHDRAWN"); + assert!(term_body["ResponsibilityTransfer"]["EndTimestamp"].is_number()); + + // Terminating again is rejected (already withdrawn). + let err = expect_err( + svc.handle(req_with( + "111111111111", + "TerminateResponsibilityTransfer", + json!({"Id": transfer_id}), + )) + .await, + ); + assert_eq!(err.code(), "ResponsibilityTransferAlreadyInStatusException"); +} + +/// The inbound side of a transfer: the target management account runs +/// its own organization, and must be able to see, accept, and correctly +/// read the direction of the transfer offered to it. +#[tokio::test] +async fn the_target_organization_sees_its_inbound_responsibility_transfer() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + + let invite = body_value( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222", "Type": "ACCOUNT"}, + }), + )) + .await + .unwrap(), + ); + let handshake_id = invite["Handshake"]["Id"].as_str().unwrap().to_string(); + + // The target lists it INBOUND -- the stored row is the source's, and + // reads OUTBOUND there. + let inbound = body_value( + svc.handle(req_with( + "222222222222", + "ListInboundResponsibilityTransfers", + json!({ "Type": "BILLING" }), + )) + .await + .unwrap(), + ); + let rows = inbound["ResponsibilityTransfers"].as_array().unwrap(); + assert_eq!(rows.len(), 1, "target must see the transfer offered to it"); + let transfer_id = rows[0]["Id"].as_str().unwrap().to_string(); + + // The same transfer is the source's OUTBOUND one. AWS's shape carries + // no Direction member -- which list you call IS the direction. + let outbound = body_value( + svc.handle(req_with( + "111111111111", + "ListOutboundResponsibilityTransfers", + json!({ "Type": "BILLING" }), + )) + .await + .unwrap(), + ); + assert_eq!( + outbound["ResponsibilityTransfers"][0]["Id"], + transfer_id.as_str() + ); + // ...and the target can describe it directly. + let described = body_value( + svc.handle(req_with( + "222222222222", + "DescribeResponsibilityTransfer", + json!({ "Id": transfer_id }), + )) + .await + .unwrap(), + ); + assert_eq!( + described["ResponsibilityTransfer"]["Id"], + transfer_id.as_str() + ); + + // Accepting the riding handshake moves the transfer with it, rather + // than leaving an ACCEPTED handshake beside a REQUESTED transfer. + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": handshake_id }), + )) + .await + .unwrap(); + let after = body_value( + svc.handle(req_with( + "111111111111", + "DescribeResponsibilityTransfer", + json!({ "Id": transfer_id }), + )) + .await + .unwrap(), + ); + assert_eq!(after["ResponsibilityTransfer"]["Status"], "ACCEPTED"); + assert!(after["ResponsibilityTransfer"]["ActiveHandshakeId"].is_null()); + + // Accepting a transfer must NOT enroll the other organization's + // management account as a member. + let listed = body_value( + svc.handle(req_with("111111111111", "ListAccounts", json!({}))) + .await + .unwrap(), + ); + let ids: Vec<&str> = listed["Accounts"] + .as_array() + .unwrap() + .iter() + .map(|a| a["Id"].as_str().unwrap()) + .collect(); + assert_eq!(ids, ["111111111111"]); +} + +/// Both parties can read a transfer and either can end the arrangement, +/// but only the source -- the one that named it -- can rename it. A +/// stranger gets "not found"; the target gets a clear AccessDenied on +/// the source-only operation rather than a not-found it cannot +/// distinguish from a bad id. +#[tokio::test] +async fn only_the_source_can_rename_a_responsibility_transfer() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .unwrap(); + let invite = body_value( + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222", "Type": "ACCOUNT"}, + }), + )) + .await + .unwrap(), + ); + let _ = invite; + let listed = body_value( + svc.handle(req_with( + "111111111111", + "ListOutboundResponsibilityTransfers", + json!({ "Type": "BILLING" }), + )) + .await + .unwrap(), + ); + let transfer_id = listed["ResponsibilityTransfers"][0]["Id"] + .as_str() + .unwrap() + .to_string(); + + // The target is a party, so not "not found" -- but renaming is the + // source's alone. + let err = expect_err( + svc.handle(req_with( + "222222222222", + "UpdateResponsibilityTransfer", + json!({ "Id": transfer_id, "Name": "renamed" }), + )) + .await, + ); + assert_eq!(err.code(), "AccessDeniedException"); + + // A stranger learns nothing beyond "no such transfer". + let err = expect_err( + svc.handle(req_with( + "999999999999", + "TerminateResponsibilityTransfer", + json!({ "Id": transfer_id }), )) - .await - .unwrap(); - let term_body = body_value(term); - assert_eq!(term_body["ResponsibilityTransfer"]["Status"], "WITHDRAWN"); - assert!(term_body["ResponsibilityTransfer"]["EndTimestamp"].is_number()); + .await, + ); + assert_eq!(err.code(), "ResponsibilityTransferNotFoundException"); - // Terminating again is rejected (already withdrawn). + // While the offer is still open, withdrawing it is the source's -- + // the target's answer to an open offer is DeclineHandshake. let err = expect_err( svc.handle(req_with( - "111111111111", + "222222222222", "TerminateResponsibilityTransfer", - json!({"Id": transfer_id}), + json!({ "Id": transfer_id }), )) .await, ); - assert_eq!(err.code(), "ResponsibilityTransferAlreadyInStatusException"); + assert_eq!(err.code(), "AccessDeniedException"); + + svc.handle(req_with( + "111111111111", + "TerminateResponsibilityTransfer", + json!({ "Id": transfer_id }), + )) + .await + .expect("the source withdraws its own open offer"); } #[tokio::test] @@ -1922,7 +3404,8 @@ async fn invite_responsibility_transfer_rejects_bad_type() { #[tokio::test] async fn a_mutation_that_changes_no_membership_notifies_nobody() { use std::sync::atomic::{AtomicUsize, Ordering}; - let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(None)); + let state: SharedOrganizationsState = + Arc::new(parking_lot::RwLock::new(OrganizationsRegistry::default())); let hooks = OrgChangeHooks::new(); let fired = Arc::new(AtomicUsize::new(0)); { @@ -1938,14 +3421,16 @@ async fn a_mutation_that_changes_no_membership_notifies_nobody() { hooks.fire_if_membership_changed(&state).await; assert_eq!(fired.load(Ordering::SeqCst), 0); - *state.write() = Some(OrganizationState::bootstrap("000000000000")); + state + .write() + .insert(OrganizationState::bootstrap("000000000000")); hooks.fire_if_membership_changed(&state).await; assert_eq!(fired.load(Ordering::SeqCst), 1); // A tag is not a membership change. state .write() - .as_mut() + .sole_mut() .unwrap() .set_resource_tags("000000000000", &[("Env".to_string(), "dev".to_string())]); hooks.fire_if_membership_changed(&state).await; @@ -1954,7 +3439,7 @@ async fn a_mutation_that_changes_no_membership_notifies_nobody() { // An account joining is. state .write() - .as_mut() + .sole_mut() .unwrap() .enroll_account_if_missing("111111111111"); hooks.fire_if_membership_changed(&state).await; @@ -1963,7 +3448,7 @@ async fn a_mutation_that_changes_no_membership_notifies_nobody() { // So is moving it, and so is an OU appearing for it to move into. let (root, ou) = { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); let root = org.root_id.clone(); let ou = org.create_ou(&root, "workloads").unwrap().id; (root, ou) @@ -1972,7 +3457,7 @@ async fn a_mutation_that_changes_no_membership_notifies_nobody() { assert_eq!(fired.load(Ordering::SeqCst), 3); state .write() - .as_mut() + .sole_mut() .unwrap() .move_account("111111111111", &root, &ou) .unwrap(); @@ -1983,9 +3468,9 @@ async fn a_mutation_that_changes_no_membership_notifies_nobody() { #[tokio::test] async fn a_membership_change_is_announced_even_after_a_reversal_races_it() { use std::sync::atomic::{AtomicUsize, Ordering}; - let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(Some( - OrganizationState::bootstrap("000000000000"), - ))); + let state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new( + OrganizationState::bootstrap("000000000000").into(), + )); let hooks = OrgChangeHooks::new(); let fired = Arc::new(AtomicUsize::new(0)); { @@ -1999,7 +3484,7 @@ async fn a_membership_change_is_announced_even_after_a_reversal_races_it() { } let (root, ou) = { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.enroll_account_if_missing("111111111111"); let root = org.root_id.clone(); let ou = org.create_ou(&root, "workloads").unwrap().id; @@ -2013,13 +3498,13 @@ async fn a_membership_change_is_announced_even_after_a_reversal_races_it() { // between — otherwise the next real move looks like no change. { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account("111111111111", &root, &ou).unwrap(); } hooks.fire_if_membership_changed(&state).await; { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account("111111111111", &ou, &root).unwrap(); } hooks.fire_if_membership_changed(&state).await; @@ -2029,9 +3514,402 @@ async fn a_membership_change_is_announced_even_after_a_reversal_races_it() { // The same move again is a real change and has to be announced. { let mut guard = state.write(); - let org = guard.as_mut().unwrap(); + let org = guard.sole_mut().unwrap(); org.move_account("111111111111", &root, &ou).unwrap(); } hooks.fire_if_membership_changed(&state).await; assert_eq!(fired.load(Ordering::SeqCst), after_round_trip + 1); } + +/// A delegated administrator runs a service's organization-wide +/// integration on the management account's behalf, which AWS documents +/// as including the organization's read operations. Gating them on the +/// management account alone left every delegated administrator unable +/// to read the organization it administers. +#[tokio::test] +async fn a_delegated_administrator_can_read_the_organization() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + + let reads = [ + ("ListHandshakesForOrganization", json!({})), + ("ListAWSServiceAccessForOrganization", json!({})), + ("ListDelegatedAdministrators", json!({})), + ( + "ListDelegatedServicesForAccount", + json!({ "AccountId": "222222222222" }), + ), + ]; + + // A plain member is refused. + for (action, body) in &reads { + let err = expect_err( + svc.handle(req_with("222222222222", action, body.clone())) + .await, + ); + assert_eq!(err.code(), "AccessDeniedException", "{action}"); + } + + svc.handle(req_with( + "111111111111", + "EnableAWSServiceAccess", + json!({ "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + svc.handle(req_with( + "111111111111", + "RegisterDelegatedAdministrator", + json!({ "AccountId": "222222222222", "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + + // Registered, it can run all four. + for (action, body) in &reads { + svc.handle(req_with("222222222222", action, body.clone())) + .await + .unwrap_or_else(|e| panic!("{action} refused a delegated administrator: {e:?}")); + } +} + +/// One live responsibility-transfer offer per target, the same rule +/// `InviteAccountToOrganization` enforces. Stacking OPEN transfers on +/// one target meant accepting any of them moved billing while the rest +/// stayed OPEN against an organization that no longer owned it. +#[tokio::test] +async fn a_second_open_transfer_to_the_same_target_errors() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let invite = |target: Value| { + svc.handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": target, + }), + )) + }; + + invite(json!({"Id": "222222222222", "Type": "ACCOUNT"})) + .await + .unwrap(); + let err = expect_err(invite(json!({"Id": "222222222222", "Type": "ACCOUNT"})).await); + assert_eq!(err.code(), "DuplicateHandshakeException"); + // The synthetic address names the same account, so it collides too. + let err = expect_err(invite(json!({"Id": "222222222222@example.com", "Type": "EMAIL"})).await); + assert_eq!(err.code(), "DuplicateHandshakeException"); + // A different target is unaffected. + invite(json!({"Id": "333333333333", "Type": "ACCOUNT"})) + .await + .unwrap(); +} + +/// `HandshakeResourceType` models RESPONSIBILITY_TRANSFER, TRANSFER_TYPE, +/// TRANSFER_START_TIMESTAMP and MANAGEMENT_ACCOUNT for one purpose: an +/// SDK reading only the handshake has to learn what is being handed over +/// and by whom. Rendering just ORGANIZATION and the target left the +/// invited account unable to tell a billing transfer from a plain invite +/// without a second call. +#[tokio::test] +async fn a_transfer_handshake_carries_the_transfer_as_a_resource() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let resp = svc + .handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222", "Type": "ACCOUNT"}, + }), + )) + .await + .unwrap(); + let handshake = body_json(&resp)["Handshake"].clone(); + let transfer = handshake["Resources"] + .as_array() + .unwrap() + .iter() + .find(|r| r["Type"] == "RESPONSIBILITY_TRANSFER") + .expect("the handshake reports the transfer it carries") + .clone(); + assert!(transfer["Value"].as_str().unwrap().starts_with("rt-")); + let nested: HashMap<&str, &str> = transfer["Resources"] + .as_array() + .unwrap() + .iter() + .map(|r| (r["Type"].as_str().unwrap(), r["Value"].as_str().unwrap())) + .collect(); + assert_eq!(nested.get("TRANSFER_TYPE"), Some(&"BILLING")); + assert_eq!(nested.get("MANAGEMENT_ACCOUNT"), Some(&"111111111111")); + assert_eq!( + nested.get("TRANSFER_START_TIMESTAMP"), + Some(&"2030-01-01T00:00:00.000Z") + ); + + // The link survives resolution: the transfer clears its + // `ActiveHandshakeId` on accept, so reading it from that side would + // have made an ACCEPTED handshake report no transfer at all. + let id = handshake["Id"].as_str().unwrap().to_string(); + svc.handle(req_with( + "222222222222", + "AcceptHandshake", + json!({ "HandshakeId": id }), + )) + .await + .unwrap(); + let described = svc + .handle(req_with( + "222222222222", + "DescribeHandshake", + json!({ "HandshakeId": id }), + )) + .await + .unwrap(); + assert!(body_json(&described)["Handshake"]["Resources"] + .as_array() + .unwrap() + .iter() + .any(|r| r["Type"] == "RESPONSIBILITY_TRANSFER")); +} + +/// An in-flight `CreateAccount` whose address is the synthetic form of +/// an id OTHER than the one it reserved is already doomed -- the +/// completion tick fails it with `EMAIL_ALREADY_EXISTS`. Letting it hold +/// the address meanwhile let any caller park another account's address +/// for the length of the creation delay, blocking that account's own +/// `CreateOrganization`. +#[tokio::test] +async fn a_doomed_reservation_does_not_hold_another_accounts_address() { + let (svc, _state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + svc.handle(req_with( + "111111111111", + "CreateAccount", + json!({ "Email": "222222222222@example.com", "AccountName": "squatter" }), + )) + .await + .unwrap(); + // The account that address actually names can still bootstrap while + // the doomed request is in flight. + svc.handle(req_with("222222222222", "CreateOrganization", json!({}))) + .await + .expect("a doomed reservation must not hold the address it cannot keep"); +} + +/// A delegated-administrator registration is an organization's grant to +/// one of its own members, so it cannot outlive the membership. Leaving +/// it behind meant `ListDelegatedServicesForAccount` still answered for +/// an account the organization no longer contains, and an account that +/// left and was later re-invited came back holding authority nobody had +/// granted it. +/// +/// (`ListDelegatedAdministrators` never showed the symptom: its handler +/// drops any registration whose account is not in `accounts`.) +#[tokio::test] +async fn leaving_the_organization_drops_the_delegated_administrator_grant() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + svc.handle(req_with( + "111111111111", + "EnableAWSServiceAccess", + json!({ "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + svc.handle(req_with( + "111111111111", + "RegisterDelegatedAdministrator", + json!({ "AccountId": "222222222222", "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + + svc.handle(req_with("222222222222", "LeaveOrganization", json!({}))) + .await + .unwrap(); + + // The organization no longer answers for its delegated services. + let listed = svc + .handle(req_with( + "111111111111", + "ListDelegatedServicesForAccount", + json!({ "AccountId": "222222222222" }), + )) + .await + .unwrap(); + assert_eq!( + body_json(&listed)["DelegatedServices"] + .as_array() + .unwrap() + .len(), + 0 + ); + + // And rejoining does not restore the grant. + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + let err = expect_err( + svc.handle(req_with( + "222222222222", + "ListDelegatedAdministrators", + json!({}), + )) + .await, + ); + assert_eq!(err.code(), "AccessDeniedException"); +} + +/// A `TRANSFER_RESPONSIBILITY` handshake written before the handshake +/// carried its transfer id deserializes without one, and nothing +/// backfills it. The transfer's own `ActiveHandshakeId` still points +/// back for every handshake this matters for -- it is cleared only on +/// resolution -- so the payload falls back to it rather than silently +/// dropping the resource for a restored organization. +#[tokio::test] +async fn a_restored_handshake_still_reports_its_transfer() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + let resp = svc + .handle(req_with( + "111111111111", + "InviteOrganizationToTransferResponsibility", + json!({ + "Type": "BILLING", + "SourceName": "handover", + "StartTimestamp": 1893456000.0, + "Target": {"Id": "222222222222", "Type": "ACCOUNT"}, + }), + )) + .await + .unwrap(); + let id = body_json(&resp)["Handshake"]["Id"] + .as_str() + .unwrap() + .to_string(); + + // Simulate the pre-upgrade snapshot: the link the handshake now + // stores did not exist when it was written. + state + .write() + .sole_mut() + .unwrap() + .handshakes + .get_mut(&id) + .unwrap() + .responsibility_transfer_id = None; + + let described = svc + .handle(req_with( + "222222222222", + "DescribeHandshake", + json!({ "HandshakeId": id }), + )) + .await + .unwrap(); + assert!(body_json(&described)["Handshake"]["Resources"] + .as_array() + .unwrap() + .iter() + .any(|r| r["Type"] == "RESPONSIBILITY_TRANSFER")); +} + +/// Tags are keyed by account id, so an account removed with tags still +/// attached came back wearing them if it was ever re-enrolled -- and +/// `ListTagsForResource` answered for the id meanwhile, though +/// `ListAccounts` no longer knew it. +#[tokio::test] +async fn removing_an_account_drops_the_tags_it_carried() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + svc.handle(req_with( + "111111111111", + "TagResource", + json!({ "ResourceId": "222222222222", "Tags": [{"Key": "env", "Value": "prod"}] }), + )) + .await + .unwrap(); + + svc.handle(req_with( + "111111111111", + "RemoveAccountFromOrganization", + json!({ "AccountId": "222222222222" }), + )) + .await + .unwrap(); + + let listed = svc + .handle(req_with( + "111111111111", + "ListTagsForResource", + json!({ "ResourceId": "222222222222" }), + )) + .await + .unwrap(); + assert_eq!(body_json(&listed)["Tags"].as_array().unwrap().len(), 0); +} + +/// `AccountNotRegisteredException` names the ACCOUNT, in both the arm +/// where the service has other delegates and the arm where it has none. +/// Passing the service principal into the second rendered "Account +/// config.amazonaws.com is not registered as a delegated administrator." +#[tokio::test] +async fn deregistering_an_unregistered_administrator_names_the_account() { + let (svc, state) = OrganizationsService::shared(); + create_org_with_root(&svc).await; + state + .write() + .sole_mut() + .unwrap() + .enroll_account_if_missing("222222222222"); + svc.handle(req_with( + "111111111111", + "EnableAWSServiceAccess", + json!({ "ServicePrincipal": "config.amazonaws.com" }), + )) + .await + .unwrap(); + + // No account is registered for the principal at all. + let err = expect_err( + svc.handle(req_with( + "111111111111", + "DeregisterDelegatedAdministrator", + json!({ "AccountId": "222222222222", "ServicePrincipal": "config.amazonaws.com" }), + )) + .await, + ); + assert_eq!(err.code(), "AccountNotRegisteredException"); + assert!( + err.to_string().contains("222222222222"), + "the error must name the account, got: {err}" + ); + assert!( + !err.to_string().contains("config.amazonaws.com"), + "the error must not name the service principal, got: {err}" + ); +} diff --git a/crates/fakecloud-organizations/src/state.rs b/crates/fakecloud-organizations/src/state.rs index 8ed463b41..d3ea6b35c 100644 --- a/crates/fakecloud-organizations/src/state.rs +++ b/crates/fakecloud-organizations/src/state.rs @@ -6,11 +6,423 @@ use parking_lot::RwLock; use serde::{Deserialize, Serialize}; use uuid::Uuid; -/// Shared, cross-account singleton. `None` until `CreateOrganization` -/// runs; at most one organization exists per fakecloud process. An AWS -/// org is not per-account state (it spans accounts), so this is NOT -/// wrapped in `MultiAccountState`. -pub type SharedOrganizationsState = Arc>>; +/// Shared, cross-account registry of every organization in the process. +/// An AWS org is not per-account state (it spans accounts), so this is +/// NOT wrapped in `MultiAccountState` — but it is not a singleton +/// either: any account that belongs to no organization can create its +/// own, and the organizations are fully independent of each other. +pub type SharedOrganizationsState = Arc>; + +/// Every organization in the process, keyed by organization id +/// (`o-...`). An account belongs to at most one organization, which is +/// what makes [`OrganizationsRegistry::org_of_account`] well defined and +/// lets most handlers resolve "the caller's organization" in one step. +#[derive(Clone, Debug, Default, Serialize, Deserialize)] +pub struct OrganizationsRegistry { + orgs: BTreeMap, +} + +/// A GovCloud mirror lives in the `aws-us-gov` partition. +fn is_gov_cloud(account: &MemberAccount) -> bool { + account.arn.starts_with("arn:aws-us-gov:") +} + +/// Resolve a handshake or transfer target to an account id. +/// +/// An `ACCOUNT` target already is one. An `EMAIL` target stores the +/// address itself, which is only resolvable when it follows the +/// `@example.com` form fakecloud mints for accounts it +/// creates (see `enroll_account_if_missing` and `complete_create_account`). +/// A genuinely external address names an account fakecloud has never +/// seen, so it stays unresolvable — the invitation can be read and +/// cancelled by its source, but no caller can prove it is the target. +/// +/// This is the id-only form. Prefer +/// [`OrganizationsRegistry::resolve_target_account`], which also matches +/// the address a member account was actually registered with — an +/// account created with `CreateAccount(Email = "team@corp.com")` keeps +/// that address, not a synthetic one. +pub fn target_account_id(target_kind: &str, target: &str) -> Option { + if target_kind != "EMAIL" { + return Some(target.to_string()); + } + let local = target.strip_suffix("@example.com")?; + if local.len() == 12 && local.chars().all(|c| c.is_ascii_digit()) { + Some(local.to_string()) + } else { + None + } +} + +impl From for OrganizationsRegistry { + fn from(org: OrganizationState) -> Self { + let mut registry = Self::default(); + registry.insert(org); + registry + } +} + +impl OrganizationsRegistry { + pub fn is_empty(&self) -> bool { + self.orgs.is_empty() + } + + pub fn len(&self) -> usize { + self.orgs.len() + } + + /// Every organization, ordered by id so callers that render or hash + /// the whole registry are deterministic. + pub fn iter(&self) -> impl Iterator { + self.orgs.values() + } + + pub fn contains_org(&self, org_id: &str) -> bool { + self.orgs.contains_key(org_id) + } + + pub fn org_by_id(&self, org_id: &str) -> Option<&OrganizationState> { + self.orgs.get(org_id) + } + + pub fn org_by_id_mut(&mut self, org_id: &str) -> Option<&mut OrganizationState> { + self.orgs.get_mut(org_id) + } + + /// The organization `account_id` belongs to, management account + /// included. `None` for a standalone account — which is every + /// account until it creates an organization, is invited into one and + /// accepts, or is created through `CreateAccount`. + pub fn org_of_account(&self, account_id: &str) -> Option<&OrganizationState> { + self.orgs + .values() + .find(|org| org.accounts.contains_key(account_id)) + } + + pub fn org_of_account_mut(&mut self, account_id: &str) -> Option<&mut OrganizationState> { + self.orgs + .values_mut() + .find(|org| org.accounts.contains_key(account_id)) + } + + /// The organization that owns handshake `handshake_id`. Handshakes + /// are looked up by id rather than through the caller's own + /// organization because the account answering an invitation is by + /// definition not yet a member of the inviting organization. + pub fn org_of_handshake(&self, handshake_id: &str) -> Option<&OrganizationState> { + self.orgs + .values() + .find(|org| org.handshakes.contains_key(handshake_id)) + } + + pub fn org_of_handshake_mut(&mut self, handshake_id: &str) -> Option<&mut OrganizationState> { + self.orgs + .values_mut() + .find(|org| org.handshakes.contains_key(handshake_id)) + } + + /// The organization holding `CreateAccount` request `request_id`. + /// Request ids are globally unique, so the background completion + /// tick finds its own request without carrying the org id. + pub fn org_of_create_account_request(&self, request_id: &str) -> Option<&OrganizationState> { + self.orgs + .values() + .find(|org| org.create_account_requests.contains_key(request_id)) + } + + pub fn org_of_create_account_request_mut( + &mut self, + request_id: &str, + ) -> Option<&mut OrganizationState> { + self.orgs + .values_mut() + .find(|org| org.create_account_requests.contains_key(request_id)) + } + + /// The only organization, when there is exactly one. `None` both + /// when there are none and when there are several — a caller that + /// means "the" organization has to say which once more than one + /// exists, rather than silently getting an arbitrary pick. + pub fn sole(&self) -> Option<&OrganizationState> { + match self.orgs.len() { + 1 => self.orgs.values().next(), + _ => None, + } + } + + pub fn sole_mut(&mut self) -> Option<&mut OrganizationState> { + match self.orgs.len() { + 1 => self.orgs.values_mut().next(), + _ => None, + } + } + + /// Resolve a handshake or transfer target to an account id, matching + /// an `EMAIL` target against the address the members of `within` are + /// actually registered with before falling back to the synthetic + /// form. Without the address lookup, a member created with a real + /// email is invisible to the "one organization per account" guards, + /// which could then open an invitation for an account already + /// enrolled. + /// + /// The lookup is deliberately scoped to ONE organization. Scanning + /// every organization would turn an invitation into an + /// email-to-account-id oracle: a caller could name an address, be + /// told "already a member of an organization", and read back a + /// 12-digit id belonging to an organization it has no relationship + /// with. + pub fn resolve_target_account( + &self, + target_kind: &str, + target: &str, + within: &str, + ) -> Option { + // The registered address wins over the synthetic form, as the doc + // above says: an account created with + // `CreateAccount(Email = "222222222222@example.com")` gets a random + // id, so decoding the address as if it spelled one would resolve + // to an account that does not exist and let an invitation open for + // a member already enrolled. + // Scoped to ONE organization on purpose: resolving an address + // against every organization would let the caller read a foreign + // account id back out of the "already a member" error. The + // boolean matcher may look wider, because it only ever confirms + // an account the caller already named. + if target_kind == "EMAIL" { + if let Some(account) = self.orgs.get(within).and_then(|org| { + org.accounts + .values() + .find(|a| a.email == target && !is_gov_cloud(a) && a.status != "SUSPENDED") + }) { + return Some(account.id.clone()); + } + } + target_account_id(target_kind, target) + } + + /// Mint an account id unused by ANY organization in the process, and + /// not already reserved by an in-flight `CreateAccount`. `besides` + /// excludes ids minted moments ago that are not recorded yet -- + /// `CreateGovCloudAccount` mints two in a row. + pub fn next_account_id_besides(&self, besides: &[&str]) -> String { + OrganizationState::mint_account_id(|id| { + besides.contains(&id) + || self.orgs.values().any(|org| { + org.accounts.contains_key(id) + || org.create_account_requests.values().any(|req| { + req.account_id.as_deref() == Some(id) + || req.gov_cloud_account_id.as_deref() == Some(id) + }) + }) + }) + } + + /// Mint an account id unused by ANY organization in the process. + pub fn next_account_id(&self) -> String { + self.next_account_id_besides(&[]) + } + + /// The account registered with `email`, if any organization has one. + /// Used to decide whether an address names a real account or should + /// fall back to the synthetic `@example.com` decode. + pub fn account_registered_with(&self, email: &str) -> Option { + self.orgs + .values() + .flat_map(|org| org.accounts.values()) + // A GovCloud mirror shares its commercial twin's address by + // design; the commercial account is the one an address names. + .find(|account| { + account.email == email && !is_gov_cloud(account) && account.status != "SUSPENDED" + }) + .map(|account| account.id.clone()) + } + + /// Like [`Self::email_in_use`], for the in-flight request + /// `request_id`: its own reservation must not count against it, and + /// only requests made BEFORE it do. Counting every other in-flight + /// request made whichever tick fired first fail itself, so the + /// caller that asked first was the one refused. + pub fn email_in_use_besides(&self, email: &str, request_id: &str) -> bool { + let mine = self + .orgs + .values() + .find_map(|org| org.create_account_requests.get(request_id)); + self.orgs + .values() + .flat_map(|org| org.accounts.values()) + .any(|account| account.email == email && account.status != "SUSPENDED") + || self.orgs.values().any(|org| { + org.create_account_requests.iter().any(|(id, req)| { + id != request_id + && Self::reservation_holds(req, email) + && mine.is_some_and(|m| { + (req.requested_timestamp, id.as_str()) + < (m.requested_timestamp, request_id) + }) + }) + }) + } + + /// True when `email` is the synthetic `@example.com` + /// form of an account OTHER than `for_account`. + /// + /// fakecloud mints those addresses for the accounts it creates, so + /// they are reserved for the id they spell whether or not that + /// account exists yet. Letting an unrelated account register one + /// meant two live accounts shared an address -- through + /// `CreateAccount`, or through `CreateOrganization`, whose + /// management account takes its own synthetic address -- and + /// `account_registered_with` then answered with whichever + /// organization sorted first, which decides who may accept an + /// EMAIL-targeted handshake. + pub fn email_reserved_for_other(email: &str, for_account: &str) -> bool { + target_account_id("EMAIL", email).is_some_and(|spelled| spelled != for_account) + } + + /// Does this in-flight request hold `email`? + /// + /// A request whose address is the synthetic form of an id OTHER than + /// the one it reserved is already doomed -- the completion tick + /// fails it with `EMAIL_ALREADY_EXISTS` -- so it must not hold the + /// address meanwhile. Otherwise anyone could park another account's + /// address for the length of the creation delay, blocking that + /// account's own `CreateOrganization`. + fn reservation_holds(req: &CreateAccountStatus, email: &str) -> bool { + if req.state != "IN_PROGRESS" || req.pending_email.as_deref() != Some(email) { + return false; + } + !req.account_id + .as_deref() + .is_some_and(|mine| Self::email_reserved_for_other(email, mine)) + } + + /// True when any account already uses `email`. AWS requires an + /// address to be unused (`EMAIL_ALREADY_EXISTS`), and this + /// resolution is authorization-relevant -- it decides who may accept + /// an `EMAIL`-targeted invitation -- so a duplicate would make that + /// answer depend on id ordering. + pub fn email_in_use(&self, email: &str) -> bool { + self.orgs + .values() + .flat_map(|org| org.accounts.values()) + // A closed account keeps its record but releases its address: + // `CloseAccount` (and the CloudFormation delete that calls it) + // only suspends, so counting those would make a deleted stack + // impossible to re-deploy. + .any(|account| account.email == email && account.status != "SUSPENDED") + || self.orgs.values().any(|org| { + org.create_account_requests + .values() + .any(|req| Self::reservation_holds(req, email)) + }) + } + + /// Does `target` (as declared by `target_kind`) name `account_id`? + /// + /// This is the single answer used by every party gate -- handshakes, + /// responsibility transfers, and the account's own handshake + /// listing. Two matchers that disagreed let an account accept an + /// invitation it could then neither read nor act on. + /// + /// An `ACCOUNT` target names the id directly. An `EMAIL` target + /// resolves to the account REGISTERED with that address if any + /// organization has one, and otherwise decodes the synthetic + /// `@example.com` form fakecloud mints. That precedence + /// is what keeps one address naming one account: accepting both + /// readings let an invitation be accepted by an account it was never + /// addressed to. The consequence is that registering an address + /// elsewhere takes over its synthetic reading, so an invitation open + /// to the spelled-out id stops matching -- rare, and the safe + /// direction to fail. + /// + /// The registered lookup spans organizations, which is safe here + /// because this only ever CONFIRMS an account the caller already + /// named; it never hands one back. Resolvers that return an id -- + /// `resolve_target_account` -- stay scoped to one organization so + /// they cannot be read as an oracle. + pub fn account_matches_target( + &self, + target_kind: &str, + target: &str, + account_id: &str, + ) -> bool { + if target_kind == "EMAIL" { + // A registered address names its own account and nothing else. + // Allowing the synthetic decode as well let one address name + // two accounts, so an account other than the intended target + // could read and accept the invitation. + if let Some(registered) = self.account_registered_with(target) { + return registered == account_id; + } + } + target_account_id(target_kind, target).as_deref() == Some(account_id) + } + + /// The organization that stores responsibility transfer `id`. A + /// transfer is recorded once, in the source organization, but both + /// management accounts are parties to it. + pub fn org_of_responsibility_transfer(&self, id: &str) -> Option<&OrganizationState> { + self.orgs + .values() + .find(|org| org.responsibility_transfers.contains_key(id)) + } + + /// Insert an organization, keyed by its own id. + pub fn insert(&mut self, org: OrganizationState) { + self.orgs.insert(org.org_id.clone(), org); + } + + pub fn remove(&mut self, org_id: &str) -> Option { + self.orgs.remove(org_id) + } + + pub fn clear(&mut self) { + self.orgs.clear(); + } + + /// True when `account_id` is already claimed by some organization -- + /// enrolled in one, or RESERVED by an in-flight `CreateAccount` that + /// has not finished enrolling it yet. + /// + /// Guards both `CreateOrganization` and the invite/accept path: an + /// account can never be in two organizations at once. Ignoring the + /// reservation let an account created by one organization create its + /// own during the completion delay, after which the background tick + /// enrolled it and it was in two. + pub fn account_is_enrolled(&self, account_id: &str) -> bool { + self.org_of_account(account_id).is_some() || self.account_is_reserved(account_id) + } + + /// The organization that already claims `account_id`, if it is one + /// other than `org_id`. Reservation-aware, so an id an in-flight + /// `CreateAccount` is about to enroll counts as claimed. + pub fn claimed_by_other_org(&self, account_id: &str, org_id: &str) -> Option { + if let Some(org) = self.org_of_account(account_id) { + return (org.org_id != org_id).then(|| org.org_id.clone()); + } + self.orgs + .values() + .find(|org| { + org.org_id != org_id + && org.create_account_requests.values().any(|req| { + req.state == "IN_PROGRESS" + && (req.account_id.as_deref() == Some(account_id) + || req.gov_cloud_account_id.as_deref() == Some(account_id)) + }) + }) + .map(|org| org.org_id.clone()) + } + + fn account_is_reserved(&self, account_id: &str) -> bool { + self.orgs.values().any(|org| { + org.create_account_requests.values().any(|req| { + req.state == "IN_PROGRESS" + && (req.account_id.as_deref() == Some(account_id) + || req.gov_cloud_account_id.as_deref() == Some(account_id)) + }) + }) + } +} pub const FEATURE_SET_ALL: &str = "ALL"; pub const FEATURE_SET_CONSOLIDATED_BILLING: &str = "CONSOLIDATED_BILLING"; @@ -26,16 +438,35 @@ pub const FULL_AWS_ACCESS_POLICY_CONTENT: &str = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}"#; /// On-disk snapshot envelope for Organizations state. Versioned so format -/// changes fail loudly on upgrade rather than silently mis-parsing. The whole -/// org is a single optional value (`None` until `CreateOrganization`). +/// changes fail loudly on upgrade rather than silently mis-parsing. +/// +/// v1 held a single optional organization; v2 holds the whole registry. +/// The v1 field is still read so an existing snapshot keeps loading — it +/// folds into the registry as one organization — but is never written +/// again. #[derive(Clone, Serialize, Deserialize)] pub struct OrganizationsSnapshot { pub schema_version: u32, - #[serde(default)] + /// v1 only. Retained for reading old snapshots; `None` in v2. + #[serde(default, skip_serializing_if = "Option::is_none")] pub organization: Option, + #[serde(default)] + pub organizations: OrganizationsRegistry, +} + +impl OrganizationsSnapshot { + /// The registry this snapshot describes, folding a v1 single + /// organization into the v2 shape. + pub fn into_registry(self) -> OrganizationsRegistry { + let mut registry = self.organizations; + if let Some(org) = self.organization { + registry.insert(org); + } + registry + } } -pub const ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION: u32 = 1; +pub const ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION: u32 = 2; #[derive(Clone, Debug, Serialize, Deserialize)] pub struct OrganizationState { @@ -111,7 +542,11 @@ impl OrganizationState { pub fn bootstrap(management_account_id: &str) -> Self { let now = Utc::now(); let org_id = format!("o-{}", random_id(10)); - let root_id = format!("r-{}", random_id(4)); + // AWS root ids are 4-32 chars. Four was fine while only one + // organization could exist; across a registry it collides at + // 1/65536, which would let one organization's root id validate as + // a target in another. + let root_id = format!("r-{}", random_id(12)); let org_arn = format!( "arn:aws:organizations::{}:organization/{}", management_account_id, org_id @@ -302,10 +737,17 @@ impl OrganizationState { out } - /// Allocate the next pseudo-random 12-digit account id that's not - /// already a member. Mirrors AWS's account-id format (numeric, - /// 12 digits, no leading zero stripping). + /// Mint an account id unused by this organization. + /// + /// Prefer [`OrganizationsRegistry::next_account_id`], which checks + /// every organization: an account id names one account process-wide, + /// and a collision across organizations would break the "at most one + /// organization per account" invariant. pub fn next_account_id(&self) -> String { + Self::mint_account_id(|id| self.accounts.contains_key(id)) + } + + fn mint_account_id(taken: impl Fn(&str) -> bool) -> String { loop { let mut id = String::with_capacity(12); for _ in 0..12 { @@ -313,7 +755,7 @@ impl OrganizationState { let byte = u.as_bytes()[0]; id.push(((byte % 10) + b'0') as char); } - if !id.starts_with('0') && !self.accounts.contains_key(&id) { + if !id.starts_with('0') && !taken(&id) { return id; } } @@ -330,15 +772,19 @@ impl OrganizationState { /// The caller is expected to spawn a background task that calls /// `complete_create_account(request_id)` after a synthetic delay so /// pollers can observe the IN_PROGRESS -> SUCCEEDED transition. + /// `account_id` is minted by the caller, which holds the registry and + /// can therefore guarantee the id is unused process-wide rather than + /// only within this organization. pub fn begin_create_account( &mut self, email: &str, name: &str, + account_id: String, gov_cloud_paired_id: Option, ) -> CreateAccountStatus { let now = Utc::now(); let request_id = format!("car-{}", random_id(20)); - let new_account_id = self.next_account_id(); + let new_account_id = account_id; let status = CreateAccountStatus { id: request_id.clone(), account_id: Some(new_account_id), @@ -402,6 +848,11 @@ impl OrganizationState { "arn:aws-us-gov:organizations::{}:account/{}/{}", self.management_account_id, self.org_id, gov_id ); + // AWS creates the GovCloud account from the same owner + // address, so the response carries it. The pair is the one + // legitimate case of two accounts sharing an address; + // `account_registered_with` skips the mirror so resolution + // still lands on exactly one account. self.accounts.insert( gov_id.clone(), MemberAccount { @@ -440,7 +891,19 @@ impl OrganizationState { status.completed_timestamp = Some(Utc::now()); status.failure_reason = Some(reason.to_string()); status.pending_email = None; - Some(status.clone()) + let snapshot = status.clone(); + // `CreateAccount` applies create-time tags to the reserved id + // straight away, on the old assumption that every request ends in + // SUCCEEDED. A failed request's id never becomes an account, so + // those tags would otherwise linger on an id `ListAccounts` does + // not know and AWS answers for with `TargetNotFoundException`. + if let Some(account_id) = &snapshot.account_id { + self.resource_tags.remove(account_id); + } + if let Some(gov_id) = &snapshot.gov_cloud_account_id { + self.resource_tags.remove(gov_id); + } + Some(snapshot) } /// Issue a new pending invitation handshake to `target_account_id`. @@ -449,19 +912,52 @@ impl OrganizationState { pub fn invite_account( &mut self, source_account_id: &str, + target_kind: &str, target_account_id: &str, target_email: Option, notes: Option, ) -> Result { - if self.accounts.contains_key(target_account_id) { - return Err(OrgError::AccountAlreadyMember( - target_account_id.to_string(), - )); + // Compare against the resolved account id: an EMAIL target records + // the address, which is never a key in `accounts` and never equal + // to another handshake's ACCOUNT-form target. Without this, an + // account already enrolled could be re-invited by email, and one + // account could hold two live invitations under its two spellings. + // The kind is the caller's declared `Target.Type`, so this agrees + // with the cross-organization guard in the service layer rather + // than re-deriving a different answer from the string's shape. + // Same order as `OrganizationsRegistry::resolve_target_account`: + // a registered address names its own account, and the synthetic + // decode is only a fallback. The reverse order resolved to an + // account nobody owns, so the already-a-member and + // duplicate-handshake guards below both missed. + let resolved = self + .accounts + .values() + .find(|account| { + target_kind == "EMAIL" + && account.email == target_account_id + && !is_gov_cloud(account) + && account.status != "SUSPENDED" + }) + .map(|account| account.id.clone()) + .or_else(|| self::target_account_id(target_kind, target_account_id)); + if let Some(target) = &resolved { + if self.accounts.contains_key(target) { + return Err(OrgError::AccountAlreadyMember(target.clone())); + } } for h in self.handshakes.values() { - if h.target_account_id == target_account_id - && matches!(h.state.as_str(), "REQUESTED" | "OPEN") - { + // Only another membership INVITE collides. A + // TRANSFER_RESPONSIBILITY handshake to the same account is a + // different arrangement entirely, and AWS keeps the two + // handshake actions independent. + if h.action != "INVITE" { + continue; + } + let same_target = h.target_account_id == target_account_id + || (resolved.is_some() + && self::target_account_id(&h.target_kind, &h.target_account_id) == resolved); + if same_target && matches!(h.state.as_str(), "REQUESTED" | "OPEN") { return Err(OrgError::DuplicateHandshakeForAccount( target_account_id.to_string(), )); @@ -473,11 +969,7 @@ impl OrganizationState { "arn:aws:organizations::{}:handshake/{}/invite/{}", self.management_account_id, self.org_id, id ); - let kind = if target_account_id.chars().all(|c| c.is_ascii_digit()) { - "ACCOUNT".to_string() - } else { - "EMAIL".to_string() - }; + let kind = target_kind.to_string(); let handshake = Handshake { id: id.clone(), arn, @@ -491,6 +983,8 @@ impl OrganizationState { target_kind: kind, notes, organization_id: self.org_id.clone(), + // An INVITE carries no responsibility transfer. + responsibility_transfer_id: None, }; self.handshakes.insert(id, handshake.clone()); Ok(handshake) @@ -501,7 +995,19 @@ impl OrganizationState { /// this just enforces lifecycle (open -> terminal). The original /// `ExpirationTimestamp` is preserved (it's the 15-day deadline, /// not a resolved-at marker). - pub fn resolve_handshake(&mut self, id: &str, new_state: &str) -> Result { + /// `enrolling_account` is the account the caller's party gate already + /// proved to be the target. Re-deriving it here from the stored + /// target -- which the gate resolves registry-aware and this could + /// only resolve id-only -- let the two disagree: an accept could + /// enroll a phantom account nobody created, or report ACCEPTED while + /// enrolling nobody. + pub fn resolve_handshake( + &mut self, + id: &str, + new_state: &str, + enrolling_account: Option<&str>, + enrolling_email: Option, + ) -> Result { let handshake = self .handshakes .get_mut(id) @@ -514,23 +1020,40 @@ impl OrganizationState { } handshake.state = new_state.to_string(); let snapshot = handshake.clone(); - if new_state == "ACCEPTED" && !self.accounts.contains_key(&snapshot.target_account_id) { + // Only an INVITE enrolls the target. A TRANSFER_RESPONSIBILITY + // handshake targets the management account of ANOTHER organization, + // so enrolling on accept would put that account in two + // organizations at once. + // An EMAIL target records the address, so resolve it to the account + // it names — enrolling the raw field would key a member account by + // an email string. + let enrolling = if new_state == "ACCEPTED" && snapshot.action == "INVITE" { + enrolling_account + .map(str::to_string) + .filter(|target| !self.accounts.contains_key(target)) + } else { + None + }; + if let Some(target) = enrolling { let now = Utc::now(); let arn = format!( "arn:aws:organizations::{}:account/{}/{}", - self.management_account_id, self.org_id, snapshot.target_account_id + self.management_account_id, self.org_id, target ); - let email = snapshot - .target_email + // The caller supplies the address: an account's address must + // be unique across the whole registry, which this organization + // cannot see on its own. + let email = enrolling_email .clone() - .unwrap_or_else(|| format!("{}@example.com", snapshot.target_account_id)); + .or_else(|| snapshot.target_email.clone()) + .unwrap_or_else(|| format!("{target}@example.com")); self.accounts.insert( - snapshot.target_account_id.clone(), + target.clone(), MemberAccount { - id: snapshot.target_account_id.clone(), + id: target.clone(), arn, email, - name: format!("Account {}", snapshot.target_account_id), + name: format!("Account {target}"), status: "ACTIVE".to_string(), joined_method: "INVITED".to_string(), joined_timestamp: now, @@ -538,20 +1061,37 @@ impl OrganizationState { }, ); } + // A TRANSFER_RESPONSIBILITY handshake carries a responsibility + // transfer. Resolving the handshake without moving the transfer + // left two sources of truth disagreeing: the handshake ACCEPTED, + // the transfer still REQUESTED with a live handshake id. + if snapshot.action == "TRANSFER_RESPONSIBILITY" { + if let Some(transfer) = self + .responsibility_transfers + .values_mut() + .find(|t| t.active_handshake_id.as_deref() == Some(id)) + { + transfer.status = new_state.to_string(); + transfer.active_handshake_id = None; + // An ACCEPTED transfer is starting, not ending -- stamping + // an end time here reported it as simultaneously active and + // already over. + if matches!(new_state, "DECLINED" | "CANCELED" | "EXPIRED") { + transfer.end_timestamp = Some(Utc::now()); + } + } + } Ok(snapshot) } - /// Return a clone of every handshake currently tracked for the org, - /// optionally filtered by destination account id. - pub fn list_handshakes(&self, only_target_account: Option<&str>) -> Vec { - self.handshakes - .values() - .filter(|h| match only_target_account { - Some(acct) => h.target_account_id == acct, - None => true, - }) - .cloned() - .collect() + /// Every handshake this organization holds. + /// + /// Filtering by target is deliberately NOT offered here: deciding + /// whether a target names an account needs the registry, so callers + /// filter with [`OrganizationsRegistry::account_matches_target`]. An + /// id-only filter here would silently encode a different rule. + pub fn list_handshakes(&self) -> Vec { + self.handshakes.values().cloned().collect() } /// Mark `service_principal` as a trusted service. Idempotent: the @@ -631,13 +1171,16 @@ impl OrganizationState { account_id: &str, service_principal: &str, ) -> Result<(), OrgError> { - let entry = self + // Both arms name the ACCOUNT, which is what the error says and + // what AWS reports. Naming the service principal when no + // account is registered for it at all rendered "Account + // config.amazonaws.com is not registered as a delegated + // administrator." + let registered = self .delegated_administrators .get_mut(service_principal) - .ok_or_else(|| { - OrgError::DelegatedAdministratorNotRegistered(service_principal.to_string()) - })?; - if entry.remove(account_id).is_none() { + .is_some_and(|entry| entry.remove(account_id).is_some()); + if !registered { return Err(OrgError::DelegatedAdministratorNotRegistered( account_id.to_string(), )); @@ -645,6 +1188,13 @@ impl OrganizationState { Ok(()) } + /// Is `account_id` a delegated administrator for any service? + pub fn is_delegated_administrator(&self, account_id: &str) -> bool { + self.delegated_administrators + .values() + .any(|admins| admins.contains_key(account_id)) + } + /// List delegated administrators, optionally filtered by service. pub fn list_delegated_administrators( &self, @@ -715,6 +1265,24 @@ impl OrganizationState { } // Detach any direct policy attachments for the now-orphan id. self.attachments.remove(account_id); + // Tags are keyed by account id, so an untagged id would come back + // wearing them if the account is ever re-enrolled. `ListAccounts` + // does not know the id meanwhile, which is the same reason + // `fail_create_account` drops the tags it reserved. + self.resource_tags.remove(account_id); + // And drop every delegated-administrator registration it held. + // The registration is an organization's grant to one of its OWN + // members, so it cannot outlive the membership: leaving it behind + // meant `ListDelegatedServicesForAccount` still answered for an + // account the organization no longer contains, and -- now that + // the registration unlocks the organization's read operations -- + // an account that left and was later re-invited came back holding + // delegated-administrator authority nobody had granted it. + for admins in self.delegated_administrators.values_mut() { + admins.remove(account_id); + } + self.delegated_administrators + .retain(|_, admins| !admins.is_empty()); Ok(()) } @@ -1118,6 +1686,10 @@ pub enum OrgError { InvalidHandshakeParty(String), DuplicateHandshakeForAccount(String), AccountAlreadyMember(String), + /// The account belongs to a DIFFERENT organization. An account can + /// be in at most one organization, so it must leave (or be removed + /// from) that one before another can invite it. + AccountInAnotherOrganization(String), AWSServiceAccessNotEnabled(String), DelegatedAdministratorAlreadyRegistered(String), DelegatedAdministratorNotRegistered(String), @@ -1189,6 +1761,15 @@ pub struct Handshake { pub target_kind: String, pub notes: Option, pub organization_id: String, + /// The responsibility transfer this handshake carries, for + /// `TRANSFER_RESPONSIBILITY` handshakes only. + /// + /// The link has to live on the handshake because the transfer's own + /// `active_handshake_id` is cleared the moment the handshake + /// resolves -- reading the link from that side made an ACCEPTED + /// handshake report no transfer at all. + #[serde(default)] + pub responsibility_transfer_id: Option, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -1331,6 +1912,64 @@ mod tests { } } + /// A v1 snapshot holds a single organization under `organization`. + /// It must fold into the registry on load — silently producing an + /// empty registry would drop the user's entire organization on the + /// first restart after upgrading. + #[test] + fn a_v1_snapshot_folds_into_the_registry() { + let raw = serde_json::json!({ + "schema_version": 1, + "organization": OrganizationState::bootstrap("111111111111"), + }); + let snapshot: OrganizationsSnapshot = serde_json::from_value(raw).unwrap(); + assert_eq!(snapshot.schema_version, 1); + let registry = snapshot.into_registry(); + assert_eq!(registry.len(), 1); + assert!(registry.org_of_account("111111111111").is_some()); + } + + /// A v2 snapshot round-trips every organization, and the legacy + /// single-organization field is no longer written. + #[test] + fn a_v2_snapshot_round_trips_every_organization() { + let mut registry = OrganizationsRegistry::default(); + registry.insert(OrganizationState::bootstrap("111111111111")); + registry.insert(OrganizationState::bootstrap("222222222222")); + + let snapshot = OrganizationsSnapshot { + schema_version: ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, + organization: None, + organizations: registry, + }; + let bytes = serde_json::to_vec(&snapshot).unwrap(); + assert!( + !String::from_utf8_lossy(&bytes).contains("\"organization\":"), + "v2 must not write the legacy single-organization field" + ); + + let loaded: OrganizationsSnapshot = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(loaded.schema_version, 2); + let restored = loaded.into_registry(); + assert_eq!(restored.len(), 2); + assert!(restored.org_of_account("111111111111").is_some()); + assert!(restored.org_of_account("222222222222").is_some()); + } + + /// An empty registry survives a round trip as an empty registry, not + /// as a missing field that fails to parse. + #[test] + fn an_empty_registry_round_trips() { + let snapshot = OrganizationsSnapshot { + schema_version: ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, + organization: None, + organizations: OrganizationsRegistry::default(), + }; + let bytes = serde_json::to_vec(&snapshot).unwrap(); + let loaded: OrganizationsSnapshot = serde_json::from_slice(&bytes).unwrap(); + assert!(loaded.into_registry().is_empty()); + } + #[test] fn enroll_account_if_missing_adds_to_root() { let mut org = OrganizationState::bootstrap("111111111111"); @@ -1875,7 +2514,7 @@ mod tests { fn invite_account_creates_open_handshake() { let mut org = OrganizationState::bootstrap("111111111111"); let h = org - .invite_account("111111111111", "222222222222", None, None) + .invite_account("111111111111", "ACCOUNT", "222222222222", None, None) .unwrap(); assert_eq!(h.state, "OPEN"); assert!(h.id.starts_with("h-")); @@ -1886,7 +2525,7 @@ mod tests { fn invite_rejects_existing_member() { let mut org = OrganizationState::bootstrap("111111111111"); let err = org - .invite_account("111111111111", "111111111111", None, None) + .invite_account("111111111111", "ACCOUNT", "111111111111", None, None) .unwrap_err(); assert!(matches!(err, OrgError::AccountAlreadyMember(_))); } @@ -1894,10 +2533,10 @@ mod tests { #[test] fn duplicate_open_invite_rejected() { let mut org = OrganizationState::bootstrap("111111111111"); - org.invite_account("111111111111", "333333333333", None, None) + org.invite_account("111111111111", "ACCOUNT", "333333333333", None, None) .unwrap(); let err = org - .invite_account("111111111111", "333333333333", None, None) + .invite_account("111111111111", "ACCOUNT", "333333333333", None, None) .unwrap_err(); assert!(matches!(err, OrgError::DuplicateHandshakeForAccount(_))); } @@ -1906,10 +2545,12 @@ mod tests { fn accept_handshake_enrolls_account() { let mut org = OrganizationState::bootstrap("111111111111"); let h = org - .invite_account("111111111111", "444444444444", None, None) + .invite_account("111111111111", "ACCOUNT", "444444444444", None, None) .unwrap(); assert!(!org.accounts.contains_key("444444444444")); - let resolved = org.resolve_handshake(&h.id, "ACCEPTED").unwrap(); + let resolved = org + .resolve_handshake(&h.id, "ACCEPTED", Some("444444444444"), None) + .unwrap(); assert_eq!(resolved.state, "ACCEPTED"); let acct = org.accounts.get("444444444444").unwrap(); assert_eq!(acct.joined_method, "INVITED"); @@ -1919,9 +2560,11 @@ mod tests { fn decline_handshake_does_not_enroll() { let mut org = OrganizationState::bootstrap("111111111111"); let h = org - .invite_account("111111111111", "555555555555", None, None) + .invite_account("111111111111", "ACCOUNT", "555555555555", None, None) + .unwrap(); + let resolved = org + .resolve_handshake(&h.id, "DECLINED", None, None) .unwrap(); - let resolved = org.resolve_handshake(&h.id, "DECLINED").unwrap(); assert_eq!(resolved.state, "DECLINED"); assert!(!org.accounts.contains_key("555555555555")); } @@ -1930,10 +2573,13 @@ mod tests { fn resolve_handshake_terminal_locked() { let mut org = OrganizationState::bootstrap("111111111111"); let h = org - .invite_account("111111111111", "666666666666", None, None) + .invite_account("111111111111", "ACCOUNT", "666666666666", None, None) + .unwrap(); + org.resolve_handshake(&h.id, "ACCEPTED", Some("666666666666"), None) .unwrap(); - org.resolve_handshake(&h.id, "ACCEPTED").unwrap(); - let err = org.resolve_handshake(&h.id, "DECLINED").unwrap_err(); + let err = org + .resolve_handshake(&h.id, "DECLINED", None, None) + .unwrap_err(); assert!(matches!(err, OrgError::HandshakeAlreadyResolved(_))); } } diff --git a/crates/fakecloud-sdk/src/types.rs b/crates/fakecloud-sdk/src/types.rs index 8f0f5961d..0ea2342a2 100644 --- a/crates/fakecloud-sdk/src/types.rs +++ b/crates/fakecloud-sdk/src/types.rs @@ -2117,6 +2117,11 @@ pub struct OrganizationsAccount { /// Tags directly attached to the account (alphabetical by key). #[serde(default)] pub tags: Vec, + /// The organization this account belongs to. Present so the + /// flattened list stays readable once more than one organization + /// exists in the process. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub organization_id: Option, /// SCP ids directly attached to the account (alphabetical). /// Does not include policies inherited from parent OUs or root. #[serde(default)] @@ -2154,11 +2159,35 @@ pub struct OrganizationsTag { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct OrganizationsAccountsResponse { + /// Every member account across every organization, each carrying its + /// own `organizationId`. pub accounts: Vec, + /// The single organization's management account. Set only when + /// exactly one organization exists, so a caller written against the + /// one-org shape keeps working; read `organizations` otherwise. #[serde(skip_serializing_if = "Option::is_none")] pub management_account_id: Option, + /// Legacy alias of `management_account_id`, matching AWS's own + /// deprecated `MasterAccountId`. #[serde(skip_serializing_if = "Option::is_none")] pub master_account_id: Option, + /// One entry per organization in the process, ordered by id. + #[serde(default)] + pub organizations: Vec, +} + +/// One organization in the process, as exposed by +/// `GET /_fakecloud/organizations/accounts`. Organizations are fully +/// independent: each has its own management account, root and SCPs. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct OrganizationsSummary { + pub organization_id: String, + pub arn: String, + pub management_account_id: String, + pub root_id: String, + /// `ALL` or `CONSOLIDATED_BILLING`. + pub feature_set: String, } /// One billing-responsibility transfer as exposed by @@ -2169,13 +2198,20 @@ pub struct OrganizationsAccountsResponse { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct OrganizationsResponsibilityTransfer { + /// The organization holding the transfer. The listing spans every + /// organization in the process. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub organization_id: Option, pub id: String, pub arn: String, pub name: String, #[serde(rename = "type")] pub transfer_type: String, pub status: String, - /// INBOUND / OUTBOUND. + /// `INBOUND` / `OUTBOUND`, relative to `organization_id`. A transfer + /// is recorded once, in the source organization, so every row reads + /// `OUTBOUND`; the target reads the same transfer as inbound through + /// `ListInboundResponsibilityTransfers`. pub direction: String, pub source_management_account_id: String, pub source_management_account_email: String, diff --git a/crates/fakecloud-server/src/introspection.rs b/crates/fakecloud-server/src/introspection.rs index 142e64a6a..3ebc0419c 100644 --- a/crates/fakecloud-server/src/introspection.rs +++ b/crates/fakecloud-server/src/introspection.rs @@ -486,18 +486,11 @@ pub(crate) fn organizations_accounts_snapshot( state: &fakecloud_organizations::SharedOrganizationsState, ) -> types::OrganizationsAccountsResponse { let guard = state.read(); - let Some(org) = guard.as_ref() else { - return types::OrganizationsAccountsResponse { - accounts: Vec::new(), - management_account_id: None, - master_account_id: None, - }; - }; - let mut accounts: Vec = org - .accounts - .values() - .map(|a| { + let mut accounts: Vec = guard + .iter() + .flat_map(|org| org.accounts.values().map(move |a| (org, a))) + .map(|(org, a)| { let mut tags: Vec = org .resource_tags .get(&a.id) @@ -538,6 +531,7 @@ pub(crate) fn organizations_accounts_snapshot( joined_method: a.joined_method.clone(), joined_timestamp: a.joined_timestamp.to_rfc3339(), parent_ou_id: Some(a.parent_id.clone()), + organization_id: Some(org.org_id.clone()), tags, scp_attached, } @@ -545,10 +539,35 @@ pub(crate) fn organizations_accounts_snapshot( .collect(); accounts.sort_by(|x, y| x.id.cmp(&y.id)); + let organizations: Vec = guard + .iter() + .map(|org| types::OrganizationsSummary { + organization_id: org.org_id.clone(), + arn: org.org_arn.clone(), + management_account_id: org.management_account_id.clone(), + root_id: org.root_id.clone(), + feature_set: org.feature_set.clone(), + }) + .collect(); + + // The flat management-account fields only make sense when there is a + // single organization to point at; with several, a caller has to read + // `organizations` (or each account's `organizationId`) instead of + // silently getting one arbitrary organization's answer. + // + // That overloads `null`, which used to mean "no organization exists". + // `organizations` is the unambiguous signal -- empty means none -- + // and the docs say so. + let single = match organizations.as_slice() { + [only] => Some(only.management_account_id.clone()), + _ => None, + }; + types::OrganizationsAccountsResponse { accounts, - management_account_id: Some(org.management_account_id.clone()), - master_account_id: Some(org.management_account_id.clone()), + management_account_id: single.clone(), + master_account_id: single, + organizations, } } @@ -830,8 +849,9 @@ mod tests { #[test] fn organizations_accounts_snapshot_empty_when_no_org() { use std::sync::Arc; - let state: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(None)); + let state: fakecloud_organizations::SharedOrganizationsState = Arc::new( + parking_lot::RwLock::new(fakecloud_organizations::OrganizationsRegistry::default()), + ); let snap = super::organizations_accounts_snapshot(&state); assert!(snap.accounts.is_empty()); assert!(snap.management_account_id.is_none()); @@ -876,7 +896,7 @@ mod tests { .expect("attach to member"); let state: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(Some(org))); + Arc::new(parking_lot::RwLock::new(org.into())); let snap = super::organizations_accounts_snapshot(&state); assert_eq!(snap.management_account_id.as_deref(), Some("111111111111")); diff --git a/crates/fakecloud-server/src/main.rs b/crates/fakecloud-server/src/main.rs index 486e45494..d299e6549 100644 --- a/crates/fakecloud-server/src/main.rs +++ b/crates/fakecloud-server/src/main.rs @@ -589,10 +589,14 @@ async fn main() { let bedrock_agent_runtime_state: fakecloud_bedrock_agent_runtime::SharedBedrockAgentRuntimeState = Arc::new( parking_lot::RwLock::new(fakecloud_bedrock_agent_runtime::BedrockAgentRuntimeAccounts::new()), ); - // Organizations state is a global singleton (one org per fakecloud - // process) — not wrapped in MultiAccountState because an AWS org is - // a cross-account construct. `None` until CreateOrganization runs. - let organizations_state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new(None)); + // Organizations state is a process-wide registry of independent + // organizations — not wrapped in MultiAccountState because an AWS org + // is a cross-account construct, but not a singleton either: every + // account outside an organization can create one of its own. Empty + // until the first CreateOrganization runs. + let organizations_state: SharedOrganizationsState = Arc::new(parking_lot::RwLock::new( + fakecloud_organizations::OrganizationsRegistry::default(), + )); let scheduler_state: fakecloud_scheduler::SharedSchedulerState = Arc::new( parking_lot::RwLock::new(fakecloud_core::multi_account::MultiAccountState::new( &cli.account_id, @@ -2356,10 +2360,15 @@ async fn main() { fakecloud_organizations::ORGANIZATIONS_SNAPSHOT_SCHEMA_VERSION, )); } - let present = snapshot.organization.is_some(); - *organizations_state.write() = snapshot.organization; + // v1 snapshots hold a single organization; v2 + // holds the registry. `into_registry` folds the + // former into the latter, so an existing + // snapshot keeps loading across the upgrade. + let registry = snapshot.into_registry(); + let organizations = registry.len(); + *organizations_state.write() = registry; tracing::info!( - organization = present, + organizations, "loaded organizations persistence snapshot" ); } @@ -11650,10 +11659,16 @@ async fn main() { let persist = persist.clone(); let changed = changed.clone(); async move { - let was_member = orgs - .read() - .as_ref() - .is_some_and(|org| org.accounts.contains_key(&body.account_id)); + // Membership of the NAMED organization, not of any + // organization: an account already in a different one + // is rejected below, and gating on registry-wide + // membership would skip the persist/notify for an + // enrollment that did happen. + let was_member = body.organization_id.as_deref().is_some_and(|org_id| { + orgs.read() + .org_by_id(org_id) + .is_some_and(|org| org.accounts.contains_key(&body.account_id)) + }); let resp = match reset::create_admin_in_account( &iam, &orgs, @@ -11720,6 +11735,7 @@ async fn main() { let responsibility_transfers = rows .into_iter() .map(|r| types::OrganizationsResponsibilityTransfer { + organization_id: Some(r.organization_id), id: r.id, arn: r.arn, name: r.name, diff --git a/crates/fakecloud-server/src/reset.rs b/crates/fakecloud-server/src/reset.rs index 14d25bc54..2c8fc4766 100644 --- a/crates/fakecloud-server/src/reset.rs +++ b/crates/fakecloud-server/src/reset.rs @@ -217,7 +217,7 @@ impl ResetState { *self.athena.write() = fakecloud_athena::AthenaAccounts::new(); } "organizations" => { - *self.organizations.write() = None; + self.organizations.write().clear(); } _ => { return Err(format!("Unknown service: {service}")); @@ -531,10 +531,10 @@ impl ResetState { fakecloud_application_autoscaling::ApplicationAutoScalingAccounts::new(); *self.wafv2.write() = fakecloud_wafv2::Wafv2Accounts::new(); *self.athena.write() = fakecloud_athena::AthenaAccounts::new(); - // Organizations is a cross-account singleton (not MultiAccountState); - // a full reset drops the org entirely so subsequent runs start - // with no org, matching the no-in-use default state. - *self.organizations.write() = None; + // Organizations is a cross-account registry (not MultiAccountState); + // a full reset drops every organization so subsequent runs start + // with none, matching the no-in-use default state. + self.organizations.write().clear(); tracing::info!("state reset via reset API"); axum::Json(types::ResetResponse { status: "ok".to_string(), @@ -568,10 +568,24 @@ pub(crate) fn create_admin_in_account( ) -> Result { if let Some(org_id) = organization_id { let mut guard = organizations.write(); + if !guard.contains_org(org_id) { + return Err(CreateAdminError::UnknownOrganization(org_id.to_string())); + } + // An account belongs to at most one organization. Without this the + // shortcut would enroll it into a second registry entry, and which + // organization's SCP ceiling, DescribeOrganization view and + // stack-set targeting applied would come down to org-id sort order. + // `CreateOrganization` and `InviteAccountToOrganization` both reject + // this; so does the shortcut. + if let Some(other) = guard.claimed_by_other_org(account_id, org_id) { + return Err(CreateAdminError::AccountInAnotherOrganization { + account_id: account_id.to_string(), + organization_id: other, + }); + } let org = guard - .as_mut() - .filter(|org| org.org_id == org_id) - .ok_or_else(|| CreateAdminError::UnknownOrganization(org_id.to_string()))?; + .org_by_id_mut(org_id) + .expect("checked just above that the organization exists"); org.enroll_account_if_missing(account_id); } @@ -641,6 +655,12 @@ pub(crate) enum CreateAdminError { /// the caller is explicitly avoiding by naming one, so this is an /// error rather than a silent fallback. UnknownOrganization(String), + /// The account is already a member of a different organization, and + /// an account can only ever be in one. + AccountInAnotherOrganization { + account_id: String, + organization_id: String, + }, } impl CreateAdminError { @@ -649,6 +669,12 @@ impl CreateAdminError { Self::UnknownOrganization(id) => { format!("no organization with id {id} exists") } + Self::AccountInAnotherOrganization { + account_id, + organization_id, + } => format!( + "account {account_id} is already a member of organization {organization_id}" + ), } } } @@ -941,7 +967,9 @@ mod tests { athena: Arc::new(parking_lot::RwLock::new( fakecloud_athena::AthenaAccounts::new(), )), - organizations: Arc::new(parking_lot::RwLock::new(None)), + organizations: Arc::new(parking_lot::RwLock::new( + fakecloud_organizations::OrganizationsRegistry::default(), + )), container_runtime: None, rds_runtime: None, elasticache_runtime: None, @@ -967,8 +995,9 @@ mod tests { let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )); - let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(None)); + let orgs: fakecloud_organizations::SharedOrganizationsState = Arc::new( + parking_lot::RwLock::new(fakecloud_organizations::OrganizationsRegistry::default()), + ); let resp = super::create_admin_in_account(&iam, &orgs, "123456789012", "admin", None) .expect("create admin"); assert_eq!(resp.account_id, "123456789012"); @@ -989,8 +1018,9 @@ mod tests { let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )); - let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(None)); + let orgs: fakecloud_organizations::SharedOrganizationsState = Arc::new( + parking_lot::RwLock::new(fakecloud_organizations::OrganizationsRegistry::default()), + ); let resp = super::create_admin_in_account(&iam, &orgs, "999999999999", "bob", None) .expect("create admin"); assert_eq!(resp.account_id, "999999999999"); @@ -1015,8 +1045,9 @@ mod tests { let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )); - let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(None)); + let orgs: fakecloud_organizations::SharedOrganizationsState = Arc::new( + parking_lot::RwLock::new(fakecloud_organizations::OrganizationsRegistry::default()), + ); let resp = super::create_admin_in_account(&iam, &orgs, "222222222222", "admin", None) .expect("create admin"); @@ -1053,15 +1084,15 @@ mod tests { fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )); let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(Some( - fakecloud_organizations::OrganizationState::bootstrap("111111111111"), - ))); + Arc::new(parking_lot::RwLock::new( + fakecloud_organizations::OrganizationState::bootstrap("111111111111").into(), + )); super::create_admin_in_account(&iam, &orgs, "222222222222", "admin", None) .expect("create admin"); let guard = orgs.read(); - let org = guard.as_ref().unwrap(); + let org = guard.sole().unwrap(); assert!( !org.accounts.contains_key("222222222222"), "a standalone bootstrap must leave the account outside the org" @@ -1078,14 +1109,14 @@ mod tests { let org_id = org.org_id.clone(); let root_id = org.root_id.clone(); let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(Some(org))); + Arc::new(parking_lot::RwLock::new(org.into())); super::create_admin_in_account(&iam, &orgs, "222222222222", "admin", Some(&org_id)) .expect("create admin"); let guard = orgs.read(); let member = guard - .as_ref() + .sole() .unwrap() .accounts .get("222222222222") @@ -1094,15 +1125,81 @@ mod tests { assert_eq!(member.status, "ACTIVE"); } + /// An account belongs to at most one organization. The bootstrap + /// shortcut must reject a second enrollment rather than putting the + /// account in two registries at once, where which organization's SCP + /// ceiling and stack-set targeting applied would be arbitrary. + #[test] + fn create_admin_cannot_enroll_an_account_into_a_second_organization() { + let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( + fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), + )); + let first = fakecloud_organizations::OrganizationState::bootstrap("111111111111"); + let second = fakecloud_organizations::OrganizationState::bootstrap("999999999999"); + let first_id = first.org_id.clone(); + let second_id = second.org_id.clone(); + let mut registry = fakecloud_organizations::OrganizationsRegistry::default(); + registry.insert(first); + registry.insert(second); + let orgs: fakecloud_organizations::SharedOrganizationsState = + Arc::new(parking_lot::RwLock::new(registry)); + + super::create_admin_in_account(&iam, &orgs, "222222222222", "admin", Some(&first_id)) + .expect("first enrollment"); + + let err = + super::create_admin_in_account(&iam, &orgs, "222222222222", "admin", Some(&second_id)) + .expect_err("a second organization must be rejected"); + assert_eq!( + err, + super::CreateAdminError::AccountInAnotherOrganization { + account_id: "222222222222".to_string(), + organization_id: first_id.clone(), + } + ); + + let guard = orgs.read(); + assert!(guard + .org_by_id(&first_id) + .unwrap() + .accounts + .contains_key("222222222222")); + assert!(!guard + .org_by_id(&second_id) + .unwrap() + .accounts + .contains_key("222222222222")); + } + + /// Re-naming the organization the account is already in is a no-op + /// rather than an error — bootstrapping admin credentials twice for + /// the same member must keep working. + #[test] + fn create_admin_into_the_account_s_own_organization_is_idempotent() { + let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( + fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), + )); + let org = fakecloud_organizations::OrganizationState::bootstrap("111111111111"); + let org_id = org.org_id.clone(); + let orgs: fakecloud_organizations::SharedOrganizationsState = + Arc::new(parking_lot::RwLock::new(org.into())); + + for _ in 0..2 { + super::create_admin_in_account(&iam, &orgs, "222222222222", "admin", Some(&org_id)) + .expect("repeat enrollment is a no-op"); + } + assert_eq!(orgs.read().org_by_id(&org_id).unwrap().accounts.len(), 2); + } + #[test] fn create_admin_with_unknown_organization_id_errors() { let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )); let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(Some( - fakecloud_organizations::OrganizationState::bootstrap("111111111111"), - ))); + Arc::new(parking_lot::RwLock::new( + fakecloud_organizations::OrganizationState::bootstrap("111111111111").into(), + )); let err = super::create_admin_in_account( &iam, @@ -1125,8 +1222,9 @@ mod tests { let iam: fakecloud_iam::SharedIamState = Arc::new(parking_lot::RwLock::new( fakecloud_core::multi_account::MultiAccountState::new("123456789012", "us-east-1", ""), )); - let orgs: fakecloud_organizations::SharedOrganizationsState = - Arc::new(parking_lot::RwLock::new(None)); + let orgs: fakecloud_organizations::SharedOrganizationsState = Arc::new( + parking_lot::RwLock::new(fakecloud_organizations::OrganizationsRegistry::default()), + ); let resp = super::create_admin_in_account(&iam, &orgs, "222222222222", "alice", None) .expect("create admin"); diff --git a/sdks/dotnet/src/FakeCloud/Types.cs b/sdks/dotnet/src/FakeCloud/Types.cs index fad120a00..aeb3a06d4 100644 --- a/sdks/dotnet/src/FakeCloud/Types.cs +++ b/sdks/dotnet/src/FakeCloud/Types.cs @@ -1094,13 +1094,34 @@ public sealed record OrganizationsAccount( string? JoinedTimestamp, string? ParentOuId, IReadOnlyList? Tags, - IReadOnlyList? ScpAttached); + IReadOnlyList? ScpAttached, + string? OrganizationId = null); +/// +/// One organization in the process. Organizations are fully independent: +/// each has its own management account, root and SCPs. +/// +public sealed record OrganizationsSummary( + string? OrganizationId, + string? Arn, + string? ManagementAccountId, + string? RootId, + string? FeatureSet); + +/// +/// Every member account across every organization. The flat +/// ManagementAccountId/MasterAccountId are set only when +/// exactly one organization exists; read Organizations (or each +/// account's OrganizationId) otherwise. +/// public sealed record OrganizationsAccountsResponse( IReadOnlyList? Accounts, string? ManagementAccountId, - string? MasterAccountId); + string? MasterAccountId, + IReadOnlyList? Organizations = null); +/// The organization holding the transfer; +/// the listing spans every organization in the process. public sealed record OrganizationsResponsibilityTransfer( string? Id, string? Arn, @@ -1114,7 +1135,8 @@ public sealed record OrganizationsResponsibilityTransfer( string? TargetManagementAccountEmail, string? StartTimestamp, string? EndTimestamp, - string? ActiveHandshakeId); + string? ActiveHandshakeId, + string? OrganizationId = null); public sealed record OrganizationsResponsibilityTransfersResponse( IReadOnlyList? ResponsibilityTransfers); diff --git a/sdks/go/types.go b/sdks/go/types.go index 6dd595ac0..cc6b35811 100644 --- a/sdks/go/types.go +++ b/sdks/go/types.go @@ -1493,26 +1493,44 @@ type OrganizationsTag struct { // or root id) and ScpAttached (SCP ids directly attached — does not // walk up the hierarchy). type OrganizationsAccount struct { - ID string `json:"id"` - ARN string `json:"arn"` - Email string `json:"email"` - Name string `json:"name"` - Status string `json:"status"` - JoinedMethod string `json:"joinedMethod"` - JoinedTimestamp string `json:"joinedTimestamp"` - ParentOuID *string `json:"parentOuId,omitempty"` - Tags []OrganizationsTag `json:"tags"` - ScpAttached []string `json:"scpAttached"` + ID string `json:"id"` + ARN string `json:"arn"` + Email string `json:"email"` + Name string `json:"name"` + Status string `json:"status"` + JoinedMethod string `json:"joinedMethod"` + JoinedTimestamp string `json:"joinedTimestamp"` + ParentOuID *string `json:"parentOuId,omitempty"` + // OrganizationID names the organization this account belongs to, + // so the flattened list stays readable with several organizations. + OrganizationID *string `json:"organizationId,omitempty"` + Tags []OrganizationsTag `json:"tags"` + ScpAttached []string `json:"scpAttached"` +} + +// OrganizationsSummary is one organization in the process. Organizations +// are fully independent: each has its own management account, root and +// SCPs. +type OrganizationsSummary struct { + OrganizationID string `json:"organizationId"` + ARN string `json:"arn"` + ManagementAccountID string `json:"managementAccountId"` + RootID string `json:"rootId"` + FeatureSet string `json:"featureSet"` } // OrganizationsAccountsResponse is the payload for -// GET /_fakecloud/organizations/accounts. Both management and master -// id fields are populated (AWS renamed Master to Management in 2020 -// but kept both around for back-compat). Empty when no org exists. +// GET /_fakecloud/organizations/accounts. Accounts spans every +// organization, each carrying its own OrganizationID. The flat +// management/master id fields are set only when exactly one +// organization exists (AWS renamed Master to Management in 2020 but +// kept both around for back-compat); read Organizations otherwise. +// Empty when no organization exists. type OrganizationsAccountsResponse struct { Accounts []OrganizationsAccount `json:"accounts"` ManagementAccountID *string `json:"managementAccountId,omitempty"` MasterAccountID *string `json:"masterAccountId,omitempty"` + Organizations []OrganizationsSummary `json:"organizations"` } // OrganizationsResponsibilityTransfer mirrors the AWS Organizations @@ -1533,6 +1551,9 @@ type OrganizationsResponsibilityTransfer struct { StartTimestamp string `json:"startTimestamp"` EndTimestamp *string `json:"endTimestamp,omitempty"` ActiveHandshakeID *string `json:"activeHandshakeId,omitempty"` + // OrganizationID names the organization holding the transfer; the + // listing spans every organization in the process. + OrganizationID *string `json:"organizationId,omitempty"` } // OrganizationsResponsibilityTransfersResponse is the payload for diff --git a/sdks/java/src/main/java/dev/fakecloud/Types.java b/sdks/java/src/main/java/dev/fakecloud/Types.java index 98d20a3ee..48f2085ae 100644 --- a/sdks/java/src/main/java/dev/fakecloud/Types.java +++ b/sdks/java/src/main/java/dev/fakecloud/Types.java @@ -1305,13 +1305,67 @@ public record OrganizationsAccount( String joinedTimestamp, String parentOuId, List tags, - List scpAttached) {} + List scpAttached, + String organizationId) { + /** The pre-multi-organization shape, with no owning org id. */ + public OrganizationsAccount( + String id, + String arn, + String email, + String name, + String status, + String joinedMethod, + String joinedTimestamp, + String parentOuId, + List tags, + List scpAttached) { + this( + id, + arn, + email, + name, + status, + joinedMethod, + joinedTimestamp, + parentOuId, + tags, + scpAttached, + null); + } + } + + /** + * One organization in the process. Organizations are fully + * independent: each has its own management account, root and SCPs. + */ + @JsonIgnoreProperties(ignoreUnknown = true) + public record OrganizationsSummary( + String organizationId, + String arn, + String managementAccountId, + String rootId, + String featureSet) {} + /** + * Every member account across every organization. The flat + * {@code managementAccountId}/{@code masterAccountId} are set only + * when exactly one organization exists; read {@code organizations} + * (or each account's {@code organizationId}) otherwise. + */ @JsonIgnoreProperties(ignoreUnknown = true) public record OrganizationsAccountsResponse( List accounts, String managementAccountId, - String masterAccountId) {} + String masterAccountId, + List organizations) { + /** The pre-multi-organization shape, with no summary list. */ + public OrganizationsAccountsResponse( + List accounts, + String managementAccountId, + String masterAccountId) { + this(accounts, managementAccountId, masterAccountId, List.of()); + } + } @JsonIgnoreProperties(ignoreUnknown = true) public record OrganizationsResponsibilityTransfer( @@ -1327,7 +1381,40 @@ public record OrganizationsResponsibilityTransfer( String targetManagementAccountEmail, String startTimestamp, String endTimestamp, - String activeHandshakeId) {} + String activeHandshakeId, + String organizationId) { + /** The pre-multi-organization shape, with no owning org id. */ + public OrganizationsResponsibilityTransfer( + String id, + String arn, + String name, + String type, + String status, + String direction, + String sourceManagementAccountId, + String sourceManagementAccountEmail, + String targetManagementAccountId, + String targetManagementAccountEmail, + String startTimestamp, + String endTimestamp, + String activeHandshakeId) { + this( + id, + arn, + name, + type, + status, + direction, + sourceManagementAccountId, + sourceManagementAccountEmail, + targetManagementAccountId, + targetManagementAccountEmail, + startTimestamp, + endTimestamp, + activeHandshakeId, + null); + } + } @JsonIgnoreProperties(ignoreUnknown = true) public record OrganizationsResponsibilityTransfersResponse( diff --git a/sdks/php/src/Types.php b/sdks/php/src/Types.php index 7718fd8fc..8aa255367 100644 --- a/sdks/php/src/Types.php +++ b/sdks/php/src/Types.php @@ -3642,6 +3642,7 @@ public function __construct( public readonly array $tags = [], /** @var string[] */ public readonly array $scpAttached = [], + public readonly ?string $organizationId = null, ) {} public static function fromArray(array $data): self @@ -3659,10 +3660,43 @@ public static function fromArray(array $data): self isset($data['parentOuId']) ? (string) $data['parentOuId'] : null, $tags, $scp, + isset($data['organizationId']) ? (string) $data['organizationId'] : null, ); } } +/** + * One organization in the process. Organizations are fully independent: + * each has its own management account, root and SCPs. + */ +final class OrganizationsSummary +{ + public function __construct( + public readonly string $organizationId, + public readonly string $arn, + public readonly string $managementAccountId, + public readonly string $rootId, + public readonly string $featureSet, + ) {} + + public static function fromArray(array $data): self + { + return new self( + (string) ($data['organizationId'] ?? ''), + (string) ($data['arn'] ?? ''), + (string) ($data['managementAccountId'] ?? ''), + (string) ($data['rootId'] ?? ''), + (string) ($data['featureSet'] ?? ''), + ); + } +} + +/** + * Every member account across every organization. The flat + * managementAccountId/masterAccountId are set only when exactly one + * organization exists; read $organizations (or each account's + * $organizationId) otherwise. + */ final class OrganizationsAccountsResponse { public function __construct( @@ -3670,6 +3704,8 @@ public function __construct( public readonly array $accounts, public readonly ?string $managementAccountId = null, public readonly ?string $masterAccountId = null, + /** @var OrganizationsSummary[] */ + public readonly array $organizations = [], ) {} public static function fromArray(array $data): self @@ -3678,6 +3714,7 @@ public static function fromArray(array $data): self array_map(OrganizationsAccount::fromArray(...), $data['accounts'] ?? []), isset($data['managementAccountId']) ? (string) $data['managementAccountId'] : null, isset($data['masterAccountId']) ? (string) $data['masterAccountId'] : null, + array_map(OrganizationsSummary::fromArray(...), $data['organizations'] ?? []), ); } } @@ -3698,6 +3735,7 @@ public function __construct( public readonly string $startTimestamp, public readonly ?string $endTimestamp = null, public readonly ?string $activeHandshakeId = null, + public readonly ?string $organizationId = null, ) {} public static function fromArray(array $data): self @@ -3716,6 +3754,7 @@ public static function fromArray(array $data): self (string) ($data['startTimestamp'] ?? ''), isset($data['endTimestamp']) ? (string) $data['endTimestamp'] : null, isset($data['activeHandshakeId']) ? (string) $data['activeHandshakeId'] : null, + isset($data['organizationId']) ? (string) $data['organizationId'] : null, ); } } diff --git a/sdks/python/src/fakecloud/types.py b/sdks/python/src/fakecloud/types.py index fb5a40b0c..844683787 100644 --- a/sdks/python/src/fakecloud/types.py +++ b/sdks/python/src/fakecloud/types.py @@ -3019,6 +3019,7 @@ class OrganizationsAccount: parent_ou_id: Optional[str] = None tags: List[OrganizationsTag] = field(default_factory=list) scp_attached: List[str] = field(default_factory=list) + organization_id: Optional[str] = None @classmethod def from_dict(cls, data: Dict[str, Any]) -> OrganizationsAccount: @@ -3035,6 +3036,7 @@ def from_dict(cls, data: Dict[str, Any]) -> OrganizationsAccount: parent_ou_id=d.get("parent_ou_id"), tags=tags, scp_attached=list(d.get("scp_attached", [])), + organization_id=d.get("organization_id"), ) @@ -3100,20 +3102,56 @@ def from_dict(cls, data: Dict[str, Any]) -> LogsFieldIndexesResponse: ) +@dataclass +class OrganizationsSummary: + """One organization in the process. Organizations are fully + independent: each has its own management account, root and SCPs.""" + + organization_id: str + arn: str + management_account_id: str + root_id: str + feature_set: str + + @classmethod + def from_dict(cls, data: Dict[str, Any]) -> OrganizationsSummary: + d = _convert_keys(data) + return cls( + organization_id=d.get("organization_id", ""), + arn=d.get("arn", ""), + management_account_id=d.get("management_account_id", ""), + root_id=d.get("root_id", ""), + feature_set=d.get("feature_set", ""), + ) + + @dataclass class OrganizationsAccountsResponse: + """Every member account across every organization. + + ``management_account_id``/``master_account_id`` are set only when + exactly one organization exists, so callers written against the + one-organization shape keep working; read ``organizations`` (or each + account's ``organization_id``) otherwise. + """ + accounts: List[OrganizationsAccount] = field(default_factory=list) management_account_id: Optional[str] = None master_account_id: Optional[str] = None + organizations: List[OrganizationsSummary] = field(default_factory=list) @classmethod def from_dict(cls, data: Dict[str, Any]) -> OrganizationsAccountsResponse: d = _convert_keys(data) accounts = [OrganizationsAccount.from_dict(a) for a in d.get("accounts", [])] + organizations = [ + OrganizationsSummary.from_dict(o) for o in d.get("organizations", []) + ] return cls( accounts=accounts, management_account_id=d.get("management_account_id"), master_account_id=d.get("master_account_id"), + organizations=organizations, ) @@ -3136,6 +3174,7 @@ class OrganizationsResponsibilityTransfer: start_timestamp: str end_timestamp: Optional[str] = None active_handshake_id: Optional[str] = None + organization_id: Optional[str] = None @classmethod def from_dict(cls, data: Dict[str, Any]) -> OrganizationsResponsibilityTransfer: @@ -3158,6 +3197,7 @@ def from_dict(cls, data: Dict[str, Any]) -> OrganizationsResponsibilityTransfer: start_timestamp=d.get("start_timestamp", ""), end_timestamp=d.get("end_timestamp"), active_handshake_id=d.get("active_handshake_id"), + organization_id=d.get("organization_id"), ) diff --git a/sdks/typescript/src/types.ts b/sdks/typescript/src/types.ts index b5993e84d..5e269e9cf 100644 --- a/sdks/typescript/src/types.ts +++ b/sdks/typescript/src/types.ts @@ -1398,19 +1398,39 @@ export interface OrganizationsAccount { /** RFC3339. */ joinedTimestamp: string; parentOuId?: string; + /** The organization this account belongs to, so the flattened list + * stays readable with several organizations. */ + organizationId?: string; tags: OrganizationsTag[]; /** SCP ids directly attached to the account. Does not include * policies inherited from the parent OU or root. */ scpAttached: string[]; } +/** One organization in the process. Organizations are fully + * independent: each has its own management account, root and SCPs. */ +export interface OrganizationsSummary { + organizationId: string; + arn: string; + managementAccountId: string; + rootId: string; + /** ALL | CONSOLIDATED_BILLING. */ + featureSet: string; +} + export interface OrganizationsAccountsResponse { + /** Every member account across every organization, each carrying its + * own `organizationId`. */ accounts: OrganizationsAccount[]; - /** `null`/undefined when no organization has been created yet. */ + /** Set only when exactly one organization exists, so callers written + * against the one-org shape keep working; read `organizations` + * otherwise. Undefined when no organization has been created yet. */ managementAccountId?: string; /** Duplicate of `managementAccountId`. AWS renamed Master to * Management in 2020 but kept the old field for back-compat. */ masterAccountId?: string; + /** One entry per organization in the process, ordered by id. */ + organizations: OrganizationsSummary[]; } export interface OrganizationsResponsibilityTransfer { @@ -1430,6 +1450,9 @@ export interface OrganizationsResponsibilityTransfer { /** RFC3339, or `null`/undefined while the transfer is still open. */ endTimestamp?: string | null; activeHandshakeId?: string | null; + /** The organization holding the transfer; the listing spans every + * organization in the process. */ + organizationId?: string; } export interface OrganizationsResponsibilityTransfersResponse { diff --git a/website/content/docs/reference/introspection.md b/website/content/docs/reference/introspection.md index 44131dde7..dab4a83ef 100644 --- a/website/content/docs/reference/introspection.md +++ b/website/content/docs/reference/introspection.md @@ -330,8 +330,8 @@ and accepts, or is created through `CreateAccount`. Naming an | Endpoint | Method | Description | | -------- | ------ | ----------- | -| `/_fakecloud/organizations/accounts` | GET | **NEW** -- List every member account with lifecycle state, parent OU, tags, and directly-attached SCPs. | -| `/_fakecloud/organizations/responsibility-transfers` | GET | **NEW** -- Every billing responsibility transfer in the org, with direction (INBOUND/OUTBOUND), lifecycle status, source/target management accounts, and the active handshake. Sorted by id. | +| `/_fakecloud/organizations/accounts` | GET | **NEW** -- List every member account across every organization with lifecycle state, parent OU, owning `organizationId`, tags, and directly-attached SCPs, plus an `organizations` summary. | +| `/_fakecloud/organizations/responsibility-transfers` | GET | **NEW** -- Every billing responsibility transfer across every organization, with the owning `organizationId`, direction (INBOUND/OUTBOUND), lifecycle status, source/target management accounts, and the active handshake. Sorted by id. | `GET /_fakecloud/organizations/responsibility-transfers` response: diff --git a/website/content/docs/reference/organizations.md b/website/content/docs/reference/organizations.md index 0ea6f647c..bee2f2cd2 100644 --- a/website/content/docs/reference/organizations.md +++ b/website/content/docs/reference/organizations.md @@ -10,7 +10,9 @@ Both the control plane and SCP enforcement are live. SCPs act as the top-of-chai ## Model -- One organization per fakecloud process. `CreateOrganization` sets the caller's account as the management account and seeds a root OU. +- Many independent organizations per fakecloud process. `CreateOrganization` sets the caller's account as the management account and seeds a root OU, and rejects only when **that account** is already in an organization (`AlreadyInOrganizationException`) -- another account having created one does not block it. +- An account belongs to at most one organization. Inviting an account another organization already holds is `HandshakeConstraintViolationException` with `Reason: ALREADY_IN_AN_ORGANIZATION`, both at invite time and again when the handshake is accepted. +- Organizations never see each other: every read resolves the caller's own organization, and a caller in none gets `AWSOrganizationsNotInUseException` -- the same answer as a process with no organizations at all. The exceptions are the operations that are cross-organization by nature, and they stay scoped to the caller's own involvement: `ListHandshakesForAccount` and `DescribeHandshake` (the account answering an invitation is not yet a member of the inviting organization, so a handshake resolves by id, readable by its two parties and by members of the organization that owns it), and the responsibility-transfer ops (a transfer is recorded once, in the source organization, and both management accounts are parties to it). - `FullAWSAccess` is auto-created and auto-attached to the root OU on `CreateOrganization`, matching AWS. Its content: ```json {"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]} diff --git a/website/content/docs/services/organizations.md b/website/content/docs/services/organizations.md index 31357dd7a..ecb6c518f 100644 --- a/website/content/docs/services/organizations.md +++ b/website/content/docs/services/organizations.md @@ -20,9 +20,9 @@ fakecloud implements **63 of 63** AWS Organizations operations at 100% Smithy co - **StackSets auto-deployment** — a membership change (an account created, invited, moved between OUs, removed or closed) reconciles every service-managed CloudFormation stack set that has `AutoDeployment.Enabled`, so an account that joins a targeted OU is provisioned with that stack set's stacks before the Organizations call returns. See [CloudFormation](/docs/services/cloudformation/#stack-sets). - **Policies** — `CreatePolicy`, `UpdatePolicy`, `DeletePolicy`, `DescribePolicy`, `ListPolicies`, `ListPoliciesForTarget`, `ListTargetsForPolicy`, `AttachPolicy`, `DetachPolicy`, `EnablePolicyType`, `DisablePolicyType`, `DescribeEffectivePolicy`. The full `PolicyType` enum is accepted on the list filters; the four types fakecloud manages (`SERVICE_CONTROL_POLICY`, `TAG_POLICY`, `BACKUP_POLICY`, `AISERVICES_OPT_OUT_POLICY`) can be created — others return `PolicyTypeNotAvailableForOrganizationException`, an out-of-enum value returns `InvalidInputException`. Policy documents are JSON-validated on create/update — malformed content is rejected with `MalformedPolicyDocumentException`. - **Effective-policy validation** — `ListAccountsWithInvalidEffectivePolicy` and `ListEffectivePolicyValidationErrors` return the honest empty result: fakecloud stores only well-formed policies, so no account ever has an invalid effective policy. -- **Billing responsibility transfers** — `InviteOrganizationToTransferResponsibility` opens a handshake-backed `BILLING` transfer; `DescribeResponsibilityTransfer`, `UpdateResponsibilityTransfer` (rename), `TerminateResponsibilityTransfer` (-> `WITHDRAWN`), and `ListInboundResponsibilityTransfers` / `ListOutboundResponsibilityTransfers` operate over the transfer records, filtered by direction. +- **Billing responsibility transfers** — `InviteOrganizationToTransferResponsibility` opens a handshake-backed `BILLING` transfer; `DescribeResponsibilityTransfer`, `UpdateResponsibilityTransfer` (rename), `TerminateResponsibilityTransfer` (-> `WITHDRAWN`), and `ListInboundResponsibilityTransfers` / `ListOutboundResponsibilityTransfers` operate over the transfer records, filtered by direction. One live offer per target: a second invitation to the same account, by id or by address, returns `DuplicateHandshakeException`. The handshake reports the transfer it carries as a nested `RESPONSIBILITY_TRANSFER` resource (with `TRANSFER_TYPE`, `TRANSFER_START_TIMESTAMP`, `MANAGEMENT_ACCOUNT` and `MANAGEMENT_EMAIL`), so the invited account can read what it is being offered without a second call. - **Resource policies** — `PutResourcePolicy`, `DescribeResourcePolicy`, `DeleteResourcePolicy` for the org-wide delegation policy. -- **Service access** — `EnableAWSServiceAccess`, `DisableAWSServiceAccess`, `ListAWSServiceAccessForOrganization`, `RegisterDelegatedAdministrator`, `DeregisterDelegatedAdministrator`, `ListDelegatedAdministrators`, `ListDelegatedServicesForAccount`. Delegated-admin registration is gated on the service having `EnableAWSServiceAccess` first, matching AWS error ordering. +- **Service access** — `EnableAWSServiceAccess`, `DisableAWSServiceAccess`, `ListAWSServiceAccessForOrganization`, `RegisterDelegatedAdministrator`, `DeregisterDelegatedAdministrator`, `ListDelegatedAdministrators`, `ListDelegatedServicesForAccount`. Delegated-admin registration is gated on the service having `EnableAWSServiceAccess` first, matching AWS error ordering. A registered delegated administrator can run the organization's read operations on the management account's behalf -- `ListHandshakesForOrganization`, `ListAWSServiceAccessForOrganization`, `ListDelegatedAdministrators`, `ListDelegatedServicesForAccount` and `DescribeResourcePolicy` -- while every mutating operation stays management-only. - **Tagging** — `TagResource`, `UntagResource`, `ListTagsForResource` on accounts, OUs, roots, and policies. ## SCP enforcement @@ -101,17 +101,31 @@ Response shape: "joinedMethod": "INVITED", "joinedTimestamp": "2026-05-11T00:00:00Z", "parentOuId": "r-1234", + "organizationId": "o-abc", "tags": [], "scpAttached": [] } ], "managementAccountId": "111111111111", - "masterAccountId": "111111111111" + "masterAccountId": "111111111111", + "organizations": [ + { + "organizationId": "o-abc", + "arn": "arn:aws:organizations::111111111111:organization/o-abc", + "managementAccountId": "111111111111", + "rootId": "r-1234", + "featureSet": "ALL" + } + ] } ``` `scpAttached` lists SCPs attached directly to the account only — to resolve the full inherited set walk up the OU tree or call `DescribeEffectivePolicy`. `accounts` is empty (and the account-id fields `null`) when no organization has been created yet. `masterAccountId` mirrors `managementAccountId` for back-compat with the AWS field renamed in 2020. +`accounts` spans **every** organization in the process, each entry carrying its own `organizationId`, and `organizations` lists one entry per organization. The flat `managementAccountId`/`masterAccountId` are set only when exactly one organization exists, so a caller written against the single-organization shape keeps working; with several, read `organizations` (or each account's `organizationId`) rather than getting one arbitrary organization's answer. + +Note that this overloads `null` on those two fields: before multi-organization support it meant "no organization has been created yet", and it now also means "more than one exists, so there is no single answer". Test `organizations` instead -- it is empty only when no organization exists. + The first-party SDKs wrap this: - Rust: `fakecloud_sdk::FakeCloud::new(url).organizations().get_accounts()`