NullPointerException when GrantedAuthority.getAuthority() returns null about spring-security HOT 3 OPEN

Thanks for reaching out @Crystark!

I can return null as a response to getAuthority which is allowed as per the documentation:

Please consider the entirety of the method's documentation:

If the GrantedAuthority can be represented as a String and that String is sufficient in precision to be relied upon for an access control decision by an AccessDecisionManager (or delegate), this method should return such a String.

If the GrantedAuthority cannot be expressed with sufficient precision as a String, null should be returned. Returning null will require an AccessDecisionManager (or delegate) to specifically support the GrantedAuthority implementation, so returning null should be avoided unless actually required.

Note: Unfortunately, this documentation doesn't mention AuthorizationManager (or the reactive counterpart) so I think it would be worth updating this documentation.

The point the docs are making is that the thing consuming the GrantedAuthority has to understand null and/or your GrantedAuthority implementation. The AuthorityReactiveAuthorizationManager does not handle this, and so you would need to provide a custom implementation.

I expect org.springframework.security.authorization.AuthorityReactiveAuthorizationManager#check to not crash on a null authority and just filter it out.

I'm not really sure it is valuable or desirable to handle null authorities by default, as this would likely hide a problem from the user. In your case, what is expected to handle the authority that is returning null?

This is actually an indication that you may have reversed the role of ReactiveAuthorizationManager and GrantedAuthority by placing logic in the GrantedAuthority itself, and in fact the logic in your custom authority should be extracted into a custom ReactiveAuthorizationManager instead.

Having said all of that, I feel that instead of ignoring null, it could be beneficial for AuthorityReactiveAuthorizationManager to return a more informative error message using Assert.hasText(). Would you be interested in submitting a PR for this?

from spring-security.

Thanks for getting back to me so fast on this @sjohnr

In your case, what is expected to handle the authority that is returning null?

The authority returning null is used in my PermissionEvaluator triggered on @PreAuthorize:

class MyPermissionEvaluator : PermissionEvaluator {
    override fun hasPermission(authentication: Authentication, targetDomainObject: Any?, permission: Any): Boolean {
        throw NotImplementedError("Use hasPermission(targetId, targetType, permission) or hasAuthority(authority) instead")
    }

    override fun hasPermission(
        authentication: Authentication,
        targetId: Serializable?,
        targetType: String,
        permission: Any
    ): Boolean =
        if (targetId == null) {
            log.warn(
                "Permission denied: unexpected null 'targetId' argument for target type '{}' and permission '{}'",
                targetType,
                permission
            )
            false
        } else if (permission !is String) {
            log.warn("Permission denied: 'permission' argument ({}) should be a String", permission.javaClass.name)
            false
        } else {
            authentication.authorities
                .filterIsInstance<MyAuthority>()
                .any { it.hasPermission(permission, targetType, targetId.toString()) }
        }

    companion object {
        val log: Logger = LoggerFactory.getLogger(MyPermissionEvaluator::class.java)
    }
}

And this is how I use this:

@PreAuthorize("hasPermission(#someId, '$RESOURCE_TYPE', '$RESOURCE_SCOPE')")

This is working fine by the way.
I only got the error I mentioned when the null-returning GrantedAuthority was moved in the JWT token before the access-returning one and because I'm using hasAuthority("access")

So I'm not directly using AuthorityReactiveAuthorizationManager right ? Maybe I should ?

The reason I did everything like that is that I wanted to be able to extract the various permissions from my JWT token and make them available to my PermissionEvaluator as well as be able to use hasAuthority("access"). Maybe I'm mixing up things that should not be ?
Do you have any suggestions ? I'm already thinking I can just return a random string instead of null 😏 but that feels a bit hacky and I'd rather try and do this the right way. Well actually I would probably concatenate the scope and the permission separated by # but still doesn't really feel right.

Thanks

from spring-security.

@Crystark thanks for your reply.

While reviewing your code, I am having a bit of difficulty understanding it and the intent of it, but it appears to be all about a resource name taken from a JWT and combined with a list of scopes. There’s a lot to unpack there and this isn’t really the place to work through your solution. I can tell you however that I’m fairly certain there are improvements to be made. If you would like to work through that, please open a stack overflow question and provide the link here and I’ll be happy to take a look.

In any case, I think returning null for the authority requires a custom AuthorizationManager which you have not provided. I feel the best we can do here is throw a more informative exception.

from spring-security.