cu-uis / cu-starterkit-project

Development repository for Pantheon's recommended (future) Drupal 9+ upstream
1 stars 2 forks source link

Feature - As a Developer, I want all AUs logged out nightly so that we can communicate updates more effectively #37

Open kreynen opened 2 years ago

kreynen commented 2 years ago

Is your feature request related to a problem? Please describe.

Web Express included this feature along with https://websupport.colorado.edu/article/304-user-dashboard.

Describe the solution you'd like

https://www.drupal.org/project/force_users_logout was added to the online.cu.edu code, but never enabled. We are now co-maintainers of the project on Drupal.org, but never prioritized https://www.drupal.org/project/force_users_logout/issues/3199921

kreynen commented 2 years ago

The method used in https://www.drupal.org/project/autologout is to use client side js in https://git.drupalcode.org/project/autologout/-/blob/8.x-1.x/js/autologout.js#L201 to make a callback to route that uses https://git.drupalcode.org/project/autologout/-/blob/8.x-1.x/src/AutologoutManager.php#L193 to log the user out. Autologout is using SessionManager to delete the session, but only on a per user basis.

The code for force_user_logout was refactored 3 months ago in https://www.drupal.org/project/force_users_logout/issues/3108854#comment-14401002 so https://git.drupalcode.org/project/force_users_logout/-/blob/2.0.x/src/Form/AllOtherUsersLogoutForm.php#L96 is now much cleaner, but still using a query to get all the users and then looping through and checking if the user is logged in. If they are it is using $session_manager->delete() to log the user out.

I'm going to update the https://git.drupalcode.org/project/force_users_logout/-/merge_requests/2 branch to see if I can get this working after the refactor.

kreynen commented 2 years ago

I think I figured it out. The issue is likely the use of entityTypeManager while running within cron as an as anonymous user.

https://www.drupal.org/forum/support/module-development-and-code-questions/2021-03-17/entity-query-not-working-in-all#comment-14035328

kreynen commented 2 years ago

@meschenbaum we need to decide if it makes more sense to add the same type of cron worker to the smaller module with fewer installs and have all users logged out at 2AM or just use the existing functionality of a larger module that is really designed for a different use case, but is installed on 40K sites.

Setting the auto logout duration to something like 6 hours would likely mean that everyone is likely logged out nightly. If you logged in at 10PM, you'd be logged out ~4AM. The problem is if we want to schedule major changes to run automatically on off hours like 3AM, we can't be sure no one is logged in.

https://www.drupal.org/project/force_users_logout/issues/3199921#comment-14531278

meschenbaum commented 2 years ago

@kreynen I would prefer to go with the smaller module. I think it is cleaner and does not added bloat or complexities that don't apply.

If you can get a hook added to the force user logout module, that would be excellent.

kreynen commented 2 years ago

@meschenbaum I help to co-maintains https://www.drupal.org/project/force_users_logout... which is why CU is listed on project page and it is listed in https://www.drupal.org/university-of-colorado#projects-supported. Technically we can take that project in any direction we want (as long as the other maintainers buy-in).

The question isn't just what we prefer. That preferences is tied to how many of these project I alone maintain. If we use https://www.drupal.org/project/autologout, I don't have to do any additional contrib work. If I write the cron worker, someone needs to write the tests for the cron worker and do the work required to help maintain this new functionality.

I too prefer cleaner solutions, but they require more work.

meschenbaum commented 2 years ago

@kreynen I hear you there, didn't realize you were a co-maintainer. Then I agree about not taking on more work.

I think it will be fine to go with the autologout module and just contrib the cron worker. It may take some time before the cron worker is merged in, but shouldn't hold us up.

johnquest commented 2 years ago

I think setting a specific logout time is fine. There would be the message in websites that an update is going to happen but could there also be a communication sent out sometime prior via e-mail to users?

kreynen commented 2 years ago

Waiting on @mattopiajones to clarify how much (if any) time we should be spending contributing back to contrib projects on Drupal.org.