Fixed GitHub login error when user has no name

Also fixed error reporting to make diagnosing the problem easier.

app/controllers/admin/AdminController.scala

@@ -1,5 +1,6 @@
 package controllers.admin
 
+import java.time.ZoneId
 import java.time.format.DateTimeFormatter
 
 import play.api.i18n.Lang
@@ -156,6 +157,6 @@ class AdminController(components: ControllerComponents, config: OAuthConfig, oau
     }
   }
 
-  private val format = DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm")
+  private val format = DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm").withZone(ZoneId.of("UTC"))
 
 }

app/controllers/oauth/OAuth2Controller.scala

@@ -2,10 +2,13 @@ package controllers.oauth
 
 import models.OAuthUser
 import play.api._
+import play.api.libs.json.{JsError, JsResultException}
 import play.api.libs.ws.WSClient
 import services._
+
 import scala.util.control.NonFatal
 import play.api.mvc._
+
 import scala.concurrent.{ExecutionContext, Future}
 
 /**
@@ -46,6 +49,8 @@ abstract class OAuth2Controller(components: ControllerComponents, ws: WSClient,
               // and then close itself.
               Ok(views.html.popup()).withSession("user" -> signatory.id.stringify)
             }).recover {
+              case JsResultException(errors) =>
+                InternalServerError(views.html.jserror(name, messagesApi.preferred(req), JsError(errors)))
               case NonFatal(t) =>
                 Logger.warn("Error logging in user to " + name, t)
                 Forbidden

app/controllers/oauth/TwitterController.scala

@@ -6,7 +6,7 @@ import play.api.libs.oauth._
 import services.{OAuthConfig, UserInfoProvider, UserService}
 
 import scala.concurrent.ExecutionContext
-import play.api.libs.json.Json
+import play.api.libs.json.{JsError, JsResultException, Json}
 import play.api._
 
 /**
@@ -32,9 +32,11 @@ class TwitterController(components: ControllerComponents, config: OAuthConfig, w
               Ok(views.html.popup()).withSession("user" -> signatory.id.stringify)
             }
           }.recover {
+            case JsResultException(errors) =>
+              InternalServerError(views.html.jserror("Twitter", messagesApi.preferred(request), JsError(errors)))
             case e =>
               Logger.warn("Error logging in user to twitter", e)
-              Forbidden(Json.toJson(Json.obj("error" -> "Twitter rejected credentials")))
+              Forbidden("Twitter rejected credentials")
           }
 
         case Left(e) =>

app/models/OAuthUser.scala

@@ -2,9 +2,6 @@ package models
 
 import java.time.Instant
 
-import org.joda.time.DateTime
-
-
 /**
  * A user that has logged in using OAuth.
  *

app/services/UserInfoProvider.scala

@@ -1,11 +1,14 @@
 package services
 
-import models.{GitHub, Google, LinkedIn, OAuthUser}
+import models._
 import play.api.libs.oauth.{OAuthCalculator, RequestToken}
 import play.api.libs.ws.{WSClient, WSRequest}
 
 import scala.concurrent.{ExecutionContext, Future}
 import scala.util.control.NonFatal
+import play.api.libs.json._
+import play.api.libs.functional.syntax._
+
 
 class UserInfoProvider(ws: WSClient, oauthConfig: OAuthConfig)(implicit ec: ExecutionContext) {
 
@@ -23,14 +26,17 @@ class UserInfoProvider(ws: WSClient, oauthConfig: OAuthConfig)(implicit ec: Exec
         ))
   }
 
+  private val githubOauthUserReads = (
+    (__ \ "id").read[Long] and
+    (__ \ "login").read[String] and
+    (__ \ "name").readNullable[String] and
+    (__ \ "avatar_url").readNullable[String]
+  ).apply((id, login, name, avatar) => OAuthUser(GitHub(id, login), name.getOrElse(login), avatar))
+
   private def makeGitHubUserRequest(request: WSRequest): Future[Option[OAuthUser]] = {
     request.get().map { response =>
       if (response.status == 200) {
-        val id = (response.json \ "id").as[Long]
-        val login = (response.json \ "login").as[String]
-        val name = (response.json \ "name").as[String]
-        val avatar = (response.json \ "avatar_url").asOpt[String]
-        Some(OAuthUser(GitHub(id, login), name, avatar))
+        Some(response.json.as(githubOauthUserReads))
       } else if (response.status == 404) {
         None
       } else {
@@ -50,13 +56,16 @@ class UserInfoProvider(ws: WSClient, oauthConfig: OAuthConfig)(implicit ec: Exec
       .addQueryStringParameters("key" -> oauthConfig.googleApiKey))
   }
 
+  private val googleOauthUserReads = (
+    (__ \ "id").read[String] and
+    (__ \ "displayName").read[String] and
+    (__ \ "image" \ "url").readNullable[String]
+  ).apply((id, name, avatar) => OAuthUser(Google(id), name, avatar))
+
   private def makeGoogleUserRequest(request: WSRequest): Future[Option[OAuthUser]] = {
     request.get().map { response =>
       if (response.status == 200) {
-        val id = (response.json \ "id").as[String]
-        val name = (response.json \ "displayName").as[String]
-        val avatar = (response.json \ "image" \ "url").asOpt[String]
-        Some(OAuthUser(Google(id), name, avatar))
+        Some(response.json.as(googleOauthUserReads))
       } else if (response.status == 404) {
         None
       } else {
@@ -77,16 +86,19 @@ class UserInfoProvider(ws: WSClient, oauthConfig: OAuthConfig)(implicit ec: Exec
       .addHttpHeaders("Authorization" -> s"Bearer ${oauthConfig.twitterBearerToken}"))
   }
 
+  private val twitterOauthUserReads = (
+    (__ \ "id").read[Long] and
+    (__ \ "screen_name").read[String] and
+    (__ \ "name").read[String] and
+    (__ \ "profile_image_url").readNullable[String]
+  ).apply((id, screenName, name, avatar) => OAuthUser(Twitter(id, screenName), name, avatar))
+
   private def makeTwitterUserRequest(request: WSRequest): Future[Option[OAuthUser]] = {
     request.get().map { response =>
 
       // Check if response is ok
       if (response.status == 200) {
-        val id = (response.json \ "id").as[Long]
-        val screenName = (response.json \ "screen_name").as[String]
-        val name = (response.json \ "name").as[String]
-        val avatar = (response.json \ "profile_image_url").asOpt[String]
-        Some(OAuthUser(models.Twitter(id, screenName), name, avatar))
+        Some(response.json.as(twitterOauthUserReads))
       } else if (response.status == 404) {
         None
       } else {
@@ -96,16 +108,19 @@ class UserInfoProvider(ws: WSClient, oauthConfig: OAuthConfig)(implicit ec: Exec
     }
   }
 
+  private val linkedinOauthUserReads = (
+    (__ \ "id").read[String] and
+    (__ \ "formattedName").read[String] and
+    (__ \ "pictureUrl").readNullable[String]
+  ).apply((id, name, avatar) => OAuthUser(LinkedIn(id), name, avatar))
+
   def lookupLinkedInCurrentUser(accessToken: String): Future[OAuthUser] = {
     ws.url("https://api.linkedin.com/v1/people/~:(id,picture-url,formatted-name)")
       .addQueryStringParameters("oauth2_access_token" -> accessToken)
       .addHttpHeaders("X-Li-Format" -> "json", "Accept" -> "application/json").get().map { response =>
       if (response.status == 200) {
         try {
-          val id = (response.json \ "id").as[String]
-          val name = (response.json \ "formattedName").as[String]
-          val avatar = (response.json \ "pictureUrl").asOpt[String]
-          OAuthUser(LinkedIn(id), name, avatar)
+          response.json.as(linkedinOauthUserReads)
         } catch {
           case NonFatal(e) => throw new IllegalArgumentException("Error parsing profile information: " + response.body)
         }

app/views/jserror.scala.html

@@ -0,0 +1,25 @@
+@import play.api.libs.json.JsError
+
+@(provider: String, messages: Messages, jsError: JsError)
+<html>
+    <head>
+        <title>Error</title>
+    </head>
+    <body>
+        <h1>Error</h1>
+        <p>An error occurred while parsing JSON from @{provider}.</p>
+        <dl>
+            @for((path, errors) <- jsError.errors) {
+                <dt>At path @path:</dt>
+                <dd>
+                    <ul>
+                    @for(error <- errors) {
+                        <li>@messages(error.message, error.args: _*)</li>
+                    }
+                    </ul>
+                </dd>
+            }
+        </dl>
+        <p>Please report this issue <a href="https://github.com/reactivemanifesto/website-manifesto">here</a>.</p>
+    </body>
+</html>

build.sbt

@@ -1,5 +1,5 @@
 lazy val `reactivemanifesto` = (project in file("."))
-  .enablePlugins(PlayScala)
+  .enablePlugins(PlayScala, LauncherJarPlugin)
 
 name := "reactivemanifesto"
 version := "1.0-SNAPSHOT"