From db6ad75f80f10eb3ebba7f09bc27530a2ba69ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Tue, 9 Jun 2026 09:01:44 +0200 Subject: [PATCH 1/2] fix(account): repair OAuth social login end-to-end (#11) - OAuth2Service.userInfo: drop null JSON values before toString to stop NPE on providers returning nullable profile fields (e.g. GitHub /user) - GitHubApiService: fall back to GET /user/emails for the primary verified email when /user returns a private (null) email, so login resolves - add post-auth redirect: callback now 302s to a configurable frontend URL (auth.oauth.callback.redirect-url / AUTH_OAUTH_CALLBACK_REDIRECT_URL) with status=success|error[&reason], session cookie set on the same response; mirrored in both tapir (OAuthServiceEndpoints) and akka-http (OAuthService) - bump version to 0.8.6 Co-Authored-By: Claude Opus 4.8 (1M context) --- build.sbt | 2 +- common/src/main/resources/reference.conf | 10 +++++ .../account/config/AccountSettings.scala | 5 +++ .../account/spi/GitHubApiService.scala | 38 +++++++++++++++++++ .../account/spi/OAuth2Service.scala | 24 ++++++++---- .../account/service/BaseAccountService.scala | 10 +++++ .../account/service/OAuthService.scala | 33 ++++++++-------- .../service/OAuthServiceEndpoints.scala | 25 +++++++++--- 8 files changed, 117 insertions(+), 30 deletions(-) diff --git a/build.sbt b/build.sbt index 3c4bda7..53f4b63 100644 --- a/build.sbt +++ b/build.sbt @@ -18,7 +18,7 @@ ThisBuild / organization := "app.softnetwork" name := "account" -ThisBuild / version := "0.8.5" +ThisBuild / version := "0.8.6" ThisBuild / scalaVersion := scala212 diff --git a/common/src/main/resources/reference.conf b/common/src/main/resources/reference.conf index ef4795a..cdf42d4 100644 --- a/common/src/main/resources/reference.conf +++ b/common/src/main/resources/reference.conf @@ -136,6 +136,13 @@ auth { path = "oauth" path = ${?AUTH_OAUTH_PATH} + callback { + # Frontend URL the social-login callback redirects to once the session is + # established; a status=success|error query param is appended. + redirect-url = "http://localhost/oauth/callback" + redirect-url = ${?AUTH_OAUTH_CALLBACK_REDIRECT_URL} + } + authorization-code { expirationTime = 5 expirationTime = ${?AUTH_AUTHORIZATION_CODE_EXPIRATION_TIME} @@ -170,6 +177,9 @@ auth { client-secret = null client-secret = ${?AUTH_OAUTH_GITHUB_CLIENT_SECRET} + + default-scope = "read:user,user:email" + default-scope = ${?AUTH_OAUTH_GITHUB_DEFAULT_SCOPE} } google { diff --git a/common/src/main/scala/app/softnetwork/account/config/AccountSettings.scala b/common/src/main/scala/app/softnetwork/account/config/AccountSettings.scala index 306053f..7ef1bb5 100644 --- a/common/src/main/scala/app/softnetwork/account/config/AccountSettings.scala +++ b/common/src/main/scala/app/softnetwork/account/config/AccountSettings.scala @@ -39,6 +39,11 @@ object AccountSettings extends StrictLogging { val OAuthPath: String = config.getString("auth.oauth.path") + /** Frontend URL the OAuth callback redirects the browser to once the session has been established + * (a `status=success|error` query param is appended). + */ + val OAuthCallbackRedirectUrl: String = config.getString("auth.oauth.callback.redirect-url") + val VerificationCodeSize: Int = config.getInt("auth.verification.code.size") val VerificationCodeExpirationTime: Int = config.getInt("auth.verification.code.expirationTime") diff --git a/common/src/main/scala/app/softnetwork/account/spi/GitHubApiService.scala b/common/src/main/scala/app/softnetwork/account/spi/GitHubApiService.scala index 4040428..19e28cf 100644 --- a/common/src/main/scala/app/softnetwork/account/spi/GitHubApiService.scala +++ b/common/src/main/scala/app/softnetwork/account/spi/GitHubApiService.scala @@ -2,6 +2,9 @@ package app.softnetwork.account.spi import com.github.scribejava.apis.GitHubApi import com.github.scribejava.core.builder.api.DefaultApi20 +import com.github.scribejava.core.model.OAuth2AccessToken + +import scala.util.Try class GitHubApiService extends OAuth2Service { override val networkName: String = "github" @@ -14,4 +17,39 @@ class GitHubApiService extends OAuth2Service { override lazy val defaultScope: String = provider.defaultScope.getOrElse("user:email") // user includes email + + /** GitHub returns `email: null` from /user when the primary email is private. The `user:email` + * scope lets us read /user/emails; resolve the primary verified address there and inject it so + * account login resolution (which keys on email) succeeds for users who keep their email + * private. + */ + protected def emailsUrl: String = "https://api.github.com/user/emails" + + override def userInfo( + code: String, + extraParameters: Map[String, String] + ): Try[Map[String, String]] = Try { + val accessToken = getAccessToken(code, extraParameters) + val user = fetch(accessToken, protectedResourceUrl) + if (user.get("email").exists(_.nonEmpty)) user + else primaryEmail(accessToken).fold(user)(email => user + ("email" -> email)) + } + + /** Returns the primary verified email from GitHub's /user/emails, falling back to any verified + * address, then the first one listed. + */ + protected def primaryEmail(accessToken: OAuth2AccessToken): Option[String] = { + import scala.jdk.CollectionConverters._ + val node = jsonMapper.readTree(get(accessToken, emailsUrl)) + if (!node.isArray) None + else { + val entries = node.elements().asScala.toList + entries + .find(e => e.path("primary").asBoolean(false) && e.path("verified").asBoolean(false)) + .orElse(entries.find(_.path("verified").asBoolean(false))) + .orElse(entries.headOption) + .map(_.path("email").asText("")) + .filter(_.nonEmpty) + } + } } diff --git a/common/src/main/scala/app/softnetwork/account/spi/OAuth2Service.scala b/common/src/main/scala/app/softnetwork/account/spi/OAuth2Service.scala index a25ee80..4c7738a 100644 --- a/common/src/main/scala/app/softnetwork/account/spi/OAuth2Service.scala +++ b/common/src/main/scala/app/softnetwork/account/spi/OAuth2Service.scala @@ -7,7 +7,7 @@ import com.fasterxml.jackson.databind.{DeserializationFeature, ObjectMapper} import com.fasterxml.jackson.module.scala.DefaultScalaModule import com.github.scribejava.core.builder.ServiceBuilder import com.github.scribejava.core.builder.api.DefaultApi20 -import com.github.scribejava.core.model.{OAuth2AccessToken, OAuthRequest, Response} +import com.github.scribejava.core.model.{OAuth2AccessToken, OAuthRequest, Response, Verb} import com.github.scribejava.core.oauth.{AccessTokenRequestParams, OAuth20Service} import scala.util.Try @@ -63,16 +63,26 @@ trait OAuth2Service { .setSerializationInclusion(Include.NON_EMPTY) jsonMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) - def userInfo(code: String, extraParameters: Map[String, String]): Try[Map[String, String]] = Try { - val accessToken = getAccessToken(code, extraParameters) - import com.github.scribejava.core.model.{OAuthRequest, Verb} - val request = - new OAuthRequest(Verb.GET, protectedResourceUrl) + /** Executes a signed GET against `url` and returns the raw response body. */ + protected def get(accessToken: OAuth2AccessToken, url: String): String = { + val request = new OAuthRequest(Verb.GET, url) signRequest(accessToken, request) + execute(request).getBody + } + + /** Fetches `url` as a JSON object and flattens it to a `Map[String, String]`. Providers (notably + * GitHub's /user) return many nullable fields; calling toString on a null value throws NPE, so + * null entries are dropped first. + */ + protected def fetch(accessToken: OAuth2AccessToken, url: String): Map[String, String] = jsonMapper - .readValue(execute(request).getBody, classOf[Map[String, Any]]) + .readValue(get(accessToken, url), classOf[Map[String, Any]]) + .filter { case (_, value) => value != null } .mapValues(_.toString) .toMap + + def userInfo(code: String, extraParameters: Map[String, String]): Try[Map[String, String]] = Try { + fetch(getAccessToken(code, extraParameters), protectedResourceUrl) } def extractData(data: Map[String, String]): OAuthData = OAuthData(networkName, data) diff --git a/core/src/main/scala/app/softnetwork/account/service/BaseAccountService.scala b/core/src/main/scala/app/softnetwork/account/service/BaseAccountService.scala index 1a0f651..ace43c2 100644 --- a/core/src/main/scala/app/softnetwork/account/service/BaseAccountService.scala +++ b/core/src/main/scala/app/softnetwork/account/service/BaseAccountService.scala @@ -1,5 +1,6 @@ package app.softnetwork.account.service +import app.softnetwork.account.config.AccountSettings import app.softnetwork.account.handlers.AccountHandler import app.softnetwork.account.message.{AccountCommand, AccountCommandResult} import app.softnetwork.persistence.service.Service @@ -10,6 +11,15 @@ import java.net.URLEncoder trait BaseAccountService extends Service[AccountCommand, AccountCommandResult] with AccountHandler { _: CommandTypeKey[AccountCommand] => + /** Frontend URL the social-login callback redirects the browser to once it completes, with a + * `status` (and optional `reason`) query param appended. + */ + protected def callbackRedirect(status: String, reason: Option[String] = None): String = + redirection( + AccountSettings.OAuthCallbackRedirectUrl, + Map("status" -> status) ++ reason.map("reason" -> _).toMap + ) + protected def redirection(uri: String, params: Map[String, String]): String = { var redirection = uri if (uri.contains("?")) { diff --git a/core/src/main/scala/app/softnetwork/account/service/OAuthService.scala b/core/src/main/scala/app/softnetwork/account/service/OAuthService.scala index f83d550..3189b13 100644 --- a/core/src/main/scala/app/softnetwork/account/service/OAuthService.scala +++ b/core/src/main/scala/app/softnetwork/account/service/OAuthService.scala @@ -214,11 +214,7 @@ trait OAuthService[SD <: SessionData with SessionDataDecorator[SD]] setSession(sc, st, session) { // create a new anti csrf token setNewCsrfToken(checkHeader) { - complete( - HttpResponse( - StatusCodes.OK - ) - ) + redirect(callbackRedirect("success"), StatusCodes.Found) } } case _ => @@ -232,30 +228,35 @@ trait OAuthService[SD <: SessionData with SessionDataDecorator[SD]] setSession(sc, st, session) { // create a new anti csrf token setNewCsrfToken(checkHeader) { - complete( - HttpResponse( - StatusCodes.OK - ) - ) + redirect(callbackRedirect("success"), StatusCodes.Found) } } case error: AccountErrorMessage => - complete( - HttpResponse(StatusCodes.BadRequest, entity = error) + log.error(s"sign up failed for ${service.networkName}: $error") + redirect( + callbackRedirect("error", Some("signup_failed")), + StatusCodes.Found + ) + case _ => + redirect( + callbackRedirect("error", Some("signup_failed")), + StatusCodes.Found ) - case _ => complete(StatusCodes.BadRequest) } } case _ => log.error(s"login not found within $data for ${service.networkName}") - complete(HttpResponse(StatusCodes.InternalServerError)) + redirect( + callbackRedirect("error", Some("login_not_found")), + StatusCodes.Found + ) } case Failure(f) => log.error(f.getMessage, f) - complete(HttpResponse(StatusCodes.InternalServerError)) + redirect(callbackRedirect("error", Some("user_info_failed")), StatusCodes.Found) } case _ => - complete(HttpResponse(StatusCodes.BadRequest)) + redirect(callbackRedirect("error", Some("missing_code")), StatusCodes.Found) } } } diff --git a/core/src/main/scala/app/softnetwork/account/service/OAuthServiceEndpoints.scala b/core/src/main/scala/app/softnetwork/account/service/OAuthServiceEndpoints.scala index 4701be8..a973d17 100644 --- a/core/src/main/scala/app/softnetwork/account/service/OAuthServiceEndpoints.scala +++ b/core/src/main/scala/app/softnetwork/account/service/OAuthServiceEndpoints.scala @@ -17,6 +17,7 @@ import sttp.capabilities import sttp.capabilities.akka.AkkaStreams import sttp.model.headers.AuthenticationScheme.{Basic, Bearer} import sttp.model.headers.WWWAuthenticateChallenge +import sttp.model.{HeaderNames, StatusCode} import sttp.tapir.EndpointIO.Example import sttp.tapir.json.json4s.jsonBody import sttp.tapir.model.UsernamePassword @@ -267,6 +268,10 @@ trait OAuthServiceEndpoints[SD <: SessionData with SessionDataDecorator[SD]] .description(s"OAuth2 ${service.networkName} callback endpoint") .securityIn(queryParams.description("authorization query parameters")) .errorOut(ApiErrors.oneOfApiErrors) + // On completion the session cookie is set (when present) and the browser + // is 302-redirected to the configured frontend URL. + .out(statusCode(StatusCode.Found)) + .out(header[String](HeaderNames.Location)) .serverSecurityLogicWithOutput(params => params.get("code") match { case Some(code) => @@ -281,7 +286,7 @@ trait OAuthServiceEndpoints[SD <: SessionData with SessionDataDecorator[SD]] session += ("provider", data.provider) session ++= data.data.toSeq Future.successful( - Right((), Some(session)) + Right((callbackRedirect("success"), Some(session))) ) case _ => run(generateUUID(), SignUpOAuth(data)) map { @@ -290,20 +295,28 @@ trait OAuthServiceEndpoints[SD <: SessionData with SessionDataDecorator[SD]] var session = companion.newSession.withId(r.account.uuid).withAnonymous(false) session += ("provider", data.provider) - Right((), Some(session)) - case other => Left(resultToApiError(other)) + Right((callbackRedirect("success"), Some(session))) + case other => + log.error(s"sign up failed for ${service.networkName}: $other") + Right((callbackRedirect("error", Some("signup_failed")), None)) } } case _ => log.error(s"login not found within $data for ${service.networkName}") - Future.successful(Left(ApiErrors.InternalServerError())) + Future.successful( + Right((callbackRedirect("error", Some("login_not_found")), None)) + ) } case Failure(f) => log.error(f.getMessage, f) - Future.successful(Left(ApiErrors.InternalServerError())) + Future.successful( + Right((callbackRedirect("error", Some("user_info_failed")), None)) + ) } case _ => - Future.successful(Left(ApiErrors.BadRequest())) + Future.successful( + Right((callbackRedirect("error", Some("missing_code")), None)) + ) } ) } From 8dc975f8d807876ba02d5f06d27baba1cf9a665a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Tue, 9 Jun 2026 09:14:53 +0200 Subject: [PATCH 2/2] fix(account): update OAuth callback status to Found --- .../app/softnetwork/account/scalatest/AccountRouteSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testkit/src/main/scala/app/softnetwork/account/scalatest/AccountRouteSpec.scala b/testkit/src/main/scala/app/softnetwork/account/scalatest/AccountRouteSpec.scala index 8a23eae..9a615e4 100644 --- a/testkit/src/main/scala/app/softnetwork/account/scalatest/AccountRouteSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/account/scalatest/AccountRouteSpec.scala @@ -272,7 +272,7 @@ trait AccountRouteSpec[ Get( s"/$RootPath/${AccountSettings.OAuthPath}/dummy/callback?code=1234" ) ~> routes ~> check { - status shouldEqual StatusCodes.OK + status shouldEqual StatusCodes.Found } } }