Compare commits

..

12 Commits

6 changed files with 60 additions and 11 deletions

View File

@@ -408,3 +408,33 @@
### Features ### Features
* Simplify authorization request creation in OpenIDConnectService and use environment variables for Keycloak configuration ([82a9706](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/82a9706deb97db193015e55a048830d496e76d83)) * Simplify authorization request creation in OpenIDConnectService and use environment variables for Keycloak configuration ([82a9706](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/82a9706deb97db193015e55a048830d496e76d83))
## (2026-01-20)
### Features
* Use environment variables for Keycloak client configuration in staging ([fccd94c](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/fccd94cc11db43f99eded13207cc93fb59d8703e))
## (2026-01-20)
### Features
* Add logging for OpenID authentication process in OpenIDController ([b729344](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/b729344d1e79c631146dd988248d033d97e12d93))
## (2026-01-20)
### Features
* Add error logging for user info retrieval in OpenIDConnectService ([861f2f1](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/861f2f1184e860fdd164481167f941c11accfedd))
## (2026-01-20)
### Features
* Add idClaimName parameter to OpenIDConnectService for flexible ID claim mapping ([2f38855](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/2f388554495d8bd44b4c533baa0f2fc27eea22c2))
## (2026-01-21)
### Features
* Change log level to warn for OpenID callback in OpenIDController ([23cdbe9](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/23cdbe9bd127bc26405fd216372bbb2d7e718a77))
## (2026-01-21)
### Features
* Change log level to warn for OpenID callback in OpenIDController ([4a6598f](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/4a6598f102d8934c7502944a0addd400dd0cbcac))

View File

@@ -2,7 +2,7 @@ package controllers
import logic.user.{SessionManager, UserManager} import logic.user.{SessionManager, UserManager}
import model.users.User import model.users.User
import play.api.Configuration import play.api.{Configuration, Logger}
import play.api.libs.json.Json import play.api.libs.json.Json
import play.api.mvc.* import play.api.mvc.*
import play.api.mvc.Cookie.SameSite.Lax import play.api.mvc.Cookie.SameSite.Lax
@@ -20,6 +20,8 @@ class OpenIDController @Inject()(
val config: Configuration val config: Configuration
)(implicit ec: ExecutionContext) extends BaseController { )(implicit ec: ExecutionContext) extends BaseController {
private val logger = Logger(this.getClass)
def loginWithProvider(provider: String): Action[AnyContent] = Action.async { implicit request => def loginWithProvider(provider: String): Action[AnyContent] = Action.async { implicit request =>
val state = openIDService.generateState() val state = openIDService.generateState()
val nonce = openIDService.generateNonce() val nonce = openIDService.generateNonce()
@@ -47,8 +49,11 @@ class OpenIDController @Inject()(
val code = request.getQueryString("code") val code = request.getQueryString("code")
val error = request.getQueryString("error") val error = request.getQueryString("error")
logger.warn(s"Received callback from $provider with state $sessionState, nonce $sessionNonce, provider $sessionProvider, returned state $returnedState, code $code, error $error")
error match { error match {
case Some(err) => case Some(err) =>
logger.error(s"Authentication failed: $err")
Future.successful(Redirect("/login").flashing("error" -> s"Authentication failed: $err")) Future.successful(Redirect("/login").flashing("error" -> s"Authentication failed: $err"))
case None => case None =>
(for { (for {
@@ -56,6 +61,7 @@ class OpenIDController @Inject()(
_ <- Option(sessionProvider.contains(provider)) _ <- Option(sessionProvider.contains(provider))
authCode <- code authCode <- code
} yield { } yield {
logger.warn(s"Authentication successful for $provider")
openIDService.exchangeCodeForTokens(provider, authCode, sessionState.get).flatMap { openIDService.exchangeCodeForTokens(provider, authCode, sessionState.get).flatMap {
case Some(tokenResponse) => case Some(tokenResponse) =>
openIDService.getUserInfo(provider, tokenResponse.accessToken).flatMap { openIDService.getUserInfo(provider, tokenResponse.accessToken).flatMap {
@@ -63,6 +69,7 @@ class OpenIDController @Inject()(
// Check if user already exists // Check if user already exists
userManager.authenticateOpenID(provider, userInfo.id) match { userManager.authenticateOpenID(provider, userInfo.id) match {
case Some(user) => case Some(user) =>
logger.warn(s"User ${userInfo.name} (${userInfo.id}) already exists, logging them in")
// User already exists, log them in // User already exists, log them in
val sessionToken = sessionManager.createSession(user) val sessionToken = sessionManager.createSession(user)
Future.successful(Redirect(config.getOptional[String]("openid.mainRoute").getOrElse("/")) Future.successful(Redirect(config.getOptional[String]("openid.mainRoute").getOrElse("/"))
@@ -75,6 +82,7 @@ class OpenIDController @Inject()(
)) ))
.removingFromSession("oauth_state", "oauth_nonce", "oauth_provider", "oauth_access_token")) .removingFromSession("oauth_state", "oauth_nonce", "oauth_provider", "oauth_access_token"))
case None => case None =>
logger.warn(s"User ${userInfo.name} (${userInfo.id}) not found, creating new user")
// New user, redirect to username selection // New user, redirect to username selection
Future.successful(Redirect(config.get[String]("openid.selectUserRoute")) Future.successful(Redirect(config.get[String]("openid.selectUserRoute"))
.withSession( .withSession(
@@ -84,12 +92,15 @@ class OpenIDController @Inject()(
)) ))
} }
case None => case None =>
logger.error("Failed to retrieve user information")
Future.successful(Redirect("/login").flashing("error" -> "Failed to retrieve user information")) Future.successful(Redirect("/login").flashing("error" -> "Failed to retrieve user information"))
} }
case None => case None =>
logger.error("Failed to exchange authorization code")
Future.successful(Redirect("/login").flashing("error" -> "Failed to exchange authorization code")) Future.successful(Redirect("/login").flashing("error" -> "Failed to exchange authorization code"))
} }
}).getOrElse { }).getOrElse {
logger.error("Invalid state parameter")
Future.successful(Redirect("/login").flashing("error" -> "Invalid state parameter")) Future.successful(Redirect("/login").flashing("error" -> "Invalid state parameter"))
} }
} }

View File

@@ -2,7 +2,7 @@ package services
import com.typesafe.config.Config import com.typesafe.config.Config
import play.api.libs.ws.WSClient import play.api.libs.ws.WSClient
import play.api.Configuration import play.api.{Configuration, Logger}
import play.api.libs.json.* import play.api.libs.json.*
import java.net.URI import java.net.URI
@@ -11,7 +11,6 @@ import scala.concurrent.{ExecutionContext, Future}
import com.nimbusds.oauth2.sdk.* import com.nimbusds.oauth2.sdk.*
import com.nimbusds.oauth2.sdk.id.* import com.nimbusds.oauth2.sdk.id.*
import com.nimbusds.openid.connect.sdk.* import com.nimbusds.openid.connect.sdk.*
import play.api.libs.ws.DefaultBodyWritables.* import play.api.libs.ws.DefaultBodyWritables.*
case class OpenIDUserInfo( case class OpenIDUserInfo(
@@ -36,7 +35,8 @@ case class OpenIDProvider(
authorizationEndpoint: String, authorizationEndpoint: String,
tokenEndpoint: String, tokenEndpoint: String,
userInfoEndpoint: String, userInfoEndpoint: String,
scopes: Set[String] = Set("openid", "profile", "email") scopes: Set[String] = Set("openid", "profile", "email"),
idClaimName: String = "id"
) )
case class TokenResponse( case class TokenResponse(
@@ -50,6 +50,8 @@ case class TokenResponse(
@Singleton @Singleton
class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit ec: ExecutionContext) { class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit ec: ExecutionContext) {
private val logger = Logger(this.getClass)
private val providers = Map( private val providers = Map(
"discord" -> OpenIDProvider( "discord" -> OpenIDProvider(
name = "Discord", name = "Discord",
@@ -69,7 +71,8 @@ class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit
authorizationEndpoint = config.get[String]("openid.keycloak.authUrl") + "/protocol/openid-connect/auth", authorizationEndpoint = config.get[String]("openid.keycloak.authUrl") + "/protocol/openid-connect/auth",
tokenEndpoint = config.get[String]("openid.keycloak.authUrl") + "/protocol/openid-connect/token", tokenEndpoint = config.get[String]("openid.keycloak.authUrl") + "/protocol/openid-connect/token",
userInfoEndpoint = config.get[String]("openid.keycloak.authUrl") + "/protocol/openid-connect/userinfo", userInfoEndpoint = config.get[String]("openid.keycloak.authUrl") + "/protocol/openid-connect/userinfo",
scopes = Set("openid", "profile", "email") scopes = Set("openid", "profile", "email"),
idClaimName = "sub"
) )
) )
@@ -135,7 +138,7 @@ class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit
if (response.status == 200) { if (response.status == 200) {
val json = response.json val json = response.json
Some(OpenIDUserInfo( Some(OpenIDUserInfo(
id = (json \ "id").as[String], id = (json \ provider.idClaimName).as[String],
email = (json \ "email").asOpt[String], email = (json \ "email").asOpt[String],
name = (json \ "name").asOpt[String].orElse((json \ "login").asOpt[String]), name = (json \ "name").asOpt[String].orElse((json \ "login").asOpt[String]),
picture = (json \ "picture").asOpt[String].orElse((json \ "avatar_url").asOpt[String]), picture = (json \ "picture").asOpt[String].orElse((json \ "avatar_url").asOpt[String]),
@@ -143,10 +146,15 @@ class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit
providerName = provider.name providerName = provider.name
)) ))
} else { } else {
logger.error(s"Failed to retrieve user info from ${provider.userInfoEndpoint}, status code ${response.status}")
None
}
}
.recover { case e => {
logger.error(s"Failed to retrieve user info from ${provider.userInfoEndpoint}", e)
None None
} }
} }
.recover { case _ => None }
case None => Future.successful(None) case None => Future.successful(None)
} }
} }

View File

@@ -25,8 +25,8 @@ openid {
} }
keycloak { keycloak {
clientId = "your-keycloak-client-id" clientId = ${?KEYCLOAK_CLIENT_ID}
clientSecret = "your-keycloak-client-secret" clientSecret = ${?KEYCLOAK_CLIENT_SECRET}
redirectUri = "https://st.knockout.janis-eccarius.de/api/auth/keycloak/callback" redirectUri = "https://st.knockout.janis-eccarius.de/api/auth/keycloak/callback"
authUrl = ${?KEYCLOAK_AUTH_URL} authUrl = ${?KEYCLOAK_AUTH_URL}
authUrl = "https://identity.janis-eccarius.de/realms/master" authUrl = "https://identity.janis-eccarius.de/realms/master"

View File

@@ -1,3 +1,3 @@
MAJOR=4 MAJOR=4
MINOR=43 MINOR=49
PATCH=0 PATCH=0