Merge pull request #40 from github/haacked/14-better-login

Better login failed error tooltip

Author: shana

src/GitHub.App/ViewModels/LoginTabViewModel.cs

@@ -41,9 +41,17 @@ protected LoginTabViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrows
 
             Login.ThrownExceptions.Subscribe(ex =>
             {
-                if (!ex.IsCriticalException())
+                if (ex.IsCriticalException()) return;
+
+                log.Info(string.Format(CultureInfo.InvariantCulture, "Error logging into '{0}' as '{1}'", BaseUri,
+                    UsernameOrEmail), ex);
+                if (ex is Octokit.ForbiddenException)
+                {
+                    ShowLogInFailedError = true;
+                    LoginFailedMessage = "Make sure to use your password and not a Personal Access token to log in.";
+                }
+                else
                 {
-                    log.Info(string.Format(CultureInfo.InvariantCulture, "Error logging into '{0}' as '{1}'", BaseUri, UsernameOrEmail), ex);
                     ShowConnectingToHostFailed = true;
                 }
             });
@@ -64,13 +72,20 @@ protected LoginTabViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrows
                 return Observable.Return(Unit.Default);
             });
         }
-        protected IRepositoryHosts RepositoryHosts { get; private set; }
+        protected IRepositoryHosts RepositoryHosts { get; }
         protected abstract Uri BaseUri { get; }
-        public IReactiveCommand<Unit> SignUp { get; private set; }
+        public IReactiveCommand<Unit> SignUp { get; }
 
-        public IReactiveCommand<AuthenticationResult> Login { get; private set; }
-        public IReactiveCommand<Unit> Reset { get; private set; }
-        public IReactiveCommand<Unit> NavigateForgotPassword { get; private set; }
+        public IReactiveCommand<AuthenticationResult> Login { get; }
+        public IReactiveCommand<Unit> Reset { get; }
+        public IReactiveCommand<Unit> NavigateForgotPassword { get; }
+
+        string loginFailedMessage = "Check your username and password, then try again";
+        public string LoginFailedMessage
+        {
+            get { return loginFailedMessage; }
+            set { this.RaiseAndSetIfChanged(ref loginFailedMessage, value); }
+        }
 
         string usernameOrEmail;
         [AllowNull]

src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs

@@ -21,7 +21,7 @@ public class LoginToGitHubForEnterpriseViewModel : LoginTabViewModel, ILoginToGi
         [ImportingConstructor]
         public LoginToGitHubForEnterpriseViewModel(IRepositoryHosts hosts, IVisualStudioBrowser browser) : base(hosts, browser)
         {
-            enterpriseUrlValidator = ReactivePropertyValidator.For(this, x => x.EnterpriseUrl)
+            EnterpriseUrlValidator = ReactivePropertyValidator.For(this, x => x.EnterpriseUrl)
                 .IfNullOrEmpty("Please enter an Enterprise URL")
                 .IfNotUri("Please enter a valid Enterprise URL")
                 .IfGitHubDotComHost("Not an Enterprise server. Please enter an Enterprise URL");
@@ -55,24 +55,13 @@ public string EnterpriseUrl
             set { this.RaiseAndSetIfChanged(ref enterpriseUrl, value); }
         }
 
-        readonly ReactivePropertyValidator enterpriseUrlValidator;
-        public ReactivePropertyValidator EnterpriseUrlValidator
-        {
-            get { return enterpriseUrlValidator; }
-        }
+        public ReactivePropertyValidator EnterpriseUrlValidator { get; }
 
-        protected override Uri BaseUri
-        {
-            get
-            {
-                return new Uri(EnterpriseUrl);
-            }
-        }
+        protected override Uri BaseUri => new Uri(EnterpriseUrl);
 
         public IReactiveCommand<Unit> NavigateLearnMore
         {
             get;
-            private set;
         }
 
         protected override async Task ResetValidation()

src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs

@@ -15,13 +15,11 @@ namespace GitHub.ViewModels
     [PartCreationPolicy(CreationPolicy.NonShared)]
     public class LoginToGitHubViewModel : LoginTabViewModel, ILoginToGitHubViewModel
     {
-        readonly Uri baseUri;
-
         [ImportingConstructor]
         public LoginToGitHubViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrowser browser)
             : base(repositoryHosts, browser)
         {
-            baseUri = HostAddress.GitHubDotComHostAddress.WebUri;
+            BaseUri = HostAddress.GitHubDotComHostAddress.WebUri;
 
             NavigatePricing = ReactiveCommand.CreateAsyncObservable(_ =>
             {
@@ -31,16 +29,13 @@ public LoginToGitHubViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBro
             });
         }
 
-        public IReactiveCommand<Unit> NavigatePricing { get; private set; }
+        public IReactiveCommand<Unit> NavigatePricing { get; }
 
-        protected override Uri BaseUri
-            {
-                get { return baseUri; }
-            }
+        protected override Uri BaseUri { get; }
 
-            protected override IObservable<AuthenticationResult> LogIn(object args)
-            {
-                return LogInToHost(HostAddress.GitHubDotComHostAddress);
-            }
+        protected override IObservable<AuthenticationResult> LogIn(object args)
+        {
+            return LogInToHost(HostAddress.GitHubDotComHostAddress);
         }
+    }
 }

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

@@ -59,6 +59,11 @@ public interface ILoginToHostViewModel
         /// </summary>
         bool ShowLogInFailedError { get; }
 
+        /// <summary>
+        /// The message to show if login failed.
+        /// </summary>
+        string LoginFailedMessage { get; }
+
         /// <summary>
         /// Gets a command which, when invoked, resets all properties 
         /// and validators.

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

@@ -1,5 +1,4 @@
 using System;
-using System.Linq;
 using System.Reactive.Linq;
 using System.Windows;
 using System.Windows.Input;
@@ -63,6 +62,7 @@ void SetupDotComBindings(Action<IDisposable> d)
             d(this.OneWayBind(ViewModel, vm => vm.GitHubLogin.NavigatePricing, v => v.pricingLink.Command));
 
             d(this.OneWayBind(ViewModel, vm => vm.GitHubLogin.ShowLogInFailedError, v => v.dotComLoginFailedMessage.Visibility));
+            d(this.OneWayBind(ViewModel, vm => vm.GitHubLogin.LoginFailedMessage, v => v.dotComLoginFailedMessage.Message));
             d(this.OneWayBind(ViewModel, vm => vm.GitHubLogin.ShowConnectingToHostFailed, v => v.dotComConnectionFailedMessage.Visibility));
         }
 
@@ -86,6 +86,7 @@ void SetupEnterpriseBindings(Action<IDisposable> d)
             d(this.OneWayBind(ViewModel, vm => vm.EnterpriseLogin.NavigateLearnMore, v => v.learnMoreLink.Command));
 
             d(this.OneWayBind(ViewModel, vm => vm.EnterpriseLogin.ShowLogInFailedError, v => v.enterpriseLoginFailedMessage.Visibility));
+            d(this.OneWayBind(ViewModel, vm => vm.EnterpriseLogin.LoginFailedMessage, v => v.enterpriseLoginFailedMessage.Message));
             d(this.OneWayBind(ViewModel, vm => vm.EnterpriseLogin.ShowConnectingToHostFailed, v => v.enterpriseConnectingFailedMessage.Visibility));
         }
 

src/UnitTests/GitHub.App/ViewModels/LoginToGitHubViewModelTests.cs

@@ -1,14 +1,39 @@
 using System;
+using System.Net;
+using System.Reactive.Linq;
+using GitHub.Authentication;
 using GitHub.Info;
 using GitHub.Models;
 using GitHub.Primitives;
 using GitHub.Services;
 using GitHub.ViewModels;
 using NSubstitute;
+using Octokit;
 using Xunit;
 
 public class LoginToGitHubViewModelTests
 {
+    public class TheLoginCommand : TestBaseClass
+    {
+        [Fact]
+        public void ShowsHelpfulTooltipWhenForbiddenResponseReceived()
+        {
+            var response = Substitute.For<IResponse>();
+            response.StatusCode.Returns(HttpStatusCode.Forbidden);
+            var repositoryHosts = Substitute.For<IRepositoryHosts>();
+            repositoryHosts.LogIn(HostAddress.GitHubDotComHostAddress, Args.String, Args.String)
+                .Returns(_ => Observable.Throw<AuthenticationResult>(new ForbiddenException(response)));
+            var browser = Substitute.For<IVisualStudioBrowser>();
+            var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser);
+
+            loginViewModel.Login.Execute(null);
+
+            Assert.True(loginViewModel.ShowLogInFailedError);
+            Assert.Equal("Make sure to use your password and not a Personal Access token to log in.",
+                loginViewModel.LoginFailedMessage);
+        }
+    }
+
     public class TheSignupCommand : TestBaseClass
     {
         [Fact]

src/UnitTests/Helpers/TestBaseClass.cs

@@ -1,10 +1,4 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-using EntryExitDecoratorInterfaces;
-using GitHub.VisualStudio;
+using EntryExitDecoratorInterfaces;
 
 /// <summary>
 /// This base class will get its methods called by the most-derived