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.

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

Send CSRF token in rawStat #1797

Merged

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Jul 10, 2023

Without it, the request fails.
Not sure why it wasn't an issue during my initial testing.

Follow up of #1709

Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge force-pushed the artonge/fix/send_csrf_token_when_fetching_updated_node branch from c06090d to 2016523 Compare July 10, 2023 13:36
@artonge artonge self-assigned this Jul 10, 2023
@artonge artonge added 3. to review Waiting for reviews bug Something isn't working labels Jul 10, 2023
@artonge
Copy link
Contributor Author

artonge commented Jul 10, 2023

/backport to stable27

@artonge
Copy link
Contributor Author

artonge commented Jul 10, 2023

/backport to stable26

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Jul 10, 2023
@artonge
Copy link
Contributor Author

artonge commented Jul 10, 2023

/backport to stable25

@artonge artonge added the javascript Javascript related ticket label Jul 10, 2023
@artonge artonge added this to the Nextcloud 28 milestone Jul 10, 2023
@artonge artonge requested a review from skjnldsv July 10, 2023 16:07
skjnldsv
skjnldsv previously approved these changes Jul 10, 2023
*/
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.

: { 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 🙈

@skjnldsv skjnldsv dismissed their stale review July 10, 2023 18:00

see comment

@artonge artonge merged commit 6f0f8f2 into master Jul 10, 2023
27 checks passed
@artonge artonge deleted the artonge/fix/send_csrf_token_when_fetching_updated_node branch July 10, 2023 20:55
@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable25
git pull origin stable25

# Create the new backport branch
git checkout -b fix/foo-stable25

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews backport-request Pending backport by the backport-bot bug Something isn't working javascript Javascript related ticket
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants