-
Notifications
You must be signed in to change notification settings - Fork 36
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
Changes from 17 commits
8a5a96e
003fe74
98deb1c
c9b5f83
755d9da
4691ad6
1e624de
33740e2
859d677
5c22a55
db8b608
40727d9
6aabd68
fdabc57
fbaa650
0b897ab
08b25d1
1ff3cf7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. embedded web view authorization strategy Add Javadoc:
|
||
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); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you still need here? If so, can we set same object on both and not create new ?
There was a problem hiding this comment.
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