Pass Microsoft EntraID Authority to CreatePublicClient to Fix Az.Ssh Issue When WAM Enabled #25944

msJinLei · 2024-08-27T01:22:08Z

Description

Before fix

After

Mandatory Checklist

Please choose the target release of Azure PowerShell. (⚠️Target release is a different concept from API readiness. Please click below links for details.)
- General release
- Public preview
- Private preview
- Engineering build
- No need for a release
Check this box to confirm: I have read the Submitting Changes section of CONTRIBUTING.md and reviewed the following information:

SHOULD update ChangeLog.md file(s) appropriately
- For SDK-based development mode, update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
  - A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
- For autorest-based development mode, include the changelog in the PR description.
- Should not change ChangeLog.md if no new release is required, such as fixing test case only.
SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
SHOULD have proper test coverage for changes in pull request.
SHOULD NOT adjust version of module manually in pull request

azure-client-tools-bot-prd · 2024-08-27T01:22:10Z

⚠️Az.Accounts

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Breaking Change Check

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Signature Check

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Help File Existence Check

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️File Change Check

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️UX Metadata Check

⚠️PowerShell Core - Windows

Type Module ResourceType SubResourceType Command Description

⚠️ Az.Accounts Microsoft.Subscription subscriptions Get-AzSubscription The path /subscriptions/{subscriptionId} doesn't contains the right resource tpye: Microsoft.Subscription

⚠️Windows PowerShell - Windows

Type Module ResourceType SubResourceType Command Description

⚠️ Az.Accounts Microsoft.Subscription subscriptions Get-AzSubscription The path /subscriptions/{subscriptionId} doesn't contains the right resource tpye: Microsoft.Subscription

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.Aks

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.ApplicationInsights

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.Compute

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.Functions

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️Az.KeyVault

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️Test

⚠️ - Linux

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 22.09 % Test coverage for the module cannot be lower than 50%.

⚠️ - MacOS

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 22.09% Test coverage for the module cannot be lower than 50%.

⚠️PowerShell Core - Windows

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 22.09% Test coverage for the module cannot be lower than 50%.

⚠️Windows PowerShell - Windows

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 22.09% Test coverage for the module cannot be lower than 50%.

️✔️Az.KubernetesConfiguration

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.ManagedServiceIdentity

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.Monitor

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.Network

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.OperationalInsights

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.PostgreSql

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.PrivateDns

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️Az.Purview

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️Test

⚠️ - Linux

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 8.70 % Test coverage for the module cannot be lower than 50%.

⚠️ - MacOS

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 8.70% Test coverage for the module cannot be lower than 50%.

⚠️PowerShell Core - Windows

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 8.70% Test coverage for the module cannot be lower than 50%.

⚠️Windows PowerShell - Windows

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 8.70% Test coverage for the module cannot be lower than 50%.

️✔️Az.Resources

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Az.Sql

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️Az.Storage

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

⚠️Test

⚠️ - Linux

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 41.76 % Test coverage for the module cannot be lower than 50%.

⚠️ - MacOS

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 41.76% Test coverage for the module cannot be lower than 50%.

⚠️PowerShell Core - Windows

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 41.76% Test coverage for the module cannot be lower than 50%.

⚠️Windows PowerShell - Windows

Type Title Current Coverage Description

⚠️ Test Coverage Less Than 50% 41.76% Test coverage for the module cannot be lower than 50%.

️✔️Az.Websites

️✔️Build

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

️✔️Test

️✔️ - Linux

️✔️ - MacOS

️✔️PowerShell Core - Windows

️✔️Windows PowerShell - Windows

isra-fel · 2024-08-27T06:57:17Z

src/Accounts/Authentication/Authentication/TokenCache/PowerShellTokenCacheProvider.cs

 namespace Microsoft.Azure.Commands.Common.Authentication
 {
    public abstract class PowerShellTokenCacheProvider
    {
        public const string PowerShellTokenCacheProviderKey = "PowerShellTokenCacheProviderKey";
-        private static readonly string CommonTenant = "organizations";
+        //Refer to https://learn.microsoft.com/en-us/dotnet/api/microsoft.identity.client.abstractapplicationbuilder-1.withauthority?view=msal-dotnet-latest#microsoft-identity-client-abstractapplicationbuilder-1-withauthority(system-string-system-boolean
+        //However, neither "commons" nor "organizations" works for MSA account


What does this mean? Need more explaination

What does this mean? Need more explaination

I rename CommonTenant to OrgnizationTenant to differentiate from the real common tenant "/common". But as the discussion with Ashok and Jiashuo, we shall always use "/organizations" for both work and school, and MSA accounts.

isra-fel · 2024-08-27T06:58:16Z

src/Accounts/Authentication/Authentication/TokenCache/PowerShellTokenCacheProvider.cs

        /// </summary>
-        public virtual IPublicClientApplication CreatePublicClient(string authority = null)
+        public virtual IPublicClientApplication CreatePublicClient(string authority, string tenantId = null)


How about making tenantId mandatory so that it doesn't conflict with the other CreatePublicClient()

Consider putting the new api into SshCredentialFactory.
Because we have two purposes to create msal client - (a) deal with token cache (b) for ssh.

I find there is a limitation to move CreatePublicClient out of PowerShellTokenCacheProvider as CreatePublicClient call RegisterCache which implements in PowerShellTokenCacheProvider.
And so I create a new CreatePublicClient with two inputs and keep all the other logic as what it is.

src/Accounts/Authentication/Factories/SshCredentialFactory.cs

Address review comments Polish change log Address review comments Address review comments

…Issue When WAM Enabled (#25944) * Address review comments Address review comments Polish change log Address review comments Address review comments * Integrate Microsoft.Identity.Client 4.65.0 * Polish change log

…Issue When WAM Enabled (#25944) (#26459) * Address review comments Address review comments Polish change log Address review comments Address review comments * Integrate Microsoft.Identity.Client 4.65.0 * Polish change log Co-authored-by: Jin Lei <54836179+msJinLei@users.noreply.github.com>

msJinLei force-pushed the ssh_fix_split branch from 71ddae4 to 33d6ab0 Compare August 27, 2024 01:26

msJinLei added this to the Az 12.3.0 (September 2024, 2024-09-03) milestone Aug 27, 2024

msJinLei force-pushed the ssh_fix_split branch 2 times, most recently from 3dfba13 to e94462e Compare August 27, 2024 06:36

msJinLei requested review from bilaakpan-ms, Sandido, haagha and grizzlytheodore as code owners August 27, 2024 06:36

msJinLei force-pushed the ssh_fix_split branch from e94462e to eff3558 Compare August 27, 2024 06:42

msJinLei changed the base branch from main to release-2024-09-03 August 27, 2024 06:43

isra-fel requested changes Aug 27, 2024

View reviewed changes

msJinLei changed the base branch from release-2024-09-03 to main August 28, 2024 07:37

msJinLei force-pushed the ssh_fix_split branch from 6a1047d to 1de9462 Compare October 11, 2024 01:14

msJinLei modified the milestones: Az 12.3.0 (September 2024, 2024-09-03), Az 12.5.0 (October 2024, 2024-11-05) Oct 11, 2024

msJinLei force-pushed the ssh_fix_split branch 2 times, most recently from 716ccf8 to 70e3996 Compare October 21, 2024 08:43

msJinLei added 2 commits October 23, 2024 17:00

Address review comments

a0f3c5c

Address review comments Polish change log Address review comments Address review comments

Integrate Microsoft.Identity.Client 4.65.0

56757a6

msJinLei force-pushed the ssh_fix_split branch from 70e3996 to 56757a6 Compare October 23, 2024 09:25

Polish change log

b01cf3c

isra-fel approved these changes Oct 23, 2024

View reviewed changes

Nickcandy merged commit d0358fe into Azure:main Oct 23, 2024
12 checks passed

VeryEarly mentioned this pull request Oct 24, 2024

Pass Microsoft EntraID Authority to CreatePublicClient to Fix Az.Ssh … #26459

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass Microsoft EntraID Authority to CreatePublicClient to Fix Az.Ssh Issue When WAM Enabled #25944

Pass Microsoft EntraID Authority to CreatePublicClient to Fix Az.Ssh Issue When WAM Enabled #25944

msJinLei commented Aug 27, 2024 •

edited

Loading

azure-client-tools-bot-prd bot commented Aug 27, 2024 •

edited

Loading

isra-fel Aug 27, 2024

msJinLei Aug 28, 2024

isra-fel Aug 27, 2024

isra-fel Aug 27, 2024

msJinLei Aug 28, 2024

Pass Microsoft EntraID Authority to CreatePublicClient to Fix Az.Ssh Issue When WAM Enabled #25944

Pass Microsoft EntraID Authority to CreatePublicClient to Fix Az.Ssh Issue When WAM Enabled #25944

Conversation

msJinLei commented Aug 27, 2024 • edited Loading

Description

Mandatory Checklist

azure-client-tools-bot-prd bot commented Aug 27, 2024 • edited Loading

isra-fel Aug 27, 2024

Choose a reason for hiding this comment

msJinLei Aug 28, 2024

Choose a reason for hiding this comment

isra-fel Aug 27, 2024

Choose a reason for hiding this comment

isra-fel Aug 27, 2024

Choose a reason for hiding this comment

msJinLei Aug 28, 2024

Choose a reason for hiding this comment

msJinLei commented Aug 27, 2024 •

edited

Loading

azure-client-tools-bot-prd bot commented Aug 27, 2024 •

edited

Loading