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

vk: improve code quality #617

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 52 additions & 40 deletions social_core/backends/vk.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,46 @@
from .oauth import BaseOAuth2
from ..exceptions import AuthTokenRevoked, AuthException

DEFAULT_API_VERSION = '5.131'


class VKApiCaller:
API_BASE_URL = 'https://api.vk.com'

def get_api_url(self, method, data):
if 'access_token' in data:
return self.API_BASE_URL + '/method/' + method
return self.API_BASE_URL + '/api.php'

def make_sure_access_token_exists(self, method, data):
if 'access_token' in data:
return data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like access_token is always present, maybe the code can be simplified?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid it's not, when we call the method from VKAppOAuth2, we don't have access_token.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I missed the isAppUser call when reviewing, and lack of test coverage there made me think it is not used. Can you add tests for that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll add.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blacktrub Any chance to get to this? 😄


# We need to perform server-side call if no access_token
key, secret = self.get_key_and_secret()
if 'api_id' not in data:
data['api_id'] = key

data['method'] = method
data['format'] = 'json'
param_list = sorted(list(item + '=' + data[item] for item in data))
data['sig'] = md5((''.join(param_list) + secret).encode('utf-8')).hexdigest()
return data

def call_api(self, method, data):
"""
Calls VK.com OpenAPI method, check:
https://vk.com/apiclub
http://goo.gl/yLcaa
"""
data['v'] = self.setting('API_VERSION', DEFAULT_API_VERSION)
data = self.make_sure_access_token_exists(method, data)
url = self.get_api_url(method, data)
try:
return self.get_json(url, params=data)
except (TypeError, KeyError, IOError, ValueError, IndexError):
return None


class VKontakteOpenAPI(BaseAuth):
"""VK.COM OpenAPI authentication backend"""
Expand Down Expand Up @@ -57,7 +97,7 @@ def auth_complete(self, *args, **kwargs):
check_str = ''.join(item + '=' + mapping[item]
for item in ['expire', 'mid', 'secret', 'sid'])

key, secret = self.get_key_and_secret()
_, secret = self.get_key_and_secret()
hash = md5((check_str + secret).encode('utf-8')).hexdigest()
if hash != mapping['sig'] or int(mapping['expire']) < time():
raise ValueError('VK.com authentication failed: Invalid Hash')
Expand All @@ -73,7 +113,7 @@ def uses_redirect(self):
return False


class VKOAuth2(BaseOAuth2):
class VKOAuth2(VKApiCaller, BaseOAuth2):
"""VKOAuth2 authentication backend"""
name = 'vk-oauth2'
ID_KEY = 'id'
Expand Down Expand Up @@ -103,10 +143,13 @@ def user_data(self, access_token, *args, **kwargs):
'photo'] + self.setting('EXTRA_DATA', [])

fields = ','.join(set(request_data))
data = vk_api(self, 'users.get', {
'access_token': access_token,
'fields': fields,
})
data = self.call_api(
'users.get',
{
'access_token': access_token,
'fields': fields,
}
)

if data and data.get('error'):
error = data['error']
Expand Down Expand Up @@ -151,11 +194,9 @@ def auth_complete(self, *args, **kwargs):
if user_check == 1:
is_user = self.data.get('is_app_user')
elif user_check == 2:
is_user = vk_api(
self,
'isAppUser',
{'user_id': user_id}
).get('response', 0)
response = self.call_api('isAppUser', {'user_id': user_id})
is_user = response.get('response', 0)

if not int(is_user):
return None

Expand All @@ -169,32 +210,3 @@ def auth_complete(self, *args, **kwargs):
}
auth_data['response'].update(json.loads(auth_data['request']['api_result'])['response'][0])
return self.strategy.authenticate(*args, **auth_data)


def vk_api(backend, method, data):
"""
Calls VK.com OpenAPI method, check:
https://vk.com/apiclub
http://goo.gl/yLcaa
"""
# We need to perform server-side call if no access_token
data['v'] = backend.setting('API_VERSION', '5.131')
if 'access_token' not in data:
key, secret = backend.get_key_and_secret()
if 'api_id' not in data:
data['api_id'] = key

data['method'] = method
data['format'] = 'json'
url = 'https://api.vk.com/api.php'
param_list = sorted(list(item + '=' + data[item] for item in data))
data['sig'] = md5(
(''.join(param_list) + secret).encode('utf-8')
).hexdigest()
else:
url = 'https://api.vk.com/method/' + method

try:
return backend.get_json(url, params=data)
except (TypeError, KeyError, OSError, ValueError, IndexError):
return None