Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions iamscope/resolver/scp_binder.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ def bind_all_scps(

For each (edge, constraint) pair, checks if the SCP's action scope
matches the edge action. OU inheritance is handled by checking if
the edge's destination account is within the constraint's scope.
the edge's source/principal account is within the constraint's scope.
SCPs govern principals, not target resources; resource pattern
matching still uses the edge destination inside bind_scp_to_edge.

Args:
edges: All edges in the graph.
Expand All @@ -150,13 +152,18 @@ def bind_all_scps(
bindings: list[EdgeConstraint] = []

for edge in edges:
dst_account = _extract_account_from_ref(edge.dst)
src_account = _extract_account_from_ref(edge.src)

for constraint in constraints:
# Check scope: does this SCP apply to the edge's destination account?
if ou_account_map is not None and dst_account:
# Check scope: does this SCP apply to the edge's source/principal
# account? If the source account is unavailable, do not fall back
# to the target/resource account; that would incorrectly apply
# target-account SCPs to external callers.
if ou_account_map is not None:
if not src_account:
continue
scope_accounts = ou_account_map.get(constraint.scope_id, set())
if dst_account not in scope_accounts:
if src_account not in scope_accounts:
continue

binding = bind_scp_to_edge(edge, constraint)
Expand Down
112 changes: 109 additions & 3 deletions tests/test_scp_binder.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,9 @@ def test_global_binding_no_scope_map(self) -> None:
assert len(bindings) == 1

def test_ou_scope_limits_binding(self) -> None:
"""With ou_account_map, SCP only binds to edges in scope."""
edge_in_scope = _make_edge(dst_id="arn:aws:iam::111111\u003111111:role/InScope")
edge_out_scope = _make_edge(dst_id="arn:aws:iam::999999\u003999999:role/OutOfScope")
"""With ou_account_map, SCP only binds to source principals in scope."""
edge_in_scope = _make_edge(src_id="arn:aws:iam::111111\u003111111:root")
edge_out_scope = _make_edge(src_id="arn:aws:iam::999999\u003999999:root")

scp = _make_scp_constraint(deny_actions=["sts:AssumeRole"], scope_id="ou-prod")

Expand Down Expand Up @@ -420,6 +420,112 @@ def test_output_sorted_by_composite_key(self) -> None:
assert keys == sorted(keys)


class TestBindAllScpsSourceAccountScope:
"""Regression coverage: SCP scope applies to the principal account, not target account."""

def _cross_account_edge(self) -> Edge:
return _make_edge(
src_id="arn:aws:iam::111111\u003111111:role/Caller",
src_type=NODE_TYPE_IAM_ROLE,
dst_id="arn:aws:iam::222222\u003222222:role/TargetRole",
)

def test_cross_account_source_scoped_deny_binds(self) -> None:
edge = self._cross_account_edge()
scp = _make_scp_constraint(deny_actions=["sts:AssumeRole"], scope_id="ou-source")
ou_map = {"ou-source": {"111111\u003111111"}}

bindings = bind_all_scps([edge], [scp], ou_account_map=ou_map)

assert len(bindings) == 1
assert bindings[0].edge_id == edge.edge_id
assert bindings[0].constraint_id == scp.constraint_id

def test_cross_account_target_scoped_deny_does_not_bind(self) -> None:
edge = self._cross_account_edge()
scp = _make_scp_constraint(deny_actions=["sts:AssumeRole"], scope_id="ou-target")
ou_map = {"ou-target": {"222222\u003222222"}}

bindings = bind_all_scps([edge], [scp], ou_account_map=ou_map)

assert bindings == []

def test_resource_matching_remains_target_based(self) -> None:
edge = self._cross_account_edge()
scp = _make_scp_constraint(
deny_actions=["sts:AssumeRole"],
resource_patterns=["arn:aws:iam::222222\u003222222:role/OtherRole"],
scope_id="ou-source",
)
ou_map = {"ou-source": {"111111\u003111111"}}

bindings = bind_all_scps([edge], [scp], ou_account_map=ou_map)

assert bindings == []

def test_principal_applicability_still_uses_source_principal(self) -> None:
edge = self._cross_account_edge()
scp = _make_scp_constraint(
deny_actions=["sts:AssumeRole"],
applicable_principal_patterns=["arn:aws:iam::111111\u003111111:role/OtherCaller"],
scope_id="ou-source",
)
ou_map = {"ou-source": {"111111\u003111111"}}

bindings = bind_all_scps([edge], [scp], ou_account_map=ou_map)

assert bindings == []

def test_same_account_behavior_still_binds(self) -> None:
edge = _make_edge(
src_id="arn:aws:iam::111111\u003111111:role/Caller",
src_type=NODE_TYPE_IAM_ROLE,
dst_id="arn:aws:iam::111111\u003111111:role/TargetRole",
)
scp = _make_scp_constraint(deny_actions=["sts:AssumeRole"], scope_id="ou-same")
ou_map = {"ou-same": {"111111\u003111111"}}

bindings = bind_all_scps([edge], [scp], ou_account_map=ou_map)

assert len(bindings) == 1

def test_ou_map_uses_source_account_not_target_account(self) -> None:
edge = self._cross_account_edge()
source_scoped_scp = _make_scp_constraint(
deny_actions=["sts:AssumeRole"],
scope_id="ou-source",
policy_id="p-source",
statement_id="source",
)
target_scoped_scp = _make_scp_constraint(
deny_actions=["sts:AssumeRole"],
scope_id="ou-target",
policy_id="p-target",
statement_id="target",
)
ou_map = {
"ou-source": {"111111\u003111111"},
"ou-target": {"222222\u003222222"},
}

bindings = bind_all_scps([edge], [target_scoped_scp, source_scoped_scp], ou_account_map=ou_map)

assert len(bindings) == 1
assert bindings[0].constraint_id == source_scoped_scp.constraint_id

def test_unavailable_source_account_does_not_fall_back_to_target_account(self) -> None:
edge = _make_edge(
src_id="__synthetic__:external-principal-set",
dst_id="arn:aws:iam::222222\u003222222:role/TargetRole",
)
scp = _make_scp_constraint(deny_actions=["sts:AssumeRole"], scope_id="ou-target")
ou_map = {"ou-target": {"222222\u003222222"}}

bindings = bind_all_scps([edge], [scp], ou_account_map=ou_map)

assert bindings == []


class TestBindingReason:
"""Tests for human-readable binding reason."""

Expand Down