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 Feb 22, 2024
1 parent 541712a commit b59547b
Show file tree
Hide file tree
Showing 24 changed files with 465 additions and 157 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;
Expand All @@ -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>();
Expand Down
12 changes: 9 additions & 3 deletions src/shared/Core.Tests/Commands/StoreCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand All @@ -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
Expand Up @@ -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" };
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
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;
Expand All @@ -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,
Expand All @@ -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]
Expand All @@ -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;
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
{
Expand Down
6 changes: 5 additions & 1 deletion src/shared/Core/Commands/GetCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Core/Commands/GitCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");

Expand Down
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>
Expand All @@ -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; }

Check failure on line 26 in src/shared/Core/Credential.cs

View workflow job for this annotation

GitHub Actions / Windows

Target runtime doesn't support default interface implementation.

/// <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; }

Check failure on line 32 in src/shared/Core/Credential.cs

View workflow job for this annotation

GitHub Actions / Windows

Target runtime doesn't support default interface implementation.
}

/// <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; }

Check failure on line 66 in src/shared/Core/Credential.cs

View workflow job for this annotation

GitHub Actions / Windows

Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported

public string Password { get; init; }

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

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

0 comments on commit b59547b

Please sign in to comment.