Problem/Motivation

For security reasons, we would like to be able to expire roles for inactive users, separate from the "standard" role expiry.

Proposed resolution

Add a field to the "edit role" functionality (where "default duration" already lives) for "Inactive expiry for the role [rolename]". Help text could be something like "Enter a time span you want to set as the duration this role should be preserved from a user's last login, or leave blank to not expire this role due to inactivity." (followed by the same examples as "default duration")

When setting the role expiry when this is set, for the "leave the field blank" option:

  • if there is no default duration, instead of saying "this role will never expire" say something like "the role will only expire if the user is inactive for [period set]"
  • If there is a default duration, add at the end "or if the user is inactive for [period set], whichever comes first"

Remaining tasks

  1. Decide if this is a feature this module wants to take on.
  2. Decide if the proposed implementation is acceptable.
  3. Implement it. :)

User interface changes

See proposed resolution.

Issue fork role_expire-3321995

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kpaxman created an issue. See original summary.

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

1. It would be nice to have this feature.
2. Proposed implementation sounds good, however once we deep in details we could change mind.
3. I maintain this module on free time. So patches are welcome ;)

kuzyawkk’s picture

kuzyawkk commented
Version: 2.x-dev » 4.x-dev
Assigned: Unassigned » kuzyawkk

This is still true for version 4.1. Therefore, I will try to implement this functionality

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Maybe this should live in a submodule as it's not part of the core functionality?

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

I agree, this should be a submodule.

kpaxman’s picture

kpaxman commented

I'd prefer it to be behind an "enable feature" checkmark, FWIW, as someone who is running way too many modules already. :)

kuzyawkk’s picture

kuzyawkk commented

I will have some free time over the weekend and will complete this task.
Yes, I also think it's better to put it in a submodule.

kuzyawkk’s picture

kuzyawkk commented

I'm sorry, I had a lot of unplanned work to do and didn't have any free time this weekend. I will definitely do it on the next one. But I would like to clarify if we are adding an “Inactive Expiration” field in the user creation form?

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

@kuzyawkk You don't have to apologize. Just do it when you can. And if you can't, just leave the issue unassigned.

On the other hand, as written in the issue description, we are adding another field in the role form (just under the current "Default duration for the role" field).

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

I thinks this feature is implemented on https://www.drupal.org/project/user_expire Maybe this issue it's just about linking to this User Expire module in the project's page.

kpaxman’s picture

kpaxman commented

Looking at the user_expire module page, but without trying it out, it seems that the difference is that that module locks the user out completely, where this request is just to remove a specified role. (For example, you want to expire their access to edit the site, but still be able to log in to fill in an authenticated form.)

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

@kpaxman You are right.

kuzyawkk opened merge request !21

kuzyawkk’s picture

kuzyawkk commented
Assigned: kuzyawkk » Unassigned
Status: Active » Needs review

I have implemented the logic, but I'm sure I could have made a mistake somewhere or overlooked certain conditions.

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented
Status: Needs review » Needs work
  1. I've improved UI texts
  2. I've tested it and it doesn't work yet. After running cron, roles doesn't expire for inactive users. Feature should be fixed and tests added. Maybe we would need a hook_update to set inactive_expiry_timestamp column for existing users
  3. We need to handle submodule uninstall: remove new expiration column
  4. The new field is not about duration, is about inactivity period, so function comments and variables names should be changed.
kuzyawkk’s picture

kuzyawkk commented

Yes, that's good. I'll take a look at it over the weekend and make some adjustments.

_tarik_’s picture

_tarik_ commented
Assigned: Unassigned » _tarik_

I believe I can start working on this task since kuzyawkk hasn’t been active here for 10 months.

_tarik_’s picture

_tarik_ commented
Assigned: _tarik_ » Unassigned
Status: Needs work » Needs review

The changes here are finished and ready for review.
Please check them and let me know if any further changes are needed or if you have any questions.

About changes:

In this issue, I refactored the submodule logic to reduce code duplication. Originally, the submodule used code copied from the role_expire module, with only minor changes to method names or variables. I removed all unused code from the submodule and created base classes to enable code reuse.

The module logic was fully rewritten. The module version before the commit 46ec1457 used the copied hook_cron from the main module. As a result, we had 2 places for roles serving that were almost totally the same. I’ve decided that it would be better to keep only one point for role deals. So, the submodule keeps hook_cron, but instead of removing roles there, it marks them for removal.

Another change is the inactive date handling. The early submodule version provided an additional database column and did some additional operations to keep it updated. In the new version, it was removed and replaced with the code that relies on the login column from the users_field_data.

Code is fully covered with Kernel/Functional tests.

The code doesn’t modify the role_expire module. It only adds 2 base classes for future reuse.

How to test:
1. Go to the role edit or creation page.
2. Fill in the “Inactivity period for role expiration“
3. Create a user who was inactive for a longer period than configured.
4. Run cron. Result: The user still has a role, but was added to the role_expire table.
5. Run cron again. Result: The user lost the role.