Make WordPress Core

Opened 11 years ago

Closed 9 years ago

#25921 closed defect (bug) (worksforme)

User has to log in twice if redirect_to URL has other scheme than login URL

Reported by: thomaswm's profile thomaswm Owned by: jbkkd's profile jbkkd
Milestone: Priority: normal
Severity: normal Version: 3.7.1
Component: Users Keywords: has-patch needs-testing
Focuses: Cc:

Description

I'm using WordPress Multisite version 3.7.1. Suppose you open the login page of one of the blogs via the following URL.

https://example.com/wp-login.php?redirect_to=http%3A%2F%2Fexample.com%2Fwp-admin%2F&reauth=1

After logging in, you will be redirected to the URL specified by the redirect_to URL parameter. But then you are redirected to another (non-HTTPS) login page where you're asked to login again.

http://example.com/wp-login.php?redirect_to=http%3A%2F%2Fexample.com%2Fwp-admin%2F&reauth=1

I think in this case WordPress should either redirect the user to the non-HTTPS login page right away or change the scheme of the redirect_to URL to HTTPS.

Attachments (2)

25921.patch (1.0 KB) - added by jbkkd 11 years ago.
Actually, correct solution would probably be this - make the cookie insecure, regardless of the state of forced ssl login. If we're in HTTPS but redirecting to HTTP, it doesn't matter if secure login is needed or not.
25921(1).patch (1.2 KB) - added by jbkkd 11 years ago.
Also fixed comment

Download all attachments as: .zip

Change History (12)

#1 @nacin
11 years ago

  • Component changed from General to Users
  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

This drives me bonkers. Needs someone to track it down for sure.

#2 @jbkkd
11 years ago

  • Keywords dev-feedback 2nd-opinion added

Can reconfirm this is easily reproducible, even without multisite.

Should the fix just replace http/s if the urls don't match? Otherwise, the fix would involve mixing http/s cookies.

@jbkkd
11 years ago

Actually, correct solution would probably be this - make the cookie insecure, regardless of the state of forced ssl login. If we're in HTTPS but redirecting to HTTP, it doesn't matter if secure login is needed or not.

@jbkkd
11 years ago

Also fixed comment

#3 @jbkkd
11 years ago

  • Keywords has-patch reporter-feedback added; needs-patch 2nd-opinion removed

#4 @nacin
11 years ago

  • Keywords dev-feedback reporter-feedback removed
  • Owner set to jbkkd
  • Status changed from new to assigned

Thanks for this, jbkkd. I'll get someone else to review this as well.

When forced SSL login is set but forced SSL admin is not, we differentiate between what the user wants with the following:

  • If the user initially visited wp-login.php over SSL, then issue a secure cookie and send them to wp-admin over SSL.
  • If the user initially visited wp-admin.php over non-SSL, then issue a non-secure cookie and send them to wp-admin over non-SSL.

We've considered ditching the concept of forced SSL logins, making it all-or-nothing for wp-admin and wp-login.php. This is mainly to avoid attacks. See #10267.

I've studied this bug report a bit more and I want to say that there should be some other way to fix this without changing the secure-state of the cookie. "Should the fix just replace http/s if the urls don't match? Otherwise, the fix would involve mixing http/s cookies." might hold the key.

Also a note, attachments don't trigger notifications, so it's helpful to post your thoughts in a comment. Thanks!

This ticket was mentioned in Slack in #core by jorbin. View the logs.


10 years ago

#6 @jorbin
10 years ago

After #10267 was fixed, we need to check to see if this is still an issue. Can someone with an SSL cert in there dev environment please check this.

#7 @johnbillion
9 years ago

  • Keywords needs-testing added; good-first-bug removed

This is still possible to reproduce, but only by manually navigating to the login page over HTTPS with an HTTP redirect URL and when FORCE_SSL_ADMIN is not set to true.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


9 years ago

#9 @jorbin
9 years ago

  • Keywords close added

I feel like the fact that now a user would now need to manually navigate to the login page over HTTPS with an HTTP redirect URL and have FORCE_SSL_ADMIN not set to true, this seems like something that we can close as worksforme.

#10 @johnbillion
9 years ago

  • Keywords close removed
  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from assigned to closed

As per above. Feel free to re-open if this is reproducible in any other way.

Note: See TracTickets for help on using tickets.