Describe the bug
The JSpecify null safety annotation on the generic type of AuthorizationManager
(and ReactiveAuthorizationManager
) is specified incorrectly. It is currently in the place of a concrete type (e.g. <@Nullable String>
) but is a generic type. See Generics in the JSpecify User Guide.
Since the package is declared with @NullMarked
, my IDE (IntelliJ, which is configured to use JSpecify) recognizes the type as @NonNull
(e.g. <T extends @NonNull Object>
). Attempts to pass null
or a @Nullable
object to an authorization manager result in a warning (e.g. "Argument 'this.object' might be null").
In addition, the <@Nullable T>
generic type is also flagged as a warning ("Nullability annotation is not applicable to type parameters").
Expected behavior
It appears the intended behavior is that we can pass null to an AuthorizationManager
. I think there are two possible ways to declare this.
Option 1
We can change the declaration to:
@FunctionalInterface
public interface AuthorizationManager<T extends @Nullable Object> {
...
}
This declares the upper bound of T as nullable instead of the implicit definition, which is <T extends @NonNull Object>
because of @NullMarked
.
Option 2
Perhaps simpler, we change the methods themselves to simply accept null:
@FunctionalInterface
public interface AuthorizationManager<T> {
default void verify(Supplier<Authentication> authentication, @Nullable T object) {
...
}
@Nullable AuthorizationResult authorize(Supplier<Authentication> authentication, @Nullable T object);
}
This is quite a bit easier for most users to understand, and achieves the same thing (I think).
In both cases however, the problem is that all authorization managers must now declare and handle null. There are some authorization managers that appear designed to accept a non-null parameter. For example, SecuredAuthorizationManager
does not handle a null input. It may be possible to just accept a warning for this and similar cases but this may not be ideal. A possible solution is to add explicit null handling. For example, with option 2:
public final class SecuredAuthorizationManager implements AuthorizationManager<MethodInvocation> {
...
@Override
public @Nullable AuthorizationResult authorize(Supplier<Authentication> authentication, @Nullable MethodInvocation mi) {
Assert.notNull(mi, "methodInvocation cannot be null");
...
}
}
But it is odd to do this. So I'm not sure exactly the best way forward...
Related gh-17534