Opened 3 years ago

Last modified 8 months ago

#14949 new defect (bug)

Login gives false assurance of having logged out

Reported by: filosofo Owned by: filosofo
Priority: normal Milestone: Future Release
Component: Administration Version: 3.1
Severity: normal Keywords: has-patch needs-testing 3.2-early
Cc: azizur, mdhansen@…

Description

If you visit wp-login.php?loggedout=true while logged in, WordPress falsely tells you that "You are now logged out."

This is a problem because it could lead you to think, e.g., that a public computer is no longer authenticated with access to your WP admin.

Patch redirects a still-authenticated user back to the admin from the login page if she requests the above page without actually having logged out.

Attachments (4)

no-false-logout-message.14949.diff (524 bytes) - added by filosofo 3 years ago.
14949-current-user-can-not-login-before-logout.patch (679 bytes) - added by hakre 3 years ago.
Prevents a signon for a user that is already logged in.
14949-user-prior-filter.patch (496 bytes) - added by hakre 3 years ago.
Similar to filosofo's patch but passes current user to login_redirect filter.
14949-signon-internals.patch (808 bytes) - added by hakre 3 years ago.
Prevents to pass NULL values to filters

Download all attachments as: .zip

Change History (17)

Hmm, my first reaction was to dislike the redirect. Then I wondered what happens if you visit wp-login.php while being logged in. You can login again *gg* (trunk).

I suggest that the requests behaves like a successfull login has been made (e.g. going to dashboard by default but running all filters and same logic so that plugins work as expected).

hakre3 years ago

Prevents a signon for a user that is already logged in.

hakre3 years ago

Similar to filosofo's patch but passes current user to login_redirect filter.

hakre3 years ago

Prevents to pass NULL values to filters

  • Keywords dev-feedback added

This is actually a problem with how admin notices are passed around.

There was a ticket proposing an API for passing admin notices using transients I think, but I can't find it now.

I agree that the notices need work. Here's my ticket about it: #11515

However, I think this particular issue has a simple fix. wp-admin/ pages redirect non-authenticated users to wp-login.php, so it seems reasonable to redirect authenticated users from wp-login.php to wp-admin/.

  • Cc azizur added
  • Keywords needs-testing added; dev-feedback removed

no-false-logout-message.14949.diff looks and tests fine. Want some extra eyes on it before continuing.

I've noticed that when running multisite -- and this may be particular to my setup -- wp-login.php redirects to wp-admin already. I haven't dug into the code enough yet to ascertain why, which is partially why I'm not committing this just yet.

Patch tests fine. I think hakre's patch 14949-user-prior-filter.patch makes more sense. It's the same code, but ensures that we have the correct user info going to the redirect filter.

Didn't look into the multisite thing, though.

Well, my thought in having it after $user is passed to the filter is that it gives you access to the error object, whereas you can access the current user globally and don't need to be concerned about whether it gets passed to the filter.

In other words, maybe you want to adjust the redirect according to the type of error that has occurred. 14949-user-prior-filter.patch doesn't let you do that.

comment:9   ryan2 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

IIRC, the reason it shows when logged in is so that you can upgrade from non-SSL to SSL cookies, or vice-versa. I think any patch would have to accommodate allowing logged in users to log in again if their cookies do not match the is_ssl() state when visiting the login page.

This is still the case in 3.3

This is still the case in 3.5-alpha-21751. But is there any way a user could even land on the login page with the logout set without manually putting it into the browser?

  • Cc mdhansen@… added
Note: See TracTickets for help on using tickets.