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

Disable user accounts script #2898

Merged

Conversation

maiyerlee
Copy link
Contributor

@maiyerlee maiyerlee commented Oct 8, 2024

What changed

  • Adds a Python script to deactivate inactive user accounts.
  • Changes are logged and user sessions, history, and event records are updated.
  • Creates a System Admin user.
  • Add an ADR for the container app job and celery spikes.
  • Add a disable inactive user service to the docker compose file.
  • Move audit functions used in ops db history to model file (to expose methods to the data import process).

Issue

#2756

How to test

  1. On startup, verify there is a disable-inactive-users image in the OPS container and a system user in the ops_user table.
  2. Update a user's last_active_at timestamp to over 60 days ago.
  3. Run the disable-inactive-users job.
  4. Check that all user sessions have the is_active flag set to false.
  5. Confirm the user's status is false and there’s a record in ops_db_history for this change.
  6. Ensure a UPDATE_USER event is logged.

Verify unit and e2e tests pass.

Screenshots

N/A

Definition of Done Checklist

  • OESA: Code refactored for clarity
  • OESA: Dependency rules followed
  • Automated unit tests updated and passed
  • Automated integration tests updated and passed
  • Automated quality tests updated and passed
  • Automated load tests updated and passed
  • Automated a11y tests updated and passed
  • Automated security tests updated and passed
  • 90%+ Code coverage achieved
  • Form validations updated

Links

N/A

@maiyerlee maiyerlee self-assigned this Oct 8, 2024
@maiyerlee maiyerlee marked this pull request as ready for review October 9, 2024 14:23
Copy link
Contributor

@fpigeonjr fpigeonjr left a comment

Choose a reason for hiding this comment

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

Looks great @maiyerlee but will defer to others. Glad we have this feature now.

@@ -0,0 +1,67 @@
# 26. Container Apps Jobs for Disabling Inactive Users
Copy link
Contributor

Choose a reason for hiding this comment

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

⭐ thx for adding the ADR

" WHERE ou.status = 'ACTIVE' "
" AND user_session.last_active_at < CURRENT_TIMESTAMP - INTERVAL '60 days'"
");"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

there it is!

@@ -0,0 +1,25 @@
EXCLUDED_USER_IDS = [520, 521, 522, 523, 525, 526]
Copy link
Contributor

Choose a reason for hiding this comment

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

These user ids will only reference fake users in lower environments - in PROD these will be real users. It would be better to use the oidc ids here to reference them. Same for the SYSTEM_USER_ID - that will only be the correct id in lower environments where test data is being loaded.

@jonnalley jonnalley requested a review from johndeange October 21, 2024 15:40
@maiyerlee maiyerlee merged commit a93f7ac into main Oct 21, 2024
40 checks passed
@maiyerlee maiyerlee deleted the 2756-implement-script-to-disable-user-accounts-in-aca-job branch October 21, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants