From e08f61059bcbc60753268eae06aced2cc31e7f9d Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 15 Jan 2019 11:11:32 +0100 Subject: [PATCH 01/49] utils --- pkg/util/encoding.go | 8 ++++++++ pkg/util/ip_address.go | 27 +++++++++++++++++++++++++++ pkg/util/ip_address_test.go | 14 ++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 pkg/util/ip_address.go create mode 100644 pkg/util/ip_address_test.go diff --git a/pkg/util/encoding.go b/pkg/util/encoding.go index 0edb721e422..e82344d73f9 100644 --- a/pkg/util/encoding.go +++ b/pkg/util/encoding.go @@ -101,3 +101,11 @@ func DecodeBasicAuthHeader(header string) (string, string, error) { return userAndPass[0], userAndPass[1], nil } + +func RandomHex(n int) (string, error) { + bytes := make([]byte, n) + if _, err := rand.Read(bytes); err != nil { + return "", err + } + return hex.EncodeToString(bytes), nil +} diff --git a/pkg/util/ip_address.go b/pkg/util/ip_address.go new file mode 100644 index 00000000000..4e9a9378c6b --- /dev/null +++ b/pkg/util/ip_address.go @@ -0,0 +1,27 @@ +package util + +import ( + "net" + "strings" +) + +// ParseIPAddress parses an IP address and removes port and/or IPV6 format +func ParseIPAddress(input string) string { + var s string + lastIndex := strings.LastIndex(input, ":") + + if lastIndex != -1 { + s = input[:lastIndex] + } + + s = strings.Replace(s, "[", "", -1) + s = strings.Replace(s, "]", "", -1) + + ip := net.ParseIP(s) + + if ip.IsLoopback() { + return "127.0.0.1" + } + + return ip.String() +} diff --git a/pkg/util/ip_address_test.go b/pkg/util/ip_address_test.go new file mode 100644 index 00000000000..644340a5e82 --- /dev/null +++ b/pkg/util/ip_address_test.go @@ -0,0 +1,14 @@ +package util + +import ( + "testing" + + . "github.com/smartystreets/goconvey/convey" +) + +func TestParseIPAddress(t *testing.T) { + Convey("Test parse ip address", t, func() { + So(ParseIPAddress("192.168.0.140:456"), ShouldEqual, "192.168.0.140") + So(ParseIPAddress("[::1:456]"), ShouldEqual, "127.0.0.1") + }) +} From b0df7280be60be815078e572f5975a14521695bf Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 15 Jan 2019 15:15:17 +0100 Subject: [PATCH 02/49] begin user auth token implementation --- pkg/services/auth/auth_token.go | 170 +++++++++++++++ pkg/services/auth/auth_token_test.go | 206 ++++++++++++++++++ pkg/services/auth/model.go | 25 +++ .../sqlstore/migrations/migrations.go | 1 + .../migrations/user_auth_token_mig.go | 32 +++ 5 files changed, 434 insertions(+) create mode 100644 pkg/services/auth/auth_token.go create mode 100644 pkg/services/auth/auth_token_test.go create mode 100644 pkg/services/auth/model.go create mode 100644 pkg/services/sqlstore/migrations/user_auth_token_mig.go diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go new file mode 100644 index 00000000000..aefcccadbd6 --- /dev/null +++ b/pkg/services/auth/auth_token.go @@ -0,0 +1,170 @@ +package auth + +import ( + "crypto/sha256" + "encoding/hex" + "time" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" + macaron "gopkg.in/macaron.v1" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/registry" + "github.com/grafana/grafana/pkg/services/sqlstore" +) + +func init() { + registry.RegisterService(&UserAuthTokenService{}) +} + +var now = time.Now + +// UserAuthTokenService are used for generating and validating user auth tokens +type UserAuthTokenService struct { + SQLStore *sqlstore.SqlStore `inject:""` + log log.Logger +} + +// Init this service +func (s *UserAuthTokenService) Init() error { + s.log = log.New("auth") + return nil +} + +const sessionCookieKey = "grafana_session" + +func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *models.ReqContext) error { + userToken, err := s.CreateToken(user.Id, c.RemoteAddr(), c.Req.UserAgent()) + if err != nil { + return err + } + + c.Resp.Header().Del("Set-Cookie") + c.SetCookie(sessionCookieKey, userToken.unhashedToken, setting.AppSubUrl+"/", setting.Domain, false, true) + + return nil +} + +func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { + c.SetCookie(sessionCookieKey, "", -1, setting.AppSubUrl+"/", setting.Domain, false, true) +} + +func (s *UserAuthTokenService) RequestMiddleware() macaron.Handler { + return func(ctx *models.ReqContext) { + authToken := ctx.GetCookie(sessionCookieKey) + userToken, err := s.lookupToken(authToken) + if err != nil { + + } + + ctx.Next() + + refreshed, err := s.refreshToken(userToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) + if err != nil { + + } + + if refreshed { + ctx.Resp.Header().Del("Set-Cookie") + ctx.SetCookie(sessionCookieKey, userToken.unhashedToken, setting.AppSubUrl+"/", setting.Domain, false, true) + } + } +} + +func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) { + clientIP = util.ParseIPAddress(clientIP) + token, err := util.RandomHex(16) + if err != nil { + return nil, err + } + + hashedToken := hashToken(token) + + userToken := userAuthToken{ + UserId: userId, + AuthToken: hashedToken, + PrevAuthToken: hashedToken, + ClientIp: clientIP, + UserAgent: userAgent, + RotatedAt: now().Unix(), + CreatedAt: now().Unix(), + UpdatedAt: now().Unix(), + SeenAt: 0, + AuthTokenSeen: false, + } + _, err = s.SQLStore.NewSession().Insert(&userToken) + if err != nil { + return nil, err + } + + userToken.unhashedToken = token + + return &userToken, nil +} + +func (s *UserAuthTokenService) lookupToken(unhashedToken string) (*userAuthToken, error) { + hashedToken := hashToken(unhashedToken) + + var userToken userAuthToken + exists, err := s.SQLStore.NewSession().Where("auth_token = ? OR prev_auth_token = ?", hashedToken, hashedToken).Get(&userToken) + if err != nil { + return nil, err + } + + if !exists { + return nil, ErrAuthTokenNotFound + } + + if userToken.AuthToken != hashedToken && userToken.PrevAuthToken == hashedToken && userToken.AuthTokenSeen { + userToken.AuthTokenSeen = false + expireBefore := now().Add(-1 * time.Minute).Unix() + affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND prev_auth_token = ? AND rotated_at < ?", userToken.Id, userToken.PrevAuthToken, expireBefore).AllCols().Update(&userToken) + if err != nil { + return nil, err + } + + if affectedRows == 0 { + s.log.Debug("prev seen token unchanged", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + } else { + s.log.Debug("prev seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + } + } + + if !userToken.AuthTokenSeen && userToken.AuthToken == hashedToken { + userTokenCopy := userToken + userTokenCopy.AuthTokenSeen = true + userTokenCopy.SeenAt = now().Unix() + affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND auth_token = ?", userTokenCopy.Id, userTokenCopy.AuthToken).AllCols().Update(&userTokenCopy) + if err != nil { + return nil, err + } + + if affectedRows == 1 { + userToken = userTokenCopy + } + + if affectedRows == 0 { + s.log.Debug("seen wrong token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + } else { + s.log.Debug("seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + } + } + + userToken.unhashedToken = unhashedToken + + return &userToken, nil +} + +func (s *UserAuthTokenService) refreshToken(token *userAuthToken, clientIP, userAgent string) (bool, error) { + // lookup token in db + // refresh token if needed + + return false, nil +} + +func hashToken(token string) string { + hashBytes := sha256.Sum256([]byte(token + setting.SecretKey)) + return hex.EncodeToString(hashBytes[:]) +} diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go new file mode 100644 index 00000000000..2e4618c10ed --- /dev/null +++ b/pkg/services/auth/auth_token_test.go @@ -0,0 +1,206 @@ +package auth + +import ( + "testing" + "time" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/services/sqlstore" + . "github.com/smartystreets/goconvey/convey" +) + +func TestUserAuthToken(t *testing.T) { + Convey("Test user auth token", t, func() { + ctx := createTestContext(t) + userAuthTokenService := ctx.tokenService + userID := int64(10) + + t := time.Date(2018, 12, 13, 13, 45, 0, 0, time.UTC) + now = func() time.Time { + return t + } + + Convey("When creating token", func() { + token, err := userAuthTokenService.CreateToken(userID, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + So(token.AuthTokenSeen, ShouldBeFalse) + + Convey("When lookup unhashed token should return user auth token", func() { + lookupToken, err := userAuthTokenService.lookupToken(token.unhashedToken) + So(err, ShouldBeNil) + So(lookupToken, ShouldNotBeNil) + So(lookupToken.UserId, ShouldEqual, userID) + So(lookupToken.AuthTokenSeen, ShouldBeTrue) + + storedAuthToken, err := ctx.getAuthTokenByID(lookupToken.Id) + So(err, ShouldBeNil) + So(storedAuthToken, ShouldNotBeNil) + So(storedAuthToken.AuthTokenSeen, ShouldBeTrue) + }) + + Convey("When lookup hashed token should return user auth token not found error", func() { + lookupToken, err := userAuthTokenService.lookupToken(token.AuthToken) + So(err, ShouldEqual, ErrAuthTokenNotFound) + So(lookupToken, ShouldBeNil) + }) + }) + + Convey("expires correctly", func() { + token, err := userAuthTokenService.CreateToken(userID, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + + _, err = userAuthTokenService.lookupToken(token.unhashedToken) + So(err, ShouldBeNil) + + token, err = ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + + // set now (now - 23 hours) + _, err = userAuthTokenService.refreshToken(token, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + + _, err = userAuthTokenService.lookupToken(token.unhashedToken) + So(err, ShouldBeNil) + + stillGood, err := userAuthTokenService.lookupToken(token.unhashedToken) + So(err, ShouldBeNil) + So(stillGood, ShouldNotBeNil) + + // set now (new - 2 hours) + notGood, err := userAuthTokenService.lookupToken(token.unhashedToken) + So(err, ShouldEqual, ErrAuthTokenNotFound) + So(notGood, ShouldBeNil) + }) + + Convey("can properly rotate tokens", func() { + token, err := userAuthTokenService.CreateToken(userID, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + + prevToken := token.AuthToken + unhashedPrev := token.unhashedToken + + refreshed, err := userAuthTokenService.refreshToken(token, "192.168.10.12:1234", "a new user agent") + So(err, ShouldBeNil) + So(refreshed, ShouldBeFalse) + + ctx.markAuthTokenAsSeen(token.Id) + token, err = ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + + // ability to auth using an old token + now = func() time.Time { + return t + } + + refreshed, err = userAuthTokenService.refreshToken(token, "192.168.10.12:1234", "a new user agent") + So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) + + unhashedToken := token.unhashedToken + + token, err = ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + token.unhashedToken = unhashedToken + + So(token.RotatedAt, ShouldEqual, t.Unix()) + So(token.ClientIp, ShouldEqual, "192.168.10.12") + So(token.UserAgent, ShouldEqual, "a new user agent") + So(token.AuthTokenSeen, ShouldBeFalse) + So(token.SeenAt, ShouldEqual, 0) + So(token.PrevAuthToken, ShouldEqual, prevToken) + + lookedUp, err := userAuthTokenService.lookupToken(token.unhashedToken) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + So(lookedUp.AuthTokenSeen, ShouldBeTrue) + So(lookedUp.SeenAt, ShouldEqual, t.Unix()) + + lookedUp, err = userAuthTokenService.lookupToken(unhashedPrev) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + So(lookedUp.Id, ShouldEqual, token.Id) + + now = func() time.Time { + return t.Add(2 * time.Minute) + } + + lookedUp, err = userAuthTokenService.lookupToken(unhashedPrev) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + + lookedUp, err = ctx.getAuthTokenByID(lookedUp.Id) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + So(lookedUp.AuthTokenSeen, ShouldBeFalse) + + refreshed, err = userAuthTokenService.refreshToken(token, "192.168.10.12:1234", "a new user agent") + So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) + + token, err = ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + So(token.SeenAt, ShouldEqual, 0) + }) + + Convey("keeps prev token valid for 1 minute after it is confirmed", func() { + + }) + + Convey("will not mark token unseen when prev and current are the same", func() { + + }) + + Reset(func() { + now = time.Now + }) + }) +} + +func createTestContext(t *testing.T) *testContext { + t.Helper() + + sqlstore := sqlstore.InitTestDB(t) + tokenService := &UserAuthTokenService{ + SQLStore: sqlstore, + log: log.New("test-logger"), + } + + return &testContext{ + sqlstore: sqlstore, + tokenService: tokenService, + } +} + +type testContext struct { + sqlstore *sqlstore.SqlStore + tokenService *UserAuthTokenService +} + +func (c *testContext) getAuthTokenByID(id int64) (*userAuthToken, error) { + sess := c.sqlstore.NewSession() + var t userAuthToken + found, err := sess.ID(id).Get(&t) + if err != nil || !found { + return nil, err + } + + return &t, nil +} + +func (c *testContext) markAuthTokenAsSeen(id int64) (bool, error) { + sess := c.sqlstore.NewSession() + res, err := sess.Exec("UPDATE user_auth_token SET auth_token_seen = ? WHERE id = ?", c.sqlstore.Dialect.BooleanStr(true), id) + if err != nil { + return false, err + } + + rowsAffected, err := res.RowsAffected() + if err != nil { + return false, err + } + return rowsAffected == 1, nil +} diff --git a/pkg/services/auth/model.go b/pkg/services/auth/model.go new file mode 100644 index 00000000000..a033b96be31 --- /dev/null +++ b/pkg/services/auth/model.go @@ -0,0 +1,25 @@ +package auth + +import ( + "errors" +) + +// Typed errors +var ( + ErrAuthTokenNotFound = errors.New("User auth token not found") +) + +type userAuthToken struct { + Id int64 + UserId int64 + AuthToken string + PrevAuthToken string + UserAgent string + ClientIp string + AuthTokenSeen bool + SeenAt int64 + RotatedAt int64 + CreatedAt int64 + UpdatedAt int64 + unhashedToken string `xorm:"-"` +} diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 36cd8e5ed62..931259ec3ed 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -32,6 +32,7 @@ func AddMigrations(mg *Migrator) { addLoginAttemptMigrations(mg) addUserAuthMigrations(mg) addServerlockMigrations(mg) + addUserAuthTokenMigrations(mg) } func addMigrationLogMigrations(mg *Migrator) { diff --git a/pkg/services/sqlstore/migrations/user_auth_token_mig.go b/pkg/services/sqlstore/migrations/user_auth_token_mig.go new file mode 100644 index 00000000000..9794b7a78c7 --- /dev/null +++ b/pkg/services/sqlstore/migrations/user_auth_token_mig.go @@ -0,0 +1,32 @@ +package migrations + +import ( + . "github.com/grafana/grafana/pkg/services/sqlstore/migrator" +) + +func addUserAuthTokenMigrations(mg *Migrator) { + userAuthTokenV1 := Table{ + Name: "user_auth_token", + Columns: []*Column{ + {Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true}, + {Name: "user_id", Type: DB_BigInt, Nullable: false}, + {Name: "auth_token", Type: DB_NVarchar, Length: 100, Nullable: false}, + {Name: "prev_auth_token", Type: DB_NVarchar, Length: 100, Nullable: false}, + {Name: "user_agent", Type: DB_NVarchar, Length: 255, Nullable: false}, + {Name: "client_ip", Type: DB_NVarchar, Length: 255, Nullable: false}, + {Name: "auth_token_seen", Type: DB_Bool, Nullable: false}, + {Name: "seen_at", Type: DB_Int, Nullable: true}, + {Name: "rotated_at", Type: DB_Int, Nullable: false}, + {Name: "created_at", Type: DB_Int, Nullable: false}, + {Name: "updated_at", Type: DB_Int, Nullable: false}, + }, + Indices: []*Index{ + {Cols: []string{"auth_token"}, Type: UniqueIndex}, + {Cols: []string{"prev_auth_token"}, Type: UniqueIndex}, + }, + } + + mg.AddMigration("create user auth token table", NewAddTableMigration(userAuthTokenV1)) + mg.AddMigration("add unique index user_auth_token.auth_token", NewAddIndexMigration(userAuthTokenV1, userAuthTokenV1.Indices[0])) + mg.AddMigration("add unique index user_auth_token.prev_auth_token", NewAddIndexMigration(userAuthTokenV1, userAuthTokenV1.Indices[1])) +} From 8764fb5aa6eee9c8df08e5fc23dec5d78d3f5682 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 15 Jan 2019 15:15:52 +0100 Subject: [PATCH 03/49] inject login/logout hooks --- pkg/api/api.go | 12 ++++++------ pkg/api/http_server.go | 17 ++++++++++------- pkg/api/login.go | 38 +++++++++++++++----------------------- pkg/api/login_oauth.go | 4 ++-- pkg/api/org_invite.go | 4 ++-- pkg/api/signup.go | 4 ++-- 6 files changed, 37 insertions(+), 42 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 0526ee80afe..07cb712f794 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -23,9 +23,9 @@ func (hs *HTTPServer) registerRoutes() { // not logged in views r.Get("/", reqSignedIn, hs.Index) - r.Get("/logout", Logout) - r.Post("/login", quota("session"), bind(dtos.LoginCommand{}), Wrap(LoginPost)) - r.Get("/login/:name", quota("session"), OAuthLogin) + r.Get("/logout", hs.Logout) + r.Post("/login", quota("session"), bind(dtos.LoginCommand{}), Wrap(hs.LoginPost)) + r.Get("/login/:name", quota("session"), hs.OAuthLogin) r.Get("/login", hs.LoginView) r.Get("/invite/:code", hs.Index) @@ -84,11 +84,11 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/signup", hs.Index) r.Get("/api/user/signup/options", Wrap(GetSignUpOptions)) r.Post("/api/user/signup", quota("user"), bind(dtos.SignUpForm{}), Wrap(SignUp)) - r.Post("/api/user/signup/step2", bind(dtos.SignUpStep2Form{}), Wrap(SignUpStep2)) + r.Post("/api/user/signup/step2", bind(dtos.SignUpStep2Form{}), Wrap(hs.SignUpStep2)) // invited r.Get("/api/user/invite/:code", Wrap(GetInviteInfoByCode)) - r.Post("/api/user/invite/complete", bind(dtos.CompleteInviteForm{}), Wrap(CompleteInvite)) + r.Post("/api/user/invite/complete", bind(dtos.CompleteInviteForm{}), Wrap(hs.CompleteInvite)) // reset password r.Get("/user/password/send-reset-email", hs.Index) @@ -109,7 +109,7 @@ func (hs *HTTPServer) registerRoutes() { r.Delete("/api/snapshots/:key", reqEditorRole, Wrap(DeleteDashboardSnapshot)) // api renew session based on remember cookie - r.Get("/api/login/ping", quota("session"), LoginAPIPing) + r.Get("/api/login/ping", quota("session"), hs.LoginAPIPing) // authed api r.Group("/api", func(apiRoute routing.RouteRegister) { diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index d4d7b41bec5..600157878fe 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -11,6 +11,8 @@ import ( "path" "time" + "github.com/grafana/grafana/pkg/services/auth" + "github.com/grafana/grafana/pkg/api/routing" "github.com/prometheus/client_golang/prometheus" @@ -49,13 +51,14 @@ type HTTPServer struct { streamManager *live.StreamManager httpSrv *http.Server - RouteRegister routing.RouteRegister `inject:""` - Bus bus.Bus `inject:""` - RenderService rendering.Service `inject:""` - Cfg *setting.Cfg `inject:""` - HooksService *hooks.HooksService `inject:""` - CacheService *cache.CacheService `inject:""` - DatasourceCache datasources.CacheService `inject:""` + RouteRegister routing.RouteRegister `inject:""` + Bus bus.Bus `inject:""` + RenderService rendering.Service `inject:""` + Cfg *setting.Cfg `inject:""` + HooksService *hooks.HooksService `inject:""` + CacheService *cache.CacheService `inject:""` + DatasourceCache datasources.CacheService `inject:""` + AuthTokenService *auth.UserAuthTokenService `inject:""` } func (hs *HTTPServer) Init() error { diff --git a/pkg/api/login.go b/pkg/api/login.go index 05afc40e59a..f0902a60f58 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/metrics" m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" ) @@ -43,7 +42,7 @@ func (hs *HTTPServer) LoginView(c *m.ReqContext) { return } - if !tryLoginUsingRememberCookie(c) { + if !hs.tryLoginUsingRememberCookie(c) { c.HTML(200, ViewIndex, viewData) return } @@ -75,7 +74,7 @@ func tryOAuthAutoLogin(c *m.ReqContext) bool { return false } -func tryLoginUsingRememberCookie(c *m.ReqContext) bool { +func (hs *HTTPServer) tryLoginUsingRememberCookie(c *m.ReqContext) bool { // Check auto-login. uname := c.GetCookie(setting.CookieUserName) if len(uname) == 0 { @@ -111,12 +110,12 @@ func tryLoginUsingRememberCookie(c *m.ReqContext) bool { } isSucceed = true - loginUserWithUser(user, c) + hs.loginUserWithUser(user, c) return true } -func LoginAPIPing(c *m.ReqContext) { - if !tryLoginUsingRememberCookie(c) { +func (hs *HTTPServer) LoginAPIPing(c *m.ReqContext) { + if !hs.tryLoginUsingRememberCookie(c) { c.JsonApiErr(401, "Unauthorized", nil) return } @@ -124,7 +123,7 @@ func LoginAPIPing(c *m.ReqContext) { c.JsonOK("Logged in") } -func LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response { +func (hs *HTTPServer) LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response { if setting.DisableLoginForm { return Error(401, "Login is disabled", nil) } @@ -146,7 +145,7 @@ func LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response { user := authQuery.User - loginUserWithUser(user, c) + hs.loginUserWithUser(user, c) result := map[string]interface{}{ "message": "Logged in", @@ -162,27 +161,20 @@ func LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response { return JSON(200, result) } -func loginUserWithUser(user *m.User, c *m.ReqContext) { +func (hs *HTTPServer) loginUserWithUser(user *m.User, c *m.ReqContext) { if user == nil { - log.Error(3, "User login with nil user") + hs.log.Error("User login with nil user") } - c.Resp.Header().Del("Set-Cookie") - - days := 86400 * setting.LogInRememberDays - if days > 0 { - c.SetCookie(setting.CookieUserName, user.Login, days, setting.AppSubUrl+"/") - c.SetSuperSecureCookie(user.Rands+user.Password, setting.CookieRememberName, user.Login, days, setting.AppSubUrl+"/") + err := hs.AuthTokenService.UserAuthenticatedHook(user, c) + if err != nil { + hs.log.Error("User auth hook failed", err) } - - c.Session.RegenerateId(c.Context) - c.Session.Set(session.SESS_KEY_USERID, user.Id) } -func Logout(c *m.ReqContext) { - c.SetCookie(setting.CookieUserName, "", -1, setting.AppSubUrl+"/") - c.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubUrl+"/") - c.Session.Destory(c.Context) +func (hs *HTTPServer) Logout(c *m.ReqContext) { + hs.AuthTokenService.UserSignedOutHook(c) + if setting.SignoutRedirectUrl != "" { c.Redirect(setting.SignoutRedirectUrl) } else { diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index fe4fa93b621..6013df8ea02 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -31,7 +31,7 @@ func GenStateString() string { return base64.URLEncoding.EncodeToString(rnd) } -func OAuthLogin(ctx *m.ReqContext) { +func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { if setting.OAuthService == nil { ctx.Handle(404, "OAuth not enabled", nil) return @@ -178,7 +178,7 @@ func OAuthLogin(ctx *m.ReqContext) { } // login - loginUserWithUser(cmd.Result, ctx) + hs.loginUserWithUser(cmd.Result, ctx) metrics.M_Api_Login_OAuth.Inc() diff --git a/pkg/api/org_invite.go b/pkg/api/org_invite.go index dfb2cf045ed..835b03a2cc9 100644 --- a/pkg/api/org_invite.go +++ b/pkg/api/org_invite.go @@ -148,7 +148,7 @@ func GetInviteInfoByCode(c *m.ReqContext) Response { }) } -func CompleteInvite(c *m.ReqContext, completeInvite dtos.CompleteInviteForm) Response { +func (hs *HTTPServer) CompleteInvite(c *m.ReqContext, completeInvite dtos.CompleteInviteForm) Response { query := m.GetTempUserByCodeQuery{Code: completeInvite.InviteCode} if err := bus.Dispatch(&query); err != nil { @@ -186,7 +186,7 @@ func CompleteInvite(c *m.ReqContext, completeInvite dtos.CompleteInviteForm) Res return rsp } - loginUserWithUser(user, c) + hs.loginUserWithUser(user, c) metrics.M_Api_User_SignUpCompleted.Inc() metrics.M_Api_User_SignUpInvite.Inc() diff --git a/pkg/api/signup.go b/pkg/api/signup.go index 200a3ebc9d1..fe577dd9ef9 100644 --- a/pkg/api/signup.go +++ b/pkg/api/signup.go @@ -51,7 +51,7 @@ func SignUp(c *m.ReqContext, form dtos.SignUpForm) Response { return JSON(200, util.DynMap{"status": "SignUpCreated"}) } -func SignUpStep2(c *m.ReqContext, form dtos.SignUpStep2Form) Response { +func (hs *HTTPServer) SignUpStep2(c *m.ReqContext, form dtos.SignUpStep2Form) Response { if !setting.AllowUserSignUp { return Error(401, "User signup is disabled", nil) } @@ -109,7 +109,7 @@ func SignUpStep2(c *m.ReqContext, form dtos.SignUpStep2Form) Response { apiResponse["code"] = "redirect-to-select-org" } - loginUserWithUser(user, c) + hs.loginUserWithUser(user, c) metrics.M_Api_User_SignUpCompleted.Inc() return JSON(200, apiResponse) From aba6148c4322ca914dd564f5a1d64d5d9ce39804 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 16 Jan 2019 14:53:59 +0100 Subject: [PATCH 04/49] login users based on token cookie --- pkg/api/http_server.go | 16 +++---- pkg/api/login.go | 8 ++-- pkg/middleware/middleware.go | 33 +++++++++++++-- pkg/middleware/org_redirect.go | 1 - pkg/services/auth/auth_token.go | 62 +++++++++++++++++----------- pkg/services/auth/auth_token_test.go | 36 ++++++++-------- 6 files changed, 95 insertions(+), 61 deletions(-) diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 600157878fe..54f0601e577 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -11,16 +11,8 @@ import ( "path" "time" - "github.com/grafana/grafana/pkg/services/auth" - - "github.com/grafana/grafana/pkg/api/routing" - "github.com/prometheus/client_golang/prometheus" - - "github.com/prometheus/client_golang/prometheus/promhttp" - - macaron "gopkg.in/macaron.v1" - "github.com/grafana/grafana/pkg/api/live" + "github.com/grafana/grafana/pkg/api/routing" httpstatic "github.com/grafana/grafana/pkg/api/static" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" @@ -29,11 +21,15 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/registry" + "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/cache" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/hooks" "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/setting" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + macaron "gopkg.in/macaron.v1" ) func init() { @@ -226,7 +222,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { m.Use(hs.healthHandler) m.Use(hs.metricsEndpoint) - m.Use(middleware.GetContextHandler()) + m.Use(middleware.GetContextHandler(hs.AuthTokenService)) m.Use(middleware.Sessioner(&setting.SessionOptions, setting.SessionConnMaxLifetime)) m.Use(middleware.OrgRedirect()) diff --git a/pkg/api/login.go b/pkg/api/login.go index f0902a60f58..37b12d03299 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -42,10 +42,10 @@ func (hs *HTTPServer) LoginView(c *m.ReqContext) { return } - if !hs.tryLoginUsingRememberCookie(c) { - c.HTML(200, ViewIndex, viewData) - return - } + //if !hs.tryLoginUsingRememberCookie(c) { + c.HTML(200, ViewIndex, viewData) + return + //} if redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")); len(redirectTo) > 0 { c.SetCookie("redirect_to", "", -1, setting.AppSubUrl+"/") diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index ace72d998eb..28f08869425 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -3,15 +3,15 @@ package middleware import ( "strconv" - "gopkg.in/macaron.v1" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/apikeygen" "github.com/grafana/grafana/pkg/log" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" + "gopkg.in/macaron.v1" ) var ( @@ -21,7 +21,7 @@ var ( ReqOrgAdmin = RoleAuth(m.ROLE_ADMIN) ) -func GetContextHandler() macaron.Handler { +func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { return func(c *macaron.Context) { ctx := &m.ReqContext{ Context: c, @@ -49,7 +49,8 @@ func GetContextHandler() macaron.Handler { case initContextWithApiKey(ctx): case initContextWithBasicAuth(ctx, orgId): case initContextWithAuthProxy(ctx, orgId): - case initContextWithUserSessionCookie(ctx, orgId): + //case initContextWithUserSessionCookie(ctx, orgId): + case initContextWithToken(ctx, orgId, ats): case initContextWithAnonymousUser(ctx): } @@ -58,6 +59,11 @@ func GetContextHandler() macaron.Handler { c.Map(ctx) + c.Next() + + //if signed in with token + //ats.RefreshToken() + // update last seen every 5min if ctx.ShouldUpdateLastSeenAt() { ctx.Logger.Debug("Updating last user_seen_at", "user_id", ctx.UserId) @@ -88,6 +94,25 @@ func initContextWithAnonymousUser(ctx *m.ReqContext) bool { return true } +func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthTokenService) bool { + user, err := ts.LookupToken(ctx) + if err != nil { + ctx.Logger.Info("failed to look up user based on cookie") + return false + } + + query := m.GetSignedInUserQuery{UserId: user.UserId, OrgId: orgID} + if err := bus.Dispatch(&query); err != nil { + ctx.Logger.Error("Failed to get user with id", "userId", user.UserId, "error", err) + return false + } + + ctx.SignedInUser = query.Result + ctx.IsSignedIn = true + + return true +} + func initContextWithUserSessionCookie(ctx *m.ReqContext, orgId int64) bool { // initialize session if err := ctx.Session.Start(ctx.Context); err != nil { diff --git a/pkg/middleware/org_redirect.go b/pkg/middleware/org_redirect.go index db263c2a17a..ca63733946c 100644 --- a/pkg/middleware/org_redirect.go +++ b/pkg/middleware/org_redirect.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" - "gopkg.in/macaron.v1" ) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index aefcccadbd6..d812124f1c1 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -3,16 +3,17 @@ package auth import ( "crypto/sha256" "encoding/hex" + "fmt" + "net/http" + "net/url" "time" - "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/util" - macaron "gopkg.in/macaron.v1" - "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" ) func init() { @@ -42,7 +43,15 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model } c.Resp.Header().Del("Set-Cookie") - c.SetCookie(sessionCookieKey, userToken.unhashedToken, setting.AppSubUrl+"/", setting.Domain, false, true) + cookie := http.Cookie{ + Name: sessionCookieKey, + Value: url.QueryEscape(userToken.unhashedToken), + HttpOnly: true, + Expires: time.Now().Add(time.Minute * 10), + Domain: setting.Domain, + } + + c.Resp.Header().Add("Set-Cookie", cookie.String()) return nil } @@ -51,27 +60,27 @@ func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { c.SetCookie(sessionCookieKey, "", -1, setting.AppSubUrl+"/", setting.Domain, false, true) } -func (s *UserAuthTokenService) RequestMiddleware() macaron.Handler { - return func(ctx *models.ReqContext) { - authToken := ctx.GetCookie(sessionCookieKey) - userToken, err := s.lookupToken(authToken) - if err != nil { +// func (s *UserAuthTokenService) RequestMiddleware() macaron.Handler { +// return func(ctx *models.ReqContext) { +// authToken := ctx.GetCookie(sessionCookieKey) +// userToken, err := s.LookupToken(authToken) +// if err != nil { - } +// } - ctx.Next() +// ctx.Next() - refreshed, err := s.refreshToken(userToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) - if err != nil { +// refreshed, err := s.RefreshToken(userToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) +// if err != nil { - } +// } - if refreshed { - ctx.Resp.Header().Del("Set-Cookie") - ctx.SetCookie(sessionCookieKey, userToken.unhashedToken, setting.AppSubUrl+"/", setting.Domain, false, true) - } - } -} +// if refreshed { +// ctx.Resp.Header().Del("Set-Cookie") +// ctx.SetCookie(sessionCookieKey, userToken.unhashedToken, setting.AppSubUrl+"/", setting.Domain, false, true) +// } +// } +// } func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) { clientIP = util.ParseIPAddress(clientIP) @@ -104,7 +113,12 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str return &userToken, nil } -func (s *UserAuthTokenService) lookupToken(unhashedToken string) (*userAuthToken, error) { +func (s *UserAuthTokenService) LookupToken(ctx *models.ReqContext) (*userAuthToken, error) { + unhashedToken := ctx.GetCookie(sessionCookieKey) + if unhashedToken == "" { + return nil, fmt.Errorf("session token cookie is empty") + } + hashedToken := hashToken(unhashedToken) var userToken userAuthToken @@ -157,7 +171,7 @@ func (s *UserAuthTokenService) lookupToken(unhashedToken string) (*userAuthToken return &userToken, nil } -func (s *UserAuthTokenService) refreshToken(token *userAuthToken, clientIP, userAgent string) (bool, error) { +func (s *UserAuthTokenService) RefreshToken(token *userAuthToken, clientIP, userAgent string) (bool, error) { // lookup token in db // refresh token if needed diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 2e4618c10ed..4a0ca952bd1 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -27,22 +27,22 @@ func TestUserAuthToken(t *testing.T) { So(token.AuthTokenSeen, ShouldBeFalse) Convey("When lookup unhashed token should return user auth token", func() { - lookupToken, err := userAuthTokenService.lookupToken(token.unhashedToken) + LookupToken, err := userAuthTokenService.LookupToken(token.unhashedToken) So(err, ShouldBeNil) - So(lookupToken, ShouldNotBeNil) - So(lookupToken.UserId, ShouldEqual, userID) - So(lookupToken.AuthTokenSeen, ShouldBeTrue) + So(LookupToken, ShouldNotBeNil) + So(LookupToken.UserId, ShouldEqual, userID) + So(LookupToken.AuthTokenSeen, ShouldBeTrue) - storedAuthToken, err := ctx.getAuthTokenByID(lookupToken.Id) + storedAuthToken, err := ctx.getAuthTokenByID(LookupToken.Id) So(err, ShouldBeNil) So(storedAuthToken, ShouldNotBeNil) So(storedAuthToken.AuthTokenSeen, ShouldBeTrue) }) Convey("When lookup hashed token should return user auth token not found error", func() { - lookupToken, err := userAuthTokenService.lookupToken(token.AuthToken) + LookupToken, err := userAuthTokenService.LookupToken(token.AuthToken) So(err, ShouldEqual, ErrAuthTokenNotFound) - So(lookupToken, ShouldBeNil) + So(LookupToken, ShouldBeNil) }) }) @@ -51,25 +51,25 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(token, ShouldNotBeNil) - _, err = userAuthTokenService.lookupToken(token.unhashedToken) + _, err = userAuthTokenService.LookupToken(token.unhashedToken) So(err, ShouldBeNil) token, err = ctx.getAuthTokenByID(token.Id) So(err, ShouldBeNil) // set now (now - 23 hours) - _, err = userAuthTokenService.refreshToken(token, "192.168.10.11:1234", "some user agent") + _, err = userAuthTokenService.RefreshToken(token, "192.168.10.11:1234", "some user agent") So(err, ShouldBeNil) - _, err = userAuthTokenService.lookupToken(token.unhashedToken) + _, err = userAuthTokenService.LookupToken(token.unhashedToken) So(err, ShouldBeNil) - stillGood, err := userAuthTokenService.lookupToken(token.unhashedToken) + stillGood, err := userAuthTokenService.LookupToken(token.unhashedToken) So(err, ShouldBeNil) So(stillGood, ShouldNotBeNil) // set now (new - 2 hours) - notGood, err := userAuthTokenService.lookupToken(token.unhashedToken) + notGood, err := userAuthTokenService.LookupToken(token.unhashedToken) So(err, ShouldEqual, ErrAuthTokenNotFound) So(notGood, ShouldBeNil) }) @@ -82,7 +82,7 @@ func TestUserAuthToken(t *testing.T) { prevToken := token.AuthToken unhashedPrev := token.unhashedToken - refreshed, err := userAuthTokenService.refreshToken(token, "192.168.10.12:1234", "a new user agent") + refreshed, err := userAuthTokenService.RefreshToken(token, "192.168.10.12:1234", "a new user agent") So(err, ShouldBeNil) So(refreshed, ShouldBeFalse) @@ -95,7 +95,7 @@ func TestUserAuthToken(t *testing.T) { return t } - refreshed, err = userAuthTokenService.refreshToken(token, "192.168.10.12:1234", "a new user agent") + refreshed, err = userAuthTokenService.RefreshToken(token, "192.168.10.12:1234", "a new user agent") So(err, ShouldBeNil) So(refreshed, ShouldBeTrue) @@ -112,13 +112,13 @@ func TestUserAuthToken(t *testing.T) { So(token.SeenAt, ShouldEqual, 0) So(token.PrevAuthToken, ShouldEqual, prevToken) - lookedUp, err := userAuthTokenService.lookupToken(token.unhashedToken) + lookedUp, err := userAuthTokenService.LookupToken(token.unhashedToken) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) So(lookedUp.AuthTokenSeen, ShouldBeTrue) So(lookedUp.SeenAt, ShouldEqual, t.Unix()) - lookedUp, err = userAuthTokenService.lookupToken(unhashedPrev) + lookedUp, err = userAuthTokenService.LookupToken(unhashedPrev) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) So(lookedUp.Id, ShouldEqual, token.Id) @@ -127,7 +127,7 @@ func TestUserAuthToken(t *testing.T) { return t.Add(2 * time.Minute) } - lookedUp, err = userAuthTokenService.lookupToken(unhashedPrev) + lookedUp, err = userAuthTokenService.LookupToken(unhashedPrev) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) @@ -136,7 +136,7 @@ func TestUserAuthToken(t *testing.T) { So(lookedUp, ShouldNotBeNil) So(lookedUp.AuthTokenSeen, ShouldBeFalse) - refreshed, err = userAuthTokenService.refreshToken(token, "192.168.10.12:1234", "a new user agent") + refreshed, err = userAuthTokenService.RefreshToken(token, "192.168.10.12:1234", "a new user agent") So(err, ShouldBeNil) So(refreshed, ShouldBeTrue) From c2accfa4c031617ca55afb3ea9fca0ec1e0ea889 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 17 Jan 2019 17:11:52 +0100 Subject: [PATCH 05/49] inital code for rotate --- pkg/middleware/middleware.go | 35 +++++++++++- pkg/models/context.go | 21 ++++++- pkg/services/auth/auth_token.go | 83 ++++++++++++++++++++++------ pkg/services/auth/auth_token_test.go | 23 ++++---- pkg/services/auth/model.go | 28 +++++----- 5 files changed, 141 insertions(+), 49 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 28f08869425..57e47bd2860 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -1,7 +1,10 @@ package middleware import ( + "net/http" + "net/url" "strconv" + "time" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/apikeygen" @@ -11,7 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" - "gopkg.in/macaron.v1" + macaron "gopkg.in/macaron.v1" ) var ( @@ -62,7 +65,27 @@ func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { c.Next() //if signed in with token - //ats.RefreshToken() + rotated, err := ats.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) + if err != nil { + ctx.Logger.Error("failed to rotate token", "error", err) + return + } + + if rotated { + ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) + //c.SetCookie("grafana_session", url.QueryEscape(ctx.UserToken.UnhashedToken), nil, setting.AppSubUrl+"/", setting.Domain, false, true) + // ctx.Resp.Header().Del("Set-Cookie") + cookie := http.Cookie{ + Name: "grafana_session", + Value: url.QueryEscape(ctx.UserToken.UnhashedToken), + HttpOnly: true, + MaxAge: int(time.Minute * 10), + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", + } + + ctx.Resp.Header().Add("Set-Cookie", cookie.String()) + } // update last seen every 5min if ctx.ShouldUpdateLastSeenAt() { @@ -95,7 +118,12 @@ func initContextWithAnonymousUser(ctx *m.ReqContext) bool { } func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthTokenService) bool { - user, err := ts.LookupToken(ctx) + unhashedToken := ctx.GetCookie("grafana_session") + if unhashedToken == "" { + return false + } + + user, err := ts.LookupToken(unhashedToken) if err != nil { ctx.Logger.Info("failed to look up user based on cookie") return false @@ -109,6 +137,7 @@ func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthToken ctx.SignedInUser = query.Result ctx.IsSignedIn = true + ctx.UserToken = user return true } diff --git a/pkg/models/context.go b/pkg/models/context.go index 7cb80a957c3..f6df8b1c2f0 100644 --- a/pkg/models/context.go +++ b/pkg/models/context.go @@ -3,17 +3,32 @@ package models import ( "strings" - "github.com/prometheus/client_golang/prometheus" - "gopkg.in/macaron.v1" - "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" + "github.com/prometheus/client_golang/prometheus" + "gopkg.in/macaron.v1" ) +type UserAuthToken struct { + Id int64 + UserId int64 + AuthToken string + PrevAuthToken string + UserAgent string + ClientIp string + AuthTokenSeen bool + SeenAt int64 + RotatedAt int64 + CreatedAt int64 + UpdatedAt int64 + UnhashedToken string `xorm:"-"` +} + type ReqContext struct { *macaron.Context *SignedInUser + UserToken *UserAuthToken Session session.SessionStore diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index d812124f1c1..e393239ef9d 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -3,7 +3,6 @@ package auth import ( "crypto/sha256" "encoding/hex" - "fmt" "net/http" "net/url" "time" @@ -45,10 +44,11 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model c.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ Name: sessionCookieKey, - Value: url.QueryEscape(userToken.unhashedToken), + Value: url.QueryEscape(userToken.UnhashedToken), HttpOnly: true, - Expires: time.Now().Add(time.Minute * 10), + MaxAge: int(time.Minute * 10), Domain: setting.Domain, + Path: setting.AppSubUrl + "/", } c.Resp.Header().Add("Set-Cookie", cookie.String()) @@ -57,7 +57,18 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model } func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { - c.SetCookie(sessionCookieKey, "", -1, setting.AppSubUrl+"/", setting.Domain, false, true) + //c.SetCookie(sessionCookieKey, "", -1, setting.AppSubUrl+"/", setting.Domain, false, true) + c.Resp.Header().Del("Set-Cookie") + cookie := http.Cookie{ + Name: sessionCookieKey, + Value: "", + HttpOnly: true, + MaxAge: -1, + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", + } + + c.Resp.Header().Add("Set-Cookie", cookie.String()) } // func (s *UserAuthTokenService) RequestMiddleware() macaron.Handler { @@ -82,7 +93,7 @@ func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { // } // } -func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) { +func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*models.UserAuthToken, error) { clientIP = util.ParseIPAddress(clientIP) token, err := util.RandomHex(16) if err != nil { @@ -91,7 +102,7 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str hashedToken := hashToken(token) - userToken := userAuthToken{ + userToken := models.UserAuthToken{ UserId: userId, AuthToken: hashedToken, PrevAuthToken: hashedToken, @@ -108,20 +119,15 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str return nil, err } - userToken.unhashedToken = token + userToken.UnhashedToken = token return &userToken, nil } -func (s *UserAuthTokenService) LookupToken(ctx *models.ReqContext) (*userAuthToken, error) { - unhashedToken := ctx.GetCookie(sessionCookieKey) - if unhashedToken == "" { - return nil, fmt.Errorf("session token cookie is empty") - } - +func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAuthToken, error) { hashedToken := hashToken(unhashedToken) - var userToken userAuthToken + var userToken models.UserAuthToken exists, err := s.SQLStore.NewSession().Where("auth_token = ? OR prev_auth_token = ?", hashedToken, hashedToken).Get(&userToken) if err != nil { return nil, err @@ -166,14 +172,55 @@ func (s *UserAuthTokenService) LookupToken(ctx *models.ReqContext) (*userAuthTok } } - userToken.unhashedToken = unhashedToken + userToken.UnhashedToken = unhashedToken return &userToken, nil } -func (s *UserAuthTokenService) RefreshToken(token *userAuthToken, clientIP, userAgent string) (bool, error) { - // lookup token in db - // refresh token if needed +func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientIP, userAgent string) (bool, error) { + if token == nil { + return false, nil + } + + var needsRotation = false + rotatedAt := time.Unix(token.RotatedAt, 0) + if token.AuthTokenSeen { + needsRotation = rotatedAt.Before(now().Add(time.Duration(-1) * time.Minute)) + } else { + needsRotation = rotatedAt.Before(now().Add(time.Duration(-30) * time.Second)) + } + + s.log.Info("refresh token", "needs rotation?", needsRotation, "auth_token_seen", token.AuthTokenSeen, "rotated_at", rotatedAt, "token.Id", token.Id) + if !needsRotation { + return false, nil + } + + newToken, _ := util.RandomHex(16) + hashedToken := hashToken(newToken) + + sql := ` + UPDATE user_auth_token + SET + auth_token_seen = false, + seen_at = null, + user_agent = ?, + client_ip = ?, + prev_auth_token = case when auth_token_seen then auth_token else prev_auth_token end, + auth_token = ?, + rotated_at = ? + WHERE id = ? AND (auth_token_seen or rotated_at < ?)` + + res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, hashedToken, now().Unix(), token.Id, now().Add(time.Duration(-30)*time.Second)) + if err != nil { + return false, err + } + + affected, _ := res.RowsAffected() + s.log.Info("rotated", "affected", affected, "auth_token_id", token.Id, "userId", token.UserId, "user_agent", userAgent, "client_ip", clientIP) + if affected > 0 { + token.UnhashedToken = newToken + return true, nil + } return false, nil } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 4a0ca952bd1..2ee7e2d67be 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" . "github.com/smartystreets/goconvey/convey" ) @@ -27,7 +28,7 @@ func TestUserAuthToken(t *testing.T) { So(token.AuthTokenSeen, ShouldBeFalse) Convey("When lookup unhashed token should return user auth token", func() { - LookupToken, err := userAuthTokenService.LookupToken(token.unhashedToken) + LookupToken, err := userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) So(LookupToken, ShouldNotBeNil) So(LookupToken.UserId, ShouldEqual, userID) @@ -51,7 +52,7 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(token, ShouldNotBeNil) - _, err = userAuthTokenService.LookupToken(token.unhashedToken) + _, err = userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) token, err = ctx.getAuthTokenByID(token.Id) @@ -61,15 +62,15 @@ func TestUserAuthToken(t *testing.T) { _, err = userAuthTokenService.RefreshToken(token, "192.168.10.11:1234", "some user agent") So(err, ShouldBeNil) - _, err = userAuthTokenService.LookupToken(token.unhashedToken) + _, err = userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) - stillGood, err := userAuthTokenService.LookupToken(token.unhashedToken) + stillGood, err := userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) So(stillGood, ShouldNotBeNil) // set now (new - 2 hours) - notGood, err := userAuthTokenService.LookupToken(token.unhashedToken) + notGood, err := userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldEqual, ErrAuthTokenNotFound) So(notGood, ShouldBeNil) }) @@ -80,7 +81,7 @@ func TestUserAuthToken(t *testing.T) { So(token, ShouldNotBeNil) prevToken := token.AuthToken - unhashedPrev := token.unhashedToken + unhashedPrev := token.UnhashedToken refreshed, err := userAuthTokenService.RefreshToken(token, "192.168.10.12:1234", "a new user agent") So(err, ShouldBeNil) @@ -99,11 +100,11 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(refreshed, ShouldBeTrue) - unhashedToken := token.unhashedToken + unhashedToken := token.UnhashedToken token, err = ctx.getAuthTokenByID(token.Id) So(err, ShouldBeNil) - token.unhashedToken = unhashedToken + token.UnhashedToken = unhashedToken So(token.RotatedAt, ShouldEqual, t.Unix()) So(token.ClientIp, ShouldEqual, "192.168.10.12") @@ -112,7 +113,7 @@ func TestUserAuthToken(t *testing.T) { So(token.SeenAt, ShouldEqual, 0) So(token.PrevAuthToken, ShouldEqual, prevToken) - lookedUp, err := userAuthTokenService.LookupToken(token.unhashedToken) + lookedUp, err := userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) So(lookedUp.AuthTokenSeen, ShouldBeTrue) @@ -180,9 +181,9 @@ type testContext struct { tokenService *UserAuthTokenService } -func (c *testContext) getAuthTokenByID(id int64) (*userAuthToken, error) { +func (c *testContext) getAuthTokenByID(id int64) (*models.UserAuthToken, error) { sess := c.sqlstore.NewSession() - var t userAuthToken + var t models.UserAuthToken found, err := sess.ID(id).Get(&t) if err != nil || !found { return nil, err diff --git a/pkg/services/auth/model.go b/pkg/services/auth/model.go index a033b96be31..4347f6d2d6a 100644 --- a/pkg/services/auth/model.go +++ b/pkg/services/auth/model.go @@ -9,17 +9,17 @@ var ( ErrAuthTokenNotFound = errors.New("User auth token not found") ) -type userAuthToken struct { - Id int64 - UserId int64 - AuthToken string - PrevAuthToken string - UserAgent string - ClientIp string - AuthTokenSeen bool - SeenAt int64 - RotatedAt int64 - CreatedAt int64 - UpdatedAt int64 - unhashedToken string `xorm:"-"` -} +// type userAuthToken struct { +// Id int64 +// UserId int64 +// AuthToken string +// PrevAuthToken string +// UserAgent string +// ClientIp string +// AuthTokenSeen bool +// SeenAt int64 +// RotatedAt int64 +// CreatedAt int64 +// UpdatedAt int64 +// unhashedToken string `xorm:"-"` +// } From 8b3fe41b0a9c7cba9a1d77f7c6ccafbd43949462 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 17 Jan 2019 17:32:33 +0100 Subject: [PATCH 06/49] log fix --- pkg/api/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index 37b12d03299..b4c6f8af58e 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -168,7 +168,7 @@ func (hs *HTTPServer) loginUserWithUser(user *m.User, c *m.ReqContext) { err := hs.AuthTokenService.UserAuthenticatedHook(user, c) if err != nil { - hs.log.Error("User auth hook failed", err) + hs.log.Error("User auth hook failed", "error", err) } } From 97c7963f176576c39d2bc5f7921e25dcf1ce29ac Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 17 Jan 2019 20:27:53 +0100 Subject: [PATCH 07/49] fix cannot set cookie when response is written --- pkg/middleware/middleware.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 57e47bd2860..aad47fccdac 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -62,8 +62,6 @@ func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { c.Map(ctx) - c.Next() - //if signed in with token rotated, err := ats.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) if err != nil { From 81879f0162652bf51e6f760df7ae0854766a3bac Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 17 Jan 2019 20:29:26 +0100 Subject: [PATCH 08/49] fix broken code --- pkg/api/common_test.go | 2 +- pkg/middleware/middleware_test.go | 2 +- pkg/middleware/recovery_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 8b66a7a468b..f99902aac51 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -123,7 +123,7 @@ func setupScenarioContext(url string) *scenarioContext { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) - sc.m.Use(middleware.GetContextHandler()) + sc.m.Use(middleware.GetContextHandler(nil)) sc.m.Use(middleware.Sessioner(&session.Options{}, 0)) return sc diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index b9a8afce6c6..73c84af09fd 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -487,7 +487,7 @@ func middlewareScenario(desc string, fn scenarioFunc) { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) - sc.m.Use(GetContextHandler()) + sc.m.Use(GetContextHandler(nil)) // mock out gc goroutine session.StartSessionGC = func() {} sc.m.Use(Sessioner(&ms.Options{}, 0)) diff --git a/pkg/middleware/recovery_test.go b/pkg/middleware/recovery_test.go index c92150f3b7d..5e70fffc45e 100644 --- a/pkg/middleware/recovery_test.go +++ b/pkg/middleware/recovery_test.go @@ -64,7 +64,7 @@ func recoveryScenario(desc string, url string, fn scenarioFunc) { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) - sc.m.Use(GetContextHandler()) + sc.m.Use(GetContextHandler(nil)) // mock out gc goroutine session.StartSessionGC = func() {} sc.m.Use(Sessioner(&ms.Options{}, 0)) From fd937e3d95ce817292dfc4a6df53dee07cad726a Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 17 Jan 2019 21:03:27 +0100 Subject: [PATCH 09/49] remove maxage from session token --- pkg/middleware/middleware.go | 10 ++++------ pkg/services/auth/auth_token.go | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index aad47fccdac..60869d7bd1f 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -4,7 +4,6 @@ import ( "net/http" "net/url" "strconv" - "time" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/apikeygen" @@ -71,15 +70,14 @@ func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { if rotated { ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) - //c.SetCookie("grafana_session", url.QueryEscape(ctx.UserToken.UnhashedToken), nil, setting.AppSubUrl+"/", setting.Domain, false, true) - // ctx.Resp.Header().Del("Set-Cookie") + ctx.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ Name: "grafana_session", Value: url.QueryEscape(ctx.UserToken.UnhashedToken), HttpOnly: true, - MaxAge: int(time.Minute * 10), - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", + //MaxAge: 600, + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", } ctx.Resp.Header().Add("Set-Cookie", cookie.String()) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index e393239ef9d..1b2c7307923 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -46,9 +46,9 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model Name: sessionCookieKey, Value: url.QueryEscape(userToken.UnhashedToken), HttpOnly: true, - MaxAge: int(time.Minute * 10), - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", + //MaxAge: 600, + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", } c.Resp.Header().Add("Set-Cookie", cookie.String()) From 47a7d93fd9a1569d61800a2399158267f16e9684 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 21 Jan 2019 08:59:01 +0100 Subject: [PATCH 10/49] moves rotation into auth since both happens before c.Next() --- pkg/middleware/middleware.go | 44 ++++++++++++++++----------------- pkg/services/auth/auth_token.go | 4 +-- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 60869d7bd1f..0635ad55c64 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -61,28 +61,6 @@ func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { c.Map(ctx) - //if signed in with token - rotated, err := ats.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) - if err != nil { - ctx.Logger.Error("failed to rotate token", "error", err) - return - } - - if rotated { - ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) - ctx.Resp.Header().Del("Set-Cookie") - cookie := http.Cookie{ - Name: "grafana_session", - Value: url.QueryEscape(ctx.UserToken.UnhashedToken), - HttpOnly: true, - //MaxAge: 600, - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", - } - - ctx.Resp.Header().Add("Set-Cookie", cookie.String()) - } - // update last seen every 5min if ctx.ShouldUpdateLastSeenAt() { ctx.Logger.Debug("Updating last user_seen_at", "user_id", ctx.UserId) @@ -114,6 +92,7 @@ func initContextWithAnonymousUser(ctx *m.ReqContext) bool { } func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthTokenService) bool { + //auth User unhashedToken := ctx.GetCookie("grafana_session") if unhashedToken == "" { return false @@ -135,6 +114,27 @@ func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthToken ctx.IsSignedIn = true ctx.UserToken = user + //rotate session token if needed. + rotated, err := ts.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) + if err != nil { + ctx.Logger.Error("failed to rotate token", "error", err, "user.id", user.UserId, "user_token.id", user.Id) + return true + } + + if rotated { + ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) + ctx.Resp.Header().Del("Set-Cookie") + cookie := http.Cookie{ + Name: "grafana_session", + Value: url.QueryEscape(ctx.UserToken.UnhashedToken), + HttpOnly: true, + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", + } + + ctx.Resp.Header().Add("Set-Cookie", cookie.String()) + } + return true } diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 1b2c7307923..5a5b5fb005c 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -190,7 +190,7 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI needsRotation = rotatedAt.Before(now().Add(time.Duration(-30) * time.Second)) } - s.log.Info("refresh token", "needs rotation?", needsRotation, "auth_token_seen", token.AuthTokenSeen, "rotated_at", rotatedAt, "token.Id", token.Id) + s.log.Debug("refresh token", "needs rotation?", needsRotation, "auth_token_seen", token.AuthTokenSeen, "rotated_at", rotatedAt, "token.Id", token.Id) if !needsRotation { return false, nil } @@ -216,7 +216,7 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI } affected, _ := res.RowsAffected() - s.log.Info("rotated", "affected", affected, "auth_token_id", token.Id, "userId", token.UserId, "user_agent", userAgent, "client_ip", clientIP) + s.log.Debug("rotated", "affected", affected, "auth_token_id", token.Id, "userId", token.UserId, "user_agent", userAgent, "client_ip", clientIP) if affected > 0 { token.UnhashedToken = newToken return true, nil From 2e97d39abedf041968b8acf396b1e8cc315ba450 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 21 Jan 2019 10:01:48 +0100 Subject: [PATCH 11/49] removes commented code --- pkg/services/auth/auth_token.go | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 5a5b5fb005c..db5b938e0fb 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -46,9 +46,8 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model Name: sessionCookieKey, Value: url.QueryEscape(userToken.UnhashedToken), HttpOnly: true, - //MaxAge: 600, - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", } c.Resp.Header().Add("Set-Cookie", cookie.String()) @@ -57,7 +56,6 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model } func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { - //c.SetCookie(sessionCookieKey, "", -1, setting.AppSubUrl+"/", setting.Domain, false, true) c.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ Name: sessionCookieKey, @@ -71,28 +69,6 @@ func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { c.Resp.Header().Add("Set-Cookie", cookie.String()) } -// func (s *UserAuthTokenService) RequestMiddleware() macaron.Handler { -// return func(ctx *models.ReqContext) { -// authToken := ctx.GetCookie(sessionCookieKey) -// userToken, err := s.LookupToken(authToken) -// if err != nil { - -// } - -// ctx.Next() - -// refreshed, err := s.RefreshToken(userToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) -// if err != nil { - -// } - -// if refreshed { -// ctx.Resp.Header().Del("Set-Cookie") -// ctx.SetCookie(sessionCookieKey, userToken.unhashedToken, setting.AppSubUrl+"/", setting.Domain, false, true) -// } -// } -// } - func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*models.UserAuthToken, error) { clientIP = util.ParseIPAddress(clientIP) token, err := util.RandomHex(16) From 0495499b4f0f82eae5599b4c44e06dcb9030771e Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 09:13:55 +0100 Subject: [PATCH 12/49] fix ip address parsing of loopback address --- pkg/util/ip_address.go | 6 ++++-- pkg/util/ip_address_test.go | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/util/ip_address.go b/pkg/util/ip_address.go index 4e9a9378c6b..d8d95ef3acd 100644 --- a/pkg/util/ip_address.go +++ b/pkg/util/ip_address.go @@ -7,11 +7,13 @@ import ( // ParseIPAddress parses an IP address and removes port and/or IPV6 format func ParseIPAddress(input string) string { - var s string + s := input lastIndex := strings.LastIndex(input, ":") if lastIndex != -1 { - s = input[:lastIndex] + if lastIndex > 0 && input[lastIndex-1:lastIndex] != ":" { + s = input[:lastIndex] + } } s = strings.Replace(s, "[", "", -1) diff --git a/pkg/util/ip_address_test.go b/pkg/util/ip_address_test.go index 644340a5e82..fd3e3ea8587 100644 --- a/pkg/util/ip_address_test.go +++ b/pkg/util/ip_address_test.go @@ -10,5 +10,7 @@ func TestParseIPAddress(t *testing.T) { Convey("Test parse ip address", t, func() { So(ParseIPAddress("192.168.0.140:456"), ShouldEqual, "192.168.0.140") So(ParseIPAddress("[::1:456]"), ShouldEqual, "127.0.0.1") + So(ParseIPAddress("[::1]"), ShouldEqual, "127.0.0.1") + So(ParseIPAddress("192.168.0.140"), ShouldEqual, "192.168.0.140") }) } From f3125b447bc1f578f06d00a7c7a68cc80390d3dd Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 09:53:53 +0100 Subject: [PATCH 13/49] dead code --- pkg/middleware/middleware.go | 38 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 0635ad55c64..109def9ff2c 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -138,28 +138,28 @@ func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthToken return true } -func initContextWithUserSessionCookie(ctx *m.ReqContext, orgId int64) bool { - // initialize session - if err := ctx.Session.Start(ctx.Context); err != nil { - ctx.Logger.Error("Failed to start session", "error", err) - return false - } +// func initContextWithUserSessionCookie(ctx *m.ReqContext, orgId int64) bool { +// // initialize session +// if err := ctx.Session.Start(ctx.Context); err != nil { +// ctx.Logger.Error("Failed to start session", "error", err) +// return false +// } - var userId int64 - if userId = getRequestUserId(ctx); userId == 0 { - return false - } +// var userId int64 +// if userId = getRequestUserId(ctx); userId == 0 { +// return false +// } - query := m.GetSignedInUserQuery{UserId: userId, OrgId: orgId} - if err := bus.Dispatch(&query); err != nil { - ctx.Logger.Error("Failed to get user with id", "userId", userId, "error", err) - return false - } +// query := m.GetSignedInUserQuery{UserId: userId, OrgId: orgId} +// if err := bus.Dispatch(&query); err != nil { +// ctx.Logger.Error("Failed to get user with id", "userId", userId, "error", err) +// return false +// } - ctx.SignedInUser = query.Result - ctx.IsSignedIn = true - return true -} +// ctx.SignedInUser = query.Result +// ctx.IsSignedIn = true +// return true +// } func initContextWithApiKey(ctx *m.ReqContext) bool { var keyString string From 0d1e3759ebbaf74644b32a96c791461d0c0abdce Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 10:20:06 +0100 Subject: [PATCH 14/49] mixor fixes --- pkg/middleware/middleware.go | 2 +- pkg/services/auth/auth_token.go | 23 +++++++++++++---------- pkg/services/auth/auth_token_test.go | 3 +++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 109def9ff2c..a6800971f4f 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -132,7 +132,7 @@ func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthToken Path: setting.AppSubUrl + "/", } - ctx.Resp.Header().Add("Set-Cookie", cookie.String()) + http.SetCookie(ctx.Resp, &cookie) } return true diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index db5b938e0fb..aefacd7788d 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -19,7 +19,11 @@ func init() { registry.RegisterService(&UserAuthTokenService{}) } -var now = time.Now +var ( + now = time.Now + RotateTime = 10 * time.Second + UrgentRotateTime = 5 * time.Second +) // UserAuthTokenService are used for generating and validating user auth tokens type UserAuthTokenService struct { @@ -50,7 +54,7 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model Path: setting.AppSubUrl + "/", } - c.Resp.Header().Add("Set-Cookie", cookie.String()) + http.SetCookie(c.Resp, &cookie) return nil } @@ -61,12 +65,10 @@ func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { Name: sessionCookieKey, Value: "", HttpOnly: true, - MaxAge: -1, Domain: setting.Domain, Path: setting.AppSubUrl + "/", } - - c.Resp.Header().Add("Set-Cookie", cookie.String()) + http.SetCookie(c.Resp, &cookie) } func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*models.UserAuthToken, error) { @@ -115,7 +117,7 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu if userToken.AuthToken != hashedToken && userToken.PrevAuthToken == hashedToken && userToken.AuthTokenSeen { userToken.AuthTokenSeen = false - expireBefore := now().Add(-1 * time.Minute).Unix() + expireBefore := now().Add(-RotateTime).Unix() affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND prev_auth_token = ? AND rotated_at < ?", userToken.Id, userToken.PrevAuthToken, expireBefore).AllCols().Update(&userToken) if err != nil { return nil, err @@ -158,12 +160,12 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI return false, nil } - var needsRotation = false + needsRotation := false rotatedAt := time.Unix(token.RotatedAt, 0) if token.AuthTokenSeen { - needsRotation = rotatedAt.Before(now().Add(time.Duration(-1) * time.Minute)) + needsRotation = rotatedAt.Before(now().Add(-RotateTime)) } else { - needsRotation = rotatedAt.Before(now().Add(time.Duration(-30) * time.Second)) + needsRotation = rotatedAt.Before(now().Add(-UrgentRotateTime)) } s.log.Debug("refresh token", "needs rotation?", needsRotation, "auth_token_seen", token.AuthTokenSeen, "rotated_at", rotatedAt, "token.Id", token.Id) @@ -171,6 +173,7 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI return false, nil } + clientIP = util.ParseIPAddress(clientIP) newToken, _ := util.RandomHex(16) hashedToken := hashToken(newToken) @@ -186,7 +189,7 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI rotated_at = ? WHERE id = ? AND (auth_token_seen or rotated_at < ?)` - res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, hashedToken, now().Unix(), token.Id, now().Add(time.Duration(-30)*time.Second)) + res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, hashedToken, now().Unix(), token.Id, now().Add(-UrgentRotateTime)) if err != nil { return false, err } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 2ee7e2d67be..bb146252fa4 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -170,6 +170,9 @@ func createTestContext(t *testing.T) *testContext { log: log.New("test-logger"), } + RotateTime = 10 * time.Minute + UrgentRotateTime = time.Minute + return &testContext{ sqlstore: sqlstore, tokenService: tokenService, From 766cfab374bd8ed7aad8870fd5c28734c92a6505 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 10:22:18 +0100 Subject: [PATCH 15/49] change rotate time --- pkg/services/auth/auth_token.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index aefacd7788d..1c687841f7e 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -21,8 +21,8 @@ func init() { var ( now = time.Now - RotateTime = 10 * time.Second - UrgentRotateTime = 5 * time.Second + RotateTime = 1 * time.Minute + UrgentRotateTime = 30 * time.Second ) // UserAuthTokenService are used for generating and validating user auth tokens From 734a7d38b28c15849389328cbf7ca0d3286f198c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 11:21:43 +0100 Subject: [PATCH 16/49] set cookie name from configuration --- pkg/middleware/middleware.go | 2 +- pkg/services/auth/auth_token.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index a6800971f4f..6cec2b9ad05 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -125,7 +125,7 @@ func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthToken ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) ctx.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ - Name: "grafana_session", + Name: setting.SessionOptions.CookieName, Value: url.QueryEscape(ctx.UserToken.UnhashedToken), HttpOnly: true, Domain: setting.Domain, diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 1c687841f7e..5929043573e 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -37,8 +37,6 @@ func (s *UserAuthTokenService) Init() error { return nil } -const sessionCookieKey = "grafana_session" - func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *models.ReqContext) error { userToken, err := s.CreateToken(user.Id, c.RemoteAddr(), c.Req.UserAgent()) if err != nil { @@ -47,11 +45,12 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model c.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ - Name: sessionCookieKey, + Name: setting.SessionOptions.CookieName, Value: url.QueryEscape(userToken.UnhashedToken), HttpOnly: true, Domain: setting.Domain, Path: setting.AppSubUrl + "/", + Secure: setting.SessionOptions.Secure, } http.SetCookie(c.Resp, &cookie) @@ -62,11 +61,12 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { c.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ - Name: sessionCookieKey, + Name: setting.SessionOptions.CookieName, Value: "", HttpOnly: true, Domain: setting.Domain, Path: setting.AppSubUrl + "/", + Secure: setting.SessionOptions.Secure, } http.SetCookie(c.Resp, &cookie) } From 55b3013eb398e70bd4545e15b028c58fcf436348 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 21 Jan 2019 11:37:44 +0100 Subject: [PATCH 17/49] moves initWithToken to auth package --- pkg/middleware/middleware.go | 52 +--------------------- pkg/services/auth/auth_token.go | 78 +++++++++++++++++++++++---------- 2 files changed, 57 insertions(+), 73 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 6cec2b9ad05..6c4ce1c20ae 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -1,8 +1,6 @@ package middleware import ( - "net/http" - "net/url" "strconv" "github.com/grafana/grafana/pkg/bus" @@ -51,8 +49,7 @@ func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { case initContextWithApiKey(ctx): case initContextWithBasicAuth(ctx, orgId): case initContextWithAuthProxy(ctx, orgId): - //case initContextWithUserSessionCookie(ctx, orgId): - case initContextWithToken(ctx, orgId, ats): + case ats.InitContextWithToken(ctx, orgId): case initContextWithAnonymousUser(ctx): } @@ -91,53 +88,6 @@ func initContextWithAnonymousUser(ctx *m.ReqContext) bool { return true } -func initContextWithToken(ctx *m.ReqContext, orgID int64, ts *auth.UserAuthTokenService) bool { - //auth User - unhashedToken := ctx.GetCookie("grafana_session") - if unhashedToken == "" { - return false - } - - user, err := ts.LookupToken(unhashedToken) - if err != nil { - ctx.Logger.Info("failed to look up user based on cookie") - return false - } - - query := m.GetSignedInUserQuery{UserId: user.UserId, OrgId: orgID} - if err := bus.Dispatch(&query); err != nil { - ctx.Logger.Error("Failed to get user with id", "userId", user.UserId, "error", err) - return false - } - - ctx.SignedInUser = query.Result - ctx.IsSignedIn = true - ctx.UserToken = user - - //rotate session token if needed. - rotated, err := ts.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) - if err != nil { - ctx.Logger.Error("failed to rotate token", "error", err, "user.id", user.UserId, "user_token.id", user.Id) - return true - } - - if rotated { - ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) - ctx.Resp.Header().Del("Set-Cookie") - cookie := http.Cookie{ - Name: setting.SessionOptions.CookieName, - Value: url.QueryEscape(ctx.UserToken.UnhashedToken), - HttpOnly: true, - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", - } - - http.SetCookie(ctx.Resp, &cookie) - } - - return true -} - // func initContextWithUserSessionCookie(ctx *m.ReqContext, orgId int64) bool { // // initialize session // if err := ctx.Session.Start(ctx.Context); err != nil { diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 5929043573e..49d40400205 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -7,6 +7,7 @@ import ( "net/url" "time" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/registry" @@ -23,6 +24,7 @@ var ( now = time.Now RotateTime = 1 * time.Minute UrgentRotateTime = 30 * time.Second + oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. ) // UserAuthTokenService are used for generating and validating user auth tokens @@ -37,38 +39,70 @@ func (s *UserAuthTokenService) Init() error { return nil } +func (s *UserAuthTokenService) InitContextWithToken(ctx *models.ReqContext, orgID int64) bool { + //auth User + unhashedToken := ctx.GetCookie(setting.SessionOptions.CookieName) + if unhashedToken == "" { + return false + } + + user, err := s.LookupToken(unhashedToken) + if err != nil { + ctx.Logger.Info("failed to look up user based on cookie", "error", err) + return false + } + + query := models.GetSignedInUserQuery{UserId: user.UserId, OrgId: orgID} + if err := bus.Dispatch(&query); err != nil { + ctx.Logger.Error("Failed to get user with id", "userId", user.UserId, "error", err) + return false + } + + ctx.SignedInUser = query.Result + ctx.IsSignedIn = true + ctx.UserToken = user + + //rotate session token if needed. + rotated, err := s.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) + if err != nil { + ctx.Logger.Error("failed to rotate token", "error", err, "user.id", user.UserId, "user_token.id", user.Id) + return true + } + + if rotated { + s.writeSessionCookie(ctx, ctx.UserToken.UnhashedToken, oneYearInSeconds) + } + + return true +} + +func (s *UserAuthTokenService) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { + ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) + ctx.Resp.Header().Del("Set-Cookie") + cookie := http.Cookie{ + Name: setting.SessionOptions.CookieName, + Value: url.QueryEscape(value), + HttpOnly: true, + Domain: setting.Domain, + Path: setting.AppSubUrl + "/", + Secure: setting.SessionOptions.Secure, + } + + http.SetCookie(ctx.Resp, &cookie) +} + func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *models.ReqContext) error { userToken, err := s.CreateToken(user.Id, c.RemoteAddr(), c.Req.UserAgent()) if err != nil { return err } - c.Resp.Header().Del("Set-Cookie") - cookie := http.Cookie{ - Name: setting.SessionOptions.CookieName, - Value: url.QueryEscape(userToken.UnhashedToken), - HttpOnly: true, - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", - Secure: setting.SessionOptions.Secure, - } - - http.SetCookie(c.Resp, &cookie) - + s.writeSessionCookie(c, userToken.UnhashedToken, oneYearInSeconds) return nil } func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { - c.Resp.Header().Del("Set-Cookie") - cookie := http.Cookie{ - Name: setting.SessionOptions.CookieName, - Value: "", - HttpOnly: true, - Domain: setting.Domain, - Path: setting.AppSubUrl + "/", - Secure: setting.SessionOptions.Secure, - } - http.SetCookie(c.Resp, &cookie) + s.writeSessionCookie(c, "", -1) } func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*models.UserAuthToken, error) { From 697ddccd8ee6a6dd934d347342440ff3357394b9 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 21 Jan 2019 11:42:10 +0100 Subject: [PATCH 18/49] set userToken on request when logging in --- pkg/services/auth/auth_token.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 49d40400205..bab08778511 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -22,7 +22,7 @@ func init() { var ( now = time.Now - RotateTime = 1 * time.Minute + RotateTime = 1 * time.Minute // this should be read from [session] configuration. UrgentRotateTime = 30 * time.Second oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. ) @@ -77,7 +77,8 @@ func (s *UserAuthTokenService) InitContextWithToken(ctx *models.ReqContext, orgI } func (s *UserAuthTokenService) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { - ctx.Logger.Info("new token", "unhashed token", ctx.UserToken.UnhashedToken) + ctx.Logger.Info("new token", "unhashed token", value) + ctx.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ Name: setting.SessionOptions.CookieName, @@ -97,6 +98,8 @@ func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *model return err } + c.UserToken = userToken + s.writeSessionCookie(c, userToken.UnhashedToken, oneYearInSeconds) return nil } From 565408194aafc1714389ccc71e762a3fa41846ea Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 13:22:20 +0100 Subject: [PATCH 19/49] handle expired tokens --- pkg/services/auth/auth_token.go | 3 ++- pkg/services/auth/auth_token_test.go | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index bab08778511..0ab4e32c0ad 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -141,9 +141,10 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAuthToken, error) { hashedToken := hashToken(unhashedToken) + expireBefore := now().Add(time.Duration(-86400*setting.LogInRememberDays) * time.Second).Unix() var userToken models.UserAuthToken - exists, err := s.SQLStore.NewSession().Where("auth_token = ? OR prev_auth_token = ?", hashedToken, hashedToken).Get(&userToken) + exists, err := s.SQLStore.NewSession().Where("(auth_token = ? OR prev_auth_token = ?) AND created_at > ?", hashedToken, hashedToken, expireBefore).Get(&userToken) if err != nil { return nil, err } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index bb146252fa4..fa67cd62869 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -58,9 +60,13 @@ func TestUserAuthToken(t *testing.T) { token, err = ctx.getAuthTokenByID(token.Id) So(err, ShouldBeNil) - // set now (now - 23 hours) - _, err = userAuthTokenService.RefreshToken(token, "192.168.10.11:1234", "some user agent") + now = func() time.Time { + return t.Add(time.Hour) + } + + refreshed, err := userAuthTokenService.RefreshToken(token, "192.168.10.11:1234", "some user agent") So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) _, err = userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) @@ -69,7 +75,9 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(stillGood, ShouldNotBeNil) - // set now (new - 2 hours) + now = func() time.Time { + return t.Add(24 * 7 * time.Hour) + } notGood, err := userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldEqual, ErrAuthTokenNotFound) So(notGood, ShouldBeNil) @@ -93,7 +101,7 @@ func TestUserAuthToken(t *testing.T) { // ability to auth using an old token now = func() time.Time { - return t + return t.Add(time.Hour) } refreshed, err = userAuthTokenService.RefreshToken(token, "192.168.10.12:1234", "a new user agent") @@ -172,6 +180,7 @@ func createTestContext(t *testing.T) *testContext { RotateTime = 10 * time.Minute UrgentRotateTime = time.Minute + setting.LogInRememberDays = 7 return &testContext{ sqlstore: sqlstore, From dd8476d81ac9e6dbd74b6946d287cc218d1840da Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 15:06:33 +0100 Subject: [PATCH 20/49] passing auth token tests --- pkg/services/auth/auth_token.go | 20 +++++++++++--------- pkg/services/auth/auth_token_test.go | 16 +++++++++++----- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 0ab4e32c0ad..54e938ecfbe 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -3,6 +3,7 @@ package auth import ( "crypto/sha256" "encoding/hex" + "fmt" "net/http" "net/url" "time" @@ -22,8 +23,8 @@ func init() { var ( now = time.Now - RotateTime = 1 * time.Minute // this should be read from [session] configuration. - UrgentRotateTime = 30 * time.Second + RotateTime = 30 * time.Second + UrgentRotateTime = 10 * time.Second oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. ) @@ -154,17 +155,18 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu } if userToken.AuthToken != hashedToken && userToken.PrevAuthToken == hashedToken && userToken.AuthTokenSeen { - userToken.AuthTokenSeen = false - expireBefore := now().Add(-RotateTime).Unix() - affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND prev_auth_token = ? AND rotated_at < ?", userToken.Id, userToken.PrevAuthToken, expireBefore).AllCols().Update(&userToken) + userTokenCopy := userToken + userTokenCopy.AuthTokenSeen = false + expireBefore := now().Add(-UrgentRotateTime).Unix() + affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND prev_auth_token = ? AND rotated_at < ?", userTokenCopy.Id, userTokenCopy.PrevAuthToken, expireBefore).AllCols().Update(&userTokenCopy) if err != nil { return nil, err } if affectedRows == 0 { - s.log.Debug("prev seen token unchanged", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + fmt.Println("prev seen token unchanged", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } else { - s.log.Debug("prev seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + fmt.Println("prev seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } } @@ -182,9 +184,9 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu } if affectedRows == 0 { - s.log.Debug("seen wrong token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + fmt.Println("seen wrong token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } else { - s.log.Debug("seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + fmt.Println("seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index fa67cd62869..a92fb7e1598 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -95,11 +95,13 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(refreshed, ShouldBeFalse) - ctx.markAuthTokenAsSeen(token.Id) + updated, err := ctx.markAuthTokenAsSeen(token.Id) + So(err, ShouldBeNil) + So(updated, ShouldBeTrue) + token, err = ctx.getAuthTokenByID(token.Id) So(err, ShouldBeNil) - // ability to auth using an old token now = func() time.Time { return t.Add(time.Hour) } @@ -114,31 +116,35 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) token.UnhashedToken = unhashedToken - So(token.RotatedAt, ShouldEqual, t.Unix()) + So(token.RotatedAt, ShouldEqual, now().Unix()) So(token.ClientIp, ShouldEqual, "192.168.10.12") So(token.UserAgent, ShouldEqual, "a new user agent") So(token.AuthTokenSeen, ShouldBeFalse) So(token.SeenAt, ShouldEqual, 0) So(token.PrevAuthToken, ShouldEqual, prevToken) + // ability to auth using an old token + lookedUp, err := userAuthTokenService.LookupToken(token.UnhashedToken) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) So(lookedUp.AuthTokenSeen, ShouldBeTrue) - So(lookedUp.SeenAt, ShouldEqual, t.Unix()) + So(lookedUp.SeenAt, ShouldEqual, now().Unix()) lookedUp, err = userAuthTokenService.LookupToken(unhashedPrev) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) So(lookedUp.Id, ShouldEqual, token.Id) + So(lookedUp.AuthTokenSeen, ShouldBeTrue) now = func() time.Time { - return t.Add(2 * time.Minute) + return t.Add(time.Hour + (2 * time.Minute)) } lookedUp, err = userAuthTokenService.LookupToken(unhashedPrev) So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) + So(lookedUp.AuthTokenSeen, ShouldBeTrue) lookedUp, err = ctx.getAuthTokenByID(lookedUp.Id) So(err, ShouldBeNil) From 92620af75f988e2ae8e87aeba51afef8ec5be284 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 21 Jan 2019 15:30:08 +0100 Subject: [PATCH 21/49] avoid calling now() multiple times --- pkg/services/auth/auth_token.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 54e938ecfbe..a7a65b2aeca 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -22,7 +22,7 @@ func init() { } var ( - now = time.Now + getTime = time.Now RotateTime = 30 * time.Second UrgentRotateTime = 10 * time.Second oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. @@ -118,15 +118,17 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str hashedToken := hashToken(token) + now := getTime().Unix() + userToken := models.UserAuthToken{ UserId: userId, AuthToken: hashedToken, PrevAuthToken: hashedToken, ClientIp: clientIP, UserAgent: userAgent, - RotatedAt: now().Unix(), - CreatedAt: now().Unix(), - UpdatedAt: now().Unix(), + RotatedAt: now, + CreatedAt: now, + UpdatedAt: now, SeenAt: 0, AuthTokenSeen: false, } @@ -142,7 +144,7 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAuthToken, error) { hashedToken := hashToken(unhashedToken) - expireBefore := now().Add(time.Duration(-86400*setting.LogInRememberDays) * time.Second).Unix() + expireBefore := getTime().Add(time.Duration(-86400*setting.LogInRememberDays) * time.Second).Unix() var userToken models.UserAuthToken exists, err := s.SQLStore.NewSession().Where("(auth_token = ? OR prev_auth_token = ?) AND created_at > ?", hashedToken, hashedToken, expireBefore).Get(&userToken) @@ -157,7 +159,7 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu if userToken.AuthToken != hashedToken && userToken.PrevAuthToken == hashedToken && userToken.AuthTokenSeen { userTokenCopy := userToken userTokenCopy.AuthTokenSeen = false - expireBefore := now().Add(-UrgentRotateTime).Unix() + expireBefore := getTime().Add(-UrgentRotateTime).Unix() affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND prev_auth_token = ? AND rotated_at < ?", userTokenCopy.Id, userTokenCopy.PrevAuthToken, expireBefore).AllCols().Update(&userTokenCopy) if err != nil { return nil, err @@ -173,7 +175,7 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu if !userToken.AuthTokenSeen && userToken.AuthToken == hashedToken { userTokenCopy := userToken userTokenCopy.AuthTokenSeen = true - userTokenCopy.SeenAt = now().Unix() + userTokenCopy.SeenAt = getTime().Unix() affectedRows, err := s.SQLStore.NewSession().Where("id = ? AND auth_token = ?", userTokenCopy.Id, userTokenCopy.AuthToken).AllCols().Update(&userTokenCopy) if err != nil { return nil, err @@ -200,19 +202,22 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI return false, nil } + now := getTime() + needsRotation := false rotatedAt := time.Unix(token.RotatedAt, 0) if token.AuthTokenSeen { - needsRotation = rotatedAt.Before(now().Add(-RotateTime)) + needsRotation = rotatedAt.Before(now.Add(-RotateTime)) } else { - needsRotation = rotatedAt.Before(now().Add(-UrgentRotateTime)) + needsRotation = rotatedAt.Before(now.Add(-UrgentRotateTime)) } - s.log.Debug("refresh token", "needs rotation?", needsRotation, "auth_token_seen", token.AuthTokenSeen, "rotated_at", rotatedAt, "token.Id", token.Id) if !needsRotation { return false, nil } + s.log.Debug("refresh token needs rotation?", "auth_token_seen", token.AuthTokenSeen, "rotated_at", rotatedAt, "token.Id", token.Id) + clientIP = util.ParseIPAddress(clientIP) newToken, _ := util.RandomHex(16) hashedToken := hashToken(newToken) @@ -229,7 +234,7 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI rotated_at = ? WHERE id = ? AND (auth_token_seen or rotated_at < ?)` - res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, hashedToken, now().Unix(), token.Id, now().Add(-UrgentRotateTime)) + res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, hashedToken, now.Unix(), token.Id, now.Add(-UrgentRotateTime)) if err != nil { return false, err } From 38efc1d7d2c1620b638b4f64b720908a99b3c4cb Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 15:51:00 +0100 Subject: [PATCH 22/49] s/print/log --- pkg/services/auth/auth_token.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index a7a65b2aeca..1fb3543d354 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -3,7 +3,6 @@ package auth import ( "crypto/sha256" "encoding/hex" - "fmt" "net/http" "net/url" "time" @@ -166,9 +165,9 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu } if affectedRows == 0 { - fmt.Println("prev seen token unchanged", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + s.log.Debug("prev seen token unchanged", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } else { - fmt.Println("prev seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + s.log.Debug("prev seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } } @@ -186,9 +185,9 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu } if affectedRows == 0 { - fmt.Println("seen wrong token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + s.log.Debug("seen wrong token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } else { - fmt.Println("seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) + s.log.Debug("seen token", "userTokenId", userToken.Id, "userId", userToken.UserId, "authToken", userToken.AuthToken, "clientIP", userToken.ClientIp, "userAgent", userToken.UserAgent) } } From f040f9a4002a4631b525f1948ed2ec4682257d93 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 16:53:00 +0100 Subject: [PATCH 23/49] fix tests after renaming now --- pkg/services/auth/auth_token_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index a92fb7e1598..2e876350e7b 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -19,7 +19,7 @@ func TestUserAuthToken(t *testing.T) { userID := int64(10) t := time.Date(2018, 12, 13, 13, 45, 0, 0, time.UTC) - now = func() time.Time { + getTime = func() time.Time { return t } @@ -60,7 +60,7 @@ func TestUserAuthToken(t *testing.T) { token, err = ctx.getAuthTokenByID(token.Id) So(err, ShouldBeNil) - now = func() time.Time { + getTime = func() time.Time { return t.Add(time.Hour) } @@ -75,7 +75,7 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(stillGood, ShouldNotBeNil) - now = func() time.Time { + getTime = func() time.Time { return t.Add(24 * 7 * time.Hour) } notGood, err := userAuthTokenService.LookupToken(token.UnhashedToken) @@ -102,7 +102,7 @@ func TestUserAuthToken(t *testing.T) { token, err = ctx.getAuthTokenByID(token.Id) So(err, ShouldBeNil) - now = func() time.Time { + getTime = func() time.Time { return t.Add(time.Hour) } @@ -116,7 +116,7 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) token.UnhashedToken = unhashedToken - So(token.RotatedAt, ShouldEqual, now().Unix()) + So(token.RotatedAt, ShouldEqual, getTime().Unix()) So(token.ClientIp, ShouldEqual, "192.168.10.12") So(token.UserAgent, ShouldEqual, "a new user agent") So(token.AuthTokenSeen, ShouldBeFalse) @@ -129,7 +129,7 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldBeNil) So(lookedUp, ShouldNotBeNil) So(lookedUp.AuthTokenSeen, ShouldBeTrue) - So(lookedUp.SeenAt, ShouldEqual, now().Unix()) + So(lookedUp.SeenAt, ShouldEqual, getTime().Unix()) lookedUp, err = userAuthTokenService.LookupToken(unhashedPrev) So(err, ShouldBeNil) @@ -137,7 +137,7 @@ func TestUserAuthToken(t *testing.T) { So(lookedUp.Id, ShouldEqual, token.Id) So(lookedUp.AuthTokenSeen, ShouldBeTrue) - now = func() time.Time { + getTime = func() time.Time { return t.Add(time.Hour + (2 * time.Minute)) } @@ -170,7 +170,7 @@ func TestUserAuthToken(t *testing.T) { }) Reset(func() { - now = time.Now + getTime = time.Now }) }) } From 777bd9ea1845893739fac9549bc4cbb0cdcc913e Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 21 Jan 2019 17:05:42 +0100 Subject: [PATCH 24/49] adds cleanup job for old session tokens --- pkg/services/auth/auth_token.go | 8 +++-- pkg/services/auth/session_cleanup.go | 38 +++++++++++++++++++++++ pkg/services/auth/session_cleanup_test.go | 37 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 pkg/services/auth/session_cleanup.go create mode 100644 pkg/services/auth/session_cleanup_test.go diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 1fb3543d354..3e3bd75869d 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -8,6 +8,7 @@ import ( "time" "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/registry" @@ -29,8 +30,9 @@ var ( // UserAuthTokenService are used for generating and validating user auth tokens type UserAuthTokenService struct { - SQLStore *sqlstore.SqlStore `inject:""` - log log.Logger + SQLStore *sqlstore.SqlStore `inject:""` + ServerLockService *serverlock.ServerLockService `inject:""` + log log.Logger } // Init this service @@ -239,7 +241,7 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI } affected, _ := res.RowsAffected() - s.log.Debug("rotated", "affected", affected, "auth_token_id", token.Id, "userId", token.UserId, "user_agent", userAgent, "client_ip", clientIP) + s.log.Debug("rotated", "affected", affected, "auth_token_id", token.Id, "userId", token.UserId) if affected > 0 { token.UnhashedToken = newToken return true, nil diff --git a/pkg/services/auth/session_cleanup.go b/pkg/services/auth/session_cleanup.go new file mode 100644 index 00000000000..16f6deb9005 --- /dev/null +++ b/pkg/services/auth/session_cleanup.go @@ -0,0 +1,38 @@ +package auth + +import ( + "context" + "time" +) + +func (srv *UserAuthTokenService) Run(ctx context.Context) error { + ticker := time.NewTicker(time.Hour * 12) + deleteSessionAfter := time.Hour * 24 * 7 * 30 + + for { + select { + case <-ticker.C: + srv.ServerLockService.LockAndExecute(ctx, "delete old sessions", time.Hour*12, func() { + srv.deleteOldSession(deleteSessionAfter) + }) + + case <-ctx.Done(): + return ctx.Err() + } + } +} + +func (srv *UserAuthTokenService) deleteOldSession(deleteSessionAfter time.Duration) (int64, error) { + sql := `DELETE from user_auth_token WHERE rotated_at < ?` + + deleteBefore := getTime().Add(-deleteSessionAfter) + res, err := srv.SQLStore.NewSession().Exec(sql, deleteBefore.Unix()) + if err != nil { + return 0, err + } + + affected, err := res.RowsAffected() + srv.log.Info("deleted old sessions", "count", affected) + + return affected, err +} diff --git a/pkg/services/auth/session_cleanup_test.go b/pkg/services/auth/session_cleanup_test.go new file mode 100644 index 00000000000..1f3b7ad0c7d --- /dev/null +++ b/pkg/services/auth/session_cleanup_test.go @@ -0,0 +1,37 @@ +package auth + +import ( + "fmt" + "testing" + "time" + + "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" +) + +func TestUserAuthTokenCleanup(t *testing.T) { + + Convey("Test user auth token cleanup", t, func() { + ctx := createTestContext(t) + + insertToken := func(token string, prev string, rotatedAt int64) { + ut := models.UserAuthToken{AuthToken: token, PrevAuthToken: prev, RotatedAt: rotatedAt, UserAgent: "", ClientIp: ""} + _, err := ctx.sqlstore.NewSession().Insert(&ut) + So(err, ShouldBeNil) + } + + // insert three old tokens that should be deleted + for i := 0; i < 3; i++ { + insertToken(fmt.Sprintf("oldA%d", i), fmt.Sprintf("oldB%d", i), int64(i)) + } + + // insert three active tokens that should not be deleted + for i := 0; i < 3; i++ { + insertToken(fmt.Sprintf("newA%d", i), fmt.Sprintf("newB%d", i), getTime().Unix()) + } + + affected, err := ctx.tokenService.deleteOldSession(time.Hour) + So(err, ShouldBeNil) + So(affected, ShouldEqual, 3) + }) +} From 366e356e080e4ccaf6e7bf988ae449aceda3fc19 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 21 Jan 2019 19:08:51 +0100 Subject: [PATCH 25/49] more auth token tests --- pkg/services/auth/auth_token.go | 11 +-- pkg/services/auth/auth_token_test.go | 111 +++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 3e3bd75869d..181cc4315c9 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -223,19 +223,20 @@ func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientI newToken, _ := util.RandomHex(16) hashedToken := hashToken(newToken) + // very important that auth_token_seen is set after the prev_auth_token = case when ... for mysql to function correctly sql := ` UPDATE user_auth_token SET - auth_token_seen = false, - seen_at = null, + seen_at = 0, user_agent = ?, client_ip = ?, - prev_auth_token = case when auth_token_seen then auth_token else prev_auth_token end, + prev_auth_token = case when auth_token_seen = ? then auth_token else prev_auth_token end, auth_token = ?, + auth_token_seen = ?, rotated_at = ? - WHERE id = ? AND (auth_token_seen or rotated_at < ?)` + WHERE id = ? AND (auth_token_seen = ? OR rotated_at < ?)` - res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, hashedToken, now.Unix(), token.Id, now.Add(-UrgentRotateTime)) + res, err := s.SQLStore.NewSession().Exec(sql, userAgent, clientIP, s.SQLStore.Dialect.BooleanStr(true), hashedToken, s.SQLStore.Dialect.BooleanStr(false), now.Unix(), token.Id, s.SQLStore.Dialect.BooleanStr(true), now.Add(-30*time.Second).Unix()) if err != nil { return false, err } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 2e876350e7b..27405059e26 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -162,11 +162,122 @@ func TestUserAuthToken(t *testing.T) { }) Convey("keeps prev token valid for 1 minute after it is confirmed", func() { + token, err := userAuthTokenService.CreateToken(userID, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + lookedUp, err := userAuthTokenService.LookupToken(token.UnhashedToken) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + + getTime = func() time.Time { + return t.Add(10 * time.Minute) + } + + prevToken := token.UnhashedToken + refreshed, err := userAuthTokenService.RefreshToken(token, "1.1.1.1", "firefox") + So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) + + getTime = func() time.Time { + return t.Add(20 * time.Minute) + } + + current, err := userAuthTokenService.LookupToken(token.UnhashedToken) + So(err, ShouldBeNil) + So(current, ShouldNotBeNil) + + prev, err := userAuthTokenService.LookupToken(prevToken) + So(err, ShouldBeNil) + So(prev, ShouldNotBeNil) }) Convey("will not mark token unseen when prev and current are the same", func() { + token, err := userAuthTokenService.CreateToken(userID, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + lookedUp, err := userAuthTokenService.LookupToken(token.UnhashedToken) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + + lookedUp, err = userAuthTokenService.LookupToken(token.UnhashedToken) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + + lookedUp, err = ctx.getAuthTokenByID(lookedUp.Id) + So(err, ShouldBeNil) + So(lookedUp, ShouldNotBeNil) + So(lookedUp.AuthTokenSeen, ShouldBeTrue) + }) + + Convey("Rotate token", func() { + token, err := userAuthTokenService.CreateToken(userID, "192.168.10.11:1234", "some user agent") + So(err, ShouldBeNil) + So(token, ShouldNotBeNil) + + prevToken := token.AuthToken + + Convey("Should rotate current token and previous token when auth token seen", func() { + updated, err := ctx.markAuthTokenAsSeen(token.Id) + So(err, ShouldBeNil) + So(updated, ShouldBeTrue) + + getTime = func() time.Time { + return t.Add(10 * time.Minute) + } + + refreshed, err := userAuthTokenService.RefreshToken(token, "1.1.1.1", "firefox") + So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) + + storedToken, err := ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + So(storedToken, ShouldNotBeNil) + So(storedToken.AuthTokenSeen, ShouldBeFalse) + So(storedToken.PrevAuthToken, ShouldEqual, prevToken) + So(storedToken.AuthToken, ShouldNotEqual, prevToken) + + prevToken = storedToken.AuthToken + + updated, err = ctx.markAuthTokenAsSeen(token.Id) + So(err, ShouldBeNil) + So(updated, ShouldBeTrue) + + getTime = func() time.Time { + return t.Add(20 * time.Minute) + } + + refreshed, err = userAuthTokenService.RefreshToken(token, "1.1.1.1", "firefox") + So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) + + storedToken, err = ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + So(storedToken, ShouldNotBeNil) + So(storedToken.AuthTokenSeen, ShouldBeFalse) + So(storedToken.PrevAuthToken, ShouldEqual, prevToken) + So(storedToken.AuthToken, ShouldNotEqual, prevToken) + }) + + Convey("Should rotate current token, but keep previous token when auth token not seen", func() { + token.RotatedAt = getTime().Add(-2 * time.Minute).Unix() + + getTime = func() time.Time { + return t.Add(2 * time.Minute) + } + + refreshed, err := userAuthTokenService.RefreshToken(token, "1.1.1.1", "firefox") + So(err, ShouldBeNil) + So(refreshed, ShouldBeTrue) + + storedToken, err := ctx.getAuthTokenByID(token.Id) + So(err, ShouldBeNil) + So(storedToken, ShouldNotBeNil) + So(storedToken.AuthTokenSeen, ShouldBeFalse) + So(storedToken.PrevAuthToken, ShouldEqual, prevToken) + So(storedToken.AuthToken, ShouldNotEqual, prevToken) + }) }) Reset(func() { From 4096449aecdf7950ad4c120db5e7c10313f482cb Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 22 Jan 2019 12:00:33 +0100 Subject: [PATCH 26/49] extract auth token interface and remove auth token from context --- pkg/api/http_server.go | 16 ++++---- pkg/middleware/middleware.go | 2 +- pkg/models/context.go | 16 -------- pkg/services/auth/auth_token.go | 49 ++++++++++++----------- pkg/services/auth/auth_token_test.go | 9 ++--- pkg/services/auth/model.go | 28 ++++++------- pkg/services/auth/session_cleanup.go | 4 +- pkg/services/auth/session_cleanup_test.go | 3 +- 8 files changed, 56 insertions(+), 71 deletions(-) diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 54f0601e577..c85bdb6f2e7 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -47,14 +47,14 @@ type HTTPServer struct { streamManager *live.StreamManager httpSrv *http.Server - RouteRegister routing.RouteRegister `inject:""` - Bus bus.Bus `inject:""` - RenderService rendering.Service `inject:""` - Cfg *setting.Cfg `inject:""` - HooksService *hooks.HooksService `inject:""` - CacheService *cache.CacheService `inject:""` - DatasourceCache datasources.CacheService `inject:""` - AuthTokenService *auth.UserAuthTokenService `inject:""` + RouteRegister routing.RouteRegister `inject:""` + Bus bus.Bus `inject:""` + RenderService rendering.Service `inject:""` + Cfg *setting.Cfg `inject:""` + HooksService *hooks.HooksService `inject:""` + CacheService *cache.CacheService `inject:""` + DatasourceCache datasources.CacheService `inject:""` + AuthTokenService auth.UserAuthTokenService `inject:""` } func (hs *HTTPServer) Init() error { diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 6c4ce1c20ae..705d28db3eb 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -21,7 +21,7 @@ var ( ReqOrgAdmin = RoleAuth(m.ROLE_ADMIN) ) -func GetContextHandler(ats *auth.UserAuthTokenService) macaron.Handler { +func GetContextHandler(ats auth.UserAuthTokenService) macaron.Handler { return func(c *macaron.Context) { ctx := &m.ReqContext{ Context: c, diff --git a/pkg/models/context.go b/pkg/models/context.go index f6df8b1c2f0..1a78c021f45 100644 --- a/pkg/models/context.go +++ b/pkg/models/context.go @@ -10,25 +10,9 @@ import ( "gopkg.in/macaron.v1" ) -type UserAuthToken struct { - Id int64 - UserId int64 - AuthToken string - PrevAuthToken string - UserAgent string - ClientIp string - AuthTokenSeen bool - SeenAt int64 - RotatedAt int64 - CreatedAt int64 - UpdatedAt int64 - UnhashedToken string `xorm:"-"` -} - type ReqContext struct { *macaron.Context *SignedInUser - UserToken *UserAuthToken Session session.SessionStore diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 181cc4315c9..0b5af181b8b 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -18,67 +18,72 @@ import ( ) func init() { - registry.RegisterService(&UserAuthTokenService{}) + registry.RegisterService(&UserAuthTokenServiceImpl{}) } var ( getTime = time.Now - RotateTime = 30 * time.Second - UrgentRotateTime = 10 * time.Second + RotateTime = 2 * time.Minute + UrgentRotateTime = 20 * time.Second oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. ) // UserAuthTokenService are used for generating and validating user auth tokens -type UserAuthTokenService struct { +type UserAuthTokenService interface { + InitContextWithToken(ctx *models.ReqContext, orgID int64) bool + UserAuthenticatedHook(user *models.User, c *models.ReqContext) error + UserSignedOutHook(c *models.ReqContext) +} + +type UserAuthTokenServiceImpl struct { SQLStore *sqlstore.SqlStore `inject:""` ServerLockService *serverlock.ServerLockService `inject:""` log log.Logger } // Init this service -func (s *UserAuthTokenService) Init() error { +func (s *UserAuthTokenServiceImpl) Init() error { s.log = log.New("auth") return nil } -func (s *UserAuthTokenService) InitContextWithToken(ctx *models.ReqContext, orgID int64) bool { +func (s *UserAuthTokenServiceImpl) InitContextWithToken(ctx *models.ReqContext, orgID int64) bool { //auth User unhashedToken := ctx.GetCookie(setting.SessionOptions.CookieName) if unhashedToken == "" { return false } - user, err := s.LookupToken(unhashedToken) + userToken, err := s.LookupToken(unhashedToken) if err != nil { ctx.Logger.Info("failed to look up user based on cookie", "error", err) return false } - query := models.GetSignedInUserQuery{UserId: user.UserId, OrgId: orgID} + query := models.GetSignedInUserQuery{UserId: userToken.UserId, OrgId: orgID} if err := bus.Dispatch(&query); err != nil { - ctx.Logger.Error("Failed to get user with id", "userId", user.UserId, "error", err) + ctx.Logger.Error("Failed to get user with id", "userId", userToken.UserId, "error", err) return false } ctx.SignedInUser = query.Result ctx.IsSignedIn = true - ctx.UserToken = user //rotate session token if needed. - rotated, err := s.RefreshToken(ctx.UserToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) + rotated, err := s.RefreshToken(userToken, ctx.RemoteAddr(), ctx.Req.UserAgent()) if err != nil { - ctx.Logger.Error("failed to rotate token", "error", err, "user.id", user.UserId, "user_token.id", user.Id) + ctx.Logger.Error("failed to rotate token", "error", err, "userId", userToken.UserId, "tokenId", userToken.Id) return true } if rotated { - s.writeSessionCookie(ctx, ctx.UserToken.UnhashedToken, oneYearInSeconds) + s.writeSessionCookie(ctx, userToken.UnhashedToken, oneYearInSeconds) } return true } -func (s *UserAuthTokenService) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { +func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { ctx.Logger.Info("new token", "unhashed token", value) ctx.Resp.Header().Del("Set-Cookie") @@ -94,23 +99,21 @@ func (s *UserAuthTokenService) writeSessionCookie(ctx *models.ReqContext, value http.SetCookie(ctx.Resp, &cookie) } -func (s *UserAuthTokenService) UserAuthenticatedHook(user *models.User, c *models.ReqContext) error { +func (s *UserAuthTokenServiceImpl) UserAuthenticatedHook(user *models.User, c *models.ReqContext) error { userToken, err := s.CreateToken(user.Id, c.RemoteAddr(), c.Req.UserAgent()) if err != nil { return err } - c.UserToken = userToken - s.writeSessionCookie(c, userToken.UnhashedToken, oneYearInSeconds) return nil } -func (s *UserAuthTokenService) UserSignedOutHook(c *models.ReqContext) { +func (s *UserAuthTokenServiceImpl) UserSignedOutHook(c *models.ReqContext) { s.writeSessionCookie(c, "", -1) } -func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent string) (*models.UserAuthToken, error) { +func (s *UserAuthTokenServiceImpl) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) { clientIP = util.ParseIPAddress(clientIP) token, err := util.RandomHex(16) if err != nil { @@ -121,7 +124,7 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str now := getTime().Unix() - userToken := models.UserAuthToken{ + userToken := userAuthToken{ UserId: userId, AuthToken: hashedToken, PrevAuthToken: hashedToken, @@ -143,11 +146,11 @@ func (s *UserAuthTokenService) CreateToken(userId int64, clientIP, userAgent str return &userToken, nil } -func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAuthToken, error) { +func (s *UserAuthTokenServiceImpl) LookupToken(unhashedToken string) (*userAuthToken, error) { hashedToken := hashToken(unhashedToken) expireBefore := getTime().Add(time.Duration(-86400*setting.LogInRememberDays) * time.Second).Unix() - var userToken models.UserAuthToken + var userToken userAuthToken exists, err := s.SQLStore.NewSession().Where("(auth_token = ? OR prev_auth_token = ?) AND created_at > ?", hashedToken, hashedToken, expireBefore).Get(&userToken) if err != nil { return nil, err @@ -198,7 +201,7 @@ func (s *UserAuthTokenService) LookupToken(unhashedToken string) (*models.UserAu return &userToken, nil } -func (s *UserAuthTokenService) RefreshToken(token *models.UserAuthToken, clientIP, userAgent string) (bool, error) { +func (s *UserAuthTokenServiceImpl) RefreshToken(token *userAuthToken, clientIP, userAgent string) (bool, error) { if token == nil { return false, nil } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 27405059e26..ee9bdf60bb9 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -7,7 +7,6 @@ import ( "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/log" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" . "github.com/smartystreets/goconvey/convey" ) @@ -290,7 +289,7 @@ func createTestContext(t *testing.T) *testContext { t.Helper() sqlstore := sqlstore.InitTestDB(t) - tokenService := &UserAuthTokenService{ + tokenService := &UserAuthTokenServiceImpl{ SQLStore: sqlstore, log: log.New("test-logger"), } @@ -307,12 +306,12 @@ func createTestContext(t *testing.T) *testContext { type testContext struct { sqlstore *sqlstore.SqlStore - tokenService *UserAuthTokenService + tokenService *UserAuthTokenServiceImpl } -func (c *testContext) getAuthTokenByID(id int64) (*models.UserAuthToken, error) { +func (c *testContext) getAuthTokenByID(id int64) (*userAuthToken, error) { sess := c.sqlstore.NewSession() - var t models.UserAuthToken + var t userAuthToken found, err := sess.ID(id).Get(&t) if err != nil || !found { return nil, err diff --git a/pkg/services/auth/model.go b/pkg/services/auth/model.go index 4347f6d2d6a..7a0f49539f2 100644 --- a/pkg/services/auth/model.go +++ b/pkg/services/auth/model.go @@ -9,17 +9,17 @@ var ( ErrAuthTokenNotFound = errors.New("User auth token not found") ) -// type userAuthToken struct { -// Id int64 -// UserId int64 -// AuthToken string -// PrevAuthToken string -// UserAgent string -// ClientIp string -// AuthTokenSeen bool -// SeenAt int64 -// RotatedAt int64 -// CreatedAt int64 -// UpdatedAt int64 -// unhashedToken string `xorm:"-"` -// } +type userAuthToken struct { + Id int64 + UserId int64 + AuthToken string + PrevAuthToken string + UserAgent string + ClientIp string + AuthTokenSeen bool + SeenAt int64 + RotatedAt int64 + CreatedAt int64 + UpdatedAt int64 + UnhashedToken string `xorm:"-"` +} diff --git a/pkg/services/auth/session_cleanup.go b/pkg/services/auth/session_cleanup.go index 16f6deb9005..64dee43d677 100644 --- a/pkg/services/auth/session_cleanup.go +++ b/pkg/services/auth/session_cleanup.go @@ -5,7 +5,7 @@ import ( "time" ) -func (srv *UserAuthTokenService) Run(ctx context.Context) error { +func (srv *UserAuthTokenServiceImpl) Run(ctx context.Context) error { ticker := time.NewTicker(time.Hour * 12) deleteSessionAfter := time.Hour * 24 * 7 * 30 @@ -22,7 +22,7 @@ func (srv *UserAuthTokenService) Run(ctx context.Context) error { } } -func (srv *UserAuthTokenService) deleteOldSession(deleteSessionAfter time.Duration) (int64, error) { +func (srv *UserAuthTokenServiceImpl) deleteOldSession(deleteSessionAfter time.Duration) (int64, error) { sql := `DELETE from user_auth_token WHERE rotated_at < ?` deleteBefore := getTime().Add(-deleteSessionAfter) diff --git a/pkg/services/auth/session_cleanup_test.go b/pkg/services/auth/session_cleanup_test.go index 1f3b7ad0c7d..eef2cd74d04 100644 --- a/pkg/services/auth/session_cleanup_test.go +++ b/pkg/services/auth/session_cleanup_test.go @@ -5,7 +5,6 @@ import ( "testing" "time" - "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" ) @@ -15,7 +14,7 @@ func TestUserAuthTokenCleanup(t *testing.T) { ctx := createTestContext(t) insertToken := func(token string, prev string, rotatedAt int64) { - ut := models.UserAuthToken{AuthToken: token, PrevAuthToken: prev, RotatedAt: rotatedAt, UserAgent: "", ClientIp: ""} + ut := userAuthToken{AuthToken: token, PrevAuthToken: prev, RotatedAt: rotatedAt, UserAgent: "", ClientIp: ""} _, err := ctx.sqlstore.NewSession().Insert(&ut) So(err, ShouldBeNil) } From 59d0c19ba8ef3c136188d9c8aa8fbf87639dc46b Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 22 Jan 2019 13:51:55 +0100 Subject: [PATCH 27/49] passing middleware tests --- pkg/api/common_test.go | 40 +++++++++++++---- pkg/middleware/middleware_test.go | 70 +++++++++++++++++------------ pkg/middleware/org_redirect_test.go | 24 +++++----- pkg/middleware/quota_test.go | 13 +++--- pkg/middleware/recovery_test.go | 3 +- 5 files changed, 92 insertions(+), 58 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index f99902aac51..3be4cd38448 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -95,13 +95,14 @@ func (sc *scenarioContext) fakeReqWithParams(method, url string, queryParams map } type scenarioContext struct { - m *macaron.Macaron - context *m.ReqContext - resp *httptest.ResponseRecorder - handlerFunc handlerFunc - defaultHandler macaron.Handler - req *http.Request - url string + m *macaron.Macaron + context *m.ReqContext + resp *httptest.ResponseRecorder + handlerFunc handlerFunc + defaultHandler macaron.Handler + req *http.Request + url string + userAuthTokenService *fakeUserAuthTokenService } func (sc *scenarioContext) exec() { @@ -123,8 +124,31 @@ func setupScenarioContext(url string) *scenarioContext { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) - sc.m.Use(middleware.GetContextHandler(nil)) + sc.userAuthTokenService = newFakeUserAuthTokenService() + sc.m.Use(middleware.GetContextHandler(sc.userAuthTokenService)) sc.m.Use(middleware.Sessioner(&session.Options{}, 0)) return sc } + +type fakeUserAuthTokenService struct { + initContextWithTokenProvider func(ctx *m.ReqContext, orgID int64) bool +} + +func newFakeUserAuthTokenService() *fakeUserAuthTokenService { + return &fakeUserAuthTokenService{ + initContextWithTokenProvider: func(ctx *m.ReqContext, orgID int64) bool { + return false + }, + } +} + +func (s *fakeUserAuthTokenService) InitContextWithToken(ctx *m.ReqContext, orgID int64) bool { + return s.initContextWithTokenProvider(ctx, orgID) +} + +func (s *fakeUserAuthTokenService) UserAuthenticatedHook(user *m.User, c *m.ReqContext) error { + return nil +} + +func (s *fakeUserAuthTokenService) UserSignedOutHook(c *m.ReqContext) {} diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 73c84af09fd..469b03c1e72 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -43,11 +43,6 @@ func TestMiddlewareContext(t *testing.T) { So(sc.resp.Header().Get("Cache-Control"), ShouldBeEmpty) }) - middlewareScenario("Non api request should init session", func(sc *scenarioContext) { - sc.fakeReq("GET", "/").exec() - So(sc.resp.Header().Get("Set-Cookie"), ShouldContainSubstring, "grafana_sess") - }) - middlewareScenario("Invalid api key", func(sc *scenarioContext) { sc.apiKey = "invalid_key_test" sc.fakeReq("GET", "/").exec() @@ -151,22 +146,17 @@ func TestMiddlewareContext(t *testing.T) { }) }) - middlewareScenario("UserId in session", func(sc *scenarioContext) { - - sc.fakeReq("GET", "/").handler(func(c *m.ReqContext) { - c.Session.Set(session.SESS_KEY_USERID, int64(12)) - }).exec() - - bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { - query.Result = &m.SignedInUser{OrgId: 2, UserId: 12} - return nil - }) + middlewareScenario("Auth token service", func(sc *scenarioContext) { + var wasCalled bool + sc.userAuthTokenService.initContextWithTokenProvider = func(ctx *m.ReqContext, orgId int64) bool { + wasCalled = true + return false + } sc.fakeReq("GET", "/").exec() - Convey("should init context with user info", func() { - So(sc.context.IsSignedIn, ShouldBeTrue) - So(sc.context.UserId, ShouldEqual, 12) + Convey("should call middleware", func() { + So(wasCalled, ShouldBeTrue) }) }) @@ -487,7 +477,8 @@ func middlewareScenario(desc string, fn scenarioFunc) { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) - sc.m.Use(GetContextHandler(nil)) + sc.userAuthTokenService = newFakeUserAuthTokenService() + sc.m.Use(GetContextHandler(sc.userAuthTokenService)) // mock out gc goroutine session.StartSessionGC = func() {} sc.m.Use(Sessioner(&ms.Options{}, 0)) @@ -508,15 +499,16 @@ func middlewareScenario(desc string, fn scenarioFunc) { } type scenarioContext struct { - m *macaron.Macaron - context *m.ReqContext - resp *httptest.ResponseRecorder - apiKey string - authHeader string - respJson map[string]interface{} - handlerFunc handlerFunc - defaultHandler macaron.Handler - url string + m *macaron.Macaron + context *m.ReqContext + resp *httptest.ResponseRecorder + apiKey string + authHeader string + respJson map[string]interface{} + handlerFunc handlerFunc + defaultHandler macaron.Handler + url string + userAuthTokenService *fakeUserAuthTokenService req *http.Request } @@ -585,3 +577,25 @@ func (sc *scenarioContext) exec() { type scenarioFunc func(c *scenarioContext) type handlerFunc func(c *m.ReqContext) + +type fakeUserAuthTokenService struct { + initContextWithTokenProvider func(ctx *m.ReqContext, orgID int64) bool +} + +func newFakeUserAuthTokenService() *fakeUserAuthTokenService { + return &fakeUserAuthTokenService{ + initContextWithTokenProvider: func(ctx *m.ReqContext, orgID int64) bool { + return false + }, + } +} + +func (s *fakeUserAuthTokenService) InitContextWithToken(ctx *m.ReqContext, orgID int64) bool { + return s.initContextWithTokenProvider(ctx, orgID) +} + +func (s *fakeUserAuthTokenService) UserAuthenticatedHook(user *m.User, c *m.ReqContext) error { + return nil +} + +func (s *fakeUserAuthTokenService) UserSignedOutHook(c *m.ReqContext) {} diff --git a/pkg/middleware/org_redirect_test.go b/pkg/middleware/org_redirect_test.go index fa08154b250..46b8776fdcc 100644 --- a/pkg/middleware/org_redirect_test.go +++ b/pkg/middleware/org_redirect_test.go @@ -7,7 +7,6 @@ import ( "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/session" . "github.com/smartystreets/goconvey/convey" ) @@ -15,18 +14,15 @@ func TestOrgRedirectMiddleware(t *testing.T) { Convey("Can redirect to correct org", t, func() { middlewareScenario("when setting a correct org for the user", func(sc *scenarioContext) { - sc.fakeReq("GET", "/").handler(func(c *m.ReqContext) { - c.Session.Set(session.SESS_KEY_USERID, int64(12)) - }).exec() - bus.AddHandler("test", func(query *m.SetUsingOrgCommand) error { return nil }) - bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { - query.Result = &m.SignedInUser{OrgId: 1, UserId: 12} - return nil - }) + sc.userAuthTokenService.initContextWithTokenProvider = func(ctx *m.ReqContext, orgId int64) bool { + ctx.SignedInUser = &m.SignedInUser{OrgId: 1, UserId: 12} + ctx.IsSignedIn = true + return true + } sc.m.Get("/", sc.defaultHandler) sc.fakeReq("GET", "/?orgId=3").exec() @@ -37,14 +33,16 @@ func TestOrgRedirectMiddleware(t *testing.T) { }) middlewareScenario("when setting an invalid org for user", func(sc *scenarioContext) { - sc.fakeReq("GET", "/").handler(func(c *m.ReqContext) { - c.Session.Set(session.SESS_KEY_USERID, int64(12)) - }).exec() - bus.AddHandler("test", func(query *m.SetUsingOrgCommand) error { return fmt.Errorf("") }) + sc.userAuthTokenService.initContextWithTokenProvider = func(ctx *m.ReqContext, orgId int64) bool { + ctx.SignedInUser = &m.SignedInUser{OrgId: 1, UserId: 12} + ctx.IsSignedIn = true + return true + } + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { query.Result = &m.SignedInUser{OrgId: 1, UserId: 12} return nil diff --git a/pkg/middleware/quota_test.go b/pkg/middleware/quota_test.go index 92c3d62674d..4f2203a5d3d 100644 --- a/pkg/middleware/quota_test.go +++ b/pkg/middleware/quota_test.go @@ -74,15 +74,12 @@ func TestMiddlewareQuota(t *testing.T) { }) middlewareScenario("with user logged in", func(sc *scenarioContext) { - // log us in, so we have a user_id and org_id in the context - sc.fakeReq("GET", "/").handler(func(c *m.ReqContext) { - c.Session.Set(session.SESS_KEY_USERID, int64(12)) - }).exec() + sc.userAuthTokenService.initContextWithTokenProvider = func(ctx *m.ReqContext, orgId int64) bool { + ctx.SignedInUser = &m.SignedInUser{OrgId: 2, UserId: 12} + ctx.IsSignedIn = true + return true + } - bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { - query.Result = &m.SignedInUser{OrgId: 2, UserId: 12} - return nil - }) bus.AddHandler("globalQuota", func(query *m.GetGlobalQuotaByTargetQuery) error { query.Result = &m.GlobalQuotaDTO{ Target: query.Target, diff --git a/pkg/middleware/recovery_test.go b/pkg/middleware/recovery_test.go index 5e70fffc45e..eb76f186f49 100644 --- a/pkg/middleware/recovery_test.go +++ b/pkg/middleware/recovery_test.go @@ -64,7 +64,8 @@ func recoveryScenario(desc string, url string, fn scenarioFunc) { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) - sc.m.Use(GetContextHandler(nil)) + sc.userAuthTokenService = newFakeUserAuthTokenService() + sc.m.Use(GetContextHandler(sc.userAuthTokenService)) // mock out gc goroutine session.StartSessionGC = func() {} sc.m.Use(Sessioner(&ms.Options{}, 0)) From d3ec8e1ccb64f3ea4605599eacc0df915590f20f Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 22 Jan 2019 14:06:44 +0100 Subject: [PATCH 28/49] creates new config section for login settings --- conf/defaults.ini | 17 +++++++++++++++++ pkg/services/auth/auth_token.go | 18 +++++++++++++----- pkg/setting/setting.go | 14 +++++++++++++- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 7f61ac96870..730246d0ede 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -106,6 +106,22 @@ path = grafana.db # For "sqlite3" only. cache mode setting used for connecting to the database cache_mode = private +#################################### Login ############################### + +[login] + +# login cookie name +cookie_name = grafana_session + +# If you want login cookies to be https only. default is false +cookie_secure = false + +# logged in user name +cookie_username = grafana_user + +# how many days an session can be unused before we inactivate it +login_remember_days = 7 + #################################### Session ############################# [session] # Either "memory", "file", "redis", "mysql", "postgres", "memcache", default is "file" @@ -124,6 +140,7 @@ provider = file provider_config = sessions + # Session cookie name cookie_name = grafana_sess diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 0b5af181b8b..b2d9ab09200 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -38,6 +38,7 @@ type UserAuthTokenService interface { type UserAuthTokenServiceImpl struct { SQLStore *sqlstore.SqlStore `inject:""` ServerLockService *serverlock.ServerLockService `inject:""` + Cfg *setting.Cfg `inject:""` log log.Logger } @@ -49,7 +50,7 @@ func (s *UserAuthTokenServiceImpl) Init() error { func (s *UserAuthTokenServiceImpl) InitContextWithToken(ctx *models.ReqContext, orgID int64) bool { //auth User - unhashedToken := ctx.GetCookie(setting.SessionOptions.CookieName) + unhashedToken := ctx.GetCookie(s.Cfg.LoginCookieName) if unhashedToken == "" { return false } @@ -84,16 +85,19 @@ func (s *UserAuthTokenServiceImpl) InitContextWithToken(ctx *models.ReqContext, } func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { - ctx.Logger.Info("new token", "unhashed token", value) + if setting.Env == setting.DEV { + ctx.Logger.Info("new token", "unhashed token", value, "cookieName", s.Cfg.LoginCookieName, "secure", s.Cfg.LoginCookieSecure) + } ctx.Resp.Header().Del("Set-Cookie") cookie := http.Cookie{ - Name: setting.SessionOptions.CookieName, + Name: s.Cfg.LoginCookieName, Value: url.QueryEscape(value), HttpOnly: true, Domain: setting.Domain, Path: setting.AppSubUrl + "/", - Secure: setting.SessionOptions.Secure, + Secure: s.Cfg.LoginCookieSecure, + MaxAge: maxAge, } http.SetCookie(ctx.Resp, &cookie) @@ -148,7 +152,11 @@ func (s *UserAuthTokenServiceImpl) CreateToken(userId int64, clientIP, userAgent func (s *UserAuthTokenServiceImpl) LookupToken(unhashedToken string) (*userAuthToken, error) { hashedToken := hashToken(unhashedToken) - expireBefore := getTime().Add(time.Duration(-86400*setting.LogInRememberDays) * time.Second).Unix() + if setting.Env == setting.DEV { + s.log.Info("looking up token", "unhashed", unhashedToken, "hashed", hashedToken) + } + + expireBefore := getTime().Add(time.Duration(-86400*s.Cfg.LoginCookieMaxDays) * time.Second).Unix() var userToken userAuthToken exists, err := s.SQLStore.NewSession().Where("(auth_token = ? OR prev_auth_token = ?) AND created_at > ?", hashedToken, hashedToken, expireBefore).Get(&userToken) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 1417392fdf8..daef13e7983 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -18,7 +18,7 @@ import ( "github.com/go-macaron/session" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/util" - "gopkg.in/ini.v1" + ini "gopkg.in/ini.v1" ) type Scheme string @@ -223,6 +223,11 @@ type Cfg struct { MetricsEndpointBasicAuthPassword string EnableAlphaPanels bool EnterpriseLicensePath string + + LoginCookieName string + LoginCookieUsername string + LoginCookieSecure bool + LoginCookieMaxDays int } type CommandLineArgs struct { @@ -546,6 +551,13 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { ApplicationName = APP_NAME_ENTERPRISE } + //login + login := iniFile.Section("login") + cfg.LoginCookieName = login.Key("cookie_name").String() + cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt() + cfg.LoginCookieSecure = login.Key("cookie_secure").MustBool(false) + cfg.LoginCookieUsername = login.Key("cookie_username").String() + Env = iniFile.Section("").Key("app_mode").MustString("development") InstanceName = iniFile.Section("").Key("instance_name").MustString("unknown_instance_name") PluginsPath = makeAbsolute(iniFile.Section("paths").Key("plugins").String(), HomePath) From 12f8338977a32f53d9979d7831f5c80619e8509a Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 22 Jan 2019 15:20:44 +0100 Subject: [PATCH 29/49] stores hashed state code in cookie --- pkg/api/login_oauth.go | 39 ++++++++++++++++++++++++++++----- pkg/services/auth/auth_token.go | 2 +- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 6013df8ea02..36c9c12905f 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -3,9 +3,11 @@ package api import ( "context" "crypto/rand" + "crypto/sha256" "crypto/tls" "crypto/x509" "encoding/base64" + "encoding/hex" "fmt" "io/ioutil" "net/http" @@ -18,12 +20,14 @@ import ( "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/metrics" m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/social" ) -var oauthLogger = log.New("oauth") +var ( + oauthLogger = log.New("oauth") + OauthStateCookieName = "oauth_state" +) func GenStateString() string { rnd := make([]byte, 32) @@ -55,7 +59,9 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { code := ctx.Query("code") if code == "" { state := GenStateString() - ctx.Session.Set(session.SESS_KEY_OAUTH_STATE, state) + hashedState := hashStatecode(state, setting.OAuthService.OAuthInfos[name].ClientSecret) + hs.writeOauthStateCookie(ctx, hashedState, 60) + if setting.OAuthService.OAuthInfos[name].HostedDomain == "" { ctx.Redirect(connect.AuthCodeURL(state, oauth2.AccessTypeOnline)) } else { @@ -64,13 +70,18 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { return } - savedState, ok := ctx.Session.Get(session.SESS_KEY_OAUTH_STATE).(string) - if !ok { + savedState := ctx.GetCookie(OauthStateCookieName) + + // delete cookie + ctx.Resp.Header().Del("Set-Cookie") + hs.writeOauthStateCookie(ctx, "", -1) + + if savedState == "" { ctx.Handle(500, "login.OAuthLogin(missing saved state)", nil) return } - queryState := ctx.Query("state") + queryState := hashStatecode(ctx.Query("state"), setting.OAuthService.OAuthInfos[name].ClientSecret) if savedState != queryState { ctx.Handle(500, "login.OAuthLogin(state mismatch)", nil) return @@ -191,6 +202,22 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { ctx.Redirect(setting.AppSubUrl + "/") } +func (hs *HTTPServer) writeOauthStateCookie(ctx *m.ReqContext, value string, maxAge int) { + http.SetCookie(ctx.Resp, &http.Cookie{ + Name: OauthStateCookieName, + MaxAge: maxAge, + Value: value, + HttpOnly: true, + Path: setting.AppSubUrl + "/", + Secure: hs.Cfg.LoginCookieSecure, + }) +} + +func hashStatecode(code, seed string) string { + hashBytes := sha256.Sum256([]byte(code + setting.SecretKey + seed)) + return hex.EncodeToString(hashBytes[:]) +} + func redirectWithError(ctx *m.ReqContext, err error, v ...interface{}) { ctx.Logger.Error(err.Error(), v...) ctx.Session.Set("loginError", err.Error()) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index b2d9ab09200..c04389ab557 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -86,7 +86,7 @@ func (s *UserAuthTokenServiceImpl) InitContextWithToken(ctx *models.ReqContext, func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { if setting.Env == setting.DEV { - ctx.Logger.Info("new token", "unhashed token", value, "cookieName", s.Cfg.LoginCookieName, "secure", s.Cfg.LoginCookieSecure) + ctx.Logger.Info("new token", "unhashed token", value) } ctx.Resp.Header().Del("Set-Cookie") From 64124b5042f829d21ec82833844cac9b37fe5ef9 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 22 Jan 2019 15:31:43 +0100 Subject: [PATCH 30/49] add setting for how to long we should keep expired tokens --- conf/defaults.ini | 9 ++++++--- pkg/services/auth/session_cleanup.go | 2 +- pkg/setting/setting.go | 16 +++++++++------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 730246d0ede..e0b087f437a 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -110,18 +110,21 @@ cache_mode = private [login] -# login cookie name +# Login cookie name cookie_name = grafana_session # If you want login cookies to be https only. default is false cookie_secure = false -# logged in user name +# Logged in user name cookie_username = grafana_user -# how many days an session can be unused before we inactivate it +# How many days an session can be unused before we inactivate it login_remember_days = 7 +# How long should Grafana keep expired tokens before deleting them +delete_expired_token_after_days = 30 + #################################### Session ############################# [session] # Either "memory", "file", "redis", "mysql", "postgres", "memcache", default is "file" diff --git a/pkg/services/auth/session_cleanup.go b/pkg/services/auth/session_cleanup.go index 64dee43d677..7e523181a7b 100644 --- a/pkg/services/auth/session_cleanup.go +++ b/pkg/services/auth/session_cleanup.go @@ -7,7 +7,7 @@ import ( func (srv *UserAuthTokenServiceImpl) Run(ctx context.Context) error { ticker := time.NewTicker(time.Hour * 12) - deleteSessionAfter := time.Hour * 24 * 7 * 30 + deleteSessionAfter := time.Hour * 24 * time.Duration(srv.Cfg.LoginDeleteExpiredTokensAfterDays) for { select { diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index daef13e7983..78424bc6388 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -224,10 +224,11 @@ type Cfg struct { EnableAlphaPanels bool EnterpriseLicensePath string - LoginCookieName string - LoginCookieUsername string - LoginCookieSecure bool - LoginCookieMaxDays int + LoginCookieName string + LoginCookieUsername string + LoginCookieSecure bool + LoginCookieMaxDays int + LoginDeleteExpiredTokensAfterDays int } type CommandLineArgs struct { @@ -553,10 +554,11 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { //login login := iniFile.Section("login") - cfg.LoginCookieName = login.Key("cookie_name").String() - cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt() + cfg.LoginCookieName = login.Key("cookie_name").MustString("grafana_session") + cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt(7) cfg.LoginCookieSecure = login.Key("cookie_secure").MustBool(false) - cfg.LoginCookieUsername = login.Key("cookie_username").String() + cfg.LoginCookieUsername = login.Key("cookie_username").MustString("grafana_username") + cfg.LoginDeleteExpiredTokensAfterDays = login.Key("delete_expired_token_after_days").MustInt(30) Env = iniFile.Section("").Key("app_mode").MustString("development") InstanceName = iniFile.Section("").Key("instance_name").MustString("unknown_instance_name") From c3ff3d644cb9d857ab52a47ace9319d184d0d5f4 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 22 Jan 2019 16:16:32 +0100 Subject: [PATCH 31/49] fixes nil ref in tests --- pkg/api/login.go | 12 ++++++------ pkg/middleware/auth_proxy.go | 2 +- pkg/services/auth/auth_token_test.go | 9 ++++++++- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index b4c6f8af58e..fbc4cc3d38a 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -47,13 +47,13 @@ func (hs *HTTPServer) LoginView(c *m.ReqContext) { return //} - if redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")); len(redirectTo) > 0 { - c.SetCookie("redirect_to", "", -1, setting.AppSubUrl+"/") - c.Redirect(redirectTo) - return - } + // if redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")); len(redirectTo) > 0 { + // c.SetCookie("redirect_to", "", -1, setting.AppSubUrl+"/") + // c.Redirect(redirectTo) + // return + // } - c.Redirect(setting.AppSubUrl + "/") + // c.Redirect(setting.AppSubUrl + "/") } func tryOAuthAutoLogin(c *m.ReqContext) bool { diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index fc109ac707f..0b980362447 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -66,7 +66,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { query.UserId = getRequestUserId(ctx) // if we're using ldap, pass authproxy login name to ldap user sync } else if setting.LdapEnabled { - ctx.Session.Delete(session.SESS_KEY_LASTLDAPSYNC) + ctx.Session.Delete(session.SESS_KEY_LASTLDAPSYNC) //makes sure we always sync with ldap if session if we only have last sync info in session but not user. syncQuery := &m.LoginUserQuery{ ReqContext: ctx, diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index ee9bdf60bb9..a34dbc673e6 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -291,7 +291,14 @@ func createTestContext(t *testing.T) *testContext { sqlstore := sqlstore.InitTestDB(t) tokenService := &UserAuthTokenServiceImpl{ SQLStore: sqlstore, - log: log.New("test-logger"), + Cfg: &setting.Cfg{ + LoginCookieName: "grafana_session", + LoginCookieUsername: "grafana_username", + LoginCookieSecure: false, + LoginCookieMaxDays: 7, + LoginDeleteExpiredTokensAfterDays: 30, + }, + log: log.New("test-logger"), } RotateTime = 10 * time.Minute From 5998646da50359a86af13899058edb53b683a338 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 23 Jan 2019 12:41:15 +0100 Subject: [PATCH 32/49] restrict session usage to auth_proxy --- pkg/api/common_test.go | 2 -- pkg/api/http_server.go | 3 ++- pkg/middleware/auth.go | 11 ----------- pkg/middleware/auth_proxy.go | 18 +++++++++++++++++- pkg/middleware/middleware.go | 2 +- pkg/middleware/middleware_test.go | 8 ++++++-- pkg/middleware/recovery_test.go | 3 +-- pkg/middleware/session.go | 26 +++++++++----------------- pkg/models/context.go | 1 + pkg/services/session/session.go | 2 -- 10 files changed, 37 insertions(+), 39 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 3be4cd38448..eb1f89e3f22 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -5,7 +5,6 @@ import ( "net/http/httptest" "path/filepath" - "github.com/go-macaron/session" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/middleware" m "github.com/grafana/grafana/pkg/models" @@ -126,7 +125,6 @@ func setupScenarioContext(url string) *scenarioContext { sc.userAuthTokenService = newFakeUserAuthTokenService() sc.m.Use(middleware.GetContextHandler(sc.userAuthTokenService)) - sc.m.Use(middleware.Sessioner(&session.Options{}, 0)) return sc } diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index c85bdb6f2e7..d4b8d8777c4 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -26,6 +26,7 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/hooks" "github.com/grafana/grafana/pkg/services/rendering" + "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -223,8 +224,8 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { m.Use(hs.healthHandler) m.Use(hs.metricsEndpoint) m.Use(middleware.GetContextHandler(hs.AuthTokenService)) - m.Use(middleware.Sessioner(&setting.SessionOptions, setting.SessionConnMaxLifetime)) m.Use(middleware.OrgRedirect()) + session.Init(&setting.SessionOptions, setting.SessionConnMaxLifetime) // needs to be after context handler if setting.EnforceDomain { diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index 5faee1e3fa7..27248342c8d 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -7,7 +7,6 @@ import ( "gopkg.in/macaron.v1" m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -17,16 +16,6 @@ type AuthOptions struct { ReqSignedIn bool } -func getRequestUserId(c *m.ReqContext) int64 { - userID := c.Session.Get(session.SESS_KEY_USERID) - - if userID != nil { - return userID.(int64) - } - - return 0 -} - func getApiKey(c *m.ReqContext) string { header := c.Req.Header.Get("Authorization") parts := strings.SplitN(header, " ", 2) diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index 0b980362447..7b517a6f5f8 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -16,7 +16,9 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -var AUTH_PROXY_SESSION_VAR = "authProxyHeaderValue" +var ( + AUTH_PROXY_SESSION_VAR = "authProxyHeaderValue" +) func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { if !setting.AuthProxyEnabled { @@ -161,6 +163,10 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { ctx.IsSignedIn = true ctx.Session.Set(session.SESS_KEY_USERID, ctx.UserId) + if err := ctx.Session.Release(); err != nil { + ctx.Logger.Error("failed to save session data", "error", err) + } + return true } @@ -192,6 +198,16 @@ var syncGrafanaUserWithLdapUser = func(query *m.LoginUserQuery) error { return nil } +func getRequestUserId(c *m.ReqContext) int64 { + userID := c.Session.Get(session.SESS_KEY_USERID) + + if userID != nil { + return userID.(int64) + } + + return 0 +} + func checkAuthenticationProxy(remoteAddr string, proxyHeaderValue string) error { if len(strings.TrimSpace(setting.AuthProxyWhitelist)) == 0 { return nil diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 705d28db3eb..04d014c6fa8 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -26,7 +26,7 @@ func GetContextHandler(ats auth.UserAuthTokenService) macaron.Handler { ctx := &m.ReqContext{ Context: c, SignedInUser: &m.SignedInUser{}, - Session: session.GetSession(), + Session: session.GetSession(), // should only be used by auth_proxy IsSignedIn: false, AllowAnonymous: false, SkipCache: false, diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 469b03c1e72..11740574d0b 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -7,7 +7,7 @@ import ( "path/filepath" "testing" - ms "github.com/go-macaron/session" + msession "github.com/go-macaron/session" "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/session" @@ -201,6 +201,7 @@ func TestMiddlewareContext(t *testing.T) { return nil }) + setting.SessionOptions = msession.Options{} sc.fakeReq("GET", "/") sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") sc.exec() @@ -469,6 +470,7 @@ func middlewareScenario(desc string, fn scenarioFunc) { defer bus.ClearBusHandlers() sc := &scenarioContext{} + viewsPath, _ := filepath.Abs("../../public/views") sc.m = macaron.New() @@ -477,11 +479,13 @@ func middlewareScenario(desc string, fn scenarioFunc) { Delims: macaron.Delims{Left: "[[", Right: "]]"}, })) + session.Init(&msession.Options{}, 0) sc.userAuthTokenService = newFakeUserAuthTokenService() sc.m.Use(GetContextHandler(sc.userAuthTokenService)) // mock out gc goroutine session.StartSessionGC = func() {} - sc.m.Use(Sessioner(&ms.Options{}, 0)) + setting.SessionOptions = msession.Options{} + sc.m.Use(OrgRedirect()) sc.m.Use(AddDefaultResponseHeaders()) diff --git a/pkg/middleware/recovery_test.go b/pkg/middleware/recovery_test.go index eb76f186f49..d4a99360e07 100644 --- a/pkg/middleware/recovery_test.go +++ b/pkg/middleware/recovery_test.go @@ -4,7 +4,6 @@ import ( "path/filepath" "testing" - ms "github.com/go-macaron/session" "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/session" @@ -68,7 +67,7 @@ func recoveryScenario(desc string, url string, fn scenarioFunc) { sc.m.Use(GetContextHandler(sc.userAuthTokenService)) // mock out gc goroutine session.StartSessionGC = func() {} - sc.m.Use(Sessioner(&ms.Options{}, 0)) + //sc.m.Use(Sessioner(&ms.Options{}, 0)) sc.m.Use(OrgRedirect()) sc.m.Use(AddDefaultResponseHeaders()) diff --git a/pkg/middleware/session.go b/pkg/middleware/session.go index 19cfa368b49..54bcfffdba2 100644 --- a/pkg/middleware/session.go +++ b/pkg/middleware/session.go @@ -1,21 +1,13 @@ package middleware -import ( - ms "github.com/go-macaron/session" - "gopkg.in/macaron.v1" +// func Sessioner(options *ms.Options, sessionConnMaxLifetime int64) macaron.Handler { +// session.Init(options, sessionConnMaxLifetime) - m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/session" -) +// return func(ctx *m.ReqContext) { +// ctx.Next() -func Sessioner(options *ms.Options, sessionConnMaxLifetime int64) macaron.Handler { - session.Init(options, sessionConnMaxLifetime) - - return func(ctx *m.ReqContext) { - ctx.Next() - - if err := ctx.Session.Release(); err != nil { - panic("session(release): " + err.Error()) - } - } -} +// if err := ctx.Session.Release(); err != nil { +// panic("session(release): " + err.Error()) +// } +// } +// } diff --git a/pkg/models/context.go b/pkg/models/context.go index 1a78c021f45..df970451304 100644 --- a/pkg/models/context.go +++ b/pkg/models/context.go @@ -14,6 +14,7 @@ type ReqContext struct { *macaron.Context *SignedInUser + // This should only be used by the auth_proxy Session session.SessionStore IsSignedIn bool diff --git a/pkg/services/session/session.go b/pkg/services/session/session.go index 5873a6a5b72..2e60b8a25d7 100644 --- a/pkg/services/session/session.go +++ b/pkg/services/session/session.go @@ -14,8 +14,6 @@ import ( const ( SESS_KEY_USERID = "uid" - SESS_KEY_OAUTH_STATE = "state" - SESS_KEY_APIKEY = "apikey_id" // used for render requests with api keys SESS_KEY_LASTLDAPSYNC = "last_ldap_sync" ) From df85cc9bb176fa6bf8f286052227196e1118d58d Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 23 Jan 2019 15:28:33 +0100 Subject: [PATCH 33/49] redirect logged in users from /login to home --- pkg/api/login.go | 62 +++++++++--------------------------------------- 1 file changed, 11 insertions(+), 51 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index fbc4cc3d38a..0348a956bbf 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -42,18 +42,18 @@ func (hs *HTTPServer) LoginView(c *m.ReqContext) { return } - //if !hs.tryLoginUsingRememberCookie(c) { - c.HTML(200, ViewIndex, viewData) - return - //} + if !c.IsSignedIn { + c.HTML(200, ViewIndex, viewData) + return + } - // if redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")); len(redirectTo) > 0 { - // c.SetCookie("redirect_to", "", -1, setting.AppSubUrl+"/") - // c.Redirect(redirectTo) - // return - // } + if redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")); len(redirectTo) > 0 { + c.SetCookie("redirect_to", "", -1, setting.AppSubUrl+"/") + c.Redirect(redirectTo) + return + } - // c.Redirect(setting.AppSubUrl + "/") + c.Redirect(setting.AppSubUrl + "/") } func tryOAuthAutoLogin(c *m.ReqContext) bool { @@ -74,48 +74,8 @@ func tryOAuthAutoLogin(c *m.ReqContext) bool { return false } -func (hs *HTTPServer) tryLoginUsingRememberCookie(c *m.ReqContext) bool { - // Check auto-login. - uname := c.GetCookie(setting.CookieUserName) - if len(uname) == 0 { - return false - } - - isSucceed := false - defer func() { - if !isSucceed { - log.Trace("auto-login cookie cleared: %s", uname) - c.SetCookie(setting.CookieUserName, "", -1, setting.AppSubUrl+"/") - c.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubUrl+"/") - return - } - }() - - userQuery := m.GetUserByLoginQuery{LoginOrEmail: uname} - if err := bus.Dispatch(&userQuery); err != nil { - return false - } - - user := userQuery.Result - - // validate remember me cookie - signingKey := user.Rands + user.Password - if len(signingKey) < 10 { - c.Logger.Error("Invalid user signingKey") - return false - } - - if val, _ := c.GetSuperSecureCookie(signingKey, setting.CookieRememberName); val != user.Login { - return false - } - - isSucceed = true - hs.loginUserWithUser(user, c) - return true -} - func (hs *HTTPServer) LoginAPIPing(c *m.ReqContext) { - if !hs.tryLoginUsingRememberCookie(c) { + if !c.IsSignedIn || !c.IsAnonymous { c.JsonApiErr(401, "Unauthorized", nil) return } From 4626f083bb36636cc38b85aa0115106348bf53c2 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 23 Jan 2019 17:01:09 +0100 Subject: [PATCH 34/49] store oauth login error messages in an encrypted cookie --- pkg/api/login.go | 47 +++++++++++++++++++++++++++++++++++++++--- pkg/api/login_oauth.go | 16 +++++++------- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index 0348a956bbf..db75b12a202 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -1,6 +1,8 @@ package api import ( + "encoding/hex" + "net/http" "net/url" "github.com/grafana/grafana/pkg/api/dtos" @@ -10,10 +12,12 @@ import ( "github.com/grafana/grafana/pkg/metrics" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" ) const ( - ViewIndex = "index" + ViewIndex = "index" + LoginErrorCookieName = "login_error" ) func (hs *HTTPServer) LoginView(c *m.ReqContext) { @@ -33,8 +37,8 @@ func (hs *HTTPServer) LoginView(c *m.ReqContext) { viewData.Settings["loginHint"] = setting.LoginHint viewData.Settings["disableLoginForm"] = setting.DisableLoginForm - if loginError, ok := c.Session.Get("loginError").(string); ok { - c.Session.Delete("loginError") + if loginError, ok := tryGetEncryptedCookie(c, LoginErrorCookieName); ok { + deleteCookie(c, LoginErrorCookieName) viewData.Settings["loginError"] = loginError } @@ -141,3 +145,40 @@ func (hs *HTTPServer) Logout(c *m.ReqContext) { c.Redirect(setting.AppSubUrl + "/login") } } + +func tryGetEncryptedCookie(ctx *m.ReqContext, cookieName string) (string, bool) { + cookie := ctx.GetCookie(cookieName) + if cookie == "" { + return "", false + } + + decoded, err := hex.DecodeString(cookie) + if err != nil { + return "", false + } + + decryptedError, err := util.Decrypt([]byte(decoded), setting.SecretKey) + return string(decryptedError), err == nil +} + +func deleteCookie(ctx *m.ReqContext, cookieName string) { + ctx.SetCookie(cookieName, "", -1, setting.AppSubUrl+"/") +} + +func (hs *HTTPServer) trySetEncryptedCookie(ctx *m.ReqContext, cookieName string, value string, maxAge int) error { + encryptedError, err := util.Encrypt([]byte(value), setting.SecretKey) + if err != nil { + return err + } + + http.SetCookie(ctx.Resp, &http.Cookie{ + Name: cookieName, + MaxAge: 60, + Value: hex.EncodeToString(encryptedError), + HttpOnly: true, + Path: setting.AppSubUrl + "/", + Secure: hs.Cfg.LoginCookieSecure, + }) + + return nil +} diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 36c9c12905f..91f0dde9e64 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -52,7 +52,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { if errorParam != "" { errorDesc := ctx.Query("error_description") oauthLogger.Error("failed to login ", "error", errorParam, "errorDesc", errorDesc) - redirectWithError(ctx, login.ErrProviderDeniedRequest, "error", errorParam, "errorDesc", errorDesc) + hs.redirectWithError(ctx, login.ErrProviderDeniedRequest, "error", errorParam, "errorDesc", errorDesc) return } @@ -142,7 +142,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { userInfo, err := connect.UserInfo(client, token) if err != nil { if sErr, ok := err.(*social.Error); ok { - redirectWithError(ctx, sErr) + hs.redirectWithError(ctx, sErr) } else { ctx.Handle(500, fmt.Sprintf("login.OAuthLogin(get info from %s)", name), err) } @@ -153,13 +153,13 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { // validate that we got at least an email address if userInfo.Email == "" { - redirectWithError(ctx, login.ErrNoEmail) + hs.redirectWithError(ctx, login.ErrNoEmail) return } // validate that the email is allowed to login to grafana if !connect.IsEmailAllowed(userInfo.Email) { - redirectWithError(ctx, login.ErrEmailNotAllowed) + hs.redirectWithError(ctx, login.ErrEmailNotAllowed) return } @@ -182,9 +182,10 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { ExternalUser: extUser, SignupAllowed: connect.IsSignupAllowed(), } + err = bus.Dispatch(cmd) if err != nil { - redirectWithError(ctx, err) + hs.redirectWithError(ctx, err) return } @@ -218,8 +219,9 @@ func hashStatecode(code, seed string) string { return hex.EncodeToString(hashBytes[:]) } -func redirectWithError(ctx *m.ReqContext, err error, v ...interface{}) { +func (hs *HTTPServer) redirectWithError(ctx *m.ReqContext, err error, v ...interface{}) { ctx.Logger.Error(err.Error(), v...) - ctx.Session.Set("loginError", err.Error()) + hs.trySetEncryptedCookie(ctx, LoginErrorCookieName, err.Error(), 60) + ctx.Redirect(setting.AppSubUrl + "/login") } From 56a521b2642ce043506ace33c1d17a8b803ee9ea Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 10:50:18 +0100 Subject: [PATCH 35/49] makes auth token rotation time configurable --- conf/defaults.ini | 3 +++ conf/sample.ini | 22 ++++++++++++++++++++++ pkg/services/auth/auth_token.go | 3 +-- pkg/services/auth/auth_token_test.go | 2 +- pkg/setting/setting.go | 2 ++ 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 60fa25e4bce..244cb7346a6 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -122,6 +122,9 @@ cookie_username = grafana_user # How many days an session can be unused before we inactivate it login_remember_days = 7 +# How often should the login token be rotated. default to '30m' +rotate_cookie_every = 30m + # How long should Grafana keep expired tokens before deleting them delete_expired_token_after_days = 30 diff --git a/conf/sample.ini b/conf/sample.ini index 96b92db6f48..29f136fa341 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -102,6 +102,28 @@ log_queries = # For "sqlite3" only. cache mode setting used for connecting to the database. (private, shared) ;cache_mode = private +#################################### Login ############################### + +[login] + +# Login cookie name +;cookie_name = grafana_session + +# If you want login cookies to be https only. default is false +;cookie_secure = false + +# Logged in user name +;cookie_username = grafana_user + +# How many days an session can be unused before we inactivate it +;login_remember_days = 7 + +# How often should the login token be rotated. default to '30m' +;rotate_cookie_every = 30m + +# How long should Grafana keep expired tokens before deleting them +;delete_expired_token_after_days = 30 + #################################### Session #################################### [session] # Either "memory", "file", "redis", "mysql", "postgres", default is "file" diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index c04389ab557..a6a0cad89e3 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -23,7 +23,6 @@ func init() { var ( getTime = time.Now - RotateTime = 2 * time.Minute UrgentRotateTime = 20 * time.Second oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. ) @@ -219,7 +218,7 @@ func (s *UserAuthTokenServiceImpl) RefreshToken(token *userAuthToken, clientIP, needsRotation := false rotatedAt := time.Unix(token.RotatedAt, 0) if token.AuthTokenSeen { - needsRotation = rotatedAt.Before(now.Add(-RotateTime)) + needsRotation = rotatedAt.Before(now.Add(-s.Cfg.LoginCookieRotation)) } else { needsRotation = rotatedAt.Before(now.Add(-UrgentRotateTime)) } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index a34dbc673e6..22a126fa7c5 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -297,11 +297,11 @@ func createTestContext(t *testing.T) *testContext { LoginCookieSecure: false, LoginCookieMaxDays: 7, LoginDeleteExpiredTokensAfterDays: 30, + LoginCookieRotation: 10 * time.Minute, }, log: log.New("test-logger"), } - RotateTime = 10 * time.Minute UrgentRotateTime = time.Minute setting.LogInRememberDays = 7 diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 66710c8e190..f6dc154235a 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -229,6 +229,7 @@ type Cfg struct { LoginCookieUsername string LoginCookieSecure bool LoginCookieMaxDays int + LoginCookieRotation time.Duration LoginDeleteExpiredTokensAfterDays int } @@ -560,6 +561,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.LoginCookieSecure = login.Key("cookie_secure").MustBool(false) cfg.LoginCookieUsername = login.Key("cookie_username").MustString("grafana_username") cfg.LoginDeleteExpiredTokensAfterDays = login.Key("delete_expired_token_after_days").MustInt(30) + cfg.LoginCookieRotation = login.Key("rotate_cookie_every").MustDuration(time.Minute * 30) Env = iniFile.Section("").Key("app_mode").MustString("development") InstanceName = iniFile.Section("").Key("instance_name").MustString("unknown_instance_name") From ff483f3782780b7a19499a08166cef74cdf3c455 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 10:55:10 +0100 Subject: [PATCH 36/49] removes old cookie auth configuration --- conf/defaults.ini | 8 -------- conf/sample.ini | 8 -------- pkg/services/auth/auth_token_test.go | 1 - pkg/setting/setting.go | 6 ------ 4 files changed, 23 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 244cb7346a6..e17373a9a14 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -116,9 +116,6 @@ cookie_name = grafana_session # If you want login cookies to be https only. default is false cookie_secure = false -# Logged in user name -cookie_username = grafana_user - # How many days an session can be unused before we inactivate it login_remember_days = 7 @@ -198,11 +195,6 @@ admin_password = admin # used for signing secret_key = SW2YcwTIb9zpOOhoPsMm -# Auto-login remember days -login_remember_days = 7 -cookie_username = grafana_user -cookie_remember_name = grafana_remember - # disable gravatar profile images disable_gravatar = false diff --git a/conf/sample.ini b/conf/sample.ini index 29f136fa341..28998961350 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -112,9 +112,6 @@ log_queries = # If you want login cookies to be https only. default is false ;cookie_secure = false -# Logged in user name -;cookie_username = grafana_user - # How many days an session can be unused before we inactivate it ;login_remember_days = 7 @@ -184,11 +181,6 @@ log_queries = # used for signing ;secret_key = SW2YcwTIb9zpOOhoPsMm -# Auto-login remember days -;login_remember_days = 7 -;cookie_username = grafana_user -;cookie_remember_name = grafana_remember - # disable gravatar profile images ;disable_gravatar = false diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 22a126fa7c5..1dce15afb1b 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -303,7 +303,6 @@ func createTestContext(t *testing.T) *testContext { } UrgentRotateTime = time.Minute - setting.LogInRememberDays = 7 return &testContext{ sqlstore: sqlstore, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index f6dc154235a..5db10a89263 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -83,9 +83,6 @@ var ( // Security settings. SecretKey string - LogInRememberDays int - CookieUserName string - CookieRememberName string DisableGravatar bool EmailCodeValidMinutes int DataProxyWhiteList map[string]bool @@ -603,9 +600,6 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { // read security settings security := iniFile.Section("security") SecretKey = security.Key("secret_key").String() - LogInRememberDays = security.Key("login_remember_days").MustInt() - CookieUserName = security.Key("cookie_username").String() - CookieRememberName = security.Key("cookie_remember_name").String() DisableGravatar = security.Key("disable_gravatar").MustBool(true) cfg.DisableBruteForceLoginProtection = security.Key("disable_brute_force_login_protection").MustBool(false) DisableBruteForceLoginProtection = cfg.DisableBruteForceLoginProtection From f257101c41c4f9580704b2184be92d5d5b71ee90 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 11:26:45 +0100 Subject: [PATCH 37/49] removes unused/commented code --- conf/defaults.ini | 1 - pkg/middleware/auth_proxy.go | 2 +- pkg/middleware/middleware.go | 23 ----------------------- pkg/middleware/recovery_test.go | 3 +-- pkg/middleware/session.go | 13 ------------- pkg/setting/setting.go | 2 -- 6 files changed, 2 insertions(+), 42 deletions(-) delete mode 100644 pkg/middleware/session.go diff --git a/conf/defaults.ini b/conf/defaults.ini index e17373a9a14..f8e5c3cf838 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -143,7 +143,6 @@ provider = file provider_config = sessions - # Session cookie name cookie_name = grafana_sess diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index 7b517a6f5f8..c5be1f32a09 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -68,7 +68,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { query.UserId = getRequestUserId(ctx) // if we're using ldap, pass authproxy login name to ldap user sync } else if setting.LdapEnabled { - ctx.Session.Delete(session.SESS_KEY_LASTLDAPSYNC) //makes sure we always sync with ldap if session if we only have last sync info in session but not user. + ctx.Session.Delete(session.SESS_KEY_LASTLDAPSYNC) syncQuery := &m.LoginUserQuery{ ReqContext: ctx, diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 04d014c6fa8..3722ac3058f 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -88,29 +88,6 @@ func initContextWithAnonymousUser(ctx *m.ReqContext) bool { return true } -// func initContextWithUserSessionCookie(ctx *m.ReqContext, orgId int64) bool { -// // initialize session -// if err := ctx.Session.Start(ctx.Context); err != nil { -// ctx.Logger.Error("Failed to start session", "error", err) -// return false -// } - -// var userId int64 -// if userId = getRequestUserId(ctx); userId == 0 { -// return false -// } - -// query := m.GetSignedInUserQuery{UserId: userId, OrgId: orgId} -// if err := bus.Dispatch(&query); err != nil { -// ctx.Logger.Error("Failed to get user with id", "userId", userId, "error", err) -// return false -// } - -// ctx.SignedInUser = query.Result -// ctx.IsSignedIn = true -// return true -// } - func initContextWithApiKey(ctx *m.ReqContext) bool { var keyString string if keyString = getApiKey(ctx); keyString == "" { diff --git a/pkg/middleware/recovery_test.go b/pkg/middleware/recovery_test.go index d4a99360e07..e041d42e56b 100644 --- a/pkg/middleware/recovery_test.go +++ b/pkg/middleware/recovery_test.go @@ -9,7 +9,7 @@ import ( "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" . "github.com/smartystreets/goconvey/convey" - "gopkg.in/macaron.v1" + macaron "gopkg.in/macaron.v1" ) func TestRecoveryMiddleware(t *testing.T) { @@ -67,7 +67,6 @@ func recoveryScenario(desc string, url string, fn scenarioFunc) { sc.m.Use(GetContextHandler(sc.userAuthTokenService)) // mock out gc goroutine session.StartSessionGC = func() {} - //sc.m.Use(Sessioner(&ms.Options{}, 0)) sc.m.Use(OrgRedirect()) sc.m.Use(AddDefaultResponseHeaders()) diff --git a/pkg/middleware/session.go b/pkg/middleware/session.go deleted file mode 100644 index 54bcfffdba2..00000000000 --- a/pkg/middleware/session.go +++ /dev/null @@ -1,13 +0,0 @@ -package middleware - -// func Sessioner(options *ms.Options, sessionConnMaxLifetime int64) macaron.Handler { -// session.Init(options, sessionConnMaxLifetime) - -// return func(ctx *m.ReqContext) { -// ctx.Next() - -// if err := ctx.Session.Release(); err != nil { -// panic("session(release): " + err.Error()) -// } -// } -// } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 5db10a89263..79c2a780d1e 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -223,7 +223,6 @@ type Cfg struct { EnterpriseLicensePath string LoginCookieName string - LoginCookieUsername string LoginCookieSecure bool LoginCookieMaxDays int LoginCookieRotation time.Duration @@ -556,7 +555,6 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.LoginCookieName = login.Key("cookie_name").MustString("grafana_session") cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt(7) cfg.LoginCookieSecure = login.Key("cookie_secure").MustBool(false) - cfg.LoginCookieUsername = login.Key("cookie_username").MustString("grafana_username") cfg.LoginDeleteExpiredTokensAfterDays = login.Key("delete_expired_token_after_days").MustInt(30) cfg.LoginCookieRotation = login.Key("rotate_cookie_every").MustDuration(time.Minute * 30) From fd0f9f2dd22f58da3f922a0971ca901efbdb4214 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 12:06:44 +0100 Subject: [PATCH 38/49] fixes broken test --- pkg/services/auth/auth_token_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 1dce15afb1b..f136dbd80be 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -293,7 +293,6 @@ func createTestContext(t *testing.T) *testContext { SQLStore: sqlstore, Cfg: &setting.Cfg{ LoginCookieName: "grafana_session", - LoginCookieUsername: "grafana_username", LoginCookieSecure: false, LoginCookieMaxDays: 7, LoginDeleteExpiredTokensAfterDays: 30, From 9ae306e417cc4b8ad3ef243cc8900c6c1de9da85 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 13:48:36 +0100 Subject: [PATCH 39/49] use defer to make sure we always release session data --- pkg/api/http_server.go | 3 ++- pkg/middleware/auth_proxy.go | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index d4b8d8777c4..7b7c1478a4c 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -65,6 +65,8 @@ func (hs *HTTPServer) Init() error { hs.macaron = hs.newMacaron() hs.registerRoutes() + session.Init(&setting.SessionOptions, setting.SessionConnMaxLifetime) + return nil } @@ -225,7 +227,6 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { m.Use(hs.metricsEndpoint) m.Use(middleware.GetContextHandler(hs.AuthTokenService)) m.Use(middleware.OrgRedirect()) - session.Init(&setting.SessionOptions, setting.SessionConnMaxLifetime) // needs to be after context handler if setting.EnforceDomain { diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index c5be1f32a09..93ee577e3c6 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -42,6 +42,12 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { return false } + defer func() { + if err := ctx.Session.Release(); err != nil { + ctx.Logger.Error("failed to save session data", "error", err) + } + }() + query := &m.GetSignedInUserQuery{OrgId: orgID} // if this session has already been authenticated by authProxy just load the user @@ -163,10 +169,6 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { ctx.IsSignedIn = true ctx.Session.Set(session.SESS_KEY_USERID, ctx.UserId) - if err := ctx.Session.Release(); err != nil { - ctx.Logger.Error("failed to save session data", "error", err) - } - return true } From 516037fbdd35235f6d839a4cb34a74e0581d8f2b Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 13:54:45 +0100 Subject: [PATCH 40/49] makes sure rotation is always higher than urgent rotation --- conf/defaults.ini | 2 +- pkg/services/auth/auth_token.go | 4 ++-- pkg/services/auth/auth_token_test.go | 2 +- pkg/setting/setting.go | 7 +++++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index f8e5c3cf838..0470515b581 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -120,7 +120,7 @@ cookie_secure = false login_remember_days = 7 # How often should the login token be rotated. default to '30m' -rotate_cookie_every = 30m +rotate_token_minutes = 30 # How long should Grafana keep expired tokens before deleting them delete_expired_token_after_days = 30 diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index a6a0cad89e3..de226342f89 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -23,7 +23,7 @@ func init() { var ( getTime = time.Now - UrgentRotateTime = 20 * time.Second + UrgentRotateTime = 1 * time.Minute oneYearInSeconds = 31557600 //used as default maxage for session cookies. We validate/rotate them more often. ) @@ -218,7 +218,7 @@ func (s *UserAuthTokenServiceImpl) RefreshToken(token *userAuthToken, clientIP, needsRotation := false rotatedAt := time.Unix(token.RotatedAt, 0) if token.AuthTokenSeen { - needsRotation = rotatedAt.Before(now.Add(-s.Cfg.LoginCookieRotation)) + needsRotation = rotatedAt.Before(now.Add(-time.Duration(s.Cfg.LoginCookieRotation) * time.Minute)) } else { needsRotation = rotatedAt.Before(now.Add(-UrgentRotateTime)) } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index f136dbd80be..ef4a093e41a 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -296,7 +296,7 @@ func createTestContext(t *testing.T) *testContext { LoginCookieSecure: false, LoginCookieMaxDays: 7, LoginDeleteExpiredTokensAfterDays: 30, - LoginCookieRotation: 10 * time.Minute, + LoginCookieRotation: 10, }, log: log.New("test-logger"), } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 79c2a780d1e..adfc21c2edb 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -225,7 +225,7 @@ type Cfg struct { LoginCookieName string LoginCookieSecure bool LoginCookieMaxDays int - LoginCookieRotation time.Duration + LoginCookieRotation int LoginDeleteExpiredTokensAfterDays int } @@ -556,7 +556,10 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt(7) cfg.LoginCookieSecure = login.Key("cookie_secure").MustBool(false) cfg.LoginDeleteExpiredTokensAfterDays = login.Key("delete_expired_token_after_days").MustInt(30) - cfg.LoginCookieRotation = login.Key("rotate_cookie_every").MustDuration(time.Minute * 30) + cfg.LoginCookieRotation = login.Key("rotate_token_minutes").MustInt(30) + if cfg.LoginCookieRotation < 2 { + cfg.LoginCookieRotation = 2 + } Env = iniFile.Section("").Key("app_mode").MustString("development") InstanceName = iniFile.Section("").Key("instance_name").MustString("unknown_instance_name") From 9153b6ed9667a394677a0f96b83a4bc3cd1124b6 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 15:17:09 +0100 Subject: [PATCH 41/49] improves readability of loginping handler --- conf/sample.ini | 4 ++-- pkg/api/login.go | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/conf/sample.ini b/conf/sample.ini index 28998961350..ba25c335768 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -115,8 +115,8 @@ log_queries = # How many days an session can be unused before we inactivate it ;login_remember_days = 7 -# How often should the login token be rotated. default to '30m' -;rotate_cookie_every = 30m +# How often should the login token be rotated. default to '30' +;rotate_token_minutes = 30 # How long should Grafana keep expired tokens before deleting them ;delete_expired_token_after_days = 30 diff --git a/pkg/api/login.go b/pkg/api/login.go index db75b12a202..24e215b0490 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -78,13 +78,12 @@ func tryOAuthAutoLogin(c *m.ReqContext) bool { return false } -func (hs *HTTPServer) LoginAPIPing(c *m.ReqContext) { - if !c.IsSignedIn || !c.IsAnonymous { - c.JsonApiErr(401, "Unauthorized", nil) - return +func (hs *HTTPServer) LoginAPIPing(c *m.ReqContext) Response { + if c.IsSignedIn || c.IsAnonymous { + return JSON(200, "Logged in") } - c.JsonOK("Logged in") + return Error(401, "Unauthorized", nil) } func (hs *HTTPServer) LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response { From d6edaa1328c854884468fded29da685b6b3fc47f Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 24 Jan 2019 19:04:58 +0100 Subject: [PATCH 42/49] moves cookie https setting to [security] --- conf/defaults.ini | 6 +++--- conf/sample.ini | 6 +++--- pkg/api/login.go | 2 +- pkg/api/login_oauth.go | 24 ++++++++++++++---------- pkg/services/auth/auth_token.go | 2 +- pkg/services/auth/auth_token_test.go | 1 - pkg/setting/setting.go | 5 +++-- 7 files changed, 25 insertions(+), 21 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 0470515b581..50548cc628d 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -113,9 +113,6 @@ cache_mode = private # Login cookie name cookie_name = grafana_session -# If you want login cookies to be https only. default is false -cookie_secure = false - # How many days an session can be unused before we inactivate it login_remember_days = 7 @@ -203,6 +200,9 @@ data_source_proxy_whitelist = # disable protection against brute force login attempts disable_brute_force_login_protection = false +# set cookies as https only. default is false +https_flag_cookies = false + #################################### Snapshots ########################### [snapshots] # snapshot sharing options diff --git a/conf/sample.ini b/conf/sample.ini index ba25c335768..eae6560bc64 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -109,9 +109,6 @@ log_queries = # Login cookie name ;cookie_name = grafana_session -# If you want login cookies to be https only. default is false -;cookie_secure = false - # How many days an session can be unused before we inactivate it ;login_remember_days = 7 @@ -190,6 +187,9 @@ log_queries = # disable protection against brute force login attempts ;disable_brute_force_login_protection = false +# set cookies as https only. default is false +;https_flag_cookies = false + #################################### Snapshots ########################### [snapshots] # snapshot sharing options diff --git a/pkg/api/login.go b/pkg/api/login.go index 24e215b0490..50c62e0835a 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -176,7 +176,7 @@ func (hs *HTTPServer) trySetEncryptedCookie(ctx *m.ReqContext, cookieName string Value: hex.EncodeToString(encryptedError), HttpOnly: true, Path: setting.AppSubUrl + "/", - Secure: hs.Cfg.LoginCookieSecure, + Secure: hs.Cfg.SecurityHTTPSCookies, }) return nil diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 91f0dde9e64..4160d48733e 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -60,8 +60,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { if code == "" { state := GenStateString() hashedState := hashStatecode(state, setting.OAuthService.OAuthInfos[name].ClientSecret) - hs.writeOauthStateCookie(ctx, hashedState, 60) - + hs.writeCookie(ctx.Resp, OauthStateCookieName, hashedState, 60) if setting.OAuthService.OAuthInfos[name].HostedDomain == "" { ctx.Redirect(connect.AuthCodeURL(state, oauth2.AccessTypeOnline)) } else { @@ -70,19 +69,20 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { return } - savedState := ctx.GetCookie(OauthStateCookieName) + cookieState := ctx.GetCookie(OauthStateCookieName) // delete cookie ctx.Resp.Header().Del("Set-Cookie") - hs.writeOauthStateCookie(ctx, "", -1) + hs.deleteCookie(ctx.Resp, OauthStateCookieName) - if savedState == "" { + if cookieState == "" { ctx.Handle(500, "login.OAuthLogin(missing saved state)", nil) return } queryState := hashStatecode(ctx.Query("state"), setting.OAuthService.OAuthInfos[name].ClientSecret) - if savedState != queryState { + oauthLogger.Info("state check", "queryState", queryState, "cookieState", cookieState) + if cookieState != queryState { ctx.Handle(500, "login.OAuthLogin(state mismatch)", nil) return } @@ -203,14 +203,18 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { ctx.Redirect(setting.AppSubUrl + "/") } -func (hs *HTTPServer) writeOauthStateCookie(ctx *m.ReqContext, value string, maxAge int) { - http.SetCookie(ctx.Resp, &http.Cookie{ - Name: OauthStateCookieName, +func (hs *HTTPServer) deleteCookie(w http.ResponseWriter, name string) { + hs.writeCookie(w, name, "", -1) +} + +func (hs *HTTPServer) writeCookie(w http.ResponseWriter, name string, value string, maxAge int) { + http.SetCookie(w, &http.Cookie{ + Name: name, MaxAge: maxAge, Value: value, HttpOnly: true, Path: setting.AppSubUrl + "/", - Secure: hs.Cfg.LoginCookieSecure, + Secure: hs.Cfg.SecurityHTTPSCookies, }) } diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index de226342f89..39f5cfdd746 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -95,7 +95,7 @@ func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, va HttpOnly: true, Domain: setting.Domain, Path: setting.AppSubUrl + "/", - Secure: s.Cfg.LoginCookieSecure, + Secure: s.Cfg.SecurityHTTPSCookies, MaxAge: maxAge, } diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index ef4a093e41a..2f75c660d9d 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -293,7 +293,6 @@ func createTestContext(t *testing.T) *testContext { SQLStore: sqlstore, Cfg: &setting.Cfg{ LoginCookieName: "grafana_session", - LoginCookieSecure: false, LoginCookieMaxDays: 7, LoginDeleteExpiredTokensAfterDays: 30, LoginCookieRotation: 10, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index adfc21c2edb..cc7e538b1d4 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -223,10 +223,11 @@ type Cfg struct { EnterpriseLicensePath string LoginCookieName string - LoginCookieSecure bool LoginCookieMaxDays int LoginCookieRotation int LoginDeleteExpiredTokensAfterDays int + + SecurityHTTPSCookies bool } type CommandLineArgs struct { @@ -554,7 +555,6 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { login := iniFile.Section("login") cfg.LoginCookieName = login.Key("cookie_name").MustString("grafana_session") cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt(7) - cfg.LoginCookieSecure = login.Key("cookie_secure").MustBool(false) cfg.LoginDeleteExpiredTokensAfterDays = login.Key("delete_expired_token_after_days").MustInt(30) cfg.LoginCookieRotation = login.Key("rotate_token_minutes").MustInt(30) if cfg.LoginCookieRotation < 2 { @@ -603,6 +603,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { SecretKey = security.Key("secret_key").String() DisableGravatar = security.Key("disable_gravatar").MustBool(true) cfg.DisableBruteForceLoginProtection = security.Key("disable_brute_force_login_protection").MustBool(false) + cfg.SecurityHTTPSCookies = security.Key("https_flag_cookies").MustBool(false) DisableBruteForceLoginProtection = cfg.DisableBruteForceLoginProtection // read snapshots settings From 6454de74e4bca90b509b520e9de1164d1aca5be1 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 24 Jan 2019 22:47:53 +0100 Subject: [PATCH 43/49] user auth token load tests using k6.io --- devenv/docker/loadtest/README.md | 69 ++++++++ devenv/docker/loadtest/auth_token_test.js | 67 ++++++++ devenv/docker/loadtest/modules/client.js | 183 ++++++++++++++++++++++ devenv/docker/loadtest/modules/util.js | 30 ++++ devenv/docker/loadtest/run.sh | 20 +++ 5 files changed, 369 insertions(+) create mode 100644 devenv/docker/loadtest/README.md create mode 100644 devenv/docker/loadtest/auth_token_test.js create mode 100644 devenv/docker/loadtest/modules/client.js create mode 100644 devenv/docker/loadtest/modules/util.js create mode 100755 devenv/docker/loadtest/run.sh diff --git a/devenv/docker/loadtest/README.md b/devenv/docker/loadtest/README.md new file mode 100644 index 00000000000..6f400a44694 --- /dev/null +++ b/devenv/docker/loadtest/README.md @@ -0,0 +1,69 @@ +# Grafana load test + +Runs load tests and checks using [k6](https://k6.io/). + +## Prerequisites + +Docker + +## Run + +Run load test for 15 minutes: + +```bash +$ ./run.sh +``` + +Run load test for custom duration: + +```bash +$ ./run.sh -d 10s +``` + +Example output: + +```bash + + /\ |‾‾| /‾‾/ /‾/ + /\ / \ | |_/ / / / + / \/ \ | | / ‾‾\ + / \ | |‾\ \ | (_) | + / __________ \ |__| \__\ \___/ .io + + execution: local + output: - + script: src/auth_token_test.js + + duration: 10s, iterations: - + vus: 2, max: 2 + + done [==========================================================] 10s / 10s + + █ user auth token test + + █ user authenticates thru ui with username and password + + ✓ response status is 200 + ✓ response has cookie 'grafana_session' with 32 characters + + █ batch tsdb requests + + ✓ response status is 200 + + checks.....................: 100.00% ✓ 364 ✗ 0 + data_received..............: 4.0 MB 402 kB/s + data_sent..................: 120 kB 12 kB/s + group_duration.............: avg=84.95ms min=31.49ms med=90.28ms max=120.08ms p(90)=118.15ms p(95)=118.47ms + http_req_blocked...........: avg=1.63ms min=2.18µs med=1.1ms max=10.94ms p(90)=3.34ms p(95)=4.28ms + http_req_connecting........: avg=1.37ms min=0s med=902.58µs max=10.47ms p(90)=2.95ms p(95)=3.82ms + http_req_duration..........: avg=58.61ms min=3.86ms med=60.49ms max=114.21ms p(90)=92.61ms p(95)=100.17ms + http_req_receiving.........: avg=36µs min=9.78µs med=31.17µs max=234.69µs p(90)=61.58µs p(95)=72.95µs + http_req_sending...........: avg=361.51µs min=19.57µs med=181.38µs max=10.56ms p(90)=642.88µs p(95)=845.28µs + http_req_tls_handshaking...: avg=0s min=0s med=0s max=0s p(90)=0s p(95)=0s + http_req_waiting...........: avg=58.22ms min=3.8ms med=59.7ms max=114.09ms p(90)=92.45ms p(95)=100.02ms + http_reqs..................: 382 38.199516/s + iteration_duration.........: avg=975.79ms min=7.98µs med=1.08s max=1.11s p(90)=1.09s p(95)=1.11s + iterations.................: 18 1.799977/s + vus........................: 2 min=2 max=2 + vus_max....................: 2 min=2 max=2 +``` diff --git a/devenv/docker/loadtest/auth_token_test.js b/devenv/docker/loadtest/auth_token_test.js new file mode 100644 index 00000000000..289dd15b9d1 --- /dev/null +++ b/devenv/docker/loadtest/auth_token_test.js @@ -0,0 +1,67 @@ +import { sleep, check, group } from 'k6'; +import { createClient, createBasicAuthClient } from './modules/client.js'; +import { createTestOrgIfNotExists, createTestdataDatasourceIfNotExists } from './modules/util.js'; + +export let options = { + noCookiesReset: true +}; + +let endpoint = __ENV.URL || 'http://localhost:3000' +const client = createClient(endpoint) + +export const setup = () => { + const basicAuthClient = createBasicAuthClient(endpoint, 'admin', 'admin'); + createTestOrgIfNotExists(basicAuthClient); + const datasourceId = createTestdataDatasourceIfNotExists(basicAuthClient); + return {datasourceId: datasourceId}; +} + +export default (data) => { + group("user auth token test", () => { + if (__ITER === 0) { + group("user authenticates thru ui with username and password", () => { + let res = client.ui.login('admin', 'admin'); + + check(res, { + 'response status is 200': (r) => r.status === 200, + 'response has cookie \'grafana_session\' with 32 characters': (r) => r.cookies.grafana_session[0].value.length === 32, + }); + }); + } + + if (__ITER !== 0) { + group("batch tsdb requests", () => { + const batchCount = 20; + const requests = []; + const payload = { + from: '1547765247624', + to: '1547768847624', + queries: [{ + refId: 'A', + scenarioId: 'random_walk', + intervalMs: 10000, + maxDataPoints: 433, + datasourceId: data.datasourceId, + }] + }; + + requests.push({ method: 'GET', url: '/api/annotations?dashboardId=2074&from=1548078832772&to=1548082432772' }); + + for (let n = 0; n < batchCount; n++) { + requests.push({ method: 'POST', url: '/api/tsdb/query', body: payload }); + } + + let responses = client.batch(requests); + for (let n = 0; n < batchCount; n++) { + check(responses[n], { + 'response status is 200': (r) => r.status === 200, + }); + } + }); + } + }); + + sleep(1) +} + +export const teardown = (data) => {} diff --git a/devenv/docker/loadtest/modules/client.js b/devenv/docker/loadtest/modules/client.js new file mode 100644 index 00000000000..c78b0ba31cf --- /dev/null +++ b/devenv/docker/loadtest/modules/client.js @@ -0,0 +1,183 @@ +import http from "k6/http"; +import encoding from 'k6/encoding'; + +export const UIEndpoint = class UIEndpoint { + constructor(httpClient) { + this.httpClient = httpClient; + } + + login(username, pwd) { + const payload = { user: username, password: pwd }; + return this.httpClient.formPost('/login', payload); + } +} + +export const DatasourcesEndpoint = class DatasourcesEndpoint { + constructor(httpClient) { + this.httpClient = httpClient; + } + + getById(id) { + return this.httpClient.get(`/datasources/${id}`); + } + + getByName(name) { + return this.httpClient.get(`/datasources/name/${name}`); + } + + create(payload) { + return this.httpClient.post(`/datasources`, JSON.stringify(payload)); + } + + delete(id) { + return this.httpClient.delete(`/datasources/${id}`); + } +} + +export const OrganizationsEndpoint = class OrganizationsEndpoint { + constructor(httpClient) { + this.httpClient = httpClient; + } + + getById(id) { + return this.httpClient.get(`/orgs/${id}`); + } + + getByName(name) { + return this.httpClient.get(`/orgs/name/${name}`); + } + + create(name) { + let payload = { + name: name, + }; + return this.httpClient.post(`/orgs`, JSON.stringify(payload)); + } + + delete(id) { + return this.httpClient.delete(`/orgs/${id}`); + } +} + +export const GrafanaClient = class GrafanaClient { + constructor(httpClient) { + httpClient.onBeforeRequest = this.onBeforeRequest; + this.raw = httpClient; + this.ui = new UIEndpoint(httpClient); + this.orgs = new OrganizationsEndpoint(httpClient.withUrl('/api')); + this.datasources = new DatasourcesEndpoint(httpClient.withUrl('/api')); + } + + batch(requests) { + return this.raw.batch(requests); + } + + withOrgId(orgId) { + this.orgId = orgId; + } + + onBeforeRequest(params) { + if (this.orgId && this.orgId > 0) { + params = params.headers || {}; + params.headers["X-Grafana-Org-Id"] = this.orgId; + } + } +} + +export const BaseClient = class BaseClient { + constructor(url, subUrl) { + if (url.endsWith('/')) { + url = url.substring(0, url.length - 1); + } + + if (subUrl.endsWith('/')) { + subUrl = subUrl.substring(0, subUrl.length - 1); + } + + this.url = url + subUrl; + this.onBeforeRequest = () => {}; + } + + withUrl(subUrl) { + return new BaseClient(this.url, subUrl); + } + + beforeRequest(params) { + + } + + get(url, params) { + params = params || {}; + this.beforeRequest(params); + this.onBeforeRequest(params); + return http.get(this.url + url, params); + } + + formPost(url, body, params) { + params = params || {}; + this.beforeRequest(params); + this.onBeforeRequest(params); + return http.post(this.url + url, body, params); + } + + post(url, body, params) { + params = params || {}; + params.headers = params.headers || {}; + params.headers['Content-Type'] = 'application/json'; + + this.beforeRequest(params); + this.onBeforeRequest(params); + return http.post(this.url + url, body, params); + } + + delete(url, params) { + params = params || {}; + this.beforeRequest(params); + this.onBeforeRequest(params); + return http.del(this.url + url, null, params); + } + + batch(requests) { + for (let n = 0; n < requests.length; n++) { + let params = requests[n].params || {}; + params.headers = params.headers || {}; + params.headers['Content-Type'] = 'application/json'; + this.beforeRequest(params); + this.onBeforeRequest(params); + requests[n].params = params; + requests[n].url = this.url + requests[n].url; + if (requests[n].body) { + requests[n].body = JSON.stringify(requests[n].body); + } + } + + return http.batch(requests); + } +} + +export class BasicAuthClient extends BaseClient { + constructor(url, subUrl, username, password) { + super(url, subUrl); + this.username = username; + this.password = password; + } + + withUrl(subUrl) { + return new BasicAuthClient(this.url, subUrl, this.username, this.password); + } + + beforeRequest(params) { + params = params || {}; + params.headers = params.headers || {}; + let token = `${this.username}:${this.password}`; + params.headers['Authorization'] = `Basic ${encoding.b64encode(token)}`; + } +} + +export const createClient = (url) => { + return new GrafanaClient(new BaseClient(url, '')); +} + +export const createBasicAuthClient = (url, username, password) => { + return new GrafanaClient(new BasicAuthClient(url, '', username, password)); +} diff --git a/devenv/docker/loadtest/modules/util.js b/devenv/docker/loadtest/modules/util.js new file mode 100644 index 00000000000..408f8f4e09a --- /dev/null +++ b/devenv/docker/loadtest/modules/util.js @@ -0,0 +1,30 @@ +export const createTestOrgIfNotExists = (client) => { + let res = client.orgs.getByName('k6'); + if (res.status === 404) { + res = client.orgs.create('k6'); + if (res.status !== 200) { + throw new Error('Expected 200 response status when creating org'); + } + } + + client.withOrgId(res.json().orgId); +} + +export const createTestdataDatasourceIfNotExists = (client) => { + const payload = { + access: 'proxy', + isDefault: false, + name: 'k6-testdata', + type: 'testdata', + }; + + let res = client.datasources.getByName(payload.name); + if (res.status === 404) { + res = client.datasources.create(payload); + if (res.status !== 200) { + throw new Error('Expected 200 response status when creating datasource'); + } + } + + return res.json().id; +} diff --git a/devenv/docker/loadtest/run.sh b/devenv/docker/loadtest/run.sh new file mode 100755 index 00000000000..9edb8879080 --- /dev/null +++ b/devenv/docker/loadtest/run.sh @@ -0,0 +1,20 @@ +#/bin/bash + +PWD=$(pwd) + +run() { + duration='15m' + + while getopts ":d:" o; do + case "${o}" in + d) + duration=${OPTARG} + ;; + esac + done + shift $((OPTIND-1)) + + docker run -t --network=host -v $PWD:/src --rm -i loadimpact/k6:master run --vus 2 --duration $duration src/auth_token_test.js +} + +run "$@" From cfd8bd711b2657e3291ba58c388ea3619c3bb506 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 24 Jan 2019 23:49:45 +0100 Subject: [PATCH 44/49] org id fix for load test --- devenv/docker/loadtest/README.md | 38 +++++++++++------------ devenv/docker/loadtest/auth_token_test.js | 8 +++-- devenv/docker/loadtest/modules/client.js | 8 +++-- devenv/docker/loadtest/modules/util.js | 7 ++++- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/devenv/docker/loadtest/README.md b/devenv/docker/loadtest/README.md index 6f400a44694..8e724637acb 100644 --- a/devenv/docker/loadtest/README.md +++ b/devenv/docker/loadtest/README.md @@ -34,10 +34,10 @@ Example output: output: - script: src/auth_token_test.js - duration: 10s, iterations: - - vus: 2, max: 2 + duration: 15m0s, iterations: - + vus: 2, max: 2 - done [==========================================================] 10s / 10s + done [==========================================================] 15m0s / 15m0s █ user auth token test @@ -50,20 +50,20 @@ Example output: ✓ response status is 200 - checks.....................: 100.00% ✓ 364 ✗ 0 - data_received..............: 4.0 MB 402 kB/s - data_sent..................: 120 kB 12 kB/s - group_duration.............: avg=84.95ms min=31.49ms med=90.28ms max=120.08ms p(90)=118.15ms p(95)=118.47ms - http_req_blocked...........: avg=1.63ms min=2.18µs med=1.1ms max=10.94ms p(90)=3.34ms p(95)=4.28ms - http_req_connecting........: avg=1.37ms min=0s med=902.58µs max=10.47ms p(90)=2.95ms p(95)=3.82ms - http_req_duration..........: avg=58.61ms min=3.86ms med=60.49ms max=114.21ms p(90)=92.61ms p(95)=100.17ms - http_req_receiving.........: avg=36µs min=9.78µs med=31.17µs max=234.69µs p(90)=61.58µs p(95)=72.95µs - http_req_sending...........: avg=361.51µs min=19.57µs med=181.38µs max=10.56ms p(90)=642.88µs p(95)=845.28µs - http_req_tls_handshaking...: avg=0s min=0s med=0s max=0s p(90)=0s p(95)=0s - http_req_waiting...........: avg=58.22ms min=3.8ms med=59.7ms max=114.09ms p(90)=92.45ms p(95)=100.02ms - http_reqs..................: 382 38.199516/s - iteration_duration.........: avg=975.79ms min=7.98µs med=1.08s max=1.11s p(90)=1.09s p(95)=1.11s - iterations.................: 18 1.799977/s - vus........................: 2 min=2 max=2 - vus_max....................: 2 min=2 max=2 + checks.....................: 100.00% ✓ 32844 ✗ 0 + data_received..............: 411 MB 457 kB/s + data_sent..................: 12 MB 14 kB/s + group_duration.............: avg=95.64ms min=16.42ms med=94.35ms max=307.52ms p(90)=137.78ms p(95)=146.75ms + http_req_blocked...........: avg=1.27ms min=942ns med=610.08µs max=48.32ms p(90)=2.92ms p(95)=4.25ms + http_req_connecting........: avg=1.06ms min=0s med=456.79µs max=47.19ms p(90)=2.55ms p(95)=3.78ms + http_req_duration..........: avg=58.16ms min=1ms med=52.59ms max=293.35ms p(90)=109.53ms p(95)=120.19ms + http_req_receiving.........: avg=38.98µs min=6.43µs med=32.55µs max=16.2ms p(90)=64.63µs p(95)=78.8µs + http_req_sending...........: avg=328.66µs min=8.09µs med=110.77µs max=44.13ms p(90)=552.65µs p(95)=1.09ms + http_req_tls_handshaking...: avg=0s min=0s med=0s max=0s p(90)=0s p(95)=0s + http_req_waiting...........: avg=57.79ms min=935.02µs med=52.15ms max=293.06ms p(90)=109.04ms p(95)=119.71ms + http_reqs..................: 34486 38.317775/s + iteration_duration.........: avg=1.09s min=1.81µs med=1.09s max=1.3s p(90)=1.13s p(95)=1.14s + iterations.................: 1642 1.824444/s + vus........................: 2 min=2 max=2 + vus_max....................: 2 min=2 max=2 ``` diff --git a/devenv/docker/loadtest/auth_token_test.js b/devenv/docker/loadtest/auth_token_test.js index 289dd15b9d1..c7c6fb98efe 100644 --- a/devenv/docker/loadtest/auth_token_test.js +++ b/devenv/docker/loadtest/auth_token_test.js @@ -11,9 +11,13 @@ const client = createClient(endpoint) export const setup = () => { const basicAuthClient = createBasicAuthClient(endpoint, 'admin', 'admin'); - createTestOrgIfNotExists(basicAuthClient); + const orgId = createTestOrgIfNotExists(basicAuthClient); const datasourceId = createTestdataDatasourceIfNotExists(basicAuthClient); - return {datasourceId: datasourceId}; + client.withOrgId(orgId); + return { + orgId: orgId, + datasourceId: datasourceId, + }; } export default (data) => { diff --git a/devenv/docker/loadtest/modules/client.js b/devenv/docker/loadtest/modules/client.js index c78b0ba31cf..bda0da64564 100644 --- a/devenv/docker/loadtest/modules/client.js +++ b/devenv/docker/loadtest/modules/client.js @@ -99,7 +99,9 @@ export const BaseClient = class BaseClient { } withUrl(subUrl) { - return new BaseClient(this.url, subUrl); + let c = new BaseClient(this.url, subUrl); + c.onBeforeRequest = this.onBeforeRequest; + return c; } beforeRequest(params) { @@ -163,7 +165,9 @@ export class BasicAuthClient extends BaseClient { } withUrl(subUrl) { - return new BasicAuthClient(this.url, subUrl, this.username, this.password); + let c = new BasicAuthClient(this.url, subUrl, this.username, this.password); + c.onBeforeRequest = this.onBeforeRequest; + return c; } beforeRequest(params) { diff --git a/devenv/docker/loadtest/modules/util.js b/devenv/docker/loadtest/modules/util.js index 408f8f4e09a..af6d4cdac09 100644 --- a/devenv/docker/loadtest/modules/util.js +++ b/devenv/docker/loadtest/modules/util.js @@ -1,13 +1,18 @@ export const createTestOrgIfNotExists = (client) => { + let orgId = 0; let res = client.orgs.getByName('k6'); if (res.status === 404) { res = client.orgs.create('k6'); if (res.status !== 200) { throw new Error('Expected 200 response status when creating org'); } + orgId = res.json().orgId; + } else { + orgId = res.json().id; } - client.withOrgId(res.json().orgId); + client.withOrgId(orgId); + return orgId; } export const createTestdataDatasourceIfNotExists = (client) => { From 75760aa892ca07eaabd8fb8877ff672cdfa5461e Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 25 Jan 2019 10:40:50 +0100 Subject: [PATCH 45/49] dont specify domain for auth cookies --- pkg/services/auth/auth_token.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 39f5cfdd746..7e9433c2d70 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -93,7 +93,6 @@ func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, va Name: s.Cfg.LoginCookieName, Value: url.QueryEscape(value), HttpOnly: true, - Domain: setting.Domain, Path: setting.AppSubUrl + "/", Secure: s.Cfg.SecurityHTTPSCookies, MaxAge: maxAge, From 95d9328c666caf4bb3b4583e75fbabc53c1c29b5 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 25 Jan 2019 12:59:50 +0100 Subject: [PATCH 46/49] set low login cookie rotate time in ha mode --- devenv/docker/ha_test/docker-compose.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/devenv/docker/ha_test/docker-compose.yaml b/devenv/docker/ha_test/docker-compose.yaml index 1195e2a977c..ef4f6225cbe 100644 --- a/devenv/docker/ha_test/docker-compose.yaml +++ b/devenv/docker/ha_test/docker-compose.yaml @@ -49,6 +49,7 @@ services: - GF_DATABASE_HOST=db:3306 - GF_SESSION_PROVIDER=mysql - GF_SESSION_PROVIDER_CONFIG=grafana:password@tcp(db:3306)/grafana?allowNativePasswords=true + - GF_LOGIN_ROTATE_TOKEN_MINUTES=2 # - GF_DATABASE_TYPE=postgres # - GF_DATABASE_HOST=db:5432 # - GF_DATABASE_SSL_MODE=disable From 806ddd63a0ee06746429e5f01cd7e860e9f76e67 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 25 Jan 2019 13:16:19 +0100 Subject: [PATCH 47/49] load test/ha fixes --- devenv/docker/ha_test/docker-compose.yaml | 3 ++- devenv/docker/loadtest/auth_token_test.js | 4 ++-- devenv/docker/loadtest/run.sh | 8 ++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/devenv/docker/ha_test/docker-compose.yaml b/devenv/docker/ha_test/docker-compose.yaml index ef4f6225cbe..e8f212e1cd5 100644 --- a/devenv/docker/ha_test/docker-compose.yaml +++ b/devenv/docker/ha_test/docker-compose.yaml @@ -55,7 +55,8 @@ services: # - GF_DATABASE_SSL_MODE=disable # - GF_SESSION_PROVIDER=postgres # - GF_SESSION_PROVIDER_CONFIG=user=grafana password=password host=db port=5432 dbname=grafana sslmode=disable - - GF_LOG_FILTERS=alerting.notifier:debug,alerting.notifier.slack:debug + - GF_LOG_FILTERS=alerting.notifier:debug,alerting.notifier.slack:debug,auth:debug + - GF_LOGIN_ROTATE_TOKEN_MINUTES=2 ports: - 3000 depends_on: diff --git a/devenv/docker/loadtest/auth_token_test.js b/devenv/docker/loadtest/auth_token_test.js index c7c6fb98efe..e1356fb6f9a 100644 --- a/devenv/docker/loadtest/auth_token_test.js +++ b/devenv/docker/loadtest/auth_token_test.js @@ -6,8 +6,8 @@ export let options = { noCookiesReset: true }; -let endpoint = __ENV.URL || 'http://localhost:3000' -const client = createClient(endpoint) +let endpoint = __ENV.URL || 'http://localhost:3000'; +const client = createClient(endpoint); export const setup = () => { const basicAuthClient = createBasicAuthClient(endpoint, 'admin', 'admin'); diff --git a/devenv/docker/loadtest/run.sh b/devenv/docker/loadtest/run.sh index 9edb8879080..474d75383b6 100755 --- a/devenv/docker/loadtest/run.sh +++ b/devenv/docker/loadtest/run.sh @@ -4,17 +4,21 @@ PWD=$(pwd) run() { duration='15m' + url='http://localhost:3000' - while getopts ":d:" o; do + while getopts ":d:u:" o; do case "${o}" in d) duration=${OPTARG} ;; + u) + url=${OPTARG} + ;; esac done shift $((OPTIND-1)) - docker run -t --network=host -v $PWD:/src --rm -i loadimpact/k6:master run --vus 2 --duration $duration src/auth_token_test.js + docker run -t --network=host -v $PWD:/src -e URL=$url --rm -i loadimpact/k6:master run --vus 2 --duration $duration src/auth_token_test.js } run "$@" From 42fa41e78d5aae8daa2776bb2c1104840bb1a860 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 25 Jan 2019 13:18:17 +0100 Subject: [PATCH 48/49] fix --- devenv/docker/ha_test/docker-compose.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/devenv/docker/ha_test/docker-compose.yaml b/devenv/docker/ha_test/docker-compose.yaml index e8f212e1cd5..504ee86404d 100644 --- a/devenv/docker/ha_test/docker-compose.yaml +++ b/devenv/docker/ha_test/docker-compose.yaml @@ -49,7 +49,6 @@ services: - GF_DATABASE_HOST=db:3306 - GF_SESSION_PROVIDER=mysql - GF_SESSION_PROVIDER_CONFIG=grafana:password@tcp(db:3306)/grafana?allowNativePasswords=true - - GF_LOGIN_ROTATE_TOKEN_MINUTES=2 # - GF_DATABASE_TYPE=postgres # - GF_DATABASE_HOST=db:5432 # - GF_DATABASE_SSL_MODE=disable From e4924795a292037a7634cd0f71887c0977bbd377 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 25 Jan 2019 13:30:26 +0100 Subject: [PATCH 49/49] change default rotate_token_minutes to 10 minutes --- conf/defaults.ini | 4 ++-- conf/sample.ini | 4 ++-- pkg/setting/setting.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 50548cc628d..6fc4cf2e4de 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -116,8 +116,8 @@ cookie_name = grafana_session # How many days an session can be unused before we inactivate it login_remember_days = 7 -# How often should the login token be rotated. default to '30m' -rotate_token_minutes = 30 +# How often should the login token be rotated. default to '10m' +rotate_token_minutes = 10 # How long should Grafana keep expired tokens before deleting them delete_expired_token_after_days = 30 diff --git a/conf/sample.ini b/conf/sample.ini index eae6560bc64..0f1c02dc231 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -112,8 +112,8 @@ log_queries = # How many days an session can be unused before we inactivate it ;login_remember_days = 7 -# How often should the login token be rotated. default to '30' -;rotate_token_minutes = 30 +# How often should the login token be rotated. default to '10' +;rotate_token_minutes = 10 # How long should Grafana keep expired tokens before deleting them ;delete_expired_token_after_days = 30 diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index cc7e538b1d4..660a00ba41d 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -556,7 +556,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.LoginCookieName = login.Key("cookie_name").MustString("grafana_session") cfg.LoginCookieMaxDays = login.Key("login_remember_days").MustInt(7) cfg.LoginDeleteExpiredTokensAfterDays = login.Key("delete_expired_token_after_days").MustInt(30) - cfg.LoginCookieRotation = login.Key("rotate_token_minutes").MustInt(30) + cfg.LoginCookieRotation = login.Key("rotate_token_minutes").MustInt(10) if cfg.LoginCookieRotation < 2 { cfg.LoginCookieRotation = 2 }