search

fuel / auth

Fuel PHP Framework - Fuel v1.x Authentication package
http://fuelphp.com/docs/packages/auth/intro.html
76 stars 57 forks source link

Added support for all three callback methods for OpAuth #73

Closed BenWoodford closed 9 years ago

BenWoodford commented 11 years ago

Defaults to 'get', but defining 'callback_transport' in the opauth config now actually works.

Tested with all three.

BenWoodford commented 11 years ago

@FrenkyNet 'whoopsie' is about all I have to say, can't believe I didn't check for a Session class.

frankdejonge commented 11 years ago

@BenWoodford IIRC Opauth sets to $_SESSION directly, since we have no native session but cookie or db(-ish) session, this won't work either. An Arr::get($_SESSION, 'opauth') should do the trick though.

BenWoodford commented 11 years ago

Fixed

WanWizard commented 11 years ago

Opauth strategies indeed use the session, always, so even of you use another callback method. This is a requirement, they simply call session_start() if no native session is active, which is far from ideal.

Perhaps we should add a native session "emulator" to the Session class, using session_set_save_handler(), and re-route the standard session calls...

BenWoodford commented 11 years ago

@WanWizard: Despite that, there is indeed still the issue of the hardcoded GET transport method. I was SSH tunnelling in to my dev box and it got really upset with me because I had an almost 3000 character URL due to the 2950~ character base64 return data as a GET parameter. Some versions of IE don't even support such long URLs.

WanWizard commented 11 years ago

I know, I don't have a problem with addressing that, I only prefer to do it properly. ;)

WanWizard commented 10 years ago

The Session class now has $_SESSION emulation, so the original Opauth session callback should now work without problems. Are you able to check this using 1.8/develop?

WanWizard commented 10 years ago

@BenWoodford any feedback?

BenWoodford commented 10 years ago

Been pretty busy with other projects, I'll have a look at this when I can sorry!

WanWizard commented 10 years ago

No problem, it will not be for 1.7.2 then.

WanWizard commented 10 years ago

@BenWoodford ping? Still a no for 1.7.2 (which goes out this weekend)?

WanWizard commented 9 years ago

Closed due to lack of response.