Skip to content

Commit bd753d9

Browse files
spikecurtisspikecurtis
authored
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.
1 parent f9087d6 commit bd753d9

File tree

6 files changed

+43
-26
lines changed

6 files changed

+43
-26
lines changed

‎coderd/database/dbgen/dbgen.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -580,9 +580,10 @@ func User(t testing.TB, db database.Store, orig database.User) database.User {
580580
require.NoError(t, err, "insert user")
581581

582582
user, err = db.UpdateUserStatus(genCtx, database.UpdateUserStatusParams{
583-
ID: user.ID,
584-
Status: takeFirst(orig.Status, database.UserStatusActive),
585-
UpdatedAt: dbtime.Now(),
583+
ID: user.ID,
584+
Status: takeFirst(orig.Status, database.UserStatusActive),
585+
UpdatedAt: dbtime.Now(),
586+
UserIsSeen: false,
586587
})
587588
require.NoError(t, err, "insert user")
588589

‎coderd/database/queries.sql.go‎

Lines changed: 13 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/users.sql‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,9 @@ UPDATE
325325
users
326326
SET
327327
status = $2,
328-
updated_at = $3
328+
updated_at = $3,
329+
-- If the user is logging in, set last_seen_at to updated_at.
330+
last_seen_at = CASE WHEN @user_is_seen :: boolean THEN $3 :: timestamptz ELSE last_seen_at END
329331
WHERE
330332
id = $1 RETURNING *;
331333

‎coderd/userauth.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -648,9 +648,10 @@ func ActivateDormantUser(logger slog.Logger, auditor *atomic.Pointer[audit.Audit
648648

649649
//nolint:gocritic // System needs to update status of the user account (dormant -> active).
650650
newUser, err := db.UpdateUserStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateUserStatusParams{
651-
ID: user.ID,
652-
Status: database.UserStatusActive,
653-
UpdatedAt: dbtime.Now(),
651+
ID: user.ID,
652+
Status: database.UserStatusActive,
653+
UpdatedAt: dbtime.Now(),
654+
UserIsSeen: true,
654655
})
655656
if err != nil {
656657
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
17991800
dormantConvertAudit.Old = user
18001801
//nolint:gocritic // System needs to update status of the user account (dormant -> active).
18011802
user, err = tx.UpdateUserStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateUserStatusParams{
1802-
ID: user.ID,
1803-
Status: database.UserStatusActive,
1804-
UpdatedAt: dbtime.Now(),
1803+
ID: user.ID,
1804+
Status: database.UserStatusActive,
1805+
UpdatedAt: dbtime.Now(),
1806+
UserIsSeen: true,
18051807
})
18061808
if err != nil {
18071809
logger.Error(ctx, "unable to update user status to active", slog.Error(err))

‎coderd/users.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -885,9 +885,10 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW
885885
}
886886

887887
targetUser, err := api.Database.UpdateUserStatus(ctx, database.UpdateUserStatusParams{
888-
ID: user.ID,
889-
Status: status,
890-
UpdatedAt: dbtime.Now(),
888+
ID: user.ID,
889+
Status: status,
890+
UpdatedAt: dbtime.Now(),
891+
UserIsSeen: false,
891892
})
892893
if err != nil {
893894
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{

‎enterprise/coderd/scim.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,9 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
256256
newUser, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
257257
ID: dbUser.ID,
258258
// The user will get transitioned to Active after logging in.
259-
Status: database.UserStatusDormant,
260-
UpdatedAt: dbtime.Now(),
259+
Status: database.UserStatusDormant,
260+
UpdatedAt: dbtime.Now(),
261+
UserIsSeen: false,
261262
})
262263
if err != nil {
263264
_ = handlerutil.WriteError(rw, err) // internal error
@@ -395,9 +396,10 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) {
395396
if dbUser.Status != newStatus {
396397
//nolint:gocritic // needed for SCIM
397398
userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
398-
ID: dbUser.ID,
399-
Status: newStatus,
400-
UpdatedAt: dbtime.Now(),
399+
ID: dbUser.ID,
400+
Status: newStatus,
401+
UpdatedAt: dbtime.Now(),
402+
UserIsSeen: false,
401403
})
402404
if err != nil {
403405
_ = handlerutil.WriteError(rw, err) // internal error
@@ -490,9 +492,10 @@ func (api *API) scimPutUser(rw http.ResponseWriter, r *http.Request) {
490492
if dbUser.Status != newStatus {
491493
//nolint:gocritic // needed for SCIM
492494
userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
493-
ID: dbUser.ID,
494-
Status: newStatus,
495-
UpdatedAt: dbtime.Now(),
495+
ID: dbUser.ID,
496+
Status: newStatus,
497+
UpdatedAt: dbtime.Now(),
498+
UserIsSeen: false,
496499
})
497500
if err != nil {
498501
_ = handlerutil.WriteError(rw, err) // internal error

0 commit comments

Comments
 (0)