thomshouse-escher / escher

Escher was a PHP MVC framework developed by Thom Stricklin from 2011-2013. Escher implemented a hybrid of Model2 MVC and PAC architectures and CMS concepts as well as plugins and configuration wizards. Development halted in 2013 in favor of Kohana and later Symfony, which closely aligned to the same design principles as Escher.
http://git.io/escher
4 stars 1 forks source link

Facebook & Twitter plugins onLogin() #8

Open adetwiler opened 13 years ago

adetwiler commented 13 years ago

Facebook & Twitter plugins onLogin() methods do not account for local auth

Example:

`if(empty($USER->facebook_uid)) { return; }``

should become

if(empty($USER->facebook_uid) || $USER->auth != 'facebook') { return; }

This is to account for Linking Facebook and Twitter accounts on the local auth type.

thomshouse commented 13 years ago

Seems like a good change for user security. I'll get this in there. :)

thomshouse commented 13 years ago

...actually, it seems to me like there are checks in place where appropriate for security reasons. Some of these things should run on login regardless of auth type, so that Escher has the most up-to-date account information.

Is there a particular field that Escher is updating that you don't want updated?

adetwiler commented 13 years ago

Yeah that's a good point, the only field that I don't think should be updated only if an account if Linked and not used as an auth method should be full name. I had to use a different helper to link accounts, I can send you my helpers and maybe we can figure out a way to consolidate them if necessary, or create a different helper. It uses most of the same code.

thomshouse commented 13 years ago

Sure, email me the code and I'll see what it does. I did plan on expanding the FB & Twitter plugins at some point; some of what you've done may be what I had in mind.

As far as the full_name... It will only update the full name if $USER->full_name is equal to the stored value of the auth full_name. However, an additional $USER->auth check here might be appropriate. Lemme think about it.