- Issue created by @thomas.frobieter
- 🇩🇪Germany Anybody Porta Westfalica
Thanks, I'll try to reproduce and fix this asap.
The 'administer user_dashboard presets' permission is required
Looks like there's a bug in the copy process from the preset to the individual dashboard, when the first portlet is placed.
Background: Until the first saved modification we're showing the presets and only create a copy if really modified.
- Status changed to Needs work
over 1 year ago 11:38am 10 November 2023 - 🇩🇪Germany Anybody Porta Westfalica
The attached MR fixes the issue, but is NOT ready to be reviewed or merged! There's a high risk of introducing bugs and security risks this way. I need to review the logics myself again and we need to ensure this works correctly for personalizing:
1. Homebox Presets
2. Admin Homeboxes of someone who also created a preset
3. Individual HomeboxesFurthermore I noticed a user without view any homebox permission is able to access the homebox on the user profile of other users via the user tab, if he has permission to access the user profiles. That might be a different issue or caused by this change. Also needs to be tested.
- Assigned to Grevil
- 🇩🇪Germany Anybody Porta Westfalica
@Grevil assigning this to you for a detailed look and test, if I can't make it due to business. But we have to check this very carefully. Please read the issue and the comments in detail.
If possible, I'll try to fix it, if I have the time.
- 🇩🇪Germany Grevil
Furthermore, I noticed a user without view any homebox permission is able to access the homebox on the user profile of other users via the user tab.
Ouch, VERY good find. This is indeed reproducible WITHOUT the patch applied here. Not only can the user view other peoples homeboxes through their user tab homebox link, BUT if he also has the "Personalize own Homebox" (but not the "Personalize any Homebox") permission, he can even edit other peoples homeboxes!
This is only the case, when accessing the homebox through their user tab. Trying to access their homeboxes through their dedicated homebox page, leads to an error, as expected.
Creating a major follow up issue.
- 🇩🇪Germany Grevil
The test pipeline is being skipped for some reason, removing the "Draft" part.
- last update
about 1 year ago Checkout Error - 🇩🇪Germany Grevil
Ok, marked it as ready, and the tests are running now. One of them should fail now, which will get fixed in 🐛 Another users Homebox can be accessed and modified through their Homebox user tab, without having the View / Personalize any Homebox permission Active .
We still need to test the preset properly here. We might actually already test the preset, since if the user has no changes on their Homebox it uses the preset? So we should actually check the access on their "real" homebox instances.
- 🇩🇪Germany Grevil
Ok the phpunit job is still not executed, maybe once we merged the gitlab-ci first.
- last update
about 1 year ago Checkout Error - last update
about 1 year ago Checkout Error - last update
about 1 year ago Checkout Error - last update
about 1 year ago Checkout Error - last update
about 1 year ago Checkout Error - last update
about 1 year ago Checkout Error - Issue was unassigned.
- Status changed to Needs review
about 1 year ago 3:09pm 5 April 2024 - 🇩🇪Germany Grevil
All done, please review!
Note, that this could need further testing, I'd say, let's commit it to dev and I'll do some further testing on monday.
- Assigned to Grevil
- Status changed to Needs work
about 1 year ago 3:40pm 5 April 2024 - 🇩🇪Germany Anybody Porta Westfalica
@Grevil thank you!
I left some comments. These are not the kind of comments that we should fix rapidly, but most of them should be done and tests with care.
So I'd say we should not merge or at least close or even leave this, unless we're 100% safe this is DONE DONE.
These changes are too complex to come back later. So feel free to merge it for further testing, if really needed, otherwise I think it might be better to test based on this MR first and as much as possible.
- last update
about 1 year ago Checkout Error - last update
about 1 year ago Checkout Error - 🇩🇪Germany Grevil
Alright, I tested this quite a bit through manual testing and added a few more tests, to verify this! I'll finally add some final tests, to test the actual
homebox.personalize.portlet_add
route (which is the main change done through this issue, so it obviously should be tested as well). - 🇩🇪Germany Grevil
Alright, added appropriate tests for the add portlet (ajax) route!! I also duplicated a few tests and created a homebox instance for each user at the start of each duplicated test, so we can make sure the access checks are identical when the users don't have their own homebox vs. when their own homebox instances exist.
One last major test would be, to modify a new homebox and see if the preset has changed, but this should be done in a follow up issue. After that, all major security implications should be resolved.
- Status changed to RTBC
about 1 year ago 11:37am 8 April 2024 - 🇩🇪Germany Anybody Porta Westfalica
Ok, I'm fine with RTBC once we have the relevant user-facing tests! :)
Just reset if you're changing further code (excl. tests)
- Issue was unassigned.
- 🇩🇪Germany Grevil
Sorry, did not see your comment!
Waiting for tests to become green.
- Status changed to Fixed
about 1 year ago 12:23pm 8 April 2024 Automatically closed - issue fixed for 2 weeks with no activity.