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

external_senders_allowed failing even when unset with Service Principal authentication #893

Closed
k-drumz opened this issue Sep 27, 2022 · 6 comments · Fixed by #1028
Closed

Comments

@k-drumz
Copy link

k-drumz commented Sep 27, 2022

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritise this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritise the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureAD Provider) Version

Terraform v1.2.3
on darwin_arm64
+ provider registry.terraform.io/datadog/datadog v3.8.1
+ provider registry.terraform.io/hashicorp/archive v2.2.0
+ provider registry.terraform.io/hashicorp/aws v3.75.2
+ provider registry.terraform.io/hashicorp/azuread v2.18.0

Affected Resource(s)

azuread_group

Terraform Configuration Files

resource "azuread_group" "accounts_qa" {
  for_each                   = local.accounts
  display_name               = format(local.account_name_template, title(replace(each.key, "_", " ")), "QA")
  description                = format(local.azure_group_description_template, title(replace(each.key, "_", " ")), "QA")
  mail_enabled               = true
  mail_nickname              = format(each.value, "qa")
  security_enabled           = false
  types                      = ["Unified"]
  assignable_to_role         = false
  visibility                 = "Private"
  auto_subscribe_new_members = true

  behaviors = ["WelcomeEmailDisabled", "SubscribeNewGroupMembers"]

  members = local.email_group_owners
  owners  = local.email_group_owners

  # This is currently broken via API per https://docs.microsoft.com/en-us/graph/known-issues#allowexternalsenders-property-can-only-be-accessed-on-unified-groups
  lifecycle {
    ignore_changes = [external_senders_allowed]
  }
}

Debug Output

I can debug if necessary.

Panic Output

Expected Behavior

It should ignore patching the external_senders_allowed if it isn't set.

Actual Behavior

It is failing with error because I previous had external_senders_allowed = true after manually changing the group setting outside of Terraform. After removing the option it is now failing to unset.

Steps to Reproduce

  1. terraform apply

Important Factoids

│ Error: Failed to set `external_senders_allowed` for group with object ID "91fde839-c0e6-494e-bfed-4f52b301fc6d"
│ 
│   with azuread_group.accounts_fedramp_qa["shared_infrastructure"],
│   on accounts_fedramp.tf line 73, in resource "azuread_group" "accounts_fedramp_qa":
│   73: resource "azuread_group" "accounts_fedramp_qa" {
│ 
│ GroupsClient.BaseClient.Patch(): unexpected status 403 with OData error: ErrorGroupsAccessDenied: User does not have permissions to execute this action.

References

Please reference #729

@bestefreund
Copy link

bestefreund commented Nov 17, 2022

I'm also facing this issue with using a service principal and trying to update an existing group which was deployed by Terraform:

resource "azuread_group" "this" {
  display_name             = "test-bjoern"
  mail_enabled             = true
  mail_nickname            = "test-bjoern"
  security_enabled         = true
  external_senders_allowed = null
  types                    = ["Unified"]

  owners = [
    data.azuread_client_config.current.object_id,
  ]
}

The deployment of azuread_group works smooth.

But when I run terraform plan -out=out.plan after the deployment of the group the output shows that the flag for external_senders_allowed was set to false:

  ~ resource "azuread_group" "this" {
      + external_senders_allowed   = false
        id                         = "54638204b-947c-fkd8-2430-7309-237hdnd843je"
        # (19 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

The group test-bjoern hasn't been touched besides Terraform.

When I'm running terraform apply out.plan afterwards I get the following error:

╷
│ Error: Failed to set `external_senders_allowed` for group with object ID "54638204b-947c-fkd8-2430-7309-237hdnd843je"
│ 
│   with module.applications["data/test-bjoern.yaml"].module.enterprise_groups.azuread_group.this,
│   on .terraform/modules/applications.enterprise_groups/main.tf line 12, in resource "azuread_group" "this":
│   12: resource "azuread_group" "this" {
│ 
│ GroupsClient.BaseClient.Patch(): unexpected status 403 with OData error: ErrorAccessDenied: Access is denied. Check credentials and try again.

@headyj
Copy link

headyj commented Nov 30, 2022

Same problem here, I cannot import an existing group on terraform. No matter which value I put for external_senders_allowed (null, true or false) or even putting it in ignore_changes lifecycle, still getting the same error above.

@lukiffer
Copy link

lukiffer commented Jan 30, 2023

Having the same problem, but wanted to share a workaround for our specific encounter with the issue (and perhaps may provide some hints when debugging).

We're importing an existing group. terraform import works fine, and even though the initial apply returns the above error, the group properties were successfully updated. If you then remove any membership changes, subsequent applys return "no changes".

It appears that when using the members attribute of the resource rather than a separate azuread_group_member resource, the provider is sending an update for the group to the Microsoft Graph API, even if there's seemingly nothing to update.

TL;DR – we were able to simply set azuread_group.members = null and then create separate azuread_group_members for the membership and it works for what we needed – we were able to successfully import the existing group.

Hope this helps.

@ccadruvi
Copy link
Contributor

ccadruvi commented Feb 15, 2023

Would a possible fix be the following in this location? I'm not sure I understand exactly what d.GetOk() and d.GetOkExists() do.

Essentially, only perform a PATCH request if the attribute has changed.

// AllowExternalSenders can only be set in its own PATCH request; including other properties returns a 400
if d.HasChange("external_senders_allowed") { //nolint:staticcheck
	v := d.Get("external_senders_allowed").(bool)
	if _, err := client.Update(ctx, msgraph.Group{
		DirectoryObject: msgraph.DirectoryObject{
			Id: group.ID(),
		},
		AllowExternalSenders: utils.Bool(v),
	}); err != nil {
		return tf.ErrorDiagF(err, "Failed to set `external_senders_allowed` for group with object ID %q", *group.ID())
	}

	// Wait for AllowExternalSenders to be updated
	if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
		client.BaseClient.DisableRetries = true
		group, err := groupGetAdditional(ctx, client, *group.ID())
		if err != nil {
			return nil, err
		}
		return utils.Bool(group.AllowExternalSenders != nil && *group.AllowExternalSenders == v), nil
	}); err != nil {
		return tf.ErrorDiagF(err, "Waiting for update of `external_senders_allowed` for group with object ID %q", *group.ID())
	}
}

The same fix would be neeeded for auto_subscribe_new_members, hide_from_address_lists and hide_from_outlook_clients.

@github-actions
Copy link

This functionality has been released in v2.35.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants