It would be nice if it were possible to place a site in or out of a "restricted-login" mode via drush, which I believe is in the spirit of this module.

This is already possible via drush + the permissions_api module, but this must be executed once for every role, by the role-name - cumbersome to say the least!

Perhaps commands like:

drush mh-restrict-logins --message "This site is undergoing maintenance; logins are restricted to maintenance accounts."
drush mh-allow-logins

CommentFileSizeAuthor
#13 914236-drush-support-13-D6.patch4.17 KBclaar
#10 914236-drush-support-10-D6.patch4.19 KBclaar
#8 914236-drush-support-8-D6.patch4.12 KBclaar

Comments

claar’s picture

claar commented
Issue tags: -drush, -maintenance mode

I realize I'm arguing with myself here, but the problem I have with a

  drush mh-restrict-logins

command unsetting every role's 'login to site' permission is that it overwrites the permissions previously set on the permissions page, creating an all-or-nothing state.

It strikes me that better would be to:

  • rename the 'login to site' permission to something like 'login during restricted-access', and
  • add a "restricted access mode" variable -- 1 for on, 0 for off

Then we would create a new checkbox on the settings form that would be used to place a site into a "restricted access mode", and the drush command becomes a simple setting of that variable.

One really nice side-effect of this "restricted access mode" method is that upon enabling this module, normal logins would *not* be affected as they are now, which I believe was the cause of the confusion in issue #366095: Provide README for Maintenance Helper.

I have created a new issue for this #915062: Add a restricted-access mode, since that is a separate feature from drush support.

claar’s picture

claar commented
Issue tags: +drush, +maintenance mode
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Issue tags: +drush, +maintenance mode

I think that if we have a few drush commands (I'm not sure if these exist or not):

  • drush list-roles "login to site"
  • drush role-grant-permission "admin" "login to site"
  • drush role-revoke-permission "authenticated" "login to site"

Between those three commands it should be possible to do everything you need, right?

I'm hesitant to add more complexity with the restricted mode idea.

claar’s picture

claar commented

Possible, yes, but not really usable.

I run 15 sites, soon to be 30+, each with many uniquely-named roles. Tonight, for example, we're going to be placing them *all* in "admin-only mode" during a server move. To do so, we type:

 # The @sites notation runs the command on all sites
 drush @sites admin-only-mode 1 --message "Logins are currently restricted to the admin user."

To do the equivalent with permission-based command would be incredibly painful.

 drush @site1 role-revoke-permission "role1" "login to site"
 drush @site1 role-revoke-permission "role2" "login to site"
 drush @site2 role-revoke-permission "roleA" "login to site"
 drush @site2 role-revoke-permission "roleB" "login to site"
   (repeat for n roles times n sites)
 drush @sites mh-set-message --login-page-only "Logins are currently restricted to the admin user."

It's not even in the same ball-park. So one thinks, "Perhaps a command could be written to revoke permissions from everyone at once!", getting us closer:

  drush @sites mh-revoke-all-login-perms --login-page-only --message="Logins are currently restricted to the admin user."

Oops, now we've just lost the permissions granularity wiping the "login to site" permission out every time we want to restrict logins via drush.

This is the line of reasoning that pushes me toward the "restricted-access" mode, which I actually believe simplifies the module from a usage and understanding stand-point, and is not complicated code-wise either.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

It simplifies the drush command, but not the general UI.

Maybe let's discuss more in irc so I can understand your needs and see if we can find a solution.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Ok, having re-read this I'm fine with the proposal in comment #2.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Right, I meant, comment #1! (the second post on this issue, but indeed the first comment).

claar’s picture

claar commented
Status: Active » Needs review
StatusFileSize
new 914236-drush-support-8-D6.patch4.12 KB

Attached is a patch against 6.x-2.x -- feedback appreciated! #914238: Configure maintenance and lockout message and visibility will require additional changes, which will be handled in that issue.

Usage: drush help mh-restricted-logins

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Needs work 
  } else {

should be

  } 
  else {

Ditto for the else ifs.

$clearcache doesn't seem to be used.

variable_set itself does a cache_clear, but just of the variables table. It seems like it would be good to add a code comment clarifying that the main intended cache to clear is the page cache.

claar’s picture

claar commented
StatusFileSize
new 914236-drush-support-10-D6.patch4.19 KB

Thanks for the excellent code review -- new patch attached.

claar’s picture

claar commented
Status: Needs work » Needs review

Forgot to set needs review.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Needs work

You are welcome :) A few more small things...

dt('Enable restricted logins and show the message \'Go Away\''),

I tend to do:

dt("Enable restricted logins and show the message 'Go Away'"),

I think it's easier for translations if you don't have to escape the '.

+  else if ($restrict_logins_setting === "0") {

Do you really need to check for both equal values and the same type of data? That seems pretty unlikely. There are a few cases of this.

claar’s picture

claar commented
Status: Needs work » Needs review
StatusFileSize
new 914236-drush-support-13-D6.patch4.17 KB

Addressed #12 -- thanks, greggles!

After discussion on IRC, decided to stick with === for the comparisons.

  echo ("01" == "1" ? "true" : "false");  # returns true -- ick

I also was calling drush_show_help with an Array argument incorrectly, causing help not to be shown. Fixed in attached patch.

claar’s picture

claar commented
Status: Needs review » Fixed

Committed #13 to 6.x-2.x

Status: Fixed » Closed (fixed)
Issue tags: -drush, -maintenance mode

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +drush, +maintenance mode

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.