diff --git a/.travis.yml b/.travis.yml index f9b6f1c..261b95e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,7 +29,7 @@ install: true before_install: - OS=linux - ARCH=x86_64 - - V=0.9.0 + - V=0.11.0 - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then OS=darwin; fi - GH_BASE="https://github.com/bazelbuild/bazel/releases/download/$V" - GH_ARTIFACT="bazel-$V-installer-$OS-$ARCH.sh" @@ -42,14 +42,8 @@ before_install: script: - | - bazel \ - --output_base=$HOME/.cache/bazel \ - --batch \ - --host_jvm_args=-Xmx500m \ - --host_jvm_args=-Xms500m \ - build \ - --verbose_failures \ - :all + bazel build --verbose_failures :all + bazel test --test_output=errors :gerrit-oauth-provider_tests notifications: email: false diff --git a/BUILD b/BUILD index 34291e7..628a1df 100644 --- a/BUILD +++ b/BUILD @@ -1,4 +1,9 @@ -load("//tools/bzl:plugin.bzl", "gerrit_plugin") +load("//tools/bzl:junit.bzl", "junit_tests") +load( + "//tools/bzl:plugin.bzl", + "gerrit_plugin", + "PLUGIN_DEPS", +) gerrit_plugin( name = "gerrit-oauth-provider", @@ -15,4 +20,14 @@ gerrit_plugin( "@commons_codec//jar:neverlink", "@scribe//jar", ], -) \ No newline at end of file +) + +junit_tests( + name = "gerrit-oauth-provider_tests", + srcs = glob(["src/test/java/**/*.java"]), + tags = ["oauth"], + deps = PLUGIN_DEPS + [ + ":gerrit-oauth-provider__plugin", + "@scribe//jar", + ], +) 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 c50689f..282a316 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java @@ -110,7 +110,7 @@ public OAuthUserInfo getUserInfo(OAuthToken token) throws IOException { String email = null, name = null, login = null; - if (attrListJson != null && attrListJson.isJsonArray()) { + if (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": [] diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/Facebook2Api.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/Facebook2Api.java new file mode 100644 index 0000000..a547bfb --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/Facebook2Api.java @@ -0,0 +1,25 @@ +// Copyright (C) 2018 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.oauth; + +import org.scribe.builder.api.FacebookApi; +import org.scribe.extractors.AccessTokenExtractor; + +public class Facebook2Api extends FacebookApi { + @Override + public AccessTokenExtractor getAccessTokenExtractor() { + return OAuth2AccessTokenJsonExtractor.instance(); + } +} diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/FacebookOAuthService.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/FacebookOAuthService.java index 44d4b36..d3ebe8e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/FacebookOAuthService.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/FacebookOAuthService.java @@ -32,7 +32,6 @@ import java.io.IOException; import javax.servlet.http.HttpServletResponse; import org.scribe.builder.ServiceBuilder; -import org.scribe.builder.api.FacebookApi; import org.scribe.model.OAuthRequest; import org.scribe.model.Response; import org.scribe.model.Token; @@ -65,7 +64,7 @@ class FacebookOAuthService implements OAuthServiceProvider { service = new ServiceBuilder() - .provider(FacebookApi.class) + .provider(Facebook2Api.class) .apiKey(cfg.getString(InitOAuth.CLIENT_ID)) .apiSecret(cfg.getString(InitOAuth.CLIENT_SECRET)) .callback(canonicalWebUrl + "oauth") diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/GitLabApi.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/GitLabApi.java index 2c27f35..db0851f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/GitLabApi.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/GitLabApi.java @@ -14,16 +14,11 @@ package com.googlesource.gerrit.plugins.oauth; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.scribe.builder.api.DefaultApi20; -import org.scribe.exceptions.OAuthException; import org.scribe.extractors.AccessTokenExtractor; import org.scribe.model.OAuthConfig; -import org.scribe.model.Token; import org.scribe.model.Verb; import org.scribe.oauth.OAuthService; -import org.scribe.utils.Preconditions; public class GitLabApi extends DefaultApi20 { private static final String AUTHORIZE_URL = @@ -57,21 +52,6 @@ public OAuthService createService(OAuthConfig config) { @Override public AccessTokenExtractor getAccessTokenExtractor() { - return new GitLabJsonTokenExtractor(); - } - - private static final class GitLabJsonTokenExtractor implements AccessTokenExtractor { - private Pattern accessTokenPattern = Pattern.compile("\"access_token\"\\s*:\\s*\"(\\S*?)\""); - - @Override - public Token extract(String response) { - Preconditions.checkEmptyString( - response, "Cannot extract a token from a null or empty String"); - Matcher matcher = accessTokenPattern.matcher(response); - if (matcher.find()) { - return new Token(matcher.group(1), "", response); - } - throw new OAuthException("Cannot extract an acces token. Response was: " + response); - } + return OAuth2AccessTokenJsonExtractor.instance(); } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/Google2Api.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/Google2Api.java index ed48549..88c640d 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/Google2Api.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/Google2Api.java @@ -16,13 +16,9 @@ import static org.scribe.utils.OAuthEncoder.encode; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.scribe.builder.api.DefaultApi20; -import org.scribe.exceptions.OAuthException; import org.scribe.extractors.AccessTokenExtractor; import org.scribe.model.OAuthConfig; -import org.scribe.model.Token; import org.scribe.model.Verb; import org.scribe.oauth.OAuthService; import org.scribe.utils.Preconditions; @@ -62,22 +58,6 @@ public OAuthService createService(OAuthConfig config) { @Override public AccessTokenExtractor getAccessTokenExtractor() { - return new GoogleJsonTokenExtractor(); - } - - private static final class GoogleJsonTokenExtractor implements AccessTokenExtractor { - private Pattern accessTokenPattern = Pattern.compile("\"access_token\"\\s*:\\s*\"(\\S*?)\""); - - @Override - public Token extract(String response) { - Preconditions.checkEmptyString( - response, "Cannot extract a token from a null or empty String"); - Matcher matcher = accessTokenPattern.matcher(response); - if (matcher.find()) { - return new Token(matcher.group(1), "", response); - } - - throw new OAuthException("Cannot extract an acces token. Response was: " + response); - } + return OAuth2AccessTokenJsonExtractor.instance(); } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/OAuth2AccessTokenJsonExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/OAuth2AccessTokenJsonExtractor.java new file mode 100644 index 0000000..6c2f1a0 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/OAuth2AccessTokenJsonExtractor.java @@ -0,0 +1,49 @@ +// Copyright (C) 2018 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.oauth; + +import static org.scribe.model.OAuthConstants.ACCESS_TOKEN; + +import com.google.common.annotations.VisibleForTesting; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.scribe.exceptions.OAuthException; +import org.scribe.extractors.AccessTokenExtractor; +import org.scribe.model.Token; +import org.scribe.utils.Preconditions; + +class OAuth2AccessTokenJsonExtractor implements AccessTokenExtractor { + private static final Pattern ACCESS_TOKEN_REGEX_PATTERN = + Pattern.compile("\"" + ACCESS_TOKEN + "\"\\s*:\\s*\"(\\S*?)\""); + + private OAuth2AccessTokenJsonExtractor() {} + + private static final AccessTokenExtractor INSTANCE = new OAuth2AccessTokenJsonExtractor(); + + static AccessTokenExtractor instance() { + return INSTANCE; + } + + @VisibleForTesting + @Override + public Token extract(String response) { + Preconditions.checkEmptyString(response, "Cannot extract a token from a null or empty String"); + Matcher matcher = ACCESS_TOKEN_REGEX_PATTERN.matcher(response); + if (matcher.find()) { + return new Token(matcher.group(1), "", response); + } + throw new OAuthException("Cannot extract an access token. Response was: " + response); + } +} diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/Office365Api.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/Office365Api.java index 12dd12f..8a28520 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/oauth/Office365Api.java +++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/Office365Api.java @@ -16,13 +16,9 @@ import static org.scribe.utils.OAuthEncoder.encode; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.scribe.builder.api.DefaultApi20; -import org.scribe.exceptions.OAuthException; import org.scribe.extractors.AccessTokenExtractor; import org.scribe.model.OAuthConfig; -import org.scribe.model.Token; import org.scribe.model.Verb; import org.scribe.oauth.OAuthService; import org.scribe.utils.Preconditions; @@ -61,22 +57,6 @@ public OAuthService createService(OAuthConfig config) { @Override public AccessTokenExtractor getAccessTokenExtractor() { - return new Office365JsonTokenExtractor(); - } - - private static final class Office365JsonTokenExtractor implements AccessTokenExtractor { - private Pattern accessTokenPattern = Pattern.compile("\"access_token\"\\s*:\\s*\"(\\S*?)\""); - - @Override - public Token extract(String response) { - Preconditions.checkEmptyString( - response, "Cannot extract a token from a null or empty String"); - Matcher matcher = accessTokenPattern.matcher(response); - if (matcher.find()) { - return new Token(matcher.group(1), "", response); - } - - throw new OAuthException("Cannot extract an acces token. Response was: " + response); - } + return OAuth2AccessTokenJsonExtractor.instance(); } } diff --git a/src/test/java/com/googlesource/gerrit/plugins/oauth/OAuth2AccessTokenJsonExtractorTest.java b/src/test/java/com/googlesource/gerrit/plugins/oauth/OAuth2AccessTokenJsonExtractorTest.java new file mode 100644 index 0000000..09df7ee --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/oauth/OAuth2AccessTokenJsonExtractorTest.java @@ -0,0 +1,70 @@ +// Copyright (C) 2018 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.oauth; + +import static org.junit.Assert.assertEquals; +import static org.scribe.model.OAuthConstants.ACCESS_TOKEN; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.scribe.exceptions.OAuthException; +import org.scribe.extractors.AccessTokenExtractor; +import org.scribe.model.Token; + +public class OAuth2AccessTokenJsonExtractorTest { + private static final AccessTokenExtractor extractor = OAuth2AccessTokenJsonExtractor.instance(); + private static final String TOKEN = "I0122HHJKLEM21F3WLPYHDKGKZULAUO4SGMV3ABKFTDT3T3X"; + private static final String RESPONSE = "{\"" + ACCESS_TOKEN + "\":\"" + TOKEN + "\"}'"; + private static final String RESPONSE_NON_JSON = ACCESS_TOKEN + "=" + TOKEN; + private static final String RESPONSE_WITH_BLANKS = + "{ \"" + ACCESS_TOKEN + "\" : \"" + TOKEN + "\"}'"; + private static final String MESSAGE = "Cannot extract a token from a null or empty String"; + + @Rule public ExpectedException exception = ExpectedException.none(); + + @Test + public void parseResponse() throws Exception { + Token token = extractor.extract(RESPONSE); + assertEquals(token.getToken(), TOKEN); + } + + @Test + public void parseResponseWithBlanks() throws Exception { + Token token = extractor.extract(RESPONSE_WITH_BLANKS); + assertEquals(token.getToken(), TOKEN); + } + + @Test + public void failParseNonJsonResponse() throws Exception { + exception.expect(OAuthException.class); + exception.expectMessage("Cannot extract an access token. Response was: " + RESPONSE_NON_JSON); + extractor.extract(RESPONSE_NON_JSON); + } + + @Test + public void shouldThrowExceptionIfForNullParameter() throws Exception { + exception.expect(IllegalArgumentException.class); + exception.expectMessage(MESSAGE); + extractor.extract(null); + } + + @Test + public void shouldThrowExceptionIfForEmptyString() throws Exception { + exception.expect(IllegalArgumentException.class); + exception.expectMessage(MESSAGE); + extractor.extract(""); + } +} diff --git a/tools/bzl/junit.bzl b/tools/bzl/junit.bzl new file mode 100644 index 0000000..3af7e58 --- /dev/null +++ b/tools/bzl/junit.bzl @@ -0,0 +1,4 @@ +load( + "@com_googlesource_gerrit_bazlets//tools:junit.bzl", + "junit_tests", +)