From 41bab3e9e06481422b8a537e87a8ba444771e160 Mon Sep 17 00:00:00 2001 From: Yeming Liu Date: Mon, 24 Aug 2020 11:55:43 +0800 Subject: [PATCH] enhance error handling (#12732) Co-authored-by: Yeming Liu --- src/KeyVault/KeyVault/ChangeLog.md | 1 + .../Models/KeyVaultManagementCmdletBase.cs | 52 ++++++++----------- .../KeyVault/Properties/Resources.Designer.cs | 9 ++++ .../KeyVault/Properties/Resources.resx | 3 ++ 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/KeyVault/KeyVault/ChangeLog.md b/src/KeyVault/KeyVault/ChangeLog.md index 5464f0bf9a68..e824ae3398ca 100644 --- 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 index 47a400674ff6..de601c004dc2 100644 --- 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 FilterByTag(List protected PSKeyVault FilterByTag(PSKeyVault keyVault, Hashtable tag) { - return (PSKeyVault) FilterByTag(new List { keyVault }, tag).FirstOrDefault(); + return (PSKeyVault)FilterByTag(new List { keyVault }, tag).FirstOrDefault(); } protected List 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> 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 { objId }); -#else - var objectCollection = ActiveDirectoryClient.GetObjectsByObjectIdsAsync(new[] { objId }, new string[] { }).GetAwaiter().GetResult(); -#endif - return objectCollection.Any(); + try + { + var objectCollection = ActiveDirectoryClient.GetObjectsByObjectId(new List { 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 index 5465a4159c9e..3b8bb54bd8f8 100644 --- a/src/KeyVault/KeyVault/Properties/Resources.Designer.cs +++ b/src/KeyVault/KeyVault/Properties/Resources.Designer.cs @@ -114,6 +114,15 @@ internal static string AddNetworkRule { } } + /// + /// Looks up a localized string similar to Please make sure you have sufficient permissions in AD Graph to get and list graph objects for validation to work. Otherwise skip witch `-BypassObjectIdValidation`.. + /// + internal static string ADGraphPermissionWarning { + get { + return ResourceManager.GetString("ADGraphPermissionWarning", resourceCulture); + } + } + /// /// Looks up a localized string similar to The Email argument specified, '{1}', matches multiple objects in the Azure Active Directory tenant '{2}'. Please use -UserPrincipalName to narrow down the filter to a single object. The TenantID displayed by the cmdlet 'Get-AzContext' is the current subscription's Azure Active Directory.. /// diff --git a/src/KeyVault/KeyVault/Properties/Resources.resx b/src/KeyVault/KeyVault/Properties/Resources.resx index 7b4377973f87..681184931138 100644 --- 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 The "import" operation is exclusive, it cannot be combined with any other value(s). + + Please make sure you have sufficient permissions in AD Graph to get and list graph objects for validation to work. Otherwise skip witch `-BypassObjectIdValidation`. + \ No newline at end of file