[MAGNOLIA-3291] Offer an SSL-aware cache policy Created: 04/Sep/10 Updated: 14/Dec/11 Resolved: 15/Nov/11 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | cache |
| Affects Version/s: | 4.3.5 |
| Fix Version/s: | 4.4.6 |
| Type: | Improvement | Priority: | Minor |
| Reporter: | Will Scheidegger | Assignee: | Ondrej Chytil |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | SSL, cache, protocol | ||
| Remaining Estimate: | 0.5h | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | 0.5h | ||
| Attachments: |
|
| Template: |
|
| Patch included: |
Yes
|
| 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)
|
| Date of First Response: |
| Description |
|
Quite often you would like to have some pages of your website served SSL-encoded via https (e.g. forms etc.) and the rest regularly via http. In these cases it is the easiest for the editor if he/she can simply check a checkbox in the page properties marking the page as "secure". Then a redirect filter could make sure that the page is actually served via https or vice versa when navigating from a secure page to a regular one. Say /foo/bar.html is marked as "secure" in the page properties. Redirecting from http://www.mydomain.com/context/foo/bar.html to https://www.mydomain.com/context/foo/bar.html however will not work with the default caching mechanism. The first request to /foo/bar.html (via http) will return a redirect to https. This redirect is then cached and all subsequent requests for /foo/bar.html (via http AND https) will return the cached redirect... leading to endless redirects. The proposed extension (see attached patch and classes) will solve this problem by providing an SSLAware cache policy. It extends the default cache policy and simply also takes in respect if the request for a page was secure or not. |
| Comments |
| Comment by Magnolia International [ 24/Feb/11 ] |
|
Hey why hasn't this been applied yet !? Thanks Will. |
| Comment by Will Scheidegger [ 09/Nov/11 ] |
|
Yes, why hasn't this been applied yet? I just now noticed it when we tried to implement a http/https redirect in Mag 4.4.2. |
| Comment by Will Scheidegger [ 09/Nov/11 ] |
|
Modified both new classes to correspond to the Magnolia checkstyle rules (header, comments). Hope this will help for the patch to be applied. |
| Comment by Danilo Ghirardelli [ 11/Nov/11 ] |
|
I agree with the issue, it was a problem also for me a few times. But I have to point out just one thing before the idea of "https checkbox" becomes a widespread practice. The idea is good for some cases (and we are also using it) but it's not always applicable: when you call (post) a page with http and then do the redirect, the page in called first with all parameters unencrypted, then the secure channel is opened and then the same params are sent again, encrypted. This behaviour will in fact void all the https meaning, because params (like the password) are sent in plain text before the redirect. It might be acceptable if you just want the "nice feeling" of security in some pages that are not that sensitive, and is anyway a good "patch" if some user try to access secured pages directly. The right solution is to create link with the explicit https scheme whenever you link one of the page with the checkbox. |
| Comment by Magnolia International [ 11/Nov/11 ] |
|
Are existing caches still compatible with the code after this merge ? |
| Comment by Will Scheidegger [ 11/Nov/11 ] |
|
@Danilo: That's why we instruct our users that both the form AND the target needs to be https. Actually the target would not even have to be https with the form module since the form page is sent to "itself" and then forwarded to the target page. But it's good practice in case they should have custom functionality somewhere which would call some target/action page directly with the parameters. @Grégory: Since it's just an additional policy nothing changes for users who don't specifically choose this policy. But someone more knowledgeable than me should actually test the cache with the policy activated. So far it worked for us. |
| Comment by Will Scheidegger [ 11/Nov/11 ] |
|
Problem discovered? |
| Comment by Ondrej Chytil [ 15/Nov/11 ] |
|
@Will: Thanks for patch. |
| Comment by Magnolia International [ 21/Nov/11 ] |
|
By the following I meant to ask if persisted (serialized) cache data would still be working (unserializable) after an update. (and it would indeed probably be less of a problem if it were a different class as originally proposed) |
| Comment by Will Scheidegger [ 26/Nov/11 ] |
|
Spent a few hours debugging the problem I've discovered above. I don't think it has anything to do with the SSL-awarenes of the cache policy nor with the AuthenticatedVoter. Here a short summary of the debugging:
So I think the patch above is "safe" and the problem is most likely somewhere else. |