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

Clarify permissions granted by pac admin commands #2676

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bartbilliet
Copy link

Clarified permissions given when running the pac admin application register command or pac admin create-service-principal commands.

Based on underlying API calls and about what is mentioned here: https://github.com/MicrosoftDocs/power-platform/blob/main/power-platform/admin/powerplatform-api-create-service-principal.md#limitations-of-service-principals

…ster commands

Clarified permissions given when running the pac admin application register command or pac admin create-service-principal commands. 

Based on underlying API calls and in reference to what is mentioned here: https://github.com/MicrosoftDocs/power-platform/blob/main/power-platform/admin/powerplatform-api-create-service-principal.md#limitations-of-service-principals
Copy link

Learn Build status updates of commit 6bce724:

✅ Validation status: passed

File Status Preview URL Details
power-platform/developer/cli/reference/admin.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

@JimDaly JimDaly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bartbilliet

Thanks for your contribution.

These reference articles are generated by a program and overwritten each time the PAC CLI reference docs are updated. For these comments to persist, they need to be moved to the corresponding INCLUDE files directly below where these comments are.

But more importantly, I need @laneswenka to approve the content here.

@laneswenka If you approve these changes, I'll move the content to the INCLUDE file.

Finally, I may be wrong, but I thought that this command achieved the same result as the steps found here: Tutorial: Register an app with Microsoft Entra ID

@bartbilliet is connecting this command with the content here Creating a service principal application using API (preview)

Aren't these different things?

@bartbilliet
Copy link
Author

bartbilliet commented Aug 30, 2024

Hi Jim, thanks for your quick follow-up!

Some background from my side:
While revere engineering this by placing Fiddler in the loop while running the pac admin create-service-principal command, we see the following messages in the output:

Registering application xxx with Dataverse... done
Creating Dataverse system user and assigning role... done

In the Fiddler logs this results in a PUT request to /providers/Microsoft.BusinessAppPlatform/adminApplications/_clientid_, hence my connection to the article Creating a service principal application using API (preview) where the same API call is found.

I didn't trace using Fiddler on the pac admin application register command, but my understanding would be that it's the same?
From my understanding, the instructions in the section 'Create new app user' (Tutorial: Register an app with Microsoft Entra ID) registers an app user in a specific power platform environment, although the documented pac admin application register command does not take an environment ID as input, which is why it makes me believe assigned roles are tenant-wide?

Feel free to correct, in case I missed or misunderstood anything!

Copy link
Author

@bartbilliet bartbilliet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change on line 778 ok by me

@phecke phecke removed their request for review September 26, 2024 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants