Skip to content

IBX-9838: FIX Edit option in three-dot menu remains clickable despite insufficient user permissions #1525

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

Merged
merged 4 commits into from
Jul 23, 2025

Conversation

Sztig
Copy link
Contributor

@Sztig Sztig commented Apr 15, 2025

🎫 Issue IBX-9838

Description:

As stated in the public ticket, when the user has a limitation, for example, only for a specific content type, lets say Article, if we go to the Content tree, click on any other content type and open the context menu, the edit button would be clickable and it would throw a permission error.
I'm fixing here the edit check to rely on this->permissionResolver->canUser instead of hasAccess method.

@Sztig Sztig requested a review from a team April 15, 2025 13:29
@konradoboza konradoboza requested a review from a team April 15, 2025 13:34
Copy link

@konradoboza konradoboza requested a review from a team April 15, 2025 13:46
@@ -226,7 +226,7 @@ private function getLocationPermissionRestrictions(Location $location): array
'restrictedLanguageCodes' => $createLimitationsValues[Limitation::LANGUAGE],
],
'edit' => [
'hasAccess' => $lookupUpdateLimitationsResult->hasAccess(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you checked other uses of this method? If it does not work correctly, it should be fixed, or all usage should be replaced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to investigate this one, we have another variable lookupUpdateLimitationsResult thats also not working correctly here, I will come back here once I have done some test.

return $this->permissionResolver->canUser(
'content',
'edit',
$content
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need to pass any targets here as a fourth argument?

@Sztig
Copy link
Contributor Author

Sztig commented May 16, 2025

In $lookupCreateLimitationsResult = $this->permissionChecker->getContentCreateLimitations($location) the getContentCreateLimitations method seems to be derpecated so I did some tests and went with a simplest implementation of canUser method. Passing the location on last arg to limit the check on the provided $location

@Sztig Sztig force-pushed the IBX-9838-ContentTreeController-fix branch from b1b6ab3 to 27182d9 Compare June 17, 2025 10:25
Copy link

@Sztig
Copy link
Contributor Author

Sztig commented Jun 17, 2025

Could I get re-reviews? I have changed the implementation slightly and I wouldn't want to push it to the QA without it.

@tomaszszopinski tomaszszopinski force-pushed the IBX-9838-ContentTreeController-fix branch from 27182d9 to 9d69b7a Compare July 23, 2025 08:37
@tomaszszopinski tomaszszopinski self-assigned this Jul 23, 2025
Copy link

Copy link
Contributor

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on Ibexa DXP 4.6 commerce.

@ViniTou ViniTou merged commit 6e8c4db into 4.6 Jul 23, 2025
28 checks passed
@ViniTou ViniTou deleted the IBX-9838-ContentTreeController-fix branch July 23, 2025 13:18
@Sztig
Copy link
Contributor Author

Sztig commented Jul 23, 2025

merged up 8113549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants