loginsrv

Unnamed repository; edit this file 'description' to name the repository.
git clone git@jamesshield.xyz:repos/loginsrv.git
Log | Files | Refs | README | LICENSE

commit 7a8566d795ccd0337dbd35078817f06ae5edccb1
parent 15423135b3c55ecbe4ca31d4247e4b022e313ecc
Author: Sebastian Mancke <sebastian.mancke@snabble.io>
Date:   Thu, 27 Dec 2018 22:13:55 +0100

Merge pull request #91 from fvosberg/feature/secure-cookie-option

[FEATURE] add config for cookie secure flag
Diffstat:
Mcaddy/handler_test.go | 9+++++----
Mcaddy/setup_test.go | 156++++++++++++++++++++-----------------------------------------------------------
Mlogin/config.go | 3+++
Mlogin/config_test.go | 4++++
Mlogin/handler.go | 41++++++++++++++++++++++-------------------
Mlogin/handler_test.go | 42++++++++++++++++++++++++++++++++++++++++--
6 files changed, 113 insertions(+), 142 deletions(-)

diff --git a/caddy/handler_test.go b/caddy/handler_test.go @@ -1,14 +1,15 @@ package caddy import ( - "github.com/dgrijalva/jwt-go" - "github.com/mholt/caddy/caddyhttp/httpserver" - "github.com/tarent/loginsrv/login" - "github.com/tarent/loginsrv/model" "net/http" "net/http/httptest" "testing" "time" + + "github.com/dgrijalva/jwt-go" + "github.com/mholt/caddy/caddyhttp/httpserver" + "github.com/tarent/loginsrv/login" + "github.com/tarent/loginsrv/model" ) //Tests a page while being logged in as a user (doesn't test that the {user} replacer changes) diff --git a/caddy/setup_test.go b/caddy/setup_test.go @@ -19,34 +19,21 @@ func TestSetup(t *testing.T) { os.Setenv("JWT_SECRET", "jwtsecret") for j, test := range []struct { - input string - shouldErr bool - config login.Config + input string + shouldErr bool + config login.Config + configCheck func(*testing.T, *login.Config) }{ - { //defaults + { input: `login { simple bob=secret }`, shouldErr: false, - config: login.Config{ - JwtSecret: "jwtsecret", - JwtAlgo: "HS512", - 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", - }, - }, - Oauth: login.Options{}, - GracePeriod: 5 * time.Second, - }}, + configCheck: func(t *testing.T, cfg *login.Config) { + expectedBackendCfg := login.Options{"simple": map[string]string{"bob": "secret"}} + Equal(t, expectedBackendCfg, cfg.Backends, "config simple auth backend") + }, + }, { input: `login { success_url successurl @@ -65,21 +52,20 @@ func TestSetup(t *testing.T) { osiam endpoint=http://localhost:8080,client_id=example-client,client_secret=secret }`, shouldErr: false, - config: login.Config{ - JwtSecret: "jwtsecret", - JwtAlgo: "algo", - 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{ + configCheck: func(t *testing.T, cfg *login.Config) { + Equal(t, cfg.SuccessURL, "successurl") + Equal(t, cfg.JwtExpiry, 42*time.Hour) + Equal(t, cfg.JwtAlgo, "algo") + Equal(t, cfg.LoginPath, "/foo/bar") + Equal(t, cfg.Redirect, true) + Equal(t, cfg.RedirectQueryParameter, "comingFrom") + Equal(t, cfg.RedirectCheckReferer, true) + Equal(t, cfg.RedirectHostFile, "domainWhitelist.txt") + Equal(t, cfg.CookieName, "cookiename") + Equal(t, cfg.CookieHTTPOnly, false) + Equal(t, cfg.CookieDomain, "example.com") + Equal(t, cfg.CookieExpiry, 23*time.Hour+23*time.Minute) + expectedBackendCfg := login.Options{ "simple": map[string]string{ "bob": "secret", }, @@ -88,92 +74,28 @@ func TestSetup(t *testing.T) { "client_id": "example-client", "client_secret": "secret", }, - }, - Oauth: login.Options{}, - GracePeriod: 5 * time.Second, - }}, - { // backwards compatibility - // * login path as argument - // * '-' in parameter names - // * backend config by 'backend provider=' + } + Equal(t, expectedBackendCfg, cfg.Backends, "config simple auth backend") + }, + }, + { input: `loginsrv /context { backend provider=simple,bob=secret cookie-name cookiename }`, shouldErr: false, - config: login.Config{ - JwtSecret: "jwtsecret", - JwtAlgo: "HS512", - 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", - }, - }, - Oauth: login.Options{}, - GracePeriod: 5 * time.Second, - }}, - { // backwards compatibility - // * login path as argument - // * '-' in parameter names - // * backend config by 'backend provider=' - input: `loginsrv / { - backend provider=simple,bob=secret - cookie-name cookiename - }`, - shouldErr: false, - config: login.Config{ - JwtSecret: "jwtsecret", - JwtAlgo: "HS512", - JwtExpiry: 24 * time.Hour, - SuccessURL: "/", - Redirect: true, - RedirectQueryParameter: "backTo", - RedirectCheckReferer: true, - LoginPath: "/login", - CookieName: "cookiename", - CookieHTTPOnly: true, - Backends: login.Options{ + configCheck: func(t *testing.T, cfg *login.Config) { + Equal(t, "/context/login", cfg.LoginPath, "Login path should be set by argument for backwards compatibility") + Equal(t, "cookiename", cfg.CookieName, "The cookie name should be set by a config name with - instead of _ for backwards compatibility") + expectedBackendCfg := login.Options{ "simple": map[string]string{ "bob": "secret", }, - }, - Oauth: login.Options{}, - GracePeriod: 5 * time.Second, - }}, - + } + Equal(t, expectedBackendCfg, cfg.Backends, "The backend config should be set by \"backend provider=\" for backwards compatibility") + }, + }, // error cases - { // duration parse error - input: `login { - simple bob=secret - }`, - shouldErr: false, - config: login.Config{ - JwtSecret: "jwtsecret", - JwtAlgo: "HS512", - 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", - }, - }, - Oauth: login.Options{}, - GracePeriod: 5 * time.Second, - }}, {input: "login {\n}", shouldErr: true}, {input: "login xx yy {\n}", shouldErr: true}, {input: "login {\n cookie_http_only 42d \n simple bob=secret \n}", shouldErr: true}, @@ -196,7 +118,7 @@ func TestSetup(t *testing.T) { return } middleware := mids[len(mids)-1](nil).(*CaddyHandler) - Equal(t, &test.config, middleware.config) + test.configCheck(t, middleware.config) }) } } @@ -222,6 +144,6 @@ func TestSetup_RelativeFiles(t *testing.T) { } middleware := mids[len(mids)-1](nil).(*CaddyHandler) - Equal(t, filepath.FromSlash(root + "/myTemplate.tpl"), middleware.config.Template) + Equal(t, filepath.FromSlash(root+"/myTemplate.tpl"), middleware.config.Template) Equal(t, "redirectDomains.txt", middleware.config.RedirectHostFile) } diff --git a/login/config.go b/login/config.go @@ -39,6 +39,7 @@ func DefaultConfig() *Config { LoginPath: "/login", CookieName: "jwt_token", CookieHTTPOnly: true, + CookieSecure: true, Backends: Options{}, Oauth: Options{}, GracePeriod: 5 * time.Second, @@ -70,6 +71,7 @@ type Config struct { CookieExpiry time.Duration CookieDomain string CookieHTTPOnly bool + CookieSecure bool Backends Options Oauth Options GracePeriod time.Duration @@ -114,6 +116,7 @@ func (c *Config) ConfigureFlagSet(f *flag.FlagSet) { f.IntVar(&c.JwtRefreshes, "jwt-refreshes", c.JwtRefreshes, "The maximum amount of jwt refreshes. 0 by Default") f.StringVar(&c.CookieName, "cookie-name", c.CookieName, "The name of the jwt cookie") f.BoolVar(&c.CookieHTTPOnly, "cookie-http-only", c.CookieHTTPOnly, "Set the cookie with the http only flag") + f.BoolVar(&c.CookieSecure, "cookie-secure", c.CookieSecure, "Set the cookie with the secure flag") 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") diff --git a/login/config_test.go b/login/config_test.go @@ -41,6 +41,7 @@ func TestConfig_ReadConfig(t *testing.T) { "--cookie-expiry=23m", "--cookie-domain=*.example.com", "--cookie-http-only=false", + "--cookie-secure=false", "--backend=provider=simple", "--backend=provider=foo", "--github=client_id=foo,client_secret=bar", @@ -68,6 +69,7 @@ func TestConfig_ReadConfig(t *testing.T) { CookieExpiry: 23 * time.Minute, CookieDomain: "*.example.com", CookieHTTPOnly: false, + CookieSecure: false, Backends: Options{ "simple": map[string]string{}, "foo": map[string]string{}, @@ -107,6 +109,7 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { NoError(t, os.Setenv("LOGINSRV_COOKIE_EXPIRY", "23m")) NoError(t, os.Setenv("LOGINSRV_COOKIE_DOMAIN", "*.example.com")) NoError(t, os.Setenv("LOGINSRV_COOKIE_HTTP_ONLY", "false")) + NoError(t, os.Setenv("LOGINSRV_COOKIE_SECURE", "false")) NoError(t, os.Setenv("LOGINSRV_SIMPLE", "foo=bar")) NoError(t, os.Setenv("LOGINSRV_GITHUB", "client_id=foo,client_secret=bar")) NoError(t, os.Setenv("LOGINSRV_GRACE_PERIOD", "4s")) @@ -132,6 +135,7 @@ func TestConfig_ReadConfigFromEnv(t *testing.T) { CookieExpiry: 23 * time.Minute, CookieDomain: "*.example.com", CookieHTTPOnly: false, + CookieSecure: false, Backends: Options{ "simple": map[string]string{ "foo": "bar", diff --git a/login/handler.go b/login/handler.go @@ -247,30 +247,33 @@ func (h *Handler) respondAuthenticated(w http.ResponseWriter, r *http.Request, u } if wantHTML(r) { - cookie := &http.Cookie{ - Name: h.config.CookieName, - Value: token, - HttpOnly: h.config.CookieHTTPOnly, - Path: "/", - } - if h.config.CookieExpiry != 0 { - cookie.Expires = time.Now().Add(h.config.CookieExpiry) - } - if h.config.CookieDomain != "" { - cookie.Domain = h.config.CookieDomain - } - - http.SetCookie(w, cookie) - - w.Header().Set("Location", h.redirectURL(r, w)) - h.deleteRedirectCookie(w, r) - w.WriteHeader(303) + h.respondAuthenticatedHTML(w, r, token) return } w.Header().Set("Content-Type", contentTypeJWT) w.WriteHeader(200) - fmt.Fprintf(w, "%s", token) + fmt.Fprint(w, token) +} + +func (h *Handler) respondAuthenticatedHTML(w http.ResponseWriter, r *http.Request, token string) { + cookie := &http.Cookie{ + Name: h.config.CookieName, + Value: token, + HttpOnly: h.config.CookieHTTPOnly, + Path: "/", + } + if h.config.CookieExpiry != 0 { + cookie.Expires = time.Now().Add(h.config.CookieExpiry) + } + if h.config.CookieDomain != "" { + cookie.Domain = h.config.CookieDomain + } + cookie.Secure = h.config.CookieSecure + http.SetCookie(w, cookie) + w.Header().Set("Location", h.redirectURL(r, w)) + h.deleteRedirectCookie(w, r) + w.WriteHeader(303) } func (h *Handler) createToken(userInfo model.UserInfo) (string, error) { diff --git a/login/handler_test.go b/login/handler_test.go @@ -243,6 +243,46 @@ func TestHandler_LoginWeb(t *testing.T) { Equal(t, recorder.Header().Get("Set-Cookie"), "") } +func TestHandler_SetSecureCookie(t *testing.T) { + tests := []struct { + name string + secure bool + }{ + {"secure", true}, + {"unsecure", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, err := http.NewRequest("OPTION", "/foobar", nil) + if err != nil { + t.Fatalf("Unable to create request: %v", err) + } + w := httptest.NewRecorder() + cfg := testConfig() + h := testHandler() + cfg.CookieSecure = tt.secure + h.config = cfg + + h.respondAuthenticatedHTML(w, r, "RANDOM_TOKEN_VALUE") + + cc := w.Result().Cookies() + foundCookie := false + for _, c := range cc { + if c.Name != cfg.CookieName { + continue + } + foundCookie = true + if c.Secure != tt.secure { + t.Errorf("Found token cookie %q with secure flag %t; expected %t", cfg.CookieName, c.Secure, tt.secure) + } + } + if !foundCookie { + t.Errorf("No token cookie with the name %q (Config.CookieName) found", cfg.CookieName) + } + }) + } +} + func TestHandler_Refresh(t *testing.T) { h := testHandler() input := model.UserInfo{Sub: "bob", Expiry: time.Now().Add(time.Second).Unix()} @@ -484,10 +524,8 @@ func TestHandler_getToken_InvalidSecret(t *testing.T) { r := &http.Request{ Header: http.Header{"Cookie": {h.config.CookieName + "=" + token + ";"}}, } - // modify secret h.config.JwtSecret = "foobar" - _, valid := h.GetToken(r) False(t, valid) }