Find-MgGraphPermission #809

FehintolaObafemi · 2021-08-06T20:48:42Z

This change adds the Find-MgGraphPermission command to the Microsoft.Graph.Authentication module as part of #704. See that issue for details on the functionality and use cases. Here are the implementation highlights:

The Find-MgGraphPermission command is implemented as a custom script command using PowerShell itself, not C#
A custom type Microsoft.Graph.Custom.Permission is defined as the output type of Find-MgGraphPermission so that other commands like Select-Object can implement auto-complete for the output of Find-MgGraphPermission
The change also includes new output formatting in the ps1xml definition for Microsoft.Graph.Custom.Permission
Existing sample scripts for Application and Connect have been updated to include relevant use cases involving Find-MgGraphPermission
Documentation is included with the command as part of comment-based documentation
Unit test coverage is accomplished through a set of Pester test cases

To understand the expected behavior, you can review:

As noted in the original proposal, the permissions data is sourced from Microsoft Graph itself. However, many users will not have the privileges required to read it (you must be able to read the Microsoft Graph service principal object in your Azure Active Directory organization; company admins will have no problem with this, but many lower-privileged users won't be able to. Due to this, there is a fallback to snapshot of that service principal included in the code. For 99% of cases, the fallback will be fine as the newest permissions are unlikely to be those that someone is looking for. We do need to periodically update the file though.

Note that the command also includes additional documented capabilities that were not part of the original proposal -- these were added to simplify automation cases where you where partial matches and non-determinism are unacceptable. The additional filtering allows the set of results to be narrowed down to just one result (or none!) which is required for scenarios such as translating a known friendly permission name to the unique id for use with Microsoft Graph REST APIs or commands for those APIs.

adamedx · 2021-08-06T22:33:45Z

@peombwa , I can't find a log of the Pester test runs in the repository -- do you have a pointer to this?

peombwa · 2021-08-06T22:47:32Z

I can't find a log of the Pester test runs in the repository -- do you have a pointer to this?

@adamedx, the Pester test run results can be found here. @georgend, would you know why Invoke-MgGraphRequest tests a failing here?

@FehintolaObafemi, you can ignore the failing Invoke-MgGraphRequest tests since they are not related to your changes.

adamedx · 2021-08-06T23:24:04Z

I can't find a log of the Pester test runs in the repository -- do you have a pointer to this?

@adamedx, the Pester test run results can be found here. @georgend, would you know why Invoke-MgGraphRequest tests a failing here?

@FehintolaObafemi, you can ignore the failing Invoke-MgGraphRequest tests since they are not related to your changes.

Thanks @peombwa -- it seems I don't have permission to see the test results, even though I'm signed in to Azure DevOps. Any way to fix that? If not, can you validate that the tests we added are indeed running? The unit test file is linked in the PR description.

adamedx · 2021-08-07T01:17:46Z

samples/9-Applications.ps1

@@ -26,6 +26,7 @@ $app3 = New-MgApplication -displayName "ImplicitWebApp" `
        }  

 # Create an registration for an ASP.NET Web App
+$scopeId_UserRead = Find-MgGraphPermission User.Read -ExactMatch -PermissionType Delegated | Select-Object -ExpandProperty Id


@peombwa you'll want to sign off on having the sample changed to use the new command.

This looks good! This is a good example of how Find-MgGraphPermission can be used to aid in scope discovery.

adamedx · 2021-08-07T01:18:14Z

samples/9-Applications.ps1

-$GroupReadAll = @{ Id = "5b567255-7703-4780-807c-7be8301ae99b"; Type = "Role" }
-$MailboxSettingsRead = @{ Id = "40f97065-369a-49f4-947c-6a255697ae91"; Type = "Role" }
-$MailSend = @{ Id = "b633e1c5-b582-4048-a93e-9f11b44c7e96"; Type = "Role" }
+# Show friendly Graph permission names given their unique identifiers


Another big change to the sample @peombwa

adamedx · 2021-08-07T01:19:17Z

samples/2-ConnectToGraph.ps1

@@ -6,6 +6,9 @@ Connect-Graph
 # Try to Get-User
 Get-MgUser

+# Search for delegated permissions related to sites


@peombwa goal here is to show you can use the new command to find permissions, and also to get to the permissions reference help.

adamedx · 2021-08-07T01:20:08Z

samples/2-ConnectToGraph.ps1

@@ -17,3 +20,7 @@ Connect-Graph -Scopes "User.Read","User.ReadWrite.All","Mail.ReadWrite",`

 # Forget all access tokens
 Disconnect-Graph
+
+# Launch detailed permissions documentation
+Get-Help Find-MgGraphPermission -Online


@msewaweru , this works because the .LINKS field in the comment help has exactly one entry, and it's the URI to the permissions docs. That means we can't provide references to other commands in .LINKS though -- hope that's ok.

@maisarissi given the pointer by @adamedx it means we cannot use related links as we had intended to store Survey Links.

You may want to sync with someone from the https://github.com/powershell/powershell or other experts on dcs + PowerShell project to confirm the behavior I observed.

If there is such a limitation they should call Related Link instead of Related Links (plural) LOL
I will try to sync with someone to confirm this behavior.

adamedx · 2021-08-07T01:22:32Z

src/Authentication/Authentication/custom/Find-MgGraphPermission.ps1

+}
+
+<#
+.SYNOPSIS


@msewaweru this is the documentation section -- I believe this will be used to generate the online docs...

adamedx · 2021-08-07T01:23:23Z

src/Authentication/Authentication/custom/Find-MgGraphPermission.ps1

+in the default table view.
+
+.LINK
+https://docs.microsoft.com/en-us/graph/permissions-reference


This allows Get-Help -Online Find-MgGraphPermission to actually point at the permissions reference rather than help for Find-MgGraphPermission. If that's not ok, we can change this.

src/Authentication/Authentication/custom/Find-MgGraphPermission.ps1

TannerTrombley · 2021-08-09T15:29:58Z

src/Authentication/Authentication/custom/Find-MgGraphPermission.ps1

+The SearchString parameter allows you to specify a string such as 'user' or 'mail' that represents the subject or domain
+of the permission you're searching for. Since permissions usually have names such as 'User.Read' or 'Mail.ReadWrite', the
+command uses the SearchString parameter to return all permissions that contain the value specified for SearchString in the
+name of the permission.


@FehintolaObafemi Clarification: I thought you said this searches the description field as well. I don't think that is the case, but just want to make sure.

@TannerTrombley Adam and I opted to excluding the description field search.

peombwa · 2021-08-09T15:35:46Z

Thanks @peombwa -- it seems I don't have permission to see the test results, even though I'm signed in to Azure DevOps. Any way to fix that? If not, can you validate that the tests we added are indeed running? The unit test file is linked in the PR description.

AzDO is picking up the Find-MgGraphPermission Pester tests and running them as expected. However, the following test is failing with:

The Find-MgGraphPermission Command.When executing the command using a constrained set of permissions returned by MS Graph and there is a connection.Should return null and not throw an exception if ExactMatch is specified and there is no match

Expected no exception to be thrown, but an exception "No results were found that exactly matched the specified permission 'IDontExist'" was thrown from C:\a\1\s\src\Authentication\Authentication\test\Find-MgGraphPermission.Tests.ps1:119 char:15
+ …           { Find-MgGraphPermission -ExactMatch IDontExist 2>&1 | out- …
+               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
at { Find-MgGraphPermission -ExactMatch IDontExist 2>&1 | out-null } | Should -Not -Throw, C:\a\1\s\src\Authentication\Authentication\test\Find-MgGraphPermission.Tests.ps1:119

cc\ @FehintolaObafemi

Find-MgGraphPermission: Define new location for common script code

Find-MgGraphPermission: Add formatting Work around Pester corrupting variable scopes Add missing files: Work around Pester corrupting variable scopes

Add help documentation for Find-MgGraphPermission

Add pipeline support, ExactMatch and explicitly return all

…tion to make errors ignorable Update samples to include usage of Find-MgGraphPermission

…ject auto-completion works

…e files

…authentication module

adamedx · 2021-08-09T20:29:59Z

Thanks @peombwa -- it seems I don't have permission to see the test results, even though I'm signed in to Azure DevOps. Any way to fix that? If not, can you validate that the tests we added are indeed running? The unit test file is linked in the PR description.

AzDO is picking up the Find-MgGraphPermission Pester tests and running them as expected. However, the following test is failing with:
The Find-MgGraphPermission Command.When executing the command using a constrained set of permissions returned by MS Graph and there is a connection.Should return null and not throw an exception if ExactMatch is specified and there is no match

Expected no exception to be thrown, but an exception "No results were found that exactly matched the specified permission 'IDontExist'" was thrown from C:\a\1\s\src\Authentication\Authentication\test\Find-MgGraphPermission.Tests.ps1:119 char:15
+ …           { Find-MgGraphPermission -ExactMatch IDontExist 2>&1 | out- …
+               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
at { Find-MgGraphPermission -ExactMatch IDontExist 2>&1 | out-null } | Should -Not -Throw, C:\a\1\s\src\Authentication\Authentication\test\Find-MgGraphPermission.Tests.ps1:119
cc\ @FehintolaObafemi

Interesting -- then my question why are the checks passing? Shouldn't this failure cause the checks to all fail and turn red?

I'm not sure why this would fail unless the $ErrorActionPreference is set to something non-standard. I have an idea how to compensate for that...

…ons-edits Edits v0.1

src/Authentication/Authentication/test/Find-MgGraphPermission.Tests.ps1

samples/2-ConnectToGraph.ps1

src/Authentication/Authentication/custom/common/Permissions.ps1

Co-authored-by: Peter Ombwa <[email protected]>

…Tests.ps1 Co-authored-by: Peter Ombwa <[email protected]>

Co-authored-by: Peter Ombwa <[email protected]>

peombwa · 2021-08-11T23:24:54Z

Thanks @peombwa -- it seems I don't have permission to see the test results, even though I'm signed in to Azure DevOps. Any way to fix that? If not, can you validate that the tests we added are indeed running? The unit test file is linked in the PR description.

AzDO is picking up the Find-MgGraphPermission Pester tests and running them as expected. However, the following test is failing with:

The Find-MgGraphPermission Command.When executing the command using a constrained set of permissions returned by MS Graph and there is a connection.Should return null and not throw an exception if ExactMatch is specified and there is no match

Expected no exception to be thrown, but an exception "No results were found that exactly matched the specified permission 'IDontExist'" was thrown from C:\a\1\s\src\Authentication\Authentication\test\Find-MgGraphPermission.Tests.ps1:119 char:15
+ …           { Find-MgGraphPermission -ExactMatch IDontExist 2>&1 | out- …
+               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
at { Find-MgGraphPermission -ExactMatch IDontExist 2>&1 | out-null } | Should -Not -Throw, C:\a\1\s\src\Authentication\Authentication\test\Find-MgGraphPermission.Tests.ps1:119

@adamedx, I've figured out why the failing tests were not causing the checks to fail. You can find the PR to fix this here.

peombwa · 2021-08-11T23:42:43Z

src/Authentication/Authentication/StartupScripts/ExportCustomCommands.ps1

+# both functions and cmdlets at export; if only one of
+# these classes is specified, nothing of the other
+# class will be exported.
+Export-ModuleMember -Function * -Cmdlet *


Just an FYI Exporting module members like this will suppress currently exported cmdlet aliases such as Connect-Graph. The Pester tests to confirm this can be found here. I've fixed this in my PR by adding Get-ModuleCmdlet and Get-ScriptCmdlet helper cmdlets to aid in exporting functions, cmdlets, and aliases.

This will be fixed #816 when we merge the 2 PRs.

Other than the issue highlighted here (which has been fixed by PR #816), everything else looks good to me!

Awesome, thank you @peombwa! What are the next steps? It sounds like this is in your queue to merge and you'll be the person to handle merging this (along with the related PR's)?

@adamedx, That's right. I'll merge the PR later today (to give others time to review it) then schedule everything for a 1.7.0 release.

@FehintolaObafemi, thank you for the excellent contribution!! I'm quite sure our customers will find this command valuable for their scripts.

peombwa

Other than the issue highlighted here (which has been fixed by PR #816), everything else looks good to me!

FehintolaObafemi requested a review from adamedx August 6, 2021 20:48

FehintolaObafemi requested review from ddyett, georgend, peombwa and shemogumbe as code owners August 6, 2021 20:48

FehintolaObafemi changed the title ~~Fehintola obafemi/permission~~ Find-MgGraphPermission Aug 6, 2021

adamedx reviewed Aug 7, 2021

View reviewed changes

TannerTrombley reviewed Aug 9, 2021

View reviewed changes

src/Authentication/Authentication/custom/Find-MgGraphPermission.ps1 Outdated Show resolved Hide resolved

TannerTrombley reviewed Aug 9, 2021

View reviewed changes

adamedx force-pushed the FehintolaObafemi/permission branch from 0a9db56 to 21c50f3 Compare August 9, 2021 20:18

adamedx and others added 9 commits August 9, 2021 16:22

Generate empty Find-MgGraphPermission command

a6f3fbb

Initial implementation of Find-MgGraphPermission

525de57

Find-MgGraphPermission: Define new location for common script code

Emit command output using the format required by the spec

525ba74

Find-MgGraphPermission: Add formatting Work around Pester corrupting variable scopes Add missing files: Work around Pester corrupting variable scopes

Minor refactoring and simplification

582f8de

Add help documentation for Find-MgGraphPermission

Add ability to limit the search scope of permissions

40524de

Add pipeline support, ExactMatch and explicitly return all

Documentation for additional Find-MgGraphPermissions, rely on ErrorAc…

fc94e03

…tion to make errors ignorable Update samples to include usage of Find-MgGraphPermission

Add type data for Microsoft.Graph.Custom.Permission so that Select-Ob…

3eeeaa9

…ject auto-completion works

Update .gitignore with additional exclusions for Emacs editor autosav…

e4770fa

…e files

Fix unnecessary output pipeline pollution in build-module script for …

ae89eec

…authentication module

adamedx force-pushed the FehintolaObafemi/permission branch from 21c50f3 to ae89eec Compare August 9, 2021 20:22

FehintolaObafemi and others added 3 commits August 9, 2021 16:39

Edits v0.1

c7315be

Edits v0.2

3c65b06

Merge pull request #812 from microsoftgraph/FehintolaObafemi/permissi…

5e3a425

…ons-edits Edits v0.1

peombwa reviewed Aug 9, 2021

View reviewed changes

src/Authentication/Authentication/test/Find-MgGraphPermission.Tests.ps1 Outdated Show resolved Hide resolved

peombwa reviewed Aug 9, 2021

View reviewed changes

samples/2-ConnectToGraph.ps1 Outdated Show resolved Hide resolved

peombwa reviewed Aug 9, 2021

View reviewed changes

src/Authentication/Authentication/custom/common/Permissions.ps1 Outdated Show resolved Hide resolved

Taofeek F. Obafemi-Babatunde and others added 3 commits August 10, 2021 11:51

Update src/Authentication/Authentication/custom/common/Permissions.ps1

f5521f3

Co-authored-by: Peter Ombwa <[email protected]>

Update src/Authentication/Authentication/test/Find-MgGraphPermission.…

b7faf6f

…Tests.ps1 Co-authored-by: Peter Ombwa <[email protected]>

Update samples/2-ConnectToGraph.ps1

d69cae3

Co-authored-by: Peter Ombwa <[email protected]>

maisarissi mentioned this pull request Aug 11, 2021

Surface feedback surveys for Identity APIs under "Related Links" of the PowerShell cmdlets Azure/autorest.powershell#805

Open

peombwa mentioned this pull request Aug 11, 2021

Fail Build When Pester Tests Fail #819

Merged

peombwa reviewed Aug 11, 2021

View reviewed changes

peombwa previously approved these changes Aug 11, 2021

View reviewed changes

georgend dismissed peombwa’s stale review via d69cae3 August 12, 2021 15:56

Merge branch 'dev' into FehintolaObafemi/permission

ecf25b2

peombwa approved these changes Aug 12, 2021

View reviewed changes

peombwa merged commit 0d1a30c into dev Aug 13, 2021

peombwa mentioned this pull request Sep 7, 2021

1.7.0 Release #847

Merged

Find-MgGraphPermission #809

Find-MgGraphPermission #809

Uh oh!

Conversation

FehintolaObafemi commented Aug 6, 2021 • edited by adamedx Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamedx commented Aug 6, 2021

Uh oh!

peombwa commented Aug 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamedx commented Aug 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgend Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maisarissi Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peombwa commented Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamedx commented Aug 9, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peombwa commented Aug 11, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peombwa Aug 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peombwa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FehintolaObafemi commented Aug 6, 2021 •

edited by adamedx

Loading

peombwa commented Aug 6, 2021 •

edited

Loading

georgend Aug 10, 2021 •

edited

Loading

maisarissi Aug 11, 2021 •

edited

Loading

peombwa commented Aug 9, 2021 •

edited

Loading

peombwa Aug 12, 2021 •

edited

Loading