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

Switch to browser on DUNA redirect URL., Fixes AB#3079799 #2553

Open
wants to merge 71 commits into
base: dev
Choose a base branch
from

Conversation

p3dr0rv
Copy link
Contributor

@p3dr0rv p3dr0rv commented Dec 5, 2024

In this PR we made the necessary changes to allow the transition from WebView to Browser when a DUNA redirect URL is present. Steps 8 and 9: DUNA flow

At this point ESTS on the WebView will invoke a broker redirect url (msauth://Microsoft.AAD.BrokerPlugin?code=switchBrowserSessionToken&action=switch_browser&action_uri=login.microsoftonline.com/switchbrowser/process)
that will trigger AzureActiveDirectoryWebViewClient to handle the DUNA redirect URL and launch the browser to the switchbrowser/process endpoint

image

How it works:

If chrome or edge is installed and the SwitchToBrowser client is enabled, then we can declare ourselves DUNA capable. see: broker related PR

If ESTS calls a broker redirect URL for DUNA AzureActiveDirectoryWebViewClient will handle the DUNA redirect URL and use EmbeddedWebViewAuthorizationStrategy to launch the browser to the switchbrowser/process endpoint. (this PR)

Main Changes:

  • BrowserSelector is now part of the PlatformComponents
  • The AndroidAuthorizationStrategyFactory signature changed:
    • Before: getAuthorizationStrategy(InteractiveTokenCommandParameters parameters);
    • After: getAuthorizationStrategy(AuthorizationAgent authorizationAgent, Browser browser, boolean isBrokerRequest);
    • The AndroidAuthorizationStrategyFactory
  • Add unit tests for validation.

AB#3079799

Copy link

github-actions bot commented Dec 5, 2024

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

@p3dr0rv p3dr0rv changed the title [WIP] Switch to browser on DUNA redirect URL. Switch to browser on DUNA redirect URL. Dec 7, 2024
Copy link

github-actions bot commented Dec 7, 2024

✅ Work item link check complete. Description contains link AB#3079799 to an Azure Boards work item.

@github-actions github-actions bot changed the title Switch to browser on DUNA redirect URL. Switch to browser on DUNA redirect URL., Fixes AB#3079799 Dec 7, 2024
@p3dr0rv p3dr0rv marked this pull request as ready for review December 7, 2024 00:55
@p3dr0rv p3dr0rv requested a review from a team as a code owner December 7, 2024 00:55
…al/AuthenticationConstants.java

Co-authored-by: Shahzaib <[email protected]>
if (throwable instanceof IErrorInformation) {
errorCode = ((IErrorInformation) throwable).getErrorCode();
} else {
errorCode = MALFORMED_URL;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we ever get here then is it guaranteed to always be a case of malformed url? We are catching generic throwable so I'm not sure if that's always the case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing error code

BrowserDescriptor.getBrowserSafeListForSwitchBrowser(),
null
)
if (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.

Is this a case of browser being uninstalled between the time interactive request started and got to this point? I'm asking because I see we already check for browser being available even before we declare switch browser capable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, at this point we expect the browser not to be null as we check prior to initiate DUNA, but because the function can return null, we do this check

val exception = Assert.assertThrows(ClientException::class.java) {
SwitchBrowserUriHelper.buildProcessUri(redirectUri)
}
Assert.assertEquals(ClientException.MALFORMED_URL, exception.errorCode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also assert the error message

…/webview/challengehandlers/SwitchBrowserUriHelper.kt

Co-authored-by: Shahzaib <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants