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

Security fix: Added nonce check to CSV importer actions #221

Merged
merged 1 commit into from
May 2, 2020
Merged

Security fix: Added nonce check to CSV importer actions #221

merged 1 commit into from
May 2, 2020

Conversation

timbocode
Copy link
Contributor

woocommerce/woocommerce@cabf9de

See also #214

Files affected:
includes/admin/class-wc-admin-importers.php
includes/admin/importers/class-wc-product-csv-importer-controller.php

@timbocode timbocode added the Security Security fixes including WC backports label Feb 23, 2020
@timbocode timbocode added this to the 1.0.0-beta1 milestone Feb 23, 2020
@timbocode timbocode requested review from nylen and bahiirwa February 23, 2020 14:51
Copy link
Collaborator

@bahiirwa bahiirwa left a comment

Choose a reason for hiding this comment

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

Changes for email 3c35038 should not be in this PR. Could you remove 3c35038?

@codecov-io
Copy link

Codecov Report

Merging #221 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #221      +/-   ##
============================================
- Coverage     40.83%   40.82%   -0.01%     
  Complexity    13447    13447              
============================================
  Files           367      367              
  Lines         51275    51280       +5     
============================================
- Hits          20938    20936       -2     
- Misses        30337    30344       +7
Impacted Files Coverage Δ Complexity Δ
...rters/class-wc-product-csv-importer-controller.php 12.57% <0%> (-0.12%) 69 <0> (ø)
includes/admin/class-wc-admin-importers.php 0% <0%> (ø) 42 <0> (ø) ⬇️
...shipping/flat-rate/includes/settings-flat-rate.php 97.95% <0%> (-2.05%) 0% <0%> (ø)
includes/class-wc-tax.php 79.67% <0%> (-0.28%) 130% <0%> (ø)
includes/class-wc-update-client.php 0% <0%> (ø) 64% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6180699...662ae59. Read the comment docs.

@timbocode
Copy link
Contributor Author

timbocode commented Mar 2, 2020

Testing

  • Manually imported properly formated CSV file. OK.
  • Manually tested the CSV import using a badly formatted CSV file. It successfully rejected the file.

Copy link
Contributor

@nylen nylen left a comment

Choose a reason for hiding this comment

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

I compared the changes here against woocommerce/woocommerce@cabf9de and this looks good.

@timbocode timbocode requested a review from bahiirwa March 13, 2020 13:23
@timbocode
Copy link
Contributor Author

Changes for email 3c35038 should not be in this PR. Could you remove 3c35038?

@bahiirwa can you review please?

Copy link
Collaborator

@bahiirwa bahiirwa left a comment

Choose a reason for hiding this comment

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

Tested the branch locally and imported a number of assets using a dummy csv and txt. So all looks fine for user endpoint.

@timbocode timbocode merged commit 03a4f90 into ClassicPress:master May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Security Security fixes including WC backports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants