Remove inactive users #305

pjcon · 2021-09-20T13:52:04Z

Users who have a lastLoginDate from more than 18 months ago will be removed.

Also catch the case where users who have not logged in since the addition of lastLoginDate, which means they don't have a lastLoginDate. They are also removed.

~~Depends on~~ #171 is now merged into dev
~~Merge after #284~~

resources/RemoveInactiveUsersRunner.php

gregcorbett · 2021-10-18T10:00:27Z

some of the commits can be squashed, as they seem to comment-out and un-comment-out bits of code.

pjcon · 2021-10-18T15:20:41Z

some of the commits can be squashed, as they seem to comment-out and un-comment-out bits of code.

How far should I go to squash commits? I've mixed in some commits together while working, and I figure it'd be faff to pick them apart.

gregcorbett · 2021-10-19T08:34:48Z

How far should I go to squash commits? I've mixed in some commits together while working, and I figure it'd be faff to pick them apart.

You can probably go quite far with the squashing, along the lines of:

Basic Script
Add functionality to email users
handle really old users without a last login

gregcorbett · 2022-05-27T09:43:05Z

I've rebased this on top of the current dev so I can see how it works with ID linking.

ineilson · 2022-06-16T09:29:45Z

#284 doesn't provided a means of cleaning up old/inactive API authentication credentials does it? Maybe in the short term we refuse to delete a user with an API authentication credential.

No, it forces the reassignment/deletion of the API credential before the user can be removed.

Short term: that sounds good given the limited API cred usage, but there should be some monitoring/log of failures from the automated script to allow follow-up.

gregcorbett · 2022-07-07T07:34:42Z

As a note to future Greg,

gocdb/lib/Gocdb_Services/User.php

Line 441 in f780966

if (!$user->getAPIAuthenticationEntities()->isEmpty()) {

should be factored out into a method that the runner makes use of.

gregcorbett · 2022-07-29T14:56:24Z

Need to rebase this to pick up the changes from #284.

- GOCDB admins email is better because it doesn't matter what view the user uses. - it's also consistent with other mails sent (e.g. role requests, ID linking).

- to be more consistent with itself and other emails sent

- for consistency and PSR-12 compliance

- as the time this script starts is already captured above.

- we don't want to return here (and skip the rest of the users), we want to continue on to the next user.

- As this script is stateless and might be run weekly, hence a used will get ~4 warnings and we don't want each one saying "30 days".

Co-authored-by: ineilson <ian.neilson@stfc.ac.uk>

- so that they can be changed in production based on how frequently the script runs.

- to prevent deleting the user and creating orphaned API credentials.

gregcorbett · 2022-07-29T15:16:13Z

There is now (#284

gocdb/lib/Gocdb_Services/User.php

Line 441 in 2bccf16

if (!$user->getAPIAuthenticationEntities()->isEmpty()) {

@ineilson: I've address the above now - in a97fc22

@tofu-rocketry: Following our discussions about how frequently this should run / how much warning a user should have get, I've softcoded thresholds for warning and deletion - in e86f594

ineilson

Looks okay. If a deletion fails for any user for some reason the whole thing stops. That's intended behaviour?

gregcorbett · 2022-08-02T07:43:35Z

Looks okay. If a deletion fails for any user for some reason the whole thing stops. That's intended behaviour?

maybe "intended" is to strong a word, but it feels like an okay thing to do.

pjcon requested a review from a team as a code owner September 20, 2021 13:52

pjcon force-pushed the cjp/remove-inactive-users branch from 7c70a86 to 44ead9c Compare September 20, 2021 15:15

gregcorbett self-assigned this Sep 20, 2021

gregcorbett added the enhancement label Sep 20, 2021

gregcorbett added this to the 5.10.0 milestone Sep 20, 2021

gregcorbett reviewed Sep 20, 2021

View reviewed changes