spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.82k stars 5.9k forks source link

SecurityContextHolder is not populated in `@BeforeAll/PostConstruct` within `@WithUserDetails` #15902

Closed bwgjoseph closed 2 weeks ago

bwgjoseph commented 1 month ago

Describe the bug

I'm not sure if this is the intended behavior where SecurityContextHolder is not populated or accessible within @BeforeAll/PostConstruct. I searched the repository and found https://github.com/spring-projects/spring-security/issues/6591 is quite close to what I experience/encounter.

This is reproduced using Spring Boot 3.3.4 but I encountered it in my project which is using Spring Boot 3.2.5, with Java 21 and JUnit 5

To Reproduce

This is the code snippet to reproduce

@ExtendWith(SpringExtension.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class DemoBugTests {
    SoftAssertions softAssertions = new SoftAssertions();

    @PostConstruct
    void postConstruct() {
        // authentication object is null
        // SecurityContextImpl [Null authentication]
        softAssertions.assertThat(SecurityContextHolder.getContext().getAuthentication()).isNotNull(); // this fail
    }

    @BeforeAll
    void beforeAll() {
        // authentication object is null
        // SecurityContextImpl [Null authentication]
        softAssertions.assertThat(SecurityContextHolder.getContext().getAuthentication()).isNotNull(); // this fail
    }

    @BeforeEach
    void beforeEach() {
        // SecurityContextImpl [Authentication=UsernamePasswordAuthenticationToken [Principal=org.springframework.security.core.userdetails.User [Username=bwgjoseph, Password=[PROTECTED], Enabled=true, AccountNonExpired=true, CredentialsNonExpired=true, AccountNonLocked=true, Granted Authorities=[admin]], Credentials=[PROTECTED], Authenticated=true, Details=null, Granted Authorities=[admin]]]
        softAssertions.assertThat(SecurityContextHolder.getContext().getAuthentication()).isNotNull(); // this pass
    }

    @Test
    @WithUserDetails
    void validateAuthentication() {
        // SecurityContextImpl [Authentication=UsernamePasswordAuthenticationToken [Principal=org.springframework.security.core.userdetails.User [Username=bwgjoseph, Password=[PROTECTED], Enabled=true, AccountNonExpired=true, CredentialsNonExpired=true, AccountNonLocked=true, Granted Authorities=[admin]], Credentials=[PROTECTED], Authenticated=true, Details=null, Granted Authorities=[admin]]]
        softAssertions.assertThat(SecurityContextHolder.getContext().getAuthentication()).isNotNull(); // this pass
        softAssertions.assertAll();
    }

    @TestConfiguration
    static class SecurityConfiguration {
        @Bean
        UserDetailsService userDetailsService() {
            return new TestUserDetails();
        }
    }

    static class TestUserDetails implements UserDetailsService {

        @Override
        public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
            return new User("bwgjoseph", "pass", List.of(new SimpleGrantedAuthority("admin")));
        }
    }
}

Expected behavior

I would expect that the authentication object is available within @BeforeAll and PostConstruct method.

I'm not using @PostConstruct actually, but added in this test to see the behavior

Extra Note

I discovered this as my current test is failing where there's a method call to insert certain data in @BeforeAll which uses SpEL within the repository method

@Query(...#{principal}...)
Data findById(...)

So the stack trace looks like the following...

... SpelEvaluationException: EL1022E: A problem occurred whilst attempting to access the property 'principal': The function 'principal' mapped to an object of type 'class org.springframework.security.access.expression.SecurityExpressionRoot' cannot be invoked
// omitted
... SpelEvaluationException: EL1022E: The function 'principal' mapped to an object of type 'class org.springframework.security.access.expression.SecurityExpressionRoot' cannot be invoked

So after some tracing, I found out that it was because the authentication object was null when called in @BeforeAll, and thus, causing the test to fail.

I'm also using custom @WithSecurityContext to provide my own @WithMockXXXUser annotation if that matters.

Let me know if more information is required.

Thanks!

jzheaux commented 2 weeks ago

@bwgjoseph, thanks for reaching out. This is by design. Given that it is a method-scoped annotation, its effects are only applied in the context of that method. @BeforeEach works because it also is a method-scoped annotation.

I discovered this as my current test is failing where there's a method call to insert certain data in @BeforeAll which uses SpEL within the repository method

This is helpful, thank you. The reason Spring Security doesn't support this approach is that each test can have a different @WithMockXXX, meaning that each method can have a completely different user. Given that, it would be a bit smelly in the general case to execute a preparatory step using one SecurityContext and then use a different SecurityContext during method execution.

As a quick example for clarity, consider if your test had two test methods like so:

@BeforeAll 
void addDatabaseRecords() {
    this.orders.addOrderForLoggedInUser(order);
}

@WithMockUser(username="hanna")
void testWithHanna() {
    // ...
}

@WIthMockUser(username="claude")
void testWithClaude() {
    // ...
}

In this case, adding the record in @BeforeAll couldn't be correct as there is no way to know which user to use since none of the methods have been invoked yet.

As it is, it's likely more correct to add the records you need using @BeforeEach.

I hope that clarifies. If not, please consider posting to SO, adding that link here, and I and others would be happy to provide more support.

bwgjoseph commented 2 weeks ago

Thank you, this is indeed helpful with the explanation provided. Appreciate it!