Add req.host to targetURL when redirecting to https (#668)
* Add req.host to targetURL when redirecting to https The req.URL.Host might not be present when redirecting to https if the given req.URL is something like "/". In such scenario, the req.Host is still present and valid. This commit adds the original req.Host to the targetURL before returning the 308 status, to avoid having a `Location: https:///` in the response. * Bring back empty line * Wrap the setting of targetURL.Host in a condition * Add a comment to the test explaining why the redirectURL includes example.com * Add changelog entry
This commit is contained in:
		
							parent
							
								
									d29766609b
								
							
						
					
					
						commit
						a999270cf3
					
				|  | @ -8,6 +8,7 @@ | ||||||
| 
 | 
 | ||||||
| ## Changes since v6.0.0 | ## Changes since v6.0.0 | ||||||
| 
 | 
 | ||||||
|  | - [#668](https://github.com/oauth2-proxy/oauth2-proxy/pull/668) Use req.Host in --force-https when req.URL.Host is empty (@zucaritask) | ||||||
| - [#660](https://github.com/oauth2-proxy/oauth2-proxy/pull/660) Use builder pattern to simplify requests to external endpoints (@JoelSpeed) | - [#660](https://github.com/oauth2-proxy/oauth2-proxy/pull/660) Use builder pattern to simplify requests to external endpoints (@JoelSpeed) | ||||||
| - [#591](https://github.com/oauth2-proxy/oauth2-proxy/pull/591) Introduce upstream package with new reverse proxy implementation (@JoelSpeed) | - [#591](https://github.com/oauth2-proxy/oauth2-proxy/pull/591) Introduce upstream package with new reverse proxy implementation (@JoelSpeed) | ||||||
| - [#576](https://github.com/oauth2-proxy/oauth2-proxy/pull/576) Separate Cookie validation out of main options validation (@JoelSpeed) | - [#576](https://github.com/oauth2-proxy/oauth2-proxy/pull/576) Separate Cookie validation out of main options validation (@JoelSpeed) | ||||||
|  |  | ||||||
|  | @ -38,6 +38,11 @@ func redirectToHTTPS(httpsPort string, next http.Handler) http.Handler { | ||||||
| 		// Set the scheme to HTTPS
 | 		// Set the scheme to HTTPS
 | ||||||
| 		targetURL.Scheme = httpsScheme | 		targetURL.Scheme = httpsScheme | ||||||
| 
 | 
 | ||||||
|  | 		// Set the req.Host when the targetURL still does not have one
 | ||||||
|  | 		if targetURL.Host == "" { | ||||||
|  | 			targetURL.Host = req.Host | ||||||
|  | 		} | ||||||
|  | 
 | ||||||
| 		// Overwrite the port if the original request was to a non-standard port
 | 		// Overwrite the port if the original request was to a non-standard port
 | ||||||
| 		if targetURL.Port() != "" { | 		if targetURL.Port() != "" { | ||||||
| 			// If Port was not empty, this should be fine to ignore the error
 | 			// If Port was not empty, this should be fine to ignore the error
 | ||||||
|  |  | ||||||
|  | @ -154,5 +154,15 @@ var _ = Describe("RedirectToHTTPS suite", func() { | ||||||
| 			expectedStatus: 200, | 			expectedStatus: 200, | ||||||
| 			expectedBody:   "test", | 			expectedBody:   "test", | ||||||
| 		}), | 		}), | ||||||
|  | 		// By using newRequest from httptest we get example.com as a Host
 | ||||||
|  | 		// when the target is just a path.
 | ||||||
|  | 		// For details: https://golang.org/pkg/net/http/httptest/#NewRequest
 | ||||||
|  | 		Entry("without TLS with a path as request", &requestTableInput{ | ||||||
|  | 			requestString:    "/", | ||||||
|  | 			useTLS:           false, | ||||||
|  | 			expectedStatus:   308, | ||||||
|  | 			expectedBody:     permanentRedirectBody("https://example.com/"), | ||||||
|  | 			expectedLocation: "https://example.com/", | ||||||
|  | 		}), | ||||||
| 	) | 	) | ||||||
| }) | }) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue