diff --git a/pkg/api/login.go b/pkg/api/login.go index 7812c2ac4eb..df7621e61a8 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -46,8 +46,12 @@ func (hs *HTTPServer) validateRedirectTo(redirectTo string) error { } func (hs *HTTPServer) cookieOptionsFromCfg() middleware.CookieOptions { + path := "/" + if len(hs.Cfg.AppSubUrl) > 0 { + path = hs.Cfg.AppSubUrl + } return middleware.CookieOptions{ - Path: hs.Cfg.AppSubUrl + "/", + Path: path, Secure: hs.Cfg.CookieSecure, SameSiteDisabled: hs.Cfg.CookieSameSiteDisabled, SameSiteMode: hs.Cfg.CookieSameSiteMode, diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index 4ff624ac21a..a0c713ac46f 100644 --- a/pkg/api/login_test.go +++ b/pkg/api/login_test.go @@ -10,8 +10,6 @@ import ( "strings" "testing" - "github.com/grafana/grafana/pkg/services/licensing" - "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" @@ -19,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/auth" + "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/stretchr/testify/assert" @@ -109,12 +108,16 @@ func TestLoginErrorCookieApiEndpoint(t *testing.T) { oauthError := errors.New("User not a member of one of the required organizations") encryptedError, _ := util.Encrypt([]byte(oauthError.Error()), setting.SecretKey) + expCookiePath := "/" + if len(setting.AppSubUrl) > 0 { + expCookiePath = setting.AppSubUrl + } cookie := http.Cookie{ Name: LoginErrorCookieName, MaxAge: 60, Value: hex.EncodeToString(encryptedError), HttpOnly: true, - Path: setting.AppSubUrl + "/", + Path: expCookiePath, Secure: hs.Cfg.CookieSecure, SameSite: hs.Cfg.CookieSameSiteMode, } @@ -210,12 +213,16 @@ func TestLoginViewRedirect(t *testing.T) { hs.Cfg.AppUrl = c.appURL hs.Cfg.AppSubUrl = c.appSubURL t.Run(c.desc, func(t *testing.T) { + expCookiePath := "/" + if len(hs.Cfg.AppSubUrl) > 0 { + expCookiePath = hs.Cfg.AppSubUrl + } cookie := http.Cookie{ Name: "redirect_to", MaxAge: 60, Value: c.url, HttpOnly: true, - Path: hs.Cfg.AppSubUrl + "/", + Path: expCookiePath, Secure: hs.Cfg.CookieSecure, SameSite: hs.Cfg.CookieSameSiteMode, } @@ -238,7 +245,7 @@ func TestLoginViewRedirect(t *testing.T) { expCookieValue = "" expCookieMaxAge = 0 } - expCookie := fmt.Sprintf("redirect_to=%v; Path=%v; Max-Age=%v; HttpOnly; Secure", expCookieValue, hs.Cfg.AppSubUrl+"/", expCookieMaxAge) + expCookie := fmt.Sprintf("redirect_to=%v; Path=%v; Max-Age=%v; HttpOnly; Secure", expCookieValue, expCookiePath, expCookieMaxAge) for _, cookieValue := range setCookie { if cookieValue == expCookie { redirectToCookieFound = true @@ -332,12 +339,16 @@ func TestLoginPostRedirect(t *testing.T) { hs.Cfg.AppUrl = c.appURL hs.Cfg.AppSubUrl = c.appSubURL t.Run(c.desc, func(t *testing.T) { + expCookiePath := "/" + if len(hs.Cfg.AppSubUrl) > 0 { + expCookiePath = hs.Cfg.AppSubUrl + } cookie := http.Cookie{ Name: "redirect_to", MaxAge: 60, Value: c.url, HttpOnly: true, - Path: hs.Cfg.AppSubUrl + "/", + Path: expCookiePath, Secure: hs.Cfg.CookieSecure, SameSite: hs.Cfg.CookieSameSiteMode, } @@ -358,7 +369,7 @@ func TestLoginPostRedirect(t *testing.T) { assert.True(t, ok, "Set-Cookie exists") assert.Greater(t, len(setCookie), 0) var redirectToCookieFound bool - expCookieValue := fmt.Sprintf("redirect_to=; Path=%v; Max-Age=0; HttpOnly; Secure", hs.Cfg.AppSubUrl+"/") + expCookieValue := fmt.Sprintf("redirect_to=; Path=%v; Max-Age=0; HttpOnly; Secure", expCookiePath) for _, cookieValue := range setCookie { if cookieValue == expCookieValue { redirectToCookieFound = true diff --git a/pkg/middleware/cookie.go b/pkg/middleware/cookie.go index 895b98f4e8f..c81cb0dfa6c 100644 --- a/pkg/middleware/cookie.go +++ b/pkg/middleware/cookie.go @@ -14,8 +14,12 @@ type CookieOptions struct { } func newCookieOptions() CookieOptions { + path := "/" + if len(setting.AppSubUrl) > 0 { + path = setting.AppSubUrl + } return CookieOptions{ - Path: setting.AppSubUrl + "/", + Path: path, Secure: setting.CookieSecure, SameSiteDisabled: setting.CookieSameSiteDisabled, SameSiteMode: setting.CookieSameSiteMode, diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 0536442beb1..5bbc60a9cbc 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -264,10 +264,14 @@ func TestMiddlewareContext(t *testing.T) { } for _, sameSitePolicy := range sameSitePolicies { setting.CookieSameSiteMode = sameSitePolicy + expectedCookiePath := "/" + if len(setting.AppSubUrl) > 0 { + expectedCookiePath = setting.AppSubUrl + } expectedCookie := &http.Cookie{ Name: setting.LoginCookieName, Value: "rotated", - Path: setting.AppSubUrl + "/", + Path: expectedCookiePath, HttpOnly: true, MaxAge: int(maxAge), Secure: setting.CookieSecure, @@ -291,10 +295,14 @@ func TestMiddlewareContext(t *testing.T) { Convey("Should not set cookie with SameSite attribute when setting.CookieSameSiteDisabled is true", func() { setting.CookieSameSiteDisabled = true setting.CookieSameSiteMode = http.SameSiteLaxMode + expectedCookiePath := "/" + if len(setting.AppSubUrl) > 0 { + expectedCookiePath = setting.AppSubUrl + } expectedCookie := &http.Cookie{ Name: setting.LoginCookieName, Value: "rotated", - Path: setting.AppSubUrl + "/", + Path: expectedCookiePath, HttpOnly: true, MaxAge: int(maxAge), Secure: setting.CookieSecure,