Merge pull request #1787 from github/refactor/clone

Refactored Clone Dialog

Author: grokys

src/GitHub.Api/ApiClientConfiguration.cs

@@ -33,9 +33,14 @@ static ApiClientConfiguration()
         public static string ClientSecret { get; private set; }
 
         /// <summary>
-        /// Gets the scopes required by the application.
+        /// Gets the minimum scopes required by the application.
         /// </summary>
-        public static IReadOnlyList<string> RequiredScopes { get; } = new[] { "user", "repo", "gist", "write:public_key" };
+        public static IReadOnlyList<string> MinimumScopes { get; } = new[] { "user", "repo", "gist", "write:public_key" };
+
+        /// <summary>
+        /// Gets the ideal scopes requested by the application.
+        /// </summary>
+        public static IReadOnlyList<string> RequestedScopes { get; } = new[] { "user", "repo", "gist", "write:public_key", "read:org" };
 
         /// <summary>
         /// Gets a note that will be stored with the OAUTH token.

src/GitHub.Api/ILoginManager.cs

@@ -21,11 +21,11 @@ public interface ILoginManager
         /// <param name="client">An octokit client configured to access the server.</param>
         /// <param name="userName">The username.</param>
         /// <param name="password">The password.</param>
-        /// <returns>The logged in user.</returns>
+        /// <returns>A <see cref="LoginResult"/> with the details of the successful login.</returns>
         /// <exception cref="AuthorizationException">
         /// The login authorization failed.
         /// </exception>
-        Task<User> Login(HostAddress hostAddress, IGitHubClient client, string userName, string password);
+        Task<LoginResult> Login(HostAddress hostAddress, IGitHubClient client, string userName, string password);
 
         /// <summary>
         /// Attempts to log into a GitHub server via OAuth in the browser.
@@ -35,11 +35,11 @@ public interface ILoginManager
         /// <param name="oauthClient">An octokit OAuth client configured to access the server.</param>
         /// <param name="openBrowser">A callback that should open a browser at the requested URL.</param>
         /// <param name="cancel">A cancellation token used to cancel the operation.</param>
-        /// <returns>The logged in user.</returns>
+        /// <returns>A <see cref="LoginResult"/> with the details of the successful login.</returns>
         /// <exception cref="AuthorizationException">
         /// The login authorization failed.
         /// </exception>
-        Task<User> LoginViaOAuth(
+        Task<LoginResult> LoginViaOAuth(
             HostAddress hostAddress,
             IGitHubClient client,
             IOauthClient oauthClient,
@@ -52,7 +52,8 @@ Task<User> LoginViaOAuth(
         /// <param name="hostAddress">The address of the server.</param>
         /// <param name="client">An octokit client configured to access the server.</param>
         /// <param name="token">The token.</param>
-        Task<User> LoginWithToken(
+        /// <returns>A <see cref="LoginResult"/> with the details of the successful login.</returns>
+        Task<LoginResult> LoginWithToken(
             HostAddress hostAddress,
             IGitHubClient client,
             string token);
@@ -62,11 +63,11 @@ Task<User> LoginWithToken(
         /// </summary>
         /// <param name="hostAddress">The address of the server.</param>
         /// <param name="client">An octokit client configured to access the server.</param>
-        /// <returns>The logged in user.</returns>
+        /// <returns>A <see cref="LoginResult"/> with the details of the successful login.</returns>
         /// <exception cref="AuthorizationException">
         /// The login authorization failed.
         /// </exception>
-        Task<User> LoginFromCache(HostAddress hostAddress, IGitHubClient client);
+        Task<LoginResult> LoginFromCache(HostAddress hostAddress, IGitHubClient client);
 
         /// <summary>
         /// Logs out of GitHub server.

src/GitHub.Api/LoginManager.cs

@@ -6,6 +6,7 @@
 using System.Threading.Tasks;
 using GitHub.Extensions;
 using GitHub.Logging;
+using GitHub.Models;
 using GitHub.Primitives;
 using Octokit;
 using Serilog;
@@ -24,7 +25,8 @@ public class LoginManager : ILoginManager
         readonly Lazy<ITwoFactorChallengeHandler> twoFactorChallengeHandler;
         readonly string clientId;
         readonly string clientSecret;
-        readonly IReadOnlyList<string> scopes;
+        readonly IReadOnlyList<string> minimumScopes;
+        readonly IReadOnlyList<string> requestedScopes;
         readonly string authorizationNote;
         readonly string fingerprint;
         IOAuthCallbackListener oauthListener;
@@ -37,7 +39,8 @@ public class LoginManager : ILoginManager
         /// <param name="oauthListener">The callback listener to signal successful login.</param>
         /// <param name="clientId">The application's client API ID.</param>
         /// <param name="clientSecret">The application's client API secret.</param>
-        /// <param name="scopes">List of scopes to authenticate for</param>
+        /// <param name="minimumScopes">The minimum acceptable scopes.</param>
+        /// <param name="requestedScopes">The scopes to request when logging in.</param>
         /// <param name="authorizationNote">An note to store with the authorization.</param>
         /// <param name="fingerprint">The machine fingerprint.</param>
         public LoginManager(
@@ -46,7 +49,8 @@ public LoginManager(
             IOAuthCallbackListener oauthListener,
             string clientId,
             string clientSecret,
-            IReadOnlyList<string> scopes,
+            IReadOnlyList<string> minimumScopes,
+            IReadOnlyList<string> requestedScopes,
             string authorizationNote = null,
             string fingerprint = null)
         {
@@ -60,13 +64,14 @@ public LoginManager(
             this.oauthListener = oauthListener;
             this.clientId = clientId;
             this.clientSecret = clientSecret;
-            this.scopes = scopes;
+            this.minimumScopes = minimumScopes;
+            this.requestedScopes = requestedScopes;
             this.authorizationNote = authorizationNote;
             this.fingerprint = fingerprint;
         }
 
         /// <inheritdoc/>
-        public async Task<User> Login(
+        public async Task<LoginResult> Login(
             HostAddress hostAddress,
             IGitHubClient client,
             string userName,
@@ -83,7 +88,7 @@ public async Task<User> Login(
 
             var newAuth = new NewAuthorization
             {
-                Scopes = scopes,
+                Scopes = requestedScopes,
                 Note = authorizationNote,
                 Fingerprint = fingerprint,
             };
@@ -121,11 +126,11 @@ public async Task<User> Login(
             } while (auth == null);
 
             await keychain.Save(userName, auth.Token, hostAddress).ConfigureAwait(false);
-            return await ReadUserWithRetry(client);
+            return await ReadUserWithRetry(client).ConfigureAwait(false);
         }
 
         /// <inheritdoc/>
-        public async Task<User> LoginViaOAuth(
+        public async Task<LoginResult> LoginViaOAuth(
             HostAddress hostAddress,
             IGitHubClient client,
             IOauthClient oauthClient,
@@ -143,18 +148,18 @@ public async Task<User> LoginViaOAuth(
 
             openBrowser(loginUrl);
 
-            var code = await listen;
+            var code = await listen.ConfigureAwait(false);
             var request = new OauthTokenRequest(clientId, clientSecret, code);
-            var token = await oauthClient.CreateAccessToken(request);
+            var token = await oauthClient.CreateAccessToken(request).ConfigureAwait(false);
 
             await keychain.Save("[oauth]", token.AccessToken, hostAddress).ConfigureAwait(false);
-            var user = await ReadUserWithRetry(client);
-            await keychain.Save(user.Login, token.AccessToken, hostAddress).ConfigureAwait(false);
-            return user;
+            var result = await ReadUserWithRetry(client).ConfigureAwait(false);
+            await keychain.Save(result.User.Login, token.AccessToken, hostAddress).ConfigureAwait(false);
+            return result;
         }
 
         /// <inheritdoc/>
-        public async Task<User> LoginWithToken(
+        public async Task<LoginResult> LoginWithToken(
             HostAddress hostAddress,
             IGitHubClient client,
             string token)
@@ -167,19 +172,19 @@ public async Task<User> LoginWithToken(
 
             try
             {
-                var user = await ReadUserWithRetry(client);
-                await keychain.Save(user.Login, token, hostAddress).ConfigureAwait(false);
-                return user;
+                var result = await ReadUserWithRetry(client).ConfigureAwait(false);
+                await keychain.Save(result.User.Login, token, hostAddress).ConfigureAwait(false);
+                return result;
             }
             catch
             {
-                await keychain.Delete(hostAddress);
+                await keychain.Delete(hostAddress).ConfigureAwait(false);
                 throw;
             }
         }
 
         /// <inheritdoc/>
-        public Task<User> LoginFromCache(HostAddress hostAddress, IGitHubClient client)
+        public Task<LoginResult> LoginFromCache(HostAddress hostAddress, IGitHubClient client)
         {
             Guard.ArgumentNotNull(hostAddress, nameof(hostAddress));
             Guard.ArgumentNotNull(client, nameof(client));
@@ -193,41 +198,7 @@ public async Task Logout(HostAddress hostAddress, IGitHubClient client)
             Guard.ArgumentNotNull(hostAddress, nameof(hostAddress));
             Guard.ArgumentNotNull(client, nameof(client));
 
-            await keychain.Delete(hostAddress);
-        }
-
-        /// <summary>
-        /// Tests if received API scopes match the required API scopes.
-        /// </summary>
-        /// <param name="required">The required API scopes.</param>
-        /// <param name="received">The received API scopes.</param>
-        /// <returns>True if all required scopes are present, otherwise false.</returns>
-        public static bool ScopesMatch(IReadOnlyList<string> required, IReadOnlyList<string> received)
-        {
-            foreach (var scope in required)
-            {
-                var found = received.Contains(scope);
-
-                if (!found && 
-                    (scope.StartsWith("read:", StringComparison.Ordinal) ||
-                     scope.StartsWith("write:", StringComparison.Ordinal)))
-                {
-                    // NOTE: Scopes are actually more complex than this, for example
-                    // `user` encompasses `read:user` and `user:email` but just use
-                    // this simple rule for now as it works for the scopes we require.
-                    var adminScope = scope
-                        .Replace("read:", "admin:")
-                        .Replace("write:", "admin:");
-                    found = received.Contains(adminScope);
-                }
-
-                if (!found)
-                {
-                    return false;
-                }
-            }
-
-            return true;
+            await keychain.Delete(hostAddress).ConfigureAwait(false);
         }
 
         async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthorization(
@@ -256,18 +227,18 @@ async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthoriza
                         twoFactorAuthenticationCode).ConfigureAwait(false);
                 }
 
-                if (result.Token == string.Empty)
+                if (string.IsNullOrEmpty(result.Token))
                 {
                     if (twoFactorAuthenticationCode == null)
                     {
-                        await client.Authorization.Delete(result.Id);
+                        await client.Authorization.Delete(result.Id).ConfigureAwait(false);
                     }
                     else
                     {
-                        await client.Authorization.Delete(result.Id, twoFactorAuthenticationCode);
+                        await client.Authorization.Delete(result.Id, twoFactorAuthenticationCode).ConfigureAwait(false);
                     }
                 }
-            } while (result.Token == string.Empty && retry++ == 0);
+            } while (string.IsNullOrEmpty(result.Token) && retry++ == 0);
 
             return result;
         }
@@ -280,7 +251,7 @@ async Task<ApplicationAuthorization> HandleTwoFactorAuthorization(
         {
             for (;;)
             {
-                var challengeResult = await twoFactorChallengeHandler.Value.HandleTwoFactorException(exception);
+                var challengeResult = await twoFactorChallengeHandler.Value.HandleTwoFactorException(exception).ConfigureAwait(false);
 
                 if (challengeResult == null)
                 {
@@ -304,7 +275,7 @@ async Task<ApplicationAuthorization> HandleTwoFactorAuthorization(
                     }
                     catch (Exception e)
                     {
-                        await twoFactorChallengeHandler.Value.ChallengeFailed(e);
+                        await twoFactorChallengeHandler.Value.ChallengeFailed(e).ConfigureAwait(false);
                         await keychain.Delete(hostAddress).ConfigureAwait(false);
                         throw;
                     }
@@ -345,7 +316,7 @@ e is ForbiddenException ||
                  apiException?.StatusCode == (HttpStatusCode)422);
         }
 
-        async Task<User> ReadUserWithRetry(IGitHubClient client)
+        async Task<LoginResult> ReadUserWithRetry(IGitHubClient client)
         {
             var retry = 0;
 
@@ -362,29 +333,29 @@ async Task<User> ReadUserWithRetry(IGitHubClient client)
 
                 // It seems that attempting to use a token immediately sometimes fails, retry a few
                 // times with a delay of of 1s to allow the token to propagate.
-                await Task.Delay(1000);
+                await Task.Delay(1000).ConfigureAwait(false);
             }
         }
 
-        async Task<User> GetUserAndCheckScopes(IGitHubClient client)
+        async Task<LoginResult> GetUserAndCheckScopes(IGitHubClient client)
         {
             var response = await client.Connection.Get<User>(
                 UserEndpoint, null, null).ConfigureAwait(false);
 
             if (response.HttpResponse.Headers.ContainsKey(ScopesHeader))
             {
-                var returnedScopes = response.HttpResponse.Headers[ScopesHeader]
+                var returnedScopes = new ScopesCollection(response.HttpResponse.Headers[ScopesHeader]
                     .Split(',')
                     .Select(x => x.Trim())
-                    .ToArray();
+                    .ToArray());
 
-                if (ScopesMatch(scopes, returnedScopes))
+                if (returnedScopes.Matches(minimumScopes))
                 {
-                    return response.Body;
+                    return new LoginResult(response.Body, returnedScopes);
                 }
                 else
                 {
-                    log.Error("Incorrect API scopes: require {RequiredScopes} but got {Scopes}", scopes, returnedScopes);
+                    log.Error("Incorrect API scopes: require {RequiredScopes} but got {Scopes}", minimumScopes, returnedScopes);
                 }
             }
             else
@@ -393,7 +364,7 @@ async Task<User> GetUserAndCheckScopes(IGitHubClient client)
             }
 
             throw new IncorrectScopesException(
-                "Incorrect API scopes. Required: " + string.Join(",", scopes));
+                "Incorrect API scopes. Required: " + string.Join(",", minimumScopes));
         }
 
         Uri GetLoginUrl(IOauthClient client, string state)
@@ -402,7 +373,7 @@ Uri GetLoginUrl(IOauthClient client, string state)
 
             request.State = state;
 
-            foreach (var scope in scopes)
+            foreach (var scope in requestedScopes)
             {
                 request.Scopes.Add(scope);
             }

src/GitHub.Api/LoginResult.cs

@@ -0,0 +1,34 @@
+using System;
+using System.Collections.Generic;
+using GitHub.Models;
+using Octokit;
+
+namespace GitHub.Api
+{
+    /// <summary>
+    /// Holds the result of a successful login by <see cref="ILoginManager"/>.
+    /// </summary>
+    public class LoginResult
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="LoginResult"/> class.
+        /// </summary>
+        /// <param name="user">The logged-in user.</param>
+        /// <param name="scopes">The login scopes.</param>
+        public LoginResult(User user, ScopesCollection scopes)
+        {
+            User = user;
+            Scopes = scopes;
+        }
+
+        /// <summary>
+        /// Gets the login scopes.
+        /// </summary>
+        public ScopesCollection Scopes { get; }
+
+        /// <summary>
+        /// Gets the logged-in user.
+        /// </summary>
+        public User User { get; }
+    }
+}

src/GitHub.App/Properties/AssemblyInfo.cs

@@ -1,6 +1,8 @@
 using System.Windows.Markup;
 
 [assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.SampleData")]
+[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.SampleData.Dialog.Clone")]
 [assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels")]
 [assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.Dialog")]
+[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.Dialog.Clone")]
 [assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.GitHubPane")]