MFA enforcement across the three roles

Current behaviour

Enforcement is all-or-nothing by design. SetMFAEnforcement (internal/auth/bootstrap.go) builds one trust policy and applies it to every role in auth.Roles — keel-admin, keel-developer, keel-viewer. There is no per-role toggle, and keel auth mfa enforce exposes none: it turns the requirement on for all three, and --disable turns it off for all three.

So "MFA should be required for every role" is already the intent, and the normal states are all enforced or none enforced.

MFAEnforcement reads the state back per role, which is why the dashboard's Team tab reports it that way. A tab showing MFA not required for: keel-developer is reporting a real drift, not a supported configuration.

The gap: enforcement is not atomic

SetMFAEnforcement iterates the roles and returns on the first error:

for _, role := range Roles {
    if _, err := client.UpdateAssumeRolePolicy(...); err != nil {
        return fmt.Errorf("updating trust policy for role %s: %w", roleName, err)
    }
}

IAM has no transaction across three UpdateAssumeRolePolicy calls, so a failure on the second one — a throttle, a transient permission problem, a dropped connection — leaves the first role enforced and the remaining ones open. keel auth mfa enforce propagates the error and stops. It does not say which roles it managed to update before failing, and nothing re-checks or repairs the state afterwards.

The consequence is worth being precise about. Partial enforcement is not a security hole in the roles that were updated; it is a hole in the ones that were not, because a member assigned to an unenforced role can assume it with their access key alone. Since keel auth team add can assign any of the three roles, the practical exposure is the same as never having run enforce.

Until the Team tab, the only way to notice was to read three trust policies by hand.

Other ways drift can happen

• Someone edits a Keel role's trust policy directly in IAM. keel-admin holds iam:UpdateAssumeRolePolicy, which mfa enforce needs, so an admin session can do this.
• Bootstrap deliberately preserves whatever enforcement a role already has and only applies requireMFA to roles it creates (see CLAUDE.md). That is correct — re-running setup must never silently turn enforcement on — but it also means a role recreated while the others are enforced comes back open.

TODO(auth-hardening): make enforcement converge

Not fixed yet. What it needs:

1. Report partial application. SetMFAEnforcement should attempt every role and return which ones succeeded and which failed, rather than stopping at the first error. A caller that knows it updated one of three can say so; one that gets a bare error cannot.
2. Make enforce idempotent and safe to re-run. It already is in effect — applying the same trust policy twice is harmless — but it should read the state first and report "already enforced" rather than rewriting three policies, so re-running after a partial failure is an obvious repair rather than a guess.
3. Detect drift without being asked. keel auth mfa status reads enforcement per role already; it should call out a mixed state explicitly rather than printing a table the reader has to compare. The dashboard's Team tab does this now, and the CLI should match it.
4. Decide whether keel up/deploy should warn. A partially enforced account is the kind of thing worth surfacing on a normal command, not only when someone thinks to look. Against that: warnings on every deploy get ignored. Probably a one-line note in keel auth whoami rather than on the deploy path.

None of this changes the three invariants in internal/auth/mfa_policy_test.go, and any change here must keep them green.

Related

• CLAUDE.md — the three invariants holding the auth model up, and why Bootstrap preserves existing enforcement.
• README.md — "Turning on MFA for a team that already exists", the ordered migration that mfa open and mfa enforce implement.
• internal/auth/mfa_policy_test.go — the trust-policy condition is asserted on the allow side deliberately: aws:MultiFactorAuthPresent is absent rather than false for long-term credentials, so an allow-side Bool denies when it is missing, whereas a Deny on "false" would never fire.