Read scopes at the same time as User.

src/GitHub.App/Api/ApiClient.cs

@@ -17,15 +17,16 @@
 using System.Reactive.Threading.Tasks;
 using Octokit.Internal;
 using System.Collections.Generic;
+using GitHub.Models;
 
 namespace GitHub.Api
 {
     public partial class ApiClient : IApiClient
     {
-        const string scopesHeader = "X-OAuth-Scopes";
-        static readonly Logger log = LogManager.GetCurrentClassLogger();
-
+        const string ScopesHeader = "X-OAuth-Scopes";
         const string ProductName = Info.ApplicationInfo.ApplicationDescription;
+        static readonly Logger log = LogManager.GetCurrentClassLogger();
+        static readonly Uri userEndpoint = new Uri("user", UriKind.Relative);
 
         readonly IObservableGitHubClient gitHubClient;
         // There are two sets of authorization scopes, old and new:
@@ -62,44 +63,30 @@ public IObservable<Gist> CreateGist(NewGist newGist)
             return gitHubClient.Gist.Create(newGist);
         }
 
-        public IObservable<User> GetUser()
+        public IObservable<UserAndScopes> GetUser()
         {
-            return gitHubClient.User.Current();
+            return GetUserInternal().ToObservable();
         }
 
-        public IObservable<string[]> GetScopes()
+        async Task<UserAndScopes> GetUserInternal()
         {
-            return GetScopesInternal().ToObservable();
-        }
+            var response = await gitHubClient.Connection.Get<User>(
+                userEndpoint, null, null).ConfigureAwait(false);
+            var scopes = default(string[]);
 
-        async Task<string[]> GetScopesInternal()
-        {
-            var connection = gitHubClient.Connection;
-
-            try
+            if (response.HttpResponse.Headers.ContainsKey(ScopesHeader))
             {
-                var response = await gitHubClient.Connection.Get<string>(
-                    new Uri("user", UriKind.Relative),
-                    TimeSpan.FromSeconds(3));
-
-                if (response.HttpResponse.Headers.ContainsKey(scopesHeader))
-                {
-                    return response.HttpResponse.Headers[scopesHeader]
-                        .Split(',')
-                        .Select(x => x.Trim())
-                        .ToArray();
-                }
-                else
-                {
-                    log.Error($"Error reading scopes: /user succeeded but {scopesHeader} was not present.");
-                }
+                scopes = response.HttpResponse.Headers[ScopesHeader]
+                    .Split(',')
+                    .Select(x => x.Trim())
+                    .ToArray();
             }
-            catch (Exception e)
+            else
             {
-                log.Error($"Error reading scopes: /user failed: {e}.");
+                log.Error($"Error reading scopes: /user succeeded but {ScopesHeader} was not present.");
             }
 
-            return new string[0];
+            return new UserAndScopes(response.Body, scopes);
         }
 
         public IObservable<ApplicationAuthorization> GetOrCreateApplicationAuthenticationCode(

src/GitHub.App/Models/RepositoryHost.cs

@@ -16,14 +16,15 @@
 using ReactiveUI;
 using System.Linq;
 using System.Reactive.Threading.Tasks;
+using System.Collections.Generic;
 
 namespace GitHub.Models
 {
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
     public class RepositoryHost : ReactiveObject, IRepositoryHost
     {
         static readonly Logger log = LogManager.GetCurrentClassLogger();
-        static readonly AccountCacheItem unverifiedUser = new AccountCacheItem();
+        static readonly UserAndScopes unverifiedUser = new UserAndScopes(null, null);
 
         readonly ITwoFactorChallengeHandler twoFactorChallengeHandler;
         readonly HostAddress hostAddress;
@@ -69,24 +70,15 @@ public IObservable<AuthenticationResult> LogInFromCache()
         {
             return GetUserFromApi()
                 .ObserveOn(RxApp.MainThreadScheduler)
-                .Catch<AccountCacheItem, Exception>(ex =>
+                .Catch<UserAndScopes, Exception>(ex =>
                 {
                     if (ex is AuthorizationException)
                     {
                         log.Warn("Got an authorization exception", ex);
-                        return Observable.Return<AccountCacheItem>(null);
                     }
-                    return ModelService.GetUserFromCache()
-                        .Catch<AccountCacheItem, Exception>(e =>
-                        {
-                            log.Warn("User does not exist in cache", e);
-                            return Observable.Return<AccountCacheItem>(null);
-                        })
-                        .ObserveOn(RxApp.MainThreadScheduler);
+                    return Observable.Return<UserAndScopes>(null);
                 })
-                .Select(user => GetScopesIfLoggedIn(user))
-                .Switch()
-                .SelectMany(x => LoginWithApiUser(x.User, x.Scopes))
+                .SelectMany(LoginWithApiUser)
                 .PublishAsync();
         }
 
@@ -127,14 +119,14 @@ public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string pa
                 .SelectMany(fingerprint => ApiClient.GetOrCreateApplicationAuthenticationCode(interceptingTwoFactorChallengeHandler))
                 .SelectMany(saveAuthorizationToken)
                 .SelectMany(_ => GetUserFromApi())
-                .Catch<AccountCacheItem, ApiException>(firstTryEx =>
+                .Catch<UserAndScopes, ApiException>(firstTryEx =>
                 {
                     var exception = firstTryEx as AuthorizationException;
                     if (isEnterprise
                         && exception != null
                         && exception.Message == "Bad credentials")
                     {
-                        return Observable.Throw<AccountCacheItem>(exception);
+                        return Observable.Throw<UserAndScopes>(exception);
                     }
 
                     // If the Enterprise host doesn't support the write:public_key scope, it'll return a 422.
@@ -170,9 +162,9 @@ public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string pa
                             .SelectMany(_ => GetUserFromApi());
                     }
 
-                    return Observable.Throw<AccountCacheItem>(firstTryEx);
+                    return Observable.Throw<UserAndScopes>(firstTryEx);
                 })
-                .Catch<AccountCacheItem, ApiException>(retryEx =>
+                .Catch<UserAndScopes, ApiException>(retryEx =>
                 {
                     // Older Enterprise hosts either don't have the API end-point to PUT an authorization, or they
                     // return 422 because they haven't white-listed our client ID. In that case, we just ignore
@@ -185,10 +177,10 @@ public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string pa
                         return GetUserFromApi();
 
                     // Other errors are "real" so we pass them along:
-                    return Observable.Throw<AccountCacheItem>(retryEx);
+                    return Observable.Throw<UserAndScopes>(retryEx);
                 })
                 .ObserveOn(RxApp.MainThreadScheduler)
-                .Catch<AccountCacheItem, Exception>(ex =>
+                .Catch<UserAndScopes, Exception>(ex =>
                 {
                     // If we get here, we have an actual login failure:
                     if (ex is TwoFactorChallengeFailedException)
@@ -197,13 +189,11 @@ public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string pa
                     }
                     if (ex is AuthorizationException)
                     {
-                        return Observable.Return(default(AccountCacheItem));
+                        return Observable.Return(default(UserAndScopes));
                     }
-                    return Observable.Throw<AccountCacheItem>(ex);
+                    return Observable.Throw<UserAndScopes>(ex);
                 })
-                .Select(user => GetScopesIfLoggedIn(user))
-                .Switch()
-                .SelectMany(x => LoginWithApiUser(x.User, x.Scopes))
+                .SelectMany(LoginWithApiUser)
                 .PublishAsync();
         }
 
@@ -232,22 +222,23 @@ public IObservable<Unit> LogOut()
                 });
         }
 
-        static IObservable<AuthenticationResult> GetAuthenticationResultForUser(AccountCacheItem account)
+        static IObservable<AuthenticationResult> GetAuthenticationResultForUser(UserAndScopes account)
         {
             return Observable.Return(account == null ? AuthenticationResult.CredentialFailure
                 : account == unverifiedUser
                     ? AuthenticationResult.VerificationFailure
                     : AuthenticationResult.Success);
         }
 
-        IObservable<AuthenticationResult> LoginWithApiUser(AccountCacheItem user, string[] scopes)
+        IObservable<AuthenticationResult> LoginWithApiUser(UserAndScopes userAndScopes)
         {
-            return GetAuthenticationResultForUser(user)
+            return GetAuthenticationResultForUser(userAndScopes)
                 .SelectMany(result =>
                 {
                     if (result.IsSuccess())
                     {
-                        return ModelService.InsertUser(user).Select(_ => result);
+                        var accountCacheItem = new AccountCacheItem(userAndScopes.User);
+                        return ModelService.InsertUser(accountCacheItem).Select(_ => result);
                     }
 
                     if (result == AuthenticationResult.VerificationFailure)
@@ -262,35 +253,21 @@ IObservable<AuthenticationResult> LoginWithApiUser(AccountCacheItem user, string
                     if (result.IsSuccess())
                     {
                         IsLoggedIn = true;
-                        SupportsGist = scopes?.Contains("gist") ?? false;
+                        SupportsGist = userAndScopes.Scopes?.Contains("gist") ?? true;
                     }
 
                     log.Info("Log in from cache for login '{0}' to host '{1}' {2}",
-                        user != null ? user.Login : "(null)",
+                        userAndScopes?.User?.Login ?? "(null)",
                         hostAddress.ApiUri,
                         result.IsSuccess() ? "SUCCEEDED" : "FAILED");
                 });
         }
 
-        IObservable<AccountCacheItem> GetUserFromApi()
+        IObservable<UserAndScopes> GetUserFromApi()
         {
-            return Observable.Defer(() => ApiClient.GetUser().WhereNotNull()
-                .Select(user => new AccountCacheItem(user)));
+            return Observable.Defer(() => ApiClient.GetUser());
         }
 
-        IObservable<UserAndScopes> GetScopesIfLoggedIn(AccountCacheItem user)
-        {
-            if (user != null)
-            {
-                return Observable.Defer(() => ApiClient.GetScopes())
-                    .Select(scopes => new UserAndScopes(user, scopes));
-            }
-            else
-            {
-                return Observable.Return(new UserAndScopes());
-            }
-        }
-
         bool disposed;
         protected virtual void Dispose(bool disposing)
         {
@@ -329,21 +306,5 @@ public IModelService ModelService
             get;
             private set;
         }
-
-        private class UserAndScopes
-        {
-            public UserAndScopes()
-            {
-            }
-
-            public UserAndScopes(AccountCacheItem user, string[] scopes)
-            {
-                User = user;
-                Scopes = scopes;
-            }
-
-            public AccountCacheItem User { get; }
-            public string[] Scopes { get; }
-        }
     }
 }

src/GitHub.Exports.Reactive/Api/IApiClient.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Reactive;
+using GitHub.Models;
 using GitHub.Primitives;
 using Octokit;
 
@@ -10,8 +11,7 @@ public interface IApiClient
         HostAddress HostAddress { get; }
         IObservable<Repository> CreateRepository(NewRepository repository, string login, bool isUser);
         IObservable<Gist> CreateGist(NewGist newGist);
-        IObservable<User> GetUser();
-        IObservable<string[]> GetScopes();
+        IObservable<UserAndScopes> GetUser();
         IObservable<Organization> GetOrganizations();
         /// <summary>
         /// Retrieves all repositories that belong to this user.

src/GitHub.Exports.Reactive/Caches/AccountCacheItem.cs

@@ -12,6 +12,11 @@ public static AccountCacheItem Create(Account apiAccount)
             return new AccountCacheItem(apiAccount);
         }
 
+        public static AccountCacheItem Create(UserAndScopes userAndScopes)
+        {
+            return new AccountCacheItem(userAndScopes.User);
+        }
+
         public AccountCacheItem()
         { }
 

src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj

@@ -87,6 +87,7 @@
     <Compile Include="Models\IAvatarContainer.cs" />
     <Compile Include="Models\IConnectionRepositoryHostMap.cs" />
     <Compile Include="Models\IRepositoryHosts.cs" />
+    <Compile Include="Models\UserAndScopes.cs" />
     <Compile Include="Services\IModelService.cs" />
     <Compile Include="Services\IGistPublishService.cs" />
     <Compile Include="ViewModels\IGistCreationViewModel.cs" />

src/GitHub.Exports.Reactive/Models/UserAndScopes.cs

@@ -0,0 +1,36 @@
+using System.Collections.Generic;
+using Octokit;
+
+namespace GitHub.Models
+{
+    /// <summary>
+    /// Holds an <see cref="Octokit.User"/> model together with the OAuth scopes that were 
+    /// received when the user was read.
+    /// </summary>
+    public class UserAndScopes
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="UserAndScopes"/> class.
+        /// </summary>
+        /// <param name="user">The user information.</param>
+        /// <param name="scopes">The scopes. May be null.</param>
+        public UserAndScopes(User user, IReadOnlyList<string> scopes)
+        {
+            User = user;
+            Scopes = scopes;
+        }
+
+        /// <summary>
+        /// Gets the user information.
+        /// </summary>
+        public User User { get; }
+
+        /// <summary>
+        /// Gets the OAuth scopes read when the user was read.
+        /// </summary>
+        /// <remarks>
+        /// A value of null means that no X-OAuth-Scopes header was received.
+        /// </remarks>
+        public IReadOnlyList<string> Scopes { get; }
+    }
+}