[MAGNOLIA-3863] An additional security filter which handles callbacks on behalf of the existing UriSecurityFilter and ContentSecurityFilter Created: 19/Oct/11 Updated: 25/Apr/12 Resolved: 09/Mar/12 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core, security |
| Affects Version/s: | None |
| Fix Version/s: | 4.5 |
| Type: | New Feature | Priority: | Neutral |
| Reporter: | Magnolia International | Assignee: | Magnolia International |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Rationale: we currently have 2 security filters, which among other things have duplicated configuration (the "callback", which presents the client with a login form). On top of this, with Implementation:
This way, any component down the filter chain can set a 401 or 403 response code, or throw an AccessDeniedException, and we'll send an appropriate response to the user. TBD: how does this behave if rendering has begun ? It is expected that an AccessDeniedException or other exception happening at that level would not be let up the chain. |
| Comments |
| Comment by Danilo Ghirardelli [ 19/Oct/11 ] |
|
IMHO using the reponse code directly to execute the callback is a bit "limiting"... It might be perfectly right on authoring where you have to pass through Magnolia login anyway, but on public instances, (expecially if some other framework is integrated) it could be legit to send a 401 or a 403 directly to the client, for parts that just happen to pass the Magnolia filter chain but are not strictly Magnolia-related. (first examples are spring security logins or any sso integration for public users) The idea of the exception might be the same discussed in |
| Comment by Magnolia International [ 02/Nov/11 ] |
|
Still to do: an update task to check (and move/backup if necessary) the callbacks configured in the contentSecurity filter. |
| Comment by Magnolia International [ 02/Nov/11 ] |
|
Another thing to take into account. When setting a status with setStatus, we don't commit the response, everything is fine. I changed the few places in our filters where sendError was used. When sendError is used, the response is committed, and that means we can't do a redirect, for example. The RedirectClientCallback can be useful in SSO scenarios (the login form is on a different host). And... I've seen a bunch of code in our codebase that uses sendError (so the RedirectClientCallback never really worked for those). Should we change that ? Or change the behavior of the ResponseWrapper in the SecurityCallbackFilter ? |
| Comment by Daniel Lipp [ 14/Nov/11 ] |
|
Along with SCRUM-604 I reverted using setStatus in AggregatorFilter. When encountering a 404 using setStatus is not enough. getErrorStream will then return null and a User would get an empty page. Only other option I could think of would be to use setStatus in AggregatorFilter and fix the errorStream later in SecurityCallbackFilter#doFilter(HttpServletRequest, HttpServletResponse, FilterChain). This does not seem to be a superior solution. |