[MAGNOLIA-8661] Magnolia does not honor X-Forwarded-Proto header processing redirections Created: 29/Nov/22 Updated: 29/Mar/23 Resolved: 29/Mar/23 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | None |
| Affects Version/s: | 6.2.26 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Jesus Alonso | Assignee: | Marek Strucka |
| Resolution: | Won't Fix | Votes: | 2 |
| Labels: | virtualMappingModule | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||
| 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)
|
||||||||||||||||
| Bug DoR: |
[X]*
Steps to reproduce, expected, and actual results filled
[X]*
Affected version filled
|
||||||||||||||||
| Date of First Response: | |||||||||||||||||
| Epic Link: | Support | ||||||||||||||||
| T-Shirt Size: | X-Small | ||||||||||||||||
| Team: | |||||||||||||||||
| Work Started: | |||||||||||||||||
| Description |
DescriptionThe X-Forwarded-Proto header is a de-facto standard header for identifying the protocol (HTTP or HTTPS) that a client used to connect to your proxy or load balancer. In deployments where a proxy (e.g. Fastly in PaaS) acts as TLS terminator with the clients but the proxy connects with Magnolia using http, Magnolia process the redirections using http. However, when Magnolia receives the X-Forwarded-Proto header, it should create the redirections using https, considering it received the X-Forwarded-Proto = https. Steps to reproduce
Expected resultsThe redirection should be to https://localhost/about/events.html Development notesThe class RequestDispatchUtil creates the new URL with this code: if (isInternal(permanentUrl)) { if (isUsingStandardPort(request)) { permanentUrl = new URL( request.getScheme(), request.getServerName(), request.getContextPath() + permanentUrl).toExternalForm(); The problem is that request.getScheme() returns http, as the connection with Magnolia uses http. We should use the header X-Forwarded-Proto if it is set. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto Reading about how Fastly behaves (https://docs.fastly.com/en/guides/tls-termination), it seems it uses a different header and different value for notifying that the client is using https, so it seems it is needed to either allow configuring the value of the header and the value that corresponds to https or maybe add different strategies and allow configuring one. |
| Comments |
| Comment by Jesus Alonso [ 14/Mar/23 ] |
|
Hello mstrucka , please take into account that the original ticket (https://jira.magnolia-cms.com/browse/HELPDESK-3539), which affects Magnolia corporate site, does not use Nginx but Fastly. So the solution should be flexible enough to fix the issue we have in our corporate site. |
| Comment by Marek Strucka [ 15/Mar/23 ] |
|
Hi, yeah, that would be the same idea, but with checking for resp.http.X-Is-SSL = "yes" I am gonna speak to better informed people from devx if the fix should be there or if there is another place to configure such things, will come back with possible solutions.{} |
| Comment by Viet Nguyen [ 27/Mar/23 ] |
|
Thanks to the.tran, We could also configure nginx to support this case on our PAAS by adding the following annotations to the ingress: nginx.ingress.kubernetes.io/proxy-redirect-from: http:// nginx.ingress.kubernetes.io/proxy-redirect-to: https:// This should be fixed in either ingress level or CDN Fastly level, but not Magnolia because internally Magnolia Virtual URI mapping works by its design already. Hope this helps! |
| Comment by Marek Strucka [ 29/Mar/23 ] |
|
closing this ticket as won't do for DevX, thanks for your help guys, feel free to come back to us when needed |