terraform-aws-control_tower_account_factory icon indicating copy to clipboard operation
terraform-aws-control_tower_account_factory copied to clipboard

Feature Request: SSO Group assignment

Open PeterBengtson opened this issue 3 years ago • 14 comments
trafficstars

When Control Tower creates a new account, it will assign three SSO Groups to it – AWSSecurityAuditors, AWSSecurityAuditPowerUsers, and AWSControlTowerAdmins. The permission sets assigned to the new account vary a little, but except for the core accounts, they are AWSReadOnlyAccess, AWSPowerUserAccess, and AWSOrganizationsFullAccess, respectively. In addition to this, the SSO User is of course given AWSSystemAdministrator access.

This means that any other groups will have to be assigned manually. AFT could come to the rescue here and supply the last missing piece of account management.

In addition to the account_tags, change_parameters, and custom_fields parameters, I propose a new additional_sso_groups parameter, e.g.:

    additional_sso_groups = {
        "Foos" = "SomePermissionSet"
        "Bars" = "AnotherPermissionSet"
    }

This would add the SSO Groups Foos and Bars to the standard three for the account in question, with the given Permission Sets.

SSO Users could also be specified this way, but I doubt that it would be as useful as SSO Group assignments. SSO User assignment is somewhat of an anti-pattern anyway.

Implementation is easy and would use the sso-admin API endpoints create-account-assignment and delete-account-assignment to manage the additional SSO Groups to be added to the account (in addition to the standard three which would always be there).

PeterBengtson avatar Feb 22 '22 12:02 PeterBengtson

Thanks for the feature request here @PeterBengtson, I've gone ahead and opened a backlog to approach this for the team

balltrev avatar Feb 22 '22 21:02 balltrev

Hi Yes, that would be definitely nice feature to have, for now I have TF in aft-global-customizations repo to assign AdministratorAccess for our SSO ORG-Admins Group on every AFT enrolled account

abelomestny avatar May 04 '22 17:05 abelomestny

@abelomestny - Would you possibly be able to share how you've done this so far please?

andy-townsend avatar May 18 '22 18:05 andy-townsend

Hi @andy-townsend

data "aws_ssoadmin_instances" "example" {}

data "aws_ssoadmin_permission_set" "admin" {
  instance_arn = tolist(data.aws_ssoadmin_instances.example.arns)[0]
  name         = "AWSAdministratorAccess"
}

data "aws_identitystore_group" "tsg_aws_org_admins" {
  identity_store_id = tolist(data.aws_ssoadmin_instances.example.identity_store_ids)[0]

  filter {
    attribute_path  = "DisplayName"
    attribute_value = "<Your_ORG_Admin_SSO_Group_Name>"
  }
}

resource "aws_ssoadmin_account_assignment" "org_admins" {
  instance_arn       = data.aws_ssoadmin_permission_set.admin.instance_arn
  permission_set_arn = data.aws_ssoadmin_permission_set.admin.arn

  principal_id   = data.aws_identitystore_group.tsg_aws_org_admins.group_id
  principal_type = "GROUP"

  target_id   = var.target_account
  target_type = "AWS_ACCOUNT"
}

this is in /terraform/modules/permission-set/main.tf the only var I'm passing for this task is <target_account> = $VENDED_ACCOUNT_ID which you have as env var in your CodeBuild. Also make sure your role has permission to execute sso:ListInstances task

abelomestny avatar May 18 '22 21:05 abelomestny

Great, thanks @abelomestny

In terms of the role, is that the AWSAFTAdmin or AWSAFTExecution one that needs updating if this is running in the global-customisations repo? From what I can see, AWSAFTExecution has : and the AWSAFTAdmin simply has permissions to assume that execution role within the account?

andy-townsend avatar May 19 '22 10:05 andy-townsend

it is AWSAFTExecution I defined CT_EXEC_ROLE_ARN in buildspecs template modules/aft-customizations/buildspecs/aft-global-customizations-terraform.yml Screen Shot 2022-05-19 at 7 18 19 PM then reassigned for aft-provider.jinja target_admin_role_arn=$VENDED_EXEC_ROLE_ARN into target_admin_role_arn=$CT_EXEC_ROLE_ARN

abelomestny avatar May 19 '22 23:05 abelomestny

I'm interested in doing something similar. @abelomestny does that mean you run your own fork of AFT to accomodate the change?

eriweb avatar May 20 '22 06:05 eriweb

Hey @terbolous I can not fork it since the target repo has to be Private one within our ORG. That's why I can't refer to the code here directly :-) But I have cloned it within our ORG and set up upstream repo to aws-ia, so I can easily commit my changes and should be able to catch up with any releases aws-ia would have from upstream repo.

abelomestny avatar May 20 '22 15:05 abelomestny

...

this is in /terraform/modules/permission-set/main.tf the only var I'm passing for this task is <target_account> = $VENDED_ACCOUNT_ID which you have as env var in your CodeBuild. Also make sure your role has permission to execute sso:ListInstances task

I'm wondering, where can I add/alter the env vars in CodeBuild after AFT initial deployment? So you cloned your version and altered the codepipeline sources ".terraform/modules/aft/modules/aft-customizations/buildspecs/aft-*.yml" for that?

dabo-devconsole avatar Jul 07 '22 10:07 dabo-devconsole

As the OP of this request, I'm checking in after a few months to see whether anything has happened to this proposed feature.

I'd also like to point out that the assignment of permission sets is more complicated if SSO delegation has been enabled, for instance to the AFT management account. If this has been done, then the SSO behaviour of refusing any assignment of any permission set already assigned from the main account will become a problem. And Control Tower as well as AFT, as it uses Service Catalog, will assign the aforementioned three permission sets - AWSSecurityAuditors, AWSSecurityAuditPowerUsers, and AWSControlTowerAdmins - to a newly created account. And this means that those permission sets can never be used from the delegated SSO admin account. That's tricky. And unclean.

I need to solve the automation problem right now. I can't afford to wait any longer. Unless this feature is soon to be released, the only workaround is to create a SNS-triggered lambda in the main org account which can be used from an AFT customisation to trigger the permission set assignments. I've just confirmed with AWS that this is the only way it can be done when delegation is used.

PeterBengtson avatar Aug 02 '22 09:08 PeterBengtson

I couldn't wait any longer. Here's my generalised solution:

https://github.com/PeterBengtson/AFT-SSO-account-configuration

PeterBengtson avatar Aug 05 '22 12:08 PeterBengtson

@balltrev - Can I ask where this sits on the teams backlog please? It's over a year now since it was first raised and I'm hoping to achieve this feature natively rather than rely on adding additional sam projects. If we could get an indication of when its likely to be ready then that'd be great.

andy-townsend avatar Mar 22 '23 11:03 andy-townsend

@andy-townsend unfortunately we do not have any update on this feature request.

snebhu3 avatar Mar 22 '23 17:03 snebhu3

this is an old issue with several workarounds, I'd like to share a workaround I'm using that doesn't require a fork of the module which might be easier for people to plug in.

in aft-global-customizations change aft-providers.jinja to the following:

## Auto generated providers.tf ##
## Updated on: {{ timestamp }} ##

locals {
  default_tags = {
    # put your default tags here
    managed_by = "AFT"
  }
}

provider "aws" {
  region = "{{ provider_region }}"
  assume_role {
    role_arn = "{{ target_admin_role_arn }}"
  }
  default_tags {
    tags = local.default_tags
  }
}

provider "aws" {
  alias  = "aft_management"
  region = "{{ provider_region }}"
  assume_role {
    role_arn = "{{ aft_admin_role_arn }}"
  }
  default_tags {
    tags = local.default_tags
  }
}

data "aws_ssm_parameter" "ct_management_account_id" {
  provider = aws.aft_management

  name = "/aft/account/ct-management/account-id"
}

provider "aws" {
  alias  = "ct_management"
  region = "{{ provider_region }}"
  assume_role {
    role_arn = "arn:aws:iam::${data.aws_ssm_parameter.ct_management_account_id.value}:role/AWSAFTExecution"
  }
  default_tags {
    tags = local.default_tags
  }
}

note: if {{ provider_region }} is different to your AWS SSO region in the CT management account then you will need to hard-code this into the ct_management aws provider or store the region as an SSM parameter and pull it via a data source the same as how the ct management account id is retrieved.


you can then use the ct_management aws provider to deploy AWS SSO Terraform resources.

here's an example that I'm using to automatically assign a group I created called AWSAdministratorAccess

# example main.tf

data "aws_caller_identity" "current" {}

data "aws_ssoadmin_instances" "this" {
  provider = aws.ct_management
}

data "aws_ssoadmin_permission_set" "aws_administrator_access" {
  provider = aws.ct_management

  instance_arn = tolist(data.aws_ssoadmin_instances.this.arns)[0]
  name         = "AWSAdministratorAccess"
}

data "aws_identitystore_group" "aws_administrator_access" {
  provider = aws.ct_management

  identity_store_id = tolist(data.aws_ssoadmin_instances.this.identity_store_ids)[0]

  alternate_identifier {
    unique_attribute {
      attribute_path  = "DisplayName"
      attribute_value = "AWSAdministratorAccess"
    }
  }
}

resource "aws_ssoadmin_account_assignment" "aws_administrator_access" {
  provider = aws.ct_management

  instance_arn       = tolist(data.aws_ssoadmin_instances.this.arns)[0]
  permission_set_arn = data.aws_ssoadmin_permission_set.aws_administrator_access.arn

  principal_id   = data.aws_identitystore_group.aws_administrator_access.group_id
  principal_type = "GROUP"

  target_id   = data.aws_caller_identity.current.account_id
  target_type = "AWS_ACCOUNT"
}

You could get clever with this and pull the custom-fields SSM parameter and automatically assign users/groups based on that. I hope this helps someone! IMO this is a fine workaround and I'm wondering if this should be documented as I think this is a pretty common use case.

kieranbrown avatar Jan 08 '24 21:01 kieranbrown