From f258adadbf60ed0a61083a89c8c84a83c3db3a59 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Fri, 3 Mar 2023 14:17:09 +0100 Subject: [PATCH] AuthN: add utility functions for different type of login responses (#64133) * AuthN: add utility functions to handle response and redirect after successful login * API: Reuse utility functions for logins if authnService flag is enabled --- pkg/api/login.go | 16 +------ pkg/api/login_oauth.go | 10 +--- pkg/services/authn/authn.go | 56 ++++++++++++++++++++++ pkg/services/authn/authnimpl/service.go | 2 +- pkg/services/authn/clients/session.go | 36 ++++++-------- pkg/services/authn/clients/session_test.go | 19 ++++++-- 6 files changed, 87 insertions(+), 52 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index 92580302b5f..efa715efa38 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -208,22 +208,8 @@ func (hs *HTTPServer) LoginPost(c *contextmodel.ReqContext) response.Response { return response.Err(err) } - cookies.WriteSessionCookie(c, hs.Cfg, identity.SessionToken.UnhashedToken, hs.Cfg.LoginMaxLifetime) - result := map[string]interface{}{ - "message": "Logged in", - } - - if redirectTo := c.GetCookie("redirect_to"); len(redirectTo) > 0 { - if err := hs.ValidateRedirectTo(redirectTo); err == nil { - result["redirectUrl"] = redirectTo - } else { - c.Logger.Info("Ignored invalid redirect_to cookie value.", "url", redirectTo) - } - cookies.DeleteCookie(c.Resp, "redirect_to", hs.CookieOptionsFromCfg) - } - metrics.MApiLoginPost.Inc() - return response.JSON(http.StatusOK, result) + return authn.HandleLoginResponse(c.Req, c.Resp, hs.Cfg, identity, hs.ValidateRedirectTo) } cmd := dtos.LoginCommand{} diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 9572f2f0f59..d302f63ea19 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -114,15 +114,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *contextmodel.ReqContext) { } metrics.MApiLoginOAuth.Inc() - cookies.WriteSessionCookie(ctx, hs.Cfg, identity.SessionToken.UnhashedToken, hs.Cfg.LoginMaxLifetime) - - redirectURL := setting.AppSubUrl + "/" - if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && hs.ValidateRedirectTo(redirectTo) == nil { - redirectURL = redirectTo - cookies.DeleteCookie(ctx.Resp, "redirect_to", hs.CookieOptionsFromCfg) - } - - ctx.Redirect(redirectURL) + authn.HandleLoginRedirect(ctx.Req, ctx.Resp, hs.Cfg, identity, hs.ValidateRedirectTo) return } diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index 9c91eb9a34f..0d092d2f583 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -4,16 +4,20 @@ import ( "context" "fmt" "net/http" + "net/url" "strconv" "strings" "time" "golang.org/x/oauth2" + "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) @@ -323,3 +327,55 @@ func IdentityFromSignedInUser(id string, usr *user.SignedInUser, params ClientPa func ClientWithPrefix(name string) string { return fmt.Sprintf("auth.client.%s", name) } + +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) *response.NormalResponse { + result := map[string]interface{}{"message": "Logged in"} + if redirectURL := handleLogin(r, w, cfg, identity, validator); redirectURL != cfg.AppSubURL+"/" { + result["redirectUrl"] = redirectURL + } + 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) { + redirectURL := handleLogin(r, w, cfg, identity, validator) + 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) *response.RedirectResponse { + return response.Redirect(handleLogin(r, w, cfg, identity, validator)) +} + +func handleLogin(r *http.Request, w http.ResponseWriter, cfg *setting.Cfg, identity *Identity, validator RedirectValidator) string { + redirectURL := cfg.AppSubURL + "/" + if redirectTo := getRedirectURL(r); len(redirectTo) > 0 && validator(redirectTo) == nil { + cookies.DeleteCookie(w, "redirect_to", nil) + redirectURL = redirectTo + } + + WriteSessionCookie(w, cfg, identity) + return redirectURL +} + +func getRedirectURL(r *http.Request) string { + cookie, err := r.Cookie("redirect_to") + if err != nil { + return "" + } + + v, _ := url.QueryUnescape(cookie.Value) + return v +} + +func WriteSessionCookie(w http.ResponseWriter, cfg *setting.Cfg, identity *Identity) { + maxAge := int(cfg.LoginMaxLifetime.Seconds()) + if cfg.LoginMaxLifetime <= 0 { + maxAge = -1 + } + + cookies.WriteCookie(w, cfg.LoginCookieName, url.QueryEscape(identity.SessionToken.UnhashedToken), maxAge, nil) +} diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 1da25ae0320..74282f665db 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -79,7 +79,7 @@ func ProvideService( s.RegisterClient(clients.ProvideAPIKey(apikeyService, userService)) if cfg.LoginCookieName != "" { - s.RegisterClient(clients.ProvideSession(sessionService, userService, cfg.LoginCookieName, cfg.LoginMaxLifetime)) + s.RegisterClient(clients.ProvideSession(sessionService, userService, cfg)) } if s.cfg.AnonymousEnabled { diff --git a/pkg/services/authn/clients/session.go b/pkg/services/authn/clients/session.go index 39cc1fc74ac..5a2ece98854 100644 --- a/pkg/services/authn/clients/session.go +++ b/pkg/services/authn/clients/session.go @@ -4,37 +4,33 @@ import ( "context" "errors" "net/url" - "time" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/network" - "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) var _ authn.HookClient = new(Session) var _ authn.ContextAwareClient = new(Session) -func ProvideSession(sessionService auth.UserTokenService, userService user.Service, - cookieName string, maxLifetime time.Duration) *Session { +func ProvideSession(sessionService auth.UserTokenService, userService user.Service, cfg *setting.Cfg) *Session { return &Session{ - loginCookieName: cookieName, - loginMaxLifetime: maxLifetime, - sessionService: sessionService, - userService: userService, - log: log.New(authn.ClientSession), + cfg: cfg, + sessionService: sessionService, + userService: userService, + log: log.New(authn.ClientSession), } } type Session struct { - loginCookieName string - loginMaxLifetime time.Duration // jguer: should be returned by session Service on rotate - sessionService auth.UserTokenService - userService user.Service - log log.Logger + cfg *setting.Cfg + sessionService auth.UserTokenService + userService user.Service + log log.Logger } func (s *Session) Name() string { @@ -42,7 +38,7 @@ func (s *Session) Name() string { } func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) { - unescapedCookie, err := r.HTTPRequest.Cookie(s.loginCookieName) + unescapedCookie, err := r.HTTPRequest.Cookie(s.cfg.LoginCookieName) if err != nil { return nil, err } @@ -73,11 +69,11 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id } func (s *Session) Test(ctx context.Context, r *authn.Request) bool { - if s.loginCookieName == "" { + if s.cfg.LoginCookieName == "" { return false } - if _, err := r.HTTPRequest.Cookie(s.loginCookieName); err != nil { + if _, err := r.HTTPRequest.Cookie(s.cfg.LoginCookieName); err != nil { return false } @@ -118,11 +114,7 @@ func (s *Session) Hook(ctx context.Context, identity *authn.Identity, r *authn.R identity.SessionToken = newToken s.log.Debug("rotated session token", "user", identity.ID) - maxAge := int(s.loginMaxLifetime.Seconds()) - if s.loginMaxLifetime <= 0 { - maxAge = -1 - } - cookies.WriteCookie(r.Resp, s.loginCookieName, url.QueryEscape(identity.SessionToken.UnhashedToken), maxAge, nil) + authn.WriteSessionCookie(w, s.cfg, identity) } }) diff --git a/pkg/services/authn/clients/session_test.go b/pkg/services/authn/clients/session_test.go index b8b363987d4..bd640ff9837 100644 --- a/pkg/services/authn/clients/session_test.go +++ b/pkg/services/authn/clients/session_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -27,13 +28,15 @@ func TestSession_Test(t *testing.T) { Header: map[string][]string{}, } validHTTPReq.AddCookie(&http.Cookie{Name: cookieName, Value: "bob-the-high-entropy-token"}) - - s := ProvideSession(&authtest.FakeUserAuthTokenService{}, &usertest.FakeUserService{}, "", 20*time.Second) + cfg := setting.NewCfg() + cfg.LoginCookieName = "" + cfg.LoginMaxLifetime = 20 * time.Second + s := ProvideSession(&authtest.FakeUserAuthTokenService{}, &usertest.FakeUserService{}, cfg) disabled := s.Test(context.Background(), &authn.Request{HTTPRequest: validHTTPReq}) assert.False(t, disabled) - s.loginCookieName = cookieName + s.cfg.LoginCookieName = cookieName good := s.Test(context.Background(), &authn.Request{HTTPRequest: validHTTPReq}) assert.True(t, good) @@ -111,7 +114,10 @@ func TestSession_Authenticate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := ProvideSession(tt.fields.sessionService, tt.fields.userService, cookieName, 20*time.Second) + cfg := setting.NewCfg() + cfg.LoginCookieName = cookieName + cfg.LoginMaxLifetime = 20 * time.Second + s := ProvideSession(tt.fields.sessionService, tt.fields.userService, cfg) got, err := s.Authenticate(context.Background(), tt.args.r) require.True(t, (err != nil) == tt.wantErr, err) @@ -142,12 +148,15 @@ func (f *fakeResponseWriter) WriteHeader(statusCode int) { } func TestSession_Hook(t *testing.T) { + cfg := setting.NewCfg() + cfg.LoginCookieName = "grafana-session" + cfg.LoginMaxLifetime = 20 * time.Second s := ProvideSession(&authtest.FakeUserAuthTokenService{ TryRotateTokenProvider: func(ctx context.Context, token *auth.UserToken, clientIP net.IP, userAgent string) (bool, *auth.UserToken, error) { token.UnhashedToken = "new-token" return true, token, nil }, - }, &usertest.FakeUserService{}, "grafana-session", 20*time.Second) + }, &usertest.FakeUserService{}, cfg) sampleID := &authn.Identity{ SessionToken: &auth.UserToken{