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

fix: pass csrf token on doi/manuscript fetch and clean up withCredentials #1281

Merged
merged 3 commits into from
Jul 8, 2024

Conversation

jaredgalanis
Copy link
Contributor

No description provided.

- withCredentials is not part of the fetch api and was just being ignored
@jaredgalanis jaredgalanis changed the title fix: pass csrf token on doi/manuscript fetch fix: pass csrf token on doi/manuscript fetch and clean up withCredentials Jul 5, 2024
@jaredgalanis jaredgalanis requested a review from markpatton July 5, 2024 20:20
@@ -13,7 +13,6 @@ export default class ApplicationAdapter extends JSONAPIAdapter {

get headers() {
return {
withCredentials: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@markpatton withCredentials doesn't appear to be part of the fetch api. credentials is, but we are explicitly passing the csrf token here.

@@ -28,7 +28,13 @@ export default class OAManuscriptService extends Service {

const url = `${this.lookUpPath}?doi=${doi}`;

return fetch(url, { method: 'GET' })
return fetch(url, {
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 one was missing the csrf token and was being spiked by the DOI service.

Copy link
Contributor

@markpatton markpatton left a comment

Choose a reason for hiding this comment

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

Looks good. Ok. So the consequence of this is that the manuscript file fetch will just not return files until this fix is present. I think I had forgotten that functionality existed.

I tried it out and did get a 200 back from the service. But strangely the ui did not say a pdf was available despite a url I tried in the returned json resolving to a pdf. This was the json:

{
  "manuscripts": [
    {
      "url": "https://bmchealthservres.biomedcentral.com/track/pdf/10.1186/s12913-020-05409-w",
      "repositoryLabel": null,
      "type": "application/pdf",
      "source": "Unpaywall",
      "name": "s12913-020-05409-w",
      "isBest": true
    },
    {
      "url": null,
      "repositoryLabel": null,
      "type": "application/pdf",
      "source": "Unpaywall",
      "name": null,
      "isBest": false
    },
    {
      "url": "https://www.researchsquare.com/article/rs-19757/v1.pdf",
      "repositoryLabel": null,
      "type": "application/pdf",
      "source": "Unpaywall",
      "name": "v1.pdf",
      "isBest": false
    },
    {
      "url": "https://www.researchsquare.com/article/rs-19757/v2.pdf?c=1631848933000",
      "repositoryLabel": null,
      "type": "application/pdf",
      "source": "Unpaywall",
      "name": "v2.pdf?c=1631848933000",
      "isBest": false
    }
  ]
}

@jaredgalanis jaredgalanis merged commit 9c34079 into main Jul 8, 2024
4 checks passed
@jaredgalanis jaredgalanis deleted the fix-pass-csrf-token-on-doi/manuscript-fetch branch July 8, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants