Merge pull request #14949 from peppy/login-error-display

Show login failure messages on login form

osu.Game.Tests/Visual/Menus/TestSceneLoginPanel.cs

@@ -6,6 +6,7 @@ using NUnit.Framework;
 using osu.Framework.Graphics;
 using osu.Framework.Testing;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Online.API;
 using osu.Game.Overlays.Login;
 
 namespace osu.Game.Tests.Visual.Menus
@@ -30,12 +31,25 @@ namespace osu.Game.Tests.Visual.Menus
         }
 
         [Test]
-        public void TestBasicLogin()
+        public void TestLoginSuccess()
         {
             AddStep("logout", () => API.Logout());
 
             AddStep("enter password", () => loginPanel.ChildrenOfType<OsuPasswordTextBox>().First().Text = "password");
             AddStep("submit", () => loginPanel.ChildrenOfType<OsuButton>().First(b => b.Text.ToString() == "Sign in").TriggerClick());
         }
+
+        [Test]
+        public void TestLoginFailure()
+        {
+            AddStep("logout", () =>
+            {
+                API.Logout();
+                ((DummyAPIAccess)API).FailNextLogin();
+            });
+
+            AddStep("enter password", () => loginPanel.ChildrenOfType<OsuPasswordTextBox>().First().Text = "password");
+            AddStep("submit", () => loginPanel.ChildrenOfType<OsuButton>().First(b => b.Text.ToString() == "Sign in").TriggerClick());
+        }
     }
 }

osu.Game/Graphics/ErrorTextFlowContainer.cs

@@ -6,7 +6,7 @@ using osu.Framework.Graphics;
 using osu.Game.Graphics.Containers;
 using osuTK.Graphics;
 
-namespace osu.Game.Overlays.AccountCreation
+namespace osu.Game.Graphics
 {
     public class ErrorTextFlowContainer : OsuTextFlowContainer
     {

osu.Game/Online/API/APIAccess.cs

@@ -35,9 +35,8 @@ namespace osu.Game.Online.API
 
         public string WebsiteRootUrl { get; }
 
-        /// <summary>
+        public Exception LastLoginError { get; private set; }
-        /// The username/email provided by the user when initiating a login.
+
-        /// </summary>
         public string ProvidedUsername { get; private set; }
 
         private string password;
@@ -136,15 +135,24 @@ namespace osu.Game.Online.API
                         // save the username at this point, if the user requested for it to be.
                         config.SetValue(OsuSetting.Username, config.Get<bool>(OsuSetting.SaveUsername) ? ProvidedUsername : string.Empty);
 
-                        if (!authentication.HasValidAccessToken && !authentication.AuthenticateWithLogin(ProvidedUsername, password))
+                        if (!authentication.HasValidAccessToken)
+                        {
+                            LastLoginError = null;
+
+                            try
+                            {
+                                authentication.AuthenticateWithLogin(ProvidedUsername, password);
+                            }
+                            catch (Exception e)
                             {
                                 //todo: this fails even on network-related issues. we should probably handle those differently.
-                            //NotificationOverlay.ShowMessage("Login failed!");
+                                LastLoginError = e;
                                 log.Add(@"Login failed!");
                                 password = null;
                                 authentication.Clear();
                                 continue;
                             }
+                        }
 
                         var userReq = new GetUserRequest();
 

osu.Game/Online/API/DummyAPIAccess.cs

@@ -32,6 +32,8 @@ namespace osu.Game.Online.API
 
         public string WebsiteRootUrl => "http://localhost";
 
+        public Exception LastLoginError { get; private set; }
+
         /// <summary>
         /// Provide handling logic for an arbitrary API request.
         /// Should return true is a request was handled. If null or false return, the request will be failed with a <see cref="NotSupportedException"/>.
@@ -40,6 +42,8 @@ namespace osu.Game.Online.API
 
         private readonly Bindable<APIState> state = new Bindable<APIState>(APIState.Online);
 
+        private bool shouldFailNextLogin;
+
         /// <summary>
         /// The current connectivity state of the API.
         /// </summary>
@@ -74,6 +78,18 @@ namespace osu.Game.Online.API
 
         public void Login(string username, string password)
         {
+            state.Value = APIState.Connecting;
+
+            if (shouldFailNextLogin)
+            {
+                LastLoginError = new APIException("Not powerful enough to login.", new ArgumentException(nameof(shouldFailNextLogin)));
+
+                state.Value = APIState.Offline;
+                shouldFailNextLogin = false;
+                return;
+            }
+
+            LastLoginError = null;
             LocalUser.Value = new User
             {
                 Username = username,
@@ -102,5 +118,7 @@ namespace osu.Game.Online.API
         IBindable<User> IAPIProvider.LocalUser => LocalUser;
         IBindableList<User> IAPIProvider.Friends => Friends;
         IBindable<UserActivity> IAPIProvider.Activity => Activity;
+
+        public void FailNextLogin() => shouldFailNextLogin = true;
     }
 }

osu.Game/Online/API/IAPIProvider.cs

@@ -3,6 +3,7 @@
 
 #nullable enable
 
+using System;
 using System.Threading.Tasks;
 using osu.Framework.Bindables;
 using osu.Game.Users;
@@ -55,6 +56,11 @@ namespace osu.Game.Online.API
         /// </summary>
         string WebsiteRootUrl { get; }
 
+        /// <summary>
+        /// The last login error that occurred, if any.
+        /// </summary>
+        Exception? LastLoginError { get; }
+
         /// <summary>
         /// The current connection state of the API.
         /// This is not thread-safe and should be scheduled locally if consumed from a drawable component.

osu.Game/Online/API/OAuth.cs

@@ -1,8 +1,10 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using System.Diagnostics;
 using System.Net.Http;
+using Newtonsoft.Json;
 using osu.Framework.Bindables;
 
 namespace osu.Game.Online.API
@@ -32,10 +34,10 @@ namespace osu.Game.Online.API
             this.endpoint = endpoint;
         }
 
-        internal bool AuthenticateWithLogin(string username, string password)
+        internal void AuthenticateWithLogin(string username, string password)
         {
-            if (string.IsNullOrEmpty(username)) return false;
+            if (string.IsNullOrEmpty(username)) throw new ArgumentException("Missing username.");
-            if (string.IsNullOrEmpty(password)) return false;
+            if (string.IsNullOrEmpty(password)) throw new ArgumentException("Missing password.");
 
             using (var req = new AccessTokenRequestPassword(username, password)
             {
@@ -49,13 +51,27 @@ namespace osu.Game.Online.API
                 {
                     req.Perform();
                 }
+                catch (Exception ex)
+                {
+                    Token.Value = null;
+
+                    var throwableException = ex;
+
+                    try
+                    {
+                        // attempt to decode a displayable error string.
+                        var error = JsonConvert.DeserializeObject<OAuthError>(req.GetResponseString() ?? string.Empty);
+                        if (error != null)
+                            throwableException = new APIException(error.UserDisplayableError, ex);
+                    }
                     catch
                     {
-                    return false;
+                    }
+
+                    throw throwableException;
                 }
 
                 Token.Value = req.ResponseObject;
-                return true;
             }
         }
 
@@ -182,5 +198,19 @@ namespace osu.Game.Online.API
                 base.PrePerform();
             }
         }
+
+        private class OAuthError
+        {
+            public string UserDisplayableError => !string.IsNullOrEmpty(Hint) ? Hint : ErrorIdentifier;
+
+            [JsonProperty("error")]
+            public string ErrorIdentifier { get; set; }
+
+            [JsonProperty("hint")]
+            public string Hint { get; set; }
+
+            [JsonProperty("message")]
+            public string Message { get; set; }
+        }
     }
 }

osu.Game/Overlays/Login/LoginForm.cs

@@ -8,6 +8,7 @@ using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Input.Events;
 using osu.Game.Configuration;
+using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Online.API;
@@ -42,6 +43,9 @@ namespace osu.Game.Overlays.Login
             Spacing = new Vector2(0, 5);
             AutoSizeAxes = Axes.Y;
             RelativeSizeAxes = Axes.X;
+
+            ErrorTextFlowContainer errorText;
+
             Children = new Drawable[]
             {
                 username = new OsuTextBox
@@ -57,6 +61,11 @@ namespace osu.Game.Overlays.Login
                     RelativeSizeAxes = Axes.X,
                     TabbableContentContainer = this,
                 },
+                errorText = new ErrorTextFlowContainer
+                {
+                    RelativeSizeAxes = Axes.X,
+                    AutoSizeAxes = Axes.Y,
+                },
                 new SettingsCheckbox
                 {
                     LabelText = "Remember username",
@@ -97,6 +106,9 @@ namespace osu.Game.Overlays.Login
             };
 
             password.OnCommit += (sender, newText) => performLogin();
+
+            if (api?.LastLoginError?.Message is string error)
+                errorText.AddErrors(new[] { error });
         }
 
         public override bool AcceptsFocus => true;