Skip to content

Commit

Permalink
Refactors and cleans up dead code in GitHub module (#137)
Browse files Browse the repository at this point in the history 
* Refactors and cleans up dead code in GitHub module
* Added rate limit header info
  • Loading branch information
Travis Tomsu authored Jan 25, 2017
1 parent 8b4f152 commit 9364762
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 203 deletions.
30 changes: 11 additions & 19 deletions fiat-github/src/main/java/com/netflix/spinnaker/fiat/config/GitHubConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import com.netflix.spinnaker.fiat.roles.github.GitHubProperties;
import com.netflix.spinnaker.fiat.roles.github.client.GitHubClient;
import com.netflix.spinnaker.fiat.roles.github.client.GitHubMaster;
import lombok.Getter;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;
import org.slf4j.Logger;
Expand All @@ -18,8 +16,6 @@
import retrofit.client.OkClient;
import retrofit.converter.JacksonConverter;

import javax.validation.Valid;

/**
* Converts the list of GitHub Configuration properties a collection of clients to access the GitHub hosts
*/
Expand All @@ -36,22 +32,18 @@ public class GitHubConfig {
@Setter
private RestAdapter.LogLevel retrofitLogLevel;

@Bean
public GitHubMaster gitHubMasters(@Valid final GitHubProperties gitHubProperties) {
log.info("bootstrapping " + gitHubProperties.getBaseUrl() + " as github");

return new GitHubMaster()
.setGitHubClient(gitHubClient(gitHubProperties.getBaseUrl(),
gitHubProperties.getAccessToken()))
.setBaseUrl(gitHubProperties.getBaseUrl());
}
@Autowired
@Setter
private GitHubProperties gitHubProperties;

private GitHubClient gitHubClient(String address, String accessToken) {
BasicAuthRequestInterceptor interceptor = new BasicAuthRequestInterceptor();
@Bean
public GitHubClient gitHubClient() {
BasicAuthRequestInterceptor interceptor = new BasicAuthRequestInterceptor()
.setAccessToken(gitHubProperties.getAccessToken());

return new RestAdapter.Builder()
.setEndpoint(Endpoints.newFixedEndpoint(address))
.setRequestInterceptor(interceptor.setAccessToken(accessToken))
.setEndpoint(Endpoints.newFixedEndpoint(gitHubProperties.getBaseUrl()))
.setRequestInterceptor(interceptor)
.setClient(okClient)
.setConverter(new JacksonConverter())
.setLogLevel(retrofitLogLevel)
Expand Down Expand Up @@ -79,13 +71,13 @@ public void log(String message) {

private static class BasicAuthRequestInterceptor implements RequestInterceptor {

@Getter
@Setter
private String accessToken;

@Override
public void intercept(RequestFacade request) {
request.addQueryParam("access_token", accessToken);
// See docs at https://developer.github.com/v3/#authentication
request.addHeader("Authorization", "token " + accessToken);
}
}
}
199 changes: 100 additions & 99 deletions ...b/src/main/java/com/netflix/spinnaker/fiat/roles/github/GithubTeamsUserRolesProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@

import com.netflix.spinnaker.fiat.model.resources.Role;
import com.netflix.spinnaker.fiat.roles.UserRolesProvider;
import com.netflix.spinnaker.fiat.roles.github.client.GitHubMaster;
import com.netflix.spinnaker.fiat.roles.github.client.GitHubClient;
import com.netflix.spinnaker.fiat.roles.github.model.Team;
import com.netflix.spinnaker.fiat.roles.github.model.TeamMembership;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;
import lombok.val;
Expand All @@ -29,154 +31,155 @@
import org.springframework.stereotype.Component;
import org.springframework.util.Assert;
import retrofit.RetrofitError;
import retrofit.client.Header;
import retrofit.client.Response;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

@Slf4j
@Component
@ConditionalOnProperty(value = "auth.groupMembership.service", havingValue = "github")
public class GithubTeamsUserRolesProvider implements UserRolesProvider, InitializingBean {

private static List<String> RATE_LIMITING_HEADERS = Arrays.asList(
"X-RateLimit-Limit",
"X-RateLimit-Remaining",
"X-RateLimit-Reset"
);

@Autowired
@Setter
GitHubMaster master;
private GitHubClient gitHubClient;

@Autowired
@Setter
GitHubProperties gitHubProperties;
private GitHubProperties gitHubProperties;

@Override
public void afterPropertiesSet() throws Exception {
Assert.state(gitHubProperties.getOrganization() != null, "Supply an organization");
}

@Override
public List<Role> loadRoles(String userName) {
log.debug("loadRoles for user " + userName);
if (StringUtils.isEmpty(userName)|| StringUtils.isEmpty(gitHubProperties.getOrganization())) {
public List<Role> loadRoles(String username) {
log.debug("loadRoles for user " + username);
if (StringUtils.isEmpty(username) || StringUtils.isEmpty(gitHubProperties.getOrganization())) {
return new ArrayList<>();
}

if (!isMemberOfOrg(username)) {
log.debug(username + "is not a member of organization " + gitHubProperties.getOrganization());
return new ArrayList<>();
}
// check organization if set.
// If organization is unset, all GitHub users can login and have full access
// If an organization is set, add it to roles to restrict users to this organization
// If organization is set AND requiredGroupMembership set, organization members will have RO access
// and requiredGroupMembership members RW access
Boolean isMemberOfOrg = false;
log.debug(username + "is a member of organization " + gitHubProperties.getOrganization());

List<Role> result = new ArrayList<>();
result.add(toRole(gitHubProperties.getOrganization()));

// Get teams of the org
List<Team> teams = getTeams();
log.debug("Found " + teams.size() + " teams in org.");

teams.forEach(t -> {
String debugMsg = username + " is a member of team " + t.getName();
if (isMemberOfTeam(t, username)) {
result.add(toRole(t.getSlug()));
debugMsg += ": true";
} else {
debugMsg += ": false";
}
log.debug(debugMsg);
});

return result;
}

private boolean isMemberOfOrg(String username) {
boolean isMemberOfOrg = false;
try {
Response response = master.getGitHubClient()
.isMemberOfOrganization(gitHubProperties.getOrganization(),
userName);
Response response = gitHubClient.isMemberOfOrganization(gitHubProperties.getOrganization(),
username);
isMemberOfOrg = (response.getStatus() == 204);
if(log.isDebugEnabled()) {
StringBuilder sb = new StringBuilder(userName).append(" is ");
if (!isMemberOfOrg) {
sb.append("not ");
}
sb.append("a member of ")
.append(gitHubProperties.getOrganization())
.append(" organization.");
log.debug(sb.toString());
}
} catch (RetrofitError e) {
if (e.getKind() == RetrofitError.Kind.NETWORK) {
log.error(String.format("Could not find the server %s", master.getBaseUrl()), e);
return new ArrayList<>();
} else if (e.getResponse().getStatus() == 404) {
log.error(String.format("%s is not a member of GitHub organization %s",
userName,
gitHubProperties.getOrganization()),
e);
return new ArrayList<>();
} else if (e.getResponse().getStatus() == 401) {
log.error(String.format("Cannot get GitHub organization %s information: Not authorized.",
gitHubProperties.getOrganization()),
e);
return new ArrayList<>();
if (e.getResponse().getStatus() != 404) {
handleNon404s(e);
}
}

if (!isMemberOfOrg) {
return new ArrayList<>();
}

List<Role> result = new ArrayList<>();
result.add(toRole(gitHubProperties.getOrganization()));
return isMemberOfOrg;
}

// Get teams of the current user
List<GitHubMaster.Team> teams = new ArrayList<>();
private List<Team> getTeams() {
List<Team> teams = new ArrayList<>();
int page = 1;
boolean hasMorePages = true;

do {
try {
log.debug("Requesting page " + page + " of teams.");
List<GitHubMaster.Team> teamsPage = master.getGitHubClient()
.getOrgTeams(gitHubProperties.getOrganization(),
page++,
gitHubProperties.paginationValue);
teams.addAll(teamsPage);
if (teamsPage.size() != gitHubProperties.paginationValue) {
hasMorePages = false;
}
log.debug("Got " + teamsPage.size() + " teams back. hasMorePages: " + hasMorePages);
} catch (RetrofitError e) {
List<Team> teamsPage = getTeamsInOrgPaginated(page++);
teams.addAll(teamsPage);
if (teamsPage.size() != gitHubProperties.paginationValue) {
hasMorePages = false;
log.error(String.format("RetrofitError %s %s ",
e.getResponse().getStatus(),
e.getResponse().getReason()),
e);
if (e.getKind() == RetrofitError.Kind.NETWORK) {
log.error(String.format("Could not find the server %s", master.getBaseUrl()), e);
} else if (e.getResponse().getStatus() == 404) {
log.error("404 when getting teams");
return result;
} else if (e.getResponse().getStatus() == 401) {
log.error(String.format("Cannot get GitHub organization %s teams: Not authorized.",
gitHubProperties.getOrganization()),
e);
return result;
}
}
log.debug("Got " + teamsPage.size() + " teams back. hasMorePages: " + hasMorePages);
} while (hasMorePages);

log.debug("Found " + teams.size() + " teams in org.");
teams.forEach(t -> {
StringBuilder sb = new StringBuilder(userName).append(" is member of team ").append(t.getName());
if (isMemberOfTeam(t, userName)) {
sb.append(": true");
result.add(toRole(t.getSlug()));
return teams;
}

private List<Team> getTeamsInOrgPaginated(int page) {
List<Team> teams = new ArrayList<>();
try {
log.debug("Requesting page " + page + " of teams.");
teams = gitHubClient.getOrgTeams(gitHubProperties.getOrganization(),
page,
gitHubProperties.paginationValue);
} catch (RetrofitError e) {
if (e.getResponse().getStatus() != 404) {
handleNon404s(e);
} else {
sb.append(": false");
log.error("404 when getting teams", e);
}
log.debug(sb.toString());
});
}

return result;
return teams;
}


private boolean isMemberOfTeam(GitHubMaster.Team t, String userName) {
private boolean isMemberOfTeam(Team t, String username) {
String ACTIVE = "active";
try {
GitHubMaster.TeamMembership response = master.getGitHubClient()
.isMemberOfTeam(t.getId(), userName);
TeamMembership response = gitHubClient.isMemberOfTeam(t.getId(), username);
return (response.getState().equals(ACTIVE));
} catch (RetrofitError e) {
if (e.getKind() == RetrofitError.Kind.NETWORK) {
log.error(String.format("Could not find the server %s", master.getBaseUrl()), e);
} else if (e.getResponse().getStatus() == 401) {
log.error(String.format("Cannot check if $userName is member of %s teams: Not authorized.",
t.getName()),
e);
if (e.getResponse().getStatus() != 404) {
handleNon404s(e);
}
}
return false;
}

@Override
public void afterPropertiesSet() throws Exception {
Assert.state(gitHubProperties.getOrganization ()!= null, "Supply an organization");
private void handleNon404s(RetrofitError e) {
String msg = "";
if (e.getKind() == RetrofitError.Kind.NETWORK) {
msg = String.format("Could not find the server %s", gitHubProperties.getBaseUrl());
} else if (e.getResponse().getStatus() == 401) {
msg = "HTTP 401 Unauthorized.";
} else if (e.getResponse().getStatus() == 403) {
val rateHeaders = e.getResponse()
.getHeaders()
.stream()
.filter(header -> RATE_LIMITING_HEADERS.contains(header.getName()))
.map(Header::toString)
.collect(Collectors.toList());

msg = "HTTP 403 Forbidden. Rate limit info: " + StringUtils.join(rateHeaders, ", ");
}
log.error(msg, e);
}

private static Role toRole(String name) {
Expand All @@ -194,6 +197,4 @@ public Map<String, Collection<Role>> multiLoadRoles(Collection<String> userEmail

return emailGroupsMap;
}


}
14 changes: 8 additions & 6 deletions fiat-github/src/main/java/com/netflix/spinnaker/fiat/roles/github/client/GitHubClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package com.netflix.spinnaker.fiat.roles.github.client;


import com.netflix.spinnaker.fiat.roles.github.model.Team;
import com.netflix.spinnaker.fiat.roles.github.model.TeamMembership;
import retrofit.client.Response;
import retrofit.http.GET;
import retrofit.http.Path;
Expand All @@ -37,15 +39,15 @@ Response isMemberOfOrganization(@Path("org") String org,
* This one should use the Current User credentials
*/
@GET("/user/teams")
List<GitHubMaster.Team> getUserTeams();
List<Team> getUserTeams();

@GET("/orgs/{org}/teams")
List<GitHubMaster.Team> getOrgTeams(@Path("org") String org,
@Query("page") int page,
@Query("per_page") int paginationValue);
List<Team> getOrgTeams(@Path("org") String org,
@Query("page") int page,
@Query("per_page") int paginationValue);

@GET("/teams/{idTeam}/memberships/{username}")
GitHubMaster.TeamMembership isMemberOfTeam(@Path("idTeam") Long idTeam,
@Path("username") String username);
TeamMembership isMemberOfTeam(@Path("idTeam") Long idTeam,
@Path("username") String username);
}

Loading

0 comments on commit 9364762

Please sign in to comment.