From f6c5f10d6599a411cae520351796a6b5fbb2d687 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 18 Dec 2018 13:24:26 +0100 Subject: [PATCH] oauth_client: cleanup --- .gitignore | 1 + passbook/oauth_client/errors.py | 11 -------- .../migrations/0002_auto_20181218_1019.py | 17 +++++++++++++ .../oauth_client/source_types/facebook.py | 3 --- passbook/oauth_client/source_types/github.py | 4 --- passbook/oauth_client/source_types/twitter.py | 3 --- passbook/oauth_client/utils.py | 5 ++-- passbook/oauth_client/views/core.py | 25 ++++--------------- 8 files changed, 25 insertions(+), 44 deletions(-) delete mode 100644 passbook/oauth_client/errors.py create mode 100644 passbook/oauth_client/migrations/0002_auto_20181218_1019.py diff --git a/.gitignore b/.gitignore index 8289a028d..18f3bfc10 100644 --- a/.gitignore +++ b/.gitignore @@ -190,3 +190,4 @@ pip-selfcheck.json # End of https://www.gitignore.io/api/python,django /static/* +local.env.yml diff --git a/passbook/oauth_client/errors.py b/passbook/oauth_client/errors.py deleted file mode 100644 index 27ded1b18..000000000 --- a/passbook/oauth_client/errors.py +++ /dev/null @@ -1,11 +0,0 @@ -"""passbook oauth_client Errors""" - - -class OAuthClientError(Exception): - """Base error for all OAuth Client errors""" - pass - - -class OAuthClientEmailMissingError(OAuthClientError): - """Error which is raised when user is missing email address from profile""" - pass diff --git a/passbook/oauth_client/migrations/0002_auto_20181218_1019.py b/passbook/oauth_client/migrations/0002_auto_20181218_1019.py new file mode 100644 index 000000000..e276234b4 --- /dev/null +++ b/passbook/oauth_client/migrations/0002_auto_20181218_1019.py @@ -0,0 +1,17 @@ +# Generated by Django 2.1.4 on 2018-12-18 10:19 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_oauth_client', '0001_initial'), + ] + + operations = [ + migrations.AlterModelOptions( + name='oauthsource', + options={'verbose_name': 'Generic OAuth Source', 'verbose_name_plural': 'Generic OAuth Sources'}, + ), + ] diff --git a/passbook/oauth_client/source_types/facebook.py b/passbook/oauth_client/source_types/facebook.py index e8d8de371..5246f8beb 100644 --- a/passbook/oauth_client/source_types/facebook.py +++ b/passbook/oauth_client/source_types/facebook.py @@ -2,7 +2,6 @@ from django.contrib.auth import get_user_model -from passbook.oauth_client.errors import OAuthClientEmailMissingError from passbook.oauth_client.source_types.manager import MANAGER, RequestKind from passbook.oauth_client.utils import user_get_or_create from passbook.oauth_client.views.core import OAuthCallback, OAuthRedirect @@ -23,8 +22,6 @@ class FacebookOAuth2Callback(OAuthCallback): """Facebook OAuth2 Callback""" def get_or_create_user(self, source, access, info): - if 'email' not in info: - raise OAuthClientEmailMissingError() user = get_user_model() user_data = { user.USERNAME_FIELD: info.get('name'), diff --git a/passbook/oauth_client/source_types/github.py b/passbook/oauth_client/source_types/github.py index 176cd1a2a..6068088e4 100644 --- a/passbook/oauth_client/source_types/github.py +++ b/passbook/oauth_client/source_types/github.py @@ -2,7 +2,6 @@ from django.contrib.auth import get_user_model -from passbook.oauth_client.errors import OAuthClientEmailMissingError from passbook.oauth_client.source_types.manager import MANAGER, RequestKind from passbook.oauth_client.utils import user_get_or_create from passbook.oauth_client.views.core import OAuthCallback @@ -13,10 +12,7 @@ class GitHubOAuth2Callback(OAuthCallback): """GitHub OAuth2 Callback""" def get_or_create_user(self, source, access, info): - if 'email' not in info or info['email'] == '': - raise OAuthClientEmailMissingError() user = get_user_model() - print(info) user_data = { user.USERNAME_FIELD: info.get('login'), 'email': info.get('email', ''), diff --git a/passbook/oauth_client/source_types/twitter.py b/passbook/oauth_client/source_types/twitter.py index f747047a5..95bd2bca8 100644 --- a/passbook/oauth_client/source_types/twitter.py +++ b/passbook/oauth_client/source_types/twitter.py @@ -6,7 +6,6 @@ from django.contrib.auth import get_user_model from requests.exceptions import RequestException from passbook.oauth_client.clients import OAuthClient -from passbook.oauth_client.errors import OAuthClientEmailMissingError from passbook.oauth_client.source_types.manager import MANAGER, RequestKind from passbook.oauth_client.utils import user_get_or_create from passbook.oauth_client.views.core import OAuthCallback @@ -37,8 +36,6 @@ class TwitterOAuthCallback(OAuthCallback): client_class = TwitterOAuthClient def get_or_create_user(self, source, access, info): - if 'email' not in info: - raise OAuthClientEmailMissingError() user = get_user_model() user_data = { user.USERNAME_FIELD: info.get('screen_name'), diff --git a/passbook/oauth_client/utils.py b/passbook/oauth_client/utils.py index d2ed8d1f0..91ab2211b 100644 --- a/passbook/oauth_client/utils.py +++ b/passbook/oauth_client/utils.py @@ -1,6 +1,4 @@ -""" -OAuth Client User Creation Utils -""" +"""OAuth Client User Creation Utils""" from django.contrib.auth import get_user_model from django.db.utils import IntegrityError @@ -13,5 +11,6 @@ def user_get_or_create(user_model=None, **kwargs): try: new_user = user_model.objects.create_user(**kwargs) except IntegrityError: + # TODO: Fix potential username change vuln new_user = user_model.objects.get(username=kwargs['username']) return new_user diff --git a/passbook/oauth_client/views/core.py b/passbook/oauth_client/views/core.py index 10f915e37..fb5819b33 100644 --- a/passbook/oauth_client/views/core.py +++ b/passbook/oauth_client/views/core.py @@ -14,14 +14,12 @@ from django.views.generic import RedirectView, View from passbook.lib.utils.reflection import app from passbook.oauth_client.clients import get_client -from passbook.oauth_client.errors import (OAuthClientEmailMissingError, - OAuthClientError) from passbook.oauth_client.models import OAuthSource, UserOAuthSourceConnection LOGGER = getLogger(__name__) -# pylint: disable=too-few-public-methods, too-many-locals +# pylint: disable=too-few-public-methods class OAuthClientMixin: "Mixin for getting OAuth client for a source." @@ -48,7 +46,8 @@ class OAuthRedirect(OAuthClientMixin, RedirectView): def get_callback_url(self, source): "Return the callback url for this source." - return reverse('oauth-client-callback', kwargs={'source_slug': source.slug}) + return reverse('passbook_oauth_client:oauth-client-callback', + kwargs={'source_slug': source.slug}) def get_redirect_url(self, **kwargs): "Build redirect url for a given source." @@ -72,7 +71,6 @@ class OAuthCallback(OAuthClientMixin, View): source_id = None source = None - # pylint: disable=too-many-return-statements def get(self, request, *args, **kwargs): """View Get handler""" slug = kwargs.get('source_slug', '') @@ -115,20 +113,7 @@ class OAuthCallback(OAuthClientMixin, View): ) user = authenticate(source=self.source, identifier=identifier, request=request) if user is None: - try: - return self.handle_new_user(self.source, connection, info) - except OAuthClientEmailMissingError as exc: - return render(request, 'common/error.html', { - 'code': 500, - 'exc_message': _("source %(name)s didn't provide an E-Mail address." % { - 'name': self.source.name - }), - }) - except OAuthClientError as exc: - return render(request, 'common/error.html', { - 'code': 500, - 'exc_message': str(exc), - }) + return self.handle_new_user(self.source, connection, info) return self.handle_existing_user(self.source, user, connection, info) # pylint: disable=unused-argument @@ -144,7 +129,7 @@ class OAuthCallback(OAuthClientMixin, View): # pylint: disable=unused-argument def get_login_redirect(self, source, user, access, new=False): "Return url to redirect authenticated users." - return 'overview' + return 'passbook_core:overview' def get_or_create_user(self, source, access, info): "Create a shell auth.User."