Skip to content

Commit

Permalink
Merge pull request #48 from bdpiparva/use-personal-access-token
Browse files Browse the repository at this point in the history
User personal access token to fetch users private organization.
  • Loading branch information
bdpiprava authored Mar 6, 2019
2 parents ec6ad8c + 6bbb2b6 commit 80031cf
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public GitHubAuthenticator() {
}

public LoggedInUserInfo authenticate(TokenInfo tokenInfo, AuthConfig authConfig) throws IOException {
final GitHub gitHub = gitHubClientBuilder.build(tokenInfo.accessToken(), authConfig.gitHubConfiguration());
final GitHub gitHub = gitHubClientBuilder.fromAccessToken(tokenInfo.accessToken(),authConfig.gitHubConfiguration());
final List<String> allowedOrganizations = authConfig.gitHubConfiguration().organizationsAllowed();
final LoggedInUserInfo loggedInUserInfo = new LoggedInUserInfo(gitHub);

Expand Down
15 changes: 10 additions & 5 deletions src/main/java/cd/go/authorization/github/GitHubClientBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,22 @@

public class GitHubClientBuilder {

public GitHub build(String usersAccessToken, GitHubConfiguration gitHubConfiguration) throws IOException {
return createGitHub(usersAccessToken, gitHubConfiguration);
public GitHub from(GitHubConfiguration gitHubConfiguration) throws IOException {
return clientFor(gitHubConfiguration.personalAccessToken(), gitHubConfiguration);
}

private GitHub createGitHub(String accessToken, GitHubConfiguration gitHubConfiguration) throws IOException {
public GitHub fromAccessToken(String accessToken, GitHubConfiguration gitHubConfiguration) throws IOException {
return clientFor(accessToken, gitHubConfiguration);
}

private GitHub clientFor(String personalAccessTokenOrUsersAccessToken, GitHubConfiguration gitHubConfiguration) throws IOException {
if (gitHubConfiguration.authenticateWith() == AuthenticateWith.GITHUB_ENTERPRISE) {
LOG.debug("Create GitHub connection to enterprise GitHub with token");
return GitHub.connectToEnterprise(gitHubConfiguration.gitHubEnterpriseUrl(), accessToken);
return GitHub.connectToEnterprise(gitHubConfiguration.gitHubEnterpriseUrl(), gitHubConfiguration.personalAccessToken());
} else {
LOG.debug("Create GitHub connection to public GitHub with token");
return new GitHubBuilder().withOAuthToken(accessToken).withRateLimitHandler(RateLimitHandler.FAIL).build();
return new GitHubBuilder()
.withOAuthToken(personalAccessTokenOrUsersAccessToken).withRateLimitHandler(RateLimitHandler.FAIL).build();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public boolean isAMemberOfAtLeastOneOrganization(GHUser ghUser, AuthConfig authC
}

private boolean checkMembershipUsingPersonalAccessToken(GHUser ghUser, AuthConfig authConfig, List<String> organizationsAllowed) throws IOException {
final GitHub gitHubForPersonalAccessToken = clientBuilder.build(null, authConfig.gitHubConfiguration());
final GitHub gitHubForPersonalAccessToken = clientBuilder.from(authConfig.gitHubConfiguration());

for (String organizationName : organizationsAllowed) {
final GHOrganization organization = gitHubForPersonalAccessToken.getOrganization(organizationName);
Expand All @@ -74,7 +74,7 @@ public boolean isAMemberOfAtLeastOneTeamOfOrganization(GHUser ghUser, AuthConfig
}

private boolean checkTeamMembershipUsingPersonalAccessToken(GHUser ghUser, AuthConfig authConfig, Map<String, List<String>> organizationAndTeamsAllowed) throws IOException {
final GitHub gitHubForPersonalAccessToken = clientBuilder.build(null, authConfig.gitHubConfiguration());
final GitHub gitHubForPersonalAccessToken = clientBuilder.from(authConfig.gitHubConfiguration());

for (String organizationName : organizationAndTeamsAllowed.keySet()) {
final GHOrganization organization = gitHubForPersonalAccessToken.getOrganization(organizationName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public GoPluginApiResponse execute() throws IOException {
return DefaultGoPluginApiResponse.success("[]");
}

GitHub gitHub = clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration());
GitHub gitHub = clientBuilder.from(request.getAuthConfig().gitHubConfiguration());
GHUser user = gitHub.getUser(request.getUsername());

if (user == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ private Set<User> searchUsers(String searchTerm, List<AuthConfig> authConfigs) {
private Set<User> search(String searchText, AuthConfig authConfig) throws IOException {
Set<User> users = new HashSet<>();
long start = System.currentTimeMillis();
GitHub client = gitHubClientBuilder.build(authConfig.gitHubConfiguration().personalAccessToken(), authConfig.gitHubConfiguration());
GitHub client = gitHubClientBuilder.from(authConfig.gitHubConfiguration());
PagedSearchIterable<GHUser> ghUsers = client.searchUsers().q(searchText).list();
long afterRequest = System.currentTimeMillis();
LOG.debug("Time for request: " + (afterRequest - start) + "ms");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public ValidateUserRequestExecutor(ValidateUserRequest request) {

@Override
public GoPluginApiResponse execute() throws Exception {
GitHub gitHub = clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration());
GitHub gitHub = clientBuilder.from(request.getAuthConfig().gitHubConfiguration());
GHUser user = gitHub.getUser(request.getUsername());
if (user == null) {
LOG.error(format("[Is Valid User] User %s does not exist in GitHub.", request.getUsername()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public class GitHubAuthenticatorTest {
private MembershipChecker membershipChecker;
private AuthConfig authConfig;
private GitHubConfiguration gitHubConfiguration;
private GitHubClientBuilder gitHubClientBuilder;
private TokenInfo tokenInfo;

@Before
Expand All @@ -51,10 +50,11 @@ public void setUp() throws Exception {
gitHubConfiguration = mock(GitHubConfiguration.class);
tokenInfo = mock(TokenInfo.class);
membershipChecker = mock(MembershipChecker.class);
gitHubClientBuilder = mock(GitHubClientBuilder.class);
GitHubClientBuilder gitHubClientBuilder = mock(GitHubClientBuilder.class);

when(tokenInfo.accessToken()).thenReturn("some-token");
when(authConfig.gitHubConfiguration()).thenReturn(gitHubConfiguration);
when(gitHubClientBuilder.build(tokenInfo.accessToken(), gitHubConfiguration)).thenReturn(gitHub);
when(gitHubClientBuilder.fromAccessToken(tokenInfo.accessToken(), gitHubConfiguration)).thenReturn(gitHub);

authenticator = new GitHubAuthenticator(membershipChecker, gitHubClientBuilder);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void setUp() throws IOException {
final GitHubClientBuilder clientBuilder = mock(GitHubClientBuilder.class);

when(authConfig.gitHubConfiguration()).thenReturn(gitHubConfiguration);
when(clientBuilder.build(null, gitHubConfiguration)).thenReturn(gitHub);
when(clientBuilder.from(gitHubConfiguration)).thenReturn(gitHub);

membershipChecker = new MembershipChecker(clientBuilder);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void setUp() {

@Test
public void shouldReturnEmptyResponseIfThereAreNoRolesProvidedFromRequest() throws Exception {
when(clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration())).thenReturn(mock(GitHub.class));
when(clientBuilder.from(request.getAuthConfig().gitHubConfiguration())).thenReturn(mock(GitHub.class));

GoPluginApiResponse response = executor.execute();

Expand All @@ -77,7 +77,7 @@ public void shouldReturnSuccessResponseWithRoles() throws IOException, JSONExcep
GitHub gitHub = mock(GitHub.class);
GHUser ghUser = mock(GHUser.class);

when(clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration())).thenReturn(gitHub);
when(clientBuilder.from(request.getAuthConfig().gitHubConfiguration())).thenReturn(gitHub);
when(gitHub.getUser("bob")).thenReturn(ghUser);
when(request.getRoles()).thenReturn(rolesWithName("blackbird", "super-admin", "view"));
when(authorizer.authorize(ghUser, request.getAuthConfig(), request.getRoles())).thenReturn(Arrays.asList("blackbird", "super-admin"));
Expand All @@ -88,7 +88,7 @@ public void shouldReturnSuccessResponseWithRoles() throws IOException, JSONExcep
JSONAssert.assertEquals("[\"blackbird\",\"super-admin\"]", response.responseBody(), true);

InOrder inOrder = inOrder(clientBuilder, gitHub, authorizer);
inOrder.verify(clientBuilder).build(null, request.getAuthConfig().gitHubConfiguration());
inOrder.verify(clientBuilder).from(request.getAuthConfig().gitHubConfiguration());
inOrder.verify(gitHub).getUser(request.getUsername());
inOrder.verify(authorizer).authorize(ghUser, request.getAuthConfig(), request.getRoles());
}
Expand All @@ -97,7 +97,7 @@ public void shouldReturnSuccessResponseWithRoles() throws IOException, JSONExcep
public void shouldReturnErrorResponseWhenUserWithProvidedUsernameNotFound() throws IOException {
GitHub gitHub = mock(GitHub.class);

when(clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration())).thenReturn(gitHub);
when(clientBuilder.from(request.getAuthConfig().gitHubConfiguration())).thenReturn(gitHub);
when(gitHub.getUser("bob")).thenReturn(null);
when(request.getRoles()).thenReturn(rolesWithName("blackbird", "super-admin", "view"));

Expand All @@ -106,7 +106,7 @@ public void shouldReturnErrorResponseWhenUserWithProvidedUsernameNotFound() thro
assertThat(response.responseCode(), is(500));

InOrder inOrder = inOrder(clientBuilder, gitHub);
inOrder.verify(clientBuilder).build(null, request.getAuthConfig().gitHubConfiguration());
inOrder.verify(clientBuilder).from(request.getAuthConfig().gitHubConfiguration());
inOrder.verify(gitHub).getUser(request.getUsername());
verifyZeroInteractions(authorizer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void shouldSearchForUsersThatMatchTheSearchTerm() throws Exception {

when(request.getSearchTerm()).thenReturn("tom");
when(request.getAuthConfigs()).thenReturn(singletonList(authConfig));
when(clientBuilder.build(null, request.getAuthConfigs().get(0).gitHubConfiguration()))
when(clientBuilder.from(request.getAuthConfigs().get(0).gitHubConfiguration()))
.thenReturn(gitHub);
when(gitHub.searchUsers()).thenReturn(userSearchBuilder);
when(userSearchBuilder.q("tom")).thenReturn(userSearchBuilder);
Expand All @@ -70,7 +70,7 @@ public void shouldNotPerformSearchIfAuthConfigsIsEmpty() throws Exception {

GoPluginApiResponse response = executor.execute();

verify(clientBuilder, never()).build(anyString(), any());
verify(clientBuilder, never()).from(any());
assertThat(response.responseCode(), is(200));
JSONAssert.assertEquals("[]", response.responseBody(), false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void setUp() {
@Test
public void shouldReturnSuccessResponseWhenUserIsAValidUser() throws Exception {
GitHub gitHub = mock(GitHub.class);
when(clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration()))
when(clientBuilder.from(request.getAuthConfig().gitHubConfiguration()))
.thenReturn(gitHub);
when(request.getUsername()).thenReturn("bob");
when(gitHub.getUser("bob")).thenReturn(mock(GHUser.class));
Expand All @@ -67,7 +67,7 @@ public void shouldReturnSuccessResponseWhenUserIsAValidUser() throws Exception {
@Test
public void shouldReturnErrorResponseWhenUserIsNotAValidUser() throws Exception {
GitHub gitHub = mock(GitHub.class);
when(clientBuilder.build(null, request.getAuthConfig().gitHubConfiguration()))
when(clientBuilder.from(request.getAuthConfig().gitHubConfiguration()))
.thenReturn(gitHub);
when(request.getUsername()).thenReturn("bob");
when(gitHub.getUser("bob")).thenReturn(null);
Expand Down

0 comments on commit 80031cf

Please sign in to comment.