Auth: Refactor for revoking user tokens within last hours (#74616)

* fix: revoked tokens within last hours

adds check for unlimited sessions out of index
adds a function for specifing the hours to look back when revoking users tokens, otherwise we "assume" the clean up takes care of them adds a index for the `user_auth_token` - `revoked_at` for faster queries when using `revoked_at`

* fix: sqllite datetime conversion with unixtimestamps

* fix: postgres dialect

* fix: mysql dialect

* fix: mysql dialect missing closing )

* refactor: delete revoked tokens directly

* fix: tests for sqlite

* AuthToken: Simplify DeleteUserRevokedTokens and add test

* fix: linting newline

* Reset get time after test

* fix: test order by revoked

* fix: order by different db

* ascending

* test with seen at

---------

Co-authored-by: Karl Persson <kalle.persson@grafana.com>

pkg/services/auth/authimpl/auth_token.go

@@ -551,11 +551,29 @@ func (s *UserAuthTokenService) ActiveTokenCount(ctx context.Context, userID *int
 	return count, err
 }
 
+func (s *UserAuthTokenService) DeleteUserRevokedTokens(ctx context.Context, userID int64, window time.Duration) error {
+	return s.sqlStore.WithDbSession(ctx, func(sess *db.Session) error {
+		query := "DELETE FROM user_auth_token WHERE user_id = ? AND revoked_at > 0 AND revoked_at <= ?"
+		res, err := sess.Exec(query, userID, time.Now().Add(-window).Unix())
+		if err != nil {
+			return err
+		}
+
+		rows, err := res.RowsAffected()
+		if err != nil {
+			return err
+		}
+
+		s.log.FromContext(ctx).Debug("Deleted user revoked tokens", "userId", userID, "count", rows)
+		return err
+	})
+}
+
 func (s *UserAuthTokenService) GetUserRevokedTokens(ctx context.Context, userId int64) ([]*auth.UserToken, error) {
 	result := []*auth.UserToken{}
 	err := s.sqlStore.WithDbSession(ctx, func(dbSession *db.Session) error {
 		var tokens []*userAuthToken
-		err := dbSession.Where("user_id = ? AND revoked_at > 0", userId).Find(&tokens)
+		err := dbSession.Where("user_id = ? AND revoked_at > 0", userId).Asc("seen_at").Find(&tokens)
 		if err != nil {
 			return err
 		}

pkg/services/auth/authimpl/auth_token_test.go

@@ -23,8 +23,7 @@ import (
 
 func TestIntegrationUserAuthToken(t *testing.T) {
 	ctx := createTestContext(t)
-	user := &user.User{ID: int64(10)}
+	usr := &user.User{ID: int64(10)}
-	// userID := user.Id
 
 	now := time.Date(2018, 12, 13, 13, 45, 0, 0, time.UTC)
 	getTime = func() time.Time { return now }
@@ -32,7 +31,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 
 	t.Run("When creating token", func(t *testing.T) {
 		createToken := func() *auth.UserToken {
-			userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+			userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 				net.ParseIP("192.168.10.11"), "some user agent")
 			require.Nil(t, err)
 			require.NotNil(t, userToken)
@@ -56,7 +55,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 			userToken, err := ctx.tokenService.LookupToken(context.Background(), userToken.UnhashedToken)
 			require.Nil(t, err)
 			require.NotNil(t, userToken)
-			require.Equal(t, user.ID, userToken.UserId)
+			require.Equal(t, usr.ID, userToken.UserId)
 			require.True(t, userToken.AuthTokenSeen)
 
 			storedAuthToken, err := ctx.getAuthTokenByID(userToken.Id)
@@ -105,27 +104,27 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 		userToken = createToken()
 
 		t.Run("When creating an additional token", func(t *testing.T) {
-			userToken2, err := ctx.tokenService.CreateToken(context.Background(), user,
+			userToken2, err := ctx.tokenService.CreateToken(context.Background(), usr,
 				net.ParseIP("192.168.10.11"), "some user agent")
 			require.Nil(t, err)
 			require.NotNil(t, userToken2)
 
 			t.Run("Can get first user token", func(t *testing.T) {
-				token, err := ctx.tokenService.GetUserToken(context.Background(), user.ID, userToken.Id)
+				token, err := ctx.tokenService.GetUserToken(context.Background(), usr.ID, userToken.Id)
 				require.Nil(t, err)
 				require.NotNil(t, token)
 				require.Equal(t, userToken.Id, token.Id)
 			})
 
 			t.Run("Can get second user token", func(t *testing.T) {
-				token, err := ctx.tokenService.GetUserToken(context.Background(), user.ID, userToken2.Id)
+				token, err := ctx.tokenService.GetUserToken(context.Background(), usr.ID, userToken2.Id)
 				require.Nil(t, err)
 				require.NotNil(t, token)
 				require.Equal(t, userToken2.Id, token.Id)
 			})
 
 			t.Run("Can get user tokens", func(t *testing.T) {
-				tokens, err := ctx.tokenService.GetUserTokens(context.Background(), user.ID)
+				tokens, err := ctx.tokenService.GetUserTokens(context.Background(), usr.ID)
 				require.Nil(t, err)
 				require.Equal(t, 2, len(tokens))
 				require.Equal(t, userToken.Id, tokens[0].Id)
@@ -133,7 +132,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 			})
 
 			t.Run("Can revoke all user tokens", func(t *testing.T) {
-				err := ctx.tokenService.RevokeAllUserTokens(context.Background(), user.ID)
+				err := ctx.tokenService.RevokeAllUserTokens(context.Background(), usr.ID)
 				require.Nil(t, err)
 
 				model, err := ctx.getAuthTokenByID(userToken.Id)
@@ -150,9 +149,9 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 			t.Run("Can revoke all users tokens", func(t *testing.T) {
 				userIds := []int64{}
 				for i := 0; i < 3; i++ {
-					userId := user.ID + int64(i+1)
+					userId := usr.ID + int64(i+1)
 					userIds = append(userIds, userId)
-					_, err := ctx.tokenService.CreateToken(context.Background(), user,
+					_, err := ctx.tokenService.CreateToken(context.Background(), usr,
 						net.ParseIP("192.168.10.11"), "some user agent")
 					require.Nil(t, err)
 				}
@@ -171,7 +170,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 
 	t.Run("expires correctly", func(t *testing.T) {
 		ctx := createTestContext(t)
-		userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+		userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 			net.ParseIP("192.168.10.11"), "some user agent")
 		require.Nil(t, err)
 
@@ -256,7 +255,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 	t.Run("can properly rotate tokens", func(t *testing.T) {
 		getTime = func() time.Time { return now }
 		ctx := createTestContext(t)
-		userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+		userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 			net.ParseIP("192.168.10.11"), "some user agent")
 		require.Nil(t, err)
 
@@ -340,7 +339,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 
 	t.Run("keeps prev token valid for 1 minute after it is confirmed", func(t *testing.T) {
 		getTime = func() time.Time { return now }
-		userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+		userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 			net.ParseIP("192.168.10.11"), "some user agent")
 		require.Nil(t, err)
 		require.NotNil(t, userToken)
@@ -371,7 +370,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 	})
 
 	t.Run("will not mark token unseen when prev and current are the same", func(t *testing.T) {
-		userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+		userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 			net.ParseIP("192.168.10.11"), "some user agent")
 		require.Nil(t, err)
 		require.NotNil(t, userToken)
@@ -393,7 +392,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 	t.Run("TryRotateToken", func(t *testing.T) {
 		t.Run("Should rotate current token and previous token when auth token seen", func(t *testing.T) {
 			getTime = func() time.Time { return now }
-			userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+			userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 				net.ParseIP("192.168.10.11"), "some user agent")
 			require.Nil(t, err)
 			require.NotNil(t, userToken)
@@ -445,7 +444,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 
 		t.Run("Should rotate current token, but keep previous token when auth token not seen", func(t *testing.T) {
 			getTime = func() time.Time { return now }
-			userToken, err := ctx.tokenService.CreateToken(context.Background(), user,
+			userToken, err := ctx.tokenService.CreateToken(context.Background(), usr,
 				net.ParseIP("192.168.10.11"), "some user agent")
 			require.Nil(t, err)
 			require.NotNil(t, userToken)
@@ -473,7 +472,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 
 	t.Run("RotateToken", func(t *testing.T) {
 		var prev string
-		token, err := ctx.tokenService.CreateToken(context.Background(), user, nil, "")
+		token, err := ctx.tokenService.CreateToken(context.Background(), usr, nil, "")
 		require.NoError(t, err)
 		t.Run("should rotate token when called with current auth token", func(t *testing.T) {
 			prev = token.UnhashedToken
@@ -496,7 +495,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 		})
 
 		t.Run("should return error when token is revoked", func(t *testing.T) {
-			revokedToken, err := ctx.tokenService.CreateToken(context.Background(), user, nil, "")
+			revokedToken, err := ctx.tokenService.CreateToken(context.Background(), usr, nil, "")
 			require.NoError(t, err)
 			// mark token as revoked
 			err = ctx.sqlstore.WithDbSession(context.Background(), func(sess *db.Session) error {
@@ -510,7 +509,7 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 		})
 
 		t.Run("should return error when token has expired", func(t *testing.T) {
-			expiredToken, err := ctx.tokenService.CreateToken(context.Background(), user, nil, "")
+			expiredToken, err := ctx.tokenService.CreateToken(context.Background(), usr, nil, "")
 			require.NoError(t, err)
 			// mark token as expired
 			err = ctx.sqlstore.WithDbSession(context.Background(), func(sess *db.Session) error {
@@ -522,6 +521,38 @@ func TestIntegrationUserAuthToken(t *testing.T) {
 			_, err = ctx.tokenService.RotateToken(context.Background(), auth.RotateCommand{UnHashedToken: expiredToken.UnhashedToken})
 			assert.ErrorIs(t, err, auth.ErrInvalidSessionToken)
 		})
+
+		t.Run("should only delete revoked tokens that are outside on specified window", func(t *testing.T) {
+			usr := &user.User{ID: 100}
+			token1, err := ctx.tokenService.CreateToken(context.Background(), usr, nil, "")
+			require.NoError(t, err)
+
+			token2, err := ctx.tokenService.CreateToken(context.Background(), usr, nil, "")
+			require.NoError(t, err)
+
+			getTime = func() time.Time {
+				return time.Now()
+			}
+			// revoked token1 with time now
+			err = ctx.tokenService.RevokeToken(context.Background(), token1, true)
+			require.NoError(t, err)
+
+			getTime = func() time.Time {
+				return time.Now().Add(-25 * time.Hour)
+			}
+			// revoked token1 with time at 25 hours ago
+			err = ctx.tokenService.RevokeToken(context.Background(), token2, true)
+			require.NoError(t, err)
+
+			err = ctx.tokenService.DeleteUserRevokedTokens(context.Background(), usr.ID, 24*time.Hour)
+			require.NoError(t, err)
+
+			revokedTokens, err := ctx.tokenService.GetUserRevokedTokens(context.Background(), usr.ID)
+			require.NoError(t, err)
+			assert.Len(t, revokedTokens, 1)
+
+			getTime = time.Now
+		})
 	})
 
 	t.Run("When populating userAuthToken from UserToken should copy all properties", func(t *testing.T) {

pkg/services/sqlstore/migrations/user_auth_token_mig.go

@@ -44,4 +44,8 @@ func addUserAuthTokenMigrations(mg *Migrator) {
 			},
 		),
 	)
+
+	mg.AddMigration("add index user_auth_token.revoked_at", NewAddIndexMigration(userAuthTokenV1, &Index{
+		Cols: []string{"revoked_at"},
+	}))
 }