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

Update-M365DSCAzureAdApplication could grant permissions to non-MS service application #2565

Closed
mlhickey opened this issue Nov 19, 2022 · 5 comments · Fixed by #2899 or #2906
Closed
Assignees
Labels
Core Engine Enhancement New feature or request

Comments

@mlhickey
Copy link
Contributor

mlhickey commented Nov 19, 2022

Details of the scenario you tried and the problem that is occurring

Update-M365DSCAzureAdApplication is used to update a dedicated application with service endpoint permissions - Microsoft Graph, Office 365 SharePoint Online and/or Office 365 Exchange Online - retrieved via Get-AzADServicePrincipal, filtering the result set based on displayName. Since displayName is a non-unique value in Azure, this opens the possibility of retrieving multiple entries for a single name and does not guarantee the servicePrincipal is the actual Microsoft service,e.g.

$gsp = Get-AzADServicePrincipal -Filter "displayName eq 'Microsoft Graph'"
$gsp.Count
3
$gsp

DisplayName     Id                                   AppId
-----------     --                                   -----
Microsoft Graph b19d498e-6687-4156-869a-2e8a95a9d659 00000003-0000-0000-c000-000000000000
Microsoft Graph ad92c665-9f52-4566-9b96-470bbcd40392 bdfa89eb-5d89-45f9-9a46-fcb772f808b9
Microsoft Graph 5bbe29f8-4ed6-48b9-a300-babff8fc21b6 fc381d29-52ab-4e40-a70d-aa2c356927a7

Verbose logs showing the problem

Suggested solution to the issue

  1. Filter the query rather than the result set - an unbound query could take significant time in a large organization
  2. Validate the returned value based on AppOwnerTenantId 1st party tenant
    e.g.
    $gsp = Get-AzADServicePrincipal -Filter "displayName eq 'Microsoft Graph'" | Where-Object -FilterScript {$_.AppOwnerTenantId-eq 'f8cdef31-a31e-4b4a-93e4-5f571e91255a'}'"

The DSC configuration that is used to reproduce the issue (as detailed as possible)

# insert configuration here

The operating system the target node is running

Version of the DSC module that was used ('dev' if using current dev branch)

@andikrueger andikrueger added Enhancement New feature or request Core Engine labels Nov 20, 2022
@andikrueger
Copy link
Collaborator

This is a valid enhancement! Thanks for the insights.

Just to add some complexity: Are the other Endpoints like SharePoint also published this ID?

@mlhickey
Copy link
Contributor Author

mlhickey commented Nov 20, 2022

Yes.

PS C:\> $endpointlist = @('Microsoft Graph', 'Office 365 SharePoint Online', 'Office 365 Exchange Online')
PS C:\> $endpointlist | % { Get-AzureADServicePrincipal -Filter "displayName eq '$_'" | Select-Object DisplayName, AppOwnerTenantId }

DisplayName                  AppOwnerTenantId
-----------                  ----------------
Microsoft Graph              f8cdef31-a31e-4b4a-93e4-5f571e91255a
Microsoft Graph              5179ae73-0aa8-428b-b7ed-d01bf771d9f9
Microsoft Graph              5179ae73-0aa8-428b-b7ed-d01bf771d9f9
Office 365 SharePoint Online f8cdef31-a31e-4b4a-93e4-5f571e91255a
Office 365 Exchange Online   f8cdef31-a31e-4b4a-93e4-5f571e91255a

Likewise, in the spirit of reducing dependencies it might be an opportune time to transition to MG in which case the property name changes to AppOwnerOrganizationId

@andikrueger andikrueger self-assigned this Nov 20, 2022
@andikrueger
Copy link
Collaborator

Do you know what the two other microsoft graph objects are for?

@andikrueger
Copy link
Collaborator

This is the output of my default tenant:

$endpointlist | % { Get-AzureADServicePrincipal -Filter "displayName eq '$_'" | Select-Object DisplayName, AppOwnerTenantId }


DisplayName                  AppOwnerTenantId
-----------                  ----------------
Microsoft Graph              f8cdef31-a31e-4b4a-93e4-5f571e91255a
Office 365 SharePoint Online f8cdef31-a31e-4b4a-93e4-5f571e91255a
Office 365 Exchange Online   f8cdef31-a31e-4b4a-93e4-5f571e91255a

@mlhickey
Copy link
Contributor Author

Yup, that is the 1st party tenant where Microsoft services are published from. The other SPs in my example are applications that individuals have registered in the tenant with the same display name as the official SP. Since there are no checks for uniqueness and no validation, it is possible for anyone who can register an application to do so with the same name as a published service. It's then up to the consumer to know what the return value actually means,

andikrueger added a commit to andikrueger/Microsoft365DSC that referenced this issue Feb 14, 2023
NikCharlebois added a commit that referenced this issue Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Engine Enhancement New feature or request
Projects
None yet
2 participants