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

Adds end-to-end feature for archiving and deleting user accounts #1449

Merged
merged 19 commits into from
Sep 13, 2023

Conversation

agosmou
Copy link
Member

@agosmou agosmou commented Sep 11, 2023

Summary

This PR solves #1395
Implemented an end-to-end feature, creating a workflow that allows security admins to archive, un-archive/restore, and delete users along with their corresponding projects.

Features

  • Archiving Users: Security admins can now archive users.
  • Un-archiving/Restoring Users: Security admins can also un-archive and restore users.
  • Deleting Users and Projects: Security admins can permanently delete users and the user's respective projects.
  • UI Enhancements: Added buttons and confirmation modals to ensure admins understand the implications of their actions.

Technical Details

  • Created a migration script to alter the tables "Login" and "Project" to add an archivedAt column that is set to NULL by default or to a timestamp if queried
  • Created new, protected API endpoints:
    • Accounts:
      • PUT /api/accounts/:id/archiveaccount
      • PUT /api/accounts/:id/unarchiveaccount
      • GET /api/accounts/archivedaccounts
      • DELETE /api/accounts/:id/deleteaccount
    • Projects:
      • GET /api/projects/archivedprojects
  • Added new components in the frontend to handle the workflow.
  • added new frontend routes to show all archived users or all archived projects

How to Test

  1. Login as a security admin.
  2. Navigate to the /Roles page.
  3. Next to each user, there should be an 'options' button(vertical ellipsis).
  4. Clicking the 'options' button will render a PopUp that can be clicked to archive the user. This will populate the archivedAt field for the user and their projects; thus, 'archiving' the user. Security Admins cannot be archived or deleted. The user is also not able to self-archive or self-delete.
  5. To see the archived users, navigate to the /archivedaccounts by following the link. Here, the security admin can 'un-archive and restore' a user OR permanently delete by clicking on the arrow or trash icons and their respective PopUp's. These actions will take effect on the user and their corresponding projects.
  6. To see all archived projects, navigate to the /archivedprojects by following the link. This page only shows all archived projects - no actions. To return to the /Roles page, follow the link on the top of the page.

Feature Notes

  • Deleting a user is irreversible - there is a currently a multi-step-process to delete a user to prevent accidental removal.
    1. select 'options'
    2. use popup to confirm user is to be archived
    3. Visit the archive page
    4. select 'delete'
    5. use popup to confirm user is to be permanently deleted
  • Only Security Admins can archive, un-archive/restore, or delete users.
  • The logged-in user cannot self-archive or self-delete
  • Archived users cannot login

Future Improvements

  • We could consider adding 2-Factor Auth such as email confirmation or PIN to delete users

Visuals

firefox_uOiRVYKpGB

@agosmou
Copy link
Member Author

agosmou commented Sep 11, 2023

Branch Notes

  • I developed using a local db so the migration must be run on the shared development db
  • 'prettier' was giving me tons of issues with line-endings as I developed on my windows machine so I made a fix on the /server and /client directories to mark the endOfLine property to auto
  • The migration I did, using my local db, caused a large change in the /server/report.html and /server/report.json files (~6,000 lines)

@agosmou agosmou added enhancement Release Note: Shows as visual or user experience Enhancement role: front-end Front End Developer role: back-end Node/Express Development Task role: database Database Development Task priority: MUST HAVE feature: API labels Sep 11, 2023
@agosmou agosmou added this to the 02 - Security milestone Sep 11, 2023
@agosmou agosmou linked an issue Sep 11, 2023 that may be closed by this pull request
2 tasks
@agosmou agosmou self-assigned this Sep 11, 2023
@agosmou agosmou marked this pull request as ready for review September 12, 2023 00:33
@agosmou
Copy link
Member Author

agosmou commented Sep 12, 2023

Ready for review!

Do note the comment above:

Branch Notes

* I developed using a local db so the migration must be run on the shared development db

* 'prettier' was giving me tons of issues with line-endings as I developed on my windows machine so I made a fix on the /server and /client directories to mark the endOfLine property to `auto`

* The migration I did, using my local db, caused a large change in the `/server/report.html` and `/server/report.json` files (~6,000 lines)

Copy link
Member

@entrotech entrotech left a comment

Choose a reason for hiding this comment

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

This PR is FANTASTIC! I ran the migration against the shared dev db and it worked great, then tested all the features you implemented and they all worked perfectly, as far as I can tell.

There is a bit of overlap of your project archive feature with a different Issue that allows users to move their own projects to the trash and eventually delete them, but the purpose is a bit different, so we need to figure out if the admin delete/archive and user delete/archive for projects can just remain separate or not.

I had seen the report.html and report.json files crop up before, but didn't look into where they came from. It turns out the Flyway, the DB migration tool auto-generates these files. I read enough to know that they document the migration changes and you can somehow run reports on the migrations using them as source data, but we might look into their utility and see if we want to use them and figure out how to disable their generation if we don't. If we do want to use them, we should probably change the flyway config to give them better names, and tuck them away in the /server/db folder so they are out of the way.

Karandeep was also having trouble with line endings in Windows. We can talk about the relevant settings tonight.

I'm sure the UI designers will want to make some tweaks to the UI appearance, but the functionality is all there.

@entrotech entrotech merged commit e48ecb7 into develop Sep 13, 2023
shanesween pushed a commit that referenced this pull request Sep 29, 2023
* adds db migration file

* adds backend feature to soft del and hard del users with their projects

* adds endpoints to client

* fixes broken endpoints

* adds functionality to prevent archived users logging in

* refactors migration and account service

* adds frontend user feedback for archived users on log in

* fixes functionality to retrieve all archived projects by security admin

* edits migration for selecting all archived projects

* adds db migration and service for unarchiving user

* adds route and controller for unarchive endpoint

* adds controller for unarchive endpoint

* fixes unarchiveUser service

* adds code to prevent self archive and self delete

* adds frontend functionality to archive and delete users

* updates migration reports

* small fix to address line endings issues when working on branches between windows and unix systems

* automatic linting fixes by lerna

* fixes lerna issues, refactors, lints
@agosmou agosmou deleted the delete-api-2 branch November 11, 2023 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Release Note: Shows as visual or user experience Enhancement feature: API priority: MUST HAVE role: back-end Node/Express Development Task role: database Database Development Task role: front-end Front End Developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a "Delete User" API Endpoint for Security Admin Only
2 participants