-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Revert default role assignment for New-AzureRmADServicePrincipal #6272
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
Changes from 4 commits
0acb1cf
574e505
a7b2839
215888e
25fb72a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,13 @@ | |
| --> | ||
| ## Current Release | ||
|
|
||
| ## Version 6.0.1 | ||
| * Revert change to `New-AzureRmADServicePrincipal` that gave service principals `Contributor` permissions over the current subscription if no values were provided for the `Role` or `Scope` parameters | ||
| - If no values are provided for `Role` or `Scope`, the service principal is created with no permissions | ||
| - If a `Role` is provided, but no `Scope`, the service principal is created with the specified `Role` permissions over the current subscription | ||
| - If a `Scope` is provided, but no `Scope`, the service principal is created with `Contributor` permissions over the specified `Scope` | ||
|
||
| - If both `Role` and `Scope` are provided, the service principal is created with the specified `Role` permissions over the specified `Scope` | ||
|
|
||
| ## Version 6.0.0 | ||
| * Set minimum dependency of module to PowerShell 5.0 | ||
| * Remove obsolete parameter -AtScopeAndBelow from Get-AzureRmRoledefinition call | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,10 +33,10 @@ function Test-GetAllADGroups | |
| .SYNOPSIS | ||
| Tests getting Active Directory groups. | ||
| #> | ||
| function Test-GetADGroupWithSearchString | ||
| function Test-GetADGroupWithSearchString | ||
| { | ||
| param([string]$displayName) | ||
|
|
||
| # Test | ||
| # Select at most 10 groups. Groups are restricted to contain "test" to fasten the test | ||
| $groups = Get-AzureRmADGroup -SearchString $displayName | ||
|
|
@@ -68,7 +68,7 @@ Tests getting Active Directory groups. | |
| function Test-GetADGroupWithObjectId | ||
| { | ||
| param([string]$objectId) | ||
|
|
||
| # Test | ||
| $groups = Get-AzureRmADGroup -ObjectId $objectId | ||
|
|
||
|
|
@@ -85,7 +85,7 @@ Tests getting Active Directory group with security enabled . | |
| function Test-GetADGroupSecurityEnabled | ||
| { | ||
| param([string]$objectId, [string]$securityEnabled) | ||
|
|
||
| # Test | ||
| $groups = Get-AzureRmADGroup -ObjectId $objectId | ||
|
|
||
|
|
@@ -134,8 +134,8 @@ function Test-GetADGroupMemberWithGroupObjectId | |
|
|
||
| # Test | ||
| $members = Get-AzureRmADGroupMember -GroupObjectId $groupObjectId | ||
| # Assert | ||
|
|
||
| # Assert | ||
| Assert-AreEqual $members.Count 1 | ||
| Assert-AreEqual $members[0].Id $userObjectId | ||
| Assert-AreEqual $members[0].DisplayName $userName | ||
|
|
@@ -148,7 +148,7 @@ Tests getting members from an Active Directory group. | |
| function Test-GetADGroupMemberWithBadGroupObjectId | ||
| { | ||
| # Test | ||
| Assert-Throws { Get-AzureRmADGroupMember -GroupObjectId "baadc0de-baad-c0de-baad-c0debaadc0de" } | ||
| Assert-Throws { Get-AzureRmADGroupMember -GroupObjectId "baadc0de-baad-c0de-baad-c0debaadc0de" } | ||
| } | ||
|
|
||
| <# | ||
|
|
@@ -160,7 +160,7 @@ function Test-GetADGroupMemberWithUserObjectId | |
| param([string]$objectId) | ||
|
|
||
| # Test | ||
| Assert-Throws { Get-AzureRmADGroupMember -GroupObjectId $objectId } | ||
| Assert-Throws { Get-AzureRmADGroupMember -GroupObjectId $objectId } | ||
| } | ||
|
|
||
| <# | ||
|
|
@@ -173,8 +173,8 @@ function Test-GetADGroupMemberFromEmptyGroup | |
|
|
||
| # Test | ||
| $members = Get-AzureRmADGroupMember -GroupObjectId $objectId | ||
| # Assert | ||
|
|
||
| # Assert | ||
| Assert-Null($members) | ||
| } | ||
|
|
||
|
|
@@ -462,7 +462,7 @@ function Test-NewADApplication | |
|
|
||
| # Assert | ||
| Assert-NotNull $application | ||
| $apps = Get-AzureRmADApplication | ||
| $apps = Get-AzureRmADApplication | ||
| Assert-NotNull $apps | ||
| Assert-True { $apps.Count -ge 0 } | ||
|
|
||
|
|
@@ -489,13 +489,13 @@ function Test-NewADApplication | |
| $newDisplayName = getAssetName | ||
| $newHomePage = "http://" + $newDisplayName + ".com" | ||
| $newIdentifierUri = "http://" + $newDisplayName | ||
|
|
||
| # Update displayName and HomePage | ||
| Set-AzureRmADApplication -ObjectId $application.ObjectId -DisplayName $newDisplayName -HomePage $newHomePage | ||
|
|
||
| # Update identifierUri | ||
| # Update identifierUri | ||
| Set-AzureRmADApplication -ApplicationId $application.ApplicationId -IdentifierUris $newIdentifierUri | ||
|
|
||
| # Get application and verify updated properties | ||
| $app1 = Get-AzureRmADApplication -ObjectId $application.ObjectId | ||
| Assert-NotNull $app1 | ||
|
|
@@ -504,7 +504,7 @@ function Test-NewADApplication | |
| Assert-AreEqual $app1.HomePage $newHomePage | ||
| Assert-AreEqual $app1.IdentifierUris[0] $newIdentifierUri | ||
|
|
||
| # Delete | ||
| # Delete | ||
| Remove-AzureRmADApplication -ObjectId $application.ObjectId -Force | ||
| } | ||
|
|
||
|
|
@@ -543,7 +543,7 @@ function Test-NewADServicePrincipal | |
| Tests Creating and deleting service principal without an exisitng application. | ||
| #> | ||
| function Test-NewADServicePrincipalWithoutApp | ||
| { | ||
| { | ||
| # Setup | ||
| $displayName = getAssetName | ||
|
|
||
|
|
@@ -573,7 +573,7 @@ function Test-NewADServicePrincipalWithoutApp | |
|
|
||
| # update SP displayName | ||
| $newDisplayName = getAssetName | ||
|
|
||
| Set-AzureRmADServicePrincipal -ObjectId $servicePrincipal.Id -DisplayName $newDisplayName | ||
|
|
||
| # Get SP and verify updated name | ||
|
|
@@ -588,12 +588,78 @@ function Test-NewADServicePrincipalWithoutApp | |
| Assert-Throws { Remove-AzureRmADServicePrincipal -ObjectId $servicePrincipal.Id -Force} | ||
| } | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Tests creating a service principal with reader permissions | ||
| #> | ||
| function Test-NewADServicePrincipalWithReaderRole | ||
| { | ||
| # Setup | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add another test where in we dont specify any role or scope and make sure that no roleassignment change happens before and after running the command
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @darshanhs90 added a check in |
||
| $displayName = getAssetName | ||
| $roleDefinitionName = "Reader" | ||
|
|
||
| # Test | ||
| $servicePrincipal = New-AzureRmADServicePrincipal -DisplayName $displayName -Role $roleDefinitionName | ||
| Assert-NotNull $servicePrincipal | ||
| Assert-AreEqual $servicePrincipal.DisplayName $displayName | ||
|
|
||
| try | ||
| { | ||
| $role = Get-AzureRmRoleAssignment -ObjectId $servicePrincipal.Id | ||
| Assert-AreEqual $role.Count 1 | ||
| Assert-AreEqual $role.DisplayName $servicePrincipal.DisplayName | ||
| Assert-AreEqual $role.ObjectId $servicePrincipal.Id | ||
| Assert-AreEqual $role.RoleDefinitionName $roleDefinitionName | ||
| Assert-AreEqual $role.ObjectType "ServicePrincipal" | ||
| } | ||
| finally | ||
| { | ||
| Remove-AzureRmADApplication -ApplicationId $servicePrincipal.ApplicationId -Force | ||
| Remove-AzureRmRoleAssignment -ObjectId $servicePrincipal.Id -RoleDefinitionName $roleDefinitionName | ||
| } | ||
| } | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Tests creating a service principal with permissions over a custom scope | ||
| #> | ||
| function Test-NewADServicePrincipalWithCustomScope | ||
| { | ||
| # Setup | ||
| $displayName = getAssetName | ||
| $defaultRoleDefinitionName = "Contributor" | ||
| $subscription = Get-AzureRmSubscription | Select -Last 1 -Wait | ||
| $resourceGroup = Get-AzureRmResourceGroup | Select -Last 1 -Wait | ||
| $scope = "/subscriptions/" + $subscription.Id + "/resourceGroups/" + $resourceGroup.ResourceGroupName | ||
|
|
||
| # Test | ||
| $servicePrincipal = New-AzureRmADServicePrincipal -DisplayName $displayName -Scope $scope | ||
| Assert-NotNull $servicePrincipal | ||
| Assert-AreEqual $servicePrincipal.DisplayName $displayName | ||
|
|
||
| try | ||
| { | ||
| $role = Get-AzureRmRoleAssignment -ObjectId $servicePrincipal.Id | ||
| Assert-AreEqual $role.Count 1 | ||
| Assert-AreEqual $role.DisplayName $servicePrincipal.DisplayName | ||
| Assert-AreEqual $role.ObjectId $servicePrincipal.Id | ||
| Assert-AreEqual $role.RoleDefinitionName $defaultRoleDefinitionName | ||
| Assert-AreEqual $role.Scope $scope | ||
| Assert-AreEqual $role.ObjectType "ServicePrincipal" | ||
| } | ||
| finally | ||
| { | ||
| Remove-AzureRmADApplication -ApplicationId $servicePrincipal.ApplicationId -Force | ||
| Remove-AzureRmRoleAssignment -ObjectId $servicePrincipal.Id -Scope $scope -RoleDefinitionName $defaultRoleDefinitionName | ||
| } | ||
| } | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Tests Creating and deleting application using Password Credentials. | ||
| #> | ||
| function Test-CreateDeleteAppPasswordCredentials | ||
| { | ||
| { | ||
| # Setup | ||
| $displayName = getAssetName | ||
| $identifierUri = "http://" + $displayName | ||
|
|
@@ -629,7 +695,7 @@ function Test-CreateDeleteAppPasswordCredentials | |
|
|
||
| # Remove cred by KeyId | ||
| Remove-AzureRmADAppCredential -ApplicationId $application.ApplicationId -KeyId $cred.KeyId -Force | ||
| $cred3 = Get-AzureRmADAppCredential -ApplicationId $application.ApplicationId | ||
| $cred3 = Get-AzureRmADAppCredential -ApplicationId $application.ApplicationId | ||
| Assert-NotNull $cred3 | ||
| Assert-AreEqual $cred3.Count 1 | ||
| Assert-AreEqual $cred3[0].KeyId $cred1.KeyId | ||
|
|
@@ -642,7 +708,7 @@ function Test-CreateDeleteAppPasswordCredentials | |
| $newApplication = Get-AzureRmADApplication -DisplayNameStartWith "PowershellTestingApp" | ||
| Assert-Throws { New-AzureRmADAppCredential -ApplicationId $newApplication.ApplicationId -Password "Somedummypwd"} | ||
|
|
||
| # Remove App | ||
| # Remove App | ||
| Remove-AzureRmADApplication -ObjectId $application.ObjectId -Force | ||
| } | ||
|
|
||
|
|
@@ -652,7 +718,7 @@ function Test-CreateDeleteAppPasswordCredentials | |
| Tests Creating and deleting application using Service Principal Credentials. | ||
| #> | ||
| function Test-CreateDeleteSpPasswordCredentials | ||
| { | ||
| { | ||
| # Setup | ||
| $displayName = getAssetName | ||
| $password = getAssetName | ||
|
|
@@ -689,7 +755,7 @@ function Test-CreateDeleteSpPasswordCredentials | |
|
|
||
| # Remove cred by KeyId | ||
| Remove-AzureRmADSpCredential -ServicePrincipalName $servicePrincipal.ServicePrincipalNames[0] -KeyId $cred.KeyId -Force | ||
| $cred3 = Get-AzureRmADSpCredential -ServicePrincipalName $servicePrincipal.ServicePrincipalNames[0] | ||
| $cred3 = Get-AzureRmADSpCredential -ServicePrincipalName $servicePrincipal.ServicePrincipalNames[0] | ||
| Assert-NotNull $cred3 | ||
| Assert-AreEqual $cred3.Count 1 | ||
| Assert-AreEqual $cred3[0].KeyId $cred1.KeyId | ||
|
|
@@ -701,7 +767,7 @@ function Test-CreateDeleteSpPasswordCredentials | |
| } | ||
| Finally | ||
| { | ||
| # Remove App | ||
| # Remove App | ||
| $app = Get-AzureRmADApplication -ApplicationId $servicePrincipal.ApplicationId | ||
| Remove-AzureRmADApplication -ObjectId $app.ObjectId -Force | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Role?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darshanhs90 oops, thanks. Fixed this in all occurrences.