Skip to content

Commit de0cd11

Browse files
eleftheriaseleftherias
committed
Fix PreAuthorize when returning Kotlin Flow
Closes gh-9676
1 parent e2993d9 commit de0cd11

File tree

6 files changed

+97
-23
lines changed

6 files changed

+97
-23
lines changed

‎config/spring-security-config.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ dependencies {
8787
}
8888
testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core'
8989
testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-reactor'
90+
testImplementation 'io.mockk:mockk'
9091

9192
testRuntimeOnly 'org.hsqldb:hsqldb'
9293
}

‎config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinEnableReactiveMethodSecurityTests.kt

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@
1616

1717
package org.springframework.security.config.annotation.method.configuration
1818

19+
import io.mockk.Called
20+
import io.mockk.clearAllMocks
21+
import io.mockk.mockk
22+
import io.mockk.verify
1923
import kotlinx.coroutines.flow.collect
2024
import kotlinx.coroutines.flow.toList
2125
import kotlinx.coroutines.runBlocking
2226
import org.assertj.core.api.Assertions.assertThat
2327
import org.assertj.core.api.Assertions.assertThatExceptionOfType
28+
import org.junit.After
2429
import org.junit.Test
2530
import org.junit.runner.RunWith
2631
import org.springframework.beans.factory.annotation.Autowired
@@ -35,26 +40,38 @@ import org.springframework.test.context.junit4.SpringRunner
3540
@ContextConfiguration
3641
class KotlinEnableReactiveMethodSecurityTests {
3742

43+
private lateinit var delegate: KotlinReactiveMessageService
44+
3845
@Autowired
3946
var messageService: KotlinReactiveMessageService? = null
4047

48+
@After
49+
fun cleanup() {
50+
clearAllMocks()
51+
}
52+
53+
@Autowired
54+
fun setConfig(config: Config) {
55+
this.delegate = config.delegate
56+
}
57+
4158
@Test
42-
fun suspendingGetResultWhenPermitAllThenSuccess() {
59+
fun `suspendingNoAuth always success`() {
4360
runBlocking {
4461
assertThat(messageService!!.suspendingNoAuth()).isEqualTo("success")
4562
}
4663
}
4764

4865
@Test
4966
@WithMockUser(authorities = ["ROLE_ADMIN"])
50-
fun suspendingPreAuthorizeHasRoleWhenGrantedThenSuccess() {
67+
fun `suspendingPreAuthorizeHasRole when user has role then success`() {
5168
runBlocking {
5269
assertThat(messageService!!.suspendingPreAuthorizeHasRole()).isEqualTo("admin")
5370
}
5471
}
5572

5673
@Test
57-
fun suspendingPreAuthorizeHasRoleWhenNoAuthenticationThenDenied() {
74+
fun `suspendingPreAuthorizeHasRole when user does not have role then denied`() {
5875
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
5976
runBlocking {
6077
messageService!!.suspendingPreAuthorizeHasRole()
@@ -64,14 +81,14 @@ class KotlinEnableReactiveMethodSecurityTests {
6481

6582
@Test
6683
@WithMockUser
67-
fun suspendingPreAuthorizeBeanWhenGrantedThenSuccess() {
84+
fun `suspendingPreAuthorizeBean when authorized then success`() {
6885
runBlocking {
6986
assertThat(messageService!!.suspendingPreAuthorizeBean(true)).isEqualTo("check")
7087
}
7188
}
7289

7390
@Test
74-
fun suspendingPreAuthorizeBeanWhenNotAuthorizedThenDenied() {
91+
fun `suspendingPreAuthorizeBean when not authorized then denied`() {
7592
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
7693
runBlocking {
7794
messageService!!.suspendingPreAuthorizeBean(false)
@@ -81,32 +98,42 @@ class KotlinEnableReactiveMethodSecurityTests {
8198

8299
@Test
83100
@WithMockUser("user")
84-
fun suspendingPostAuthorizeWhenAuthorizedThenSuccess() {
101+
fun `suspendingPostAuthorize when authorized then success`() {
85102
runBlocking {
86103
assertThat(messageService!!.suspendingPostAuthorizeContainsName()).isEqualTo("user")
87104
}
88105
}
89106

90107
@Test
91108
@WithMockUser("other-user")
92-
fun suspendingPostAuthorizeWhenNotAuthorizedThenDenied() {
109+
fun `suspendingPostAuthorize when not authorized then denied`() {
93110
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
94111
runBlocking {
95112
messageService!!.suspendingPostAuthorizeContainsName()
96113
}
97114
}
98115
}
99116

117+
@Test
118+
fun `suspendingPreAuthorizeDelegate when user does not have role then delegate not called`() {
119+
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
120+
runBlocking {
121+
messageService!!.suspendingPreAuthorizeDelegate()
122+
}
123+
}
124+
verify { delegate wasNot Called }
125+
}
126+
100127
@Test
101128
@WithMockUser(authorities = ["ROLE_ADMIN"])
102-
fun suspendingFlowPreAuthorizeHasRoleWhenGrantedThenSuccess() {
129+
fun `suspendingFlowPreAuthorize when user has role then success`() {
103130
runBlocking {
104131
assertThat(messageService!!.suspendingFlowPreAuthorize().toList()).containsExactly(1, 2, 3)
105132
}
106133
}
107134

108135
@Test
109-
fun suspendingFlowPreAuthorizeHasRoleWhenNoAuthenticationThenDenied() {
136+
fun `suspendingFlowPreAuthorize when user does not have role then denied`() {
110137
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
111138
runBlocking {
112139
messageService!!.suspendingFlowPreAuthorize().collect()
@@ -115,31 +142,41 @@ class KotlinEnableReactiveMethodSecurityTests {
115142
}
116143

117144
@Test
118-
fun suspendingFlowPostAuthorizeWhenAuthorizedThenSuccess() {
145+
fun `suspendingFlowPostAuthorize when authorized then success`() {
119146
runBlocking {
120147
assertThat(messageService!!.suspendingFlowPostAuthorize(true).toList()).containsExactly(1, 2, 3)
121148
}
122149
}
123150

124151
@Test
125-
fun suspendingFlowPostAuthorizeWhenNotAuthorizedThenDenied() {
152+
fun `suspendingFlowPostAuthorize when not authorized then denied`() {
126153
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
127154
runBlocking {
128155
messageService!!.suspendingFlowPostAuthorize(false).collect()
129156
}
130157
}
131158
}
132159

160+
@Test
161+
fun `suspendingFlowPreAuthorizeDelegate when not authorized then delegate not called`() {
162+
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
163+
runBlocking {
164+
messageService!!.suspendingFlowPreAuthorizeDelegate().collect()
165+
}
166+
}
167+
verify { delegate wasNot Called }
168+
}
169+
133170
@Test
134171
@WithMockUser(authorities = ["ROLE_ADMIN"])
135-
fun flowPreAuthorizeHasRoleWhenGrantedThenSuccess() {
172+
fun `flowPreAuthorize when user has role then success`() {
136173
runBlocking {
137174
assertThat(messageService!!.flowPreAuthorize().toList()).containsExactly(1, 2, 3)
138175
}
139176
}
140177

141178
@Test
142-
fun flowPreAuthorizeHasRoleWhenNoAuthenticationThenDenied() {
179+
fun `flowPreAuthorize when user does not have role then denied`() {
143180
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
144181
runBlocking {
145182
messageService!!.flowPreAuthorize().collect()
@@ -148,28 +185,39 @@ class KotlinEnableReactiveMethodSecurityTests {
148185
}
149186

150187
@Test
151-
fun flowPostAuthorizeWhenAuthorizedThenSuccess() {
188+
fun `flowPostAuthorize when authorized then success`() {
152189
runBlocking {
153190
assertThat(messageService!!.flowPostAuthorize(true).toList()).containsExactly(1, 2, 3)
154191
}
155192
}
156193

157194
@Test
158-
fun flowPostAuthorizeWhenNotAuthorizedThenDenied() {
195+
fun `flowPostAuthorize when not authorized then denied`() {
159196
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
160197
runBlocking {
161198
messageService!!.flowPostAuthorize(false).collect()
162199
}
163200
}
164201
}
165202

203+
@Test
204+
fun `flowPreAuthorizeDelegate when user does not have role then delegate not called`() {
205+
assertThatExceptionOfType(AccessDeniedException::class.java).isThrownBy {
206+
runBlocking {
207+
messageService!!.flowPreAuthorizeDelegate().collect()
208+
}
209+
}
210+
verify { delegate wasNot Called }
211+
}
212+
166213
@EnableReactiveMethodSecurity
167214
@Configuration
168215
open class Config {
216+
var delegate = mockk<KotlinReactiveMessageService>()
169217

170218
@Bean
171219
open fun messageService(): KotlinReactiveMessageServiceImpl {
172-
return KotlinReactiveMessageServiceImpl()
220+
return KotlinReactiveMessageServiceImpl(this.delegate)
173221
}
174222

175223
@Bean

‎config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageService.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,17 @@ interface KotlinReactiveMessageService {
2828

2929
suspend fun suspendingPostAuthorizeContainsName(): String
3030

31+
suspend fun suspendingPreAuthorizeDelegate(): String
32+
3133
suspend fun suspendingFlowPreAuthorize(): Flow<Int>
3234

3335
suspend fun suspendingFlowPostAuthorize(id: Boolean): Flow<Int>
3436

37+
suspend fun suspendingFlowPreAuthorizeDelegate(): Flow<Int>
38+
3539
fun flowPreAuthorize(): Flow<Int>
3640

3741
fun flowPostAuthorize(id: Boolean): Flow<Int>
42+
43+
fun flowPreAuthorizeDelegate(): Flow<Int>
3844
}

‎config/src/test/kotlin/org/springframework/security/config/annotation/method/configuration/KotlinReactiveMessageServiceImpl.kt

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import kotlinx.coroutines.flow.flow
2222
import org.springframework.security.access.prepost.PostAuthorize
2323
import org.springframework.security.access.prepost.PreAuthorize
2424

25-
class KotlinReactiveMessageServiceImpl : KotlinReactiveMessageService {
25+
class KotlinReactiveMessageServiceImpl(val delegate: KotlinReactiveMessageService) : KotlinReactiveMessageService {
2626

2727
override suspend fun suspendingNoAuth(): String {
2828
delay(1)
@@ -47,6 +47,11 @@ class KotlinReactiveMessageServiceImpl : KotlinReactiveMessageService {
4747
return "user"
4848
}
4949

50+
@PreAuthorize("hasRole('ADMIN')")
51+
override suspend fun suspendingPreAuthorizeDelegate(): String {
52+
return delegate.suspendingPreAuthorizeHasRole()
53+
}
54+
5055
@PreAuthorize("hasRole('ADMIN')")
5156
override suspend fun suspendingFlowPreAuthorize(): Flow<Int> {
5257
delay(1)
@@ -69,6 +74,12 @@ class KotlinReactiveMessageServiceImpl : KotlinReactiveMessageService {
6974
}
7075
}
7176

77+
@PreAuthorize("hasRole('ADMIN')")
78+
override suspend fun suspendingFlowPreAuthorizeDelegate(): Flow<Int> {
79+
delay(1)
80+
return delegate.flowPreAuthorize()
81+
}
82+
7283
@PreAuthorize("hasRole('ADMIN')")
7384
override fun flowPreAuthorize(): Flow<Int> {
7485
return flow {
@@ -88,4 +99,9 @@ class KotlinReactiveMessageServiceImpl : KotlinReactiveMessageService {
8899
}
89100
}
90101
}
102+
103+
@PreAuthorize("hasRole('ADMIN')")
104+
override fun flowPreAuthorizeDelegate(): Flow<Int> {
105+
return delegate.flowPreAuthorize()
106+
}
91107
}

‎core/src/main/java/org/springframework/security/access/prepost/PrePostAdviceReactiveMethodInterceptor.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,19 +121,21 @@ public Object invoke(final MethodInvocation invocation) {
121121
.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
122122
}
123123
if (hasFlowReturnType) {
124-
Publisher<?> publisher;
124+
Flux<?> response;
125125
if (isSuspendingFunction) {
126-
publisher = CoroutinesUtils.invokeSuspendingFunction(invocation.getMethod(), invocation.getThis(),
127-
invocation.getArguments());
126+
response = toInvoke.flatMapMany((auth) -> Flux
127+
.from(CoroutinesUtils.invokeSuspendingFunction(invocation.getMethod(), invocation.getThis(),
128+
invocation.getArguments()))
129+
.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
128130
}
129131
else {
130132
ReactiveAdapter adapter = ReactiveAdapterRegistry.getSharedInstance().getAdapter(returnType);
131133
Assert.state(adapter != null, () -> "The returnType " + returnType + " on " + method
132134
+ " must have a org.springframework.core.ReactiveAdapter registered");
133-
publisher = adapter.toPublisher(PrePostAdviceReactiveMethodInterceptor.flowProceed(invocation));
135+
response = toInvoke.flatMapMany((auth) -> Flux
136+
.from(adapter.toPublisher(PrePostAdviceReactiveMethodInterceptor.flowProceed(invocation)))
137+
.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
134138
}
135-
Flux<?> response = toInvoke.flatMapMany((auth) -> Flux.from(publisher)
136-
.map((r) -> (attr != null) ? this.postAdvice.after(auth, invocation, attr, r) : r));
137139
return KotlinDelegate.asFlow(response);
138140
}
139141
if (isSuspendingFunction) {

‎dependencies/spring-security-dependencies.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ dependencies {
2525
api "commons-codec:commons-codec:1.15"
2626
api "commons-collections:commons-collections:3.2.2"
2727
api "commons-logging:commons-logging:1.2"
28+
api "io.mockk:mockk:1.10.2"
2829
api "io.projectreactor.tools:blockhound:1.0.6.RELEASE"
2930
api "javax.annotation:jsr250-api:1.0"
3031
api "javax.servlet.jsp.jstl:javax.servlet.jsp.jstl-api:1.2.2"

0 commit comments

Comments
 (0)