feat: improved user creation validation

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Add compute task category unknown value
+- Improved validation at user creation.
 
 ### Fixed
 

backend/users/tests/test_user.py

@@ -44,6 +44,15 @@ def test_user_create_unauthorized(self):
 
         assert response.status_code == status.HTTP_401_UNAUTHORIZED
 
+    @pytest.mark.django_db
+    def test_user_create_duplicate(self):
+        data = {"username": "substra", "password": "pas$w0rdtestofdrea6S43"}
+
+        response = AuthenticatedClient(role=UserChannel.Role.ADMIN, channel=self.channel).post(self.url, data=data)
+
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert response.data.get("message") == "Username already exists"
+
     @pytest.mark.django_db
     def test_user_create_short_password(self):
         data = {"username": "toto", "password": "password"}
@@ -52,6 +61,14 @@ def test_user_create_short_password(self):
 
         assert response.status_code == status.HTTP_400_BAD_REQUEST
 
+    @pytest.mark.django_db
+    def test_user_create_empty_password(self):
+        data = {"username": "toto", "password": ""}
+
+        response = AuthenticatedClient(role=UserChannel.Role.ADMIN, channel=self.channel).post(self.url, data=data)
+
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+
     @pytest.mark.django_db
     def test_user_create_role_admin(self):
         data = {"username": "toto", "password": "pas$w0rdtestofdrea6S43", "role": "ADMIN"}
@@ -114,6 +131,18 @@ def test_update_password_unauthorized(self):
 
         assert response.status_code == status.HTTP_403_FORBIDDEN
 
+    @pytest.mark.django_db
+    def test_update_password_empty(self):
+        data = {"username": "toto", "password": "pas$w0rdtestofdrea6S43"}
+        response = AuthenticatedClient(role=UserChannel.Role.ADMIN, channel=self.channel).post(self.url, data=data)
+        assert response.status_code == status.HTTP_201_CREATED
+
+        url = reverse("user:users-password", args=["toto"])
+        data = {"password": ""}
+        response = AuthenticatedClient(role=UserChannel.Role.ADMIN, channel=self.channel).put(url, data=data)
+
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
     @pytest.mark.django_db
     def test_request_reset_token(self):
         url = reverse("user:users-reset-password", args=["substra"])

backend/users/views/user.py

@@ -1,4 +1,5 @@
 import datetime
+from urllib.parse import unquote
 
 import jwt
 from django.conf import settings
@@ -21,6 +22,7 @@
 from rest_framework.mixins import CreateModelMixin
 from rest_framework.viewsets import GenericViewSet
 
+from api.errors import BadRequestError
 from api.views.filters_utils import MatchFilter
 from api.views.utils import ApiResponse
 from api.views.utils import get_channel_name
@@ -40,12 +42,20 @@ def _validate_channel(name):
 
 
 def _validate_password(password, user):
+    if not password:
+        raise ValidationError("Missing password")
     try:
         validate_password(password, user)
     except djangoValidationError as err:
         raise ValidationError(err.error_list)
 
 
+def _validate_username(username):
+    user_model = get_user_model()
+    if user_model.objects.filter(username=username).exists():
+        raise BadRequestError("Username already exists")
+
+
 def _validate_role(role):
     try:
         role = UserChannel.Role[role]
@@ -124,6 +134,7 @@ def create(self, request, *args, **kwargs):
         role = request.data.get("role")
 
         _validate_channel(channel)
+        _validate_username(username)
         _validate_password(password, self.user_model(username=username))
 
         channel_data = {"channel_name": channel}
@@ -173,7 +184,7 @@ def set_password(self, request, *args, **kwargs):
         token = request.data.get("token")
         new_password = request.data.get("password")
 
-        username = kwargs.get("username")
+        username = unquote(kwargs.get("username"))
         instance = self.user_model.objects.get(username=username)
 
         secret = _xor_secrets(instance.password, force_str(settings.SECRET_KEY))
@@ -193,7 +204,7 @@ def verify_token(self, request, *args, **kwargs):
         """Return 200 if reset token is valid 401 otherwise. Accepts unauthenticated request"""
         token = request.query_params.get("token", None)
 
-        username = kwargs.get("username")
+        username = unquote(kwargs.get("username"))
         instance = self.user_model.objects.get(username=username)
 
         secret = _xor_secrets(instance.password, force_str(settings.SECRET_KEY))