Persitent login for some roles only

Feature request != critical issue

I would say "won't fix" because you're anonymous when you have the option to choose "Remember me" or not. And if you were to ignore that option per role, that would be really confusing.

I can see an argument for disallowing PL for admin accounts. That way you can be sure, for example, that if the admin's notebook computer is stolen, the thief cannot access the site without knowing the password.

I'm not too worried about the usability issue. The 'remember me' checkbox would only not work for admins, and we can display a message saying "sorry, PL is configured not to work for you" or something.

So, I'd accept a patch for this.

Anything here better rolled for D6 version first.

Ok, let get back to this one.

hmm... trying to follow up on latest Barry's comment...

Still I'm not convinced this option should be added. Isn't it easier to not check the "Remember me" option when login from my laptop? That way I'm sure no one else but me can login from my laptop as I'll always need to provide the password.

Let me mark this issue as "won't fix". Please, re-open if you can provide a use case where it really makes sense to disallow PL for admins. Thanks

I would like the functionality described in the ticket. I run a school website. The user roles include students, parents, faculty. We would like to encourage parents and students to stay logged in so they get personalized content that is not very confidential. Faculty, however, have access to a lot of more sensitive information and we do not want them to be able to have a working persistent login.

This is not really indented to be a patch to the module, but it is an example of how you can achieve the described functionality on your site. This is what we do.

Let me set it back to "won't fix".

Note that #6 could be implemented from a custom module that implements hook_form_alter() in 2 steps: 1) alter the form elements to add/change description, 2) inject a validate callback that can check the roles of the user and ignore the "Remember me" check box when a match is found.

I'd like to re-open this.

I think that it would be helpful from a security POV to allow only low-level roles to use this feature.

I would not want to allow the user the decision by clicking the box. In one case, I've had to make the box default to true and hide it (client request...) to make the login form "simpler".

At the very least a drupal_alter should be added in _persistent_login_check to allow other modules to deny the login.

Moving feature request to 8.x

Created a patch which adds role restriction.

Some feedback on a first pass review:
---
1.

+    $allRoles = Role::loadMultiple();

In the configuration form I would prefer to use dependency injection rather than a static method.

2.
The TokenManager class only handles storing/retrieving/validating/deleting tokens from the database. Checking against valid roles should probably happen in TokenHandler::setNewSessionToken()

3. Any services classes (TokenHandler or TokenManager) must use dependency injection rather than static methods such as User::load($uid)

4.
Roles should be filtered when configuration is saved, not when roles are checked at login.

5. It's probably possible to use something like array_intersect() to check if there is a match between the restricted roles and the roles of the user that is logging in rather than a loop.

6. The configuration schema needs to be updated to include the new keys, the default configuration file needs to be updated with the default values for those keys, and an update hook needs to set default values for existing sites.

@gapple Thanks for the review. Created a new patch with fixed issues that you found.

I applied and checked patch #12. It works there is just one problem, that I get the following error after running functional test

2) Drupal\Tests\persistent_login\Functional\PersistentLoginTest::testPersistentLogin with data set #1 (true, 'The user should be logged in ...ssion.')
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for persistent_login.settings with the following errors: persistent_login.settings:roles variable type is integer but applied schema class is Drupal\Core\Config\Schema\Sequence

Problem is in settings file (default value for roles):
roles: 0
which should be

roles:
  -

because in schema definition roles is sequence. After correcting settings file test passes.

Here's @nkoporec's patch with tiny change that passes the test.

Thanks #14 will fix the problem

Tested the patch and works ok.

One thing that I think it must be added/changed. Let say that persistent login is enabled for an editor role (just for example) and then after some time admin disable persistent login for this role. The problem is that cookie can still exist (if it's not yet expired) when user will log-in next time and thus he will see the information about persistent logins on his profile, so I suggest that when a user logs out we check if user role is disabled and if the cookie exists, then we can remove cookie. The only problem that I see atm is with users that have multiple roles.

@denisveg Thanks for the comment, that's an important consideration that tokens are invalidated if necessary when config is updated. If the restricted roles are changed, a batch operation could check any existing tokens in the database to ensure that none are assigned to users with any of the restricted roles, and remove them as necessary.
The token cookie on the user side won't be a problem: when the user next accesses the site their token will fail validation because it isn't found in the database, the browser will be told to delete the invalid cookie, and the user will remain logged out of the site. It's the same process that occurs for a token that has expired due to surpassing its lifetime.

Access to the persistent login page for a user is currently controlled by checking if the current user has edit permissions, so that an administrator can also view or revoke tokens for any user. The page should probably return a 404, and the menu task not be visible for users who are restricted from creating persistent logins. I'm fine leaving this part to a followup issue.

I'm not sure if Batch API will be usable in this instance - to handle cases where configuration is changed through an import instead of directly through the admin form, tokens should be checked on Config import / save events.

A large number of tokens could risk timeouts if config is imported through the web UI, but I expect that any site that would have enough users / sessions / tokens to risk timeouts would be running config imports through Drush where timeouts shouldn't be a concern.