Skip to content

Commit 6f51fdb

Browse files
mjcheethammjcheetham
committed
Refactor to allow HostProviders to override store and erase
Refactor the core commands and host provider types to permit providers to customise how they store and erase credentials. This would allow for providers that do not want to use the OS credential store for example.
1 parent 37a7c05 commit 6f51fdb

File tree

19 files changed

+552
-463
lines changed

19 files changed

+552
-463
lines changed

‎src/shared/GitHub.Tests/GitHubHostProviderTests.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void GitHubHostProvider_IsSupported_GitHubHost_UnencryptedHttp_ReturnsTru
2424
var provider = new GitHubHostProvider(new TestCommandContext());
2525

2626
// We report that we support unencrypted HTTP here so that we can fail and
27-
// show a helpful error message in the call to `CreateCredentialAsync` instead.
27+
// show a helpful error message in the call to `GenerateCredentialAsync` instead.
2828
Assert.True(provider.IsSupported(input));
2929
}
3030

@@ -40,7 +40,7 @@ public void GitHubHostProvider_IsSupported_GistHost_UnencryptedHttp_ReturnsTrue(
4040
var provider = new GitHubHostProvider(new TestCommandContext());
4141

4242
// We report that we support unencrypted HTTP here so that we can fail and
43-
// show a helpful error message in the call to `CreateCredentialAsync` instead.
43+
// show a helpful error message in the call to `GenerateCredentialAsync` instead.
4444
Assert.True(provider.IsSupported(input));
4545
}
4646

@@ -102,7 +102,7 @@ public void GitHubHostProvider_IsSupported_NonGitHub_ReturnsFalse()
102102
[Fact]
103103
public void GitHubHostProvider_GetCredentialKey_GitHubHost_ReturnsCorrectKey()
104104
{
105-
const string expectedKey = "https://github.com";
105+
const string expectedKey = "git:https://github.com";
106106
var input = new InputArguments(new Dictionary<string, string>
107107
{
108108
["protocol"] = "https",
@@ -117,7 +117,7 @@ public void GitHubHostProvider_GetCredentialKey_GitHubHost_ReturnsCorrectKey()
117117
[Fact]
118118
public void GitHubHostProvider_GetCredentialKey_GistHost_ReturnsCorrectKey()
119119
{
120-
const string expectedKey = "https://github.com";
120+
const string expectedKey = "git:https://github.com";
121121
var input = new InputArguments(new Dictionary<string, string>
122122
{
123123
["protocol"] = "https",
@@ -130,7 +130,7 @@ public void GitHubHostProvider_GetCredentialKey_GistHost_ReturnsCorrectKey()
130130
}
131131

132132
[Fact]
133-
public async Task GitHubHostProvider_CreateCredentialAsync_UnencryptedHttp_ThrowsException()
133+
public async Task GitHubHostProvider_GenerateCredentialAsync_UnencryptedHttp_ThrowsException()
134134
{
135135
var input = new InputArguments(new Dictionary<string, string>
136136
{
@@ -144,11 +144,11 @@ public async Task GitHubHostProvider_CreateCredentialAsync_UnencryptedHttp_Throw
144144

145145
var provider = new GitHubHostProvider(context, ghApi, ghAuth);
146146

147-
await Assert.ThrowsAsync<Exception>(() => provider.CreateCredentialAsync(input));
147+
await Assert.ThrowsAsync<Exception>(() => provider.GenerateCredentialAsync(input));
148148
}
149149

150150
[Fact]
151-
public async Task GitHubHostProvider_CreateCredentialAsync_1FAOnly_ReturnsCredential()
151+
public async Task GitHubHostProvider_GenerateCredentialAsync_1FAOnly_ReturnsCredential()
152152
{
153153
var input = new InputArguments(new Dictionary<string, string>
154154
{
@@ -181,15 +181,15 @@ public async Task GitHubHostProvider_CreateCredentialAsync_1FAOnly_ReturnsCreden
181181

182182
var provider = new GitHubHostProvider(context, ghApiMock.Object, ghAuthMock.Object);
183183

184-
GitCredential credential = await provider.CreateCredentialAsync(input);
184+
ICredential credential = await provider.GenerateCredentialAsync(input);
185185

186186
Assert.NotNull(credential);
187187
Assert.Equal(Constants.PersonalAccessTokenUserName, credential.UserName);
188188
Assert.Equal(patValue, credential.Password);
189189
}
190190

191191
[Fact]
192-
public async Task GitHubHostProvider_CreateCredentialAsync_2FARequired_ReturnsCredential()
192+
public async Task GitHubHostProvider_GenerateCredentialAsync_2FARequired_ReturnsCredential()
193193
{
194194
var input = new InputArguments(new Dictionary<string, string>
195195
{
@@ -228,7 +228,7 @@ public async Task GitHubHostProvider_CreateCredentialAsync_2FARequired_ReturnsCr
228228

229229
var provider = new GitHubHostProvider(context, ghApiMock.Object, ghAuthMock.Object);
230230

231-
GitCredential credential = await provider.CreateCredentialAsync(input);
231+
ICredential credential = await provider.GenerateCredentialAsync(input);
232232

233233
Assert.NotNull(credential);
234234
Assert.Equal(Constants.PersonalAccessTokenUserName, credential.UserName);

‎src/shared/GitHub/GitHubHostProvider.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ public override string GetCredentialKey(InputArguments input)
5151
// Trim trailing slash
5252
if (url.EndsWith("/"))
5353
{
54-
return url.Substring(0, url.Length - 1);
54+
url = url.Substring(0, url.Length - 1);
5555
}
5656

57-
return url;
57+
return $"git:{url}";
5858
}
5959

60-
public override async Task<GitCredential> CreateCredentialAsync(InputArguments input)
60+
public override async Task<ICredential> GenerateCredentialAsync(InputArguments input)
6161
{
6262
// We should not allow unencrypted communication and should inform the user
6363
if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http"))

‎src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs‎

Lines changed: 9 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -114,67 +114,7 @@ public void AzureReposProvider_IsSupported_NonAzureRepos_ReturnsFalse()
114114
}
115115

116116
[Fact]
117-
public void AzureReposProvider_GetCredentialKey_AzureHost_ReturnsCorrectKey()
118-
{
119-
const string expectedKey = "https://dev.azure.com/org";
120-
var input = new InputArguments(new Dictionary<string, string>
121-
{
122-
["protocol"] = "https",
123-
["host"] = "dev.azure.com",
124-
["path"] = "org/proj/_git/repo",
125-
});
126-
127-
var provider = new AzureReposHostProvider(new TestCommandContext());
128-
string actualKey = provider.GetCredentialKey(input);
129-
Assert.Equal(expectedKey, actualKey);
130-
}
131-
132-
[Fact]
133-
public void AzureReposProvider_GetCredentialKey_AzureHost_MissingPath_UseUserAsOrg()
134-
{
135-
const string expectedKey = "https://dev.azure.com/userorg";
136-
var input = new InputArguments(new Dictionary<string, string>
137-
{
138-
["protocol"] = "https",
139-
["host"] = "dev.azure.com",
140-
["username"] = "userorg",
141-
});
142-
143-
var provider = new AzureReposHostProvider(new TestCommandContext());
144-
string actualKey = provider.GetCredentialKey(input);
145-
Assert.Equal(expectedKey, actualKey);
146-
}
147-
148-
[Fact]
149-
public void AzureReposProvider_GetCredentialKey_AzureHost_MissingPathAndUser_ThrowsException()
150-
{
151-
var input = new InputArguments(new Dictionary<string, string>
152-
{
153-
["protocol"] = "https",
154-
["host"] = "dev.azure.com",
155-
});
156-
157-
var provider = new AzureReposHostProvider(new TestCommandContext());
158-
Assert.Throws<InvalidOperationException>(() => provider.GetCredentialKey(input));
159-
}
160-
161-
[Fact]
162-
public void AzureReposProvider_GetCredentialKey_VisualStudioHost_ReturnsCorrectKey()
163-
{
164-
const string expectedKey = "https://org.visualstudio.com/";
165-
var input = new InputArguments(new Dictionary<string, string>
166-
{
167-
["protocol"] = "https",
168-
["host"] = "org.visualstudio.com",
169-
});
170-
171-
var provider = new AzureReposHostProvider(new TestCommandContext());
172-
string actualKey = provider.GetCredentialKey(input);
173-
Assert.Equal(expectedKey, actualKey);
174-
}
175-
176-
[Fact]
177-
public async Task AzureReposProvider_CreateCredentialAsync_UnencryptedHttp_ThrowsException()
117+
public async Task AzureReposProvider_GetCredentialAsync_UnencryptedHttp_ThrowsException()
178118
{
179119
var input = new InputArguments(new Dictionary<string, string>
180120
{
@@ -189,11 +129,11 @@ public async Task AzureReposProvider_CreateCredentialAsync_UnencryptedHttp_Throw
189129

190130
var provider = new AzureReposHostProvider(context, azDevOps, msAuth);
191131

192-
await Assert.ThrowsAsync<Exception>(() => provider.CreateCredentialAsync(input));
132+
await Assert.ThrowsAsync<Exception>(() => provider.GetCredentialAsync(input));
193133
}
194134

195135
[Fact]
196-
public async Task AzureReposProvider_CreateCredentialAsync_ReturnsCredential()
136+
public async Task AzureReposProvider_GetCredentialAsync_ReturnsCredential()
197137
{
198138
var input = new InputArguments(new Dictionary<string, string>
199139
{
@@ -208,32 +148,27 @@ public async Task AzureReposProvider_CreateCredentialAsync_ReturnsCredential()
208148
var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri;
209149
var expectedResource = AzureDevOpsConstants.AadResourceId;
210150
var accessToken = "ACCESS-TOKEN";
211-
var pat = "PERSONAL-ACCESS-TOKEN";
212-
IEnumerable<string> expectedPatScopes = new[]
213-
{
214-
AzureDevOpsConstants.PersonalAccessTokenScopes.ReposWrite,
215-
AzureDevOpsConstants.PersonalAccessTokenScopes.ArtifactsRead
216-
};
151+
var personalAccessToken = "PERSONAL-ACCESS-TOKEN";
217152

218153
var context = new TestCommandContext();
219154

220155
var azDevOpsMock = new Mock<IAzureDevOpsRestApi>();
221156
azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri))
222157
.ReturnsAsync(authorityUrl);
223-
azDevOpsMock.Setup(x => x.CreatePersonalAccessTokenAsync(expectedOrgUri, accessToken, expectedPatScopes))
224-
.ReturnsAsync(pat);
158+
azDevOpsMock.Setup(x => x.CreatePersonalAccessTokenAsync(expectedOrgUri, accessToken, It.IsAny<IEnumerable<string>>()))
159+
.ReturnsAsync(personalAccessToken);
225160

226161
var msAuthMock = new Mock<IMicrosoftAuthentication>();
227162
msAuthMock.Setup(x => x.GetAccessTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedResource))
228163
.ReturnsAsync(accessToken);
229164

230165
var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object);
231166

232-
GitCredential credential = await provider.CreateCredentialAsync(input);
167+
ICredential credential = await provider.GetCredentialAsync(input);
233168

234169
Assert.NotNull(credential);
235-
Assert.Equal(Constants.PersonalAccessTokenUserName, credential.UserName);
236-
Assert.Equal(pat, credential.Password);
170+
Assert.Equal(personalAccessToken, credential.Password);
171+
// We don't care about the username value
237172
}
238173
}
239174
}

‎src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
33
using System;
4-
using System.Collections.Generic;
54
using System.Threading.Tasks;
65
using Microsoft.Git.CredentialManager;
76
using Microsoft.Git.CredentialManager.Authentication;
@@ -17,7 +16,7 @@ public AzureReposHostProvider(ICommandContext context)
1716
: this(context, new AzureDevOpsRestApi(context), new MicrosoftAuthentication(context)) { }
1817

1918
public AzureReposHostProvider(ICommandContext context, IAzureDevOpsRestApi azDevOps, IMicrosoftAuthentication msAuth)
20-
: base (context)
19+
: base(context)
2120
{
2221
EnsureArgument.NotNull(azDevOps, nameof(azDevOps));
2322
EnsureArgument.NotNull(msAuth, nameof(msAuth));
@@ -42,10 +41,10 @@ public override bool IsSupported(InputArguments input)
4241

4342
public override string GetCredentialKey(InputArguments input)
4443
{
45-
return UriHelpers.CreateOrganizationUri(input).AbsoluteUri;
44+
return $"git:{UriHelpers.CreateOrganizationUri(input).AbsoluteUri}";
4645
}
4746

48-
public override async Task<GitCredential> CreateCredentialAsync(InputArguments input)
47+
public override async Task<ICredential> GenerateCredentialAsync(InputArguments input)
4948
{
5049
// We should not allow unencrypted communication and should inform the user
5150
if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http"))

0 commit comments

Comments
 (0)