From 5f1096d38770c41030668aba62e456d5bdc46714 Mon Sep 17 00:00:00 2001 From: Lincoln Karuhanga Date: Sun, 24 May 2020 14:17:08 +0300 Subject: [PATCH 1/2] dev-patches --- Dockerfile-dependencies | 7 ++++--- requirements.txt | 2 +- requirements/base.txt | 2 ++ requirements/local.txt | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Dockerfile-dependencies b/Dockerfile-dependencies index e6613da3..9ecee658 100644 --- a/Dockerfile-dependencies +++ b/Dockerfile-dependencies @@ -23,11 +23,12 @@ RUN gem install --no-rdoc --no-ri compass -v 1.0.3 RUN mkdir /code -ADD requirements.txt /code/ -ADD requirements/ /code/requirements/ ADD package.json /code/ WORKDIR /code -RUN pip install -r requirements.txt RUN npm install RUN npm install -g grunt-cli + +ADD requirements.txt /code/ +ADD requirements/ /code/requirements/ +RUN pip install -r requirements.txt diff --git a/requirements.txt b/requirements.txt index d1197135..e62fee90 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ # This file is here because many Platforms as a Service look for # requirements.txt in the root directory of a project. --r requirements/production.txt +-r requirements/local.txt diff --git a/requirements/base.txt b/requirements/base.txt index 0ad4a448..04692a64 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -37,3 +37,5 @@ python-dateutil==2.2 sqlparse==0.1.19 +# explicitly call out this because later versions dropped python2 support +django-appconf==1.0.2 diff --git a/requirements/local.txt b/requirements/local.txt index ba98c7fc..49a9695d 100644 --- a/requirements/local.txt +++ b/requirements/local.txt @@ -1,5 +1,5 @@ # Local development dependencies go here --r base.txt +-r production.txt mock==2.0.0 coverage==3.7.1 django-discover-runner==1.0 From bb2e860b0bc3c9cd4f00649076275ba87990fc1e Mon Sep 17 00:00:00 2001 From: Lincoln Karuhanga Date: Sun, 24 May 2020 22:09:17 +0300 Subject: [PATCH 2/2] OpenConceptLab/ocl_issues#220: Refactor Login and Signup to use API --- ocl_web/config/settings.py | 6 ++--- ocl_web/config/urls.py | 4 +++ ocl_web/users/adapter.py | 47 ++++++++++++++++++++++++++++++--- ocl_web/users/auth_backends.py | 48 ++++++++++++++++++++++++++++++++++ ocl_web/users/base_forms.py | 39 +++++++++++++++++++++++++++ ocl_web/users/forms.py | 43 +++++++++++------------------- ocl_web/users/models.py | 33 ++++++++++++++++------- ocl_web/users/signals.py | 44 +++++-------------------------- ocl_web/users/views.py | 9 ++++++- 9 files changed, 192 insertions(+), 81 deletions(-) create mode 100644 ocl_web/users/auth_backends.py create mode 100644 ocl_web/users/base_forms.py diff --git a/ocl_web/config/settings.py b/ocl_web/config/settings.py index 2d9a9eee..fcaa825d 100644 --- a/ocl_web/config/settings.py +++ b/ocl_web/config/settings.py @@ -237,14 +237,14 @@ class Common(Configuration): ########## AUTHENTICATION CONFIGURATION AUTHENTICATION_BACKENDS = ( "django.contrib.auth.backends.ModelBackend", - "allauth.account.auth_backends.AuthenticationBackend", + "users.auth_backends.APIAuthenticationBackend", ) # Some really nice defaults ACCOUNT_AUTHENTICATION_METHOD = "username" ACCOUNT_EMAIL_REQUIRED = True - ACCOUNT_EMAIL_VERIFICATION = "mandatory" - ACCOUNT_SIGNUP_FORM_CLASS = 'users.forms.SignupForm' + ACCOUNT_EMAIL_VERIFICATION = "none" # API does not yet support email verification + ACCOUNT_SIGNUP_FORM_CLASS = 'users.base_forms.BaseSignupForm' # refer to class for context ACCOUNT_ADAPTER = 'users.adapter.OCLAccountAdapter' ########## END AUTHENTICATION CONFIGURATION diff --git a/ocl_web/config/urls.py b/ocl_web/config/urls.py index cc1eb927..5b4776e5 100644 --- a/ocl_web/config/urls.py +++ b/ocl_web/config/urls.py @@ -18,6 +18,9 @@ # Uncomment the next two lines to enable the admin: from django.contrib import admin + +from users.views import signup + admin.autodiscover() @@ -39,6 +42,7 @@ # User management url(r'^users/', include("users.urls", namespace="users")), + url(r"^accounts/signup/$", signup, name="account_signup"), # override signup to stop the form saving EmailAddresses url(r'^accounts/', include('allauth.urls')), # User avatars - /avatar/... diff --git a/ocl_web/users/adapter.py b/ocl_web/users/adapter.py index b68a1431..6188aae3 100644 --- a/ocl_web/users/adapter.py +++ b/ocl_web/users/adapter.py @@ -1,18 +1,53 @@ +import logging import re +from django.contrib.auth import authenticate from django.utils.translation import ugettext_lazy as _ from django import forms from config import settings from allauth.account.adapter import DefaultAccountAdapter -from urlparse import urlsplit, urlunsplit + +from libs.ocl import OclApi + + +logger = logging.getLogger('oclweb') + class OCLAccountAdapter(DefaultAccountAdapter): """ - A custom adapter for django-allauth so that we can limit username - to a smaller set of allowed characters for the API. + A custom adapter for django-allauth so that we can; + 1. limit username to a smaller set of allowed characters for the API. + 2. create users using the API and not in the DB. """ + def save_user(self, request, user, form, commit=True): + user = super(OCLAccountAdapter, self).save_user(request, user, form, commit=False) + ocl = OclApi(admin=True, debug=True) + data = { + 'username': user.username, + 'email': user.email, + 'hashed_password': user.password, + 'name': '%s %s' % (user.first_name, user.last_name), # not great + } + + result = ocl.create_user(data) + if result.status_code == 201: + authenticated_user = authenticate(username=form.cleaned_data['username'], password=form.cleaned_data["password1"]) + # don't return a new user object because the old one was passed to us + # and we cannot control whether or not the caller continues to use it + user.backend = authenticated_user.backend + user.ocl_api_token_object = authenticated_user and authenticated_user.ocl_api_token_object + return user + # elif result.status_code == 400: + # # todo: agree on cleaner way of handling account re-activations, + # leaving this here for context into what we were doing before. + # result = ocl.reactivate_user(user.username) + # if result != 204: + # return + + result.raise_for_status() + def clean_username(self, username): """ Validates the username. You can hook into this if you want to @@ -37,3 +72,9 @@ def send_mail(self, template_prefix, email, context): msg = self.render_mail(template_prefix, email, context) msg.send() + + def confirm_email(self, request, email_address): + """ + Marks the email address as confirmed on the db + """ + logger.debug("Email Confirmed signal for %s", request.user.username) diff --git a/ocl_web/users/auth_backends.py b/ocl_web/users/auth_backends.py new file mode 100644 index 00000000..85897e67 --- /dev/null +++ b/ocl_web/users/auth_backends.py @@ -0,0 +1,48 @@ +from allauth.account import app_settings +from allauth.account.auth_backends import AuthenticationBackend + +from libs.ocl import OclApi +from users.models import User + + +class APIAuthenticationBackend(AuthenticationBackend): + def get_user(self, user_id): + return User(username=user_id) + + def _authenticate_by_username(self, **credentials): + username_field = app_settings.USER_MODEL_USERNAME_FIELD + if not username_field: + return None + return self._authenticate(username=credentials.get(username_field), password=credentials.get('password')) + + def _authenticate_by_email(self, **credentials): + email = credentials.get('email', credentials.get('username')) + return self._authenticate(username=email, password=credentials.get('password')) + + @staticmethod + def _authenticate(username=None, password=None): + ocl = OclApi() + + def authenticate_with_api(_username, _password, hashed=True): + api_response = ocl.get_user_auth(_username, _password, hashed=hashed) + + if api_response.status_code == 200: + _user = User(username=username) + _user.ocl_api_token_object = api_response.json() + return _user + return None + + result = authenticate_with_api(username, password, hashed=False) + if result: + return result + else: + # try to fallback to existing user with hashed password + try: + user = User.objects.get(username=username) + result = authenticate_with_api(user.username, user.password) + if result: + return result + except User.DoesNotExist: + pass + + return None diff --git a/ocl_web/users/base_forms.py b/ocl_web/users/base_forms.py new file mode 100644 index 00000000..aee3c76e --- /dev/null +++ b/ocl_web/users/base_forms.py @@ -0,0 +1,39 @@ +""" +Pull BaseSignupForm here to prevent circular import between us and django-allauth +""" + +from django.utils.translation import ugettext as _ +from django import forms + +from .models import User + + +class BaseSignupForm(forms.ModelForm): + """ + Custom form for user to sign up for an account. + Used by django-allauth as a **base class** for their signup form + """ + required_css_class = 'required' + + first_name = forms.CharField(max_length=30, label=_('First Name'), required=True) + last_name = forms.CharField(max_length=30, label=_('Last Name'), required=True) + + class Meta: + """ Meta class """ + # Set this form to use the User model. + model = User + + # Constrain the UserForm to just these fields. + fields = ("first_name", "last_name") + + def signup(self, request, user): + """ signup """ + user.first_name = self.cleaned_data['first_name'] + user.last_name = self.cleaned_data['last_name'] + user.save() + + def save(self, user): + """ save """ + print 'In SignupForm save:', user.username + print user.first_name, user.email + return user diff --git a/ocl_web/users/forms.py b/ocl_web/users/forms.py index 9928fbb9..b16a38c8 100644 --- a/ocl_web/users/forms.py +++ b/ocl_web/users/forms.py @@ -2,12 +2,14 @@ """ Forms for users. """ +from allauth.account.adapter import get_adapter +from allauth.account.forms import SignupForm as AllAuthSignupForm from django.utils.translation import ugettext as _ from django import forms -#from django.core.urlresolvers import reverse from .models import User + class UserForm(forms.Form): """ User edit form """ required_css_class = 'required' @@ -32,31 +34,16 @@ def clean_email(self): return data -class SignupForm(forms.ModelForm): - """ - Custom form for user to sign up for an account, used by django-allauth - """ - required_css_class = 'required' - - first_name = forms.CharField(max_length=30, label=_('First Name'), required=True) - last_name = forms.CharField(max_length=30, label=_('Last Name'), required=True) - - class Meta: - """ Meta class """ - # Set this form to use the User model. - model = User - - # Constrain the UserForm to just these fields. - fields = ("first_name", "last_name") - - def signup(self, request, user): - """ signup """ - user.first_name = self.cleaned_data['first_name'] - user.last_name = self.cleaned_data['last_name'] - user.save() - - def save(self, user): - """ save """ - print 'In SignupForm save:', user.username - print user.first_name, user.email +class SignupForm(AllAuthSignupForm): + def save(self, request): + """ + Carried from `allauth.account.forms.SignupForm` as of django-allauth==0.15.0. + We override the view and use this form to stop the form saving EmailAddresses. + """ + adapter = get_adapter() + user = adapter.new_user(request) + adapter.save_user(request, user, self) + # formerly + # super(SignupForm, self).save(user) + # setup_user_email(request, user, []) return user diff --git a/ocl_web/users/models.py b/ocl_web/users/models.py index eb0ca4c6..e56f1d37 100644 --- a/ocl_web/users/models.py +++ b/ocl_web/users/models.py @@ -1,11 +1,11 @@ """ Users models """ # -*- coding: utf-8 -*- -from django.contrib.auth.models import AbstractUser -#from django.utils.translation import ugettext_lazy as _ +from django.contrib.auth.models import AbstractUser, update_last_login +from django.contrib.auth.signals import user_logged_in as django_auth_signals_user_logged_in -from allauth.account.signals import (user_signed_up, user_logged_in, email_confirmed, password_reset) -from .signals import (user_created_handler, user_logged_in_handler, email_confirmed_handler, user_password_reset_handler) +from allauth.account.signals import user_logged_in, email_confirmed, password_reset +from .signals import (user_logged_in_handler, email_confirmed_handler, user_password_reset_handler) # Subclass AbstractUser @@ -13,17 +13,32 @@ class User(AbstractUser): """ This is the extended Django user on the web application, so that we can add additional data and behavior to the base user model. - - Each front end django user has a corresponding user entry in the - API backend. """ + # used to pass this object between the (APIAuthBackend or AccountAdapter) and the post login handler + ocl_api_token_object = None def __unicode__(self): return self.username + @property + def pk(self): + """ + The Django auth system uses this as the session id and + passes it to out backend to fetch the user on every request. + + Fallback to something we know for those situations. + """ + return super(User, self).pk or self.username + + def save(self, force_insert=False, force_update=False, using=None, + update_fields=None): + raise NotImplementedError("User management is done via the API so you should probably not be calling this.") + # Hook up signals to django-allauth -user_signed_up.connect(user_created_handler) email_confirmed.connect(email_confirmed_handler) user_logged_in.connect(user_logged_in_handler) -password_reset.connect(user_password_reset_handler) \ No newline at end of file +password_reset.connect(user_password_reset_handler) + +# disconnect this receiver because it attempts to update the last_login and save: we're not storing user objects anymore +django_auth_signals_user_logged_in.disconnect(update_last_login) diff --git a/ocl_web/users/signals.py b/ocl_web/users/signals.py index 0c886235..ba141eb2 100644 --- a/ocl_web/users/signals.py +++ b/ocl_web/users/signals.py @@ -1,36 +1,8 @@ """ Signal handlers to interface with the django-allauth package. - - Tie the front end user models with the backend, each front end - user has a correponding entry in the API database. """ -from libs.ocl import OclApi - - -def user_created_handler(sender, request, user, **kwargs): - """ - Signal handler called when a new user is created, so that we can create - a corresponding user at the backend. - """ - ocl = OclApi(admin=True, debug=True) - data = { - 'username': user.username, - 'email': user.email, - 'hashed_password': user.password, - 'name': '%s %s' % (user.first_name, user.last_name), # not great - } - result = ocl.create_user(data) - if result.status_code == 201: - return - elif result.status_code == 400: - # try reactivate for now, this is very not secure, #TODO - result = ocl.reactivate_user(user.username) - if result != 204: - return - - raise Exception('Failed to create user due to: %s' % result) - +from libs.ocl import OclApi def email_confirmed_handler(sender, request, email_address, **kwargs): @@ -49,15 +21,14 @@ def user_logged_in_handler(sender, request, user, **kwargs): Signal handler called when a user logged into the web app. We need to retrieve the backend auth token for subsequent access. The token is saved in the session. + + Ideally, we would have handled this in the authentication backend, but this version of Django + doesn't pass a request object to the `users.auth_backend.APIAuthenticationBackend.authenticate` method. + The first version that does is 1.11(https://docs.djangoproject.com/en/3.0/releases/1.11/). """ ocl = OclApi() - if 'password' in request.POST: - #Login with not hashed password by default, because some users have been created prior to api and web using to the same hashing - result = ocl.get_user_auth(user.username, request.POST['password'], False) - else: - result = ocl.get_user_auth(user.username, user.password) - if result.status_code == 200: - ocl.save_auth_token(request, result.json()) + ocl.save_auth_token(request, user.ocl_api_token_object) + def user_password_reset_handler(sender, request, user, **kwargs): ocl = OclApi(admin=True, debug=True) @@ -66,4 +37,3 @@ def user_password_reset_handler(sender, request, user, **kwargs): result = ocl.get_user_auth(user.username, user.password) if result.status_code == 200: ocl.save_auth_token(request, result.json()) - diff --git a/ocl_web/users/views.py b/ocl_web/users/views.py index eaa43cfb..1fd8fe15 100644 --- a/ocl_web/users/views.py +++ b/ocl_web/users/views.py @@ -2,6 +2,7 @@ """ import simplejson as json +from allauth.account.views import SignupView as AllAuthSignupView from django.utils.translation import ugettext as _ from django.core.urlresolvers import reverse @@ -19,7 +20,7 @@ # Import the form from users/forms.py from django.views.generic import View -from .forms import UserForm +from .forms import UserForm, SignupForm # Import the customized User model from .models import User @@ -27,6 +28,12 @@ from django.http import HttpResponse +class SignupView(AllAuthSignupView): + form_class = SignupForm + + +signup = SignupView.as_view() + class UserDetailView(LoginRequiredMixin, DetailView): """OCL Users Detail view