Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vkontakte Auth: Change users.get to secure.checkToken #2880

Merged
merged 3 commits into from
Oct 19, 2016

Conversation

antigp
Copy link
Contributor

@antigp antigp commented Oct 18, 2016

You can't get user info by client token due vk restrictions for standalone app. Current code did work, and always get error in production. (User authorization failed: access_token was given to another ip address.) You must check token via secure.checkToken.
https://vk.com/dev/secure.checkToken

To enable client token verification, you mast set appIds and appSecret parameter in oauth dictionary:

oauth: {
        vkontakte: {
            appIds: "1",
            appSecret: "abcdefghijklmn"
        } 
    },

You can't get user info by client token due vk restrictions. You must check token via secure.checkToken.
@antigp antigp changed the title Change users.get to secure.checkToken Vkontakte Auth: Change users.get to secure.checkToken Oct 18, 2016
@flovilmart
Copy link
Contributor

@antigp Thanks for the PR! Can you add some checks that the vk oauth is properly configured with the clientSecret etc... ?

@antigp
Copy link
Contributor Author

antigp commented Oct 18, 2016

@flovilmart Where should I do it? In validateAppId or validateAuthData?
I think it should be in validateAppId.
But i found that at the beginning is called validateAuthData and then only on success will call validateAppId.

@flovilmart
Copy link
Contributor

It should be in validateAuthData as you need the app access token in order to validated the user access token right?

@antigp
Copy link
Contributor Author

antigp commented Oct 18, 2016

Yes.
Ok, i will do this in validateAuthData.

@facebook-github-bot
Copy link

@antigp updated the pull request - view changes

@flovilmart
Copy link
Contributor

Looks like a test if failing, which is good as we enforced new parameters. Can you update the tests?

@facebook-github-bot
Copy link

@antigp updated the pull request - view changes

@antigp
Copy link
Contributor Author

antigp commented Oct 19, 2016

@flovilmart, Sorry but i am new in nodejs, i am iOS dev. I'm don't found how i can check new params in test. I change code to correspond "OAuth Should validate structure of vkontakte" test. Now it always return promise, that check config params. And add error message to logger on error, as it make twitter auth.

@flovilmart
Copy link
Contributor

@antigp it's looking very good! Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants