Skip to content

Commit 60224e0

Browse files
hickfordhickford
committed
introduce PasswordExpiryUTC and OAuthRefreshToken
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.
1 parent 17250be commit 60224e0

36 files changed

+636
-195
lines changed

‎src/shared/Atlassian.Bitbucket.Tests/BitbucketHostProviderTest.cs‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,11 @@ public async Task BitbucketHostProvider_GetCredentialAsync_Valid_New_OAuth(
221221

222222
Assert.Equal(username, credential.Account);
223223
Assert.Equal(accessToken, credential.Password);
224+
Assert.Equal(refreshToken, credential.OAuthRefreshToken);
224225

225226
VerifyInteractiveAuthRan(input);
226227
VerifyOAuthFlowRan(input, accessToken);
227228
VerifyValidateAccessTokenRan(input, accessToken);
228-
VerifyOAuthRefreshTokenStored(context, input, refreshToken);
229229
}
230230

231231
[Theory]
@@ -234,12 +234,12 @@ public async Task BitbucketHostProvider_GetCredentialAsync_Valid_New_OAuth(
234234
public async Task BitbucketHostProvider_GetCredentialAsync_MissingAT_OAuth_Refresh(
235235
string protocol, string host, string username, string refreshToken, string accessToken)
236236
{
237-
var input = MockInput(protocol, host, username);
237+
var input = MockInput(protocol, host, username, refreshToken);
238238

239239
var context = new TestCommandContext();
240240

241241
// AT has does not exist, but RT is still valid
242-
MockStoredRefreshToken(context, input, refreshToken);
242+
MockStoredAccount(context, input, null);
243243
MockRemoteAccessTokenValid(input, accessToken);
244244
MockRemoteRefreshTokenValid(input, refreshToken, accessToken);
245245

@@ -261,15 +261,13 @@ public async Task BitbucketHostProvider_GetCredentialAsync_MissingAT_OAuth_Refre
261261
public async Task BitbucketHostProvider_GetCredentialAsync_ExpiredAT_OAuth_Refresh(
262262
string protocol, string host, string username, string refreshToken, string expiredAccessToken, string accessToken)
263263
{
264-
var input = MockInput(protocol, host, username);
264+
var input = MockInput(protocol, host, username, refreshToken);
265265

266266
var context = new TestCommandContext();
267267

268268
// AT exists but has expired, but RT is still valid
269269
MockStoredAccount(context, input, expiredAccessToken);
270270
MockRemoteAccessTokenExpired(input, expiredAccessToken);
271-
272-
MockStoredRefreshToken(context, input, refreshToken);
273271
MockRemoteAccessTokenValid(input, accessToken);
274272
MockRemoteRefreshTokenValid(input, refreshToken, accessToken);
275273

@@ -291,13 +289,12 @@ public async Task BitbucketHostProvider_GetCredentialAsync_ExpiredAT_OAuth_Refre
291289
public async Task BitbucketHostProvider_GetCredentialAsync_PreconfiguredMode_OAuth_ValidRT_IsRespected(
292290
string protocol, string host, string username, string refreshToken, string accessToken)
293291
{
294-
var input = MockInput(protocol, host, username);
292+
var input = MockInput(protocol, host, username, refreshToken);
295293

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

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

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

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

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

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

438434
#region Test helpers
439435

440-
private static InputArguments MockInput(string protocol, string host, string username)
436+
private static InputArguments MockInput(string protocol, string host, string username, string refreshToken = null)
441437
{
442438
return new InputArguments(new Dictionary<string, string>
443439
{
444440
["protocol"] = protocol,
445441
["host"] = host,
446-
["username"] = username
442+
["username"] = username,
443+
["oauth_refresh_token"] = refreshToken,
447444
});
448445
}
449446

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

554-
private static void MockStoredRefreshToken(TestCommandContext context, InputArguments input, string token)
555-
{
556-
var remoteUri = input.GetRemoteUri();
557-
var refreshService = BitbucketHostProvider.GetRefreshTokenServiceName(remoteUri);
558-
context.CredentialStore.Add(refreshService, new TestCredential(refreshService, input.UserName, token));
559-
}
560-
561551
private void MockRemoteOAuthTokenCreate(InputArguments input, string accessToken, string refreshToken)
562552
{
563553
bitbucketAuthentication.Setup(x => x.CreateOAuthCredentialsAsync(input))

‎src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace Atlassian.Bitbucket
1111
{
12+
// TODO: simplify and inherit from HostProvider
1213
public class BitbucketHostProvider : IHostProvider
1314
{
1415
private readonly ICommandContext _context;
@@ -139,9 +140,10 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
139140
var refreshTokenService = GetRefreshTokenServiceName(remoteUri);
140141

141142
_context.Trace.WriteLine("Checking for refresh token...");
142-
ICredential refreshToken = SupportsOAuth(authModes)
143-
? _context.CredentialStore.Get(refreshTokenService, input.UserName)
144-
: null;
143+
string refreshToken = input.OAuthRefreshToken;
144+
if (!_context.CredentialStore.CanStoreOAuthRefreshToken && SupportsOAuth(authModes)) {
145+
refreshToken ??= _context.CredentialStore.Get(refreshTokenService, input.UserName)?.Password;
146+
}
145147

146148
if (refreshToken is null)
147149
{
@@ -199,26 +201,28 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
199201
return await GetOAuthCredentialsViaInteractiveBrowserFlow(input);
200202
}
201203

202-
private async Task<ICredential> GetOAuthCredentialsViaRefreshFlow(InputArguments input, ICredential refreshToken)
204+
private async Task<ICredential> GetOAuthCredentialsViaRefreshFlow(InputArguments input, string refreshToken)
203205
{
204206
Uri remoteUri = input.GetRemoteUri();
205207

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

209-
OAuth2TokenResult refreshResult = await _bitbucketAuth.RefreshOAuthCredentialsAsync(input, refreshToken.Password);
211+
OAuth2TokenResult oauthResult = await _bitbucketAuth.RefreshOAuthCredentialsAsync(input, refreshToken);
210212

211213
// Resolve the username
212214
_context.Trace.WriteLine("Resolving username for refreshed OAuth credential...");
213-
string refreshUserName = await ResolveOAuthUserNameAsync(input, refreshResult.AccessToken);
214-
_context.Trace.WriteLine($"Username for refreshed OAuth credential is '{refreshUserName}'");
215+
string newUserName = await ResolveOAuthUserNameAsync(input, oauthResult.AccessToken);
216+
_context.Trace.WriteLine($"Username for refreshed OAuth credential is '{newUserName}'");
215217

216-
// Store the refreshed RT
217-
_context.Trace.WriteLine("Storing new refresh token...");
218-
_context.CredentialStore.AddOrUpdate(refreshTokenService, remoteUri.GetUserName(), refreshResult.RefreshToken);
218+
if (!_context.CredentialStore.CanStoreOAuthRefreshToken) {
219+
// Store the refreshed RT
220+
_context.Trace.WriteLine("Storing new refresh token...");
221+
_context.CredentialStore.AddOrUpdate(refreshTokenService, remoteUri.GetUserName(), oauthResult.RefreshToken);
222+
}
219223

220224
// Return new access token
221-
return new GitCredential(refreshUserName, refreshResult.AccessToken);
225+
return new GitCredential(oauthResult, newUserName);
222226
}
223227

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

242-
// Store the new RT
243-
_context.Trace.WriteLine("Storing new refresh token...");
244-
_context.CredentialStore.AddOrUpdate(refreshTokenService, newUserName, oauthResult.RefreshToken);
245-
_context.Trace.WriteLine("Refresh token was successfully stored.");
246+
if (!_context.CredentialStore.CanStoreOAuthRefreshToken) {
247+
// Store the new RT
248+
_context.Trace.WriteLine("Storing new refresh token...");
249+
_context.CredentialStore.AddOrUpdate(refreshTokenService, newUserName, oauthResult.RefreshToken);
250+
_context.Trace.WriteLine("Refresh token was successfully stored.");
251+
}
246252

247253
// Return the new AT as the credential
248-
return new GitCredential(newUserName, oauthResult.AccessToken);
254+
return new GitCredential(oauthResult, newUserName);
249255
}
250256

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

335341
_context.Trace.WriteLine("Storing credential...");
336-
_context.CredentialStore.AddOrUpdate(service, input.UserName, input.Password);
342+
_context.CredentialStore.AddOrUpdate(service, new GitCredential(input));
337343
_context.Trace.WriteLine("Credential was successfully stored.");
338344

339345
return Task.CompletedTask;
@@ -450,7 +456,7 @@ private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredenti
450456
return true;
451457
}
452458

453-
private static string GetServiceName(Uri remoteUri)
459+
internal static string GetServiceName(Uri remoteUri)
454460
{
455461
return remoteUri.WithoutUserInfo().AbsoluteUri.TrimEnd('/');
456462
}
@@ -473,5 +479,7 @@ public void Dispose()
473479
_restApiRegistry.Dispose();
474480
_bitbucketAuth.Dispose();
475481
}
482+
483+
public Task<bool> ValidateCredentialAsync(Uri remoteUri, ICredential credential) => Task.FromResult(credential.PasswordExpiry == null || credential.PasswordExpiry >= DateTimeOffset.UtcNow);
476484
}
477485
}

‎src/shared/Core.Tests/Commands/GetCommandTests.cs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.IO;
34
using System.Text;
@@ -16,14 +17,21 @@ public async Task GetCommand_ExecuteAsync_CallsHostProviderAndWritesCredential()
1617
{
1718
const string testUserName = "john.doe";
1819
const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
19-
ICredential testCredential = new GitCredential(testUserName, testPassword);
20+
const string testRefreshToken = "xyzzy";
21+
const long testExpiry = 1919539847;
22+
ICredential testCredential = new GitCredential(testUserName, testPassword) {
23+
OAuthRefreshToken = testRefreshToken,
24+
PasswordExpiry = DateTimeOffset.FromUnixTimeSeconds(testExpiry),
25+
};
2026
var stdin = $"protocol=http\nhost=example.com\n\n";
2127
var expectedStdOutDict = new Dictionary<string, string>
2228
{
2329
["protocol"] = "http",
2430
["host"] = "example.com",
2531
["username"] = testUserName,
26-
["password"] = testPassword
32+
["password"] = testPassword,
33+
["password_expiry_utc"] = testExpiry.ToString(),
34+
["oauth_refresh_token"] = testRefreshToken,
2735
};
2836

2937
var providerMock = new Mock<IHostProvider>();

‎src/shared/Core.Tests/Commands/StoreCommandTests.cs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,17 @@ public async Task StoreCommand_ExecuteAsync_CallsHostProvider()
1313
{
1414
const string testUserName = "john.doe";
1515
const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
16-
var stdin = $"protocol=http\nhost=example.com\nusername={testUserName}\npassword={testPassword}\n\n";
16+
const string testRefreshToken = "xyzzy";
17+
const long testExpiry = 1919539847;
18+
var stdin = $"protocol=http\nhost=example.com\nusername={testUserName}\npassword={testPassword}\noauth_refresh_token={testRefreshToken}\npassword_expiry_utc={testExpiry}\n\n";
1719
var expectedInput = new InputArguments(new Dictionary<string, string>
1820
{
1921
["protocol"] = "http",
2022
["host"] = "example.com",
2123
["username"] = testUserName,
22-
["password"] = testPassword
24+
["password"] = testPassword,
25+
["oauth_refresh_token"] = testRefreshToken,
26+
["password_expiry_utc"] = testExpiry.ToString(),
2327
});
2428

2529
var providerMock = new Mock<IHostProvider>();
@@ -46,7 +50,9 @@ bool AreInputArgumentsEquivalent(InputArguments a, InputArguments b)
4650
a.Host == b.Host &&
4751
a.Path == b.Path &&
4852
a.UserName == b.UserName &&
49-
a.Password == b.Password;
53+
a.Password == b.Password &&
54+
a.OAuthRefreshToken == b.OAuthRefreshToken &&
55+
a.PasswordExpiry == b.PasswordExpiry;
5056
}
5157
}
5258
}

‎src/shared/Core.Tests/GenericHostProviderTests.cs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ public async Task GenericHostProvider_GenerateCredentialAsync_OAuth_CompleteOAut
201201
const string testAcessToken = "OAUTH_TOKEN";
202202
const string testRefreshToken = "OAUTH_REFRESH_TOKEN";
203203
const string testResource = "https://git.example.com/foo";
204-
const string expectedRefreshTokenService = "https://refresh_token.git.example.com/foo";
205204

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

255254
var provider = new GenericHostProvider(context, basicAuthMock.Object, wiaAuthMock.Object, oauthMock.Object);
@@ -259,10 +258,7 @@ public async Task GenericHostProvider_GenerateCredentialAsync_OAuth_CompleteOAut
259258
Assert.NotNull(credential);
260259
Assert.Equal(testUserName, credential.Account);
261260
Assert.Equal(testAcessToken, credential.Password);
262-
263-
Assert.True(context.CredentialStore.TryGet(expectedRefreshTokenService, null, out TestCredential refreshToken));
264-
Assert.Equal(testUserName, refreshToken.Account);
265-
Assert.Equal(testRefreshToken, refreshToken.Password);
261+
Assert.Equal(testRefreshToken, credential.OAuthRefreshToken);
266262

267263
oauthMock.Verify(x => x.GetAuthenticationModeAsync(testResource, OAuthAuthenticationModes.All), Times.Once);
268264
oauthMock.Verify(x => x.GetTokenByBrowserAsync(It.IsAny<OAuth2Client>(), scopes), Times.Once);

0 commit comments

Comments
 (0)