Skip to content

Commit fa94543

Browse files
SteveDesmond-caSteveDesmond-ca
committed
Add additional up-front auth validation
1 parent f856542 commit fa94543

8 files changed

Lines changed: 227 additions & 26 deletions

File tree

‎src/App.cs‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Security.Authentication;
33
using System.Threading.Tasks;
4+
using Octokit;
45

56
namespace GitHubLabelSync
67
{
@@ -22,13 +23,7 @@ public async Task Run(Settings settings)
2223
_setStatus($"Starting...");
2324
_log(settings.Name);
2425

25-
var validation = await _sync.ValidateAccess();
26-
if(!validation.Successful)
27-
{
28-
throw new AuthenticationException(validation.Message);
29-
}
30-
31-
var account = await _sync.GetAccount(settings.Name);
26+
var account = await GetValidAccount(settings);
3227
var repos = await _sync.GetRepositories(account);
3328
var labels = await _sync.GetAccountLabels(account);
3429
_log(string.Empty);
@@ -41,5 +36,24 @@ public async Task Run(Settings settings)
4136

4237
_log("Done!");
4338
}
39+
40+
private async Task<Account> GetValidAccount(Settings settings)
41+
{
42+
var access = await _sync.ValidateAccess();
43+
if (!access.Successful)
44+
{
45+
throw new AuthenticationException(access.Message);
46+
}
47+
48+
var account = await _sync.GetAccount(settings.Name);
49+
50+
var auth = await _sync.ValidateUser(account);
51+
if (!auth.Successful)
52+
{
53+
throw new AuthenticationException(auth.Message);
54+
}
55+
56+
return account;
57+
}
4458
}
4559
}

‎src/GitHub.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ public async Task<Account> GetOrganization(string name)
3737
return account;
3838
}
3939

40+
public async Task<Account> GetCurrentUser()
41+
{
42+
_setStatus("Finding current user...");
43+
var account = await _client.User.Current();
44+
_log($"{"User ID",-13} : {account.Id}");
45+
return account;
46+
}
47+
4048
public async Task<Account> GetUser(string name)
4149
{
4250
_setStatus($"Finding user for {name}...");
@@ -45,6 +53,21 @@ public async Task<Account> GetUser(string name)
4553
return account;
4654
}
4755

56+
public async Task<MembershipRole?> GetRole(string user, string org)
57+
{
58+
_setStatus($"Finding membership for {user} in {org}...");
59+
try
60+
{
61+
var membership = await _client.Organization.Member.GetOrganizationMembership(org, user);
62+
_log($"{"Role",-13} : {membership.Role}");
63+
return membership.Role.Value;
64+
}
65+
catch
66+
{
67+
return null;
68+
}
69+
}
70+
4871
public async Task<IReadOnlyList<Repository>> GetRepositoriesForOrganization(Account account)
4972
{
5073
_setStatus($"Finding repositories for {account.Login}...");

‎src/IGitHub.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ public interface IGitHub
88
{
99
Task<IReadOnlyList<string>> GetAccess();
1010
Task<Account> GetOrganization(string name);
11+
Task<Account> GetCurrentUser();
1112
Task<Account> GetUser(string name);
13+
Task<MembershipRole?> GetRole(string user, string org);
1214

1315
Task<IReadOnlyList<Repository>> GetRepositoriesForOrganization(Account account);
1416
Task<IReadOnlyList<Repository>> GetRepositoriesForUser(Account account);

‎src/ISynchronizer.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ namespace GitHubLabelSync
88
public interface ISynchronizer
99
{
1010
Task<ValidationResult> ValidateAccess();
11+
Task<ValidationResult> ValidateUser(Account account);
12+
1113
Task<Account> GetAccount(string name);
1214
Task<IEnumerable<Repository>> GetRepositories(Account account);
1315
Task<IReadOnlyList<Label>> GetAccountLabels(Account account);
16+
1417
Task SyncRepo(Repository repo, Settings settings, IReadOnlyList<Label> accountLabels);
1518
}
1619
}

‎src/Synchronizer.cs‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,29 @@ public async Task<ValidationResult> ValidateAccess()
2626
{
2727
var access = await _gitHub.GetAccess();
2828

29-
if (!access.Any(a => a == "repo"))
30-
{
31-
return ValidationResult.Error("API key does not have `repo` access");
32-
}
29+
return access.All(a => a != "repo") ? ValidationResult.Error("API key does not have `repo` access")
30+
: access.All(a => a != "delete_repo") ? ValidationResult.Error("API key does not have `delete_repo` access")
31+
: ValidationResult.Success();
32+
}
3333

34-
if (!access.Any(a => a == "delete_repo"))
35-
{
36-
return ValidationResult.Error("API key does not have `delete_repo` access");
37-
}
34+
public async Task<ValidationResult> ValidateUser(Account account)
35+
{
36+
var current = await _gitHub.GetCurrentUser();
3837

39-
return ValidationResult.Success();
38+
if (account.Type == AccountType.User)
39+
return current.Login == account.Login
40+
? ValidationResult.Success()
41+
: ValidationResult.Error($"API key for {current.Login} does not match target user {account.Login}");
42+
43+
var role = await _gitHub.GetRole(current.Login, account.Login);
44+
return role == MembershipRole.Admin
45+
? ValidationResult.Success()
46+
: ValidationResult.Error($"{current.Login} is not an administrator of {account.Login}");
4047
}
4148

4249
public async Task<Account> GetAccount(string name)
4350
{
4451
_setStatus($"Finding information for {name}...");
45-
4652
try
4753
{
4854
return await _gitHub.GetOrganization(name);

‎test/AppTests.cs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public async Task SyncsAllReposFound()
1818
//arrange
1919
var sync = Substitute.For<ISynchronizer>();
2020
sync.ValidateAccess().Returns(ValidationResult.Success());
21+
sync.ValidateUser(Arg.Any<Account>()).Returns(ValidationResult.Success());
2122
sync.GetRepositories(Arg.Any<Account>()).Returns(new[] { new Repository(), new Repository(), new Repository() });
2223
var app = new App(sync, NoOp, NoOp);
2324

@@ -32,11 +33,30 @@ public async Task SyncsAllReposFound()
3233
}
3334

3435
[Fact]
35-
public async Task ThrowsOnValidationFailure()
36+
public async Task ThrowsOnAccessValidationFailure()
3637
{
3738
//arrange
3839
var sync = Substitute.For<ISynchronizer>();
3940
sync.ValidateAccess().Returns(ValidationResult.Error(":("));
41+
sync.ValidateUser(Arg.Any<Account>()).Returns(ValidationResult.Success());
42+
var app = new App(sync, NoOp, NoOp);
43+
44+
var settings = new Settings { Name = "ecoAPM" };
45+
46+
//act
47+
var task = app.Run(settings);
48+
49+
//assert
50+
await Assert.ThrowsAnyAsync<Exception>(async () => await task);
51+
}
52+
53+
[Fact]
54+
public async Task ThrowsOnUserValidationFailure()
55+
{
56+
//arrange
57+
var sync = Substitute.For<ISynchronizer>();
58+
sync.ValidateAccess().Returns(ValidationResult.Success());
59+
sync.ValidateUser(Arg.Any<Account>()).Returns(ValidationResult.Error(":("));
4060
var app = new App(sync, NoOp, NoOp);
4161

4262
var settings = new Settings { Name = "ecoAPM" };

‎test/GitHubTests.cs‎

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Collections.ObjectModel;
3+
using System.Net;
44
using System.Threading.Tasks;
55
using NSubstitute;
6+
using NSubstitute.ExceptionExtensions;
67
using Octokit;
78
using Xunit;
89

@@ -20,7 +21,7 @@ public async Task CanGetAccess()
2021
{
2122
{"X-OAuth-Scopes", "repo, delete_repo"}
2223
};
23-
24+
2425
var http = Substitute.For<IResponse>();
2526
http.Headers.Returns(headers);
2627

@@ -31,7 +32,7 @@ public async Task CanGetAccess()
3132
client.Connection.Get<string>(Arg.Any<Uri>(), null, null).Returns(response);
3233
client.Connection.BaseAddress.Returns(new Uri("http://localhost/"));
3334
var gitHub = new GitHub(client, _noop, _noop);
34-
35+
3536
//act
3637
var access = await gitHub.GetAccess();
3738

@@ -42,7 +43,7 @@ public async Task CanGetAccess()
4243
}
4344

4445
[Fact]
45-
public async Task CanGetOrganizationFromClient()
46+
public async Task CanGetOrganization()
4647
{
4748
//arrange
4849
var client = Substitute.For<IGitHubClient>();
@@ -57,7 +58,7 @@ public async Task CanGetOrganizationFromClient()
5758
}
5859

5960
[Fact]
60-
public async Task CanGetUserFromClient()
61+
public async Task CanGetUser()
6162
{
6263
//arrange
6364
var client = Substitute.For<IGitHubClient>();
@@ -71,6 +72,53 @@ public async Task CanGetUserFromClient()
7172
Assert.Equal("SteveDesmond-ca", account.Login);
7273
}
7374

75+
[Fact]
76+
public async Task CanGetCurrentUser()
77+
{
78+
//arrange
79+
var client = Substitute.For<IGitHubClient>();
80+
client.User.Current().Returns(new Stubs.User("SteveDesmond-ca"));
81+
var gitHub = new GitHub(client, _noop, _noop);
82+
83+
//act
84+
var account = await gitHub.GetCurrentUser();
85+
86+
//assert
87+
Assert.Equal("SteveDesmond-ca", account.Login);
88+
}
89+
90+
[Fact]
91+
public async Task CanGetRoleForUser()
92+
{
93+
//arrange
94+
var client = Substitute.For<IGitHubClient>();
95+
var membership = new OrganizationMembership(null, new StringEnum<MembershipState>(MembershipState.Active), new StringEnum<MembershipRole>(MembershipRole.Admin), null, null, null);
96+
client.Organization.Member.GetOrganizationMembership("ecoAPM", "SteveDesmond-ca").Returns(membership);
97+
var gitHub = new GitHub(client, _noop, _noop);
98+
99+
//act
100+
var role = await gitHub.GetRole("SteveDesmond-ca", "ecoAPM");
101+
102+
//assert
103+
Assert.Equal(MembershipRole.Admin, role);
104+
}
105+
106+
[Fact]
107+
public async Task NonExistentOrgMemberReturnsNull()
108+
{
109+
//arrange
110+
var client = Substitute.For<IGitHubClient>();
111+
var membership = new OrganizationMembership(null, new StringEnum<MembershipState>(MembershipState.Active), new StringEnum<MembershipRole>(MembershipRole.Admin), null, null, null);
112+
client.Organization.Member.GetOrganizationMembership("ecoAPM", "SteveDesmond-ca").Throws(new NotFoundException(":(", HttpStatusCode.NotFound));
113+
var gitHub = new GitHub(client, _noop, _noop);
114+
115+
//act
116+
var role = await gitHub.GetRole("SteveDesmond-ca", "ecoAPM");
117+
118+
//assert
119+
Assert.Null(role);
120+
}
121+
74122
[Fact]
75123
public async Task CanGetRepositoriesForOrganization()
76124
{

0 commit comments

Comments
 (0)