Skip to content

Commit e86b3e9

Browse files
RandalliLamaRandalliLama
committed
Remove email addresses from UPN query.
Before this change, when a UPN argument is passed to one of the vault policy cmdlets the query for object id would include email addresses as well. After this change UPN will mean UPN and there will be a seperate email argument if the user wants to look up the object id by email address.
1 parent d4693a2 commit e86b3e9

File tree

10 files changed

+331
-141
lines changed

10 files changed

+331
-141
lines changed

‎src/ResourceManager/KeyVault/Commands.KeyVault.Test/Scripts/ControlPlane/KeyVaultManagementTests.ps1

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,25 @@ function Test-SetRemoveAccessPolicyByUPN
440440
Assert-AreEqual 0 $vault.AccessPolicies.Count
441441
}
442442

443+
function Test-SetRemoveAccessPolicyByEmail
444+
{
445+
Param($existingVaultName, $rgName, $email, $upn)
446+
447+
$PermToKeys = @("encrypt", "decrypt", "unwrapKey", "wrapKey", "verify", "sign", "get", "list", "update", "create", "import", "delete", "backup", "restore")
448+
$PermToSecrets = @("get", "list", "set", "delete")
449+
$PermToCertificates = @("get", "list", "create", "delete")
450+
451+
$vault = Set-AzureRmKeyVaultAccessPolicy -VaultName $existingVaultName -ResourceGroupName $rgName -Mail $email -PermissionsToKeys $PermToKeys -PermissionsToSecrets $PermToSecrets -PermissionsToCertificates $PermToCertificates -PassThru
452+
453+
CheckVaultAccessPolicy $vault $PermToKeys $PermToSecrets $PermToCertificates
454+
if (-not $global:noADCmdLetMode) {
455+
Assert-AreEqual $vault.AccessPolicies[0].ObjectId (Get-AzureRmADUser -Mail $upn).Id
456+
}
457+
458+
$vault = Remove-AzureRmKeyVaultAccessPolicy -VaultName $existingVaultName -ResourceGroupName $rgName -Mail $email -PassThru
459+
Assert-AreEqual 0 $vault.AccessPolicies.Count
460+
}
461+
443462
function Test-SetRemoveAccessPolicyBySPN
444463
{
445464
Param($existingVaultName, $rgName, $spn)

‎src/ResourceManager/KeyVault/Commands.KeyVault.Test/Scripts/RunKeyVaultTests.ps1

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,12 @@ function Run-AllControlPlaneTests
211211

212212
# Set-AzureRmKeyVaultAccessPolicy & Remove-AzureRmKeyVaultAccessPolicy tests.
213213
Run-TestProtected { Run-VaultTest { Test_SetRemoveAccessPolicyByUPN } "Test_SetRemoveAccessPolicyByUPN" } "Test_SetRemoveAccessPolicyByUPN"
214+
215+
<#
216+
This test is disabled because it requires a user with an email address that matches their UPN.
217+
Run-TestProtected { Run-VaultTest { Test_SetRemoveAccessPolicyByEmail } "Test_SetRemoveAccessPolicyByEmail" } "Test_SetRemoveAccessPolicyByEmail"
218+
#>
219+
214220
Run-TestProtected { Run-VaultTest { Test_SetRemoveAccessPolicyBySPN } "Test_SetRemoveAccessPolicyBySPN" } "Test_SetRemoveAccessPolicyBySPN"
215221
Run-TestProtected { Run-VaultTest { Test_SetRemoveAccessPolicyByObjectId } "Test_SetRemoveAccessPolicyByObjectId" } "Test_SetRemoveAccessPolicyByObjectId"
216222
Run-TestProtected { Run-VaultTest { Test_SetRemoveAccessPolicyByBypassObjectIdValidation } "Test_SetRemoveAccessPolicyByBypassObjectIdValidation" } "Test_SetRemoveAccessPolicyByBypassObjectIdValidation"

‎src/ResourceManager/KeyVault/Commands.KeyVault.Test/Scripts/VaultManagementTests.ps1

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ function Test_SetRemoveAccessPolicyByUPN
126126
Test-SetRemoveAccessPolicyByUPN $global:testVault $global:resourceGroupName $user
127127
}
128128

129+
function Test_SetRemoveAccessPolicyByEmail
130+
{
131+
# ASSUMPTION: The logged in users UPN is the same as their email address.
132+
$user = (Get-AzureRmContext).Account.Id
133+
Reset-PreCreatedVault
134+
Test-SetRemoveAccessPolicyByEmail $global:testVault $global:resourceGroupName $user $user
135+
}
136+
129137
function Test_SetRemoveAccessPolicyBySPN
130138
{
131139
Reset-PreCreatedVault
@@ -330,7 +338,7 @@ function Initialize-TemporaryState
330338
{
331339
Write-Host "Skipping resource group creation since the resource group $global:resourceGroupName is already provided."
332340
}
333-
341+
334342
if ($global:testVault -ne "" -and $global:testVault -ne $null)
335343
{
336344
Write-Host "Skipping vault creation since the vault $global:testVault is already provided."
@@ -339,7 +347,7 @@ function Initialize-TemporaryState
339347

340348
# Create a vault using ARM.
341349
$vaultName = Get-VaultName $suffix
342-
$tenantId = (Get-AzureRmContext).Tenant.TenantId
350+
$tenantId = (Get-AzureRmContext).Tenant.Id
343351
$sku = "premium"
344352
if ($global:standardVaultOnly)
345353
{

‎src/ResourceManager/KeyVault/Commands.KeyVault/Commands/RemoveAzureKeyVaultAccessPolicy.cs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public class RemoveAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
3131
private const string ByObjectId = "ByObjectId";
3232
private const string ByServicePrincipalName = "ByServicePrincipalName";
3333
private const string ByUserPrincipalName = "ByUserPrincipalName";
34+
private const string ByEmail = "ByEmail";
3435
private const string ForVault = "ForVault";
3536

3637
#endregion
@@ -89,6 +90,16 @@ public class RemoveAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
8990
[ValidateNotNullOrEmpty()]
9091
public string ObjectId { get; set; }
9192

93+
/// <summary>
94+
/// Email address
95+
/// </summary>
96+
[Parameter(Mandatory = true,
97+
ParameterSetName = ByEmail,
98+
ValueFromPipelineByPropertyName = true,
99+
HelpMessage = "Specifies the email address of the user in Azure Active Directory for which to grant permissions.")]
100+
[ValidateNotNullOrEmpty()]
101+
public string Mail { get; set; }
102+
92103
/// <summary>
93104
/// Id of the application to which a user delegate to
94105
/// </summary>
@@ -160,11 +171,14 @@ public override void ExecuteCmdlet()
160171

161172
// Update vault policies
162173
var updatedPolicies = existingVault.AccessPolicies;
163-
if (!string.IsNullOrEmpty(UserPrincipalName) || !string.IsNullOrEmpty(ServicePrincipalName) || !string.IsNullOrWhiteSpace(this.ObjectId))
174+
if (!string.IsNullOrEmpty(UserPrincipalName)
175+
|| !string.IsNullOrEmpty(ServicePrincipalName)
176+
|| !string.IsNullOrWhiteSpace(this.ObjectId)
177+
|| !string.IsNullOrWhiteSpace(this.Mail))
164178
{
165179
if (string.IsNullOrWhiteSpace(this.ObjectId))
166180
{
167-
ObjectId = GetObjectId(this.ObjectId, this.UserPrincipalName, this.ServicePrincipalName);
181+
ObjectId = GetObjectId(this.ObjectId, this.UserPrincipalName, this.Mail, this.ServicePrincipalName);
168182
}
169183
updatedPolicies = existingVault.AccessPolicies.Where(ap => !ShallBeRemoved(ap, ObjectId, this.ApplicationId)).ToArray();
170184
}

‎src/ResourceManager/KeyVault/Commands.KeyVault/Commands/SetAzureKeyVaultAccessPolicy.cs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public class SetAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
9191
private const string ByObjectId = "ByObjectId";
9292
private const string ByServicePrincipalName = "ByServicePrincipalName";
9393
private const string ByUserPrincipalName = "ByUserPrincipalName";
94+
private const string ByEmail = "ByEmail";
9495
private const string ForVault = "ForVault";
9596

9697
#endregion
@@ -149,6 +150,16 @@ public class SetAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
149150
[ValidateNotNullOrEmpty()]
150151
public string ObjectId { get; set; }
151152

153+
/// <summary>
154+
/// Email address
155+
/// </summary>
156+
[Parameter(Mandatory = true,
157+
ParameterSetName = ByEmail,
158+
ValueFromPipelineByPropertyName = true,
159+
HelpMessage = "Specifies the email address of the user in Azure Active Directory for which to grant permissions.")]
160+
[ValidateNotNullOrEmpty()]
161+
public string Mail { get; set; }
162+
152163
/// <summary>
153164
/// Id of the application to which a user delegate to
154165
/// </summary>
@@ -173,6 +184,10 @@ public class SetAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
173184
ParameterSetName = ByUserPrincipalName,
174185
ValueFromPipelineByPropertyName = true,
175186
HelpMessage = "Specifies key operation permissions to grant to a user or service principal.")]
187+
[Parameter(Mandatory = false,
188+
ParameterSetName = ByEmail,
189+
ValueFromPipelineByPropertyName = true,
190+
HelpMessage = "Specifies key operation permissions to grant to a user or service principal.")]
176191
[ValidateSet("decrypt", "encrypt", "unwrapKey", "wrapKey", "verify", "sign", "get", "list", "update", "create", "import", "delete", "backup", "restore", "recover", "purge", "all")]
177192
public string[] PermissionsToKeys { get; set; }
178193

@@ -191,6 +206,10 @@ public class SetAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
191206
ParameterSetName = ByUserPrincipalName,
192207
ValueFromPipelineByPropertyName = true,
193208
HelpMessage = "Specifies secret operation permissions to grant to a user or service principal.")]
209+
[Parameter(Mandatory = false,
210+
ParameterSetName = ByEmail,
211+
ValueFromPipelineByPropertyName = true,
212+
HelpMessage = "Specifies key operation permissions to grant to a user or service principal.")]
194213
[ValidateSet("get", "list", "set", "delete", "backup", "restore", "recover", "purge", "all")]
195214
public string[] PermissionsToSecrets { get; set; }
196215

@@ -209,6 +228,10 @@ public class SetAzureKeyVaultAccessPolicy : KeyVaultManagementCmdletBase
209228
ParameterSetName = ByUserPrincipalName,
210229
ValueFromPipelineByPropertyName = true,
211230
HelpMessage = "Specifies certificate operation permissions to grant to a user or service principal.")]
231+
[Parameter(Mandatory = false,
232+
ParameterSetName = ByEmail,
233+
ValueFromPipelineByPropertyName = true,
234+
HelpMessage = "Specifies key operation permissions to grant to a user or service principal.")]
212235
[ValidateSet("get", "list", "delete", "create", "import", "update", "managecontacts", "getissuers", "listissuers", "setissuers", "deleteissuers", "manageissuers", "all")]
213236
public string[] PermissionsToCertificates { get; set; }
214237

@@ -293,12 +316,15 @@ public override void ExecuteCmdlet()
293316

294317
// Update vault policies
295318
PSKeyVaultModels.PSVaultAccessPolicy[] updatedListOfAccessPolicies = vault.AccessPolicies;
296-
if (!string.IsNullOrEmpty(UserPrincipalName) || !string.IsNullOrEmpty(ServicePrincipalName) || !string.IsNullOrWhiteSpace(this.ObjectId))
319+
if (!string.IsNullOrEmpty(UserPrincipalName)
320+
|| !string.IsNullOrEmpty(ServicePrincipalName)
321+
|| !string.IsNullOrWhiteSpace(this.ObjectId)
322+
|| !string.IsNullOrWhiteSpace(this.Mail))
297323
{
298324
var objId = this.ObjectId;
299325
if (!this.BypassObjectIdValidation.IsPresent)
300326
{
301-
objId = GetObjectId(this.ObjectId, this.UserPrincipalName, this.ServicePrincipalName);
327+
objId = GetObjectId(this.ObjectId, this.UserPrincipalName, this.Mail, this.ServicePrincipalName);
302328
}
303329

304330
if (ApplicationId.HasValue && ApplicationId.Value == Guid.Empty)

‎src/ResourceManager/KeyVault/Commands.KeyVault/Models/KeyVaultManagementCmdletBase.cs

Lines changed: 95 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -183,47 +183,118 @@ protected string GetCurrentUsersObjectId()
183183
if (DefaultContext.Account == null)
184184
throw new InvalidOperationException(PSKeyVaultProperties.Resources.NoDefaultUserAccount);
185185

186+
string objectId = null;
187+
if (DefaultContext.Account.Type == AzureAccount.AccountType.User)
188+
{
189+
var userFetcher = ActiveDirectoryClient.Me.ToUser();
190+
var user = userFetcher.ExecuteAsync().Result;
191+
objectId = user.ObjectId;
192+
}
186193

187-
return GetObjectId(
188-
upn: DefaultContext.Account.Id,
189-
objectId: string.Empty,
190-
spn: null
191-
);
194+
return objectId;
192195
}
193196

194-
protected string GetObjectId(string objectId, string upn, string spn)
197+
private void ThrowIfMultipleObjectIds<AADType>(IEnumerable<AADType> principal, string objectFilter)
195198
{
196-
var objId = string.Empty;
197-
var objectFilter = objectId ?? string.Empty;
199+
// If there is a second element then there are too many.
200+
if (principal.ElementAtOrDefault(1) != null)
201+
{
202+
throw new ArgumentException(string.Format(PSKeyVaultProperties.Resources.ADObjectAmbiguous, objectFilter,
203+
(_dataServiceCredential != null) ? _dataServiceCredential.TenantId : string.Empty));
204+
}
205+
}
198206

207+
private string GetObjectIdByUpn(string upn)
208+
{
209+
string objId = null;
199210
if (!string.IsNullOrWhiteSpace(upn))
200211
{
201-
objectFilter = upn;
202-
var user = ActiveDirectoryClient.Users.Where(FilterByUpn(upn)).ExecuteAsync().GetAwaiter().GetResult().CurrentPage.FirstOrDefault();
212+
var user = ActiveDirectoryClient.Users.Where(u => u.UserPrincipalName.Equals(upn, StringComparison.OrdinalIgnoreCase))
213+
.ExecuteAsync().GetAwaiter().GetResult().CurrentPage.SingleOrDefault();
203214
if (user != null)
204215
objId = user.ObjectId;
205216
}
206-
else if (!string.IsNullOrWhiteSpace(spn))
217+
return objId;
218+
}
219+
220+
private string GetObjectIdBySpn(string spn)
221+
{
222+
string objId = null;
223+
if (!string.IsNullOrWhiteSpace(spn))
207224
{
208-
objectFilter = spn;
209225
var servicePrincipal = ActiveDirectoryClient.ServicePrincipals.Where(s =>
210226
s.ServicePrincipalNames.Any(n => n.Equals(spn, StringComparison.OrdinalIgnoreCase)))
211-
.ExecuteAsync().GetAwaiter().GetResult().CurrentPage.FirstOrDefault();
227+
.ExecuteAsync().GetAwaiter().GetResult().CurrentPage.SingleOrDefault();
212228
if (servicePrincipal != null)
213229
objId = servicePrincipal.ObjectId;
214230
}
215-
else if (!string.IsNullOrWhiteSpace(objectId))
231+
return objId;
232+
}
233+
234+
private string GetObjectIdByEmail(string email)
235+
{
236+
string objId = null;
237+
// In ADFS, Graph cannot handle this particular combination of filters.
238+
if (!DefaultProfile.DefaultContext.Environment.OnPremise && !string.IsNullOrWhiteSpace(email))
216239
{
217-
var objectCollection = ActiveDirectoryClient.GetObjectsByObjectIdsAsync(new[] { objectId }, new string[] { }).GetAwaiter().GetResult();
218-
if (objectCollection.Any())
219-
objId = objectId;
240+
var users = ActiveDirectoryClient.Users.Where(FilterByEmail(email)).ExecuteAsync().GetAwaiter().GetResult().CurrentPage;
241+
if (users != null)
242+
{
243+
ThrowIfMultipleObjectIds(users, email);
244+
var user = users.FirstOrDefault();
245+
objId = user?.ObjectId;
246+
}
220247
}
248+
return objId;
249+
}
221250

251+
private bool ValidateObjectId(string objId)
252+
{
253+
bool isValid = false;
222254
if (!string.IsNullOrWhiteSpace(objId))
223-
return objId;
255+
{
256+
var objectCollection = ActiveDirectoryClient.GetObjectsByObjectIdsAsync(new[] { objId }, new string[] { }).GetAwaiter().GetResult();
257+
if (objectCollection.Any())
258+
{
259+
isValid = true;
260+
}
261+
}
262+
return isValid;
263+
}
224264

225-
throw new ArgumentException(string.Format(PSKeyVaultProperties.Resources.ADObjectNotFound, objectFilter,
226-
(_dataServiceCredential != null) ? _dataServiceCredential.TenantId : string.Empty));
265+
protected string GetObjectId(string objectId, string upn, string email, string spn)
266+
{
267+
string objId = null;
268+
var objectFilter = objectId ?? string.Empty;
269+
270+
if (!string.IsNullOrEmpty(objectId))
271+
{
272+
objId = ValidateObjectId(objectId) ? objectId : null;
273+
}
274+
else if (!string.IsNullOrEmpty(upn))
275+
{
276+
objId = GetObjectIdByUpn(upn);
277+
}
278+
else if (!string.IsNullOrEmpty(email))
279+
{
280+
objId = GetObjectIdByEmail(email);
281+
}
282+
else if (!string.IsNullOrEmpty(spn))
283+
{
284+
objId = GetObjectIdBySpn(spn);
285+
}
286+
287+
if (string.IsNullOrWhiteSpace(objId))
288+
throw new ArgumentException(string.Format(PSKeyVaultProperties.Resources.ADObjectNotFound, objectFilter,
289+
(_dataServiceCredential != null) ? _dataServiceCredential.TenantId : string.Empty));
290+
291+
return objId;
292+
}
293+
294+
private bool IsValidGUid(string stringGuid)
295+
{
296+
Guid parsedGuid;
297+
return Guid.TryParse(stringGuid, out parsedGuid);
227298
}
228299

229300
/// <summary>
@@ -246,21 +317,13 @@ protected bool IsValidObjectIdSyntax(string objectId)
246317
}
247318

248319
// In AAD, object IDs must be parsable as Guids.
249-
Guid dummyValue;
250-
return Guid.TryParse(objectId, out dummyValue);
320+
return IsValidGUid(objectId);
251321
}
252322

253-
private Expression<Func<IUser, bool>> FilterByUpn(string upn)
323+
private Expression<Func<IUser, bool>> FilterByEmail(string email)
254324
{
255-
// In ADFS, Graph cannot handle this particular combination of filters.
256-
if (!DefaultProfile.DefaultContext.Environment.OnPremise)
257-
{
258-
return u => u.UserPrincipalName.Equals(upn, StringComparison.OrdinalIgnoreCase) ||
259-
u.Mail.Equals(upn, StringComparison.OrdinalIgnoreCase) ||
260-
u.OtherMails.Any(m => m.Equals(upn, StringComparison.OrdinalIgnoreCase));
261-
}
262-
263-
return u => u.UserPrincipalName.Equals(upn, StringComparison.OrdinalIgnoreCase);
325+
return u => u.Mail.Equals(email, StringComparison.OrdinalIgnoreCase) ||
326+
u.OtherMails.Any(m => m.Equals(email, StringComparison.OrdinalIgnoreCase));
264327
}
265328

266329
protected readonly string[] DefaultPermissionsToKeys =

0 commit comments

Comments
 (0)