[ENG-5178] Allow unauthenticated users to see public files #10645
Conversation
This file contains 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
Comment on lines
-345
to
-346
| if not auth.user: | ||
| raise HTTPError(http_status.HTTP_401_UNAUTHORIZED, 'User authentication failed.') |
There was a problem hiding this comment.
Should it check for the node being public instead of just not checking at all, or does that happen elsewhere?
There was a problem hiding this comment.
That's covered in check_resource_permissions. The fact that we have early checking of permissions via the token scopes (and that the token scopes need to double check whether a resource is public instead of just accepting a permission check) is part of the smell that I was hoping to remove with the previous PR, but the LOE is too high at the moment.
brianjgeiger
approved these changes
Jun 21, 2024
Johnetordoff
pushed a commit
to Johnetordoff/osf.io
that referenced
this pull request
Jul 25, 2024
into remove-quickfiles-code * 'develop' of https://github.com/CenterForOpenScience/osf.io: (166 commits) update dataverse dep revision to get changes Update CHANGELOG, bump version [CR][ENG-5681] Great Big Python Upgrade (CenterForOpenScience#10648) Revert "[ENG-3685] Add permissions for withdrawn registration files (CenterForOpenScience#10650)" (CenterForOpenScience#10666) Check Registration READ perms on the Registration - Do not record download metrics for renders Fix signature Allow DOI metadata updates to be queued [ENG-3685] Add permissions for withdrawn registration files (CenterForOpenScience#10650) Update CHANGELOG, bump version [ENG-5030] Preprints Phase 2 - BE (CenterForOpenScience#10617) Update CHANGELOG, bump version Ensure Assumed-HAM users do not get autobanned [ENG-5762] Get GV set up in osf docker configs (CenterForOpenScience#10643) [ENG-5718] Use `make_auth` to avoid assumptions about `auth.user` (CenterForOpenScience#10647) [ENG-5699] Framework for getting Addon Info from GV (CenterForOpenScience#10641) [ENG-5178] Allow unauthenticated users to see public files (CenterForOpenScience#10645) Fix RelationshipPostMakesNoChanges exception in project creation (CenterForOpenScience#10644) [ENG - 5008] Support Unicode and special characters in file names during archiving (CenterForOpenScience#10627) Set Default Resource Type for Registrations to "Study Registration" (CenterForOpenScience#10636) Configurable GV Mock + HMAC Auth (CenterForOpenScience#10623) ... # Conflicts: # addons/base/views.py # api/caching/tasks.py # api_tests/files/views/test_file_detail.py # api_tests/wb/views/test_wb_hooks.py # osf/management/commands/data_storage_usage.py # osf/management/commands/reindex_quickfiles.py # osf/management/commands/transfer_quickfiles_to_projects.py # osf/models/__init__.py # osf/models/private_link.py # osf/models/quickfiles.py # osf/models/user.py # scripts/fix_merged_user_quickfiles.py # tests/test_views.py # website/mails/mails.py # website/search/elastic_search.py # website/settings/defaults.py
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.
Purpose
Restore expected functionality that unauthenticated users should be able to render/download public files.
NOTE:
This PR does not address the code smells of having additional auth checks that duplicate the work done by
@collect_auth. #10642 proposes an alternative which keeps us from needing to re-confirm token scopes with CAS, but the team lacks time to address all of the test breakages it causes. This PR, meanwhile restores the status quo.Changes
Remove explicit error case when auth.user does not exist.
QA Notes
Please make verification statements inspired by your code and what your code touches.
What are the areas of risk?
Any concerns/considerations/questions that development raised?
Documentation
Side Effects
Ticket