diff --git a/CLAUDE.md b/CLAUDE.md index 22ddf04..0b79c50 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -206,7 +206,7 @@ terraform/state/ | `scripts/check-aws-access.sh` | CI (app tf-plan): skips AWS steps with a notice when the run has no OIDC token (fork/Dependabot PRs) (tests: `scripts/tests/`) | | `scripts/auth-doctor.py` | CI (after app apply) / local: checks an app's Cognito sign-in end to end from auth-status + HTTP probes (tests: `scripts/tests/`) | | `scripts/reconcile-app-auth.sh` | CI (after platform apply): queue an app-auth-sync reconcile | -| `scripts/sync-team-access.sh` | CI (after platform apply): queue team-provisioner `sync_team_access` (new teams' permission set + account assignment once their guardrails policy exists) | +| `scripts/sync-team-access.sh` | CI (after platform apply): queue team-provisioner `sync_team_access` (switches teams from read-only to full access once their guardrails policy exists; rolls inline policy changes out) | ## Naming @@ -223,7 +223,7 @@ terraform/state/ | SSM params (app) | `/javabin/apps/{team}/{service}/{name}` | | ECS cluster | `javabin-platform` | | ECS execution roles | `javabin-ecs-exec-{team}` (platform-owned, reads only `/javabin/apps/{team}/*`) | -| SSO team guardrails | `javabin-team-{team}-guardrails` (developer boundary with the team written in, for `javabin-team-{team}` permission sets) | +| SSO team guardrails | `javabin-team-{team}-guardrails` (developer boundary with the team written in; the permissions boundary of `javabin-team-{team}` permission sets) | | ECR repos | `{team}-{service}` | | SNS topics | `javabin-{alerts,security,budget-enforcement}` | | Log groups | `/ecs/{team}/{service}` | @@ -333,7 +333,7 @@ Team group OWNERs are admin-managed and never removed by the sync; non-@java.no - **ECS deploy stabilization**: platform-test-app task registers but service fails health check - **Cognito**: pools deployed; internal pool's `GoogleSAML` IdP was created outside Terraform. It is replaced by the Terraform-managed Google OIDC IdP at cutover (docs/cognito-google-setup.md), then deleted - **Google SCIM provisions only direct members** of `drift@`, `styret@` and `javabin-developer@java.no` to Identity Center; it does not expand nested groups. Nobody gets an Identity Center user (and so no AWS access, team role included) unless they are a direct member of one of those. The team provisioner adds team members and (via `flatten: true` in the registry's `groups/access.yaml`) kodesmia members to `javabin-developer@` one by one; SCIM creates the user later, and the 3-hourly `sync_access_members` run adds them to their Identity Center groups. Don't create Identity Center users through the API: they'd have no link to Google (see `docs/lambda-functions.md`). -- **SSO sessions carry no `team` tag**: the Identity Center ABAC attribute (`${path:enterprise.team}`) is never set on users, so `${aws:PrincipalTag/team}` is empty for humans. Per-team permission sets (`javabin-team-{team}`) therefore use `javabin-team-{team}-guardrails` (the developer boundary rendered with the team name) instead. The ABAC-based `javabin-developer` permission set in `terraform/org` cannot reach team resources +- **SSO sessions carry no `team` tag**: the Identity Center ABAC attribute (`${path:enterprise.team}`) is never set on users, so `${aws:PrincipalTag/team}` is empty for humans. Per-team permission sets (`javabin-team-{team}`) therefore use `javabin-team-{team}-guardrails` (the developer boundary rendered with the team name) as their permissions boundary instead. It must stay a boundary: its baseline `Allow *` attached as a policy grants everything its Denies miss (every untagged resource). A team with no guardrails policy (`platform`, or a new team before platform CI) gets read-only access only. The ABAC-based `javabin-developer` permission set in `terraform/org` cannot reach team resources - **App databases share one network path** (fixed by `app-network`): service-rds now allows ingress only from the app's own `{team}-{app}-tasks` SG; each app moves off the shared ECS tasks SG on its next pipeline run - **Team provisioner Lambda**: All sync functions working (Google/GitHub/Budget/Cognito/Identity Center). Password-set flow deployed. - **`registered_app_repos` manually managed**: Being replaced with team-scoped IAM roles (repo→team resolved via GitHub API at runtime) diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index a5456cf..50bf81c 100644 --- a/docs/lambda-functions.md +++ b/docs/lambda-functions.md @@ -139,21 +139,37 @@ makes someone a plain member. Registering a team (`javabin register-team`) write member's existing role is changed in place, and logins not in the registry are removed. The Google group keeps **MEMBER** as its default (only `role: maintainer` makes a Google MANAGER). +### Team permission set (`javabin-team-{team}`) + +`_ensure_team_permission_set` creates the Identity Center permission set for each registry team and assigns it to the team's group. It is the only AWS access team members get: + +| Part | What it is | Effect | +|------|-----------|--------| +| Permissions boundary | `javabin-team-{team}-guardrails` (platform Terraform, `iam/boundary.tf`) | The developer boundary's Denies with the team written in. A boundary only limits; its baseline `Allow *` grants nothing | +| AWS managed policy | `ViewOnlyAccess` | Console browsing (List/Describe, no object, parameter or secret reads), as `javabin-readonly` | +| Inline policy | `_team_inline_policy` | Everything on resources tagged `team={team}` (`StringEquals`, so untagged resources, S3 objects, Route53 records and creates are not included); by name: `/javabin/apps/{team}/*` parameters (read, overwrite), `/ecs/{team}/*` log groups, `{team}-*-{account}` buckets, the cluster half of ECS Exec; a few untagged reads for `javabin status`. Platform-managed parameters and new parameters stay denied | + +So a team member can read and overwrite their secrets (`javabin secret`), read their logs, push and pull their ECR repos, redeploy, stop or exec into their services, and browse the console. They cannot change shared or untagged resources (state bucket, legacy buckets and instances, Route53 zones, untagged roles and Lambdas) or create resources by hand: infrastructure goes through `app.yaml` and CI. + +Order on each run: set the boundary, then detach any `javabin-team-{team}-guardrails` or `javabin-developer-boundary` attached as a policy by older versions, attach `ViewOnlyAccess`, put the inline policy, provision. If the boundary cannot be set the permission set is left as it is and the result has an `error`; a `ViewOnlyAccess` failure is reported as `view_only` and the rest still applies. A team without a Terraform-managed guardrails policy (`pending`, e.g. a brand-new team before platform CI or `platform` itself, or `refused`) gets read-only access instead: no boundary, the legacy policies detached, `ViewOnlyAccess`, and `_read_only_inline_policy` (only the untagged reads). Without a boundary, the team inline policy's tag allow would also cover platform-tagged resources. The result reports the state as `guardrails`, and the next run after platform CI creates the policy switches the team to full access. + ### Team AWS access (`sync_identity_center_group`, `sync_team_access`) -The team sync puts members in the Identity Center group `team-{name}` and gives that group the -permission set `javabin-team-{name}` on the account, with the customer-managed policy -`javabin-team-{name}-guardrails`. That policy is made by platform Terraform, which only learns of a -new team when platform CI runs on the registry's `teams-changed` dispatch, after the team sync. -Until it exists the team sync stops before the inline policy and the account assignment and reports -`guardrails: pending` (`:hourglass_flowing_sand:` in Slack): without the guardrails the inline -policy's `Action: *` would be unbounded. +The team sync puts members in the Identity Center group `team-{name}` and runs +`_ensure_team_permission_set` (above) for the permission set `javabin-team-{name}`. Its boundary, +`javabin-team-{name}-guardrails`, is made by platform Terraform, which only learns of a new team when +platform CI runs on the registry's `teams-changed` dispatch, after the team sync. Until it exists +the team sync gives the team read-only access (no boundary, `ViewOnlyAccess`, +`_read_only_inline_policy`, account assignment) and reports `guardrails: pending` +(`:hourglass_flowing_sand:` in Slack): without the boundary the team inline policy's tag allow would +also cover platform-tagged resources. Platform CI finishes the job: after every apply it queues `{"action": "sync_team_access"}` (`scripts/sync-team-access.sh`, async). For every `team-*` Identity Center group whose guardrails -policy exists and is tagged `managed-by=terraform`, `repo=javaBin/platform`, it attaches the -guardrails, writes the inline policy, makes the account assignment and re-provisions the permission -set; teams without one (e.g. `platform`) are left alone. `{"teams": [...]}` limits the run. It posts +policy exists and is tagged `managed-by=terraform`, `repo=javaBin/platform`, it sets the guardrails +as the boundary, writes the team inline policy, makes the account assignment and re-provisions the +permission set; teams without one (e.g. `platform`) are left alone (they keep the read-only access +the team sync gave them). `{"teams": [...]}` limits the run. It posts to #javabin-infra-alerts only when a team got access for the first time or something failed (`CreateAccountAssignment` accepts an assignment that already exists without a conflict, so the handler checks `ListAccountAssignments` first; if that lookup fails it stays quiet). Since it diff --git a/docs/platform-modules.md b/docs/platform-modules.md index b9a63fc..2a1fc21 100644 --- a/docs/platform-modules.md +++ b/docs/platform-modules.md @@ -110,7 +110,7 @@ SNS topics, EventBridge rules, Config, GuardDuty, Security Hub, account guardrai GuardDuty filters only affect new findings; archive existing matches once in the console. -Team SSO roles' `CredentialAccess:IAMUser/AnomalousBehavior` on `ssm:GetParameter` is deliberately not filtered. The `javabin-team-{team}` permission set is `Action "*"` scoped by `StringEqualsIfExists aws:ResourceTag/team`, so untagged parameters are readable too, and this is the finding a stolen SSO session produces. Check the source country and confirm with the user before archiving one. +Team SSO roles' `CredentialAccess:IAMUser/AnomalousBehavior` on `ssm:GetParameter` is deliberately not filtered. The `javabin-team-{team}` permission set can read its own team's parameters (`/javabin/apps/{team}/*`, see [team-provisioner](lambda-functions.md#team-permission-set-javabin-teamteam)), and this is the finding a stolen SSO session produces. Check the source country and confirm with the user before archiving one. ## lambdas diff --git a/terraform/lambda-src/team_provisioner/handler.py b/terraform/lambda-src/team_provisioner/handler.py index 5bc3c60..34120bd 100644 --- a/terraform/lambda-src/team_provisioner/handler.py +++ b/terraform/lambda-src/team_provisioner/handler.py @@ -1832,6 +1832,7 @@ def handle_sync_access_members(event): DEVELOPER_BOUNDARY_NAME = f"{PROJECT}-developer-boundary" +VIEW_ONLY_POLICY_ARN = "arn:aws:iam::aws:policy/job-function/ViewOnlyAccess" def _assignment_exists(ps_arn, group_id): @@ -1919,12 +1920,14 @@ def _start_account_assignment(ps_arn, group_id, label): def _ensure_team_permission_set(team_name, group_id): """Create a per-team permission set and assign it to the team's IC group. - Guardrails come from javabin-team-{team}-guardrails (platform Terraform): - the developer boundary's rules with the team name written in. The shared - developer boundary can't be used here: its rules compare resource tags - with ${aws:PrincipalTag/team}, and Identity Center sessions carry no team - tag, so it would deny every team-scoped resource. The inline policy - allows broad access scoped to resources tagged with the team name. + The permissions boundary is javabin-team-{team}-guardrails (platform + Terraform): the developer boundary's rules with the team name written + in. The shared developer boundary can't be used here: its rules compare + resource tags with ${aws:PrincipalTag/team}, and Identity Center sessions + carry no team tag, so it would deny every team-scoped resource. The + boundary only limits; access itself comes + from AWS ViewOnlyAccess (console browsing, no data reads) plus the inline + policy from _team_inline_policy (the team's own resources). """ ps_name = f"{PROJECT}-team-{team_name}" @@ -1950,52 +1953,174 @@ def _ensure_team_permission_set(team_name, group_id): if not ps_arn: return {"error": "could not create or find permission set"} - # Attach the team's guardrails; drop the shared boundary from older setups. + # The team's guardrails are the permissions boundary. Older setups had + # them (or the shared developer boundary) attached as a policy, where + # their baseline "Allow *" granted everything the Denies didn't cover; + # detach those only once the boundary is in place. # Only a policy platform Terraform created: platform CI may not have made # it yet for a brand-new team (or never, for non-app teams like - # platform). Then leave the permission set as it is and report pending. + # platform). Without a boundary the team policy's tag allow would cover + # platform-tagged resources too, so such a team gets read-only access + # (ViewOnlyAccess and _read_only_inline_policy) until the guardrails exist. guardrails = f"{PROJECT}-team-{team_name}-guardrails" state = _guardrails_state(guardrails) - if state != "ok": - logger.warning("Not switching %s to %s: %s", ps_name, guardrails, state) - return {"permission_set": ps_name, "guardrails": state} + if state == "ok": + try: + sso_client.put_permissions_boundary_to_permission_set( + InstanceArn=SSO_INSTANCE_ARN, + PermissionSetArn=ps_arn, + PermissionsBoundary={"CustomerManagedPolicyReference": {"Name": guardrails}}, + ) + except Exception as e: + logger.error("Failed to set %s as boundary of %s: %s", guardrails, ps_name, e) + return {"permission_set": ps_name, "error": str(e)[:200]} + policy = _team_inline_policy(team_name) + else: + logger.warning("No %s for %s (%s): read-only access", guardrails, ps_name, state) + policy = _read_only_inline_policy() + for name in (guardrails, DEVELOPER_BOUNDARY_NAME): + try: + sso_client.detach_customer_managed_policy_reference_from_permission_set( + InstanceArn=SSO_INSTANCE_ARN, + PermissionSetArn=ps_arn, + CustomerManagedPolicyReference={"Name": name}, + ) + logger.info("Detached policy %s from %s", name, ps_name) + except sso_client.exceptions.ResourceNotFoundException: + pass # not attached + except Exception as e: + logger.error("Failed to detach %s from %s: %s", name, ps_name, e) + + view_only = _attach_view_only(ps_arn, ps_name) + try: - sso_client.attach_customer_managed_policy_reference_to_permission_set( + sso_client.put_inline_policy_to_permission_set( InstanceArn=SSO_INSTANCE_ARN, PermissionSetArn=ps_arn, - CustomerManagedPolicyReference={"Name": guardrails}, + InlinePolicy=json.dumps(policy), ) - except sso_client.exceptions.ConflictException: - pass # already attached except Exception as e: - logger.error("Failed to attach %s to %s: %s", guardrails, ps_name, e) + logger.error("Failed to set inline policy for %s: %s", ps_name, e) + return {"error": str(e)[:200]} + + # Assign to group and wait for completion + result = _create_account_assignment(ps_arn, group_id, ps_name) + result["permission_set"] = ps_name + result["view_only"] = view_only + if state != "ok": + result["guardrails"] = state + # Push policy changes to the role in the account (needed for existing + # assignments; a new assignment provisions on creation) + result["provisioned"] = _provision_permission_set(ps_arn, ps_name) + return result + + +def _attach_view_only(ps_arn, ps_name): + """Attach AWS ViewOnlyAccess (the same read-only browsing as + javabin-readonly). 'ok', or the error: the inline policy is still set, + so the team keeps access to its own resources either way.""" try: - sso_client.detach_customer_managed_policy_reference_from_permission_set( + sso_client.attach_managed_policy_to_permission_set( InstanceArn=SSO_INSTANCE_ARN, PermissionSetArn=ps_arn, - CustomerManagedPolicyReference={"Name": DEVELOPER_BOUNDARY_NAME}, + ManagedPolicyArn=VIEW_ONLY_POLICY_ARN, ) - logger.info("Detached %s from %s", DEVELOPER_BOUNDARY_NAME, ps_name) - except sso_client.exceptions.ResourceNotFoundException: - pass # not attached + except sso_client.exceptions.ConflictException: + pass # already attached except Exception as e: - logger.error("Failed to detach %s from %s: %s", DEVELOPER_BOUNDARY_NAME, ps_name, e) + logger.error("Failed to attach ViewOnlyAccess to %s: %s", ps_name, e) + return f"error: {str(e)[:200]}" + return "ok" + - # Set inline policy — broad allow scoped to team-tagged resources - policy = { +# Kept although ViewOnlyAccess covers most of it: javabin-cli (status, +# secret) must not depend on the managed attachment +_UNTAGGED_READS = { + "Sid": "AllowUntaggedReads", + "Effect": "Allow", + "Action": [ + "ecs:ListClusters", + "ecs:ListServices", + "ecs:ListTasks", + "ecs:DescribeServices", + "ecs:DescribeTasks", + "ecr:GetAuthorizationToken", + "elasticloadbalancing:Describe*", + "route53:ListHostedZones", + "logs:DescribeLogGroups", + "ssm:DescribeParameters", + "cloudwatch:ListMetrics", + "cloudwatch:GetMetricData", + "cloudwatch:GetMetricStatistics", + "ce:GetCostAndUsage", + "ce:GetCostForecast", + ], + "Resource": "*", +} + + +def _read_only_inline_policy(): + """Inline policy of a team permission set whose guardrails don't exist: + with no boundary to limit it, only reads (with ViewOnlyAccess).""" + return {"Version": "2012-10-17", "Statement": [_UNTAGGED_READS]} + + +def _team_inline_policy(team_name): + """Inline policy of javabin-team-{team}: write access only to the team's + own resources, on top of ViewOnlyAccess and under the team guardrails. + + The tag allow is StringEquals, not StringEqualsIfExists: an action whose + request carries no aws:ResourceTag (S3 objects, Route53, creates, any + untagged legacy resource) is not allowed by it. The team's own resources + that can't be matched by tag are allowed by name instead: SSM parameters + (PutParameter has no aws:ResourceTag), log groups, buckets + ({team}-{name}-{account}) and ECS Exec on the shared cluster (the task is + checked by its propagated team tag). + """ + params = f"arn:aws:ssm:*:{ACCOUNT_ID}:parameter/{PROJECT}/apps/{team_name}" + buckets = f"arn:aws:s3:::{team_name}-*-{ACCOUNT_ID}" + return { "Version": "2012-10-17", "Statement": [ { - "Sid": "AllowTeamResources", + "Sid": "AllowTeamTaggedResources", "Effect": "Allow", "Action": "*", "Resource": "*", - "Condition": { - "StringEqualsIfExists": { - "aws:ResourceTag/team": team_name, - } - }, + "Condition": {"StringEquals": {"aws:ResourceTag/team": team_name}}, + }, + { + "Sid": "AllowTeamParameters", + "Effect": "Allow", + "Action": ["ssm:GetParameter", "ssm:GetParameters", "ssm:GetParametersByPath", + "ssm:GetParameterHistory", "ssm:PutParameter"], + "Resource": [params, f"{params}/*"], + }, + { + # Guardrails DenyXLogs already limits log reads to these groups + "Sid": "AllowTeamLogs", + "Effect": "Allow", + "Action": ["logs:GetLogEvents", "logs:FilterLogEvents", "logs:DescribeLogStreams", + "logs:StartQuery", "logs:StartLiveTail", "logs:GetLogGroupFields"], + "Resource": f"arn:aws:logs:*:{ACCOUNT_ID}:log-group:/ecs/{team_name}/*", + }, + { + "Sid": "AllowTeamBuckets", + "Effect": "Allow", + "Action": ["s3:ListBucket", "s3:ListBucketVersions", "s3:GetBucketLocation", + "s3:GetObject", "s3:GetObjectVersion", "s3:PutObject", + "s3:DeleteObject", "s3:DeleteObjectVersion", + "s3:AbortMultipartUpload", "s3:ListMultipartUploadParts"], + "Resource": [buckets, f"{buckets}/*"], + }, + { + # The cluster half of ECS Exec; the task half needs the team tag + "Sid": "AllowExecOnPlatformCluster", + "Effect": "Allow", + "Action": "ecs:ExecuteCommand", + "Resource": f"arn:aws:ecs:*:{ACCOUNT_ID}:cluster/{PROJECT}-platform", }, + _UNTAGGED_READS, { # Platform-managed parameters in the team path: Cognito # credentials and auth-status (app-auth-sync), the auth @@ -2022,54 +2147,9 @@ def _ensure_team_permission_set(team_name, group_id): "Resource": f"arn:aws:ssm:*:{ACCOUNT_ID}:parameter/{PROJECT}/apps/*", "Condition": {"BoolIfExists": {"ssm:Overwrite": "false"}}, }, - { - # ECS Exec into the team's tasks (apps with compute.exec). - # ExecuteCommand is authorized against the cluster and the - # task: the shared cluster is allowed here, the task only by - # AllowTeamResources (its propagated team tag), so another - # team's tasks stay out of reach. - "Sid": "AllowExecOnPlatformCluster", - "Effect": "Allow", - "Action": "ecs:ExecuteCommand", - "Resource": f"arn:aws:ecs:*:{ACCOUNT_ID}:cluster/{PROJECT}-platform", - }, - { - "Sid": "AllowUntaggedReads", - "Effect": "Allow", - "Action": [ - "ecs:ListClusters", - "ecs:ListServices", - "ecr:GetAuthorizationToken", - "elasticloadbalancing:Describe*", - "route53:ListHostedZones", - "logs:DescribeLogGroups", - "cloudwatch:ListMetrics", - "ce:GetCostAndUsage", - "ce:GetCostForecast", - ], - "Resource": "*", - }, ], } - try: - sso_client.put_inline_policy_to_permission_set( - InstanceArn=SSO_INSTANCE_ARN, - PermissionSetArn=ps_arn, - InlinePolicy=json.dumps(policy), - ) - except Exception as e: - logger.error("Failed to set inline policy for %s: %s", ps_name, e) - return {"error": str(e)[:200]} - - # Assign to group and wait for completion - result = _create_account_assignment(ps_arn, group_id, ps_name) - result["permission_set"] = ps_name - # Push policy changes to the role in the account (needed for existing - # assignments; a new assignment provisions on creation) - result["provisioned"] = _provision_permission_set(ps_arn, ps_name) - return result - def _guardrails_state(policy_name): """'ok' if the team guardrails policy exists and was created by platform diff --git a/terraform/lambda-src/tests/test_team_provisioner_memberships.py b/terraform/lambda-src/tests/test_team_provisioner_memberships.py index fe1bd86..5553a86 100644 --- a/terraform/lambda-src/tests/test_team_provisioner_memberships.py +++ b/terraform/lambda-src/tests/test_team_provisioner_memberships.py @@ -63,7 +63,7 @@ def test_prune_removes_gone_groups_but_keeps_team_and_client_rows(self): class TeamPermissionSetTest(unittest.TestCase): - def test_team_permission_set_uses_team_guardrails_and_reprovisions(self): + def _provisioner(self): h = load(FakeDynamo([])) sso = mock.MagicMock() sso.exceptions.ConflictException = type("Conflict", (Exception,), {}) @@ -75,31 +75,96 @@ def test_team_permission_set_uses_team_guardrails_and_reprovisions(self): h._create_account_assignment = lambda *a, **k: {"assigned": True} h.time.sleep = lambda s: None h._guardrails_state = lambda name: "ok" + return h, sso + + def test_team_guardrails_are_the_boundary_not_an_attached_policy(self): + h, sso = self._provisioner() result = h._ensure_team_permission_set("cupcake", "group-1") - attach = sso.attach_customer_managed_policy_reference_to_permission_set.call_args.kwargs - self.assertEqual(attach["CustomerManagedPolicyReference"], {"Name": "javabin-team-cupcake-guardrails"}) - detach = sso.detach_customer_managed_policy_reference_from_permission_set.call_args.kwargs - self.assertEqual(detach["CustomerManagedPolicyReference"], {"Name": "javabin-developer-boundary"}) + boundary = sso.put_permissions_boundary_to_permission_set.call_args.kwargs + self.assertEqual(boundary["PermissionsBoundary"], + {"CustomerManagedPolicyReference": {"Name": "javabin-team-cupcake-guardrails"}}) + # Attached as a policy, the guardrails' baseline "Allow *" granted everything + sso.attach_customer_managed_policy_reference_to_permission_set.assert_not_called() + detached = {c.kwargs["CustomerManagedPolicyReference"]["Name"] + for c in sso.detach_customer_managed_policy_reference_from_permission_set.call_args_list} + self.assertEqual(detached, {"javabin-team-cupcake-guardrails", "javabin-developer-boundary"}) + view_only = sso.attach_managed_policy_to_permission_set.call_args.kwargs + self.assertEqual(view_only["ManagedPolicyArn"], "arn:aws:iam::aws:policy/job-function/ViewOnlyAccess") + self.assertEqual(result["view_only"], "ok") + self.assertEqual(result["provisioned"], {"status": "SUCCEEDED"}) policy = json.loads(sso.put_inline_policy_to_permission_set.call_args.kwargs["InlinePolicy"]) - sids = {st["Sid"] for st in policy["Statement"]} - self.assertTrue({"DenyPlatformManagedParameters", "DenyCreatingParameters"} <= sids) + self.assertEqual(policy, h._team_inline_policy("cupcake")) + # The old policies only go once the boundary is in place + calls = [c[0] for c in sso.mock_calls] + first_detach = calls.index("detach_customer_managed_policy_reference_from_permission_set") + self.assertLess(calls.index("put_permissions_boundary_to_permission_set"), first_detach) + self.assertLess(first_detach, calls.index("provision_permission_set")) + + def test_boundary_failure_leaves_policies_alone(self): + h, sso = self._provisioner() + sso.put_permissions_boundary_to_permission_set.side_effect = RuntimeError("AccessDenied") + + result = h._ensure_team_permission_set("cupcake", "group-1") + + self.assertIn("AccessDenied", result["error"]) + sso.detach_customer_managed_policy_reference_from_permission_set.assert_not_called() + sso.put_inline_policy_to_permission_set.assert_not_called() + sso.provision_permission_set.assert_not_called() + + def test_view_only_failure_still_sets_the_team_policy(self): + h, sso = self._provisioner() + sso.attach_managed_policy_to_permission_set.side_effect = RuntimeError("AccessDenied") + + result = h._ensure_team_permission_set("cupcake", "group-1") + + self.assertTrue(result["view_only"].startswith("error")) + sso.put_inline_policy_to_permission_set.assert_called_once() self.assertEqual(result["provisioned"], {"status": "SUCCEEDED"}) - deny = next(st for st in policy["Statement"] if st["Sid"] == "DenyCreatingParameters") + + def test_inline_policy_allows_only_the_teams_own_resources(self): + h = load(FakeDynamo([])) + h.ACCOUNT_ID = "123456789012" + policy = h._team_inline_policy("cupcake") + st = {s["Sid"]: s for s in policy["Statement"]} + + # No IfExists: a request without the resource's team tag (untagged + # resources, S3 objects, Route53, creates) is not allowed by it + self.assertEqual(st["AllowTeamTaggedResources"]["Condition"], + {"StringEquals": {"aws:ResourceTag/team": "cupcake"}}) + for s in policy["Statement"]: + if s["Effect"] != "Allow" or s["Sid"] == "AllowTeamTaggedResources": + continue + self.assertNotEqual(s["Action"], "*", s["Sid"]) + for action in s["Action"] if isinstance(s["Action"], list) else [s["Action"]]: + self.assertNotIn(action.split(":")[1], ("*", "Delete*", "Put*"), s["Sid"]) + if s["Resource"] == "*": + self.assertEqual(s["Sid"], "AllowUntaggedReads") + self.assertTrue(all(a.split(":")[1].startswith(("List", "Describe", "Get")) + for a in st["AllowUntaggedReads"]["Action"])) + self.assertEqual(st["AllowTeamParameters"]["Resource"], [ + "arn:aws:ssm:*:123456789012:parameter/javabin/apps/cupcake", + "arn:aws:ssm:*:123456789012:parameter/javabin/apps/cupcake/*", + ]) + self.assertEqual(st["AllowTeamLogs"]["Resource"], "arn:aws:logs:*:123456789012:log-group:/ecs/cupcake/*") + # {team}-{name}-{account}: never a legacy bucket like javazone-backup + self.assertEqual(st["AllowTeamBuckets"]["Resource"], [ + "arn:aws:s3:::cupcake-*-123456789012", "arn:aws:s3:::cupcake-*-123456789012/*"]) + self.assertEqual(st["AllowExecOnPlatformCluster"]["Resource"], + "arn:aws:ecs:*:123456789012:cluster/javabin-platform") + deny = st["DenyCreatingParameters"] self.assertEqual(deny["Condition"], {"BoolIfExists": {"ssm:Overwrite": "false"}}) # app-auth-sync's outputs are readable but not writable by the team - managed = next(st for st in policy["Statement"] if st["Sid"] == "DenyPlatformManagedParameters") + managed = st["DenyPlatformManagedParameters"] self.assertTrue(any(r.endswith("/apps/*/*/auth-status") for r in managed["Resource"])) self.assertTrue(any(r.endswith("/apps/*/*/cognito-client-*") for r in managed["Resource"])) + self.assertLess(len(json.dumps(policy)), 32768) # permission set inline limit # ECS Exec: the shared cluster is allowed outright; tasks only via the - # team-tag condition on AllowTeamResources - exec_stmt = next(st for st in policy["Statement"] if st["Sid"] == "AllowExecOnPlatformCluster") + # team-tag condition on AllowTeamTaggedResources + exec_stmt = st["AllowExecOnPlatformCluster"] self.assertEqual(exec_stmt["Action"], "ecs:ExecuteCommand") - self.assertTrue(exec_stmt["Resource"].endswith(":cluster/javabin-platform")) self.assertNotIn("Condition", exec_stmt) - team_stmt = next(st for st in policy["Statement"] if st["Sid"] == "AllowTeamResources") - self.assertEqual(team_stmt["Condition"], {"StringEqualsIfExists": {"aws:ResourceTag/team": "cupcake"}}) def _guardrails_with(self, tags=None, missing=False): h = load(FakeDynamo([])) @@ -118,16 +183,30 @@ def test_guardrails_must_exist_and_be_terraform_managed(self): self.assertTrue(self._guardrails_with(missing=True)._guardrails_state("p").startswith("pending")) self.assertTrue(self._guardrails_with({"managed-by": "me"})._guardrails_state("p").startswith("refused")) - def test_missing_guardrails_leave_the_permission_set_untouched(self): - h = load(FakeDynamo([])) - h.sso_client = mock.MagicMock() - h._resolve_permission_set_arn = lambda name: "arn:ps" - h._guardrails_state = lambda name: "pending: policy not created yet" - result = h._ensure_team_permission_set("newteam", "group-1") - self.assertTrue(result["guardrails"].startswith("pending")) - h.sso_client.detach_customer_managed_policy_reference_from_permission_set.assert_not_called() - h.sso_client.attach_customer_managed_policy_reference_to_permission_set.assert_not_called() - + def test_missing_guardrails_give_read_only_access(self): + # e.g. javabin-team-platform: no guardrails, and the developer + # boundary attached as a policy by older versions (its "Allow *" + # granted everything). Without a boundary, no write access at all. + for state in ("pending: policy not created yet", "refused: not created by platform Terraform"): + h, sso = self._provisioner() + h._guardrails_state = lambda name, state=state: state + + result = h._ensure_team_permission_set("platform", "group-1") + + self.assertEqual(result["guardrails"], state) + sso.put_permissions_boundary_to_permission_set.assert_not_called() + detached = {c.kwargs["CustomerManagedPolicyReference"]["Name"] + for c in sso.detach_customer_managed_policy_reference_from_permission_set.call_args_list} + self.assertEqual(detached, {"javabin-team-platform-guardrails", "javabin-developer-boundary"}) + self.assertEqual(sso.attach_managed_policy_to_permission_set.call_args.kwargs["ManagedPolicyArn"], + "arn:aws:iam::aws:policy/job-function/ViewOnlyAccess") + policy = json.loads(sso.put_inline_policy_to_permission_set.call_args.kwargs["InlinePolicy"]) + self.assertEqual(policy, h._read_only_inline_policy()) + for st in policy["Statement"]: + self.assertEqual(st["Effect"], "Allow") + self.assertTrue(all(a.split(":")[1].startswith(("List", "Describe", "Get")) + for a in st["Action"]), st["Sid"]) + self.assertEqual(result["provisioned"], {"status": "SUCCEEDED"}) if __name__ == "__main__": unittest.main() diff --git a/terraform/platform/iam/boundary.tf b/terraform/platform/iam/boundary.tf index c5ba24c..4fa598e 100644 --- a/terraform/platform/iam/boundary.tf +++ b/terraform/platform/iam/boundary.tf @@ -294,8 +294,9 @@ resource "aws_iam_policy" "developer_boundary" { # never set on users), so ${aws:PrincipalTag/team} is empty for humans and # DenyXTeam would deny everything team-scoped. Each per-team permission set # (javabin-team-{team}, created by team-provisioner) knows its team, so it -# attaches this rendered copy instead. Multi-team heroes pick the team by -# picking the permission set. +# uses this rendered copy as its permissions boundary instead. Never attach +# it as a policy: the baseline "Allow *" would then grant everything the +# Denies miss. Multi-team heroes pick the team by picking the permission set. # # Self-protected by DenyBndMod in the developer boundary (team CI roles # cannot create new versions). diff --git a/terraform/platform/lambdas/main.tf b/terraform/platform/lambdas/main.tf index 3cfb888..e442892 100644 --- a/terraform/platform/lambdas/main.tf +++ b/terraform/platform/lambdas/main.tf @@ -678,19 +678,24 @@ resource "aws_iam_role_policy" "team_provisioner" { "sso:DescribePermissionSet", "sso:CreatePermissionSet", "sso:PutInlinePolicyToPermissionSet", - "sso:AttachCustomerManagedPolicyReferenceToPermissionSet", "sso:DetachCustomerManagedPolicyReferenceFromPermissionSet", + "sso:AttachManagedPolicyToPermissionSet", + "sso:PutPermissionsBoundaryToPermissionSet", "sso:ProvisionPermissionSet", "sso:DescribePermissionSetProvisioningStatus", ] Resource = "*" }, { - # Provisioning a permission set reads its customer-managed policies - Sid = "ReadTeamGuardrails" - Effect = "Allow" - Action = ["iam:GetPolicy", "iam:GetPolicyVersion", "iam:ListPolicyTags"] - Resource = "arn:aws:iam::${var.aws_account_id}:policy/${var.project}-team-*-guardrails" + # Provisioning a permission set reads its policies (team guardrails, + # and the ViewOnlyAccess attached to javabin-team-{team}) + Sid = "ReadTeamGuardrails" + Effect = "Allow" + Action = ["iam:GetPolicy", "iam:GetPolicyVersion", "iam:ListPolicyTags"] + Resource = [ + "arn:aws:iam::${var.aws_account_id}:policy/${var.project}-team-*-guardrails", + "arn:aws:iam::aws:policy/job-function/ViewOnlyAccess", + ] }, { Sid = "IAMForSSOAssignment" @@ -715,6 +720,17 @@ resource "aws_iam_role_policy" "team_provisioner" { "arn:aws:iam::${var.aws_account_id}:saml-provider/AWSSSO_*", ] }, + { + # Provisioning javabin-team-{team} in this (management) account sets + # the team guardrails as boundary of the SSO role with our credentials + Sid = "SetTeamGuardrailsBoundary" + Effect = "Allow" + Action = "iam:PutRolePermissionsBoundary" + Resource = "arn:aws:iam::${var.aws_account_id}:role/aws-reserved/sso.amazonaws.com/*" + Condition = { + ArnLike = { "iam:PermissionsBoundary" = "arn:aws:iam::${var.aws_account_id}:policy/${var.project}-team-*-guardrails" } + } + }, ] }) }