Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provider/aws: aws_iam_user_login_profile resource #9605

Merged
merged 8 commits into from
Oct 26, 2016

Conversation

jen20
Copy link
Contributor

@jen20 jen20 commented Oct 25, 2016

This commit introduces an aws_iam_user_login_profile resource which creates a password for an IAM user, and encrypts it using a PGP key specified in the configuration or obtained from Keybase.

For example:

resource "aws_iam_user" "u" {
       name = "auser"
       path = "/"
       force_destroy = true
}

resource "aws_iam_user_login_profile" "u" {
       user = "${aws_iam_user.u.name}"
       pgp_key = "keybase:some_person_that_exists"
}

output "password" {
value = "${aws_iam_user_login_profile.u.encrypted_password}"
}

The resulting attribute "encrypted_password" can be decrypted using PGP or Keybase - for example:

terraform output password | base64 --decode | keybase pgp decrypt

Optionally the user can retain the password rather than the default of being forced to change it at first login. Generated passwords are currently 20 characters long by default, though this can be overriden.

It is unclear how to test this correctly - we should discuss!

This commit implements reusable functions for when resources have no
need to implement a particular operation:

- Noop - does nothing and returns no error.
- RemoveFromState - sets the resource ID to empty string (removing it
  from state) and returns no error.
This commit introduces an `aws_iam_user_login_profile` resource which
creates a password for an IAM user, and encrypts it using a PGP key
specified in the configuration or obtained from Keybase.

For example:

```
resource "aws_iam_user" "u" {
        name = "auser"
        path = "/"
        force_destroy = true
}

resource "aws_iam_user_login_profile" "u" {
        user = "${aws_iam_user.u.name}"
        pgp_key = "keybase:some_person_that_exists"
}

output "password" {
	value = "${aws_iam_user_login_profile.u.encrypted_password}"
}
```

The resulting attribute "encrypted_password" can be decrypted using
PGP or Keybase - for example:

```
terraform output password | base64 --decode | keybase pgp decrypt
```

Optionally the user can retain the password rather than the default of
being forced to change it at first login. Generated passwords are
currently 20 characters long.
This commit modifies password generation such that it is highly likely
to match any AWS password policy.
If an IAM user already has a login profile, we bring it under management
- we will NOT modify it - but we cannot set the password.
@mitchellh
Copy link
Contributor

Looks great, but I think the following should be done:

Destroy: DeleteLoginProfile exists and I think we should support it. It looks simple enough. If its useful, you can copy the delete_on_destroy type setting from Nomad provider so the user can choose to disable destroying it. But I think for testing reasons this is important.

Exists: GetLoginProfile exists and should be used to verify a login profile exists.

Testing: I think the easiest way to test this is just a basic create/destory test that just verifies it succeeds. We could go more hardcore and actually use the resulting password to try to run more API calls. I don't think its necessary but if you want.

We can probably skip update for now.

Add a test with a bad explicitly specified GPG key and a keybase user
(that we own) with no public keys.
@jen20
Copy link
Contributor Author

jen20 commented Oct 25, 2016

Hi @mitchellh! I added some acceptance tests that verify the behaviour for good and bad keys. For the non-keybase keys we verify that the password is usable (by changing it to something else random).

I don't think we should implement destroy (and thus exists is not necessary) - there is too much risk of removing a user's login credentials, and aws_iam_user will remove it at any rate, if force_destroy is specified (in the same manner as access keys).

Acceptance test run:

make testacc TEST=./builtin/providers/aws TESTARGS="-run TestAccAWSUserLoginProfile"
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/25 16:28:13 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run TestAccAWSUserLoginProfile -timeout 120m
=== RUN   TestAccAWSUserLoginProfile_basic
--- PASS: TestAccAWSUserLoginProfile_basic (22.80s)
=== RUN   TestAccAWSUserLoginProfile_keybase
--- PASS: TestAccAWSUserLoginProfile_keybase (40.11s)
=== RUN   TestAccAWSUserLoginProfile_keybaseDoesntExist
--- PASS: TestAccAWSUserLoginProfile_keybaseDoesntExist (11.65s)
=== RUN   TestAccAWSUserLoginProfile_notAKey
--- PASS: TestAccAWSUserLoginProfile_notAKey (7.19s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    81.773s

@mitchellh
Copy link
Contributor

Awesome, LGTM!

@ghost
Copy link

ghost commented Apr 18, 2020

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants