From 0539ccf10df408850514eea202abefd764046067 Mon Sep 17 00:00:00 2001 From: Misi Date: Mon, 7 Oct 2024 14:59:00 +0200 Subject: [PATCH] Auth: Fix redirection when auto_login is enabled (#94311) * Fix for SAML auto login * Fix for OAuth auto login --- pkg/api/login.go | 22 +++++++++++++++++++ pkg/api/login_oauth.go | 5 +++++ pkg/middleware/auth.go | 9 ++++---- pkg/middleware/auth_test.go | 2 +- pkg/services/authn/authn.go | 44 ++++++++++++++++++++++++------------- 5 files changed, 62 insertions(+), 20 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index de3bacdf7dc..0a66f7000b5 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/network" + "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" @@ -181,6 +182,9 @@ func (hs *HTTPServer) tryAutoLogin(c *contextmodel.ReqContext) bool { for providerName, provider := range oauthInfos { if provider.AutoLogin || hs.Cfg.OAuthAutoLogin { redirectUrl := hs.Cfg.AppSubURL + "/login/" + providerName + if hs.Features.IsEnabledGlobally(featuremgmt.FlagUseSessionStorageForRedirection) { + redirectUrl += hs.getRedirectToForAutoLogin(c) + } c.Logger.Info("OAuth auto login enabled. Redirecting to " + redirectUrl) c.Redirect(redirectUrl, 307) return true @@ -189,6 +193,9 @@ func (hs *HTTPServer) tryAutoLogin(c *contextmodel.ReqContext) bool { if samlAutoLogin { redirectUrl := hs.Cfg.AppSubURL + "/login/saml" + if hs.Features.IsEnabledGlobally(featuremgmt.FlagUseSessionStorageForRedirection) { + redirectUrl += hs.getRedirectToForAutoLogin(c) + } c.Logger.Info("SAML auto login enabled. Redirecting to " + redirectUrl) c.Redirect(redirectUrl, 307) return true @@ -197,6 +204,21 @@ func (hs *HTTPServer) tryAutoLogin(c *contextmodel.ReqContext) bool { return false } +func (hs *HTTPServer) getRedirectToForAutoLogin(c *contextmodel.ReqContext) string { + redirectTo := c.Req.FormValue("redirectTo") + if hs.Cfg.AppSubURL != "" && strings.HasPrefix(redirectTo, hs.Cfg.AppSubURL) { + redirectTo = strings.TrimPrefix(redirectTo, hs.Cfg.AppSubURL) + } + + if redirectTo == "/" { + return "" + } + + // remove any forceLogin=true params + redirectTo = middleware.RemoveForceLoginParams(redirectTo) + return "?redirectTo=" + url.QueryEscape(redirectTo) +} + func (hs *HTTPServer) LoginAPIPing(c *contextmodel.ReqContext) response.Response { if c.IsSignedIn || c.IsAnonymous { return response.JSON(http.StatusOK, util.DynMap{"message": "Logged in"}) diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index f59b09045a3..51a0857f96a 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/services/authn" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/web" ) @@ -25,6 +26,7 @@ func (hs *HTTPServer) OAuthLogin(reqCtx *contextmodel.ReqContext) { } code := reqCtx.Query("code") + redirectTo := reqCtx.Query("redirectTo") req := &authn.Request{HTTPRequest: reqCtx.Req} if code == "" { @@ -36,6 +38,9 @@ func (hs *HTTPServer) OAuthLogin(reqCtx *contextmodel.ReqContext) { cookies.WriteCookie(reqCtx.Resp, OauthStateCookieName, redirect.Extra[authn.KeyOAuthState], hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) + if hs.Features.IsEnabledGlobally(featuremgmt.FlagUseSessionStorageForRedirection) { + cookies.WriteCookie(reqCtx.Resp, "redirectTo", redirectTo, hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) + } if pkce := redirect.Extra[authn.KeyOAuthPKCE]; pkce != "" { cookies.WriteCookie(reqCtx.Resp, OauthPKCECookieName, pkce, hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) } diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index 338ceb77f25..c85342dd0ce 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -98,7 +98,7 @@ func writeRedirectCookie(c *contextmodel.ReqContext) { } // remove any forceLogin=true params - redirectTo = removeForceLoginParams(redirectTo) + redirectTo = RemoveForceLoginParams(redirectTo) cookies.WriteCookie(c.Resp, "redirect_to", url.QueryEscape(redirectTo), 0, nil) } @@ -113,13 +113,13 @@ func getRedirectToQueryParam(c *contextmodel.ReqContext) string { } // remove any forceLogin=true params - redirectTo = removeForceLoginParams(redirectTo) + redirectTo = RemoveForceLoginParams(redirectTo) return "?redirectTo=" + url.QueryEscape(redirectTo) } var forceLoginParamsRegexp = regexp.MustCompile(`&?forceLogin=true`) -func removeForceLoginParams(str string) string { +func RemoveForceLoginParams(str string) string { return forceLoginParamsRegexp.ReplaceAllString(str, "") } @@ -138,7 +138,8 @@ func CanAdminPlugins(cfg *setting.Cfg, accessControl ac.AccessControl) func(c *c } func RoleAppPluginAuth(accessControl ac.AccessControl, ps pluginstore.Store, features featuremgmt.FeatureToggles, - logger log.Logger) func(c *contextmodel.ReqContext) { + logger log.Logger, +) func(c *contextmodel.ReqContext) { return func(c *contextmodel.ReqContext) { pluginID := web.Params(c.Req)[":id"] p, exists := ps.Plugin(c.Req.Context(), pluginID) diff --git a/pkg/middleware/auth_test.go b/pkg/middleware/auth_test.go index 3530c90f211..6db5d11cff2 100644 --- a/pkg/middleware/auth_test.go +++ b/pkg/middleware/auth_test.go @@ -352,7 +352,7 @@ func TestRemoveForceLoginparams(t *testing.T) { } for i, tc := range tcs { t.Run(fmt.Sprintf("testcase %d", i), func(t *testing.T) { - require.Equal(t, tc.exp, removeForceLoginParams(tc.inp)) + require.Equal(t, tc.exp, RemoveForceLoginParams(tc.inp)) }) } } diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index 3c43822e9e8..a024beaf2fb 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -32,9 +32,10 @@ const ( ) const ( - MetaKeyUsername = "username" - MetaKeyAuthModule = "authModule" - MetaKeyIsLogin = "isLogin" + MetaKeyUsername = "username" + MetaKeyAuthModule = "authModule" + MetaKeyIsLogin = "isLogin" + defaultRedirectToCookieKey = "redirect_to" ) // ClientParams are hints to the auth service about how to handle the identity management @@ -74,9 +75,11 @@ type FetchPermissionsParams struct { Roles []string } -type PostAuthHookFn func(ctx context.Context, identity *Identity, r *Request) error -type PostLoginHookFn func(ctx context.Context, identity *Identity, r *Request, err error) -type PreLogoutHookFn func(ctx context.Context, requester identity.Requester, sessionToken *usertoken.UserToken) error +type ( + PostAuthHookFn func(ctx context.Context, identity *Identity, r *Request) error + PostLoginHookFn func(ctx context.Context, identity *Identity, r *Request, err error) + PreLogoutHookFn func(ctx context.Context, requester identity.Requester, sessionToken *usertoken.UserToken) error +) type Authenticator interface { // Authenticate authenticates a request @@ -233,41 +236,52 @@ type RedirectValidator func(url string) error // HandleLoginResponse is a utility function to perform common operations after a successful login and returns response.NormalResponse func HandleLoginResponse(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator, features featuremgmt.FeatureToggles) *response.NormalResponse { result := map[string]any{"message": "Logged in"} - result["redirectUrl"] = handleLogin(r, w, cfg, identity, validator, features) + result["redirectUrl"] = handleLogin(r, w, cfg, identity, validator, features, "") return response.JSON(http.StatusOK, result) } // HandleLoginRedirect is a utility function to perform common operations after a successful login and redirects func HandleLoginRedirect(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator, features featuremgmt.FeatureToggles) { - redirectURL := handleLogin(r, w, cfg, identity, validator, features) + redirectURL := handleLogin(r, w, cfg, identity, validator, features, "redirectTo") http.Redirect(w, r, redirectURL, http.StatusFound) } // HandleLoginRedirectResponse is a utility function to perform common operations after a successful login and return a response.RedirectResponse -func HandleLoginRedirectResponse(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator, features featuremgmt.FeatureToggles) *response.RedirectResponse { - return response.Redirect(handleLogin(r, w, cfg, identity, validator, features)) +func HandleLoginRedirectResponse(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator, features featuremgmt.FeatureToggles, redirectToCookieName string) *response.RedirectResponse { + return response.Redirect(handleLogin(r, w, cfg, identity, validator, features, redirectToCookieName)) } -func handleLogin(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator, features featuremgmt.FeatureToggles) string { +func handleLogin(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator, features featuremgmt.FeatureToggles, redirectToCookieName string) string { WriteSessionCookie(w, cfg, identity.SessionToken) + redirectURL := cfg.AppSubURL + "/" if features.IsEnabledGlobally(featuremgmt.FlagUseSessionStorageForRedirection) { - return cfg.AppSubURL + "/" + if redirectToCookieName != "" { + scopedRedirectToCookie, err := r.Cookie(redirectToCookieName) + if err == nil { + redirectTo, _ := url.QueryUnescape(scopedRedirectToCookie.Value) + if redirectTo != "" && validator(redirectTo) == nil { + redirectURL = cfg.AppSubURL + redirectTo + } + cookies.DeleteCookie(w, redirectToCookieName, cookieOptions(cfg)) + } + } + return redirectURL } - redirectURL := cfg.AppSubURL + "/" + redirectURL = cfg.AppSubURL + "/" if redirectTo := getRedirectURL(r); len(redirectTo) > 0 { if validator(redirectTo) == nil { redirectURL = redirectTo } - cookies.DeleteCookie(w, "redirect_to", cookieOptions(cfg)) + cookies.DeleteCookie(w, defaultRedirectToCookieKey, cookieOptions(cfg)) } return redirectURL } func getRedirectURL(r *http.Request) string { - cookie, err := r.Cookie("redirect_to") + cookie, err := r.Cookie(defaultRedirectToCookieKey) if err != nil { return "" }