Move to Azure Identity #1201
Move to Azure Identity #1201
Conversation
peombwa
requested review from
ddyett,
georgend,
shemogumbe and
timayabi2020
as code owners
|
do you recall why we added the accesstoken on invoke-mggraphrequest to begin with? |
| { | ||
| InteractiveAuthenticationProvider, | ||
| DeviceCodeProvider, | ||
| DeviceCodeProviderFallBack, |
There was a problem hiding this comment.
what was this one covering?
There was a problem hiding this comment.
DeviceCodeProviderFallBack was used to detect when to fall back to DeviceCode when MSAL can't open browser in environments such as Linux servers. I've changed the implementation in https://github.com/microsoftgraph/msgraph-sdk-powershell/blob/enhancements/MSIdentity/src/Authentication/Authentication.Core/Utilities/AuthenticationHelpers.cs#L157-L162 to catch the right MSAL exception and retry with device code.
| <PackageReference Include="System.IdentityModel.Tokens.Jwt" Version="5.6.0" /> | ||
| <PackageReference Include="Azure.Core" Version="1.22.0" /> | ||
| <PackageReference Include="Azure.Identity" Version="1.5.0" /> | ||
| <PackageReference Include="Microsoft.Graph.Core" Version="2.0.8" /> |
There was a problem hiding this comment.
by the time this goes out wouldn't kiota likely be the release? Is the plan to convert this later and align with CLI?
There was a problem hiding this comment.
That's the plan. Swapping the current MS Graph .NET core for Kiota based core should be trivial. I have a work item to integrate dependabot for verion updates - #1002.
|
can you say more about why the token caches for mac/linux are no longer necessary? |
| public class InMemoryTokenCacheOptions : UnsafeTokenCacheOptions | ||
| { | ||
| private readonly ReaderWriterLockSlim _sessionLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion); | ||
| internal ReadOnlyMemory<byte> TokenCache { get; private set; } |
There was a problem hiding this comment.
seems odd that the options class is holding the reference to the cache and operating on the object. This looks more like cache manager than options class
There was a problem hiding this comment.
Good point! It is a cache manager. It's Azure Identity that named the base class UnsafeTokenCacheOptions. I opted to use the options suffix to be in sync with Azure Identity naming.
It was because we didn't expose |
We no longer need them because Azure.Identity securely caches tokens on Windows, MacOS, and Linux (when libsecret-1.so.0 is available). We will fall back to in-memory cache when Azure.Identity can't persist tokens to the underlying OS mechanism (MsalCachePersistenceException is thrown). |
|
We should verify that successfully sign in and token caching is possible on the following platforms (I'll check them once verified):
|
We had added it for BYOT scenarios. For example authenticating against Managed Identity in a Virtual Machine, getting a token and (re)-using it with the SDK. |
This PR closes #1144 by:
The following improvements have been made to the Authentication module as part of this transition:
-AccessTokenfromInvoke-MgGraphRequest. The goal is to have one authentication/authorization point,Connect-MgGraph.-ForceRefreshfromConnect-MgGraph.-ForeceRefreshis not exposed by Azure Identity as the library will fetch new tokens automatically when new scopes are added.In-Memorytoken cache for process context scope.