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.

Comments

greggles’s picture

Status:Active» Needs review
StatusFileSize
new415 bytes

Turbo boosted.

moshe weitzman’s picture

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

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

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

Status:Fixed» Needs review
StatusFileSize
new14.65 KB

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

Owen Barton’s picture

Status:Needs review» Closed (fixed)

Committed

greggles’s picture

Status:Closed (fixed)» Needs work
StatusFileSize
new1.06 KB

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

The patch had

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

Status:Needs work» Needs review
greg.1.anderson’s picture

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

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

Status:Needs review» Needs work
greggles’s picture

Status:Needs work» Needs review
StatusFileSize
new1.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

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

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.