fix: address PR review comments for OAuth2 implementation #32

Merged
skettiks merged 25 commits from feat/oauth2-user-service-26 into develop 2026-05-16 19:30:55 +00:00
19 changed files with 343 additions and 10 deletions
+2 -5
View File
3
@@ -30,11 +30,10 @@ java {
dependencies {
implementation(platform(libs.sentry.bom))
devitq commented 2026-05-15 19:38:51 +00:00 (Migrated from github.com)
Review

do not remove any deps, this removal does not make any sense to your PR

do not remove any deps, this removal does not make any sense to your PR
implementation(platform(libs.spring.grpc.bom))
implementation(libs.spring.boot.starter.web)
implementation(libs.spring.boot.starter.actuator)
// implementation(libs.spring.boot.starter.security)
implementation(libs.spring.boot.starter.security)
implementation(libs.spring.boot.starter.cache)
implementation(libs.spring.boot.starter.data.jdbc)
implementation(libs.spring.boot.starter.validation)
@@ -46,8 +45,7 @@ dependencies {
implementation(libs.opentelemetry.exporter.otlp)
implementation(libs.sentry.spring.boot.starter)
implementation(libs.spring.grpc.starter)
implementation(libs.grpc.services)
implementation(libs.spring.boot.starter.oauth2.client)
runtimeOnly(libs.micrometer.registry.prometheus)
runtimeOnly(libs.h2)
@@ -57,7 +55,6 @@ dependencies {
testImplementation(libs.spring.boot.starter.test)
testImplementation(libs.kotlin.test.junit5)
testImplementation(libs.spring.grpc.test)
testImplementation(libs.mockk)
testRuntimeOnly(libs.junit.platform.launcher)
}
devitq commented 2026-05-15 19:38:46 +00:00 (Migrated from github.com)
Review

do not remove any deps, this removal does not make any sense to your PR

do not remove any deps, this removal does not make any sense to your PR
4
+2 -1
View File
@@ -11,9 +11,10 @@ spring-grpc = "1.0.1"
protoc = "3.25.1"
grpc-java = "1.60.0"
springdoc = "2.8.6"
mockk = "1.13.12"
mockk = "1.13.13"
[libraries]
spring-boot-starter-oauth2-client = { module = "org.springframework.boot:spring-boot-starter-oauth2-client" }
spring-boot-starter-web = { module = "org.springframework.boot:spring-boot-starter-web" }
spring-boot-starter-actuator = { module = "org.springframework.boot:spring-boot-starter-actuator" }
spring-boot-starter-security = { module = "org.springframework.boot:spring-boot-starter-security" }
1
@@ -27,7 +27,20 @@ class UserRepository(
}
override fun save(user: User): User {
val entity = user.toEntity()
val existingUser = findById(user.id)
val entity =
devitq commented 2026-05-08 17:28:14 +00:00 (Migrated from github.com)
Review

create a mapper to reduce code repetion

create a mapper to reduce code repetion
devitq commented 2026-05-15 19:37:53 +00:00 (Migrated from github.com)
Review

well, nevermind

well, nevermind
if (existingUser != null) {
val existingEntity = existingUser.toEntity()
user.toEntity(
provider = existingEntity.provider?.let { AuthProvider.valueOf(it) },
providerId = existingEntity.providerId,
createdAt = existingEntity.createdAt,
)
} else {
user.toEntity()
}
val updatedRows =
jdbc.update(
"""
@@ -41,6 +54,7 @@ class UserRepository(
entity.providerId,
entity.id,
)
if (updatedRows == 0) {
jdbc.update(
"""
@@ -68,6 +82,16 @@ class UserRepository(
return entities.firstOrNull()?.toDomain()
}
override fun findByEmail(email: String): User? {
val entities =
jdbc.query(
"SELECT id, name, email, provider, provider_id, created_at FROM users WHERE email = ?",
userEntityRowMapper,
email,
)
return entities.firstOrNull()?.toDomain()
}
override fun findAll(): List<User> =
jdbc
.query(
@@ -0,0 +1,86 @@
package com.project.movienight.adapters.security
import com.project.movienight.adapters.persistence.entity.toDomain
import com.project.movienight.adapters.persistence.entity.toEntity
import com.project.movienight.application.ports.input.security.OAuth2UserInfo
import com.project.movienight.application.ports.output.IdGenerator
import com.project.movienight.application.ports.output.UserRepositoryPort
import com.project.movienight.domain.model.AuthProvider
import com.project.movienight.domain.model.User
import org.slf4j.LoggerFactory
import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest
import org.springframework.security.oauth2.core.OAuth2AuthenticationException
import org.springframework.security.oauth2.core.user.OAuth2User
import org.springframework.stereotype.Service
@Service
class CustomOAuth2UserService(
private val userRepository: UserRepositoryPort,
private val idGenerator: IdGenerator,
) : DefaultOAuth2UserService() {
companion object {
private val log = LoggerFactory.getLogger(CustomOAuth2UserService::class.java)
}
override fun loadUser(userRequest: OAuth2UserRequest): OAuth2User {
val oAuth2User = super.loadUser(userRequest)
val registrationId = userRequest.clientRegistration.registrationId
log.debug("Processing OAuth2 login for provider: {}", registrationId)
return try {
val userInfo = OAuth2UserInfoFactory.getOAuth2UserInfo(registrationId, oAuth2User)
val user = findOrCreateUser(userInfo)
UserPrincipal.create(user, oAuth2User.attributes)
} catch (e: IllegalArgumentException) {
log.error("OAuth2 authentication failed: ${e.message}", e)
throw OAuth2AuthenticationException("Failed to process OAuth2 user data")
} catch (e: OAuth2AuthenticationException) {
log.error("OAuth2 authentication failed: ${e.message}", e)
throw e
}
}
private fun findOrCreateUser(userInfo: OAuth2UserInfo): User {
val provider = AuthProvider.valueOf(userInfo.getProvider().uppercase())
val existingUser =
userRepository.findByProviderAndProviderId(
provider,
userInfo.getProviderId(),
)
return if (existingUser != null) {
log.debug("User found by provider: {}", userInfo.getProvider())
existingUser
} else {
val userByEmail = userRepository.findByEmail(userInfo.getEmail())
if (userByEmail != null) {
log.debug("Linking OAuth2 account to existing user: {}", userInfo.getEmail())
val entity =
userByEmail.toEntity(
provider = provider,
providerId = userInfo.getProviderId(),
)
userRepository.save(entity.toDomain())
} else {
log.debug("Creating new user for provider: {}", userInfo.getProvider())
val newUser =
User(
id = idGenerator.generateId(),
name = userInfo.getName(),
email = userInfo.getEmail(),
library = null,
)
val entity =
newUser.toEntity(
provider = provider,
providerId = userInfo.getProviderId(),
)
userRepository.save(entity.toDomain())
}
}
}
}
@@ -0,0 +1,17 @@
package com.project.movienight.adapters.security
import com.project.movienight.application.ports.input.security.OAuth2UserInfo
class GoogleOAuth2UserInfo(
private val attributes: Map<String, Any>,
) : OAuth2UserInfo {
override fun getProviderId(): String = attributes["sub"] as String
override fun getEmail(): String = attributes["email"] as String
override fun getName(): String = attributes["name"] as String
override fun getProvider(): String = "google"
override fun getAttributes(): Map<String, Any> = attributes
}
@@ -0,0 +1,21 @@
package com.project.movienight.adapters.security
import com.project.movienight.application.ports.input.security.OAuth2UserInfo
import org.springframework.security.oauth2.core.OAuth2AuthenticationException
import org.springframework.security.oauth2.core.user.OAuth2User
object OAuth2UserInfoFactory {
fun getOAuth2UserInfo(
registrationId: String,
user: OAuth2User,
): OAuth2UserInfo {
val attributes = user.attributes
return when (registrationId.lowercase()) {
"google" -> GoogleOAuth2UserInfo(attributes)
"yandex" -> YandexOAuth2UserInfo(attributes)
"vk" -> VkOAuth2UserInfo(attributes)
else -> throw OAuth2AuthenticationException("Unknown provider: $registrationId")
}
}
}
@@ -0,0 +1,42 @@
package com.project.movienight.adapters.security
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.security.config.annotation.web.builders.HttpSecurity
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
import org.springframework.security.web.SecurityFilterChain
@Configuration
@EnableWebSecurity
class SecurityConfiguration(
private val customOAuth2UserService: CustomOAuth2UserService,
) {
@Bean
fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
http
.oauth2Login { oauth2 ->
oauth2
.userInfoEndpoint { userInfo ->
userInfo.userService(customOAuth2UserService)
}.defaultSuccessUrl("/api/users/me", true)
}.authorizeHttpRequests { auth ->
auth
.requestMatchers("/", "/login/**", "/oauth2/**", "/h2-console/**", "/actuator/health")
.permitAll()
.requestMatchers("/api/users/me")
.authenticated()
.requestMatchers("/api/**")
.authenticated()
.anyRequest()
.authenticated()
}.headers { headers ->
headers.frameOptions { frameOptions ->
frameOptions.sameOrigin()
}
}.csrf { csrf ->
csrf.disable()
}
return http.build()
}
}
@@ -0,0 +1,44 @@
package com.project.movienight.adapters.security
import com.project.movienight.domain.model.User
import org.springframework.security.core.GrantedAuthority
import org.springframework.security.core.authority.SimpleGrantedAuthority
import org.springframework.security.core.userdetails.UserDetails
import org.springframework.security.oauth2.core.user.OAuth2User
import java.util.UUID
class UserPrincipal(
private val user: User,
private val attributes: Map<String, Any>? = null,
) : OAuth2User,
UserDetails {
fun getId(): UUID = user.id
override fun getName(): String = user.name
override fun getAttributes(): Map<String, Any> = attributes ?: emptyMap()
override fun getAuthorities(): Collection<GrantedAuthority> =
listOf(
SimpleGrantedAuthority("ROLE_USER"),
)
override fun getPassword(): String = ""
override fun getUsername(): String = user.email
override fun isAccountNonExpired(): Boolean = true
override fun isAccountNonLocked(): Boolean = true
override fun isCredentialsNonExpired(): Boolean = true
override fun isEnabled(): Boolean = true
companion object {
fun create(
user: User,
attributes: Map<String, Any>? = null,
): UserPrincipal = UserPrincipal(user, attributes)
}
}
@@ -0,0 +1,28 @@
package com.project.movienight.adapters.security
import com.project.movienight.application.ports.input.security.OAuth2UserInfo
class VkOAuth2UserInfo(
private val attributes: Map<String, Any>,
) : OAuth2UserInfo {
override fun getProviderId(): String =
(attributes["response"] as? List<*>)
?.firstOrNull()
?.let { it as? Map<*, *> }
?.get("id")
?.toString() ?: ""
override fun getEmail(): String = attributes["email"]?.toString() ?: ""
override fun getName(): String {
val response = attributes["response"] as? List<*>
val first = response?.firstOrNull() as? Map<*, *>
val firstName = first?.get("first_name")?.toString() ?: ""
val lastName = first?.get("last_name")?.toString() ?: ""
return "$firstName $lastName".trim()
}
override fun getProvider(): String = "vk"
override fun getAttributes(): Map<String, Any> = attributes
}
@@ -0,0 +1,22 @@
package com.project.movienight.adapters.security
import com.project.movienight.application.ports.input.security.OAuth2UserInfo
class YandexOAuth2UserInfo(
private val attributes: Map<String, Any>,
) : OAuth2UserInfo {
override fun getProviderId(): String = attributes["id"]?.toString() ?: ""
override fun getEmail(): String =
(attributes["emails"] as? List<*>)
?.firstOrNull()
?.let { it as? Map<*, *> }
?.get("value")
?.toString() ?: ""
override fun getName(): String = attributes["display_name"]?.toString() ?: ""
override fun getProvider(): String = "yandex"
override fun getAttributes(): Map<String, Any> = attributes
}
@@ -68,6 +68,7 @@ class FilmLibraryController(
getFilmLibraryUseCase.getLibrary(
GetFilmLibraryQuery(userId = userId),
)
val film = getFilmByIdUseCase.getById(library.filmId)
return listOf(FilmResponse.fromDomain(film))
}
@@ -0,0 +1,13 @@
package com.project.movienight.application.ports.input.security
interface OAuth2UserInfo {
fun getProviderId(): String
fun getEmail(): String
fun getName(): String
fun getProvider(): String
fun getAttributes(): Map<String, Any>
}
@@ -9,6 +9,8 @@ interface UserRepositoryPort {
fun findById(id: UUID): User?
fun findByEmail(email: String): User?
fun findAll(): List<User>
fun deleteById(id: UUID)
+32
View File
@@ -25,6 +25,38 @@ spring:
console:
enabled: ${SPRING_H2_CONSOLE_ENABLED:true}
path: /h2-console
security:
oauth2:
client:
registration:
google:
client-id: ${OAUTH2_GOOGLE_CLIENT_ID:test-client-id}
client-secret: ${OAUTH2_GOOGLE_CLIENT_SECRET:test-secret}
scope: email,profile
yandex:
client-id: ${OAUTH2_YANDEX_CLIENT_ID:test-client-id}
client-secret: ${OAUTH2_YANDEX_CLIENT_SECRET:test-secret}
authorization-grant-type: authorization_code
redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}"
scope: login:email,login:avatar
vk:
client-id: ${OAUTH2_VK_CLIENT_ID:test-client-id}
client-secret: ${OAUTH2_VK_CLIENT_SECRET:test-secret}
authorization-grant-type: authorization_code
client-authentication-method: client_secret_post
redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}"
scope: email
provider:
yandex:
authorization-uri: https://oauth.yandex.ru/authorize
token-uri: https://oauth.yandex.ru/token
user-info-uri: https://login.yandex.ru/info
user-name-attribute: id
vk:
authorization-uri: https://oauth.vk.com/authorize
token-uri: https://oauth.vk.com/access_token
user-info-uri: https://api.vk.com/method/users.get?v=5.131&fields=photo_200
user-name-attribute: response
server:
shutdown: graceful
@@ -2,6 +2,7 @@ CREATE TABLE IF NOT EXISTS public.users (
id UUID PRIMARY KEY,
name VARCHAR(255) NOT NULL,
email VARCHAR(320) NOT NULL UNIQUE,
password VARCHAR(255),
provider VARCHAR(64),
provider_id VARCHAR(255),
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
@@ -0,0 +1,2 @@
CREATE UNIQUE INDEX IF NOT EXISTS idx_users_provider_provider_id
ON users(provider, provider_id);
@@ -17,7 +17,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPat
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
@SpringBootTest
@AutoConfigureMockMvc
@AutoConfigureMockMvc(addFilters = false)
class FilmControllerTest {
@Autowired
private lateinit var mockMvc: MockMvc
@@ -17,7 +17,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import org.springframework.transaction.annotation.Transactional
@SpringBootTest
@AutoConfigureMockMvc
@AutoConfigureMockMvc(addFilters = false)
@Transactional
class FilmLibraryControllerTest {
@Autowired
@@ -17,7 +17,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import org.springframework.transaction.annotation.Transactional
@SpringBootTest
@AutoConfigureMockMvc
@AutoConfigureMockMvc(addFilters = false)
@Transactional
class UserControllerTest {
@Autowired