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>