Skip to content

feat: update presigned url api to support visibility, svg image#100

Open
lethanhdat762003 wants to merge 4 commits into
devfrom
feat/update-presigned-url-api
Open

feat: update presigned url api to support visibility, svg image#100
lethanhdat762003 wants to merge 4 commits into
devfrom
feat/update-presigned-url-api

Conversation

@lethanhdat762003

Copy link
Copy Markdown
Member

What?

  • In this PR, I update the presigned url api to support visibility (public/private), SVG image type.

Why?

POST /api/presigned-url

Body: {
  "file_name": "avatar.png",
  "content_type": "image/png",
  "visibility": "public"
}

→ 200
{
  "presigned_url": "https://bucket.s3.amazonaws.com/...",
  "file_path": "public/550e8400-e29b_avatar.png"
}

GET /api/presigned-url/public/550e8400-e29b_avatar.png

→ 200
{
  "url": "https://bucket.s3.amazonaws.com/..."
}

How?

Testing?

  • Functional Testing
  • Security
  • Performance
  • Error Handling
  • Code Quality
  • Documentation
  • Database
  • Deployment
  • Final Review

Anything Else?

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the S3 presigned URL generation service by introducing a serializer for input validation, changing the upload URL endpoint to a POST request, and allowing dynamic file paths for retrieval. While these changes improve the API structure, several critical security and reliability issues were identified. Specifically, the retrieval endpoint is vulnerable to arbitrary S3 bucket access and needs path validation and authentication. Additionally, the upload serializer should restrict allowed MIME types to prevent unrestricted file uploads, input filenames should be sanitized to prevent path traversal, and settings access should be made safer to avoid potential AttributeErrors. Finally, it is recommended to use S3 Bucket Policies instead of deprecated Object ACLs for public files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread common/apps/upload_file/views.py Outdated
Comment thread common/apps/upload_file/serializers.py
Comment thread common/apps/upload_file/views.py Outdated
Comment thread common/apps/upload_file/service.py Outdated
Comment thread common/apps/upload_file/service.py Outdated
Comment on lines +46 to +47
if visibility == "public":
params["ACL"] = "public-read"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

S3 Object ACLs Deprecation / Block Public Access: Setting ACL='public-read' requires S3 Object ACLs to be enabled on the bucket and 'Block Public Access' to be disabled. AWS now strongly discourages the use of ACLs and enables 'Bucket owner enforced' (which disables ACLs) by default for new buckets.\n\nIf ACLs are disabled on your bucket, the client will receive a 403 Forbidden error when attempting to upload using this presigned URL.\n\nRecommendation: Instead of using Object ACLs, manage public visibility via an S3 Bucket Policy that grants s3:GetObject permission to anonymous users for any keys starting with the public/ prefix. This allows you to keep ACLs disabled and 'Block Public Access' enabled (except for the specific bucket policy allowance).

@lethanhdat762003
lethanhdat762003 force-pushed the feat/update-presigned-url-api branch 2 times, most recently from cb4cc84 to 6957df4 Compare June 16, 2026 07:36
@lethanhdat762003
lethanhdat762003 force-pushed the feat/update-presigned-url-api branch from 58a87ec to cf58043 Compare June 17, 2026 01:58
@lethanhdat762003
lethanhdat762003 force-pushed the feat/update-presigned-url-api branch from d2e8e80 to dc87978 Compare July 1, 2026 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant