fix authentication errors (#193)

Co-authored-by: Michael Hoennig <michael@hoennig.de>
Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/193
Reviewed-by: Timotheus Pokorra <timotheus.pokorra@hostsharing.net>

src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java

@@ -16,6 +16,7 @@ import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.lang.Nullable;
 import org.springframework.orm.jpa.JpaObjectRetrievalFailureException;
 import org.springframework.orm.jpa.JpaSystemException;
+import org.springframework.security.access.AccessDeniedException;
 import org.springframework.validation.FieldError;
 import org.springframework.validation.method.ParameterValidationResult;
 import org.springframework.web.bind.MethodArgumentNotValidException;
@@ -44,6 +45,14 @@ public class RestResponseEntityExceptionHandler
     @Autowired(required = false)
     private final List<RetroactiveTranslator> retroactiveTranslators;
 
+    @ExceptionHandler(AccessDeniedException.class)
+    protected ResponseEntity<CustomErrorResponse> handleAccessDeniedException(
+            final AccessDeniedException exc, final WebRequest request) {
+        final var fullMaybeLocalizedMessage = localizedMessage(NestedExceptionUtils.getMostSpecificCause(exc));
+        final var sprippedMaybeLocalizedMessage = stripTechnicalDetails(fullMaybeLocalizedMessage);
+        return errorResponse(request, HttpStatus.UNAUTHORIZED, sprippedMaybeLocalizedMessage);
+    }
+
     @ExceptionHandler(DataIntegrityViolationException.class)
     protected ResponseEntity<CustomErrorResponse> handleConflict(
             final RuntimeException exc, final WebRequest request) {

src/main/java/net/hostsharing/hsadminng/ping/PingController.java

@@ -1,8 +1,8 @@
 package net.hostsharing.hsadminng.ping;
 
 import io.micrometer.core.annotation.Timed;
-import io.swagger.v3.oas.annotations.security.SecurityRequirement;
 import net.hostsharing.hsadminng.config.MessageTranslator;
+import net.hostsharing.hsadminng.config.NoSecurityRequirement;
 import net.hostsharing.hsadminng.generated.api.v1.api.TestApi;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.ResponseEntity;
@@ -12,7 +12,7 @@ import org.springframework.web.bind.annotation.RestController;
 
 @RestController
 @PreAuthorize("isAuthenticated()")
-@SecurityRequirement(name = "casTicket")
+@NoSecurityRequirement
 public class PingController implements TestApi {
 
     @Autowired

src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java

@@ -2,8 +2,10 @@ package net.hostsharing.hsadminng.arch;
 
 import com.tngtech.archunit.base.DescribedPredicate;
 import com.tngtech.archunit.core.domain.JavaClass;
+import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.domain.JavaMethod;
 import com.tngtech.archunit.core.domain.JavaModifier;
+import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.junit.AnalyzeClasses;
 import com.tngtech.archunit.junit.ArchTest;
 import com.tngtech.archunit.lang.ArchCondition;
@@ -29,6 +31,9 @@ import jakarta.persistence.Table;
 import java.lang.annotation.Annotation;
 
 import static com.tngtech.archunit.core.domain.JavaModifier.PUBLIC;
+import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith;
+import static com.tngtech.archunit.core.importer.ImportOption.Predefined.DO_NOT_INCLUDE_TESTS;
+import static com.tngtech.archunit.lang.conditions.ArchPredicates.are;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.*;
 import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices;
 import static java.lang.String.format;
@@ -38,6 +43,10 @@ public class ArchitectureTest {
 
     public static final String NET_HOSTSHARING_HSADMINNG = "net.hostsharing.hsadminng";
 
+    private static final JavaClasses PRODUCTION_CLASSES = new ClassFileImporter()
+            .withImportOption(DO_NOT_INCLUDE_TESTS)
+            .importPackages(NET_HOSTSHARING_HSADMINNG);
+
     @ArchTest
     @SuppressWarnings("unused")
     public static final ArchRule onlyValidPackages = noClasses()
@@ -363,18 +372,17 @@ public class ArchitectureTest {
 
     @ArchTest
     @SuppressWarnings("unused")
-    static final ArchRule restControllerSecurityRequirement =
+    static final ArchRule restControllerSecurityRequirement = classes()
-            // TODO.impl: seems that the Spring templates for the OpenAPI generator don't support this,
+            .that(belongToProductionClasses().and(are(annotatedWith(RestController.class))))
-            //  thus we need this annotation to support Swagger UI authorization.
+            .should()
-            classes().that().areAnnotatedWith(RestController.class).should()
             .beAnnotatedWith(SecurityRequirement.class).orShould()
             .beAnnotatedWith(NoSecurityRequirement.class);
 
     @ArchTest
     @SuppressWarnings("unused")
     static final ArchRule everyRestControllerShouldRequireAuthentication =
-            classes()
+            classes().that(belongToProductionClasses()
-                    .that().areAnnotatedWith(RestController.class)
+                    .and(are(annotatedWith(RestController.class))))
                     .should(havePreAuthorizeWithValue("isAuthenticated()"))
                     .because("Every REST controller should require authentication by default, use @PreAuthorize(...) to override this at the endpoint method level.");
 
@@ -395,11 +403,10 @@ public class ArchitectureTest {
         };
     }
 
-
     @ArchTest
     @SuppressWarnings("unused")
-    static final ArchRule restControllerMethods = classes()
+    static final ArchRule restControllerMethods = classes().that(belongToProductionClasses())
-            .that().areAnnotatedWith(RestController.class)
+            .and(are(annotatedWith(RestController.class)))
             .and().resideOutsideOfPackages("net.hostsharing.hsadminng.rbac.test", "net.hostsharing.hsadminng.rbac.test.*")
             .should(haveAllPublicMethodsAnnotatedWith(Timed.class));
 
@@ -410,8 +417,8 @@ public class ArchitectureTest {
 
     @ArchTest
     @SuppressWarnings("unused")
-    static final ArchRule repositoryMethods = classes()
+    static final ArchRule repositoryMethods = classes().that(belongToProductionClasses())
-            .that().areAssignableTo(Repository.class)
+            .and().areAssignableTo(Repository.class)
             .and().resideOutsideOfPackages("net.hostsharing.hsadminng.rbac.test", "net.hostsharing.hsadminng.rbac.test.*")
             .should(haveAllPublicMethodsAnnotatedWith(Timed.class));
 
@@ -483,4 +490,20 @@ public class ArchitectureTest {
         // this is a heuristic, ideally we can determine all methods with generated database calls
         return item.isAssignableTo(Repository.class) && !method.getModifiers().contains(JavaModifier.ABSTRACT);
     }
+
+    private static DescribedPredicate<JavaClass> belongToProductionClasses() {
+        return new DescribedPredicate<>("belong to production classes") {
+            @Override
+            public boolean test(JavaClass javaClass) {
+                // Check if this exact class is in our production classes
+                for (JavaClass prodClass : PRODUCTION_CLASSES) {
+                    if (prodClass.getName().equals(javaClass.getName())) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        };
+    }
+
 }

src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerRestTest.java

@@ -0,0 +1,54 @@
+package net.hostsharing.hsadminng.errors;
+
+import net.hostsharing.hsadminng.config.DisableSecurityConfig;
+import net.hostsharing.hsadminng.config.JsonObjectMapperConfiguration;
+import net.hostsharing.hsadminng.config.MessageTranslator;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
+import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Import;
+import org.springframework.http.ResponseEntity;
+import org.springframework.security.access.AccessDeniedException;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+@WebMvcTest(controllers = RestResponseEntityExceptionHandlerRestTest.TestController.class)
+@Import({JsonObjectMapperConfiguration.class, MessageTranslator.class, DisableSecurityConfig.class, RestResponseEntityExceptionHandler.class, RestResponseEntityExceptionHandlerRestTest.TestConfig.class})
+@Tag("generalIntegrationTest")
+@ActiveProfiles("test")
+class RestResponseEntityExceptionHandlerRestTest {
+
+    @TestConfiguration
+    static class TestConfig {
+        @Bean
+        public TestController testController() {
+            return new TestController();
+        }
+    }
+
+    @RestController
+    static class TestController {
+        @GetMapping("/api/test/exception")
+        public ResponseEntity<String> testEndpoint() {
+            throw new AccessDeniedException("Access is denied");
+        }
+    }
+
+    @Autowired
+    MockMvc mockMvc;
+
+    @Test
+    void handleAccessDeniedExceptionReturnsUnauthorizedResponse() throws Exception {
+        mockMvc.perform(get("/api/test/exception"))
+                .andExpect(status().isUnauthorized());
+    }
+}