From f1fc1203aee99052820cbce8724f178d9608d8ba Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 28 Aug 2025 12:06:15 +0200 Subject: [PATCH] fix authentication errors (#193) Co-authored-by: Michael Hoennig Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/193 Reviewed-by: Timotheus Pokorra --- .../RestResponseEntityExceptionHandler.java | 9 ++++ .../hsadminng/ping/PingController.java | 4 +- .../hsadminng/arch/ArchitectureTest.java | 49 ++++++++++++----- ...esponseEntityExceptionHandlerRestTest.java | 54 +++++++++++++++++++ 4 files changed, 101 insertions(+), 15 deletions(-) create mode 100644 src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerRestTest.java diff --git a/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java b/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java index 3e701ffc..92822364 100644 --- a/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java +++ b/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 retroactiveTranslators; + @ExceptionHandler(AccessDeniedException.class) + protected ResponseEntity 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 handleConflict( final RuntimeException exc, final WebRequest request) { diff --git a/src/main/java/net/hostsharing/hsadminng/ping/PingController.java b/src/main/java/net/hostsharing/hsadminng/ping/PingController.java index eb39e2cd..7d47fa0a 100644 --- a/src/main/java/net/hostsharing/hsadminng/ping/PingController.java +++ b/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 diff --git a/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java b/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java index 811b1cbe..08a08d8b 100644 --- a/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java +++ b/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 = - // TODO.impl: seems that the Spring templates for the OpenAPI generator don't support this, - // thus we need this annotation to support Swagger UI authorization. - classes().that().areAnnotatedWith(RestController.class).should() - .beAnnotatedWith(SecurityRequirement.class).orShould() - .beAnnotatedWith(NoSecurityRequirement.class); + static final ArchRule restControllerSecurityRequirement = classes() + .that(belongToProductionClasses().and(are(annotatedWith(RestController.class)))) + .should() + .beAnnotatedWith(SecurityRequirement.class).orShould() + .beAnnotatedWith(NoSecurityRequirement.class); @ArchTest @SuppressWarnings("unused") static final ArchRule everyRestControllerShouldRequireAuthentication = - classes() - .that().areAnnotatedWith(RestController.class) + classes().that(belongToProductionClasses() + .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() - .that().areAnnotatedWith(RestController.class) + static final ArchRule restControllerMethods = classes().that(belongToProductionClasses()) + .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() - .that().areAssignableTo(Repository.class) + static final ArchRule repositoryMethods = classes().that(belongToProductionClasses()) + .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 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; + } + }; + } + } diff --git a/src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerRestTest.java b/src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerRestTest.java new file mode 100644 index 00000000..040c7bdf --- /dev/null +++ b/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 testEndpoint() { + throw new AccessDeniedException("Access is denied"); + } + } + + @Autowired + MockMvc mockMvc; + + @Test + void handleAccessDeniedExceptionReturnsUnauthorizedResponse() throws Exception { + mockMvc.perform(get("/api/test/exception")) + .andExpect(status().isUnauthorized()); + } +}