Skip to content

Commit c4e0d6a

Browse files
committed
Address review feedback for authorization commands
1 parent af617d6 commit c4e0d6a

File tree

5 files changed

+38
-10
lines changed

5 files changed

+38
-10
lines changed

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,28 +130,26 @@ public PSRoleAssignment CreateRoleAssignment(FilterRoleAssignmentsOptions parame
130130
/// </summary>
131131
/// <param name="options">The filtering options</param>
132132
/// <param name="currentSubscription">The current subscription</param>
133-
/// <param name="expandPrincipalGroups">If true, assignments to the specified (user) principal's groups are also returned, in addition to direct assignments to the user</param>
134-
/// <param name="includeClassicAdmins">If true, the subscription classic administrators are also returned as assignments</param>
135133
/// <returns>The filtered role assignments</returns>
136-
public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions options, string currentSubscription, bool expandPrincipalGroups = false, bool includeClassicAdmins = false)
134+
public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions options, string currentSubscription)
137135
{
138136
List<PSRoleAssignment> result = new List<PSRoleAssignment>();
139137
ListAssignmentsFilterParameters parameters = new ListAssignmentsFilterParameters();
140138

141139
if (options.ADObjectFilter.HasFilter)
142140
{
143141
// Filter first by principal
144-
if (expandPrincipalGroups)
142+
if (options.ExpandPrincipalGroups)
145143
{
146144
PSADObject adObject = ActiveDirectoryClient.GetADObject(options.ADObjectFilter);
147145
if (adObject == null)
148146
{
149-
throw new KeyNotFoundException("The provided information does not map to an AD object.");
147+
throw new KeyNotFoundException(ProjectResources.PrincipalNotFound);
150148
}
151149

152150
if (!(adObject is PSADUser))
153151
{
154-
throw new InvalidOperationException("ExpandPrincipalGroups is only supported for a User principal");
152+
throw new InvalidOperationException(ProjectResources.ExpandGroupsNotSupported);
155153
}
156154

157155
parameters.AssignedToPrincipalId = adObject.Id;
@@ -188,7 +186,7 @@ public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions
188186
result = result.Where(r => r.RoleDefinitionName.Equals(options.RoleDefinition, StringComparison.OrdinalIgnoreCase)).ToList();
189187
}
190188

191-
if (includeClassicAdmins)
189+
if (options.IncludeClassicAdministrators)
192190
{
193191
// Get classic administrator access assignments
194192
List<ClassicAdministrator> classicAdministrators = AuthorizationManagementClient.ClassicAdministrators.List().ClassicAdministrators.ToList();

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,9 @@ public string Scope
5353
public ResourceIdentifier ResourceIdentifier { get; set; }
5454

5555
public ADObjectFilterOptions ADObjectFilter { get; set; }
56+
57+
public bool ExpandPrincipalGroups { get; set; }
58+
59+
public bool IncludeClassicAdministrators { get; set; }
5660
}
5761
}

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

Lines changed: 19 additions & 1 deletion
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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,4 +306,10 @@
306306
<data name="DeploymentDoesntExist" xml:space="preserve">
307307
<value>Deployment '{0}' doesn't exist under resource group '{1}'</value>
308308
</data>
309+
<data name="ExpandGroupsNotSupported" xml:space="preserve">
310+
<value>ExpandPrincipalGroups is only supported for a User principal</value>
311+
</data>
312+
<data name="PrincipalNotFound" xml:space="preserve">
313+
<value>Cannot find principal using the specified options</value>
314+
</data>
309315
</root>

src/ResourceManager/Resources/Commands.Resources/RoleAssignments/GetAzureRoleAssignmentCommand.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,12 @@ protected override void ProcessRecord()
262262
ResourceName = ResourceName,
263263
ResourceType = ResourceType,
264264
Subscription = string.IsNullOrEmpty(ResourceGroupName) ? null : DefaultProfile.DefaultContext.Subscription.Id.ToString()
265-
}
265+
},
266+
ExpandPrincipalGroups = ExpandPrincipalGroups.IsPresent,
267+
IncludeClassicAdministrators = IncludeClassicAdministrators.IsPresent
266268
};
267269

268-
WriteObject(PoliciesClient.FilterRoleAssignments(options, DefaultProfile.DefaultContext.Subscription.Id.ToString(), ExpandPrincipalGroups.IsPresent, IncludeClassicAdministrators.IsPresent), true);
270+
WriteObject(PoliciesClient.FilterRoleAssignments(options, DefaultProfile.DefaultContext.Subscription.Id.ToString()), true);
269271
}
270272
}
271273
}

0 commit comments

Comments
 (0)