Skip to content

Commit

Permalink
fix: return otp secret
Browse files Browse the repository at this point in the history
  • Loading branch information
SKairinos committed Jan 23, 2025
1 parent 7d04e3e commit 3c99909
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/api/permissions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
Created on 24/04/2024 at 11:57:02(+01:00).
"""

from .has_auth_factor import HasAuthFactor
from .is_invited_school_teacher import IsInvitedSchoolTeacher
27 changes: 27 additions & 0 deletions src/api/permissions/has_auth_factor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"""
© Ocado Group
Created on 20/01/2025 at 18:52:16(+00:00).
"""

from codeforlife.permissions import IsAuthenticated
from codeforlife.user.models import AuthFactor, User


class HasAuthFactor(IsAuthenticated):
"""Request's user must have a auth factor enabled."""

def __init__(self, t: AuthFactor.Type):
super().__init__()

self.t = t

def __eq__(self, other):
return isinstance(other, self.__class__) and self.t == other.t

def has_permission(self, request, view):
user = request.user
return (
super().has_permission(request, view)
and isinstance(user, User)
and user.auth_factors.filter(type=self.t).exists()
)
16 changes: 12 additions & 4 deletions src/api/views/auth_factor.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
"""

import pyotp
from codeforlife.permissions import AllowNone
from codeforlife.permissions import NOT, AllowNone
from codeforlife.request import Request
from codeforlife.response import Response
from codeforlife.user.models import AuthFactor, User
from codeforlife.user.permissions import IsTeacher
from codeforlife.views import ModelViewSet, action

from ..filters import AuthFactorFilterSet
from ..permissions import HasAuthFactor
from ..serializers import AuthFactorSerializer


Expand Down Expand Up @@ -43,16 +44,23 @@ def get_queryset(self):
def get_permissions(self):
if self.action in ["retrieve", "bulk"]:
return [AllowNone()]
if self.action == "get_otp_secret":
return [IsTeacher(), NOT(HasAuthFactor(AuthFactor.Type.OTP))]

return [IsTeacher()]

@action(detail=False, methods=["get"])
def generate_otp_provisioning_uri(self, request: Request[User]):
"""Generate a time-based one-time-password provisioning URI."""
def get_otp_secret(self, request: Request[User]):
"""Get the secret for the user's one-time-password."""
# TODO: make otp_secret non-nullable and delete code block
user = request.auth_user
if not user.userprofile.otp_secret:
user.userprofile.otp_secret = pyotp.random_base32()
user.userprofile.save(update_fields=["otp_secret"])

return Response(user.totp_provisioning_uri, content_type="text/plain")
return Response(
{
"secret": user.totp.secret,
"provisioning_uri": user.totp_provisioning_uri,
}
)
42 changes: 18 additions & 24 deletions src/api/views/auth_factor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
Created on 23/01/2024 at 11:22:16(+00:00).
"""

from unittest.mock import patch

import pyotp
from codeforlife.permissions import AllowNone
from codeforlife.permissions import NOT, AllowNone
from codeforlife.tests import ModelViewSetTestCase
from codeforlife.user.models import (
AdminSchoolTeacherUser,
Expand All @@ -18,8 +16,8 @@
)
from codeforlife.user.permissions import IsTeacher
from django.db.models import Count
from pyotp import TOTP

from ..permissions import HasAuthFactor
from .auth_factor import AuthFactorViewSet

# pylint: disable=missing-class-docstring
Expand Down Expand Up @@ -103,12 +101,12 @@ def test_get_queryset__destroy__non_admin(self):
request=self.client.request_factory.get(user=user),
)

def test_get_queryset__generate_otp_provisioning_uri(self):
"""Can only generate an OTP provisioning URI yourself."""
def test_get_queryset__get_otp_secret(self):
"""Can only get your own OTP secret."""
user = self.mfa_non_admin_school_teacher_user

self.assert_get_queryset(
action="generate_otp_provisioning_uri",
action="get_otp_secret",
values=list(user.auth_factors.all()),
request=self.client.request_factory.get(user=user),
)
Expand All @@ -135,10 +133,11 @@ def test_get_permissions__destroy(self):
"""Only a teacher-user can disable an auth factor."""
self.assert_get_permissions([IsTeacher()], action="destroy")

def test_get_permissions__generate_otp_provisioning_uri(self):
"""Only a teacher-user can generate a OTP provisioning URI."""
def test_get_permissions__get_otp_secret(self):
"""Only a teacher-user can get an OTP secret."""
self.assert_get_permissions(
[IsTeacher()], action="generate_otp_provisioning_uri"
[IsTeacher(), NOT(HasAuthFactor(AuthFactor.Type.OTP))],
action="get_otp_secret",
)

# test: actions
Expand Down Expand Up @@ -208,7 +207,7 @@ def test_destroy(self):
self.client.login_as(user)
self.client.destroy(auth_factor)

def test_generate_otp_provisioning_uri(self):
def test_get_otp_secret(self):
"""Can successfully generate a OTP provisioning URI."""
user = TeacherUser.objects.exclude(
auth_factors__type__in=[AuthFactor.Type.OTP]
Expand All @@ -218,17 +217,12 @@ def test_generate_otp_provisioning_uri(self):
# TODO: normalize password to "password"
self.client.login_as(user, password="abc123")

with patch.object(
TOTP, "provisioning_uri", return_value=user.totp_provisioning_uri
) as provisioning_uri:
response = self.client.get(
self.reverse_action("generate_otp_provisioning_uri")
)

provisioning_uri.assert_called_once_with(
name=user.email,
issuer_name="Code for Life",
)
response = self.client.get(self.reverse_action("get_otp_secret"))

assert response.data == provisioning_uri.return_value
assert response.content_type == "text/plain"
self.assertDictEqual(
response.json(),
{
"secret": user.totp.secret,
"provisioning_uri": user.totp_provisioning_uri,
},
)

0 comments on commit 3c99909

Please sign in to comment.