Support uploading files with administrator privileges #845
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
We should do what's mocked up at #453 (comment) I can transplant that into an alert for a mockup for the time being though. |
|
Same success state concept, transferred to a standard standalone alert, so we can immediately use it:
|
jelly
force-pushed
the
upload-file-permissions
branch
from
0e40e15 to
b9050b4
Compare
jelly
force-pushed
the
upload-file-permissions
branch
3 times, most recently
from
edba4f8 to
57046a2
Compare
src/upload-button.tsx
Outdated
| } catch (exc) { | ||
| await cockpit.spawn(["rm", "-f", destination], { superuser: "require" }); | ||
| console.warn("Cannot set initial file permissions", exc); | ||
| addAlert(_("Cancelled"), AlertVariant.warning, "upload", | ||
| cockpit.format(_("Cancelled upload of $0"), file.name)); | ||
| cancelledUploads.push(file); | ||
| return; |
There was a problem hiding this comment.
Not entirely sure how to test this, I'll give it some more thought.
| addAlert: (title: string, variant: AlertVariant, key: string, detail?: string | React.ReactNode, | ||
| actionLinks?: React.ReactNode) => void, |
There was a problem hiding this comment.
This basically repeats interface Alert. Couldn't this be changed to pass an alert: Alert object instead? that would make the typing more useful and stronger. It would also simplify the addAlert() implementation below (to the point of being trivial and it could be dropped)
There was a problem hiding this comment.
I don't think that works does it? Doesn't seem too, because then there is one argument instead of multiple and an interface cannot be an argument. In general open to making this easier. But feels out of scope now.
There was a problem hiding this comment.
Uh, what? addAlert: (alert: Alert) => void and calling it as alert({ title: "foo", variant: ... }) doesn't work? Then I'm rather disappointed by types. This duplication is annoying and error prone, and most of all does not express what we actually want.
If it's too hard, then ok to ignore of course.
jelly
force-pushed
the
upload-file-permissions
branch
2 times, most recently
from
f26f8fe to
f4c5c2e
Compare
Files could not yet support uploading to non-logged in user directories as these files would always be uploaded as `root:root` and this might not be wanted in directories not owned by `root`. Therefore they are now uploaded by default as the owner of the directory with the ability to change ownership after upload in the shown alert.
jelly
force-pushed
the
upload-file-permissions
branch
from
f4c5c2e to
2a1b189
Compare
| cwdInfo: FileInfo | null, | ||
| } | ||
|
|
||
| export const FilesContext = React.createContext({ | ||
| addAlert: () => console.warn("FilesContext not initialized"), | ||
| removeAlert: () => console.warn("FilesContext not initialized"), |
There was a problem hiding this comment.
This added line is not executed by any test.
| } catch (exc) { | ||
| console.warn(`Unable to cleanup file: ${destination}, err: ${exc}`); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test.
| } catch (exc) { | ||
| console.warn("Unable to move file to final destination", exc); | ||
| addAlert(_("Upload error"), AlertVariant.danger, "upload-error", | ||
| _("Unable to move uploaded file to final destination")); | ||
| try { | ||
| await cockpit.file(destination, { superuser: "require" }).replace(null); | ||
| } catch (exc) { | ||
| console.warn(`Unable to cleanup file: ${destination}, err: ${exc}`); |
There was a problem hiding this comment.
These 8 added lines are not executed by any test.
| } catch (exc) { | ||
| console.warn(`Unable to cleanup file: ${destination}, err: ${exc}`); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test.
Files could not yet support uploading to non-logged in user directories as these files would always be uploaded as
root:rootand this might not be wanted in directories not owned byroot.Allow uploading files as administrator
Cockpit-files now allows you to upload files into directories not owned by the logged in user, when you are administrator. It will by default upload the file with the same user/group as the current directory. You can change the ownership after uploading.