Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Commit 3e3ba0c

Browse files
StanleyGoldmanStanleyGoldman
committed
Adding more Keychain unit tests
1 parent 2503a0b commit 3e3ba0c

File tree

8 files changed

+194
-72
lines changed

8 files changed

+194
-72
lines changed

‎src/GitHub.Api/Authentication/IKeychain.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@ namespace GitHub.Unity
55
{
66
interface IKeychain
77
{
8-
KeychainAdapter Connect(UriString host);
9-
Task<KeychainAdapter> Load(UriString host);
10-
void Clear(UriString host);
11-
void Clear();
12-
Task Flush(UriString host);
8+
IKeychainAdapter Connect(UriString host);
9+
Task<IKeychainAdapter> Load(UriString host);
10+
Task Clear(UriString host, bool deleteFromCredentialManager);
11+
Task Save(UriString host);
1312
void UpdateToken(UriString host, string token);
14-
void Save(ICredential credential);
13+
void SetCredentials(ICredential credential);
1514
void Initialize();
1615
IList<UriString> Connections { get; }
1716
bool HasKeys { get; }
17+
void SetToken(UriString host, string token);
1818
}
1919
}

‎src/GitHub.Api/Authentication/Keychain.cs

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -58,31 +58,27 @@ public Keychain(IEnvironment environment, IFileSystem fileSystem, ICredentialMan
5858
this.credentialManager = credentialManager;
5959
}
6060

61-
public KeychainAdapter Connect(UriString host)
61+
public IKeychainAdapter Connect(UriString host)
6262
{
6363
return FindOrCreateAdapter(host);
6464
}
6565

66-
public async Task<KeychainAdapter> Load(UriString host)
66+
public async Task<IKeychainAdapter> Load(UriString host)
6767
{
68-
logger.Trace("Load KeychainAdapter Host:\"{0}\"", host);
69-
7068
var keychainAdapter = FindOrCreateAdapter(host);
7169

72-
var keychainAdapterCredential = keychainAdapter.Credential;
73-
if (keychainAdapterCredential == null)
74-
{
75-
throw new NullReferenceException($"{nameof(keychainAdapterCredential)} is null");
76-
}
77-
78-
logger.Trace("Load KeychainAdapter Host:\"{0}\" Username:\"{1}\"", keychainAdapterCredential.Host ?? "NULL",
79-
keychainAdapterCredential.Username ?? "NULL");
80-
70+
logger.Trace("Load KeychainAdapter Host:\"{0}\"", host);
8171
var keychainItem = await credentialManager.Load(host);
8272

83-
logger.Trace("Loading KeychainItem:{0}", keychainItem?.ToString() ?? "NULL");
84-
85-
keychainAdapter.Set(keychainItem);
73+
if (keychainItem == null)
74+
{
75+
await Clear(host, false);
76+
}
77+
else
78+
{
79+
logger.Trace("Loading KeychainItem:{0}", keychainItem?.ToString() ?? "NULL");
80+
keychainAdapter.Set(keychainItem);
81+
}
8682

8783
return keychainAdapter;
8884
}
@@ -145,7 +141,7 @@ private void WriteCacheToDisk()
145141
var connectionCacheItems =
146142
connectionCache.Select(
147143
pair =>
148-
new ConnectionCacheItem() {
144+
new ConnectionCacheItem {
149145
Host = pair.Value.Host.ToString(),
150146
Username = pair.Value.Username
151147
}).ToArray();
@@ -154,35 +150,27 @@ private void WriteCacheToDisk()
154150
fileSystem.WriteAllText(cachePath, json);
155151
}
156152

157-
/// <summary>
158-
/// Call Flush() to apply these changes
159-
/// </summary>
160-
public void Clear(UriString host)
153+
public async Task Clear(UriString host, bool deleteFromCredentialManager)
161154
{
162155
logger.Trace("Clear Host:{0}", host);
163-
156+
164157
// delete connection in the connection list
165158
connectionCache.Remove(host);
166159

167-
// delete credential in octokit store
160+
//clear octokit credentials
168161
FindOrCreateAdapter(host).Clear();
169-
}
170162

171-
/// <summary>
172-
/// Call Flush() to apply these changes
173-
/// </summary>
174-
public void Clear()
175-
{
176-
foreach (var k in keychainAdapters.Values.ToArray())
163+
WriteCacheToDisk();
164+
165+
if (deleteFromCredentialManager)
177166
{
178-
k.Clear();
167+
await credentialManager.Delete(host);
179168
}
180-
connectionCache.Clear();
181169
}
182170

183-
public async Task Flush(UriString host)
171+
public async Task Save(UriString host)
184172
{
185-
logger.Trace("Flush: {0}", host);
173+
logger.Trace("Save: {0}", host);
186174

187175
KeychainAdapter credentialAdapter;
188176
if (!keychainAdapters.TryGetValue(host, out credentialAdapter))
@@ -198,6 +186,7 @@ public async Task Flush(UriString host)
198186
// create new connection in the connection cache for this host
199187
if (connectionCache.ContainsKey(host))
200188
connectionCache.Remove(host);
189+
201190
connectionCache.Add(host, new Connection { Host = host, Username = credentialAdapter.OctokitCredentials.Login });
202191

203192
// flushes credential cache to disk (host and username only)
@@ -208,14 +197,22 @@ public async Task Flush(UriString host)
208197
await credentialManager.Save(credentialAdapter.Credential);
209198
}
210199

211-
public void Save(ICredential credential)
200+
public void SetCredentials(ICredential credential)
212201
{
213-
logger.Trace("Save Host:{0}", credential.Host);
202+
logger.Trace("SetCredentials Host:{0}", credential.Host);
214203

215204
var credentialAdapter = FindOrCreateAdapter(credential.Host);
216205
credentialAdapter.Set(credential);
217206
}
218207

208+
public void SetToken(UriString host, string token)
209+
{
210+
logger.Trace("SetToken Host:{0}", host);
211+
212+
var credentialAdapter = FindOrCreateAdapter(host);
213+
credentialAdapter.UpdateToken(token);
214+
}
215+
219216
public void UpdateToken(UriString host, string token)
220217
{
221218
logger.Trace("UpdateToken Host:{0}", host);

‎src/GitHub.Api/Authentication/KeychainAdapter.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
namespace GitHub.Unity
55
{
6-
class KeychainAdapter : ICredentialStore
6+
class KeychainAdapter : IKeychainAdapter
77
{
88
public Credentials OctokitCredentials { get; private set; } = Credentials.Anonymous;
99
public ICredential Credential { get; private set; }
@@ -33,4 +33,10 @@ Task<Credentials> ICredentialStore.GetCredentials()
3333
return TaskEx.FromResult(OctokitCredentials);
3434
}
3535
}
36+
37+
interface IKeychainAdapter: ICredentialStore
38+
{
39+
Credentials OctokitCredentials { get; }
40+
ICredential Credential { get; }
41+
}
3642
}

‎src/GitHub.Api/Authentication/LoginManager.cs

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public async Task<LoginResultData> Login(
7272
// until an authorization token has been created and acquired:
7373
var keychainAdapter = keychain.Connect(host);
7474
var keychainItem = new Credential(host, username, password);
75-
keychainAdapter.Set(keychainItem);
75+
keychain.SetCredentials(keychainItem);
7676

7777
var newAuth = new NewAuthorization
7878
{
@@ -110,15 +110,15 @@ public async Task<LoginResultData> Login(
110110
{
111111
logger.Warning(e, "Login LoginAttemptsExceededException: {0}", e.Message);
112112

113-
keychain.Clear(host);
113+
await keychain.Clear(host, false);
114114
return new LoginResultData(LoginResultCodes.LockedOut, Localization.LockedOut, host);
115115
}
116116
catch (ApiValidationException e)
117117
{
118118
logger.Warning(e, "Login ApiValidationException: {0}", e.Message);
119119

120120
var message = e.ApiError.FirstErrorMessageSafe();
121-
keychain.Clear(host);
121+
await keychain.Clear(host, false);
122122
return new LoginResultData(LoginResultCodes.Failed, message, host);
123123
}
124124
catch (Exception e)
@@ -134,13 +134,13 @@ public async Task<LoginResultData> Login(
134134
}
135135
else
136136
{
137-
keychain.Clear(host);
137+
await keychain.Clear(host, false);
138138
return new LoginResultData(LoginResultCodes.Failed, Localization.LoginFailed, host);
139139
}
140140
}
141141

142-
keychainAdapter.UpdateToken(auth.Token);
143-
await keychain.Flush(host);
142+
keychain.SetToken(host, auth.Token);
143+
await keychain.Save(host);
144144

145145
return new LoginResultData(LoginResultCodes.Success, "Success", host);
146146
}
@@ -161,10 +161,8 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
161161
twofacode);
162162
EnsureNonNullAuthorization(auth);
163163

164-
165-
var keychainAdapter = keychain.Connect(host);
166-
keychainAdapter.UpdateToken(auth.Token);
167-
await keychain.Flush(host);
164+
keychain.SetToken(host, auth.Token);
165+
await keychain.Save(host);
168166

169167
return new LoginResultData(LoginResultCodes.Success, "", host);
170168
}
@@ -179,14 +177,14 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
179177
logger.Debug(e, "2FA ApiValidationException: {0}", e.Message);
180178

181179
var message = e.ApiError.FirstErrorMessageSafe();
182-
keychain.Clear(host);
180+
await keychain.Clear(host, false);
183181
return new LoginResultData(LoginResultCodes.Failed, message, host);
184182
}
185183
catch (Exception e)
186184
{
187185
logger.Debug(e, "Exception: {0}", e.Message);
188186

189-
keychain.Clear(host);
187+
await keychain.Clear(host, false);
190188
return new LoginResultData(LoginResultCodes.Failed, e.Message, host);
191189
}
192190
}
@@ -198,8 +196,7 @@ public async Task Logout(UriString hostAddress)
198196

199197
logger.Trace("Logout");
200198

201-
keychain.Clear(hostAddress);
202-
await keychain.Flush(hostAddress);
199+
await keychain.Clear(hostAddress, true);
203200
}
204201

205202
private async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthorization(

‎src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
using System;
44
using System.Linq;
5+
using System.Threading.Tasks;
56
using UnityEditor;
67
using UnityEngine;
78

@@ -222,18 +223,10 @@ private void GoToProfile(object obj)
222223
}
223224
private void SignOut(object obj)
224225
{
225-
var credentialManager = Platform.CredentialManager;
226-
var cachedCredentialsHost = credentialManager.CachedCredentials.Host;
227-
228-
new ActionTask(credentialManager.Delete(cachedCredentialsHost))
229-
.Then(s =>
230-
{
231-
if (s)
232-
{
233-
Platform.Keychain.Clear(Repository.CloneUrl.ToRepositoryUrl());
234-
Platform.Keychain.Flush(Repository.CloneUrl.ToRepositoryUrl());
235-
}
236-
}).Start();
226+
var uriString = Platform.Keychain.Connections.First();
227+
228+
new ActionTask(Platform.Keychain.Clear(uriString, true))
229+
.Start();
237230
}
238231

239232
private bool ValidateSettings()

‎src/tests/TestUtils/Helpers/Args.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ static class Args
1111
public static string String { get { return Arg.Any<string>(); } }
1212
public static bool Bool { get { return Arg.Any<bool>(); } }
1313
public static int Int { get { return Arg.Any<int>(); } }
14+
public static UriString UriString { get { return Arg.Any<UriString>(); } }
1415
public static SearchOption SearchOption { get { return Arg.Any<SearchOption>(); } }
1516
public static GitFileStatus GitFileStatus { get { return Arg.Any<GitFileStatus>(); } }
1617
public static GitConfigSource GitConfigSource { get { return Arg.Any<GitConfigSource>(); } }
1718
public static GitStatus GitStatus { get { return Arg.Any<GitStatus>(); } }
1819
public static IEnumerable<GitLock> EnumerableGitLock { get { return Arg.Any<IEnumerable<GitLock>>(); } }
20+
1921
public static ITask<GitStatus?> GitStatusTask
2022
{
2123
get
@@ -34,5 +36,6 @@ public static ITask<List<GitLock>> GitListLocksTask
3436
return task;
3537
}
3638
}
39+
3740
}
3841
}

0 commit comments

Comments
 (0)