Adds interstitial login screen for selecting GitHub OAuth scopes
When Gerrit admin to configure different GitHub login profiles
with a set of associated scopes, the login process will first show
a list of scopes available and allow the user to choose one.
Change-Id: I8c1f24b46fbe9a868237b4958f112daec6385719
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 f9afeeb..8a358e5 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
@@ -38,6 +38,7 @@
import org.slf4j.LoggerFactory;
import com.google.common.base.MoreObjects;
+import com.google.common.base.Strings;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken;
@@ -108,15 +109,22 @@
log.debug("Login-SUCCESS " + this);
response.sendRedirect(OAuthProtocol.getTargetUrl(request));
return true;
- } else {
- return false;
}
} else {
- this.loginScopes = getScopes(getScopesKey(request, response), scopes);
- log.debug("Login-PHASE1 " + this);
- state = oauth.loginPhase1(request, response, loginScopes);
- return false;
+ Set<String> configuredScopesProfiles = config.scopes.keySet();
+ String scopeRequested = getScopesKey(request, response);
+ if (Strings.isNullOrEmpty(scopeRequested)
+ && configuredScopesProfiles.size() > 1) {
+ response.sendRedirect(config.scopeSelectionUrl);
+ } else {
+ this.loginScopes =
+ getScopes(MoreObjects.firstNonNull(scopeRequested, "scopes"),
+ scopes);
+ log.debug("Login-PHASE1 " + this);
+ state = oauth.loginPhase1(request, response, loginScopes);
+ }
}
+ return false;
}
public void logout() {
@@ -146,14 +154,17 @@
scopeRequested = getScopesKeyFromCookie(request);
}
- if (scopeRequested != null) {
+ boolean rememberScope =
+ Strings.nullToEmpty(request.getParameter("rememberScope"))
+ .equalsIgnoreCase("on");
+ if (scopeRequested != null && rememberScope) {
Cookie scopeCookie = new Cookie("scope", scopeRequested);
scopeCookie.setPath("/");
scopeCookie.setMaxAge((int) SCOPE_COOKIE_NEVER_EXPIRES);
response.addCookie(scopeCookie);
}
-
- return MoreObjects.firstNonNull(scopeRequested, "scopes");
+
+ return scopeRequested;
}
private String getScopesKeyFromCookie(HttpServletRequest request) {
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 ca242d6..d8f620f 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
@@ -13,8 +13,17 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.oauth;
-import com.google.common.base.MoreObjects;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import lombok.Getter;
+
+import org.eclipse.jgit.lib.Config;
+
import com.google.common.base.CharMatcher;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
@@ -26,14 +35,6 @@
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
-import org.eclipse.jgit.lib.Config;
-
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
@Singleton
public
class GitHubOAuthConfig {
@@ -47,6 +48,7 @@
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 static final String GITHUB_PLUGIN_OAUTH_SCOPE = "/plugins/github-plugin/static/scope.html";
public final String gitHubUrl;
public final String gitHubApiUrl;
@@ -58,15 +60,19 @@
public final String oAuthFinalRedirectUrl;
public final String gitHubOAuthAccessTokenUrl;
public final boolean enabled;
+
+ @Getter
public final Map<String, List<OAuthProtocol.Scope>> scopes;
+
public final int fileUpdateMaxRetryCount;
public final int fileUpdateMaxRetryIntervalMsec;
public final String oauthHttpHeader;
+ public final String scopeSelectionUrl;
@Inject
protected
GitHubOAuthConfig(@GerritServerConfig Config config,
- @CanonicalWebUrl String canonicalWebUrl,
+ @CanonicalWebUrl String canonicalWebUrl,
AuthConfig authConfig) {
httpHeader =
Preconditions.checkNotNull(
@@ -95,6 +101,11 @@
config.getString(CONF_SECTION, null, "logoutRedirectUrl");
oAuthFinalRedirectUrl =
trimTrailingSlash(canonicalWebUrl) + GERRIT_OAUTH_FINAL;
+ scopeSelectionUrl =
+ trimTrailingSlash(canonicalWebUrl)
+ + MoreObjects.firstNonNull(
+ config.getString(CONF_SECTION, null, "scopeSelectionUrl"),
+ GITHUB_PLUGIN_OAUTH_SCOPE);
enabled =
config.getString("auth", null, "type").equalsIgnoreCase(
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 19479d7..5b5a0a8 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
@@ -37,6 +37,8 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import lombok.Getter;
+
import com.google.gerrit.extensions.auth.oauth.OAuthToken;
import com.google.gerrit.extensions.auth.oauth.OAuthVerifier;
@@ -53,44 +55,44 @@
* Grants read-only access to public information (includes public user
* profile info, public repository info, and gists)
*/
- DEFAULT(""),
+ DEFAULT("", "Read-only public information"),
/**
* Grants read/write access to profile info only. Note that this scope
* includes user:email and user:follow.
*/
- USER("user"),
+ USER("user", "Read/write profile info"),
/**
* Grants read access to a user’s email addresses.
*/
- USER_EMAIL("user:email"),
+ USER_EMAIL("user:email", "Read-only user's e-mail"),
/**
* Grants access to follow or unfollow other users.
*/
- USER_FOLLOW("user:follow"),
+ USER_FOLLOW("user:follow", "(Un)Follow users"),
/**
* Grants read/write access to code, commit statuses, collaborators, and
- * deployment statuses for public repositories and organizations. Also
+ * deployment statuses for public repositories. Also
* required for starring public repositories.
*/
- PUBLIC_REPO("public_repo"),
+ PUBLIC_REPO("public_repo", "Read/write to public repositories"),
/**
* Grants read/write access to code, commit statuses, collaborators, and
* deployment statuses for public and private repositories and
* organizations.
*/
- REPO("repo"),
+ REPO("repo", "Read/write to all public/private repositories"),
/**
* Grants access to deployment statuses for public and private repositories.
* This scope is only necessary to grant other users or services access to
* deployment statuses, without granting access to the code.
*/
- REPO_DEPLOYMENT("repo_deployment"),
+ REPO_DEPLOYMENT("repo_deployment", "Read-only deployment statuses"),
/**
* Grants read/write access to public and private repository commit
@@ -98,40 +100,43 @@
* access to private repository commit statuses without granting access to
* the code.
*/
- REPO_STATUS("repo:status"),
+ REPO_STATUS("repo:status", "Read/write commit statuses"),
/**
* Grants access to delete admin-able repositories.
*/
- DELETE_REPO("delete_repo"),
+ DELETE_REPO("delete_repo", "Delete repositories"),
/**
* Grants read access to a user’s notifications. repo also provides this
* access.
*/
- NOTIFICATIONS("notifications"),
+ NOTIFICATIONS("notifications", "Read user's notifications"),
/**
* Grants write access to gists.
*/
- GIST("gist"),
+ GIST("gist", "Write Gists"),
/**
* Grants read and ping access to hooks in public or private repositories.
*/
- READ_REPO_HOOK("read:repo_hook"),
+ READ_REPO_HOOK("read:repo_hook",
+ "Read/ping public/private repositories' hooks"),
/**
* Grants read, write, and ping access to hooks in public or private
* repositories.
*/
- WRITE_REPO_HOOK("write:repo_hook"),
+ WRITE_REPO_HOOK("write:repo_hook",
+ "Read/write/ping public/private repositories' hooks"),
/**
* Grants read, write, ping, and delete access to hooks in public or private
* repositories.
*/
- ADMIN_REPO_HOOK("admin:repo_hook"),
+ ADMIN_REPO_HOOK("admin:repo_hook",
+ "Read/write/ping/delete access to public/private repositories' hooks"),
/**
* Grants read, write, ping, and delete access to organization hooks. Note:
@@ -140,46 +145,48 @@
* will only be able to perform these actions on organization hooks created
* by a user.
*/
- ADMIN_ORG_HOOK("admin:org_hook"),
+ ADMIN_ORG_HOOK("admin:org_hook",
+ "Read/write/ping/delete access to public/private organizations' hooks"),
/**
* Read-only access to organization, teams, and membership.
*/
- READ_ORG("read:org"),
+ READ_ORG("read:org", "Read-only organizations"),
/**
* Publicize and un-publicize organization membership.
*/
- WRITE_ORG("write:org"),
+ WRITE_ORG("write:org", "(Un)Publicize organizations membership"),
/**
* Fully manage organization, teams, and memberships.
*/
- ADMIN_ORG("admin:org"),
+ ADMIN_ORG("admin:org", "Manage organizations, teams and memberships"),
/**
* List and view details for public keys.
*/
- READ_PUBLIC_KEY("read:public_key"),
+ READ_PUBLIC_KEY("read:public_key", "Read-only user's public keys"),
/**
* Create, list, and view details for public keys.
*/
- WRITE_PUBLIC_KEY("write:public_key"),
+ WRITE_PUBLIC_KEY("write:public_key", "Read/write/list user's public keys"),
/**
* Fully manage public keys.
*/
- ADMIN_PUBLIC_KEY("admin:public_key");
+ ADMIN_PUBLIC_KEY("admin:public_key", "Fully manage user's public keys");
+ @Getter
private final String value;
- public String getValue() {
- return value;
- }
+ @Getter
+ private final String description;
- private Scope(final String value) {
+ private Scope(final String value, final String description) {
this.value = value;
+ this.description = description;
}
}
private static final String ME_SEPARATOR = ",";
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 59ceb46..2bac968 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
@@ -15,7 +15,6 @@
import com.google.inject.Inject;
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.OAuthProtocol;
@@ -26,6 +25,8 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
@@ -40,9 +41,13 @@
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;
@Inject
public GitHubOAuthFilter(ScopedProvider<GitHubLogin> loginProvider,
@@ -50,6 +55,7 @@
this.loginProvider = loginProvider;
this.authScopes = githubOAuthConfig.getDefaultScopes();
this.oauth = oauth;
+ this.config = githubOAuthConfig;
}
@Override
@@ -61,7 +67,7 @@
FilterChain chain) throws IOException, ServletException {
GitHubLogin hubLogin = loginProvider.get((HttpServletRequest) request);
LOG.debug("GitHub login: " + hubLogin);
- if (!hubLogin.isLoggedIn()) {
+ if (!hubLogin.isLoggedIn() && !isWhiteListed(request)) {
hubLogin.login((HttpServletRequest) request,
(HttpServletResponse) response, oauth, authScopes);
return;
@@ -70,6 +76,16 @@
}
}
+ private boolean isWhiteListed(ServletRequest request) {
+ String requestUri = ((HttpServletRequest) request).getRequestURI();
+ for (String suffix : whiteList) {
+ if (requestUri.endsWith(suffix)) {
+ return true;
+ }
+ }
+ return config.scopeSelectionUrl.endsWith(requestUri);
+ }
+
@Override
public void destroy() {
}
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 75e3c47..9b876d4 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
@@ -14,11 +14,13 @@
package com.googlesource.gerrit.plugins.github.velocity;
import com.google.common.base.MoreObjects;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
+import com.googlesource.gerrit.plugins.github.GitHubConfig;
import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin;
import com.googlesource.gerrit.plugins.github.oauth.ScopedProvider;
@@ -50,18 +52,22 @@
private final RuntimeInstance velocityRuntime;
private final Provider<PluginVelocityModel> modelProvider;
private final ScopedProvider<GitHubLogin> loginProvider;
- private final Provider<IdentifiedUser> userProvider;
+ private final Provider<CurrentUser> userProvider;
+ private final GitHubConfig config;
@Inject
public VelocityViewServlet(
@Named("PluginRuntimeInstance") final RuntimeInstance velocityRuntime,
Provider<PluginVelocityModel> modelProvider,
- ScopedProvider<GitHubLogin> loginProvider, Provider<IdentifiedUser> userProvider) {
+ ScopedProvider<GitHubLogin> loginProvider,
+ Provider<CurrentUser> userProvider,
+ GitHubConfig config) {
this.velocityRuntime = velocityRuntime;
this.modelProvider = modelProvider;
this.loginProvider = loginProvider;
this.userProvider = userProvider;
+ this.config = config;
}
@@ -98,8 +104,13 @@
PluginVelocityModel model = modelProvider.get();
GitHubLogin gitHubLogin = loginProvider.get(request);
model.put("myself", gitHubLogin.getMyself());
- model.put("user", userProvider.get());
- model.put("hub", gitHubLogin.getHub());
+ model.put("config", config);
+
+ CurrentUser user = userProvider.get();
+ if (user.isIdentifiedUser()) {
+ model.put("user", user);
+ model.put("hub", gitHubLogin.getHub());
+ }
for (Entry<String, String[]> reqPar : request.getParameterMap().entrySet()) {
model.put(reqPar.getKey(), reqPar.getValue());
diff --git a/github-plugin/src/main/resources/static/scope.html b/github-plugin/src/main/resources/static/scope.html
new file mode 100644
index 0000000..2b131fc
--- /dev/null
+++ b/github-plugin/src/main/resources/static/scope.html
@@ -0,0 +1,74 @@
+<!DOCTYPE html>
+<html dir="ltr" lang="en-US">
+ <head>
+ <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+ <title>GitHub plugin for Gerrit Code Review - Login Scope</title>
+ #include ("static/styles.html")
+ #include ("static/scripts.html")
+ </head>
+ <body>
+ <!-- div.header start -->
+ <div class="header">
+ <div>
+ <div class="center">
+ #include ("static/header.html")
+ <div class="page-title">
+ <div>
+ <h2>Login Scope Selection</h2>
+ <div class="right">
+ <button type="button" onclick="window.location='/'" id="cancel">
+ <span class="button"><span>Cancel</span></span>
+ </button>
+ <button type="submit" onclick="document.forms[0].submit()">
+ <span class="button green"><span>Login ></span></span>
+ </button>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ <!-- div.header end -->
+
+ <!-- div.container start -->
+ <div class="container">
+ <div class="center">
+ <form class="signupform" method="get" action="/login">
+ <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>
+ #end
+ </li>
+ </ul>
+ <ul class="rememberScope">
+ <li>
+ <input type="checkbox" name="rememberScope" checked="checked"><label>Remember my choice</label>
+ </li>
+ </ul>
+ </form>
+ </div>
+ </div>
+ <!--div.container end -->
+
+ #include ("static/footer.html")
+
+ <script type='text/javascript' src='js/jquery/jquery.form.js?ver=2.02m'></script>
+ </body>
+</html>
