FriendsOfSymfony / FOSFacebookBundle

NOT MAINTAINED - see https://github.com/hwi/HWIOAuthBundle
322 stars 140 forks source link

Enable user authentication by access token #225

Closed jhkchan closed 11 years ago

jhkchan commented 11 years ago

As the case written in http://stackoverflow.com/questions/4623974/design-for-facebook-authentication-in-an-ios-app-that-also-accesses-a-secured-we, it is possible that access tokens obtained from mobile apps (iOS or Android) would be used for server side authentication.

This change set retrieves access_token pass by request during facebook login check to enable facebook user authentication by access token. This also handles the issue #210.

diegoholiveira commented 11 years ago

Using the access_token for auth can cause security issues? I mean, if someone get your access_token it will be possible to pass as you, right?

jhkchan commented 11 years ago

I agree. So developers should understand that it should be passed by HTTPS only just like using graph.facebook.com. This is a valid use-case.

diegoholiveira commented 11 years ago

Can you provide some documentation about it? If you include examples of how and where to use it will be awesome.

jhkchan commented 11 years ago

Should I do it here, or edit README.mdand pull again? I will provide detailed steps on API calls using Facebook SDK and called by mobile app.

diegoholiveira commented 11 years ago

Do it in the README.md and pull again, please. :)

jhkchan commented 11 years ago

Is this readme okay?

diegoholiveira commented 11 years ago

It's fine.

I'll merge it once the tests are green. Take a look here: https://travis-ci.org/FriendsOfSymfony/FOSFacebookBundle/jobs/4073267

jhkchan commented 11 years ago

I read it but don't understand why updating readme will make build fail? I saw the signature of AuthenticationException is different but I didn't change there. Is there something that I need to fix?

jhkchan commented 11 years ago

I passed the unit test on my local side, but not on Travis. I find Travis uses Symfony 2.2, not Symfony 2.1. That could be the problem.

diegoholiveira commented 11 years ago

I'll tag specific versions for symfony 2.1 and symfony 2.2 (I guess I can do it later today).

jhkchan commented 11 years ago

I have tried to reproduce the issue and identified the problem exists in the current codebase.

To illustrate, I would like to reproduce the error by the following reduced example:

<?php

try {
    throw new \Exception('message');
} catch (\Exception $failed) {
    throw new \RuntimeException($failed->getMessage(), null, (int)$failed->getCode(), $failed);
}

?>

This will return

PHP Fatal error:  Wrong parameters for Exception([string $exception [, long $code [, Exception $previous = NULL]]]) in
XXX/xxx.php on line 6

The correct code would be removing the argument null.

References: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Core/Exception/AuthenticationException.php http://php.net/manual/en/class.runtimeexception.php http://php.net/manual/en/class.exception.php

Why the current codebase would not trigger this because all are caught by the first catch identifying AuthenticationException. I am not sure why my code triggered the coverage path to \Exception but that is the case. Should I commit the fix to here as well? Thanks!

diegoholiveira commented 11 years ago

Could you update your PR with the last version on master? This last version use a stable version of symfony 2.1 and maybe it can solve this problem. If not, I'll take a look to see what's happen.

thanks.

jhkchan commented 11 years ago

Please let me know what I can do. Thanks.

diegoholiveira commented 11 years ago

I'll take a look on it and give you an answer.

jhkchan commented 11 years ago

I would suggest you to rebuild again on Travis. Something was wrong. Now I have mine passed: https://travis-ci.org/jhkchan/FOSFacebookBundle

diegoholiveira commented 11 years ago

I will take a look on your branch on my machine, but I need to take some time for do it :(

On Thu, Jan 31, 2013 at 4:22 PM, Jacky Chan notifications@github.comwrote:

I would suggest you to rebuild again on Travis. Something was wrong. Now I have mine passed: https://travis-ci.org/jhkchan/FOSFacebookBundle

— Reply to this email directly or view it on GitHubhttps://github.com/FriendsOfSymfony/FOSFacebookBundle/pull/225#issuecomment-12957271.