terraform-aws-modules / terraform-aws-iam

Terraform module to create AWS IAM resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/iam/aws
Apache License 2.0
787 stars 996 forks source link

MFA test condition is Bool where it should be BoolIfExists #516

Open DutchEllie opened 1 month ago

DutchEllie commented 1 month ago

Description

In the IAM assumable role module, the option to create a role with an MFA condition is not working properly when using long-term credentials. The condition to check if MFA is enabled is currently Bool, which fails when the aws:MultiFactorAuthPresent variable is not present (such as when running with long-term credentials using the AWS CLI). AWS strongly recommends not doing this, as this breaks the above use case of the AWS CLI.

Instead, they recommend that you use the BoolIfExists operator to check this condition. Therefor I suggest that you change the operator to BoolIfExists. In addition, I suggest changing the operator for the aws:MultiFactorAuthAge condition to NumericLessThanIfExists to make sure this does not fail either when using a long-term credential type.

Versions

Reproduction Code

I understand that you ask for "code that works without modifications", but uh no.. I will redact things.

module "describecluster_policy" {
  source = "terraform-aws-modules/iam/aws//modules/iam-policy"
  version = ">= 5.44.0, < 6.0.0"

  name        = "describecluster"
  path        = "/"
  description = "Policy with the DescribeCluster permission for the cluster"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": "eks:DescribeCluster",
      "Resource": "<Redacted>"
    }
  ]
}
EOF

}

module "cluster_admin_role" {
  source      = "terraform-aws-modules/iam/aws//modules/iam-assumable-role"
  version = ">= 5.44.0, < 6.0.0"
  create_role = true

  trusted_role_arns = [
    "<Redacted>",
  ]
  role_name         = "cluster_admin_role"
  role_requires_mfa = true

  custom_role_policy_arns = [
    module.describecluster_policy.arn
  ]
}

Steps to reproduce the behavior:

  1. It doesn't matter how you set up the describecluster policy, as long as you can assume the role it's fine.
  2. Set up your user's arn in the trusted_role_arns array.
  3. Deploy the code
  4. Use long-term credentials to try and assume the role (via the CLI for example)

Expected behavior

I expect that using the role_requires_mfa makes it so that I can actually use the role using my terminal as well.
I also expect that this AWS module follows AWS's recommendations.

Following from that, I expect the operation of assuming the role to succeed.

Actual behavior

Assuming the role doesn't succeed. I don't have access, because long-term credentials don't have the aws:MultiFactorAuthPresent condition set to anything, so the Bool operator fails.

github-actions[bot] commented 2 days ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days