diff --git a/apps/api/.env.example b/apps/api/.env.example index f158e3d7c..4c84bd683 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -32,6 +32,9 @@ AWS_S3_ENDPOINT_URL="http://localhost:9000" AWS_S3_BUCKET_NAME="uploads" # Maximum file upload limit FILE_SIZE_LIMIT=5242880 +# Signed URL expiration time in seconds (default: 3600 = 1 hour) +# Set to 30 for 30 seconds, 300 for 5 minutes, etc. +SIGNED_URL_EXPIRATION=3600 # Settings related to Docker DOCKERIZED=1 # deprecated diff --git a/apps/api/plane/settings/storage.py b/apps/api/plane/settings/storage.py index 0a0720086..7c048f679 100644 --- a/apps/api/plane/settings/storage.py +++ b/apps/api/plane/settings/storage.py @@ -29,6 +29,8 @@ class S3Storage(S3Boto3Storage): self.aws_region = os.environ.get("AWS_REGION") # Use the AWS_S3_ENDPOINT_URL environment variable for the endpoint URL self.aws_s3_endpoint_url = os.environ.get("AWS_S3_ENDPOINT_URL") or os.environ.get("MINIO_ENDPOINT_URL") + # Use the SIGNED_URL_EXPIRATION environment variable for the expiration time (default: 3600 seconds) + self.signed_url_expiration = int(os.environ.get("SIGNED_URL_EXPIRATION", "3600")) if os.environ.get("USE_MINIO") == "1": # Determine protocol based on environment variable @@ -56,8 +58,10 @@ class S3Storage(S3Boto3Storage): config=boto3.session.Config(signature_version="s3v4"), ) - def generate_presigned_post(self, object_name, file_type, file_size, expiration=3600): + def generate_presigned_post(self, object_name, file_type, file_size, expiration=None): """Generate a presigned URL to upload an S3 object""" + if expiration is None: + expiration = self.signed_url_expiration fields = {"Content-Type": file_type} conditions = [ @@ -104,13 +108,15 @@ class S3Storage(S3Boto3Storage): def generate_presigned_url( self, object_name, - expiration=3600, + expiration=None, http_method="GET", disposition="inline", filename=None, ): - content_disposition = self._get_content_disposition(disposition, filename) """Generate a presigned URL to share an S3 object""" + if expiration is None: + expiration = self.signed_url_expiration + content_disposition = self._get_content_disposition(disposition, filename) try: response = self.s3_client.generate_presigned_url( "get_object", diff --git a/apps/api/plane/tests/unit/settings/__init__.py b/apps/api/plane/tests/unit/settings/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/apps/api/plane/tests/unit/settings/test_storage.py b/apps/api/plane/tests/unit/settings/test_storage.py new file mode 100644 index 000000000..fe8cf43f8 --- /dev/null +++ b/apps/api/plane/tests/unit/settings/test_storage.py @@ -0,0 +1,202 @@ +import os +from unittest.mock import Mock, patch +import pytest +from plane.settings.storage import S3Storage + + +@pytest.mark.unit +class TestS3StorageSignedURLExpiration: + """Test the configurable signed URL expiration in S3Storage""" + + @patch.dict(os.environ, {}, clear=True) + @patch("plane.settings.storage.boto3") + def test_default_expiration_without_env_variable(self, mock_boto3): + """Test that default expiration is 3600 seconds when env variable is not set""" + # Mock the boto3 client + mock_boto3.client.return_value = Mock() + + # Create S3Storage instance without SIGNED_URL_EXPIRATION env variable + storage = S3Storage() + + # Assert default expiration is 3600 + assert storage.signed_url_expiration == 3600 + + @patch.dict(os.environ, {"SIGNED_URL_EXPIRATION": "30"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_custom_expiration_with_env_variable(self, mock_boto3): + """Test that expiration is read from SIGNED_URL_EXPIRATION env variable""" + # Mock the boto3 client + mock_boto3.client.return_value = Mock() + + # Create S3Storage instance with SIGNED_URL_EXPIRATION=30 + storage = S3Storage() + + # Assert expiration is 30 + assert storage.signed_url_expiration == 30 + + @patch.dict(os.environ, {"SIGNED_URL_EXPIRATION": "300"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_custom_expiration_multiple_values(self, mock_boto3): + """Test that expiration works with different custom values""" + # Mock the boto3 client + mock_boto3.client.return_value = Mock() + + # Create S3Storage instance with SIGNED_URL_EXPIRATION=300 + storage = S3Storage() + + # Assert expiration is 300 + assert storage.signed_url_expiration == 300 + + @patch.dict( + os.environ, + { + "AWS_ACCESS_KEY_ID": "test-key", + "AWS_SECRET_ACCESS_KEY": "test-secret", + "AWS_S3_BUCKET_NAME": "test-bucket", + "AWS_REGION": "us-east-1", + }, + clear=True, + ) + @patch("plane.settings.storage.boto3") + def test_generate_presigned_post_uses_default_expiration(self, mock_boto3): + """Test that generate_presigned_post uses the configured default expiration""" + # Mock the boto3 client and its response + mock_s3_client = Mock() + mock_s3_client.generate_presigned_post.return_value = { + "url": "https://test-url.com", + "fields": {}, + } + mock_boto3.client.return_value = mock_s3_client + + # Create S3Storage instance + storage = S3Storage() + + # Call generate_presigned_post without explicit expiration + storage.generate_presigned_post("test-object", "image/png", 1024) + + # Assert that the boto3 method was called with the default expiration (3600) + mock_s3_client.generate_presigned_post.assert_called_once() + call_kwargs = mock_s3_client.generate_presigned_post.call_args[1] + assert call_kwargs["ExpiresIn"] == 3600 + + @patch.dict( + os.environ, + { + "AWS_ACCESS_KEY_ID": "test-key", + "AWS_SECRET_ACCESS_KEY": "test-secret", + "AWS_S3_BUCKET_NAME": "test-bucket", + "AWS_REGION": "us-east-1", + "SIGNED_URL_EXPIRATION": "60", + }, + clear=True, + ) + @patch("plane.settings.storage.boto3") + def test_generate_presigned_post_uses_custom_expiration(self, mock_boto3): + """Test that generate_presigned_post uses custom expiration from env variable""" + # Mock the boto3 client and its response + mock_s3_client = Mock() + mock_s3_client.generate_presigned_post.return_value = { + "url": "https://test-url.com", + "fields": {}, + } + mock_boto3.client.return_value = mock_s3_client + + # Create S3Storage instance with SIGNED_URL_EXPIRATION=60 + storage = S3Storage() + + # Call generate_presigned_post without explicit expiration + storage.generate_presigned_post("test-object", "image/png", 1024) + + # Assert that the boto3 method was called with custom expiration (60) + mock_s3_client.generate_presigned_post.assert_called_once() + call_kwargs = mock_s3_client.generate_presigned_post.call_args[1] + assert call_kwargs["ExpiresIn"] == 60 + + @patch.dict( + os.environ, + { + "AWS_ACCESS_KEY_ID": "test-key", + "AWS_SECRET_ACCESS_KEY": "test-secret", + "AWS_S3_BUCKET_NAME": "test-bucket", + "AWS_REGION": "us-east-1", + }, + clear=True, + ) + @patch("plane.settings.storage.boto3") + def test_generate_presigned_url_uses_default_expiration(self, mock_boto3): + """Test that generate_presigned_url uses the configured default expiration""" + # Mock the boto3 client and its response + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = "https://test-url.com" + mock_boto3.client.return_value = mock_s3_client + + # Create S3Storage instance + storage = S3Storage() + + # Call generate_presigned_url without explicit expiration + storage.generate_presigned_url("test-object") + + # Assert that the boto3 method was called with the default expiration (3600) + mock_s3_client.generate_presigned_url.assert_called_once() + call_kwargs = mock_s3_client.generate_presigned_url.call_args[1] + assert call_kwargs["ExpiresIn"] == 3600 + + @patch.dict( + os.environ, + { + "AWS_ACCESS_KEY_ID": "test-key", + "AWS_SECRET_ACCESS_KEY": "test-secret", + "AWS_S3_BUCKET_NAME": "test-bucket", + "AWS_REGION": "us-east-1", + "SIGNED_URL_EXPIRATION": "30", + }, + clear=True, + ) + @patch("plane.settings.storage.boto3") + def test_generate_presigned_url_uses_custom_expiration(self, mock_boto3): + """Test that generate_presigned_url uses custom expiration from env variable""" + # Mock the boto3 client and its response + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = "https://test-url.com" + mock_boto3.client.return_value = mock_s3_client + + # Create S3Storage instance with SIGNED_URL_EXPIRATION=30 + storage = S3Storage() + + # Call generate_presigned_url without explicit expiration + storage.generate_presigned_url("test-object") + + # Assert that the boto3 method was called with custom expiration (30) + mock_s3_client.generate_presigned_url.assert_called_once() + call_kwargs = mock_s3_client.generate_presigned_url.call_args[1] + assert call_kwargs["ExpiresIn"] == 30 + + @patch.dict( + os.environ, + { + "AWS_ACCESS_KEY_ID": "test-key", + "AWS_SECRET_ACCESS_KEY": "test-secret", + "AWS_S3_BUCKET_NAME": "test-bucket", + "AWS_REGION": "us-east-1", + "SIGNED_URL_EXPIRATION": "30", + }, + clear=True, + ) + @patch("plane.settings.storage.boto3") + def test_explicit_expiration_overrides_default(self, mock_boto3): + """Test that explicit expiration parameter overrides the default""" + # Mock the boto3 client and its response + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = "https://test-url.com" + mock_boto3.client.return_value = mock_s3_client + + # Create S3Storage instance with SIGNED_URL_EXPIRATION=30 + storage = S3Storage() + + # Call generate_presigned_url with explicit expiration=120 + storage.generate_presigned_url("test-object", expiration=120) + + # Assert that the boto3 method was called with explicit expiration (120) + mock_s3_client.generate_presigned_url.assert_called_once() + call_kwargs = mock_s3_client.generate_presigned_url.call_args[1] + assert call_kwargs["ExpiresIn"] == 120