WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 2 months ago

#19898 reviewing enhancement

Create a is_login() function similar to is_admin()

Reported by: dcowgill Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3
Component: Administration Keywords: 2nd-opinion has-patch
Focuses: Cc:
PR Number:

Description

It would be useful for developers to have better detection of being on the wp-login.php and wp-register.php pages. Sure, this can currently be done by using the $pagenow global variable but having a similar function like is_admin() would make things easier/cleaner.

One use case:

-Using a hosted javascript file but then requiring SSL (FORCE_SSL_LOGIN).

function javascript_init() {
    if ( $pagenow == 'wp-login.php' || is_admin() )
        return;

    wp_deregister_script( 'jquery' );
    wp_register_script( 'jquery', 'http://ajax.googleapis.com/ajax/libs/jquery/1.6/jquery.min.js');
    wp_enqueue_script( 'jquery' );
}
add_action( 'wp_print_scripts', 'javascript_init' );

Proposed solution:
Create a new function that does all the checking.

function is_login() {
    return in_array( $GLOBALS['pagenow'], array( 'wp-login.php', 'wp-register.php' ) );
}

Attachments (6)

19898.patch (530 bytes) - added by dcowgill 7 years ago.
19898.2.diff (745 bytes) - added by scribu 7 years ago.
19898.3.diff (2.9 KB) - added by scribu 7 years ago.
19898.4.diff (2.9 KB) - added by scribu 7 years ago.
19898.5.diff (924 bytes) - added by scribu 7 years ago.
19898.6.diff (662 bytes) - added by donmhico 2 months ago.

Download all attachments as: .zip

Change History (36)

#1 @husobj
7 years ago

  • Cc ben@… added

#2 @stephenh1988
7 years ago

  • Cc contact@… added

#3 @scribu
7 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.5

#4 @scribu
7 years ago

The login pages are a grey area; you're not in wp-admin, but you're not on the front-end either.

#5 @wonderboymusic
7 years ago

  • Keywords has-patch added; needs-patch removed

The attached patch does exactly what WP_ADMIN does for WP_LOGIN - adds is_login() which will be true for any page rendered by wp-login.php or any action handled by it

@dcowgill
7 years ago

#6 follow-up: @nacin
7 years ago

  • Keywords 2nd-opinion added

I feel like the name is_login() could cause confusion in that it could be referring to the current user (i.e. what is_user_logged_in() is for). This is already rather confusing with is_admin(), given is_super_admin() and a number of other functions (is_blog_admin, is_user_admin, is_site_admin).

wp-register.php no longer exists (and for older installs, it simply redirects to wp-login.php). That means that is_login() simply returns $GLOBALS['pagenow'] == 'wp-login.php, which is not really much of a shortcut. Doesn't seem all that needed, in that case.

#7 @nacin
7 years ago

  • Keywords close added

#8 @nacin
7 years ago

  • Milestone changed from 3.5 to Future Release

#9 in reply to: ↑ 6 @scribu
7 years ago

  • Keywords close removed

Replying to nacin:

That means that is_login() simply returns $GLOBALS['pagenow'] == 'wp-login.php, which is not really much of a shortcut. Doesn't seem all that needed, in that case.

It would be more useful if it had a filter, which could be used for defining custom login areas for themes.

@scribu
7 years ago

#10 follow-up: @scribu
7 years ago

Actually, we don't even need a new filter. is_login() can just compare wp_login_url() agains the current URL (which I really think deserves a get_current_url() helper, btw).

Behold: 19898.2.diff

@scribu
7 years ago

#11 @scribu
7 years ago

  • Keywords needs-unit-tests added; 2nd-opinion removed

19898.3.diff:

  • renames is_login() to is_login_page() and introduces is_register_page()
  • adds necessary get_current_url() and wp_register_url() - see #17950

@scribu
7 years ago

#12 @meloniq
7 years ago

  • Cc meloniq@… added

#13 @nacin
7 years ago

While get_current_url() might be a nice-to-have, it isn't needed here — remove_query_arg( array( 'action', 'redirect_to', 'loggedout' ) will automatically act against the current URL.

#14 follow-up: @scribu
7 years ago

  • Keywords needs-unit-tests removed

To be exact, remove_query_arg() returns the current URL, sans the domain.

Either way, remove_query_arg() isn't reliable for is_register_page(): ?action=register.

It turns out, we can just use did_action( 'login_form_login' ) and did_action( 'login_form_register' ).

The only hitch is wp-signup.php.

@scribu
7 years ago

#15 @scribu
7 years ago

  • Keywords 2nd-opinion added

There: 19898.5.diff

At this point, we're back to comment 6.

#16 @robmiller
7 years ago

#22852 was marked as a duplicate.

#17 in reply to: ↑ 10 @nacin
7 years ago

Replying to scribu:

Actually, we don't even need a new filter. is_login() can just compare wp_login_url() agains the current URL (which I really think deserves a get_current_url() helper, btw).

is_admin() is frequently used to decide whether we are in the admin or not for purposes of, say, loading code. Allowing is_login() to be true for a theme page doesn't really make sense, nor does it solve that wp-login.php is in no man's land. It would be better to offer a login.php template (along with signup.php, activate.php, register.php, and probably other login actions) if this was about themes.

The original use case can be solved by either checking is_ssl() or just using a protocol-relative URL (//), FWIW.

#18 @wonderboymusic
7 years ago

I currently add Apache rewrites to allow different theme'd login pages - so +1 on the theme vibe

#19 @scribu
7 years ago

A login.php template would be great, but wouldn't you need a conditional flag for template-redirect.php then anyway?

And also, for example, themes might want to do something like this in header.php:

if ( is_login() ) {
  // enqueue some custom js 
}

#20 @toscho
7 years ago

  • Cc info@… added

#21 in reply to: ↑ 14 @kitchin
7 years ago

The proposed patch Attachment 19898.5.diff​ does not work when an add_action('init', ...) callback needs to detect wp-login. In that case @$GLOBALS['pagenow'] == 'wp-login.php' does work.

My particular situation is that a plugin author used add_action('init', ...) to call session_start(). The session creation is failing due to file errors on the server, and I am disabling sessions on the admin side to debug.

Replying to scribu:

It turns out, we can just use did_action( 'login_form_login' ) and did_action( 'login_form_register' ).

#22 @chriscct7
5 years ago

  • Keywords needs-refresh added; has-patch 2nd-opinion removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed
  • Version changed from 3.3.1 to 3.3

So no activity has happened on this ticket for 2+ years now. The latest patch doesn't really save anyone who really wanted to use a function like this that much code, and as pointed out there's issues in the current patch. Closing as wontfix

#23 @SergeyBiryukov
3 years ago

#37483 was marked as a duplicate.

#24 follow-up: @tazotodua
3 years ago

My topic (#37620) was closed marked as "DUPLICATE" of this topic.

However, MY REQUEST IS COMPLETELY DIFFERENT!

WHAT ON EARTH, if you add
define('LOGIN_PAGE_EXECUTED', true);
or something like that in wp-login.php

why dont you make a great help to programmers?

we cant determine when login page is executed (when we output login page on other page then wp-login.php url, like many security plugins do that)

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#25 in reply to: ↑ 24 @SergeyBiryukov
3 years ago

Replying to tazotodua:

My topic (#37620) was closed marked as "DUPLICATE" of this topic.

That ticket is still open. That said, it's unlikely a new constant will be introduced here. Constants are not a great way to check for context, as they are not filterable and hard to use in unit tests, see the discussion in #25669.

#37483 was closed, because the IS_LOGIN_PAGE constant proposed there does seem similar to the is_login() function proposed here. Why is it "completely different"?

#26 follow-ups: @iandunn
3 months ago

  • Keywords 2nd-opinion added
  • Resolution wontfix deleted
  • Status changed from closed to reopened

I think this is worth reconsidering.

is_login() simply returns $GLOBALSpagenow? == 'wp-login.php, which is not really much of a shortcut

latest patch doesn't really save anyone who really wanted to use a function like this that much code

It doesn't save a lot of lines of code, but what it does save is a lot of time spent investigating the most reliable way to solve this problem.

As the various patches and WPSE answers demonstrate, there are a handful of ways that devs have tried to solve this problem, and many of them are buggy. I personally just spent an hour of research and testing before settling on a solution I'm happy with, and it's still possible that I missed some edge cases.

It'd save devs a lot of time and bugs if Core provided a reliable solution.

we can just use did_action( 'login_form_login' )

I think the flaw with 19898.5.diff is that login_form_{$action} executes after plugins have loaded, so it wouldn't solve the use case of a bootstraper that's trying to load code exclusively on the login screen.

I would do this instead:

function is_login_screen() {
	return false !== stripos( wp_login_url(), $_SERVER['SCRIPT_NAME'] );
}

#27 in reply to: ↑ 26 @SergeyBiryukov
3 months ago

  • Milestone set to 5.3

Replying to iandunn:

I would do this instead:

function is_login_screen() {
	return false !== stripos( wp_login_url(), $_SERVER['SCRIPT_NAME'] );
}

Makes sense to me.

@donmhico
2 months ago

#28 in reply to: ↑ 26 @donmhico
2 months ago

  • Keywords has-patch added; needs-refresh removed

Replying to iandunn:

I would do this instead:

function is_login_screen() {
	return false !== stripos( wp_login_url(), $_SERVER['SCRIPT_NAME'] );
}

Looks good to me too. I created a patch, 19898.6.diff , using @iandunn's solution. I inserted the function in wp-includes/load.php as other similar functions such as is_admin() resides there,

#29 @SergeyBiryukov
2 months ago

  • Owner set to SergeyBiryukov
  • Status changed from reopened to reviewing

#30 @davidbaumwald
2 months ago

  • Milestone changed from 5.3 to Future Release

With version 5.3 Beta 1 releasing shortly, the deadline for enhancements is now passed. This is being moved to Future Release. @SergeyBiryukov if you feel this can be included in 5.4, feel free to move up the milestone.

Note: See TracTickets for help on using tickets.