Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: auditors cannot upload/delete the document #357

Merged
merged 7 commits into from
Jul 7, 2022

Conversation

jLebioda
Copy link
Collaborator

@jLebioda jLebioda commented Jul 6, 2022

No description provided.

@jLebioda jLebioda requested a review from vildead July 6, 2022 13:52
@@ -98,13 +98,18 @@ def download_document(request, pk):
@permission_required(Permissions.PROTECTED, (Document, 'pk', 'pk'))
def delete_document(request, pk):
document = get_object_or_404(Document, pk=pk)

if not request.user.is_superuser and not document.can_user_delete(request.user):
Copy link
Member

Choose a reason for hiding this comment

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

This is too restrictive. We can rely on PROTECTED decorator here.
Lets just check if the user is auditor
if yes -> 403
if no -> continue

The same should be done for upload_document() - which should also get a PROTECTED decorator

@vildead vildead changed the title feat: only local custodians can delete the document feat: auditors cannot delete the document Jul 7, 2022
@vildead vildead changed the title feat: auditors cannot delete the document feat: auditors cannot upload/delete the document Jul 7, 2022
@vildead vildead force-pushed the correct-who-can-delete-the-document branch from c164240 to bccb967 Compare July 7, 2022 09:28
@vildead vildead merged commit 5504e58 into develop Jul 7, 2022
@jLebioda jLebioda deleted the correct-who-can-delete-the-document branch July 7, 2022 09:51
@vildead vildead mentioned this pull request Sep 19, 2022
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.

2 participants