diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index 39053aa9da9..e7bf6a66b65 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -37,6 +37,7 @@ const ( const ( MetaKeyUsername = "username" MetaKeyAuthModule = "authModule" + MetaKeyIsLogin = "isLogin" ) // ClientParams are hints to the auth service about how to handle the identity management diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 2afc7349bc4..32dae82d428 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -155,7 +155,7 @@ func ProvideService( s.RegisterPostAuthHook(userSyncService.SyncUserHook, 10) s.RegisterPostAuthHook(userSyncService.EnableDisabledUserHook, 20) s.RegisterPostAuthHook(orgUserSyncService.SyncOrgRolesHook, 30) - s.RegisterPostAuthHook(userSyncService.SyncLastSeenHook, 40) + s.RegisterPostAuthHook(userSyncService.SyncLastSeenHook, 120) if features.IsEnabled(featuremgmt.FlagAccessTokenExpirationCheck) { s.RegisterPostAuthHook(sync.ProvideOAuthTokenSync(oauthTokenService, sessionService, socialService).SyncOauthTokenHook, 60) @@ -270,6 +270,7 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i return nil, authn.ErrClientNotConfigured.Errorf("client not configured: %s", client) } + r.SetMeta(authn.MetaKeyIsLogin, "true") identity, err = s.authenticate(ctx, c, r) if err != nil { s.metrics.failedLogin.WithLabelValues(client).Inc() diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index b2ea898cd99..9dfb46f4086 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -3,7 +3,6 @@ package sync import ( "context" "errors" - "time" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/authn" @@ -129,7 +128,12 @@ func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Iden return nil } -func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { +func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identity, r *authn.Request) error { + if r.GetMeta(authn.MetaKeyIsLogin) != "" { + // Do not sync last seen for login requests + return nil + } + namespace, id := identity.NamespacedID() if namespace != authn.NamespaceUser && namespace != authn.NamespaceServiceAccount { @@ -137,10 +141,6 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit return nil } - if !shouldUpdateLastSeen(identity.LastSeenAt) { - return nil - } - go func(userID int64) { defer func() { if err := recover(); err != nil { @@ -148,7 +148,9 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit } }() - if err := s.userService.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{UserID: userID}); err != nil { + if err := s.userService.UpdateLastSeenAt(context.Background(), + &user.UpdateUserLastSeenAtCommand{UserID: userID, OrgID: r.OrgID}); err != nil && + !errors.Is(err, user.ErrLastSeenUpToDate) { s.log.Error("Failed to update last_seen_at", "err", err, "userId", userID) } }(id) @@ -393,7 +395,3 @@ func syncSignedInUserToIdentity(usr *user.SignedInUser, identity *authn.Identity identity.IsDisabled = usr.IsDisabled identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin } - -func shouldUpdateLastSeen(t time.Time) bool { - return time.Since(t) > time.Minute*5 -} diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 2833a0e7431..5b38ede7502 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -242,7 +242,8 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler { // update last seen every 5min if reqContext.ShouldUpdateLastSeenAt() { reqContext.Logger.Debug("Updating last user_seen_at", "user_id", reqContext.UserID) - if err := h.userService.UpdateLastSeenAt(mContext.Req.Context(), &user.UpdateUserLastSeenAtCommand{UserID: reqContext.UserID}); err != nil { + err := h.userService.UpdateLastSeenAt(mContext.Req.Context(), &user.UpdateUserLastSeenAtCommand{UserID: reqContext.UserID, OrgID: reqContext.OrgID}) + if err != nil && !errors.Is(err, user.ErrLastSeenUpToDate) { reqContext.Logger.Error("Failed to update last_seen_at", "error", err) } } diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index e7d16343ef8..cc4be041288 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -27,6 +27,7 @@ var ( ErrLastGrafanaAdmin = errors.New("cannot remove last grafana admin") ErrProtectedUser = errors.New("cannot adopt protected user") ErrNoUniqueID = errors.New("identifying id not found") + ErrLastSeenUpToDate = errors.New("last seen is already up to date") ) type User struct { @@ -95,6 +96,7 @@ type ChangeUserPasswordCommand struct { type UpdateUserLastSeenAtCommand struct { UserID int64 + OrgID int64 } type SetUsingOrgCommand struct { diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 9bb06ef129b..43751d3cb87 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -219,9 +219,26 @@ func (s *Service) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswo } func (s *Service) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error { + u, err := s.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{ + UserID: cmd.UserID, + OrgID: cmd.OrgID, + }) + + if err != nil { + return err + } + + if !shouldUpdateLastSeen(u.LastSeenAt) { + return user.ErrLastSeenUpToDate + } + return s.store.UpdateLastSeenAt(ctx, cmd) } +func shouldUpdateLastSeen(t time.Time) bool { + return time.Since(t) > time.Minute*5 +} + func (s *Service) SetUsingOrg(ctx context.Context, cmd *user.SetUsingOrgCommand) error { getOrgsForUserCmd := &org.GetUserOrgListQuery{UserID: cmd.UserID} orgsForUser, err := s.orgService.GetUserOrgList(ctx, getOrgsForUserCmd) diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 4f3b87d23b4..b66c8996dd6 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -288,3 +289,29 @@ func (f *FakeUserStore) Search(ctx context.Context, query *user.SearchUsersQuery func (f *FakeUserStore) Count(ctx context.Context) (int64, error) { return 0, nil } + +func TestUpdateLastSeenAt(t *testing.T) { + userStore := newUserStoreFake() + orgService := orgtest.NewOrgServiceFake() + userService := Service{ + store: userStore, + orgService: orgService, + cacheService: localcache.ProvideService(), + teamService: &teamtest.FakeService{}, + } + userService.cfg = setting.NewCfg() + + t.Run("update last seen at", func(t *testing.T) { + userStore.ExpectedSignedInUser = &user.SignedInUser{UserID: 1, OrgID: 1, Email: "email", Login: "login", Name: "name", LastSeenAt: time.Now().Add(-10 * time.Minute)} + err := userService.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{UserID: 1, OrgID: 1}) + require.NoError(t, err) + }) + + userService.cacheService.Flush() + + t.Run("do not update last seen at", func(t *testing.T) { + userStore.ExpectedSignedInUser = &user.SignedInUser{UserID: 1, OrgID: 1, Email: "email", Login: "login", Name: "name", LastSeenAt: time.Now().Add(-1 * time.Minute)} + err := userService.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{UserID: 1, OrgID: 1}) + require.ErrorIs(t, err, user.ErrLastSeenUpToDate, err) + }) +}