Compare commits

..

14 Commits

13 changed files with 138 additions and 75 deletions

View File

@@ -388,3 +388,38 @@
### Features ### Features
* Add logging for user management operations in HibernateUserManager ([9ca1813](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/9ca1813f06539cffeb573d0e00571e4f2d5144f1)) * Add logging for user management operations in HibernateUserManager ([9ca1813](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/9ca1813f06539cffeb573d0e00571e4f2d5144f1))
## (2026-01-20)
### Features
* Integrate UserManager and HibernateUserManager in session management ([e32f4eb](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/e32f4eb8fff9daec46f20284e28e94a59231d033))
## (2026-01-20)
### Features
* Implement transaction management for user addition and removal in HibernateUserManager ([45dec00](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/45dec00b86a1395457226ed62ac319c61e38739a))
## (2026-01-20)
### Features
* Add mainRoute configuration for OpenID in application and environment files ([4f52c1a](https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/commit/4f52c1a0f30cf0b917452149a52b53b94d82a7c9))
## (2026-01-20)
### 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))
## (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))

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.info(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 {
@@ -58,22 +63,43 @@ class OpenIDController @Inject()(
} yield { } yield {
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).map { openIDService.getUserInfo(provider, tokenResponse.accessToken).flatMap {
case Some(userInfo) => case Some(userInfo) =>
// Store user info in session for username selection // Check if user already exists
Redirect(config.get[String]("openid.selectUserRoute")) userManager.authenticateOpenID(provider, userInfo.id) match {
case Some(user) =>
logger.info(s"User ${userInfo.name} (${userInfo.id}) already exists, logging them in")
// User already exists, log them in
val sessionToken = sessionManager.createSession(user)
Future.successful(Redirect(config.getOptional[String]("openid.mainRoute").getOrElse("/"))
.withCookies(Cookie(
name = "accessToken",
value = sessionToken,
httpOnly = true,
secure = false,
sameSite = Some(Lax)
))
.removingFromSession("oauth_state", "oauth_nonce", "oauth_provider", "oauth_access_token"))
case None =>
logger.info(s"User ${userInfo.name} (${userInfo.id}) not found, creating new user")
// New user, redirect to username selection
Future.successful(Redirect(config.get[String]("openid.selectUserRoute"))
.withSession( .withSession(
"oauth_user_info" -> Json.toJson(userInfo).toString(), "oauth_user_info" -> Json.toJson(userInfo).toString(),
"oauth_provider" -> provider, "oauth_provider" -> provider,
"oauth_access_token" -> tokenResponse.accessToken "oauth_access_token" -> tokenResponse.accessToken
) ))
case None =>
Redirect("/login").flashing("error" -> "Failed to retrieve user information")
} }
case None => case None =>
logger.error("Failed to retrieve user information")
Future.successful(Redirect("/login").flashing("error" -> "Failed to retrieve user information"))
}
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

@@ -1,25 +0,0 @@
package di
import com.google.inject.AbstractModule
import com.google.inject.name.Names
import logic.user.impl.HibernateUserManager
import play.api.db.DBApi
import play.api.{Configuration, Environment}
class ProductionModule(
environment: Environment,
configuration: Configuration
) extends AbstractModule {
override def configure(): Unit = {
// Bind HibernateUserManager for production
bind(classOf[logic.user.UserManager])
.to(classOf[logic.user.impl.HibernateUserManager])
.asEagerSingleton()
// Bind EntityManager for JPA
bind(classOf[jakarta.persistence.EntityManager])
.toProvider(classOf[EntityManagerProvider])
.asEagerSingleton()
}
}

View File

@@ -4,7 +4,7 @@ import com.auth0.jwt.algorithms.Algorithm
import com.auth0.jwt.{JWT, JWTVerifier} import com.auth0.jwt.{JWT, JWTVerifier}
import com.github.benmanes.caffeine.cache.{Cache, Caffeine} import com.github.benmanes.caffeine.cache.{Cache, Caffeine}
import com.typesafe.config.Config import com.typesafe.config.Config
import logic.user.SessionManager import logic.user.{SessionManager, UserManager}
import model.users.User import model.users.User
import scalafx.util.Duration import scalafx.util.Duration
import services.JwtKeyProvider import services.JwtKeyProvider
@@ -16,7 +16,7 @@ import javax.inject.{Inject, Singleton}
import scala.util.Try import scala.util.Try
@Singleton @Singleton
class BaseSessionManager @Inject()(val keyProvider: JwtKeyProvider, val userManager: StubUserManager, val config: Config) extends SessionManager { class BaseSessionManager @Inject()(val keyProvider: JwtKeyProvider, val userManager: UserManager, val config: Config) extends SessionManager {
private val algorithm = Algorithm.RSA512(keyProvider.publicKey, keyProvider.privateKey) private val algorithm = Algorithm.RSA512(keyProvider.publicKey, keyProvider.privateKey)
private val verifier: JWTVerifier = JWT.require(algorithm) private val verifier: JWTVerifier = JWT.require(algorithm)

View File

@@ -18,7 +18,9 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
private val logger = Logger(getClass.getName) private val logger = Logger(getClass.getName)
override def addUser(name: String, password: String): Boolean = { override def addUser(name: String, password: String): Boolean = {
val tx = em.getTransaction
try { try {
tx.begin()
// Check if user already exists // Check if user already exists
val existing = em.createQuery("SELECT u FROM UserEntity u WHERE u.username = :username", classOf[UserEntity]) val existing = em.createQuery("SELECT u FROM UserEntity u WHERE u.username = :username", classOf[UserEntity])
.setParameter("username", name) .setParameter("username", name)
@@ -26,6 +28,7 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
if (!existing.isEmpty) { if (!existing.isEmpty) {
logger.warn(s"User $name already exists") logger.warn(s"User $name already exists")
tx.rollback()
return false return false
} }
@@ -39,10 +42,12 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
em.persist(userEntity) em.persist(userEntity)
em.flush() em.flush()
tx.commit()
true true
} catch { } catch {
case e: Exception => { case e: Exception => {
if (tx.isActive) tx.rollback()
logger.error(s"Error adding user $name", e) logger.error(s"Error adding user $name", e)
false false
} }
@@ -50,7 +55,9 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
} }
override def addOpenIDUser(name: String, userInfo: OpenIDUserInfo): Boolean = { override def addOpenIDUser(name: String, userInfo: OpenIDUserInfo): Boolean = {
val tx = em.getTransaction
try { try {
tx.begin()
// Check if user already exists // Check if user already exists
val existing = em.createQuery("SELECT u FROM UserEntity u WHERE u.username = :username", classOf[UserEntity]) val existing = em.createQuery("SELECT u FROM UserEntity u WHERE u.username = :username", classOf[UserEntity])
.setParameter("username", name) .setParameter("username", name)
@@ -58,6 +65,7 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
if (!existing.isEmpty) { if (!existing.isEmpty) {
logger.warn(s"User $name already exists") logger.warn(s"User $name already exists")
tx.rollback()
return false return false
} }
@@ -72,6 +80,7 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
if (!existingOpenID.isEmpty) { if (!existingOpenID.isEmpty) {
logger.warn(s"OpenID user ${userInfo.provider}_${userInfo.id} already exists") logger.warn(s"OpenID user ${userInfo.provider}_${userInfo.id} already exists")
tx.rollback()
return false return false
} }
@@ -80,9 +89,11 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
em.persist(userEntity) em.persist(userEntity)
em.flush() em.flush()
tx.commit()
true true
} catch { } catch {
case e: Exception => { case e: Exception => {
if (tx.isActive) tx.rollback()
logger.error(s"Error adding OpenID user ${userInfo.provider}_${userInfo.id}", e) logger.error(s"Error adding OpenID user ${userInfo.provider}_${userInfo.id}", e)
false false
} }
@@ -167,20 +178,27 @@ class HibernateUserManager @Inject()(em: EntityManager, config: Config) extends
} }
override def removeUser(name: String): Boolean = { override def removeUser(name: String): Boolean = {
val tx = em.getTransaction
try { try {
tx.begin()
val users = em.createQuery("SELECT u FROM UserEntity u WHERE u.username = :username", classOf[UserEntity]) val users = em.createQuery("SELECT u FROM UserEntity u WHERE u.username = :username", classOf[UserEntity])
.setParameter("username", name) .setParameter("username", name)
.getResultList .getResultList
if (users.isEmpty) { if (users.isEmpty) {
tx.rollback()
false false
} else { } else {
em.remove(users.get(0)) em.remove(users.get(0))
em.flush() em.flush()
tx.commit()
true true
} }
} catch { } catch {
case _: Exception => false case _: Exception => {
if (tx.isActive) tx.rollback()
false
}
} }
} }
} }

View File

@@ -1,10 +1,24 @@
package modules package modules
import com.google.inject.AbstractModule import com.google.inject.AbstractModule
import di.EntityManagerProvider
import jakarta.persistence.EntityManager
import logic.Gateway import logic.Gateway
import logic.user.UserManager
import logic.user.impl.HibernateUserManager
class GatewayModule extends AbstractModule { class GatewayModule extends AbstractModule {
override def configure(): Unit = { override def configure(): Unit = {
bind(classOf[Gateway]).asEagerSingleton() bind(classOf[Gateway]).asEagerSingleton()
// Bind HibernateUserManager for production (when GatewayModule is used)
bind(classOf[UserManager])
.to(classOf[HibernateUserManager])
.asEagerSingleton()
// Bind EntityManager for JPA
bind(classOf[EntityManager])
.toProvider(classOf[EntityManagerProvider])
.asEagerSingleton()
} }
} }

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(
@@ -50,6 +49,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",
@@ -75,21 +76,7 @@ class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit
def getAuthorizationUrl(providerName: String, state: String, nonce: String): Option[String] = { def getAuthorizationUrl(providerName: String, state: String, nonce: String): Option[String] = {
providers.get(providerName).map { provider => providers.get(providerName).map { provider =>
val authRequest = if (provider.scopes.contains("openid")) { val authRequest = new AuthorizationRequest.Builder(
// Use OpenID Connect AuthenticationRequest for OpenID providers
new AuthenticationRequest.Builder(
new ResponseType(ResponseType.Value.CODE),
new com.nimbusds.oauth2.sdk.Scope(provider.scopes.mkString(" ")),
new com.nimbusds.oauth2.sdk.id.ClientID(provider.clientId),
URI.create(provider.redirectUri)
)
.state(new com.nimbusds.oauth2.sdk.id.State(state))
.nonce(new Nonce(nonce))
.endpointURI(URI.create(provider.authorizationEndpoint))
.build()
} else {
// Use standard OAuth2 AuthorizationRequest for non-OpenID providers (like Discord)
new AuthorizationRequest.Builder(
new ResponseType(ResponseType.Value.CODE), new ResponseType(ResponseType.Value.CODE),
new com.nimbusds.oauth2.sdk.id.ClientID(provider.clientId) new com.nimbusds.oauth2.sdk.id.ClientID(provider.clientId)
) )
@@ -98,7 +85,6 @@ class OpenIDConnectService@Inject(ws: WSClient, config: Configuration)(implicit
.redirectionURI(URI.create(provider.redirectUri)) .redirectionURI(URI.create(provider.redirectUri))
.endpointURI(URI.create(provider.authorizationEndpoint)) .endpointURI(URI.create(provider.authorizationEndpoint))
.build() .build()
}
authRequest.toURI.toString authRequest.toURI.toString
} }
@@ -158,10 +144,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

@@ -8,9 +8,10 @@
<persistence-unit name="defaultPersistenceUnit"> <persistence-unit name="defaultPersistenceUnit">
<provider>org.hibernate.jpa.HibernatePersistenceProvider</provider> <provider>org.hibernate.jpa.HibernatePersistenceProvider</provider>
<class>model.users.UserEntity</class>
<properties> <properties>
<!-- Hibernate specific settings --> <!-- Hibernate specific settings -->
<property name="hibernate.dialect" value="org.hibernate.dialect.PostgreSQLDialect"/>
<property name="hibernate.hbm2ddl.auto" value="update"/> <property name="hibernate.hbm2ddl.auto" value="update"/>
<property name="hibernate.archive.autodetection" value="class"/> <property name="hibernate.archive.autodetection" value="class"/>
<property name="hibernate.show_sql" value="false"/> <property name="hibernate.show_sql" value="false"/>

View File

@@ -29,6 +29,7 @@ play.filters.cors {
# Local Development OpenID Connect Configuration # Local Development OpenID Connect Configuration
openid { openid {
selectUserRoute="http://localhost:5173/select-username" selectUserRoute="http://localhost:5173/select-username"
mainRoute="http://localhost:5173/"
discord { discord {
clientId = ${?DISCORD_CLIENT_ID} clientId = ${?DISCORD_CLIENT_ID}

View File

@@ -18,17 +18,18 @@ play.filters.cors {
openid { openid {
selectUserRoute="https://knockout.janis-eccarius.de/select-username" selectUserRoute="https://knockout.janis-eccarius.de/select-username"
mainRoute="https://knockout.janis-eccarius.de/"
discord { discord {
clientId = ${?DISCORD_CLIENT_ID} clientId = ${?DISCORD_CLIENT_ID}
clientSecret = ${?DISCORD_CLIENT_SECRET} clientSecret = ${?DISCORD_CLIENT_SECRET}
redirectUri = ${?DISCORD_REDIRECT_URI} redirectUri = ${?DISCORD_REDIRECT_URI}
redirectUri = "https://knockout.janis-eccarius.de/auth/discord/callback" redirectUri = "https://knockout.janis-eccarius.de/api/auth/discord/callback"
} }
keycloak { keycloak {
clientId = "your-keycloak-client-id" clientId = ${?KEYCLOAK_CLIENT_ID}
clientSecret = "your-keycloak-client-secret" clientSecret = ${?KEYCLOAK_CLIENT_SECRET}
redirectUri = "https://knockout.janis-eccarius.de/api/auth/keycloak/callback" redirectUri = "https://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

@@ -15,17 +15,18 @@ play.filters.cors {
openid { openid {
selectUserRoute="https://st.knockout.janis-eccarius.de/select-username" selectUserRoute="https://st.knockout.janis-eccarius.de/select-username"
mainRoute="https://st.knockout.janis-eccarius.de/"
discord { discord {
clientId = ${?DISCORD_CLIENT_ID} clientId = ${?DISCORD_CLIENT_ID}
clientSecret = ${?DISCORD_CLIENT_SECRET} clientSecret = ${?DISCORD_CLIENT_SECRET}
redirectUri = ${?DISCORD_REDIRECT_URI} redirectUri = ${?DISCORD_REDIRECT_URI}
redirectUri = "https://st.knockout.janis-eccarius.de/auth/discord/callback" redirectUri = "https://st.knockout.janis-eccarius.de/api/auth/discord/callback"
} }
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=39 MINOR=46
PATCH=0 PATCH=0