Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebTestClient ignores WebSessionManager bean [SPR-17094] #21631

Closed
spring-issuemaster opened this issue Jul 26, 2018 · 6 comments
Closed

WebTestClient ignores WebSessionManager bean [SPR-17094] #21631

spring-issuemaster opened this issue Jul 26, 2018 · 6 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Isamu Nojima opened SPR-17094 and commented

I am using Webflux in Spring Boot 2.0.3.RELEASE to create REST API. With that implementation, I customize and use the webSessionManager as below. (Sorry, the code is written in Kotlin.)

// Kotlin
@EnableWebFluxSecurity
@Configuration
class SecurityConfiguration {
    @Bean
    fun webSessionManager(): WebSessionManager {
        return DefaultWebSessionManager().apply {
            sessionIdResolver = HeaderWebSessionIdResolver().apply {
                headerName = "X-Sample"
            }
            sessionStore = InMemoryWebSessionStore()
        }
    }

    // ...
}

And in order to test the REST API, I created a test code as follows. (addUser and signin are extension functions.)

// Kotlin
@RunWith(SpringRunner::class)
@SpringBootTest
@AutoConfigureWebTestClient
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
class UserTests {
    @Autowired
    private lateinit var client: WebTestClient

    @Test
    fun testGetUserInfo() {
        client.addUser(defaultUser)
        val sessionKey = client.signin(defaultUser)

        client.get().uri(userPath)
                .header("X-Sample", sessionKey)
                .exchange()
                .expectStatus().isOk
                .expectBody()
                .jsonInStrict("""
                {
                  "user": {
                    "mail_address": "user@example.com"
                  }
                }
                """.trimIndent())
    }

    // ...
}

The test failed. It is refused by authorization. However, if I start the server and run it from curl it will succeed in the authorization.

After investigating the cause, it turned out that org.springframework.test.web.reactive.server.AbstractMockServerSpec set webSessionManager to DefaultWebSessionManager. The default is used, not the webSessionManager I customized. For this reason, it could not get the session ID.

I think that it is better to have the following implementation, what do you think?

// Java
abstract class AbstractMockServerSpec<B extends WebTestClient.MockServerSpec<B>>
        implements WebTestClient.MockServerSpec<B> {

    // ...

    private WebSessionManager sessionManager = null;

    // ...

    @Override
    public WebTestClient.Builder configureClient() {
        WebHttpHandlerBuilder builder = initHttpHandlerBuilder();
        builder.filters(theFilters -> theFilters.addAll(0, this.filters));
        if (this.sessionManager != null) {
            builder.sessionManager(this.sessionManager);
        }
        this.configurers.forEach(configurer -> configurer.beforeServerCreated(builder));
        return new DefaultWebTestClientBuilder(builder);
    }

    // ...
}

Affects: 5.0.7

Issue Links:

  • #20233 Provide hook for framework customizations of the WebTestClient MockServerSpec
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Brian Clozel commented

Sorry Isamu Nojima,

The Spring Boot issue tracker is located here: https://github.com/spring-projects/spring-boot/issues

This tracker is for Spring Framework only. Could you create that issue there?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Brian Clozel commented

Note: Spring Framework already provides a method there to customize the webSessionManager, but Spring Boot does not use it to customize it.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Isamu Nojima commented

Sorry.

I wrote Spring Boot 2.0.3.RELEASE, but this is a report on the *spring-test* code.

  1. Spring Boot call WebTestClient.bindToApplicationContext(applicationContext).configureClient()

This call ApplicationContextSpec::configureClient. ApplicationContextSpec extends AbstractMockServerSpec. configureClient method is included in AbstractMockServerSpec.

ApplicationContextSpec
AbstractMockServerSpec

  1. AbstractMockServerSpec::configureClient method call ApplicationContextSpec::initHttpHandlerBuilder method.

  2. ApplicationContextSpec::initHttpHandlerBuilder method call WebHttpHandlerBuilder::applicationContext.

  3. WebHttpHandlerBuilder::applicationContext method sets webSessionManager to WebHttpHandlerBuilder.

  4. ApplicationContextSpec::initHttpHandlerBuilder return this WebHttpHandlerBuilder.

  5. AbstractMockServerSpec::configureClient overrides DefaultWebSessionManager in the webSessionManager of WebHttpHandlerBuilder.

Spring Boot has nothing to do with these processes. I thought it was an issue of AbstractMockServerSpec::configureClient method, so I report it here.

I am writing using Google Translate, so it may be difficult to understand...

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Isamu Nojima commented

What I think is an issue is to overwrite the webSessionManager set with the applicationContext method.

Unless we call AbstractMockServerSpec::webSessionManager, I think that it is better to use the webSessionManager bean. 

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Brian Clozel commented

Alright, I'm reopening this issue to get rstoyanchev opinion on this - should we get any WebSessionManager bean from the application context and configure it automatically?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2018

Rossen Stoyanchev commented

Indeed this looks like a bug, related to #20233 where the goal was to ensure the session manager instance is re-used across requests. However if one is configured as a bean, it should be used instead. Since I'm already in the middle of the solution, I'll just wrap this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.