From be0d6fa559b2c3af8f1ff51af8f4c8602fe2a03d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 10 Jun 2026 11:29:25 +0200 Subject: [PATCH] Fix email verification code step of account registration only working if done correctly on the first try (#38032) Closes https://github.com/ppy/osu/issues/37988 ## [Fix registration detail entry screen pushing verification screen when not current](https://github.com/ppy/osu/commit/7a11e4e207f3d1c1a67222f447368368103b6329) Fixes the immediate crash described in https://github.com/ppy/osu/issues/37988. ## [Fix second factor authentication form being single-use](https://github.com/ppy/osu/commit/120eb2c4f8c5929bad4b11cf5746a4456d0b3fe6) `SecondFactorAuthForm` assumes that it has to do nothing after the code is entered, something else higher up will handle the actual error if the code is wrong. This was *mostly* passable in the context of the login overlay but doesn't really work in the registration flow. To that end, enable and clear the code text box, and show the last error, if verification fails the first time round. --------- Co-authored-by: Dean Herbert --- .../Online/TestSceneAccountCreationOverlay.cs | 63 ++++++++++++++++++- .../Overlays/AccountCreation/ScreenEntry.cs | 2 +- .../Overlays/Login/SecondFactorAuthForm.cs | 34 +++++++++- 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs index b9d7312233..58bba28d87 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs @@ -4,14 +4,17 @@ #nullable disable using System.Linq; +using System.Net; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.UserInterface; using osu.Framework.Testing; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; +using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays; using osu.Game.Overlays.AccountCreation; @@ -94,8 +97,66 @@ namespace osu.Game.Tests.Visual.Online .ChildrenOfType().Single().TriggerClick()); AddUntilStep("verification screen is present", () => accountCreation.ChildrenOfType().SingleOrDefault()?.IsPresent == true); - AddStep("verify", () => ((DummyAPIAccess)API).AuthenticateSecondFactor("abcdefgh")); + AddStep("verify", () => + accountCreation.ChildrenOfType().Single().ChildrenOfType().Single().Current.Value = "abcdefgh"); AddUntilStep("overlay is hidden", () => accountCreation.State.Value == Visibility.Hidden); } + + [Test] + public void TestFailedVerification() + { + AddStep("set up API", () => ((DummyAPIAccess)API).HandleRequest = req => + { + switch (req) + { + case VerifySessionRequest verifySessionRequest: + if (verifySessionRequest.VerificationKey == "abcdefgh") + verifySessionRequest.TriggerSuccess(); + else + { + Scheduler.AddDelayed( + () => verifySessionRequest.TriggerFailure(new APIException("Incorrect verification code.", null, HttpStatusCode.UnprocessableEntity)), + 1000); + } + + return true; + + default: + return false; + } + }); + + AddStep("log out", () => API.Logout()); + + AddStep("show manually", () => accountCreation.Show()); + AddUntilStep("overlay is visible", () => accountCreation.State.Value, () => Is.EqualTo(Visibility.Visible)); + + AddStep("click button", () => accountCreation.ChildrenOfType().Single().TriggerClick()); + AddUntilStep("warning screen is present", () => accountCreation.ChildrenOfType().SingleOrDefault()?.IsPresent, () => Is.True); + + AddStep("proceed", () => accountCreation.ChildrenOfType().Single().TriggerClick()); + AddUntilStep("entry screen is present", () => accountCreation.ChildrenOfType().SingleOrDefault()?.IsPresent, () => Is.True); + + AddStep("input details", () => + { + var entryScreen = accountCreation.ChildrenOfType().Single(); + entryScreen.ChildrenOfType().ElementAt(0).Text = "new_user"; + entryScreen.ChildrenOfType().ElementAt(1).Text = "new.user@fake.mail"; + entryScreen.ChildrenOfType().ElementAt(2).Text = "password"; + }); + AddStep("click button", () => accountCreation.ChildrenOfType().Single() + .ChildrenOfType().Single().TriggerClick()); + AddUntilStep("verification screen is present", () => accountCreation.ChildrenOfType().SingleOrDefault()?.IsPresent, () => Is.True); + + AddStep("fail to verify", () => + accountCreation.ChildrenOfType().Single().ChildrenOfType().Single().Current.Value = "abcdefff"); + AddUntilStep("overlay is still visible", () => accountCreation.State.Value, () => Is.EqualTo(Visibility.Visible)); + AddUntilStep("text box back to enabled", + () => accountCreation.ChildrenOfType().Single().ChildrenOfType().Single().Current.Disabled, () => Is.False); + + AddStep("verify", () => + accountCreation.ChildrenOfType().Single().ChildrenOfType().Single().Current.Value = "abcdefgh"); + AddUntilStep("overlay is hidden", () => accountCreation.State.Value, () => Is.EqualTo(Visibility.Hidden)); + } } } diff --git a/osu.Game/Overlays/AccountCreation/ScreenEntry.cs b/osu.Game/Overlays/AccountCreation/ScreenEntry.cs index c801e9304f..f7f5e6728b 100644 --- a/osu.Game/Overlays/AccountCreation/ScreenEntry.cs +++ b/osu.Game/Overlays/AccountCreation/ScreenEntry.cs @@ -227,7 +227,7 @@ namespace osu.Game.Overlays.AccountCreation apiState.BindValueChanged(state => { - if (state.NewValue == APIState.RequiresSecondFactorAuth) + if (this.IsCurrentScreen() && state.NewValue == APIState.RequiresSecondFactorAuth) this.Push(new ScreenEmailVerification()); }); diff --git a/osu.Game/Overlays/Login/SecondFactorAuthForm.cs b/osu.Game/Overlays/Login/SecondFactorAuthForm.cs index 38025de1d9..b26cbcee5d 100644 --- a/osu.Game/Overlays/Login/SecondFactorAuthForm.cs +++ b/osu.Game/Overlays/Login/SecondFactorAuthForm.cs @@ -3,6 +3,7 @@ using System.Threading.Tasks; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input; @@ -28,6 +29,7 @@ namespace osu.Game.Overlays.Login private LoadingLayer loading = null!; private FillFlowContainer contentFlow = null!; private OsuTextBox codeTextBox = null!; + private readonly IBindable apiState = new Bindable(); [Resolved] private IAPIProvider api { get; set; } = null!; @@ -71,13 +73,43 @@ namespace osu.Game.Overlays.Login } }; + updateLastError(); + + showContent(api.SessionVerificationMethod!.Value); + apiState.BindTo(api.State); + } + + private void updateLastError() + { if (api.LastLoginError?.Message is string error) { errorText.Alpha = 1; errorText.AddErrors(new[] { error }); } + } - showContent(api.SessionVerificationMethod!.Value); + protected override void LoadComplete() + { + base.LoadComplete(); + + apiState.BindValueChanged(val => + { + // this handles failed verifications. + // in the case of failed verifications, `apiState` will briefly change to `Connecting` and then revert to `RequiresSecondFactorAuth`. + // the login overlay doesn't need this logic as it will construct a new instance of this screen anyway, + // but the *registration* overlay has no such logic and thus needs special handling. + if (val.NewValue == APIState.RequiresSecondFactorAuth) + { + // scheduling required as `APIAccess.State` value can be changed from threads that aren't update + // see: `APIAccess.run()` (which is given a dedicated thread) calls `APIAccess.attemptConnect()` which mutates `APIAccess.State` + Schedule(() => + { + codeTextBox.Current.Disabled = false; + codeTextBox.Current.Value = string.Empty; + updateLastError(); + }); + } + }); } private void showContent(SessionVerificationMethod sessionVerificationMethod)