From 2cc2a3c41296488191b5ebcd070e842decb04e5e Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 15:35:14 +0000 Subject: [PATCH 1/7] new cfl package --- Pipfile | 4 ++-- Pipfile.lock | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Pipfile b/Pipfile index 34c02ba9..db6df4f3 100644 --- a/Pipfile +++ b/Pipfile @@ -23,7 +23,7 @@ name = "pypi" # 5. Run `pipenv install --dev` in your terminal. [packages] -codeforlife = {ref = "v0.20.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} # 🚫 Don't add [packages] below that are inherited from the CFL package. pyjwt = "==2.6.0" # TODO: upgrade to latest version # TODO: Needed by RR. Remove when RR has moved to new system. @@ -32,7 +32,7 @@ django-sekizai = "==2.0.0" django-classy-tags = "==2.0.0" [dev-packages] -codeforlife = {ref = "v0.20.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} +codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} # codeforlife = {file = "../codeforlife-package-python", editable = true, extras = ["dev"]} # 🚫 Don't add [dev-packages] below that are inherited from the CFL package. diff --git a/Pipfile.lock b/Pipfile.lock index d3ab18a1..2f302917 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "2d76db5e5625c1ba7d4a7ac0b2d0a461b0d10bb8b07849fd028108e4d95fdd6f" + "sha256": "89ce3a71a998d0972e556dd872cae78589a28581de26f03fd1ebf040dcbb05c1" }, "pipfile-spec": 6, "requires": { @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "ecc5db6b0b45e64488d60eea18915ab6bb224393" + "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" }, "codeforlife-portal": { "hashes": [ @@ -1099,7 +1099,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "ecc5db6b0b45e64488d60eea18915ab6bb224393" + "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" }, "codeforlife-portal": { "hashes": [ From 334a8581e92bda1784d351b0b2ebd9236dafb6b3 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 15:39:48 +0000 Subject: [PATCH 2/7] fix linting errors --- src/api/serializers/auth_factor.py | 2 +- src/api/views/school.py | 1 + src/rapid_router/models/user.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/api/serializers/auth_factor.py b/src/api/serializers/auth_factor.py index a3318478..6069fa8f 100644 --- a/src/api/serializers/auth_factor.py +++ b/src/api/serializers/auth_factor.py @@ -9,7 +9,7 @@ from rest_framework import serializers -# pylint: disable-next=missing-class-docstring +# pylint: disable-next=missing-class-docstring,too-many-ancestors class AuthFactorSerializer(ModelSerializer[User, AuthFactor]): class Meta: model = AuthFactor diff --git a/src/api/views/school.py b/src/api/views/school.py index 6f10d09f..70c2c1d0 100644 --- a/src/api/views/school.py +++ b/src/api/views/school.py @@ -31,6 +31,7 @@ def get_permissions(self): return super().get_permissions() + # pylint: disable-next=missing-function-docstring def destroy(self, request, *args, **kwargs): school = self.get_object() diff --git a/src/rapid_router/models/user.py b/src/rapid_router/models/user.py index e3663ee9..af330c64 100644 --- a/src/rapid_router/models/user.py +++ b/src/rapid_router/models/user.py @@ -20,6 +20,7 @@ TypedModelMeta = object +# pylint: disable-next=too-many-ancestors class User(_User): """A Rapid Router user.""" From 361beed1d149b7c0957abff729cbbf3caeba0da4 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 15:47:42 +0000 Subject: [PATCH 3/7] disable=too-many-ancestors --- Pipfile.lock | 4 ++-- src/api/serializers/auth_factor_test.py | 4 +++- src/api/serializers/klass_test.py | 1 + src/api/serializers/school_teacher_invitation_test.py | 2 +- src/api/serializers/school_test.py | 4 +++- src/api/serializers/teacher_test.py | 1 + src/api/serializers/user_test.py | 1 + src/rapid_router/serializers/level_test.py | 1 + 8 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 2f302917..6d4e529e 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" + "ref": "882de7316c5c5054b7ca02d8351704a8b298d398" }, "codeforlife-portal": { "hashes": [ @@ -1099,7 +1099,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" + "ref": "882de7316c5c5054b7ca02d8351704a8b298d398" }, "codeforlife-portal": { "hashes": [ diff --git a/src/api/serializers/auth_factor_test.py b/src/api/serializers/auth_factor_test.py index c8053e21..f322de1a 100644 --- a/src/api/serializers/auth_factor_test.py +++ b/src/api/serializers/auth_factor_test.py @@ -9,8 +9,10 @@ from ..views import AuthFactorViewSet from .auth_factor import AuthFactorSerializer +# pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors + -# pylint: disable-next=missing-class-docstring class TestAuthFactorSerializer(ModelSerializerTestCase[User, AuthFactor]): model_serializer_class = AuthFactorSerializer fixtures = ["school_2", "non_school_teacher"] diff --git a/src/api/serializers/klass_test.py b/src/api/serializers/klass_test.py index 4f63a41c..de268cff 100644 --- a/src/api/serializers/klass_test.py +++ b/src/api/serializers/klass_test.py @@ -14,6 +14,7 @@ from .klass import WriteClassSerializer # pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors class TestWriteClassSerializer(ModelSerializerTestCase[User, Class]): diff --git a/src/api/serializers/school_teacher_invitation_test.py b/src/api/serializers/school_teacher_invitation_test.py index 80a2a0b9..b29e1cf2 100644 --- a/src/api/serializers/school_teacher_invitation_test.py +++ b/src/api/serializers/school_teacher_invitation_test.py @@ -24,6 +24,7 @@ ) # pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors class TestSchoolTeacherInvitationSerializer( @@ -68,7 +69,6 @@ def test_create(self, invitation_make_password: Mock): invitation_make_password.assert_called_once() -# pylint: disable-next=missing-class-docstring class TestRefreshSchoolTeacherInvitationSerializer( ModelSerializerTestCase[User, SchoolTeacherInvitation] ): diff --git a/src/api/serializers/school_test.py b/src/api/serializers/school_test.py index 21df1e39..f2a532bb 100644 --- a/src/api/serializers/school_test.py +++ b/src/api/serializers/school_test.py @@ -9,8 +9,10 @@ from ..views.school import SchoolViewSet from .school import SchoolSerializer +# pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors + -# pylint: disable-next=missing-class-docstring class TestSchoolSerializer(ModelSerializerTestCase[User, School]): model_serializer_class = SchoolSerializer fixtures = ["school_1"] diff --git a/src/api/serializers/teacher_test.py b/src/api/serializers/teacher_test.py index f8e9dfee..74e5c5a8 100644 --- a/src/api/serializers/teacher_test.py +++ b/src/api/serializers/teacher_test.py @@ -22,6 +22,7 @@ ) # pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors class TestCreateTeacherSerializer(ModelSerializerTestCase[User, Teacher]): diff --git a/src/api/serializers/user_test.py b/src/api/serializers/user_test.py index 018e4e69..f5d24267 100644 --- a/src/api/serializers/user_test.py +++ b/src/api/serializers/user_test.py @@ -32,6 +32,7 @@ ) # pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors class TestBaseUserSerializer(ModelSerializerTestCase[User, User]): diff --git a/src/rapid_router/serializers/level_test.py b/src/rapid_router/serializers/level_test.py index c4c814e8..b53f91c2 100644 --- a/src/rapid_router/serializers/level_test.py +++ b/src/rapid_router/serializers/level_test.py @@ -12,6 +12,7 @@ from .level import LockLevelListSerializer, LockLevelSerializer # pylint: disable=missing-class-docstring +# pylint: disable=too-many-ancestors class TestLockLevelSerializer(ModelSerializerTestCase[User, Level]): From 772af74877b7ef40ed49c013bfb63f2e7c69efa7 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 17:01:53 +0000 Subject: [PATCH 4/7] abstract base login view and form --- src/sso/forms.py | 76 ++++-------------------------------------------- src/sso/views.py | 66 ++++------------------------------------- 2 files changed, 11 insertions(+), 131 deletions(-) diff --git a/src/sso/forms.py b/src/sso/forms.py index 6484f727..acfb1c29 100644 --- a/src/sso/forms.py +++ b/src/sso/forms.py @@ -3,77 +3,13 @@ Created on 01/12/2023 at 16:00:24(+00:00). """ +from codeforlife.forms import BaseLoginForm from codeforlife.user.models import User from django import forms -from django.contrib.auth import authenticate -from django.core.exceptions import ValidationError -from django.core.handlers.wsgi import WSGIRequest from django.core.validators import RegexValidator -class BaseLoginForm(forms.Form): - """ - Base login form that all other login forms must inherit. - """ - - user: User - - def __init__(self, request: WSGIRequest, *args, **kwargs): - self.request = request - super().__init__(*args, **kwargs) - - def clean(self): - """Authenticates a user. - - Raises: - ValidationError: If there are form errors. - ValidationError: If the user's credentials were incorrect. - ValidationError: If the user's account is deactivated. - - Returns: - The cleaned form data. - """ - - if self.errors: - raise ValidationError( - "Found form errors. Skipping authentication.", - code="form_errors", - ) - - user = authenticate( - self.request, - **{key: self.cleaned_data[key] for key in self.fields.keys()} - ) - if user is None: - raise ValidationError( - self.get_invalid_login_error_message(), - code="invalid_login", - ) - if not isinstance(user, User): - raise ValidationError( - "Incorrect user class.", - code="incorrect_user_class", - ) - self.user = user - - if not user.is_active: - raise ValidationError( - "User is not active", - code="user_not_active", - ) - - return self.cleaned_data - - def get_invalid_login_error_message(self) -> str: - """Returns the error message if the user failed to login. - - Raises: - NotImplementedError: If message is not set. - """ - raise NotImplementedError() - - -class EmailLoginForm(BaseLoginForm): +class EmailLoginForm(BaseLoginForm[User]): """Log in with an email address.""" email = forms.EmailField() @@ -86,7 +22,7 @@ def get_invalid_login_error_message(self): ) -class OtpLoginForm(BaseLoginForm): +class OtpLoginForm(BaseLoginForm[User]): """Log in with an OTP code.""" otp = forms.CharField( @@ -99,7 +35,7 @@ def get_invalid_login_error_message(self): return "Please enter the correct one-time password." -class OtpBypassTokenLoginForm(BaseLoginForm): +class OtpBypassTokenLoginForm(BaseLoginForm[User]): """Log in with an OTP-bypass token.""" token = forms.CharField(min_length=8, max_length=8) @@ -108,7 +44,7 @@ def get_invalid_login_error_message(self): return "Must be exactly 8 characters. A token can only be used once." -class StudentLoginForm(BaseLoginForm): +class StudentLoginForm(BaseLoginForm[User]): """Log in as a student.""" first_name = forms.CharField() @@ -133,7 +69,7 @@ def get_invalid_login_error_message(self): ) -class StudentAutoLoginForm(BaseLoginForm): +class StudentAutoLoginForm(BaseLoginForm[User]): """Log in with the user's id.""" student_id = forms.IntegerField(min_value=1) diff --git a/src/sso/views.py b/src/sso/views.py index 37015274..afd835a0 100644 --- a/src/sso/views.py +++ b/src/sso/views.py @@ -7,26 +7,20 @@ split these views into multiple files. """ -import json import logging import typing as t -from urllib.parse import quote_plus from codeforlife.mixins import CronMixin from codeforlife.request import HttpRequest +from codeforlife.user.models import User +from codeforlife.views import BaseLoginView from common.models import UserSession # type: ignore -from django.conf import settings -from django.contrib.auth import login -from django.contrib.auth.views import LoginView as _LoginView from django.contrib.sessions.models import Session, SessionManager from django.core import management -from django.http import JsonResponse -from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView from .forms import ( - BaseLoginForm, EmailLoginForm, OtpBypassTokenLoginForm, OtpLoginForm, @@ -36,7 +30,7 @@ # pylint: disable-next=too-many-ancestors -class LoginView(_LoginView): +class LoginView(BaseLoginView[HttpRequest[User], User]): """ Extends Django's login view to allow a user to log in using one of the approved forms. @@ -45,8 +39,6 @@ class LoginView(_LoginView): industry standard security measures that a login view should have. """ - request: HttpRequest - def get_form_class(self): form = self.kwargs["form"] if form == "login-with-email": @@ -62,21 +54,7 @@ def get_form_class(self): raise NameError(f'Unsupported form: "{form}".') - def get_form_kwargs(self): - form_kwargs = super().get_form_kwargs() - form_kwargs["data"] = json.loads(self.request.body) - - return form_kwargs - - def form_valid(self, form: BaseLoginForm): # type: ignore - user = form.user - - # Clear expired sessions. - self.request.session.clear_expired(user.pk) - - # Create session (without data). - login(self.request, user) - + def get_session_metadata(self, user): # TODO: use google analytics user_session: t.Dict[str, t.Any] = {"user": user} if self.get_form_class() in [StudentAutoLoginForm, StudentLoginForm]: @@ -88,17 +66,13 @@ def form_valid(self, form: BaseLoginForm): # type: ignore ) UserSession.objects.create(**user_session) - # Save session (with data). - self.request.session.save() - user_type = "indy" if user.teacher: user_type = "teacher" elif user.student and user.student.class_field: user_type = "student" - # Get session metadata. - session_metadata = { + return { "user_id": user.id, "user_type": user_type, "auth_factors": list( @@ -109,36 +83,6 @@ def form_valid(self, form: BaseLoginForm): # type: ignore "otp_bypass_token_exists": user.otp_bypass_tokens.exists(), } - # Return session metadata in response and a non-HTTP-only cookie. - response = JsonResponse(session_metadata) - response.set_cookie( - key=settings.SESSION_METADATA_COOKIE_NAME, - value=quote_plus( - json.dumps( - session_metadata, - separators=(",", ":"), - indent=None, - ) - ), - max_age=( - None - if settings.SESSION_EXPIRE_AT_BROWSER_CLOSE - else settings.SESSION_COOKIE_AGE - ), - secure=settings.SESSION_COOKIE_SECURE, - samesite=t.cast( - t.Optional[t.Literal["Lax", "Strict", "None", False]], - settings.SESSION_COOKIE_SAMESITE, - ), - domain=settings.SESSION_COOKIE_DOMAIN, - httponly=False, - ) - - return response - - def form_invalid(self, form: BaseLoginForm): # type: ignore - return JsonResponse(form.errors, status=status.HTTP_400_BAD_REQUEST) - class ClearExpiredView(CronMixin, APIView): # type: ignore """Clear all expired sessions.""" From 0950556f6b507d81b1bd6577904b3bf899ed18fc Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 17:04:30 +0000 Subject: [PATCH 5/7] add todo --- src/sso/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sso/views.py b/src/sso/views.py index afd835a0..e3ea8988 100644 --- a/src/sso/views.py +++ b/src/sso/views.py @@ -84,6 +84,7 @@ def get_session_metadata(self, user): } +# TODO: move to python package and make work on AWS class ClearExpiredView(CronMixin, APIView): # type: ignore """Clear all expired sessions.""" From 4dbb5edc42a6c420ece416f7ec4cfae4621b032f Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 17:32:17 +0000 Subject: [PATCH 6/7] new cfl package --- Pipfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 6d4e529e..790e4fac 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "882de7316c5c5054b7ca02d8351704a8b298d398" + "ref": "655604e6c749ace4866e12abd76d97962f017e66" }, "codeforlife-portal": { "hashes": [ @@ -1099,7 +1099,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "882de7316c5c5054b7ca02d8351704a8b298d398" + "ref": "655604e6c749ace4866e12abd76d97962f017e66" }, "codeforlife-portal": { "hashes": [ From 690e0afccd3243c2511c45118293d067bb97d566 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Wed, 13 Nov 2024 12:00:09 +0000 Subject: [PATCH 7/7] new cfl package --- Pipfile | 4 ++-- Pipfile.lock | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Pipfile b/Pipfile index db6df4f3..8fd4a36d 100644 --- a/Pipfile +++ b/Pipfile @@ -23,7 +23,7 @@ name = "pypi" # 5. Run `pipenv install --dev` in your terminal. [packages] -codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "v0.21.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} # 🚫 Don't add [packages] below that are inherited from the CFL package. pyjwt = "==2.6.0" # TODO: upgrade to latest version # TODO: Needed by RR. Remove when RR has moved to new system. @@ -32,7 +32,7 @@ django-sekizai = "==2.0.0" django-classy-tags = "==2.0.0" [dev-packages] -codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} +codeforlife = {ref = "v0.21.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} # codeforlife = {file = "../codeforlife-package-python", editable = true, extras = ["dev"]} # 🚫 Don't add [dev-packages] below that are inherited from the CFL package. diff --git a/Pipfile.lock b/Pipfile.lock index 790e4fac..9baff89f 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "89ce3a71a998d0972e556dd872cae78589a28581de26f03fd1ebf040dcbb05c1" + "sha256": "c1290d1b64cbe7993e90796c558e5d4fc0400bf95f2d258a7332c655bd46b926" }, "pipfile-spec": 6, "requires": { @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "655604e6c749ace4866e12abd76d97962f017e66" + "ref": "8ccf3d86cf3ea7826567dc654120a4f27a1f10b1" }, "codeforlife-portal": { "hashes": [ @@ -1099,7 +1099,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "655604e6c749ace4866e12abd76d97962f017e66" + "ref": "8ccf3d86cf3ea7826567dc654120a4f27a1f10b1" }, "codeforlife-portal": { "hashes": [