From e7ccf9e774b6c58a2f1e0337619223b7bca34e70 Mon Sep 17 00:00:00 2001 From: Enver Cicak Date: Mon, 30 Jan 2023 14:59:14 +0100 Subject: [PATCH 1/5] Disable force MFA statement by default --- modules/iam-group-with-policies/main.tf | 9 +++++++++ modules/iam-group-with-policies/policies.tf | 3 ++- modules/iam-group-with-policies/variables.tf | 12 ++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/modules/iam-group-with-policies/main.tf b/modules/iam-group-with-policies/main.tf index aa0675a4..ee45d828 100644 --- a/modules/iam-group-with-policies/main.tf +++ b/modules/iam-group-with-policies/main.tf @@ -52,6 +52,15 @@ resource "aws_iam_policy" "iam_self_management" { tags = var.tags } +resource "aws_iam_policy" "force_mfa" { + count = var.attach_force_mfa_policy ? 1 : 0 + + name_prefix = var.force_mfa_policy_name_prefix + policy = data.aws_iam_policy_document.force_mfa.json + + tags = var.tags +} + resource "aws_iam_policy" "custom" { count = length(var.custom_group_policies) diff --git a/modules/iam-group-with-policies/policies.tf b/modules/iam-group-with-policies/policies.tf index c2b7c318..e91d8b9f 100644 --- a/modules/iam-group-with-policies/policies.tf +++ b/modules/iam-group-with-policies/policies.tf @@ -125,9 +125,10 @@ data "aws_iam_policy_document" "iam_self_management" { ] resources = ["arn:${local.partition}:iam::${local.aws_account_id}:user/$${aws:username}"] - } +} +data "aws_iam_policy_document" "force_mfa" { statement { sid = "DenyAllExceptListedIfNoMFA" diff --git a/modules/iam-group-with-policies/variables.tf b/modules/iam-group-with-policies/variables.tf index 7c5d764d..38d3fded 100644 --- a/modules/iam-group-with-policies/variables.tf +++ b/modules/iam-group-with-policies/variables.tf @@ -28,6 +28,18 @@ variable "custom_group_policies" { default = [] } +variable "attach_force_mfa_policy" { + description = "Whether to attach IAM policy which users to use MFA for console and API requests" + type = bool + default = false +} + +variable "force_mfa_policy_name_prefix" { + description = "Name prefix for IAM policy to create with Force MFA statements" + type = string + default = "ForceMFA-" +} + variable "attach_iam_self_management_policy" { description = "Whether to attach IAM policy which allows IAM users to manage their credentials and MFA" type = bool From 1bc56fa9911ec3a54ed8e5fad4a79dc34e4e48a1 Mon Sep 17 00:00:00 2001 From: Enver Cicak Date: Mon, 30 Jan 2023 22:19:47 +0100 Subject: [PATCH 2/5] Attach force mfa policy --- modules/iam-group-with-policies/main.tf | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/iam-group-with-policies/main.tf b/modules/iam-group-with-policies/main.tf index ee45d828..5d325119 100644 --- a/modules/iam-group-with-policies/main.tf +++ b/modules/iam-group-with-policies/main.tf @@ -26,6 +26,13 @@ resource "aws_iam_group_policy_attachment" "iam_self_management" { policy_arn = aws_iam_policy.iam_self_management[0].arn } +resource "aws_iam_group_policy_attachment" "force_mfa" { + count = var.attach_force_mfa_policy ? 1 : 0 + + group = local.group_name + policy_arn = aws_iam_policy.force_mfa[0].arn +} + resource "aws_iam_group_policy_attachment" "custom_arns" { count = length(var.custom_group_policy_arns) From 981a7ce959cebfef49ed973844ba7e17d007d4d3 Mon Sep 17 00:00:00 2001 From: Enver Cicak Date: Mon, 30 Jan 2023 22:20:07 +0100 Subject: [PATCH 3/5] Added test --- examples/iam-group-with-policies/main.tf | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/examples/iam-group-with-policies/main.tf b/examples/iam-group-with-policies/main.tf index e77887bb..abffe003 100644 --- a/examples/iam-group-with-policies/main.tf +++ b/examples/iam-group-with-policies/main.tf @@ -37,6 +37,25 @@ module "iam_group_superadmins" { ] } +##################################################################################### +# IAM group for billing with full Administrator access +##################################################################################### +module "iam_group_force_mfa" { + source = "../../modules/iam-group-with-policies" + + name = "billing" + + group_users = [ + module.iam_user1.iam_user_name, + module.iam_user2.iam_user_name, + ] + + custom_group_policy_arns = [ + "arn:aws:iam::aws:policy/job-function/Billing", + ] + attach_force_mfa_policy = true +} + ##################################################################################### # IAM group for users with custom access ##################################################################################### From 5393fc4387087f0860977660c6b0363f54ed943c Mon Sep 17 00:00:00 2001 From: Enver Cicak Date: Tue, 31 Jan 2023 08:38:11 +0100 Subject: [PATCH 4/5] Update docs --- examples/iam-group-with-policies/README.md | 1 + modules/iam-group-with-policies/README.md | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/examples/iam-group-with-policies/README.md b/examples/iam-group-with-policies/README.md index a01542d1..f0a09ac0 100644 --- a/examples/iam-group-with-policies/README.md +++ b/examples/iam-group-with-policies/README.md @@ -32,6 +32,7 @@ Run `terraform destroy` when you don't need these resources. | Name | Source | Version | |------|--------|---------| +| [iam\_group\_force\_mfa](#module\_iam\_group\_force\_mfa) | ../../modules/iam-group-with-policies | n/a | | [iam\_group\_superadmins](#module\_iam\_group\_superadmins) | ../../modules/iam-group-with-policies | n/a | | [iam\_group\_with\_custom\_policies](#module\_iam\_group\_with\_custom\_policies) | ../../modules/iam-group-with-policies | n/a | | [iam\_user1](#module\_iam\_user1) | ../../modules/iam-user | n/a | diff --git a/modules/iam-group-with-policies/README.md b/modules/iam-group-with-policies/README.md index bbd813fc..d85f517a 100644 --- a/modules/iam-group-with-policies/README.md +++ b/modules/iam-group-with-policies/README.md @@ -28,10 +28,13 @@ No modules. | [aws_iam_group_membership.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_membership) | resource | | [aws_iam_group_policy_attachment.custom](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | | [aws_iam_group_policy_attachment.custom_arns](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | +| [aws_iam_group_policy_attachment.force_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | | [aws_iam_group_policy_attachment.iam_self_management](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | | [aws_iam_policy.custom](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | +| [aws_iam_policy.force_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_iam_policy.iam_self_management](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source | +| [aws_iam_policy_document.force_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_iam_policy_document.iam_self_management](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source | @@ -39,11 +42,13 @@ No modules. | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| +| [attach\_force\_mfa\_policy](#input\_attach\_force\_mfa\_policy) | Whether to attach IAM policy which users to use MFA for console and API requests | `bool` | `false` | no | | [attach\_iam\_self\_management\_policy](#input\_attach\_iam\_self\_management\_policy) | Whether to attach IAM policy which allows IAM users to manage their credentials and MFA | `bool` | `true` | no | | [aws\_account\_id](#input\_aws\_account\_id) | AWS account id to use inside IAM policies. If empty, current AWS account ID will be used. | `string` | `""` | no | | [create\_group](#input\_create\_group) | Whether to create IAM group | `bool` | `true` | no | | [custom\_group\_policies](#input\_custom\_group\_policies) | List of maps of inline IAM policies to attach to IAM group. Should have `name` and `policy` keys in each element. | `list(map(string))` | `[]` | no | | [custom\_group\_policy\_arns](#input\_custom\_group\_policy\_arns) | List of IAM policies ARNs to attach to IAM group | `list(string)` | `[]` | no | +| [force\_mfa\_policy\_name\_prefix](#input\_force\_mfa\_policy\_name\_prefix) | Name prefix for IAM policy to create with Force MFA statements | `string` | `"ForceMFA-"` | no | | [group\_users](#input\_group\_users) | List of IAM users to have in an IAM group which can assume the role | `list(string)` | `[]` | no | | [iam\_self\_management\_policy\_name\_prefix](#input\_iam\_self\_management\_policy\_name\_prefix) | Name prefix for IAM policy to create with IAM self-management permissions | `string` | `"IAMSelfManagement-"` | no | | [name](#input\_name) | Name of IAM group | `string` | `""` | no | From 7101d4308f0991887710f7d3cae2343c9103b693 Mon Sep 17 00:00:00 2001 From: Bryant Biggs Date: Wed, 22 Mar 2023 20:25:31 -0400 Subject: [PATCH 5/5] fix: Allow MFA enforcement to be disabled --- examples/iam-group-with-policies/README.md | 1 - examples/iam-group-with-policies/main.tf | 19 -------- modules/iam-group-with-policies/README.md | 6 +-- modules/iam-group-with-policies/main.tf | 16 ------- modules/iam-group-with-policies/policies.tf | 47 ++++++++++---------- modules/iam-group-with-policies/variables.tf | 12 ++--- 6 files changed, 27 insertions(+), 74 deletions(-) diff --git a/examples/iam-group-with-policies/README.md b/examples/iam-group-with-policies/README.md index f0a09ac0..a01542d1 100644 --- a/examples/iam-group-with-policies/README.md +++ b/examples/iam-group-with-policies/README.md @@ -32,7 +32,6 @@ Run `terraform destroy` when you don't need these resources. | Name | Source | Version | |------|--------|---------| -| [iam\_group\_force\_mfa](#module\_iam\_group\_force\_mfa) | ../../modules/iam-group-with-policies | n/a | | [iam\_group\_superadmins](#module\_iam\_group\_superadmins) | ../../modules/iam-group-with-policies | n/a | | [iam\_group\_with\_custom\_policies](#module\_iam\_group\_with\_custom\_policies) | ../../modules/iam-group-with-policies | n/a | | [iam\_user1](#module\_iam\_user1) | ../../modules/iam-user | n/a | diff --git a/examples/iam-group-with-policies/main.tf b/examples/iam-group-with-policies/main.tf index 45dd2b24..69dacf18 100644 --- a/examples/iam-group-with-policies/main.tf +++ b/examples/iam-group-with-policies/main.tf @@ -38,25 +38,6 @@ module "iam_group_superadmins" { ] } -##################################################################################### -# IAM group for billing with full Administrator access -##################################################################################### -module "iam_group_force_mfa" { - source = "../../modules/iam-group-with-policies" - - name = "billing" - - group_users = [ - module.iam_user1.iam_user_name, - module.iam_user2.iam_user_name, - ] - - custom_group_policy_arns = [ - "arn:aws:iam::aws:policy/job-function/Billing", - ] - attach_force_mfa_policy = true -} - ##################################################################################### # IAM group for users with custom access ##################################################################################### diff --git a/modules/iam-group-with-policies/README.md b/modules/iam-group-with-policies/README.md index d85f517a..02497251 100644 --- a/modules/iam-group-with-policies/README.md +++ b/modules/iam-group-with-policies/README.md @@ -28,13 +28,10 @@ No modules. | [aws_iam_group_membership.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_membership) | resource | | [aws_iam_group_policy_attachment.custom](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | | [aws_iam_group_policy_attachment.custom_arns](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | -| [aws_iam_group_policy_attachment.force_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | | [aws_iam_group_policy_attachment.iam_self_management](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_group_policy_attachment) | resource | | [aws_iam_policy.custom](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | -| [aws_iam_policy.force_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_iam_policy.iam_self_management](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource | | [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source | -| [aws_iam_policy_document.force_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_iam_policy_document.iam_self_management](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source | @@ -42,13 +39,12 @@ No modules. | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| -| [attach\_force\_mfa\_policy](#input\_attach\_force\_mfa\_policy) | Whether to attach IAM policy which users to use MFA for console and API requests | `bool` | `false` | no | | [attach\_iam\_self\_management\_policy](#input\_attach\_iam\_self\_management\_policy) | Whether to attach IAM policy which allows IAM users to manage their credentials and MFA | `bool` | `true` | no | | [aws\_account\_id](#input\_aws\_account\_id) | AWS account id to use inside IAM policies. If empty, current AWS account ID will be used. | `string` | `""` | no | | [create\_group](#input\_create\_group) | Whether to create IAM group | `bool` | `true` | no | | [custom\_group\_policies](#input\_custom\_group\_policies) | List of maps of inline IAM policies to attach to IAM group. Should have `name` and `policy` keys in each element. | `list(map(string))` | `[]` | no | | [custom\_group\_policy\_arns](#input\_custom\_group\_policy\_arns) | List of IAM policies ARNs to attach to IAM group | `list(string)` | `[]` | no | -| [force\_mfa\_policy\_name\_prefix](#input\_force\_mfa\_policy\_name\_prefix) | Name prefix for IAM policy to create with Force MFA statements | `string` | `"ForceMFA-"` | no | +| [enable\_mfa\_enforcment](#input\_enable\_mfa\_enforcment) | Determines whether permissions are added to the policy which requires the groups IAM users to use MFA | `bool` | `true` | no | | [group\_users](#input\_group\_users) | List of IAM users to have in an IAM group which can assume the role | `list(string)` | `[]` | no | | [iam\_self\_management\_policy\_name\_prefix](#input\_iam\_self\_management\_policy\_name\_prefix) | Name prefix for IAM policy to create with IAM self-management permissions | `string` | `"IAMSelfManagement-"` | no | | [name](#input\_name) | Name of IAM group | `string` | `""` | no | diff --git a/modules/iam-group-with-policies/main.tf b/modules/iam-group-with-policies/main.tf index 5d325119..aa0675a4 100644 --- a/modules/iam-group-with-policies/main.tf +++ b/modules/iam-group-with-policies/main.tf @@ -26,13 +26,6 @@ resource "aws_iam_group_policy_attachment" "iam_self_management" { policy_arn = aws_iam_policy.iam_self_management[0].arn } -resource "aws_iam_group_policy_attachment" "force_mfa" { - count = var.attach_force_mfa_policy ? 1 : 0 - - group = local.group_name - policy_arn = aws_iam_policy.force_mfa[0].arn -} - resource "aws_iam_group_policy_attachment" "custom_arns" { count = length(var.custom_group_policy_arns) @@ -59,15 +52,6 @@ resource "aws_iam_policy" "iam_self_management" { tags = var.tags } -resource "aws_iam_policy" "force_mfa" { - count = var.attach_force_mfa_policy ? 1 : 0 - - name_prefix = var.force_mfa_policy_name_prefix - policy = data.aws_iam_policy_document.force_mfa.json - - tags = var.tags -} - resource "aws_iam_policy" "custom" { count = length(var.custom_group_policies) diff --git a/modules/iam-group-with-policies/policies.tf b/modules/iam-group-with-policies/policies.tf index a8f95905..c3763b8e 100644 --- a/modules/iam-group-with-policies/policies.tf +++ b/modules/iam-group-with-policies/policies.tf @@ -144,31 +144,30 @@ data "aws_iam_policy_document" "iam_self_management" { "arn:${local.partition}:iam::${local.aws_account_id}:user/*/$${aws:username}" ] } -} - -data "aws_iam_policy_document" "force_mfa" { - statement { - sid = "DenyAllExceptListedIfNoMFA" - - effect = "Deny" - - not_actions = [ - "iam:ChangePassword", - "iam:CreateVirtualMFADevice", - "iam:EnableMFADevice", - "iam:GetUser", - "iam:ListMFADevices", - "iam:ListVirtualMFADevices", - "iam:ResyncMFADevice", - "sts:GetSessionToken" - ] - - resources = ["*"] - condition { - test = "BoolIfExists" - variable = "aws:MultiFactorAuthPresent" - values = ["false"] + dynamic "statement" { + for_each = var.enable_mfa_enforcment ? [1] : [] + + content { + sid = "DenyAllExceptListedIfNoMFA" + effect = "Deny" + not_actions = [ + "iam:ChangePassword", + "iam:CreateVirtualMFADevice", + "iam:EnableMFADevice", + "iam:GetUser", + "iam:ListMFADevices", + "iam:ListVirtualMFADevices", + "iam:ResyncMFADevice", + "sts:GetSessionToken" + ] + resources = ["*"] + + condition { + test = "BoolIfExists" + variable = "aws:MultiFactorAuthPresent" + values = ["false"] + } } } } diff --git a/modules/iam-group-with-policies/variables.tf b/modules/iam-group-with-policies/variables.tf index 38d3fded..4f75fc21 100644 --- a/modules/iam-group-with-policies/variables.tf +++ b/modules/iam-group-with-policies/variables.tf @@ -28,16 +28,10 @@ variable "custom_group_policies" { default = [] } -variable "attach_force_mfa_policy" { - description = "Whether to attach IAM policy which users to use MFA for console and API requests" +variable "enable_mfa_enforcment" { + description = "Determines whether permissions are added to the policy which requires the groups IAM users to use MFA" type = bool - default = false -} - -variable "force_mfa_policy_name_prefix" { - description = "Name prefix for IAM policy to create with Force MFA statements" - type = string - default = "ForceMFA-" + default = true } variable "attach_iam_self_management_policy" {