-
Notifications
You must be signed in to change notification settings - Fork 235
Lozensky/enable managed identity #2650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 38 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
19ab825
Add logic to default to using managed identity if provided.
JoshLozensky 7936251
remove blank line
JoshLozensky 1dd20cc
Updated with caching and new design
JoshLozensky f4539b2
rearranging methods
JoshLozensky a0e9d90
made GetOrBuildManagedIdentityApplication async
JoshLozensky 72143cf
added unit test for application caching
JoshLozensky 887c340
finished unit test first draft
JoshLozensky e1e53b8
minor changes
JoshLozensky a9f5b6e
changed according to PR feedback
JoshLozensky f1bf949
Add logic to default to using managed identity if provided.
JoshLozensky 1d27aa5
remove blank line
JoshLozensky 3a23496
Updated with caching and new design
JoshLozensky 5e685bf
rearranging methods
JoshLozensky d21e94c
made GetOrBuildManagedIdentityApplication async
JoshLozensky 81f6429
added unit test for application caching
JoshLozensky d790365
finished unit test first draft
JoshLozensky 70aa77b
minor changes
JoshLozensky dcf4a8c
changed according to PR feedback
JoshLozensky 00fb49a
Rebase onto main
JoshLozensky 6be269a
Merge branch 'lozensky/EnableManagedIdentity' of https://github.com/A…
JoshLozensky 95c1bf6
added system-assigned managed identity e2e test
JoshLozensky 3e4cea5
Implemented PR feedback
JoshLozensky 846c476
changing test to use user-assigned managed identity
JoshLozensky 5c2e9b3
fixing tests
JoshLozensky 7c91ab6
Added configuration to e2e test
JoshLozensky 194ae37
moved build to after identity options config
JoshLozensky aa263b1
moving builder back
JoshLozensky f771e8d
fixed bug with TokenAcquisitionOptions/DefaultAuthorizationHeaderProv…
JoshLozensky dbb3f27
simplified e2e test
JoshLozensky 94013f2
added concurrency test and removed reflection
JoshLozensky 8a65307
Merge branch 'master' into lozensky/EnableManagedIdentity
JoshLozensky 73ec9a0
addressed PR comments and removed unnecessary code
JoshLozensky 423783c
Merge branch 'lozensky/EnableManagedIdentity' of https://github.com/A…
JoshLozensky a016213
removed extra space
JoshLozensky b82848a
addressed PR feedback
JoshLozensky b4ea34c
Merge branch 'lozensky/EnableManagedIdentity' of https://github.com/A…
JoshLozensky 2e38893
Merge branch 'master' into lozensky/EnableManagedIdentity
JoshLozensky 3d4e81d
making changes per PR comments
JoshLozensky d999e3d
removing test traces
JoshLozensky 3facf90
Merge branch 'master' into lozensky/EnableManagedIdentity
jmprieur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
114 changes: 114 additions & 0 deletions
114
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.ManagedIdentity.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
using System.Collections.Concurrent; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.Identity.Abstractions; | ||
using Microsoft.Identity.Client; | ||
using Microsoft.Identity.Client.AppConfig; | ||
using Microsoft.IdentityModel.Tokens; | ||
|
||
namespace Microsoft.Identity.Web | ||
{ | ||
/// <summary> | ||
/// Portion of the TokenAcquisition class that handles logic unique to managed identity. | ||
/// </summary> | ||
internal partial class TokenAcquisition | ||
{ | ||
private readonly ConcurrentDictionary<string, IManagedIdentityApplication> _managedIdentityApplicationsByClientId = new(); | ||
private readonly SemaphoreSlim _managedIdSemaphore = new(1, 1); | ||
private const string SystemAssignedManagedIdentityKey = "SYSTEM"; | ||
|
||
/// <summary> | ||
/// Gets a cached ManagedIdentityApplication object or builds a new one if not found. | ||
/// </summary> | ||
/// <param name="mergedOptions">The configuration options for the app.</param> | ||
/// <param name="managedIdentityOptions">The configuration specific to managed identity.</param> | ||
/// <returns>The application object used to request a token with managed identity.</returns> | ||
internal async Task<IManagedIdentityApplication> GetOrBuildManagedIdentityApplication( | ||
MergedOptions mergedOptions, | ||
ManagedIdentityOptions managedIdentityOptions) | ||
{ | ||
string key = GetCacheKeyForManagedId(managedIdentityOptions); | ||
|
||
// Check if the application is already built, if so return it without grabbing the lock | ||
if (_managedIdentityApplicationsByClientId.TryGetValue(key, out IManagedIdentityApplication? application)) | ||
{ | ||
return application; | ||
} | ||
|
||
// Lock the potential write of the dictionary to prevent multiple threads from creating the same application. | ||
await _managedIdSemaphore.WaitAsync(); | ||
try | ||
{ | ||
// Check if the application is already built (could happen between previous check and obtaining the key) | ||
if (_managedIdentityApplicationsByClientId.TryGetValue(key, out application)) | ||
{ | ||
return application; | ||
} | ||
|
||
// Set managedIdentityId to the correct value for either system or user assigned | ||
ManagedIdentityId managedIdentityId; | ||
if (key == SystemAssignedManagedIdentityKey) | ||
{ | ||
managedIdentityId = ManagedIdentityId.SystemAssigned; | ||
} | ||
else | ||
{ | ||
managedIdentityId = ManagedIdentityId.WithUserAssignedClientId(key); | ||
} | ||
|
||
// Build the application | ||
application = BuildManagedIdentityApplication( | ||
managedIdentityId, | ||
mergedOptions.ConfidentialClientApplicationOptions.EnablePiiLogging | ||
); | ||
|
||
// Add the application to the cache | ||
_managedIdentityApplicationsByClientId.TryAdd(key, application); | ||
} | ||
finally | ||
{ | ||
// Now that the dictionary is updated, release the semaphore | ||
_managedIdSemaphore.Release(); | ||
} | ||
return application; | ||
} | ||
|
||
/// <summary> | ||
/// Creates a managed identity client application. | ||
/// </summary> | ||
/// <param name="managedIdentityId">Indicates if system-assigned or user-assigned managed identity is used.</param> | ||
/// <param name="enablePiiLogging">Indicates if logging that may contain personally identifiable information is enabled.</param> | ||
/// <returns>A managed identity application.</returns> | ||
private IManagedIdentityApplication BuildManagedIdentityApplication(ManagedIdentityId managedIdentityId, bool enablePiiLogging) | ||
{ | ||
return ManagedIdentityApplicationBuilder | ||
.Create(managedIdentityId) | ||
.WithLogging( | ||
Log, | ||
ConvertMicrosoftExtensionsLogLevelToMsal(_logger), | ||
enablePiiLogging: enablePiiLogging) | ||
.Build(); | ||
} | ||
|
||
/// <summary> | ||
/// Gets the key value for the Managed Identity cache, the default key for system-assigned identity is used if there is | ||
/// no clientId for a user-assigned identity specified. The method is internal rather than private for testing purposes. | ||
/// </summary> | ||
/// <param name="managedIdOptions">Holds the clientId for managed identity if none is present.</param> | ||
/// <returns>A key value for the Managed Identity cache.</returns> | ||
internal static string GetCacheKeyForManagedId(ManagedIdentityOptions managedIdOptions) | ||
{ | ||
if (managedIdOptions.UserAssignedClientId.IsNullOrEmpty()) | ||
{ | ||
return SystemAssignedManagedIdentityKey; | ||
} | ||
else | ||
{ | ||
return managedIdOptions.UserAssignedClientId!; | ||
} | ||
} | ||
} | ||
} | ||
jennyf19 marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoshLozensky marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
tests/E2E Tests/TokenAcquirerTests/OnlyOnAzureDevopsFactAttribute.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
using System; | ||
using Xunit; | ||
|
||
namespace TokenAcquirerTests | ||
{ | ||
public sealed class OnlyOnAzureDevopsFactAttribute : FactAttribute | ||
{ | ||
public OnlyOnAzureDevopsFactAttribute() | ||
{ | ||
if (IgnoreOnAzureDevopsFactAttribute.IsRunningOnAzureDevOps()) | ||
{ | ||
return; | ||
} | ||
Skip = "Ignored when not on Azure DevOps"; | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file added
BIN
+2.82 MB
...onTests/PlaywrightTraces/B2CWebAppCallsWebApiLocally_TodoAppFunctionsCorrectly_net6.0.zip
JoshLozensky marked this conversation as resolved.
Show resolved
Hide resolved
|
Binary file not shown.
Binary file added
BIN
+2.71 MB
...onTests/PlaywrightTraces/B2CWebAppCallsWebApiLocally_TodoAppFunctionsCorrectly_net7.0.zip
Binary file not shown.
Binary file added
BIN
+2.4 MB
...Tests/PlaywrightTraces/B2CWebAppCallsWebApiLocally_TodoAppFunctionsCorrectly_net7k33p.zip
Binary file not shown.
Binary file added
BIN
+2.45 KB
tests/IntegrationTests/PlaywrightTraces/TestingWebAppLocally_ValidEmailPassword_net6.0.zip
Binary file not shown.
Binary file added
BIN
+6.56 KB
tests/IntegrationTests/PlaywrightTraces/TestingWebAppLocally_ValidEmailPassword_net7.0.zip
Binary file not shown.
Binary file added
BIN
+2.66 MB
...sts/PlaywrightTraces/WebAppCallsApiCallsGraphLocally_TodoAppFunctionsCorrectly_net6.0.zip
Binary file not shown.
Binary file added
BIN
+2.37 MB
...sts/PlaywrightTraces/WebAppCallsApiCallsGraphLocally_TodoAppFunctionsCorrectly_net7.0.zip
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.