Correct management of OAuth token serialisation returned by GitHub
When the OAuth/Phase-2 fails (last POST to GitHub API with the
one-time-code returned to the /oauth callback) we need to
read the error codes and descriptions returned and avoid
to perform the subsequent login operations.
Additionally the OAuth token bean specs should respect the common
camelCase notation adopted in Gerrit.
Change-Id: Ib7e42aba761a6f4ff4f7870509113b8d53efa764
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 0d66026..79cd552 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
@@ -126,7 +126,11 @@
if (OAuthProtocol.isOAuthFinal(request)) {
log.debug("Login-FINAL " + this);
- login(oauth.loginPhase2(request, response));
+ AccessToken loginAccessToken = oauth.loginPhase2(request, response);
+ if(loginAccessToken != null && !loginAccessToken.isError()) {
+ login(loginAccessToken);
+ }
+
if (isLoggedIn()) {
log.debug("Login-SUCCESS " + this);
response.sendRedirect(OAuthProtocol.getTargetUrl(request));
@@ -153,9 +157,9 @@
}
public GitHub login(AccessToken authToken) throws IOException {
- log.debug("Logging in using access token {}", authToken.access_token);
+ log.debug("Logging in using access token {}", authToken.accessToken);
this.token = authToken;
- this.hub = GitHub.connectUsingOAuth(authToken.access_token);
+ this.hub = GitHub.connectUsingOAuth(authToken.accessToken);
this.myself = hub.getMyself();
return this.hub;
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GsonProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GsonProvider.java
new file mode 100644
index 0000000..fd004c0
--- /dev/null
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GsonProvider.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2014 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.github.oauth;
+
+import com.google.gson.FieldNamingPolicy;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+@Singleton
+public class GsonProvider implements Provider<Gson> {
+
+ @Override
+ public Gson get() {
+ return new GsonBuilder().setFieldNamingPolicy(
+ FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES).create();
+ }
+}
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 fae8219..61f4890 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
@@ -1,10 +1,12 @@
package com.googlesource.gerrit.plugins.github.oauth;
import java.io.IOException;
+import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.HttpURLConnection;
import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
@@ -25,13 +27,14 @@
import com.google.common.base.Charsets;
import com.google.common.base.Strings;
+import com.google.common.io.CharStreams;
import com.google.gson.Gson;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@Singleton
public class OAuthProtocol {
-
+
public static enum Scope {
DEFAULT(""),
USER("user"),
@@ -63,8 +66,11 @@
private final Gson gson;
public static class AccessToken {
- public String access_token;
- public String token_type;
+ public String accessToken;
+ public String tokenType;
+ public String error;
+ public String errorDescription;
+ public String errorUri;
public AccessToken() {
}
@@ -75,14 +81,19 @@
public AccessToken(String token, String type, Scope... scopes) {
this();
- this.access_token = token;
- this.token_type = type;
+ this.accessToken = token;
+ this.tokenType = type;
}
@Override
public String toString() {
- return "AccessToken [access_token=" + access_token + ", token_type="
- + token_type + "]";
+ if (isError()) {
+ return "Error AccessToken [error=" + error + ", error_description="
+ + errorDescription + ", error_uri=" + errorUri + "]";
+ } else {
+ return "AccessToken [access_token=" + accessToken + ", token_type="
+ + tokenType + "]";
+ }
}
@Override
@@ -91,9 +102,9 @@
int result = 1;
result =
prime * result
- + ((access_token == null) ? 0 : access_token.hashCode());
+ + ((accessToken == null) ? 0 : accessToken.hashCode());
result =
- prime * result + ((token_type == null) ? 0 : token_type.hashCode());
+ prime * result + ((tokenType == null) ? 0 : tokenType.hashCode());
return result;
}
@@ -103,21 +114,29 @@
if (obj == null) return false;
if (getClass() != obj.getClass()) return false;
AccessToken other = (AccessToken) obj;
- if (access_token == null) {
- if (other.access_token != null) return false;
- } else if (!access_token.equals(other.access_token)) return false;
- if (token_type == null) {
- if (other.token_type != null) return false;
- } else if (!token_type.equals(other.token_type)) return false;
+ if (accessToken == null) {
+ if (other.accessToken != null) return false;
+ } else if (!accessToken.equals(other.accessToken)) return false;
+ if (tokenType == null) {
+ if (other.tokenType != null) return false;
+ } else if (!tokenType.equals(other.tokenType)) return false;
return true;
}
+
+ public boolean isError() {
+ return !Strings.isNullOrEmpty(error);
+ }
}
@Inject
- public OAuthProtocol(GitHubOAuthConfig config, GitHubHttpProvider httpClientProvider, Gson gson) {
+ public OAuthProtocol(GitHubOAuthConfig config, GitHubHttpProvider httpClientProvider,
+ /* We need to explicitly tell Guice which Provider<> we need as this class may be
+ instantiated outside the standard Guice Module set-up (e.g. initial Servlet login
+ filter) */
+ GsonProvider gsonProvider) {
this.config = config;
this.http = httpClientProvider.get();
- this.gson = gson;
+ this.gson = gsonProvider.get();
}
public void loginPhase1(HttpServletRequest request,
@@ -195,21 +214,23 @@
LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
+ " request for access token failed with status "
+ postResponse.getStatusLine());
- response.sendError(HttpURLConnection.HTTP_UNAUTHORIZED,
- "Request for access token not authorised");
EntityUtils.consume(postResponse.getEntity());
return null;
}
- AccessToken token =
- gson.fromJson(new InputStreamReader(postResponse.getEntity()
- .getContent(), "UTF-8"), AccessToken.class);
+ InputStream content = postResponse.getEntity().getContent();
+ String tokenJsonString =
+ CharStreams.toString(new InputStreamReader(content,
+ StandardCharsets.UTF_8));
+ AccessToken token = gson.fromJson(tokenJsonString, AccessToken.class);
+ if (token.isError()) {
+ LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
+ + " returned an error token: " + token);
+ }
return token;
} catch (IOException e) {
LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
+ " request for access token failed", e);
- response.sendError(HttpURLConnection.HTTP_UNAUTHORIZED,
- "Request for access token not authorised");
return null;
}
}
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 f546753..a8823a9 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
@@ -92,7 +92,7 @@
new AuthenticatedHttpRequest(httpRequest, config.httpHeader,
ghLogin.getMyself().getLogin(),
config.oauthHttpHeader,
- GITHUB_EXT_ID + ghLogin.getToken().access_token);
+ GITHUB_EXT_ID + ghLogin.getToken().accessToken);
}
if (OAuthProtocol.isOAuthFinalForOthers(httpRequest)) {
@@ -125,7 +125,7 @@
String user = myself.getLogin();
updateSecureConfigWithRetry(ghLogin.hub.getMyOrganizations().keySet(),
- user, ghLogin.token.access_token);
+ user, ghLogin.token.accessToken);
}
}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java
index 7722d63..a677958 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitHubRepository.java
@@ -42,7 +42,7 @@
public String getCloneUrl() {
return cloneUrl.replace("://", "://" + ghLogin.getMyself().getLogin() + ":"
- + ghLogin.getToken().access_token + "@");
+ + ghLogin.getToken().accessToken + "@");
}
public String getOrganisation() {
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java
index d4b52e8..3881c86 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/ReplicateProjectStep.java
@@ -48,7 +48,7 @@
this.replicationConfig = replicationConfig;
GitHubLogin ghLogin = ghLoginProvider.get();
this.authUsername = ghLogin.getMyself().getLogin();
- this.authToken = ghLogin.getToken().access_token;
+ this.authToken = ghLogin.getToken().accessToken;
this.gitHubUrl = gitHubUrl;
}
