From 2963e7c6b9fa52efb9ad50abf7452c5663b7f4e3 Mon Sep 17 00:00:00 2001
From: Iain Samuel McLean Elder <iain@isme.es>
Date: Wed, 18 Oct 2023 15:26:54 +0200
Subject: [PATCH] Exclude inactive accounts in lookup cache

The `lookup_accounts_for_ou` function yields accounts in two branches. Branch 1
handles uncached accounts and branch 2 handles cached accounts.

PR #81 added a check to exclude inactive accounts in branch 1 without adding
the same check to branch 2.

In that way it solved #80 but only when the OU containing a suspended account
doesn't repeat.

This PR copies the check from branch 1 to branch 2 for consistent behavior in a
template with many assgnment groups to the same target OU.

The second assignment group no longer generates an assignment for a suspended
account, which causes CloudFormation to fail with an error like this:

```text
Resource handler returned message: "Error occurred during operation 'Request REDACTED failed due to:
AWS SSO is unable to complete your request at this time.
Obtaining permissions to manage your AWS account 'REDACTED' is taking longer than usual.
```

Test the deployed macro with a template like this:

```yaml
AWSTemplateFormatVersion: "2010-09-09"
Transform: AWS-SSO-Util-2020-11-08
Resources:
  Test:
    Type: SSOUtil::SSO::AssignmentGroup
    Properties:
      Name: MacroTest
      Principal:
        - Type: GROUP
          Id:
            - 11111111-1111-1111-1111-111111111111
      PermissionSet:
        - arn:aws:sso:::permissionSet/ssoins-2222222222222222/ps-3333333333333333
      Target:
        - { Type: AWS_OU, Id: r-4444 }
  Test2:
    Type: SSOUtil::SSO::AssignmentGroup
    Properties:
      Name: MacroTest2
      Principal:
        - Type: GROUP
          Id:
            - 55555555-5555-5555-5555-555555555555
      PermissionSet:
        - arn:aws:sso:::permissionSet/ssoins-2222222222222222/ps-3333333333333333
      Target:
        - { Type: AWS_OU, Id: r-4444 }
```

Consider an organization with active member accounts `111111111111` and `222222222222` and suspneded member account `333333333333`.

Before this change, the CloudWatch logs group shows the following message for
the first assignment group. It lists just active accounts as targets.

```text
[DEBUG]	2023-10-17T13:13:49.592Z	eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee	Got targets:
[ACCOUNT:111111111111[r-4444], ACCOUNT:222222222222[r-4444]
```

The group shows the following message for the second assignment group. It lists
the active and suspended accounts as targets. This is incorrect behavior and
later causes the CloudFormation error.

```text
[DEBUG]	2023-10-17T13:13:49.593Z	334ac992-cf53-4af0-adac-7aa15704a573	Got targets:
[ACCOUNT:111111111111[r-4444], ACCOUNT:222222222222[r-4444], ACCOUNT:333333333333[r-4444]]
```

After this change, each assignment group logs just the active accounts as targets.
---
 lib/aws_sso_lib/lookup.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/aws_sso_lib/lookup.py b/lib/aws_sso_lib/lookup.py
index 7cbdce0..4599145 100644
--- a/lib/aws_sso_lib/lookup.py
+++ b/lib/aws_sso_lib/lookup.py
@@ -557,6 +557,9 @@ def lookup_accounts_for_ou(session, ou, *, recursive, refresh=False, cache=None,
         else:
             LOGGER.debug(f"Loaded cached accounts for {ou_type} {ou}: (empty list)")
         for account in cache[ou_accounts_key]:
+            if exclude_inactive_accts and account["Status"] != "ACTIVE":
+                LOGGER.debug(f"Skipping account {account['Id']}: {account['Status']}")
+                continue
             if org_mgmt_acct and account["Id"] == org_mgmt_acct:
                 continue
             yield account