Merge pull request #621 from gameyfin/fix/sso-users

Fix various SSO issues
Fixes #614

app/src/main/frontend/components/administration/SsoManagement.tsx

@@ -3,8 +3,8 @@ import withConfigPage from "Frontend/components/administration/withConfigPage";
 import * as Yup from 'yup';
 import ConfigFormField from "Frontend/components/administration/ConfigFormField";
 import Section from "Frontend/components/general/Section";
-import {addToast, Button} from "@heroui/react";
+import {addToast, Button, Checkbox, CheckboxGroup, Tooltip} from "@heroui/react";
-import {MagicWand} from "@phosphor-icons/react";
+import {MagicWand, Warning} from "@phosphor-icons/react";
 
 function SsoManagementLayout({getConfig, formik, setSaveMessage}: any) {
 
@@ -50,8 +50,21 @@ function SsoManagementLayout({getConfig, formik, setSaveMessage}: any) {
 
                     <Section title="SSO user handling"/>
                     <div className="flex flex-row items-baseline">
-                        <ConfigFormField configElement={getConfig("sso.oidc.auto-register-new-users")}
+                        <CheckboxGroup className="flex flex-col flex-1 items-baseline gap-2"
-                                         isDisabled={!formik.values.sso.oidc.enabled}/>
+                                       value={["auto-register-new-users"]}>
+                            <div className="flex flex-row gap-2">
+                                <Checkbox className="items-baseline" value="auto-register-new-users" isDisabled>
+                                    Automatically create new users after registration
+                                </Checkbox>
+                                <Tooltip content={"Currently not configurable (always enabled)"} placement="right">
+                                    <Warning weight="fill"/>
+                                </Tooltip>
+                            </div>
+                        </CheckboxGroup>
+                        {/*TODO: enable when the issues with unregistered SSO users are sorted
+
+                            <ConfigFormField configElement={getConfig("sso.oidc.auto-register-new-users")} isDisabled={!formik.values.sso.oidc.enabled}/>
+                        */}
                         <ConfigFormField configElement={getConfig("sso.oidc.match-existing-users-by")}
                                          isDisabled={!formik.values.sso.oidc.enabled ||
                                              !formik.values.sso.oidc["auto-register-new-users"]}/>

app/src/main/frontend/views/MainLayout.tsx

@@ -13,7 +13,6 @@ import {UserPreferenceService} from "Frontend/util/user-preference-service";
 import SearchBar from "Frontend/components/general/SearchBar";
 import {useSnapshot} from "valtio/react";
 import {gameState} from "Frontend/state/GameState";
-import {scanState} from "Frontend/state/ScanState";
 import ScanProgressPopover from "Frontend/components/general/ScanProgressPopover";
 import {isAdmin} from "Frontend/util/utils";
 
@@ -27,7 +26,6 @@ export default function MainLayout() {
     const isHomePage = location.pathname === "/";
     const [isExploding, setIsExploding] = useState(false);
     const games = useSnapshot(gameState).games;
-    const scans = useSnapshot(scanState);
 
     useEffect(() => {
         let newTitle = `Gameyfin - ${routeMetadata?.title}`;

app/src/main/kotlin/org/gameyfin/app/config/ConfigEndpoint.kt

@@ -7,15 +7,15 @@ import jakarta.annotation.security.RolesAllowed
 import org.gameyfin.app.config.dto.ConfigEntryDto
 import org.gameyfin.app.config.dto.ConfigUpdateDto
 import org.gameyfin.app.core.Role
+import org.gameyfin.app.users.UserService
 import org.gameyfin.app.users.util.isAdmin
-import org.springframework.security.core.context.SecurityContextHolder
-import org.springframework.security.core.userdetails.UserDetails
 import reactor.core.publisher.Flux
 
 @Endpoint
 @RolesAllowed(Role.Names.ADMIN)
 class ConfigEndpoint(
-    private val configService: ConfigService
+    private val configService: ConfigService,
+    private val userService: UserService,
 ) {
     companion object {
         val log = KotlinLogging.logger { }
@@ -25,7 +25,7 @@ class ConfigEndpoint(
 
     @PermitAll
     fun subscribe(): Flux<List<ConfigUpdateDto>> {
-        val user = SecurityContextHolder.getContext().authentication.principal as UserDetails
+        val user = userService.getCurrentUser()
         return if (user.isAdmin()) ConfigService.subscribe()
         else Flux.empty()
     }

app/src/main/kotlin/org/gameyfin/app/core/logging/LogEndpoint.kt

@@ -4,15 +4,15 @@ import com.vaadin.hilla.Endpoint
 import jakarta.annotation.security.PermitAll
 import jakarta.annotation.security.RolesAllowed
 import org.gameyfin.app.core.Role
+import org.gameyfin.app.users.UserService
 import org.gameyfin.app.users.util.isAdmin
-import org.springframework.security.core.context.SecurityContextHolder
-import org.springframework.security.core.userdetails.UserDetails
 import reactor.core.publisher.Flux
 
 @Endpoint
 @RolesAllowed(Role.Names.ADMIN)
 class LogEndpoint(
-    private val logService: LogService
+    private val logService: LogService,
+    private val userService: UserService,
 ) {
 
     fun reloadLogConfig() {
@@ -21,7 +21,7 @@ class LogEndpoint(
 
     @PermitAll
     fun getApplicationLogs(): Flux<String> {
-        val user = SecurityContextHolder.getContext().authentication.principal as UserDetails
+        val user = userService.getCurrentUser()
         return if (user.isAdmin()) logService.streamLogs()
         else Flux.empty()
     }

app/src/main/kotlin/org/gameyfin/app/core/plugins/PluginEndpoint.kt

@@ -5,21 +5,21 @@ import jakarta.annotation.security.PermitAll
 import jakarta.annotation.security.RolesAllowed
 import org.gameyfin.app.core.Role
 import org.gameyfin.app.core.plugins.dto.PluginUpdateDto
+import org.gameyfin.app.users.UserService
 import org.gameyfin.app.users.util.isAdmin
 import org.gameyfin.pluginapi.core.config.PluginConfigValidationResult
-import org.springframework.security.core.context.SecurityContextHolder
-import org.springframework.security.core.userdetails.UserDetails
 import reactor.core.publisher.Flux
 
 @Endpoint
 @RolesAllowed(Role.Names.ADMIN)
 class PluginEndpoint(
-    private val pluginService: PluginService
+    private val pluginService: PluginService,
+    private val userService: UserService,
 ) {
 
     @PermitAll
     fun subscribe(): Flux<List<PluginUpdateDto>> {
-        val user = SecurityContextHolder.getContext().authentication.principal as UserDetails
+        val user = userService.getCurrentUser()
         return if (user.isAdmin()) PluginService.subscribe()
         else Flux.empty()
     }

app/src/main/kotlin/org/gameyfin/app/core/security/CustomAuthenticationEntryPoint.kt

@@ -0,0 +1,22 @@
+package org.gameyfin.app.core.security
+
+import jakarta.servlet.http.HttpServletRequest
+import jakarta.servlet.http.HttpServletResponse
+import org.springframework.context.annotation.Conditional
+import org.springframework.security.core.AuthenticationException
+import org.springframework.security.web.AuthenticationEntryPoint
+
+@Conditional(SsoEnabledCondition::class)
+class CustomAuthenticationEntryPoint : AuthenticationEntryPoint {
+    override fun commence(
+        request: HttpServletRequest,
+        response: HttpServletResponse,
+        authException: AuthenticationException?
+    ) {
+        if (request.getParameter("direct") == "1") {
+            response.sendRedirect("/login")
+        } else {
+            response.sendRedirect("/oauth2/authorization/${SecurityConfig.SSO_PROVIDER_KEY}")
+        }
+    }
+}

app/src/main/kotlin/org/gameyfin/app/core/security/SecurityConfig.kt

@@ -30,7 +30,9 @@ class SecurityConfig(
     private val sessionRegistry: SessionRegistry
 ) : VaadinWebSecurity() {
 
-    private val ssoProviderKey: String = "oidc"
+    companion object {
+        const val SSO_PROVIDER_KEY = "oidc"
+    }
 
     @Throws(Exception::class)
     override fun configure(http: HttpSecurity) {
@@ -56,14 +58,18 @@ class SecurityConfig(
 
         super.configure(http)
 
+        setLoginView(http, "/login", "/")
+
         if (config.get(ConfigProperties.SSO.OIDC.Enabled) == true) {
-            setOAuth2LoginPage(http, "/oauth2/authorization/$ssoProviderKey")
             // Use custom success handler to handle user registration
             http.oauth2Login { oauth2Login -> oauth2Login.successHandler(ssoAuthenticationSuccessHandler) }
             // Prevent unnecessary redirects
             http.logout { logout -> logout.logoutSuccessHandler((HttpStatusReturningLogoutSuccessHandler(HttpStatus.OK))) }
-        } else {
+
-            setLoginView(http, "/login", "/")
+            // Custom authentication entry point to support SSO and direct login
+            http.exceptionHandling { exceptionHandling ->
+                exceptionHandling.authenticationEntryPoint(CustomAuthenticationEntryPoint())
+            }
         }
     }
 
@@ -79,7 +85,7 @@ class SecurityConfig(
     @Bean
     @Conditional(SsoEnabledCondition::class)
     fun clientRegistrationRepository(): ClientRegistrationRepository? {
-        val clientRegistration = ClientRegistration.withRegistrationId(ssoProviderKey)
+        val clientRegistration = ClientRegistration.withRegistrationId(SSO_PROVIDER_KEY)
             .clientId(config.get(ConfigProperties.SSO.OIDC.ClientId))
             .clientSecret(config.get(ConfigProperties.SSO.OIDC.ClientSecret))
             .scope("openid", "profile", "email")

app/src/main/kotlin/org/gameyfin/app/core/security/SsoAuthenticationSuccessHandler.kt

@@ -1,12 +1,12 @@
 package org.gameyfin.app.core.security
 
-import org.gameyfin.app.config.ConfigService
-import org.gameyfin.app.users.UserService
 import jakarta.servlet.http.HttpServletRequest
 import jakarta.servlet.http.HttpServletResponse
 import org.gameyfin.app.config.ConfigProperties
+import org.gameyfin.app.config.ConfigService
 import org.gameyfin.app.config.MatchUsersBy
 import org.gameyfin.app.users.RoleService
+import org.gameyfin.app.users.UserService
 import org.gameyfin.app.users.entities.User
 import org.springframework.security.core.Authentication
 import org.springframework.security.oauth2.core.oidc.user.OidcUser
@@ -42,11 +42,13 @@ class SsoAuthenticationSuccessHandler(
 
         // User could not be found in the database
         if (matchedUser == null) {
+            // TODO: User registration is currently forced, but this should be configurable.
+            //  However, this causes conflict with user preferences and game entities (since both reference the user entity)
             // Check if new user registration is enabled
-            if (config.get(ConfigProperties.SSO.OIDC.AutoRegisterNewUsers) == false) {
+            //if (config.get(ConfigProperties.SSO.OIDC.AutoRegisterNewUsers) == false) {
-                response.sendRedirect("/")
+            //    response.sendRedirect("/")
-                return
+            //    return
-            }
+            //
 
             // Register as new user
             matchedUser = User(oidcUser)

app/src/main/kotlin/org/gameyfin/app/games/GameService.kt

@@ -26,6 +26,7 @@ import org.gameyfin.pluginapi.gamemetadata.*
 import org.springframework.data.repository.findByIdOrNull
 import org.springframework.security.core.context.SecurityContextHolder
 import org.springframework.security.core.userdetails.UserDetails
+import org.springframework.security.oauth2.core.oidc.user.OidcUser
 import org.springframework.stereotype.Service
 import org.springframework.transaction.annotation.Transactional
 import reactor.core.publisher.Flux
@@ -125,8 +126,12 @@ class GameService(
         val existingGame = gameRepository.findByIdOrNull(gameUpdateDto.id)
             ?: throw IllegalArgumentException("Game with ID $gameUpdateDto.id not found")
 
-        val userDetails = SecurityContextHolder.getContext().authentication.principal as UserDetails
+        val userDetails = SecurityContextHolder.getContext().authentication.principal
-        val user = userService.getByUsernameNonNull(userDetails.username)
+        val user = when (userDetails) {
+            is UserDetails -> userService.getByUsernameNonNull(userDetails.username)
+            is OidcUser -> userService.getByUsernameNonNull(userDetails.preferredUsername)
+            else -> throw IllegalStateException("Unkown user type: ${userDetails::class.java.name}")
+        }
 
         // Update only non-null fields
         gameUpdateDto.title?.let {

app/src/main/kotlin/org/gameyfin/app/libraries/LibraryEndpoint.kt

@@ -1,23 +1,23 @@
 package org.gameyfin.app.libraries
 
 import com.vaadin.hilla.Endpoint
-import org.gameyfin.app.libraries.dto.LibraryDto
-import org.gameyfin.app.libraries.dto.LibraryEvent
 import jakarta.annotation.security.PermitAll
 import jakarta.annotation.security.RolesAllowed
 import org.gameyfin.app.core.Role
+import org.gameyfin.app.libraries.dto.LibraryDto
+import org.gameyfin.app.libraries.dto.LibraryEvent
 import org.gameyfin.app.libraries.dto.LibraryScanProgress
 import org.gameyfin.app.libraries.dto.LibraryUpdateDto
 import org.gameyfin.app.libraries.enums.ScanType
+import org.gameyfin.app.users.UserService
 import org.gameyfin.app.users.util.isAdmin
-import org.springframework.security.core.context.SecurityContextHolder
-import org.springframework.security.core.userdetails.UserDetails
 import reactor.core.publisher.Flux
 
 @Endpoint
 @PermitAll
 class LibraryEndpoint(
-    private val libraryService: LibraryService
+    private val libraryService: LibraryService,
+    private val userService: UserService,
 ) {
     fun subscribeToLibraryEvents(): Flux<List<LibraryEvent>> {
         return LibraryService.subscribeToLibraryEvents()
@@ -27,7 +27,7 @@ class LibraryEndpoint(
 
 
     fun subscribeToScanProgressEvents(): Flux<List<LibraryScanProgress>> {
-        val user = SecurityContextHolder.getContext().authentication.principal as UserDetails
+        val user = userService.getCurrentUser()
         return if (user.isAdmin()) LibraryService.subscribeToScanProgressEvents()
         else Flux.empty()
     }

app/src/main/kotlin/org/gameyfin/app/users/UserEndpoint.kt

@@ -1,12 +1,12 @@
 package org.gameyfin.app.users
 
 import com.vaadin.hilla.Endpoint
-import org.gameyfin.app.users.dto.UserUpdateDto
-import org.gameyfin.app.users.enums.RoleAssignmentResult
 import jakarta.annotation.security.PermitAll
 import jakarta.annotation.security.RolesAllowed
 import org.gameyfin.app.core.Role
 import org.gameyfin.app.users.dto.UserInfoDto
+import org.gameyfin.app.users.dto.UserUpdateDto
+import org.gameyfin.app.users.enums.RoleAssignmentResult
 import org.springframework.security.core.Authentication
 import org.springframework.security.core.context.SecurityContextHolder
 
@@ -23,8 +23,7 @@ class UserEndpoint(
 
     @PermitAll
     fun getUserInfo(): UserInfoDto {
-        val auth: Authentication = SecurityContextHolder.getContext().authentication
+        return userService.getUserInfo()
-        return userService.getUserInfo(auth)
     }
 
     @PermitAll

app/src/main/kotlin/org/gameyfin/app/users/UserService.kt

@@ -1,25 +1,20 @@
 package org.gameyfin.app.users
 
+import io.github.oshai.kotlinlogging.KotlinLogging
+import org.gameyfin.app.config.ConfigProperties
 import org.gameyfin.app.config.ConfigService
+import org.gameyfin.app.core.Role
+import org.gameyfin.app.core.Utils
+import org.gameyfin.app.core.events.*
 import org.gameyfin.app.games.entities.Image
+import org.gameyfin.app.media.ImageService
+import org.gameyfin.app.users.dto.UserInfoDto
+import org.gameyfin.app.users.dto.UserRegistrationDto
 import org.gameyfin.app.users.dto.UserUpdateDto
 import org.gameyfin.app.users.emailconfirmation.EmailConfirmationService
 import org.gameyfin.app.users.enums.RoleAssignmentResult
 import org.gameyfin.app.users.persistence.UserRepository
-import io.github.oshai.kotlinlogging.KotlinLogging
-import org.gameyfin.app.config.ConfigProperties
-import org.gameyfin.app.core.Role
-import org.gameyfin.app.core.Utils
-import org.gameyfin.app.core.events.AccountDeletedEvent
-import org.gameyfin.app.core.events.AccountStatusChangedEvent
-import org.gameyfin.app.core.events.EmailNeedsConfirmationEvent
-import org.gameyfin.app.core.events.RegistrationAttemptWithExistingEmailEvent
-import org.gameyfin.app.core.events.UserRegistrationWaitingForApprovalEvent
-import org.gameyfin.app.media.ImageService
-import org.gameyfin.app.users.dto.UserInfoDto
-import org.gameyfin.app.users.dto.UserRegistrationDto
 import org.springframework.context.ApplicationEventPublisher
-import org.springframework.security.core.Authentication
 import org.springframework.security.core.GrantedAuthority
 import org.springframework.security.core.authority.SimpleGrantedAuthority
 import org.springframework.security.core.context.SecurityContextHolder
@@ -52,6 +47,19 @@ class UserService(
     override fun loadUserByUsername(username: String): UserDetails {
         val user = getByUsernameNonNull(username)
 
+        if (user.oidcProviderId != null && user.password == null) {
+            // If the user is an OIDC user, we return a UserDetails with no password
+            return User(
+                user.username,
+                "", // OIDC users do not have a password
+                user.enabled,
+                true,
+                true,
+                true,
+                toAuthorities(user.roles)
+            )
+        }
+
         return User(
             user.username,
             user.password,
@@ -66,7 +74,8 @@ class UserService(
     fun existsByUsername(username: String): Boolean = userRepository.existsByUsername(username)
     fun existsByEmail(email: String): Boolean = userRepository.existsByEmail(email)
 
-    fun findByOidcProviderId(oidcProviderId: String): org.gameyfin.app.users.entities.User? = userRepository.findByOidcProviderId(oidcProviderId)
+    fun findByOidcProviderId(oidcProviderId: String): org.gameyfin.app.users.entities.User? =
+        userRepository.findByOidcProviderId(oidcProviderId)
 
     fun getAllUsers(): List<UserInfoDto> {
         return userRepository.findAll().map { u -> toUserInfo(u) }
@@ -84,7 +93,8 @@ class UserService(
         return userRepository.findByUsername(username) ?: throw UsernameNotFoundException("Unknown user '$username'")
     }
 
-    fun getUserInfo(auth: Authentication): UserInfoDto {
+    fun getUserInfo(): UserInfoDto {
+        val auth = SecurityContextHolder.getContext().authentication
         val principal = auth.principal
 
         if (principal is OidcUser) {
@@ -99,6 +109,15 @@ class UserService(
         return toUserInfo(user)
     }
 
+    fun getCurrentUser(): org.gameyfin.app.users.entities.User {
+        val auth = SecurityContextHolder.getContext().authentication
+        if (auth.principal is OidcUser) {
+            return userRepository.findByOidcProviderId((auth.principal as OidcUser).subject)
+                ?: throw UsernameNotFoundException("OIDC user not found")
+        }
+        return getByUsernameNonNull(auth.name)
+    }
+
     fun getAvatar(username: String): Image? {
         val user = getByUsernameNonNull(username)
         return user.avatar
@@ -126,7 +145,11 @@ class UserService(
     }
 
     fun registerOrUpdateUser(user: org.gameyfin.app.users.entities.User): org.gameyfin.app.users.entities.User {
-        user.password = passwordEncoder.encode(user.password)
+        // OIDC users can have null passwords, so we only encode if a password is provided
+        if (user.password != null) {
+            user.password = passwordEncoder.encode(user.password)
+        }
+
         return userRepository.save(user)
     }
 
@@ -174,7 +197,10 @@ class UserService(
         }
     }
 
-    fun registerUserFromInvitation(registration: UserRegistrationDto, email: String): org.gameyfin.app.users.entities.User {
+    fun registerUserFromInvitation(
+        registration: UserRegistrationDto,
+        email: String
+    ): org.gameyfin.app.users.entities.User {
         val user = org.gameyfin.app.users.entities.User(
             username = registration.username,
             password = passwordEncoder.encode(registration.password),

app/src/main/kotlin/org/gameyfin/app/users/util/UserDetailsExtensions.kt

@@ -14,4 +14,8 @@ fun UserDetails.hasRole(role: Role): Boolean {
 
 fun UserDetails.isAdmin(): Boolean {
     return hasRole(Role.SUPERADMIN) || hasRole(Role.ADMIN)
+}
+
+fun User.isAdmin(): Boolean {
+    return hasRole(Role.SUPERADMIN) || hasRole(Role.ADMIN)
 }

app/src/main/resources/application.yml

@@ -10,6 +10,7 @@ server:
   servlet:
     session:
       tracking-modes: cookie
+  forward-headers-strategy: framework
 
 management:
   server: