Skip to content

Commit 4126dcb

Browse files
committed
Fix 2FA verification via link not working correctly
Closes ppy#26835. I must have not re-tested this correctly after all the refactors... Basically the issue is that the websocket connection would only come online when the API state changed to full `Online`. In particular the connector would not attempt to connect when the API state was `RequiresSecondFactorAuth`, giving the link-based flow no chance to actually work. The change in `WebSocketNotificationsClientConnector` is relevant in that queueing requests does nothing before the API state changes to full `Online`. It also cleans up things a bit code-wise so... win? And yes, this means that the _other_ `PersistentEndpointClientConnector` implementations (i.e. SignalR connectors) will also come online earlier after this. Based on previous discussions (ppy#25480 (comment)) I think this is fine, but if it is _not_ fine, then it can be fixed by exposing a virtual that lets a connector to decide when to come alive, I guess.
1 parent 6931af6 commit 4126dcb

File tree

2 files changed

+6
-8
lines changed

2 files changed

+6
-8
lines changed

osu.Game/Online/Notifications/WebSocket/WebSocketNotificationsClientConnector.cs

+4-7
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,11 @@ public WebSocketNotificationsClientConnector(IAPIProvider api)
2929

3030
protected override async Task<PersistentEndpointClient> BuildConnectionAsync(CancellationToken cancellationToken)
3131
{
32-
var tcs = new TaskCompletionSource<string>();
33-
3432
var req = new GetNotificationsRequest();
35-
req.Success += bundle => tcs.SetResult(bundle.Endpoint);
36-
req.Failure += ex => tcs.SetException(ex);
37-
api.Queue(req);
38-
39-
string endpoint = await tcs.Task.ConfigureAwait(false);
33+
// must use `PerformAsync()`, since we may not be fully online yet
34+
// (see `APIState.RequiresSecondFactorAuth` - in this state queued requests will not execute).
35+
await api.PerformAsync(req).ConfigureAwait(false);
36+
string endpoint = req.Response!.Endpoint;
4037

4138
ClientWebSocket socket = new ClientWebSocket();
4239
socket.Options.SetRequestHeader(@"Authorization", @$"Bearer {api.AccessToken}");

osu.Game/Online/PersistentEndpointClientConnector.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ private async Task connectIfPossible()
6969
break;
7070

7171
case APIState.Online:
72+
case APIState.RequiresSecondFactorAuth:
7273
await connect().ConfigureAwait(true);
7374
break;
7475
}
@@ -83,7 +84,7 @@ private async Task connect()
8384

8485
try
8586
{
86-
while (apiState.Value == APIState.Online)
87+
while (apiState.Value == APIState.RequiresSecondFactorAuth || apiState.Value == APIState.Online)
8788
{
8889
// ensure any previous connection was disposed.
8990
// this will also create a new cancellation token source.

0 commit comments

Comments
 (0)