From 2a70c730258b8e12227476d95b8111295b88a576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agn=C3=A8s=20Toulet?= <35176601+AgnesToulet@users.noreply.github.com> Date: Tue, 19 Jan 2021 17:55:53 +0100 Subject: [PATCH] Auth: add expired token error and update CreateToken function (#30203) * Auth: add error for expired token * Auth: save token error into context data * Auth: send full user and req context to CreateToken * Auth: add token ID in context * add TokenExpiredError struct * update auth tests * remove most of the changes to CreateToken func * clean up * Login: add requestURI in CreateToken ctx * update RequestURIKey comment --- pkg/api/login.go | 5 +++- pkg/models/user_auth.go | 4 +++ pkg/models/user_token.go | 9 ++++++- pkg/services/auth/auth_token.go | 17 +++++++----- pkg/services/auth/auth_token_test.go | 27 ++++++++++--------- pkg/services/auth/testing.go | 8 +++--- pkg/services/contexthandler/contexthandler.go | 2 ++ 7 files changed, 47 insertions(+), 25 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index c19ae305de0..30c303a530b 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/hex" "errors" "net/http" @@ -259,10 +260,12 @@ func (hs *HTTPServer) loginUserWithUser(user *models.User, c *models.ReqContext) } hs.log.Debug("Got IP address from client address", "addr", addr, "ip", ip) - userToken, err := hs.AuthTokenService.CreateToken(c.Req.Context(), user.Id, ip, c.Req.UserAgent()) + ctx := context.WithValue(c.Req.Context(), models.RequestURIKey{}, c.Req.RequestURI) + userToken, err := hs.AuthTokenService.CreateToken(ctx, user, ip, c.Req.UserAgent()) if err != nil { return errutil.Wrap("failed to create auth token", err) } + c.UserToken = userToken hs.log.Info("Successful Login", "User", user.Email) cookies.WriteSessionCookie(c, hs.Cfg, userToken.UnhashedToken, hs.Cfg.LoginMaxLifetime) diff --git a/pkg/models/user_auth.go b/pkg/models/user_auth.go index 94d74ba81ad..2061cf048be 100644 --- a/pkg/models/user_auth.go +++ b/pkg/models/user_auth.go @@ -46,6 +46,10 @@ type LoginInfo struct { Error error } +// RequestURIKey is used as key to save request URI in contexts +// (used for the Enterprise auditing feature) +type RequestURIKey struct{} + // --------------------- // COMMANDS diff --git a/pkg/models/user_token.go b/pkg/models/user_token.go index d32bda57cec..fec4ee18112 100644 --- a/pkg/models/user_token.go +++ b/pkg/models/user_token.go @@ -11,6 +11,13 @@ var ( ErrUserTokenNotFound = errors.New("user token not found") ) +type TokenExpiredError struct { + UserID int64 + TokenID int64 +} + +func (e *TokenExpiredError) Error() string { return "user token expired" } + // UserToken represents a user token type UserToken struct { Id int64 @@ -33,7 +40,7 @@ type RevokeAuthTokenCmd struct { // UserTokenService are used for generating and validating user tokens type UserTokenService interface { - CreateToken(ctx context.Context, userId int64, clientIP net.IP, userAgent string) (*UserToken, error) + CreateToken(ctx context.Context, user *User, clientIP net.IP, userAgent string) (*UserToken, error) LookupToken(ctx context.Context, unhashedToken string) (*UserToken, error) TryRotateToken(ctx context.Context, token *UserToken, clientIP net.IP, userAgent string) (bool, error) RevokeToken(ctx context.Context, token *UserToken) error diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index a4affaded2a..738b2118687 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -60,7 +60,7 @@ func (s *UserAuthTokenService) ActiveTokenCount(ctx context.Context) (int64, err return count, err } -func (s *UserAuthTokenService) CreateToken(ctx context.Context, userId int64, clientIP net.IP, userAgent string) (*models.UserToken, error) { +func (s *UserAuthTokenService) CreateToken(ctx context.Context, user *models.User, clientIP net.IP, userAgent string) (*models.UserToken, error) { token, err := util.RandomHex(16) if err != nil { return nil, err @@ -75,7 +75,7 @@ func (s *UserAuthTokenService) CreateToken(ctx context.Context, userId int64, cl } userAuthToken := userAuthToken{ - UserId: userId, + UserId: user.Id, AuthToken: hashedToken, PrevAuthToken: hashedToken, ClientIp: clientIPStr, @@ -116,11 +116,9 @@ func (s *UserAuthTokenService) LookupToken(ctx context.Context, unhashedToken st var exists bool var err error err = s.SQLStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { - exists, err = dbSession.Where("(auth_token = ? OR prev_auth_token = ?) AND created_at > ? AND rotated_at > ?", + exists, err = dbSession.Where("(auth_token = ? OR prev_auth_token = ?)", hashedToken, - hashedToken, - s.createdAfterParam(), - s.rotatedAfterParam()). + hashedToken). Get(&model) return err @@ -133,6 +131,13 @@ func (s *UserAuthTokenService) LookupToken(ctx context.Context, unhashedToken st return nil, models.ErrUserTokenNotFound } + if model.CreatedAt <= s.createdAfterParam() || model.RotatedAt <= s.rotatedAfterParam() { + return nil, &models.TokenExpiredError{ + UserID: model.UserId, + TokenID: model.Id, + } + } + if model.AuthToken != hashedToken && model.PrevAuthToken == hashedToken && model.AuthTokenSeen { modelCopy := model modelCopy.AuthTokenSeen = false diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 46ae87feb64..7e71464158c 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -20,7 +20,8 @@ func TestUserAuthToken(t *testing.T) { Convey("Test user auth token", t, func() { ctx := createTestContext(t) userAuthTokenService := ctx.tokenService - userID := int64(10) + user := &models.User{Id: int64(10)} + userID := user.Id t := time.Date(2018, 12, 13, 13, 45, 0, 0, time.UTC) getTime = func() time.Time { @@ -28,7 +29,7 @@ func TestUserAuthToken(t *testing.T) { } Convey("When creating token", func() { - userToken, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) So(userToken, ShouldNotBeNil) @@ -80,7 +81,7 @@ func TestUserAuthToken(t *testing.T) { }) Convey("When creating an additional token", func() { - userToken2, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken2, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) So(userToken2, ShouldNotBeNil) @@ -127,7 +128,7 @@ func TestUserAuthToken(t *testing.T) { for i := 0; i < 3; i++ { userId := userID + int64(i+1) userIds = append(userIds, userId) - _, err := userAuthTokenService.CreateToken(context.Background(), userId, + _, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) } @@ -145,7 +146,7 @@ func TestUserAuthToken(t *testing.T) { }) Convey("expires correctly", func() { - userToken, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) @@ -181,13 +182,13 @@ func TestUserAuthToken(t *testing.T) { So(stillGood, ShouldNotBeNil) }) - Convey("when rotated_at is 7:00:00 ago should not find token", func() { + Convey("when rotated_at is 7:00:00 ago should return token expired error", func() { getTime = func() time.Time { return time.Unix(model.RotatedAt, 0).Add(24 * 7 * time.Hour) } notGood, err := userAuthTokenService.LookupToken(context.Background(), userToken.UnhashedToken) - So(err, ShouldEqual, models.ErrUserTokenNotFound) + So(err, ShouldHaveSameTypeAs, &models.TokenExpiredError{}) So(notGood, ShouldBeNil) Convey("should not find active token when expired", func() { @@ -211,7 +212,7 @@ func TestUserAuthToken(t *testing.T) { So(stillGood, ShouldNotBeNil) }) - Convey("when rotated_at is 5 days ago and created_at is 30 days ago should not find token", func() { + Convey("when rotated_at is 5 days ago and created_at is 30 days ago should return token expired error", func() { updated, err := ctx.updateRotatedAt(model.Id, time.Unix(model.CreatedAt, 0).Add(24*25*time.Hour).Unix()) So(err, ShouldBeNil) So(updated, ShouldBeTrue) @@ -221,13 +222,13 @@ func TestUserAuthToken(t *testing.T) { } notGood, err := userAuthTokenService.LookupToken(context.Background(), userToken.UnhashedToken) - So(err, ShouldEqual, models.ErrUserTokenNotFound) + So(err, ShouldHaveSameTypeAs, &models.TokenExpiredError{}) So(notGood, ShouldBeNil) }) }) Convey("can properly rotate tokens", func() { - userToken, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) @@ -312,7 +313,7 @@ func TestUserAuthToken(t *testing.T) { }) Convey("keeps prev token valid for 1 minute after it is confirmed", func() { - userToken, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) So(userToken, ShouldNotBeNil) @@ -345,7 +346,7 @@ func TestUserAuthToken(t *testing.T) { }) Convey("will not mark token unseen when prev and current are the same", func() { - userToken, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) So(userToken, ShouldNotBeNil) @@ -365,7 +366,7 @@ func TestUserAuthToken(t *testing.T) { }) Convey("Rotate token", func() { - userToken, err := userAuthTokenService.CreateToken(context.Background(), userID, + userToken, err := userAuthTokenService.CreateToken(context.Background(), user, net.ParseIP("192.168.10.11"), "some user agent") So(err, ShouldBeNil) So(userToken, ShouldNotBeNil) diff --git a/pkg/services/auth/testing.go b/pkg/services/auth/testing.go index 2b1718ed21b..44fd674a6db 100644 --- a/pkg/services/auth/testing.go +++ b/pkg/services/auth/testing.go @@ -8,7 +8,7 @@ import ( ) type FakeUserAuthTokenService struct { - CreateTokenProvider func(ctx context.Context, userId int64, clientIP net.IP, userAgent string) (*models.UserToken, error) + CreateTokenProvider func(ctx context.Context, user *models.User, clientIP net.IP, userAgent string) (*models.UserToken, error) TryRotateTokenProvider func(ctx context.Context, token *models.UserToken, clientIP net.IP, userAgent string) (bool, error) LookupTokenProvider func(ctx context.Context, unhashedToken string) (*models.UserToken, error) RevokeTokenProvider func(ctx context.Context, token *models.UserToken) error @@ -21,7 +21,7 @@ type FakeUserAuthTokenService struct { func NewFakeUserAuthTokenService() *FakeUserAuthTokenService { return &FakeUserAuthTokenService{ - CreateTokenProvider: func(ctx context.Context, userId int64, clientIP net.IP, userAgent string) (*models.UserToken, error) { + CreateTokenProvider: func(ctx context.Context, user *models.User, clientIP net.IP, userAgent string) (*models.UserToken, error) { return &models.UserToken{ UserId: 0, UnhashedToken: "", @@ -63,8 +63,8 @@ func (s *FakeUserAuthTokenService) Init() error { return nil } -func (s *FakeUserAuthTokenService) CreateToken(ctx context.Context, userId int64, clientIP net.IP, userAgent string) (*models.UserToken, error) { - return s.CreateTokenProvider(context.Background(), userId, clientIP, userAgent) +func (s *FakeUserAuthTokenService) CreateToken(ctx context.Context, user *models.User, clientIP net.IP, userAgent string) (*models.UserToken, error) { + return s.CreateTokenProvider(context.Background(), user, clientIP, userAgent) } func (s *FakeUserAuthTokenService) LookupToken(ctx context.Context, unhashedToken string) (*models.UserToken, error) { diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 064864d6186..29ff730d874 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -258,6 +258,8 @@ func (h *ContextHandler) initContextWithToken(ctx *models.ReqContext, orgID int6 if err != nil { ctx.Logger.Error("Failed to look up user based on cookie", "error", err) cookies.WriteSessionCookie(ctx, h.Cfg, "", -1) + + ctx.Data["lookupTokenErr"] = err return false }