humhub / rest

HumHub Rest API Module
24 stars 24 forks source link

parent::beforeAction($action) returns false #29

Closed marc-farre closed 3 years ago

marc-farre commented 4 years ago

If I send an API request (for example creating a user), in https://github.com/humhub/humhub-modules-rest/blob/master/components/BaseController.php#L68 parent::beforeAction($action) returns false on my server in HTTPS, so the actions are not executed.

On my local test server (same OS and params, but HTTP, no SSL), it returns true, so all is OK.

On the server where I have the problem (the HTTPS one), in BaseController.php, if I replace use humhub\components\Controller; by use yii\rest\Controller; the problem is solved, everything work (the user is created in my example).

luke- commented 4 years ago

Strange, can you try to comment out the "beforeAction" method in the humhub\compoents\Controller ?

marc-farre commented 4 years ago

To execute actions, beforeAction needs to return true. So if I comment, I will not work. But if I add return true; just before (here: https://github.com/humhub/humhub-modules-rest/blob/master/components/BaseController.php#L67), actions are executed and all is fine.

luke- commented 4 years ago

If you comment out, the parent implementation should be executed and return true.

For me it is interesting where & why on the HumHub base controller impl returns false.

marc-farre commented 4 years ago

OK. So I tried, and same problem when commented, actions are not executed.

I have tried a couple of changes to replace the return parent::beforeAction($action); but nothing works. If have replace it by yii\base\Controller::beforeAction($action) and yii\web\Controller::beforeAction($action)

And I tried to erase the behaviors() with the one of yii\rest\Controller but I doesn't work.

The only thing working is adding return true; before, or replacing use humhub\components\Controller; by use yii\rest\Controller;, I could not find an othe solution.

But anyway, for a rest API, it could be good to use the rest controller, it is done for dealing with JSON and XML formats

luke- commented 4 years ago

@yurabakhtin Any idea?

yurabakhtin commented 4 years ago

@luke- I tested and debugged the beforeAction() with https:// but couldn't reproduce this.

Of course we shouldn't replace the return parent::beforeAction($action); with return true;. Probably yes it would be correctly to use yii\rest\Controller instead of yii\web\Controller for rest api BaseController, but not sure because firstly we need to reproduce the bug in order to understand why this happens.

It looks like when @funkycram uses \humhub\components\Controller with behaviors:

'acl' => [
    'class' => AccessControl::class,
    'rules' => $this->getAccessRules()
]

then HTTPS is restricted by this filter somehow, but I don't find how and where. and when @funkycram uses \yii\rest\Controller with filters:

'contentNegotiator' => [
    'class' => ContentNegotiator::className(),
    'formats' => [
        'application/json' => Response::FORMAT_JSON,
        'application/xml' => Response::FORMAT_XML,
    ],
],
'verbFilter' => [
    'class' => VerbFilter::className(),
    'actions' => $this->verbs(),
],
'authenticator' => [
    'class' => CompositeAuth::className(),
],
'rateLimiter' => [
    'class' => RateLimiter::className(),
],

then it works, so probably the problem in the AccessControl::class or $this->getAccessRules() but currently the rest api user controller has no access rules, we will add some access rules only in next version like this https://github.com/humhub/humhub-modules-rest/pull/40/files#diff-88784db4fe45649d005493b2c697489f1b316a375c53e674765812a48ac67647R32, so I have no idea where it may be restricted for HTTPS requests.

luke- commented 3 years ago

@yurabakhtin thanks for your detailed evaluation.

@funkycram Can you give some more input or a code sample here?

marc-farre commented 3 years ago

Thanks a lot @yurabakhtin for your investigation.

I have finally founded the source of the problem.

When legal module is activated and in the configuration Legal Update is checked and the user used by the rest module didn't accept the legal update, this return doesn't occur: https://github.com/humhub-contrib/legal/blob/master/Events.php#L85

This solves the issue: https://github.com/humhub-contrib/legal/pull/15