search

shane-tomlinson / browserid-wordpress

Wordpress plugin that adds Persona authentication
23 stars 16 forks source link

Logout error #92

Open ptrsmk opened 10 years ago

ptrsmk commented 10 years ago

I have verified that this occurs when Persona is the only activated plugin, and it does not occur when I deactivate Persona.

When I click the Logout link in the Admin bar, I am brought to a page that says:

You are attempting to log out of [site name]

Do you really want to log out?

The url for the page is:

wp-login.php?action=logout&_wpnonce=1f9f2cae2a
krydos commented 10 years ago

Hello @dvrcthewrld :) It's looks like regular wordpress confirmation. Do you think will be better without it?

ptrsmk commented 10 years ago

It is a confirmation generated by WordPress; that is true. However, it is not supposed to show up, which is why it doesn't appear on a fresh install. It is related to a deprecated logout procedure.

shane-tomlinson commented 10 years ago

@dvrcthewrld - I have not been able to figure out how to disable that message, any ideas? @KryDos?

krydos commented 10 years ago

@shane-tomlinson I did a little research for this issue and found something. This page shows when logout link looks like 

    <a href="/wp-login.php?action=logout">Logout</a>

"manual logout". It's not a good way. It's not a wordpress way. So wordpress did a wp_logout_url() function for this purpose which adds new parameter _wpnonce. And if logout url contains _wpnonce then this confirmation page should not be displayed.

But I also found that you, @shane-tomlinson, use this function in this plugin. So all should be alright. But it's not. I have no ideas. I think need more research. Maybe it's related with JS logout. I will do research again when I get a free time. If you not mind of course :)

shane-tomlinson commented 10 years ago

Thanks for the research @KryDos! No pressure, I'll try to have a look too!

krydos commented 10 years ago

What I found... We have this problem because we use JS logout.

We can add wp_logout_url() to the href of admin logout link (lib/browserid-admin.php:99) and remove event.preventDefault() from browserid.js:42 and remove urlLogoutRedirect from browserid.js:214,215.

I fix it in my branch bug92... But I'm not sure that it's a good fix because I found the comment about bug in Chrome at the browserid.js:214,215. I checked wordpress logout and it works in the last Firefox and last Chromium. Also I don't know what compress tool you use for javascript compression so I used Minify Javascript Online :)

@shane-tomlinson, do you not mind if I make pull request and if something wrong you decline it, ok?

shane-tomlinson commented 10 years ago

@KryDos - I'm never going to say no to a pull request!

kitchin commented 9 years ago

This fix did not make it into version 0.50. To test it, install 0.50 and then download a zip by going to one of the two "fix logout error issue #92" links above. The two zips are identical except for the minification. They are built on version 0.49.

From the zip, upload only these three files:

browserid/lib/browserid-admin.php browserid/browserid.min.js browserid/browserid.js

Doing it this way instead of installing the zip should keep WP for prompting for an update 0.49 -> 0.50, and you will have the compiled language files (.mo).

I'm testing to see if fix #92 resolves a conflict with the plugin Limit Login Attempts. We get into an endless cycle where it is impossible to login.