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

Meteor method permissions fixes #4883

Merged
merged 5 commits into from
Dec 19, 2018
Merged

Meteor method permissions fixes #4883

merged 5 commits into from
Dec 19, 2018

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Dec 17, 2018

Impact: minor
Type: bugfix

Issue

  • "discounts/deleteRate" method did not prevent you from deleting a discount code belonging to another shop as long as you had permission for your own shop.
  • "discounts/editCode" method did not prevent you from editing a discount code belonging to another shop as long as you had permission for your own shop. Additionally, it checked "discount-codes" permission while other methods checked "discounts" permission.
  • "discounts/addCode" method checked "discount-codes" permission while other methods checked "discounts" permission.
  • "discounts/editRate" method was unused.
  • "discounts/setRate" method was unused.
  • "taxes/deleteRate" method did not prevent you from deleting a tax rate belonging to another shop as long as you had permission for your own shop.
  • "taxes/editRate" method did not prevent you from editing a tax rate belonging to another shop as long as you had permission for your own shop.
  • "discounts/codes/apply" method allowed anyone to attach a discount code to a cart as long as they had enough information
  • "discounts/codes/remove" method allowed anyone to remove a discount code from a cart as long as they had enough information
  • "emails/retryFailed" method would allow you to trigger retry of a failed email if you had "reaction-email" permission for any shop.
  • The "shipping/rates/*" methods were listed as deleted in the 2.0 RC1 release notes, but for some reason they're back, but still unused.

Solution

  • "discounts/deleteRate" method is renamed to "discounts/deleteCode" and now ensures that you can't remove a discount owned by another shop
  • "discounts/editCode" method now checks "discounts" rather than "discount-codes" permission and ensures that you can't remove a discount owned by another shop
  • "discounts/addCode" method now checks "discounts" rather than "discount-codes" permission
  • "discounts/editRate" method is removed
  • "discounts/setRate" method is removed
  • "taxes/deleteRate" method now ensures that you can't remove a tax rate owned by another shop
  • "taxes/editRate" method now ensures that you can't edit a tax rate owned by another shop
  • "discounts/codes/apply" method now requires that you are the cart owner or pass a token for an anonymous cart, or that you have "discounts" permission for the cart shop
  • "discounts/codes/remove" method now requires that you are the cart owner or pass a token for an anonymous cart, or that you have "discounts" permission for the cart shop
  • "emails/retryFailed" method will allow you to trigger retry of a failed email if you have "reaction-email" permission for the primary shop.
  • The "shipping/rates/*" methods are deleted again

Breaking changes

Custom code that relies on the removed Meteor methods must be rewritten.

Testing

  1. Verify the described permission changes
  2. Ensure that you're still able to add, edit, and delete discount codes and tax rates as an admin.
  3. Verify that you can still add and remove a discount code during checkout when logged in as non-admin and when not logged in.

@aldeed aldeed changed the title [WIP] Meteor method permissions fixes Meteor method permissions fixes Dec 17, 2018
@aldeed aldeed self-assigned this Dec 17, 2018
@aldeed aldeed added this to the 🏔 Shavano milestone Dec 17, 2018
@aldeed aldeed requested a review from kieckhafer December 17, 2018 21:54
@@ -38,9 +39,6 @@ export default class DiscountForm extends Component {
}
});
}, 800);

this.handleChange = this.handleChange.bind(this);
this.handleClick = this.handleClick.bind(this);
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@kieckhafer kieckhafer left a comment

Choose a reason for hiding this comment

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

Verified all changes, code looks good 👍

@kieckhafer kieckhafer merged commit 70e018c into develop Dec 19, 2018
@kieckhafer kieckhafer deleted the fix-aldeed-security-4 branch December 19, 2018 05:47
@spencern spencern mentioned this pull request Jan 18, 2019
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