Add permission check to datasets/download#2348
Merged
Conversation
Collaborator
The only problem with this putting 404 everywhere is that it would make it harder to debug when someone or something is having issues with this fix |
Collaborator
|
Also, there is an "error" in the logs when the permission is denied, is that normal ? 2026-04-21 15:10:28.214 | WARNING | django.utils.log:log_response:253 - Forbidden (Permission denied): /datasets/download/*hidden_url*/
Traceback (most recent call last):
File "/root/.local/share/uv/python/cpython-3.13.11-linux-x86_64-gnu/lib/python3.13/threading.py", line 1015, in _bootstrap
self._bootstrap_inner()
│ └ <function Thread._bootstrap_inner at 0x76ee264f79c0>
└ <Thread(asyncio_0, started 130765090645696)>
File "/root/.local/share/uv/python/cpython-3.13.11-linux-x86_64-gnu/lib/python3.13/threading.py", line 1044, in _bootstrap_inner
self.run()
│ └ <function Thread.run at 0x76ee264f7740>
└ <Thread(asyncio_0, started 130765090645696)>
File "/root/.local/share/uv/python/cpython-3.13.11-linux-x86_64-gnu/lib/python3.13/threading.py", line 995, in run
self._target(*self._args, **self._kwargs)
│ │ │ │ │ └ {}
│ │ │ │ └ <Thread(asyncio_0, started 130765090645696)>
│ │ │ └ (<weakref at 0x76ee1ed4b4c0; to 'concurrent.futures.thread.ThreadPoolExecutor' at 0x76ee1edbb250>, <_queue.SimpleQueue object...
│ │ └ <Thread(asyncio_0, started 130765090645696)>
│ └ <function _worker at 0x76ee25b8a520>
└ <Thread(asyncio_0, started 130765090645696)>
File "/root/.local/share/uv/python/cpython-3.13.11-linux-x86_64-gnu/lib/python3.13/concurrent/futures/thread.py", line 93, in _worker
work_item.run()
│ └ <function _WorkItem.run at 0x76ee25b8a660>
└ <concurrent.futures.thread._WorkItem object at 0x76ee0f7f5710>
File "/root/.local/share/uv/python/cpython-3.13.11-linux-x86_64-gnu/lib/python3.13/concurrent/futures/thread.py", line 59, in run
result = self.fn(*self.args, **self.kwargs)
│ │ │ │ │ └ {}
│ │ │ │ └ <concurrent.futures.thread._WorkItem object at 0x76ee0f7f5710>
│ │ │ └ ()
│ │ └ <concurrent.futures.thread._WorkItem object at 0x76ee0f7f5710>
│ └ functools.partial(<bound method SyncToAsync.thread_handler of <asgiref.sync.SyncToAsync object at 0x76ee0f779250>>, <_UnixSel...
└ <concurrent.futures.thread._WorkItem object at 0x76ee0f7f5710>
> File "/.venv/lib/python3.13/site-packages/asgiref/sync.py", line 557, in thread_handler
raise exc_info[1]
└ (<class 'django.core.exceptions.PermissionDenied'>, PermissionDenied(), <traceback object at 0x76ee1d492780>)
File "/.venv/lib/python3.13/site-packages/django/core/handlers/exception.py", line 42, in inner
response = await get_response(request)
│ └ <ASGIRequest: GET '/datasets/download/*hidden_url*/'>
└ <bound method BaseHandler._get_response_async of <django.core.handlers.asgi.ASGIHandler object at 0x76ee2046b0e0>>
File "/.venv/lib/python3.13/site-packages/asgiref/sync.py", line 557, in thread_handler
raise exc_info[1]
└ (<class 'django.core.exceptions.PermissionDenied'>, PermissionDenied(), <traceback object at 0x76ee0f629880>)
File "/.venv/lib/python3.13/site-packages/django/core/handlers/base.py", line 253, in _get_response_async
response = await wrapped_callback(
└ <asgiref.sync.SyncToAsync object at 0x76ee0f7f85f0>
File "/.venv/lib/python3.13/site-packages/asgiref/sync.py", line 506, in __call__
ret = await asyncio.shield(exec_coro)
│ │ └ <Future finished exception=PermissionDenied()>
│ └ <function shield at 0x76ee25e57060>
└ <module 'asyncio' from '/root/.local/share/uv/python/cpython-3.13.11-linux-x86_64-gnu/lib/python3.13/asyncio/__init__.py'>
File "/.venv/lib/python3.13/site-packages/asgiref/current_thread_executor.py", line 40, in run
result = self.fn(*self.args, **self.kwargs)
│ │ └ None
│ └ None
└ None
File "/.venv/lib/python3.13/site-packages/asgiref/sync.py", line 561, in thread_handler
return func(*args, **kwargs)
│ │ └ {}
│ └ (functools.partial(<function download at 0x76ee1fe1e5c0>, <ASGIRequest: GET '/datasets/download/*hidden_url*...
└ <built-in method run of _contextvars.Context object at 0x76ee1d4e34c0>
File "/app/src/apps/datasets/views.py", line 106, in download
raise PermissionDenied()
└ <class 'django.core.exceptions.PermissionDenied'>
django.core.exceptions.PermissionDenied |
Collaborator
Member
Author
Member
Author
|
Fixed. By the way it looks like we can't even update |
10 tasks
ObadaS
approved these changes
Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Description
Check permission when accessing
<DOMAIN_NAME>/datasets/download/key.Comment: the code returns 404 when not logged in and 403 when logged in but without permission. Maybe we want 404 everywhere to not disclose the existence of the object.
Issues this PR resolves
A checklist for hand testing
Checklist