-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add share attributes + prevent download permission #32482
Conversation
0bb8ded
to
ffc23cd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
39e0151
to
98fa5b9
Compare
to test:
=> "Access denied". Note: this PR doesn't adjust the frontend yet, so the download action is still visible |
98fa5b9
to
e35f472
Compare
=> no, keep booleans due to prioritization |
d79d7aa
to
d70c3e3
Compare
I've fixed more tests, hope they all pass now (minus the node one that is broken on master) |
sqlite only not reproducible locally 😮
|
d70c3e3
to
4661ad3
Compare
4661ad3
to
9def5b3
Compare
rebased for CI, hopefully everything green after that! |
=> let's keep this as is |
I've added a "Allow download" checkbox now in the frontend and also the logic to read and save it. More work needed to:
|
would be good to have a first pass review already. |
if (this.share.hasDownloadPermission !== isDownloadChecked) { | ||
this.share.hasDownloadPermission = isDownloadChecked | ||
this.queueUpdate('attributes') | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: check if we can also skip "permissions"
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Fix tests - Use non deprecated event stuff - Add a bit of type hinting to the new stuff - More safe handling of instanceOfStorage (share might not be the first wrapper) - Fix resharing Signed-off-by: Carl Schwan <carl@carlschwan.eu>
73e3c4a
to
7b72381
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ci issue unrelated
/backport to stable24 |
Hello, @PVince81 |
@elhananjair yes, the different apps also need to be adjusted to properly recognize the permission and act accordingly. |
I filed nextcloud/files_pdfviewer#649 about this |
So, I've finally tried the feature, and unchecked "Allow download" when sharing a folder with an internal user, and the user would get "Access forbidden" with basically any document including .pdf. Is there something I'm missing inside the Administration Settings? Also, can we have the option to "Allow download" when sharing a single file with an internal user? |
Close #16413
extension points for secure view ?not for now