From ca1748e8777b6ced583f74a89a8af649687ae03c Mon Sep 17 00:00:00 2001 From: ptx Date: Sat, 2 Mar 2019 11:43:52 +0100 Subject: [PATCH] Added flag to allow redirect between subdomains --- README.md | 1 + caddy/setup_test.go | 2 ++ login/config.go | 3 ++ login/redirect.go | 32 +++++++++++++++++++ login/redirect_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+) diff --git a/README.md b/README.md index 13f5b2d0..f2decad3 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,7 @@ _Note for Caddy users_: Not all parameters are available in Caddy. See the table | -osiam | value | | X | OSIAM login backend opts: endpoint=..,client_id=..,client_secret=.. | | -port | string | "6789" | - | Port to listen on | | -redirect | boolean | true | X | Allow dynamic overwriting of the the success by query parameter | +| -redirect-allow-subdomain | bool | false | X | If true redirect is allowed when the target is on a different subdomain | | -redirect-query-parameter | string | "backTo" | X | URL parameter for the redirect target | | -redirect-check-referer | boolean | true | X | Check the referer header to ensure it matches the host header on dynamic redirects | | -redirect-host-file | string | "" | X | A file containing a list of domains that redirects are allowed to, one domain per line | diff --git a/caddy/setup_test.go b/caddy/setup_test.go index d2351654..061c391c 100644 --- a/caddy/setup_test.go +++ b/caddy/setup_test.go @@ -43,6 +43,7 @@ func TestSetup(t *testing.T) { redirect_query_parameter comingFrom redirect_check_referer true redirect_host_file domainWhitelist.txt + redirect_allow_subdomain true cookie_name cookiename cookie_http_only false cookie_domain example.com @@ -60,6 +61,7 @@ func TestSetup(t *testing.T) { Equal(t, cfg.RedirectQueryParameter, "comingFrom") Equal(t, cfg.RedirectCheckReferer, true) Equal(t, cfg.RedirectHostFile, "domainWhitelist.txt") + Equal(t, cfg.RedirectAllowSubdomain, true) Equal(t, cfg.CookieName, "cookiename") Equal(t, cfg.CookieHTTPOnly, false) Equal(t, cfg.CookieDomain, "example.com") diff --git a/login/config.go b/login/config.go index c11e7ca6..d920d00b 100644 --- a/login/config.go +++ b/login/config.go @@ -40,6 +40,7 @@ func DefaultConfig() *Config { RedirectQueryParameter: "backTo", RedirectCheckReferer: true, RedirectHostFile: "", + RedirectAllowSubdomain: false, LogoutURL: "", LoginPath: "/login", CookieName: "jwt_token", @@ -73,6 +74,7 @@ type Config struct { RedirectQueryParameter string RedirectCheckReferer bool RedirectHostFile string + RedirectAllowSubdomain bool LogoutURL string Template string LoginPath string @@ -152,6 +154,7 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.StringVar(&c.RedirectQueryParameter, "redirect-query-parameter", c.RedirectQueryParameter, "URL parameter for the redirect target") f.BoolVar(&c.RedirectCheckReferer, "redirect-check-referer", c.RedirectCheckReferer, "When redirecting check that the referer is the same domain") f.StringVar(&c.RedirectHostFile, "redirect-host-file", c.RedirectHostFile, "A file containing a list of domains that redirects are allowed to, one domain per line") + f.BoolVar(&c.RedirectAllowSubdomain, "redirect-allow-subdomain", c.RedirectAllowSubdomain, "If true a redirect is allowed if the target is a different subdomain than loginsrv") f.StringVar(&c.LogoutURL, "logout-url", c.LogoutURL, "The url or path to redirect after logout") f.StringVar(&c.Template, "template", c.Template, "An alternative template for the login form") diff --git a/login/redirect.go b/login/redirect.go index 1c161535..734a35b4 100644 --- a/login/redirect.go +++ b/login/redirect.go @@ -49,6 +49,7 @@ func (h *Handler) allowRedirect(r *http.Request) bool { logging.Application(r.Header).Warnf("couldn't parse redirect url %s", err) return false } + if referer.Host != r.Host { logging.Application(r.Header).Warnf("redirect from referer domain: '%s', not matching current domain '%s'", referer.Host, r.Host) return false @@ -56,6 +57,34 @@ func (h *Handler) allowRedirect(r *http.Request) bool { return true } +func removeSubdomain(host string) string { + parts := strings.Split(host, ".") + if len(parts) == 1 { + return host + } + return strings.Join(parts[1:], ".") +} + +// haveSubdomain checks that there's at least one subdomain +func haveSubdomain(host string) bool { + trimmed := strings.Trim(host, ".") + parts := strings.Split(trimmed, ".") + return len(parts) > 2 +} + +func (h *Handler) isSubdomainAllowed(target string, host string) bool { + if !h.config.RedirectAllowSubdomain { + return false + } + if target == "" || host == "" { + return false + } + if !haveSubdomain(target) || !haveSubdomain(host) { + return false + } + return removeSubdomain(target) == removeSubdomain(host) +} + func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { targetURL, foundTarget := h.getRedirectTarget(r) if foundTarget && h.config.Redirect { @@ -63,6 +92,9 @@ func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { if sameHost && targetURL.Path != "" { return targetURL.Path } + if h.isSubdomainAllowed(targetURL.Host, r.Host) { + return targetURL.String() + } if !sameHost && h.isRedirectDomainWhitelisted(r, targetURL.Host) { return targetURL.String() } diff --git a/login/redirect_test.go b/login/redirect_test.go index 4f3328eb..383ca746 100644 --- a/login/redirect_test.go +++ b/login/redirect_test.go @@ -1,6 +1,7 @@ package login import ( + "fmt" "net/http/httptest" "os" "testing" @@ -122,5 +123,75 @@ func TestRedirect_Whitelisting(t *testing.T) { h.ServeHTTP(recorder, req("POST", "/login?backTo=https://evildomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) Equal(t, 303, recorder.Code) Equal(t, "/", recorder.Header().Get("Location")) +} + +func TestRemoveSubDomain(t *testing.T) { + tests := []struct { + input string + output string + }{ + {input: "sub.home.com", output: "home.com"}, + {input: "tld", output: "tld"}, + {input: "home.com", output: "com"}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%s should be %s", tt.input, tt.output), func(t *testing.T) { + Equal(t, tt.output, removeSubdomain(tt.input)) + }) + } +} + +func TestHaveSubdomain(t *testing.T) { + tests := []struct { + input string + expect bool + }{ + {input: "sub.home.com", expect: true}, + {input: "tld", expect: false}, + {input: "home.com", expect: false}, + {input: "home.com.", expect: false}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%s should be %v", tt.input, tt.expect), func(t *testing.T) { + Equal(t, tt.expect, haveSubdomain(tt.input)) + }) + } +} + +func TestRedirect_Subdomain(t *testing.T) { + + cfg := DefaultConfig() + cfg.RedirectAllowSubdomain = true + h := &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder := httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "http://auth.home.com/login?backTo=https://sub.home.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "https://sub.home.com/website", recorder.Header().Get("Location")) + + // need at least one subdomain + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "http://home.com/login?backTo=https://google.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + // make sure extra . is ignored + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "http://home.com./login?backTo=https://google.com./website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + // not allowed if current host is unknown + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://sub.home.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) }