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

Update sendRequest for axios to align on parameter deprecation (withCredentials) #2063

Closed
evgarthub opened this issue Mar 13, 2024 · 7 comments · Fixed by #2067
Closed

Update sendRequest for axios to align on parameter deprecation (withCredentials) #2063

evgarthub opened this issue Mar 13, 2024 · 7 comments · Fixed by #2067
Assignees

Comments

@evgarthub
Copy link
Collaborator

evgarthub commented Mar 13, 2024

Hi @ferdikoomen!

Axios fixed vulnerability related to withCredentials request parameter by introducing new parameter withXSRFToken, details here:
axios/axios#6046
GHSA-wf5p-g6vw-rhxx

Does it make sense to include new parameter next to withCredentials as author suggests?

We verified internally that indeed passing both parameters with true value prevents client from behaving differently after axios update.
image

I will quickly create PR for it, let me know if it does make sense?

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 13, 2024

@evgarthub I think we'd want a separate parameter for this. It's a breaking change in Axios, therefore it should be a breaking change in this package, too.

@evgarthub
Copy link
Collaborator Author

@mrlubos do you mean we should extend config with WITH_XSRF_TOKEN?

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 13, 2024

Sounds like it. I'd need to dig more into it, but if this feature is only available in Axios, config should also have that flag only when using Axios client. That's what it looks like to me right now

@evgarthub evgarthub self-assigned this Mar 14, 2024
@evgarthub
Copy link
Collaborator Author

I'm not able to run any tests, or rather they are failing because of #2052 issue (I'm on Windows).

@evgarthub evgarthub linked a pull request Mar 14, 2024 that will close this issue
ferdikoomen added a commit that referenced this issue Mar 17, 2024
…-axios-to-align-on-parameter-deprecation-withcredentials

fix: updated axios sendRequest (#2063)
@jordanshatford
Copy link

@evgarthub The windows issue should be fixed in our fork https://github.com/hey-api/openapi-ts. We have also added support for interceptors, so if you would like to do something like this you can see examples in this ticket:

hey-api/openapi-ts#76

@evgarthub
Copy link
Collaborator Author

Hi @jordanshatford ! Thanks for offering a solution! But it's already been fixed in this repo.

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 28, 2024

@evgarthub yes, except it isn't actively maintained and still has many other bugs that are fixed in openapi-ts #2064

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 a pull request may close this issue.

3 participants