Affects: 2.4.0
Our code:
fun me(@ApiIgnore principal: Principal): UserDetail {
used to inject the principal in Spring Boot versions prior to 2.4.0. Under 2.4.0 the argument is then null and our application breaks.
If the @ApiIgnore annotation is removed then the principal is then injected, however we don't want the principal to be exposed in our API documentation since it is an internal parameter.
It appears that the bug was introduced in https://github.com/spring-projects/spring-framework/pull/25780. That PR doesn't check to see if there is a AuthenticationPrincipal annotation on the field, merely that the parameter has any annotations at all so even Nonnull will break the injection.
We've tried adding the AuthenticationPrincipal annotation on the field, however that doesn't work since the parameter resolver tries to inject authentication.getPrincipal which in our case is a String since we're using spring security oauth2. We want the Principal injected instead.
Comment From: electrified
I have found a resolution to the issue by adding the following:
@Configuration
class WebConfig : WebMvcConfigurer {
override fun addArgumentResolvers(resolvers: MutableList<HandlerMethodArgumentResolver>) {
super.addArgumentResolvers(resolvers)
resolvers.add(PrincipalMethodArgumentResolver())
}
}
The new PrincipalMethodArgumentResolver is in the defaultInitBinderArgumentResolvers but not the defaultArgumentResolvers https://github.com/spring-projects/spring-framework/blob/master/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java#L718
Comment From: rstoyanchev
Yes it looks like the fix in #25981 was incomplete.
Comment From: michaelbrewer
@rstoyanchev i am still experiencing this issue. Is the fix @electrified recommending required for this. Or is there another reason why this is closed?
Comment From: michaelbrewer
Same issue as @petergphillips
fun createPayment(
request: HttpServletRequest,
@ApiIgnore @AuthenticationPrincipal authentication: JwtAuthentication<PaymentGatewayClaim>,
@RequestBody @Valid paymentCreateRequest: PaymentCreateRequest
): PaymentCreateResponse {
Adding this did fix this, but i don't understand why this was changed?
@Configuration
class WebConfig : WebMvcConfigurer {
override fun addArgumentResolvers(resolvers: MutableList<HandlerMethodArgumentResolver>) {
super.addArgumentResolvers(resolvers)
resolvers.add(PrincipalMethodArgumentResolver())
}
}
Comment From: rstoyanchev
@michaelbrewer if you look at the timeline you will see that it was closed with a commit. That commit adds the same resolver and there are tests for it. Can you check that you are running with the fix from that commit, which should be included in version 5.3.2?
Comment From: michaelbrewer
@rstoyanchev i am still experiencing this with SpringBoot 2.4.1 (maybe i will have to manually bump spring-framework to 5.3.2?)
Comment From: michaelbrewer
@rstoyanchev So i have to use the PrincipalMethodArgumentResolver to fix this regression by Spring? SpringBoot 2.4.1 is using Spring Framework 5.3.2 Upgrade to Spring Framework 5.3.2 #24278
Comment From: rstoyanchev
@michaelbrewer please check the fix 05e3f271b62ffbecb2f1f0defe96d4959334843f. It does exactly the same. If you can double check in the source that this change is present. Also maybe debug to see what happens? I don't see why you have to add this resolver if it is there.
Comment From: michaelbrewer
@rstoyanchev still does not work for me. But i am using manual @Import and not using MVC. Is there a specific configuration this works for? (I do won't why regression bugs like this are needed.)
Comment From: fletchgqc
@rstoyanchev I see your point, that the fix does indeed add the resolver. Nevertheless, I can confirm that I also suffer this problem under Spring Boot 2.4.1, and I have confirmed that the fix is visible in the code.
When I add the following (Java version of @electrified's fix), then the problem disappears.
@Configuration
public class Something implements WebMvcConfigurer {
@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(new PrincipalMethodArgumentResolver());
}
}
Is the order of the resolvers relevant? I debugged a bit and with the @Configuration fix, the PrincipalMethodArgumentResolver is now added twice, the @Configuration class causes it to be added in spot 25 in the list for my example, along with being added to spot 33 by the RequestMappingHandlerAdapter.
Comment From: rstoyanchev
@michaelbrewer, I'm not sure what manual @Import vs not using MVC is exactly or why that would have an impact since the PrincipalMethodArgumentResolver is inserted inside RequestMappingHandlerAdapter.
@fletchgqc a search on usages for where the resolver is instantiated does not explain why it would be added twice. Could it be that it's twice because you're adding it as well? The framework adds it once only.
Can either of you provide a sample that demonstrates the issue?
Comment From: fletchgqc
@rstoyanchev It's added twice, because I use Spring 2.4.1 (which has your fix), and also the fix mentioned at the top of this thread. With plain Spring 2.4.1 it's only added once, but that doesn't fix the problem.
Comment From: petergphillips
Our issue is fixed in Spring 2.4.1. We've removed @electrified fix and the application still works as expected :grin:
Comment From: krm1312
Attempted upgrade from boot 2.3.4 to 2.4.1 and @AuthenticationPrincipal is now null in our relatively large application as well. Will try to debug a bit.
Comment From: rstoyanchev
@krm1312 thanks and if there is a regression please create a separate issue to provide the details.
Comment From: michaelbrewer
@rstoyanchev Still not fixed for me.
Following code works fine on SpringBoot 2.3.8 but on SpringBoot 2.4.2 the auth parameter is null:
java.lang.NullPointerException: Parameter specified as non-null is null: method temp.FakeController.fake, parameter auth
at temp.FakeController.fake(AppTest.kt)
package temp
import okhttp3.OkHttpClient
import okhttp3.Request
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import org.junit.jupiter.api.extension.ExtendWith
import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.boot.web.server.LocalServerPort
import org.springframework.context.annotation.Configuration
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken
import org.springframework.security.config.annotation.web.builders.HttpSecurity
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter
import org.springframework.security.core.Authentication
import org.springframework.security.core.annotation.AuthenticationPrincipal
import org.springframework.security.core.context.SecurityContextHolder
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter
import org.springframework.test.context.ContextConfiguration
import org.springframework.test.context.junit.jupiter.SpringExtension
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RestController
import org.springframework.web.filter.GenericFilterBean
import javax.servlet.FilterChain
import javax.servlet.ServletRequest
import javax.servlet.ServletResponse
@SpringBootApplication
class FakeApplication
class FakeSecurityFilter : GenericFilterBean() {
override fun doFilter(request: ServletRequest, response: ServletResponse, chain: FilterChain) {
SecurityContextHolder.getContext().authentication = UsernamePasswordAuthenticationToken("x", "y")
chain.doFilter(request, response)
}
}
@Configuration
class CustomWebSecurityConfigurerAdapter : WebSecurityConfigurerAdapter() {
override fun configure(http: HttpSecurity) {
http.addFilterAfter(FakeSecurityFilter(), BasicAuthenticationFilter::class.java)
}
}
@RestController
class FakeController {
@GetMapping(path = ["/fake"])
fun fake(@AuthenticationPrincipal auth: Authentication) {
println(auth)
}
}
@ExtendWith(SpringExtension::class)
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ContextConfiguration(classes = [FakeApplication::class])
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class Tests {
@LocalServerPort
private val port = 0
@Test
fun checkCall() {
val request = Request.Builder().get().url("http://localhost:$port/fake").build()
val response = OkHttpClient().newCall(request).execute()
Assertions.assertEquals(200, response.code)
}
}
Comment From: rstoyanchev
@michaelbrewer sounds like a duplicate of #26380.
Comment From: michaelbrewer
Ok thanks so you are not supposed to use @AuthenticationPrincipal ?
Comment From: michaelbrewer
So removing the @AuthenticationPrincipal works in the controller example
OR putting in this.
@Configuration
class WebConfig : WebMvcConfigurer {
override fun addArgumentResolvers(resolvers: MutableList<HandlerMethodArgumentResolver>) {
super.addArgumentResolvers(resolvers)
resolvers.add(PrincipalMethodArgumentResolver())
}
}
Comment From: rstoyanchev
so you are not supposed to use @AuthenticationPrincipal ?
No, it's not that you are not supposed but that it doesn't provide what you are trying to get. Your argument is Authentication but as per the Javadoc the annotation provides access to Authentication#getPrincipal(). The former is Principal, the latter is not.
Comment From: michaelbrewer
so you are not supposed to use @AuthenticationPrincipal ?
No, it's not that you are not supposed but that it doesn't provide what you are trying to get. Your argument is
Authenticationbut as per the Javadoc the annotation provides access toAuthentication#getPrincipal(). The former isPrincipal, the latter is not.
Thanks for the clarification, interesting that adding PrincipalMethodArgumentResolver allows for this “bug” to continue to work.
Comment From: rstoyanchev
It is a bit confusing because the word "principal" is used in multiple contexts but there is no bug I think.
Authenticationis the top-level container class from Spring Security that implementsPrincipal. It can be injected into a controller method without any annotations because it is accessible viaHttpServletRequest#getUserPrincipal.Authentication#getPrincipal()exposes the user identity which as explained in the Javadoc can be as simple as a String but more likely aUserDetailsor some other custom user object that is created by theAuthenticationManager.
@AuthenticationPrincipal provides a shortcut for 2), i.e. to the UserDetails or the some custom user object, without having to go through Spring Security's Authentication. This object is typically not a Principal but it could be.
Therefore when injecting with the annotation, it is important to use the correct user type. At the same time it is perfectly legitimate to inject Authentication or Principal if what you want is 1).
Comment From: rstoyanchev
In addition to the above change, the wiki has also been updated.