From dbba63f98e3698deb51dc0c091b3bc413497f64d Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Fri, 17 Nov 2017 17:33:55 +0000 Subject: [PATCH 01/12] preliminary implemntation of redirect logic --- login/config.go | 74 +++++++++++++++++++++---------------- login/handler.go | 96 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 133 insertions(+), 37 deletions(-) diff --git a/login/config.go b/login/config.go index bd7fb47..6732faf 100644 --- a/login/config.go +++ b/login/config.go @@ -23,20 +23,23 @@ func init() { // DefaultConfig for the loginsrv handler func DefaultConfig() *Config { return &Config{ - Host: "localhost", - Port: "6789", - LogLevel: "info", - JwtSecret: jwtDefaultSecret, - JwtExpiry: 24 * time.Hour, - JwtRefreshes: 0, - SuccessURL: "/", - LogoutURL: "", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, - Backends: Options{}, - Oauth: Options{}, - GracePeriod: 5 * time.Second, + Host: "localhost", + Port: "6789", + LogLevel: "info", + JwtSecret: jwtDefaultSecret, + JwtExpiry: 24 * time.Hour, + JwtRefreshes: 0, + SuccessURL: "/", + AllowRedirects: true, + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LogoutURL: "", + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, + Backends: Options{}, + Oauth: Options{}, + GracePeriod: 5 * time.Second, } } @@ -44,24 +47,27 @@ const envPrefix = "LOGINSRV_" // Config for the loginsrv handler type Config struct { - Host string - Port string - LogLevel string - TextLogging bool - JwtSecret string - JwtExpiry time.Duration - JwtRefreshes int - SuccessURL string - LogoutURL string - Template string - LoginPath string - CookieName string - CookieExpiry time.Duration - CookieDomain string - CookieHTTPOnly bool - Backends Options - Oauth Options - GracePeriod time.Duration + Host string + Port string + LogLevel string + TextLogging bool + JwtSecret string + JwtExpiry time.Duration + JwtRefreshes int + SuccessURL string + AllowRedirects bool + PreventExternalRedirects bool + CheckRefererOnRedirects bool + LogoutURL string + Template string + LoginPath string + CookieName string + CookieExpiry time.Duration + CookieDomain string + CookieHTTPOnly bool + Backends Options + Oauth Options + GracePeriod time.Duration } // Options is the configuration structure for oauth and backend provider @@ -104,6 +110,10 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.DurationVar(&c.CookieExpiry, "cookie-expiry", c.CookieExpiry, "The expiry duration for the cookie, e.g. 2h or 3h30m. Default is browser session") f.StringVar(&c.CookieDomain, "cookie-domain", c.CookieDomain, "The optional domain parameter for the cookie") f.StringVar(&c.SuccessURL, "success-url", c.SuccessURL, "The url to redirect after login") + f.BoolVar(&c.AllowRedirects, "allow-redirects", c.AllowRedirects, "Allow dynamic redirects by parameter") + f.BoolVar(&c.PreventExternalRedirects, "prevent-external-redirects", c.PreventExternalRedirects, "Prevent dynamic redirects from redirecting to an external domain") + f.BoolVar(&c.CheckRefererOnRedirects, "check-referer-on-redirects", c.CheckRefererOnRedirects, "When redirecting check that the referer is the same domain") + 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") f.StringVar(&c.LoginPath, "login-path", c.LoginPath, "The path of the login resource") diff --git a/login/handler.go b/login/handler.go index 28ef13b..1faf78e 100644 --- a/login/handler.go +++ b/login/handler.go @@ -4,14 +4,17 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" + "net/http" + "net/url" + "strings" + "time" + + "github.com/davecgh/go-spew/spew" "github.com/dgrijalva/jwt-go" "github.com/tarent/loginsrv/logging" "github.com/tarent/loginsrv/model" "github.com/tarent/loginsrv/oauth2" - "io/ioutil" - "net/http" - "strings" - "time" ) const contentTypeHTML = "text/html; charset=utf-8" @@ -71,11 +74,34 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.handleOauth(w, r) return } + if h.shouldSetCookie(r) { + queries, _ := url.ParseQuery(r.URL.RawQuery) + cookie := http.Cookie{Name: "redirect_url", Value: queries.Get("backTo")} + http.SetCookie(w, &cookie) + } h.handleLogin(w, r) return } +func (h *Handler) shouldSetCookie(r *http.Request) bool { + if h.config.AllowRedirects { + if h.config.CheckRefererOnRedirects { + referer, _ := url.Parse(r.Header.Get("Referer")) + if referer.Host != r.Host { + logging.Application(r.Header).Warnf( + "Referer domain: '%s' does not match current domain '%s'", + referer.Host, + r.Host, + ) + return false + } + } + return true + } + return false +} + func (h *Handler) handleOauth(w http.ResponseWriter, r *http.Request) { startedFlow, authenticated, userInfo, err := h.oauth.Handle(w, r) @@ -231,7 +257,10 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u http.SetCookie(w, cookie) - w.Header().Set("Location", h.config.SuccessURL) + //redirectURL := h.config.SuccessURL + //fmt.Printf("redirectURL is: %s\n", h.redirectURL(r)) + + w.Header().Set("Location", h.redirectURL(r)) w.WriteHeader(303) return } @@ -241,6 +270,63 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u fmt.Fprintf(w, "%s", token) } +func (h *Handler) redirectURL(r *http.Request) string { + origin, _ := url.Parse(r.Header.Get("Origin")) + originScheme := origin.Scheme + originHost := origin.Host + redirectURL := fmt.Sprintf("%s://%s%s", originScheme, originHost, h.config.SuccessURL) + if h.config.AllowRedirects { + cookie, _ := r.Cookie("redirect_url") + parsedURL, err := url.Parse(cookie.Value) + if err != nil { + spew.Dump("oops can't parse that url") + } + scheme := parsedURL.Scheme + host := parsedURL.Host + path := parsedURL.Path + //spew.Dump(scheme) + //spew.Dump(host) + //spew.Dump(path) + + if scheme == "" { + scheme = originScheme + } + + if host == "" { + host = originHost + } else { + if h.config.PreventExternalRedirects { + if host != originHost { + logging.Application(r.Header).Warnf( + "Redirect attempt to '%s' but -prevent-external-redirects is set to true", + host, + ) + return redirectURL + } else { + host = originHost + } + } else { + //TODO read domains from file + domains := "2google.com" + if host != domains { + logging.Application(r.Header).Warnf( + "Redirect attempt to '%s' but it is not in domain whitelist", + host, + ) + return redirectURL + } + } + } + + if path == "" { + path = h.config.SuccessURL + } + + redirectURL = fmt.Sprintf("%s://%s%s", scheme, host, path) + } + return redirectURL +} + func (h *Handler) createToken(userInfo jwt.Claims) (string, error) { token := jwt.NewWithClaims(jwt.SigningMethodHS512, userInfo) return token.SignedString([]byte(h.config.JwtSecret)) From 3f9fb92386d6fe693ff186bfd7e8e9858e1c770b Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Thu, 23 Nov 2017 16:24:55 +0000 Subject: [PATCH 02/12] Allow for dynamic redirects using query parameters and cookies. Allow redirects to external domains by whitelist only. Check referer header before allowing a redirect. For https://github.com/tarent/loginsrv/issues/45 --- README.md | 51 ++++++++++-------- caddy/README.md | 21 ++++++++ caddy/setup_test.go | 100 +++++++++++++++++++++-------------- login/config.go | 6 +++ login/config_test.go | 79 ++++++++++++++++++---------- login/handler.go | 107 +++++++------------------------------ login/handler_test.go | 117 +++++++++++++++++++++++++++++++++++++++-- login/redirect.go | 119 ++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 419 insertions(+), 181 deletions(-) create mode 100644 login/redirect.go diff --git a/README.md b/README.md index 192e452..54fc002 100644 --- a/README.md +++ b/README.md @@ -42,29 +42,34 @@ For questions and support please use the [Gitter chat room](https://gitter.im/ta _Note for Caddy users_: Not all parameters are available in Caddy. See the table for details. With Caddy, the parameter names can be also be used with `_` in the names, e.g. `cookie_http_only`. -| Parameter | Type | Default | Caddy | Description | -|-------------------|-------------|--------------|-------|--------------------------------------------------------------------------------------| -| -cookie-domain | string | | X | The optional domain parameter for the cookie | -| -cookie-expiry | string | session | X | The expiry duration for the cookie, e.g. 2h or 3h30m | -| -cookie-http-only | boolean | true | X | Set the cookie with the http only flag | -| -cookie-name | string | "jwt_token" | X | The name of the jwt cookie | -| -github | value | | X | Oauth config in the form: client_id=..,client_secret=..[,scope=..,][redirect_uri=..] | -| -google | value | | X | Oauth config in the form: client_id=..,client_secret=..,scope=..[redirect_uri=..] | -| -host | string | "localhost" | - | The host to listen on | -| -htpasswd | value | | X | Htpasswd login backend opts: file=/path/to/pwdfile | -| -jwt-expiry | go duration | 24h | X | The expiry duration for the jwt token, e.g. 2h or 3h30m | -| -jwt-secret | string | "random key" | X | The secret to sign the jwt token | -| -log-level | string | "info" | - | The log level | -| -login-path | string | "/login" | X | The path of the login resource | -| -logout-url | string | | X | The url or path to redirect after logout | -| -osiam | value | | X | OSIAM login backend opts: endpoint=..,client_id=..,client_secret=.. | -| -port | string | "6789" | - | The port to listen on | -| -simple | value | | X | Simple login backend opts: user1=password,user2=password,.. | -| -success-url | string | "/" | X | The url to redirect after login | -| -template | string | | X | An alternative template for the login form | -| -text-logging | boolean | true | - | Log in text format instead of json | -| -jwt-refreshes | int | 0 | X | The maximum amount of jwt refreshes. | -| -grace-period | go duration | 5s | - | Duration to wait after SIGINT/SIGTERM for existing requests. No new requests are accepted. | +| Parameter | Type | Default | Caddy | Description | +|-----------------------------|-------------|--------------|-------|--------------------------------------------------------------------------------------------| +| -cookie-domain | string | | X | The optional domain parameter for the cookie | +| -cookie-expiry | string | session | X | The expiry duration for the cookie, e.g. 2h or 3h30m | +| -cookie-http-only | boolean | true | X | Set the cookie with the http only flag | +| -cookie-name | string | "jwt_token" | X | The name of the jwt cookie | +| -github | value | | X | Oauth config in the form: client_id=..,client_secret=..[,scope=..,][redirect_uri=..] | +| -google | value | | X | Oauth config in the form: client_id=..,client_secret=..,scope=..[redirect_uri=..] | +| -host | string | "localhost" | - | The host to listen on | +| -htpasswd | value | | X | Htpasswd login backend opts: file=/path/to/pwdfile | +| -jwt-expiry | go duration | 24h | X | The expiry duration for the jwt token, e.g. 2h or 3h30m | +| -jwt-secret | string | "random key" | X | The secret to sign the jwt token | +| -log-level | string | "info" | - | The log level | +| -login-path | string | "/login" | X | The path of the login resource | +| -logout-url | string | | X | The url or path to redirect after logout | +| -osiam | value | | X | OSIAM login backend opts: endpoint=..,client_id=..,client_secret=.. | +| -port | string | "6789" | - | The port to listen on | +| -simple | value | | X | Simple login backend opts: user1=password,user2=password,.. | +| -success-url | string | "/" | X | The url to redirect after login | +| -allow-redirects | boolean | true | X | Allow dynamic redirects | +| -redirect-query-parameter | string | "backTo" | X | The query parameter to find the dynamic redirect in | +| -prevent-external-redirects | boolean | true | X | Prevent dynamic redirects to external domains | +| -check-referer-on-redirects | boolean | true | X | Check the referer header to ensure it matches the host header on dynamic redirects | +| -whitelist-domains-file | string | "" | X | File containing whitelist of domains for dynamic redirects, one domain per line | +| -template | string | | X | An alternative template for the login form | +| -text-logging | boolean | true | - | Log in text format instead of json | +| -jwt-refreshes | int | 0 | X | The maximum amount of jwt refreshes. | +| -grace-period | go duration | 5s | - | Duration to wait after SIGINT/SIGTERM for existing requests. No new requests are accepted. | ### Environment Variables All of the above Config Options can also be applied as environment variable, where the name is written in the way: `LOGINSRV_OPTION_NAME`. diff --git a/caddy/README.md b/caddy/README.md index 7353da3..ef1c9c4 100644 --- a/caddy/README.md +++ b/caddy/README.md @@ -50,3 +50,24 @@ login { simple bob=secret,alice=secret } ``` + +### Example caddyfile with dynamic redirects +``` +127.0.0.1 + +root {$PWD} +browse + +jwt { + path / + except /favicon.ico + redirect /login?backTo={rewrite_uri} + allow sub bob + allow sub alice +} + +login { + simple bob=secret,alice=secret + check_referer_on_redirects false +} +``` diff --git a/caddy/setup_test.go b/caddy/setup_test.go index a8b7ff5..9a894be 100644 --- a/caddy/setup_test.go +++ b/caddy/setup_test.go @@ -2,15 +2,16 @@ package caddy import ( "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" + "github.com/mholt/caddy" "github.com/mholt/caddy/caddyhttp/httpserver" . "github.com/stretchr/testify/assert" "github.com/tarent/loginsrv/login" - "io/ioutil" - "path/filepath" - "os" - "testing" - "time" ) func TestSetup(t *testing.T) { @@ -28,12 +29,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -47,6 +52,9 @@ func TestSetup(t *testing.T) { success_url successurl jwt_expiry 42h login_path /foo/bar + allow-redirects true + prevent-external-redirects true + check-referer-on-redirects true cookie_name cookiename cookie_http_only false cookie_domain example.com @@ -56,14 +64,18 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 42 * time.Hour, - SuccessURL: "successurl", - LoginPath: "/foo/bar", - CookieName: "cookiename", - CookieDomain: "example.com", - CookieExpiry: 23*time.Hour + 23*time.Minute, - CookieHTTPOnly: false, + JwtSecret: "jwtsecret", + JwtExpiry: 42 * time.Hour, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/foo/bar", + CookieName: "cookiename", + CookieDomain: "example.com", + CookieExpiry: 23*time.Hour + 23*time.Minute, + CookieHTTPOnly: false, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -87,12 +99,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/context/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/context/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -111,12 +127,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -133,12 +153,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -178,7 +202,7 @@ func TestSetup_RelativeTemplateFile(t *testing.T) { caddyfile := "loginsrv {\n template myTemplate.tpl\n simple bob=secret\n}" root, _ := ioutil.TempDir("", "") expectedPath := filepath.FromSlash(root + "/myTemplate.tpl") - + c := caddy.NewTestController("http", caddyfile) c.Key = "RelativeTemplateFileTest" config := httpserver.GetConfig(c) diff --git a/login/config.go b/login/config.go index 6732faf..ee14275 100644 --- a/login/config.go +++ b/login/config.go @@ -31,8 +31,10 @@ func DefaultConfig() *Config { JwtRefreshes: 0, SuccessURL: "/", AllowRedirects: true, + RedirectQueryParameter: "backTo", PreventExternalRedirects: true, CheckRefererOnRedirects: true, + WhitelistDomainsFile: "", LogoutURL: "", LoginPath: "/login", CookieName: "jwt_token", @@ -56,8 +58,10 @@ type Config struct { JwtRefreshes int SuccessURL string AllowRedirects bool + RedirectQueryParameter string PreventExternalRedirects bool CheckRefererOnRedirects bool + WhitelistDomainsFile string LogoutURL string Template string LoginPath string @@ -111,8 +115,10 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.StringVar(&c.CookieDomain, "cookie-domain", c.CookieDomain, "The optional domain parameter for the cookie") f.StringVar(&c.SuccessURL, "success-url", c.SuccessURL, "The url to redirect after login") f.BoolVar(&c.AllowRedirects, "allow-redirects", c.AllowRedirects, "Allow dynamic redirects by parameter") + f.StringVar(&c.RedirectQueryParameter, "redirect-query-parameter", c.RedirectQueryParameter, "Allow dynamic redirects by parameter") f.BoolVar(&c.PreventExternalRedirects, "prevent-external-redirects", c.PreventExternalRedirects, "Prevent dynamic redirects from redirecting to an external domain") f.BoolVar(&c.CheckRefererOnRedirects, "check-referer-on-redirects", c.CheckRefererOnRedirects, "When redirecting check that the referer is the same domain") + f.StringVar(&c.WhitelistDomainsFile, "whitelist-domains-file", c.WhitelistDomainsFile, "the file containing a list of domains that redirects are allowed to, one domain per line") 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/config_test.go b/login/config_test.go index e93817c..95dc59d 100644 --- a/login/config_test.go +++ b/login/config_test.go @@ -2,10 +2,11 @@ package login import ( "flag" - . "github.com/stretchr/testify/assert" "os" "testing" "time" + + . "github.com/stretchr/testify/assert" ) func TestConfig_ReadConfigDefaults(t *testing.T) { @@ -28,6 +29,11 @@ func TestConfig_ReadConfig(t *testing.T) { "--jwt-secret=jwtsecret", "--jwt-expiry=42h42m", "--success-url=successurl", + "--allow-redirects=true", + "--redirect-query-parameter=backTo", + "--prevent-external-redirects=true", + "--check-referer-on-redirects=true", + "--whitelist-domains-file=File", "--logout-url=logouturl", "--template=template", "--login-path=loginpath", @@ -42,20 +48,25 @@ func TestConfig_ReadConfig(t *testing.T) { } expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + WhitelistDomainsFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{}, "foo": map[string]string{}, @@ -82,6 +93,11 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_JWT_SECRET", "jwtsecret")) NoError(t, os.Setenv("LOGINSRV_JWT_EXPIRY", "42h42m")) NoError(t, os.Setenv("LOGINSRV_SUCCESS_URL", "successurl")) + NoError(t, os.Setenv("LOGINSRV_ALLOW_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_REDIRECT_QUERY_PARAMETER", "backTo")) + NoError(t, os.Setenv("LOGINSRV_PREVENT_EXTERNAL_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_CHECK_REFERER_ON_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_WHITELIST_DOMAINS_FILE", "File")) NoError(t, os.Setenv("LOGINSRV_LOGOUT_URL", "logouturl")) NoError(t, os.Setenv("LOGINSRV_TEMPLATE", "template")) NoError(t, os.Setenv("LOGINSRV_LOGIN_PATH", "loginpath")) @@ -94,20 +110,25 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_GRACE_PERIOD", "4s")) expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + WhitelistDomainsFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{ "foo": "bar", diff --git a/login/handler.go b/login/handler.go index 1faf78e..36530a0 100644 --- a/login/handler.go +++ b/login/handler.go @@ -69,39 +69,28 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if (h.shouldRedirect(r)) && (r.Method != "POST") { + queries, _ := url.ParseQuery(r.URL.RawQuery) + if queries.Get(h.config.RedirectQueryParameter) != "" { + cookie := http.Cookie{ + Name: h.config.RedirectQueryParameter, + Value: queries.Get(h.config.RedirectQueryParameter), + } + spew.Dump(cookie) + http.SetCookie(w, &cookie) + } + } + _, err := h.oauth.GetConfigFromRequest(r) if err == nil { h.handleOauth(w, r) return } - if h.shouldSetCookie(r) { - queries, _ := url.ParseQuery(r.URL.RawQuery) - cookie := http.Cookie{Name: "redirect_url", Value: queries.Get("backTo")} - http.SetCookie(w, &cookie) - } h.handleLogin(w, r) return } -func (h *Handler) shouldSetCookie(r *http.Request) bool { - if h.config.AllowRedirects { - if h.config.CheckRefererOnRedirects { - referer, _ := url.Parse(r.Header.Get("Referer")) - if referer.Host != r.Host { - logging.Application(r.Header).Warnf( - "Referer domain: '%s' does not match current domain '%s'", - referer.Host, - r.Host, - ) - return false - } - } - return true - } - return false -} - func (h *Handler) handleOauth(w http.ResponseWriter, r *http.Request) { startedFlow, authenticated, userInfo, err := h.oauth.Handle(w, r) @@ -143,7 +132,7 @@ func (h *Handler) handleLogin(w http.ResponseWriter, r *http.Request) { r.ParseForm() if r.Method == "DELETE" || r.FormValue("logout") == "true" { - h.deleteToken(w) + h.deleteCookie(w, h.config.CookieName) if h.config.LogoutURL != "" { w.Header().Set("Location", h.config.LogoutURL) w.WriteHeader(303) @@ -218,9 +207,9 @@ func (h *Handler) handleRefresh(w http.ResponseWriter, r *http.Request, userInfo } } -func (h *Handler) deleteToken(w http.ResponseWriter) { +func (h *Handler) deleteCookie(w http.ResponseWriter, cookieName string) { cookie := &http.Cookie{ - Name: h.config.CookieName, + Name: cookieName, Value: "delete", HttpOnly: true, Expires: time.Unix(0, 0), @@ -257,10 +246,11 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u http.SetCookie(w, cookie) - //redirectURL := h.config.SuccessURL - //fmt.Printf("redirectURL is: %s\n", h.redirectURL(r)) - - w.Header().Set("Location", h.redirectURL(r)) + w.Header().Set("Location", h.redirectURL(r, w)) + _, err := r.Cookie(h.config.RedirectQueryParameter) + if err == nil { + h.deleteCookie(w, h.config.RedirectQueryParameter) + } w.WriteHeader(303) return } @@ -270,63 +260,6 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u fmt.Fprintf(w, "%s", token) } -func (h *Handler) redirectURL(r *http.Request) string { - origin, _ := url.Parse(r.Header.Get("Origin")) - originScheme := origin.Scheme - originHost := origin.Host - redirectURL := fmt.Sprintf("%s://%s%s", originScheme, originHost, h.config.SuccessURL) - if h.config.AllowRedirects { - cookie, _ := r.Cookie("redirect_url") - parsedURL, err := url.Parse(cookie.Value) - if err != nil { - spew.Dump("oops can't parse that url") - } - scheme := parsedURL.Scheme - host := parsedURL.Host - path := parsedURL.Path - //spew.Dump(scheme) - //spew.Dump(host) - //spew.Dump(path) - - if scheme == "" { - scheme = originScheme - } - - if host == "" { - host = originHost - } else { - if h.config.PreventExternalRedirects { - if host != originHost { - logging.Application(r.Header).Warnf( - "Redirect attempt to '%s' but -prevent-external-redirects is set to true", - host, - ) - return redirectURL - } else { - host = originHost - } - } else { - //TODO read domains from file - domains := "2google.com" - if host != domains { - logging.Application(r.Header).Warnf( - "Redirect attempt to '%s' but it is not in domain whitelist", - host, - ) - return redirectURL - } - } - } - - if path == "" { - path = h.config.SuccessURL - } - - redirectURL = fmt.Sprintf("%s://%s%s", scheme, host, path) - } - return redirectURL -} - func (h *Handler) createToken(userInfo jwt.Claims) (string, error) { token := jwt.NewWithClaims(jwt.SigningMethodHS512, userInfo) return token.SignedString([]byte(h.config.JwtSecret)) diff --git a/login/handler_test.go b/login/handler_test.go index 12032ff..2287a08 100644 --- a/login/handler_test.go +++ b/login/handler_test.go @@ -3,22 +3,27 @@ package login import ( "errors" "fmt" - "github.com/dgrijalva/jwt-go" - . "github.com/stretchr/testify/assert" - "github.com/tarent/loginsrv/model" - "github.com/tarent/loginsrv/oauth2" + "io/ioutil" "net/http" "net/http/httptest" + "os" "strconv" "strings" "testing" "time" + + "github.com/dgrijalva/jwt-go" + . "github.com/stretchr/testify/assert" + "github.com/tarent/loginsrv/model" + "github.com/tarent/loginsrv/oauth2" ) const TypeJSON = "Content-Type: application/json" const TypeForm = "Content-Type: application/x-www-form-urlencoded" const AcceptHTML = "Accept: text/html" const AcceptJwt = "Accept: application/jwt" +const Host = "Host: example.com" +const BadReferer = "Referer: http://evildomain.com" func testConfig() *Config { testConfig := DefaultConfig() @@ -238,6 +243,110 @@ func TestHandler_LoginWeb(t *testing.T) { Equal(t, recorder.Header().Get("Set-Cookie"), "") } +func TestHandler_Redirect(t *testing.T) { + //by default set redirect_cookie + recorder := call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML)) + setCookieList := readSetCookies(recorder.Header()) + Equal(t, 1, len(setCookieList)) + cookie := setCookieList[0] + Equal(t, "backTo", cookie.Name) + Equal(t, "/website", cookie.Value) + + //by default allowed redirects + recorder = call(req("POST", "/context/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/website", recorder.Header().Get("Location")) + + //redirect to SuccessURL if AllowRedirects is false + cfg := DefaultConfig() + cfg.AllowRedirects = false + h := &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //by default don't set redirect cookie if Referer doesn't match origin + recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 0, len(setCookieList)) + + //don't set redirect cookie if referrer is malformed + recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, "Referer: :notvalid")) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 0, len(setCookieList)) + + //set redirect cookie with mismatch referer if CheckRefererOnRedirects is false + cfg = DefaultConfig() + cfg.CheckRefererOnRedirects = false + h = &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("GET", "/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 1, len(setCookieList)) + cookie = setCookieList[0] + Equal(t, "backTo", cookie.Name) + Equal(t, "/website", cookie.Value) + + //by default prevent redirect to external site + recorder = call(req("POST", "/context/login?backTo=//evildomain.com/phishing.html", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //by default if the parsed path is empty redirect to SuccessURL + recorder = call(req("POST", "/context/login?backTo=https://evildomain.com", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //redirect to success url if domains whitelist file doesn't exist + cfg = DefaultConfig() + cfg.PreventExternalRedirects = false + cfg.WhitelistDomainsFile = "domains_whitelist.txt" + h = &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //setup domain whitelist file + d1 := []byte("gooddomain.com\n") + _ = ioutil.WriteFile("domains_whitelist.txt", d1, 0644) + + //allow redirect to domains on whitelist + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "https://gooddomain.com/website", recorder.Header().Get("Location")) + + //allow redirect to domains on whitelist + recorder = httptest.NewRecorder() + 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")) + + //remove domains whitelist file + err := os.Remove("domains_whitelist.txt") + Equal(t, nil, err) +} + func TestHandler_Refresh(t *testing.T) { h := testHandler() input := model.UserInfo{Sub: "bob", Expiry: time.Now().Add(time.Second).Unix()} diff --git a/login/redirect.go b/login/redirect.go new file mode 100644 index 0000000..256c5ef --- /dev/null +++ b/login/redirect.go @@ -0,0 +1,119 @@ +package login + +import ( + "bufio" + "errors" + "fmt" + "net/http" + "net/url" + "os" + + "github.com/tarent/loginsrv/logging" +) + +func (h *Handler) shouldRedirect(r *http.Request) bool { + if h.config.AllowRedirects { + if h.config.CheckRefererOnRedirects { + referer, err := url.Parse(r.Header.Get("Referer")) + if err != nil { + logging.Application(r.Header).Warnf( + "couldn't parse redirect url %s", + err, + ) + return false + } + if referer.Host != r.Host { + logging.Application(r.Header).Warnf( + "Referer domain: '%s' does not match current domain '%s'", + referer.Host, + r.Host, + ) + return false + } + } + return true + } + return false +} + +func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { + if h.config.AllowRedirects { + parsedURL, err := h.parseURL(r) + if err != nil { + logging.Application(r.Header).Warnf( + "error parsing redict URL: %s", + err, + ) + return h.config.SuccessURL + } + if h.config.PreventExternalRedirects { + if parsedURL.Path == "" { + return h.config.SuccessURL + } else { + if (parsedURL.Host != "") && (r.Host != parsedURL.Host) { + logging.Application(r.Header).Warnf( + "Attempted redirect to %s", + parsedURL.Host, + ) + return h.config.SuccessURL + } + return parsedURL.Path + } + } else { + if h.checkWhiteListDomains(r, parsedURL.Host) { + return fmt.Sprintf( + "%s://%s%s", + parsedURL.Scheme, + parsedURL.Host, + parsedURL.Path, + ) + } else { + return h.config.SuccessURL + } + } + } + return h.config.SuccessURL +} + +func (h *Handler) parseURL(r *http.Request) (*url.URL, error) { + cookie, err := r.Cookie(h.config.RedirectQueryParameter) + if err != nil { + //try reading parameter as it might be a POST request and so not have set the cookie yet + queries, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + return nil, err + } + if queries.Get(h.config.RedirectQueryParameter) != "" { + parsedURL, err := url.Parse(queries.Get(h.config.RedirectQueryParameter)) + return parsedURL, err + } else { + return nil, errors.New("no redirect") + } + } + parsedURL, err := url.Parse(cookie.Value) + return parsedURL, err +} + +func (h *Handler) checkWhiteListDomains(r *http.Request, host string) bool { + f, err := os.Open(h.config.WhitelistDomainsFile) + defer f.Close() + if err != nil { + logging.Application(r.Header).Warnf( + "can't open domains file '%s'", + h.config.WhitelistDomainsFile, + ) + return false + } + scanner := bufio.NewScanner(f) + scanner.Split(bufio.ScanLines) + for scanner.Scan() { + if host == scanner.Text() { + return true + } + } + logging.Application(r.Header).Warnf( + "Domain '%s' not in whitelist", + host, + ) + return false +} From 63b850f2b59fff2faeefd72b71f101c51962d3b2 Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Fri, 17 Nov 2017 17:33:55 +0000 Subject: [PATCH 03/12] preliminary implemntation of redirect logic --- login/config.go | 74 +++++++++++++++++++++---------------- login/handler.go | 96 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 133 insertions(+), 37 deletions(-) diff --git a/login/config.go b/login/config.go index bd7fb47..6732faf 100644 --- a/login/config.go +++ b/login/config.go @@ -23,20 +23,23 @@ func init() { // DefaultConfig for the loginsrv handler func DefaultConfig() *Config { return &Config{ - Host: "localhost", - Port: "6789", - LogLevel: "info", - JwtSecret: jwtDefaultSecret, - JwtExpiry: 24 * time.Hour, - JwtRefreshes: 0, - SuccessURL: "/", - LogoutURL: "", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, - Backends: Options{}, - Oauth: Options{}, - GracePeriod: 5 * time.Second, + Host: "localhost", + Port: "6789", + LogLevel: "info", + JwtSecret: jwtDefaultSecret, + JwtExpiry: 24 * time.Hour, + JwtRefreshes: 0, + SuccessURL: "/", + AllowRedirects: true, + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LogoutURL: "", + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, + Backends: Options{}, + Oauth: Options{}, + GracePeriod: 5 * time.Second, } } @@ -44,24 +47,27 @@ const envPrefix = "LOGINSRV_" // Config for the loginsrv handler type Config struct { - Host string - Port string - LogLevel string - TextLogging bool - JwtSecret string - JwtExpiry time.Duration - JwtRefreshes int - SuccessURL string - LogoutURL string - Template string - LoginPath string - CookieName string - CookieExpiry time.Duration - CookieDomain string - CookieHTTPOnly bool - Backends Options - Oauth Options - GracePeriod time.Duration + Host string + Port string + LogLevel string + TextLogging bool + JwtSecret string + JwtExpiry time.Duration + JwtRefreshes int + SuccessURL string + AllowRedirects bool + PreventExternalRedirects bool + CheckRefererOnRedirects bool + LogoutURL string + Template string + LoginPath string + CookieName string + CookieExpiry time.Duration + CookieDomain string + CookieHTTPOnly bool + Backends Options + Oauth Options + GracePeriod time.Duration } // Options is the configuration structure for oauth and backend provider @@ -104,6 +110,10 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.DurationVar(&c.CookieExpiry, "cookie-expiry", c.CookieExpiry, "The expiry duration for the cookie, e.g. 2h or 3h30m. Default is browser session") f.StringVar(&c.CookieDomain, "cookie-domain", c.CookieDomain, "The optional domain parameter for the cookie") f.StringVar(&c.SuccessURL, "success-url", c.SuccessURL, "The url to redirect after login") + f.BoolVar(&c.AllowRedirects, "allow-redirects", c.AllowRedirects, "Allow dynamic redirects by parameter") + f.BoolVar(&c.PreventExternalRedirects, "prevent-external-redirects", c.PreventExternalRedirects, "Prevent dynamic redirects from redirecting to an external domain") + f.BoolVar(&c.CheckRefererOnRedirects, "check-referer-on-redirects", c.CheckRefererOnRedirects, "When redirecting check that the referer is the same domain") + 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") f.StringVar(&c.LoginPath, "login-path", c.LoginPath, "The path of the login resource") diff --git a/login/handler.go b/login/handler.go index 28ef13b..1faf78e 100644 --- a/login/handler.go +++ b/login/handler.go @@ -4,14 +4,17 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" + "net/http" + "net/url" + "strings" + "time" + + "github.com/davecgh/go-spew/spew" "github.com/dgrijalva/jwt-go" "github.com/tarent/loginsrv/logging" "github.com/tarent/loginsrv/model" "github.com/tarent/loginsrv/oauth2" - "io/ioutil" - "net/http" - "strings" - "time" ) const contentTypeHTML = "text/html; charset=utf-8" @@ -71,11 +74,34 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.handleOauth(w, r) return } + if h.shouldSetCookie(r) { + queries, _ := url.ParseQuery(r.URL.RawQuery) + cookie := http.Cookie{Name: "redirect_url", Value: queries.Get("backTo")} + http.SetCookie(w, &cookie) + } h.handleLogin(w, r) return } +func (h *Handler) shouldSetCookie(r *http.Request) bool { + if h.config.AllowRedirects { + if h.config.CheckRefererOnRedirects { + referer, _ := url.Parse(r.Header.Get("Referer")) + if referer.Host != r.Host { + logging.Application(r.Header).Warnf( + "Referer domain: '%s' does not match current domain '%s'", + referer.Host, + r.Host, + ) + return false + } + } + return true + } + return false +} + func (h *Handler) handleOauth(w http.ResponseWriter, r *http.Request) { startedFlow, authenticated, userInfo, err := h.oauth.Handle(w, r) @@ -231,7 +257,10 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u http.SetCookie(w, cookie) - w.Header().Set("Location", h.config.SuccessURL) + //redirectURL := h.config.SuccessURL + //fmt.Printf("redirectURL is: %s\n", h.redirectURL(r)) + + w.Header().Set("Location", h.redirectURL(r)) w.WriteHeader(303) return } @@ -241,6 +270,63 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u fmt.Fprintf(w, "%s", token) } +func (h *Handler) redirectURL(r *http.Request) string { + origin, _ := url.Parse(r.Header.Get("Origin")) + originScheme := origin.Scheme + originHost := origin.Host + redirectURL := fmt.Sprintf("%s://%s%s", originScheme, originHost, h.config.SuccessURL) + if h.config.AllowRedirects { + cookie, _ := r.Cookie("redirect_url") + parsedURL, err := url.Parse(cookie.Value) + if err != nil { + spew.Dump("oops can't parse that url") + } + scheme := parsedURL.Scheme + host := parsedURL.Host + path := parsedURL.Path + //spew.Dump(scheme) + //spew.Dump(host) + //spew.Dump(path) + + if scheme == "" { + scheme = originScheme + } + + if host == "" { + host = originHost + } else { + if h.config.PreventExternalRedirects { + if host != originHost { + logging.Application(r.Header).Warnf( + "Redirect attempt to '%s' but -prevent-external-redirects is set to true", + host, + ) + return redirectURL + } else { + host = originHost + } + } else { + //TODO read domains from file + domains := "2google.com" + if host != domains { + logging.Application(r.Header).Warnf( + "Redirect attempt to '%s' but it is not in domain whitelist", + host, + ) + return redirectURL + } + } + } + + if path == "" { + path = h.config.SuccessURL + } + + redirectURL = fmt.Sprintf("%s://%s%s", scheme, host, path) + } + return redirectURL +} + func (h *Handler) createToken(userInfo jwt.Claims) (string, error) { token := jwt.NewWithClaims(jwt.SigningMethodHS512, userInfo) return token.SignedString([]byte(h.config.JwtSecret)) From 31a491062e6979fe8afdbacc7a0495c01cc6c85d Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Thu, 23 Nov 2017 16:33:59 +0000 Subject: [PATCH 04/12] Fix merge conflict in README.md --- README.md | 52 +++++++++--------- caddy/README.md | 21 ++++++++ caddy/setup_test.go | 100 +++++++++++++++++++++-------------- login/config.go | 6 +++ login/config_test.go | 79 ++++++++++++++++++---------- login/handler.go | 107 +++++++------------------------------ login/handler_test.go | 117 +++++++++++++++++++++++++++++++++++++++-- login/redirect.go | 119 ++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 419 insertions(+), 182 deletions(-) create mode 100644 login/redirect.go diff --git a/README.md b/README.md index 467eb33..d1d1605 100644 --- a/README.md +++ b/README.md @@ -43,30 +43,34 @@ For questions and support please use the [Gitter chat room](https://gitter.im/ta _Note for Caddy users_: Not all parameters are available in Caddy. See the table for details. With Caddy, the parameter names can be also be used with `_` in the names, e.g. `cookie_http_only`. -| Parameter | Type | Default | Caddy | Description | -|-------------------|-------------|--------------|-------|--------------------------------------------------------------------------------------| -| -cookie-domain | string | | X | The optional domain parameter for the cookie | -| -cookie-expiry | string | session | X | The expiry duration for the cookie, e.g. 2h or 3h30m | -| -cookie-http-only | boolean | true | X | Set the cookie with the http only flag | -| -cookie-name | string | "jwt_token" | X | The name of the jwt cookie | -| -github | value | | X | Oauth config in the form: client_id=..,client_secret=..[,scope=..,][redirect_uri=..] | -| -google | value | | X | Oauth config in the form: client_id=..,client_secret=..,scope=..[redirect_uri=..] | -| -bitbucket | value | | X | Oauth config in the form: client_id=..,client_secret=..,[scope=..][redirect_uri=..] | -| -host | string | "localhost" | - | The host to listen on | -| -htpasswd | value | | X | Htpasswd login backend opts: file=/path/to/pwdfile | -| -jwt-expiry | go duration | 24h | X | The expiry duration for the jwt token, e.g. 2h or 3h30m | -| -jwt-secret | string | "random key" | X | The secret to sign the jwt token | -| -log-level | string | "info" | - | The log level | -| -login-path | string | "/login" | X | The path of the login resource | -| -logout-url | string | | X | The url or path to redirect after logout | -| -osiam | value | | X | OSIAM login backend opts: endpoint=..,client_id=..,client_secret=.. | -| -port | string | "6789" | - | The port to listen on | -| -simple | value | | X | Simple login backend opts: user1=password,user2=password,.. | -| -success-url | string | "/" | X | The url to redirect after login | -| -template | string | | X | An alternative template for the login form | -| -text-logging | boolean | true | - | Log in text format instead of json | -| -jwt-refreshes | int | 0 | X | The maximum amount of jwt refreshes. | -| -grace-period | go duration | 5s | - | Duration to wait after SIGINT/SIGTERM for existing requests. No new requests are accepted. | +| Parameter | Type | Default | Caddy | Description | +|-----------------------------|-------------|--------------|-------|--------------------------------------------------------------------------------------------| +| -cookie-domain | string | | X | The optional domain parameter for the cookie | +| -cookie-expiry | string | session | X | The expiry duration for the cookie, e.g. 2h or 3h30m | +| -cookie-http-only | boolean | true | X | Set the cookie with the http only flag | +| -cookie-name | string | "jwt_token" | X | The name of the jwt cookie | +| -github | value | | X | Oauth config in the form: client_id=..,client_secret=..[,scope=..,][redirect_uri=..] | +| -google | value | | X | Oauth config in the form: client_id=..,client_secret=..,scope=..[redirect_uri=..] | +| -host | string | "localhost" | - | The host to listen on | +| -htpasswd | value | | X | Htpasswd login backend opts: file=/path/to/pwdfile | +| -jwt-expiry | go duration | 24h | X | The expiry duration for the jwt token, e.g. 2h or 3h30m | +| -jwt-secret | string | "random key" | X | The secret to sign the jwt token | +| -log-level | string | "info" | - | The log level | +| -login-path | string | "/login" | X | The path of the login resource | +| -logout-url | string | | X | The url or path to redirect after logout | +| -osiam | value | | X | OSIAM login backend opts: endpoint=..,client_id=..,client_secret=.. | +| -port | string | "6789" | - | The port to listen on | +| -simple | value | | X | Simple login backend opts: user1=password,user2=password,.. | +| -success-url | string | "/" | X | The url to redirect after login | +| -allow-redirects | boolean | true | X | Allow dynamic redirects | +| -redirect-query-parameter | string | "backTo" | X | The query parameter to find the dynamic redirect in | +| -prevent-external-redirects | boolean | true | X | Prevent dynamic redirects to external domains | +| -check-referer-on-redirects | boolean | true | X | Check the referer header to ensure it matches the host header on dynamic redirects | +| -whitelist-domains-file | string | "" | X | File containing whitelist of domains for dynamic redirects, one domain per line | +| -template | string | | X | An alternative template for the login form | +| -text-logging | boolean | true | - | Log in text format instead of json | +| -jwt-refreshes | int | 0 | X | The maximum amount of jwt refreshes. | +| -grace-period | go duration | 5s | - | Duration to wait after SIGINT/SIGTERM for existing requests. No new requests are accepted. | ### Environment Variables All of the above Config Options can also be applied as environment variable, where the name is written in the way: `LOGINSRV_OPTION_NAME`. diff --git a/caddy/README.md b/caddy/README.md index 7353da3..ef1c9c4 100644 --- a/caddy/README.md +++ b/caddy/README.md @@ -50,3 +50,24 @@ login { simple bob=secret,alice=secret } ``` + +### Example caddyfile with dynamic redirects +``` +127.0.0.1 + +root {$PWD} +browse + +jwt { + path / + except /favicon.ico + redirect /login?backTo={rewrite_uri} + allow sub bob + allow sub alice +} + +login { + simple bob=secret,alice=secret + check_referer_on_redirects false +} +``` diff --git a/caddy/setup_test.go b/caddy/setup_test.go index a8b7ff5..9a894be 100644 --- a/caddy/setup_test.go +++ b/caddy/setup_test.go @@ -2,15 +2,16 @@ package caddy import ( "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" + "github.com/mholt/caddy" "github.com/mholt/caddy/caddyhttp/httpserver" . "github.com/stretchr/testify/assert" "github.com/tarent/loginsrv/login" - "io/ioutil" - "path/filepath" - "os" - "testing" - "time" ) func TestSetup(t *testing.T) { @@ -28,12 +29,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -47,6 +52,9 @@ func TestSetup(t *testing.T) { success_url successurl jwt_expiry 42h login_path /foo/bar + allow-redirects true + prevent-external-redirects true + check-referer-on-redirects true cookie_name cookiename cookie_http_only false cookie_domain example.com @@ -56,14 +64,18 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 42 * time.Hour, - SuccessURL: "successurl", - LoginPath: "/foo/bar", - CookieName: "cookiename", - CookieDomain: "example.com", - CookieExpiry: 23*time.Hour + 23*time.Minute, - CookieHTTPOnly: false, + JwtSecret: "jwtsecret", + JwtExpiry: 42 * time.Hour, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/foo/bar", + CookieName: "cookiename", + CookieDomain: "example.com", + CookieExpiry: 23*time.Hour + 23*time.Minute, + CookieHTTPOnly: false, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -87,12 +99,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/context/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/context/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -111,12 +127,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -133,12 +153,16 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -178,7 +202,7 @@ func TestSetup_RelativeTemplateFile(t *testing.T) { caddyfile := "loginsrv {\n template myTemplate.tpl\n simple bob=secret\n}" root, _ := ioutil.TempDir("", "") expectedPath := filepath.FromSlash(root + "/myTemplate.tpl") - + c := caddy.NewTestController("http", caddyfile) c.Key = "RelativeTemplateFileTest" config := httpserver.GetConfig(c) diff --git a/login/config.go b/login/config.go index 6732faf..ee14275 100644 --- a/login/config.go +++ b/login/config.go @@ -31,8 +31,10 @@ func DefaultConfig() *Config { JwtRefreshes: 0, SuccessURL: "/", AllowRedirects: true, + RedirectQueryParameter: "backTo", PreventExternalRedirects: true, CheckRefererOnRedirects: true, + WhitelistDomainsFile: "", LogoutURL: "", LoginPath: "/login", CookieName: "jwt_token", @@ -56,8 +58,10 @@ type Config struct { JwtRefreshes int SuccessURL string AllowRedirects bool + RedirectQueryParameter string PreventExternalRedirects bool CheckRefererOnRedirects bool + WhitelistDomainsFile string LogoutURL string Template string LoginPath string @@ -111,8 +115,10 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.StringVar(&c.CookieDomain, "cookie-domain", c.CookieDomain, "The optional domain parameter for the cookie") f.StringVar(&c.SuccessURL, "success-url", c.SuccessURL, "The url to redirect after login") f.BoolVar(&c.AllowRedirects, "allow-redirects", c.AllowRedirects, "Allow dynamic redirects by parameter") + f.StringVar(&c.RedirectQueryParameter, "redirect-query-parameter", c.RedirectQueryParameter, "Allow dynamic redirects by parameter") f.BoolVar(&c.PreventExternalRedirects, "prevent-external-redirects", c.PreventExternalRedirects, "Prevent dynamic redirects from redirecting to an external domain") f.BoolVar(&c.CheckRefererOnRedirects, "check-referer-on-redirects", c.CheckRefererOnRedirects, "When redirecting check that the referer is the same domain") + f.StringVar(&c.WhitelistDomainsFile, "whitelist-domains-file", c.WhitelistDomainsFile, "the file containing a list of domains that redirects are allowed to, one domain per line") 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/config_test.go b/login/config_test.go index e93817c..95dc59d 100644 --- a/login/config_test.go +++ b/login/config_test.go @@ -2,10 +2,11 @@ package login import ( "flag" - . "github.com/stretchr/testify/assert" "os" "testing" "time" + + . "github.com/stretchr/testify/assert" ) func TestConfig_ReadConfigDefaults(t *testing.T) { @@ -28,6 +29,11 @@ func TestConfig_ReadConfig(t *testing.T) { "--jwt-secret=jwtsecret", "--jwt-expiry=42h42m", "--success-url=successurl", + "--allow-redirects=true", + "--redirect-query-parameter=backTo", + "--prevent-external-redirects=true", + "--check-referer-on-redirects=true", + "--whitelist-domains-file=File", "--logout-url=logouturl", "--template=template", "--login-path=loginpath", @@ -42,20 +48,25 @@ func TestConfig_ReadConfig(t *testing.T) { } expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + WhitelistDomainsFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{}, "foo": map[string]string{}, @@ -82,6 +93,11 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_JWT_SECRET", "jwtsecret")) NoError(t, os.Setenv("LOGINSRV_JWT_EXPIRY", "42h42m")) NoError(t, os.Setenv("LOGINSRV_SUCCESS_URL", "successurl")) + NoError(t, os.Setenv("LOGINSRV_ALLOW_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_REDIRECT_QUERY_PARAMETER", "backTo")) + NoError(t, os.Setenv("LOGINSRV_PREVENT_EXTERNAL_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_CHECK_REFERER_ON_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_WHITELIST_DOMAINS_FILE", "File")) NoError(t, os.Setenv("LOGINSRV_LOGOUT_URL", "logouturl")) NoError(t, os.Setenv("LOGINSRV_TEMPLATE", "template")) NoError(t, os.Setenv("LOGINSRV_LOGIN_PATH", "loginpath")) @@ -94,20 +110,25 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_GRACE_PERIOD", "4s")) expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + PreventExternalRedirects: true, + CheckRefererOnRedirects: true, + WhitelistDomainsFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{ "foo": "bar", diff --git a/login/handler.go b/login/handler.go index 1faf78e..36530a0 100644 --- a/login/handler.go +++ b/login/handler.go @@ -69,39 +69,28 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if (h.shouldRedirect(r)) && (r.Method != "POST") { + queries, _ := url.ParseQuery(r.URL.RawQuery) + if queries.Get(h.config.RedirectQueryParameter) != "" { + cookie := http.Cookie{ + Name: h.config.RedirectQueryParameter, + Value: queries.Get(h.config.RedirectQueryParameter), + } + spew.Dump(cookie) + http.SetCookie(w, &cookie) + } + } + _, err := h.oauth.GetConfigFromRequest(r) if err == nil { h.handleOauth(w, r) return } - if h.shouldSetCookie(r) { - queries, _ := url.ParseQuery(r.URL.RawQuery) - cookie := http.Cookie{Name: "redirect_url", Value: queries.Get("backTo")} - http.SetCookie(w, &cookie) - } h.handleLogin(w, r) return } -func (h *Handler) shouldSetCookie(r *http.Request) bool { - if h.config.AllowRedirects { - if h.config.CheckRefererOnRedirects { - referer, _ := url.Parse(r.Header.Get("Referer")) - if referer.Host != r.Host { - logging.Application(r.Header).Warnf( - "Referer domain: '%s' does not match current domain '%s'", - referer.Host, - r.Host, - ) - return false - } - } - return true - } - return false -} - func (h *Handler) handleOauth(w http.ResponseWriter, r *http.Request) { startedFlow, authenticated, userInfo, err := h.oauth.Handle(w, r) @@ -143,7 +132,7 @@ func (h *Handler) handleLogin(w http.ResponseWriter, r *http.Request) { r.ParseForm() if r.Method == "DELETE" || r.FormValue("logout") == "true" { - h.deleteToken(w) + h.deleteCookie(w, h.config.CookieName) if h.config.LogoutURL != "" { w.Header().Set("Location", h.config.LogoutURL) w.WriteHeader(303) @@ -218,9 +207,9 @@ func (h *Handler) handleRefresh(w http.ResponseWriter, r *http.Request, userInfo } } -func (h *Handler) deleteToken(w http.ResponseWriter) { +func (h *Handler) deleteCookie(w http.ResponseWriter, cookieName string) { cookie := &http.Cookie{ - Name: h.config.CookieName, + Name: cookieName, Value: "delete", HttpOnly: true, Expires: time.Unix(0, 0), @@ -257,10 +246,11 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u http.SetCookie(w, cookie) - //redirectURL := h.config.SuccessURL - //fmt.Printf("redirectURL is: %s\n", h.redirectURL(r)) - - w.Header().Set("Location", h.redirectURL(r)) + w.Header().Set("Location", h.redirectURL(r, w)) + _, err := r.Cookie(h.config.RedirectQueryParameter) + if err == nil { + h.deleteCookie(w, h.config.RedirectQueryParameter) + } w.WriteHeader(303) return } @@ -270,63 +260,6 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u fmt.Fprintf(w, "%s", token) } -func (h *Handler) redirectURL(r *http.Request) string { - origin, _ := url.Parse(r.Header.Get("Origin")) - originScheme := origin.Scheme - originHost := origin.Host - redirectURL := fmt.Sprintf("%s://%s%s", originScheme, originHost, h.config.SuccessURL) - if h.config.AllowRedirects { - cookie, _ := r.Cookie("redirect_url") - parsedURL, err := url.Parse(cookie.Value) - if err != nil { - spew.Dump("oops can't parse that url") - } - scheme := parsedURL.Scheme - host := parsedURL.Host - path := parsedURL.Path - //spew.Dump(scheme) - //spew.Dump(host) - //spew.Dump(path) - - if scheme == "" { - scheme = originScheme - } - - if host == "" { - host = originHost - } else { - if h.config.PreventExternalRedirects { - if host != originHost { - logging.Application(r.Header).Warnf( - "Redirect attempt to '%s' but -prevent-external-redirects is set to true", - host, - ) - return redirectURL - } else { - host = originHost - } - } else { - //TODO read domains from file - domains := "2google.com" - if host != domains { - logging.Application(r.Header).Warnf( - "Redirect attempt to '%s' but it is not in domain whitelist", - host, - ) - return redirectURL - } - } - } - - if path == "" { - path = h.config.SuccessURL - } - - redirectURL = fmt.Sprintf("%s://%s%s", scheme, host, path) - } - return redirectURL -} - func (h *Handler) createToken(userInfo jwt.Claims) (string, error) { token := jwt.NewWithClaims(jwt.SigningMethodHS512, userInfo) return token.SignedString([]byte(h.config.JwtSecret)) diff --git a/login/handler_test.go b/login/handler_test.go index 12032ff..2287a08 100644 --- a/login/handler_test.go +++ b/login/handler_test.go @@ -3,22 +3,27 @@ package login import ( "errors" "fmt" - "github.com/dgrijalva/jwt-go" - . "github.com/stretchr/testify/assert" - "github.com/tarent/loginsrv/model" - "github.com/tarent/loginsrv/oauth2" + "io/ioutil" "net/http" "net/http/httptest" + "os" "strconv" "strings" "testing" "time" + + "github.com/dgrijalva/jwt-go" + . "github.com/stretchr/testify/assert" + "github.com/tarent/loginsrv/model" + "github.com/tarent/loginsrv/oauth2" ) const TypeJSON = "Content-Type: application/json" const TypeForm = "Content-Type: application/x-www-form-urlencoded" const AcceptHTML = "Accept: text/html" const AcceptJwt = "Accept: application/jwt" +const Host = "Host: example.com" +const BadReferer = "Referer: http://evildomain.com" func testConfig() *Config { testConfig := DefaultConfig() @@ -238,6 +243,110 @@ func TestHandler_LoginWeb(t *testing.T) { Equal(t, recorder.Header().Get("Set-Cookie"), "") } +func TestHandler_Redirect(t *testing.T) { + //by default set redirect_cookie + recorder := call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML)) + setCookieList := readSetCookies(recorder.Header()) + Equal(t, 1, len(setCookieList)) + cookie := setCookieList[0] + Equal(t, "backTo", cookie.Name) + Equal(t, "/website", cookie.Value) + + //by default allowed redirects + recorder = call(req("POST", "/context/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/website", recorder.Header().Get("Location")) + + //redirect to SuccessURL if AllowRedirects is false + cfg := DefaultConfig() + cfg.AllowRedirects = false + h := &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //by default don't set redirect cookie if Referer doesn't match origin + recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 0, len(setCookieList)) + + //don't set redirect cookie if referrer is malformed + recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, "Referer: :notvalid")) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 0, len(setCookieList)) + + //set redirect cookie with mismatch referer if CheckRefererOnRedirects is false + cfg = DefaultConfig() + cfg.CheckRefererOnRedirects = false + h = &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("GET", "/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 1, len(setCookieList)) + cookie = setCookieList[0] + Equal(t, "backTo", cookie.Name) + Equal(t, "/website", cookie.Value) + + //by default prevent redirect to external site + recorder = call(req("POST", "/context/login?backTo=//evildomain.com/phishing.html", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //by default if the parsed path is empty redirect to SuccessURL + recorder = call(req("POST", "/context/login?backTo=https://evildomain.com", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //redirect to success url if domains whitelist file doesn't exist + cfg = DefaultConfig() + cfg.PreventExternalRedirects = false + cfg.WhitelistDomainsFile = "domains_whitelist.txt" + h = &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + //setup domain whitelist file + d1 := []byte("gooddomain.com\n") + _ = ioutil.WriteFile("domains_whitelist.txt", d1, 0644) + + //allow redirect to domains on whitelist + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "https://gooddomain.com/website", recorder.Header().Get("Location")) + + //allow redirect to domains on whitelist + recorder = httptest.NewRecorder() + 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")) + + //remove domains whitelist file + err := os.Remove("domains_whitelist.txt") + Equal(t, nil, err) +} + func TestHandler_Refresh(t *testing.T) { h := testHandler() input := model.UserInfo{Sub: "bob", Expiry: time.Now().Add(time.Second).Unix()} diff --git a/login/redirect.go b/login/redirect.go new file mode 100644 index 0000000..256c5ef --- /dev/null +++ b/login/redirect.go @@ -0,0 +1,119 @@ +package login + +import ( + "bufio" + "errors" + "fmt" + "net/http" + "net/url" + "os" + + "github.com/tarent/loginsrv/logging" +) + +func (h *Handler) shouldRedirect(r *http.Request) bool { + if h.config.AllowRedirects { + if h.config.CheckRefererOnRedirects { + referer, err := url.Parse(r.Header.Get("Referer")) + if err != nil { + logging.Application(r.Header).Warnf( + "couldn't parse redirect url %s", + err, + ) + return false + } + if referer.Host != r.Host { + logging.Application(r.Header).Warnf( + "Referer domain: '%s' does not match current domain '%s'", + referer.Host, + r.Host, + ) + return false + } + } + return true + } + return false +} + +func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { + if h.config.AllowRedirects { + parsedURL, err := h.parseURL(r) + if err != nil { + logging.Application(r.Header).Warnf( + "error parsing redict URL: %s", + err, + ) + return h.config.SuccessURL + } + if h.config.PreventExternalRedirects { + if parsedURL.Path == "" { + return h.config.SuccessURL + } else { + if (parsedURL.Host != "") && (r.Host != parsedURL.Host) { + logging.Application(r.Header).Warnf( + "Attempted redirect to %s", + parsedURL.Host, + ) + return h.config.SuccessURL + } + return parsedURL.Path + } + } else { + if h.checkWhiteListDomains(r, parsedURL.Host) { + return fmt.Sprintf( + "%s://%s%s", + parsedURL.Scheme, + parsedURL.Host, + parsedURL.Path, + ) + } else { + return h.config.SuccessURL + } + } + } + return h.config.SuccessURL +} + +func (h *Handler) parseURL(r *http.Request) (*url.URL, error) { + cookie, err := r.Cookie(h.config.RedirectQueryParameter) + if err != nil { + //try reading parameter as it might be a POST request and so not have set the cookie yet + queries, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + return nil, err + } + if queries.Get(h.config.RedirectQueryParameter) != "" { + parsedURL, err := url.Parse(queries.Get(h.config.RedirectQueryParameter)) + return parsedURL, err + } else { + return nil, errors.New("no redirect") + } + } + parsedURL, err := url.Parse(cookie.Value) + return parsedURL, err +} + +func (h *Handler) checkWhiteListDomains(r *http.Request, host string) bool { + f, err := os.Open(h.config.WhitelistDomainsFile) + defer f.Close() + if err != nil { + logging.Application(r.Header).Warnf( + "can't open domains file '%s'", + h.config.WhitelistDomainsFile, + ) + return false + } + scanner := bufio.NewScanner(f) + scanner.Split(bufio.ScanLines) + for scanner.Scan() { + if host == scanner.Text() { + return true + } + } + logging.Application(r.Header).Warnf( + "Domain '%s' not in whitelist", + host, + ) + return false +} From 2217bfaddb9d271400a158baac5460c6d943b244 Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Thu, 23 Nov 2017 16:40:57 +0000 Subject: [PATCH 05/12] Remove forgotten spew --- login/handler.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/login/handler.go b/login/handler.go index 36530a0..1299823 100644 --- a/login/handler.go +++ b/login/handler.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/davecgh/go-spew/spew" "github.com/dgrijalva/jwt-go" "github.com/tarent/loginsrv/logging" "github.com/tarent/loginsrv/model" @@ -76,7 +75,6 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { Name: h.config.RedirectQueryParameter, Value: queries.Get(h.config.RedirectQueryParameter), } - spew.Dump(cookie) http.SetCookie(w, &cookie) } } From 3474245879f45587e39278b288969ffbcfefaeea Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Wed, 29 Nov 2017 14:40:16 +0000 Subject: [PATCH 06/12] don't reuse the deleteToken function but explicitly sdelete it like we set it --- login/handler.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/login/handler.go b/login/handler.go index 1299823..f0f7a52 100644 --- a/login/handler.go +++ b/login/handler.go @@ -130,7 +130,7 @@ func (h *Handler) handleLogin(w http.ResponseWriter, r *http.Request) { r.ParseForm() if r.Method == "DELETE" || r.FormValue("logout") == "true" { - h.deleteCookie(w, h.config.CookieName) + h.deleteToken(w) if h.config.LogoutURL != "" { w.Header().Set("Location", h.config.LogoutURL) w.WriteHeader(303) @@ -205,9 +205,9 @@ func (h *Handler) handleRefresh(w http.ResponseWriter, r *http.Request, userInfo } } -func (h *Handler) deleteCookie(w http.ResponseWriter, cookieName string) { +func (h *Handler) deleteToken(w http.ResponseWriter) { cookie := &http.Cookie{ - Name: cookieName, + Name: h.config.CookieName, Value: "delete", HttpOnly: true, Expires: time.Unix(0, 0), @@ -247,7 +247,12 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u w.Header().Set("Location", h.redirectURL(r, w)) _, err := r.Cookie(h.config.RedirectQueryParameter) if err == nil { - h.deleteCookie(w, h.config.RedirectQueryParameter) + cookie := http.Cookie{ + Name: h.config.RedirectQueryParameter, + Value: "delete", + Expires: time.Unix(0, 0), + } + http.SetCookie(w, &cookie) } w.WriteHeader(303) return From d62ed493ba371d5044588bd3423f26715bdf6b46 Mon Sep 17 00:00:00 2001 From: "luke.hopkins" Date: Wed, 29 Nov 2017 14:41:28 +0000 Subject: [PATCH 07/12] fix typo --- login/redirect.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/login/redirect.go b/login/redirect.go index 256c5ef..4988650 100644 --- a/login/redirect.go +++ b/login/redirect.go @@ -41,7 +41,7 @@ func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { parsedURL, err := h.parseURL(r) if err != nil { logging.Application(r.Header).Warnf( - "error parsing redict URL: %s", + "error parsing redirect URL: %s", err, ) return h.config.SuccessURL From ea40df699b8c3f3fc0f8b1db309ef83e2afc0858 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 9 Jan 2018 22:29:59 +0100 Subject: [PATCH 08/12] Overworked the dynamic redirect patch * Reduced nesting in functions with early returns * Some renamings * Resolve whitelist path relative to caddy config * Moved all redirect methods to redirect.go and redirect_test.go * Only read the url direct from the parameter, if it is a post request * Removed the parameter prevent-external-redirects, because it can be implicit assumed, when a whitelist is configured --- caddy/setup.go | 8 ++- caddy/setup_test.go | 120 ++++++++++++++++----------------- login/config.go | 85 ++++++++++++----------- login/config_test.go | 88 ++++++++++++------------ login/handler.go | 22 +----- login/handler_test.go | 108 ------------------------------ login/redirect.go | 148 +++++++++++++++++++---------------------- login/redirect_test.go | 126 +++++++++++++++++++++++++++++++++++ 8 files changed, 347 insertions(+), 358 deletions(-) create mode 100644 login/redirect_test.go diff --git a/caddy/setup.go b/caddy/setup.go index 10362d5..926608f 100644 --- a/caddy/setup.go +++ b/caddy/setup.go @@ -43,6 +43,10 @@ func setup(c *caddy.Controller) error { config.Template = filepath.Join(httpserver.GetConfig(c).Root, config.Template) } + if config.WhitelistDomainsFile != "" && !filepath.IsAbs(config.WhitelistDomainsFile) { + config.WhitelistDomainsFile = filepath.Join(httpserver.GetConfig(c).Root, config.WhitelistDomainsFile) + } + if len(args) == 1 { logging.Logger.Warnf("DEPRECATED: Please set the login path by parameter login_path and not as directive argument (%v:%v)", c.File(), c.Line()) config.LoginPath = path.Join(args[0], "/login") @@ -89,11 +93,11 @@ func parseConfig(c *caddy.Controller) (*login.Config, error) { f := fs.Lookup(name) if f == nil { - return cfg, c.ArgErr() + return cfg, fmt.Errorf("Unknown parameter for login directive: %v (%v:%v)", name, c.File(), c.Line()) } err := f.Value.Set(value) if err != nil { - return cfg, c.Err(err.Error()) + return cfg, fmt.Errorf("Invalid value for parameter %v: %v (%v:%v)", name, value, c.File(), c.Line()) } } diff --git a/caddy/setup_test.go b/caddy/setup_test.go index 9a894be..613db2f 100644 --- a/caddy/setup_test.go +++ b/caddy/setup_test.go @@ -4,7 +4,6 @@ import ( "fmt" "io/ioutil" "os" - "path/filepath" "testing" "time" @@ -29,16 +28,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -52,9 +50,10 @@ func TestSetup(t *testing.T) { success_url successurl jwt_expiry 42h login_path /foo/bar - allow-redirects true - prevent-external-redirects true - check-referer-on-redirects true + allow_redirects true + redirect_query_parameter comingFrom + check_referer_on_redirects true + whitelist_domains_file domainWhitelist.txt cookie_name cookiename cookie_http_only false cookie_domain example.com @@ -64,18 +63,18 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 42 * time.Hour, - SuccessURL: "successurl", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - LoginPath: "/foo/bar", - CookieName: "cookiename", - CookieDomain: "example.com", - CookieExpiry: 23*time.Hour + 23*time.Minute, - CookieHTTPOnly: false, + JwtSecret: "jwtsecret", + JwtExpiry: 42 * time.Hour, + SuccessURL: "successurl", + AllowRedirects: true, + RedirectQueryParameter: "comingFrom", + CheckRefererOnRedirects: true, + WhitelistDomainsFile: "domainWhitelist.txt", + LoginPath: "/foo/bar", + CookieName: "cookiename", + CookieDomain: "example.com", + CookieExpiry: 23*time.Hour + 23*time.Minute, + CookieHTTPOnly: false, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -99,16 +98,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - LoginPath: "/context/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + CheckRefererOnRedirects: true, + LoginPath: "/context/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -127,16 +125,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - LoginPath: "/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -153,16 +150,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + CheckRefererOnRedirects: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -198,10 +194,13 @@ func TestSetup(t *testing.T) { } } -func TestSetup_RelativeTemplateFile(t *testing.T) { - caddyfile := "loginsrv {\n template myTemplate.tpl\n simple bob=secret\n}" +func TestSetup_RelativeFiles(t *testing.T) { + caddyfile := `loginsrv { + template myTemplate.tpl + whitelist_domains_file redirectDomains.txt + simple bob=secret + }` root, _ := ioutil.TempDir("", "") - expectedPath := filepath.FromSlash(root + "/myTemplate.tpl") c := caddy.NewTestController("http", caddyfile) c.Key = "RelativeTemplateFileTest" @@ -216,5 +215,6 @@ func TestSetup_RelativeTemplateFile(t *testing.T) { } middleware := mids[len(mids)-1](nil).(*CaddyHandler) - Equal(t, expectedPath, middleware.config.Template) + Equal(t, root+"/myTemplate.tpl", middleware.config.Template) + Equal(t, root+"/redirectDomains.txt", middleware.config.WhitelistDomainsFile) } diff --git a/login/config.go b/login/config.go index ee14275..2759b80 100644 --- a/login/config.go +++ b/login/config.go @@ -23,25 +23,24 @@ func init() { // DefaultConfig for the loginsrv handler func DefaultConfig() *Config { return &Config{ - Host: "localhost", - Port: "6789", - LogLevel: "info", - JwtSecret: jwtDefaultSecret, - JwtExpiry: 24 * time.Hour, - JwtRefreshes: 0, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - WhitelistDomainsFile: "", - LogoutURL: "", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, - Backends: Options{}, - Oauth: Options{}, - GracePeriod: 5 * time.Second, + Host: "localhost", + Port: "6789", + LogLevel: "info", + JwtSecret: jwtDefaultSecret, + JwtExpiry: 24 * time.Hour, + JwtRefreshes: 0, + SuccessURL: "/", + AllowRedirects: true, + RedirectQueryParameter: "backTo", + CheckRefererOnRedirects: true, + WhitelistDomainsFile: "", + LogoutURL: "", + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, + Backends: Options{}, + Oauth: Options{}, + GracePeriod: 5 * time.Second, } } @@ -49,29 +48,28 @@ const envPrefix = "LOGINSRV_" // Config for the loginsrv handler type Config struct { - Host string - Port string - LogLevel string - TextLogging bool - JwtSecret string - JwtExpiry time.Duration - JwtRefreshes int - SuccessURL string - AllowRedirects bool - RedirectQueryParameter string - PreventExternalRedirects bool - CheckRefererOnRedirects bool - WhitelistDomainsFile string - LogoutURL string - Template string - LoginPath string - CookieName string - CookieExpiry time.Duration - CookieDomain string - CookieHTTPOnly bool - Backends Options - Oauth Options - GracePeriod time.Duration + Host string + Port string + LogLevel string + TextLogging bool + JwtSecret string + JwtExpiry time.Duration + JwtRefreshes int + SuccessURL string + AllowRedirects bool + RedirectQueryParameter string + CheckRefererOnRedirects bool + WhitelistDomainsFile string + LogoutURL string + Template string + LoginPath string + CookieName string + CookieExpiry time.Duration + CookieDomain string + CookieHTTPOnly bool + Backends Options + Oauth Options + GracePeriod time.Duration } // Options is the configuration structure for oauth and backend provider @@ -116,9 +114,8 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.StringVar(&c.SuccessURL, "success-url", c.SuccessURL, "The url to redirect after login") f.BoolVar(&c.AllowRedirects, "allow-redirects", c.AllowRedirects, "Allow dynamic redirects by parameter") f.StringVar(&c.RedirectQueryParameter, "redirect-query-parameter", c.RedirectQueryParameter, "Allow dynamic redirects by parameter") - f.BoolVar(&c.PreventExternalRedirects, "prevent-external-redirects", c.PreventExternalRedirects, "Prevent dynamic redirects from redirecting to an external domain") f.BoolVar(&c.CheckRefererOnRedirects, "check-referer-on-redirects", c.CheckRefererOnRedirects, "When redirecting check that the referer is the same domain") - f.StringVar(&c.WhitelistDomainsFile, "whitelist-domains-file", c.WhitelistDomainsFile, "the file containing a list of domains that redirects are allowed to, one domain per line") + f.StringVar(&c.WhitelistDomainsFile, "whitelist-domains-file", c.WhitelistDomainsFile, "A file containing a list of domains that redirects are allowed to, one domain per line") 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/config_test.go b/login/config_test.go index 95dc59d..93ace72 100644 --- a/login/config_test.go +++ b/login/config_test.go @@ -29,10 +29,9 @@ func TestConfig_ReadConfig(t *testing.T) { "--jwt-secret=jwtsecret", "--jwt-expiry=42h42m", "--success-url=successurl", - "--allow-redirects=true", - "--redirect-query-parameter=backTo", - "--prevent-external-redirects=true", - "--check-referer-on-redirects=true", + "--allow-redirects=false", + "--redirect-query-parameter=comingFrom", + "--check-referer-on-redirects=false", "--whitelist-domains-file=File", "--logout-url=logouturl", "--template=template", @@ -48,25 +47,24 @@ func TestConfig_ReadConfig(t *testing.T) { } expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - WhitelistDomainsFile: "File", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + AllowRedirects: false, + RedirectQueryParameter: "comingFrom", + CheckRefererOnRedirects: false, + WhitelistDomainsFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{}, "foo": map[string]string{}, @@ -93,10 +91,9 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_JWT_SECRET", "jwtsecret")) NoError(t, os.Setenv("LOGINSRV_JWT_EXPIRY", "42h42m")) NoError(t, os.Setenv("LOGINSRV_SUCCESS_URL", "successurl")) - NoError(t, os.Setenv("LOGINSRV_ALLOW_REDIRECTS", "true")) - NoError(t, os.Setenv("LOGINSRV_REDIRECT_QUERY_PARAMETER", "backTo")) - NoError(t, os.Setenv("LOGINSRV_PREVENT_EXTERNAL_REDIRECTS", "true")) - NoError(t, os.Setenv("LOGINSRV_CHECK_REFERER_ON_REDIRECTS", "true")) + NoError(t, os.Setenv("LOGINSRV_ALLOW_REDIRECTS", "false")) + NoError(t, os.Setenv("LOGINSRV_REDIRECT_QUERY_PARAMETER", "comingFrom")) + NoError(t, os.Setenv("LOGINSRV_CHECK_REFERER_ON_REDIRECTS", "false")) NoError(t, os.Setenv("LOGINSRV_WHITELIST_DOMAINS_FILE", "File")) NoError(t, os.Setenv("LOGINSRV_LOGOUT_URL", "logouturl")) NoError(t, os.Setenv("LOGINSRV_TEMPLATE", "template")) @@ -110,25 +107,24 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_GRACE_PERIOD", "4s")) expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - PreventExternalRedirects: true, - CheckRefererOnRedirects: true, - WhitelistDomainsFile: "File", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + AllowRedirects: false, + RedirectQueryParameter: "comingFrom", + CheckRefererOnRedirects: false, + WhitelistDomainsFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{ "foo": "bar", diff --git a/login/handler.go b/login/handler.go index f0f7a52..572c2ea 100644 --- a/login/handler.go +++ b/login/handler.go @@ -6,7 +6,6 @@ import ( "fmt" "io/ioutil" "net/http" - "net/url" "strings" "time" @@ -68,16 +67,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - if (h.shouldRedirect(r)) && (r.Method != "POST") { - queries, _ := url.ParseQuery(r.URL.RawQuery) - if queries.Get(h.config.RedirectQueryParameter) != "" { - cookie := http.Cookie{ - Name: h.config.RedirectQueryParameter, - Value: queries.Get(h.config.RedirectQueryParameter), - } - http.SetCookie(w, &cookie) - } - } + h.setRedirectCookie(w, r) _, err := h.oauth.GetConfigFromRequest(r) if err == nil { @@ -245,15 +235,7 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u http.SetCookie(w, cookie) w.Header().Set("Location", h.redirectURL(r, w)) - _, err := r.Cookie(h.config.RedirectQueryParameter) - if err == nil { - cookie := http.Cookie{ - Name: h.config.RedirectQueryParameter, - Value: "delete", - Expires: time.Unix(0, 0), - } - http.SetCookie(w, &cookie) - } + h.deleteRedirectCookie(w, r) w.WriteHeader(303) return } diff --git a/login/handler_test.go b/login/handler_test.go index 2287a08..e1d659c 100644 --- a/login/handler_test.go +++ b/login/handler_test.go @@ -3,10 +3,8 @@ package login import ( "errors" "fmt" - "io/ioutil" "net/http" "net/http/httptest" - "os" "strconv" "strings" "testing" @@ -22,8 +20,6 @@ const TypeJSON = "Content-Type: application/json" const TypeForm = "Content-Type: application/x-www-form-urlencoded" const AcceptHTML = "Accept: text/html" const AcceptJwt = "Accept: application/jwt" -const Host = "Host: example.com" -const BadReferer = "Referer: http://evildomain.com" func testConfig() *Config { testConfig := DefaultConfig() @@ -243,110 +239,6 @@ func TestHandler_LoginWeb(t *testing.T) { Equal(t, recorder.Header().Get("Set-Cookie"), "") } -func TestHandler_Redirect(t *testing.T) { - //by default set redirect_cookie - recorder := call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML)) - setCookieList := readSetCookies(recorder.Header()) - Equal(t, 1, len(setCookieList)) - cookie := setCookieList[0] - Equal(t, "backTo", cookie.Name) - Equal(t, "/website", cookie.Value) - - //by default allowed redirects - recorder = call(req("POST", "/context/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) - Equal(t, 303, recorder.Code) - Equal(t, "/website", recorder.Header().Get("Location")) - - //redirect to SuccessURL if AllowRedirects is false - cfg := DefaultConfig() - cfg.AllowRedirects = false - h := &Handler{ - backends: []Backend{ - NewSimpleBackend(map[string]string{"bob": "secret"}), - }, - oauth: oauth2.NewManager(), - config: cfg, - } - recorder = httptest.NewRecorder() - h.ServeHTTP(recorder, req("POST", "/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) - Equal(t, 303, recorder.Code) - Equal(t, "/", recorder.Header().Get("Location")) - - //by default don't set redirect cookie if Referer doesn't match origin - recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) - setCookieList = readSetCookies(recorder.Header()) - Equal(t, 0, len(setCookieList)) - - //don't set redirect cookie if referrer is malformed - recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, "Referer: :notvalid")) - setCookieList = readSetCookies(recorder.Header()) - Equal(t, 0, len(setCookieList)) - - //set redirect cookie with mismatch referer if CheckRefererOnRedirects is false - cfg = DefaultConfig() - cfg.CheckRefererOnRedirects = false - h = &Handler{ - backends: []Backend{ - NewSimpleBackend(map[string]string{"bob": "secret"}), - }, - oauth: oauth2.NewManager(), - config: cfg, - } - recorder = httptest.NewRecorder() - h.ServeHTTP(recorder, req("GET", "/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) - setCookieList = readSetCookies(recorder.Header()) - Equal(t, 1, len(setCookieList)) - cookie = setCookieList[0] - Equal(t, "backTo", cookie.Name) - Equal(t, "/website", cookie.Value) - - //by default prevent redirect to external site - recorder = call(req("POST", "/context/login?backTo=//evildomain.com/phishing.html", "username=bob&password=secret", TypeForm, AcceptHTML)) - Equal(t, 303, recorder.Code) - Equal(t, "/", recorder.Header().Get("Location")) - - //by default if the parsed path is empty redirect to SuccessURL - recorder = call(req("POST", "/context/login?backTo=https://evildomain.com", "username=bob&password=secret", TypeForm, AcceptHTML)) - Equal(t, 303, recorder.Code) - Equal(t, "/", recorder.Header().Get("Location")) - - //redirect to success url if domains whitelist file doesn't exist - cfg = DefaultConfig() - cfg.PreventExternalRedirects = false - cfg.WhitelistDomainsFile = "domains_whitelist.txt" - h = &Handler{ - backends: []Backend{ - NewSimpleBackend(map[string]string{"bob": "secret"}), - }, - oauth: oauth2.NewManager(), - config: cfg, - } - recorder = httptest.NewRecorder() - h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) - Equal(t, 303, recorder.Code) - Equal(t, "/", recorder.Header().Get("Location")) - - //setup domain whitelist file - d1 := []byte("gooddomain.com\n") - _ = ioutil.WriteFile("domains_whitelist.txt", d1, 0644) - - //allow redirect to domains on whitelist - recorder = httptest.NewRecorder() - h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) - Equal(t, 303, recorder.Code) - Equal(t, "https://gooddomain.com/website", recorder.Header().Get("Location")) - - //allow redirect to domains on whitelist - recorder = httptest.NewRecorder() - 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")) - - //remove domains whitelist file - err := os.Remove("domains_whitelist.txt") - Equal(t, nil, err) -} - func TestHandler_Refresh(t *testing.T) { h := testHandler() input := model.UserInfo{Sub: "bob", Expiry: time.Now().Add(time.Second).Unix()} diff --git a/login/redirect.go b/login/redirect.go index 4988650..650c972 100644 --- a/login/redirect.go +++ b/login/redirect.go @@ -2,118 +2,110 @@ package login import ( "bufio" - "errors" - "fmt" "net/http" "net/url" "os" "github.com/tarent/loginsrv/logging" + "strings" + "time" ) -func (h *Handler) shouldRedirect(r *http.Request) bool { - if h.config.AllowRedirects { - if h.config.CheckRefererOnRedirects { - referer, err := url.Parse(r.Header.Get("Referer")) - if err != nil { - logging.Application(r.Header).Warnf( - "couldn't parse redirect url %s", - err, - ) - return false - } - if referer.Host != r.Host { - logging.Application(r.Header).Warnf( - "Referer domain: '%s' does not match current domain '%s'", - referer.Host, - r.Host, - ) - return false - } +func (h *Handler) setRedirectCookie(w http.ResponseWriter, r *http.Request) { + redirectTo := r.URL.Query().Get(h.config.RedirectQueryParameter) + if redirectTo != "" && h.allowRedirect(r) && r.Method != "POST" { + cookie := http.Cookie{ + Name: h.config.RedirectQueryParameter, + Value: redirectTo, } + http.SetCookie(w, &cookie) + } +} + +func (h *Handler) deleteRedirectCookie(w http.ResponseWriter, r *http.Request) { + _, err := r.Cookie(h.config.RedirectQueryParameter) + if err == nil { + cookie := http.Cookie{ + Name: h.config.RedirectQueryParameter, + Value: "delete", + Expires: time.Unix(0, 0), + } + http.SetCookie(w, &cookie) + } +} + +func (h *Handler) allowRedirect(r *http.Request) bool { + if !h.config.AllowRedirects { + return false + } + if !h.config.CheckRefererOnRedirects { return true } - return false + + referer, err := url.Parse(r.Header.Get("Referer")) + if err != nil { + 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 + } + return true } func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { - if h.config.AllowRedirects { - parsedURL, err := h.parseURL(r) - if err != nil { - logging.Application(r.Header).Warnf( - "error parsing redirect URL: %s", - err, - ) - return h.config.SuccessURL + targetURL, foundTarget := h.getRedirectTarget(r) + if foundTarget && h.config.AllowRedirects { + sameHost := targetURL.Host == "" || r.Host == targetURL.Host + if sameHost && targetURL.Path != "" { + return targetURL.Path } - if h.config.PreventExternalRedirects { - if parsedURL.Path == "" { - return h.config.SuccessURL - } else { - if (parsedURL.Host != "") && (r.Host != parsedURL.Host) { - logging.Application(r.Header).Warnf( - "Attempted redirect to %s", - parsedURL.Host, - ) - return h.config.SuccessURL - } - return parsedURL.Path - } - } else { - if h.checkWhiteListDomains(r, parsedURL.Host) { - return fmt.Sprintf( - "%s://%s%s", - parsedURL.Scheme, - parsedURL.Host, - parsedURL.Path, - ) - } else { - return h.config.SuccessURL - } + if !sameHost && h.isRedirectDomainWhitelisted(r, targetURL.Host) { + return targetURL.String() } } return h.config.SuccessURL } -func (h *Handler) parseURL(r *http.Request) (*url.URL, error) { +func (h *Handler) getRedirectTarget(r *http.Request) (*url.URL, bool) { cookie, err := r.Cookie(h.config.RedirectQueryParameter) - if err != nil { - //try reading parameter as it might be a POST request and so not have set the cookie yet - queries, err := url.ParseQuery(r.URL.RawQuery) + if err == nil { + url, err := url.Parse(cookie.Value) if err != nil { - return nil, err - } - if queries.Get(h.config.RedirectQueryParameter) != "" { - parsedURL, err := url.Parse(queries.Get(h.config.RedirectQueryParameter)) - return parsedURL, err - } else { - return nil, errors.New("no redirect") + logging.Application(r.Header).Warnf("error parsing redirect URL: %s", err) + return nil, false } + return url, true } - parsedURL, err := url.Parse(cookie.Value) - return parsedURL, err + + // try reading parameter as it might be a POST request and so not have set the cookie yet + redirectTo := r.URL.Query().Get(h.config.RedirectQueryParameter) + if redirectTo == "" || r.Method != "POST" { + return nil, false + } + url, err := url.Parse(redirectTo) + if err != nil { + logging.Application(r.Header).Warnf("error parsing redirect URL: %s", err) + return nil, false + } + return url, true } -func (h *Handler) checkWhiteListDomains(r *http.Request, host string) bool { +func (h *Handler) isRedirectDomainWhitelisted(r *http.Request, host string) bool { f, err := os.Open(h.config.WhitelistDomainsFile) - defer f.Close() if err != nil { - logging.Application(r.Header).Warnf( - "can't open domains file '%s'", - h.config.WhitelistDomainsFile, - ) + logging.Application(r.Header).Warnf("can't open redirect whitelist domains file '%s'", h.config.WhitelistDomainsFile) return false } + defer f.Close() scanner := bufio.NewScanner(f) scanner.Split(bufio.ScanLines) for scanner.Scan() { - if host == scanner.Text() { + if host == strings.TrimSpace(scanner.Text()) { return true } } - logging.Application(r.Header).Warnf( - "Domain '%s' not in whitelist", - host, - ) + logging.Application(r.Header).Warnf("domain '%s' not in redirect whitelist", host) return false } diff --git a/login/redirect_test.go b/login/redirect_test.go new file mode 100644 index 0000000..fd25740 --- /dev/null +++ b/login/redirect_test.go @@ -0,0 +1,126 @@ +package login + +import ( + "net/http/httptest" + "os" + "testing" + + . "github.com/stretchr/testify/assert" + "github.com/tarent/loginsrv/oauth2" + "io/ioutil" +) + +const BadReferer = "Referer: http://evildomain.com" + +func TestRedirect(t *testing.T) { + // by default set redirect_cookie + recorder := call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML)) + setCookieList := readSetCookies(recorder.Header()) + Equal(t, 1, len(setCookieList)) + cookie := setCookieList[0] + Equal(t, "backTo", cookie.Name) + Equal(t, "/website", cookie.Value) + + // by default allowed redirects + recorder = call(req("POST", "/context/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/website", recorder.Header().Get("Location")) +} + +func TestRedirect_NotAllowed(t *testing.T) { + // redirect to SuccessURL if AllowRedirects is false + cfg := DefaultConfig() + cfg.AllowRedirects = false + h := &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder := httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=/website", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) +} + +func TestRedirect_NonMatchingReferrer(t *testing.T) { + // by default don't set redirect cookie if Referer doesn't match origin + recorder := call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) + setCookieList := readSetCookies(recorder.Header()) + Equal(t, 0, len(setCookieList)) + + // don't set redirect cookie if referrer is malformed + recorder = call(req("GET", "/context/login?backTo=/website", "", TypeForm, AcceptHTML, "Referer: :notvalid")) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 0, len(setCookieList)) + + // set redirect cookie with mismatch referer if CheckRefererOnRedirects is false + cfg := DefaultConfig() + cfg.CheckRefererOnRedirects = false + h := &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("GET", "/login?backTo=/website", "", TypeForm, AcceptHTML, BadReferer)) + setCookieList = readSetCookies(recorder.Header()) + Equal(t, 1, len(setCookieList)) + cookie := setCookieList[0] + Equal(t, "backTo", cookie.Name) + Equal(t, "/website", cookie.Value) +} + +func TestRedirect_PreventExternal(t *testing.T) { + // by default prevent redirect to external site + recorder := call(req("POST", "/context/login?backTo=//evildomain.com/phishing.html", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + // by default if the parsed path is empty redirect to SuccessURL + recorder = call(req("POST", "/context/login?backTo=https://evildomain.com", "username=bob&password=secret", TypeForm, AcceptHTML)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) +} + +func TestRedirect_Whitelisting(t *testing.T) { + whitelistFile, _ := ioutil.TempFile("", "loginsrv_test_domains_whitelist") + whitelistFile.Close() + os.Remove(whitelistFile.Name()) + + // redirect to success url if domains whitelist file doesn't exist + cfg := DefaultConfig() + cfg.WhitelistDomainsFile = whitelistFile.Name() + h := &Handler{ + backends: []Backend{ + NewSimpleBackend(map[string]string{"bob": "secret"}), + }, + oauth: oauth2.NewManager(), + config: cfg, + } + recorder := httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "/", recorder.Header().Get("Location")) + + // setup domain whitelist file + domains := []byte("foo.com\ngooddomain.com \nbar.com") + _ = ioutil.WriteFile(whitelistFile.Name(), domains, 0644) + defer os.Remove(whitelistFile.Name()) + + // allow redirect to domains on whitelist + recorder = httptest.NewRecorder() + h.ServeHTTP(recorder, req("POST", "/login?backTo=https://gooddomain.com/website", "username=bob&password=secret", TypeForm, AcceptHTML, BadReferer)) + Equal(t, 303, recorder.Code) + Equal(t, "https://gooddomain.com/website", recorder.Header().Get("Location")) + + // still permit access to domains which are not in the whitelist + recorder = httptest.NewRecorder() + 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")) + +} From 83ccded6f013ef53cfb465e8b0a6cdbeae2a4715 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 9 Jan 2018 23:16:04 +0100 Subject: [PATCH 09/12] rename redirect config parameters allow-redirects -> redirect redirect-query-parameter -> redirect-query-parameter check-referer-on-redirects -> redirect-check-referer whitelist-domains-file -> redirect-host-file --- caddy/setup.go | 4 +- caddy/setup_test.go | 106 ++++++++++++++++++++--------------------- login/config.go | 88 +++++++++++++++++----------------- login/config_test.go | 84 ++++++++++++++++---------------- login/redirect.go | 17 ++++--- login/redirect_test.go | 10 ++-- 6 files changed, 157 insertions(+), 152 deletions(-) diff --git a/caddy/setup.go b/caddy/setup.go index 926608f..8a83206 100644 --- a/caddy/setup.go +++ b/caddy/setup.go @@ -43,8 +43,8 @@ func setup(c *caddy.Controller) error { config.Template = filepath.Join(httpserver.GetConfig(c).Root, config.Template) } - if config.WhitelistDomainsFile != "" && !filepath.IsAbs(config.WhitelistDomainsFile) { - config.WhitelistDomainsFile = filepath.Join(httpserver.GetConfig(c).Root, config.WhitelistDomainsFile) + if config.RedirectHostFile != "" && !filepath.IsAbs(config.RedirectHostFile) { + config.RedirectHostFile = filepath.Join(httpserver.GetConfig(c).Root, config.RedirectHostFile) } if len(args) == 1 { diff --git a/caddy/setup_test.go b/caddy/setup_test.go index 613db2f..7dc0987 100644 --- a/caddy/setup_test.go +++ b/caddy/setup_test.go @@ -28,15 +28,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - CheckRefererOnRedirects: true, - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + Redirect: true, + RedirectQueryParameter: "backTo", + RedirectCheckReferer: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -50,10 +50,10 @@ func TestSetup(t *testing.T) { success_url successurl jwt_expiry 42h login_path /foo/bar - allow_redirects true + redirect true redirect_query_parameter comingFrom - check_referer_on_redirects true - whitelist_domains_file domainWhitelist.txt + redirect_check_referer true + redirect_host_file domainWhitelist.txt cookie_name cookiename cookie_http_only false cookie_domain example.com @@ -63,18 +63,18 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 42 * time.Hour, - SuccessURL: "successurl", - AllowRedirects: true, - RedirectQueryParameter: "comingFrom", - CheckRefererOnRedirects: true, - WhitelistDomainsFile: "domainWhitelist.txt", - LoginPath: "/foo/bar", - CookieName: "cookiename", - CookieDomain: "example.com", - CookieExpiry: 23*time.Hour + 23*time.Minute, - CookieHTTPOnly: false, + JwtSecret: "jwtsecret", + JwtExpiry: 42 * time.Hour, + SuccessURL: "successurl", + Redirect: true, + RedirectQueryParameter: "comingFrom", + RedirectCheckReferer: true, + RedirectHostFile: "domainWhitelist.txt", + LoginPath: "/foo/bar", + CookieName: "cookiename", + CookieDomain: "example.com", + CookieExpiry: 23*time.Hour + 23*time.Minute, + CookieHTTPOnly: false, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -98,15 +98,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - CheckRefererOnRedirects: true, - LoginPath: "/context/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + Redirect: true, + RedirectQueryParameter: "backTo", + RedirectCheckReferer: true, + LoginPath: "/context/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -125,15 +125,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - CheckRefererOnRedirects: true, - LoginPath: "/login", - CookieName: "cookiename", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + Redirect: true, + RedirectQueryParameter: "backTo", + RedirectCheckReferer: true, + LoginPath: "/login", + CookieName: "cookiename", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -150,15 +150,15 @@ func TestSetup(t *testing.T) { }`, shouldErr: false, config: login.Config{ - JwtSecret: "jwtsecret", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - CheckRefererOnRedirects: true, - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, + JwtSecret: "jwtsecret", + JwtExpiry: 24 * time.Hour, + SuccessURL: "/", + Redirect: true, + RedirectQueryParameter: "backTo", + RedirectCheckReferer: true, + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, Backends: login.Options{ "simple": map[string]string{ "bob": "secret", @@ -197,7 +197,7 @@ func TestSetup(t *testing.T) { func TestSetup_RelativeFiles(t *testing.T) { caddyfile := `loginsrv { template myTemplate.tpl - whitelist_domains_file redirectDomains.txt + redirect_host_file redirectDomains.txt simple bob=secret }` root, _ := ioutil.TempDir("", "") @@ -216,5 +216,5 @@ func TestSetup_RelativeFiles(t *testing.T) { middleware := mids[len(mids)-1](nil).(*CaddyHandler) Equal(t, root+"/myTemplate.tpl", middleware.config.Template) - Equal(t, root+"/redirectDomains.txt", middleware.config.WhitelistDomainsFile) + Equal(t, root+"/redirectDomains.txt", middleware.config.RedirectHostFile) } diff --git a/login/config.go b/login/config.go index 2759b80..2f695b8 100644 --- a/login/config.go +++ b/login/config.go @@ -23,24 +23,24 @@ func init() { // DefaultConfig for the loginsrv handler func DefaultConfig() *Config { return &Config{ - Host: "localhost", - Port: "6789", - LogLevel: "info", - JwtSecret: jwtDefaultSecret, - JwtExpiry: 24 * time.Hour, - JwtRefreshes: 0, - SuccessURL: "/", - AllowRedirects: true, - RedirectQueryParameter: "backTo", - CheckRefererOnRedirects: true, - WhitelistDomainsFile: "", - LogoutURL: "", - LoginPath: "/login", - CookieName: "jwt_token", - CookieHTTPOnly: true, - Backends: Options{}, - Oauth: Options{}, - GracePeriod: 5 * time.Second, + Host: "localhost", + Port: "6789", + LogLevel: "info", + JwtSecret: jwtDefaultSecret, + JwtExpiry: 24 * time.Hour, + JwtRefreshes: 0, + SuccessURL: "/", + Redirect: true, + RedirectQueryParameter: "backTo", + RedirectCheckReferer: true, + RedirectHostFile: "", + LogoutURL: "", + LoginPath: "/login", + CookieName: "jwt_token", + CookieHTTPOnly: true, + Backends: Options{}, + Oauth: Options{}, + GracePeriod: 5 * time.Second, } } @@ -48,28 +48,28 @@ const envPrefix = "LOGINSRV_" // Config for the loginsrv handler type Config struct { - Host string - Port string - LogLevel string - TextLogging bool - JwtSecret string - JwtExpiry time.Duration - JwtRefreshes int - SuccessURL string - AllowRedirects bool - RedirectQueryParameter string - CheckRefererOnRedirects bool - WhitelistDomainsFile string - LogoutURL string - Template string - LoginPath string - CookieName string - CookieExpiry time.Duration - CookieDomain string - CookieHTTPOnly bool - Backends Options - Oauth Options - GracePeriod time.Duration + Host string + Port string + LogLevel string + TextLogging bool + JwtSecret string + JwtExpiry time.Duration + JwtRefreshes int + SuccessURL string + Redirect bool + RedirectQueryParameter string + RedirectCheckReferer bool + RedirectHostFile string + LogoutURL string + Template string + LoginPath string + CookieName string + CookieExpiry time.Duration + CookieDomain string + CookieHTTPOnly bool + Backends Options + Oauth Options + GracePeriod time.Duration } // Options is the configuration structure for oauth and backend provider @@ -112,10 +112,10 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.DurationVar(&c.CookieExpiry, "cookie-expiry", c.CookieExpiry, "The expiry duration for the cookie, e.g. 2h or 3h30m. Default is browser session") f.StringVar(&c.CookieDomain, "cookie-domain", c.CookieDomain, "The optional domain parameter for the cookie") f.StringVar(&c.SuccessURL, "success-url", c.SuccessURL, "The url to redirect after login") - f.BoolVar(&c.AllowRedirects, "allow-redirects", c.AllowRedirects, "Allow dynamic redirects by parameter") - f.StringVar(&c.RedirectQueryParameter, "redirect-query-parameter", c.RedirectQueryParameter, "Allow dynamic redirects by parameter") - f.BoolVar(&c.CheckRefererOnRedirects, "check-referer-on-redirects", c.CheckRefererOnRedirects, "When redirecting check that the referer is the same domain") - f.StringVar(&c.WhitelistDomainsFile, "whitelist-domains-file", c.WhitelistDomainsFile, "A file containing a list of domains that redirects are allowed to, one domain per line") + f.BoolVar(&c.Redirect, "redirect", c.Redirect, "Allow dynamic overwriting of the the success by query parameter") + 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.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/config_test.go b/login/config_test.go index 93ace72..b7f8a35 100644 --- a/login/config_test.go +++ b/login/config_test.go @@ -29,10 +29,10 @@ func TestConfig_ReadConfig(t *testing.T) { "--jwt-secret=jwtsecret", "--jwt-expiry=42h42m", "--success-url=successurl", - "--allow-redirects=false", + "--redirect=false", "--redirect-query-parameter=comingFrom", - "--check-referer-on-redirects=false", - "--whitelist-domains-file=File", + "--redirect-check-referer=false", + "--redirect-host-file=File", "--logout-url=logouturl", "--template=template", "--login-path=loginpath", @@ -47,24 +47,24 @@ func TestConfig_ReadConfig(t *testing.T) { } expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - AllowRedirects: false, - RedirectQueryParameter: "comingFrom", - CheckRefererOnRedirects: false, - WhitelistDomainsFile: "File", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + Redirect: false, + RedirectQueryParameter: "comingFrom", + RedirectCheckReferer: false, + RedirectHostFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{}, "foo": map[string]string{}, @@ -91,10 +91,10 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_JWT_SECRET", "jwtsecret")) NoError(t, os.Setenv("LOGINSRV_JWT_EXPIRY", "42h42m")) NoError(t, os.Setenv("LOGINSRV_SUCCESS_URL", "successurl")) - NoError(t, os.Setenv("LOGINSRV_ALLOW_REDIRECTS", "false")) + NoError(t, os.Setenv("LOGINSRV_REDIRECT", "false")) NoError(t, os.Setenv("LOGINSRV_REDIRECT_QUERY_PARAMETER", "comingFrom")) - NoError(t, os.Setenv("LOGINSRV_CHECK_REFERER_ON_REDIRECTS", "false")) - NoError(t, os.Setenv("LOGINSRV_WHITELIST_DOMAINS_FILE", "File")) + NoError(t, os.Setenv("LOGINSRV_REDIRECT_CHECK_REFERER", "false")) + NoError(t, os.Setenv("LOGINSRV_REDIRECT_HOST_FILE", "File")) NoError(t, os.Setenv("LOGINSRV_LOGOUT_URL", "logouturl")) NoError(t, os.Setenv("LOGINSRV_TEMPLATE", "template")) NoError(t, os.Setenv("LOGINSRV_LOGIN_PATH", "loginpath")) @@ -107,24 +107,24 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_GRACE_PERIOD", "4s")) expected := &Config{ - Host: "host", - Port: "port", - LogLevel: "loglevel", - TextLogging: true, - JwtSecret: "jwtsecret", - JwtExpiry: 42*time.Hour + 42*time.Minute, - SuccessURL: "successurl", - AllowRedirects: false, - RedirectQueryParameter: "comingFrom", - CheckRefererOnRedirects: false, - WhitelistDomainsFile: "File", - LogoutURL: "logouturl", - Template: "template", - LoginPath: "loginpath", - CookieName: "cookiename", - CookieExpiry: 23 * time.Minute, - CookieDomain: "*.example.com", - CookieHTTPOnly: false, + Host: "host", + Port: "port", + LogLevel: "loglevel", + TextLogging: true, + JwtSecret: "jwtsecret", + JwtExpiry: 42*time.Hour + 42*time.Minute, + SuccessURL: "successurl", + Redirect: false, + RedirectQueryParameter: "comingFrom", + RedirectCheckReferer: false, + RedirectHostFile: "File", + LogoutURL: "logouturl", + Template: "template", + LoginPath: "loginpath", + CookieName: "cookiename", + CookieExpiry: 23 * time.Minute, + CookieDomain: "*.example.com", + CookieHTTPOnly: false, Backends: Options{ "simple": map[string]string{ "foo": "bar", diff --git a/login/redirect.go b/login/redirect.go index 650c972..b6907cf 100644 --- a/login/redirect.go +++ b/login/redirect.go @@ -35,10 +35,10 @@ func (h *Handler) deleteRedirectCookie(w http.ResponseWriter, r *http.Request) { } func (h *Handler) allowRedirect(r *http.Request) bool { - if !h.config.AllowRedirects { + if !h.config.Redirect { return false } - if !h.config.CheckRefererOnRedirects { + if !h.config.RedirectCheckReferer { return true } @@ -56,7 +56,7 @@ func (h *Handler) allowRedirect(r *http.Request) bool { func (h *Handler) redirectURL(r *http.Request, w http.ResponseWriter) string { targetURL, foundTarget := h.getRedirectTarget(r) - if foundTarget && h.config.AllowRedirects { + if foundTarget && h.config.Redirect { sameHost := targetURL.Host == "" || r.Host == targetURL.Host if sameHost && targetURL.Path != "" { return targetURL.Path @@ -93,9 +93,14 @@ func (h *Handler) getRedirectTarget(r *http.Request) (*url.URL, bool) { } func (h *Handler) isRedirectDomainWhitelisted(r *http.Request, host string) bool { - f, err := os.Open(h.config.WhitelistDomainsFile) + if h.config.RedirectHostFile == "" { + logging.Application(r.Header).Warnf("redirect attempt to '%s', but no whitelist domain file given", host) + return false + } + + f, err := os.Open(h.config.RedirectHostFile) if err != nil { - logging.Application(r.Header).Warnf("can't open redirect whitelist domains file '%s'", h.config.WhitelistDomainsFile) + logging.Application(r.Header).Warnf("can't open redirect whitelist domains file '%s'", h.config.RedirectHostFile) return false } defer f.Close() @@ -106,6 +111,6 @@ func (h *Handler) isRedirectDomainWhitelisted(r *http.Request, host string) bool return true } } - logging.Application(r.Header).Warnf("domain '%s' not in redirect whitelist", host) + logging.Application(r.Header).Warnf("redirect attempt to '%s', but not in redirect whitelist", host) return false } diff --git a/login/redirect_test.go b/login/redirect_test.go index fd25740..4f3328e 100644 --- a/login/redirect_test.go +++ b/login/redirect_test.go @@ -28,9 +28,9 @@ func TestRedirect(t *testing.T) { } func TestRedirect_NotAllowed(t *testing.T) { - // redirect to SuccessURL if AllowRedirects is false + // redirect to SuccessURL if Redirect is false cfg := DefaultConfig() - cfg.AllowRedirects = false + cfg.Redirect = false h := &Handler{ backends: []Backend{ NewSimpleBackend(map[string]string{"bob": "secret"}), @@ -55,9 +55,9 @@ func TestRedirect_NonMatchingReferrer(t *testing.T) { setCookieList = readSetCookies(recorder.Header()) Equal(t, 0, len(setCookieList)) - // set redirect cookie with mismatch referer if CheckRefererOnRedirects is false + // set redirect cookie with mismatch referer if RedirectCheckReferer is false cfg := DefaultConfig() - cfg.CheckRefererOnRedirects = false + cfg.RedirectCheckReferer = false h := &Handler{ backends: []Backend{ NewSimpleBackend(map[string]string{"bob": "secret"}), @@ -93,7 +93,7 @@ func TestRedirect_Whitelisting(t *testing.T) { // redirect to success url if domains whitelist file doesn't exist cfg := DefaultConfig() - cfg.WhitelistDomainsFile = whitelistFile.Name() + cfg.RedirectHostFile = whitelistFile.Name() h := &Handler{ backends: []Backend{ NewSimpleBackend(map[string]string{"bob": "secret"}), From b23087714995ef3493c693d184c19806b88507b2 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 9 Jan 2018 23:55:55 +0100 Subject: [PATCH 10/12] redirect documentation and caddy demo --- README.md | 37 +++++++++++++++++++++++------------ caddy/demo/Caddyfile | 1 + caddy/demo/redirect_hosts.txt | 1 + caddy/demo/webroot/index.html | 4 +++- 4 files changed, 30 insertions(+), 13 deletions(-) create mode 100644 caddy/demo/redirect_hosts.txt diff --git a/README.md b/README.md index d1d1605..2e7452e 100644 --- a/README.md +++ b/README.md @@ -60,13 +60,13 @@ _Note for Caddy users_: Not all parameters are available in Caddy. See the table | -logout-url | string | | X | The url or path to redirect after logout | | -osiam | value | | X | OSIAM login backend opts: endpoint=..,client_id=..,client_secret=.. | | -port | string | "6789" | - | The port to listen on | +| -redirect | boolean | true | X | Allow dynamic overwriting of the the success by query parameter (default true) | +| -redirect-query-parameter | string | "backTo" | X | URL parameter for the redirect target (default "backTo") | +| -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 | | -simple | value | | X | Simple login backend opts: user1=password,user2=password,.. | | -success-url | string | "/" | X | The url to redirect after login | -| -allow-redirects | boolean | true | X | Allow dynamic redirects | -| -redirect-query-parameter | string | "backTo" | X | The query parameter to find the dynamic redirect in | | -prevent-external-redirects | boolean | true | X | Prevent dynamic redirects to external domains | -| -check-referer-on-redirects | boolean | true | X | Check the referer header to ensure it matches the host header on dynamic redirects | -| -whitelist-domains-file | string | "" | X | File containing whitelist of domains for dynamic redirects, one domain per line | | -template | string | | X | An alternative template for the login form | | -text-logging | boolean | true | - | Log in text format instead of json | | -jwt-refreshes | int | 0 | X | The maximum amount of jwt refreshes. | @@ -110,14 +110,15 @@ Performs the login and returns the JWT. Depending on the content-type and parame #### Runtime Parameters -| Parameter-Type | Parameter | Description | | -| ------------------|--------------------------------------------------|-----------------------------------------------------------|----------| -| Http-Header | Accept: text/html | Set the JWT-Token as Cookie 'jwt_token'. | default | -| Http-Header | Accept: application/jwt | Returns the JWT-Token within the body. No Cookie is set. | | -| Http-Header | Content-Type: application/x-www-form-urlencoded | Expect the credentials as form encoded parameters. | default | -| Http-Header | Content-Type: application/json | Take the credentials from the provided json object. | | -| Post-Parameter | username | The username | | -| Post-Parameter | password | The password | | +| Parameter-Type | Parameter | Description | | +| ------------------|--------------------------------------------------|-------------------------------------------------------------------|--------------| +| Http-Header | Accept: text/html | Set the JWT-Token as Cookie 'jwt_token'. | default | +| Http-Header | Accept: application/jwt | Returns the JWT-Token within the body. No Cookie is set. | | +| Http-Header | Content-Type: application/x-www-form-urlencoded | Expect the credentials as form encoded parameters. | default | +| Http-Header | Content-Type: application/json | Take the credentials from the provided json object. | | +| Post-Parameter | username | The username | | +| Post-Parameter | password | The password | | +| Get or Post | backTo | Dynamic redirect target after login (see (Redirects)[#redirects]) | -success-url | #### Possible Return Codes @@ -177,6 +178,17 @@ Location: / Set-Cookie: jwt_token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJib2IifQ.-51G5JQmpJleARHp8rIljBczPFanWT93d_N_7LQGUXU; HttpOnly ``` +### Redirects + +The api has support for a redirect query paramter, e.g. `?backTo=/dynamic/return/path`. For security reasons, the default behaviour is very restrictive: + +* Only local redirects (same host) are allowed. +* The `Referer` header is checked in the way, that the call to the login page has to come from the same page. + +These restrictions are there, to prevent you from unchecked redirect attacks, e.g. using your site for fishing or doing login attacks. +If you know, what you are doing, you can disable the referer check with `--redirect-check-referer=false` and provide a whitelist file +for allowed external domains with `--redirect-host-file=/some/domains.txt`. + ## The JWT Token Depending on the provider, the token may look as follows: ``` @@ -309,3 +321,4 @@ When you specify a custom template, only the layout of the original template is ``` + diff --git a/caddy/demo/Caddyfile b/caddy/demo/Caddyfile index 18abd50..f835532 100644 --- a/caddy/demo/Caddyfile +++ b/caddy/demo/Caddyfile @@ -15,6 +15,7 @@ http://localhost:8080 { login { success_url /private htpasswd file=passwords + redirect_host_file ../redirect_hosts.txt } } diff --git a/caddy/demo/redirect_hosts.txt b/caddy/demo/redirect_hosts.txt new file mode 100644 index 0000000..9e42a1c --- /dev/null +++ b/caddy/demo/redirect_hosts.txt @@ -0,0 +1 @@ +www.example.org diff --git a/caddy/demo/webroot/index.html b/caddy/demo/webroot/index.html index 174ae33..cab2bc9 100644 --- a/caddy/demo/webroot/index.html +++ b/caddy/demo/webroot/index.html @@ -26,7 +26,9 @@

Caddy Login Demo Application

- Please login with demo/demo. +
Please login as demo/demo.
+ +
Or login with redirect as demo/demo.
From c39a242d4aa22d3d90b7bac7716f8f084ea29fb8 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 9 Jan 2018 23:59:35 +0100 Subject: [PATCH 11/12] fixed caddy readme --- caddy/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/caddy/README.md b/caddy/README.md index ef1c9c4..e83b764 100644 --- a/caddy/README.md +++ b/caddy/README.md @@ -68,6 +68,7 @@ jwt { login { simple bob=secret,alice=secret - check_referer_on_redirects false + redirect_check_referer false + redirect_host_file ../redirect_hosts.txt } ``` From b68609443c28e900a8e6f99a17f4088603deefe2 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Fri, 16 Feb 2018 09:26:54 +0100 Subject: [PATCH 12/12] changed lookup path for RedirectHostFile to current working directory --- caddy/demo/Caddyfile | 2 +- caddy/setup.go | 4 ---- caddy/setup_test.go | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/caddy/demo/Caddyfile b/caddy/demo/Caddyfile index f835532..069e746 100644 --- a/caddy/demo/Caddyfile +++ b/caddy/demo/Caddyfile @@ -15,7 +15,7 @@ http://localhost:8080 { login { success_url /private htpasswd file=passwords - redirect_host_file ../redirect_hosts.txt + redirect_host_file redirect_hosts.txt } } diff --git a/caddy/setup.go b/caddy/setup.go index 8a83206..a692ccc 100644 --- a/caddy/setup.go +++ b/caddy/setup.go @@ -43,10 +43,6 @@ func setup(c *caddy.Controller) error { config.Template = filepath.Join(httpserver.GetConfig(c).Root, config.Template) } - if config.RedirectHostFile != "" && !filepath.IsAbs(config.RedirectHostFile) { - config.RedirectHostFile = filepath.Join(httpserver.GetConfig(c).Root, config.RedirectHostFile) - } - if len(args) == 1 { logging.Logger.Warnf("DEPRECATED: Please set the login path by parameter login_path and not as directive argument (%v:%v)", c.File(), c.Line()) config.LoginPath = path.Join(args[0], "/login") diff --git a/caddy/setup_test.go b/caddy/setup_test.go index 7dc0987..71dadd5 100644 --- a/caddy/setup_test.go +++ b/caddy/setup_test.go @@ -216,5 +216,5 @@ func TestSetup_RelativeFiles(t *testing.T) { middleware := mids[len(mids)-1](nil).(*CaddyHandler) Equal(t, root+"/myTemplate.tpl", middleware.config.Template) - Equal(t, root+"/redirectDomains.txt", middleware.config.RedirectHostFile) + Equal(t, "redirectDomains.txt", middleware.config.RedirectHostFile) }