fixup! introduce PasswordExpiryUTC and OAuthRefreshToken

Limit iterations in GetCredentialAsync

Author: hickford

src/shared/Core.Tests/HostProviderTests.cs

@@ -76,7 +76,7 @@ public async Task HostProvider_GetCredentialAsync_CredentialDoesNotExist_Returns
         }
 
         [Fact]
-        public async Task HostProvider_GetCredentialAsync_InvalidCredentialStored_ReturnsNewGeneratedCredential()
+        public async Task HostProvider_GetCredentialAsync_InvalidCredentialsStored_ReturnsNewGeneratedCredential()
         {
             const string userName = "john.doe";
             const string password = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
@@ -93,6 +93,7 @@ public async Task HostProvider_GetCredentialAsync_InvalidCredentialStored_Return
             string refreshTokenSeenByGenerate = null;
             var context = new TestCommandContext();
             context.CredentialStore.Add(service, new TestCredential(service, "stored-user", "stored-password") { OAuthRefreshToken = storedRefreshToken});
+            context.CredentialStore.Add(service, new TestCredential(service, "another-stored-user", "another-stored-password"));
             var provider = new TestHostProvider(context)
             {
                 ValidateCredentialFunc = (_, _) => false,
@@ -114,7 +115,7 @@ public async Task HostProvider_GetCredentialAsync_InvalidCredentialStored_Return
             Assert.Equal(userName, actualCredential.Account);
             Assert.Equal(password, actualCredential.Password);
             Assert.Equal(refreshToken, actualCredential.OAuthRefreshToken);
-            // Invalid credential should be erased
+            // Invalid credentials should be erased
             Assert.Equal(0, context.CredentialStore.Count);
         }
 

src/shared/Core/HostProvider.cs

@@ -131,8 +131,9 @@ public virtual async Task<ICredential> GetCredentialAsync(InputArguments input)
             Context.Trace.WriteLine($"Looking for existing credential in store with service={service} account={input.UserName}...");
 
             // Query for matching credentials
-            ICredential credential = null;
-            while (true)
+            ICredential credential;
+            // Limit iterations to avoid infinite loop if CredentialStore.Remove misbehaves
+            for (int i = 0; i < 3; i++)
             {
                 Context.Trace.WriteLine("Querying for existing credentials...");
                 credential = Context.CredentialStore.Get(service, input.UserName);
@@ -141,28 +142,25 @@ public virtual async Task<ICredential> GetCredentialAsync(InputArguments input)
                     Context.Trace.WriteLine("No existing credentials found.");
                     break;
                 }
+                Context.Trace.WriteLine("Existing credential found.");
+                if (await ValidateCredentialAsync(input.GetRemoteUri(), credential))
+                {
+                    Context.Trace.WriteLine("Existing credential satisfies validation.");
+                    return credential;
+                }
                 else
                 {
-                    Context.Trace.WriteLine("Existing credential found.");
-                    if (await ValidateCredentialAsync(input.GetRemoteUri(), credential))
-                    {
-                        Context.Trace.WriteLine("Existing credential satisfies validation.");
-                        return credential;
-                    }
-                    else
+                    Context.Trace.WriteLine("Existing credential fails validation.");
+                    if (credential.OAuthRefreshToken != null)
                     {
-                        Context.Trace.WriteLine("Existing credential fails validation.");
-                        if (credential.OAuthRefreshToken != null)
-                        {
-                            Context.Trace.WriteLine("Found OAuth refresh token.");
-                            input = new InputArguments(input, credential.OAuthRefreshToken);
-                        }
-                        Context.Trace.WriteLine("Erasing invalid credential...");
-                        // Why necessary to erase? We can't be sure that storing a fresh
-                        // credential will overwrite the invalid credential, particularly
-                        // if the usernames differ.
-                        Context.CredentialStore.Remove(service, credential);
+                        Context.Trace.WriteLine("Found OAuth refresh token.");
+                        input = new InputArguments(input, credential.OAuthRefreshToken);
                     }
+                    Context.Trace.WriteLine("Erasing invalid credential...");
+                    // Why necessary to erase? We can't be sure that storing a fresh
+                    // credential will overwrite the invalid credential, particularly
+                    // if the usernames differ.
+                    Context.CredentialStore.Remove(service, credential);
                 }
             }
             Context.Trace.WriteLine("Creating new credential...");