owncloud-archive / search_lucene

The ownCloud search_lucene app
GNU Affero General Public License v3.0
18 stars 17 forks source link

Fix url links to lucene result page with OC 9.1 #124

Closed flaracca closed 2 years ago

flaracca commented 8 years ago

The issue is that lucene generates this type of url:

http://domain.com/files/index.php?dir=%2FDocuments&file=Example.odt

and OC 9.1 wants this:

http://domain.com/remote.php/webdav/Documents/Example.odt

mention-bot commented 8 years ago

@flaracca, thanks for your PR! By analyzing the history of the files in this pull request, we identified @butonic, @VicDeo and @icewind1991 to be potential reviewers.

mrow4a commented 8 years ago

@DeepDiver1975 @PVince81

https://central.owncloud.org/t/full-text-search-on-owncloud-9-1/1332/10

PVince81 commented 8 years ago

Well it depends what we want the behavior to be... The URL http://domain.com/files/index.php?dir=%2FDocuments&file=Example.odt should still be valid in 9.1 (although I thought "file" shuld be "scrollto"). This would display the file within the file list.

But changing it to the Webdav URL would trigger a download instead of displaying it in the list. In some use cases, for example people who have the documents app, it makes more sense to display the result in the original file list to allow the user to then click on it to open in the documents app. (yes, ideal would be direct open, something for later)

PVince81 commented 8 years ago

Even better would be to change it to a permalink: http://localhost/owncloud/index.php/f/$fileid That URL will automatically redirect to the file list with the correct file/directory name depending on the currently logged in user.

See https://github.com/owncloud/core/pull/24434

butonic commented 8 years ago

Currently all search results are user specific and searching in shared files is not yet implemented in search_lucene. Resolving a user specific path is nod yet necessardy. Nevertheless the link generation suffered bitrot.

For files the link should point to the containing directory and scroll to the file. For directories, the link should just point to the directory. Or in php:

        if ($this->mime_type === 'httpd/unix-directory') {
            $this->link = \OC::$server->getURLGenerator()->linkToRoute(
                'files.view.index',
                [
                    'dir' => $this->path,
                ]
            );
        } else {
            $this->link = \OC::$server->getURLGenerator()->linkToRoute(
                'files.view.index',
                [
                    'dir' => dirname($this->path),
                    'scrollto' => $this->name,
                ]
            );
        }

@flaracca could you test with the above snippet? @PVince81 I don't like using the fileid in the url, because it leaks our internal ids ... see http://stackoverflow.com/a/12384744 and other answers in that thread. I'd like an opinion of @Peter-Prochaska on that.

peterprochaska commented 8 years ago

@butonic What are your security concerns with a fileID in the url? If we check the permissions correct... Sure, if there is another opportunity, then we should do that.

PVince81 commented 8 years ago

@butonic then we'd need to get rid of the permalinks feature completely, because that's how it works, see https://github.com/owncloud/core/pull/24434 which was approved before.

Also note that the internal fileid can be found by inspecting the "tr" rows in the file list.

The permissions are checked, yes. Only users who have access to that file will get a view. The check is done through a call that basically says "tell me which path this fileid has for the currently logged in user" (because the path can be different for share recipients). If the user has no access to the file, it gets a "Not found" error.

PVince81 commented 8 years ago

also: permalinks only work for logged in users

butonic commented 8 years ago

just call me paranoid. In the back of my mind a voice said 'don't expose internal ids or you will never get rid of them'. I tried to find a better explation, eg a blog article but could only come up with stackoverflow. ANd the answers are all about security concerns. AFAICT we will need to identify files by storage id and fileid ... but thats a different topic. Nevermind, lets focus on getting the right link here ...