Kdyby / Facebook

[DEPRECATED] Use https://github.com/thephpleague/oauth2-facebook instead
Other
42 stars 36 forks source link

getPermissions() is returning only one permission #29

Closed zaxxx closed 9 years ago

zaxxx commented 9 years ago

Is there any reason for Profile::getPermissions() to return only one permission?

Dumping $response in the method shows two granted permissions (I ask for e-mail in my app), but for some reason the method returns $response->data[0], which usually returns "public_profile" (which is always granted), making it pretty useless for actually checking permissions. No idea why would anyone want that... Is that intended behavior?

Passing ['email'] as param didn't help, but that just could be my lack of FB API knowledge, but even if that worked - I wouldn't want to call api many times to check permissions one by one, because... you know.

fprochazka commented 9 years ago

@zaxxx It's possible the API changed, but I'm pretty sure it worked for v1.0 of fb api. Will you please investigate and send a pullrequest?

zaxxx commented 9 years ago

@fprochazka Alright, I'll see what I can do - I'm new to FB development so I have no clue what changed, but I need it in my new job. BTW We are using 2.x (2.1 IIRC), 1.0 will be disabled soon. Thanks for the quick reply!

fprochazka commented 9 years ago

@zaxxx I know, we're using it too, I just haven't used the permissions in the new app yet, so I didn't have the chance to test it.

zaxxx commented 9 years ago

Found relevant info in the changelog: https://developers.facebook.com/docs/apps/changelog?locale=cs_CZ#v2_0_graph_api

Changes from v1.0 to v2.0:

  • The format of the /me/permissions endpoint has changed. It now includes a list of permissions and a status field denoting if they were granted or declined
integer commented 9 years ago

Permissions changed from (v1.0):

{
    "data": [
        {
            "basic_info": 1,
            "public_profile": 1,
            "publish_actions": 1,
            "user_notes": 1,
            "user_friends": 1
        }
    ],
}

to (v2.x)

{
    "data": [
        {
            "permission": "public_profile",
            "status": "granted"
        },
        {
            "permission": "email",
            "status": "granted"
        },
        {
            "permission": "publish_actions",
            "status": "granted"
        },
        {
            "permission": "user_birthday",
            "status": "granted"
        },
        {
            "permission": "user_friends",
            "status": "declined"
        }
    ]
}

I prepared patch with backword compatible output https://github.com/integer/Facebook/commit/3272518bf6da61d1d768de12f64f274523899b17

@zaxxx , can you test it?

zaxxx commented 9 years ago

@integer Sorry, I can't. We abandoned Kdyby/Facebook because we need to handle access tokens by ourselves and there are some other bumps, such as hardcoded param for signed request. Maybe in my free time, but honestly, it's unlikely to happen. Your patch seems okay though.

fprochazka commented 9 years ago

@zaxxx I would apreciate if you at least opened an issue, instead of just throwing the lib away and not telling anybody why.

integer commented 9 years ago

@fprochazka i think he opened this issue. Maybe its not enough to leave library, but nevermind. What about my patch, is it pullable or I should add tests or change something?

fprochazka commented 9 years ago

@integer I like it, please open a pullrequest and I will look into it more :)

zaxxx commented 9 years ago

@fprochazka Sorry, but throwing the lib away wasn't really my call. These issues I mentioned don't hurt me at all, but they seem to hurt my colleague. I'm just passing the message. I wish I could do more, but honestly I'm too busy (and lazy) for that atm. I don't even have time to release some of my libs that I planned to release :-(

fprochazka commented 9 years ago

This has been in https://github.com/Kdyby/Facebook/commit/70fe926b825768fedf494a08be9afe8ea7764840. Thank you for reporting.