Skip to content

Petterh/issue 726: Do not treat web view errors as "cancel" #1018

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

Merged
merged 18 commits into from
Sep 6, 2019

Conversation

petterh
Copy link
Contributor

@petterh petterh commented Oct 27, 2017

This fixes issue 726: Web view errors treated as cancel.

This change properly propagates server-side authentication errors to client instead of pretending the user cancelled the flow.

Other minor fixes include removal of trailing white space, fixing the occasional typo and protecting against a possible NRE in Discovery.

Petter Hesselberg added 6 commits October 27, 2017 13:40
Added input type to email address field in sample app
of pretending the user cancelled the flow

Fixed some typos
Reduced visibility of classes and methods
A bit less trailing white space
# Conflicts:
#	adal/src/main/java/com/microsoft/aad/adal/PromptBehavior.java
# Conflicts:
#	adal/src/main/java/com/microsoft/aad/adal/AuthenticationDialog.java
#	adal/src/main/java/com/microsoft/aad/adal/Discovery.java
#	sample/src/main/res/layout/activity_main.xml
Petter Hesselberg and others added 4 commits November 17, 2017 11:40
# Conflicts:
#	adal/src/main/java/com/microsoft/aad/adal/AuthenticationActivity.java
#	adal/src/main/java/com/microsoft/aad/adal/AuthenticationConstants.java
#	adal/src/main/java/com/microsoft/aad/adal/AuthenticationDialog.java
#	adal/src/main/java/com/microsoft/aad/adal/BasicWebViewClient.java
Petter Hesselberg added 3 commits January 14, 2019 14:12
…activedirectory-library-for-android into petterh/issue-726

# Conflicts:
#	adal/src/main/java/com/microsoft/aad/adal/AuthenticationDialog.java
@petterh
Copy link
Contributor Author

petterh commented Jan 14, 2019

@iambmelt Any chance of anyone reviewing this? It's been over a year since I created the PR, but the bug is still there.

@iambmelt
Copy link
Contributor

Hi @petterh -- thanks for updating your PR -- personally, I'd really like to see this merge but am heads-down on some other work before I can dedicate cycles to this. This item has recently moved up visibility-wise and will be prioritized in an upcoming planning meeting.

@iambmelt iambmelt removed the request for review from amishra-dev January 16, 2019 00:05
Petter Hesselberg added 2 commits May 7, 2019 13:48
# Conflicts:
#	adal/build.gradle
#	common
# Conflicts:
#	adal/build.gradle
Petter Hesselberg added 3 commits September 6, 2019 08:53
# Conflicts:
#	adal/src/androidTest/java/com/microsoft/aad/adal/BasicWebViewClientTests.java
#	adal/src/main/java/com/microsoft/aad/adal/AuthenticationActivity.java
#	adal/src/main/java/com/microsoft/aad/adal/BasicWebViewClient.java
@petterh
Copy link
Contributor Author

petterh commented Sep 6, 2019

@iambmelt @heidijinxujia Any chance of this ever getting in? It's been hanging for over 22 months now.

@iambmelt iambmelt requested review from kreedula and removed request for heidijinxujia September 6, 2019 20:42
@iambmelt iambmelt requested a review from rpdome September 6, 2019 20:42
Copy link
Contributor

@iambmelt iambmelt left a comment

Choose a reason for hiding this comment

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

Thanks for your patience, @petterh -- I'm approving this PR and sending it over to @kreedula and @rpdome for a second look. Assuming they don't have any concerns we'll merge this in and roll it out in an upcoming release

Copy link
Contributor

@kreedula kreedula left a comment

Choose a reason for hiding this comment

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

LGTM

@iambmelt iambmelt merged commit 91e392a into AzureAD:dev Sep 6, 2019
@iambmelt
Copy link
Contributor

iambmelt commented Sep 6, 2019

@petterh Merged - thanks for the nudge :-)

@petterh petterh deleted the petterh/issue-726 branch September 9, 2019 12:55
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.

WebView errors treated as cancel
3 participants