From 4d325a761ea0af53ef3f0f3d02071b50ff23e8a3 Mon Sep 17 00:00:00 2001 From: f43nd1r Date: Wed, 16 Aug 2023 23:54:23 +0200 Subject: [PATCH] Fix some method security issues. Closes #438 --- .gitignore | 1 + .../mailsettings/MailSettingsRepository.kt | 6 +-- .../acra/persistence/user/UserRepository.kt | 10 ++-- .../security/MethodSecurityConfiguration.kt | 16 +++++-- .../acra/security/WebSecurityConfiguration.kt | 47 ++++++++++--------- .../faendir/acra/ui/component/UserEditor.kt | 2 +- acrarium/tsconfig.json | 16 ++----- 7 files changed, 54 insertions(+), 44 deletions(-) diff --git a/.gitignore b/.gitignore index f215bfb2..bbdbd3e5 100644 --- a/.gitignore +++ b/.gitignore @@ -46,3 +46,4 @@ pnpmfile.js /acrarium/vite.generated.ts /acrarium/.pnpm-debug.log /acrarium/frontend/generated +/acrarium/.pnpmfile.cjs diff --git a/acrarium/src/main/kotlin/com/faendir/acra/persistence/mailsettings/MailSettingsRepository.kt b/acrarium/src/main/kotlin/com/faendir/acra/persistence/mailsettings/MailSettingsRepository.kt index 20dd4888..34a5e4b0 100644 --- a/acrarium/src/main/kotlin/com/faendir/acra/persistence/mailsettings/MailSettingsRepository.kt +++ b/acrarium/src/main/kotlin/com/faendir/acra/persistence/mailsettings/MailSettingsRepository.kt @@ -1,5 +1,5 @@ /* - * (C) Copyright 2022 Lukas Morawietz (https://github.com/F43nd1r) + * (C) Copyright 2022-2023 Lukas Morawietz (https://github.com/F43nd1r) * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,7 +26,7 @@ import org.springframework.stereotype.Repository @Repository class MailSettingsRepository(private val jooq: DSLContext) { - @PreAuthorize("#username == principal.username") + @PreAuthorize("isCurrentUser(#username)") fun find(appId: AppId, username: String): MailSettings? = jooq.selectFrom(MAIL_SETTINGS).where(MAIL_SETTINGS.APP_ID.eq(appId), MAIL_SETTINGS.USERNAME.eq(username)).fetchValueInto() @@ -35,7 +35,7 @@ class MailSettingsRepository(private val jooq: DSLContext) { fun findAll(appId: AppId): List = jooq.selectFrom(MAIL_SETTINGS).where(MAIL_SETTINGS.APP_ID.eq(appId)).fetchListInto() - @PreAuthorize("#mailSettings.username == principal.username") + @PreAuthorize("isCurrentUser(#mailSettings.username)") fun store(mailSettings: MailSettings) { jooq.insertInto(MAIL_SETTINGS) .set(MAIL_SETTINGS.APP_ID, mailSettings.appId) diff --git a/acrarium/src/main/kotlin/com/faendir/acra/persistence/user/UserRepository.kt b/acrarium/src/main/kotlin/com/faendir/acra/persistence/user/UserRepository.kt index 2b3d8175..2b664710 100644 --- a/acrarium/src/main/kotlin/com/faendir/acra/persistence/user/UserRepository.kt +++ b/acrarium/src/main/kotlin/com/faendir/acra/persistence/user/UserRepository.kt @@ -1,5 +1,5 @@ /* - * (C) Copyright 2022 Lukas Morawietz (https://github.com/F43nd1r) + * (C) Copyright 2022-2023 Lukas Morawietz (https://github.com/F43nd1r) * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,14 +37,14 @@ class UserRepository(private val jooq: DSLContext, private val passwordEncoder: fun exists(username: String): Boolean = jooq.fetchExists(USER, USER.USERNAME.eq(username)) - @PreAuthorize("#username == principal.username || isAdmin()") + @PreAuthorize("isCurrentUser(#username) || isAdmin()") fun find(username: String): User? = jooq.selectFrom(USER).where(USER.USERNAME.eq(username)).fetchValueInto() /** * @return if the user was successfully created */ @Transactional - @PreAuthorize("isAdmin()") + @PreAuthorize("isAdmin() || hasNoAdmins()") fun create(username: String, rawPassword: String, mail: String?, vararg roles: Role): Boolean = try { val name = username.lowercase() jooq.insertInto(USER) @@ -64,7 +64,7 @@ class UserRepository(private val jooq: DSLContext, private val passwordEncoder: } @Transactional - @PreAuthorize("#username == principal.username || isAdmin()") + @PreAuthorize("isCurrentUser(#username) || isAdmin()") fun update(username: String, rawPassword: String?, mail: String?) { jooq.update(USER) .apply { if (rawPassword != null) set(USER.PASSWORD, passwordEncoder.encode(rawPassword)) } @@ -107,7 +107,7 @@ class UserRepository(private val jooq: DSLContext, private val passwordEncoder: .fetchListInto() @Transactional - @PreAuthorize("#username != principal.username && isAdmin()") + @PreAuthorize("!isCurrentUser(#username) && isAdmin()") fun delete(username: String) { jooq.deleteFrom(USER).where(USER.USERNAME.eq(username)).execute() } diff --git a/acrarium/src/main/kotlin/com/faendir/acra/security/MethodSecurityConfiguration.kt b/acrarium/src/main/kotlin/com/faendir/acra/security/MethodSecurityConfiguration.kt index dcc691ef..144d63ed 100644 --- a/acrarium/src/main/kotlin/com/faendir/acra/security/MethodSecurityConfiguration.kt +++ b/acrarium/src/main/kotlin/com/faendir/acra/security/MethodSecurityConfiguration.kt @@ -18,11 +18,13 @@ package com.faendir.acra.security import com.faendir.acra.persistence.app.AppId import com.faendir.acra.persistence.user.Permission import com.faendir.acra.persistence.user.Role +import com.faendir.acra.persistence.user.UserRepository import org.aopalliance.intercept.MethodInvocation import org.springframework.aop.framework.AopProxyUtils import org.springframework.aop.support.AopUtils import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Lazy import org.springframework.context.annotation.Primary import org.springframework.context.expression.MethodBasedEvaluationContext import org.springframework.expression.EvaluationContext @@ -36,7 +38,7 @@ import java.util.function.Supplier @Configuration @EnableMethodSecurity(prePostEnabled = true) -class MethodSecurityConfiguration { +class MethodSecurityConfiguration(@Lazy private val userRepository: UserRepository) { @Primary @Bean fun customMethodSecurityExpressionHandler(): MethodSecurityExpressionHandler = object : DefaultMethodSecurityExpressionHandler() { @@ -46,7 +48,7 @@ class MethodSecurityConfiguration { private fun createCustomSecurityExpressionRoot( authentication: Supplier, invocation: MethodInvocation - ) = CustomMethodSecurityExpressionRoot(authentication).apply { + ) = CustomMethodSecurityExpressionRoot(authentication, userRepository).apply { setThis(invocation.`this`) setPermissionEvaluator(permissionEvaluator) setTrustResolver(trustResolver) @@ -65,7 +67,11 @@ class MethodSecurityConfiguration { } } -class CustomMethodSecurityExpressionRoot(authentication: Supplier) : SecurityExpressionRoot(authentication), MethodSecurityExpressionOperations { +class CustomMethodSecurityExpressionRoot( + authentication: Supplier, + private val userRepository: UserRepository +) : SecurityExpressionRoot(authentication), + MethodSecurityExpressionOperations { private var filterObject: Any? = null private var returnObject: Any? = null @@ -96,6 +102,8 @@ class CustomMethodSecurityExpressionRoot(authentication: Supplier u.username = value.lowercase(Locale.getDefault()) } else null) content.add(username) val mail = Translatable.createTextField(Messages.EMAIL) diff --git a/acrarium/tsconfig.json b/acrarium/tsconfig.json index e248dfab..1d3096d9 100644 --- a/acrarium/tsconfig.json +++ b/acrarium/tsconfig.json @@ -4,14 +4,14 @@ // You might want to change the configurations to fit your preferences // For more information about the configurations, please refer to http://www.typescriptlang.org/docs/handbook/tsconfig-json.html { - "flow_version": "23.3.4", + "_version": "9", "compilerOptions": { "sourceMap": true, "jsx": "react-jsx", "inlineSources": true, "module": "esNext", "target": "es2020", - "moduleResolution": "node", + "moduleResolution": "bundler", "strict": true, "skipLibCheck": true, "noFallthroughCasesInSwitch": true, @@ -24,15 +24,9 @@ "useDefineForClassFields": false, "baseUrl": "frontend", "paths": { - "@vaadin/flow-frontend": [ - "generated/jar-resources" - ], - "@vaadin/flow-frontend/*": [ - "generated/jar-resources/*" - ], - "Frontend/*": [ - "*" - ] + "@vaadin/flow-frontend": ["generated/jar-resources"], + "@vaadin/flow-frontend/*": ["generated/jar-resources/*"], + "Frontend/*": ["*"] } }, "include": [