Check for null values and log user endpoint response

To log the response, log4j logger should be activated for the plugin: log4j.logger.com.googlesource.gerrit.plugins.oauth=debug
davido · Apr 14, 2015 · af4ff69 · af4ff69
1 parent 45eeae4
commit af4ff69
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 7 deletions.
diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/GitHubOAuthService.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/GitHubOAuthService.java
@@ -37,13 +37,17 @@
 import org.scribe.model.Verb;
 import org.scribe.model.Verifier;
 import org.scribe.oauth.OAuthService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 
 import javax.servlet.http.HttpServletResponse;
 
 @Singleton
 class GitHubOAuthService implements OAuthServiceProvider {
+  private static final Logger log =
+      LoggerFactory.getLogger(GitHubOAuthService.class);
   static final String CONFIG_SUFFIX = "-github-oauth";
   private static final String PROTECTED_RESOURCE_URL =
       "https://api.github.com/user";
@@ -82,16 +86,23 @@ public OAuthUserInfo getUserInfo(OAuthToken token) throws IOException {
     JsonElement userJson =
         OutputFormat.JSON.newGson().fromJson(response.getBody(),
             JsonElement.class);
+    if (log.isDebugEnabled()) {
+      log.debug("User info response: {}", response.getBody());
+    }
     if (userJson.isJsonObject()) {
       JsonObject jsonObject = userJson.getAsJsonObject();
+      JsonElement id = jsonObject.get("id");
+      if (id == null || id.isJsonNull()) {
+        throw new IOException(String.format(
+            "Response doesn't contain id field"));
+      }
       JsonElement email = jsonObject.get("email");
       JsonElement name = jsonObject.get("name");
-      JsonElement id = jsonObject.get("id");
       JsonElement login = jsonObject.get("login");
       return new OAuthUserInfo(id.getAsString(),
-          login.isJsonNull() ? null : login.getAsString(),
-          email.isJsonNull() ? null : email.getAsString(),
-          name.isJsonNull() ? null : name.getAsString(),
+          login == null || login.isJsonNull() ? null : login.getAsString(),
+          email == null || email.isJsonNull() ? null : email.getAsString(),
+          name == null || name.isJsonNull() ? null : name.getAsString(),
           null);
     } else {
         throw new IOException(String.format(

diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/GoogleOAuthService.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/GoogleOAuthService.java
@@ -105,10 +105,13 @@ public OAuthUserInfo getUserInfo(OAuthToken token) throws IOException {
     JsonElement userJson =
         OutputFormat.JSON.newGson().fromJson(response.getBody(),
             JsonElement.class);
+    if (log.isDebugEnabled()) {
+      log.debug("User info response: {}", response.getBody());
+    }
     if (userJson.isJsonObject()) {
       JsonObject jsonObject = userJson.getAsJsonObject();
       JsonElement id = jsonObject.get("id");
-      if (id.isJsonNull()) {
+      if (id == null || id.isJsonNull()) {
         throw new IOException(String.format(
             "Response doesn't contain id field"));
       }
@@ -121,8 +124,8 @@ public OAuthUserInfo getUserInfo(OAuthToken token) throws IOException {
       }
       return new OAuthUserInfo(id.getAsString() /*externalId*/,
           null /*username*/,
-          email.isJsonNull() ? null : email.getAsString() /*email*/,
-          name.isJsonNull() ? null : name.getAsString() /*displayName*/,
+          email == null || email.isJsonNull() ? null : email.getAsString() /*email*/,
+          name == null || name.isJsonNull() ? null : name.getAsString() /*displayName*/,
 	      claimedIdentifier /*claimedIdentity*/);
     } else {
         throw new IOException(String.format(