diff --git a/pkg/api/login.go b/pkg/api/login.go index e227b9b4940..5667f0741ed 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -35,7 +35,9 @@ func (hs *HTTPServer) validateRedirectTo(redirectTo string) error { if to.IsAbs() { return login.ErrAbsoluteRedirectTo } - if hs.Cfg.AppSubUrl != "" && !strings.HasPrefix(to.Path, "/"+hs.Cfg.AppSubUrl) { + // when using a subUrl, the redirect_to should have a relative or absolute path that includes the subUrl, otherwise the redirect + // will send the user to the wrong location + if hs.Cfg.AppSubUrl != "" && !strings.HasPrefix(to.Path, hs.Cfg.AppSubUrl) && !strings.HasPrefix(to.Path, "/"+hs.Cfg.AppSubUrl) { return login.ErrInvalidRedirectTo } return nil @@ -177,6 +179,11 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext, cmd dtos.LoginCommand) Res if redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")); len(redirectTo) > 0 { if err := hs.validateRedirectTo(redirectTo); err == nil { + // remove subpath if it exists at the beginning of the redirect_to + // LoginCtrl.tsx is already prepending the redirectUrl with the subpath + if setting.AppSubUrl != "" && strings.Index(redirectTo, setting.AppSubUrl) == 0 { + redirectTo = strings.Replace(redirectTo, setting.AppSubUrl, "", 1) + } result["redirectUrl"] = redirectTo } else { log.Info("Ignored invalid redirect_to cookie value: %v", redirectTo) diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index 09565935400..76f2b53d32d 100644 --- a/pkg/api/login_test.go +++ b/pkg/api/login_test.go @@ -72,6 +72,7 @@ type redirectCase struct { err error appURL string appSubURL string + path string } func TestLoginErrorCookieApiEndpoint(t *testing.T) { @@ -154,6 +155,7 @@ func TestLoginViewRedirect(t *testing.T) { desc: "grafana relative url without subpath", url: "/profile", appURL: "http://localhost:3000", + path: "/", status: 302, }, { @@ -161,6 +163,15 @@ func TestLoginViewRedirect(t *testing.T) { url: "/grafana/profile", appURL: "http://localhost:3000", appSubURL: "grafana", + path: "grafana/", + status: 302, + }, + { + desc: "grafana slashed relative url with subpath", + url: "/grafana/profile", + appURL: "http://localhost:3000", + appSubURL: "grafana", + path: "/grafana/", status: 302, }, { @@ -168,13 +179,23 @@ func TestLoginViewRedirect(t *testing.T) { url: "/profile", appURL: "http://localhost:3000", appSubURL: "grafana", + path: "grafana/", status: 200, err: login.ErrInvalidRedirectTo, }, + { + desc: "grafana subpath absolute url", + url: "http://localhost:3000/grafana/profile", + appURL: "http://localhost:3000", + appSubURL: "grafana", + path: "/grafana/profile", + status: 200, + }, { desc: "grafana absolute url", url: "http://localhost:3000/profile", appURL: "http://localhost:3000", + path: "/", status: 200, err: login.ErrAbsoluteRedirectTo, }, @@ -182,6 +203,7 @@ func TestLoginViewRedirect(t *testing.T) { desc: "non grafana absolute url", url: "http://example.com", appURL: "http://localhost:3000", + path: "/", status: 200, err: login.ErrAbsoluteRedirectTo, }, @@ -189,6 +211,7 @@ func TestLoginViewRedirect(t *testing.T) { desc: "invalid url", url: ":foo", appURL: "http://localhost:3000", + path: "/", status: 200, err: login.ErrInvalidRedirectTo, }, @@ -203,7 +226,7 @@ func TestLoginViewRedirect(t *testing.T) { MaxAge: 60, Value: c.url, HttpOnly: true, - Path: hs.Cfg.AppSubUrl + "/", + Path: c.path, Secure: hs.Cfg.CookieSecure, SameSite: hs.Cfg.CookieSameSiteMode, } @@ -219,7 +242,7 @@ func TestLoginViewRedirect(t *testing.T) { assert.True(t, ok, "Set-Cookie exists") assert.Greater(t, len(setCookie), 0) var redirectToCookieFound bool - expCookieValue := fmt.Sprintf("redirect_to=%v; Path=%v; Max-Age=60; HttpOnly; Secure", c.url, hs.Cfg.AppSubUrl+"/") + expCookieValue := fmt.Sprintf("redirect_to=%v; Path=%v; Max-Age=60; HttpOnly; Secure", c.url, c.path) for _, cookieValue := range setCookie { if cookieValue == expCookieValue { redirectToCookieFound = true @@ -281,6 +304,12 @@ func TestLoginPostRedirect(t *testing.T) { appURL: "https://localhost:3000", appSubURL: "grafana", }, + { + desc: "grafana no slash relative url with subpath", + url: "grafana/profile", + appURL: "https://localhost:3000", + appSubURL: "grafana", + }, { desc: "relative url with missing subpath", url: "/profile", diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index aef8830f983..27e2c2e0cf2 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -47,7 +47,7 @@ func notAuthorized(c *m.ReqContext) { return } - WriteCookie(c.Resp, "redirect_to", url.QueryEscape(setting.AppSubUrl+c.Req.RequestURI), 0, newCookieOptions) + WriteCookie(c.Resp, "redirect_to", url.QueryEscape(c.Req.RequestURI), 0, newCookieOptions) c.Redirect(setting.AppSubUrl + "/login") }