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
261 stars 75 forks source link

Can Kernel/Autoload/Znuny4OTOBOPasswordPolicy.pm be removed? #987

Closed bschmalhofer closed 1 year ago

bschmalhofer commented 3 years ago

This module overrides Kernel::Output::HTML::Layout::Redirect() such that specific Redirects are disabled. This looks like a workaround that was necessary because Znuny4OTRS-PasswordPolicy could not modify OTRS code. But this should be no problem in OTOBO. So my guess is that https://github.com/znuny/Znuny4OTRS-PasswordPolicy/commit/d46b6ef7fefacf92cdd86800d639e1d16f77aafb would alleviate the need for the Autoload module.

@rolfschmidt: what do you think ?

rolfschmidt commented 3 years ago

Hi @bschmalhofer ,

i'm not involved in your project but i would guess that you want to increase the code quality of your project so i would agree to that it would make sense to rework the code in your framework to remove this autoload module.

Maybe you ask @svenoe or the maintainer of this project how to proceed. I don't think i'm the right person to make code design decisions here, sorry.

svenoe commented 3 years ago

Hi to both of you,

yes, there is plenty of stuff which needs to be incorporated properly. Currently this is just a time issue, and as this is running as intended...

bschmalhofer commented 3 years ago

The Autoload module was not created as a workaround for core OTRS code, but as a workaround for Kernel/Modules/AgentTimeAccounting in the OTRS package TimeAccounting. The history of that module is available, via the kind folks at Znuny, at https://github.com/znuny/TimeAccounting. There already is an OTOBO port of TimeAccounting available at https://github.com/RotherOSS/TimeAccounting.

So, I propose the following:

The required change would be something like:

    # ignore pre application module if it is calling self
    return
        if $Self->{Action} =~ /^(CustomerPassword|AgentPassword|AdminPackage|AdminSystemConfiguration|CustomerAccept)/;

as seen in https://github.com/RotherOSS/otobo/blob/rel-10_1/Kernel/Modules/BasePassword.pm.

@svenoe : does that make sense ? An alternative might be to integrate TimeAccounting into core OTOBO 10.1.x .

bschmalhofer commented 1 year ago

I have to admit that I don't really understand why the special case for AgentTimeAccountingEdit is needed. But the code for https://github.com/RotherOSS/TimeAccounting/ can easily be adapted in case such a workaround is still needed. So let's remove the workaround in OTOBO 11.

bschmalhofer commented 1 year ago

Removed the workaround. Closing this issue.