Store scopes in the Connection.

Author: grokys

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;
@@ -200,40 +201,6 @@ public async Task Logout(HostAddress hostAddress, IGitHubClient client)
             await keychain.Delete(hostAddress).ConfigureAwait(false);
         }
 
-        /// <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;
-        }
-
         async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthorization(
             IGitHubClient client,
             NewAuthorization newAuth,
@@ -377,12 +344,12 @@ async Task<LoginResult> GetUserAndCheckScopes(IGitHubClient client)
 
             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(minimumScopes, returnedScopes))
+                if (returnedScopes.Matches(minimumScopes))
                 {
                     return new LoginResult(response.Body, returnedScopes);
                 }

src/GitHub.Api/LoginResult.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using GitHub.Models;
 using Octokit;
 
 namespace GitHub.Api
@@ -14,7 +15,7 @@ public class LoginResult
         /// </summary>
         /// <param name="user">The logged-in user.</param>
         /// <param name="scopes">The login scopes.</param>
-        public LoginResult(User user, IReadOnlyList<string> scopes)
+        public LoginResult(User user, ScopesCollection scopes)
         {
             User = user;
             Scopes = scopes;
@@ -23,7 +24,7 @@ public LoginResult(User user, IReadOnlyList<string> scopes)
         /// <summary>
         /// Gets the login scopes.
         /// </summary>
-        public IReadOnlyList<string> Scopes { get; }
+        public ScopesCollection Scopes { get; }
 
         /// <summary>
         /// Gets the logged-in user.

src/GitHub.App/SampleData/SampleViewModels.cs

@@ -199,9 +199,8 @@ class Conn : IConnection
             public HostAddress HostAddress { get; set; }
 
             public string Username { get; set; }
-            public ObservableCollection<ILocalRepositoryModel> Repositories { get; set; }
-
             public Octokit.User User => null;
+            public ScopesCollection Scopes => null;
             public bool IsLoggedIn => true;
             public bool IsLoggingIn => false;
 

src/GitHub.Exports/Models/IConnection.cs

@@ -1,6 +1,5 @@
 using System;
 using System.ComponentModel;
-using System.Threading.Tasks;
 using GitHub.Primitives;
 using Octokit;
 
@@ -29,6 +28,11 @@ public interface IConnection : INotifyPropertyChanged
         /// </remarks>
         User User { get; }
 
+        /// <summary>
+        /// Gets the login scopes.
+        /// </summary>
+        ScopesCollection Scopes { get; }
+
         /// <summary>
         /// Gets a value indicating whether the login of the account succeeded.
         /// </summary>

src/GitHub.Exports/Models/ScopesCollection.cs

@@ -0,0 +1,72 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace GitHub.Models
+{
+    /// <summary>
+    /// A collection of OAuth scopes.
+    /// </summary>
+    public class ScopesCollection : IReadOnlyList<string>
+    {
+        readonly IReadOnlyList<string> inner;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="Scopes"/> class.
+        /// </summary>
+        /// <param name="scopes">The scopes.</param>
+        public ScopesCollection(IReadOnlyList<string> scopes)
+        {
+            inner = scopes;
+        }
+
+        /// <inheritdoc/>
+        public string this[int index] => inner[index];
+
+        /// <inheritdoc/>
+        public int Count => inner.Count;
+
+        /// <inheritdoc/>
+        public IEnumerator<string> GetEnumerator() => inner.GetEnumerator();
+
+        /// <inheritdoc/>
+        IEnumerator IEnumerable.GetEnumerator() => inner.GetEnumerator();
+
+        /// <summary>
+        /// Tests if received API scopes match the required API scopes.
+        /// </summary>
+        /// <param name="required">The required API scopes.</param>
+        /// <returns>True if all required scopes are present, otherwise false.</returns>
+        public bool Matches(IReadOnlyList<string> required)
+        {
+            foreach (var scope in required)
+            {
+                var found = inner.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 = inner.Contains(adminScope);
+                }
+
+                if (!found)
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        /// <inheritdoc/>
+        public override string ToString() => string.Join(",", inner);
+    }
+}

src/GitHub.Exports/Services/Connection.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.ComponentModel;
 using System.Runtime.CompilerServices;
 using GitHub.Models;
@@ -13,25 +14,29 @@ public class Connection : IConnection
     {
         string username;
         Octokit.User user;
+        ScopesCollection scopes;
         bool isLoggedIn;
         bool isLoggingIn;
         Exception connectionError;
 
-        public Connection(HostAddress hostAddress)
+        public Connection(
+            HostAddress hostAddress,
+            string username)
         {
+            this.username = username;
             HostAddress = hostAddress;
             isLoggedIn = false;
             isLoggingIn = true;
         }
 
         public Connection(
             HostAddress hostAddress,
-            string username,
-            Octokit.User user)
+            Octokit.User user,
+            ScopesCollection scopes)
         {
             HostAddress = hostAddress;
-            this.username = username;
             this.user = user;
+            this.scopes = scopes;
             isLoggedIn = true;
         }
 
@@ -44,14 +49,20 @@ public string Username
             get => username;
             private set => RaiseAndSetIfChanged(ref username, value);
         }
-
         /// <inheritdoc/>
         public Octokit.User User
         {
             get => user;
             private set => RaiseAndSetIfChanged(ref user, value);
         }
 
+        /// <inheritdoc/>
+        public ScopesCollection Scopes
+        {
+            get => scopes;
+            private set => RaiseAndSetIfChanged(ref scopes, value);
+        }
+
         /// <inheritdoc/>
         public bool IsLoggedIn
         {
@@ -82,20 +93,21 @@ public void SetLoggingIn()
             IsLoggedIn = false;
             IsLoggingIn = true;
             User = null;
-            Username = null;
+            Scopes = null;
         }
 
         public void SetError(Exception e)
         {
             ConnectionError = e;
             IsLoggingIn = false;
             IsLoggedIn = false;
+            Scopes = null;
         }
 
-        public void SetSuccess(Octokit.User user)
+        public void SetSuccess(Octokit.User user, ScopesCollection scopes)
         {
             User = user;
-            Username = user.Login;
+            Scopes = scopes;
             IsLoggingIn = false;
             IsLoggedIn = true;
         }

src/GitHub.VisualStudio/Services/ConnectionManager.cs

@@ -80,7 +80,7 @@ public async Task<IConnection> LogIn(HostAddress address, string userName, strin
 
             var client = CreateClient(address);
             var login = await loginManager.Login(address, client, userName, password);
-            var connection = new Connection(address, userName, login.User);
+            var connection = new Connection(address, login.User, login.Scopes);
 
             conns.Add(connection);
             await SaveConnections();
@@ -101,7 +101,7 @@ public async Task<IConnection> LogInViaOAuth(HostAddress address, CancellationTo
             var client = CreateClient(address);
             var oauthClient = new OauthClient(client.Connection);
             var login = await loginManager.LoginViaOAuth(address, client, oauthClient, OpenBrowser, cancel);
-            var connection = new Connection(address, login.User.Login, login.User);
+            var connection = new Connection(address, login.User, login.Scopes);
 
             conns.Add(connection);
             await SaveConnections();
@@ -122,7 +122,7 @@ public async Task<IConnection> LogInWithToken(HostAddress address, string token)
 
             var client = CreateClient(address);
             var login = await loginManager.LoginWithToken(address, client, token);
-            var connection = new Connection(address, login.User.Login, login.User);
+            var connection = new Connection(address, login.User, login.Scopes);
 
             conns.Add(connection);
             await SaveConnections();
@@ -157,7 +157,7 @@ public async Task Retry(IConnection connection)
             {
                 var client = CreateClient(c.HostAddress);
                 var login = await loginManager.LoginFromCache(connection.HostAddress, client);
-                c.SetSuccess(login.User);
+                c.SetSuccess(login.User, login.Scopes);
                 await usageTracker.IncrementCounter(x => x.NumberOfLogins);
             }
             catch (Exception e)
@@ -194,7 +194,7 @@ async Task LoadConnections(ObservableCollection<IConnection> result)
             {
                 foreach (var c in await cache.Load())
                 {
-                    var connection = new Connection(c.HostAddress);
+                    var connection = new Connection(c.HostAddress, c.UserName);
                     result.Add(connection);
                 }
 
@@ -206,7 +206,7 @@ async Task LoadConnections(ObservableCollection<IConnection> result)
                     try
                     {
                         login = await loginManager.LoginFromCache(connection.HostAddress, client);
-                        connection.SetSuccess(login.User);
+                        connection.SetSuccess(login.User, login.Scopes);
                         await usageTracker.IncrementCounter(x => x.NumberOfLogins);
                     }
                     catch (Exception e)

test/GitHub.Api.UnitTests/LoginManagerTests.cs

@@ -311,39 +311,4 @@ IGitHubClient CreateClient(User user = null, string[] responseScopes = null)
             return result;
         }
     }
-
-    public class TheScopesMatchMethod
-    {
-        [Test]
-        public void ReturnsFalseWhenMissingScopes()
-        {
-            var received = new[] { "user", "repo", "write:public_key" };
-
-            Assert.False(LoginManager.ScopesMatch(scopes, received));
-        }
-
-        [Test]
-        public void ReturnsTrueWhenScopesEqual()
-        {
-            var received = new[] { "user", "repo", "gist", "write:public_key" };
-
-            Assert.True(LoginManager.ScopesMatch(scopes, received));
-        }
-
-        [Test]
-        public void ReturnsTrueWhenExtraScopesReturned()
-        {
-            var received = new[] { "user", "repo", "gist", "foo", "write:public_key" };
-
-            Assert.True(LoginManager.ScopesMatch(scopes, received));
-        }
-
-        [Test]
-        public void ReturnsTrueWhenAdminScopeReturnedInsteadOfWrite()
-        {
-            var received = new[] { "user", "repo", "gist", "foo", "admin:public_key" };
-
-            Assert.True(LoginManager.ScopesMatch(scopes, received));
-        }
-    }
 }