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 Oct 26, 2023
1 parent 749e287 commit fdd7759
Show file tree
Hide file tree
Showing 36 changed files with 635 additions and 198 deletions.
28 changes: 9 additions & 19 deletions src/shared/Atlassian.Bitbucket.Tests/BitbucketHostProviderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,11 @@ public async Task BitbucketHostProvider_GetCredentialAsync_Valid_New_OAuth(

Assert.Equal(username, credential.Account);
Assert.Equal(accessToken, credential.Password);
Assert.Equal(refreshToken, credential.OAuthRefreshToken);

VerifyInteractiveAuthRan(input);
VerifyOAuthFlowRan(input, accessToken);
VerifyValidateAccessTokenRan(input, accessToken);
VerifyOAuthRefreshTokenStored(context, input, refreshToken);
}

[Theory]
Expand All @@ -234,12 +234,12 @@ public async Task BitbucketHostProvider_GetCredentialAsync_Valid_New_OAuth(
public async Task BitbucketHostProvider_GetCredentialAsync_MissingAT_OAuth_Refresh(
string protocol, string host, string username, string refreshToken, string accessToken)
{
var input = MockInput(protocol, host, username);
var input = MockInput(protocol, host, username, refreshToken);

var context = new TestCommandContext();

// AT has does not exist, but RT is still valid
MockStoredRefreshToken(context, input, refreshToken);
MockStoredAccount(context, input, null);
MockRemoteAccessTokenValid(input, accessToken);
MockRemoteRefreshTokenValid(input, refreshToken, accessToken);

Expand All @@ -261,15 +261,13 @@ public async Task BitbucketHostProvider_GetCredentialAsync_MissingAT_OAuth_Refre
public async Task BitbucketHostProvider_GetCredentialAsync_ExpiredAT_OAuth_Refresh(
string protocol, string host, string username, string refreshToken, string expiredAccessToken, string accessToken)
{
var input = MockInput(protocol, host, username);
var input = MockInput(protocol, host, username, refreshToken);

var context = new TestCommandContext();

// AT exists but has expired, but RT is still valid
MockStoredAccount(context, input, expiredAccessToken);
MockRemoteAccessTokenExpired(input, expiredAccessToken);

MockStoredRefreshToken(context, input, refreshToken);
MockRemoteAccessTokenValid(input, accessToken);
MockRemoteRefreshTokenValid(input, refreshToken, accessToken);

Expand All @@ -291,13 +289,12 @@ public async Task BitbucketHostProvider_GetCredentialAsync_ExpiredAT_OAuth_Refre
public async Task BitbucketHostProvider_GetCredentialAsync_PreconfiguredMode_OAuth_ValidRT_IsRespected(
string protocol, string host, string username, string refreshToken, string accessToken)
{
var input = MockInput(protocol, host, username);
var input = MockInput(protocol, host, username, refreshToken);

var context = new TestCommandContext();
context.Environment.Variables.Add(BitbucketConstants.EnvironmentVariables.AuthenticationModes, "oauth");

// We have a stored RT so we can just use that without any prompts
MockStoredRefreshToken(context, input, refreshToken);
MockRemoteAccessTokenValid(input, accessToken);
MockRemoteRefreshTokenValid(input, refreshToken, accessToken);

Expand All @@ -316,15 +313,14 @@ public async Task BitbucketHostProvider_GetCredentialAsync_PreconfiguredMode_OAu
public async Task BitbucketHostProvider_GetCredentialAsync_AlwaysRefreshCredentials_OAuth_IsRespected(
string protocol, string host, string username, string storedToken, string newToken, string refreshToken)
{
var input = MockInput(protocol, host, username);
var input = MockInput(protocol, host, username, refreshToken);

var context = new TestCommandContext();
context.Environment.Variables.Add(
BitbucketConstants.EnvironmentVariables.AlwaysRefreshCredentials, bool.TrueString);

// User has stored access token that we shouldn't use - RT should be used to mint new AT
MockStoredAccount(context, input, storedToken);
MockStoredRefreshToken(context, input, refreshToken);
MockRemoteAccessTokenValid(input, newToken);
MockRemoteRefreshTokenValid(input, refreshToken, newToken);

Expand Down Expand Up @@ -437,13 +433,14 @@ public async Task BitbucketHostProvider_EraseCredentialAsync(string protocol, st

#region Test helpers

private static InputArguments MockInput(string protocol, string host, string username)
private static InputArguments MockInput(string protocol, string host, string username, string refreshToken = null)
{
return new InputArguments(new Dictionary<string, string>
{
["protocol"] = protocol,
["host"] = host,
["username"] = username
["username"] = username,
["oauth_refresh_token"] = refreshToken,
});
}

Expand Down Expand Up @@ -551,13 +548,6 @@ private static void MockStoredAccount(TestCommandContext context, InputArguments
context.CredentialStore.Add(remoteUrl, new TestCredential(input.Host, input.UserName, password));
}

private static void MockStoredRefreshToken(TestCommandContext context, InputArguments input, string token)
{
var remoteUri = input.GetRemoteUri();
var refreshService = BitbucketHostProvider.GetRefreshTokenServiceName(remoteUri);
context.CredentialStore.Add(refreshService, new TestCredential(refreshService, input.UserName, token));
}

private void MockRemoteOAuthTokenCreate(InputArguments input, string accessToken, string refreshToken)
{
bitbucketAuthentication.Setup(x => x.CreateOAuthCredentialsAsync(input))
Expand Down
46 changes: 27 additions & 19 deletions src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace Atlassian.Bitbucket
{
// TODO: simplify and inherit from HostProvider
public class BitbucketHostProvider : IHostProvider
{
private readonly ICommandContext _context;
Expand Down Expand Up @@ -139,10 +140,11 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
var refreshTokenService = GetRefreshTokenServiceName(remoteUri);

_context.Trace.WriteLine("Checking for refresh token...");
ICredential refreshToken = SupportsOAuth(authModes)
? _context.CredentialStore.Get(refreshTokenService, input.UserName)
: null;

string refreshToken = input.OAuthRefreshToken;
if (!_context.CredentialStore.CanStoreOAuthRefreshToken && SupportsOAuth(authModes)) {
refreshToken ??= _context.CredentialStore.Get(refreshTokenService, input.UserName)?.Password;
}

if (refreshToken is null)
{
_context.Trace.WriteLine("No stored refresh token found");
Expand Down Expand Up @@ -199,26 +201,28 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
return await GetOAuthCredentialsViaInteractiveBrowserFlow(input);
}

private async Task<ICredential> GetOAuthCredentialsViaRefreshFlow(InputArguments input, ICredential refreshToken)
private async Task<ICredential> GetOAuthCredentialsViaRefreshFlow(InputArguments input, string refreshToken)
{
Uri remoteUri = input.GetRemoteUri();

var refreshTokenService = GetRefreshTokenServiceName(remoteUri);
_context.Trace.WriteLine("Refreshing OAuth credentials using refresh token...");

OAuth2TokenResult refreshResult = await _bitbucketAuth.RefreshOAuthCredentialsAsync(input, refreshToken.Password);
OAuth2TokenResult oauthResult = await _bitbucketAuth.RefreshOAuthCredentialsAsync(input, refreshToken);

// Resolve the username
_context.Trace.WriteLine("Resolving username for refreshed OAuth credential...");
string refreshUserName = await ResolveOAuthUserNameAsync(input, refreshResult.AccessToken);
_context.Trace.WriteLine($"Username for refreshed OAuth credential is '{refreshUserName}'");
string newUserName = await ResolveOAuthUserNameAsync(input, oauthResult.AccessToken);
_context.Trace.WriteLine($"Username for refreshed OAuth credential is '{newUserName}'");

// Store the refreshed RT
_context.Trace.WriteLine("Storing new refresh token...");
_context.CredentialStore.AddOrUpdate(refreshTokenService, remoteUri.GetUserName(), refreshResult.RefreshToken);
if (!_context.CredentialStore.CanStoreOAuthRefreshToken) {
// Store the refreshed RT
_context.Trace.WriteLine("Storing new refresh token...");
_context.CredentialStore.AddOrUpdate(refreshTokenService, remoteUri.GetUserName(), oauthResult.RefreshToken);
}

// Return new access token
return new GitCredential(refreshUserName, refreshResult.AccessToken);
return new GitCredential(oauthResult, newUserName);
}

private async Task<ICredential> GetOAuthCredentialsViaInteractiveBrowserFlow(InputArguments input)
Expand All @@ -239,13 +243,15 @@ private async Task<ICredential> GetOAuthCredentialsViaInteractiveBrowserFlow(Inp
string newUserName = await ResolveOAuthUserNameAsync(input, oauthResult.AccessToken);
_context.Trace.WriteLine($"Username for OAuth credential is '{newUserName}'");

// Store the new RT
_context.Trace.WriteLine("Storing new refresh token...");
_context.CredentialStore.AddOrUpdate(refreshTokenService, newUserName, oauthResult.RefreshToken);
_context.Trace.WriteLine("Refresh token was successfully stored.");
if (!_context.CredentialStore.CanStoreOAuthRefreshToken) {
// Store the new RT
_context.Trace.WriteLine("Storing new refresh token...");
_context.CredentialStore.AddOrUpdate(refreshTokenService, newUserName, oauthResult.RefreshToken);
_context.Trace.WriteLine("Refresh token was successfully stored.");
}

// Return the new AT as the credential
return new GitCredential(newUserName, oauthResult.AccessToken);
return new GitCredential(oauthResult, newUserName);
}

private static bool SupportsOAuth(AuthenticationModes authModes)
Expand Down Expand Up @@ -333,7 +339,7 @@ public Task StoreCredentialAsync(InputArguments input)
string service = GetServiceName(remoteUri);

_context.Trace.WriteLine("Storing credential...");
_context.CredentialStore.AddOrUpdate(service, input.UserName, input.Password);
_context.CredentialStore.AddOrUpdate(service, new GitCredential(input));
_context.Trace.WriteLine("Credential was successfully stored.");

return Task.CompletedTask;
Expand Down Expand Up @@ -450,7 +456,7 @@ private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredenti
return true;
}

private static string GetServiceName(Uri remoteUri)
internal static string GetServiceName(Uri remoteUri)
{
return remoteUri.WithoutUserInfo().AbsoluteUri.TrimEnd('/');
}
Expand All @@ -473,5 +479,7 @@ public void Dispose()
_restApiRegistry.Dispose();
_bitbucketAuth.Dispose();
}

public Task<bool> ValidateCredentialAsync(Uri remoteUri, ICredential credential) => Task.FromResult(credential.PasswordExpiry == null || credential.PasswordExpiry >= DateTimeOffset.UtcNow);
}
}
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
Loading

0 comments on commit fdd7759

Please sign in to comment.