Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add "dormant" user state #8644

Merged
merged 80 commits into from
Aug 2, 2023
Merged

feat: add "dormant" user state #8644

merged 80 commits into from
Aug 2, 2023

Conversation

mtojek
Copy link
Member

@mtojek mtojek commented Jul 21, 2023

Related: #8128

This PR officially introduces another user account status - dormant. Users who have not logged in yet to the Coder platform will be marked as dormant, and will not be included in the license seat calculations.

Screenshot 2023-07-27 at 14 55 39 Screenshot 2023-07-27 at 14 55 56

Low-level changes:

  • database: define dormant user status
  • coderd: modify business logic to automatically switch user account to active once they log in
  • coderd: adjust login&password worfklow to activate a dormant account
  • coderd: tweak ExtractAPIKeyMW to switch a "dormant" account to "active"
  • coderd: fix unit tests that use active accounts by default
  • site: add "Learn more" menu item to navigate to docs
  • cli: support "dormant" accounts
  • cli: refresh testdata (scaletest commands were stale)
  • docs: describe user statuses

Testing:

  • GitHub login using custom Github App for testing purposes
  • OpenID using custom Auth0 domain and @coder.com Google account

Notes:

  • I didn't touch SCIM API as I'm not sure if this is something we should touch? SCIM supports only active/inactive state.

Next steps:

  • Configure a periodic job to mark inactive accounts (90 days) as dormant

@mtojek mtojek marked this pull request as ready for review August 2, 2023 10:10
@mtojek mtojek requested a review from johnstcn August 2, 2023 10:10
Comment on lines 48 to 63
require.Eventually(t, func() bool {
rows, err = db.GetUsers(ctx, database.GetUsersParams{})
if err != nil {
return false
}

var c int
for _, row := range rows {
if row.Status == database.UserStatusDormant {
c++
}
}
// 6 users in total, 3 dormant
return len(rows) == 6 && c == 3
}, testutil.WaitShort, testutil.IntervalMedium)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: if you allow passing in a tick chan time.Time and updated chan int into CheckInactiveUsersWithOptions you can then conceivably remove this require.Eventually - you write a time to tick, and read the number of updated users from updated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good hint in case we want to do something more with these accounts than validate in unit tests. I would keep it simple as is.

SET
user_status = 'dormant'::user_status
WHERE
last_seen_at < @last_seen_after :: timestamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for readers: this is correct as last_seen_at is timestamp without time zone.

docs/admin/users.md Outdated Show resolved Hide resolved
@mtojek mtojek merged commit d6e9870 into main Aug 2, 2023
22 of 24 checks passed
@mtojek mtojek deleted the 8128-new-user-state-dormant branch August 2, 2023 14:31
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants