Find another solution for skipping onBeforeControllerAction

[buddh4]

https://github.com/humhub-contrib/legal/blob/master/Events.php#L95 is very hacky and there is no way of adding other modules to the skip list. Also there is no information why some of those modules are even skipped here. I'am not sure whats the best way to solve this, but I'am sure there will be 100 other modules listed here in few month. We should think about in which cases we need to skip this and maybe find another way around, e.g. some kind of controller flag or maybe trigger an (non legal module) event in which other modules can prevent request abortion in such cases.

[buddh4]

Maybe also filter for full page reload or pjax requests? Perhaps this would remove the need to block e.g. poll and mail. Regarding the rest module it maybe would be nice if the rest controller would implement a RestController marker interface provided by the core.

[ArchBlood]

Maybe deal with this through behaviors?

https://stackoverflow.com/questions/27180059/execute-my-code-before-any-action-of-any-controller

Another option, taken from the legal module

if ($event->action->controller->module->id === Yii::$app->getModule($moduleId)) {
            return;
        }

Only issue that I see is getting $moduleId I myself wouldn't know how to do this part

[yurabakhtin]

@buddh4 I think in order to replace the code https://github.com/humhub-contrib/legal/blob/master/Events.php#L88-L103:

if ($event->action->controller->module->id === 'user' && $event->action->controller->id === 'account' && $event->action->id === 'delete') {
    return;
}
if ($event->action->controller->module->id === 'user' && $event->action->controller->id === 'auth') {
    return;
}
if ($event->action->controller->module->id === 'mail' && $event->action->controller->id === 'mail') {
    return;
}
if ($event->action->controller->id === 'poll') {
    return;
}
if ($event->action->controller->module->id === 'rest') {
    return;
}

we should implement new property array like public $allowedActions = []; for \humhub\components\Module, then move the conditions to proper modules, for example, two first conditions will be moved into the \humhub\modules\user\Module:

public $allowedActions = [
    ['account', 'delete'],
    'auth', // means action == 'index'
];

then we need to implement new method like \humhub\components\Controller::isAllowedAction($actionId) and call single if ($event->action->controller->isAllowedAction($event->action->id)) instead of all those conditions above. Also same we need this in all other similar event where we have a reidrect, e.g. in 2FA module we can replace the condition Yii::$app->getModule('twofa')->isTwofaCheckUrl() here https://github.com/humhub/humhub-modules-twofa/blob/master/Events.php#L57.

[buddh4]

@yurabakhtin How would you document the $allowedActions fields, what is its purpose considering the core does not know anything about the legal module?

Maybe such an array should rather be moved to the legal module itself, and at least can be extend it by configuration this way. But as I understand the general issue we have is that we have some modules intercepting the request of most (but not all) actions if a specific condition is met. Here are some use cases where this could be the case:

• Maintenance mode
• Must change password
• Two factor auth
• Legal module
• Live poll
• Rest

All core controller/actions could be directly defined within the module e.g. by an similar array as you mentioned. Some use-cases above maybe could be solved by some kind of controller interface, so the modules can kind of negotiate and somehow manage the priority:

Very rough concept:

MyController extends Controller implements RequestInterceptionController

In modules:

if($event->action->controller instanceof RequestInterceptionController && $this->priority < $event->action->controller->getPriority()) {
    return;
}

Similar with the rest controller, there should be a marker interface for this, or the rest controllers could also implement the interface above.

[luke-]

@buddh4 @yurabakhtin The interface sounds like a nice approach here. The priority is probably difficult to implement, for this somehow the upcoming event handlers needs to be checked/executed.

Do you have any ideas regarding the priority?

If not, I would suggest that we simply take the eventhandler which comes first. Then we can either create an empty interface RequestInterceptionController or alternatively a new humhub\components\Controller attribute $doNotInterceptActionIds => [];

[buddh4]

My suggestion was to implement the priority check within the event handler of the modules itself (at least in a first step). We could define priorities for our known modules seperated by steps of 100 in order to add custom handlers in between. But did not really thinkg about this in detail yet.

[luke-]

Closed here: I opened an issue for this in the Core Repo.