From d87453446b6dbe6aea36d069dac7aef7b42e6c5e Mon Sep 17 00:00:00 2001 From: fredbi <frederic@oneconcern.com> Date: Fri, 23 Aug 2019 18:46:32 +0200 Subject: [PATCH] [KEYCLOAK-9045] Add CORS headers on proxied upstream responses (#441) * fixes #9045 * enables CORS debug logging with Verbose option * Fix possible conflict with upstream serving CORS * Revert on optionality of sending headers to upstream Signed-off-by: Frederic BIDON <frederic@oneconcern.com> --- doc.go | 1 - e2e_test.go | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++ forwarding.go | 4 ++ server.go | 6 ++ 4 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 e2e_test.go diff --git a/doc.go b/doc.go index f0686fd..9986990 100644 --- a/doc.go +++ b/doc.go @@ -295,7 +295,6 @@ type Config struct { CorsCredentials bool `json:"cors-credentials" yaml:"cors-credentials" usage:"credentials access control header (Access-Control-Allow-Credentials)"` // CorsMaxAge is the age for CORS CorsMaxAge time.Duration `json:"cors-max-age" yaml:"cors-max-age" usage:"max age applied to cors headers (Access-Control-Max-Age)"` - // Hostnames is a list of hostname's the service should response to Hostnames []string `json:"hostnames" yaml:"hostnames" usage:"list of hostnames the service will respond to"` diff --git a/e2e_test.go b/e2e_test.go new file mode 100644 index 0000000..37a6bce --- /dev/null +++ b/e2e_test.go @@ -0,0 +1,164 @@ +/* +Copyright 2015 All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "io" + "io/ioutil" + "log" + "net/http" + "net/url" + "path" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +const ( + e2eUpstreamURL = "/upstream" + e2eUpstreamListener = "127.0.0.1:12345" + e2eProxyListener = "127.0.0.1:54321" + e2eOauthListener = "127.0.0.1:23456" + e2eOauthURL = "/.well-known/openid-configuration" +) + +// checkListenOrBail waits on a endpoint listener to respond. +// This avoids race conditions with test listieners as go routines +func checkListenOrBail(endpoint string) bool { + const ( + maxWaitCycles = 10 + waitTime = 100 * time.Millisecond + ) + checkListen := http.Client{} + _, err := checkListen.Get(endpoint) + limit := 0 + for err != nil && limit < maxWaitCycles { + time.Sleep(waitTime) + _, err = checkListen.Get(endpoint) + limit++ + } + return limit < maxWaitCycles +} + +func runTestGatekeeper(t *testing.T, config *Config) error { + proxy, err := newProxy(config) + if err != nil { + return err + } + _ = proxy.Run() + if !assert.True(t, checkListenOrBail("http://"+config.Listen+"/oauth/login")) { + err := fmt.Errorf("cannot connect to test http listener on: %s", "http://"+config.Listen+"/oauth/login") + t.Logf("%v", err) + t.FailNow() + return err + } + return nil +} + +func runTestUpstream(t *testing.T) error { + go func() { + upstreamHandler := func(w http.ResponseWriter, req *http.Request) { + _, _ = io.WriteString(w, `{"message": "test"}`) + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Upstream-Response-Header", "test") + } + http.HandleFunc(e2eUpstreamURL, upstreamHandler) + _ = http.ListenAndServe(e2eUpstreamListener, nil) + }() + if !assert.True(t, checkListenOrBail("http://"+path.Join(e2eUpstreamListener, e2eUpstreamURL))) { + err := fmt.Errorf("cannot connect to test http listener on: %s", "http://"+path.Join(e2eUpstreamListener, e2eUpstreamURL)) + t.Logf("%v", err) + t.FailNow() + return err + } + return nil +} + +func runTestAuth(t *testing.T) error { + go func() { + configurationHandler := func(w http.ResponseWriter, req *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{ + "issuer": "http://`+e2eOauthListener+`", + "subject_types_supported":["public","pairwise"], + "id_token_signing_alg_values_supported":["ES384","RS384","HS256","HS512","ES256","RS256","HS384","ES512","RS512"], + "userinfo_signing_alg_values_supported":["ES384","RS384","HS256","HS512","ES256","RS256","HS384","ES512","RS512","none"], + "authorization_endpoint":"http://`+e2eOauthListener+`/auth/realms/master/protocol/openid-connect/auth", + "token_endpoint":"http://`+e2eOauthListener+`/auth/realms/master/protocol/openid-connect/token", + "jwks_uri":"http://`+e2eOauthListener+`/auth/realms/master/protocol/openid-connect/certs" + }`) + } + http.HandleFunc(e2eOauthURL, configurationHandler) + _ = http.ListenAndServe(e2eOauthListener, nil) + }() + if !assert.True(t, checkListenOrBail("http://"+path.Join(e2eOauthListener, e2eOauthURL))) { + err := fmt.Errorf("cannot connect to test http listener on: %s", "http://"+path.Join(e2eOauthListener, e2eOauthURL)) + t.Logf("%v", err) + t.FailNow() + return err + } + return nil +} + +func TestCorsWithUpstream(t *testing.T) { + log.SetOutput(ioutil.Discard) + config := newDefaultConfig() + config.Listen = e2eProxyListener + config.DiscoveryURL = "http://" + e2eOauthListener + e2eOauthURL + config.Upstream = "http://" + e2eUpstreamListener + config.CorsOrigins = []string{"*"} + config.Verbose = false + config.DisableAllLogging = true + config.Resources = []*Resource{ + { + URL: e2eUpstreamURL, + Methods: []string{"GET"}, + WhiteListed: true, + }, + } + + // launch fake upstream resource server + _ = runTestUpstream(t) + + // launch fake oauth OIDC server + _ = runTestAuth(t) + + // launch keycloak-gatekeeper proxy + _ = runTestGatekeeper(t, config) + + // ok now exercise the ensemble with a CORS-enabled request + client := http.Client{} + u, _ := url.Parse("http://" + e2eProxyListener + e2eUpstreamURL) + h := make(http.Header, 1) + h.Set("Content-Type", "application/json") + h.Add("Origin", "myorigin.com") + + resp, err := client.Do(&http.Request{ + Method: "GET", + URL: u, + Header: h, + }) + assert.NoError(t, err) + buf, erb := ioutil.ReadAll(resp.Body) + assert.NoError(t, erb) + assert.JSONEq(t, `{"message":"test"}`, string(buf)) // check this is our test resource + if assert.NotEmpty(t, resp.Header) && assert.Contains(t, resp.Header, "Access-Control-Allow-Origin") { + // check the returned upstream response after proxying contains CORS headers + assert.Equal(t, []string{"*"}, resp.Header["Access-Control-Allow-Origin"]) + } +} diff --git a/forwarding.go b/forwarding.go index 4c25eda..9284985 100644 --- a/forwarding.go +++ b/forwarding.go @@ -44,6 +44,10 @@ func (r *oauthProxy) proxyMiddleware(next http.Handler) http.Handler { req.Header.Set("X-Forwarded-Host", req.Host) req.Header.Set("X-Forwarded-Proto", req.Header.Get("X-Forwarded-Proto")) + if len(r.config.CorsOrigins) > 0 { + // if CORS is enabled by gatekeeper, do not propagate CORS requests upstream + req.Header.Del("Origin") + } // @step: add any custom headers to the request for k, v := range r.config.Headers { req.Header.Set(k, v) diff --git a/server.go b/server.go index 761967a..c156a7e 100644 --- a/server.go +++ b/server.go @@ -185,6 +185,7 @@ func (r *oauthProxy) createReverseProxy() error { AllowCredentials: r.config.CorsCredentials, ExposedHeaders: r.config.CorsExposedHeaders, MaxAge: int(r.config.CorsMaxAge.Seconds()), + Debug: r.config.Verbose, }) engine.Use(c.Handler) } @@ -605,6 +606,11 @@ func (r *oauthProxy) createUpstreamProxy(upstream *url.URL) error { // create the forwarding proxy proxy := goproxy.NewProxyHttpServer() + + // headers formed by middleware before proxying to upstream shall be + // kept in response. This is true for CORS headers ([KEYCOAK-9045]) + // and for refreshed cookies (htts://github.com/keycloak/keycloak-gatekeeper/pulls/456]) + proxy.KeepDestinationHeaders = true proxy.Logger = httplog.New(ioutil.Discard, "", 0) r.upstream = proxy -- GitLab