From 97516727a4fb5a4f6293cdc68beeb66e29bfe57d Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 15 Nov 2023 11:48:37 -0700 Subject: [PATCH] Add Coroutine Support Closes gh-12080 --- ...ionManagerReactiveMethodSecurityTests.java | 4 +- .../EnableReactiveMethodSecurityTests.java | 2 +- ...otlinEnableReactiveMethodSecurityTests.kt} | 92 ++++++++++++++++++- .../KotlinReactiveMessageService.kt | 8 +- .../KotlinReactiveMessageServiceImpl.kt | 31 ++++++- ...ManagerAfterReactiveMethodInterceptor.java | 51 ++++++++-- ...anagerBeforeReactiveMethodInterceptor.java | 50 ++++++++-- 7 files changed, 214 insertions(+), 24 deletions(-) rename config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/{KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests.kt => KotlinEnableReactiveMethodSecurityTests.kt} (72%) diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableAuthorizationManagerReactiveMethodSecurityTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableAuthorizationManagerReactiveMethodSecurityTests.java index 2777bec5d98..52a04363421 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableAuthorizationManagerReactiveMethodSecurityTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableAuthorizationManagerReactiveMethodSecurityTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -80,7 +80,7 @@ public void notPublisherPreAuthorizeFindByIdThenThrowsIllegalStateException() { .withMessage("The returnType class java.lang.String on public abstract java.lang.String " + "org.springframework.security.config.annotation.method.configuration.ReactiveMessageService" + ".notPublisherPreAuthorizeFindById(long) must return an instance of org.reactivestreams" - + ".Publisher (for example, a Mono or Flux) in order to support Reactor Context"); + + ".Publisher (for example, a Mono or Flux) or the function must be a Kotlin coroutine in order to support Reactor Context"); } @Test diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableReactiveMethodSecurityTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableReactiveMethodSecurityTests.java index ff322323ed3..28cf08e1b97 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableReactiveMethodSecurityTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/EnableReactiveMethodSecurityTests.java @@ -78,7 +78,7 @@ public void notPublisherPreAuthorizeFindByIdThenThrowsIllegalStateException() { .withMessage("The returnType class java.lang.String on public abstract java.lang.String " + "org.springframework.security.config.annotation.method.configuration.ReactiveMessageService" + ".notPublisherPreAuthorizeFindById(long) must return an instance of org.reactivestreams" - + ".Publisher (for example, a Mono or Flux) in order to support Reactor Context"); + + ".Publisher (for example, a Mono or Flux) or the function must be a Kotlin coroutine in order to support Reactor Context"); } @Test diff --git a/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests.kt b/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityTests.kt similarity index 72% rename from config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests.kt rename to config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityTests.kt index 32a3379b262..0c22ebfa716 100644 --- a/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests.kt +++ b/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityTests.kt @@ -41,8 +41,7 @@ import org.springframework.test.context.junit.jupiter.SpringExtension @ExtendWith(SpringExtension::class) @ContextConfiguration -// no authorization manager due to https://github.com/spring-projects/spring-security/issues/12080 -class KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests { +class KotlinEnableReactiveMethodSecurityTests { private lateinit var delegate: KotlinReactiveMessageService @@ -138,6 +137,39 @@ class KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests { coVerify(exactly = 1) { delegate.suspendingPreAuthorizeHasRole() } } + @Test + @WithMockUser + fun `suspendingPrePostAuthorizeHasRoleContainsName when not pre authorized then delegate not called`() { + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy { + runBlocking { + messageService!!.suspendingPrePostAuthorizeHasRoleContainsName() + } + } + verify { delegate wasNot Called } + } + + @Test + @WithMockUser(authorities = ["ROLE_ADMIN"]) + fun `suspendingPrePostAuthorizeHasRoleContainsName when not post authorized then exception`() { + coEvery { delegate.suspendingPrePostAuthorizeHasRoleContainsName() } returns "wrong" + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy { + runBlocking { + messageService!!.suspendingPrePostAuthorizeHasRoleContainsName() + } + } + coVerify(exactly = 1) { delegate.suspendingPrePostAuthorizeHasRoleContainsName() } + } + + @Test + @WithMockUser(authorities = ["ROLE_ADMIN"]) + fun `suspendingPrePostAuthorizeHasRoleContainsName when authorized then success`() { + coEvery { delegate.suspendingPrePostAuthorizeHasRoleContainsName() } returns "user" + runBlocking { + assertThat(messageService!!.suspendingPrePostAuthorizeHasRoleContainsName()).contains("user") + } + coVerify(exactly = 1) { delegate.suspendingPrePostAuthorizeHasRoleContainsName() } + } + @Test @WithMockUser(authorities = ["ROLE_ADMIN"]) fun `suspendingFlowPreAuthorize when user has role then success`() { @@ -181,6 +213,33 @@ class KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests { verify { delegate wasNot Called } } + @Test + fun `suspendingFlowPrePostAuthorizeBean when not pre authorized then delegate not called`() { + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy { + runBlocking { + messageService!!.suspendingFlowPrePostAuthorizeBean(true).collect() + } + } + } + + @Test + @WithMockUser(roles = ["ADMIN"]) + fun `suspendingFlowPrePostAuthorizeBean when not post authorized then denied`() { + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy { + runBlocking { + messageService!!.suspendingFlowPrePostAuthorizeBean(false).collect() + } + } + } + + @Test + @WithMockUser(roles = ["ADMIN"]) + fun `suspendingFlowPrePostAuthorizeBean when authorized then success`() { + runBlocking { + assertThat(messageService!!.suspendingFlowPrePostAuthorizeBean(true).toList()).containsExactly(1, 2, 3) + } + } + @Test @WithMockUser(authorities = ["ROLE_ADMIN"]) fun `suspendingFlowPreAuthorizeDelegate when user has role then delegate called`() { @@ -244,8 +303,35 @@ class KotlinEnableReactiveMethodSecurityNoAuthorizationManagerTests { coVerify(exactly = 1) { delegate.flowPreAuthorize() } } + @Test + fun `flowPrePostAuthorize when not pre authorized then denied`() { + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy { + runBlocking { + messageService!!.flowPrePostAuthorize(true).collect() + } + } + } + + @Test + @WithMockUser(roles = ["ADMIN"]) + fun `flowPrePostAuthorize when not post authorized then denied`() { + assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy { + runBlocking { + messageService!!.flowPrePostAuthorize(false).collect() + } + } + } + + @Test + @WithMockUser(roles = ["ADMIN"]) + fun `flowPrePostAuthorize when authorized then success`() { + runBlocking { + assertThat(messageService!!.flowPrePostAuthorize(true).toList()).containsExactly(1, 2, 3) + } + } + @Configuration - @EnableReactiveMethodSecurity(useAuthorizationManager = false) + @EnableReactiveMethodSecurity open class Config { var delegate = mockk() diff --git a/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageService.kt b/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageService.kt index 90ed0561d79..fd568df15c6 100644 --- a/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageService.kt +++ b/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageService.kt @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 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. @@ -30,15 +30,21 @@ interface KotlinReactiveMessageService { suspend fun suspendingPreAuthorizeDelegate(): String + suspend fun suspendingPrePostAuthorizeHasRoleContainsName(): String + suspend fun suspendingFlowPreAuthorize(): Flow suspend fun suspendingFlowPostAuthorize(id: Boolean): Flow suspend fun suspendingFlowPreAuthorizeDelegate(): Flow + suspend fun suspendingFlowPrePostAuthorizeBean(id: Boolean): Flow + fun flowPreAuthorize(): Flow fun flowPostAuthorize(id: Boolean): Flow fun flowPreAuthorizeDelegate(): Flow + + fun flowPrePostAuthorize(id: Boolean): Flow } diff --git a/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageServiceImpl.kt b/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageServiceImpl.kt index 9fd7c2e51f8..ccaf5cdf111 100644 --- a/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageServiceImpl.kt +++ b/config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageServiceImpl.kt @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 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. @@ -47,6 +47,12 @@ class KotlinReactiveMessageServiceImpl(val delegate: KotlinReactiveMessageServic return "user" } + @PreAuthorize("hasRole('ADMIN')") + @PostAuthorize("returnObject?.contains(authentication?.name)") + override suspend fun suspendingPrePostAuthorizeHasRoleContainsName(): String { + return delegate.suspendingPrePostAuthorizeHasRoleContainsName() + } + @PreAuthorize("hasRole('ADMIN')") override suspend fun suspendingPreAuthorizeDelegate(): String { return delegate.suspendingPreAuthorizeHasRole() @@ -80,6 +86,18 @@ class KotlinReactiveMessageServiceImpl(val delegate: KotlinReactiveMessageServic return delegate.flowPreAuthorize() } + @PreAuthorize("hasRole('ADMIN')") + @PostAuthorize("@authz.check(#id)") + override suspend fun suspendingFlowPrePostAuthorizeBean(id: Boolean): Flow { + delay(1) + return flow { + for (i in 1..3) { + delay(1) + emit(i) + } + } + } + @PreAuthorize("hasRole('ADMIN')") override fun flowPreAuthorize(): Flow { return flow { @@ -104,4 +122,15 @@ class KotlinReactiveMessageServiceImpl(val delegate: KotlinReactiveMessageServic override fun flowPreAuthorizeDelegate(): Flow { return delegate.flowPreAuthorize() } + + @PreAuthorize("hasRole('ADMIN')") + @PostAuthorize("@authz.check(#id)") + override fun flowPrePostAuthorize(id: Boolean): Flow { + return flow { + for (i in 1..3) { + delay(1) + emit(i) + } + } + } } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java index df5fd8daf22..550b8fbdef3 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerAfterReactiveMethodInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -19,6 +19,7 @@ import java.lang.reflect.Method; import java.util.function.Function; +import kotlinx.coroutines.reactive.ReactiveFlowKt; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; @@ -29,6 +30,8 @@ import org.springframework.aop.Pointcut; import org.springframework.aop.PointcutAdvisor; import org.springframework.aop.framework.AopInfrastructureBean; +import org.springframework.core.KotlinDetector; +import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; @@ -48,6 +51,10 @@ public final class AuthorizationManagerAfterReactiveMethodInterceptor implements Ordered, MethodInterceptor, PointcutAdvisor, AopInfrastructureBean { + private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; + + private static final int RETURN_TYPE_METHOD_PARAMETER_INDEX = -1; + private final Pointcut pointcut; private final ReactiveAuthorizationManager authorizationManager; @@ -99,15 +106,32 @@ public AuthorizationManagerAfterReactiveMethodInterceptor(Pointcut pointcut, public Object invoke(MethodInvocation mi) throws Throwable { Method method = mi.getMethod(); Class type = method.getReturnType(); - Assert - .state(Publisher.class.isAssignableFrom(type), - () -> String.format( - "The returnType %s on %s must return an instance of org.reactivestreams.Publisher " - + "(for example, a Mono or Flux) in order to support Reactor Context", - type, method)); + boolean isSuspendingFunction = KotlinDetector.isSuspendingFunction(method); + boolean hasFlowReturnType = COROUTINES_FLOW_CLASS_NAME + .equals(new MethodParameter(method, RETURN_TYPE_METHOD_PARAMETER_INDEX).getParameterType().getName()); + boolean hasReactiveReturnType = Publisher.class.isAssignableFrom(type) || isSuspendingFunction + || hasFlowReturnType; + Assert.state(hasReactiveReturnType, + () -> "The returnType " + type + " on " + method + + " must return an instance of org.reactivestreams.Publisher " + + "(for example, a Mono or Flux) or the function must be a Kotlin coroutine " + + "in order to support Reactor Context"); Mono authentication = ReactiveAuthenticationUtils.getAuthentication(); Function> postAuthorize = (result) -> postAuthorize(authentication, mi, result); ReactiveAdapter adapter = ReactiveAdapterRegistry.getSharedInstance().getAdapter(type); + if (hasFlowReturnType) { + if (isSuspendingFunction) { + Publisher publisher = ReactiveMethodInvocationUtils.proceed(mi); + return Flux.from(publisher).flatMap(postAuthorize); + } + else { + Assert.state(adapter != null, () -> "The returnType " + type + " on " + method + + " must have a org.springframework.core.ReactiveAdapter registered"); + Flux response = Flux.defer(() -> adapter.toPublisher(ReactiveMethodInvocationUtils.proceed(mi))) + .flatMap(postAuthorize); + return KotlinDelegate.asFlow(response); + } + } Publisher publisher = ReactiveMethodInvocationUtils.proceed(mi); if (isMultiValue(type, adapter)) { Flux flux = Flux.from(publisher).flatMap(postAuthorize); @@ -121,7 +145,7 @@ private boolean isMultiValue(Class returnType, ReactiveAdapter adapter) { if (Flux.class.isAssignableFrom(returnType)) { return true; } - return adapter == null || adapter.isMultiValue(); + return adapter != null && adapter.isMultiValue(); } private Mono postAuthorize(Mono authentication, MethodInvocation mi, Object result) { @@ -153,4 +177,15 @@ public void setOrder(int order) { this.order = order; } + /** + * Inner class to avoid a hard dependency on Kotlin at runtime. + */ + private static class KotlinDelegate { + + private static Object asFlow(Publisher publisher) { + return ReactiveFlowKt.asFlow(publisher); + } + + } + } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java index 9c0d99edf1a..f3d1cce8bfa 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationManagerBeforeReactiveMethodInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -18,6 +18,7 @@ import java.lang.reflect.Method; +import kotlinx.coroutines.reactive.ReactiveFlowKt; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; @@ -28,6 +29,8 @@ import org.springframework.aop.Pointcut; import org.springframework.aop.PointcutAdvisor; import org.springframework.aop.framework.AopInfrastructureBean; +import org.springframework.core.KotlinDetector; +import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; @@ -48,6 +51,10 @@ public final class AuthorizationManagerBeforeReactiveMethodInterceptor implements Ordered, MethodInterceptor, PointcutAdvisor, AopInfrastructureBean { + private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; + + private static final int RETURN_TYPE_METHOD_PARAMETER_INDEX = -1; + private final Pointcut pointcut; private final ReactiveAuthorizationManager authorizationManager; @@ -99,15 +106,31 @@ public AuthorizationManagerBeforeReactiveMethodInterceptor(Pointcut pointcut, public Object invoke(MethodInvocation mi) throws Throwable { Method method = mi.getMethod(); Class type = method.getReturnType(); - Assert - .state(Publisher.class.isAssignableFrom(type), - () -> String.format( - "The returnType %s on %s must return an instance of org.reactivestreams.Publisher " - + "(for example, a Mono or Flux) in order to support Reactor Context", - type, method)); + boolean isSuspendingFunction = KotlinDetector.isSuspendingFunction(method); + boolean hasFlowReturnType = COROUTINES_FLOW_CLASS_NAME + .equals(new MethodParameter(method, RETURN_TYPE_METHOD_PARAMETER_INDEX).getParameterType().getName()); + boolean hasReactiveReturnType = Publisher.class.isAssignableFrom(type) || isSuspendingFunction + || hasFlowReturnType; + Assert.state(hasReactiveReturnType, + () -> "The returnType " + type + " on " + method + + " must return an instance of org.reactivestreams.Publisher " + + "(for example, a Mono or Flux) or the function must be a Kotlin coroutine " + + "in order to support Reactor Context"); Mono authentication = ReactiveAuthenticationUtils.getAuthentication(); ReactiveAdapter adapter = ReactiveAdapterRegistry.getSharedInstance().getAdapter(type); Mono preAuthorize = this.authorizationManager.verify(authentication, mi); + if (hasFlowReturnType) { + if (isSuspendingFunction) { + return preAuthorize.thenMany(Flux.defer(() -> ReactiveMethodInvocationUtils.proceed(mi))); + } + else { + Assert.state(adapter != null, () -> "The returnType " + type + " on " + method + + " must have a org.springframework.core.ReactiveAdapter registered"); + Flux response = preAuthorize + .thenMany(Flux.defer(() -> adapter.toPublisher(ReactiveMethodInvocationUtils.proceed(mi)))); + return KotlinDelegate.asFlow(response); + } + } if (isMultiValue(type, adapter)) { Publisher publisher = Flux.defer(() -> ReactiveMethodInvocationUtils.proceed(mi)); Flux result = preAuthorize.thenMany(publisher); @@ -122,7 +145,7 @@ private boolean isMultiValue(Class returnType, ReactiveAdapter adapter) { if (Flux.class.isAssignableFrom(returnType)) { return true; } - return adapter == null || adapter.isMultiValue(); + return adapter != null && adapter.isMultiValue(); } @Override @@ -149,4 +172,15 @@ public void setOrder(int order) { this.order = order; } + /** + * Inner class to avoid a hard dependency on Kotlin at runtime. + */ + private static class KotlinDelegate { + + private static Object asFlow(Publisher publisher) { + return ReactiveFlowKt.asFlow(publisher); + } + + } + }