Always use 'login' as $scheme parameter for "login-ish" URLs, and other inconsistencies

[GregLone]

Hi.

I found a lot of references of site_url() used for login/register/whatever that do not use the second parameter $scheme (it should be 'login'). But this is a first step, we should use wp_login_url(), wp_registration_url(), and friends.

So far I found the following:

In wp-signup.php:
wp_redirect( site_url('wp-login.php?action=register') ); should use wp_registration_url().

$login_url = site_url( 'wp-login.php?redirect_to=' . urlencode( network_site_url( 'wp-signup.php' ) ) ); should use wp_login_url( network_site_url( 'wp-signup.php' ) ).

Even worse, the function confirm_another_blog_signup() uses "http://" . $domain.$path . "wp-login.php".

In wp-activate.php:
wp_redirect( site_url( '/wp-login.php?action=register' ) ); should use wp_registration_url().
Also note the "/" at the beginning. Not important, but not consistent with other URLs.

And few lines later:
$url . 'wp-login.php' should be replaced by get_site_url( (int) $result['blog_id'], 'wp-login.php', 'login' ) imho.

I also wonder if network_site_url('wp-login.php', 'login') shouldn't be replaced by site_url('wp-login.php', 'login'), since this is the only file where it is used that way anyway.

In wp-includes/canonical.php:
wp_redirect( site_url('wp-login.php?action=register') ); should use wp_registration_url().

In wp-admin/install.php:
There's a '</p><p class="step"><a href="../wp-login.php" class="button button-large">' . __( 'Log In' ) . '</a></p></body></html>'.
And another <a href="../wp-login.php" class="button button-large"><?php _e( 'Log In' ); ?></a>.

In wp-includes/ms-functions.php:
There is Log in here: BLOG_URLwp-login.php in an email template.
And another <a href="../wp-login.php" class="button button-large"><?php _e( 'Log In' ); ?></a>.

In wp-admin/network.php:
<a href="<?php echo esc_url( site_url( 'wp-login.php' ) ); ?>"><?php _e( 'Log In' ); ?></a>

In wp-includes/schema.php:
There is Log in here: BLOG_URLwp-login.php in an email template.

Any thought?

[DrewAPicture]

Would you like to make a patch? Standouts I would skip are:

• wp-activate.php: The use of network_site_url(). I think that needs to be left alone because it's specifically for the multisite/network URL use-case. If it isn't multisite, it'll fall back to site_url() anyway.
• wp-includes/ms-functions.php: email template string
• wp-includes/schema.php: email template

For the others, it might be worth a try fixing. Some of those have been in core since we switched from using direct file links more than four years ago.

[DrewAPicture]

Related:

#19097

use wp_register() in wp-login.php

#21089

Add admin url to install notification email

#27632

`wp_registration_url()` doesn't have a redirect argument

[GregLone]

Replace some site_url( 'wp-login.php' ) with wp_login_url() and friends

[GregLone]

In 31495.patch:
wp-activate.php:

• Use wp_registration_url() for redirection instead of site_url( '/wp-login.php?action=register' ).
• Use switch_to_blog() + wp_login_url() + restore_current_blog() instead of $url . 'wp-login.php'.

wp-signup.php:

• Use wp_registration_url() for redirection instead of site_url( 'wp-login.php?action=register' ).
• In validate_another_blog_signup(), use the blog ID returned by wpmu_create_blog() and pass it to confirm_another_blog_signup(). In confirm_another_blog_signup(), use the new parameter $blog_id to get the login URL. Also removed two unused parameters $user_email and $meta.
• Use wp_login_url( network_site_url( 'wp-signup.php' ) ) instead of site_url( 'wp-login.php?redirect_to=' . urlencode( network_site_url( 'wp-signup.php' ) ) ).

wp-admin/install.php:

• Use esc_url( wp_login_url() ) instead of href="../wp-login.php".

wp-admin/network.php:

• Use esc_url( wp_login_url() ) instead of esc_url( site_url( 'wp-login.php' ) ).

wp-includes/canonical.php:

• Use wp_registration_url() for redirection instead of site_url( 'wp-login.php?action=register' ).
• Use wp_login_url() for redirection instead of site_url( 'wp-login.php', 'login' ).

[GregLone]

Replying to DrewAPicture:

• wp-activate.php: The use of network_site_url(). I think that needs to be left alone because it's specifically for the multisite/network URL use-case. If it isn't multisite, it'll fall back to site_url() anyway.

As far as I can tell, network_site_url() is used only for "lost password" and "reset password". Is there any reason we should keep using network_site_url('wp-login.php', 'login') here for the login address? As it is not used anywhere else in core, it should be replaced by esc_url( wp_login_url() ) imho. At least, it should be discussed.

• wp-includes/ms-functions.php: email template string
• wp-admin/includes/schema.php: email template

I agree for schema.php, we should leave it as it is now.
May I suggest something simple for ms-functions.php? A bit hacky, but back-compat'.

switch_to_blog( $blog_id );
$login_url = wp_login_url();
restore_current_blog();

$welcome_email = str_replace( 'BLOG_URLwp-login.php', $login_url, $welcome_email );

Some of those have been in core since we switched from using direct file links more than four years ago.

Yes, sorry for the "Version 4.1.1" x)

[johnbillion]

31495.2.patch​ refreshes the patch for the non-contentious instances of wp-login.php. Note that since [28609], the login_post scheme is the same as the login and admin schemes, so we can also replace a couple more instances where the login_post scheme is used.

[GregLone]

What about:

• the hard-coded wp-login.php in confirm_another_blog_signup(),
• network_site_url( 'wp-login.php', 'login' ) in wp-activate.php?

[johnbillion]

In 34213:

Implement wp_login_url() and wp_registration_url() in places where wp-login.php is currently hard-coded.

See #31495
Props GregLone

[johnbillion]

In 34358:

Implement some more uses of wp_login_url() in places where wp-login.php is hard-coded.

See #31495

[johnbillion]

31495.3.patch​ tidies up the remaining changes originally patched by @GregLone.

Just needs a second set of eyes and the nod.

[johnbillion]

@jeremyfelt 31495.3.patch​ introduces two situations where home_url() is nestled between switch_to_blog() and restore_current_blog(). Can you envisage something breaking in this case?

[jeremyfelt]

Replying to johnbillion:

@jeremyfelt 31495.3.patch​ introduces two situations where home_url() is nestled between switch_to_blog() and restore_current_blog(). Can you envisage something breaking in this case?

I think the only breakage would be if someone was blindly filtering home_url without being aware of the context in multisite. It doesn't seem like much to worry about here as this is all happening at the network level on new sites.

We may want to run things through esc_url() though, as they are filtered.

[johnbillion]

In 34790:

Use wp_login_url() for login links when signing up for a new blog or activating a new blog on Multisite.

Fixes #31495
Props GregLone for the intial patch