Opened 14 years ago
Last modified 6 months ago
#14949 accepted defect (bug)
Login gives false assurance of having logged out
Reported by: | filosofo | Owned by: | rajinsharwar |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Login and Registration | Keywords: | has-patch needs-testing dev-feedback 2nd-opinion |
Focuses: | administration, multisite | Cc: |
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 (7)
Change History (52)
#3
@
14 years ago
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.
#4
@
14 years ago
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/
.
#6
follow-up:
↓ 14
@
14 years ago
- 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.
#7
@
14 years ago
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.
#8
@
14 years ago
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.
#10
@
14 years ago
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.
#12
@
12 years ago
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?
#14
in reply to:
↑ 6
@
11 years ago
In 3.6 (and trunk) if I go to domain.com/wp-login.php on single or Multisite it provides the login form.
Replying to nacin:
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.
This part I can't reproduce.
#15
@
11 years ago
- Component changed from Administration to Login and Registration
- Focuses administration multisite added
#17
@
11 years ago
Worth sharing: [bbpress5305].
#21
@
8 years ago
Please has this issue been fixed?
I have a website that uses Peter's Login Redirect where users are expected to get redirected to a custom page after logging in, but if an already logged-in user tries to login again, he gets redirected to the /wp-admin page.
Please what should I do?
Thanks
#23
@
8 years ago
Thanks @lukecavanagh ... I also found out that...
Seeing that this has been the case for years, is there any chance that this might change anytime soon?
#24
@
8 years ago
This patch applies cleanly in WP 4.7.3. So if an end-user hits wp-login.php?loggedout=true if a user is already logged in, then it will redirect to wp-admin.
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
7 years ago
#28
@
5 years ago
Issue still persist in 5.3 and the patch still resolves the problem. I attached a refreshed patch above. Is there any reason / concern why we aren't merging this to trunk?
#30
@
5 years ago
If I am able to visit the wp-login.php page and see a form asking for a username and password, as a user, I would assume I am not authenticated.
Making the text "You are now logged out" not display doesn't go far enough in my opinion.
#31
@
5 years ago
Patch 14949.3.diff confirmed. When visiting /wp-login.php
while already logged in, I now get redirected to /wp-admin
@desrosj @SergeyBiryukov please consider reviewing and maybe adding to the next release?
#32
@
5 years ago
- Keywords 3.2-early removed
- Milestone changed from Future Release to 5.4
This needs testing with plugins which make changes to the login screen, or add extra functionality to it such as two-factor authentication. Best to check in case there's any unexpected problems.
#33
@
5 years ago
- Resolution set to worksforme
- Status changed from new to closed
Patch 14949.3.diff confirmed working with Simple Login Captcha in WP 5.4-beta1-47286.
Redirected /wp-login.php and /wp-login.php?loggedout=true to /wp-admin if the user is already logged-in.
#34
@
5 years ago
- Resolution worksforme deleted
- Status changed from closed to reopened
Leaving it open for others to test.
#35
@
5 years ago
- Keywords dev-feedback added
Confirmed working with Two Factor using email, Authy/QR and OTP codes. Using WP trunk as of commit 1911d0db (14 Feb 2020). Redirected /wp-login.php and /wp-login.php?loggedout=true to /wp-admin if the user is already logged-in, then able to logout and login again without any noticeable changes.
@johnbillion do you have other plugins in mind that should be tested, or are these two sufficient? I can test more in the next couple days, but I'm not familiar with the available login-changing plugins.
#36
@
5 years ago
- Milestone changed from 5.4 to Future Release
Hi,
With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.
#37
@
16 months ago
- Milestone changed from Future Release to 6.4
- Owner changed from filosofo to rajinsharwar
- Status changed from reopened to accepted
- Version 3.1 deleted
Taking ownership of this old ticket, and putting this for 6.4.
Related: #40768
#38
@
16 months ago
- Keywords 2nd-opinion added
Okay, so testing with the patch(14949.3.diff). Works as expected, but I am worried about two scenarios.
- If the user is not an Administrator, and the admin doesn't want his users to visit their wp-admin dash, it that case, the user with a non-Administrator role will be forced redirected to the admin page.
- If the user is not logged in, and the link is opened "http://localhost:10058/wp-login.php?loggedout=true", WordPress again falsely tells you that "You are now logged out.". This might not cause any issues but isn't correct logically.
Here is my proposed solution.
We can check if the user is logged in. If he is logged in and tries to access "/wp-login.php?loggedout=true", instead of redirecting them to the admin dash, we can show him the "You are attempting to log out of %s. Do you really want to <a href="%s">log out</a>?". That means we can show him the screen which we currently show for the "/wp-login.php?action=logout". This way they won't be forced redirected anywhere, and they can log out from there if they want.
Secondly, if they are not logged in, we can just redirect them to the login URL, instead of showing the message You have logged out".
Let me know what others think about this.
This ticket was mentioned in Slack in #core by nicolefurlan. View the logs.
15 months ago
#40
@
15 months ago
This ticket was discussed in today's bug scrub.
As @hellofromTonya mentioned:
Any solution here will need a lot of testing. The comment @johnbillion made years ago is still true today:
This needs testing with plugins which make changes to the login screen, or add extra functionality to it such as two-factor authentication. Best to check in case there's any unexpected problems.
The idea proposed would be good to potentially workflow (proof of concept) with more discussion and then testing. Once there's consensus and a plan for testing across multiple plugins, then it could be re-milestoned.
@joemcgill mentioned that since this new proposal still needs discussion and a patch, this ticket is likely to get moved to Future Release
.
@rajinsharwar, what are your thoughts?
#41
@
15 months ago
- Milestone changed from 6.4 to Future Release
I do agree @nicolefurlan. Let's move this to a future release for now then.
#42
@
8 months ago
- Milestone changed from Future Release to 6.6
I would like to move this to 6.6. Specially, because of the issue mentioned in #60801.
Excessive amount of unnecessary session data in the database. We've seen some large websites with tens of thousands of session entries in the database.
What I would suggest here is like, to redirect to the page from where the wp-login.php was referred from, if the user is already logged in, and has an active session. We can have some Dev notes released before so that appropriate plugin owners are known of what change to expect.
Feel free to drop off your suggestions on this.
#43
@
8 months ago
I like the idea @rajinsharwar
Just to confirm, the solution will apply whenever someone visits the login page, regardless of the parameters, such as "loggedout=true", correct?
So it would also solve the issue in issue #60801(https://core.trac.wordpress.org/ticket/60801)
#44
@
6 months ago
Test Report
Description
Before applying the patch, when I visited mydomain.com/wp-login.php?loggedout=true
, it showed the message "You are now logged out" (see screenshot: https://prnt.sc/XSWd-6hPLDrV). However, if I returned to the homepage, it still showed that I was logged in (see screenshot: https://prnt.sc/RVDFFMnLpc3A). After applying the patch 14949.3.diff, visiting the same URL (mydomain.com/wp-login.php?loggedout=true
) now redirects to the dashboard.
Patch tested: 14949.3.diff
Environment
- WordPress: 6.6-alpha-57778-src
- PHP: 8.3.7
- Server: nginx/1.25.4
- Database: mysqli (Server: 8.3.0 / Client: mysqlnd 8.3.7)
- Browser: Chrome 125.0.0.0
- OS: macOS
- Theme: Twenty Eleven 4.6
- MU Plugins: None activated
- Plugins:
- Test Reports 1.1.0
#45
@
6 months ago
- Milestone changed from 6.6 to Future Release
Just to confirm, the solution will apply whenever someone visits the login page, regardless of the parameters, such as "loggedout=true", correct?
Yes, I am proposing a solution like that @robert681.
I am punting this to a Future Release, as this is highly unlikely to land in 6.6. Also, if this is the solution agreed upon, this would also require thorough testing with different plugins and themes. So, let's keep it in Future Release until some movement takes place. 🙂
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).