[BUU] Add missing permission check on product actions #12868
Conversation
Plus request spec
rioug
added
the
user facing changes
|
See result of the test below :
Updated the clone link to point to id 27 which the user is not allowed to managed
Result in 401 error in the console |
There was a problem hiding this comment.
Great, it's wonderful to have a request spec for products_v3_controller.
Hopefully next time we work on these we can add some specs for success cases also.
It's worth noting that these specs don't check if the actions themselves weren't prevented (eg the record could still get deleted and we wouldn't know). But at least we're testing more than before.
| include AuthenticationHelper | ||
|
|
||
| let(:user) { create(:user) } | ||
| let(:headers) { { Accept: "text/vnd.turbo-stream.html" } } |
There was a problem hiding this comment.
Hmm, it would be nice if the turbo gem gave us shortcuts like it does for controller specs (format: :turbo_stream)
| end | ||
| end | ||
|
|
||
| describe "DELETE /admin/product_v3/destroy_variant/:id" do |
There was a problem hiding this comment.
Hum, I think destroy_variant should have been on a variant controller.. but that's nothing to do with this PR of course.
|
|
||
| expect(response).to redirect_to('/unauthorized') | ||
| end | ||
| end |
There was a problem hiding this comment.
I notice that we're not checking that the product wasn't actually deleted.
|
I think there's no need for further dev testing ✅ I would say it's "Technical changes only", because it's not something a user can see without hacking. |
sigmundpetersen
changed the title
dacook
added
technical changes only
dacook
changed the title
What? Why?
Some of the Bulk product edit page actions where missing permission check, so someone could potentially delete product they are not managing. This PR add the missing permission checks
NOTE, this PR #12867 will make unauthorised error more obvious to the user
What should we test?
This should probably be tested by a dev, as it requires use of the developer tools
As an enterprise user :
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates