Comment update to register app as owner

[anshuman-goel]

Summary of the Pull Request

For better instruction updating the comment to indicate adding application as owner in resource group.

PR Checklist

• Applies to work item: #xxx
• CLA signed. If not, go over here and sign the CLI.
• Tests added/passed
• Requires documentation to be updated
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

[bmc-msft]

While the Owner role is sufficient to deploy onefuzz, we should recommend using the minimum required roles.

Currently, deployment-role.json shows most of the roles required, but needs to be expanded given the recent change to user assigned managed identity creation on install.

[anshuman-goel]

While the Owner role is sufficient to deploy onefuzz, we should recommend using the minimum required roles.

Currently, deployment-role.json shows most of the roles required, but needs to be expanded given the recent change to user assigned managed identity creation on install.

Currently, user assigned managed identity don't work with service principal credential deployment. I will return back to this PR after the fix.

[chkeita]

@anshuman-goel the deployment issue is caused by the fact that your deployment credential is missing the permission AppRoleAssignment.ReadWrite.All which require admin approval currently

[anshuman-goel]

While the Owner role is sufficient to deploy onefuzz, we should recommend using the minimum required roles.

Currently, deployment-role.json shows most of the roles required, but needs to be expanded given the recent change to user assigned managed identity creation on install.

It turns out that I cannot create because there is a limit of 2000 custom roles per tenant.

[bmc-msft]

A few thoughts.

1. Recommending owner is still probably not preferred. Would it be acceptable to you to adjust it to "Please see X link for the roles required"?
2. With the change in Make the role assignment step optional in the deployment #271, the additional role of AppRoleAssignment.ReadWrite.All is only required for install, not upgrade.

[anshuman-goel]

A few thoughts.

1. Recommending owner is still probably not preferred. Would it be acceptable to you to adjust it to "Please see X link for the roles required"?
2. With the change in Make the role assignment step optional in the deployment #271, the additional role of AppRoleAssignment.ReadWrite.All is only required for install, not upgrade.

That looks reasonable. I will update the comment shortly.

[ryanspletzer]

@anshuman-goel the deployment issue is caused by the fact that your deployment credential is missing the permission AppRoleAssignment.ReadWrite.All which require admin approval currently

To clarify is this the AppRoleAssignment.ReadWrite.All a delegated or application permission? Former is OK but latter is much higher privilege for the service principal... We have someone trying to set this up at our org and we're reviewing it.

While the Owner role is sufficient to deploy onefuzz, we should recommend using the minimum required roles.

Currently, deployment-role.json shows most of the roles required, but needs to be expanded given the recent change to user assigned managed identity creation on install.

I'm assuming that of the common roles Contributor vs. Owner, you need to be in the Owner role on the sub for this to work, correct? Unless you use one of the more specific roles beyond these that includes the necessary permissions (or if you make a custom role for this).

[chkeita]

@ryanspletzer

To clarify is this the AppRoleAssignment.ReadWrite.All a delegated or application permission? Former is OK but latter is much higher privilege for the service principal... We have someone trying to set this up at our org and we're reviewing it.

the delegated permission should suffice according to the documentation

I'm assuming that of the common roles Contributor vs. Owner, you need to be in the Owner role on the sub for this to work,

Correct

Unless you use one of the more specific roles beyond these that includes the necessary permissions (or if you make a custom role for this).

Yes, and the json file linked contains the definition for the custom role