-
Notifications
You must be signed in to change notification settings - Fork 31
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
refactor(neon_http_client): use nextcloud csrf request #2502
refactor(neon_http_client): use nextcloud csrf request #2502
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2502 +/- ##
==========================================
+ Coverage 28.37% 28.41% +0.04%
==========================================
Files 352 352
Lines 135903 135903
==========================================
+ Hits 38559 38615 +56
+ Misses 97344 97288 -56
*This pull request uses carry forward flags. Click here to find out more.
|
Why are you splitting the logic for vm and web and basically making vm noop? IMO we should use the same logic regardless of the target. In this case it only increases code complexity while having not much benefits. |
We already had this split logic (it was just in a single class). I also find the unit tests more readable now that they are split. I can drop the commit if you prefer to keep it unified. |
Instead of having two implementations I would rather just not add it on vm. |
Signed-off-by: Nikolas Rimikis <[email protected]>
Signed-off-by: Nikolas Rimikis <[email protected]>
…nterceptor Signed-off-by: Nikolas Rimikis <[email protected]>
dbacc3e
to
62f8c50
Compare
I unified the csrf logic as previously discussed in our call. |
I'll change the webdav csrf interceptor (in the nc package) as a followup |
Signed-off-by: Nikolas Rimikis <[email protected]>
62f8c50
to
74fd815
Compare
some housekeeping to use and test the now documented csrf endpoint