SEC-1525: PostAuthorize being evaluated even on exception #1766

Closed
spring-issuemaster opened this Issue Jul 28, 2010 · 4 comments

1 participant

@spring-issuemaster

christophe blin (Migrated from SEC-1525) said:

Given the code :
@PostAuthorize(hasPermission(returnObject.field, 'admin'))
MyObject someMethod();

when someMethod throws an exception, the PostAuthorize processing throws an exception :
org.springframework.expression.spel.SpelEvaluation Exception: EL1007Epos 27): Field or property 'field' cannot be found on null

IMHO, the PostAuthorize should not be evaluated when returnObject is null (exactly like PostFilter)
This is possible if we consider that PostAuthorize is a nonsense if you do not use the returnObject inside the expression (and IMHO this consideration is true because if you do not use returnObject in the expression then you could have use PreAuthorize...)

By code inspection, the problem comes from ExpressionBasedPostInvocationAdvice :

when you do :
if (postAuthorize != null && !ExpressionUtils.evaluateAsBoolean(postAuthorize, ctx))

you should have done :
if (postAuthorize != null && returnedObject != null && !ExpressionUtils.evaluateAsBoolean(postAuthorize, ctx))

BTW, this is exactly what you do in PostFilter 10 lines above...

@spring-issuemaster

Luke Taylor said:

I disagree. I don't think it's "a nonsense" to consider the possibility that there may be use cases where the authorization decision can only realistically be made after the method invocation has taken place, and which may not require the use of the returned object (or even have one).

I'm not sure what the best option is for dealing gracefully with an exception condition, though. It might make more sense if the MethodSecurityInterceptor didn't actually perform any after-invocation checks at all.

@spring-issuemaster

christophe blin said:

At the moment, from the moment the annotated method may throw an exception (i.e > 90% of the cases), the current implementation requires that the programmer writes PostAuthorize like this :
@PostAuthorize("returnObject == null or returnObject.res.libraryId == #libraryId")

If he does not, a NPE will be thrown when the exception occurs...

Please, provide a use case where the user can not rewrite its PostAuthorize with a PreAuthorize when the returnObject is not used so that I am comfortable with the idea that I do not write all this "returnObject == null or " for nothing

@spring-issuemaster

christophe blin said:

and please, if the implementation stays the same, update the documentation to mention that returnObject is null when the method has thrown an exception so that programmers know they have this test to add to each and every PostAuthorize of their app

@spring-issuemaster

Luke Taylor said:

See SEC-1635

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment