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

SecurityMockMvcResultMatchers - Kotlin examples don't trigger assertions #11793

Open
aSemy opened this issue Sep 8, 2022 · 2 comments
Open
Assignees
Labels
in: docs An issue in Documentation or samples type: bug A general bug

Comments

@aSemy
Copy link
Contributor

aSemy commented Sep 8, 2022

Describe the bug

https://github.com/spring-projects/spring-security/blob/5.7.3/docs/modules/ROOT/pages/servlet/test/mockmvc/result-matchers.adoc#authenticated-assertion

The Kotlin examples for the MockMvc assertions call the Java functions, but these methods are not compatible with the Kotlin DSL, and so this test will always pass:

fun `user should be authenticated`() {
  mvc
      .perform(formLogin())
      .andExpect { authenticated() }
      .andExpect { unauthenticated() }
      .andExpect { authenticated().withRoles("USER", "ADMIN") }
      .andExpect { authenticated().withRoles("blah", "qwerty") }
}

authenticated() returns a AuthenticatedMatcher instance, but the returned value isn't used for anything.

Expected behavior

The examples for Kotlin SecurityMockMvcResultMatchers are corrected.

Sample

mvc-kt-matchers.zip

@aSemy aSemy added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 8, 2022
@sjohnr
Copy link
Member

sjohnr commented Sep 9, 2022

Thanks for the report, @aSemy!

I believe the example doesn't work because the Kotlin code (e.g. .andExpect { authenticated() }) implements the functional interface ResultMatcher as a no-op.

these methods are not compatible with the Kotlin DSL

I don't believe the framework is intending to expose a Kotlin DSL (though the Kotlin compiler allows a functional parameter that is the last parameter of a method call to be expressed this way). Code similar to Java should work in Kotlin:

@Test
fun `user should be authenticated`() {
  mvc
    .perform(formLogin())
    .andExpect(authenticated())
    .andExpect(unauthenticated())
    .andExpect(authenticated().withRoles("USER", "ADMIN"))
    .andExpect(authenticated().withRoles("blah", "qwerty"))
}

Do you agree? If so, would you be interested in submitting a PR to correct the Kotlin example(s) in the docs?

@sjohnr sjohnr added in: docs An issue in Documentation or samples and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 9, 2022
@sjohnr sjohnr self-assigned this Sep 9, 2022
@aSemy
Copy link
Contributor Author

aSemy commented Oct 15, 2022

Hi @sjohnr, I've done a bit of looking into it and I think this is more than just documentation problem.

There is a Spring Test Kotlin DSL for MockMVC https://github.com/spring-projects/spring-framework/blob/eeebea1da32e6decae381e33c4ef0aa2cafbf620/spring-test/src/main/kotlin/org/springframework/test/web/servlet/MockMvcExtensions.kt

However, this DSL is not compatible with the Java .andExpect() functions. They're not available when using the Kotlin DSL. For example:

import org.junit.jupiter.api.Test
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors
import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers
import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.get

lateinit var mvc: MockMvc

@Test
fun test() {
    mvc.get("/admin") {
        with(SecurityMockMvcRequestPostProcessors.user("admin").password("pass").roles("USER","ADMIN"))
    }.andExpect(SecurityMockMvcResultMatchers.authenticated())
    // ERROR Type mismatch.
    // Required: MockMvcResultMatchersDsl.() → Unit
    // Found: SecurityMockMvcResultMatchers.AuthenticatedMatcher!
}

So there's no 'correct' way to document this. The Kotlin DSL needs to be adjusted, either to be compatible with the existing andExpect() functions, or to make .andExpect { authenticated() } work.

I think making .andExpect { authenticated() } work is much more preferable because users will have already copied and pasted the docs, and those assertions will be silently ignored. However, I'm not sure if it's technically possible (a new function might require a specific import).

I ended up working around this by not using the Kotlin DSL at all, which is a shame, but at least it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: docs An issue in Documentation or samples type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants