Skip to content
This repository has been archived by the owner on May 31, 2022. It is now read-only.

[SECOAUTH-285] Security expression handler should only throw exception at the end of the decision (if that's possible) #118

Closed
dsyer opened this issue Jan 9, 2014 · 0 comments

Comments

@dsyer
Copy link
Contributor

dsyer commented Jan 9, 2014

Priority: Major
Original Assignee: Rob Winch
Reporter: Dave Syer
Created At: Tue, 12 Jun 2012 10:12:20 +0100
Last Updated on Jira: Fri, 12 Apr 2013 09:16:13 +0100

Security expression handler should only throw exception at the end of the decision (if that's possible). The current implementation is OK for simple use cases where there is only one or multiple expressions combined with AND, but with OR, you don't want one branch of the condition to throw an exception without the other having a chance to be evaluated.

Comments:

david_syer on Tue, 12 Jun 2012 15:47:06 +0100

Done: revised exception handling in expression handlers

  • Using a shared expression root for the oauth bits makes sense
  • Need to use ThreadLocal (boo) to work around a limitation of
    SpEL
  • Added special wrapper method oauthSufficientScope(...) to be
    used where complex expressions might test for invalid scopes
    but eventually still result in a positive access decision

rwinch on Fri, 15 Jun 2012 14:52:29 +0100

Instead we will leverage SEC-1971. This avoids the ThreadLocal which causes a memory leak in the application.

david_syer on Fri, 15 Jun 2012 17:28:12 +0100

An interim solution until SEC-1971 is resolved and the new Spring Security is released is to set throwExceptionOnInvalidScope=false in the expression handler, and then use #oauth2.sufficientScope() to wrap all the expressions. Maybe we can even get something that works with both versions of Spring Security if we retain that flag?

rwinch on Fri, 15 Jun 2012 19:23:56 +0100

Pull request #41

@dsyer dsyer modified the milestones: 2.0.0.RC1, 2.0.0 Mar 31, 2014
@rwinch rwinch closed this as completed in ef27402 Mar 31, 2014
dsyer added a commit that referenced this issue Apr 2, 2014
... otherwise the AccessDeniedHandler doesn't get a chance to send
the proper response and user just sees 500 status

Fixes gh-118
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

1 participant