fix item search entity restrict - #25217
Open
Megachip wants to merge 8 commits into
Open
Conversation
#BUG On location view, I can assign documents to a location, on document view there is no option to choose a location
Added tests for entity restrictions on ITIL items based on recursion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist before requesting a review
Description
Fixes an inconsistency where a recursive Ticket/Change/Problem cannot be
linked, via the item search dropdowns, to an asset located in one of its
sub-entities — even though the rights model already allows that exact
link.
Root cause
CommonDBRelation::can()'s entity coherency check(
src/CommonDBRelation.php) allows linking two items when eitherextremity is recursive over an ancestor entity of the other:
So a recursive Ticket in entity
Rootcan already be linked to aNetworkEquipmentin a child entityRoot > Site A—can()returnstrue.However,
CommonItilObject_Item::displayItemAddForm()passed the ITILobject's own single entity (
$obj->getEntityID()) as theentity_restrictfor the item search dropdowns(
dropdownAllDevices()/dropdownMyDevices()), with no downwardexpansion for recursive objects. Down the call chain,
DbUtils::getEntitiesRestrictCriteria()only expands upward(ancestors of the given entity) when the item being searched is
recursive — it never expands to the ITIL object's descendant
entities. For a Root-entity ticket, ancestors of Root are empty, so the
search criteria collapses to
entities_id = 0: assets in any childentity never appear as candidates, regardless of the ticket's own
recursivity.
Net effect: users had to split one logical ticket/change into N
per-entity tickets, purely because the search UI didn't surface
candidates the data layer already allowed linking to.
Fix
displayItemAddForm()now computes the search's entity scope as theobject's own entity plus all of its descendants (
getSonsOf())when the object
isRecursive(), and leaves it unchanged (singleentity) otherwise. This only widens the candidate pool;
Session::getMatchingActiveEntities()— applied inajax/dropdownTrackingDeviceType.phpandDropdown::getDropdownFindNum()— still narrows the actual result towhichever entities are active in the current user's session, exactly
as for every other entity-restricted search in GLPI. A technician
still needs "Root + sub-entities" selected in the entity switcher to
see cross-entity results; this change doesn't grant new visibility, it
only stops hiding what the coherency check already permits.
Applies uniformly to
Item_Ticket,Item_ChangeandItem_Problem,since all three share
CommonItilObject_Item.Related prior art
#22851 removed a comparable, overly-strict entity restriction for
linked ITIL-to-ITIL objects (originally introduced by #10989), for the
same underlying reason: the restriction didn't match what the rights
model already allowed. That PR also flagged that session-based entity
filtering doesn't work in contexts without a "current user" (e.g.
notification rendering) — not applicable here, since this codepath is
only reached interactively from the item-search AJAX endpoint, which
always has a real session.
Tests added
testItemAddFormEntityRestrictOnRecursiveItil(inAbstractCommonItilObject_ItemTest, so it runs for Ticket, Change andProblem): a recursive ITIL object's item search must include both its
own entity and a child entity.
testItemAddFormEntityRestrictOnNonRecursiveItil: a non-recursiveITIL object's item search must stay restricted to its own entity only.