Skip to content

Commit 387f72f

Browse files
markcowlmarkcowl
authored
Merge pull request #4019 from wisexp/preview
force input parameter in NewAzureRoleAssignmentCommand to follow vali…
2 parents afea140 + c2392ca commit 387f72f

File tree

9 files changed

+1016
-14
lines changed

9 files changed

+1016
-14
lines changed

‎src/ResourceManager/Resources/Commands.Resources.Test/Commands.Resources.Test.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@
201201
<Compile Include="Providers\UnregisterResourceProviderCmdletTests.cs" />
202202
<Compile Include="ResourceGroupDeployments\RemoveAzureResourceGroupDeploymentCommandTests.cs" />
203203
<Compile Include="ResourceGroups\SetAzureResourceGroupCommandTests.cs" />
204+
<Compile Include="RoleAssignment\RoleAssignmentUnitTests.cs" />
204205
<Compile Include="ScenarioTests\ActiveDirectoryTests.cs" />
205206
<Compile Include="ScenarioTests\AuthorizationTests.cs" />
206207
<Content Include="ScenarioTests\ProviderFeatureTests.ps1">
@@ -664,6 +665,9 @@
664665
<None Include="SessionRecords\Microsoft.Azure.Commands.Resources.Test.ScenarioTests.RoleAssignmentTests\RaUserPermissions_Test.json">
665666
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
666667
</None>
668+
<None Include="SessionRecords\Microsoft.Azure.Commands.Resources.Test.ScenarioTests.RoleAssignmentTests\RaValidateInputParameters.json">
669+
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
670+
</None>
667671
<None Include="SessionRecords\Microsoft.Azure.Commands.Resources.Test.ScenarioTests.RoleDefinitionTests\RdNegativeScenarios.json">
668672
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
669673
</None>

‎src/ResourceManager/Resources/Commands.Resources.Test/RoleAssignment/RoleAssignmentUnitTests.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
using Microsoft.Azure.Commands.Resources.Models.Authorization;
2+
using Microsoft.WindowsAzure.Commands.ScenarioTest;
3+
using System;
4+
using System.Collections.Generic;
5+
using Xunit;
6+
7+
namespace Microsoft.Azure.Commands.Resources.Test {
8+
public class RoleAssignmentUnitTests
9+
{
10+
private void VerifyInvalidScope(string scope, string error)
11+
{
12+
try
13+
{
14+
AuthorizationClient.ValidateScope(scope);
15+
Assert.True(false);
16+
}
17+
catch(ArgumentException ex)
18+
{
19+
Assert.Equal(ex.Message, error);
20+
}
21+
}
22+
23+
private void VerifyValidScope(string scope)
24+
{
25+
AuthorizationClient.ValidateScope(scope);
26+
}
27+
28+
[Fact]
29+
[Trait(Category.AcceptanceType, Category.CheckIn)]
30+
public void VerifyInvalidScopes()
31+
{
32+
Dictionary<string, string> scopeAndErrors = new Dictionary<string, string>();
33+
scopeAndErrors.Add("test", "Scope 'test' should begin with '/subscriptions' or '/providers'.");
34+
scopeAndErrors.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/Should be 'ResourceGroups'/any group name", "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/Should be 'ResourceGroups'/any group name' should begin with '/subscriptions/<subid>/resourceGroups'.");
35+
scopeAndErrors.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups", "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups' should have even number of parts.");
36+
scopeAndErrors.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/", "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/' should not have any empty part.");
37+
scopeAndErrors.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Should be 'Providers'/any provider name", "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Should be 'Providers'/any provider name' should begin with '/subscriptions/<subid>/resourceGroups/<groupname>/providers'.");
38+
scopeAndErrors.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Providers/providername", "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Providers/providername' should have at least one pair of resource type and resource name. e.g. '/subscriptions/<subid>/resourceGroups/<groupname>/providers/<providername>/<resourcetype>/<resourcename>'.");
39+
foreach (var kvp in scopeAndErrors)
40+
{
41+
VerifyInvalidScope(kvp.Key, kvp.Value);
42+
}
43+
}
44+
45+
[Fact]
46+
[Trait(Category.AcceptanceType, Category.CheckIn)]
47+
public void VerifyValidScopes()
48+
{
49+
List<string> validScopes = new List<string>();
50+
validScopes.Add("/");
51+
validScopes.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab");
52+
validScopes.Add("/providers/providername");
53+
validScopes.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname");
54+
validScopes.Add("/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Providers/providername/type/typename");
55+
56+
foreach (var scope in validScopes)
57+
{
58+
VerifyValidScope(scope);
59+
}
60+
}
61+
}
62+
}

‎src/ResourceManager/Resources/Commands.Resources.Test/ScenarioTests/RoleAssignmentTests.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,14 @@ public void RaByResource()
7878
{
7979
ResourcesController.NewInstance.RunPsTest("Test-RaByResource");
8080
}
81-
81+
82+
[Fact]
83+
[Trait(Category.AcceptanceType, Category.CheckIn)]
84+
public void RaValidateInputParameters()
85+
{
86+
ResourcesController.NewInstance.RunPsTest("Test-RaValidateInputParameters");
87+
}
88+
8289
[Fact]
8390
[Trait(Category.AcceptanceType, Category.CheckIn)]
8491
public void RaByServicePrincipal()

‎src/ResourceManager/Resources/Commands.Resources.Test/ScenarioTests/RoleAssignmentTests.ps1

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,60 @@ function Test-RaByResource
169169
VerifyRoleAssignmentDeleted $newAssignment
170170
}
171171

172+
<#
173+
.SYNOPSIS
174+
Tests validate input parameters
175+
#>
176+
function Test-RaValidateInputParameters
177+
{
178+
# Setup
179+
Add-Type -Path ".\\Microsoft.Azure.Commands.Resources.dll"
180+
181+
$definitionName = 'Owner'
182+
$groups = Get-AzureRmADGroup | Select-Object -Last 1 -Wait
183+
Assert-AreEqual 1 $groups.Count "There should be at least one group to run the test."
184+
$resourceGroups = Get-AzureRmResourceGroup | Select-Object -Last 1 -Wait
185+
Assert-AreEqual 1 $resourceGroups.Count "No resource group found. Unable to run the test."
186+
$resource = Get-AzureRmResource | Select-Object -Last 1 -Wait
187+
Assert-NotNull $resource "Cannot find any resource to continue test execution."
188+
189+
# Test
190+
# Check if Scope is valid.
191+
$scope = "/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/Should be 'ResourceGroups'/any group name"
192+
$invalidScope = "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/Should be 'ResourceGroups'/any group name' should begin with '/subscriptions/<subid>/resourceGroups'."
193+
Assert-Throws { New-AzureRmRoleAssignment -Scope $scope -ObjectId $groups[0].Id.Guid -RoleDefinitionName $definitionName } $invalidScope
194+
195+
$scope = "/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups"
196+
$invalidScope = "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups' should have even number of parts."
197+
Assert-Throws { New-AzureRmRoleAssignment -Scope $scope -ObjectId $groups[0].Id.Guid -RoleDefinitionName $definitionName } $invalidScope
198+
199+
$scope = "/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/"
200+
$invalidScope = "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/' should not have any empty part."
201+
Assert-Throws { New-AzureRmRoleAssignment -Scope $scope -ObjectId $groups[0].Id.Guid -RoleDefinitionName $definitionName } $invalidScope
202+
203+
$scope = "/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Should be 'Providers'/any provider name"
204+
$invalidScope = "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Should be 'Providers'/any provider name' should begin with '/subscriptions/<subid>/resourceGroups/<groupname>/providers'."
205+
Assert-Throws { New-AzureRmRoleAssignment -Scope $scope -ObjectId $groups[0].Id.Guid -RoleDefinitionName $definitionName } $invalidScope
206+
207+
$scope = "/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Providers/providername"
208+
$invalidScope = "Scope '/subscriptions/e9ee799d-6ab2-4084-b952-e7c86344bbab/ResourceGroups/groupname/Providers/providername' should have at least one pair of resource type and resource name. e.g. '/subscriptions/<subid>/resourceGroups/<groupname>/providers/<providername>/<resourcetype>/<resourcename>'."
209+
Assert-Throws { New-AzureRmRoleAssignment -Scope $scope -ObjectId $groups[0].Id.Guid -RoleDefinitionName $definitionName } $invalidScope
210+
211+
# Check if ResourceType is valid
212+
[Microsoft.Azure.Commands.Resources.Models.Authorization.AuthorizationClient]::RoleAssignmentNames.Enqueue("4FAB3AF0-E7CF-4305-97D1-23A65EDCE8E6")
213+
Assert-AreEqual $resource.ResourceType "Microsoft.Sql/servers"
214+
215+
# Below invalid resource type should not return 'Not supported api version'.
216+
$resource.ResourceType = "Microsoft.Sql/"
217+
$invalidResourceType = "Scope '/subscriptions/4004a9fd-d58e-48dc-aeb2-4a4aec58606f/resourceGroups/testrg16987/providers/Microsoft.Sql/testserver13673' should have even number of parts."
218+
Assert-Throws { New-AzureRmRoleAssignment `
219+
-ObjectId $groups[0].Id.Guid `
220+
-RoleDefinitionName $definitionName `
221+
-ResourceGroupName $resource.ResourceGroupName `
222+
-ResourceType $resource.ResourceType `
223+
-ResourceName $resource.Name } $invalidResourceType
224+
}
225+
172226
<#
173227
.SYNOPSIS
174228
Tests verifies creation and deletion of a RoleAssignments for Service principal name

‎src/ResourceManager/Resources/Commands.Resources.Test/SessionRecords/Microsoft.Azure.Commands.Resources.Test.ScenarioTests.RoleAssignmentTests/RaValidateInputParameters.json

Lines changed: 744 additions & 0 deletions
Large diffs are not rendered by default.

‎src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationClient.cs

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,11 @@ public PSRoleAssignment CreateRoleAssignment(FilterRoleAssignmentsOptions parame
157157
{
158158
Guid principalId = ActiveDirectoryClient.GetObjectId(parameters.ADObjectFilter);
159159
Guid roleAssignmentId = RoleAssignmentNames.Count == 0 ? Guid.NewGuid() : RoleAssignmentNames.Dequeue();
160+
string scope = parameters.Scope;
161+
ValidateScope(scope);
160162
string roleDefinitionId = !string.IsNullOrEmpty(parameters.RoleDefinitionName)
161-
? AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(parameters.Scope, GetSingleRoleDefinitionByName(parameters.RoleDefinitionName, parameters.Scope).Id)
162-
: AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(parameters.Scope, parameters.RoleDefinitionId);
163+
? AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(scope, GetSingleRoleDefinitionByName(parameters.RoleDefinitionName, scope).Id)
164+
: AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(scope, parameters.RoleDefinitionId);
163165

164166
RoleAssignmentCreateParameters createParameters = new RoleAssignmentCreateParameters
165167
{
@@ -498,5 +500,66 @@ private static void ValidateRoleDefinition(PSRoleDefinition roleDefinition)
498500
throw new ArgumentException(ProjectResources.InvalidActions);
499501
}
500502
}
503+
504+
public static void ValidateScope(string scope)
505+
{
506+
507+
if (scope.Length == 0 || !scope.StartsWith("/"))
508+
{
509+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldBeginWithSubscriptionsOrProviders, scope));
510+
}
511+
512+
// "/" is a valid scope.
513+
if (string.Compare(scope, "/") == 0)
514+
{
515+
return;
516+
}
517+
518+
var parts = scope.Substring(1).Split('/'); // Skip the leading '/'
519+
520+
if (parts.Contains(string.Empty))
521+
{
522+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldHaveNoEmptyPart, scope));
523+
}
524+
525+
int count = parts.Count();
526+
527+
if (count % 2 != 0)
528+
{
529+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldHaveEvenNumberOfParts, scope));
530+
}
531+
532+
// For Scope that on tenant level, without subscriptions, should start with "providers"
533+
// in this case we only ensure that the number of parts is even.
534+
if (string.Compare(parts[0], "providers", true) == 0)
535+
{
536+
return;
537+
}
538+
else if (string.Compare(parts[0], "subscriptions", true) != 0)
539+
{
540+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldBeginWithSubscriptionsOrProviders, scope));
541+
}
542+
543+
// Now check the case that scope begins with '/subscriptions'
544+
545+
if (count >= 4 && string.Compare(parts[2], "resourceGroups", true) != 0)
546+
{
547+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldBeginWithSubscriptionsAndResourceGroups, scope));
548+
}
549+
550+
if (count >= 6)
551+
{
552+
if (string.Compare(parts[4], "providers", true) != 0)
553+
{
554+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldBeginWithSubscriptionsAndResourceGroupsAndProviders, scope));
555+
}
556+
557+
if (count < 8)
558+
{
559+
throw new ArgumentException(string.Format(ProjectResources.ScopeShouldHaveAtLeastOnePairOfResourceTypeAndName, scope));
560+
}
561+
}
562+
563+
}
501564
}
502565
}

‎src/ResourceManager/Resources/Commands.Resources/Models.Authorization/FilterRoleAssignmentsOptions.cs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,19 @@ public string Scope
3131
{
3232
get
3333
{
34-
string result;
35-
string resourceIdentifier = ResourceIdentifier.ToString();
36-
3734
if (!string.IsNullOrEmpty(scope))
3835
{
39-
result = scope;
40-
}
41-
else if (!string.IsNullOrEmpty(resourceIdentifier))
42-
{
43-
result = resourceIdentifier;
36+
return scope;
4437
}
45-
else
38+
39+
string resourceIdentifier = ResourceIdentifier.ToString();
40+
41+
if (!string.IsNullOrEmpty(resourceIdentifier))
4642
{
47-
result = null;
43+
return resourceIdentifier;
4844
}
4945

50-
return result;
46+
return null;
5147
}
5248
set
5349
{

‎src/ResourceManager/Resources/Commands.Resources/Properties/Resources.Designer.cs

Lines changed: 54 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/ResourceManager/Resources/Commands.Resources/Properties/Resources.resx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,24 @@
243243
<data name="ResourceProviderNotFound" xml:space="preserve">
244244
<value>Could not find the resource provider '{0}'.</value>
245245
</data>
246+
<data name="ScopeShouldBeginWithSubscriptionsAndResourceGroups" xml:space="preserve">
247+
<value>Scope '{0}' should begin with '/subscriptions/&lt;subid&gt;/resourceGroups'.</value>
248+
</data>
249+
<data name="ScopeShouldBeginWithSubscriptionsOrProviders" xml:space="preserve">
250+
<value>Scope '{0}' should begin with '/subscriptions' or '/providers'.</value>
251+
</data>
252+
<data name="ScopeShouldBeginWithSubscriptionsAndResourceGroupsAndProviders" xml:space="preserve">
253+
<value>Scope '{0}' should begin with '/subscriptions/&lt;subid&gt;/resourceGroups/&lt;groupname&gt;/providers'.</value>
254+
</data>
255+
<data name="ScopeShouldHaveAtLeastOnePairOfResourceTypeAndName" xml:space="preserve">
256+
<value>Scope '{0}' should have at least one pair of resource type and resource name. e.g. '/subscriptions/&lt;subid&gt;/resourceGroups/&lt;groupname&gt;/providers/&lt;providername&gt;/&lt;resourcetype&gt;/&lt;resourcename&gt;'.</value>
257+
</data>
258+
<data name="ScopeShouldHaveEvenNumberOfParts" xml:space="preserve">
259+
<value>Scope '{0}' should have even number of parts.</value>
260+
</data>
261+
<data name="ScopeShouldHaveNoEmptyPart" xml:space="preserve">
262+
<value>Scope '{0}' should not have any empty part.</value>
263+
</data>
246264
<data name="UnregisteringProvider" xml:space="preserve">
247265
<value>Are you sure you want to unregister the provider '{0}'</value>
248266
</data>

0 commit comments

Comments
 (0)