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.

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

[WAM Broker] Check if scopes are passed when broker is invoked #3664

Merged
merged 5 commits into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 25 additions & 7 deletions src/client/Microsoft.Identity.Client.Broker/RuntimeBroker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ private static void OnProcessExit(object sender, EventArgs e)

if (authenticationRequestParameters?.Account?.HomeAccountId?.ObjectId != null)
{
using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
using (var authParams = WamAdapters.GetCommonAuthParameters(
authenticationRequestParameters,
_wamOptions.MsaPassthrough,
_logger))
{
using (var readAccountResult = await s_lazyCore.Value.ReadAccountByIdAsync(
authenticationRequestParameters.Account.HomeAccountId.ObjectId,
Expand Down Expand Up @@ -161,7 +164,10 @@ private static void OnProcessExit(object sender, EventArgs e)
var cancellationToken = authenticationRequestParameters.RequestContext.UserCancellationToken;
Debug.Assert(s_lazyCore.Value != null, "Should not call this API if msal runtime init failed");

using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
using (var authParams = WamAdapters.GetCommonAuthParameters(
authenticationRequestParameters,
_wamOptions.MsaPassthrough,
_logger))
{
//Login Hint
string loginHint = authenticationRequestParameters.LoginHint ?? authenticationRequestParameters?.Account?.Username;
Expand Down Expand Up @@ -193,7 +199,10 @@ private static void OnProcessExit(object sender, EventArgs e)

_logger.Verbose("[WamBroker] Signing in with the default user account.");

using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
using (var authParams = WamAdapters.GetCommonAuthParameters(
authenticationRequestParameters,
_wamOptions.MsaPassthrough,
_logger))
{
using (NativeInterop.AuthResult result = await s_lazyCore.Value.SignInAsync(
_parentHandle,
Expand Down Expand Up @@ -230,7 +239,10 @@ private IntPtr GetParentWindow(CoreUIParent uiParent)

_logger.Verbose("[WamBroker] Acquiring token silently.");

using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
using (var authParams = WamAdapters.GetCommonAuthParameters(
authenticationRequestParameters,
_wamOptions.MsaPassthrough,
_logger))
{
using (var readAccountResult = await s_lazyCore.Value.ReadAccountByIdAsync(
acquireTokenSilentParameters.Account.HomeAccountId.ObjectId,
Expand Down Expand Up @@ -274,7 +286,10 @@ private IntPtr GetParentWindow(CoreUIParent uiParent)

_logger.Verbose("[WamBroker] Acquiring token silently for default account.");

using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
using (var authParams = WamAdapters.GetCommonAuthParameters(
authenticationRequestParameters,
_wamOptions.MsaPassthrough,
_logger))
{
using (NativeInterop.AuthResult result = await s_lazyCore.Value.SignInSilentlyAsync(
authParams,
Expand All @@ -300,7 +315,10 @@ private IntPtr GetParentWindow(CoreUIParent uiParent)

_logger.Verbose("[WamBroker] Acquiring token with Username Password flow.");

using (AuthParameters authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
using (AuthParameters authParams = WamAdapters.GetCommonAuthParameters(
authenticationRequestParameters,
_wamOptions.MsaPassthrough,
_logger))
{
authParams.Properties["MSALRuntime_Username"] = acquireTokenByUsernamePasswordParameters.Username;
authParams.Properties["MSALRuntime_Password"] = acquireTokenByUsernamePasswordParameters.Password;
Expand Down Expand Up @@ -424,7 +442,7 @@ public bool IsBrokerInstalledAndInvokable(AuthorityType authorityType)
return false;
}

_logger.Verbose($"[WAM Broker] MsalRuntime init succesful.");
_logger.Verbose($"[WAM Broker] MsalRuntime init successful.");
return true;
}
}
Expand Down
32 changes: 31 additions & 1 deletion src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,15 @@ private enum ResponseStatus
/// </summary>
/// <param name="authenticationRequestParameters"></param>
/// <param name="isMsaPassthrough"></param>
/// <param name="logger"></param>
public static NativeInterop.AuthParameters GetCommonAuthParameters(
AuthenticationRequestParameters authenticationRequestParameters,
bool isMsaPassthrough)
bool isMsaPassthrough,
ILoggerAdapter logger)
{
logger.Verbose("[WamBroker] Validating Common Auth Parameters.");
ValidateAuthParams(authenticationRequestParameters, logger);

var authParams = new NativeInterop.AuthParameters
(authenticationRequestParameters.AppConfig.ClientId,
authenticationRequestParameters.Authority.AuthorityInfo.CanonicalAuthority.ToString());
Expand Down Expand Up @@ -171,6 +176,8 @@ private enum ResponseStatus

AddPopParams(authenticationRequestParameters, authParams);

logger.Verbose("[WamBroker] Acquired Common Auth Parameters.");

return authParams;
}

Expand Down Expand Up @@ -281,5 +288,28 @@ private static string GetExpectedRedirectUri(string clientId)
{
return $"ms-appx-web://microsoft.aad.brokerplugin/{clientId}";
}

/// <summary>
/// Validate common auth params
/// </summary>
/// <param name="authenticationRequestParameters"></param>
/// <param name="logger"></param>
/// <exception cref="MsalClientException"></exception>
public static void ValidateAuthParams(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is used in only one place in the same class. Please make it private. (or write unit test :))

AuthenticationRequestParameters authenticationRequestParameters,
ILoggerAdapter logger)
{
//MSAL Runtime throws an ApiContractViolation Exception with Tag: 0x2039c1cb (InvalidArg)
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
//When no scopes are passed, this will check if user is passing scopes
if (!authenticationRequestParameters.HasScopes)
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
{
logger.Error($"[WamBroker] {MsalError.WamScopesRequired} " +
$"{MsalErrorMessage.ScopesRequired}");

throw new MsalClientException(
MsalError.WamScopesRequired,
MsalErrorMessage.ScopesRequired);
}
}
}
}
6 changes: 6 additions & 0 deletions src/client/Microsoft.Identity.Client/MsalError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,12 @@ public static class MsalError
/// </summary>
public const string WamPickerError = "wam_interactive_picker_error";

/// <summary>
/// <para>What happens?</para>No scopes have been requested
/// <para>Mitigation</para>At least one scope must be specified for MSAL Runtime WAM
/// </summary>
public const string WamScopesRequired = "scopes_required_wam";

/// <summary>
/// <para>What happens?</para>The embedded browser cannot be started because a runtime component is missing.
/// <para>Mitigation</para>"The embedded browser needs WebView2 runtime to be installed. An end user of the app can download and install the WebView2 runtime from https://go.microsoft.com/fwlink/p/?LinkId=2124703 and restart the app.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ private async Task<Bundle> PerformContentResolverOperationAsync(ContentResolverO

if (resultBundle != null)
{
_logger.Verbose($"[Android broker] Content resolver operation completed succesfully. Operation: {operation} URI: {contentResolverUri}");
_logger.Verbose($"[Android broker] Content resolver operation completed successfully. Operation: {operation} URI: {contentResolverUri}");
}

return resultBundle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal static class MsalMockHelpers
}

/// <summary>
/// Configures a web ui that returns a succesfull result
/// Configures a web ui that returns a successfull result
/// </summary>
public static void ConfigureMockWebUI(
this IServiceBundle serviceBundle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ await app.AcquireTokenSilent(TestConstants.s_scope, account).ExecuteAsync()
await app.AcquireTokenSilent(singleScope, account).WithForceRefresh(true).ExecuteAsync()
.ConfigureAwait(false);

// there will still be an entry here because the refresh token written by the first succesful ATS
// there will still be an entry here because the refresh token written by the first successful ATS
// is different from the initial RT
AssertThrottlingCacheEntryCount(throttlingManager, uiRequiredEntryCount: 1);
}
Expand Down
4 changes: 3 additions & 1 deletion tests/devapps/WAM/NetCoreWinFormsWam/Form1.cs
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,11 @@ private async Task<AuthenticationResult> RunAtsAsync(IPublicClientApplication pc
private string[] GetScopes()
{
string[] result = null;

cbxScopes.Invoke((MethodInvoker)delegate
{
result = cbxScopes.Text.Split(' ');
if(!string.IsNullOrWhiteSpace(cbxScopes.Text))
result = cbxScopes.Text.Split(' ');
});

return result;
Expand Down
3 changes: 2 additions & 1 deletion tests/devapps/WAM/NetFrameworkWam/Form1.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ private string[] GetScopes()
string[] result = null;
cbxScopes.Invoke((MethodInvoker)delegate
{
result = cbxScopes.Text.Split(' ');
if (!string.IsNullOrWhiteSpace(cbxScopes.Text))
result = cbxScopes.Text.Split(' ');
});

return result;
Expand Down