Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Send CSRF token in rawStat #1797

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Send CSRF token in rawStat #1797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions js/viewer-main.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/viewer-main.js.map
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion src/services/FileInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import { getClient } from './WebdavClient'
import { genFileInfo, type FileInfo } from '../utils/fileUtils'
import { createClient, type FileStat, type ResponseDataDetailed } from 'webdav'
import { getRequestToken } from '@nextcloud/auth'

const statData = `<?xml version="1.0"?>
<d:propfind xmlns:d="DAV:"
Expand Down Expand Up @@ -53,6 +54,8 @@

/**
* Retrieve the files list
* @param path

Check warning on line 57 in src/services/FileInfo.ts

View workflow job for this annotation

GitHub Actions / eslint 

Missing JSDoc @param "path" description
* @param options

Check warning on line 58 in src/services/FileInfo.ts

View workflow job for this annotation

GitHub Actions / eslint 

Missing JSDoc @param "options" description
*/
export default async function(path: string, options = {}): Promise<FileInfo> {
const response = await getClient().stat(path, Object.assign({
Expand All @@ -64,9 +67,12 @@

/**
* Retrieve the files list
* @param origin
* @param path
* @param options
*/
export async function rawStat(origin: string, path: string, options = {}) {
const response = await createClient(origin).stat(path, {
const response = await createClient(origin, { headers: { requesttoken: getRequestToken() || '' } }).stat(path, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

viewer/src/services/WebdavClient.ts

Line 30 in 2016523

: { headers: { requesttoken: getRequestToken() || '' } },

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this?
We already inject the token in the client, do you actually needs to set it again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This client only work with /dav/files endpoints, and I need it to work with other endpoints like /dav/files_versions.
Or am I missing something ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I saw the import { getClient } from './WebdavClient' and assumed you were using it as well, didn't realize the createClient.

The rawStat createClient is confusing. No other dav implementation in Viewer accept out-of-user-root paths
I didn't see that in the PR that added this, I would have blocked it otherwise.

Viewer does NOT support files outside of the user root anyway ? Any reason you're not reusing our WebdavClient.ts then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Viewer need to be able to edit pictures from /dav/photos/user/albums and equivalent.
It will also need to be able display files' versions.

Any reason viewer should not support files outside of users' root ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason viewer should not support files outside of users' root ?

Not implemented.
We need to implement the entire Viewer using the File/Folder new standard.
That way we'll know what to expect.

Let's leave it like that, it's a bit hacky but t works™ :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason viewer should not support files outside of users' root ?

Inconsistency in what the backend supports (dav properties)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will also need to be able display files' versions.

Well, let's have a call about this 🙈

...options,
data: statData,
details: true,
Expand Down