[MAGNOLIA-6043] LoginFilter: Allow dynamic redirects after authentication Created: 13/Jan/15 Updated: 28/Sep/17 Resolved: 31/Mar/15 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | security |
| Affects Version/s: | None |
| Fix Version/s: | 5.3.8 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Christian Ringele | Assignee: | Evzen Fochr |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||||||||||||||
| Release notes required: |
Yes
|
||||||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||||||
| Description |
|
Due to MAGNOLIA-5991 the redirect behavior after authentication was changed in the LoginFilter. This has some negative impacts to customer implementations regarding public user related "portals", in short words of the customer: |
| Comments |
| Comment by Jan Haderka [ 13/Jan/15 ] |
|
This request is quite problematic and will be perceived by many as security vulnerability. You don't want anyone to be able to craft a request with redirect that will be followed after user logs in automatically without user knowing about it. Having such "feature" would allow attacker to craft request that can delete, or publish content or change configuration, so if going ahead with it, implementation should still require some sort of predefined targets that are allowed and have admin explicitly configure what is allowed with nothing being allowed out of the box to prevent exploits. |
| Comment by Edgar Vonk [ 13/Jan/15 ] |
|
Hi Jan, I sort of understand where you are going but have some remarks: First of all I think most users will actually expect such a feature, instead of being surprised by it, as they are accustomed to it from other web sites. An example in our Magnolia-powered ResearchAnt Hub site maybe clarifies this. On a 'compound' page such as the one for penicillin a registered user needs to log in in order to use specific functionality (such as 'suggest a new related microorganism'). We have a link on this (dynamic) page to the login page. After logging in the user is redirected back to this specific compound page so that he/she can perform the specific action (ps: this feature has not been deployed yet on the site). The user will actually expect this to happen in our view. Regarding security: the 'only' thing we would like in this feature is the ability to redirect to a (relative) target URL after login. How could this be crafted to delete or publish content etc? Using the referrer as target page after login could be an option but then we need a way to transfer the referrer across sessions since the logged in user gets a completely new session. I have not delved into this but would be curious to see how other CMSs have implemented this? cheers Edgar |
| Comment by Jan Haderka [ 14/Jan/15 ] |
|
I'm not saying it can't be done, I'm saying we need to be careful about what we allow to be done. The case you describe is for example fine |
| Comment by Edgar Vonk [ 14/Jan/15 ] |
|
Clear! And I agree about being careful. |
| Comment by Evzen Fochr [ 24/Mar/15 ] |
|
Solution 1 - in MAGNOLIA-5991 (5.3.6) was added redirect in LoginFilter, but that brokes logic of redirecting in AuthenticationModel@earlyExecute because mgnlModelExecutionUUID param is lost due to redirect, so to correct it and still do POST/REDIRECT/GET pattern we need to move redirect to self from LoginFilter after conditional redirect done in AuthenticationModel@earlyExecute (ModelExecutionFilter) --not used |
| Comment by Evzen Fochr [ 31/Mar/15 ] |
|
I will use RequestDispatchUtil for redirecting, it should take care of ContextPath if missing and log error if redirect fails. Test is present in this UtilTest class |
| Comment by Evzen Fochr [ 01/Apr/15 ] |
|
Solution 2 - added mgnlReturnTo param to loginForm, in loginFilter after successful login we redirect to mgnlReturnTo if present or to requestURL as fallback location --implemented |
| Comment by Edgar Vonk [ 01/Apr/15 ] |
|
Thanks guys! Looking forward to start using this functionality in our projects. |