Skip to content

Commit 5f7ffcb

Browse files
authored
- 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
1 parent 584a1aa commit 5f7ffcb

File tree

4 files changed

+214
-3
lines changed

4 files changed

+214
-3
lines changed

apps/api/.env.example

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ AWS_S3_ENDPOINT_URL="http://localhost:9000"
3232
AWS_S3_BUCKET_NAME="uploads"
3333
# Maximum file upload limit
3434
FILE_SIZE_LIMIT=5242880
35+
# Signed URL expiration time in seconds (default: 3600 = 1 hour)
36+
# Set to 30 for 30 seconds, 300 for 5 minutes, etc.
37+
SIGNED_URL_EXPIRATION=3600
3538

3639
# Settings related to Docker
3740
DOCKERIZED=1 # deprecated

apps/api/plane/settings/storage.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ def __init__(self, request=None):
2929
self.aws_region = os.environ.get("AWS_REGION")
3030
# Use the AWS_S3_ENDPOINT_URL environment variable for the endpoint URL
3131
self.aws_s3_endpoint_url = os.environ.get("AWS_S3_ENDPOINT_URL") or os.environ.get("MINIO_ENDPOINT_URL")
32+
# Use the SIGNED_URL_EXPIRATION environment variable for the expiration time (default: 3600 seconds)
33+
self.signed_url_expiration = int(os.environ.get("SIGNED_URL_EXPIRATION", "3600"))
3234

3335
if os.environ.get("USE_MINIO") == "1":
3436
# Determine protocol based on environment variable
@@ -56,8 +58,10 @@ def __init__(self, request=None):
5658
config=boto3.session.Config(signature_version="s3v4"),
5759
)
5860

59-
def generate_presigned_post(self, object_name, file_type, file_size, expiration=3600):
61+
def generate_presigned_post(self, object_name, file_type, file_size, expiration=None):
6062
"""Generate a presigned URL to upload an S3 object"""
63+
if expiration is None:
64+
expiration = self.signed_url_expiration
6165
fields = {"Content-Type": file_type}
6266

6367
conditions = [
@@ -104,13 +108,15 @@ def _get_content_disposition(self, disposition, filename=None):
104108
def generate_presigned_url(
105109
self,
106110
object_name,
107-
expiration=3600,
111+
expiration=None,
108112
http_method="GET",
109113
disposition="inline",
110114
filename=None,
111115
):
112-
content_disposition = self._get_content_disposition(disposition, filename)
113116
"""Generate a presigned URL to share an S3 object"""
117+
if expiration is None:
118+
expiration = self.signed_url_expiration
119+
content_disposition = self._get_content_disposition(disposition, filename)
114120
try:
115121
response = self.s3_client.generate_presigned_url(
116122
"get_object",

apps/api/plane/tests/unit/settings/__init__.py

Whitespace-only changes.
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
import os
2+
from unittest.mock import Mock, patch
3+
import pytest
4+
from plane.settings.storage import S3Storage
5+
6+
7+
@pytest.mark.unit
8+
class TestS3StorageSignedURLExpiration:
9+
"""Test the configurable signed URL expiration in S3Storage"""
10+
11+
@patch.dict(os.environ, {}, clear=True)
12+
@patch("plane.settings.storage.boto3")
13+
def test_default_expiration_without_env_variable(self, mock_boto3):
14+
"""Test that default expiration is 3600 seconds when env variable is not set"""
15+
# Mock the boto3 client
16+
mock_boto3.client.return_value = Mock()
17+
18+
# Create S3Storage instance without SIGNED_URL_EXPIRATION env variable
19+
storage = S3Storage()
20+
21+
# Assert default expiration is 3600
22+
assert storage.signed_url_expiration == 3600
23+
24+
@patch.dict(os.environ, {"SIGNED_URL_EXPIRATION": "30"}, clear=True)
25+
@patch("plane.settings.storage.boto3")
26+
def test_custom_expiration_with_env_variable(self, mock_boto3):
27+
"""Test that expiration is read from SIGNED_URL_EXPIRATION env variable"""
28+
# Mock the boto3 client
29+
mock_boto3.client.return_value = Mock()
30+
31+
# Create S3Storage instance with SIGNED_URL_EXPIRATION=30
32+
storage = S3Storage()
33+
34+
# Assert expiration is 30
35+
assert storage.signed_url_expiration == 30
36+
37+
@patch.dict(os.environ, {"SIGNED_URL_EXPIRATION": "300"}, clear=True)
38+
@patch("plane.settings.storage.boto3")
39+
def test_custom_expiration_multiple_values(self, mock_boto3):
40+
"""Test that expiration works with different custom values"""
41+
# Mock the boto3 client
42+
mock_boto3.client.return_value = Mock()
43+
44+
# Create S3Storage instance with SIGNED_URL_EXPIRATION=300
45+
storage = S3Storage()
46+
47+
# Assert expiration is 300
48+
assert storage.signed_url_expiration == 300
49+
50+
@patch.dict(
51+
os.environ,
52+
{
53+
"AWS_ACCESS_KEY_ID": "test-key",
54+
"AWS_SECRET_ACCESS_KEY": "test-secret",
55+
"AWS_S3_BUCKET_NAME": "test-bucket",
56+
"AWS_REGION": "us-east-1",
57+
},
58+
clear=True,
59+
)
60+
@patch("plane.settings.storage.boto3")
61+
def test_generate_presigned_post_uses_default_expiration(self, mock_boto3):
62+
"""Test that generate_presigned_post uses the configured default expiration"""
63+
# Mock the boto3 client and its response
64+
mock_s3_client = Mock()
65+
mock_s3_client.generate_presigned_post.return_value = {
66+
"url": "https://test-url.com",
67+
"fields": {},
68+
}
69+
mock_boto3.client.return_value = mock_s3_client
70+
71+
# Create S3Storage instance
72+
storage = S3Storage()
73+
74+
# Call generate_presigned_post without explicit expiration
75+
storage.generate_presigned_post("test-object", "image/png", 1024)
76+
77+
# Assert that the boto3 method was called with the default expiration (3600)
78+
mock_s3_client.generate_presigned_post.assert_called_once()
79+
call_kwargs = mock_s3_client.generate_presigned_post.call_args[1]
80+
assert call_kwargs["ExpiresIn"] == 3600
81+
82+
@patch.dict(
83+
os.environ,
84+
{
85+
"AWS_ACCESS_KEY_ID": "test-key",
86+
"AWS_SECRET_ACCESS_KEY": "test-secret",
87+
"AWS_S3_BUCKET_NAME": "test-bucket",
88+
"AWS_REGION": "us-east-1",
89+
"SIGNED_URL_EXPIRATION": "60",
90+
},
91+
clear=True,
92+
)
93+
@patch("plane.settings.storage.boto3")
94+
def test_generate_presigned_post_uses_custom_expiration(self, mock_boto3):
95+
"""Test that generate_presigned_post uses custom expiration from env variable"""
96+
# Mock the boto3 client and its response
97+
mock_s3_client = Mock()
98+
mock_s3_client.generate_presigned_post.return_value = {
99+
"url": "https://test-url.com",
100+
"fields": {},
101+
}
102+
mock_boto3.client.return_value = mock_s3_client
103+
104+
# Create S3Storage instance with SIGNED_URL_EXPIRATION=60
105+
storage = S3Storage()
106+
107+
# Call generate_presigned_post without explicit expiration
108+
storage.generate_presigned_post("test-object", "image/png", 1024)
109+
110+
# Assert that the boto3 method was called with custom expiration (60)
111+
mock_s3_client.generate_presigned_post.assert_called_once()
112+
call_kwargs = mock_s3_client.generate_presigned_post.call_args[1]
113+
assert call_kwargs["ExpiresIn"] == 60
114+
115+
@patch.dict(
116+
os.environ,
117+
{
118+
"AWS_ACCESS_KEY_ID": "test-key",
119+
"AWS_SECRET_ACCESS_KEY": "test-secret",
120+
"AWS_S3_BUCKET_NAME": "test-bucket",
121+
"AWS_REGION": "us-east-1",
122+
},
123+
clear=True,
124+
)
125+
@patch("plane.settings.storage.boto3")
126+
def test_generate_presigned_url_uses_default_expiration(self, mock_boto3):
127+
"""Test that generate_presigned_url uses the configured default expiration"""
128+
# Mock the boto3 client and its response
129+
mock_s3_client = Mock()
130+
mock_s3_client.generate_presigned_url.return_value = "https://test-url.com"
131+
mock_boto3.client.return_value = mock_s3_client
132+
133+
# Create S3Storage instance
134+
storage = S3Storage()
135+
136+
# Call generate_presigned_url without explicit expiration
137+
storage.generate_presigned_url("test-object")
138+
139+
# Assert that the boto3 method was called with the default expiration (3600)
140+
mock_s3_client.generate_presigned_url.assert_called_once()
141+
call_kwargs = mock_s3_client.generate_presigned_url.call_args[1]
142+
assert call_kwargs["ExpiresIn"] == 3600
143+
144+
@patch.dict(
145+
os.environ,
146+
{
147+
"AWS_ACCESS_KEY_ID": "test-key",
148+
"AWS_SECRET_ACCESS_KEY": "test-secret",
149+
"AWS_S3_BUCKET_NAME": "test-bucket",
150+
"AWS_REGION": "us-east-1",
151+
"SIGNED_URL_EXPIRATION": "30",
152+
},
153+
clear=True,
154+
)
155+
@patch("plane.settings.storage.boto3")
156+
def test_generate_presigned_url_uses_custom_expiration(self, mock_boto3):
157+
"""Test that generate_presigned_url uses custom expiration from env variable"""
158+
# Mock the boto3 client and its response
159+
mock_s3_client = Mock()
160+
mock_s3_client.generate_presigned_url.return_value = "https://test-url.com"
161+
mock_boto3.client.return_value = mock_s3_client
162+
163+
# Create S3Storage instance with SIGNED_URL_EXPIRATION=30
164+
storage = S3Storage()
165+
166+
# Call generate_presigned_url without explicit expiration
167+
storage.generate_presigned_url("test-object")
168+
169+
# Assert that the boto3 method was called with custom expiration (30)
170+
mock_s3_client.generate_presigned_url.assert_called_once()
171+
call_kwargs = mock_s3_client.generate_presigned_url.call_args[1]
172+
assert call_kwargs["ExpiresIn"] == 30
173+
174+
@patch.dict(
175+
os.environ,
176+
{
177+
"AWS_ACCESS_KEY_ID": "test-key",
178+
"AWS_SECRET_ACCESS_KEY": "test-secret",
179+
"AWS_S3_BUCKET_NAME": "test-bucket",
180+
"AWS_REGION": "us-east-1",
181+
"SIGNED_URL_EXPIRATION": "30",
182+
},
183+
clear=True,
184+
)
185+
@patch("plane.settings.storage.boto3")
186+
def test_explicit_expiration_overrides_default(self, mock_boto3):
187+
"""Test that explicit expiration parameter overrides the default"""
188+
# Mock the boto3 client and its response
189+
mock_s3_client = Mock()
190+
mock_s3_client.generate_presigned_url.return_value = "https://test-url.com"
191+
mock_boto3.client.return_value = mock_s3_client
192+
193+
# Create S3Storage instance with SIGNED_URL_EXPIRATION=30
194+
storage = S3Storage()
195+
196+
# Call generate_presigned_url with explicit expiration=120
197+
storage.generate_presigned_url("test-object", expiration=120)
198+
199+
# Assert that the boto3 method was called with explicit expiration (120)
200+
mock_s3_client.generate_presigned_url.assert_called_once()
201+
call_kwargs = mock_s3_client.generate_presigned_url.call_args[1]
202+
assert call_kwargs["ExpiresIn"] == 120

0 commit comments

Comments
 (0)