msgraph-sdk-powershell icon indicating copy to clipboard operation
msgraph-sdk-powershell copied to clipboard

WAM Integration

Open FehintolaObafemi opened this issue 3 years ago • 10 comments

Fixes #1133

Changes proposed in this pull request

  • Work item for #1133
  • Changes to AuthenticationHelpers.cs file to include WAM support as part of #1118
  • Updating the dependencies to the latest versions

Other links

  • https://github.com/microsoftgraph/msgraph-sdk-powershell/issues/1133

FehintolaObafemi avatar Jun 24 '22 15:06 FehintolaObafemi

Since we are using MSAL 4.44.X in this implementation, you may want to wait for Azure.Identity.BrokeredAuthentication to provide us with an implementation that uses the new .WithBrokerPreview(). See https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/wam#new-wam-preview-in-msal-444 and Azure/Identity's implementation.

Otherwise, we you'll also need to ensure the prerequisites in https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#availability are met for it to work across multiple targets. You can find PowerShell's .NET runtime version using [System.Runtime.InteropServices.RuntimeInformation]::FrameworkDescription. For example, PowerShell 5.1 runs on .NET Framework 4.8.X while PowerShell 7.2.5. runs on .NET 6.0.6.

cc\ @FehintolaObafemi

peombwa avatar Jun 24 '22 22:06 peombwa

Fixes #1133

Changes proposed in this pull request

Other links

One question @peombwa -- do we need to make this behaavior opt-in via Connect-MgGraph? I spoke to one of the implementers of WAM in Windows and I'm not clear on which Windows versions support WAM and also if there are any corner cases we'd like to provide a way to avoid.

adamedx avatar Jun 28 '22 15:06 adamedx

One question @peombwa -- do we need to make this behaavior opt-in via Connect-MgGraph? I spoke to one of the implementers of WAM in Windows and I'm not clear on which Windows versions support WAM and also if there are any corner cases we'd like to provide a way to avoid.

@adamedx, good point! We should make this an opt-in feature since WAM has couple of limitations that customers may have a hard time with:

New accounts are automatically remembered by Windows. Work and School have the option of joining the organization's directory or opting out completely, in which case the account won't appear under "Email & Accounts". Microsoft accounts are automatically added to Windows.

See https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#other-considerations.

MSAL's implementation is OS aware, and should fallback to a browser when WAM is not supported. See https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#wam-limitations.

peombwa avatar Jun 29 '22 19:06 peombwa

Since we are using MSAL 4.44.X in this implementation, you may want to wait for Azure.Identity.BrokeredAuthentication to provide us with an implementation that uses the new .WithBrokerPreview(). See https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/wam#new-wam-preview-in-msal-444 and Azure/Identity's implementation.

Otherwise, we you'll also need to ensure the prerequisites in https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#availability are met for it to work across multiple targets. You can find PowerShell's .NET runtime version using [System.Runtime.InteropServices.RuntimeInformation]::FrameworkDescription. For example, PowerShell 5.1 runs on .NET Framework 4.8.X while PowerShell 7.2.5. runs on .NET 6.0.6.

@FehintolaObafemi, I'm getting the following errors when I run Connect-MgGraph in PowerShell 7.2.5:

➜ .\tools\GenerateAuthenticationModule.ps1 -Build -Run
➜ Connect-MgGraph
Connect-MgGraph: InteractiveBrowserCredential authentication failed: If you have a Windows application which targets net5 or net5-windows, please change the target to net5-windows10.0.17763.0. 
Your app can still run on earlier versions of Windows such as Win7 if you add <SupportedOSPlatformVersion>7</SupportedOSPlatformVersion> in the csproj.
 The broker (WAM) is available only on Win10 and this library will fallback to a browser on older systems. 
If you have a NET5 cross-platform (Windows, Mac, Linux) application, please dual target net5 and net5-windows10.0.17763.0. Your installer should deploy the net5 version on Mac and Linux and the net5-window10.0.17763.0 on Windows.
If you have a .NET Core 3.1 application, please install the NuGet package named Microsoft.Identity.Client.Desktop and call the extension method .WithWindowsBroker() first. 
If you want to try the new broker preview, please install the NuGet package named Microsoft.Identity.Client.Broker and call the extension method .WithBrokerPreview(). 
For details see https://aka.ms/msal-net-wam and https://github.com/dotnet/designs/blob/main/accepted/2020/platform-checks/platform-checks.md 

peombwa avatar Jun 29 '22 19:06 peombwa

Since we are using MSAL 4.44.X in this implementation, you may want to wait for Azure.Identity.BrokeredAuthentication to provide us with an implementation that uses the new .WithBrokerPreview(). See https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/wam#new-wam-preview-in-msal-444 and Azure/Identity's implementation. Otherwise, we you'll also need to ensure the prerequisites in https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#availability are met for it to work across multiple targets. You can find PowerShell's .NET runtime version using [System.Runtime.InteropServices.RuntimeInformation]::FrameworkDescription. For example, PowerShell 5.1 runs on .NET Framework 4.8.X while PowerShell 7.2.5. runs on .NET 6.0.6.

@FehintolaObafemi, I'm getting the following errors when I run Connect-MgGraph in PowerShell 7.2.5:

➜ .\tools\GenerateAuthenticationModule.ps1 -Build -Run
➜ Connect-MgGraph
Connect-MgGraph: InteractiveBrowserCredential authentication failed: If you have a Windows application which targets net5 or net5-windows, please change the target to net5-windows10.0.17763.0. 
Your app can still run on earlier versions of Windows such as Win7 if you add <SupportedOSPlatformVersion>7</SupportedOSPlatformVersion> in the csproj.
 The broker (WAM) is available only on Win10 and this library will fallback to a browser on older systems. 
If you have a NET5 cross-platform (Windows, Mac, Linux) application, please dual target net5 and net5-windows10.0.17763.0. Your installer should deploy the net5 version on Mac and Linux and the net5-window10.0.17763.0 on Windows.
If you have a .NET Core 3.1 application, please install the NuGet package named Microsoft.Identity.Client.Desktop and call the extension method .WithWindowsBroker() first. 
If you want to try the new broker preview, please install the NuGet package named Microsoft.Identity.Client.Broker and call the extension method .WithBrokerPreview(). 
For details see https://aka.ms/msal-net-wam and https://github.com/dotnet/designs/blob/main/accepted/2020/platform-checks/platform-checks.md 

@peombwa I can't seem to recreate this error on my end following the step you highlighted above.

FehintolaObafemi avatar Jun 30 '22 17:06 FehintolaObafemi

@FehintolaObafemi, are you getting a WAM account picker pop up when you sign in? If not, then you are using an access token from the cache and not going through WAM. Run Disconnect-MgGraph after Connect-MgGraph then try again using the steps I provided above.

The issue is also present in PowerShell 5.1 as of commit #1fde021649fffc4fbbfcda886485c5a1a13bd23b:

Connect-MgGraph : InteractiveBrowserCredential authentication failed: The Windows broker is not directly available on MSAL for .NET
Framework To use it, please install the NuGet package named Microsoft.Identity.Client.Desktop and call the extension method
.WithWindowsBroker() first.If you want to try the new broker preview, please install the NuGet package named
Microsoft.Identity.Client.Broker and call the extension method .WithBrokerPreview().
At line:1 char:1
+ Connect-MgGraph
+ ~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Connect-MgGraph], AuthenticationFailedException
    + FullyQualifiedErrorId : Microsoft.Graph.PowerShell.Authentication.Cmdlets.ConnectMgGraph

We can sync offline for the repro steps.

peombwa avatar Jun 30 '22 17:06 peombwa

One question @peombwa -- do we need to make this behaavior opt-in via Connect-MgGraph? I spoke to one of the implementers of WAM in Windows and I'm not clear on which Windows versions support WAM and also if there are any corner cases we'd like to provide a way to avoid.

@adamedx, good point! We should make this an opt-in feature since WAM has couple of limitations that customers may have a hard time with:

New accounts are automatically remembered by Windows. Work and School have the option of joining the organization's directory or opting out completely, in which case the account won't appear under "Email & Accounts". Microsoft accounts are automatically added to Windows.

See https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#other-considerations.

MSAL's implementation is OS aware, and should fallback to a browser when WAM is not supported. See https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#wam-limitations.

@peombwa , @maisarissi, suggestion for how to expose it? Should it be a preference variable, or just a new option, e.g. SigninUi with the options of Auto, Browser, and Native, where Browser is the current mechanism (web browser), Native is WAM (not embedded web view, though we could have that option as well), and Auto is whatever we decide we want to default to, initially Browser to maintain current behavior.

adamedx avatar Jul 05 '22 15:07 adamedx

@peombwa , @maisarissi, suggestion for how to expose it? Should it be a preference variable, or just a new option, e.g. SigninUi with the options of Auto, Browser, and Native, where Browser is the current mechanism (web browser), Native is WAM (not embedded web view, though we could have that option as well), and Auto is whatever we decide we want to default to, initially Browser to maintain current behavior.

@adamedx, I like the idea of having a -SigninUi parameter. We can work with just Browser and Native as the parameter values by making -SigninUi parameter optional with a default value of Browser. @maisarissi, thoughts?

@FehintolaObafemi, let's make -SigninUi an optional parameter on Connect-MgGraph with:

  • Browser and Native as the enum values.
  • UserParameterSet as the parameterSetName
  • GraphSession object can be used to hold the value of -SigninUi for the lifetime of the current process.

See -ContextScope example as a reference.

peombwa avatar Jul 05 '22 20:07 peombwa

@adamedx, I like the idea of having a -SigninUi parameter. We can work with just Browser and Native as the parameter values by making -SigninUi parameter optional with a default value of Browser. @maisarissi, thoughts?

Agreed! I like this idea.

maisarissi avatar Jul 06 '22 12:07 maisarissi

@FehintolaObafemi , once everything looks good, would be great to rebase this into a smaller set of commits to simplify the change log.

adamedx avatar Aug 17 '22 20:08 adamedx

@FehintolaObafemi, I've resolved the msalRuntime.dll not found exception in https://github.com/microsoftgraph/msgraph-sdk-powershell/pull/1344/commits/56464e817bd9fe93709ee8d988a14fe6d6f8b097. Please update the PR with the suggestions I've made in the comments above for us to approve the PR.

peombwa avatar Oct 12 '22 18:10 peombwa

https://github.com/Azure/azure-sdk-for-net/issues/31844

Issue has been filed with the Azure Identity team

FehintolaObafemi avatar Oct 17 '22 19:10 FehintolaObafemi

Continued in #2034 due to this PR being closed on accident and not re-opening.

FehintolaObafemi avatar May 24 '23 15:05 FehintolaObafemi