hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.29k stars 1.72k forks source link

Remove roles for a member (remove-iam-policy-binding) #11724

Closed edwardsPaul421 closed 4 months ago

edwardsPaul421 commented 2 years ago

If you have a support request or question please submit them to one of these resources:

I want to modify a default service account permissions (Default='roles/editor'). I want to remove the 'editor' permission. Is there any workaround to do it?

I didn't find any implementation for remove-iam-policy-binding

References

ScottSuarez commented 2 years ago

If the permission is inherited you'll have to change the default wherever it was inherited it to my knowledge.

You can manage directly the permissions for a given role with google_iam_policy. Although this won't cover inherited permissions.

https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_policy

edwardsPaul421 commented 2 years ago

If the permission is inherited you'll have to change the default wherever it was inherited it to my knowledge.

You can manage directly the permissions for a given role with google_iam_policy. Although this won't cover inherited permissions.

https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam#google_project_iam_policy

I see that this module is replacing the whole project policies, which is really dangerous. When using this role, I would like to get the policy of a project, then modifying specifically the SA I want to change, then import it with google_project_iam_policy. Do you know a way to get project policy?

intotecho commented 2 years ago

I have the same issue. App Engine and Compute Engine both create a default service account with Editor Role. I want to restrict the account to the few specific roles that the app or vm actually needs. It's not recommended to delete the default service account - even though I can use a custom service account.

google_project_iam_policy: works on the whole project - which is over reach for this scenario and can can lead to a project lock-out

google_service_account_iam_policy is not right because it treats the service account as the resource, not the principal.

So I am looking for a google_project_iam_service_account_policy with a syntax like:

resource "google_project_iam_service_account_policy {
  service_account_id = google_service_account.sa.name
  roles = [
       "roles/compute.instanceAdmin",
       "role = "roles/storage.objectViewer"
  ]
 }

After running this the service account will only have the listed roles. Under the hood I guess it would call gcloud projects remove-iam-policy-binding. Obviously this would also have the potential to conflict with member and bind policies. But this is already the case with the google_service_account_iam_policy and google_service_account_iam_member.

I think this feature would make it easier to make app engine and compute engine more secure. I can't find a suitable workaround to remove the Editor role.

duxbuse commented 1 year ago

Until there is a terraform perhaps this is a workaround

  provisioner "local-exec" {
    command = "gcloud projects remove-iam-policy-binding"
  }
ScottSuarez commented 1 year ago

We now support data sources for all iam policies. It's maybe possible to retrieve the policy with the data source then directly modify it before provisioning it with the resource. I hesitate to recommend such a solution though.

Nornode commented 1 year ago

Remove roles for a member/SA/domain is also needed if you bootstrap an organization and want to remove the default "roles/resourcemanager.projectCreator" & "roles/billing.creator" given to the whole member=domain of and on the organization.

This is a quite vulnerable default setting which should be possible to unset with terraform resources rather than currently used local_exec.

ScottSuarez commented 1 year ago

Could this ticket be triaged? I'm not sure policy removal is something we would want to support but removal of default policy settings has valid use cases.

If we decide to implement this feature we can look at forwarding it to the IAM team.

rileykarson commented 11 months ago

Note: gcloud projects remove-iam-policy-binding is the equivalent of removing a google_*_iam_member resource

SarahFrench commented 9 months ago

To summarise, the automatically permissions mentioned in the issue that users may want to remove are:

It seems like it's a better idea to create special 'remove iam membership' resource for projects (and also folder, org, billing account) to handle situations like the above, but we wouldn't want to implement those 'remove' IAM resources across all resources as it's more appropriate/less risky to manage though *_iam_binding and *_iam_policy resources.

It could look something like this for project:

resource "google_project_iam_member_remove" "gce-default" {
    project     = "your-project-id"
    role       = "roles/Editor"
    member    = "default-gce-sa@example.com"
}

These resources could be handwritten instead of implementing them through the existing IAM resource generation code (but reusing whatever code possible from there).

Edit: I've labelled this for the Terraform team as it's an atypical resource and there are more involved decisions about how to implement this

bloodwithmilk25 commented 8 months ago

In my case, I needed to remove roles/editor from <project_number>-compute@developer.gserviceaccount.com and <project_id>@appspot.gserviceaccount.com. I decided that since I would add roles/iam.roleViewer to them anyway, I could just import editor and replace it with iam.roleViewer. Two birds with one stone

import {
  id = "${var.PROJECT_ID} roles/editor "${data.google_project.project.number}-compute@developer.gserviceaccount.com"
  to = google_project_iam_member.compute_service_account_remove_editor_role
}

resource "google_project_iam_member" "compute_service_account_remove_editor_role" {
  project = var.PROJECT_ID
  role    = "roles/iam.roleViewer"
  member  = "serviceAccount:${data.google_project.project.number}-compute@developer.gserviceaccount.com"
}

import {
  id = "${var.PROJECT_ID} roles/editor ${var.PROJECT_ID}@appspot.gserviceaccount.com"
  to = google_project_iam_member.appspot_service_account_remove_editor_role
}

resource "google_project_iam_member" "appspot_service_account_remove_editor_role" {
  project = var.PROJECT_ID
  role    = "roles/iam.roleViewer"
  member  = "serviceAccount:${var.PROJECT_ID}@appspot.gserviceaccount.com"
}
hao-nan-li commented 6 months ago

To summarise, the automatically permissions mentioned in the issue that users may want to remove are:

It seems like it's a better idea to create special 'remove iam membership' resource for projects (and also folder, org, billing account) to handle situations like the above, but we wouldn't want to implement those 'remove' IAM resources across all resources as it's more appropriate/less risky to manage though *_iam_binding and *_iam_policy resources.

It could look something like this for project:

resource "google_project_iam_member_remove" "gce-default" {
    project     = "your-project-id"
    role       = "roles/Editor"
    member    = "default-gce-sa@example.com"
}

These resources could be handwritten instead of implementing them through the existing IAM resource generation code (but reusing whatever code possible from there).

Edit: I've labelled this for the Terraform team as it's an atypical resource and there are more involved decisions about how to implement this

I'm currently working on handwriting the new resource google_project_iam_member_remove. What would be a good test for this particular resource?

rileykarson commented 6 months ago

For an absence check like this I'd suggest handwriting a TestCheck that verifies the lack of presence in the API. Theoretically drift detection on the resource will accomplish the same.

SarahFrench commented 6 months ago

I imagine the test would need to use time_sleep resources similar to this. The test could create a new membership, pause, remove that membership via the new resource, pause, and then maybe use the google_project_iam_policy data source to get data about the final state of the policy?

hao-nan-li commented 6 months ago

Thanks Riley and Sarah for the suggestions.

rileykarson commented 4 months ago

https://github.com/GoogleCloudPlatform/magic-modules/pull/10376 adds the project-level version of this. Other variants should be trivial to add in PRs if required, although if we get to more than the resourcemanager resources (project, billing account, organization, folder) we'll want to fully templatize it. For any further variants past project I'd ask for folks to file new issues.

github-actions[bot] commented 3 months ago

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.