From 2c4b7b55b8d8d6a566968b0ce4bf9edafc26f8af Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Fri, 28 Jul 2017 06:25:15 +0200 Subject: [PATCH 1/3] WORKSPACE: Reuse external plugins dependencies Change-Id: I9eb4a08393a9ca06a1ea351724c6fbd118d69fca --- WORKSPACE | 15 ++------------- external_plugin_deps.bzl | 11 +++++++++-- tools/bzl/maven_jar.bzl | 1 + 3 files changed, 12 insertions(+), 15 deletions(-) create mode 100644 tools/bzl/maven_jar.bzl diff --git a/WORKSPACE b/WORKSPACE index 3245bc1..5dfb6bd 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -15,16 +15,5 @@ load( gerrit_api() -load("@com_googlesource_gerrit_bazlets//tools:maven_jar.bzl", "maven_jar") - -maven_jar( - name = "scribe", - artifact = "org.scribe:scribe:1.3.7", - sha1 = "583921bed46635d9f529ef5f14f7c9e83367bc6e", -) - -maven_jar( - name = "commons_codec", - artifact = "commons-codec:commons-codec:1.4", - sha1 = "4216af16d38465bbab0f3dff8efa14204f7a399a", -) +load(":external_plugin_deps.bzl", "external_plugin_deps") +external_plugin_deps(omit_commons_codec = False) diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl index d04c600..026221a 100644 --- a/external_plugin_deps.bzl +++ b/external_plugin_deps.bzl @@ -1,8 +1,15 @@ load("//tools/bzl:maven_jar.bzl", "maven_jar") -def external_plugin_deps(): +def external_plugin_deps(omit_commons_codec = True): maven_jar( name = "scribe", artifact = "org.scribe:scribe:1.3.7", sha1 = "583921bed46635d9f529ef5f14f7c9e83367bc6e", - ) + ) + if not omit_commons_codec: + maven_jar( + name = "commons_codec", + artifact = "commons-codec:commons-codec:1.4", + sha1 = "4216af16d38465bbab0f3dff8efa14204f7a399a", + ) + diff --git a/tools/bzl/maven_jar.bzl b/tools/bzl/maven_jar.bzl new file mode 100644 index 0000000..2eabedb --- /dev/null +++ b/tools/bzl/maven_jar.bzl @@ -0,0 +1 @@ +load("@com_googlesource_gerrit_bazlets//tools:maven_jar.bzl", "maven_jar") From 2926f603cff282ec23dab48cf680cc3e782284ea Mon Sep 17 00:00:00 2001 From: Matthew Webber Date: Wed, 2 Aug 2017 14:35:14 +0100 Subject: [PATCH 2/3] Fix CAS OAuth support (for CAS v5 and higher) CAS OAuth support was (probably) broken for all CAS versions. This change fixes support for CAS v5+ (tested against CAS 5.1). It will not work with CAS 4 and earlier. Documentation on CAS as an OAuth server: https://apereo.github.io/cas/5.1.x/installation/OAuth-OpenId-Authentication.html Bug: https://github.com/davido/gerrit-oauth-provider/issues/92 Change-Id: I74874577d2e055d345fad94984b9b3792305894c --- .../gerrit/plugins/oauth/CasApi.java | 68 ++++++++++++++++++- .../gerrit/plugins/oauth/CasOAuthService.java | 37 ++++++---- src/main/resources/Documentation/config.md | 2 + 3 files changed, 91 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/CasApi.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/CasApi.java index 5df1e64..3ae65dc 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/CasApi.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/CasApi.java @@ -16,10 +16,18 @@ import org.scribe.builder.api.DefaultApi20; import org.scribe.model.OAuthConfig; +import org.scribe.model.OAuthConstants; +import org.scribe.model.OAuthRequest; +import org.scribe.model.Response; +import org.scribe.model.Token; +import org.scribe.model.Verb; +import org.scribe.model.Verifier; +import org.scribe.oauth.OAuthService; import org.scribe.utils.OAuthEncoder; public class CasApi extends DefaultApi20 { - private static final String AUTHORIZE_URL = "%s/oauth2.0/authorize?client_id=%s&redirect_uri=%s"; + private static final String AUTHORIZE_URL = + "%s/oauth2.0/authorize?response_type=code&client_id=%s&redirect_uri=%s"; private final String rootUrl; @@ -37,4 +45,62 @@ public String getAuthorizationUrl(OAuthConfig config) { return String.format( AUTHORIZE_URL, rootUrl, config.getApiKey(), OAuthEncoder.encode(config.getCallback())); } + + @Override + public Verb getAccessTokenVerb() { + return Verb.POST; + } + + @Override + public OAuthService createService(OAuthConfig config) { + return new CasOAuthService(this, config); + } + + private static final class CasOAuthService implements OAuthService { + private static final String VERSION = "2.0"; + private static final String GRANT_TYPE = "grant_type"; + private static final String GRANT_TYPE_VALUE = "authorization_code"; + + private final DefaultApi20 api; + private final OAuthConfig config; + + private CasOAuthService(DefaultApi20 api, OAuthConfig config) { + this.config = config; + this.api = api; + } + + @Override + public Token getAccessToken(Token token, Verifier verifier) { + OAuthRequest request = + new OAuthRequest(api.getAccessTokenVerb(), api.getAccessTokenEndpoint()); + request.addBodyParameter(GRANT_TYPE, GRANT_TYPE_VALUE); + request.addBodyParameter(OAuthConstants.CLIENT_ID, config.getApiKey()); + request.addBodyParameter(OAuthConstants.CLIENT_SECRET, config.getApiSecret()); + request.addBodyParameter(OAuthConstants.CODE, verifier.getValue()); + request.addBodyParameter(OAuthConstants.REDIRECT_URI, config.getCallback()); + Response response = request.send(); + return api.getAccessTokenExtractor().extract(response.getBody()); + } + + @Override + public Token getRequestToken() { + throw new UnsupportedOperationException( + "Unsupported operation, please use 'getAuthorizationUrl' and redirect your users there"); + } + + @Override + public String getVersion() { + return VERSION; + } + + @Override + public void signRequest(Token token, OAuthRequest request) { + request.addQuerystringParameter(OAuthConstants.ACCESS_TOKEN, token.getToken()); + } + + @Override + public String getAuthorizationUrl(Token token) { + return api.getAuthorizationUrl(config); + } + } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java index d855ab2..c50689f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java @@ -99,28 +99,35 @@ public OAuthUserInfo getUserInfo(OAuthToken token) throws IOException { JsonElement id = jsonObject.get("id"); if (id == null || id.isJsonNull()) { - throw new IOException(String.format("Response doesn't contain %s field", "id")); + throw new IOException(String.format("CAS response missing id: %s", response.getBody())); } JsonElement attrListJson = jsonObject.get("attributes"); - if (attrListJson == null || !attrListJson.isJsonArray()) { - throw new IOException(String.format("Invalid JSON '%s': not a JSON Array", attrListJson)); + if (attrListJson == null) { + throw new IOException( + String.format("CAS response missing attributes: %s", response.getBody())); } String email = null, name = null, login = null; - JsonArray attrJson = attrListJson.getAsJsonArray(); - for (JsonElement elem : attrJson) { - if (elem == null || !elem.isJsonObject()) { - throw new IOException(String.format("Invalid JSON '%s': not a JSON Object", elem)); + + if (attrListJson != null && attrListJson.isJsonArray()) { + // It is possible for CAS to be configured to not return any attributes (email, name, login), in which case, + // CAS returns an empty JSON object "attributes":{}, rather than "null" or an empty JSON array "attributes": [] + + JsonArray attrJson = attrListJson.getAsJsonArray(); + for (JsonElement elem : attrJson) { + if (elem == null || !elem.isJsonObject()) { + throw new IOException(String.format("Invalid JSON '%s': not a JSON Object", elem)); + } + JsonObject obj = elem.getAsJsonObject(); + + String property = getStringElement(obj, "email"); + if (property != null) email = property; + property = getStringElement(obj, "name"); + if (property != null) name = property; + property = getStringElement(obj, "login"); + if (property != null) login = property; } - JsonObject obj = elem.getAsJsonObject(); - - String property = getStringElement(obj, "email"); - if (property != null) email = property; - property = getStringElement(obj, "name"); - if (property != null) name = property; - property = getStringElement(obj, "login"); - if (property != null) login = property; } return new OAuthUserInfo( diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 785133f..8bb6969 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md @@ -85,6 +85,8 @@ plugin.gerrit-oauth-provider-cas-oauth.root-url = "https://example.com/cas" is required, since CAS is a self-hosted application. +Note that the CAS OAuth plugin only supports CAS V5 and higher. + The plugin expects CAS to make several attributes available to it: | Name | Description | Required | From e49765639b1177637e84dff8abe146692d4f1bc8 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Thu, 31 Aug 2017 19:04:48 +0200 Subject: [PATCH 3/3] Update bazlets to most recent version Change-Id: I53b7f61cad8c7d9a69aca9a1ab223c095620ff9a --- WORKSPACE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WORKSPACE b/WORKSPACE index 5dfb6bd..68d9be1 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -3,7 +3,7 @@ workspace(name = "com_github_davido_gerrit_oauth_provider") load("//:bazlets.bzl", "load_bazlets") load_bazlets( - commit = "d1dd04380a2e41a32fca265c999936c35fcfae14", + commit = "b7514d03a7798905ff1513295b46620e57b8f386", # local_path = "/home//projects/bazlets", )