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

Can't login when using login_url filter #256

Closed
cameronjonesweb opened this issue Oct 31, 2018 · 9 comments · Fixed by #258
Closed

Can't login when using login_url filter #256

cameronjonesweb opened this issue Oct 31, 2018 · 9 comments · Fixed by #258
Milestone

Comments

@cameronjonesweb
Copy link

I've got a site that uses the login_url filter to set the login URL to a page with a login form shortcode, in this instance provided by EDD. When attempting to log in, the 2FA screen shows, but when you enter the code it just goes back to the login form. If I edit the source of the 2FA screen form and change the action to go to wp-login.php instead of the custom login form the login works correctly

@kasparsd
Copy link
Collaborator

Thanks for reporting the issue @cameronjonesweb!

This seems to be a duplicate of #222. Let me know if that's not the case.

@cameronjonesweb
Copy link
Author

I don't really think so, but the other ticket seems a bit more ambiguous so it may be.

For my specific issue, I believe this could be fixed by adding a new filter for the form action here: https://github.com/georgestephanis/two-factor/blob/master/class.two-factor-core.php#L338. That way, I can filter the action of the form so that logins will work correctly

@kasparsd
Copy link
Collaborator

kasparsd commented Nov 1, 2018

@cameronjonesweb That form is already using the filtered login URL:

https://github.com/georgestephanis/two-factor/blob/6d14edcaa11c0745efa1dc35f3b8b134f0508827/class.two-factor-core.php#L319

For re-using this form on other pages we would need to remove all the extra login page header and footer HTML. We would also have to make sure that all processing attached to login_form_{$action} is firing as expected on those custom pages:

https://github.com/georgestephanis/two-factor/blob/99d654404d2bd908c710649bfc04a37274c23b6b/class.two-factor-core.php#L41-L42

Currently only the wp-login.php file has that action firing.

@cameronjonesweb
Copy link
Author

That's not what I meant at all. The fact that you're using the filtered value is what's causing the issue, when that URL for the form action needs to be wp-login.php to work. I'm suggesting adding another filter so that the form action URL can be fixed without changing the value that wp_login_url should be

@kasparsd
Copy link
Collaborator

kasparsd commented Nov 1, 2018

Thanks for pointing me in the right direction. Looks like WP core has it hard-coded to wp-login.php:

https://github.com/WordPress/WordPress/blob/da7a80d67fea29c2badfc538bfc01c8a585f0cbe/wp-login.php#L1082

which makes a lot of sense, so I agree we should hard-code it too. Would you like to open a pull request for this?

@kasparsd kasparsd reopened this Nov 1, 2018
@cameronjonesweb
Copy link
Author

Happy to

@kasparsd
Copy link
Collaborator

kasparsd commented Nov 2, 2018

This should also fix #257.

@kasparsd kasparsd removed the Duplicate label Nov 2, 2018
@kasparsd
Copy link
Collaborator

kasparsd commented Nov 3, 2018

As discovered by @ccoley, the wp_login_url() was introduced in #62. Not sure it has ever worked, though.

@kasparsd
Copy link
Collaborator

kasparsd commented Nov 3, 2018

Just confirmed locally that WooCommerce doesn't override the wp_login_url() and it works correctly with the current logic which defaults to wp-login.php. The checkout login returns our custom form for the two-factor processing and then redirects back to the checkout page as expected.

We're safe to hard-code the form destination as wp-login.php.

@kasparsd kasparsd mentioned this issue Nov 6, 2018
@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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants