Skip to content

Commit

Permalink
Use authentication to update User (#2632)
Browse files Browse the repository at this point in the history
- use `authentication.userId()` instead of `UserInfo.name` to get `User` to update it
- removed `UserInfo.oldName`
- removed `HACKER`
  • Loading branch information
nulls authored Oct 2, 2023
1 parent 284f097 commit 69b20d8
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 94 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

package com.saveourtool.save.gateway.security

import com.saveourtool.save.authservice.utils.username
import com.saveourtool.save.gateway.config.ConfigurationProperties
import com.saveourtool.save.gateway.service.BackendService
import com.saveourtool.save.gateway.utils.StoringServerAuthenticationSuccessHandler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import com.saveourtool.save.backend.service.UserDetailsService
import com.saveourtool.save.configs.RequiresAuthorizationSourceHeader
import com.saveourtool.save.domain.Role
import com.saveourtool.save.domain.UserSaveStatus
import com.saveourtool.save.entities.User
import com.saveourtool.save.info.UserInfo
import com.saveourtool.save.info.UserStatus
import com.saveourtool.save.utils.*
Expand All @@ -18,6 +17,7 @@ import io.swagger.v3.oas.annotations.Parameters
import io.swagger.v3.oas.annotations.enums.ParameterIn
import io.swagger.v3.oas.annotations.responses.ApiResponse
import org.springframework.data.domain.Pageable
import org.springframework.data.repository.findByIdOrNull

import org.springframework.http.HttpStatus
import org.springframework.http.ResponseEntity
Expand Down Expand Up @@ -109,36 +109,38 @@ class UsersDetailsController(
UserSaveStatus.INVALID_NAME.message
}
.blockingMap {
val user: User = userRepository.findByName(newUserInfo.oldName ?: newUserInfo.name).orNotFound()
userRepository.findByIdOrNull(authentication.userId())
}
.switchIfEmptyToNotFound {
"User with id ${authentication.userId()} not found in database"
}
.blockingMap { user ->
val oldName = user.name.takeUnless { it == newUserInfo.name }
val oldStatus = user.status
if (user.id == authentication.userId()) {
val newStatus = when (oldStatus) {
UserStatus.CREATED -> UserStatus.NOT_APPROVED
UserStatus.ACTIVE -> UserStatus.ACTIVE
else -> null
}
newStatus?.let {
userDetailsService.saveUser(
user.apply {
name = newUserInfo.name
email = newUserInfo.email
company = newUserInfo.company
location = newUserInfo.location
gitHub = newUserInfo.gitHub
linkedin = newUserInfo.linkedin
twitter = newUserInfo.twitter
status = newStatus
website = newUserInfo.website
realName = newUserInfo.realName
freeText = newUserInfo.freeText
},
newUserInfo.oldName,
oldStatus
)
} ?: UserSaveStatus.FORBIDDEN
} else {
UserSaveStatus.HACKER
val newStatus = when (oldStatus) {
UserStatus.CREATED -> UserStatus.NOT_APPROVED
UserStatus.ACTIVE -> UserStatus.ACTIVE
else -> null
}
newStatus?.let {
userDetailsService.saveUser(
user.apply {
name = newUserInfo.name
email = newUserInfo.email
company = newUserInfo.company
location = newUserInfo.location
gitHub = newUserInfo.gitHub
linkedin = newUserInfo.linkedin
twitter = newUserInfo.twitter
status = newStatus
website = newUserInfo.website
realName = newUserInfo.realName
freeText = newUserInfo.freeText
},
oldName,
oldStatus,
)
} ?: UserSaveStatus.FORBIDDEN
}
.filter { status -> status == UserSaveStatus.UPDATE }
.switchIfEmptyToResponseException(HttpStatus.CONFLICT) {
Expand Down Expand Up @@ -192,14 +194,13 @@ class UsersDetailsController(
): Mono<StringResponse> = blockingToMono {
userDetailsService.deleteUser(userName, authentication)
}
.filter { status ->
status == UserSaveStatus.DELETED
}
.switchIfEmptyToResponseException(HttpStatus.CONFLICT) {
UserSaveStatus.HACKER.message
}
.map { status ->
ResponseEntity.ok(status.message)
when (status) {
UserSaveStatus.DELETED ->
ResponseEntity.ok(status.message)
else ->
ResponseEntity.status(HttpStatus.CONFLICT).body(status.message)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import com.saveourtool.save.entities.User
import com.saveourtool.save.info.UserStatus
import com.saveourtool.save.utils.*
import org.slf4j.Logger
import org.springframework.data.repository.findByIdOrNull

import org.springframework.security.core.Authentication
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional
import reactor.core.scheduler.Schedulers
import java.lang.IllegalArgumentException

import java.util.*
Expand Down Expand Up @@ -100,12 +102,10 @@ class UserDetailsService(
* @param authentication
* @return global [Role] of authenticated user
*/
fun getGlobalRole(authentication: Authentication): Role = authentication.authorities
.map { grantedAuthority ->
Role.values().find { role -> role.asSpringSecurityRole() == grantedAuthority.authority }
fun getGlobalRole(authentication: Authentication): Role = userRepository.findByIdOrNull(authentication.userId())
?.let { user ->
Role.values().find { role -> role.asSpringSecurityRole() == user.role }
}
.sortedBy { it?.priority }
.lastOrNull()
?: Role.VIEWER

/**
Expand Down Expand Up @@ -217,31 +217,31 @@ class UserDetailsService(
name: String,
authentication: Authentication,
): UserSaveStatus {
val user: User = userRepository.findByName(name).orNotFound()
val newName = "Deleted-${user.id}"
if (user.id == authentication.userId()) {
userRepository.deleteHighLevelName(user.name)
userRepository.saveHighLevelName(newName)
userRepository.save(user.apply {
this.name = newName
this.status = UserStatus.DELETED
this.avatar = null
this.company = null
this.twitter = null
this.email = null
this.gitHub = null
this.linkedin = null
this.location = null
})
} else {
return UserSaveStatus.HACKER
val user: User = userRepository.findByIdOrNull(authentication.userId()).orNotFound {
"User with id ${authentication.userId()} not found in database"
}
val newName = "Deleted-${user.id}"
userRepository.deleteHighLevelName(user.name)
userRepository.saveHighLevelName(newName)
userRepository.save(user.apply {
this.name = newName
this.status = UserStatus.DELETED
this.avatar = null
this.company = null
this.twitter = null
this.email = null
this.gitHub = null
this.linkedin = null
this.location = null
})

val avatarKey = AvatarKey(
AvatarType.USER,
name,
)
avatarStorage.delete(avatarKey)
.subscribeOn(Schedulers.boundedElastic())
.subscribe()

originalLoginRepository.deleteByUserId(user.requiredId())
lnkUserProjectRepository.deleteByUserId(user.requiredId())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class UsersDetailsControllerTest {

val newUserInfo = UserInfo(
name = "Kuleshov",
oldName = "user",
email = "[email protected]",
company = "Example",
)
Expand Down Expand Up @@ -87,7 +86,6 @@ class UsersDetailsControllerTest {
val newUserInfo = UserInfo(
id = 1,
name = "admin",
oldName = null,
email = "[email protected]",
company = "Example Company",
)
Expand All @@ -109,15 +107,14 @@ class UsersDetailsControllerTest {
name = "admin",
email = "[email protected]",
company = "Example",
oldName = "admin"
)

webClient.post()
.uri("/api/$v1/users/save")
.bodyValue(newUserInfo)
.exchange()
.expectStatus()
.isEqualTo(HttpStatus.CONFLICT)
.isEqualTo(HttpStatus.OK)
}

@Test
Expand All @@ -127,7 +124,6 @@ class UsersDetailsControllerTest {

val newUserInfo = UserInfo(
name = "JohnDoe",
oldName = "admin",
email = "[email protected]",
company = "Example",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package com.saveourtool.save.backend.utils

import com.saveourtool.save.authservice.utils.SaveUserDetails
import com.saveourtool.save.authservice.utils.status
import com.saveourtool.save.info.UserStatus
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken
import org.springframework.security.core.context.SecurityContextHolder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ enum class UserSaveStatus(val message: String) {
*/
FORBIDDEN("You are trying to update user, while it is not yet active"),

/**
* currentUser.id != changedUser.id
*/
HACKER("You are trying to update other user that is not you"),

/**
* User name longer than [NAMING_MAX_LENGTH] characters
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import kotlinx.serialization.Serializable
* @property globalRole
* @property id
* @property status
* @property oldName is always null except for the process of renaming the user.
* @property originalLogins
* @property rating
* @property website
Expand All @@ -37,7 +36,6 @@ import kotlinx.serialization.Serializable
data class UserInfo(
val name: String,
val id: Long? = null,
val oldName: String? = null,
val originalLogins: Map<String, String> = emptyMap(),
val projects: Map<String, Role> = emptyMap(),
val organizations: Map<String, Role> = emptyMap(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ fun <T> Mono<T>.switchIfErrorToConflict(
* @see ResponseSpec.blockingToBodilessEntity
* @see BlockingBridge
*/
fun <T : Any, R : Any> Mono<T>.blockingMap(function: (T) -> R): Mono<R> = flatMap { value ->
fun <T : Any, R : Any> Mono<T>.blockingMap(function: (T) -> R?): Mono<R> = flatMap { value ->
blockingToMono { function(value) }
}

Expand Down
31 changes: 17 additions & 14 deletions save-frontend/src/main/kotlin/com/saveourtool/save/frontend/App.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

package com.saveourtool.save.frontend

import com.saveourtool.save.*
import com.saveourtool.save.domain.Role
import com.saveourtool.save.frontend.components.*
import com.saveourtool.save.frontend.components.basic.cookieBanner
Expand All @@ -17,10 +16,10 @@ import com.saveourtool.save.frontend.utils.*
import com.saveourtool.save.info.UserInfo
import com.saveourtool.save.validation.FrontendRoutes

import org.w3c.fetch.Response
import react.*
import react.dom.client.createRoot
import react.dom.html.ReactHTML.div
import react.router.*
import react.router.dom.BrowserRouter
import web.cssom.ClassName
import web.dom.document
Expand All @@ -44,19 +43,17 @@ val App: VFC = FC {
jsonHeaders,
loadingHandler = ::loadingHandler,
responseHandler = ::noopResponseHandler
).run {
val responseText = text().await()
if (!ok || responseText == "null") null else responseText
}
).validTextOrNull()

val globalRole: Role? = get(
"$apiUrl/users/global-role",
jsonHeaders,
loadingHandler = ::loadingHandler,
responseHandler = ::noopResponseHandler
).run {
val responseText = text().await()
if (!ok || responseText == "null") null else Json.decodeFromString(responseText)
val globalRole: Role? = userName?.let {
get(
"$apiUrl/users/global-role",
jsonHeaders,
loadingHandler = ::loadingHandler,
responseHandler = ::noopResponseHandler
)
.validTextOrNull()
?.let { Json.decodeFromString(it) }
}

val user: UserInfo? = userName?.let {
Expand Down Expand Up @@ -97,6 +94,12 @@ val App: VFC = FC {
}
}

private suspend fun Response.validTextOrNull(): String? = text()
.await()
.takeIf {
ok && it.isNotEmpty() && it != "null"
}

fun main() {
/* Workaround for issue: https://youtrack.jetbrains.com/issue/KT-31888 */
@Suppress("UnsafeCastFromDynamic")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ val registrationView: FC<RegistrationProps> = FC { props ->

val saveUser = useDeferredRequest {
val newUserInfo = userInfo.copy(
oldName = props.userInfo?.name!!,
status = UserStatus.ACTIVE,
)
val response = post(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ data class SettingsInputFields(
val newName = this.userName.value?.trim()
return userInfo.copy(
name = newName ?: userInfo.name,
// `oldName` is not saved into database, basically it's just a flag for
// backend to understand that name was or wasn't changed on the frontend
// need to pass `null` to backend if the field
// if it is passed as null, we will use this thing inside a controller
oldName = newName?.let { userInfo.name },
email = this.userEmail.value?.trim() ?: userInfo.email,
company = this.company.value?.trim() ?: userInfo.company,
location = this.location.value?.trim() ?: userInfo.location,
Expand Down

0 comments on commit 69b20d8

Please sign in to comment.