From 9cd8f037560f661e59e8e0b5b90f02efa71df063 Mon Sep 17 00:00:00 2001 From: Cavin Date: Wed, 1 Jul 2026 14:56:46 +0300 Subject: [PATCH] fix: role hierarchy security when listing users --- .../common/exception/DomainExceptions.kt | 10 +++-- .../domain/repository/UserRepository.kt | 6 +-- .../devcavin/backend/service/UserService.kt | 44 +++++++++++-------- .../web/error/GlobalExceptionHandler.kt | 22 +++++++++- backend/src/main/resources/application.yaml | 6 +++ .../db/migration/V7__seed_super_admin.sql | 12 ++--- compose.yaml | 2 +- 7 files changed, 69 insertions(+), 33 deletions(-) diff --git a/backend/src/main/kotlin/io/github/devcavin/backend/common/exception/DomainExceptions.kt b/backend/src/main/kotlin/io/github/devcavin/backend/common/exception/DomainExceptions.kt index 99b2cf1..ef91836 100644 --- a/backend/src/main/kotlin/io/github/devcavin/backend/common/exception/DomainExceptions.kt +++ b/backend/src/main/kotlin/io/github/devcavin/backend/common/exception/DomainExceptions.kt @@ -1,14 +1,16 @@ package io.github.devcavin.backend.common.exception +import org.springframework.security.authentication.BadCredentialsException + // base type sealed class DomainException(message: String) : RuntimeException(message) // 401 — bad credentials or invalid/expired tokens sealed class UnauthorizedException(message: String): DomainException(message) -class InvalidCredentialsException : UnauthorizedException("Invalid username or password") +class InvalidCredentialsException : BadCredentialsException("Invalid username or password") -class InvalidRefreshTokenException : UnauthorizedException("Invalid or expired refresh token") +class InvalidRefreshTokenException : BadCredentialsException("Invalid or expired refresh token") // 403 - authenticated but !permitted class AccountDisabledException : DomainException("Account is disabled") @@ -20,4 +22,6 @@ class ResourceNotFoundException(resource: String, id: Any) : DomainException("Re class ConflictException(message: String) : DomainException(message) // 422 - semantically invalid request (e.g. checking out already checked out visitor) -class InvalidStateException(message: String) : DomainException(message) \ No newline at end of file +class InvalidStateException(message: String) : DomainException(message) + +class AccessDeniedException(message: String) : RuntimeException(message) \ No newline at end of file diff --git a/backend/src/main/kotlin/io/github/devcavin/backend/domain/repository/UserRepository.kt b/backend/src/main/kotlin/io/github/devcavin/backend/domain/repository/UserRepository.kt index 2bc04c5..cf2b07e 100644 --- a/backend/src/main/kotlin/io/github/devcavin/backend/domain/repository/UserRepository.kt +++ b/backend/src/main/kotlin/io/github/devcavin/backend/domain/repository/UserRepository.kt @@ -4,7 +4,7 @@ import io.github.devcavin.backend.domain.model.User import org.springframework.data.jpa.repository.JpaRepository import org.springframework.data.jpa.repository.Query import org.springframework.stereotype.Repository -import java.util.UUID +import java.util.* @Repository interface UserRepository : JpaRepository{ @@ -14,7 +14,7 @@ interface UserRepository : JpaRepository{ fun findAllBySiteIdAndIsActiveTrue(siteId: UUID): List @Query("SELECT DISTINCT u FROM User u JOIN FETCH u.role WHERE u.id = :id") - fun findByIdWithRole(id: UUID): User? + fun findByIdWithRole(id: UUID): Optional @Query("SELECT DISTINCT u FROM User u JOIN FETCH u.role WHERE u.email = :email") fun findByEmailWithRole(email: String): User? @@ -25,6 +25,6 @@ interface UserRepository : JpaRepository{ @Query("SELECT DISTINCT u FROM User u JOIN FETCH u.role WHERE u.site.id = :siteId") fun findAllBySiteIdWithRole(siteId: UUID): List - @Query("SELECT u FROM User u JOIN FETCH u.role JOIN FETCH u.site WHERE u.id = :id") + @Query("SELECT DISTINCT u FROM User u JOIN FETCH u.role JOIN FETCH u.site WHERE u.id = :id") fun findByIdWithRoleAndSite(id: UUID): User? } \ No newline at end of file diff --git a/backend/src/main/kotlin/io/github/devcavin/backend/service/UserService.kt b/backend/src/main/kotlin/io/github/devcavin/backend/service/UserService.kt index bf14208..b7252c9 100644 --- a/backend/src/main/kotlin/io/github/devcavin/backend/service/UserService.kt +++ b/backend/src/main/kotlin/io/github/devcavin/backend/service/UserService.kt @@ -13,9 +13,9 @@ import io.github.devcavin.backend.web.dto.user.CreateUserRequest import io.github.devcavin.backend.web.dto.user.UpdateUserRequest import io.github.devcavin.backend.web.dto.user.UserResponse import io.github.devcavin.backend.web.dto.user.toResponse -import org.springframework.security.access.AccessDeniedException import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.stereotype.Service +import io.github.devcavin.backend.common.exception.AccessDeniedException import org.springframework.transaction.annotation.Transactional import java.util.* @@ -50,22 +50,24 @@ class UserService( @Transactional(readOnly = true) fun getAll(requestedBy: User): List { - val roleName = requestedBy.role.name - val siteId = requestedBy.site.id!! - return when (roleName) { + return when (requestedBy.role.name) { "SUPER_ADMIN" -> userRepository.findAllWithRole().map { it.toResponse() } - else -> userRepository - .findAllBySiteIdWithRole(siteId) + "MANAGER" -> userRepository + .findAllBySiteIdWithRole(requestedBy.site.id!!) + .filter { it.role.name == "STAFF" } .map { it.toResponse() } + else -> emptyList() } } @Transactional(readOnly = true) fun getById(requestedBy: User, userId: UUID): UserResponse { - val user = userRepository.findById(userId) + val user = userRepository.findByIdWithRole(userId) .orElseThrow { ResourceNotFoundException("User", userId) } - enforceSiteBoundary(requestedBy, user) + + enforceVisibilityRules(requestedBy, user) + return user.toResponse() } @@ -78,7 +80,7 @@ class UserService( val target = userRepository.findById(userId) .orElseThrow { ResourceNotFoundException("User", userId) } - enforceSiteBoundary(requestedBy, target) + enforceVisibilityRules(requestedBy, target) enforceUpdateRules(requestedBy, target, request.roleName) if (request.email != target.email && @@ -105,7 +107,7 @@ class UserService( val target = userRepository.findById(userId) .orElseThrow { ResourceNotFoundException("User", userId) } - enforceSiteBoundary(requestedBy, target) + enforceVisibilityRules(requestedBy, target) enforceDeactivationRules(requestedBy, target) target.isActive = false @@ -116,7 +118,7 @@ class UserService( fun activate(requestedBy: User, userId: UUID): UserResponse { val target = userRepository.findById(userId) .orElseThrow { ResourceNotFoundException("User", userId) } - enforceSiteBoundary(requestedBy, target) + enforceVisibilityRules(requestedBy, target) target.isActive = true return userRepository.save(target).toResponse() } @@ -151,14 +153,6 @@ class UserService( } } - private fun enforceSiteBoundary(requestedBy: User, target: User) { - if (requestedBy.role.name != "SUPER_ADMIN" && - requestedBy.site.id != target.site.id - ) { - throw AccessDeniedException("User does not belong to your site") - } - } - private fun enforceDeactivationRules(requestedBy: User, target: User) { when (requestedBy.role.name) { "SUPER_ADMIN" -> Unit @@ -186,4 +180,16 @@ class UserService( else -> throw AccessDeniedException("Insufficient privilege to update users") } } + + private fun enforceVisibilityRules(requestedBy: User, target: User) { + if (requestedBy.role.name == "SUPER_ADMIN") return + + if (requestedBy.site.id != target.site.id) { + throw AccessDeniedException("User doesnt belong to your site") + } + + if (requestedBy.role.name == "MANAGER" && target.role.name != "STAFF") { + throw AccessDeniedException("Managers can only view staff accounts") + } + } } \ No newline at end of file diff --git a/backend/src/main/kotlin/io/github/devcavin/backend/web/error/GlobalExceptionHandler.kt b/backend/src/main/kotlin/io/github/devcavin/backend/web/error/GlobalExceptionHandler.kt index 61064ca..400d16e 100644 --- a/backend/src/main/kotlin/io/github/devcavin/backend/web/error/GlobalExceptionHandler.kt +++ b/backend/src/main/kotlin/io/github/devcavin/backend/web/error/GlobalExceptionHandler.kt @@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory import org.springframework.http.HttpStatus import org.springframework.http.ResponseEntity import org.springframework.security.authentication.BadCredentialsException +import org.springframework.security.authorization.AuthorizationDeniedException import org.springframework.web.bind.MethodArgumentNotValidException import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.RestControllerAdvice @@ -19,6 +20,11 @@ class GlobalExceptionHandler { return build(HttpStatus.UNAUTHORIZED, e.message ?: "Unauthorized", request) } + @ExceptionHandler(AccessDeniedException::class) + fun handleAccessDenied(e: AccessDeniedException, request: HttpServletRequest): ResponseEntity { + return build(HttpStatus.FORBIDDEN, e.message ?: "Forbidden", request) + } + @ExceptionHandler(AccountDisabledException::class) fun handleForbidden(e: AccountDisabledException, request: HttpServletRequest): ResponseEntity { return build(HttpStatus.FORBIDDEN, e.message ?: "Forbidden", request) @@ -54,7 +60,7 @@ class GlobalExceptionHandler { @ExceptionHandler(BadCredentialsException::class) fun handleBadCredentials(e: BadCredentialsException, request: HttpServletRequest): ResponseEntity { - return build(HttpStatus.UNAUTHORIZED, e.message ?: "Invalid username or password", request) + return build(HttpStatus.BAD_REQUEST, e.message ?: "Invalid username or password", request) } @ExceptionHandler(Exception::class) @@ -68,6 +74,20 @@ class GlobalExceptionHandler { ) } + @ExceptionHandler(AuthorizationDeniedException::class) + fun handleAuthorizationDenied( + e: AuthorizationDeniedException, + request: HttpServletRequest + ): ResponseEntity { + logger.warn("Authorization denied for {}: {}", request.requestURI, e.message) + + return build( + HttpStatus.FORBIDDEN, + "Access denied", + request + ) + } + private fun build( status: HttpStatus, diff --git a/backend/src/main/resources/application.yaml b/backend/src/main/resources/application.yaml index fca2e7f..f888654 100644 --- a/backend/src/main/resources/application.yaml +++ b/backend/src/main/resources/application.yaml @@ -27,6 +27,12 @@ spring: locations: classpath:db/migration baseline-on-migrate: false +management: + endpoints: + web: + exposure: + include: health + server: port: 8080 diff --git a/backend/src/main/resources/db/migration/V7__seed_super_admin.sql b/backend/src/main/resources/db/migration/V7__seed_super_admin.sql index 54167fc..2889e0f 100644 --- a/backend/src/main/resources/db/migration/V7__seed_super_admin.sql +++ b/backend/src/main/resources/db/migration/V7__seed_super_admin.sql @@ -1,17 +1,17 @@ INSERT INTO sites (id, name, location) VALUES ( - '00000000-0000-0000-0000-000000000001', + '794bdb3d-4074-4b02-8741-31dc33d912dd', 'Default Site', 'Set during onboarding' ); INSERT INTO users (id, name, email, password_hash, role_id, site_id) SELECT - '00000000-0000-0000-0000-000000000002', - 'Super Admin', - 'admin@gatelog.dev', - '$2b$10$NG7pE/TCZUCLwvs2RQYpDu1g8LFt1wfnjGqQznMkNSSPYyPZhMfBS', + '70dd0d7d-73ff-4930-b50c-a3b16592f518', + 'Admin', + 'admin@gatelog.app', + '$2b$10$S6FDrSOw75yL4g6DQZM26OHFKUjua6A0dZx3l/UlDh4Jo35Am8JPi', r.id, - '00000000-0000-0000-0000-000000000001' + '794bdb3d-4074-4b02-8741-31dc33d912dd' FROM roles r WHERE r.name = 'SUPER_ADMIN'; \ No newline at end of file diff --git a/compose.yaml b/compose.yaml index 73cf642..d398691 100644 --- a/compose.yaml +++ b/compose.yaml @@ -1,6 +1,6 @@ services: postgres: - image: postgres:15-alpine + image: postgres:16-alpine container_name: gatelog environment: POSTGRES_DB: gatelog