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

Don't protect static assets from XSRF #592

Merged
merged 6 commits into from
May 8, 2024
Merged

Conversation

minrk
Copy link
Contributor

@minrk minrk commented May 3, 2024

JupyterHub 4.1 applies XSRF checks to authenticated GET requests, which is not necessary for static assets. It would be a valid alternative to not authenticate these requests.

This solves the static asset request, described in jupyterhub/jupyterhub#4800

The userprofile request must be addressed in https://github.com/cylc/cylc-ui

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • No dependency changes
  • Tests are not needed for this
  • CHANGES.md entry included if this is a change that can affect users
  • cylc-doc PR not needed
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

minrk added 2 commits May 3, 2024 10:41
JupyterHub 4.1 applies XSRF checks on authenticated GET requests by default
@MetRonnie
Copy link
Member

@oliver-sanders I think this should be on 1.4.5 milestone?

@oliver-sanders oliver-sanders modified the milestones: 1.5.0, 1.4.5 May 3, 2024
@oliver-sanders
Copy link
Member

(we will probs skip 1.4.5 since we are near ready for 1.5.0 release but we can merge it through as normal)

@minrk
Copy link
Contributor Author

minrk commented May 3, 2024

I added one more commit based on jupyterhub/jupyterhub#4800 (comment) to ensure the xsrf cookie is set on GET /cylc/

cylc/uiserver/handlers.py Outdated Show resolved Hide resolved
Co-authored-by: Ronnie Dutta <[email protected]>
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

🚀 Tested with JupyterHub 4.1.5 and 4.0.2

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Makes sense, happy removing xsrf checks for static resources, tested with JupyterHub 4.1.5, couldn't force a page load error. Many thanks!

@oliver-sanders oliver-sanders merged commit 399ea47 into cylc:1.4.x May 8, 2024
4 of 5 checks passed
@MetRonnie MetRonnie modified the milestones: 1.4.5, 1.5.0 Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants