Motivation:
Drush is for people who get shit done fast and understand edge cases. The login link is optimized for edge cases that slows down 90% of users (seriously #24398: password reset is not working in some cases).

Solution:
Just login when you click.

CommentFileSizeAuthor
#12 1829596_be_more_explicit_about_which_user_12.patch1.39 KBgreggles
#7 1829596_be_more_explicit_about_which_user.patch1.06 KBgreggles
#5 user-login-browse-refactor.patch14.65 KBOwen Barton
#1 1829596_uli_needs_a_turbo.patch415 bytesgreggles
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Active » Needs review
FileSize
1829596_uli_needs_a_turbo.patch415 bytes

Turbo boosted.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Fixed

Committed to master. Not committed to 5.x since thats technically an API change (I could be convinced otherwise).

greggles’s picture

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

Makes sense. There are cases where drush is used as an api for other things (e.g. Aegir) where the 10% case might matter.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic commented

In Aegir, we generate this login link by calling user_pass_reset_url() directly. But thanks for taking it into account.

Owen Barton’s picture

Owen Barton CreditAttribution: Owen Barton commented
Status: Fixed » Needs review
FileSize
user-login-browse-refactor.patch14.65 KB

More turbo - more flexibility in parameters, path redirection, browser detection/opening.

Owen Barton’s picture

Owen Barton CreditAttribution: Owen Barton commented
Status: Needs review » Closed (fixed)

Committed

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Closed (fixed) » Needs work
FileSize
1829596_be_more_explicit_about_which_user.patch1.06 KB

Holy cow that is a lot more turbo. Nice work.

The patch had

- drush_set_error("The user account with the name " . $name . " could not be loaded or is blocked!");
+ drush_set_error("The user account could not be loaded or is blocked!");
?>

That name was added explicitly in #1645788: if user is blocked throw a warning instead of returning a login link so I'd love to know a bit more why it was removed. My sense is that with the refactoring it's a bit trickier to say which user. I've tried to add back some information that I think makes sense here, but didn't test it a ton.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Needs work » Needs review
greg.1.anderson’s picture

greg.1.anderson CreditAttribution: greg.1.anderson commented
Status: Needs review » Needs work

Instead of $condition = 'could not be found and';, couldn't we just return drush_set_error here? The generated error message in this sense is a bit awkward, as the "or is blocked" clause is clearly not applicable if the user could not be found.

Owen Barton’s picture

Owen Barton CreditAttribution: Owen Barton commented
Status: Needs work » Needs review

I was having trouble coming up with reasonable sounding wording and working on the basis that the default of uid 1 is pretty well known (and in any other case is provided by the parameter you supply), but perhaps that is not a valid assumption.

No objection to adding it though, although the wording "The user account could not be found and could not be loaded or is blocked!" strikes me as still fairly hard to grok.

Owen Barton’s picture

Owen Barton CreditAttribution: Owen Barton commented
Status: Needs review » Needs work
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Needs work » Needs review
FileSize
1829596_be_more_explicit_about_which_user_12.patch1.39 KB

So, now that I've looked into _drush_user_get_users_from_options_and_arguments I see that it does it's own helpful drush_set_error in the case the account is not found so there's no need to handle that case in this function.

Seem good?

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
CreditAttribution: jonhattan commented
Status: Needs review » Needs work 
+++ b/commands/user/user.drush.inc
@@ -433,7 +438,7 @@ function drush_user_login($user = NULL, $path = NULL) {
+    drush_set_error("The user account " . $condition . " could not be loaded or is blocked!");

dt() is needed here.

greg.1.anderson’s picture

greg.1.anderson CreditAttribution: greg.1.anderson commented
Version: » 8.x-6.x-dev
Status: Needs work » Closed (won't fix)
Issue tags: +Needs migration

This issue was marked closed (won't fix) because Drush has moved to Github.

If this feature is still desired, you may copy it to our Github project. For best results, create a Pull Request that has been updated for the master branch. Post a link here to the PR, and please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.