Support for password policies

[alexwilcox9]

Closes: #136

@manicminer is this the kind of thing you had in mind?
One thing I haven't checked is if both are true will graph always return them in that order or should we just check if the string contains DisablePasswordExpiration/DisableStrongPassword rather than going for an exact match

[manicminer]

@alexwilcox9 This looks good! I think we could probably make them top-level properties like force_password_change, there isn't much to gain by putting them in a block?

[alexwilcox9]

@manicminer I just love blocks! That's fair enough I've rearranged it a bit now, also not sure what to call the two functions as they're not really expand/flatten functions so suggestions are welcome!

[manicminer]

I like blocks too, it's just that they introduce complexity when interacting with the TF plugin SDK :)

To be honest I don't think we need flatten/expand functions in this case, we can do it inline in the create/update funcs without much boilerplate. The only thing I would change with the parsing of the API response for passwordPolicies is I would split it into a slice on the comma, and do a case insensitive (space trimmed) comparison with each value, to account for the API reordering or changing them.

[alexwilcox9]

Agreed, splitting it into a slice makes more sense. I've scrapped the functions and moved the contents into the create/update/read functions

[alexwilcox9]

I've hit a new issue now that I'm hoping you can help advise on.

Currently I set passwordPolicies to a pointer to an empty string if neither options are specified however that lead to me getting the following

        UsersClient.BaseClient.Post(): unexpected status 400 with OData error:
        Request_BadRequest: Invalid value specified for property 'passwordPolicies'
        of resource 'User'.

So I then had the idea to set passwordPolicies to nil if neither option were selected which did initially solved the problem but seems to have broken the update function should you go from having either option set to neither of them as sending nil doesn't update the password policies

    testcase.go:60: Step 5/6 error: After applying this test step, the plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # azuread_user.test will be updated in-place
          ~ resource "azuread_user" "test" {
              ~ disable_password_expiration = true -> false
              ~ disable_strong_password     = true -> false
                id                          = "b6df9c72-649e-41d3-91fb-fec9461d596e"
                # (16 unchanged attributes hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccUser_update (30.32s)

So I can't work out what value you set if you want to turn both options off..

I hope that makes sense! Any ideas?

[manicminer]

Hi @alexwilcox9, this is a fairly common issue which is easily resolvable, but requires a change in the SDK (hamilton). For values that should be unset by sending null instead of a blank string, there is a custom type StringNullWhenEmpty which should be used instead of a regular string.

I've tried to catch as many of these as I can based on documentation, but it doesn't always indicate in the docs, and sometimes I miss them. You can fix it in Hamilton and it should solve this.

The line to fix is https://github.com/manicminer/hamilton/blob/main/msgraph/models.go#L1126

It is currently:

	PasswordPolicies                *string                  `json:"passwordPolicies,omitempty"`

but it should be:

	PasswordPolicies                *StringNullWhenEmpty     `json:"passwordPolicies,omitempty"`

Give me a shout on Slack if I can assist with making this change in your dev environment. I often add a go.mod override and then vendor from a local git clone, and then PR the change in Hamilton once i'm confident it works.

[alexwilcox9]

Thanks for the tip and help getting my environment set up, I've created a PR in the SDK repo for this change
manicminer/hamilton#96

[alexwilcox9]

Just thinking... whilst I'm at it should I update the user data resource to include these new attributes?

[manicminer]

This looks and works great, just some minor suggestions on variable naming but otherwise this LGTM 👍

[manicminer]

LGTM 🚀

[github-actions]

This functionality has been released in v2.2.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]

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 contributions.
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.