Skip to content

Commit

Permalink
introduce PasswordExpiryUTC and OAuthRefreshToken
Browse files Browse the repository at this point in the history
Add properties ICredential.PasswordExpiryUTC and
ICredential.OAuthRefreshToken. These correspond to Git credential
attributes password_expiry_utc and oauth_refresh_token, see
https://git-scm.com/docs/git-credential#IOFMT. Previously these
attributes were silently disarded.

Plumb these properties from input to host provider to credential store
to output.

Credential store support for these attributes is optional, marked by
new properties ICredentialStore.CanStorePasswordExpiryUTC and
ICredentialStore.CanStoreOAuthRefreshToken. Implement support in
CredentialCacheStore, SecretServiceCollection and
WindowsCredentialManager.

Add method IHostProvider.ValidateCredentialAsync. The default
implementation simply checks expiry.

Improve implementations of GenericHostProvider and GitLabHostProvider.
Previously, GetCredentialAsync saved credentials as a side effect. This
is no longer necessary. The workaround to store OAuth refresh tokens
under a separate service is no longer necessary assuming
CredentialStore.CanStoreOAuthRefreshToken. Querying GitLab to check
token expiration is no longer necessary assuming
CredentialStore.CanStorePasswordExpiryUTC.
  • Loading branch information
hickford committed Nov 4, 2023
1 parent 2365cac commit 2a14b66
Showing 24 changed files with 441 additions and 156 deletions.
12 changes: 10 additions & 2 deletions src/shared/Core.Tests/Commands/GetCommandTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Text;
@@ -16,14 +17,21 @@ public async Task GetCommand_ExecuteAsync_CallsHostProviderAndWritesCredential()
{
const string testUserName = "john.doe";
const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
ICredential testCredential = new GitCredential(testUserName, testPassword);
const string testRefreshToken = "xyzzy";
const long testExpiry = 1919539847;
ICredential testCredential = new GitCredential(testUserName, testPassword) {
OAuthRefreshToken = testRefreshToken,
PasswordExpiry = DateTimeOffset.FromUnixTimeSeconds(testExpiry),
};
var stdin = $"protocol=http\nhost=example.com\n\n";
var expectedStdOutDict = new Dictionary<string, string>
{
["protocol"] = "http",
["host"] = "example.com",
["username"] = testUserName,
["password"] = testPassword
["password"] = testPassword,
["password_expiry_utc"] = testExpiry.ToString(),
["oauth_refresh_token"] = testRefreshToken,
};

var providerMock = new Mock<IHostProvider>();
12 changes: 9 additions & 3 deletions src/shared/Core.Tests/Commands/StoreCommandTests.cs
Original file line number Diff line number Diff line change
@@ -13,13 +13,17 @@ public async Task StoreCommand_ExecuteAsync_CallsHostProvider()
{
const string testUserName = "john.doe";
const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
var stdin = $"protocol=http\nhost=example.com\nusername={testUserName}\npassword={testPassword}\n\n";
const string testRefreshToken = "xyzzy";
const long testExpiry = 1919539847;
var stdin = $"protocol=http\nhost=example.com\nusername={testUserName}\npassword={testPassword}\noauth_refresh_token={testRefreshToken}\npassword_expiry_utc={testExpiry}\n\n";
var expectedInput = new InputArguments(new Dictionary<string, string>
{
["protocol"] = "http",
["host"] = "example.com",
["username"] = testUserName,
["password"] = testPassword
["password"] = testPassword,
["oauth_refresh_token"] = testRefreshToken,
["password_expiry_utc"] = testExpiry.ToString(),
});

var providerMock = new Mock<IHostProvider>();
@@ -46,7 +50,9 @@ bool AreInputArgumentsEquivalent(InputArguments a, InputArguments b)
a.Host == b.Host &&
a.Path == b.Path &&
a.UserName == b.UserName &&
a.Password == b.Password;
a.Password == b.Password &&
a.OAuthRefreshToken == b.OAuthRefreshToken &&
a.PasswordExpiry == b.PasswordExpiry;
}
}
}
8 changes: 2 additions & 6 deletions src/shared/Core.Tests/GenericHostProviderTests.cs
Original file line number Diff line number Diff line change
@@ -201,7 +201,6 @@ public async Task GenericHostProvider_GenerateCredentialAsync_OAuth_CompleteOAut
const string testAcessToken = "OAUTH_TOKEN";
const string testRefreshToken = "OAUTH_REFRESH_TOKEN";
const string testResource = "https://git.example.com/foo";
const string expectedRefreshTokenService = "https://refresh_token.git.example.com/foo";

var authMode = OAuthAuthenticationModes.Browser;
string[] scopes = { "code:write", "code:read" };
@@ -249,7 +248,7 @@ public async Task GenericHostProvider_GenerateCredentialAsync_OAuth_CompleteOAut
.ReturnsAsync(new OAuth2TokenResult(testAcessToken, "access_token")
{
Scopes = scopes,
RefreshToken = testRefreshToken
RefreshToken = testRefreshToken,
});

var provider = new GenericHostProvider(context, basicAuthMock.Object, wiaAuthMock.Object, oauthMock.Object);
@@ -259,10 +258,7 @@ public async Task GenericHostProvider_GenerateCredentialAsync_OAuth_CompleteOAut
Assert.NotNull(credential);
Assert.Equal(testUserName, credential.Account);
Assert.Equal(testAcessToken, credential.Password);

Assert.True(context.CredentialStore.TryGet(expectedRefreshTokenService, null, out TestCredential refreshToken));
Assert.Equal(testUserName, refreshToken.Account);
Assert.Equal(testRefreshToken, refreshToken.Password);
Assert.Equal(testRefreshToken, credential.OAuthRefreshToken);

oauthMock.Verify(x => x.GetAuthenticationModeAsync(testResource, OAuthAuthenticationModes.All), Times.Once);
oauthMock.Verify(x => x.GetTokenByBrowserAsync(It.IsAny<OAuth2Client>(), scopes), Times.Once);
67 changes: 62 additions & 5 deletions src/shared/Core.Tests/HostProviderTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using System.Runtime;
using System.Threading.Tasks;
using GitCredentialManager.Tests.Objects;
using Xunit;
@@ -15,16 +17,16 @@ public async Task HostProvider_GetCredentialAsync_CredentialExists_ReturnsExisti
const string userName = "john.doe";
const string password = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
const string service = "https://example.com";
const string refreshToken = "xyzzy";
DateTimeOffset expiry = DateTimeOffset.FromUnixTimeSeconds(1919539847);
var input = new InputArguments(new Dictionary<string, string>
{
["protocol"] = "https",
["host"] = "example.com",
["username"] = userName,
["password"] = password, // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
});

var context = new TestCommandContext();
context.CredentialStore.Add(service, userName, password);
context.CredentialStore.Add(service, new TestCredential(service, userName, password) { OAuthRefreshToken = refreshToken, PasswordExpiry = expiry});
var provider = new TestHostProvider(context)
{
IsSupportedFunc = _ => true,
@@ -39,6 +41,8 @@ public async Task HostProvider_GetCredentialAsync_CredentialExists_ReturnsExisti

Assert.Equal(userName, actualCredential.Account);
Assert.Equal(password, actualCredential.Password);
Assert.Equal(refreshToken, actualCredential.OAuthRefreshToken);
Assert.Equal(expiry, actualCredential.PasswordExpiry);
}

[Fact]
@@ -50,8 +54,6 @@ public async Task HostProvider_GetCredentialAsync_CredentialDoesNotExist_Returns
{
["protocol"] = "https",
["host"] = "example.com",
["username"] = userName,
["password"] = password, // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
});

bool generateWasCalled = false;
@@ -73,6 +75,49 @@ public async Task HostProvider_GetCredentialAsync_CredentialDoesNotExist_Returns
Assert.Equal(password, actualCredential.Password);
}

[Fact]
public async Task HostProvider_GetCredentialAsync_InvalidCredentialStored_ReturnsNewGeneratedCredential()
{
const string userName = "john.doe";
const string password = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
const string service = "https://example.com";
const string storedRefreshToken = "first";
const string refreshToken = "second";
var input = new InputArguments(new Dictionary<string, string>
{
["protocol"] = "https",
["host"] = "example.com",
});

bool generateWasCalled = false;
string refreshTokenSeenByGenerate = null;
var context = new TestCommandContext();
context.CredentialStore.Add(service, new TestCredential(service, "stored-user", "stored-password") { OAuthRefreshToken = storedRefreshToken});
var provider = new TestHostProvider(context)
{
ValidateCredentialFunc = (_, _) => false,
IsSupportedFunc = _ => true,
GenerateCredentialFunc = input =>
{
generateWasCalled = true;
refreshTokenSeenByGenerate = input.OAuthRefreshToken;
return new GitCredential(userName, password) {
OAuthRefreshToken = refreshToken,
};
},
};

ICredential actualCredential = await ((IHostProvider) provider).GetCredentialAsync(input);

Assert.True(generateWasCalled);
Assert.Equal(storedRefreshToken, refreshTokenSeenByGenerate);
Assert.Equal(userName, actualCredential.Account);
Assert.Equal(password, actualCredential.Password);
Assert.Equal(refreshToken, actualCredential.OAuthRefreshToken);
// Invalid credential should be erased
Assert.Equal(0, context.CredentialStore.Count);
}


#endregion

@@ -252,6 +297,18 @@ public async Task HostProvider_EraseCredentialAsync_DifferentHost_DoesNothing()
Assert.True(context.CredentialStore.Contains(service3, userName));
}

[Fact]
public void HostProvider_ValidateCredentialAsync()
{
var context = new TestCommandContext();
var provider = new TestHostProvider(context);
Assert.True(provider.ValidateCredentialAsync(null, new GitCredential("username", "pass")).Result);
Assert.True(provider.ValidateCredentialAsync(null, new GitCredential("username", "pass") {PasswordExpiry
= DateTimeOffset.UtcNow + TimeSpan.FromHours(1)}).Result);
Assert.False(provider.ValidateCredentialAsync(null, new GitCredential("username", "pass") {PasswordExpiry
= DateTimeOffset.UtcNow - TimeSpan.FromMinutes(1)}).Result);
}

#endregion
}
}
Original file line number Diff line number Diff line change
@@ -17,18 +17,22 @@ public void SecretServiceCollection_ReadWriteDelete()
string service = $"https://example.com/{Guid.NewGuid():N}";
const string userName = "john.doe";
const string password = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
const string testRefreshToken = "xyzzy";
DateTimeOffset testExpiry = DateTimeOffset.FromUnixTimeSeconds(1919539847);

try
{
// Write
collection.AddOrUpdate(service, userName, password);
collection.AddOrUpdate(service, new GitCredential(userName, password) { PasswordExpiry = testExpiry, OAuthRefreshToken = testRefreshToken});

// Read
ICredential outCredential = collection.Get(service, userName);

Assert.NotNull(outCredential);
Assert.Equal(userName, userName);
Assert.Equal(password, outCredential.Password);
Assert.Equal(testRefreshToken, outCredential.OAuthRefreshToken);
Assert.Equal(testExpiry, outCredential.PasswordExpiry);
}
finally
{
Original file line number Diff line number Diff line change
@@ -19,13 +19,14 @@ public void WindowsCredentialManager_ReadWriteDelete()
string service = $"https://example.com/{uniqueGuid}";
const string userName = "john.doe";
const string password = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
const string testRefreshToken = "xyzzy";

string expectedTargetName = $"{TestNamespace}:https://example.com/{uniqueGuid}";

try
{
// Write
credManager.AddOrUpdate(service, userName, password);
credManager.AddOrUpdate(service, new GitCredential(userName, password) { OAuthRefreshToken = testRefreshToken});

// Read
ICredential cred = credManager.Get(service, userName);
@@ -37,6 +38,7 @@ public void WindowsCredentialManager_ReadWriteDelete()
Assert.Equal(password, winCred.Password);
Assert.Equal(service, winCred.Service);
Assert.Equal(expectedTargetName, winCred.TargetName);
Assert.Equal(testRefreshToken, winCred.OAuthRefreshToken);
}
finally
{
6 changes: 5 additions & 1 deletion src/shared/Core/Commands/GetCommand.cs
Original file line number Diff line number Diff line change
@@ -35,9 +35,13 @@ protected override async Task ExecuteInternalAsync(InputArguments input, IHostPr
// Return the credential to Git
output["username"] = credential.Account;
output["password"] = credential.Password;
if (credential.PasswordExpiry.HasValue)
output["password_expiry_utc"] = credential.PasswordExpiry.Value.ToUnixTimeSeconds().ToString();
if (!string.IsNullOrEmpty(credential.OAuthRefreshToken))
output["oauth_refresh_token"] = credential.OAuthRefreshToken;

Context.Trace.WriteLine("Writing credentials to output:");
Context.Trace.WriteDictionarySecrets(output, new []{ "password" }, StringComparer.OrdinalIgnoreCase);
Context.Trace.WriteDictionarySecrets(output, new []{ "password", "oauth_refresh_token" }, StringComparer.OrdinalIgnoreCase);

// Write the values to standard out
Context.Streams.Out.WriteDictionary(output);
2 changes: 1 addition & 1 deletion src/shared/Core/Commands/GitCommandBase.cs
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ internal async Task ExecuteAsync()

// Determine the host provider
Context.Trace.WriteLine("Detecting host provider for input:");
Context.Trace.WriteDictionarySecrets(inputDict, new []{ "password" }, StringComparer.OrdinalIgnoreCase);
Context.Trace.WriteDictionarySecrets(inputDict, new []{ "password", "oauth_refresh_token" }, StringComparer.OrdinalIgnoreCase);
IHostProvider provider = await _hostProviderRegistry.GetProviderAsync(input);
Context.Trace.WriteLine($"Host provider '{provider.Name}' was selected.");

45 changes: 42 additions & 3 deletions src/shared/Core/Credential.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@

using System;
using GitCredentialManager.Authentication.OAuth;

namespace GitCredentialManager
{
/// <summary>
@@ -15,21 +18,57 @@ public interface ICredential
/// Password.
/// </summary>
string Password { get; }

/// <summary>
/// The expiry date of the password. This is Git's password_expiry_utc
/// attribute. https://git-scm.com/docs/git-credential#Documentation/git-credential.txt-codepasswordexpiryutccode
/// </summary>
DateTimeOffset? PasswordExpiry { get => null; }

/// <summary>
/// An OAuth refresh token. This is Git's oauth_refresh_token
/// attribute. https://git-scm.com/docs/git-credential#Documentation/git-credential.txt-codeoauthrefreshtokencode
/// </summary>
string OAuthRefreshToken { get => null; }
}

/// <summary>
/// Represents a credential (username/password pair) that Git can use to authenticate to a remote repository.
/// </summary>
public class GitCredential : ICredential
public record GitCredential : ICredential
{
public GitCredential(string userName, string password)
{
Account = userName;
Password = password;
}

public string Account { get; }
public GitCredential(InputArguments input)
{
Account = input.UserName;
Password = input.Password;
OAuthRefreshToken = input.OAuthRefreshToken;
if (long.TryParse(input.PasswordExpiry, out long x)) {
PasswordExpiry = DateTimeOffset.FromUnixTimeSeconds(x);
}
}

public GitCredential(OAuth2TokenResult tokenResult, string userName)
{
Account = userName;
Password = tokenResult.AccessToken;
OAuthRefreshToken = tokenResult.RefreshToken;
if (tokenResult.ExpiresIn.HasValue) {
PasswordExpiry = DateTimeOffset.UtcNow + tokenResult.ExpiresIn.Value;
}
}

public string Account { get; init; }

public string Password { get; init; }

public string Password { get; }
public DateTimeOffset? PasswordExpiry { get; init; }

public string OAuthRefreshToken { get; init; }
}
}
Loading

0 comments on commit 2a14b66

Please sign in to comment.