fix(api): endpoint for books in readlist is not filtered properly

if the user does not have rights on all libraries
the returned list of books could contain inaccessible books
This commit is contained in:
Gauthier Roebroeck 2021-01-06 11:50:53 +08:00
parent dd81854c3d
commit cdca78b38f
4 changed files with 68 additions and 6 deletions

View file

@ -63,21 +63,33 @@ class BookDtoDao(
override fun findAll(search: BookSearchWithReadProgress, userId: String, pageable: Pageable): Page<BookDto> {
val conditions = search.toCondition()
return findAll(conditions, userId, pageable, search.toJoinConditions())
return findAll(conditions, userId, pageable, search.toJoinConditions(), null)
}
override fun findByReadListId(readListId: String, userId: String, pageable: Pageable): Page<BookDto> {
override fun findByReadListId(
readListId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,
pageable: Pageable
): Page<BookDto> {
val conditions = rlb.READLIST_ID.eq(readListId)
return findAll(conditions, userId, pageable, JoinConditions(selectReadListNumber = true))
return findAll(conditions, userId, pageable, JoinConditions(selectReadListNumber = true), filterOnLibraryIds)
}
private fun findAll(conditions: Condition, userId: String, pageable: Pageable, joinConditions: JoinConditions = JoinConditions()): Page<BookDto> {
private fun findAll(
conditions: Condition,
userId: String,
pageable: Pageable,
joinConditions: JoinConditions = JoinConditions(),
filterOnLibraryIds: Collection<String>?,
): Page<BookDto> {
val count = dsl.selectDistinct(b.ID)
.from(b)
.leftJoin(m).on(b.ID.eq(m.BOOK_ID))
.leftJoin(d).on(b.ID.eq(d.BOOK_ID))
.leftJoin(r).on(b.ID.eq(r.BOOK_ID)).and(readProgressCondition(userId))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.apply { if (joinConditions.tag) leftJoin(bt).on(b.ID.eq(bt.BOOK_ID)) }
.apply { if (joinConditions.selectReadListNumber) leftJoin(rlb).on(b.ID.eq(rlb.BOOK_ID)) }
.where(conditions)
@ -89,6 +101,7 @@ class BookDtoDao(
val dtos = selectBase(userId, joinConditions)
.where(conditions)
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.orderBy(orderBy)
.apply { if (pageable.isPaged) limit(pageable.pageSize).offset(pageable.offset) }
.fetchAndMap()

View file

@ -166,7 +166,12 @@ class ReadListController(
sort
)
bookDtoRepository.findByReadListId(readList.id, principal.user.id, pageRequest)
bookDtoRepository.findByReadListId(
readList.id,
principal.user.id,
principal.user.getAuthorizedLibraryIds(null),
pageRequest
)
.map { it.restrictUrl(!principal.user.roleAdmin) }
} ?: throw ResponseStatusException(HttpStatus.NOT_FOUND)
}

View file

@ -7,7 +7,13 @@ import org.springframework.data.domain.Pageable
interface BookDtoRepository {
fun findAll(search: BookSearchWithReadProgress, userId: String, pageable: Pageable): Page<BookDto>
fun findByReadListId(readListId: String, userId: String, pageable: Pageable): Page<BookDto>
fun findByReadListId(
readListId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,
pageable: Pageable
): Page<BookDto>
fun findByIdOrNull(bookId: String, userId: String): BookDto?
fun findPreviousInSeries(bookId: String, userId: String): BookDto?
fun findNextInSeries(bookId: String, userId: String): BookDto?

View file

@ -164,6 +164,44 @@ class ReadListControllerTest(
}
}
@Nested
inner class GetBooksAndFilter {
@Test
@WithMockCustomUser
fun `given user with access to all libraries when getting books from single read list then it is not filtered`() {
makeReadLists()
mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books")
.andExpect {
status { isOk() }
jsonPath("$.content.length()") { value(10) }
}
}
@Test
@WithMockCustomUser(sharedAllLibraries = false, sharedLibraries = ["1"])
fun `given user with access to a single library when getting books from single read list with items from 2 libraries then it is filtered`() {
makeReadLists()
mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books")
.andExpect {
status { isOk() }
jsonPath("$.content.length()") { value(5) }
}
}
@Test
@WithMockCustomUser(sharedAllLibraries = false, sharedLibraries = ["1"])
fun `given user with access to a single library when getting books from single read list from another library then return not found`() {
makeReadLists()
mockMvc.get("/api/v1/readlists/${rlLib2.id}/books")
.andExpect {
status { isNotFound() }
}
}
}
@Nested
inner class Creation {
@Test