DUNA flow: Resume Auth , Fixes AB#3079802 #2557
Conversation
…D/microsoft-authentication-library-common-for-android into pedroro/switch-to-browser
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
|
✅ Work item link check complete. Description contains link AB#3079802 to an Azure Boards work item. |
|
❌ Work item link check failed. Description contains AB#3079802 Click here to learn more. |
github-actions
bot
changed the title
...va/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationActivityFactory.java
Show resolved
Hide resolved
...va/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java
Outdated
Show resolved
Hide resolved
...va/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java
Outdated
Show resolved
Hide resolved
| */ | ||
| private boolean isSwitchBrowserResumeFlow(@NonNull final Bundle extras) { | ||
| final String methodTag = TAG + ":isSwitchBrowserResumeFlow"; | ||
| final boolean containsActionUri = extras.containsKey( |
There was a problem hiding this comment.
Are we also checking the path to validate?
redirect_uri/switch_browser_resume
I think we should validate that.
If action_uri and code are missing while in a switch browser flow then that should be considered a bug.
There was a problem hiding this comment.
yes, we are validating the path on the BrokerBrowserRedirectActivity, reworked, so here if action_uri and code are missing and is a resume flow we consider it an error
| if (actionUri == null || code == null) { | ||
| Logger.error(methodTag, "Action URI is null: " + (actionUri == null) + | ||
| ", code is null: " + (code == null), null); | ||
| cancelAuthorization(false); |
There was a problem hiding this comment.
How is this error surfaced back to client? Does this come back as User cancel?
We need to send this back as cancel with some error information about the required fields being missing and also need to report this in telemetry
There was a problem hiding this comment.
Reworked and call sendResult directly to surface the exception to the client.
regarding telemetry I will have my final PR with the telemetry :)
.../com/microsoft/identity/common/internal/ui/webview/challengehandlers/SwitchBrowserHandler.kt
Outdated
Show resolved
Hide resolved
...om/microsoft/identity/common/internal/ui/webview/challengehandlers/SwitchBrowserUriHelper.kt
Outdated
Show resolved
Hide resolved
...icrosoft/identity/common/internal/ui/webview/challengehandlers/SwitchBrowserUriHelperTest.kt
Outdated
Show resolved
Hide resolved
| @@ -120,6 +126,8 @@ public class WebViewAuthorizationFragment extends AuthorizationFragment { | |||
|
|
|||
| // This is used by LegacyFido2ApiManager to launch a PendingIntent received by the legacy API. | |||
| private ActivityResultLauncher<LegacyFido2ApiObject> mFidoLauncher; | |||
| // This flag is used to determine if the fragment is waiting for the switch browser resume endpoint. | |||
There was a problem hiding this comment.
Doesn't seem like a flag. This is really an object
| return null; | ||
| } | ||
| ); | ||
| } catch (ClientException e) { |
There was a problem hiding this comment.
| } catch (ClientException e) { | |
| } catch (final ClientException e) { |
| * It contains the URI to be opened in the new browser. | ||
| */ | ||
| data class SwitchBrowserChallenge( |
There was a problem hiding this comment.
I kinda liked this name more
| } | ||
|
|
||
| fun unbind() { | ||
| /** | ||
| * Check if the request is to start the switch the browser flow. |
There was a problem hiding this comment.
| * Check if the request is to start the switch the browser flow. | |
| * Check if the request is to start the switch browser flow. |
This PR addresses step 16 of the Switch browser flow
BrokerBrowserRedirectActivity will re-start the WebViewAuthorizationFragment so we update the intent data and handle the activity being resumed on WebViewAuthorizationFragment.onResume
The switch browser resume endpoint requires the client ID, so we need to pass this value at the moment we start the WebAuthorizaFragment.
Refactor AuthorizationActivityFactory to work with one data parameter to create the auth intent.
AB#3079802