Skip to content
This repository was archived by the owner on Dec 7, 2020. It is now read-only.

Added --preserve-host option to retain host header in upstream request. #328

Merged
merged 3 commits into from
Jul 5, 2018
Merged

Added --preserve-host option to retain host header in upstream request. #328

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca5e1aa
Added --preserve-host option to retain host header in upstream request.
Mar 8, 2018
bac1368
Merge branch 'master' into preserve_host
jgroffen Jun 27, 2018
c20a59e
Merge branch 'master' into preserve_host
gambol99 Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ FEATURES:
* Added spelling check to the tests [#PR322](https://github.com/gambol99/keycloak-proxy/pull/322)
* Added the X-Auth-Audience to the upstream headers [#PR319](https://github.com/gambol99/keycloak-proxy/pull/319)
* Added the ability to control the timeout on the initial openid configuration from .well-known/openid-configuration [#PR315](https://github.com/gambol99/keycloak-proxy/pull/315)
* Added a --preserve-host option to preserve the host header of the proxied request in the upstream request [#PR328](https://github.com/gambol99/keycloak-proxy/pull/328)
* Added the feature to customize the oauth prefix (defaults to /oauth) [#PR326](https://github.com/gambol99/keycloak-proxy/pull/326)
* Adding additional metrics covering provider request latency, token breakdown [#PR324](https://github.com/gambol99/keycloak-proxy/pull/324)
* Changed the upstream-keepalive to default to true [#PR321](https://github.com/gambol99/keycloak-proxy/pull/321)
Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ GLOBAL OPTIONS:
--upstream-ca value the path to a file container a CA certificate to validate the upstream tls endpoint
--resources value list of resources 'uri=/admin*|methods=GET,PUT|roles=role1,role2'
--headers value custom headers to the upstream request, key=value
--preserve-host preserve the host header of the proxied request in the upstream request (default: false)
--enable-logout-redirect indicates we should redirect to the identity provider for logging out (default: false)
--enable-default-deny enables a default denial on all requests, you have to explicitly say what is permitted (recommended) (default: false)
--enable-encrypted-token enable encryption for the access tokens (default: false)
Expand Down
1 change: 1 addition & 0 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func newDefaultConfig() *Config {
MatchClaims: make(map[string]string),
OAuthURI: "/oauth",
OpenIDProviderTimeout: 30 * time.Second,
PreserveHost: false,
SecureCookie: true,
ServerIdleTimeout: 120 * time.Second,
ServerReadTimeout: 10 * time.Second,
Expand Down
2 changes: 2 additions & 0 deletions doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ type Config struct {
Resources []*Resource `json:"resources" yaml:"resources" usage:"list of resources 'uri=/admin*|methods=GET,PUT|roles=role1,role2'"`
// Headers permits adding customs headers across the board
Headers map[string]string `json:"headers" yaml:"headers" usage:"custom headers to the upstream request, key=value"`
// PreserveHost preserves the host header of the proxied request in the upstream request
PreserveHost bool `json:"preserve-host" yaml:"preserve-host" usage:"preserve the host header of the proxied request in the upstream request"`

// EnableLogoutRedirect indicates we should redirect to the identity provider for logging out
EnableLogoutRedirect bool `json:"enable-logout-redirect" yaml:"enable-logout-redirect" usage:"indicates we should redirect to the identity provider for logging out"`
Expand Down
2 changes: 1 addition & 1 deletion forwarding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (r *oauthProxy) proxyMiddleware(next http.Handler) http.Handler {
if v := req.Header.Get("Host"); v != "" {
req.Host = v
req.Header.Del("Host")
} else {
} else if !r.config.PreserveHost {
Copy link
Contributor

@gambol99 gambol99 Mar 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @jgroffen ... so flag is used to control the Host header from the incoming request be preserved for upstream, which the if statement just above is doing. .. perhaps i'm missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @gambol99, I found that without this patch all code paths would change the host header of the upstream request - either to the configured upstream host name or an overridden header in the headers config. Maybe there was something screwy with my config though, I'll mark it as WIP and check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @gambol99 - I'm finding that this if statement on line 50 is never equating to true - I don't know why because the incoming request does have a host header.

I verified the following configuration:

listen : kcproxy.internal.com
upstream-url : webserver.internal.com

... when keycloak-proxy is behind a public-facing proxy with a request chain that looks like:

  • public.com --proxy to--> kcproxy.internal.com --proxy to--> webserver.internal.com

... produces the following results:

  1. request to public.com host header: public.com
  2. request to kcpublic.internal.com host header: public.com
  3. request to webserver.internal.com host header: webserver.internal.com

Note that PreserveHost is enabled for the public.com proxy - an Apache HTTPD proxy in this case.

I verified this result using the 2.2.1 released binary. I also checked a request with a valid token vs a request that has no auth yet and performs OIDC redicrect to keycloak (where SSO happens).

If the preserve-host parameter is set in my changed codebase, the following happens instead:

  1. request to public.com host header: public.com
  2. request to kcpublic.internal.com host header: public.com
  3. request to webserver.internal.com host header: public.com

We made a change where we don't need to preserve the host header anymore (why I took so long to get back to you, sorry :( ). Preserving host headers is not a preferred approach, but some applications rely on it.

req.Host = r.endpoint.Host
}

Expand Down