Code Review Request for Custom Email-based Authentication in Django REST Framework
Hi everyone,
I am currently learning Django REST Framework (DRF). After working with the built-in token authentication, I decided to experiment by implementing a custom authentication flow using only email (no passwords, just for practice).
I am aware that this is not a secure approach for production environments, but my goal was to understand how custom authentication classes and workflows operate under the hood in DRF.
I would appreciate it if you could review my implementation. I am looking for feedback on:
Code quality and best practices - Are there more idiomatic ways to handle this?
Logical structure - Is the authentication process flow sound?
Security improvements - Even for a practice project, I want to learn the "right" way to handle things.
Alternative approaches -How would you handle this for a specific use case like this?
Here is my current implementation:
models.py
class UserManager(BaseUserManager):
def create_user(self, email, password, **extra_field):
if not email:
raise ValueError("Email field must be set")
email = self.normalize_email(email)
user = self.model(email=email, **extra_field)
user.set_password(password)
user.save()
return user
def create_superuser(self, email, password, **extra_field):
user = self.create_user(email=email, password=password, **extra_field)
user.is_superuser = True
user.is_staff = True
user.save()
return user
class User(AbstractBaseUser, PermissionsMixin):
created_at = models.DateTimeField(auto_now_add=True)
email = models.EmailField(unique=True, db_index=True)
is_staff = models.BooleanField(default=False)
USERNAME_FIELD = "email"
REQUIRED_FIELDS = []
objects = UserManager()
serializers.py
class SignupSerializer(serializers.ModelSerializer):
class Meta:
model = User
fields = ("id", "email", "password")
extra_kwargs = {
"password": {"write_only": True, "style": {"input_type": "password"}},
"id": {"read_only": True}
}
def create(self, validated_data):
return User.objects.create_user(**validated_data)
class UserAuthTokenSerializer(AuthTokenSerializer):
username = None
email = serializers.EmailField()
password = serializers.CharField(
min_length=8,
style={"input_type": "password"},
write_only=True
)
def validate(self, attrs):
email = attrs.get("email")
password = attrs.get("password")
if email and password:
user = authenticate(
request=self.context.get("request"),
email=email,
password=password
)
if not user:
raise serializers.ValidationError(
"Unable to log in with provided credentials"
)
else:
raise serializers.ValidationError(
"Must include username and password."
)
attrs["user"] = user
return attrs
views.py
class SignupAPIView(generics.CreateAPIView):
serializer_class = SignupSerializer
permission_classes = (permissions.AllowAny,)
queryset = User.objects.all()
class TokenAPIView(ObtainAuthToken):
serializer_class = UserAuthTokenSerializer
renderer_classes = api_settings.DEFAULT_RENDERER_CLASSES
urls.py
app_name = "users"
urlpatterns = [
path("signup/", SignupAPIView.as_view(), name="signup"),
path("token/", TokenAPIView.as_view(), name="token")
]
I have provided the core logic above. Any tips, criticism, or suggestions for refactoring would be greatly appreciated!
Thanks in advance for your time and help.
I will note in passing that SE has a whole site for this sort of thing: https://CodeReview.stackexchange.com
Mostly it looks fine, modulo some minor nits. LGTM, ship it! 🚢
plural
def create_user( ... , **extra_field):
That parameter, being a container, kind of feels like
it contains zero-or-more optional fields.
Which suggests a name of extra_fields.
high- vs. low- level methods
Hmmm, now I am reviewing the Django project's .create_user() docs and yeah, it turns out they pluralize the parameter.
user = self.model( ... )
user.set_password(password)
...
user = self.create_user( ... )
I don't understand why the first case is a Good Thing.
It's calling into the low-level .model() method
which is called by Django's .create_user().
In the second case, I don't understand why we're not taking advantage of Django's .create_superuser().
There might be good reasons for these, but the Review Context, docstrings, and comments have not explained them. (Or is this perhaps related to the unusual "no passwords!" requirement?)
diagnostic error message
This is nice enough:
raise serializers.ValidationError(
"Must include username and password."
Consider offering a clue about whether username or password is missing, as an aid to the person trying to fix the error.