From 6a98b5aea0daac005d82a1946825f8c98248fac7 Mon Sep 17 00:00:00 2001 From: jolov Date: Wed, 24 Jan 2024 16:50:39 -0800 Subject: [PATCH 1/3] Fix role assignment for user auth --- .../TestResources/New-TestResources.ps1 | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/eng/common/TestResources/New-TestResources.ps1 b/eng/common/TestResources/New-TestResources.ps1 index 0f997fd1e9..d9adbde826 100644 --- a/eng/common/TestResources/New-TestResources.ps1 +++ b/eng/common/TestResources/New-TestResources.ps1 @@ -621,7 +621,7 @@ try { $TestApplicationOid = (Get-AzADUser -UserPrincipalName (Get-AzContext).Account).Id $TestApplicationId = $testApplicationOid - Log "User-based app id '$TestApplicationId' will be used." + Log "User authentication with user '$TestApplicationId' will be used." } # If no test application ID was specified during an interactive session, create a new service principal. elseif (!$CI -and !$TestApplicationId) { @@ -686,11 +686,11 @@ try { $PSBoundParameters['TestApplicationOid'] = $TestApplicationOid $PSBoundParameters['TestApplicationSecret'] = $TestApplicationSecret - # If the role hasn't been explicitly assigned to the resource group and a cached service principal is in use, + # If the role hasn't been explicitly assigned to the resource group and a cached service principal or user authentication is in use, # query to see if the grant is needed. - if (!$resourceGroupRoleAssigned -and $AzureTestPrincipal) { + if (!$resourceGroupRoleAssigned -and $TestApplicationOid) { $roleAssignment = Get-AzRoleAssignment ` - -ObjectId $AzureTestPrincipal.Id ` + -ObjectId TestApplicationOid ` -RoleDefinitionName 'Owner' ` -ResourceGroupName "$ResourceGroupName" ` -ErrorAction SilentlyContinue @@ -702,19 +702,19 @@ try { # considered a critical failure, as the test application may have subscription-level permissions and not require # the explicit grant. if (!$resourceGroupRoleAssigned) { - Log "Attempting to assigning the 'Owner' role for '$ResourceGroupName' to the Test Application '$TestApplicationId'" - $principalOwnerAssignment = New-AzRoleAssignment ` - -RoleDefinitionName "Owner" ` - -ApplicationId "$TestApplicationId" ` - -ResourceGroupName "$ResourceGroupName" ` - -ErrorAction SilentlyContinue - - if ($principalOwnerAssignment.RoleDefinitionName -eq 'Owner') { - Write-Verbose "Successfully assigned ownership of '$ResourceGroupName' to the Test Application '$TestApplicationId'" + Log "Attempting to assign the 'Owner' role for '$ResourceGroupName' to the Test Application or User with ID '$TestApplicationId'" + $ownerAssignment = New-AzRoleAssignment ` + -RoleDefinitionName "Owner" ` + -ObjectId "$TestApplicationOId" ` + -ResourceGroupName "$ResourceGroupName" ` + -ErrorAction SilentlyContinue + + if ($ownerAssignment.RoleDefinitionName -eq 'Owner') { + Write-Verbose "Successfully assigned ownership of '$ResourceGroupName' to the Test Application or User with ID '$TestApplicationId'" } else { Write-Warning ("The 'Owner' role for '$ResourceGroupName' could not be assigned. " + "You may need to manually grant 'Owner' for the resource group to the " + - "Test Application '$TestApplicationId' if it does not have subscription-level permissions.") + "Test Application or User with ID '$TestApplicationId' if it does not have subscription-level permissions.") } } From 4064403907c8e513fe54e512f95c7d5a57008729 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 25 Jan 2024 09:01:26 -0800 Subject: [PATCH 2/3] PR fb --- eng/common/TestResources/New-TestResources.ps1 | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/eng/common/TestResources/New-TestResources.ps1 b/eng/common/TestResources/New-TestResources.ps1 index d9adbde826..6064e03afd 100644 --- a/eng/common/TestResources/New-TestResources.ps1 +++ b/eng/common/TestResources/New-TestResources.ps1 @@ -619,9 +619,11 @@ try { Write-Warning "The specified TestApplicationId '$TestApplicationId' will be ignored when UserAuth is set." } - $TestApplicationOid = (Get-AzADUser -UserPrincipalName (Get-AzContext).Account).Id + $userAccount = (Get-AzADUser -UserPrincipalName (Get-AzContext).Account) + $TestApplicationOid = $userAccount.Id $TestApplicationId = $testApplicationOid - Log "User authentication with user '$TestApplicationId' will be used." + $userAccountName = $userAccount.UserPrincipalName + Log "User authentication with user '$userAccountName'('$TestApplicationId') will be used." } # If no test application ID was specified during an interactive session, create a new service principal. elseif (!$CI -and !$TestApplicationId) { @@ -690,7 +692,7 @@ try { # query to see if the grant is needed. if (!$resourceGroupRoleAssigned -and $TestApplicationOid) { $roleAssignment = Get-AzRoleAssignment ` - -ObjectId TestApplicationOid ` + -ObjectId $TestApplicationOid ` -RoleDefinitionName 'Owner' ` -ResourceGroupName "$ResourceGroupName" ` -ErrorAction SilentlyContinue @@ -702,7 +704,8 @@ try { # considered a critical failure, as the test application may have subscription-level permissions and not require # the explicit grant. if (!$resourceGroupRoleAssigned) { - Log "Attempting to assign the 'Owner' role for '$ResourceGroupName' to the Test Application or User with ID '$TestApplicationId'" + $idSlug = if ($userAuth) { "user '$userAccountName'('$TestApplicationId')"} else { "Test Application '$TestApplicationId'"}; + Log "Attempting to assign the 'Owner' role for '$ResourceGroupName' to the $idSlug" $ownerAssignment = New-AzRoleAssignment ` -RoleDefinitionName "Owner" ` -ObjectId "$TestApplicationOId" ` @@ -710,11 +713,11 @@ try { -ErrorAction SilentlyContinue if ($ownerAssignment.RoleDefinitionName -eq 'Owner') { - Write-Verbose "Successfully assigned ownership of '$ResourceGroupName' to the Test Application or User with ID '$TestApplicationId'" + Write-Verbose "Successfully assigned ownership of '$ResourceGroupName' to the $idSlug" } else { Write-Warning ("The 'Owner' role for '$ResourceGroupName' could not be assigned. " + "You may need to manually grant 'Owner' for the resource group to the " + - "Test Application or User with ID '$TestApplicationId' if it does not have subscription-level permissions.") + "$idSlug if it does not have subscription-level permissions.") } } From 7505bf48f0baf1efbcfb9e75690a156432951c99 Mon Sep 17 00:00:00 2001 From: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com> Date: Thu, 25 Jan 2024 11:07:49 -0800 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Heath Stewart --- eng/common/TestResources/New-TestResources.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eng/common/TestResources/New-TestResources.ps1 b/eng/common/TestResources/New-TestResources.ps1 index 6064e03afd..76fbfc51a6 100644 --- a/eng/common/TestResources/New-TestResources.ps1 +++ b/eng/common/TestResources/New-TestResources.ps1 @@ -623,7 +623,7 @@ try { $TestApplicationOid = $userAccount.Id $TestApplicationId = $testApplicationOid $userAccountName = $userAccount.UserPrincipalName - Log "User authentication with user '$userAccountName'('$TestApplicationId') will be used." + Log "User authentication with user '$userAccountName' ('$TestApplicationId') will be used." } # If no test application ID was specified during an interactive session, create a new service principal. elseif (!$CI -and !$TestApplicationId) { @@ -704,7 +704,7 @@ try { # considered a critical failure, as the test application may have subscription-level permissions and not require # the explicit grant. if (!$resourceGroupRoleAssigned) { - $idSlug = if ($userAuth) { "user '$userAccountName'('$TestApplicationId')"} else { "Test Application '$TestApplicationId'"}; + $idSlug = if ($userAuth) { "User '$userAccountName' ('$TestApplicationId')"} else { "Test Application '$TestApplicationId'"}; Log "Attempting to assign the 'Owner' role for '$ResourceGroupName' to the $idSlug" $ownerAssignment = New-AzRoleAssignment ` -RoleDefinitionName "Owner" `