Skip to content

Fix for either over- or under-specific file permissions #1279

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

Merged
merged 2 commits into from
Jul 10, 2025

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Jul 10, 2025

As reported a few times by @jdbocarsly and @PeterKraus, file permissions are currently too all-or-nothing; either all users who know the file ID should be able to access it (typically only possible if they have access to the item that the file is attached to), or they have to have full access to the file as an uploader. The former approach is problematic, as a user may lose permissions but still be able to access the file.

This PR alters that by tying file permissions to the item permissions more directly. This is a semi-temporary fix until full granular object permissions are implemented, but I imagine this will be the default after that refactor anyway.

@ml-evs ml-evs added API For issues/PRs pertaining to the API server usability labels Jul 10, 2025
Copy link

codecov bot commented Jul 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.57%. Comparing base (436bea0) to head (600cc58).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1279      +/-   ##
==========================================
+ Coverage   74.50%   74.57%   +0.06%     
==========================================
  Files          66       66              
  Lines        4535     4535              
==========================================
+ Hits         3379     3382       +3     
+ Misses       1156     1153       -3     
Files with missing lines Coverage Δ
pydatalab/src/pydatalab/routes/v0_1/files.py 60.91% <100.00%> (+1.14%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

cypress bot commented Jul 10, 2025

datalab    Run #3584

Run Properties:  status check passed Passed #3584  •  git commit 43f112ea89 ℹ️: Merge 600cc589d0405d23f8da88ad7b405038ee1a29f9 into 436bea0236cf484c14da87a4fe3e...
Project datalab
Branch Review ml-evs/fix-file-permissions
Run status status check passed Passed #3584
Run duration 08m 20s
Commit git commit 43f112ea89 ℹ️: Merge 600cc589d0405d23f8da88ad7b405038ee1a29f9 into 436bea0236cf484c14da87a4fe3e...
Committer Matthew Evans
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 504
View all changes introduced in this branch ↗︎

@ml-evs ml-evs enabled auto-merge (squash) July 10, 2025 16:16
@ml-evs ml-evs disabled auto-merge July 10, 2025 16:27
@ml-evs ml-evs merged commit a7ad44c into main Jul 10, 2025
18 checks passed
@ml-evs ml-evs deleted the ml-evs/fix-file-permissions branch July 10, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API For issues/PRs pertaining to the API server usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant