From 4e5aefce274b8e812facafd7db6f13ebd24defe8 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Mon, 23 Feb 2026 15:41:31 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20a=20download=20actio?= =?UTF-8?q?n=20returning=20the=20media=20url?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The media url is subject to change if the file is renamed. If someone is using it as a permalink, it will lose the ability to download the file. We created a new download action, always returning a redirection to the media url. --- CHANGELOG.md | 3 +- src/backend/core/api/viewsets.py | 27 ++- src/backend/core/models.py | 1 + .../tests/items/test_api_items_download.py | 222 ++++++++++++++++++ src/backend/core/tests/test_models_items.py | 8 + .../core/tests/test_models_items_root.py | 9 + 6 files changed, 267 insertions(+), 3 deletions(-) create mode 100644 src/backend/core/tests/items/test_api_items_download.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d43c25f7..3f92560d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to - 👷(docker) add arm64 platform support for image builds - ✨(global) add create file from template feature - ✨(global) add FRONTEND_CSS_URL and FRONTEND_JS_URL settings +- ✨(backend) add a download action returning the media url ### Fixed @@ -328,5 +329,3 @@ and this project adheres to [v0.2.0]: https://github.com/suitenumerique/drive/releases/v0.2.0 [v0.1.1]: https://github.com/suitenumerique/drive/releases/v0.1.1 [v0.1.0]: https://github.com/suitenumerique/drive/releases/v0.1.0 - -## [v0.11.1] - 2026-01-13 diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 03aaf7ff..3dd91a10 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -6,7 +6,7 @@ import logging import os import re from io import BytesIO -from urllib.parse import unquote, urlparse +from urllib.parse import quote, unquote, urlparse from django.conf import settings from django.contrib.postgres.aggregates import ArrayAgg @@ -1514,6 +1514,31 @@ class ItemViewSet( ) return url_params, user_abilities, request.user.id, item + @drf.decorators.action(detail=True, methods=["get"], url_path="download") + def download(self, request, *args, **kwargs): + """ + Permalink endpoint for downloading an item's file. + + Returns a redirect to the current media URL for the item, so this link + remains valid even after the item is renamed. Authentication is still + enforced by the existing media-auth mechanism on the redirected URL. + """ + item = self.get_object() + + if item.type != models.ItemTypeChoices.FILE: + raise drf.exceptions.PermissionDenied() + + if item.upload_state == models.ItemUploadStateChoices.PENDING: + raise drf.exceptions.PermissionDenied() + + redirect_url = ( + f"{settings.MEDIA_BASE_URL}{settings.MEDIA_URL}{quote(item.file_key)}" + ) + return drf.response.Response( + status=status.HTTP_302_FOUND, + headers={"Location": redirect_url}, + ) + @drf.decorators.action(detail=False, methods=["get"], url_path="media-auth") def media_auth(self, request, *args, **kwargs): """ diff --git a/src/backend/core/models.py b/src/backend/core/models.py index c294fcef..37968b3d 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -794,6 +794,7 @@ class Item(TreeModel, BaseModel): "children_list": can_get, "children_create": can_create_children, "destroy": can_destroy, + "download": can_get, "hard_delete": can_hard_delete, "favorite": can_get and user.is_authenticated, "link_configuration": is_owner_or_admin, diff --git a/src/backend/core/tests/items/test_api_items_download.py b/src/backend/core/tests/items/test_api_items_download.py new file mode 100644 index 00000000..532cc849 --- /dev/null +++ b/src/backend/core/tests/items/test_api_items_download.py @@ -0,0 +1,222 @@ +""" +Test the item download permalink endpoint in drive's core app. +""" + +import pytest +from rest_framework.test import APIClient + +from core import factories, models +from core.tests.conftest import TEAM, USER, VIA + +pytestmark = pytest.mark.django_db + + +def test_api_items_download_anonymous_public(): + """Anonymous users should be redirected when the item is public.""" + item = factories.ItemFactory( + link_reach="public", + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + ) + + response = APIClient().get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 302 + assert item.filename in response["Location"] + assert f"item/{item.pk!s}" in response["Location"] + + +@pytest.mark.parametrize("reach", ["authenticated", "restricted"]) +def test_api_items_download_anonymous_authenticated_or_restricted(reach): + """ + Anonymous users should not be allowed to download items with link reach + set to authenticated or restricted. + """ + item = factories.ItemFactory( + link_reach=reach, + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + ) + + response = APIClient().get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 401 + + +@pytest.mark.parametrize("reach", ["public", "authenticated"]) +def test_api_items_download_authenticated_public_or_authenticated(reach): + """ + Authenticated users without explicit access to a public or authenticated item + should be redirected to the media URL. + """ + item = factories.ItemFactory( + link_reach=reach, + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + ) + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 302 + assert item.filename in response["Location"] + + +def test_api_items_download_authenticated_restricted(): + """ + Authenticated users without explicit access to a restricted item + should not be allowed to download it. + """ + item = factories.ItemFactory( + link_reach="restricted", + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + ) + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 403 + + +@pytest.mark.parametrize("via", VIA) +@pytest.mark.parametrize( + "upload_state", + [ + models.ItemUploadStateChoices.READY, + models.ItemUploadStateChoices.ANALYZING, + models.ItemUploadStateChoices.FILE_TOO_LARGE_TO_ANALYZE, + ], +) +def test_api_items_download_related(via, mock_user_teams, upload_state): + """ + Users with explicit access to an item should be redirected to the media URL. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + item = factories.ItemFactory( + type=models.ItemTypeChoices.FILE, + update_upload_state=upload_state, + ) + if via == USER: + factories.UserItemAccessFactory(item=item, user=user) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamItemAccessFactory(item=item, team="lasuite") + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 302 + assert item.filename in response["Location"] + assert f"item/{item.pk!s}" in response["Location"] + + +def test_api_items_download_redirect_url_stable_after_rename(): + """ + The download permalink URL must remain valid after an item is renamed. + The redirect target should point to the current filename. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + item = factories.ItemFactory( + type=models.ItemTypeChoices.FILE, + filename="original_name.pdf", + update_upload_state=models.ItemUploadStateChoices.READY, + users=[(user, models.RoleChoices.EDITOR)], + ) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + assert response.status_code == 302 + assert "original_name.pdf" in response["Location"] + + # Simulate a rename by updating the filename directly + item.filename = "renamed_file.pdf" + item.save() + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + assert response.status_code == 302 + assert "renamed_file.pdf" in response["Location"] + assert "original_name.pdf" not in response["Location"] + + +def test_api_items_download_item_not_a_file(): + """Folders should not be downloadable via the permalink endpoint.""" + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + item = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + factories.UserItemAccessFactory(item=item, user=user) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 403 + + +def test_api_items_download_item_pending(): + """Pending items (upload not complete) should not be downloadable.""" + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + item = factories.ItemFactory( + type=models.ItemTypeChoices.FILE, + upload_state=models.ItemUploadStateChoices.PENDING, + ) + factories.UserItemAccessFactory(item=item, user=user) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 403 + + +def test_api_items_download_suspicious_item_non_creator(): + """ + Users who are not the creator of a suspicious item should not be able + to download it via the permalink endpoint. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + item = factories.ItemFactory( + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.SUSPICIOUS, + users=[(user, models.RoleChoices.OWNER)], + ) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 404 + + +def test_api_items_download_suspicious_item_creator(): + """ + The creator of a suspicious item should be redirected when downloading + via the permalink endpoint. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + item = factories.ItemFactory( + creator=user, + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.SUSPICIOUS, + users=[(user, models.RoleChoices.OWNER)], + ) + + response = client.get(f"/api/v1.0/items/{item.pk}/download/") + + assert response.status_code == 302 + assert item.filename in response["Location"] diff --git a/src/backend/core/tests/test_models_items.py b/src/backend/core/tests/test_models_items.py index ccd4dc95..3a224136 100644 --- a/src/backend/core/tests/test_models_items.py +++ b/src/backend/core/tests/test_models_items.py @@ -252,6 +252,7 @@ def test_models_items_get_abilities_forbidden( "favorite": False, "invite_owner": False, "media_auth": False, + "download": False, "move": False, "link_configuration": False, "link_select_options": {}, @@ -301,6 +302,7 @@ def test_models_items_get_abilities_reader( "link_configuration": False, "link_select_options": {}, "media_auth": True, + "download": True, "move": False, "partial_update": False, "restore": False, @@ -352,6 +354,7 @@ def test_models_items_get_abilities_editor( "link_configuration": False, "link_select_options": {}, "media_auth": True, + "download": True, "move": False, "partial_update": True, "restore": False, @@ -396,6 +399,7 @@ def test_models_items_not_root_get_abilities_owner(django_assert_num_queries): "restricted": None, }, "media_auth": True, + "download": True, "move": True, "partial_update": True, "restore": True, @@ -422,6 +426,7 @@ def test_models_items_not_root_get_abilities_owner(django_assert_num_queries): "link_configuration": False, "link_select_options": {}, "media_auth": False, + "download": False, "move": False, "partial_update": False, "restore": True, @@ -456,6 +461,7 @@ def test_models_items_not_root_get_abilities_administrator(django_assert_num_que "restricted": None, }, "media_auth": True, + "download": True, "move": True, "partial_update": True, "restore": False, @@ -499,6 +505,7 @@ def test_models_items_not_root_get_abilities_editor_user(django_assert_num_queri "link_configuration": False, "link_select_options": link_select_options, "media_auth": True, + "download": True, "move": False, "partial_update": True, "restore": False, @@ -547,6 +554,7 @@ def test_models_items_not_root_get_abilities_reader_user(django_assert_num_queri "restricted": None, }, "media_auth": True, + "download": True, "move": False, "partial_update": access_from_link, "restore": False, diff --git a/src/backend/core/tests/test_models_items_root.py b/src/backend/core/tests/test_models_items_root.py index b9eb36dc..f78abad9 100644 --- a/src/backend/core/tests/test_models_items_root.py +++ b/src/backend/core/tests/test_models_items_root.py @@ -10,6 +10,8 @@ from core import factories, models pytestmark = pytest.mark.django_db +# # pylint: disable=duplicate-code + def test_models_sub_item_abilities_downgraded(): """ @@ -58,6 +60,7 @@ def test_models_sub_item_abilities_downgraded(): "restricted": None, }, "media_auth": True, + "download": True, "move": False, "partial_update": True, "restore": False, @@ -90,6 +93,7 @@ def test_models_sub_item_abilities_downgraded(): "restricted": None, }, "media_auth": True, + "download": True, "move": False, "partial_update": False, "restore": False, @@ -121,6 +125,7 @@ def test_models_items_root_get_abilities_owner(django_assert_num_queries): "link_configuration": True, "link_select_options": link_select_options, "media_auth": True, + "download": True, "move": True, "partial_update": True, "restore": True, @@ -147,6 +152,7 @@ def test_models_items_root_get_abilities_owner(django_assert_num_queries): "link_configuration": False, "link_select_options": {}, "media_auth": False, + "download": False, "move": False, "partial_update": False, "restore": True, @@ -178,6 +184,7 @@ def test_models_items_root_get_abilities_administrator(django_assert_num_queries "link_configuration": True, "link_select_options": link_select_options, "media_auth": True, + "download": True, "move": True, "partial_update": True, "restore": False, @@ -218,6 +225,7 @@ def test_models_items_root_get_abilities_editor_user(django_assert_num_queries): "link_configuration": False, "link_select_options": link_select_options, "media_auth": True, + "download": True, "move": False, "partial_update": True, "restore": False, @@ -259,6 +267,7 @@ def test_models_items_root_get_abilities_reader_user(django_assert_num_queries): "link_configuration": False, "link_select_options": link_select_options, "media_auth": True, + "download": True, "move": False, "partial_update": access_from_link, "restore": False,