[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: Text File nginx.conf    
Issue Links:
Cloners
Problem/Incident
causality
dependency
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: DeveloperX
Work Started:

 Description   

Description

The 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

  1. Create a rewrite rule (in Magnolia) to redirect /events.html into /about/events.html
  2. Access to http://localhost/events.html (with the correct port and context) with Postman (or curl) using http and sending X-Forwarded-Proto = https
  3. Magnolia returns a 301 response to http://localhost/about/events.html

Expected results

The redirection should be to https://localhost/about/events.html

Development notes

The 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

Generated at Mon Feb 12 04:34:40 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.