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

Organize browser selection classes and change signature for get AuthorizationStrategy, Fixes AB#3119409 #2564

Merged
merged 18 commits into from
Jan 24, 2025
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
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
vNext
----------
- [MINOR] Organize browser selection classes and change signature for get AuthorizationStrategy (#2564)
- [MINOR] Add support for OneBox Environment (#2559)
- [MINOR] Add support for claims requests for native authentication (#2572)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.microsoft.identity.common.internal.platform.AndroidPlatformUtil;
import com.microsoft.identity.common.internal.providers.oauth2.AndroidTaskStateGenerator;
import com.microsoft.identity.common.internal.ui.AndroidAuthorizationStrategyFactory;
import com.microsoft.identity.common.internal.ui.browser.AndroidBrowserSelector;
import com.microsoft.identity.common.java.WarningType;
import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
import com.microsoft.identity.common.java.interfaces.PlatformComponents;
Expand Down Expand Up @@ -127,14 +128,16 @@ public static void fillBuilderWithBasicImplementations(
.storageSupplier(new AndroidStorageSupplier(context,
new AndroidAuthSdkStorageEncryptionManager(context)))
.platformUtil(new AndroidPlatformUtil(context, activity))
.httpClientWrapper(new DefaultHttpClientWrapper());
.httpClientWrapper(new DefaultHttpClientWrapper())
.browserSelector(new AndroidBrowserSelector(context));
Copy link
Contributor

Choose a reason for hiding this comment

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

.browserSelector(new AndroidBrowserSelector(context));

would you still need here? If so, can we set same object on both and not create new ?

Copy link
Contributor Author

@p3dr0rv p3dr0rv Jan 22, 2025

Choose a reason for hiding this comment

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

YES.
when duna is enabled, we need to use the browser selector to check if we have a duna broker available.
see: https://github.com/AzureAD/ad-accounts-for-android/pull/3017/files/70382d1f9a954f545ecb58e7396e54c678a7a802#r1890858348

yes, I can use the same object here


if (activity != null){
builder.authorizationStrategyFactory(
AndroidAuthorizationStrategyFactory.builder()
.context(activity.getApplicationContext())
.activity(activity)
.fragment(fragment)
.browserSelector(new AndroidBrowserSelector(context))
.build())
.stateGenerator(new AndroidTaskStateGenerator(activity.getTaskId()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.microsoft.identity.common.java.authorities.Authority;
import com.microsoft.identity.common.java.authscheme.AbstractAuthenticationScheme;
import com.microsoft.identity.common.java.authscheme.IPoPAuthenticationSchemeParams;
import com.microsoft.identity.common.java.browser.Browser;
import com.microsoft.identity.common.java.cache.ICacheRecord;
import com.microsoft.identity.common.java.commands.parameters.CommandParameters;
import com.microsoft.identity.common.java.commands.parameters.DeviceCodeFlowCommandParameters;
Expand Down Expand Up @@ -226,7 +227,14 @@ private AuthorizationResult performAuthorizationRequest(@NonNull final OAuth2Str
.getPlatformUtil()
.throwIfNetworkNotAvailable(parameters.isPowerOptCheckEnabled());

mAuthorizationStrategy = parameters.getPlatformComponents().getAuthorizationStrategyFactory().getAuthorizationStrategy(parameters);
mAuthorizationStrategy = parameters.getPlatformComponents()
.getAuthorizationStrategyFactory()
.getAuthorizationStrategy(
parameters.getAuthorizationAgent(),
parameters.getBrowserSafeList(),
parameters.getPreferredBrowser(),
false
);
mAuthorizationRequest = getAuthorizationRequest(strategy, parameters);

// Suppressing unchecked warnings due to casting of AuthorizationRequest to GenericAuthorizationRequest and AuthorizationStrategy to GenericAuthorizationStrategy in the arguments of call to requestAuthorization method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,12 @@
import com.microsoft.identity.common.java.flighting.CommonFlight;
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
import com.microsoft.identity.common.java.logging.Logger;
import com.microsoft.identity.common.java.ui.BrowserDescriptor;
import com.microsoft.identity.common.java.util.IPlatformUtil;
import com.microsoft.identity.common.java.util.StringUtil;

import java.security.NoSuchAlgorithmException;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;

Expand All @@ -82,28 +80,6 @@ public class AndroidPlatformUtil implements IPlatformUtil {
@Nullable
private final Activity mActivity;

/**
* List of System Browsers which can be used from broker, currently only Chrome is supported.
* This information here is populated from the default browser safe-list in MSAL.
*
* @return
*/
@Override
public List<BrowserDescriptor> getBrowserSafeListForBroker() {
List<BrowserDescriptor> browserDescriptors = new ArrayList<>();
final HashSet<String> signatureHashes = new HashSet<String>();
signatureHashes.add("7fmduHKTdHHrlMvldlEqAIlSfii1tl35bxj1OXN5Ve8c4lU6URVu4xtSHc3BVZxS6WWJnxMDhIfQN0N0K2NDJg==");
final BrowserDescriptor chrome = new BrowserDescriptor(
"com.android.chrome",
signatureHashes,
null,
null
);
browserDescriptors.add(chrome);

return browserDescriptors;
}

@Nullable
@Override
public String getInstalledCompanyPortalVersion() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationResult;
import com.microsoft.identity.common.java.providers.oauth2.AuthorizationResult;
import com.microsoft.identity.common.java.ui.BrowserDescriptor;
import com.microsoft.identity.common.java.util.BrokerProtocolVersionUtil;
import com.microsoft.identity.common.java.util.ObjectMapper;
import com.microsoft.identity.common.java.authorities.AzureActiveDirectoryAuthority;
Expand All @@ -77,9 +76,6 @@
import com.microsoft.identity.common.logging.Logger;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;

public class MsalBrokerRequestAdapter implements IBrokerRequestAdapter {

Expand Down Expand Up @@ -490,27 +486,6 @@ private boolean getMultipleCloudsSupported(@NonNull final TokenCommandParameters
}
}

/**
* List of System Browsers which can be used from broker, currently only Chrome is supported.
* This information here is populated from the default browser safelist in MSAL.
*
* @return
*/
public static List<BrowserDescriptor> getBrowserSafeListForBroker() {
List<BrowserDescriptor> browserDescriptors = new ArrayList<>();
final HashSet<String> signatureHashes = new HashSet<String>();
signatureHashes.add("7fmduHKTdHHrlMvldlEqAIlSfii1tl35bxj1OXN5Ve8c4lU6URVu4xtSHc3BVZxS6WWJnxMDhIfQN0N0K2NDJg==");
final BrowserDescriptor chrome = new BrowserDescriptor(
"com.android.chrome",
signatureHashes,
null,
null
);
browserDescriptors.add(chrome);

return browserDescriptors;
}

/**
* adds required broker protocol version key in request bundle if not null.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,97 +26,104 @@
import android.content.Context;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.fragment.app.Fragment;

import com.microsoft.identity.common.internal.ui.browser.Browser;
import com.microsoft.identity.common.java.browser.Browser;
import com.microsoft.identity.common.internal.ui.browser.DefaultBrowserAuthorizationStrategy;
import com.microsoft.identity.common.java.WarningType;
import com.microsoft.identity.common.java.exception.ClientException;
import com.microsoft.identity.common.java.exception.ErrorStrings;
import com.microsoft.identity.common.java.commands.parameters.BrokerInteractiveTokenCommandParameters;
import com.microsoft.identity.common.java.commands.parameters.InteractiveTokenCommandParameters;
import com.microsoft.identity.common.java.browser.IBrowserSelector;
import com.microsoft.identity.common.java.configuration.LibraryConfiguration;
import com.microsoft.identity.common.java.providers.oauth2.IAuthorizationStrategy;
import com.microsoft.identity.common.internal.ui.browser.BrowserSelector;
import com.microsoft.identity.common.internal.ui.webview.EmbeddedWebViewAuthorizationStrategy;
import com.microsoft.identity.common.java.ui.AuthorizationAgent;
import com.microsoft.identity.common.java.ui.BrowserDescriptor;
import com.microsoft.identity.common.logging.Logger;
import com.microsoft.identity.common.java.strategies.IAuthorizationStrategyFactory;


import java.util.List;

import edu.umd.cs.findbugs.annotations.Nullable;
import lombok.Builder;
import lombok.experimental.Accessors;

// Suppressing rawtype warnings due to the generic types AuthorizationStrategy, AuthorizationStrategyFactory, EmbeddedWebViewAuthorizationStrategy and BrowserAuthorizationStrategy
@SuppressWarnings(WarningType.rawtype_warning)
@Builder
@Accessors(prefix = "m")
public class AndroidAuthorizationStrategyFactory implements IAuthorizationStrategyFactory{
public class AndroidAuthorizationStrategyFactory implements IAuthorizationStrategyFactory<IAuthorizationStrategy> {
private static final String TAG = AndroidAuthorizationStrategyFactory.class.getSimpleName();

private final Context mContext;
private final Activity mActivity;
private final Fragment mFragment;
private final IBrowserSelector mBrowserSelector;

/**
* Get the authorization strategy based on the authorization agent and browser.
* If the authorization agent is WEBVIEW or the browser is null,
* return the embedded web view authorization strategy.
* Otherwise, return the browser authorization strategy.
*
* @param authorizationAgent The authorization agent provided by the caller.
* @param browserSafeList The browser safe list provided by the caller.
* @param preferredBrowserDescriptor The preferred browser descriptor provided by the caller.
* @param isBrokerRequest True if the request is from broker.
* @return The authorization strategy.
*/
@Override
@NonNull
public IAuthorizationStrategy getAuthorizationStrategy(
@NonNull final InteractiveTokenCommandParameters parameters) {
@NonNull final AuthorizationAgent authorizationAgent,
@NonNull final List<BrowserDescriptor> browserSafeList,
@Nullable final BrowserDescriptor preferredBrowserDescriptor,
final boolean isBrokerRequest) {
final String methodTag = TAG + ":getAuthorizationStrategy";
//Valid if available browser installed. Will fallback to embedded webView if no browser available.

if (parameters.getAuthorizationAgent() == AuthorizationAgent.WEBVIEW) {
Logger.info(methodTag, "Use webView for authorization.");
return getGenericAuthorizationStrategy();
}

try {
final Browser browser = BrowserSelector.select(
mContext,
parameters.getBrowserSafeList(),
parameters.getPreferredBrowser());
final Browser browser = mBrowserSelector.selectBrowser(browserSafeList, preferredBrowserDescriptor);

Logger.info(methodTag, "Use browser for authorization.");
return getBrowserAuthorizationStrategy(
browser,
(parameters instanceof BrokerInteractiveTokenCommandParameters));
} catch (final ClientException e) {
Logger.info(methodTag, "Unable to use browser to do the authorization because "
+ ErrorStrings.NO_AVAILABLE_BROWSER_FOUND + " Use embedded webView instead.");
if (authorizationAgent == AuthorizationAgent.WEBVIEW || browser == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If someone passes authorization agent as BROWSER but passes null browser, then what strategy is picked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

embedded web view authorization strategy

Add Javadoc:

  • Get the authorization strategy based on the authorization agent and browser.
  • If the authorization agent is WEBVIEW or the browser is null,
  • return the embedded web view authorization strategy.
  • Otherwise, return the browser authorization strategy.

Logger.info(methodTag, "WebView authorization, browser: " + browser);
return getGenericAuthorizationStrategy();
}

Logger.info(methodTag, "Browser authorization, browser: " + browser);
return getBrowserAuthorizationStrategy(browser, isBrokerRequest);
}

/**
* Get current task browser authorization strategy or default browser authorization strategy.
* If the authorization is in current task, use current task browser authorization strategy.
*
* @param browser The browser to use for authorization.
* @param isBrokerRequest True if the request is from broker.
* @return The browser authorization strategy.
*/
private IAuthorizationStrategy getBrowserAuthorizationStrategy(@NonNull final Browser browser,
final boolean isBrokerRequest) {
if (LibraryConfiguration.getInstance().isAuthorizationInCurrentTask()) {
final CurrentTaskBrowserAuthorizationStrategy currentTaskBrowserAuthorizationStrategy =
new CurrentTaskBrowserAuthorizationStrategy(
mContext,
mActivity,
mFragment);
currentTaskBrowserAuthorizationStrategy.setBrowser(browser);
return currentTaskBrowserAuthorizationStrategy;
return new CurrentTaskBrowserAuthorizationStrategy(
mContext,
mActivity,
mFragment,
browser);
} else {
final DefaultBrowserAuthorizationStrategy defaultBrowserAuthorizationStrategy = new DefaultBrowserAuthorizationStrategy(
return new DefaultBrowserAuthorizationStrategy(
mContext,
mActivity,
mFragment,
isBrokerRequest
isBrokerRequest,
browser
);
defaultBrowserAuthorizationStrategy.setBrowser(browser);
return defaultBrowserAuthorizationStrategy;
}
}

// Suppressing unchecked warnings due to casting of EmbeddedWebViewAuthorizationStrategy to GenericAuthorizationStrategy
@SuppressWarnings(WarningType.unchecked_warning)
/**
* Get the generic authorization strategy.
*
* @return The embedded web view authorization strategy.
*/
private IAuthorizationStrategy getGenericAuthorizationStrategy() {
return new EmbeddedWebViewAuthorizationStrategy(
mContext,
mActivity,
mFragment);
return new EmbeddedWebViewAuthorizationStrategy(mContext, mActivity, mFragment);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import androidx.annotation.Nullable;
import androidx.fragment.app.Fragment;

import com.microsoft.identity.common.java.browser.Browser;
import com.microsoft.identity.common.internal.ui.browser.BrowserAuthorizationStrategy;
import com.microsoft.identity.common.java.WarningType;
import com.microsoft.identity.common.java.providers.oauth2.AuthorizationRequest;
Expand All @@ -41,10 +42,11 @@ public class CurrentTaskBrowserAuthorizationStrategy<
GenericOAuth2Strategy extends OAuth2Strategy,
GenericAuthorizationRequest extends AuthorizationRequest>
extends BrowserAuthorizationStrategy<GenericOAuth2Strategy, GenericAuthorizationRequest> {
public CurrentTaskBrowserAuthorizationStrategy(@NonNull Context applicationContext,
@NonNull Activity activity,
@Nullable Fragment fragment) {
super(applicationContext, activity, fragment);
public CurrentTaskBrowserAuthorizationStrategy(@NonNull final Context applicationContext,
@NonNull final Activity activity,
@Nullable final Fragment fragment,
@NonNull final Browser browser) {
super(applicationContext, activity, fragment, browser);
}

@Override
Expand Down
Loading
Loading