nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.47k stars 4.08k forks source link

Disable move function on shared folder, if target is out of scope of shared root folder #40881

Open Rusty-Weasel opened 1 year ago

Rusty-Weasel commented 1 year ago

How to use GitHub

Is your feature request related to a problem? Please describe.

Example usecase:

Then u got a problem with the "moveFileFunction", as everbybody is able to move files from shared folder to his own root folder (in user space) or to a subfolder (in user space). Then further from his user space, he can do everything, also sync!, what is in upper usecase not allowed.

Actually i found no way to protect it, also tried things like "File access control" and "Group folders".

Describe the solution you'd like

Describe alternatives you've considered

Additional context Here some examples:

martin-77 commented 1 year ago

any news on this? Will it be added to the roadmap? I think this is an essential feature we need to have!

Rusty-Weasel commented 11 months ago

Short Update:

just updated my test server from 27.1.4 to latest 28.0.0 and the result is:

Vue.js is faster = nice, but (as expected) also bringing back old bugs....same problem as with 27.1.4...i'll fix it again by my self.

Further thanks for a not working upgrade progress (neither by webui or occ upgrade) from 27.1.4 to 28.0.0, never see such a way to upgrade, what a nightmare, sorry but that rly bad.

Rusty-Weasel commented 11 months ago

Intresting behavior by testing my old way to fix,

i change the filelist.js with my fixed edition, and spooki things happening

rohit1dhiman commented 7 months ago

@GoWeasel can you share GIT link of your solution, i am also facing similar problem

Rusty-Weasel commented 7 months ago

@GoWeasel can you share GIT link of your solution, i am also facing similar problem

i'll prepare it after testing on latest update, as the problem is old, i need to re-check it first, in new version.

rohit1dhiman commented 7 months ago

@GoWeasel can you share GIT link of your solution, i am also facing similar problem

i'll prepare it after testing on latest update, as the problem is old, i need to re-check it first, in new version.

I need that for non productive usage, doing some POC, so need it for a quick turnaround, share me even no tested version, will help me to draw some inspiration , sorry for pushing 😁😁😁😁😁

Rusty-Weasel commented 7 months ago

Modified file: /var/www/nextcloud/apps/files/js/filelist.js

you find the needed part at lines: image

That was the rudimentary way to fix in NC 27.1.2 from start of issue, as mentioned in NC 28 upper, there was something not as expected, but hopefully u can work out a nice way for the future.

I marked the changed areas for you in MODIFICATION start/end, hope you can find inspiration for something useful with this rudimentary and actually untested way

Basically it is just a root-path comparism between source and target, if not matching = forbidden, if matching = allowed.

Very simple, but effective.

I wish some one implement such feature in a prof. way, as without, NC is in my case not applicable in any mid business environment, concerning new directives coming up in europe.

I have not enough time for it - at moment, too much focused on rust, as security matters.

        /**
         * Moves a file to a given target folder.
         *
         * @param fileNames array of file names to move
         * @param targetPath absolute target path
         * @param callback function to call when movement is finished
         * @param dir the dir path where fileNames are located (optional, will take current folder if undefined)
         */
        move: function(fileNames, targetPath, callback, dir) {
            var self = this;

            dir = typeof dir === 'string' ? dir : this.getCurrentDirectory();
            if (dir.charAt(dir.length - 1) !== '/') {
                dir += '/';
            }

            var target = OC.basename(targetPath);
            if (!_.isArray(fileNames)) {
                fileNames = [fileNames];
            }

// MODIFICATION START

            // Source -> Target ? Check RootPath
            var rootPath = dir.substring(0,dir.indexOf("/",1))

            console.log("SourcePath: " + dir.toString());
            console.log("TargetPath: " + targetPath.toString());
            console.log("Allowed RootPath: " + rootPath);

            // If targetPath starts with RootPath == true, then move is allowed, else forbidden
            if (targetPath.toString().startsWith(rootPath) === true ){
                console.log("Allowed - Move on same RootPath structure");

                var moveFileFunction = function(fileName) {
                    var $tr = self.findFileEl(fileName);
                    self.showFileBusyState($tr, true);
                    if (targetPath.charAt(targetPath.length - 1) !== '/') {
                        // make sure we move the files into the target dir,
                        // not overwrite it
                        targetPath = targetPath + '/';
                    }

                    return self.filesClient.move(dir + fileName, targetPath + fileName)
                        .done(function() {
                            // if still viewing the same directory
                            if (OC.joinPaths(self.getCurrentDirectory(), '/') === OC.joinPaths(dir, '/')) {
                                // recalculate folder size
                                var oldFile = self.findFileEl(target);
                                var newFile = self.findFileEl(fileName);
                                var oldSize = oldFile.data('size');
                                var newSize = oldSize + newFile.data('size');
                                oldFile.data('size', newSize);
                                oldFile.find('td.filesize').text(OC.Util.humanFileSize(newSize));
                                self.remove(fileName);
                            }
                        })
                        .fail(function(status) {
                            if (status === 412) {
                                // TODO: some day here we should invoke the conflict dialog
                                OC.Notification.show(t('files', 'Could not move "{file}", target exists',
                                    {file: fileName}), {type: 'error'}
                                );
                            } else {
                                OC.Notification.show(t('files', 'Could not move "{file}"',
                                    {file: fileName}), {type: 'error'}
                                );
                            }
                        })
                        .always(function() {
                            self.showFileBusyState($tr, false);
                        });
                };

            } else{
                console.log("Forbidden - Move in other RootPath is not allowed");
                OC.Notification.show(t('files', 'Forbidden - Move in other RootPath is not allowed'));

            }

            return this.reportOperationProgress(fileNames, moveFileFunction, callback).then(function() {
                self.updateStorageStatistics();
                self.updateStorageQuotas();
            });
        },
// MODIFICATION END
        _reflect: function (promise){
            return promise.then(function(v){ return {};}, function(e){ return {};});
        },

        reportOperationProgress: function (fileNames, operationFunction, callback){
            var self = this;
            self._operationProgressBar.showProgressBar(false);
            var mcSemaphore = new OCA.Files.Semaphore(5);
            var counter = 0;
            var promises = _.map(fileNames, function(arg) {
                return mcSemaphore.acquire().then(function(){
                    return operationFunction(arg).always(function(){
                        mcSemaphore.release();
                        counter++;
                        self._operationProgressBar.setProgressBarValue(100.0*counter/fileNames.length);
                    });
                });
            });

            return Promise.all(_.map(promises, self._reflect)).then(function(){
                if (callback) {
                    callback();
                }
                self._operationProgressBar.hideProgressBar();
            });
        },
Rusty-Weasel commented 7 months ago

As expected, just tested on NC 28.04, and something changed, does not work any more :-1:

rohit1dhiman commented 7 months ago

thanks @GoWeasel for your support, let me try this, at least i have something to start with. thanks a ton for your quick help and response

martin-77 commented 7 months ago

this function is as basic as essential - still do not understand, why this is not part of nextcloud. LetΒ΄s try upvoting in the related help.nextcloud-thread...

chriswong33383 commented 6 months ago

As expected, just tested on NC 28.04, and something changed, does not work any more πŸ‘Ž

hi @GoWeasel, may I know did you find alternative way for after NC28?

Rusty-Weasel commented 6 months ago

As expected, just tested on NC 28.04, and something changed, does not work any more πŸ‘Ž

hi @GoWeasel, may I know did you find alternative way for after NC28?

no, no time for it, as working on upcoming NIS2 and CRA (Cyber Resillience Act), but at moment i see no posibility to use NC under the mentioned directives.