ch4mpy / spring-addons

Ease spring OAuth2 resource-servers configuration and testing
Apache License 2.0
557 stars 90 forks source link

KeycloakAuthenticationToken is not added as principal in MockHttpServletRequest #36

Closed schmitzhermes closed 2 years ago

schmitzhermes commented 2 years ago

Describe the bug First of all - thanks for your amazing work!

This is a bit of a tricky issue, so I try to be concise:

Code sample Controller Code:

@RestController
class TestController() {
    @GetMapping("/method")
    fun myMethodWithAuth(
            authentication: Authentication
        ): ResponseEntity<IdentCheckStandardAuskunft> {
            // this won't work in runtime, since the authentication is null
        }
}

Test Code

@AutoConfigureMockMvc
@SpringBootTest
@ActiveProfiles("test")
class FailingTest {
    @Test
    @WithMockKeycloakAuth(
        authorities = [ "ROLE_abc" ],
        accessToken = KeycloakAccessToken(
            resourceAccess = [
                KeycloakResourceAccess(resourceId = "my-client", access = KeycloakAccess(roles = ["abc"]))
            ],
        ),
        claims = OpenIdClaims(
            otherClaims = Claims(
                stringClaims = [
                    StringClaim(name = "something", value = "different")
                ]
            )
        )
    )
    fun test() {
        mockMvc
            .perform(
                post("/method")
                    .contentType(MediaType.APPLICATION_JSON)
                    .content(
                        """
                        {"some" : "content"}
                    """.trimIndent()
                    )
            )
            .andExpectAll(
                status().isOk
            )
    }
}

Expected behavior Expected: The authentication parameter is not null and contains the configured KeycloakAuthenticationToken object. Actual: The authentication parameter is null and leads to a NPE.

Additional context There is a workaround by setting the principal explicitly (using the Authentication object in the SecurityContext but this feels wrong. Full working test:

@AutoConfigureMockMvc
@SpringBootTest
@ActiveProfiles("test")
class FailingTest {
    @Test
    @WithMockKeycloakAuth(
        authorities = [ "ROLE_abc" ],
        accessToken = KeycloakAccessToken(
            resourceAccess = [
                KeycloakResourceAccess(resourceId = "my-client", access = KeycloakAccess(roles = ["abc"]))
            ],
        ),
        claims = OpenIdClaims(
            otherClaims = Claims(
                stringClaims = [
                    StringClaim(name = "something", value = "different")
                ]
            )
        )
    )
    fun test() {
        mockMvc
            .perform(
                post("/method")
                    .contentType(MediaType.APPLICATION_JSON)
                    .principal(SecurityContextHolder.getContext().authentication)
                    .content(
                        """
                        {"some" : "content"}
                    """.trimIndent()
                    )
            )
            .andExpectAll(
                status().isOk
            )
    }
}
ch4mpy commented 2 years ago

@schmitzhermes I am a little confused with htis one :/

This is something I'm using all the time (injected authentication in controllers methods) and it works like a charm...

Did you forget to add @WithMockKeycloakAuth when you got the NPE? or maybe did you try to cast to a wrong authentication impl? (but you'd have had soemthing else than NPE, I guess...)

I added explicit tests for what I understand your use-case is (see the linked commit just above). Do not easitate to open a PR with some more tests if I missunderstood what you're trying to do (or close this issue if there is no problem in the end)

schmitzhermes commented 2 years ago

Hmm your tests in 61a5d look pretty much like what I am doing... the thing is: it's not the first time I hit this issue, just this time I invested some time to investigate further. Maybe a good next step would be to set up a vanilla setup on my side and see whether it works there or not.

Maybe one thing to understand the issue a little better: If you debug the session and you set a breakpoint in the ServletRequestMethodArgumentResolver on the respective line. What is the type of the request object in your setup? Is it a MockHttpServletRequest as well?

ch4mpy commented 2 years ago

Here is the request.getClass() output: class org.springframework.security.web.servletapi.HttpServlet3RequestFactory$Servlet3SecurityContextHolderAwareRequestWrapper

But spring-security-tests should ensure that, inside @WebMvcTest, all MockMvc requests are populated with TestSecurityContext content (whatever the actual request implementation it chooses...)

That reminds me of an other check: are your failing tests decorated with @WebMvcTest?

schmitzhermes commented 2 years ago

class org.springframework.security.web.servletapi.HttpServlet3RequestFactory$Servlet3SecurityContextHolderAwareRequestWrapper

That's the crucial difference then, since the SecurityContextAwareRequestWrapper sets the principal based on the SecurityContext.

That reminds me of an other check: are your failing tests decorated with @WebMvcTest?

No, they are decorated with @SpringBootTest and @AutoConfigureMockMvc... this seems to be the difference then, hm?

ch4mpy commented 2 years ago

Maybe security config is missing in the test configuration you import? Did you have to @MockBean JwtDecoder jwtDecoder for the tests to pass with Keycloak down? If you set a breakpoint in some security bean factory, do you hit it?

ch4mpy commented 2 years ago

The test pass if I replace @WebMvcTest with @SpringBootTest:

@RunWith(SpringRunner.class)
@SpringBootTest(webEnvironment = WebEnvironment.MOCK)
@AutoConfigureMockMvc
@Import({ ServletKeycloakAuthUnitTestingSupport.UnitTestConfig.class, KeycloakSpringBootSampleApp.KeycloakConfig.class })
// because this sample stands in the middle of non spring-boot-keycloak projects, keycloakproperties are isolated in
// application-keycloak.properties
@ActiveProfiles("keycloak")
public class GreetingControllerAnnotatedTest {
    ...
}

Which webEnvironment are you using? Do you import Keycloak conf?

ch4mpy commented 2 years ago

@schmitzhermes I can't reproduce and need more feedback.

For now, I believe in test misconfiguration, reason for closing. Please re-open if I'm wrong.

schmitzhermes commented 2 years ago

Fine. For now I accepted the solution to specify the principal manually, I will let you know when I had time to investigate the issue further. Thanks for ur support