enhance error handling (Azure#12732)

Co-authored-by: Yeming Liu <yeliu@microsoft.com>
abhi7860 · Aug 24, 2020 · 41bab3e · 41bab3e
1 parent 172a22e
commit 41bab3e
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 29 deletions.
diff --git a/src/KeyVault/KeyVault/ChangeLog.md b/src/KeyVault/KeyVault/ChangeLog.md
@@ -18,6 +18,7 @@
         - Additional information about change #1
 -->
 ## Upcoming Release
+* Enhanced error handling in `Set-AzKeyVaultAccessPolicy` [#4007]
 
 ## Version 2.1.0
 * Added warning messages for planning to disable soft delete

diff --git a/src/KeyVault/KeyVault/Models/KeyVaultManagementCmdletBase.cs b/src/KeyVault/KeyVault/Models/KeyVaultManagementCmdletBase.cs
@@ -12,36 +12,27 @@
 // limitations under the License.
 // ----------------------------------------------------------------------------------
 
-// TODO: Remove IfDef
-#if NETSTANDARD
-using Microsoft.Azure.Graph.RBAC.Version1_6.ActiveDirectory;
-#else
-using Microsoft.Azure.ActiveDirectory.GraphClient;
-#endif
-using System;
-using System.Collections;
-using System.Collections.Generic;
-using System.Linq;
-using System.Linq.Expressions;
-using System.Threading.Tasks;
 using Microsoft.Azure.Commands.Common.Authentication;
 using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
 using Microsoft.Azure.Commands.KeyVault.Models;
+using Microsoft.Azure.Commands.KeyVault.Properties;
 using Microsoft.Azure.Commands.ResourceManager.Common;
+using Microsoft.Azure.Commands.ResourceManager.Common.Paging;
 using Microsoft.Azure.Commands.ResourceManager.Common.Tags;
+using Microsoft.Azure.Graph.RBAC.Version1_6.ActiveDirectory;
 using Microsoft.Azure.Management.Internal.Resources;
 using Microsoft.Azure.Management.Internal.Resources.Models;
 using Microsoft.Azure.Management.Internal.Resources.Utilities;
 using Microsoft.Azure.Management.Internal.Resources.Utilities.Models;
-using PSKeyVaultModels = Microsoft.Azure.Commands.KeyVault.Models;
-using PSKeyVaultProperties = Microsoft.Azure.Commands.KeyVault.Properties;
-using Microsoft.Rest.Azure;
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using CertPerms = Microsoft.Azure.Management.KeyVault.Models.CertificatePermissions;
 using KeyPerms = Microsoft.Azure.Management.KeyVault.Models.KeyPermissions;
+using PSKeyVaultProperties = Microsoft.Azure.Commands.KeyVault.Properties;
 using SecretPerms = Microsoft.Azure.Management.KeyVault.Models.SecretPermissions;
-using CertPerms = Microsoft.Azure.Management.KeyVault.Models.CertificatePermissions;
 using StoragePerms = Microsoft.Azure.Management.KeyVault.Models.StoragePermissions;
-using Microsoft.Azure.Management.KeyVault.Models;
-using Microsoft.Azure.Commands.ResourceManager.Common.Paging;
 
 namespace Microsoft.Azure.Commands.KeyVault
 {
@@ -128,7 +119,7 @@ protected List<PSKeyVaultIdentityItem> FilterByTag(List<PSKeyVaultIdentityItem>
 
         protected PSKeyVault FilterByTag(PSKeyVault keyVault, Hashtable tag)
         {
-            return (PSKeyVault) FilterByTag(new List<PSKeyVaultIdentityItem> { keyVault }, tag).FirstOrDefault();
+            return (PSKeyVault)FilterByTag(new List<PSKeyVaultIdentityItem> { keyVault }, tag).FirstOrDefault();
         }
 
         protected List<PSKeyVaultIdentityItem> ListVaults(string resourceGroupName, Hashtable tag)
@@ -235,7 +226,7 @@ protected string GetCurrentUsersObjectId()
             {
 // TODO: Remove IfDef
 #if NETSTANDARD
-                objectId = ActiveDirectoryClient.GetObjectId(new ADObjectFilterOptions {UPN = DefaultContext.Account.Id}).ToString();
+                objectId = ActiveDirectoryClient.GetObjectId(new ADObjectFilterOptions { UPN = DefaultContext.Account.Id }).ToString();
 #else
                 var userFetcher = ActiveDirectoryClient.Me.ToUser();
                 var user = userFetcher.ExecuteAsync().Result;
@@ -335,13 +326,16 @@ private Expression<Func<IUser, bool>> FilterByEmail(string email)
         private bool ValidateObjectId(string objId)
         {
             if (string.IsNullOrWhiteSpace(objId)) return false;
-// TODO: Remove IfDef
-#if NETSTANDARD
-            var objectCollection = ActiveDirectoryClient.GetObjectsByObjectId(new List<string> { objId });
-#else
-            var objectCollection = ActiveDirectoryClient.GetObjectsByObjectIdsAsync(new[] { objId }, new string[] { }).GetAwaiter().GetResult();
-#endif
-            return objectCollection.Any();
+            try
+            {
+                var objectCollection = ActiveDirectoryClient.GetObjectsByObjectId(new List<string> { objId });
+                return objectCollection.Any();
+            }
+            catch (Exception ex)
+            {
+                WriteWarning(Resources.ADGraphPermissionWarning);
+                throw ex;
+            }
         }
 
         protected string GetObjectId(string objectId, string upn, string email, string spn)
@@ -410,7 +404,7 @@ protected bool IsValidObjectIdSyntax(string objectId)
             KeyPerms.Recover
         };
 
-        protected readonly string[] DefaultPermissionsToSecrets = 
+        protected readonly string[] DefaultPermissionsToSecrets =
         {
             SecretPerms.Get,
             SecretPerms.List,
@@ -440,7 +434,7 @@ protected bool IsValidObjectIdSyntax(string objectId)
             CertPerms.Restore
         };
 
-        protected readonly string[] DefaultPermissionsToStorage = 
+        protected readonly string[] DefaultPermissionsToStorage =
         {
             StoragePerms.Delete,
             StoragePerms.Deletesas,

diff --git a/src/KeyVault/KeyVault/Properties/Resources.Designer.cs b/src/KeyVault/KeyVault/Properties/Resources.Designer.cs
diff --git a/src/KeyVault/KeyVault/Properties/Resources.resx b/src/KeyVault/KeyVault/Properties/Resources.resx
@@ -498,4 +498,7 @@ You can find the object ID using Azure Active Directory Module for Windows Power
   <data name="KeyOpsImportIsExclusive" xml:space="preserve">
     <value>The "import" operation is exclusive, it cannot be combined with any other value(s).</value>
   </data>
+  <data name="ADGraphPermissionWarning" xml:space="preserve">
+    <value>Please make sure you have sufficient permissions in AD Graph to get and list graph objects for validation to work. Otherwise skip witch `-BypassObjectIdValidation`.</value>
+  </data>
 </root>