Backup_Repos/komga
1
0
Fork 0
mirror of https://github.com/gotson/komga.git synced 2025-12-10 10:34:37 +01:00
Code Issues Projects Releases Wiki Activity

refactor(api): rework client-settings API

Browse source
This commit is contained in:
Gauthier Roebroeck 2025-02-06 10:13:58 +08:00
parent c82c8b0c73
commit 0b37257be7
6 changed files with 205 additions and 155 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDao.kt
View file

@ -12,16 +12,25 @@ class ClientSettingsDtoDao(
private val g = Tables.CLIENT_SETTINGS_GLOBAL
private val u = Tables.CLIENT_SETTINGS_USER
fun findAllGlobal(): Collection<ClientSettingDto> =
dsl.selectFrom(g)
.map { ClientSettingDto(it.key, it.value, it.allowUnauthorized, null) }
fun findAllGlobal(onlyUnauthorized: Boolean = false): Map<String, ClientSettingDto> =
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<ClientSettingDto> =
dsl.selectFrom(u)
fun findAllUser(userId: String): Map<String, ClientSettingDto> =
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<String>) {
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<String>,
) {
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()
}

2
komga/src/main/kotlin/org/gotson/komga/infrastructure/security/SecurityConfiguration.kt
View file

@ -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

67
komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsController.kt
View file

@ -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<ClientSettingDto> {
if (principal == null) {
// return only global settings that allow unauthorized access
return clientSettingsDtoDao
.findAllGlobal()
.filter { it.allowUnauthorized == true }
.map { it.copy(allowUnauthorized = null) }
}
): Map<String, ClientSettingDto> = 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<String, ClientSettingDto> = 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<String, ClientSettingGlobalUpdateDto>,
) {
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<String, ClientSettingUserUpdateDto>,
) {
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<String>,
) {
clientSettingsDtoDao.deleteGlobalByKeys(keysToDelete)
}
@DeleteMapping("user")
@ResponseStatus(HttpStatus.NO_CONTENT)
fun deleteGlobalSetting(
@AuthenticationPrincipal principal: KomgaPrincipal,
@RequestBody keysToDelete: Set<String>,
) {
clientSettingsDtoDao.deleteByUserIdAndKeys(principal.user.id, keysToDelete)
}
}

4
komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/dto/ClientSettingDto.kt
View file

@ -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,
)

107
komga/src/test/kotlin/org/gotson/komga/infrastructure/jooq/main/ClientSettingsDtoDaoTest.kt
View file

@ -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()
}
}
}

137
komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/ClientSettingsControllerTest.kt
View file

@ -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) }
}
}
}