From fe776bc43cd91cdaaa72d6f6bf94e6ac221aa954 Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Tue, 12 Sep 2023 14:05:35 +0300 Subject: [PATCH] Deduplicate logic with SaveUserDetails in api-gateway (#2577) - added an attibute for `SaveUserDetails` - refactored method createNewIfRequired -- now it returns `User` - `SaveUserDetails` is stored in `WebSession` for OAuth2 --- .../controller/SecurityInfoController.kt | 9 ++- .../gateway/security/WebSecurityConfig.kt | 8 ++- .../save/gateway/service/BackendService.kt | 67 +++++++++---------- ...uthorizationHeadersGatewayFilterFactory.kt | 24 ++----- ...oringServerAuthenticationSuccessHandler.kt | 7 +- .../controllers/internal/UsersController.kt | 42 ++++-------- .../backend/service/UserDetailsService.kt | 44 ++++++++---- .../com/saveourtool/save/utils/Constants.kt | 5 ++ 8 files changed, 104 insertions(+), 102 deletions(-) diff --git a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/controller/SecurityInfoController.kt b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/controller/SecurityInfoController.kt index 3ac7dc3159..1eddb5fa23 100644 --- a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/controller/SecurityInfoController.kt +++ b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/controller/SecurityInfoController.kt @@ -5,6 +5,7 @@ import com.saveourtool.save.info.OauthProviderInfo import org.springframework.security.core.Authentication import org.springframework.security.oauth2.client.registration.InMemoryReactiveClientRegistrationRepository import org.springframework.web.bind.annotation.* +import org.springframework.web.server.WebSession import reactor.core.publisher.Mono /** @@ -33,8 +34,14 @@ class SecurityInfoController( * Endpoint that provides the information about the current logged-in user (powered by spring security and OAUTH) * * @param authentication + * @param session * @return user information */ @GetMapping("/user") - fun currentUserName(authentication: Authentication?): Mono = backendService.findNameByAuthentication(authentication) + fun currentUserName( + authentication: Authentication?, + session: WebSession, + ): Mono = authentication + ?.let { principal -> backendService.findByPrincipal(principal, session).map { it.name } } + ?: Mono.empty() } diff --git a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/security/WebSecurityConfig.kt b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/security/WebSecurityConfig.kt index 2e6693d798..228cbb752d 100644 --- a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/security/WebSecurityConfig.kt +++ b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/security/WebSecurityConfig.kt @@ -196,7 +196,13 @@ private fun userStatusBasedAuthorizationDecision( backendService: BackendService, authentication: Mono, authorizationContext: AuthorizationContext, -) = authentication.flatMap { backendService.findByAuthentication(it) } +) = authentication + .flatMap { principal -> + authorizationContext.exchange.session + .flatMap { session -> + backendService.findByPrincipal(principal, session) + } + } .filter { it.isEnabled } .flatMap { authorizationManagerAuthorizationDecision(authentication, authorizationContext) } .defaultIfEmpty(AuthorizationDecision(false)) diff --git a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/service/BackendService.kt b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/service/BackendService.kt index 7e1430ef8f..cf936c40ab 100644 --- a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/service/BackendService.kt +++ b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/service/BackendService.kt @@ -3,19 +3,24 @@ package com.saveourtool.save.gateway.service import com.saveourtool.save.authservice.utils.SaveUserDetails import com.saveourtool.save.entities.User import com.saveourtool.save.gateway.config.ConfigurationProperties +import com.saveourtool.save.utils.SAVE_USER_DETAILS_ATTIBUTE import com.saveourtool.save.utils.orNotFound +import com.saveourtool.save.utils.switchIfEmptyToResponseException +import org.springframework.http.HttpStatus import org.springframework.http.MediaType +import org.springframework.security.authentication.BadCredentialsException import org.springframework.security.authentication.UsernamePasswordAuthenticationToken -import org.springframework.security.core.Authentication import org.springframework.security.core.userdetails.UserDetails import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken import org.springframework.stereotype.Service import org.springframework.web.reactive.function.client.WebClient import org.springframework.web.reactive.function.client.toEntity import org.springframework.web.server.ResponseStatusException +import org.springframework.web.server.WebSession import reactor.core.publisher.Mono import reactor.kotlin.core.publisher.toMono +import java.security.Principal /** * A service to backend to lookup users in DB @@ -47,44 +52,27 @@ class BackendService( private fun findAuthenticationUserDetails(uri: String): Mono = webClient.get() .uri(uri) .retrieve() - .onStatus({ it.is4xxClientError }) { - Mono.error(ResponseStatusException(it.statusCode())) - } - .toEntity() - .flatMap { responseEntity -> - responseEntity.body.toMono().orNotFound { "Authentication body is empty" } - } + .getSaveUserDetails() /** - * Find current user [SaveUserDetails] by [authentication]. + * Find current user [SaveUserDetails] by [principal]. * - * @param authentication current user [Authentication] + * @param principal current user [Principal] + * @param session current [WebSession] * @return current user [SaveUserDetails] */ - fun findByAuthentication(authentication: Authentication): Mono = when (authentication) { - is UsernamePasswordAuthenticationToken -> findByName(authentication.name) - is OAuth2AuthenticationToken -> { - val source = authentication.authorizedClientRegistrationId - val nameInSource = authentication.name - findByOriginalLogin(source, nameInSource) - } - else -> Mono.empty() - } - - /** - * Find current username by [authentication]. - * - * @param authentication current user [Authentication] - * @return current username - */ - fun findNameByAuthentication(authentication: Authentication?): Mono = when (authentication) { - is UsernamePasswordAuthenticationToken -> authentication.name.toMono() - is OAuth2AuthenticationToken -> { - val source = authentication.authorizedClientRegistrationId - val nameInSource = authentication.name - findByOriginalLogin(source, nameInSource).map { it.name } - } - else -> Mono.empty() + fun findByPrincipal(principal: Principal, session: WebSession): Mono = when (principal) { + is OAuth2AuthenticationToken -> session.getAttribute(SAVE_USER_DETAILS_ATTIBUTE) + .toMono() + .switchIfEmptyToResponseException(HttpStatus.INTERNAL_SERVER_ERROR) { + "Not found attribute $SAVE_USER_DETAILS_ATTIBUTE for ${OAuth2AuthenticationToken::class}" + } + is UsernamePasswordAuthenticationToken -> (principal.principal as? SaveUserDetails) + .toMono() + .switchIfEmptyToResponseException(HttpStatus.INTERNAL_SERVER_ERROR) { + "Unexpected principal type ${principal.principal.javaClass} in ${UsernamePasswordAuthenticationToken::class}" + } + else -> Mono.error(BadCredentialsException("Unsupported authentication type: ${principal::class}")) } /** @@ -94,13 +82,18 @@ class BackendService( * @param nameInSource * @return empty [Mono] */ - fun createNewIfRequired(source: String, nameInSource: String): Mono = webClient.post() + fun createNewIfRequired(source: String, nameInSource: String): Mono = webClient.post() .uri("/internal/users/new/$source/$nameInSource") .contentType(MediaType.APPLICATION_JSON) .retrieve() + .getSaveUserDetails() + + private fun WebClient.ResponseSpec.getSaveUserDetails(): Mono = this .onStatus({ it.is4xxClientError }) { Mono.error(ResponseStatusException(it.statusCode())) } - .toBodilessEntity() - .then() + .toEntity() + .flatMap { responseEntity -> + responseEntity.body.toMono().orNotFound { "Authentication body is empty" } + } } diff --git a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/AuthorizationHeadersGatewayFilterFactory.kt b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/AuthorizationHeadersGatewayFilterFactory.kt index b7cfe26605..b93b4246b4 100644 --- a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/AuthorizationHeadersGatewayFilterFactory.kt +++ b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/AuthorizationHeadersGatewayFilterFactory.kt @@ -1,20 +1,12 @@ package com.saveourtool.save.gateway.utils -import com.saveourtool.save.authservice.utils.SaveUserDetails import com.saveourtool.save.gateway.service.BackendService -import com.saveourtool.save.utils.switchIfEmptyToResponseException import org.springframework.cloud.gateway.filter.GatewayFilter import org.springframework.cloud.gateway.filter.GatewayFilterChain import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory import org.springframework.http.HttpHeaders -import org.springframework.http.HttpStatus -import org.springframework.security.authentication.BadCredentialsException -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken -import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken import org.springframework.stereotype.Component import org.springframework.web.server.ServerWebExchange -import reactor.core.publisher.Mono -import reactor.kotlin.core.publisher.toMono import java.security.Principal /** @@ -28,7 +20,11 @@ class AuthorizationHeadersGatewayFilterFactory( ) : AbstractGatewayFilterFactory() { override fun apply(config: Any?): GatewayFilter = GatewayFilter { exchange: ServerWebExchange, chain: GatewayFilterChain -> exchange.getPrincipal() - .flatMap { resolveSaveUser(it) } + .flatMap { principal -> + exchange.session.flatMap { session -> + backendService.findByPrincipal(principal, session) + } + } .map { user -> exchange.mutate() .request { builder -> @@ -42,14 +38,4 @@ class AuthorizationHeadersGatewayFilterFactory( .defaultIfEmpty(exchange) .flatMap { chain.filter(it) } } - - private fun resolveSaveUser(principal: Principal): Mono = when (principal) { - is OAuth2AuthenticationToken -> backendService.findByOriginalLogin(principal.authorizedClientRegistrationId, principal.name) - is UsernamePasswordAuthenticationToken -> (principal.principal as? SaveUserDetails) - .toMono() - .switchIfEmptyToResponseException(HttpStatus.INTERNAL_SERVER_ERROR) { - "Unexpected principal type ${principal.principal.javaClass} in ${UsernamePasswordAuthenticationToken::class}" - } - else -> Mono.error(BadCredentialsException("Unsupported authentication type: ${principal::class}")) - } } diff --git a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/StoringServerAuthenticationSuccessHandler.kt b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/StoringServerAuthenticationSuccessHandler.kt index 5fc1e86920..0faf43ae6f 100644 --- a/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/StoringServerAuthenticationSuccessHandler.kt +++ b/api-gateway/src/main/kotlin/com/saveourtool/save/gateway/utils/StoringServerAuthenticationSuccessHandler.kt @@ -1,6 +1,7 @@ package com.saveourtool.save.gateway.utils import com.saveourtool.save.gateway.service.BackendService +import com.saveourtool.save.utils.SAVE_USER_DETAILS_ATTIBUTE import org.slf4j.LoggerFactory import org.springframework.security.authentication.BadCredentialsException @@ -29,6 +30,10 @@ class StoringServerAuthenticationSuccessHandler( } else { throw BadCredentialsException("Not supported authentication type ${authentication::class}") } - return backendService.createNewIfRequired(source, nameInSource) + return backendService.createNewIfRequired(source, nameInSource).flatMap { saveUser -> + webFilterExchange.exchange.session.map { + it.attributes[SAVE_USER_DETAILS_ATTIBUTE] = saveUser + } + }.then() } } diff --git a/save-backend/src/main/kotlin/com/saveourtool/save/backend/controllers/internal/UsersController.kt b/save-backend/src/main/kotlin/com/saveourtool/save/backend/controllers/internal/UsersController.kt index 619e276306..b18e42688d 100644 --- a/save-backend/src/main/kotlin/com/saveourtool/save/backend/controllers/internal/UsersController.kt +++ b/save-backend/src/main/kotlin/com/saveourtool/save/backend/controllers/internal/UsersController.kt @@ -1,13 +1,10 @@ package com.saveourtool.save.backend.controllers.internal import com.saveourtool.save.authservice.utils.SaveUserDetails -import com.saveourtool.save.backend.repository.OriginalLoginRepository import com.saveourtool.save.backend.service.UserDetailsService -import com.saveourtool.save.domain.Role +import com.saveourtool.save.utils.blockingToMono -import org.slf4j.LoggerFactory import org.springframework.http.ResponseEntity -import org.springframework.transaction.annotation.Transactional import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.PostMapping @@ -24,33 +21,22 @@ typealias SaveUserDetailsResponse = ResponseEntity @RequestMapping("/internal/users") class UsersController( private val userService: UserDetailsService, - private val originalLoginRepository: OriginalLoginRepository, ) { - private val logger = LoggerFactory.getLogger(javaClass) - /** - * Stores user in the DB with provided [name] with [roleForNewUser] as role. + * Stores user in the DB with provided [name] with default role. * And add a link to [source] for created user * * @param source user source * @param name user name */ @PostMapping("/new/{source}/{name}") - @Transactional fun saveNewUserIfRequired( @PathVariable source: String, @PathVariable name: String, - ) { - val userFind = originalLoginRepository.findByNameAndSource(name, source) - - userFind?.user?.let { - logger.debug("User $name ($source) is already present in the DB") - } ?: run { - logger.info("Saving user $name ($source) with authorities $roleForNewUser to the DB") - val savedUser = userService.saveNewUser(name, roleForNewUser) - userService.addSource(savedUser, name, source) + ): Mono = blockingToMono { userService.saveNewUserIfRequired(source, name) } + .map { + ResponseEntity.ok().body(SaveUserDetails(it)) } - } /** * Find user by name @@ -61,9 +47,10 @@ class UsersController( @GetMapping("/find-by-name/{userName}") fun findByName( @PathVariable userName: String, - ): Mono = userService.findByName(userName).map { - ResponseEntity.ok().body(SaveUserDetails(it)) - } + ): Mono = blockingToMono { userService.findByName(userName) } + .map { + ResponseEntity.ok().body(SaveUserDetails(it)) + } /** * Find user by name and source @@ -76,11 +63,8 @@ class UsersController( fun findByOriginalLogin( @PathVariable source: String, @PathVariable nameInSource: String, - ): Mono = userService.findByOriginalLogin(nameInSource, source).map { - ResponseEntity.ok().body(SaveUserDetails(it)) - } - - companion object { - private val roleForNewUser = Role.VIEWER.asSpringSecurityRole() - } + ): Mono = blockingToMono { userService.findByOriginalLogin(nameInSource, source) } + .map { + ResponseEntity.ok().body(SaveUserDetails(it)) + } } diff --git a/save-backend/src/main/kotlin/com/saveourtool/save/backend/service/UserDetailsService.kt b/save-backend/src/main/kotlin/com/saveourtool/save/backend/service/UserDetailsService.kt index 8d3c9172a2..487ed50c60 100644 --- a/save-backend/src/main/kotlin/com/saveourtool/save/backend/service/UserDetailsService.kt +++ b/save-backend/src/main/kotlin/com/saveourtool/save/backend/service/UserDetailsService.kt @@ -12,10 +12,8 @@ import com.saveourtool.save.domain.UserSaveStatus import com.saveourtool.save.entities.OriginalLogin import com.saveourtool.save.entities.User import com.saveourtool.save.info.UserStatus -import com.saveourtool.save.utils.AVATARS_PACKS_DIR -import com.saveourtool.save.utils.AvatarType -import com.saveourtool.save.utils.blockingToMono -import com.saveourtool.save.utils.orNotFound +import com.saveourtool.save.utils.* +import org.slf4j.Logger import org.springframework.security.core.Authentication import org.springframework.stereotype.Service @@ -39,9 +37,7 @@ class UserDetailsService( * @param username * @return spring's UserDetails retrieved from save's user found by provided values */ - fun findByName(username: String) = blockingToMono { - userRepository.findByName(username) - } + fun findByName(username: String) = userRepository.findByName(username) /** * @param name @@ -54,9 +50,8 @@ class UserDetailsService( * @param source source (where the user identity is coming from) * @return spring's UserDetails retrieved from save's user found by provided values */ - fun findByOriginalLogin(username: String, source: String) = blockingToMono { - originalLoginRepository.findByNameAndSource(username, source)?.user - } + fun findByOriginalLogin(username: String, source: String) = + originalLoginRepository.findByNameAndSource(username, source)?.user /** * We change the version just to work-around the caching on the frontend @@ -147,13 +142,32 @@ class UserDetailsService( } } + /** + * @param source + * @param name + * @return existed [User] or a new one + */ + @Transactional + fun saveNewUserIfRequired(source: String, name: String): User = + originalLoginRepository.findByNameAndSource(name, source) + ?.user + ?.also { + log.debug("User $name ($source) is already present in the DB") + } + ?: run { + log.info { + "Saving user $name ($source) with authorities $roleForNewUser to the DB" + } + saveNewUser(name).also { savedUser -> + addSource(savedUser, name, source) + } + } + /** * @param userNameCandidate - * @param userRole * @return created [User] */ - @Transactional - fun saveNewUser(userNameCandidate: String, userRole: String): User { + private fun saveNewUser(userNameCandidate: String): User { val existedUser = userRepository.findByName(userNameCandidate) val name = existedUser?.let { val prefix = "$userNameCandidate$UNIQUE_NAME_SEPARATOR" @@ -171,7 +185,7 @@ class UserDetailsService( User( name = name, password = null, - role = userRole, + role = roleForNewUser, status = UserStatus.CREATED, ) ) @@ -265,6 +279,8 @@ class UserDetailsService( } companion object { + private val log: Logger = getLogger() private const val UNIQUE_NAME_SEPARATOR = "_" + private val roleForNewUser = Role.VIEWER.asSpringSecurityRole() } } diff --git a/save-cloud-common/src/commonMain/kotlin/com/saveourtool/save/utils/Constants.kt b/save-cloud-common/src/commonMain/kotlin/com/saveourtool/save/utils/Constants.kt index 46702ec8f1..0992634779 100644 --- a/save-cloud-common/src/commonMain/kotlin/com/saveourtool/save/utils/Constants.kt +++ b/save-cloud-common/src/commonMain/kotlin/com/saveourtool/save/utils/Constants.kt @@ -73,6 +73,11 @@ const val AUTHORIZATION_NAME = "X-Authorization-Name" */ const val AUTHORIZATION_ROLES = "X-Authorization-Roles" +/** + * An attribute to store info about save user + */ +const val SAVE_USER_DETAILS_ATTIBUTE = "save-user-details" + /** * Default time to execute setup.sh */