diff --git a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostAuthorizeAspectTests.java b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostAuthorizeAspectTests.java index a58dd4888fe..ddc72e9ff90 100644 --- a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostAuthorizeAspectTests.java +++ b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostAuthorizeAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -103,6 +103,13 @@ public void denyAllPreAuthorizeDeniesAccess() { assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.prePostSecured::denyAllMethod); } + @Test + public void nestedDenyAllPostAuthorizeDeniesAccess() { + SecurityContextHolder.getContext().setAuthentication(this.anne); + assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(() -> this.secured.myObject().denyAllMethod()); + } + interface SecuredInterface { @PostAuthorize("hasRole('X')") @@ -134,6 +141,10 @@ void publicCallsPrivate() { privateMethod(); } + NestedObject myObject() { + return new NestedObject(); + } + } static class SecuredImplSubclass extends SecuredImpl { @@ -157,4 +168,13 @@ void denyAllMethod() { } + static class NestedObject { + + @PostAuthorize("denyAll") + void denyAllMethod() { + + } + + } + } diff --git a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostFilterAspectTests.java b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostFilterAspectTests.java index 5194e30b38e..bf114bae25a 100644 --- a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostFilterAspectTests.java +++ b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PostFilterAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,6 +54,11 @@ public void preFilterMethodWhenListThenFilters() { assertThat(this.prePostSecured.postFilterMethod(objects)).containsExactly("apple", "aubergine"); } + @Test + public void nestedDenyAllPostFilterDeniesAccess() { + assertThat(this.prePostSecured.myObject().denyAllMethod()).isEmpty(); + } + static class PrePostSecured { @PostFilter("filterObject.startsWith('a')") @@ -61,6 +66,19 @@ List postFilterMethod(List objects) { return objects; } + NestedObject myObject() { + return new NestedObject(); + } + + } + + static class NestedObject { + + @PostFilter("filterObject == null") + List denyAllMethod() { + return new ArrayList<>(List.of("deny")); + } + } } diff --git a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java index ce3bd383e98..f80ad94d633 100644 --- a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java +++ b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreAuthorizeAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -103,6 +103,13 @@ public void denyAllPreAuthorizeDeniesAccess() { assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.prePostSecured::denyAllMethod); } + @Test + public void nestedDenyAllPreAuthorizeDeniesAccess() { + SecurityContextHolder.getContext().setAuthentication(this.anne); + assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(() -> this.secured.myObject().denyAllMethod()); + } + interface SecuredInterface { @PreAuthorize("hasRole('X')") @@ -134,6 +141,10 @@ void publicCallsPrivate() { privateMethod(); } + NestedObject myObject() { + return new NestedObject(); + } + } static class SecuredImplSubclass extends SecuredImpl { @@ -157,4 +168,13 @@ void denyAllMethod() { } + static class NestedObject { + + @PreAuthorize("denyAll") + void denyAllMethod() { + + } + + } + } diff --git a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreFilterAspectTests.java b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreFilterAspectTests.java index 81654ac65ce..9e0ec20c275 100644 --- a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreFilterAspectTests.java +++ b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/PreFilterAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,6 +54,11 @@ public void preFilterMethodWhenListThenFilters() { assertThat(this.prePostSecured.preFilterMethod(objects)).containsExactly("apple", "aubergine"); } + @Test + public void nestedDenyAllPreFilterDeniesAccess() { + assertThat(this.prePostSecured.myObject().denyAllMethod(new ArrayList<>(List.of("deny")))).isEmpty(); + } + static class PrePostSecured { @PreFilter("filterObject.startsWith('a')") @@ -61,6 +66,19 @@ List preFilterMethod(List objects) { return objects; } + NestedObject myObject() { + return new NestedObject(); + } + + } + + static class NestedObject { + + @PreFilter("filterObject == null") + List denyAllMethod(List list) { + return list; + } + } } diff --git a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/SecuredAspectTests.java b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/SecuredAspectTests.java index c175aa72e6f..51ce1b12bde 100644 --- a/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/SecuredAspectTests.java +++ b/aspects/src/test/java/org/springframework/security/authorization/method/aspectj/SecuredAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java index 9b561eb221e..5772e6afafb 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/AuthorizationProxyConfiguration.java @@ -19,6 +19,8 @@ import java.util.ArrayList; import java.util.List; +import org.aopalliance.intercept.MethodInterceptor; + import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.BeanDefinition; @@ -27,6 +29,7 @@ import org.springframework.context.annotation.Role; import org.springframework.security.authorization.AuthorizationAdvisorProxyFactory; import org.springframework.security.authorization.method.AuthorizationAdvisor; +import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; @Configuration(proxyBeanMethods = false) final class AuthorizationProxyConfiguration implements AopInfrastructureBean { @@ -41,4 +44,17 @@ static AuthorizationAdvisorProxyFactory authorizationProxyFactory(ObjectProvider return factory; } + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + static MethodInterceptor authorizeReturnObjectMethodInterceptor(ObjectProvider provider, + AuthorizationAdvisorProxyFactory authorizationProxyFactory) { + AuthorizeReturnObjectMethodInterceptor interceptor = new AuthorizeReturnObjectMethodInterceptor( + authorizationProxyFactory); + List advisors = new ArrayList<>(); + provider.forEach(advisors::add); + advisors.add(interceptor); + authorizationProxyFactory.setAdvisors(advisors); + return interceptor; + } + } diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java index 409f6fa1ea2..a5f82428947 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityAdvisorRegistrar.java @@ -33,6 +33,7 @@ public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, B registerAsAdvisor("postAuthorizeAuthorization", registry); registerAsAdvisor("securedAuthorization", registry); registerAsAdvisor("jsr250Authorization", registry); + registerAsAdvisor("authorizeReturnObject", registry); } private void registerAsAdvisor(String prefix, BeanDefinitionRegistry registry) { diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java index b7e3b07fbdc..da16b6a968a 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/ReactiveAuthorizationProxyConfiguration.java @@ -19,6 +19,8 @@ import java.util.ArrayList; import java.util.List; +import org.aopalliance.intercept.MethodInterceptor; + import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.BeanDefinition; @@ -27,6 +29,7 @@ import org.springframework.context.annotation.Role; import org.springframework.security.authorization.ReactiveAuthorizationAdvisorProxyFactory; import org.springframework.security.authorization.method.AuthorizationAdvisor; +import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; @Configuration(proxyBeanMethods = false) final class ReactiveAuthorizationProxyConfiguration implements AopInfrastructureBean { @@ -42,4 +45,17 @@ static ReactiveAuthorizationAdvisorProxyFactory authorizationProxyFactory( return factory; } + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + static MethodInterceptor authorizeReturnObjectMethodInterceptor(ObjectProvider provider, + ReactiveAuthorizationAdvisorProxyFactory authorizationProxyFactory) { + AuthorizeReturnObjectMethodInterceptor interceptor = new AuthorizeReturnObjectMethodInterceptor( + authorizationProxyFactory); + List advisors = new ArrayList<>(); + provider.forEach(advisors::add); + advisors.add(interceptor); + authorizationProxyFactory.setAdvisors(advisors); + return interceptor; + } + } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java index f653d3c5875..203d00e9fba 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java @@ -21,7 +21,10 @@ import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import java.util.function.Supplier; @@ -60,6 +63,7 @@ import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.authorization.method.AuthorizationInterceptorsOrder; import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizeReturnObject; import org.springframework.security.authorization.method.MethodInvocationResult; import org.springframework.security.authorization.method.PrePostTemplateDefaults; import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig; @@ -80,6 +84,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -662,6 +667,79 @@ public void methodWhenPostFilterMetaAnnotationThenFilters() { .containsExactly("dave"); } + @Test + @WithMockUser(authorities = "airplane:read") + public void findByIdWhenAuthorizedResultThenAuthorizes() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + Flight flight = flights.findById("1"); + assertThatNoException().isThrownBy(flight::getAltitude); + assertThatNoException().isThrownBy(flight::getSeats); + } + + @Test + @WithMockUser(authorities = "seating:read") + public void findByIdWhenUnauthorizedResultThenDenies() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + Flight flight = flights.findById("1"); + assertThatNoException().isThrownBy(flight::getSeats); + assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(flight::getAltitude); + } + + @Test + @WithMockUser(authorities = "seating:read") + public void findAllWhenUnauthorizedResultThenDenies() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + flights.findAll().forEachRemaining((flight) -> { + assertThatNoException().isThrownBy(flight::getSeats); + assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(flight::getAltitude); + }); + } + + @Test + public void removeWhenAuthorizedResultThenRemoves() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + flights.remove("1"); + } + + @Test + @WithMockUser(authorities = "airplane:read") + public void findAllWhenPostFilterThenFilters() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + flights.findAll() + .forEachRemaining((flight) -> assertThat(flight.getPassengers()).extracting(Passenger::getName) + .doesNotContain("Kevin Mitnick")); + } + + @Test + @WithMockUser(authorities = "airplane:read") + public void findAllWhenPreFilterThenFilters() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + flights.findAll().forEachRemaining((flight) -> { + flight.board(new ArrayList<>(List.of("John"))); + assertThat(flight.getPassengers()).extracting(Passenger::getName).doesNotContain("John"); + flight.board(new ArrayList<>(List.of("John Doe"))); + assertThat(flight.getPassengers()).extracting(Passenger::getName).contains("John Doe"); + }); + } + + @Test + @WithMockUser(authorities = "seating:read") + public void findAllWhenNestedPreAuthorizeThenAuthorizes() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + flights.findAll().forEachRemaining((flight) -> { + List passengers = flight.getPassengers(); + passengers.forEach((passenger) -> assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(passenger::getName)); + }); + } + private static Consumer disallowBeanOverriding() { return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false); } @@ -1061,4 +1139,113 @@ List resultsContainDave(List list) { } + @EnableMethodSecurity + @Configuration + static class AuthorizeResultConfig { + + @Bean + FlightRepository flights() { + FlightRepository flights = new FlightRepository(); + Flight one = new Flight("1", 35000d, 35); + one.board(new ArrayList<>(List.of("Marie Curie", "Kevin Mitnick", "Ada Lovelace"))); + flights.save(one); + Flight two = new Flight("2", 32000d, 72); + two.board(new ArrayList<>(List.of("Albert Einstein"))); + flights.save(two); + return flights; + } + + @Bean + RoleHierarchy roleHierarchy() { + return RoleHierarchyImpl.withRolePrefix("").role("airplane:read").implies("seating:read").build(); + } + + } + + @AuthorizeReturnObject + static class FlightRepository { + + private final Map flights = new ConcurrentHashMap<>(); + + Iterator findAll() { + return this.flights.values().iterator(); + } + + Flight findById(String id) { + return this.flights.get(id); + } + + Flight save(Flight flight) { + this.flights.put(flight.getId(), flight); + return flight; + } + + void remove(String id) { + this.flights.remove(id); + } + + } + + static class Flight { + + private final String id; + + private final Double altitude; + + private final Integer seats; + + private final List passengers = new ArrayList<>(); + + Flight(String id, Double altitude, Integer seats) { + this.id = id; + this.altitude = altitude; + this.seats = seats; + } + + String getId() { + return this.id; + } + + @PreAuthorize("hasAuthority('airplane:read')") + Double getAltitude() { + return this.altitude; + } + + @PreAuthorize("hasAuthority('seating:read')") + Integer getSeats() { + return this.seats; + } + + @AuthorizeReturnObject + @PostAuthorize("hasAuthority('seating:read')") + @PostFilter("filterObject.name != 'Kevin Mitnick'") + List getPassengers() { + return this.passengers; + } + + @PreAuthorize("hasAuthority('seating:read')") + @PreFilter("filterObject.contains(' ')") + void board(List passengers) { + for (String passenger : passengers) { + this.passengers.add(new Passenger(passenger)); + } + } + + } + + public static class Passenger { + + String name; + + public Passenger(String name) { + this.name = name; + } + + @PreAuthorize("hasAuthority('airplane:read')") + public String getName() { + return this.name; + } + + } + } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java index 79149fd41c3..5f2045e9099 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,20 +16,36 @@ package org.springframework.security.config.annotation.method.configuration; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.expression.EvaluationContext; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.expression.SecurityExpressionRoot; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.intercept.method.MockMethodInvocation; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.access.prepost.PreFilter; import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.authorization.method.AuthorizeReturnObject; import org.springframework.security.config.core.GrantedAuthorityDefaults; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.core.context.ReactiveSecurityContextHolder; import static org.assertj.core.api.Assertions.assertThat; @@ -85,6 +101,112 @@ public void rolePrefixWithGrantedAuthorityDefaultsAndSubclassWithProxyingEnabled assertThat(root.hasRole("ABC")).isTrue(); } + @Test + public void findByIdWhenAuthorizedResultThenAuthorizes() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "airplane:read"); + StepVerifier + .create(flights.findById("1") + .flatMap(Flight::getAltitude) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .expectNextCount(1) + .verifyComplete(); + StepVerifier + .create(flights.findById("1") + .flatMap(Flight::getSeats) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .expectNextCount(1) + .verifyComplete(); + } + + @Test + public void findByIdWhenUnauthorizedResultThenDenies() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "seating:read"); + StepVerifier + .create(flights.findById("1") + .flatMap(Flight::getSeats) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .expectNextCount(1) + .verifyComplete(); + StepVerifier + .create(flights.findById("1") + .flatMap(Flight::getAltitude) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .verifyError(AccessDeniedException.class); + } + + @Test + public void findAllWhenUnauthorizedResultThenDenies() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "seating:read"); + StepVerifier + .create(flights.findAll() + .flatMap(Flight::getSeats) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .expectNextCount(2) + .verifyComplete(); + StepVerifier + .create(flights.findAll() + .flatMap(Flight::getAltitude) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .verifyError(AccessDeniedException.class); + } + + @Test + public void removeWhenAuthorizedResultThenRemoves() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "seating:read"); + StepVerifier.create(flights.remove("1").contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .verifyComplete(); + } + + @Test + public void findAllWhenPostFilterThenFilters() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "airplane:read"); + StepVerifier + .create(flights.findAll() + .flatMap(Flight::getPassengers) + .flatMap(Passenger::getName) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .expectNext("Marie Curie", "Ada Lovelace", "Albert Einstein") + .verifyComplete(); + } + + @Test + public void findAllWhenPreFilterThenFilters() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "airplane:read"); + StepVerifier + .create(flights.findAll() + .flatMap((flight) -> flight.board(Flux.just("John Doe", "John")).then(Mono.just(flight))) + .flatMap(Flight::getPassengers) + .flatMap(Passenger::getName) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .expectNext("Marie Curie", "Ada Lovelace", "John Doe", "Albert Einstein", "John Doe") + .verifyComplete(); + } + + @Test + public void findAllWhenNestedPreAuthorizeThenAuthorizes() { + this.spring.register(AuthorizeResultConfig.class).autowire(); + FlightRepository flights = this.spring.getContext().getBean(FlightRepository.class); + TestingAuthenticationToken pilot = new TestingAuthenticationToken("user", "pass", "seating:read"); + StepVerifier + .create(flights.findAll() + .flatMap(Flight::getPassengers) + .flatMap(Passenger::getName) + .contextWrite(ReactiveSecurityContextHolder.withAuthentication(pilot))) + .verifyError(AccessDeniedException.class); + } + @Configuration @EnableReactiveMethodSecurity // this imports ReactiveMethodSecurityConfiguration static class WithRolePrefixConfiguration { @@ -108,4 +230,112 @@ public void bar(String param) { } + @EnableReactiveMethodSecurity + @Configuration + static class AuthorizeResultConfig { + + @Bean + FlightRepository flights() { + FlightRepository flights = new FlightRepository(); + Flight one = new Flight("1", 35000d, 35); + one.board(Flux.just("Marie Curie", "Kevin Mitnick", "Ada Lovelace")).block(); + flights.save(one).block(); + Flight two = new Flight("2", 32000d, 72); + two.board(Flux.just("Albert Einstein")).block(); + flights.save(two).block(); + return flights; + } + + @Bean + Function> isNotKevin() { + return (passenger) -> passenger.getName().map((name) -> !name.equals("Kevin Mitnick")); + } + + } + + @AuthorizeReturnObject + static class FlightRepository { + + private final Map flights = new ConcurrentHashMap<>(); + + Flux findAll() { + return Flux.fromIterable(this.flights.values()); + } + + Mono findById(String id) { + return Mono.just(this.flights.get(id)); + } + + Mono save(Flight flight) { + this.flights.put(flight.getId(), flight); + return Mono.just(flight); + } + + Mono remove(String id) { + this.flights.remove(id); + return Mono.empty(); + } + + } + + static class Flight { + + private final String id; + + private final Double altitude; + + private final Integer seats; + + private final List passengers = new ArrayList<>(); + + Flight(String id, Double altitude, Integer seats) { + this.id = id; + this.altitude = altitude; + this.seats = seats; + } + + String getId() { + return this.id; + } + + @PreAuthorize("hasAuthority('airplane:read')") + Mono getAltitude() { + return Mono.just(this.altitude); + } + + @PreAuthorize("hasAnyAuthority('seating:read', 'airplane:read')") + Mono getSeats() { + return Mono.just(this.seats); + } + + @AuthorizeReturnObject + @PostAuthorize("hasAnyAuthority('seating:read', 'airplane:read')") + @PostFilter("@isNotKevin.apply(filterObject)") + Flux getPassengers() { + return Flux.fromIterable(this.passengers); + } + + @PreAuthorize("hasAnyAuthority('seating:read', 'airplane:read')") + @PreFilter("filterObject.contains(' ')") + Mono board(Flux passengers) { + return passengers.doOnNext((passenger) -> this.passengers.add(new Passenger(passenger))).then(); + } + + } + + public static class Passenger { + + String name; + + public Passenger(String name) { + this.name = name; + } + + @PreAuthorize("hasAuthority('airplane:read')") + public Mono getName() { + return Mono.just(this.name); + } + + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java index ceffb7ded11..9e7db95dab2 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactory.java @@ -42,6 +42,7 @@ import org.springframework.security.authorization.method.AuthorizationAdvisor; import org.springframework.security.authorization.method.AuthorizationManagerAfterMethodInterceptor; import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; import org.springframework.security.authorization.method.PostFilterAuthorizationMethodInterceptor; import org.springframework.security.authorization.method.PreFilterAuthorizationMethodInterceptor; import org.springframework.util.ClassUtils; @@ -83,6 +84,7 @@ public AuthorizationAdvisorProxyFactory() { advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize()); advisors.add(new PreFilterAuthorizationMethodInterceptor()); advisors.add(new PostFilterAuthorizationMethodInterceptor()); + advisors.add(new AuthorizeReturnObjectMethodInterceptor(this)); setAdvisors(advisors); } diff --git a/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java index 5720082862d..2e410f4d141 100644 --- a/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java +++ b/core/src/main/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactory.java @@ -32,6 +32,7 @@ import org.springframework.security.authorization.method.AuthorizationAdvisor; import org.springframework.security.authorization.method.AuthorizationManagerAfterReactiveMethodInterceptor; import org.springframework.security.authorization.method.AuthorizationManagerBeforeReactiveMethodInterceptor; +import org.springframework.security.authorization.method.AuthorizeReturnObjectMethodInterceptor; import org.springframework.security.authorization.method.PostFilterAuthorizationReactiveMethodInterceptor; import org.springframework.security.authorization.method.PreFilterAuthorizationReactiveMethodInterceptor; @@ -72,6 +73,7 @@ public ReactiveAuthorizationAdvisorProxyFactory() { advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize()); advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor()); advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor()); + advisors.add(new AuthorizeReturnObjectMethodInterceptor(this)); this.defaults.setAdvisors(advisors); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java index da6a26bf6e0..c92d39db185 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationInterceptorsOrder.java @@ -43,12 +43,14 @@ public enum AuthorizationInterceptorsOrder { JSR250, - POST_AUTHORIZE, + SECURE_RESULT(450), + + POST_AUTHORIZE(500), /** * {@link PostFilterAuthorizationMethodInterceptor} */ - POST_FILTER, + POST_FILTER(600), LAST(Integer.MAX_VALUE); diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObject.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObject.java new file mode 100644 index 00000000000..b9d3c4945eb --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObject.java @@ -0,0 +1,41 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Wraps Spring Security method authorization advice around the return object of any + * method this annotation is applied to. + * + *

+ * Placing this at the class level is semantically identical to placing it on each method + * in that class. + *

+ * + * @author Josh Cummings + * @since 6.3 + * @see AuthorizeReturnObjectMethodInterceptor + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.METHOD }) +public @interface AuthorizeReturnObject { + +} diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java new file mode 100644 index 00000000000..2bdf4b2a9c0 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java @@ -0,0 +1,110 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authorization.method; + +import java.lang.reflect.Method; +import java.util.function.Predicate; + +import org.aopalliance.aop.Advice; +import org.aopalliance.intercept.MethodInvocation; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.Pointcuts; +import org.springframework.aop.support.StaticMethodMatcherPointcut; +import org.springframework.security.authorization.AuthorizationProxyFactory; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; + +/** + * A method interceptor that applies the given {@link AuthorizationProxyFactory} to any + * return value annotated with {@link AuthorizeReturnObject} + * + * @author Josh Cummings + * @since 6.3 + * @see org.springframework.security.authorization.AuthorizationAdvisorProxyFactory + */ +public final class AuthorizeReturnObjectMethodInterceptor implements AuthorizationAdvisor { + + private final AuthorizationProxyFactory authorizationProxyFactory; + + private Pointcut pointcut = Pointcuts.intersection( + new MethodReturnTypePointcut(Predicate.not(ClassUtils::isVoidType)), + AuthorizationMethodPointcuts.forAnnotations(AuthorizeReturnObject.class)); + + private int order = AuthorizationInterceptorsOrder.SECURE_RESULT.getOrder(); + + public AuthorizeReturnObjectMethodInterceptor(AuthorizationProxyFactory authorizationProxyFactory) { + Assert.notNull(authorizationProxyFactory, "authorizationManager cannot be null"); + this.authorizationProxyFactory = authorizationProxyFactory; + } + + @Override + public Object invoke(MethodInvocation mi) throws Throwable { + Object result = mi.proceed(); + if (result == null) { + return null; + } + return this.authorizationProxyFactory.proxy(result); + } + + @Override + public int getOrder() { + return this.order; + } + + public void setOrder(int order) { + this.order = order; + } + + /** + * {@inheritDoc} + */ + @Override + public Pointcut getPointcut() { + return this.pointcut; + } + + public void setPointcut(Pointcut pointcut) { + this.pointcut = pointcut; + } + + @Override + public Advice getAdvice() { + return this; + } + + @Override + public boolean isPerInstance() { + return true; + } + + static final class MethodReturnTypePointcut extends StaticMethodMatcherPointcut { + + private final Predicate> returnTypeMatches; + + MethodReturnTypePointcut(Predicate> returnTypeMatches) { + this.returnTypeMatches = returnTypeMatches; + } + + @Override + public boolean matches(Method method, Class targetClass) { + return this.returnTypeMatches.test(method.getReturnType()); + } + + } + +} diff --git a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java index 9ca1ce9b4df..a027013170a 100644 --- a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java +++ b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java @@ -284,7 +284,7 @@ public void proxyWhenPreAuthorizeForIterableThenHonors() { public void proxyWhenPreAuthorizeForClassThenHonors() { AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(); Class clazz = proxy(factory, Flight.class); - assertThat(clazz.getSimpleName()).contains("SpringCGLIB$$0"); + assertThat(clazz.getSimpleName()).contains("SpringCGLIB$$"); Flight secured = proxy(factory, this.flight); assertThat(secured.getClass()).isSameAs(clazz); SecurityContextHolder.getContext().setAuthentication(this.user); diff --git a/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java b/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java index 1dc8afccdce..5b90c954c67 100644 --- a/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java +++ b/core/src/test/java/org/springframework/security/authorization/ReactiveAuthorizationAdvisorProxyFactoryTests.java @@ -117,7 +117,7 @@ public void proxyWhenPreAuthorizeOnFluxThenHonors() { public void proxyWhenPreAuthorizeForClassThenHonors() { ReactiveAuthorizationAdvisorProxyFactory factory = new ReactiveAuthorizationAdvisorProxyFactory(); Class clazz = proxy(factory, Flight.class); - assertThat(clazz.getSimpleName()).contains("SpringCGLIB$$0"); + assertThat(clazz.getSimpleName()).contains("SpringCGLIB$$"); Flight secured = proxy(factory, this.flight); StepVerifier .create(secured.getAltitude().contextWrite(ReactiveSecurityContextHolder.withAuthentication(this.user))) diff --git a/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc b/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc index 62d956ce2b6..e6bbee7abc6 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/method-security.adoc @@ -1707,8 +1707,7 @@ For interfaces, either annotations or the `-parameters` approach must be used. Spring Security also supports wrapping any object that is annotated its method security annotations. -To achieve this, you can autowire the provided `AuthorizationProxyFactory` instance, which is based on which method security interceptors you have configured. -If you are using `@EnableMethodSecurity`, then this means that it will by default have the interceptors for `@PreAuthorize`, `@PostAuthorize`, `@PreFilter`, and `@PostFilter`. +The simplest way to achieve this is to mark any method that returns the object you wish to authorize with the `@AuthorizeReturnObject` annotation. For example, consider the following `User` class: @@ -1746,6 +1745,89 @@ class User (val name:String, @get:PreAuthorize("hasAuthority('user:read')") val ---- ====== +Given an interface like this one: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +public class UserRepository { + @AuthorizeReturnObject + Optional findByName(String name) { + // ... + } +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +class UserRepository { + @AuthorizeReturnObject + fun findByName(name:String?): Optional? { + // ... + } +} +---- +====== + +Then any `User` that is returned from `findById` will be secured like other Spring Security-protected components: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Autowired +UserRepository users; + +@Test +void getEmailWhenProxiedThenAuthorizes() { + Optional securedUser = users.findByName("name"); + assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(() -> securedUser.get().getEmail()); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- + +import jdk.incubator.vector.VectorOperators.Test +import java.nio.file.AccessDeniedException +import java.util.* + +@Autowired +var users:UserRepository? = null + +@Test +fun getEmailWhenProxiedThenAuthorizes() { + val securedUser: Optional = users.findByName("name") + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy{securedUser.get().getEmail()} +} +---- +====== + +[NOTE] +==== +`@AuthorizeReturnObject` can be placed at the class level. Note, though, that this means Spring Security will proxy any return object, including ``String``, ``Integer`` and other types. +This is often not what you want to do. + +In most cases, you will want to annotate the individual methods. +==== + +=== Programmatically Proxying + +You can also programmatically proxy a given object. + +To achieve this, you can autowire the provided `AuthorizationProxyFactory` instance, which is based on which method security interceptors you have configured. +If you are using `@EnableMethodSecurity`, then this means that it will by default have the interceptors for `@PreAuthorize`, `@PostAuthorize`, `@PreFilter`, and `@PostFilter`. + + You can proxy an instance of user in the following way: [tabs] diff --git a/docs/modules/ROOT/pages/whats-new.adoc b/docs/modules/ROOT/pages/whats-new.adoc index dc7122d483f..f915ff66bc5 100644 --- a/docs/modules/ROOT/pages/whats-new.adoc +++ b/docs/modules/ROOT/pages/whats-new.adoc @@ -11,6 +11,7 @@ Below are the highlights of the release. == Authorization - https://github.com/spring-projects/spring-security/issues/14596[gh-14596] - xref:servlet/authorization/method-security.adoc[docs] - Add Programmatic Proxy Support for Method Security +- https://github.com/spring-projects/spring-security/issues/14597[gh-14597] - xref:servlet/authorization/method-security.adoc[docs] - Add Securing of Return Values == Configuration