azuread_application: add password property

[HappyTobi]

Add new inline property "app_password"

The app_password generate a application credential with the first request where the application is created.
The improvement here is that ist a bit faster than the external "application_password_resource" creation because there is no wait time until the credential can be used.

The feature set of the "app_password" is equal to "application_password_resource"

Regards

[manicminer]

Hi @HappyTobi, thanks for submitting this PR. I agree that it would be nice for it to be quicker when adding a password credential to an application, however there are several platform limitations that necessitate the extensive checks we have put in place. Due to internal architectural and eventual consistency constraints, it can actually take up to 20 minutes for a newly created application and its password credential(s) to be usable for authentication.

Adding these properties to the azuread_application resource would not alleviate these issues, in fact we'd need to add extra checks which would be likely to take longer than it currently does. Additionally, if we were to add these, we'd need to support substituting any number of azuread_application_password resources in a declarative manner (i.e. be able to manage all password for an application). Due to the complexity involved, it's unlikely we'll add this to the azuread_application resource, particularly as the direction in which we are leaning is more towards composing the application into further separate resources.

As such, whilst I'd like to thank you again for looking at this, I'm going to close this for now since, as mentioned above, this is not the general direction we're heading in, and if we were to add this, there would be substantially more checks and tests required in order to avoid breaking existing configurations, and to ensure that the new block worked for multiple passwords in multiple scenarios.

[manicminer]

Thanks for working on this @HappyTobi, this is looking good but there are some changes we will need to make to separate the logic for Create and Read, which I have added inline below.

Additionally, there is currently no Update implementation, so I've put together an initial suggestion with which the tests seem to pass. In the Update function, after setting the application logo, we can add something like this to ensure any password changes are effected. This is quite lengthy in order to handle eventual consistency issues.

	// Remove and/or set a new application password, if changed
	if d.HasChange("password") {
		oldPasswordRaw, newPasswordRaw := d.GetChange("password")
		oldPasswordBlock := oldPasswordRaw.(*pluginsdk.Set).List()
		oldPassword := make(map[string]interface{})
		if len(oldPasswordBlock) > 0 {
			oldPassword = oldPasswordBlock[0].(map[string]interface{})
		}

		if oldPassword["key_id"] != nil {
			keyIdToRemove := oldPassword["key_id"].(string)
			if _, err = client.RemovePassword(ctx, id.ApplicationId, keyIdToRemove); err != nil {
				return tf.ErrorDiagF(err, "Removing password credential %q from application with object ID %q", id.ApplicationId, keyIdToRemove)
			}

			// Wait for application password to be deleted
			if err = helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
				defer func() { client.BaseClient.DisableRetries = false }()
				client.BaseClient.DisableRetries = true

				app, _, err := client.Get(ctx, id.ApplicationId, odata.Query{})
				if err != nil {
					return nil, err
				}

				credential := helpers.GetPasswordCredential(app.PasswordCredentials, keyIdToRemove)
				if credential == nil {
					return pointer.To(false), nil
				}

				return pointer.To(true), nil
			}); err != nil {
				return tf.ErrorDiagF(err, "Waiting for deletion of password credential %q from application with object ID %q", keyIdToRemove, id.ApplicationId)
			}
		}

		newPasswordBlock := newPasswordRaw.(*pluginsdk.Set).List()
		if len(newPasswordBlock) > 1 {
			return tf.ErrorDiagPathF(errors.New("`password` must have one element"), "password", "Multiple passwords are not supported with this resource")
		}

		// Proceed to add a new password to replace the now-removed one, if the password block is present in the configuration
		if len(newPasswordBlock) > 0 {
			newPassword := newPasswordBlock[0].(map[string]interface{})

			credential, err := helpers.PasswordCredential(newPassword)
			if err != nil {
				attr := ""
				if kerr, ok := err.(helpers.CredentialError); ok {
					attr = kerr.Attr()
				}
				return tf.ErrorDiagPathF(err, attr, "Generating password credential for %s", id.ApplicationId)
			}

			newCredential, _, err := client.AddPassword(ctx, id.ApplicationId, *credential)
			if err != nil {
				return tf.ErrorDiagF(err, "Adding password for application with object ID %q", id.ApplicationId)
			}
			if newCredential == nil {
				return tf.ErrorDiagF(errors.New("nil credential received when adding password"), "API error adding password for application with object ID %q", id.ApplicationId)
			}
			if newCredential.KeyId == nil {
				return tf.ErrorDiagF(errors.New("nil or empty keyId received"), "API error adding password for application with object ID %q", id.ApplicationId)
			}
			if newCredential.SecretText == nil || len(*newCredential.SecretText) == 0 {
				return tf.ErrorDiagF(errors.New("nil or empty password received"), "API error adding password for application with object ID %q", id.ApplicationId)
			}

			// Wait for the credential to appear in the application manifest, this can take several minutes
			timeout, _ := ctx.Deadline()
			polledForCredential, err := (&pluginsdk.StateChangeConf{ //nolint:staticcheck
				Pending:                   []string{"Waiting"},
				Target:                    []string{"Done"},
				Timeout:                   time.Until(timeout),
				MinTimeout:                1 * time.Second,
				ContinuousTargetOccurence: 5,
				Refresh: func() (interface{}, string, error) {
					app, _, err := client.Get(ctx, id.ApplicationId, odata.Query{})
					if err != nil {
						return nil, "Error", err
					}

					if app.PasswordCredentials != nil {
						for _, cred := range *app.PasswordCredentials {
							if cred.KeyId != nil && strings.EqualFold(*cred.KeyId, *newCredential.KeyId) {
								return &cred, "Done", nil
							}
						}
					}

					return nil, "Waiting", nil
				},
			}).WaitForStateContext(ctx)

			if err != nil {
				return tf.ErrorDiagF(err, "Waiting for password credential for application with object ID %q", id.ApplicationId)
			} else if polledForCredential == nil {
				return tf.ErrorDiagF(errors.New("password credential not found in application manifest"), "Waiting for password credential for application with object ID %q", id.ApplicationId)
			}

			// Ensure the new value is persisted to state
			newPassword["key_id"] = pointer.From(newCredential.KeyId)
			newPassword["value"] = pointer.From(newCredential.SecretText)
			tf.Set(d, "password", []interface{}{newPassword})
		}
	}

Additionally, in the Update function, we should add an internal lock against the application object to prevent clobbering when the azuread_application_password resource is also used. This should possibly have been added here already, but it will definitely be needed now.

	// add near the top of the Update function, after parsing the resource ID but before reading any values from config

	tf.LockByName(applicationResourceName, id.ApplicationId)
	defer tf.UnlockByName(applicationResourceName, id.ApplicationId)

We will also need to document the password property. Can you write up a proposal for explaining the use case for this? It doesn't need to be long, ideally something that will fit into an info box, for example:

If you can look at incorporating these changes, and there are no test breakages in the applications package, this should be good to merge. Thanks!

[HappyTobi]

ACC tests runs fine:

--- PASS: TestAccApplication_password (25.09s)
--- PASS: TestAccApplication_passwordUpdate (52.37s)

[manicminer]

Test results