From 82b786c256b88b8927a8841d61dcb693ab40cd21 Mon Sep 17 00:00:00 2001
From: Harsh Gupta <42064744+Harshg999@users.noreply.github.com>
Date: Wed, 5 Feb 2025 22:09:23 +0530
Subject: [PATCH] [api] Refactor /mkdir API with better validation checks

---
 apps/filebrowser/src/filebrowser/api.py      | 26 ++++++-
 apps/filebrowser/src/filebrowser/api_test.py | 79 +++++++++++++++++++-
 2 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/apps/filebrowser/src/filebrowser/api.py b/apps/filebrowser/src/filebrowser/api.py
index 98d64ece1dc..bf33d64efed 100644
--- a/apps/filebrowser/src/filebrowser/api.py
+++ b/apps/filebrowser/src/filebrowser/api.py
@@ -536,14 +536,34 @@ def upload_file(request):
 
 @api_error_handler
 def mkdir(request):
+  """
+  Create a new directory at the specified path with the given name.
+
+  Args:
+    request (HttpRequest): The HTTP request object containing the data.
+
+  Returns:
+    A HttpResponse with a status code and message indicating the success or failure of the directory creation.
+  """
   # TODO: Check if this needs to be a PUT request
   path = request.POST.get('path')
   name = request.POST.get('name')
 
-  if name and (posixpath.sep in name or "#" in name):
-    return HttpResponse(f"Error creating {name} directory. Slashes or hashes are not allowed in directory name.", status=400)
+  # Check if source and destination paths are provided
+  if not path or not name:
+    return HttpResponse("Missing required parameters: path and name are required.", status=400)
+
+  # Validate the 'name' parameter for invalid characters
+  if posixpath.sep in name or "#" in name:
+    return HttpResponse(f"Slashes or hashes are not allowed in directory name. Please choose a different name.", status=400)
+
+  dir_path = request.fs.join(path, name)
+
+  # Check if the directory already exists
+  if request.fs.isdir(dir_path):
+    return HttpResponse(f"Error creating {name} directory: Directory already exists.", status=409)
 
-  request.fs.mkdir(request.fs.join(path, name))
+  request.fs.mkdir(dir_path)
   return HttpResponse(status=201)
 
 
diff --git a/apps/filebrowser/src/filebrowser/api_test.py b/apps/filebrowser/src/filebrowser/api_test.py
index 297d55605e2..e868a3bd87d 100644
--- a/apps/filebrowser/src/filebrowser/api_test.py
+++ b/apps/filebrowser/src/filebrowser/api_test.py
@@ -20,7 +20,7 @@
 
 from django.core.files.uploadedfile import SimpleUploadedFile
 
-from filebrowser.api import get_all_filesystems, rename, upload_file
+from filebrowser.api import get_all_filesystems, mkdir, rename, upload_file
 from filebrowser.conf import (
   MAX_FILE_SIZE_UPLOAD_LIMIT,
   RESTRICT_FILE_EXTENSIONS,
@@ -330,6 +330,83 @@ def test_file_upload_failure(self):
           reset()
 
 
+class TestMkdirAPI:
+  def test_mkdir_success(self):
+    request = Mock(
+      method='POST',
+      POST={'path': 's3a://test-bucket/test-user/', 'name': 'new_dir'},
+      fs=Mock(
+        mkdir=Mock(),
+        isdir=Mock(return_value=False),
+        join=Mock(return_value='s3a://test-bucket/test-user/new_dir'),
+      ),
+    )
+    response = mkdir(request)
+
+    assert response.status_code == 201
+    request.fs.mkdir.assert_called_once_with('s3a://test-bucket/test-user/new_dir')
+
+  def test_mkdir_directory_exists(self):
+    request = Mock(
+      method='POST',
+      POST={'path': 's3a://test-bucket/test-user/', 'name': 'new_dir'},
+      fs=Mock(
+        mkdir=Mock(),
+        isdir=Mock(return_value=True),
+        join=Mock(return_value='s3a://test-bucket/test-user/new_dir'),
+      ),
+    )
+    response = mkdir(request)
+
+    assert response.status_code == 409
+    assert response.content.decode('utf-8') == 'Error creating new_dir directory: Directory already exists.'
+
+  def test_mkdir_invalid_name(self):
+    request = Mock(
+      method='POST',
+      POST={'path': 's3a://test-bucket/test-user/', 'name': 'new#dir'},
+      fs=Mock(
+        mkdir=Mock(),
+        isdir=Mock(return_value=False),
+        join=Mock(return_value='s3a://test-bucket/test-user/new#dir'),
+      ),
+    )
+    response = mkdir(request)
+
+    assert response.status_code == 400
+    assert response.content.decode('utf-8') == 'Slashes or hashes are not allowed in directory name. Please choose a different name.'
+
+  def test_mkdir_no_path(self):
+    request = Mock(
+      method='POST',
+      POST={'name': 'new_dir'},
+      fs=Mock(
+        mkdir=Mock(),
+        isdir=Mock(return_value=False),
+        join=Mock(return_value='s3a://test-bucket/test-user/new_dir'),
+      ),
+    )
+    response = mkdir(request)
+
+    assert response.status_code == 400
+    assert response.content.decode('utf-8') == 'Missing required parameters: path and name are required.'
+
+  def test_mkdir_no_name(self):
+    request = Mock(
+      method='POST',
+      POST={'path': 's3a://test-bucket/test-user/'},
+      fs=Mock(
+        mkdir=Mock(),
+        isdir=Mock(return_value=False),
+        join=Mock(return_value='s3a://test-bucket/test-user/new_dir'),
+      ),
+    )
+    response = mkdir(request)
+
+    assert response.status_code == 400
+    assert response.content.decode('utf-8') == 'Missing required parameters: path and name are required.'
+
+
 class TestRenameAPI:
   def test_rename_success(self):
     request = Mock(