-
Notifications
You must be signed in to change notification settings - Fork 2
BENCH-418 & BENCH-422: Authentication and permissions #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
044d1f8
ceeafc4
33dea30
af7b954
d9dac78
eaf3fd5
f2e6b3f
3e35558
07b999a
945cbf0
ec0d75c
83a6b93
57dbcb6
02b0382
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
from rest_framework.permissions import IsAdminUser, BasePermission | ||
|
||
|
||
class IsStaffUser(IsAdminUser): | ||
""" | ||
Allows access to Staff and Managers only. | ||
""" | ||
|
||
pass | ||
|
||
|
||
class IsManagerUser(BasePermission): | ||
""" | ||
Allows access to Manager's only. | ||
""" | ||
|
||
def has_permission(self, request, view): | ||
return bool(request.user and request.user.is_staff) | ||
|
||
|
||
class IsOwner(BasePermission): | ||
""" | ||
Custom permission to only allow owners of an object to edit it. | ||
""" | ||
|
||
def has_permission(self, request, view): | ||
return bool(request.user and request.user.is_authenticated) | ||
|
||
def has_object_permission(self, request, view, obj): | ||
return bool(obj.owner == request.user) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,17 @@ | ||
import re | ||
from abc import ABC | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need this? |
||
from typing import OrderedDict | ||
|
||
from django.contrib.auth.models import User | ||
from django.contrib.auth.password_validation import validate_password | ||
from django.core.validators import ( | ||
MaxValueValidator, | ||
MinValueValidator, | ||
RegexValidator, | ||
) | ||
from rest_framework import serializers, status | ||
from rest_framework.validators import UniqueValidator | ||
from rest_framework_simplejwt.serializers import TokenObtainPairSerializer | ||
|
||
from answerking_app.models.models import ( | ||
Category, | ||
|
@@ -246,9 +252,10 @@ class OrderSerializer(serializers.ModelSerializer): | |
lineItems = LineItemSerializer( | ||
source="lineitem_set", many=True, required=False | ||
) | ||
owner = serializers.ReadOnlyField(source="owner.username") | ||
|
||
def create(self, validated_data: dict) -> Order: | ||
order: Order = Order.objects.create() | ||
order: Order = Order.objects.create(owner=validated_data["owner"]) | ||
if "lineitem_set" in validated_data: | ||
line_items_data = validated_data["lineitem_set"] | ||
self.create_order_line_items( | ||
|
@@ -298,17 +305,95 @@ def validate_lineItems(self, line_items_data): | |
|
||
class Meta: | ||
model = Order | ||
fields = ( | ||
fields = [ | ||
"id", | ||
"createdOn", | ||
"lastUpdated", | ||
"orderStatus", | ||
"orderTotal", | ||
"lineItems", | ||
) | ||
"owner", | ||
] | ||
depth = 3 | ||
|
||
|
||
class ManagerAuthSerializer(serializers.ModelSerializer): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are going with the permission as "IsStaffUser" we may want to change this to StaffAuthSerializer. Similar with a few other naming conventions further down also. |
||
username = serializers.CharField( | ||
required=True, | ||
validators=[UniqueValidator(queryset=User.objects.all())], | ||
max_length=50, | ||
min_length=4, | ||
) | ||
email = serializers.EmailField( | ||
required=True, | ||
validators=[UniqueValidator(queryset=User.objects.all())], | ||
) | ||
password = serializers.CharField( | ||
write_only=True, required=True, validators=[validate_password] | ||
) | ||
password2 = serializers.CharField( | ||
write_only=True, | ||
required=True, | ||
) | ||
|
||
class Meta: | ||
model = User | ||
fields = ( | ||
"username", | ||
"password", | ||
"password2", | ||
Comment on lines
+343
to
+344
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1 single password should be sent, password confirmation should be done by the front end |
||
"email", | ||
"first_name", | ||
"last_name", | ||
) | ||
|
||
def validate(self, attrs): | ||
if attrs["password"] != attrs["password2"]: | ||
raise ProblemDetails( | ||
status=status.HTTP_400_BAD_REQUEST, | ||
detail="The passwords supplied do not match", | ||
PietroConvalleAD marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
return attrs | ||
|
||
def validate_first_name(self, value: str) -> str: | ||
return compress_white_spaces(value) | ||
|
||
def validate_last_name(self, value: str) -> str: | ||
return compress_white_spaces(value) | ||
|
||
def create(self, validated_data): | ||
user = User.objects.create( | ||
username=validated_data["username"], | ||
email=validated_data["email"], | ||
first_name=validated_data["first_name"], | ||
last_name=validated_data["last_name"], | ||
is_staff=True, | ||
is_superuser=False, | ||
) | ||
|
||
user.set_password(validated_data["password"]) | ||
user.save() | ||
return user | ||
|
||
|
||
class LoginSerializer(TokenObtainPairSerializer): | ||
@classmethod | ||
def get_token(cls, user): | ||
token = super().get_token(user) | ||
token["username"] = user.username | ||
token["email"] = user.email | ||
return token | ||
|
||
def validate(self, attrs): | ||
attrs = super().validate(attrs) | ||
return { | ||
"username": self.user.username, | ||
"email": self.user.email, | ||
"first_name": self.user.first_name, | ||
**attrs, | ||
} | ||
|
||
|
||
class ErrorDetailSerializer(serializers.Serializer): | ||
name = serializers.CharField() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
from functools import partial | ||
|
||
from django.urls import path | ||
from rest_framework_simplejwt.views import TokenRefreshView, TokenBlacklistView | ||
|
||
from answerking_app.views import auth_views | ||
|
||
urlpatterns: list[partial] = [ | ||
path( | ||
"register/manager", | ||
auth_views.RegisterManagerView.as_view(), | ||
name="register_manager", | ||
), | ||
path("login", auth_views.LoginView.as_view(), name="token_obtain_pair"), | ||
path("login/refresh", TokenRefreshView.as_view(), name="token_refresh"), | ||
path("logout", TokenBlacklistView.as_view(), name="token_blacklist"), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,7 +118,7 @@ | |
problem_detail_example: ProblemDetails = { | ||
"errors": {"name": "The name field is required."}, | ||
"type": "https://testserver/problems/error/", | ||
"title": "One or more validation errors occurred.", | ||
"title": "One or more permissions errors occurred.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably would change this to "One or more validation or permissions errors occurred." |
||
"status": 400, | ||
"traceID": "00-f40e09a437a87f4ebcd2f39b128bb8f3-4b2ad798ac046140-00", | ||
"detail": "string", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
from rest_framework import generics | ||
from rest_framework.permissions import AllowAny | ||
from rest_framework_simplejwt.views import TokenObtainPairView | ||
|
||
from answerking_app.models.serializers import ( | ||
ManagerAuthSerializer, | ||
LoginSerializer, | ||
) | ||
|
||
|
||
class RegisterManagerView( | ||
generics.CreateAPIView, | ||
): | ||
permission_classes = (AllowAny,) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be "IsStaffUser" rather than AllowAny? |
||
serializer_class = ManagerAuthSerializer | ||
|
||
|
||
class LoginView( | ||
TokenObtainPairView, | ||
): | ||
serializer_class = LoginSerializer |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the same as IsStaffUser so can get rid of this