From bd753d9cb9467ccfd2370e7687a841ce17d37fa2 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Wed, 17 Dec 2025 16:49:40 +0400 Subject: [PATCH] fix: mark users seen when activating on login (#21305) fixes #21303 Update user last_seen_at when we mark them active on login. This prevents a narrow race where they can be re-marked dormant and fail to log in. --- coderd/database/dbgen/dbgen.go | 7 ++++--- coderd/database/queries.sql.go | 18 +++++++++++++----- coderd/database/queries/users.sql | 4 +++- coderd/userauth.go | 14 ++++++++------ coderd/users.go | 7 ++++--- enterprise/coderd/scim.go | 19 +++++++++++-------- 6 files changed, 43 insertions(+), 26 deletions(-) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index c1051c5754..d2f2bd94b2 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -580,9 +580,10 @@ func User(t testing.TB, db database.Store, orig database.User) database.User { require.NoError(t, err, "insert user") user, err = db.UpdateUserStatus(genCtx, database.UpdateUserStatusParams{ - ID: user.ID, - Status: takeFirst(orig.Status, database.UserStatusActive), - UpdatedAt: dbtime.Now(), + ID: user.ID, + Status: takeFirst(orig.Status, database.UserStatusActive), + UpdatedAt: dbtime.Now(), + UserIsSeen: false, }) require.NoError(t, err, "insert user") diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c803d56936..d440f0178d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -17141,19 +17141,27 @@ UPDATE users SET status = $2, - updated_at = $3 + updated_at = $3, + -- If the user is logging in, set last_seen_at to updated_at. + last_seen_at = CASE WHEN $4 :: boolean THEN $3 :: timestamptz ELSE last_seen_at END WHERE id = $1 RETURNING id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, login_type, avatar_url, deleted, last_seen_at, quiet_hours_schedule, name, github_com_user_id, hashed_one_time_passcode, one_time_passcode_expires_at, is_system ` type UpdateUserStatusParams struct { - ID uuid.UUID `db:"id" json:"id"` - Status UserStatus `db:"status" json:"status"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + ID uuid.UUID `db:"id" json:"id"` + Status UserStatus `db:"status" json:"status"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + UserIsSeen bool `db:"user_is_seen" json:"user_is_seen"` } func (q *sqlQuerier) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) { - row := q.db.QueryRowContext(ctx, updateUserStatus, arg.ID, arg.Status, arg.UpdatedAt) + row := q.db.QueryRowContext(ctx, updateUserStatus, + arg.ID, + arg.Status, + arg.UpdatedAt, + arg.UserIsSeen, + ) var i User err := row.Scan( &i.ID, diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 889e99a330..1107eaa29a 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -325,7 +325,9 @@ UPDATE users SET status = $2, - updated_at = $3 + updated_at = $3, + -- If the user is logging in, set last_seen_at to updated_at. + last_seen_at = CASE WHEN @user_is_seen :: boolean THEN $3 :: timestamptz ELSE last_seen_at END WHERE id = $1 RETURNING *; diff --git a/coderd/userauth.go b/coderd/userauth.go index adf150461e..bab159b18c 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -648,9 +648,10 @@ func ActivateDormantUser(logger slog.Logger, auditor *atomic.Pointer[audit.Audit //nolint:gocritic // System needs to update status of the user account (dormant -> active). newUser, err := db.UpdateUserStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateUserStatusParams{ - ID: user.ID, - Status: database.UserStatusActive, - UpdatedAt: dbtime.Now(), + ID: user.ID, + Status: database.UserStatusActive, + UpdatedAt: dbtime.Now(), + UserIsSeen: true, }) if err != nil { logger.Error(ctx, "unable to update user status to active", slog.Error(err)) @@ -1799,9 +1800,10 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C dormantConvertAudit.Old = user //nolint:gocritic // System needs to update status of the user account (dormant -> active). user, err = tx.UpdateUserStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateUserStatusParams{ - ID: user.ID, - Status: database.UserStatusActive, - UpdatedAt: dbtime.Now(), + ID: user.ID, + Status: database.UserStatusActive, + UpdatedAt: dbtime.Now(), + UserIsSeen: true, }) if err != nil { logger.Error(ctx, "unable to update user status to active", slog.Error(err)) diff --git a/coderd/users.go b/coderd/users.go index 94d4dece24..0e64757724 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -885,9 +885,10 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW } targetUser, err := api.Database.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ - ID: user.ID, - Status: status, - UpdatedAt: dbtime.Now(), + ID: user.ID, + Status: status, + UpdatedAt: dbtime.Now(), + UserIsSeen: false, }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index d6bb6b368b..5d0b248abd 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -256,8 +256,9 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { newUser, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ ID: dbUser.ID, // The user will get transitioned to Active after logging in. - Status: database.UserStatusDormant, - UpdatedAt: dbtime.Now(), + Status: database.UserStatusDormant, + UpdatedAt: dbtime.Now(), + UserIsSeen: false, }) if err != nil { _ = handlerutil.WriteError(rw, err) // internal error @@ -395,9 +396,10 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { if dbUser.Status != newStatus { //nolint:gocritic // needed for SCIM userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ - ID: dbUser.ID, - Status: newStatus, - UpdatedAt: dbtime.Now(), + ID: dbUser.ID, + Status: newStatus, + UpdatedAt: dbtime.Now(), + UserIsSeen: false, }) if err != nil { _ = handlerutil.WriteError(rw, err) // internal error @@ -490,9 +492,10 @@ func (api *API) scimPutUser(rw http.ResponseWriter, r *http.Request) { if dbUser.Status != newStatus { //nolint:gocritic // needed for SCIM userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ - ID: dbUser.ID, - Status: newStatus, - UpdatedAt: dbtime.Now(), + ID: dbUser.ID, + Status: newStatus, + UpdatedAt: dbtime.Now(), + UserIsSeen: false, }) if err != nil { _ = handlerutil.WriteError(rw, err) // internal error