Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ class ReadingRecordController(
return ResponseEntity.status(HttpStatus.CREATED).body(response)
}

@GetMapping("/detail/{readingRecordId}")
override fun getReadingRecordDetail(
@AuthenticationPrincipal userId: UUID,
@PathVariable readingRecordId: UUID
): ResponseEntity<ReadingRecordResponse> {
val response = readingRecordUseCase.getReadingRecordDetail(
userId = userId,
readingRecordId = readingRecordId
)
return ResponseEntity.ok(response)
}

@GetMapping("/{userBookId}")
override fun getReadingRecords(
@AuthenticationPrincipal userId: UUID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,30 @@ interface ReadingRecordControllerApi {
@Valid @RequestBody @Parameter(description = "독서 기록 생성 요청 객체") request: CreateReadingRecordRequest
): ResponseEntity<ReadingRecordResponse>

@Operation(
summary = "독서 기록 상세 조회",
description = "독서 기록 ID로 독서 기록 상세 정보를 조회합니다."
)
@ApiResponses(
value = [
ApiResponse(
responseCode = "200",
description = "독서 기록 상세 조회 성공",
content = [Content(schema = Schema(implementation = ReadingRecordResponse::class))]
),
ApiResponse(
responseCode = "404",
description = "사용자 또는 독서 기록을 찾을 수 없음",
content = [Content(schema = Schema(implementation = ErrorResponse::class))]
)
]
)
@GetMapping("/detail/{readingRecordId}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

API 경로 구조에 대한 제안사항입니다.

현재 /detail/{readingRecordId} 경로는 기능적으로 올바르지만, RESTful 설계 관점에서 /{readingRecordId}만으로도 충분할 수 있습니다. 다른 리소스와의 일관성을 고려해보세요.

-@GetMapping("/detail/{readingRecordId}")
+@GetMapping("/{readingRecordId}")

하지만 기존 /userBookId 엔드포인트와의 충돌을 피하기 위해 현재 구조가 더 명확할 수 있습니다.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@GetMapping("/detail/{readingRecordId}")
@GetMapping("/{readingRecordId}")
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt
at line 81, the API endpoint uses the path "/detail/{readingRecordId}" which is
functionally correct but could be simplified to "/{readingRecordId}" for better
RESTful design consistency. Review other endpoints like "/userBookId" to ensure
no conflicts arise, and if safe, rename the mapping to just "/{readingRecordId}"
to align with RESTful conventions and improve path clarity.

fun getReadingRecordDetail(
@AuthenticationPrincipal @Parameter(description = "인증된 사용자 ID") userId: UUID,
@PathVariable @Parameter(description = "조회할 독서 기록 ID") readingRecordId: UUID
): ResponseEntity<ReadingRecordResponse>

@Operation(
summary = "독서 기록 목록 조회",
description = "사용자의 책에 대한 독서 기록을 페이징하여 조회합니다. 정렬은 페이지 번호 또는 최신 등록순으로 가능합니다."
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.yapp.apis.readingrecord.exception

import org.springframework.http.HttpStatus
import org.yapp.globalutils.exception.BaseErrorCode

enum class ReadingRecordErrorCode(
private val status: HttpStatus,
private val code: String,
private val message: String
) : BaseErrorCode {
READING_RECORD_NOT_FOUND(HttpStatus.NOT_FOUND, "READING_RECORD_001", "독서 기록을 찾을 수 없습니다.");

override fun getHttpStatus(): HttpStatus = status
override fun getCode(): String = code
override fun getMessage(): String = message
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package org.yapp.apis.readingrecord.exception

import org.yapp.globalutils.exception.CommonException

class ReadingRecordNotFoundException(
errorCode: ReadingRecordErrorCode,
message: String? = null
) : CommonException(errorCode, message)
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import org.springframework.stereotype.Service
import org.yapp.apis.book.service.UserBookService
import org.yapp.apis.readingrecord.dto.request.CreateReadingRecordRequest
import org.yapp.apis.readingrecord.dto.response.ReadingRecordResponse
import org.yapp.apis.readingrecord.exception.ReadingRecordErrorCode
import org.yapp.apis.readingrecord.exception.ReadingRecordNotFoundException
import org.yapp.domain.book.BookDomainService
import org.yapp.domain.readingrecord.ReadingRecordDomainService
import org.yapp.domain.readingrecord.ReadingRecordSortType
import java.util.*
Expand Down Expand Up @@ -35,6 +38,20 @@ class ReadingRecordService(
return ReadingRecordResponse.from(readingRecordInfoVO)
}

fun getReadingRecordDetail(
userId: UUID,
readingRecordId: UUID
): ReadingRecordResponse {
val readingRecordInfoVO = readingRecordDomainService.findReadingRecordById(readingRecordId)
?: throw ReadingRecordNotFoundException(
ReadingRecordErrorCode.READING_RECORD_NOT_FOUND,
"Reading record not found with id: $readingRecordId"
)

userBookService.validateUserBookExists(userId, readingRecordInfoVO.userBookId.value)

return ReadingRecordResponse.from(readingRecordInfoVO)
}
Comment on lines +41 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

보안 취약점: 인가되지 않은 사용자에게 정보 누출 가능성이 있습니다.

현재 구현에서는 독서 기록 존재 여부를 먼저 확인한 후 사용자 권한을 검증하는데, 이는 권한이 없는 사용자에게도 특정 ID의 독서 기록 존재 여부를 알려줄 수 있습니다.

다음과 같이 수정하여 보안을 강화하는 것을 권장합니다:

 fun getReadingRecordDetail(
     userId: UUID,
     readingRecordId: UUID
 ): ReadingRecordResponse {
     val readingRecordInfoVO = readingRecordDomainService.findReadingRecordById(readingRecordId)
         ?: throw ReadingRecordNotFoundException(
             ReadingRecordErrorCode.READING_RECORD_NOT_FOUND,
             "Reading record not found with id: $readingRecordId"
         )

-    userBookService.validateUserBookExists(userId, readingRecordInfoVO.userBookId.value)
+    // 사용자 권한 검증을 먼저 수행하여 정보 누출 방지
+    try {
+        userBookService.validateUserBookExists(userId, readingRecordInfoVO.userBookId.value)
+    } catch (e: Exception) {
+        // 권한이 없는 경우에도 동일한 예외를 던져 정보 누출 방지
+        throw ReadingRecordNotFoundException(
+            ReadingRecordErrorCode.READING_RECORD_NOT_FOUND,
+            "Reading record not found with id: $readingRecordId"
+        )
+    }

     return ReadingRecordResponse.from(readingRecordInfoVO)
 }
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt
around lines 43 to 56, the current code checks for the existence of the reading
record before validating user authorization, which can leak information about
the record's existence to unauthorized users. To fix this, first call
userBookService.validateUserBookExists with the userId and userBookId to verify
the user's permission, then retrieve the reading record by ID. This way,
unauthorized users cannot infer the existence of records they shouldn't access.


fun getReadingRecordsByDynamicCondition(
userBookId: UUID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ class ReadingRecordUseCase(
)
}

fun getReadingRecordDetail(
userId: UUID,
readingRecordId: UUID
): ReadingRecordResponse {
userAuthService.validateUserExists(userId)

return readingRecordService.getReadingRecordDetail(
userId = userId,
readingRecordId = readingRecordId
)
}
Comment on lines +38 to +48
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

보안 취약점: 사용자 권한 검증이 누락되었습니다.

현재 구현에서는 사용자 존재 여부만 확인하고 있어, 다른 사용자의 독서기록에 접근할 수 있는 보안 취약점이 있습니다. 독서기록 소유권 검증이 필요합니다.

다음과 같이 수정하여 보안을 강화하세요:

 fun getReadingRecordDetail(
     userId: UUID,
     readingRecordId: UUID
 ): ReadingRecordResponse {
     userAuthService.validateUserExists(userId)
+    
+    // 독서기록 소유권 검증 추가 필요
+    // readingRecordService에서 userId와 readingRecord의 소유자가 일치하는지 확인
 
     return readingRecordService.getReadingRecordDetail(
         userId = userId,
         readingRecordId = readingRecordId
     )
 }

또는 service layer에서 소유권 검증이 이루어지고 있는지 확인해주세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun getReadingRecordDetail(
userId: UUID,
readingRecordId: UUID
): ReadingRecordResponse {
userAuthService.validateUserExists(userId)
return readingRecordService.getReadingRecordDetail(
userId = userId,
readingRecordId = readingRecordId
)
}
fun getReadingRecordDetail(
userId: UUID,
readingRecordId: UUID
): ReadingRecordResponse {
userAuthService.validateUserExists(userId)
// 독서기록 소유권 검증 추가 필요
// readingRecordService에서 userId와 readingRecord의 소유자가 일치하는지 확인
return readingRecordService.getReadingRecordDetail(
userId = userId,
readingRecordId = readingRecordId
)
}
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt
between lines 45 and 55, the method getReadingRecordDetail only validates that
the user exists but does not verify if the user owns the requested reading
record, creating a security vulnerability. To fix this, add a check to confirm
that the reading record belongs to the user before returning the detail, either
by adding ownership validation in this use case method or ensuring that the
service layer method getReadingRecordDetail performs this ownership verification
and throws an error if the user is unauthorized.


fun getReadingRecordsByUserBookId(
userId: UUID,
userBookId: UUID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,27 @@ class ReadingRecordDomainService(
)
}

fun findReadingRecordById(readingRecordId: UUID): ReadingRecordInfoVO? {
val readingRecord = readingRecordRepository.findById(readingRecordId) ?: return null

return buildReadingRecordInfoVO(readingRecord)
}

private fun buildReadingRecordInfoVO(readingRecord: ReadingRecord): ReadingRecordInfoVO {
val readingRecordTags = readingRecordTagRepository.findByReadingRecordId(readingRecord.id.value)
val tagIds = readingRecordTags.map { it.tagId.value }
val tags = tagRepository.findByIds(tagIds)
val userBook = userBookRepository.findById(readingRecord.userBookId.value)

return ReadingRecordInfoVO.newInstance(
readingRecord = readingRecord,
emotionTags = tags.map { it.name },
bookTitle = userBook?.title,
bookPublisher = userBook?.publisher,
bookCoverImageUrl = userBook?.coverImageUrl
)
}

fun findReadingRecordsByDynamicCondition(
userBookId: UUID,
sort: ReadingRecordSortType?,
Expand Down