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

[document_repository] Fix download path to correct user #8378

Merged
merged 5 commits into from Mar 21, 2023

Conversation

zaliqarosli
Copy link
Contributor

@zaliqarosli zaliqarosli commented Feb 15, 2023

Brief summary of changes

  • Have you updated related documentation?

Testing instructions (if applicable)

  1. Checkout branch 24.1-release and run make dev or npm run compile
  2. Upload a file as User 1 (i.e. your own). Download that file once uploaded and confirm that the download worked.
  3. Login as User 2 (i.e. admin). Try to download the same file and confirm that the download fails.
  4. Go to the dashboard and click on one of the items in the 'Document Repository Notifications' widget. Confirm that the download fails.
  5. Checkout this PR branch. Download the same file as User 2 and confirm that the download worked
  6. Go to the dashboard and click on one of the items in the 'Document Repository Notifications' widget. Confirm that the download worked.

Link(s) to related issue(s)

  • Resolves # (Reference the issue this fixes, if any.)

@zaliqarosli zaliqarosli added Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) UI PR or issue introducing/requiring improvements to the LORIS User Interface labels Feb 21, 2023
@ridz1208
Copy link
Collaborator

ridz1208 commented Mar 6, 2023

@zaliqarosli @charliehenrib I'm confused how this is not a much much bigger issue. Shouldn't this prevent pretty much anyone from downloading any file that they didnt upload themselves ??

@ridz1208
Copy link
Collaborator

ridz1208 commented Mar 6, 2023

@CamilleBeau do you have a VM where you can pull this for charlie to test ?

@CamilleBeau
Copy link
Contributor

@CamilleBeau do you have a VM where you can pull this for charlie to test ?

Yes I will pull this onto our dev VM

@zaliqarosli
Copy link
Contributor Author

@ridz1208 i think it depends on what projects have this bug cz it was fixed for a certain version, and then the code refactored that re-introduced it. i know that it was something I had to fix for COPN on 21.. but it had already been fixed for a later version

@charliehenrib
Copy link
Contributor

@CamilleBeau @ridz1208 I followed the testing steps above and it works exactly as described. Is there a passed manual testing label that I should add?

@zaliqarosli zaliqarosli added the Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix label Mar 17, 2023
@zaliqarosli zaliqarosli force-pushed the 2023-02-15-FixDocRepoDownload branch from f6182f5 to 48bb685 Compare March 20, 2023 19:53
@zaliqarosli zaliqarosli removed the Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix label Mar 20, 2023
@zaliqarosli
Copy link
Contributor Author

@driusan i fixed it to what you suggested. can you re-review?

@driusan
Copy link
Collaborator

driusan commented Mar 21, 2023

Code looks good to me. @charliehenrib can you re-test it?

@charliehenrib
Copy link
Contributor

I retested and it works as it should :)

@driusan driusan merged commit 99f2698 into aces:24.1-release Mar 21, 2023
9 checks passed
@ridz1208 ridz1208 added this to the 24.1.3 milestone Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) UI PR or issue introducing/requiring improvements to the LORIS User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants