WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 19 months ago

#20279 closed enhancement (fixed)

Functions defined in wp-login.php should be moved to a separate file to make them re-usable

Reported by: beaulebens Owned by: SergeyBiryukov
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

Currently, there are a series of functions defined at the top of wp-login.php:

login_header()
login_footer()
wp_shake_js()
retrieve_password()
check_password_reset_key()
reset_password()
register_new_user()

If these functions were moved to a separate file, they would be reusable by folks who are trying to do more advanced things involving the login flow.

As an example, if you want to create a page resembling wp-login right now, there's no easy way to do it without just duplicating everything from this file. Moving these functions means you can use login_header() and login_footer() to recreate the bulk of the page, then handle the "content block" yourself. This marginally improves the re-usability of the login system.

Patch attached which does this.

Attachments (3)

20279.diff (24.3 KB) - added by beaulebens 3 years ago.
Dotted filename used only to match the other functions.*.php files in wp-includes. Feel free to change it.
20279.2.diff (7.7 KB) - added by SergeyBiryukov 19 months ago.
20279.3.diff (7.8 KB) - added by SergeyBiryukov 19 months ago.
Refreshed after [25203]

Download all attachments as: .zip

Change History (17)

@beaulebens3 years ago

Dotted filename used only to match the other functions.*.php files in wp-includes. Feel free to change it.

comment:1 @nacin3 years ago

See also #15384. I think some of these functions are good as a separate file, while others are pretty intrinsic to the login skin/flow.

We don't use dotted filenames — those files are from BackPress. This would simply be wp-includes/login.php.

Also, 19852#comment:31, where we may end up moving wp-login.php to the admin pot for 3.4.

comment:2 @wycks3 years ago

  • Cc wycks added

I was just looking at wp_shake_js and it looks strange there, all for cleaning this up a bit.

comment:3 @emeraldimp3 years ago

  • Cc emeraldimp added

comment:4 @scottconnerly3 years ago

  • Cc scott@… added

comment:5 @zombrows3 years ago

  • Cc zombrows added

comment:7 @adrian72 years ago

Me too waiting for this enhancement. Maybe moved to pluggable.php. Also retrieve_password and others taking values directly from $_POST should accept one or more parameters.

For now a solution for me was to fork these functions and put them in functions.php of my theme.

comment:8 @SergeyBiryukov19 months ago

  • Milestone changed from Awaiting Review to 3.7

I wanted to use register_new_user() in a plugin recently, but making it reusable currently requires a dirty hack (error suppression is to hide the "Constant ABSPATH already defined" notice):

ob_start();
@require_once( ABSPATH . 'wp-login.php' );
ob_end_clean();

Moving for review.

comment:9 follow-up: @nacin19 months ago

Two general objections:

  • I think some of these functions are good as a separate file, while others are pretty intrinsic to the login skin/flow.
  • Some of these functions might not deserve to see the light of day, so to speak — we need to review them.

I think we can overcome these.

Here's the list of functions defined in wp-login.php that I think are intrinsic to wp-login.php and shouldn't leave that file for now:

function login_header($title = 'Log In', $message = '', $wp_error = '') {
function login_footer($input_id = '') {
function wp_shake_js() {
function wp_attempt_focus(){

Here are the other four functions:

function retrieve_password() {
function check_password_reset_key($key, $login) {
function reset_password($user, $new_pass) {
function register_new_user( $user_login, $user_email ) {

I gave a quick scan through all of them to ensure they were sane and were not specific to wp-login.php.

I think check_password_reset_key(), reset_password(), and register_new_user() all meet those parameters. These are under 90 lines of code (including documentation) and, I think, belong in user.php.

I think retrieve_password() may be too tied to the wp-login.php process to be immediately extracted. Other opinions?

comment:10 @beaulebens19 months ago

FWIW, this ticket was originally created in large part because we found ourselves needing to use login_header() and login_footer() to create a modified login experience that closely-resembled the core experience.

It looks like maybe there are more hooks and things in there now, so perhaps the original use-case for this is less important, but thought I'd throw it out there.

comment:11 in reply to: ↑ 9 @SergeyBiryukov19 months ago

Replying to nacin:

I think check_password_reset_key(), reset_password(), and register_new_user() all meet those parameters. These are under 90 lines of code (including documentation) and, I think, belong in user.php.

Sounds good to me.

@SergeyBiryukov19 months ago

comment:12 @SergeyBiryukov19 months ago

20279.2.diff moves check_password_reset_key(), reset_password(), and register_new_user() to wp-includes/user.php.

comment:13 @SergeyBiryukov19 months ago

  • Keywords commit added

@SergeyBiryukov19 months ago

Refreshed after [25203]

comment:14 @SergeyBiryukov19 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 25231:

Move check_password_reset_key(), reset_password(), and register_new_user() from wp-login.php to wp-includes/user.php, to make them reusable. props beaulebens for initial patch. fixes #20279.

Note: See TracTickets for help on using tickets.