Allows user changing scope within a Gerrit session
Even after the user has been already authenticated with GitHub with a
default OAuth scope, it is possible now to visualize the current scope
and changing it on-the-fly.
This is particularly useful during repositories import where a user
realises that the initial scope wasn’t enough to display private
organizations or repositories.
Change-Id: I5af497571db7482bd74ddf25ffccc26fea3bcaa3
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 8a358e5..8cc52ae 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
@@ -93,11 +93,8 @@
return token != null;
}
- public boolean login(HttpServletRequest request,
+ public void login(HttpServletRequest request,
HttpServletResponse response, OAuthProtocol oauth, Scope... scopes) throws IOException {
- if (isLoggedIn()) {
- return true;
- }
log.debug("Login " + this);
if (OAuthProtocol.isOAuthFinal(request)) {
@@ -108,7 +105,6 @@
if (isLoggedIn()) {
log.debug("Login-SUCCESS " + this);
response.sendRedirect(OAuthProtocol.getTargetUrl(request));
- return true;
}
} else {
Set<String> configuredScopesProfiles = config.scopes.keySet();
@@ -124,7 +120,6 @@
state = oauth.loginPhase1(request, response, loginScopes);
}
}
- return false;
}
public void logout() {
@@ -144,7 +139,11 @@
}
public GitHub getHub() throws IOException {
- return GitHub.connectUsingOAuth(config.gitHubApiUrl, token.accessToken);
+ if (token == null) {
+ return null;
+ } else {
+ return GitHub.connectUsingOAuth(config.gitHubApiUrl, token.accessToken);
+ }
}
private String getScopesKey(HttpServletRequest request,
@@ -167,7 +166,7 @@
return scopeRequested;
}
- private String getScopesKeyFromCookie(HttpServletRequest request) {
+ public String getScopesKeyFromCookie(HttpServletRequest request) {
Cookie[] cookies = request.getCookies();
if (cookies == null) {
return null;
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 d8f620f..e2ea0f8 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
@@ -67,6 +67,8 @@
public final int fileUpdateMaxRetryCount;
public final int fileUpdateMaxRetryIntervalMsec;
public final String oauthHttpHeader;
+
+ @Getter
public final String scopeSelectionUrl;
@Inject
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
index 978fd34..2f98970 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.oauth;
+import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.google.gerrit.httpd.GitOverHttpServlet;
import com.google.gerrit.httpd.XGerritAuth;
@@ -25,6 +26,7 @@
import java.io.IOException;
import java.util.Arrays;
+import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;
@@ -43,8 +45,12 @@
private static Pattern GIT_HTTP_REQUEST_PATTERN = Pattern
.compile(GitOverHttpServlet.URL_REGEX);
private static final Set<String> GERRIT_STATIC_RESOURCES_EXTS = Sets
- .newHashSet(Arrays.asList("css", "png", "jpg", "gif", "woff", "otf",
- "ttf", "map", "js", "swf", "txt"));
+ .newHashSet("css", "png", "jpg", "gif", "woff", "otf",
+ "ttf", "map", "js", "swf", "txt");
+ private static final Set<String> GERRIT_WHITELISTED_PATHS = Sets
+ .newHashSet("Documentation");
+ private static final Set<String> GERRIT_WHITELISTED_PAGES = Sets
+ .newHashSet("scope.html");
private final GitHubOAuthConfig config;
private final OAuthGitFilter gitFilter;
@@ -70,36 +76,54 @@
FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpRequest = (HttpServletRequest) request;
- if (!config.enabled || isStaticResource(httpRequest)
- || isRpcCall(httpRequest) || isAuthenticatedRestCall(httpRequest)) {
- chain.doFilter(request, response);
- return;
- }
-
String requestUrl = httpRequest.getRequestURI();
- if (GIT_HTTP_REQUEST_PATTERN.matcher(requestUrl).matches()) {
- gitFilter.doFilter(request, response, chain);
+
+ if (!config.enabled || skipOAuth(httpRequest)) {
+ chain.doFilter(request, response);
} else {
- webFilter.doFilter(request, response, chain);
+
+ if (GIT_HTTP_REQUEST_PATTERN.matcher(requestUrl).matches()) {
+ gitFilter.doFilter(request, response, chain);
+ } else {
+ System.out.println("Authorising " + requestUrl);
+ webFilter.doFilter(request, response, chain);
+ }
}
}
- private boolean isAuthenticatedRestCall(HttpServletRequest httpRequest) {
+ public static boolean skipOAuth(HttpServletRequest httpRequest) {
+ return isStaticResource(httpRequest)
+ || isRpcCall(httpRequest) || isAuthenticatedRestCall(httpRequest)
+ || isWhitelisted(httpRequest);
+ }
+
+ private static boolean isAuthenticatedRestCall(HttpServletRequest httpRequest) {
return !StringUtils.isEmpty(httpRequest
.getHeader(XGerritAuth.X_GERRIT_AUTH));
}
- private boolean isStaticResource(HttpServletRequest httpRequest) {
- String pathExt =
- StringUtils.substringAfterLast(httpRequest.getRequestURI(), ".");
+ private static boolean isStaticResource(HttpServletRequest httpRequest) {
+ String requestURI = httpRequest.getRequestURI();
+ String pathExt = StringUtils.substringAfterLast(requestURI, ".");
if (StringUtils.isEmpty(pathExt)) {
return false;
}
- return GERRIT_STATIC_RESOURCES_EXTS.contains(pathExt.toLowerCase());
+ boolean staticResource =
+ GERRIT_STATIC_RESOURCES_EXTS.contains(pathExt.toLowerCase());
+ System.out.println("requestUri: " + requestURI + " pathExt: " + pathExt
+ + " static: " + staticResource);
+ return staticResource;
}
- private boolean isRpcCall(HttpServletRequest httpRequest) {
+ private static boolean isWhitelisted(HttpServletRequest httpRequest) {
+ String[] requestPathParts = httpRequest.getRequestURI().split("/");
+ return (requestPathParts.length > 1 && (GERRIT_WHITELISTED_PATHS
+ .contains(requestPathParts[1]) || GERRIT_WHITELISTED_PAGES
+ .contains(requestPathParts[requestPathParts.length - 1])));
+ }
+
+ private static boolean isRpcCall(HttpServletRequest httpRequest) {
return httpRequest.getRequestURI().indexOf("/rpc/") >= 0;
}
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 5b5a0a8..a349c42 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
@@ -192,6 +192,7 @@
private static final String ME_SEPARATOR = ",";
private static final Logger log = LoggerFactory
.getLogger(OAuthProtocol.class);
+ private static final String FINAL_URL_PARAM = "final";
private static SecureRandom randomState = newRandomGenerator();
private final GitHubOAuthConfig config;
@@ -315,16 +316,18 @@
}
public static boolean isOAuthFinal(HttpServletRequest request) {
- return Strings.emptyToNull(request.getParameter("code")) != null;
+ return Strings.emptyToNull(request.getParameter("code")) != null
+ && request.getParameter(FINAL_URL_PARAM) == null;
}
public static boolean isOAuthFinalForOthers(HttpServletRequest request) {
String targetUrl = getTargetUrl(request);
- if(targetUrl.equals(request.getRequestURI())) {
+ if (targetUrl.equals(request.getRequestURI())) {
return false;
}
-
- return Strings.emptyToNull(request.getParameter("code")) != null;
+
+ return Strings.emptyToNull(request.getParameter("code")) != null
+ && request.getParameter(FINAL_URL_PARAM) == null;
}
public String newRandomState(String redirectUrl) {
@@ -334,7 +337,9 @@
}
public static boolean isOAuthLogin(HttpServletRequest request) {
- return request.getRequestURI().indexOf(GitHubOAuthConfig.GERRIT_LOGIN) >= 0;
+ String requestUri = request.getRequestURI();
+ return requestUri.indexOf(GitHubOAuthConfig.GERRIT_LOGIN) >= 0
+ && request.getParameter(FINAL_URL_PARAM) == null;
}
public static boolean isOAuthLogout(HttpServletRequest request) {
@@ -349,7 +354,8 @@
HttpServletResponse response, String state) throws IOException {
String requestState = request.getParameter("state");
if (!Objects.equals(state, requestState)) {
- throw new IOException("Invalid authentication state");
+ throw new IOException("Invalid authentication state: expected '" + state
+ + "' but was '" + requestState + "'");
}
return getAccessToken(new OAuthVerifier(request.getParameter("code")));
@@ -398,10 +404,11 @@
public static String getTargetUrl(ServletRequest request) {
int meEnd = state(request).indexOf(ME_SEPARATOR);
+ String finalUrlSuffix = "?" + FINAL_URL_PARAM + "=true";
if (meEnd > 0) {
- return state(request).substring(meEnd + 1);
+ return state(request).substring(meEnd + 1) + finalUrlSuffix;
} else {
- return "";
+ return finalUrlSuffix;
}
}
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 6291481..d11745e 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
@@ -85,26 +85,21 @@
try {
GitHubLogin ghLogin = loginProvider.get(httpRequest);
- if (OAuthProtocol.isOAuthLogout(httpRequest)) {
- logout(request, response, chain, httpRequest);
- } else if (OAuthProtocol.isOAuthRequest(httpRequest)
- && !ghLogin.isLoggedIn()) {
+ if (OAuthProtocol.isOAuthRequest(httpRequest)) {
login(request, httpRequest, httpResponse, ghLogin);
} else {
+ if (OAuthProtocol.isOAuthLogout(httpRequest)) {
+ response = logout(request, response, chain, httpRequest);
+ }
+
if (ghLogin != null && ghLogin.isLoggedIn()) {
httpRequest =
new AuthenticatedHttpRequest(httpRequest, config.httpHeader,
- ghLogin.getMyself().getLogin(),
- config.oauthHttpHeader,
+ ghLogin.getMyself().getLogin(), config.oauthHttpHeader,
GITHUB_EXT_ID + ghLogin.getToken().accessToken);
}
- if (OAuthProtocol.isOAuthFinalForOthers(httpRequest)) {
- httpResponse.sendRedirect(OAuthProtocol
- .getTargetOAuthFinal(httpRequest));
- } else {
- chain.doFilter(httpRequest, response);
- }
+ chain.doFilter(httpRequest, response);
}
} finally {
HttpSession httpSession = httpRequest.getSession();
@@ -124,7 +119,8 @@
private void login(ServletRequest request, HttpServletRequest httpRequest,
HttpServletResponse httpResponse, GitHubLogin ghLogin) throws IOException {
- if (ghLogin.login(httpRequest, httpResponse, oauth)) {
+ ghLogin.login(httpRequest, httpResponse, oauth);
+ if (ghLogin.isLoggedIn()) {
GHMyself myself = ghLogin.getMyself();
String user = myself.getLogin();
@@ -133,14 +129,12 @@
}
}
- private void logout(ServletRequest request, ServletResponse response,
- FilterChain chain, HttpServletRequest httpRequest) throws IOException,
- ServletException {
+ private ServletResponse logout(ServletRequest request,
+ ServletResponse response, FilterChain chain,
+ HttpServletRequest httpRequest) throws IOException, ServletException {
getGitHubLogin(request).logout();
- GitHubLogoutServletResponse bufferedResponse =
- new GitHubLogoutServletResponse((HttpServletResponse) response,
- config.logoutRedirectUrl);
- chain.doFilter(httpRequest, bufferedResponse);
+ return new GitHubLogoutServletResponse((HttpServletResponse) response,
+ config.logoutRedirectUrl);
}
private GitHubLogin getGitHubLogin(ServletRequest request) {
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubTopMenu.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubTopMenu.java
index 20696ee..61431e0 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubTopMenu.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubTopMenu.java
@@ -38,10 +38,12 @@
@Inject
public GitHubTopMenu(@PluginName String pluginName,
Provider<CurrentUser> userProvider,
- AuthConfig authConfig) {
+ AuthConfig authConfig,
+ GitHubConfig ghConfig) {
String baseUrl = "/plugins/" + pluginName;
this.menuEntries =
Arrays.asList(new MenuEntry("GitHub", Arrays.asList(
+ getItem("Scope", ghConfig.scopeSelectionUrl),
getItem("Profile", baseUrl + "/static/account.html"),
getItem("Repositories", baseUrl + "/static/repositories.html"),
getItem("Pull Requests", baseUrl + "/static/pullrequests.html"))));
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 2bac968..933533f 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
@@ -13,11 +13,22 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.filters;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Collections2;
+import com.google.gerrit.reviewdb.client.AccountExternalId;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin;
import com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthFilter;
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthWebFilter;
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
import com.googlesource.gerrit.plugins.github.oauth.ScopedProvider;
@@ -26,6 +37,7 @@
import java.io.IOException;
import java.util.Arrays;
+import java.util.Collection;
import java.util.List;
import javax.servlet.Filter;
@@ -41,21 +53,25 @@
public class GitHubOAuthFilter implements Filter {
private Logger LOG = LoggerFactory.getLogger(GitHubOAuthFilter.class);
- private static final List<String> whiteList = Arrays.asList(".png", ".jpg",
- ".js", ".css");
-
private final ScopedProvider<GitHubLogin> loginProvider;
private final Scope[] authScopes;
private final OAuthProtocol oauth;
private final GitHubOAuthConfig config;
+ private final Provider<CurrentUser> userProvider;
+ private final AccountCache accountCache;
@Inject
public GitHubOAuthFilter(ScopedProvider<GitHubLogin> loginProvider,
- GitHubOAuthConfig githubOAuthConfig, OAuthProtocol oauth) {
+ GitHubOAuthConfig githubOAuthConfig,
+ OAuthProtocol oauth,
+ Provider<CurrentUser> userProvider,
+ AccountCache accountCache) {
this.loginProvider = loginProvider;
this.authScopes = githubOAuthConfig.getDefaultScopes();
this.oauth = oauth;
this.config = githubOAuthConfig;
+ this.userProvider = userProvider;
+ this.accountCache = accountCache;
}
@Override
@@ -67,23 +83,41 @@
FilterChain chain) throws IOException, ServletException {
GitHubLogin hubLogin = loginProvider.get((HttpServletRequest) request);
LOG.debug("GitHub login: " + hubLogin);
- if (!hubLogin.isLoggedIn() && !isWhiteListed(request)) {
- hubLogin.login((HttpServletRequest) request,
- (HttpServletResponse) response, oauth, authScopes);
- return;
- } else {
- chain.doFilter(request, response);
+ CurrentUser user = userProvider.get();
+ if (!hubLogin.isLoggedIn()
+ && !OAuthFilter.skipOAuth((HttpServletRequest) request)
+ && user.isIdentifiedUser()) {
+ AccountExternalId gitHubExtId = getGitHubExternalId(user);
+
+ String oauthToken =
+ gitHubExtId.getSchemeRest()
+ .substring(OAuthWebFilter.GITHUB_EXT_ID.length());
+ hubLogin.login(new AccessToken(oauthToken));
}
+
+ chain.doFilter(request, response);
}
- private boolean isWhiteListed(ServletRequest request) {
- String requestUri = ((HttpServletRequest) request).getRequestURI();
- for (String suffix : whiteList) {
- if (requestUri.endsWith(suffix)) {
- return true;
- }
+ private AccountExternalId getGitHubExternalId(CurrentUser user) {
+ Collection<AccountExternalId> accountExtIds =
+ accountCache.get(((IdentifiedUser) user).getAccountId())
+ .getExternalIds();
+ Collection<AccountExternalId> gitHubExtId =
+ Collections2.filter(accountExtIds,
+ new Predicate<AccountExternalId>() {
+ @Override
+ public boolean apply(AccountExternalId externalId) {
+ return externalId.isScheme(AccountExternalId.SCHEME_EXTERNAL)
+ && externalId.getSchemeRest().startsWith(
+ OAuthWebFilter.GITHUB_EXT_ID);
+ }
+ });
+
+ if (gitHubExtId.isEmpty()) {
+ throw new IllegalStateException("Current Gerrit user "
+ + user.getUserName() + " has no GitHub OAuth external ID");
}
- return config.scopeSelectionUrl.endsWith(requestUri);
+ return gitHubExtId.iterator().next();
}
@Override
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 9b876d4..c91b853 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
@@ -110,6 +110,7 @@
if (user.isIdentifiedUser()) {
model.put("user", user);
model.put("hub", gitHubLogin.getHub());
+ model.put("scopeCookie", gitHubLogin.getScopesKeyFromCookie(request));
}
for (Entry<String, String[]> reqPar : request.getParameterMap().entrySet()) {
diff --git a/github-plugin/src/main/resources/static/repositories.html b/github-plugin/src/main/resources/static/repositories.html
index e4cd855..fe8deb1 100644
--- a/github-plugin/src/main/resources/static/repositories.html
+++ b/github-plugin/src/main/resources/static/repositories.html
@@ -53,6 +53,7 @@
</select>
<input type="text" id="filter" class="filter" name="filter" placeholder="Filter by name" />
</li>
+ <li class="info"><p>Not seeing your organizations or repositories? <a href="$config.scopeSelectionUrl">Login with a different GitHub Scope</a> and try again.</p></li>
</ul>
<div class="loading">
<p>Loading list of GitHub repositories ...</p>
diff --git a/github-plugin/src/main/resources/static/scope.html b/github-plugin/src/main/resources/static/scope.html
index 2b131fc..29795f0 100644
--- a/github-plugin/src/main/resources/static/scope.html
+++ b/github-plugin/src/main/resources/static/scope.html
@@ -37,23 +37,28 @@
<h5>Which level of GitHub access do you need?</h5>
<ul class="scopes">
#foreach ( $scope in $config.scopes.keySet() )
- <li>
- #set ( $scopeName = $scope.substring(6) )
- #if ( $scopeName == "")
- <input type="radio" name="scope" value="scopes" checked><p class="scope">DEFAULT</p>
- #else
- <input type="radio" name="scope" value="scopes$scopeName"><p class="scope">$scopeName</p>
- #end
- <p>
- #set ( $scopeItems = $config.scopes.get($scope) )
- #foreach ( $scopeItem in $scopeItems )
- $scopeItem.description
- #if ( $velocityCount < $scopeItems.size())
- ,
- #end
- #end
- </p>
- </li>
+ <li>
+ #set ( $scopeName = $scope.substring(6) )
+ #set ( $checked = "" )
+ #if ( ( $scopeCookie && $scopeCookie == $scope ) || $scopeName == "" )
+ #set ( $checked = "checked" )
+ #end
+
+ #if ( $scopeName == "")
+ <input type="radio" name="scope" value="scopes" $checked ><p class="scope">DEFAULT</p>
+ #else
+ <input type="radio" name="scope" value="scopes$scopeName" $checked ><p class="scope">$scopeName</p>
+ #end
+ <p>
+ #set ( $scopeItems = $config.scopes.get($scope) )
+ #foreach ( $scopeItem in $scopeItems )
+ $scopeItem.description
+ #if ( $velocityCount < $scopeItems.size())
+ ,
+ #end
+ #end
+ </p>
+ </li>
#end
</li>
</ul>
