diff --git a/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDao.kt b/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDao.kt index 5e8453c5c..d74c162b0 100644 --- a/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDao.kt +++ b/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDao.kt @@ -12,16 +12,25 @@ class ClientSettingsDtoDao( private val g = Tables.CLIENT_SETTINGS_GLOBAL private val u = Tables.CLIENT_SETTINGS_USER - fun findAllGlobal(): Collection = - dsl.selectFrom(g) - .map { ClientSettingDto(it.key, it.value, it.allowUnauthorized, null) } + fun findAllGlobal(onlyUnauthorized: Boolean = false): Map = + dsl + .selectFrom(g) + .apply { if (onlyUnauthorized) where(g.ALLOW_UNAUTHORIZED.isTrue) } + .fetch() + .associate { it.key to ClientSettingDto(it.value, it.allowUnauthorized) } - fun findAllUser(userId: String): Collection = - dsl.selectFrom(u) + fun findAllUser(userId: String): Map = + dsl + .selectFrom(u) .where(u.USER_ID.eq(userId)) - .map { ClientSettingDto(it.key, it.value, null, it.userId) } + .fetch() + .associate { it.key to ClientSettingDto(it.value, null) } - fun saveGlobal(key: String, value: String, allowUnauthorized: Boolean) { + fun saveGlobal( + key: String, + value: String, + allowUnauthorized: Boolean, + ) { dsl .insertInto(g, g.KEY, g.VALUE, g.ALLOW_UNAUTHORIZED) .values(key, value, allowUnauthorized) @@ -30,7 +39,11 @@ class ClientSettingsDtoDao( .execute() } - fun saveForUser(userId: String, key: String, value: String) { + fun saveForUser( + userId: String, + key: String, + value: String, + ) { dsl .insertInto(u, u.USER_ID, u.KEY, u.VALUE) .values(userId, key, value) @@ -44,23 +57,27 @@ class ClientSettingsDtoDao( dsl.deleteFrom(u).execute() } - fun deleteGlobalByKey(key: String) { + fun deleteGlobalByKeys(keys: Collection) { dsl .deleteFrom(g) - .where(g.KEY.eq(key)) + .where(g.KEY.`in`(keys)) .execute() } - fun deleteByUserIdAndKey(userId: String, key: String) { + fun deleteByUserIdAndKeys( + userId: String, + keys: Collection, + ) { dsl .deleteFrom(u) - .where(u.KEY.eq(key)) + .where(u.KEY.`in`(keys)) .and(u.USER_ID.eq(userId)) .execute() } fun deleteByUserId(userId: String) { - dsl.deleteFrom(u) + dsl + .deleteFrom(u) .where(u.USER_ID.eq(userId)) .execute() } diff --git a/komga/src/main/kotlin/org/gotson/komga/infrastructure/security/SecurityConfiguration.kt b/komga/src/main/kotlin/org/gotson/komga/infrastructure/security/SecurityConfiguration.kt index c0597140e..7274dc4a8 100644 --- a/komga/src/main/kotlin/org/gotson/komga/infrastructure/security/SecurityConfiguration.kt +++ b/komga/src/main/kotlin/org/gotson/komga/infrastructure/security/SecurityConfiguration.kt @@ -89,7 +89,7 @@ class SecurityConfiguration( // used by webui "/api/v1/oauth2/providers", // used by webui, we check for authorization within the controller method directly and filter results from there - "/api/v1/client-settings/list", + "/api/v1/client-settings/global/list", // epub resources - fonts are always requested anonymously, so we check for authorization within the controller method directly "/api/v1/books/{bookId}/resource/**", // dynamic fonts diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsController.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsController.kt index 9738ebcba..3423d385e 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsController.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsController.kt @@ -9,8 +9,9 @@ import org.springframework.http.HttpStatus import org.springframework.http.MediaType import org.springframework.security.access.prepost.PreAuthorize import org.springframework.security.core.annotation.AuthenticationPrincipal +import org.springframework.web.bind.annotation.DeleteMapping import org.springframework.web.bind.annotation.GetMapping -import org.springframework.web.bind.annotation.PutMapping +import org.springframework.web.bind.annotation.PatchMapping import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.ResponseStatus @@ -21,48 +22,52 @@ import org.springframework.web.bind.annotation.RestController class ClientSettingsController( private val clientSettingsDtoDao: ClientSettingsDtoDao, ) { - - @GetMapping("list") - fun getSettings( + @GetMapping("global/list") + fun getGlobalSettings( @AuthenticationPrincipal principal: KomgaPrincipal?, - ): Collection { - if (principal == null) { - // return only global settings that allow unauthorized access - return clientSettingsDtoDao - .findAllGlobal() - .filter { it.allowUnauthorized == true } - .map { it.copy(allowUnauthorized = null) } - } + ): Map = clientSettingsDtoDao.findAllGlobal(principal == null) - // merge global and user settings - val global = clientSettingsDtoDao.findAllGlobal().associateBy { it.key } - val user = clientSettingsDtoDao.findAllUser(principal.user.id).associateBy { it.key } + @GetMapping("user/list") + fun getUserSettings( + @AuthenticationPrincipal principal: KomgaPrincipal, + ): Map = clientSettingsDtoDao.findAllUser(principal.user.id) - val settings = (global + user).values - .let { - // if the user is not admin, we sanitize the allowUnauthorized values - if (!principal.user.isAdmin) it.map { s -> s.copy(allowUnauthorized = null) } - else it - } - - return settings - } - - @PutMapping("global") + @PatchMapping("global") @ResponseStatus(HttpStatus.NO_CONTENT) @PreAuthorize("hasRole('ADMIN')") fun saveGlobalSetting( - @RequestBody newSetting: ClientSettingGlobalUpdateDto, + @RequestBody newSettings: Map, ) { - clientSettingsDtoDao.saveGlobal(newSetting.key, newSetting.value, newSetting.allowUnauthorized) + newSettings.forEach { (key, setting) -> + clientSettingsDtoDao.saveGlobal(key, setting.value, setting.allowUnauthorized) + } } - @PutMapping("user") + @PatchMapping("user") @ResponseStatus(HttpStatus.NO_CONTENT) fun saveUserSetting( @AuthenticationPrincipal principal: KomgaPrincipal, - @RequestBody newSetting: ClientSettingUserUpdateDto, + @RequestBody newSettings: Map, ) { - clientSettingsDtoDao.saveForUser(principal.user.id, newSetting.key, newSetting.value) + newSettings.forEach { (key, setting) -> + clientSettingsDtoDao.saveForUser(principal.user.id, key, setting.value) + } + } + + @DeleteMapping("global") + @ResponseStatus(HttpStatus.NO_CONTENT) + fun deleteGlobalSetting( + @RequestBody keysToDelete: Set, + ) { + clientSettingsDtoDao.deleteGlobalByKeys(keysToDelete) + } + + @DeleteMapping("user") + @ResponseStatus(HttpStatus.NO_CONTENT) + fun deleteGlobalSetting( + @AuthenticationPrincipal principal: KomgaPrincipal, + @RequestBody keysToDelete: Set, + ) { + clientSettingsDtoDao.deleteByUserIdAndKeys(principal.user.id, keysToDelete) } } diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/dto/ClientSettingDto.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/dto/ClientSettingDto.kt index 976def409..896b5e075 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/dto/ClientSettingDto.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/dto/ClientSettingDto.kt @@ -4,19 +4,15 @@ import com.fasterxml.jackson.annotation.JsonInclude @JsonInclude(JsonInclude.Include.NON_NULL) data class ClientSettingDto( - val key: String, val value: String, val allowUnauthorized: Boolean?, - val userId: String?, ) data class ClientSettingGlobalUpdateDto( - val key: String, val value: String, val allowUnauthorized: Boolean, ) data class ClientSettingUserUpdateDto( - val key: String, val value: String, ) diff --git a/komga/src/test/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDaoTest.kt b/komga/src/test/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDaoTest.kt index dd0c89102..742efbbf6 100644 --- a/komga/src/test/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDaoTest.kt +++ b/komga/src/test/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDaoTest.kt @@ -7,6 +7,7 @@ import org.gotson.komga.interfaces.api.rest.dto.ClientSettingDto import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest @@ -35,51 +36,87 @@ class ClientSettingsDtoDaoTest( userRepository.deleteAll() } - @Test - fun `when saving global setting then it is persisted`() { - clientSettingsDtoDao.saveGlobal("setting1", "value1", true) - clientSettingsDtoDao.saveGlobal("setting2", "value2", false) + @Nested + inner class Global { + @Test + fun `when saving global setting then it is persisted`() { + clientSettingsDtoDao.saveGlobal("setting1", "value1", true) + clientSettingsDtoDao.saveGlobal("setting2", "value2", false) - val fetch = clientSettingsDtoDao.findAllGlobal() + val fetch = clientSettingsDtoDao.findAllGlobal() - assertThat(fetch).containsExactlyInAnyOrder( - ClientSettingDto("setting1", "value1", true, null), - ClientSettingDto("setting2", "value2", false, null), - ) + assertThat(fetch).containsAllEntriesOf( + mapOf( + "setting1" to ClientSettingDto("value1", true), + "setting2" to ClientSettingDto("value2", false), + ), + ) + } + + @Test + fun `given existing global setting when saving global setting then it is updated`() { + clientSettingsDtoDao.saveGlobal("setting1", "value1", true) + clientSettingsDtoDao.saveGlobal("setting1", "updated", true) + val fetch = clientSettingsDtoDao.findAllGlobal() + + assertThat(fetch).containsAllEntriesOf( + mapOf( + "setting1" to ClientSettingDto("updated", true), + ), + ) + } + + @Test + fun `given existing global setting when deleting global setting then it is deleted`() { + clientSettingsDtoDao.saveGlobal("setting1", "value1", true) + + clientSettingsDtoDao.deleteGlobalByKeys(listOf("setting1")) + + val fetch = clientSettingsDtoDao.findAllGlobal() + + assertThat(fetch).isEmpty() + } } - @Test - fun `given existing global setting when saving global setting then it is updated`() { - clientSettingsDtoDao.saveGlobal("setting1", "value1", true) - clientSettingsDtoDao.saveGlobal("setting1", "updated", true) - val fetch = clientSettingsDtoDao.findAllGlobal() + @Nested + inner class User { + @Test + fun `when saving user setting then it is persisted`() { + clientSettingsDtoDao.saveForUser(user1.id, "setting1", "value1") + clientSettingsDtoDao.saveForUser(user2.id, "setting2", "value2") - assertThat(fetch).containsExactlyInAnyOrder( - ClientSettingDto("setting1", "updated", true, null), - ) - } + val fetch = clientSettingsDtoDao.findAllUser(user1.id) - @Test - fun `when saving user setting then it is persisted`() { - clientSettingsDtoDao.saveForUser(user1.id, "setting1", "value1") - clientSettingsDtoDao.saveForUser(user2.id, "setting2", "value2") + assertThat(fetch).containsAllEntriesOf( + mapOf( + "setting1" to ClientSettingDto("value1", null), + ), + ) + } - val fetch = clientSettingsDtoDao.findAllUser(user1.id) + @Test + fun `given existing user setting when saving user setting then it is updated`() { + clientSettingsDtoDao.saveForUser(user1.id, "setting1", "value1") + clientSettingsDtoDao.saveForUser(user1.id, "setting1", "updated") - assertThat(fetch).containsExactlyInAnyOrder( - ClientSettingDto("setting1", "value1", null, user1.id), - ) - } + val fetch = clientSettingsDtoDao.findAllUser(user1.id) - @Test - fun `given existing user setting when saving user setting then it is updated`() { - clientSettingsDtoDao.saveForUser(user1.id, "setting1", "value1") - clientSettingsDtoDao.saveForUser(user1.id, "setting1", "updated") + assertThat(fetch).containsAllEntriesOf( + mapOf( + "setting1" to ClientSettingDto("updated", null), + ), + ) + } - val fetch = clientSettingsDtoDao.findAllUser(user1.id) + @Test + fun `given existing user setting when deleting user setting then it is deleted`() { + clientSettingsDtoDao.saveForUser(user1.id, "setting1", "value1") - assertThat(fetch).containsExactlyInAnyOrder( - ClientSettingDto("setting1", "updated", null, user1.id), - ) + clientSettingsDtoDao.deleteByUserIdAndKeys(user1.id, listOf("setting1")) + + val fetch = clientSettingsDtoDao.findAllUser(user1.id) + + assertThat(fetch).isEmpty() + } } } diff --git a/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsControllerTest.kt b/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsControllerTest.kt index 22be0209e..cba606e53 100644 --- a/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsControllerTest.kt +++ b/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsControllerTest.kt @@ -15,7 +15,7 @@ import org.springframework.http.MediaType import org.springframework.security.test.context.support.WithAnonymousUser import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.get -import org.springframework.test.web.servlet.put +import org.springframework.test.web.servlet.patch @SpringBootTest @AutoConfigureMockMvc(printOnlyOnFailure = false) @@ -38,11 +38,12 @@ class ClientSettingsControllerTest( clientSettingsDtoDao.saveGlobal("restricted", "value", false) mockMvc - .get("/api/v1/client-settings/list") + .get("/api/v1/client-settings/global/list") .andExpect { status { isOk() } - jsonPath("$.length()") { value(1) } - jsonPath("$[0].allowUnauthorized") { doesNotExist() } + jsonPath("$.size()") { value(1) } + jsonPath("$.forall.value") { value("value") } + jsonPath("$.forall.allowUnauthorized") { value(true) } } } @@ -53,18 +54,20 @@ class ClientSettingsControllerTest( val jsonString = """ { - "key": "setting", - "value": "value", - "allowUnauthorized": false + "setting": { + "value": "value", + "allowUnauthorized": false + } } """.trimIndent() - mockMvc.put("/api/v1/client-settings/global") { - contentType = MediaType.APPLICATION_JSON - content = jsonString - }.andExpect { - status { isUnauthorized() } - } + mockMvc + .patch("/api/v1/client-settings/global") { + contentType = MediaType.APPLICATION_JSON + content = jsonString + }.andExpect { + status { isUnauthorized() } + } } } @@ -84,16 +87,19 @@ class ClientSettingsControllerTest( @Test @WithMockCustomUser - fun `given authenticated user when retrieving settings then returns all settings`() { + fun `given authenticated user when retrieving global settings then returns all settings`() { clientSettingsDtoDao.saveGlobal("forall", "value", true) clientSettingsDtoDao.saveGlobal("restricted", "value", false) mockMvc - .get("/api/v1/client-settings/list") + .get("/api/v1/client-settings/global/list") .andExpect { status { isOk() } - jsonPath("$.length()") { value(2) } - jsonPath("$[0].allowUnauthorized") { doesNotExist() } + jsonPath("$.size()") { value(2) } + jsonPath("$.forall.value") { value("value") } + jsonPath("$.forall.allowUnauthorized") { value(true) } + jsonPath("$.restricted.value") { value("value") } + jsonPath("$.restricted.allowUnauthorized") { value(false) } } } @@ -104,18 +110,20 @@ class ClientSettingsControllerTest( val jsonString = """ { - "key": "setting", - "value": "value", - "allowUnauthorized": false + "setting": { + "value": "value", + "allowUnauthorized": false + } } """.trimIndent() - mockMvc.put("/api/v1/client-settings/global") { - contentType = MediaType.APPLICATION_JSON - content = jsonString - }.andExpect { - status { isForbidden() } - } + mockMvc + .patch("/api/v1/client-settings/global") { + contentType = MediaType.APPLICATION_JSON + content = jsonString + }.andExpect { + status { isForbidden() } + } } @Test @@ -125,43 +133,26 @@ class ClientSettingsControllerTest( val jsonString = """ { - "key": "setting", - "value": "value" + "setting": { + "value": "value" + } } """.trimIndent() - mockMvc.put("/api/v1/client-settings/user") { - contentType = MediaType.APPLICATION_JSON - content = jsonString - }.andExpect { - status { isNoContent() } - } - mockMvc - .get("/api/v1/client-settings/list") - .andExpect { - status { isOk() } - jsonPath("$.length()") { value(1) } - jsonPath("$[0].key") { value("setting") } - jsonPath("$[0].value") { value("value") } - jsonPath("$[0].userId") { value("user1") } + .patch("/api/v1/client-settings/user") { + contentType = MediaType.APPLICATION_JSON + content = jsonString + }.andExpect { + status { isNoContent() } } - } - - @Test - @WithMockCustomUser(id = "user1") - fun `given non-admin user when retrieving settings then user settings take precedence over global settings`() { - clientSettingsDtoDao.saveGlobal("setting", "global", false) - clientSettingsDtoDao.saveForUser(user1.id, "setting", "local") mockMvc - .get("/api/v1/client-settings/list") + .get("/api/v1/client-settings/user/list") .andExpect { status { isOk() } - jsonPath("$.length()") { value(1) } - jsonPath("$[0].key") { value("setting") } - jsonPath("$[0].value") { value("local") } - jsonPath("$[0].userId") { value("user1") } + jsonPath("$.size()") { value(1) } + jsonPath("$.setting.value") { value("value") } } } } @@ -175,11 +166,14 @@ class ClientSettingsControllerTest( clientSettingsDtoDao.saveGlobal("restricted", "value", false) mockMvc - .get("/api/v1/client-settings/list") + .get("/api/v1/client-settings/global/list") .andExpect { status { isOk() } - jsonPath("$.length()") { value(2) } - jsonPath("$[0].allowUnauthorized") { value(true) } + jsonPath("$.size()") { value(2) } + jsonPath("$.forall.value") { value("value") } + jsonPath("$.forall.allowUnauthorized") { value(true) } + jsonPath("$.restricted.value") { value("value") } + jsonPath("$.restricted.allowUnauthorized") { value(false) } } } @@ -190,27 +184,28 @@ class ClientSettingsControllerTest( val jsonString = """ { - "key": "setting", - "value": "value", - "allowUnauthorized": false + "setting": { + "value": "value", + "allowUnauthorized": false + } } """.trimIndent() - mockMvc.put("/api/v1/client-settings/global") { - contentType = MediaType.APPLICATION_JSON - content = jsonString - }.andExpect { - status { isNoContent() } - } + mockMvc + .patch("/api/v1/client-settings/global") { + contentType = MediaType.APPLICATION_JSON + content = jsonString + }.andExpect { + status { isNoContent() } + } mockMvc - .get("/api/v1/client-settings/list") + .get("/api/v1/client-settings/global/list") .andExpect { status { isOk() } - jsonPath("$.length()") { value(1) } - jsonPath("$[0].key") { value("setting") } - jsonPath("$[0].value") { value("value") } - jsonPath("$[0].allowUnauthorized") { value(false) } + jsonPath("$.size()") { value(1) } + jsonPath("$.setting.value") { value("value") } + jsonPath("$.setting.allowUnauthorized") { value(false) } } } }