Add Multi-tenancy support for Reactive Resource Server

[rhamedy]

Fixes: gh-6727
Fixes: gh-6723

[rhamedy]

@jzheaux I have decided to create a draft pull request to get some feedback before proceeding with updating the javadoc and adding tests.

Seems like draft pull requests does not trigger build, I wish it did that way we would have figured whether the changes added break any tests or not.

[jzheaux]

Thanks for the PR, @rhamedy! I've left some feedback inline.

Also, I figure that you are waiting to add unit tests and updating javadocs until we are further along, so I've left most of that feedback out of this review.

[rhamedy]

@jzheaux thank you for the initial review. I think draft was a good idea as I wasn't sure if I have understood the scope of work or not. I have addressed most of your comments and pushed my changes up for review.
I have not written test for all cases and changes, I only added 1 test for the AuthenticationWebFilterTests class for now.
I am curious to what we can and cannot test with regards to changes in this PR?

[jzheaux]

Thanks again, @rhamedy. Yes, I think a draft PR was a good idea in the beginning - if you think it's ready, go ahead and had any remaining tests and java docs and the convert into a formal PR.

[rhamedy]

Hi @jzheaux

The switch to flatMap was what I needed. Pushed my changes as well as added some unit tests to the AuthenticationWebFilter.
Not sure if we have to write tests for changes in OAuth2ResourceServerSpec? If yes, where can I find existing tests? The ServerHttpSecurityTest does not seem to have any for OAuth2ResourceServerSpec 😕

Also, please let me know if any changes need to be made in method names (naming is not my strongest skill set), javadoc, and remaining tests (if any).

Thanks for your feedback.

[jzheaux]

@rhamedy, I answered your question about tests in the PR. Thanks!

Also, it appears that the branch has some conflicts, let me know if you have any questions about resolving those.

[rhamedy]

Hey @jzheaux sorry, I had some git hiccup and had to do a few hard resets before seeing my changes re-appear magically. I usually do rebase but, in this case rebasing wanted me to resolve changes not related to my code so I fallback to merging master to my branch and before this PR is in good shape to be merged then I will squash my commits 👍 For now hopefully, I have carried over the master changes to my branch without breaking anything 🙏

I have pushed my changes that contains the test that is getWhenUsingCustomAuthenticationManagerResolverThenUsesItAccordingly using CustomAuthenticationManagerResolverConfig in OAuth2ResourceServerSpecTests and when running the test I get the follwoing error:

java.lang.AssertionError: 
Expected: a string starting with "Bearer error=\"mock-failure\""
     but: was "Bearer"

> GET 
> WebTestClient-Request-Id: [1]
> Authorization: [Bearer eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJtb2NrLXN1YmplY3QiLCJzY29wZSI6Im1lc3NhZ2U6cmVhZCIsImV4cCI6NDY4ODY0MTQxM30.cRl1bv_dDYcAN5U4NlIVKj8uu4mLMwjABF93P4dShiq-GQ-owzaqTSlB4YarNFgV3PKQvT9wxN1jBpGribvISljakoC0E8wDV-saDi8WxN-qvImYsn1zLzYFiZXCfRIxCmonJpydeiAPRxMTPtwnYDS9Ib0T_iA80TBGd-INhyxUUfrwRW5sqKRbjUciRJhpp7fW2ZYXmi9iPt3HDjRQA4IloJZ7f4-spt5Q9wl5HcQTv1t4XrX4eqhVbE5cCoIkFQnKPOc-jhVM44_eazLU6Xk-CCXP8C_UT5pX0luRS2cJrVFfHp2IR_AWxC-shItg6LNEmNFD4Zc-JLZcr0Q86Q]

No content

< 401 UNAUTHORIZED Unauthorized
< Pragma: [no-cache]
< Expires: [0]
< X-Frame-Options: [DENY]
< X-Content-Type-Options: [nosniff]
< Referrer-Policy: [no-referrer]
< WWW-Authenticate: [Bearer]
< Cache-Control: [no-cache, no-store, max-age=0, must-revalidate]
< X-XSS-Protection: [1 ; mode=block]

0 bytes of content (unknown content-type).

	at org.springframework.test.web.reactive.server.ExchangeResult.assertWithDiagnostics(ExchangeResult.java:200)
	at org.springframework.test.web.reactive.server.HeaderAssertions.value(HeaderAssertions.java:81)
	at org.springframework.security.config.web.server.OAuth2ResourceServerSpecTests.getWhenUsingCustomAuthenticationManagerResolverThenUsesItAccordingly(OAuth2ResourceServerSpecTests.java:252)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.springframework.test.context.junit4.statements.RunBeforeTestExecutionCallbacks.evaluate(RunBeforeTestExecutionCallbacks.java:74)
	at org.springframework.test.context.junit4.statements.RunAfterTestExecutionCallbacks.evaluate(RunAfterTestExecutionCallbacks.java:84)
	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:75)
	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:86)
	at org.springframework.security.config.test.SpringTestRule$1.evaluate(SpringTestRule.java:36)
	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:84)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:251)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:97)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:190)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:538)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:760)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:206)
Caused by: java.lang.AssertionError: 
Expected: a string starting with "Bearer error=\"mock-failure\""
     but: was "Bearer"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
	at org.springframework.test.web.reactive.server.HeaderAssertions.lambda$value$1(HeaderAssertions.java:81)
	at org.springframework.test.web.reactive.server.ExchangeResult.assertWithDiagnostics(ExchangeResult.java:197)
	... 34 more

even though I have stubbed the authentication manager to return error on authentication which leads me to think that the mocked authentication manager is not used 🤔

[jzheaux]

@rhamedy, I think the problem is that when just authenticationManagerResolver() is called, no filter is getting added.

I'd recommend something similar to this:

if (this.authenticationManagerResolver != null) {
    AuthenticationWebFilter oauth2 = new AuthenticationWebFilter(this.authenticationManagerResolver);
    oauth2.setServerAuthenticationConverter(bearerTokenConverter);
    oauth2.setAuthenticationFailureHandler(new 
            ServerAuthenticationEntryPointFailureHandler(entryPoint));
    http.addFilterAt(oauth2, SecurityWebFiltersOrder.AUTHENTICATION);
} else if (this.jwt != null) {
    this.jwt.configure(http);
} else if (this.opaqueToken != null) {
    this.jwt.configure(http);
}

The extra support in jwt.configure and opaqueToken.configure for resolvers is unnecessary, which aligns with the fact that these are mutually exclusive configurations.

[rhamedy]

Hi @jzheaux thanks for hint above, that seemed to be the issue with failing tests. I squashed my commits and pushed my changes. Please let me know if there is need for more tests and/or javadoc.

[jzheaux]

@rhamedy, this looks great, thank you again.

Just one quick change, which I've described inline, and we should be good.

[jzheaux]

Thanks, @rhamedy, this is now merged into master!