redirect_to wp-admin Should Force SSL If FORCE_SSL_ADMIN is enabled

[g30rg3x]

Around Lines 406 to 426 on wp-login.php:

	$secure_cookie = '';

	// If the user wants ssl but the session is not ssl, force a secure cookie.
	if ( !empty($_POST['log']) && !force_ssl_admin() ) {
		$user_name = sanitize_user($_POST['log']);
		if ( $user = get_userdatabylogin($user_name) ) {
			if ( get_user_option('use_ssl', $user->ID) ) {
				$secure_cookie = true;
				force_ssl_admin(true);
			}
		}
	}

	if ( isset( $_REQUEST['redirect_to'] ) ) {
		$redirect_to = $_REQUEST['redirect_to'];
		// Redirect to https if user wants ssl
		if ( $secure_cookie && false !== strpos($redirect_to, 'wp-admin') )
			$redirect_to = preg_replace('|^http://|', 'https://', $redirect_to);
	} else {
		$redirect_to = admin_url();
	}

As we can see on the present code, if a redirection is set while login and this redirection goes to the plain version of the dashboard then client will go to the non-SSL version of the dashboard which therefore will move the client to the secure version (generating and extra request).
I know this is kinda a tongue twister sentence so i think is better to put a request example of the problem...

Client: POST ​http://foo.bar/wp-login.php?redirect_to=http%3A%2F%2Ffoo.bar%2Fwp-admin%2Findex.php
Server: HTTP 302 ... Location: ​http://foo.bar/wp-admin/index.php
Client: GET ​http://foo.bar/wp-admin/index.php
Server: HTTP 302 ... Location: ​https://foo.bar/wp-admin/index.php

I know that wordpress is actually working as suppose to work (cause we told to move to non-SSL version of the dashboard) but and a extra http request is issued.
IMHO if we (admins) have enabled FORCE_SSL_ADMIN, then all redirections to wp-admin should go SSL/HTTPs even if we fill redirect_to with the plain version of the dashboard.
There is part of the code that detect this and replace it but it has issues or well it isn't prepared to do this.
At the moment we can filter login_redirect to fix this but (again) IMHO this should move to the core...

[janeforshort]

Punting due to feature freeze. Reconsider with next release.

[hakre]

That code need to stripslash he value because since it was written, $_REQUEST has been changed and is slashed now.

[hakre]

I put this ticket on my list at for a ​secure wordpress setup

[hakre]

Attached you find a patch against current head that solves the problem.

The routines were infact already in there, only the FORCE_SSL_ADMIN define was not reflected properly. While doing the review I used the time as well to make the code simpler (and therefore less errorprone).

I already tested the patch and it would be great if the original reporter still is available to test if this is fit for the reported case.

The case infact is a bug because it should have been redirected properly already as far I was able to read the code (see the user wants ssl scenario). So this is not optimization nor an enhancement. I relate this to security because SSL is a security layer.

[hakre]

Related: #11643

[hakre]

(In [12666]) Typo fix in comment. see #11643 -> patch needs an update, typo in comment as well.

[keytuna]

Attempted to reproduce issue with version 3.5.1 of WordPress, XAMPP 1.8.1, and Curl 7.29.0.

Used default settings, attempted with this command:

curl -v -X POST ​http://localhost/wordpress/wp-login.php?redirect_to=http%3A%2F%2Flocalhost%2wordpress%2Fwp-admin%2Findex.php

Expected to see a 302 redirect, found a 200 OK response.

Is anyone else still able to reproduce this issue?

[nacin]

In trunk, here's what happens:

• Go to ​http://wordpress/wp-admin/
• auth_redirect() sends you to ​https://wordpress/wp-admin/
• auth_redirect() then authenticates you, redirects to ​https://wordpress/wp-login.php?redirect_to=https://wordpress/wp-admin/
• Logging in takes you directly to ​https://wordpress/wp-admin/

This is proper behavior. That said, it is still possible to go to ​https://wordpress/wp-login.php?redirect_to=http://wordpress/wp-admin/, which will indeed end up with the double-redirect being described.

The reason is because $secure_cookie is set to '' under a forced SSL situation, and only set to true when we're not forcing SSL but the user's preference is an SSL experience. (This user preference is largely a dead concept as of #10267.) The authentication functions then set $secure_cookie to true when '' and under is_ssl(). Trusting if ( $secure_cookie ) rather than if ( force_ssl_admin() ) thus triggers a double-redirect for forced SSL.

This is actually by design, though. This happened in [8701] simply to ensure we are following the user's preference. Since this no longer (never?) can happen normally (as going to http wp-admin redirects to https wp-admin before redirecting to wp-login), there's nothing to fix here. Closing out.