Hey y’all! I recently migrated from the Ben Parry plugin to the Patreon Connect plugin (version 1.1) and am having a couple issues.
1) Error handling when user email already exists but username is unexpected
If you try to login with Patreon, and your email is assigned to an already-existing wp user, but the username isn’t what it’s expecting (“patreon_” + patreon username), it craps out.
It just needs error handling before line 228 in patreon_login.php, to verify $user_id isn’t of type WP_Error. A sample returned WP_Error would be:
object(WP_Error)[1053]
public ‘errors’ =>
array (size=1)
‘existing_user_email’ =>
array (size=1)
…
public ‘error_data’ =>
array (size=0)
empty
Beyond that, I believe that I should be able to get it to work if I just run through and preface all the usernames with “patreon_” (the Ben Parry plugin didn’t do that, so I have a ton already on there with now improper usernames).
2) On login, state is being returned as “None” or null
When running through the Patreon login flow, the “state” I’m getting returned is now consistently just the string “None” - which breaks login. The login process is expecting an encoded nonce variable to be returned. The code then checks the state’s nonce variable against a cookie with the same variable. Because the state variable is never returned properly, the login fails, and it then goes into a redirect loop.
If I set my own state variables, they get passed through, but without a nonce variable attached.
As an aside: the documentation is frustratingly vague on how state variables are supposed to work – I had to reverse-engineer that it had changed to a set of variables that I had to serialize, base64_encode, and then urlencode. In the Ben Parry version, I just set it as a string and that worked (there was also no nonce at that point in time). Moreover, there’s no mention of special state variables like “final-redirect-uri” (which I would like to use, but I’m unsure if it’s future-proof).
Jason, thanks for reporting these. Very useful post.
1 - im going to look into the existing username/email case by tomorrow. Ill let you know any fix we implement so you can use that one and your custom code wont be overwritten when plugin is updated.
2 - Currently the state var is supposed to be encoded and sent and received back at all times, even if as an empty array.If its not being sent, this may be a problem. Ill try to reproduce this case and break that flow on a test installation tomorrow.
Dev Documentation: You are definitely right about current lack of documentation and i apologize for that. We wanted to get the core user features out of the door fast in the first stage, and then expand on 3rd party dev support. Having accomplished the former just last week, included next in the menu are things like more hooks/filters for developers and documentation.
There should already be a means to filter the final redirect uri, however if a special case is breaking sending/receiving it, we would need to fix it first. I’ll let you know tomorrow.
The main thing I can’t work around (without altering the plugin to disable a security check) is the lack of nonce in the returned state variable. If that can get fixed, I’ll be fine.
Lastly: am I to understand that passing “final-redirect-uri” in the state variable will be supported going forward? That is, is it safe to use (and thus I shouldn’t write my own redirection code with my own custom state variable)?
If security nonce is not being sent in the serialized, encoded array, its not going to be received back. So it must be sent in the first place.
Actually you can not only filter the redirect uri, but entire state array thats being sent. final-redirect-uri is reliable. But it would be wise to wait the result of the fix we may come up with tomorrow.
Preliminary investigation shows the issue to be as you mentioned - for already existing WP users, if these users are registered with their Patreon email, and if they are logging in for the first time with this plugin without currently being logged into WP, it results in an unexpected case for the plugin.
Trying to connect with Patreon while already having been logged into the WP site with that user may fix the problem. But a fix must be developed for this special exception case anyway.
In any case, email matching to identify users was removed from user identification due to security concerns and it possibly wont be coming back.
Ah, I see – I wasn’t aware I had to send it myself (I couldn’t find any sample code). Makes sense. I’ll do that, then.
For the time being I’ll likely make my own redirect code. The Patreon features on my site are already out of commission because of this issue, so I need to implement a change asap. I will wait to hear what you come up with tomorrow.
Matching by email: makes sense. I’ll plan on prefacing all the existing usernames with “patreon_” to fit the new naming scheme. Look forward to hearing more tomorrow.
1 - Non creation of users issue, copying my earlier post from another topic with the same issue:
I think we wont be able to allow direct linking of WP accounts which have the same email with a Patreon account if they were not linked before due to security concerns. This could potentially allow some not so serious security issues.
As this issue only happens when there is an existing, unlinked account in WP with the same Patreon email and user is not already logged into WP, it should be solved by first logging into WP before linking the account.
Please try this with your test account now:
First, log into the WP account which has the same Patreon email
Second, try to log in via Patreon from your site while logged into WP
This sequence is supposed to link your Patreon account to your WP account directly.
For this issue, best option would be to give out a message to the user saying that there is already an account linked for this email and they should log in with it. This would remove the confusion caused.
2 - For the state var issue, i wasnt able to reproduce it on my test install. We may need to investigate it in your environment. If this is a disposable dev environment and i could get wp admin and ftp access to it, it would be great. But if not, we may try to understand why the state var is not being encoded and pushed to Patreon by following the flow if you can provide a link to a locked post in your site.
Users logging in isn’t an option; they have no way of doing so if it isn’t with the Patreon login. The entire purpose of their account in this case is to authenticate their Patreon status, and they have randomly-generated WP passwords that are never revealed to them. If I didn’t have to store a copy of their data on WordPress, I wouldn’t. It seems that the only recourse is for me to change all the usernames to prepend “patreon_” to them. Is this correct?
State var: this is partially my own fault. I am not using any of the Patreon Connect widgets or frontend stuff; I am rolling my own login link and integrating it into my site in my own way. I didn’t realize what I had to put together for it to work using the Patreon Connect plugin (specifically, the encoding and the nonce cookie), but now I do and I can work with it. My only question is, will this workflow change in the future? Am I safe coding up my own login link that mirrors what’s in Patreon Connect? I did not see any, say, shortcodes for dropping down a login link.
To give you a sense of what I’m doing, see the link on the upper-right-hand portion of www.rejectedprincesses.com. (it’s also duplicated in a menu that only appears on mobile)
Lastly: I’d advocate for some unique identifier on the Patreon end (patreon_id or somesuch) to help link WP profiles and Patreon ones. The current login flow could break if the user changes their login email on Patreon or WordPress. That’s fragile.
That would be catastrophic in my case, as I’m doing a fair bit of building on top of their accounts,to attach metadata that Patreon doesn’t currently allow (such as if they bought a book from me).
1 - Adding a user meta named patreon_user_id with the patreon id of the user to every user affected may solve your problem. This is how the linkage is accomplished.
2 - Patreon_Frontend::patreonMakeLoginLink($client_id,$state,$post) for login links (all params can be set to false or skipped)
and
Patreon_Frontend::MakeUniversalFlowLink($pledge_level,$state,$client_id,$post) for flow/unlock links anything except pledge level can be false or skipped)
these two functions can generate the links for you. They will just return a URL, including the over-writable state var.
[patreon_login_button] shortcode drops the login links wherever shortcodes are applicable.
Lastly: I’d advocate for some unique identifier on the Patreon end (patreon_id or somesuch) to help link WP profiles and Patreon ones. The current login flow could break if the user changes their login email on Patreon or WordPress. That’s fragile.
Accounts are linked through patreon_user_id carrying their patreon user id. Emails are irrelevant. User id is supplied for a user in user’s API return.
Gotcha! Okay, that’s stuff that I’d missed in my combing through the plugin. I’ll do my best to update and get the existing user stuff in line with the current plugin – and I’ll use the Patreon_Frontend functions. Thank you!
Posting the same hotfix which shows a message to a user affected by this issue:
You may try this by deactivating, deleting the existing plugin, and then uploading and activating this one. You will need to have an unlinked WP account which has a matching email with the Patreon user you want to test this with.
Please note that if you have Patron Pro, you shouldnt use this hotfix. It will get a patch in the coming days.