- 🇹🇭Thailand AlfTheCat
This looks like a bug to me.
Steps to reproduce:
- Create a view with product variations
- Unpublish a product variation shown in the view
- Access the view as anonymous user and you will see that the unpublished variation shows up.
In my case, the web store displays variations, not products. So this is a big problem.
- First commit to issue fork.
- Status changed to Needs review
2 months ago 2:28pm 3 September 2024 - 🇩🇪Germany tgauges
I created an issue fork with the existing patch applied. The tests seem fine. I'm currently in the process of testing the code in a project.
- 🇩🇪Germany tgauges
I tested the changes against version 2.37 of the module and it works as expected. One could argue that this is not really a bug and merging this change would amount to a breaking change. In this case this issue lends itself to version 3 of this module, but that is for the maintainer to decide.
- Status changed to Needs work
2 months ago 9:40am 12 September 2024 - 🇩🇪Germany Anybody Porta Westfalica
@tgauges thank you, great work!
Tests are failing and it looks related:
Drupal\Tests\commerce_product\Kernel\ProductVariationStorage 0 passes 1 fails
- Status changed to Needs review
2 months ago 2:31pm 12 September 2024 - 🇩🇪Germany tgauges
I fixed the relevant test but now another unrelated (?) test is failing. Could you take a look at it?
- 🇩🇪Germany Anybody Porta Westfalica
@tgauges the MR targets the wrong branch! It needs to target 3.0.x-dev! I can't change that!
- 🇩🇪Germany Anybody Porta Westfalica
PS: Maybe you could alternatively create a separate MR against 3.0.x-dev so the other one is kept for 2.x?
- 🇩🇪Germany Anybody Porta Westfalica
Whao, @tgauges you just saved my day. Some weeks ago I came here for a minor impact, but now I found it's major and you MR fixes it!
We have a product with several variations. To switch them, we're using rendered entity display (swatches).
Now the client reported, that even disabled product variations are visible to end-users! And he's correct.Below some screenshots as logged out user - both after clearing caches.
Without patch:
With patch:
So I think this is at least a major, if not even a critical bug, if you'd rate this as information exposure (while I don't think product variations typically contain highly sensitive information)
But I think this needs to be fixed asap and maybe even in 2.x? At least definitely in 3.0.x
- Merge request !361Fix Unpublished variations are not properly checked for access → (Open) created by Anybody
- 🇩🇪Germany Anybody Porta Westfalica
Created MR!361 against 3.0.x-dev. Hopefully the tests will pass there :)
I couldn't find any unexpected side-effects yet, but of course, access-related changes always need to be done carefully. Thank you for the tests!
- 🇩🇪Germany Anybody Porta Westfalica
@tgauges test fails, would you have a final look?
- 🇩🇪Germany Anybody Porta Westfalica
Great work @tgauges thank you!
I think @jsacksick should now have a look and decide if this should be backported to 2.x or just go into 3.0.x-dev.
- 🇩🇪Germany Anybody Porta Westfalica
As MR!361 applies on 2.0.x and 3.0.x I'm now hiding MR !327.
Sadly I have to put this back to "Needs review", as I found a case, where it doesn't seem to work as expected for me, but I didn't find the root cause yet, why it works in some products and doesn't work in others (unpublished variations are shown).
- 🇩🇪Germany Anybody Porta Westfalica
Uploading MR!361 as static patch for composer until this is resolved. I'd vote to also commit this fix to 2.0.x.
https://www.drupal.org/files/issues/2024-11-04/commerce-unpublished_vari... → works both for 3.0.x and 2.0.x (and 2.40.0) currently!