Fix some method security issues. Closes #438

.gitignore

@@ -46,3 +46,4 @@ pnpmfile.js
 /acrarium/vite.generated.ts
 /acrarium/.pnpm-debug.log
 /acrarium/frontend/generated
+/acrarium/.pnpmfile.cjs

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<MailSettings> =
         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)

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<User>()
 
     /**
      * @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<Permission>()
 
     @Transactional
-    @PreAuthorize("#username != principal.username && isAdmin()")
+    @PreAuthorize("!isCurrentUser(#username) && isAdmin()")
     fun delete(username: String) {
         jooq.deleteFrom(USER).where(USER.USERNAME.eq(username)).execute()
     }

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<Authentication>,
             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<Authentication>) : SecurityExpressionRoot(authentication), MethodSecurityExpressionOperations {
+class CustomMethodSecurityExpressionRoot(
+    authentication: Supplier<Authentication>,
+    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<Authentication
         return target
     }
 
+    fun hasNoAdmins() = !userRepository.hasAnyAdmin()
+
     fun isAdmin() = SecurityUtils.hasRole(Role.ADMIN)
 
     fun isUser() = SecurityUtils.hasRole(Role.USER)
@@ -104,6 +112,8 @@ class CustomMethodSecurityExpressionRoot(authentication: Supplier<Authentication
 
     fun isApi() = SecurityUtils.hasRole(Role.API)
 
+    fun isCurrentUser(username: String) = SecurityUtils.getUsername() == username
+
     @JvmName("hasViewPermission")
     fun hasViewPermission(appId: AppId) = SecurityUtils.hasPermission(appId, Permission.Level.VIEW)
 

acrarium/src/main/kotlin/com/faendir/acra/security/WebSecurityConfiguration.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.
@@ -31,15 +31,16 @@ import org.springframework.security.authentication.AuthenticationProvider
 import org.springframework.security.authentication.BadCredentialsException
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken
 import org.springframework.security.config.BeanIds
+import org.springframework.security.config.Customizer
 import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder
 import org.springframework.security.config.annotation.web.builders.HttpSecurity
 import org.springframework.security.config.annotation.web.builders.WebSecurity
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
 import org.springframework.security.core.Authentication
 import org.springframework.security.web.SecurityFilterChain
 import org.springframework.security.web.authentication.Http403ForbiddenEntryPoint
-import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint
 import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher
+import org.springframework.security.web.util.matcher.AntPathRequestMatcher.antMatcher
 import org.springframework.security.web.util.matcher.OrRequestMatcher
 import org.springframework.web.servlet.handler.HandlerMappingIntrospector
 
@@ -77,31 +78,34 @@ class WebSecurityConfiguration(private val userRepository: UserRepository) : Vaa
     @Order(1)
     fun reportSecurityChain(http: HttpSecurity): SecurityFilterChain =
         http.securityMatcher("/$REPORT_PATH")
-            .csrf().disable()
-            .exceptionHandling().authenticationEntryPoint(Http403ForbiddenEntryPoint()).and()
-            .authorizeHttpRequests { it.anyRequest().hasRole(Role.REPORTER.name) }.httpBasic().and()
+            .csrf { it.disable() }
+            .exceptionHandling { it.authenticationEntryPoint(Http403ForbiddenEntryPoint()) }
+            .authorizeHttpRequests { it.anyRequest().hasRole(Role.REPORTER.name) }
+            .httpBasic(Customizer.withDefaults())
             .build()
 
     @Bean
     @Order(2)
     fun apiSecurityChain(http: HttpSecurity): SecurityFilterChain =
         http.securityMatcher("/$API_PATH/.*")
-            .csrf().disable()
-            .headers().disable()
-            .anonymous().disable()
-            .exceptionHandling().authenticationEntryPoint(Http403ForbiddenEntryPoint()).and()
-            .authorizeHttpRequests { it.anyRequest().hasRole(Role.API.name) }.httpBasic().and()
+            .csrf { it.disable() }
+            .headers { it.disable() }
+            .anonymous { it.disable() }
+            .exceptionHandling { it.authenticationEntryPoint(Http403ForbiddenEntryPoint()) }
+            .authorizeHttpRequests { it.anyRequest().hasRole(Role.API.name) }
+            .httpBasic(Customizer.withDefaults())
             .build()
 
     @Bean
     @Order(3)
     fun actuatorSecurityChain(http: HttpSecurity, introspector: HandlerMappingIntrospector): SecurityFilterChain =
         http.securityMatcher(OrRequestMatcher(EndpointRequest.toAnyEndpoint(), MvcRequestMatcher(introspector, ".*/swagger-ui/.*")))
-            .csrf().disable()
-            .headers().disable()
-            .anonymous().disable()
-            .exceptionHandling().authenticationEntryPoint(BasicAuthenticationEntryPoint()).and()
-            .authorizeHttpRequests { it.anyRequest().hasRole(Role.API.name) }.httpBasic().and()
+            .csrf { it.disable() }
+            .headers { it.disable() }
+            .anonymous { it.disable() }
+            .exceptionHandling { it.authenticationEntryPoint(Http403ForbiddenEntryPoint()) }
+            .authorizeHttpRequests { it.anyRequest().hasRole(Role.ADMIN.name) }
+            .httpBasic(Customizer.withDefaults())
             .build()
 
     @Bean("VaadinSecurityFilterChainBean")
@@ -111,24 +115,25 @@ class WebSecurityConfiguration(private val userRepository: UserRepository) : Vaa
     }
 
     override fun configure(http: HttpSecurity) {
-        http.authorizeHttpRequests().requestMatchers("/${SetupView.ROUTE}").permitAll()
-        http.formLogin().loginPage("/${LoginView.ROUTE}").permitAll()
+        http.authorizeHttpRequests { it.requestMatchers(antMatcher("/${SetupView.ROUTE}")).permitAll() }
+        http.formLogin { it.loginPage("/${LoginView.ROUTE}").permitAll() }
         super.configure(http)
     }
 
     override fun configure(web: WebSecurity) {
         super.configure(web)
         web.ignoring().requestMatchers(
-            "/images/**",
+            antMatcher("/images/**"),
 
             // Vaadin Flow static resources
-            "/VAADIN/**",
+            antMatcher("/VAADIN/**"),
 
             // the robots exclusion standard
-            "/robots.txt",
+            antMatcher("/robots.txt"),
 
             // (production mode) static resources
-            "/frontend-es5/**", "/frontend-es6/**"
+            antMatcher("/frontend-es5/**"),
+            antMatcher("/frontend-es6/**"),
         )
     }
 }

acrarium/src/main/kotlin/com/faendir/acra/ui/component/UserEditor.kt

@@ -57,7 +57,7 @@ class UserEditor(userRepository: UserRepository, mailService: MailService?, exis
         if (!isExistingUser) {
             usernameBindingBuilder.asRequired(getTranslation(Messages.USERNAME_REQUIRED))
         }
-        usernameBindingBuilder.withValidator({ it == user.username || userRepository.find(it) == null }, getTranslation(Messages.USERNAME_TAKEN))
+        usernameBindingBuilder.withValidator({ it == user.username || !userRepository.exists(it) }, getTranslation(Messages.USERNAME_TAKEN))
             .bind({ it.username }, if (!isExistingUser) Setter { u, value -> u.username = value.lowercase(Locale.getDefault()) } else null)
         content.add(username)
         val mail = Translatable.createTextField(Messages.EMAIL)

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": [