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

[Port dspace-7_x] Ensures CSRF token is initialized prior to first modifying (non-GET) request #3063

Open
wants to merge 5 commits into
base: dspace-7_x
Choose a base branch
from

Conversation

tdonohue
Copy link
Member

@tdonohue tdonohue commented May 17, 2024

@tdonohue
Copy link
Member Author

NOTE: The e2e test failures in this PR are the result of the Backend PR (DSpace/DSpace#9599) not yet being merged. I've verified locally (via yarn e2e) that all e2e tests succeed when run against the Backend PR.

@tdonohue
Copy link
Member Author

tdonohue commented May 17, 2024

👍 I've tested this locally alongside DSpace/DSpace#9599 and verified that this functionality is successfully backported. Namely:

  • On first request to the homepage, the POST /server/api/statistics/viewevents call always succeeds now. Previously it would often fail because the CSRF token would not yet be initialized.
  • When that first request occurs, I can verify (from the backend dspace.log) that the new /api/security/csrf endpoint was called to obtain the initial token. In that scenario, you'll see this in the logs:
    2024-05-17 16:47:45,267 INFO  unknown 373bb807-3346-4813-8120-bc6942b629cc org.dspace.app.rest.utils.DSpaceAPIRequestLoggingFilter @ Before request [GET /server/api/security/csrf] originated from /
    

Copy link

Hi @tdonohue,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

@tdonohue
Copy link
Member Author

I rebased this on dspace-7_x after a very minor merge conflict with latest dspace-7_x (in a edit-relationship-list.component.spec.ts).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge bug component: statistics high priority
Projects
Status: 👍 Reviewer Approved
Development

Successfully merging this pull request may close these issues.

None yet

3 participants