Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a login issue on WP-Engine hosting #257

Closed
wants to merge 1 commit into from

Conversation

ccoley
Copy link

@ccoley ccoley commented Nov 2, 2018

WP-Engine requires additional parameters to exist on the URL for POST requests, and it applies those using a filter on the site_url hook when using the login_post scheme. So I've made sure to apply that filter when running in a WP-Engine environment.

Fixes #201

WP-Engine requires additional parameters to exist on the URL for POST requests,
and it applies those using a filter on the site_url hook when using the
login_post scheme. So I've made sure to apply that filter when running in a
WP-Engine environment.
@ccoley ccoley force-pushed the fix/wpengine-login-502-error branch from 5113dac to 856e79c Compare November 2, 2018 01:09
@@ -318,6 +318,12 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg
$interim_login = isset( $_REQUEST['interim-login'] ); // WPCS: override ok.
$wp_login_url = wp_login_url();

// If we're running in a WP-Engine environment then we need to apply the
// site_url filter with the login_post scheme for the form action.
if ( function_exists( 'is_wpe' ) && is_wpe() ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccoley This is a host specific integration and the function name is pretty generic and could conflict with existing code bases.

In #256 @cameronjonesweb suggested to switch to site_url() for the form action URLs to match how the WP core does it, which should also resolve this issue.

Copy link
Author

@ccoley ccoley Nov 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasparsd I was going to use site_url() but I looked back in the code and saw that you were originally using that, but then changed it to use wp_login_url() in this pull request to fix WooCommerce login forms and issue #61.

I thought that my way of doing it was less likely to cause negative side effects, but I'm happy to use site_url() instead if that is what you want.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if the is_wpe() function is too generic, then we could also check for existence of the IS_WPE environment variable.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccoley That is great find regarding the #62!

All of the two-factor processing currently relies on the wp-login.php logic and hooks so we must POST the form to wp-login.php. I'm not sure it has ever worked with a filtered wp-login.php destination. Will test it with a local install of WooCommerce.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that WooCommerce on-page login form works as expected with the wp-login.php form action.

@kasparsd
Copy link
Collaborator

kasparsd commented Nov 6, 2018

Fixed in #258.

@kasparsd kasparsd closed this Nov 6, 2018
@kasparsd kasparsd mentioned this pull request Nov 6, 2018
@ccoley ccoley deleted the fix/wpengine-login-502-error branch November 6, 2018 17:27
@jeffpaul jeffpaul added this to the 0.3.0 milestone Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

This is not working on a WP Engine site
3 participants