mirror of
https://github.com/makeplane/plane.git
synced 2025-12-16 11:57:56 +01:00
- Add SIGNED_URL_EXPIRATION environment variable (#8136)
- Update S3Storage to use configurable expiration time - Default remains 3600 seconds (1 hour) for backward compatibility - Add comprehensive unit tests with mocked S3 client - Update .env.example with documentation and examples
This commit is contained in:
committed by
GitHub
parent
584a1aa725
commit
5f7ffcb37a
@@ -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
|
||||
|
||||
@@ -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",
|
||||
|
||||
0
apps/api/plane/tests/unit/settings/__init__.py
Normal file
0
apps/api/plane/tests/unit/settings/__init__.py
Normal file
202
apps/api/plane/tests/unit/settings/test_storage.py
Normal file
202
apps/api/plane/tests/unit/settings/test_storage.py
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user