Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 032cc0f

Browse files
committed
Cancel OAuth listener if necessary.
Cancel OAuth listener when login dialog is closed, or the user switches between .com and enterprise while waiting for an OAuth callback.
1 parent 75e1041 commit 032cc0f

File tree

4 files changed

+66
-21
lines changed

4 files changed

+66
-21
lines changed

src/GitHub.App/Services/OAuthCallbackListener.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ public OAuthCallbackListener(IHttpListener httpListener)
4141

4242
public async Task<string> Listen(string id, CancellationToken cancel)
4343
{
44-
var internalCancel = new CancellationTokenSource();
45-
var combinedCancel = CancellationTokenSource.CreateLinkedTokenSource(cancel, internalCancel.Token);
46-
4744
if (httpListener.IsListening) httpListener.Stop();
4845
httpListener.Start();
4946

src/GitHub.App/ViewModels/LoginTabViewModel.cs

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Diagnostics.CodeAnalysis;
3+
using System.Net;
34
using System.Reactive;
45
using System.Reactive.Linq;
56
using System.Reactive.Threading.Tasks;
@@ -23,6 +24,7 @@ namespace GitHub.ViewModels
2324
public abstract class LoginTabViewModel : ReactiveObject
2425
{
2526
static readonly ILogger log = LogManager.ForContext<LoginTabViewModel>();
27+
CancellationTokenSource oauthCancel;
2628

2729
protected LoginTabViewModel(
2830
IConnectionManager connectionManager,
@@ -46,25 +48,12 @@ protected LoginTabViewModel(
4648
(x, y) => x.Value && y.Value).ToProperty(this, x => x.CanLogin);
4749

4850
Login = ReactiveCommand.CreateAsyncObservable(this.WhenAny(x => x.CanLogin, x => x.Value), LogIn);
49-
50-
Login.ThrownExceptions.Subscribe(ex =>
51-
{
52-
if (ex.IsCriticalException()) return;
53-
54-
log.Information(ex, "Error logging into '{BaseUri}' as '{UsernameOrEmail}'", BaseUri, UsernameOrEmail);
55-
if (ex is Octokit.ForbiddenException)
56-
{
57-
Error = new UserError(Resources.LoginFailedForbiddenMessage, ex.Message);
58-
}
59-
else
60-
{
61-
Error = new UserError(ex.Message);
62-
}
63-
});
51+
Login.ThrownExceptions.Subscribe(HandleError);
6452

6553
LoginViaOAuth = ReactiveCommand.CreateAsyncTask(
6654
this.WhenAnyValue(x => x.IsLoggingIn, x => !x),
6755
LogInViaOAuth);
56+
LoginViaOAuth.ThrownExceptions.Subscribe(HandleError);
6857

6958
isLoggingIn = Login.IsExecuting.ToProperty(this, x => x.IsLoggingIn);
7059

@@ -146,6 +135,8 @@ public UserError Error
146135
set { this.RaiseAndSetIfChanged(ref error, value); }
147136
}
148137

138+
public void Deactivated() => oauthCancel?.Cancel();
139+
149140
protected abstract IObservable<AuthenticationResult> LogIn(object args);
150141
protected abstract Task<AuthenticationResult> LogInViaOAuth(object args);
151142

@@ -200,13 +191,16 @@ protected IObservable<AuthenticationResult> LogInToHost(HostAddress hostAddress)
200191

201192
protected async Task LoginToHostViaOAuth(HostAddress address)
202193
{
194+
oauthCancel = new CancellationTokenSource();
195+
203196
try
204197
{
205-
await ConnectionManager.LogInViaOAuth(address, CancellationToken.None);
198+
await ConnectionManager.LogInViaOAuth(address, oauthCancel.Token);
206199
}
207-
catch (Exception e)
200+
finally
208201
{
209-
Error = new UserError(e.Message);
202+
oauthCancel.Dispose();
203+
oauthCancel = null;
210204
}
211205
}
212206

@@ -224,5 +218,32 @@ protected virtual Task ResetValidation()
224218
// noop
225219
return Task.FromResult(0);
226220
}
221+
222+
void HandleError(Exception ex)
223+
{
224+
// The Windows ERROR_OPERATION_ABORTED error code.
225+
const int operationAborted = 995;
226+
227+
if (ex is HttpListenerException &&
228+
((HttpListenerException)ex).ErrorCode == operationAborted)
229+
{
230+
// An Oauth listener was aborted, probably because the user closed the login
231+
// dialog or switched between the GitHub and Enterprise tabs while listening
232+
// for an Oauth callbacl.
233+
return;
234+
}
235+
236+
if (ex.IsCriticalException()) return;
237+
238+
log.Information(ex, "Error logging into '{BaseUri}' as '{UsernameOrEmail}'", BaseUri, UsernameOrEmail);
239+
if (ex is Octokit.ForbiddenException)
240+
{
241+
Error = new UserError(Resources.LoginFailedForbiddenMessage, ex.Message);
242+
}
243+
else
244+
{
245+
Error = new UserError(ex.Message);
246+
}
247+
}
227248
}
228249
}

src/GitHub.Exports.Reactive/ViewModels/ILoginToHostViewModel.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,5 +74,10 @@ public interface ILoginToHostViewModel
7474
/// Gets an error to display to the user.
7575
/// </summary>
7676
UserError Error { get; }
77+
78+
/// <summary>
79+
/// Called when the login UI is hidden or dismissed.
80+
/// </summary>
81+
void Deactivated();
7782
}
7883
}

src/GitHub.VisualStudio/UI/Views/Controls/LoginControl.xaml.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public LoginControl()
3232
SetupDotComBindings(d);
3333
SetupEnterpriseBindings(d);
3434
SetupSelectedAndVisibleTabBindings(d);
35+
d(Disposable.Create(Deactivate));
3536
});
3637

3738
IsVisibleChanged += (s, e) =>
@@ -45,6 +46,27 @@ public LoginControl()
4546
x => x.ViewModel.GitHubLogin.LoginViaOAuth,
4647
x => x.ViewModel.EnterpriseLogin.LoginViaOAuth)
4748
.Subscribe(_ => Application.Current.MainWindow?.Activate());
49+
50+
hostTabControl.SelectionChanged += (s, e) =>
51+
{
52+
foreach (var i in e.RemovedItems)
53+
{
54+
if (i == dotComTab)
55+
{
56+
ViewModel?.GitHubLogin.Deactivated();
57+
}
58+
else if (i == enterpriseTab)
59+
{
60+
ViewModel?.EnterpriseLogin.Deactivated();
61+
}
62+
}
63+
};
64+
}
65+
66+
void Deactivate()
67+
{
68+
ViewModel?.GitHubLogin.Deactivated();
69+
ViewModel?.EnterpriseLogin.Deactivated();
4870
}
4971

5072
void SetupDotComBindings(Action<IDisposable> d)

0 commit comments

Comments
 (0)