Problem/Motivation

Part of #3582246: [meta] CLI in Core community initiative Phase 2. Referenced from theCLI in Core Initiative spreadsheet.

user:login is one of the top 5 most-used commands in drush.

Steps to reproduce

Proposed resolution

A one-time login URL for the specified user should be generated and printed as output in the terminal using the same mechanism as the reset password form.

Remaining tasks

None.

User interface changes

The default output of vendor/bin/dr shows another command you can run.

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3594332

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:

Comments

mradcliffe created an issue. See original summary.

moshe weitzman’s picture

Assigned: Unassigned » moshe weitzman

moshe weitzman’s picture

Status: Active » Needs review

MR is in good shape except for a test failure when actually logging in with the link during a BrowserTest. The links work outside of tests but are giving a 404 inside the test. Help wanted.

mstrelan’s picture

Wonder if we should check for the super user access policy before falling back on user 1? Or maybe it's fine, since in most cases user 1 is going to be the primary admin, regardless of that policy.

moshe weitzman’s picture

✅ Its ready for review now.

What would we do if the site disabled the superuser via access policy? I bet some folks would ask for configuration for a fallback uid. I'm not keen on adding configuration for commands. This cost us dearly in complexity in Drush. I'm inclined to point folks to wrappers like DDEV commands, composer scripts, and Task

catch’s picture

For #5 I think falling back is OK - we may eventually deprecate the super user access policy entirely, and that will mean giving user/1 the admin role so it can do the same thing for existing sites. If a site has a non-admin user/1 then the fallback isn't useful but also doesn't do any harm.

Configuration would mean a site either has a shared admin account that's not user/1, or has multiple admin users and needs to pick one. If there's really a big demand from that from people who won't also write a ddev command or whatever, we could always look at adding it later.

dww’s picture

Mostly looks great, thanks! Opened a few MR threads for consideration. Not moving to NW over any of them. Very excited to see this moving forward! It's probably my 2nd most used drush command (after cr, which dr already supports).

dww’s picture

Assigned: moshe weitzman » Unassigned
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Moshe addressed my feedback. Pipeline is green. Cleaning up the summary a bit by removing "To be determined" from a bunch of sections that aren't relevant. Title is accurate. Nothing else to complain about or improve. RTBC.

Thanks!
-Derek

heddn’s picture

+1 on RTBC

godotislate’s picture

Status: Reviewed & tested by the community » Needs work

Comments on the MR.

dww’s picture

Status: Needs work » Needs review

Thanks for the review, godotislate! Addressed all points.

dww’s picture

Also, moved the string change to #3600628: Remove misleading help text after using a 1-time login link so that this is simple and non-disruptive for 11.4.x backport.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Feedback addressed

dww’s picture

StatusFileSize
new764 bytes
new827 bytes

It occurred to me that none of the tests of the command failing need a browser, since we don't have a link to test. All of that can be Kernel. Pushed a commit. Locally, it cuts the time for the new test coverage in half.

Previous (commit f98299cc) with all Functional:
Time: 00:18.475, Memory: 10.00 MB

Now (commit 73df00da) with the original Functional, and all the failure cases Kernel:
Time: 00:09.029, Memory: 12.00 MB

See attached phpunit output for details.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new1.83 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

dww’s picture

Status: Needs work » Reviewed & tested by the community