RotherOSS / otobo

OTOBO is one of the most flexible web-based ticketing systems used for Customer Service, Help Desk, IT Service Management. https://otobo.io/
GNU General Public License v3.0
263 stars 75 forks source link

ACL: Owner in ConfigChange doesn't check for a unique value #2200

Open StefanAbel-OTOBO opened 1 year ago

StefanAbel-OTOBO commented 1 year ago

When you create an ACL where you try to hide (or show) an Owner, you use: Possible(Not|Add)? => Ticket => Owner => your value Unfortunately the filter you use here, only works with the agent's:

and not with:

So in order to hide the root@localhost (which is named by default "- -") you have to create an ACL: PossibleNot => Ticket => Owner => - -

image

This might cause problems, as in several companies there are people with the exact same first and lastname combination - and of course it is not very intuitive to use.

stefanhaerter commented 1 year ago

As far as I understand it, the cause is rooted deeply within Kernel/System/Ticket/TicketACL.pm. The data, e.g. owner data, is present as a hash like 1 => '- -', 2 => someusername, and the comparison is done only on the keys, not on the values. My suggestion would be to add a check on the keys also and evaluate both, but since I think this changes a very sensible part of OTOBO, I would like to have @svenoe 's approval on this first.

Relevant code (e.g.):

https://github.com/RotherOSS/otobo/blob/45b8b2849bde18271e7d7e6e6eb2ebdb3b10c852/Kernel/System/Ticket/TicketACL.pm#L872-L876

stefanhaerter commented 1 year ago

Addendum: Also some code in the modules would have to be adjusted, e.g. here:

https://github.com/RotherOSS/otobo/blob/45b8b2849bde18271e7d7e6e6eb2ebdb3b10c852/Kernel/Modules/AgentTicketPhone.pm#L2478-L2485

I think result subtypes Owner and OwnerID would have to be checked. If there are ACLs for both, would they need to be merged somehow?

StefanAbel-OTOBO commented 1 year ago

As far as I understand it, the cause is rooted deeply within Kernel/System/Ticket/TicketACL.pm. The data, e.g. owner data, is present as a hash like 1 => '- -', 2 => someusername, and the comparison is done only on the keys, not on the values. My suggestion would be to add a check on the keys also and evaluate both, but since I think this changes a very sensible part of OTOBO, I would like to have @svenoe 's approval on this first.

I just used root@localhost as an example. Using the username/login for any other user doesn't work here as well.

https://github.com/RotherOSS/otobo/issues/2200#issuecomment-1448201650>I think result subtypes Owner and OwnerID would have to be checked. If there are ACLs for both, would they need to be merged somehow?

Just to make sure (as there was some confusion beforehand), that my issue is not concerning the ConfigMatch-part of an ACL, but about the ConfigChange-part. In ConfigMatch. I tested it with OwnerID, which doesn't work (yet).

stefanhaerter commented 1 year ago

I just used root@localhost as an example. Using the username/login for any other user doesn't work here as well.

The usage of '- -' as described in your initial report indicates that the ticket owner attribute holds the first name and last name of the user when it is evaluated by the ACLs. The user login attribute (e.g. root@localhost) could be evaluated as well, but based on my current knowledge of the ACL mechanism I would see this as a non-trivial enhancement.

Just to make sure (as there was some confusion beforehand), that my issue is not concerning the ConfigMatch-part of an ACL, but about the ConfigChange-part. In ConfigMatch. I tested it with OwnerID, which doesn't work (yet).

Yes, my investigation was on the ConfigChange part also. The first code fragment is about filtering the content for the dropdown which is then displayed later on.

I hope my explanations are somehow comprehensible - if not, don't hesitate to ask me.

stefanhaerter commented 4 months ago

Duplicate of #1453. Closing the earlier issue as this one holds more information.