feat: Allow multiple MFA devices and users to manage MFA devices #313
Conversation
| @@ -43,7 +43,7 @@ data "aws_iam_policy_document" "iam_self_management" { | |||
| resources = [ | |||
| "arn:${local.partition}:iam::${local.aws_account_id}:user/*/$${aws:username}", | |||
| "arn:${local.partition}:iam::${local.aws_account_id}:user/$${aws:username}", | |||
| "arn:${local.partition}:iam::${local.aws_account_id}:mfa/$${aws:username}", | |||
There was a problem hiding this comment.
I think we should make this a variable where the default is the current behavior, but users can override with a wildcard for multiple MFA devices
There was a problem hiding this comment.
I will take another look to confirm (create a new IAM user and try to add a virtual MFA device with current IAMSelfManagement policy).
But it seems to me that since this new feature was released, the very first MFA device resource ARN take the name defined by user during his creation : arn:aws:iam::<aws_account_id>:mfa/<virtual_mfa_device_name>
There was a problem hiding this comment.
Sorry for not getting back to you sooner, but after a new test, I can confirm the issue.
I have tested the following scenarios from a brand-new IAM user deployed with the module :
- Add virtual MFA device : 👎
- Remove existing virtual MFA device : 👎
After modifying the IAMSelfManagement policy with arn:aws:iam::<aws_account_id>:mfa/* :
- Add virtual MFA device : 👍
- Remove existing virtual MFA device when logged in without MFA : not working, that's normal 👍
- Remove virtual MFA device : 👍
The day after the PR, I noticed that the AWS documentation was updated : see AllowManageOwnVirtualMFADevice statement.
But this is not updated everywhere yet.
There was a problem hiding this comment.
awesome, thank you for investigating!
- I see, you are absolutely correct on the wildcard
- However, we need to really update our policy. Could you update to match what they have provided in that doc which is more scoped now when we introduce wildcards. Note: I didn't fully check/compare so please do that - but we should look to split out to multiple statements based on the resource being specified
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "AllowViewAccountInfo",
"Effect": "Allow",
"Action": "iam:ListVirtualMFADevices",
"Resource": "*"
},
{
"Sid": "AllowManageOwnVirtualMFADevice",
"Effect": "Allow",
"Action": [
"iam:CreateVirtualMFADevice"
],
"Resource": "arn:aws:iam::*:mfa/*"
},
{
"Sid": "AllowManageOwnUserMFA",
"Effect": "Allow",
"Action": [
"iam:DeactivateMFADevice",
"iam:EnableMFADevice",
"iam:GetUser",
"iam:ListMFADevices",
"iam:ResyncMFADevice"
],
"Resource": "arn:aws:iam::*:user/${aws:username}"
},
{
"Sid": "DenyAllExceptListedIfNoMFA",
"Effect": "Deny",
"NotAction": [
"iam:CreateVirtualMFADevice",
"iam:EnableMFADevice",
"iam:GetUser",
"iam:ListMFADevices",
"iam:ListVirtualMFADevices",
"iam:ResyncMFADevice",
"sts:GetSessionToken"
],
"Resource": "*",
"Condition": {
"BoolIfExists": {"aws:MultiFactorAuthPresent": "false"}
}
}
]
}There was a problem hiding this comment.
To have a more scoped policy with no impacts, I made few changes :
- Rollback previous changes (
AllowSelfManagement&AllowDeactivateMFADevicestatements) - Add 2 more explicit statements :
AllowManageOwnVirtualMFADevice=> to allow actioniam:CreateVirtualMFADevicewith the correct resource nameAllowDeleteVirtualMFADevice=> to allow the action with the correct resource name
There was a problem hiding this comment.
Hello,
I made few changes here, according to this AWS documentation .
The AWS policy allows IAM users that are authenticated using multi-factor authentication (MFA) to manage their own credentials on the My security credentials page. It also requires the user to set up and authenticate using MFA before performing any other operations in AWS !
In real terms, that enforce MFA utilization. Because the IAM user cannot do anything before his MFA is configured, and he is authenticated with.
This is a great security improvement but this is a big change too. Is it what we want ?
There was a problem hiding this comment.
@bryantbiggs Are you OK with that ?
If you're not, I can try to add a new variable to allow the user to choose between the standard policy (which do not force MFA utilization) or the more secure (which force MFA).
There was a problem hiding this comment.
yes, I think this looks great and a good improvement; thank you
There was a problem hiding this comment.
This looks great, thank you - @antonbabenko what do you think?
bryantbiggs
changed the title
rballan
changed the title
## [5.11.0](v5.10.0...v5.11.0) (2023-01-19) ### Features * Allow multiple MFA devices and users to manage MFA devices ([#313](#313)) ([57a5d70](57a5d70))
|
This PR is included in version 5.11.0 🎉 |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Fix the
IAMSelfManagementpolicy to allow virtual MFA devices creation.Due to this new feature, the resource name of virtual MFA devices change and the policy doesn't work anymore.
The wildcard in the MFA resource name is required now, because the user must specify a name for his virtual MFA device, and this is a part of the resource name.
Motivation and Context
Breaking Changes
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull requestIAMSelfManagementpolicy and added successfully a new virtual MFA device.