Merge pull request Azure#1175 from shuagarw/DocumentationUpdatesAndFixes

stankovski · stankovski · commit dd85df91b29a · 2015-10-27T09:04:10.000-07:00
Role commands documentation updates and some bug fixes
diff --git a/src/Common/Commands.Common/Commands.Common.csproj b/src/Common/Commands.Common/Commands.Common.csproj
@@ -173,6 +173,7 @@
     <Compile Include="TestMockSupport.cs" />
     <Compile Include="PSAzureAccount.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="ValidateGuidNotEmpty.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="Properties\Resources.resx">
diff --git a/src/Common/Commands.Common/ValidateGuidNotEmpty.cs b/src/Common/Commands.Common/ValidateGuidNotEmpty.cs
@@ -0,0 +1,27 @@
+﻿using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Management.Automation;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.WindowsAzure.Commands.Common
+{
+    [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
+    public class ValidateGuidNotEmptyAttribute : ValidateArgumentsAttribute
+    {
+        protected override void Validate(object arguments, EngineIntrinsics engineIntrinsics)
+        {
+            if (arguments == null)
+            {
+                throw new ValidationMetadataException("Specify a parameter of type 'System.Guid' and try again.");
+            }
+
+            Guid param = (Guid)arguments;
+            if (param == Guid.Empty)
+            {
+                throw new ValidationMetadataException("Specify a non empty value of type 'System.Guid' and try again.");
+            }
+        }
+    }
+}
diff --git a/src/ResourceManager/Resources/Commands.Resources/Microsoft.Azure.Commands.Resources.dll-Help.xml b/src/ResourceManager/Resources/Commands.Resources/Microsoft.Azure.Commands.Resources.dll-Help.xml
diff --git a/src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationClient.cs b/src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationClient.cs
@@ -134,7 +134,7 @@ public PSRoleAssignment CreateRoleAssignment(FilterRoleAssignmentsOptions parame
             Guid roleAssignmentId = RoleAssignmentNames.Count == 0 ? Guid.NewGuid() : RoleAssignmentNames.Dequeue();
             string roleDefinitionId = !string.IsNullOrEmpty(parameters.RoleDefinitionName)
                 ? AuthorizationHelper.GetRoleDefinitionFullyQualifiedId(subscriptionId, GetRoleRoleDefinition(parameters.RoleDefinitionName).Id)
-                : parameters.RoleDefinitionId;
+                : AuthorizationHelper.GetRoleDefinitionFullyQualifiedId(subscriptionId, parameters.RoleDefinitionId);
 
             RoleAssignmentCreateParameters createParameters = new RoleAssignmentCreateParameters
             {
@@ -189,7 +189,7 @@ public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions
 
                 result.AddRange(AuthorizationManagementClient.RoleAssignments.List(parameters)
                     .RoleAssignments
-                    .FilterRoleAssignmentsOnRoleId(options.RoleDefinitionId)
+                    .FilterRoleAssignmentsOnRoleId(AuthorizationHelper.GetRoleDefinitionFullyQualifiedId(currentSubscription, options.RoleDefinitionId))
                     .ToPSRoleAssignments(this, ActiveDirectoryClient, options.ExcludeAssignmentsForDeletedPrincipals));
 
                 // Filter out by scope
@@ -205,13 +205,15 @@ public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions
 
                 result.AddRange(AuthorizationManagementClient.RoleAssignments.ListForScope(options.Scope, parameters)
                     .RoleAssignments
-                    .FilterRoleAssignmentsOnRoleId(options.RoleDefinitionId)
+                    .FilterRoleAssignmentsOnRoleId(AuthorizationHelper.GetRoleDefinitionFullyQualifiedId(currentSubscription, options.RoleDefinitionId))
                     .ToPSRoleAssignments(this, ActiveDirectoryClient, options.ExcludeAssignmentsForDeletedPrincipals));
             }
             else
             {
                 result.AddRange(AuthorizationManagementClient.RoleAssignments.List(parameters)
-                    .RoleAssignments.ToPSRoleAssignments(this, ActiveDirectoryClient, options.ExcludeAssignmentsForDeletedPrincipals));
+                    .RoleAssignments
+                    .FilterRoleAssignmentsOnRoleId(AuthorizationHelper.GetRoleDefinitionFullyQualifiedId(currentSubscription, options.RoleDefinitionId))
+                    .ToPSRoleAssignments(this, ActiveDirectoryClient, options.ExcludeAssignmentsForDeletedPrincipals));
             }
 
             if (!string.IsNullOrEmpty(options.RoleDefinitionName))
@@ -251,11 +253,11 @@ public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions
         /// </summary>
         /// <param name="options">The role assignment filtering options</param>
         /// <returns>The deleted role assignments</returns>
-        public IEnumerable<PSRoleAssignment> RemoveRoleAssignment(FilterRoleAssignmentsOptions options)
+        public IEnumerable<PSRoleAssignment> RemoveRoleAssignment(FilterRoleAssignmentsOptions options, string subscriptionId)
         {
             // Match role assignments at exact scope. Ideally, atmost 1 roleAssignment should match the criteria 
             // but an edge case can have multiple role assignments to the same role or multiple role assignments to different roles, with same name.
-            IEnumerable<PSRoleAssignment> roleAssignments = FilterRoleAssignments(options, currentSubscription: string.Empty)
+            IEnumerable<PSRoleAssignment> roleAssignments = FilterRoleAssignments(options, subscriptionId)
                                                 .Where(ra => ra.Scope == options.Scope.TrimEnd('/'));
 
             if (roleAssignments == null || !roleAssignments.Any())
@@ -362,6 +364,8 @@ public PSRoleDefinition UpdateRoleDefinition(PSRoleDefinition role, string subsc
             roleDefinition.AssignableScopes = role.AssignableScopes ?? roleDefinition.AssignableScopes;
             roleDefinition.Description = role.Description ?? roleDefinition.Description;
 
+            ValidateRoleDefinition(roleDefinition);
+
             return
                 AuthorizationManagementClient.RoleDefinitions.CreateOrUpdate(
                     roleDefinitionId,
@@ -444,6 +448,11 @@ private static void ValidateRoleDefinition(PSRoleDefinition roleDefinition)
                 throw new ArgumentException(ProjectResources.InvalidRoleDefinitionName);
             }
 
+            if (string.IsNullOrWhiteSpace(roleDefinition.Description))
+            {
+                throw new ArgumentException(ProjectResources.InvalidRoleDefinitionDescription);
+            }
+
             if (roleDefinition.AssignableScopes == null || !roleDefinition.AssignableScopes.Any())
             {
                 throw new ArgumentException(ProjectResources.InvalidAssignableScopes);
diff --git a/src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationHelper.cs b/src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationHelper.cs
@@ -12,6 +12,11 @@ public class AuthorizationHelper
 
         public static string GetRoleDefinitionFullyQualifiedId(string subscriptionId, string roleDefinitionGuid)
         {
+            if(string.IsNullOrEmpty(roleDefinitionGuid))
+            {
+                return null;
+            }
+
             return string.Concat(string.Format(AuthorizationHelper.roleDefinitionIdPrefixFormat, subscriptionId), roleDefinitionGuid);
         }
     }
diff --git a/src/ResourceManager/Resources/Commands.Resources/Models.Authorization/FilterRoleAssignmentsOptions.cs b/src/ResourceManager/Resources/Commands.Resources/Models.Authorization/FilterRoleAssignmentsOptions.cs
@@ -18,26 +18,12 @@ namespace Microsoft.Azure.Commands.Resources.Models.Authorization
 {
     public class FilterRoleAssignmentsOptions
     {
-        private string roleDefinitionGuid;
-
         public string RoleDefinitionName { get; set; }
 
-        public string RoleDefinitionId
-        {
-            get
-            {
-                if (string.IsNullOrEmpty(roleDefinitionGuid))
-                {
-                    return null;
-                }
-
-                return AuthorizationHelper.GetRoleDefinitionFullyQualifiedId(this.ResourceIdentifier.Subscription, roleDefinitionGuid);
-            }
-            set
-            {
-                roleDefinitionGuid = value;
-            }
-        }
+        /// <summary>
+        /// RoleDefinitionId Guid
+        /// </summary>
+        public string RoleDefinitionId { get; set; }
 
         private string scope;
 
diff --git a/src/ResourceManager/Resources/Commands.Resources/Properties/Resources.Designer.cs b/src/ResourceManager/Resources/Commands.Resources/Properties/Resources.Designer.cs
diff --git a/src/ResourceManager/Resources/Commands.Resources/Properties/Resources.resx b/src/ResourceManager/Resources/Commands.Resources/Properties/Resources.resx
@@ -333,4 +333,10 @@
   <data name="ProviderOperationUnsupportedWildcard" xml:space="preserve">
     <value>Wildcard character ? is not supported.</value>
   </data>
+  <data name="InvalidRoleDefinitionDescription" xml:space="preserve">
+    <value>RoleDefinition Description is invalid.</value>
+  </data>
+  <data name="RemoveRoleDefinitionWithName" xml:space="preserve">
+    <value>Are you sure you want to remove role definition with name '{0}'.</value>
+  </data>
 </root>
diff --git a/src/ResourceManager/Resources/Commands.Resources/RoleAssignments/GetAzureRoleAssignmentCommand.cs b/src/ResourceManager/Resources/Commands.Resources/RoleAssignments/GetAzureRoleAssignmentCommand.cs
@@ -15,6 +15,7 @@
 using Microsoft.Azure.Commands.Resources.Models;
 using Microsoft.Azure.Commands.Resources.Models.ActiveDirectory;
 using Microsoft.Azure.Commands.Resources.Models.Authorization;
+using Microsoft.WindowsAzure.Commands.Common;
 using System;
 using System.Collections.Generic;
 using System.Management.Automation;
@@ -35,7 +36,9 @@ public class GetAzureRoleAssignmentCommand : ResourcesBaseCmdlet
             HelpMessage = "The user or group object id.")]
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.ScopeWithObjectId,
             HelpMessage = "The user or group object id.")]
-        [ValidateNotNullOrEmpty]
+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
+            HelpMessage = "The user or group object id.")]
+        [ValidateGuidNotEmpty]
         [Alias("Id", "PrincipalId")]
         public Guid ObjectId { get; set; }
 
@@ -150,6 +153,11 @@ public class GetAzureRoleAssignmentCommand : ResourcesBaseCmdlet
         [ValidateNotNullOrEmpty]
         public string RoleDefinitionName { get; set; }
 
+        [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
+            HelpMessage = "Role Id the principal is assigned to.")]
+        [ValidateGuidNotEmpty]
+        public Guid RoleDefinitionId { get; set; }
+
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.Scope,
             HelpMessage = "Scope of the role assignment. In the format of relative URI.")]
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.ScopeWithObjectId,
@@ -158,6 +166,8 @@ public class GetAzureRoleAssignmentCommand : ResourcesBaseCmdlet
             HelpMessage = "Scope of the role assignment. In the format of relative URI.")]
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.ScopeWithSPN,
             HelpMessage = "Scope of the role assignment. In the format of relative URI.")]
+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
+            HelpMessage = "Scope of the role assignment. In the format of relative URI.")]
         [ValidateNotNullOrEmpty]
         public string Scope { get; set; }
 
@@ -207,6 +217,7 @@ protected override void ProcessRecord()
             {
                 Scope = Scope,
                 RoleDefinitionName = RoleDefinitionName,
+                RoleDefinitionId = RoleDefinitionId == Guid.Empty ? null : RoleDefinitionId.ToString(),
                 ADObjectFilter = new ADObjectFilterOptions
                 {
                     SignInName = SignInName,
diff --git a/src/ResourceManager/Resources/Commands.Resources/RoleAssignments/NewAzureRoleAssignmentCommand.cs b/src/ResourceManager/Resources/Commands.Resources/RoleAssignments/NewAzureRoleAssignmentCommand.cs
@@ -15,6 +15,7 @@
 using Microsoft.Azure.Commands.Resources.Models;
 using Microsoft.Azure.Commands.Resources.Models.ActiveDirectory;
 using Microsoft.Azure.Commands.Resources.Models.Authorization;
+using Microsoft.WindowsAzure.Commands.Common;
 using System;
 using System.Management.Automation;
 
@@ -36,7 +37,7 @@ public class NewAzureRoleAssignmentCommand : ResourcesBaseCmdlet
             HelpMessage = "The user or group object id.")]
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
             HelpMessage = "The user or group object id.")]
-        [ValidateNotNullOrEmpty]
+        [ValidateGuidNotEmpty]
         [Alias("Id", "PrincipalId")]
         public Guid ObjectId { get; set; }
 
@@ -140,7 +141,7 @@ public class NewAzureRoleAssignmentCommand : ResourcesBaseCmdlet
 
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
             HelpMessage = "Role Id the principal is assigned to.")]
-        [ValidateNotNullOrEmpty]
+        [ValidateGuidNotEmpty]
         public Guid RoleDefinitionId { get; set; }
 
         protected override void ProcessRecord()
diff --git a/src/ResourceManager/Resources/Commands.Resources/RoleAssignments/RemoveAzureRoleAssignmentCommand.cs b/src/ResourceManager/Resources/Commands.Resources/RoleAssignments/RemoveAzureRoleAssignmentCommand.cs
@@ -15,6 +15,7 @@
 using Microsoft.Azure.Commands.Resources.Models;
 using Microsoft.Azure.Commands.Resources.Models.ActiveDirectory;
 using Microsoft.Azure.Commands.Resources.Models.Authorization;
+using Microsoft.WindowsAzure.Commands.Common;
 using System;
 using System.Collections.Generic;
 using System.Management.Automation;
@@ -38,7 +39,7 @@ public class RemoveAzureRoleAssignmentCommand : ResourcesBaseCmdlet
             HelpMessage = "The user or group object id.")]
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
             HelpMessage = "The user or group object id.")]
-        [ValidateNotNullOrEmpty]
+        [ValidateGuidNotEmpty]
         [Alias("Id", "PrincipalId")]
         public Guid ObjectId { get; set; }
 
@@ -142,7 +143,7 @@ public class RemoveAzureRoleAssignmentCommand : ResourcesBaseCmdlet
 
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleIdWithScopeAndObjectId,
             HelpMessage = "Role Id the principal is assigned to.")]
-        [ValidateNotNullOrEmpty]
+        [ValidateGuidNotEmpty]
         public Guid RoleDefinitionId { get; set; }
 
         [Parameter(Mandatory = false)]
@@ -184,7 +185,7 @@ protected override void ProcessRecord()
                 options.RoleDefinitionName ?? RoleDefinitionId.ToString()),
                 ProjectResources.RemovingRoleAssignment,
                 null,
-                () => roleAssignments = PoliciesClient.RemoveRoleAssignment(options));
+                () => roleAssignments = PoliciesClient.RemoveRoleAssignment(options, DefaultProfile.Context.Subscription.Id.ToString()));
 
             if (PassThru)
             {
diff --git a/src/ResourceManager/Resources/Commands.Resources/RoleDefinitions/GetAzureRoleDefinitionCommand.cs b/src/ResourceManager/Resources/Commands.Resources/RoleDefinitions/GetAzureRoleDefinitionCommand.cs
@@ -15,6 +15,7 @@
 using Microsoft.Azure.Commands.Resources.Models;
 using Microsoft.Azure.Commands.Resources.Models.ActiveDirectory;
 using Microsoft.Azure.Commands.Resources.Models.Authorization;
+using Microsoft.WindowsAzure.Commands.Common;
 using System;
 using System.Collections.Generic;
 using System.Management.Automation;
@@ -34,7 +35,7 @@ public class GetAzureRoleDefinitionCommand : ResourcesBaseCmdlet
 
         [Parameter(Position = 0, Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleDefinitionId,
             HelpMessage = "Role definition id.")]
-        [ValidateNotNullOrEmpty]
+        [ValidateGuidNotEmpty]
         public Guid Id { get; set; }
 
         [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleDefinitionCustom,
diff --git a/src/ResourceManager/Resources/Commands.Resources/RoleDefinitions/RemoveAzureRoleDefinitionCommand.cs b/src/ResourceManager/Resources/Commands.Resources/RoleDefinitions/RemoveAzureRoleDefinitionCommand.cs
@@ -18,6 +18,7 @@
 using Microsoft.Azure.Commands.Resources.Models.Authorization;
 using ProjectResources = Microsoft.Azure.Commands.Resources.Properties.Resources;
 using System;
+using Microsoft.WindowsAzure.Commands.Common;
 
 namespace Microsoft.Azure.Commands.Resources
 {
@@ -29,7 +30,7 @@ public class RemoveAzureRoleDefinitionCommand : ResourcesBaseCmdlet
     {
         [Parameter(Position = 0, Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleDefinitionId,
             HelpMessage = "Role definition id")]
-        [ValidateNotNullOrEmpty]
+        [ValidateGuidNotEmpty]
         public Guid Id { get; set; }
 
         [Parameter(Position = 0, Mandatory = false, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleDefinitionName,
@@ -47,19 +48,22 @@ protected override void ProcessRecord()
         {
             PSRoleDefinition roleDefinition = null;
             Action action = null;
+            string confirmMessage = null;
 
             if(Id != Guid.Empty)
             {
                 action = (() => roleDefinition = PoliciesClient.RemoveRoleDefinition(Id, DefaultProfile.Context.Subscription.Id.ToString()));
+                confirmMessage = string.Format(ProjectResources.RemoveRoleDefinition, Id);
             }
             else
             {
                 action = (() => roleDefinition = PoliciesClient.RemoveRoleDefinition(Name, DefaultProfile.Context.Subscription.Id.ToString()));
+                confirmMessage = string.Format(ProjectResources.RemoveRoleDefinitionWithName, Name);
             }
 
             ConfirmAction(
                 Force.IsPresent,
-                string.Format(ProjectResources.RemoveRoleDefinition, Id),
+                confirmMessage,
                 ProjectResources.RemoveRoleDefinition,
                 Id.ToString(),
                 action);

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,11 @@ public class AuthorizationHelper`
`12`	`12`
`13`	`13`	`public static string GetRoleDefinitionFullyQualifiedId(string subscriptionId, string roleDefinitionGuid)`
`14`	`14`	`{`
	`15`	`+ if(string.IsNullOrEmpty(roleDefinitionGuid))`
	`16`	`+ {`
	`17`	`+ return null;`
	`18`	`+ }`
	`19`	`+`
`15`	`20`	`return string.Concat(string.Format(AuthorizationHelper.roleDefinitionIdPrefixFormat, subscriptionId), roleDefinitionGuid);`
`16`	`21`	`}`
`17`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@`
`18`	`18`	`using Microsoft.Azure.Commands.Resources.Models.Authorization;`
`19`	`19`	`using ProjectResources = Microsoft.Azure.Commands.Resources.Properties.Resources;`
`20`	`20`	`using System;`
	`21`	`+using Microsoft.WindowsAzure.Commands.Common;`
`21`	`22`
`22`	`23`	`namespace Microsoft.Azure.Commands.Resources`
`23`	`24`	`{`
`@@ -29,7 +30,7 @@ public class RemoveAzureRoleDefinitionCommand : ResourcesBaseCmdlet`
`29`	`30`	`{`
`30`	`31`	`[Parameter(Position = 0, Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleDefinitionId,`
`31`	`32`	`HelpMessage = "Role definition id")]`
`32`		`- [ValidateNotNullOrEmpty]`
	`33`	`+ [ValidateGuidNotEmpty]`
`33`	`34`	`public Guid Id { get; set; }`
`34`	`35`
`35`	`36`	`[Parameter(Position = 0, Mandatory = false, ValueFromPipelineByPropertyName = true, ParameterSetName = ParameterSet.RoleDefinitionName,`
`@@ -47,19 +48,22 @@ protected override void ProcessRecord()`
`47`	`48`	`{`
`48`	`49`	`PSRoleDefinition roleDefinition = null;`
`49`	`50`	`Action action = null;`
	`51`	`+ string confirmMessage = null;`
`50`	`52`
`51`	`53`	`if(Id != Guid.Empty)`
`52`	`54`	`{`
`53`	`55`	`action = (() => roleDefinition = PoliciesClient.RemoveRoleDefinition(Id, DefaultProfile.Context.Subscription.Id.ToString()));`
	`56`	`+ confirmMessage = string.Format(ProjectResources.RemoveRoleDefinition, Id);`
`54`	`57`	`}`
`55`	`58`	`else`
`56`	`59`	`{`
`57`	`60`	`action = (() => roleDefinition = PoliciesClient.RemoveRoleDefinition(Name, DefaultProfile.Context.Subscription.Id.ToString()));`
	`61`	`+ confirmMessage = string.Format(ProjectResources.RemoveRoleDefinitionWithName, Name);`
`58`	`62`	`}`
`59`	`63`
`60`	`64`	`ConfirmAction(`
`61`	`65`	`Force.IsPresent,`
`62`		`- string.Format(ProjectResources.RemoveRoleDefinition, Id),`
	`66`	`+ confirmMessage,`
`63`	`67`	`ProjectResources.RemoveRoleDefinition,`
`64`	`68`	`Id.ToString(),`
`65`	`69`	`action);`