-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Surchages query missing permission validation #6634
Comments
Hi @tedraykov, I would like to work on this. |
You can take onto the issue. In order to add a new role, you need to add a new migration in the authorization plugin as so: You also have to update the migration in the following repo: |
@tedraykov Need some help here. I'm getting this error while starting dev server with |
@tedraykov Never mind, I have fixed the above error. But there seems to be a new one |
@tedraykov Do I have to write any additional tests? I ran |
You should write two additional tests. One that it works when you have the correct permissions (which might be covered by existing tests if you changed them) and one that it fails when you don't have the correct permissions |
@zenweasel Okay, so if I got this right, It's for
|
@tedraykov Can we confirm that this query without permissions is not used anywhere in the storefront by customer-facing code @Vinitvh Yes that seems right. Not sure if this particular query returns a cursor or not though |
I checked that. The only public access to the surcharges are trough the cart, but the cart resolvers does not use this query but rather directly fetch the data from the database. |
Prerequisites
Issue Description
The surcharges query in
api-plugin-surcharges
is missing theread
permission validation.reaction/packages/api-plugin-surcharges/src/queries/surcharges.js
Line 15 in b11e47f
This means every user can query the surcharges regardless the permission they have.
Possible Solution
An example of a query that has the desired permission validation.
reaction/packages/api-plugin-accounts/src/queries/groups.js
Line 14 in b11e47f
The text was updated successfully, but these errors were encountered: