Skip to content

Commit

Permalink
fix enable/disable otp
Browse files Browse the repository at this point in the history
  • Loading branch information
SKairinos committed Feb 16, 2024
1 parent 2c6df75 commit 3653d7b
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 79 deletions.
4 changes: 2 additions & 2 deletions backend/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ name = "pypi"
# Before adding a new package, check it's not listed under [packages] at
# https://github.com/ocadotechnology/codeforlife-package-python/blob/{ref}/Pipfile
# Replace "{ref}" in the above URL with the ref set below.
codeforlife = {ref = "v0.13.6", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
codeforlife = {ref = "fix_otp", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
# TODO: check if we need the below packages
whitenoise = "==6.5.0"
django-pipeline = "==2.0.8"
Expand All @@ -34,7 +34,7 @@ google-cloud-container = "==2.3.0"
# Before adding a new package, check it's not listed under [dev-packages] at
# https://github.com/ocadotechnology/codeforlife-package-python/blob/{ref}/Pipfile
# Replace "{ref}" in the above URL with the ref set below.
codeforlife = {ref = "v0.13.6", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]}
codeforlife = {ref = "fix_otp", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]}
# TODO: check if we need the below packages
django-selenium-clean = "==0.3.3"
django-test-migrations = "==1.2.0"
Expand Down
14 changes: 7 additions & 7 deletions backend/Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 20 additions & 2 deletions backend/api/fixtures/school_2.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,16 @@
"pk": 25,
"fields": {
"user": 25,
"is_verified": true
"is_verified": true,
"otp_secret": "KI6FA34LPRQU265KQBFYS2MTDYHE2EIG"
}
},
{
"model": "user.authfactor",
"pk": 1,
"fields": {
"user": 25,
"type": "otp"
}
},
{
Expand Down Expand Up @@ -61,7 +70,16 @@
"pk": 26,
"fields": {
"user": 26,
"is_verified": true
"is_verified": true,
"otp_secret": "KI6FA34LPRQU265KQBFYS2MTDYHE2EIG"
}
},
{
"model": "user.authfactor",
"pk": 2,
"fields": {
"user": 26,
"type": "otp"
}
},
{
Expand Down
36 changes: 33 additions & 3 deletions backend/api/serializers/auth_factor.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
Created on 23/01/2024 at 11:05:37(+00:00).
"""

from codeforlife.request import Request
import pyotp
from codeforlife.serializers import ModelSerializer
from codeforlife.user.models import AuthFactor
from rest_framework import serializers


# pylint: disable-next=missing-class-docstring
Expand All @@ -20,7 +21,36 @@ class Meta:
"id": {"read_only": True},
}

# pylint: disable-next=missing-function-docstring
def validate_type(self, value: str):
if AuthFactor.objects.filter(
user=self.request_user, type=value
).exists():
raise serializers.ValidationError(
"You already have this authentication factor enabled.",
code="already_exists",
)

return value

def create(self, validated_data):
request: Request = self.context["request"]
validated_data["user"] = request.user
user = self.request_user
if not user.userprofile.otp_secret:
user.userprofile.otp_secret = pyotp.random_base32()
user.userprofile.save()

validated_data["user"] = user
return super().create(validated_data)

def to_representation(self, instance):
representation = super().to_representation(instance)

if (
self.view.action == "create"
and instance.type == AuthFactor.Type.OTP
):
representation[
"totp_provisioning_uri"
] = self.request_user.totp_provisioning_uri

return representation
92 changes: 92 additions & 0 deletions backend/api/tests/serializers/test_auth_factor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
"""
© Ocado Group
Created on 15/02/2024 at 15:44:25(+00:00).
"""

from unittest.mock import patch

import pyotp
from codeforlife.tests import ModelSerializerTestCase
from codeforlife.user.models import AuthFactor, TeacherUser

from ...serializers import AuthFactorSerializer
from ...views import AuthFactorViewSet


# pylint: disable-next=missing-class-docstring
class TestAuthFactorSerializer(ModelSerializerTestCase[AuthFactor]):
model_serializer_class = AuthFactorSerializer
fixtures = ["school_2", "non_school_teacher"]

def setUp(self):
self.multi_auth_factor_teacher_user = TeacherUser.objects.get(
email="[email protected]"
)
assert self.multi_auth_factor_teacher_user.auth_factors.exists()

self.uni_auth_factor_teacher_user = TeacherUser.objects.get(
email="[email protected]"
)
assert not self.uni_auth_factor_teacher_user.auth_factors.exists()

# test: validate

def test_validate_type__already_exists(self):
"""Validate type is not doing this."""
auth_factor = self.multi_auth_factor_teacher_user.auth_factors.first()
assert auth_factor

self.assert_validate_field(
"type",
auth_factor.type,
error_code="already_exists",
context={
"request": self.request_factory.post(
user=self.multi_auth_factor_teacher_user
)
},
)

# test: create

def test_create(self):
"""Enabling OTP will ensure the user's OTP-secret is set."""
assert not self.uni_auth_factor_teacher_user.otp_secret

otp_secret = pyotp.random_base32()
with patch.object(pyotp, "random_base32", return_value=otp_secret):
self.assert_create(
validated_data={"type": AuthFactor.Type.OTP},
context={
"request": self.request_factory.post(
user=self.uni_auth_factor_teacher_user
)
},
new_data={
"user": {
"id": self.uni_auth_factor_teacher_user.id,
"userprofile": {"otp_secret": otp_secret},
},
},
)

# test: to representation

def test_to_representation(self):
"""User's TOTP provisioning URI is returned when enabling OTP."""
assert self.multi_auth_factor_teacher_user.otp_secret

self.assert_to_representation(
instance=AuthFactor(type=AuthFactor.Type.OTP),
new_data={
"totp_provisioning_uri": (
self.multi_auth_factor_teacher_user.totp_provisioning_uri
),
},
context={
"view": AuthFactorViewSet(action="create"),
"request": self.request_factory.post(
user=self.multi_auth_factor_teacher_user
),
},
)
Loading

0 comments on commit 3653d7b

Please sign in to comment.