xamarin / Xamarin.Auth

Xamarin.Auth
Apache License 2.0
545 stars 351 forks source link

Cancelling Twitter login causes 401 and crashes Xamarin App #323

Open tipa opened 5 years ago

tipa commented 5 years ago

Xamarin.Auth Issue

Version

  1. Trigger above code
  2. On Twitter screen click 'Cancel'
  3. Then click 'Return to ' button

Platform:

Expected behaviour

Exception should not be thrown. If instead of hitting Cancel you use phone back button it fires an Error event which is picked up in OnAuthError(). I think it should do the same.

Actual behaviour

Exception raised, see Stack Trace below.

06-26 15:10:42.156 I/MonoDroid( 6789): UNHANDLED EXCEPTION:
06-26 15:10:42.177 I/MonoDroid( 6789): System.AggregateException: One or more errors occurred. ---> System.Net.WebException: The remote server returned an error: (401) Authorization Required.
06-26 15:10:42.177 I/MonoDroid( 6789):   at System.Net.HttpWebRequest.EndGetResponse (System.IAsyncResult asyncResult) [0x00052] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/System/System.Net/HttpWebRequest.cs:1031 
06-26 15:10:42.177 I/MonoDroid( 6789):   at System.Threading.Tasks.TaskFactory`1[TResult].FromAsyncCoreLogic (System.IAsyncResult iar, System.Func`2[T,TResult] endFunction, System.Action`1[T] endAction, System.Threading.Tasks.Task`1[TResult] promise, System.Boolean requiresSynchronization) [0x0000f] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/Tasks/FutureFactory.cs:550 
06-26 15:10:42.178 I/MonoDroid( 6789):    --- End of inner exception stack trace ---
06-26 15:10:42.178 I/MonoDroid( 6789):   at System.Threading.Tasks.Task.ThrowIfExceptional (System.Boolean includeTaskCanceledExceptions) [0x00011] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/Tasks/Task.cs:2164 
06-26 15:10:42.178 I/MonoDroid( 6789):   at System.Threading.Tasks.Task`1[TResult].GetResultCore (System.Boolean waitCompletionNotification) [0x0002b] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/Tasks/Future.cs:562 
06-26 15:10:42.178 I/MonoDroid( 6789):   at System.Threading.Tasks.Task`1[TResult].get_Result () [0x00000] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/Tasks/Future.cs:532 
06-26 15:10:42.178 I/MonoDroid( 6789):   at Xamarin.Auth.OAuth1Authenticator.GetAccessTokenAsync () [0x00071] in <7c8b52626aea4529928c6992480c7d50>:0 
06-26 15:10:42.178 I/MonoDroid( 6789):   at Xamarin.Auth.OAuth1Authenticator.OnPageLoaded (System.Uri url) [0x00050] in <7c8b52626aea4529928c6992480c7d50>:0 
06-26 15:10:42.178 I/MonoDroid( 6789):   at Xamarin.Auth.WebAuthenticatorActivity+Client.OnPageFinished (Android.Webkit.WebView view, System.String url) [0x00017] in <7c8b52626aea4529928c6992480c7d50>:0 
06-26 15:10:42.178 I/MonoDroid( 6789):   at Android.Webkit.WebViewClient.n_OnPageFinished_Landroid_webkit_WebView_Ljava_lang_String_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.IntPtr native_url) [0x00018] in /Users/builder/data/lanes/5945/342b2ce9/source/monodroid/external/xamarin-android/src/Mono.Android/obj/Release/android-27/mcw/Android.Webkit.WebViewClient.cs:202 
06-26 15:10:42.178 I/MonoDroid( 6789):   at (wrapper dynamic-method) System.Object.586ccde1-6287-4d21-bedc-c32695000fdb(intptr,intptr,intptr,intptr)
06-26 15:10:42.178 I/MonoDroid( 6789): ---> (Inner Exception #0) System.Net.WebException: The remote server returned an error: (401) Authorization Required.
06-26 15:10:42.178 I/MonoDroid( 6789):   at System.Net.HttpWebRequest.EndGetResponse (System.IAsyncResult asyncResult) [0x00052] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/System/System.Net/HttpWebRequest.cs:1031 
06-26 15:10:42.178 I/MonoDroid( 6789):   at System.Threading.Tasks.TaskFactory`1[TResult].FromAsyncCoreLogic (System.IAsyncResult iar, System.Func`2[T,TResult] endFunction, System.Action`1[T] endAction, System.Threading.Tasks.Task`1[TResult] promise, System.Boolean requiresSynchronization) [0x0000f] in /Users/builder/jenkins/workspace/xamarin-android-d15-7/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/Tasks/FutureFactory.cs:550 <---

VS bug #733072, VS bug #1266971

newky2k commented 5 years ago

@tipa could you please provide a small sample that demonstrates these issues so we can investigate please.

Regards

tipa commented 5 years ago

Some more code:

var auth = new OAuth1Authenticator(
    consumerKey: _oAuthInfo.ConsumerKey,
    consumerSecret: _oAuthInfo.ConsumerSecret,
    requestTokenUrl: new Uri("https://api.twitter.com/oauth/request_token"),
    authorizeUrl: new Uri("https://api.twitter.com/oauth/authorize"),
    accessTokenUrl: new Uri("https://api.twitter.com/oauth/access_token"),
    callbackUrl: new Uri("http://www.facebook.com/connect/login_success.html"));

activity.StartActivity(auth.GetUI(activity));

I understand the callback url is a bit weird, but it's at least one that correctly sends me back to the app after the user authorized the app

newky2k commented 5 years ago

@tipa thanks for the update, i have assigned @moljac to investigate

tipa commented 5 years ago

Thanks! Crash also happens for Xamarin.iOS btw

bombez commented 5 years ago

I'm facing the same error. There's an update on this? Thanks

DeanZhuo commented 3 years ago

it's 2020 and the same error occured. any fix?

TheObliterator commented 2 years ago

For anyone still encountering this problem today, I found a simple workaround is to inherit from OAuth1Authenticator and override the OnPageLoaded() method to catch the exception.

I choose to detect the specific error condition and surface this as a TaskCancelledException, but you could simply surface the 401 error directly if you prefer. The exception is then passed on to your auth.Error event callback as you would expect.

    /// <summary>
    /// A helper extension to Xamarin XAuth's OAuth1Authenticator to fix an issue with cancellation
    /// https://github.com/xamarin/Xamarin.Auth/issues/323
    /// </summary>
    public class OAuth1AuthenticatorEx : OAuth1Authenticator
    {
        public OAuth1AuthenticatorEx(string consumerKey, string consumerSecret, Uri requestTokenUrl, Uri authorizeUrl, Uri accessTokenUrl, Uri callbackUrl, GetUsernameAsyncFunc getUsernameAsync = null, bool isUsingNativeUI = false)
            : base(consumerKey, consumerSecret, requestTokenUrl, authorizeUrl, accessTokenUrl, callbackUrl, getUsernameAsync, isUsingNativeUI)
        {
        }

        /// <summary>
        /// An override to provide a workaround for OAuth1 throwing an uncaught (401) Authorization Required exception if the user cancels the login request
        /// https://github.com/xamarin/Xamarin.Auth/issues/323
        /// </summary>
        /// <param name="url">The url of the page loaded by OAuth</param>
        public override void OnPageLoaded(Uri url)
        {
            try
            {
                base.OnPageLoaded(url);
            }
            catch (Exception ex)
            {
                WebException wex = (WebException)ex.InnerException;
                if (wex != null)
                {
                    // Looks like the user cancelled the request?
                    HttpWebResponse response = (HttpWebResponse)wex.Response;
                    if (response != null && response.StatusCode == HttpStatusCode.Unauthorized)    
                    {
                        OnError(new TaskCanceledException("Authorisation Cancelled"));  // Or we could pass the inner exception directly
                        OnCancelled();  // Ensure the UI is closed
                        return;
                    }
                }

                OnError(ex);   // Some other error, just pass it along                 
            }
        }
    }

To use the fixed version, simply instantiate from the OAuth1AuthenticatorEx instead of the base OAuth1Authenticator. i.e.

var auth = new OAuth1AuthenticatorEx( consumerKey: _oAuthInfo.ConsumerKey, consumerSecret: _oAuthInfo.ConsumerSecret, ...