yii2
yii2 copied to clipboard
CompositeAuth requires auth methods to be ActionFilters
What steps will reproduce the problem?
Prior to #19418 it was allowed to create an auth implementation that implemented AuthInterface.
What is the expected result?
Minor changes shouldn't break backwards compatibility.
Based on the docblock from CompositeAuth.php:
/**
* @var array the supported authentication methods. This property should take a list of supported
* authentication methods, each represented by an authentication class or configuration.
*
* If this property is empty, no authentication will be performed.
*
* Note that an auth method class must implement the [[\yii\filters\auth\AuthInterface]] interface.
*/
public $authMethods = [];
I'd expect that the only requirement is to implement the mentioned interface.
What do you get instead?
#19418 blindly calls a function isActive which is not in defined in AuthInterface, without checking whether the concrete authentication method implemention actually extends ActionFilter.
Proposed solution
Fix it so that it checks whether the auth method extends ActionFilter before blindly calling a function on it:
if ($auth instanceof \yii\base\ActionFilter) && isset($this->owner->action) && $auth->isActive($this->owner->action)) {
$identity = $auth->authenticate($user, $request, $response);
if ($identity !== null) {
return $identity;
}
}
This will fix the issue without breaking stuff. It'll even leave the "bugfix" intact.
Note that the original bugfix still leaves room for changed behavior; for example when an auth method is used inside CompositeAuth, no events are fired since the filter's beforeFilter and afterFilter methods are never called.
Additional info
| Q | A |
|---|---|
| Yii version | 2.0.46 |
| PHP version | N/A |
| Operating system | N/A |
if (
$this->owner instanceof Controller
&& (
!isset($this->owner->action)
|| (
$auth instanceof \yii\base\ActionFilter
&& !$auth->isActive($this->owner->action)
)
)
) {
continue;
}
That's not the code that's in master AFAIK? Or are you proposing an alternative fix?
@SamMousa master + your fix
could you maybe release this as a fix? 2.0.46.1? we could not switch to 2.0.46 because of this and https://github.com/yiisoft/yii2/issues/19534