Improvements of feedback from review I86fb8fab3
GitHub authentication has been submitted 9 months ago to Gerrit as
extra authentication backend: a lot of suggestions and after 57 (!)
patch-sets many improvements have been added to the code and are now
incorporated into GitHub master:
- Introduction of random state to prevent OAuth code injection and
replay attacks
- Upgrade to HTTPClient 4.4 and connection pooling
- Introduction of HTTP proxy for outbound connections when installed
within company firewalls
- Removal of heavy non serialisable objects from the HTTP Session
- Early release of HTTP connections to the pool to improve scalability
- Simplified set-up during init
- Improved documentation
Many thanks to all the people that participated in the Code-Review and
contributions: as Git doesn’t allow to have more than one author,
I will mention them below.
Thanks to: Edwin Kempin, David Pursehouse, Thomas Broyer,
David Ostrovsky, Sasa Zivkov, Andre Saddler, Dariusz Luksza,
Derek Hunter, Will DeBerry, Tom Hughes, Shawn Pearce
Change-Id: I19e761485692d9b4bc0ade8d0f3ad761b0fa5369
diff --git a/github-oauth/pom.xml b/github-oauth/pom.xml
index c0ab78f..e6d70e7 100644
--- a/github-oauth/pom.xml
+++ b/github-oauth/pom.xml
@@ -112,7 +112,7 @@
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
- <version>4.2.5</version>
+ <version>4.4</version>
</dependency>
</dependencies>
</project>
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubHttpProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubHttpProvider.java
deleted file mode 100644
index 367a390..0000000
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubHttpProvider.java
+++ /dev/null
@@ -1,56 +0,0 @@
-// Copyright (C) 2013 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.googlesource.gerrit.plugins.github.oauth;
-
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-
-import org.apache.http.HttpHost;
-import org.apache.http.client.HttpClient;
-import org.apache.http.conn.routing.HttpRoute;
-import org.apache.http.conn.scheme.PlainSocketFactory;
-import org.apache.http.conn.scheme.Scheme;
-import org.apache.http.conn.scheme.SchemeRegistry;
-import org.apache.http.conn.ssl.SSLSocketFactory;
-import org.apache.http.impl.client.DefaultHttpClient;
-import org.apache.http.impl.conn.PoolingClientConnectionManager;
-
-@Singleton
-public class GitHubHttpProvider implements Provider<HttpClient> {
- private static final int MAX_TOTAL_CONN = 1024;
- private static final int MAX_CONN_PER_ROUTE = 10;
- private static final int MAX_LOCALHOST_CONN = 512;
- private final PoolingClientConnectionManager connectionManager;
-
- public GitHubHttpProvider() {
- SchemeRegistry schemeRegistry = new SchemeRegistry();
- schemeRegistry.register(new Scheme("http", 80, PlainSocketFactory
- .getSocketFactory()));
- schemeRegistry.register(new Scheme("https", 443, SSLSocketFactory
- .getSocketFactory()));
-
- connectionManager = new PoolingClientConnectionManager(schemeRegistry);
- connectionManager.setMaxTotal(MAX_TOTAL_CONN);
- connectionManager.setDefaultMaxPerRoute(MAX_CONN_PER_ROUTE);
- HttpHost localhost = new HttpHost("locahost", 80);
- connectionManager.setMaxPerRoute(new HttpRoute(localhost),
- MAX_LOCALHOST_CONN);
- }
-
- @Override
- public HttpClient get() {
- return new DefaultHttpClient(connectionManager);
- }
-
-}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
index 2950996..32f751a 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
@@ -16,20 +16,8 @@
import static java.util.concurrent.TimeUnit.DAYS;
-import com.google.common.base.MoreObjects;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken;
-import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
-
-import org.apache.http.HttpStatus;
-import org.kohsuke.github.GHMyself;
-import org.kohsuke.github.GitHub;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.io.IOException;
+import java.io.Serializable;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
@@ -38,21 +26,30 @@
import java.util.SortedSet;
import java.util.TreeSet;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import lombok.Getter;
-public class GitHubLogin {
+import org.kohsuke.github.GHMyself;
+import org.kohsuke.github.GitHub;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.MoreObjects;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
+
+public class GitHubLogin implements Serializable {
+ private static final long serialVersionUID = 1L;
private static final Logger log = LoggerFactory.getLogger(GitHubLogin.class);
private static final List<Scope> DEFAULT_SCOPES = Arrays.asList(
Scope.PUBLIC_REPO, Scope.USER_EMAIL);
- private static final int YEARS = 365;
private static final long SCOPE_COOKIE_NEVER_EXPIRES = DAYS
- .toSeconds(50 * YEARS);
+ .toSeconds(50 * 365);
@Singleton
public static class Provider extends HttpSessionProvider<GitHubLogin> {
@@ -63,21 +60,16 @@
}
@Getter
- protected AccessToken token;
+ private AccessToken token;
- @Getter
- protected GitHub hub;
-
- protected GHMyself myself;
-
- private transient OAuthProtocol oauth;
+ private String state;
private SortedSet<Scope> loginScopes;
private final GitHubOAuthConfig config;
- public GHMyself getMyself() {
+ public GHMyself getMyself() throws IOException {
if (isLoggedIn()) {
- return myself;
+ return getHub().getMyself();
} else {
return null;
}
@@ -85,92 +77,68 @@
public Set<String> getMyOrganisationsLogins() throws IOException {
if (isLoggedIn()) {
- return hub.getMyOrganizations().keySet();
+ return getHub().getMyOrganizations().keySet();
} else {
return Collections.emptySet();
}
}
@Inject
- public GitHubLogin(final OAuthProtocol oauth, final GitHubOAuthConfig config) {
- this.oauth = oauth;
+ public GitHubLogin(final GitHubOAuthConfig config) {
this.config = config;
}
public boolean isLoggedIn() {
- boolean loggedIn = token != null && hub != null;
- if (loggedIn && myself == null) {
- try {
- myself = hub.getMyself();
- } catch (Throwable e) {
- log.error("Connection to GitHub broken: logging out", e);
- logout();
- loggedIn = false;
- }
- }
- return loggedIn;
- }
-
- public boolean login(ServletRequest request, ServletResponse response,
- Scope... scopes) throws IOException {
- return login((HttpServletRequest) request, (HttpServletResponse) response,
- scopes);
+ return token != null;
}
public boolean login(HttpServletRequest request,
- HttpServletResponse response, Scope... scopes) throws IOException {
+ HttpServletResponse response, OAuthProtocol oauth, Scope... scopes) throws IOException {
if (isLoggedIn()) {
return true;
}
log.debug("Login " + this);
-
if (OAuthProtocol.isOAuthFinal(request)) {
log.debug("Login-FINAL " + this);
- AccessToken loginAccessToken = oauth.loginPhase2(request, response);
- if(loginAccessToken != null && !loginAccessToken.isError()) {
- login(loginAccessToken);
- }
+ login(oauth.loginPhase2(request, state));
+ this.state = ""; // Make sure state is used only once
if (isLoggedIn()) {
log.debug("Login-SUCCESS " + this);
response.sendRedirect(OAuthProtocol.getTargetUrl(request));
return true;
} else {
- response.sendError(HttpStatus.SC_UNAUTHORIZED);
return false;
}
} else {
this.loginScopes = getScopes(getScopesKey(request, response), scopes);
log.debug("Login-PHASE1 " + this);
- oauth.loginPhase1(request, response, loginScopes);
+ state = oauth.loginPhase1(request, response, loginScopes);
return false;
}
}
public void logout() {
- hub = null;
token = null;
}
- public OAuthProtocol getOAuthProtocol() {
- return oauth;
- }
-
public GitHub login(AccessToken authToken) throws IOException {
log.debug("Logging in using access token {}", authToken.accessToken);
this.token = authToken;
- this.hub = GitHub.connectUsingOAuth(authToken.accessToken);
- this.myself = hub.getMyself();
- return this.hub;
+ return getHub();
}
@Override
public String toString() {
- return "GitHubLogin [token=" + token + ", myself=" + myself + ", scopes="
+ return "GitHubLogin [token=" + token + ", scopes="
+ loginScopes + "]";
}
+ public GitHub getHub() throws IOException {
+ return GitHub.connectUsingOAuth(this.token.accessToken);
+ }
+
private String getScopesKey(HttpServletRequest request,
HttpServletResponse response) {
String scopeRequested = request.getParameter("scope");
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
index c41be26..7d30d19 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
@@ -14,36 +14,41 @@
package com.googlesource.gerrit.plugins.github.oauth;
import com.google.common.base.MoreObjects;
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
import com.google.gerrit.reviewdb.client.AuthType;
+import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
import org.eclipse.jgit.lib.Config;
-import java.net.MalformedURLException;
-import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@Singleton
-public class GitHubOAuthConfig {
- protected static final String CONF_SECTION = "github";
- private static final String LOGIN_OAUTH_AUTHORIZE = "/login/oauth/authorize";
- private static final String GITHUB_URL = "https://github.com";
- public static final String OAUTH_FINAL = "oauth";
- public static final String LOGIN_OAUTH_ACCESS_TOKEN =
+public
+class GitHubOAuthConfig {
+ public static final String CONF_SECTION = "github";
+ public static final String GITHUB_OAUTH_AUTHORIZE = "/login/oauth/authorize";
+ public static final String GITHUB_OAUTH_ACCESS_TOKEN =
"/login/oauth/access_token";
- public static final String OAUTH_LOGIN = "/login";
- public static final String OAUTH_LOGOUT = "/logout";
+ public static final String GITHUB_GET_USER = "/user";
+ public static final String GERRIT_OAUTH_FINAL = "/oauth";
+ public static final String GITHUB_URL_DEFAULT = "https://github.com";
+ public static final String GITHUB_API_URL_DEFAULT = "https://api.github.com";
+ public static final String GERRIT_LOGIN = "/login";
+ public static final String GERRIT_LOGOUT = "/logout";
public final String gitHubUrl;
+ public final String gitHubApiUrl;
public final String gitHubClientId;
public final String gitHubClientSecret;
public final String logoutRedirectUrl;
@@ -55,34 +60,49 @@
public final Map<String, List<OAuthProtocol.Scope>> scopes;
public final int fileUpdateMaxRetryCount;
public final int fileUpdateMaxRetryIntervalMsec;
- public final Config gerritConfig;
public final String oauthHttpHeader;
@Inject
- public GitHubOAuthConfig(@GerritServerConfig Config config)
- throws MalformedURLException {
- this.gerritConfig = config;
+ protected
+ GitHubOAuthConfig(@GerritServerConfig Config config,
+ @CanonicalWebUrl String canonicalWebUrl, AuthConfig authConfig) {
+ httpHeader =
+ Preconditions.checkNotNull(
+ config.getString("auth", null, "httpHeader"),
+ "HTTP Header for GitHub user must be provided");
+ gitHubUrl =
+ trimTrailingSlash(MoreObjects.firstNonNull(
+ config.getString(CONF_SECTION, null, "url"), GITHUB_URL_DEFAULT));
+ gitHubApiUrl =
+ trimTrailingSlash(MoreObjects.firstNonNull(
+ config.getString(CONF_SECTION, null, "apiUrl"),
+ GITHUB_API_URL_DEFAULT));
+ gitHubClientId =
+ Preconditions.checkNotNull(
+ config.getString(CONF_SECTION, null, "clientId"),
+ "GitHub `clientId` must be provided");
+ gitHubClientSecret =
+ Preconditions.checkNotNull(
+ config.getString(CONF_SECTION, null, "clientSecret"),
+ "GitHub `clientSecret` must be provided");
- httpHeader = config.getString("auth", null, "httpHeader");
oauthHttpHeader = config.getString("auth", null, "httpExternalIdHeader");
- gitHubUrl = dropTrailingSlash(
- MoreObjects.firstNonNull(config.getString(CONF_SECTION, null, "url"),
- GITHUB_URL));
- gitHubClientId = config.getString(CONF_SECTION, null, "clientId");
- gitHubClientSecret = config.getString(CONF_SECTION, null, "clientSecret");
- gitHubOAuthUrl = getUrl(gitHubUrl, LOGIN_OAUTH_AUTHORIZE);
- gitHubOAuthAccessTokenUrl = getUrl(gitHubUrl, LOGIN_OAUTH_ACCESS_TOKEN);
- logoutRedirectUrl = config.getString(CONF_SECTION, null, "logoutRedirectUrl");
+ gitHubOAuthUrl = gitHubUrl + GITHUB_OAUTH_AUTHORIZE;
+ gitHubOAuthAccessTokenUrl = gitHubUrl + GITHUB_OAUTH_ACCESS_TOKEN;
+ logoutRedirectUrl =
+ config.getString(CONF_SECTION, null, "logoutRedirectUrl");
oAuthFinalRedirectUrl =
- getUrl(config.getString("gerrit", null, "canonicalWebUrl"), OAUTH_FINAL);
+ trimTrailingSlash(canonicalWebUrl) + GERRIT_OAUTH_FINAL;
enabled =
config.getString("auth", null, "type").equalsIgnoreCase(
AuthType.HTTP.toString());
scopes = getScopes(config);
- fileUpdateMaxRetryCount = config.getInt(CONF_SECTION, "fileUpdateMaxRetryCount", 3);
- fileUpdateMaxRetryIntervalMsec = config.getInt(CONF_SECTION, "fileUpdateMaxRetryIntervalMsec", 3000);
+ fileUpdateMaxRetryCount =
+ config.getInt(CONF_SECTION, "fileUpdateMaxRetryCount", 3);
+ fileUpdateMaxRetryIntervalMsec =
+ config.getInt(CONF_SECTION, "fileUpdateMaxRetryIntervalMsec", 3000);
}
private Map<String, List<Scope>> getScopes(Config config) {
@@ -97,13 +117,13 @@
return scopes;
}
- private String dropTrailingSlash(String url) {
- return (url.endsWith("/") ? url.substring(0, url.length()-1):url);
+ private String trimTrailingSlash(String url) {
+ return CharMatcher.is('/').trimTrailingFrom(url);
}
private List<Scope> parseScopesString(String scopesString) {
ArrayList<Scope> scopes = new ArrayList<OAuthProtocol.Scope>();
- if(Strings.emptyToNull(scopesString) != null) {
+ if (Strings.emptyToNull(scopesString) != null) {
String[] scopesStrings = scopesString.split(",");
for (String scope : scopesStrings) {
scopes.add(Enum.valueOf(Scope.class, scope));
@@ -113,11 +133,6 @@
return scopes;
}
- private static String getUrl(String baseUrl, String path)
- throws MalformedURLException {
- return new URL(new URL(baseUrl), path).toExternalForm();
- }
-
public Scope[] getDefaultScopes() {
if (scopes == null || scopes.get("scopes") == null) {
return new Scope[0];
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
index 29d8840..646b602 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
@@ -14,6 +14,12 @@
package com.googlesource.gerrit.plugins.github.oauth;
+import java.io.IOException;
+import java.util.Collection;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.server.IdentifiedUser;
@@ -22,34 +28,25 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.IOException;
-import java.util.Collection;
-
@Singleton
public class IdentifiedUserGitHubLoginProvider implements
UserScopedProvider<GitHubLogin> {
- private static final Logger log = LoggerFactory.getLogger(IdentifiedUserGitHubLoginProvider.class);
+ private static final Logger log = LoggerFactory
+ .getLogger(IdentifiedUserGitHubLoginProvider.class);
private static final String EXTERNAL_ID_PREFIX = "external:"
+ OAuthWebFilter.GITHUB_EXT_ID;
private final Provider<IdentifiedUser> userProvider;
- private OAuthProtocol oauth;
private GitHubOAuthConfig config;
private AccountCache accountCache;
@Inject
public IdentifiedUserGitHubLoginProvider(
final Provider<IdentifiedUser> identifiedUserProvider,
- final OAuthProtocol oauth, final GitHubOAuthConfig config,
- final AccountCache accountCache) {
+ final GitHubOAuthConfig config, final AccountCache accountCache) {
this.userProvider = identifiedUserProvider;
- this.oauth = oauth;
this.config = config;
this.accountCache = accountCache;
}
@@ -66,15 +63,14 @@
try {
AccessToken accessToken = newAccessTokenFromUser(username);
if (accessToken != null) {
- GitHubLogin login = new GitHubLogin(oauth, config);
+ GitHubLogin login = new GitHubLogin(config);
login.login(accessToken);
return login;
} else {
return null;
}
} catch (IOException e) {
- log.error("Cannot login to GitHub as '" + username
- + "'", e);
+ log.error("Cannot login to GitHub as '" + username + "'", e);
return null;
}
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java
index e2a8f26..c0f3897 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java
@@ -22,14 +22,15 @@
import com.google.gerrit.httpd.XGerritAuth;
import com.google.gerrit.server.account.AccountCache;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
-
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang.StringUtils;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
+import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpPut;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
@@ -68,7 +69,7 @@
private final OAuthCache oauthCache;
private final AccountCache accountCache;
- private final GitHubHttpProvider httpClientProvider;
+ private final Provider<HttpClient> httpClientProvider;
private final GitHubOAuthConfig config;
private final XGerritAuth xGerritAuth;
private ScopedProvider<GitHubLogin> ghLoginProvider;
@@ -115,7 +116,7 @@
@Inject
public OAuthGitFilter(OAuthCache oauthCache, AccountCache accountCache,
- GitHubHttpProvider httpClientProvider, GitHubOAuthConfig config,
+ PooledHttpClientProvider httpClientProvider, GitHubOAuthConfig config,
XGerritAuth xGerritAuth, GitHubLogin.Provider ghLoginProvider) {
this.oauthCache = oauthCache;
this.accountCache = accountCache;
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java
index 563a71e..4358c0d 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java
@@ -2,9 +2,11 @@
import com.google.common.base.Charsets;
import com.google.common.base.Strings;
+import com.google.common.io.BaseEncoding;
import com.google.common.io.CharStreams;
import com.google.gson.Gson;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.apache.http.HttpResponse;
@@ -24,8 +26,11 @@
import java.net.HttpURLConnection;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
import javax.servlet.ServletRequest;
@@ -58,12 +63,13 @@
}
}
private static final String ME_SEPARATOR = ",";
- private static final Logger LOG = LoggerFactory
+ private static final Logger log = LoggerFactory
.getLogger(OAuthProtocol.class);
+ private static SecureRandom randomState = newRandomGenerator();
private final GitHubOAuthConfig config;
- private final HttpClient http;
private final Gson gson;
+ private final Provider<HttpClient> httpProvider;
public static class AccessToken {
public String accessToken;
@@ -98,29 +104,13 @@
@Override
public int hashCode() {
- final int prime = 31;
- int result = 1;
- result =
- prime * result
- + ((accessToken == null) ? 0 : accessToken.hashCode());
- result =
- prime * result + ((tokenType == null) ? 0 : tokenType.hashCode());
- return result;
+ return Objects.hash(accessToken, tokenType, error, errorDescription,
+ errorUri);
}
@Override
public boolean equals(Object obj) {
- if (this == obj) return true;
- if (obj == null) return false;
- if (getClass() != obj.getClass()) return false;
- AccessToken other = (AccessToken) obj;
- if (accessToken == null) {
- if (other.accessToken != null) return false;
- } else if (!accessToken.equals(other.accessToken)) return false;
- if (tokenType == null) {
- if (other.tokenType != null) return false;
- } else if (!tokenType.equals(other.tokenType)) return false;
- return true;
+ return Objects.deepEquals(this, obj);
}
public boolean isError() {
@@ -129,36 +119,49 @@
}
@Inject
- public OAuthProtocol(GitHubOAuthConfig config, GitHubHttpProvider httpClientProvider,
- /* We need to explicitly tell Guice which Provider<> we need as this class may be
- instantiated outside the standard Guice Module set-up (e.g. initial Servlet login
- filter) */
+ public OAuthProtocol(GitHubOAuthConfig config,
+ PooledHttpClientProvider httpClientProvider,
+ /*
+ * We need to explicitly tell Guice which Provider<> we need as this class
+ * may be instantiated outside the standard Guice Module set-up (e.g.
+ * initial Servlet login filter)
+ */
GsonProvider gsonProvider) {
this.config = config;
- this.http = httpClientProvider.get();
+ this.httpProvider = httpClientProvider;
this.gson = gsonProvider.get();
}
- public void loginPhase1(HttpServletRequest request,
- HttpServletResponse response, Set<Scope> scopes) throws IOException {
+ private static SecureRandom newRandomGenerator() {
+ try {
+ return SecureRandom.getInstance("SHA1PRNG");
+ } catch (NoSuchAlgorithmException e) {
+ throw new IllegalArgumentException(
+ "No SecureRandom available for GitHub authentication", e);
+ }
+ }
+ public String loginPhase1(HttpServletRequest request,
+ HttpServletResponse response, Set<Scope> scopes) throws IOException {
String scopesString = getScope(scopes);
- LOG.debug("Initiating GitHub Login for ClientId=" + config.gitHubClientId + " Scopes=" + scopesString);
+ log.debug("Initiating GitHub Login for ClientId=" + config.gitHubClientId
+ + " Scopes=" + scopesString);
+ String state = newRandomState(request.getRequestURI().toString());
response.sendRedirect(String.format(
- "%s?client_id=%s%s&redirect_uri=%s&state=%s%s", config.gitHubOAuthUrl,
- config.gitHubClientId, scopesString,
- getURLEncoded(config.oAuthFinalRedirectUrl),
- me(), getURLEncoded(request.getRequestURI().toString())));
+ "%s?client_id=%s%s&redirect_uri=%s&state=%s",
+ config.gitHubOAuthUrl, config.gitHubClientId, scopesString,
+ getURLEncoded(config.oAuthFinalRedirectUrl), getURLEncoded(state)));
+ return state;
}
private String getScope(Set<Scope> scopes) {
- if(scopes.size() <= 0) {
+ if (scopes.size() <= 0) {
return "";
}
-
+
StringBuilder out = new StringBuilder();
for (Scope scope : scopes) {
- if(out.length() > 0) {
+ if (out.length() > 0) {
out.append(",");
}
out.append(scope.getValue());
@@ -179,28 +182,33 @@
return Strings.emptyToNull(request.getParameter("code")) != null;
}
- public String me() {
- return "" + hashCode() + ME_SEPARATOR;
+ public String newRandomState(String redirectUrl) {
+ byte[] stateBin = new byte[20]; // SHA-1 size
+ randomState.nextBytes(stateBin);
+ return BaseEncoding.base64Url().encode(stateBin) + ME_SEPARATOR + redirectUrl;
}
public static boolean isOAuthLogin(HttpServletRequest request) {
- return request.getRequestURI().indexOf(GitHubOAuthConfig.OAUTH_LOGIN) >= 0;
+ return request.getRequestURI().indexOf(GitHubOAuthConfig.GERRIT_LOGIN) >= 0;
}
public static boolean isOAuthLogout(HttpServletRequest request) {
- return request.getRequestURI().indexOf(GitHubOAuthConfig.OAUTH_LOGOUT) >= 0;
+ return request.getRequestURI().indexOf(GitHubOAuthConfig.GERRIT_LOGOUT) >= 0;
}
public static boolean isOAuthRequest(HttpServletRequest httpRequest) {
return OAuthProtocol.isOAuthLogin(httpRequest) || OAuthProtocol.isOAuthFinal(httpRequest);
}
- public AccessToken loginPhase2(HttpServletRequest request,
- HttpServletResponse response) throws IOException {
+ public AccessToken loginPhase2(HttpServletRequest request, String state)
+ throws IOException {
- HttpPost post = null;
+ String requestState = request.getParameter("state");
+ if (!Objects.equals(state, requestState)) {
+ throw new IOException("Invalid authentication state");
+ }
- post = new HttpPost(config.gitHubOAuthAccessTokenUrl);
+ HttpPost post = new HttpPost(config.gitHubOAuthAccessTokenUrl);
post.setHeader("Accept", "application/json");
List<NameValuePair> nvps = new ArrayList<NameValuePair>();
nvps.add(new BasicNameValuePair("client_id", config.gitHubClientId));
@@ -208,46 +216,40 @@
nvps.add(new BasicNameValuePair("code", request.getParameter("code")));
post.setEntity(new UrlEncodedFormEntity(nvps, Charsets.UTF_8));
- try {
- HttpResponse postResponse = http.execute(post);
- if (postResponse.getStatusLine().getStatusCode() != HttpURLConnection.HTTP_OK) {
- LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
- + " request for access token failed with status "
- + postResponse.getStatusLine());
- EntityUtils.consume(postResponse.getEntity());
- return null;
- }
-
- InputStream content = postResponse.getEntity().getContent();
- String tokenJsonString =
- CharStreams.toString(new InputStreamReader(content,
- StandardCharsets.UTF_8));
- AccessToken token = gson.fromJson(tokenJsonString, AccessToken.class);
- if (token.isError()) {
- LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
- + " returned an error token: " + token);
- }
- return token;
- } catch (IOException e) {
- LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
- + " request for access token failed", e);
- return null;
+ HttpResponse postResponse = httpProvider.get().execute(post);
+ if (postResponse.getStatusLine().getStatusCode() != HttpURLConnection.HTTP_OK) {
+ log.error("POST " + config.gitHubOAuthAccessTokenUrl
+ + " request for access token failed with status "
+ + postResponse.getStatusLine());
+ EntityUtils.consume(postResponse.getEntity());
+ throw new IOException("GitHub OAuth request failed");
}
+
+ InputStream content = postResponse.getEntity().getContent();
+ String tokenJsonString =
+ CharStreams.toString(new InputStreamReader(content,
+ StandardCharsets.UTF_8));
+ AccessToken token = gson.fromJson(tokenJsonString, AccessToken.class);
+ if (token.isError()) {
+ log.error("POST " + config.gitHubOAuthAccessTokenUrl
+ + " returned an error token: " + token);
+ throw new IOException("Invalid GitHub OAuth token");
+ }
+ return token;
}
private static String getURLEncoded(String url) {
try {
return URLEncoder.encode(url, "UTF-8");
} catch (UnsupportedEncodingException e) {
- // UTF-8 is hardcoded, cannot fail
- return null;
+ throw new IllegalStateException("Cannot find UTF-8 encoding", e);
}
}
public static String getTargetUrl(ServletRequest request) {
int meEnd = state(request).indexOf(ME_SEPARATOR);
if (meEnd > 0) {
- return state(request).substring(meEnd+1);
+ return state(request).substring(meEnd + 1);
} else {
return "";
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java
index bf0a4b1..6929d16 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java
@@ -51,16 +51,20 @@
private final GitHubOAuthConfig config;
private final Random retryRandom = new Random(System.currentTimeMillis());
- private SitePaths sites;
- private ScopedProvider<GitHubLogin> loginProvider;
+ private final SitePaths sites;
+ private final ScopedProvider<GitHubLogin> loginProvider;
+ private final OAuthProtocol oauth;
@Inject
- public OAuthWebFilter(GitHubOAuthConfig config, SitePaths sites,
+ public OAuthWebFilter(GitHubOAuthConfig config,
+ SitePaths sites,
+ OAuthProtocol oauth,
// We need to explicitly tell Guice the correct implementation
// as this filter is instantiated with a standard Gerrit WebModule
GitHubLogin.Provider loginProvider) {
this.config = config;
this.sites = sites;
+ this.oauth = oauth;
this.loginProvider = loginProvider;
}
@@ -120,12 +124,12 @@
private void login(ServletRequest request, HttpServletRequest httpRequest,
HttpServletResponse httpResponse, GitHubLogin ghLogin) throws IOException {
- if (ghLogin.login(httpRequest, httpResponse)) {
+ if (ghLogin.login(httpRequest, httpResponse, oauth)) {
GHMyself myself = ghLogin.getMyself();
String user = myself.getLogin();
- updateSecureConfigWithRetry(ghLogin.hub.getMyOrganizations().keySet(),
- user, ghLogin.token.accessToken);
+ updateSecureConfigWithRetry(ghLogin.getHub().getMyOrganizations().keySet(),
+ user, ghLogin.getToken().accessToken);
}
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PooledHttpClientProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PooledHttpClientProvider.java
new file mode 100644
index 0000000..0263877
--- /dev/null
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PooledHttpClientProvider.java
@@ -0,0 +1,75 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.github.oauth;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.httpd.ProxyProperties;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.apache.http.HttpHost;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.client.HttpClient;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.client.ProxyAuthenticationStrategy;
+import org.eclipse.jgit.lib.Config;
+
+import java.net.URL;
+
+public class PooledHttpClientProvider implements Provider<HttpClient> {
+ private final int maxTotalConnection;
+ private final int maxConnectionPerRoute;
+ private final ProxyProperties proxy;
+
+ @Inject
+ PooledHttpClientProvider(@GerritServerConfig Config config,
+ ProxyProperties proxyProperties) {
+ this.proxy = proxyProperties;
+ maxConnectionPerRoute = config.getInt("http", null,
+ "pooledMaxConnectionsPerRoute", 16);
+ maxTotalConnection = config.getInt("http", null,
+ "pooledMaxTotalConnections", 32);
+ }
+
+ @Override
+ public HttpClient get() {
+ HttpClientBuilder builder = HttpClientBuilder
+ .create()
+ .setMaxConnPerRoute(maxConnectionPerRoute)
+ .setMaxConnTotal(maxTotalConnection);
+
+ if (proxy.getProxyUrl() != null) {
+ URL url = proxy.getProxyUrl();
+ builder.setProxy(new HttpHost(url.getHost(), url.getPort()));
+ if (!Strings.isNullOrEmpty(proxy.getUsername())
+ && !Strings.isNullOrEmpty(proxy.getPassword())) {
+ UsernamePasswordCredentials creds = new UsernamePasswordCredentials(
+ proxy.getUsername(), proxy.getPassword());
+ CredentialsProvider credsProvider = new BasicCredentialsProvider();
+ credsProvider.setCredentials(new AuthScope(url.getHost(),
+ url.getPort()), creds);
+ builder.setDefaultCredentialsProvider(credsProvider);
+ builder.setProxyAuthenticationStrategy(
+ new ProxyAuthenticationStrategy());
+ }
+ }
+
+ return builder.build();
+ }
+}
diff --git a/github-plugin/pom.xml b/github-plugin/pom.xml
index fcf52a5..259f559 100644
--- a/github-plugin/pom.xml
+++ b/github-plugin/pom.xml
@@ -136,7 +136,7 @@
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
- <version>4.0</version>
+ <version>4.4</version>
</dependency>
<dependency>
<groupId>javax.mail</groupId>
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java
index f4d070d..17c3b9e 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java
@@ -16,11 +16,12 @@
import com.google.common.base.MoreObjects;
import com.google.common.collect.Maps;
import com.google.gerrit.server.config.AllProjectsNameProvider;
+import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-
import com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig;
import org.eclipse.jgit.lib.Config;
@@ -69,9 +70,13 @@
@Inject
- public GitHubConfig(@GerritServerConfig Config config, final SitePaths site, AllProjectsNameProvider allProjectsNameProvider)
+ public GitHubConfig(@GerritServerConfig Config config,
+ final SitePaths site,
+ AllProjectsNameProvider allProjectsNameProvider,
+ @CanonicalWebUrl String canonicalWebUrl,
+ AuthConfig authConfig)
throws MalformedURLException {
- super(config);
+ super(config, canonicalWebUrl, authConfig);
String[] wizardFlows =
config.getStringList(CONF_SECTION, null, CONF_WIZARD_FLOW);
for (String fromTo : wizardFlows) {
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java
index 29727a4..6bfbb16 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java
@@ -17,7 +17,6 @@
import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.google.inject.name.Names;
import com.google.inject.servlet.ServletModule;
-
import com.googlesource.gerrit.plugins.github.filters.GitHubOAuthFilter;
import com.googlesource.gerrit.plugins.github.git.CreateProjectStep;
import com.googlesource.gerrit.plugins.github.git.GitCloneStep;
@@ -25,8 +24,8 @@
import com.googlesource.gerrit.plugins.github.git.GitImporter;
import com.googlesource.gerrit.plugins.github.git.PullRequestImportJob;
import com.googlesource.gerrit.plugins.github.git.ReplicateProjectStep;
-import com.googlesource.gerrit.plugins.github.oauth.GitHubHttpProvider;
import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin;
+import com.googlesource.gerrit.plugins.github.oauth.PooledHttpClientProvider;
import com.googlesource.gerrit.plugins.github.oauth.ScopedProvider;
import com.googlesource.gerrit.plugins.github.replication.RemoteSiteUser;
import com.googlesource.gerrit.plugins.github.velocity.PluginVelocityRuntimeProvider;
@@ -41,7 +40,7 @@
@Override
protected void configureServlets() {
- bind(HttpClient.class).toProvider(GitHubHttpProvider.class);
+ bind(HttpClient.class).toProvider(PooledHttpClientProvider.class);
bind(new TypeLiteral<ScopedProvider<GitHubLogin>>() {}).to(GitHubLogin.Provider.class);
bind(new TypeLiteral<ScopedProvider<GitImporter>>() {}).to(GitImporter.Provider.class);
@@ -68,9 +67,9 @@
serve("*.css", "*.js", "*.png", "*.jpg", "*.woff", "*.gif", "*.ttf").with(
VelocityStaticServlet.class);
- serve("*.html").with(VelocityViewServlet.class);
serve("*.gh").with(VelocityControllerServlet.class);
+ serve("/static/*").with(VelocityViewServlet.class);
filter("*").through(GitHubOAuthFilter.class);
}
}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
index 261587f..564fd3e 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
@@ -13,16 +13,25 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github;
+import java.net.URISyntaxException;
+
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitStep;
+import com.google.gerrit.pgm.init.api.InitUtil;
import com.google.gerrit.pgm.init.api.Section;
import com.google.inject.Inject;
public class InitGitHub implements InitStep {
+ private static final String GITHUB_URL = "https://github.com";
+ private static final String GITHUB_API_URL = "https://api.github.com";
+ private static final String GITHUB_REGISTER_APPLICATION_PATH = "/settings/applications/new";
+ private static final String GERRIT_OAUTH_CALLBACK_PATH = "oauth";
+
private final ConsoleUI ui;
private final Section auth;
private final Section httpd;
- private Section github;
+ private final Section github;
+ private final Section gerrit;
@Inject
InitGitHub(final ConsoleUI ui, final Section.Factory sections) {
@@ -30,29 +39,60 @@
this.github = sections.get("github", null);
this.httpd = sections.get("httpd", null);
this.auth = sections.get("auth", null);
+ this.gerrit = sections.get("gerrit", null);
}
@Override
public void run() throws Exception {
ui.header("GitHub Integration");
- github.string("GitHub URL", "url", "https://github.com");
+ auth.set("httpHeader", "GITHUB_USER");
+ auth.set("httpExternalIdHeader", "GITHUB_OAUTH_TOKEN");
+ auth.set("loginUrl","/login");
+ auth.set("loginText", "Sign-in with GitHub");
+ auth.set("registerPageUrl", "/#/register");
- boolean gitHubAuth = ui.yesno(true, "Use GitHub for Gerrit login ?");
- if(gitHubAuth) {
- configureAuth();
- }
- }
+ github.string("GitHub URL", "url", GITHUB_URL);
+ github.string("GitHub API URL", "apiUrl", GITHUB_API_URL);
+ ui.message("\nNOTE: You might need to configure a proxy using http.proxy"
+ + " if you run Gerrit behind a firewall.\n");
- private void configureAuth() {
- github.string("ClientId", "clientId", null);
- github.string("ClientSecret", "clientSecret", null);
+ String gerritUrl = getAssumedCanonicalWebUrl();
+ ui.header("GitHub OAuth registration and credentials");
+ ui.message(
+ "Register Gerrit as GitHub application on:\n" +
+ "%s%s\n\n",
+ github.get("url"), GITHUB_REGISTER_APPLICATION_PATH);
+ ui.message("Settings (assumed Gerrit URL: %s)\n", gerritUrl);
+ ui.message("* Application name: Gerrit Code Review\n");
+ ui.message("* Homepage URL: %s\n", gerritUrl);
+ ui.message("* Authorization callback URL: %s%s\n\n", gerritUrl, GERRIT_OAUTH_CALLBACK_PATH);
+ ui.message("After registration is complete, enter the generated OAuth credentials:\n");
+ github.string("GitHub Client ID", "clientId", null);
+ github.passwordForKey("GitHub Client Secret", "clientSecret");
auth.string("HTTP Authentication Header", "httpHeader", "GITHUB_USER");
auth.set("type", "HTTP");
httpd.set("filterClass", "com.googlesource.gerrit.plugins.github.oauth.OAuthFilter");
}
+ private String getAssumedCanonicalWebUrl() {
+ String url = gerrit.get("canonicalWebUrl");
+ if (url != null) {
+ return url;
+ }
+
+ String httpListen = httpd.get("listenUrl");
+ if (httpListen != null) {
+ try {
+ return InitUtil.toURI(httpListen).toString();
+ } catch (URISyntaxException e) {
+ }
+ }
+
+ return String.format("http://%s:8080/", InitUtil.hostname());
+ }
+
@Override
public void postRun() throws Exception {
}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java
index b844ecc..59ceb46 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java
@@ -18,6 +18,7 @@
import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin;
import com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol;
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
import com.googlesource.gerrit.plugins.github.oauth.ScopedProvider;
@@ -33,6 +34,7 @@
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
@Singleton
public class GitHubOAuthFilter implements Filter {
@@ -40,12 +42,14 @@
private final ScopedProvider<GitHubLogin> loginProvider;
private final Scope[] authScopes;
+ private final OAuthProtocol oauth;
@Inject
- public GitHubOAuthFilter(final ScopedProvider<GitHubLogin> loginProvider,
- final GitHubOAuthConfig githubOAuthConfig) {
+ public GitHubOAuthFilter(ScopedProvider<GitHubLogin> loginProvider,
+ GitHubOAuthConfig githubOAuthConfig, OAuthProtocol oauth) {
this.loginProvider = loginProvider;
this.authScopes = githubOAuthConfig.getDefaultScopes();
+ this.oauth = oauth;
}
@Override
@@ -58,7 +62,8 @@
GitHubLogin hubLogin = loginProvider.get((HttpServletRequest) request);
LOG.debug("GitHub login: " + hubLogin);
if (!hubLogin.isLoggedIn()) {
- hubLogin.login(request, response, authScopes);
+ hubLogin.login((HttpServletRequest) request,
+ (HttpServletResponse) response, oauth, authScopes);
return;
} else {
chain.doFilter(request, response);
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java
index 8e4f57e..c5cf07a 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java
@@ -25,6 +25,7 @@
import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.URIish;
import org.kohsuke.github.GHRepository;
+import org.kohsuke.github.GitHub;
import java.io.IOException;
@@ -39,14 +40,15 @@
private final String organisation;
private final String repository;
- private final GitHubLogin ghLogin;
private final String cloneUrl;
+ private final String username;
+ private final String password;
@Delegate
private GHRepository ghRepository;
public String getCloneUrl() {
- return cloneUrl.replace("://", "://" + ghLogin.getMyself().getLogin() + "@");
+ return cloneUrl.replace("://", "://" + username + "@");
}
public String getOrganisation() {
@@ -65,9 +67,12 @@
this.cloneUrl = gitHubUrl + "/" + organisation + "/" + repository + ".git";
this.organisation = organisation;
this.repository = repository;
- this.ghLogin = ghLoginProvider.get();
+ GitHubLogin ghLogin = ghLoginProvider.get();
+ GitHub gh = ghLogin.getHub();
+ this.username = ghLogin.getMyself().getLogin();
+ this.password = ghLogin.getToken().accessToken;
this.ghRepository =
- ghLogin.getHub().getRepository(organisation + "/" + repository);
+ gh.getRepository(organisation + "/" + repository);
}
public CredentialsProvider getCredentialsProvider() {
@@ -97,13 +102,13 @@
throws UnsupportedCredentialItem {
String username = uri.getUser();
if (username == null) {
- username = ghLogin.getMyself().getLogin();
+ username = GitHubRepository.this.username;
}
if (username == null) {
return false;
}
- String password = ghLogin.getToken().accessToken;
+ String password = GitHubRepository.this.password;
if (password == null) {
return false;
}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java
index 44e5a23..5410c78 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.git;
+import java.io.IOException;
+
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -43,7 +45,7 @@
final ScopedProvider<GitHubLogin> ghLoginProvider,
@GitHubURL String gitHubUrl,
@Assisted("organisation") String organisation,
- @Assisted("name") String repository) {
+ @Assisted("name") String repository) throws IOException {
super(gitHubUrl, organisation, repository, gitHubRepoFactory);
LOG.debug("Gerrit ReplicateProject " + organisation + "/" + repository);
this.replicationConfig = replicationConfig;
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java
index b9f5d34..cd1a202 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java
@@ -82,10 +82,7 @@
+ nextPage.uri;
}
- String pathInfo = MoreObjects.firstNonNull(destUrl, servletPath);
- if (!pathInfo.startsWith(STATIC_PREFIX)) {
- resp.sendError(HttpStatus.SC_NOT_FOUND);
- }
+ String pathInfo = STATIC_PREFIX + MoreObjects.firstNonNull(destUrl, servletPath);
try {
Template template = velocityRuntime.getTemplate(pathInfo, "UTF-8");
@@ -104,7 +101,7 @@
}
}
- private PluginVelocityModel initVelocityModel(HttpServletRequest request) {
+ private PluginVelocityModel initVelocityModel(HttpServletRequest request) throws IOException {
PluginVelocityModel model = modelProvider.get();
GitHubLogin gitHubLogin = loginProvider.get(request);
model.put("myself", gitHubLogin.getMyself());
diff --git a/github-plugin/src/main/resources/Documentation/about.md b/github-plugin/src/main/resources/Documentation/about.md
new file mode 100644
index 0000000..5ba04a6
--- /dev/null
+++ b/github-plugin/src/main/resources/Documentation/about.md
@@ -0,0 +1,26 @@
+
+This plugins allows to integrate Gerrit with external set of users configured
+on GitHub.
+It relies on the installation of the github-oauth Java library under the `$GERRIT_SITE/lib`
+in order filter all the HTTP requests through the GitHub OAuth 2.0 secure authentication.
+
+GitHub application registration
+-------------------------------
+
+GitHub uses OAuth2 as protocol to allow external apps request authorization to private
+details in a user's GitHub account without getting their password. This is
+preferred over Basic Authentication because tokens can be limited to specific
+types of data, and can be revoked by users at any time.
+
+Site owners have to register their application before getting started. For
+more information see [GitHub application registration page](https://github.com/settings/applications/new).
+A registered OAuth application is assigned a unique `Client ID` and `Client
+Secret`. The `Client Secret` should never be shared.
+
+The Gerrit OAuth callback `<gerrit canonical URL>/oauth`
+needs to be specified in the GitHub application registration to establish mutual
+trust between the two domains and exchange the authorization codes. The use of HTTPS
+for Gerrit is strongly recommended for keeping the secrets exchange confidential.
+
+`auth.httpHeader` is set to `GITHUB_USER` with this authentication method and `auth.type`
+must be set to HTTP.
\ No newline at end of file
diff --git a/github-plugin/src/main/resources/Documentation/config.md b/github-plugin/src/main/resources/Documentation/config.md
index 2d18944..9d7c87b 100644
--- a/github-plugin/src/main/resources/Documentation/config.md
+++ b/github-plugin/src/main/resources/Documentation/config.md
@@ -1,13 +1,6 @@
-Plugin @PLUGIN@
-===============
-This plugins allows to integrate Gerrit with external set of users configured
-on GitHub.
-It relies on the installation of the github-oauth Java library under the $GERRIT_SITE/lib
-in order filter all the HTTP requests through the GitHub OAuth 2.0 secure authentication.
-
-GitHub init step
-----------------
+GitHub configuration during Gerrit init
+---------------------------------------
This plugin provides a customized Gerrit init step for the self-configuration of
the main GitHub and Gerrit authentication settings for allowing the github-oauth
@@ -18,19 +11,58 @@
See below a sample session of relevant init steps for a default
configuration pointing to the Web GitHub instance:
- *** User Authentication
- ***
+```
+ *** GitHub Integration
+ ***
- Authentication method []: HTTP
- Get username from custom HTTP header [Y/n]? Y
- Username HTTP header []: GITHUB_USER
- SSO logout URL : /oauth/reset
+ GitHub URL [https://github.com]:
+ GitHub API URL [https://api.github.com]:
+ NOTE: You might need to configure a proxy using http.proxy if you run Gerrit behind a firewall.
- *** GitHub Integration
- ***
+ *** GitHub OAuth registration and credentials
+ ***
- GitHub URL [https://github.com]:
- Use GitHub for Gerrit login ? [Y/n]? Y
- ClientId []: 384cbe2e8d98192f9799
- ClientSecret []: f82c3f9b3802666f2adcc4c8cacfb164295b0a99
+ Register Gerrit as GitHub application on:
+ https://github.com/settings/applications/new
+
+ Settings (assumed Gerrit URL: http://localhost:8080/)
+ * Application name: Gerrit Code Review
+ * Homepage URL: http://localhost:8080/
+ * Authorization callback URL: http://localhost:8080/oauth
+
+ After registration is complete, enter the generated OAuth credentials:
+ GitHub Client ID [1ebea047915210179cf5]:
+ ClientSecret []: f82c3f9b3802666f2adcc4c8cacfb164295b0a99
+ confirm password :
+ HTTP Authentication Header [GITHUB_USER]:
+```
+
+Configuration
+-------------
+
+GitHub plugin read his configuration from gerrit.config under the `[github]` section.
+
+github.url
+: GitHub homepage URL. Default is `https://github.com`. Can be customized to a different
+ value for GitHub:Enterprise integration
+
+**NOTE** You might need to configure a proxy using the configuration `http.proxy` if you run
+Gerrit behind a firewall.
+
+github.apiUrl
+: GitHub API URL. Default is `https://api.github.com`. Can be customized to a different
+ value for GitHub:Enterprise integration
+
+github.clientId
+: The `Client ID`, that was received from GitHub when the application was registered. Required.
+
+github.clientSecret
+: The `Client Secret`, that was received from GitHub when the application was registered. Required.
+
+github.scopes
+: The GitHub scopes for allowing access to the user's public or private profile, organisations and
+ repositories. See [GitHub Scopes definition](https://developer.github.com/v3/oauth/#scopes)
+ for a detailed description of available scopes and their associated permissions.
+ Default is empty read-only access to public
+ information (includes public user profile info, public repository info, and gists).
