Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Identity] Expose MsaPassthrough as an option #31227

Closed
wants to merge 4 commits into from
Closed

[Identity] Expose MsaPassthrough as an option #31227

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90a951d
allow msa passthru as an option
isra-fel Sep 19, 2022
71c8481
regen public api
isra-fel Sep 20, 2022
5f50cff
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-net int…
isra-fel Nov 29, 2022
f8d1f3f
Passthrough -> PassThrough
isra-fel Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...Azure.Identity.BrokeredAuthentication/api/Azure.Identity.BrokeredAuthentication.net462.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ namespace Azure.Identity.BrokeredAuthentication
{
public partial class InteractiveBrowserCredentialBrokerOptions : Azure.Identity.InteractiveBrowserCredentialOptions
{
public InteractiveBrowserCredentialBrokerOptions(System.IntPtr parentWindowHandle) { }
public InteractiveBrowserCredentialBrokerOptions(System.IntPtr parentWindowHandle, bool msaPassthrough = false) { }
isra-fel marked this conversation as resolved.
Show resolved Hide resolved
}
public partial class SharedTokenCacheCredentialBrokerOptions : Azure.Identity.SharedTokenCacheCredentialOptions
{
public SharedTokenCacheCredentialBrokerOptions() { }
public SharedTokenCacheCredentialBrokerOptions(Azure.Identity.TokenCachePersistenceOptions tokenCacheOptions) { }
public SharedTokenCacheCredentialBrokerOptions(Azure.Identity.TokenCachePersistenceOptions tokenCacheOptions, bool msaPassthrough = false) { }
public SharedTokenCacheCredentialBrokerOptions(bool msaPassthrough = false) { }
}
}
6 changes: 3 additions & 3 deletions ...dentity.BrokeredAuthentication/api/Azure.Identity.BrokeredAuthentication.netcoreapp3.1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ namespace Azure.Identity.BrokeredAuthentication
{
public partial class InteractiveBrowserCredentialBrokerOptions : Azure.Identity.InteractiveBrowserCredentialOptions
{
public InteractiveBrowserCredentialBrokerOptions(System.IntPtr parentWindowHandle) { }
public InteractiveBrowserCredentialBrokerOptions(System.IntPtr parentWindowHandle, bool msaPassthrough = false) { }
}
public partial class SharedTokenCacheCredentialBrokerOptions : Azure.Identity.SharedTokenCacheCredentialOptions
{
public SharedTokenCacheCredentialBrokerOptions() { }
public SharedTokenCacheCredentialBrokerOptions(Azure.Identity.TokenCachePersistenceOptions tokenCacheOptions) { }
public SharedTokenCacheCredentialBrokerOptions(Azure.Identity.TokenCachePersistenceOptions tokenCacheOptions, bool msaPassthrough = false) { }
public SharedTokenCacheCredentialBrokerOptions(bool msaPassthrough = false) { }
}
}
6 changes: 3 additions & 3 deletions ...entity.BrokeredAuthentication/api/Azure.Identity.BrokeredAuthentication.netstandard2.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ namespace Azure.Identity.BrokeredAuthentication
{
public partial class InteractiveBrowserCredentialBrokerOptions : Azure.Identity.InteractiveBrowserCredentialOptions
{
public InteractiveBrowserCredentialBrokerOptions(System.IntPtr parentWindowHandle) { }
public InteractiveBrowserCredentialBrokerOptions(System.IntPtr parentWindowHandle, bool msaPassthrough = false) { }
}
public partial class SharedTokenCacheCredentialBrokerOptions : Azure.Identity.SharedTokenCacheCredentialOptions
{
public SharedTokenCacheCredentialBrokerOptions() { }
public SharedTokenCacheCredentialBrokerOptions(Azure.Identity.TokenCachePersistenceOptions tokenCacheOptions) { }
public SharedTokenCacheCredentialBrokerOptions(Azure.Identity.TokenCachePersistenceOptions tokenCacheOptions, bool msaPassthrough = false) { }
public SharedTokenCacheCredentialBrokerOptions(bool msaPassthrough = false) { }
}
}
12 changes: 10 additions & 2 deletions ...ty/Azure.Identity.BrokeredAuthentication/src/InteractiveBrowserCredentialBrokerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,29 @@ namespace Azure.Identity.BrokeredAuthentication
public class InteractiveBrowserCredentialBrokerOptions : InteractiveBrowserCredentialOptions, IMsalPublicClientInitializerOptions
{
private IntPtr _parentWindowHandle;
private bool _msaPassthrough;

/// <summary>
/// Creates a new instance of <see cref="InteractiveBrowserCredentialBrokerOptions"/> to configure a <see cref="InteractiveBrowserCredential"/>.
/// </summary>
/// <param name="parentWindowHandle">Handle of the parent window the system authentication broker should be docked to.</param>
public InteractiveBrowserCredentialBrokerOptions(IntPtr parentWindowHandle) : base()
/// <param name="msaPassthrough">A legacy option available only to old Microsoft applications. Should be avoided where possible. Support is experimental.</param>
public InteractiveBrowserCredentialBrokerOptions(IntPtr parentWindowHandle, bool msaPassthrough = false) : base()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New default param is a subtle breaking change (a binary breaking change). Prefer to use a new ctor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing and good to know that :) Azure.Identity.BrokeredAuthentication is a beta so I thought we needn't worry too much about breaking change. Anyway, I'll let @schaabs 's team decide the final look of the API.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way of doing this without exposing public API is to turn on this option based on client id and authority. AzurePS client id is configured as MSA-PT. So basically, hardcode some client_ids.

Afaik, it is not possible to configure an app to move away from being MSA-PT. Identity recommends folks to create a new client_id.

{
_parentWindowHandle = parentWindowHandle;
_msaPassthrough = msaPassthrough;
}

Action<PublicClientApplicationBuilder> IMsalPublicClientInitializerOptions.BeforeBuildClient => AddBroker;

private void AddBroker(PublicClientApplicationBuilder builder)
{
builder.WithBrokerPreview().WithParentActivityOrWindow(() => _parentWindowHandle);
builder.WithBrokerPreview()
.WithParentActivityOrWindow(() => _parentWindowHandle)
.WithWindowsBrokerOptions(new WindowsBrokerOptions()
{
MsaPassthrough = _msaPassthrough
});
}
}
}
16 changes: 12 additions & 4 deletions ...tity/Azure.Identity.BrokeredAuthentication/src/SharedTokenCacheCredentialBrokerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,35 @@ namespace Azure.Identity.BrokeredAuthentication
/// </summary>
public class SharedTokenCacheCredentialBrokerOptions : SharedTokenCacheCredentialOptions, IMsalPublicClientInitializerOptions
{
private bool _msaPassthrough;

/// <summary>
/// Initializes a new instance of <see cref="SharedTokenCacheCredentialBrokerOptions"/>.
/// </summary>
public SharedTokenCacheCredentialBrokerOptions()
: this(null)
public SharedTokenCacheCredentialBrokerOptions(bool msaPassthrough = false)
: this(null, msaPassthrough)
{ }

/// <summary>
/// Initializes a new instance of <see cref="SharedTokenCacheCredentialBrokerOptions"/>.
/// </summary>
/// <param name="tokenCacheOptions">The <see cref="TokenCachePersistenceOptions"/> that will apply to the token cache used by this credential.</param>
public SharedTokenCacheCredentialBrokerOptions(TokenCachePersistenceOptions tokenCacheOptions)
/// <param name="msaPassthrough">A legacy option available only to old Microsoft applications. Should be avoided where possible. Support is experimental.</param>
public SharedTokenCacheCredentialBrokerOptions(TokenCachePersistenceOptions tokenCacheOptions, bool msaPassthrough = false)
: base(tokenCacheOptions)
{
_msaPassthrough = msaPassthrough;
}

Action<PublicClientApplicationBuilder> IMsalPublicClientInitializerOptions.BeforeBuildClient => AddBroker;

private void AddBroker(PublicClientApplicationBuilder builder)
{
builder.WithBrokerPreview();
builder.WithBrokerPreview()
.WithWindowsBrokerOptions(new WindowsBrokerOptions()
{
MsaPassthrough = _msaPassthrough
});
}
}
}
2 changes: 1 addition & 1 deletion ...re.Identity.BrokeredAuthentication/tests/ManualInteractiveBrowserCredentialBrokerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public async Task AuthenticateWithBrokerAsync()

// to fully manually verify the InteractiveBrowserCredential this test should be run both authenticating with a
// school / organization account as well as a personal live account, i.e. a @outlook.com, @live.com, or @hotmail.com
var cred = new InteractiveBrowserCredential(new InteractiveBrowserCredentialBrokerOptions(parentWindowHandle));
var cred = new InteractiveBrowserCredential(new InteractiveBrowserCredentialBrokerOptions(parentWindowHandle, true));

AccessToken token = await cred.GetTokenAsync(new TokenRequestContext(new string[] { "https://vault.azure.net/.default" })).ConfigureAwait(false);

Expand Down
5 changes: 3 additions & 2 deletions ...zure.Identity.BrokeredAuthentication/tests/ManualSharedTokenCacheCredentialBrokerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ public async Task SilentAuthenticateWithBrokerAsync()

// to fully manually verify the InteractiveBrowserCredential this test should be run both authenticating with a
// school / organization account as well as a personal live account, i.e. a @outlook.com, @live.com, or @hotmail.com
var cred = new InteractiveBrowserCredential(new InteractiveBrowserCredentialBrokerOptions(parentWindowHandle) { TokenCachePersistenceOptions = persistenceOptions});
const bool MsaPassthrough = true;
var cred = new InteractiveBrowserCredential(new InteractiveBrowserCredentialBrokerOptions(parentWindowHandle, MsaPassthrough) { TokenCachePersistenceOptions = persistenceOptions});

AccessToken token = await cred.GetTokenAsync(new TokenRequestContext(new string[] { "https://vault.azure.net/.default" })).ConfigureAwait(false);

Assert.NotNull(token.Token);

var silentCred = new SharedTokenCacheCredential(new SharedTokenCacheCredentialBrokerOptions());
var silentCred = new SharedTokenCacheCredential(new SharedTokenCacheCredentialBrokerOptions(MsaPassthrough));

// The calls below this should be silent and not require user interaction
token = await cred.GetTokenAsync(new TokenRequestContext(new string[] { "https://vault.azure.net/.default" })).ConfigureAwait(false);
Expand Down