From 035a9ce0a8642dc1218f4b5d587dfde4daa382f2 Mon Sep 17 00:00:00 2001 From: SilverFire - Dmitry Naumenko Date: Thu, 19 Nov 2015 19:00:12 +0200 Subject: [PATCH] Added `yii\filters\auth\AuthMetod::optional` property Added `yii\base\ActionFilter::getActionId()` - code extracted from `ActionFilter::isActive()` --- framework/CHANGELOG.md | 3 +- framework/base/ActionFilter.php | 24 ++- framework/filters/auth/AuthMethod.php | 49 +++++- framework/filters/auth/HttpBearerAuth.php | 2 +- tests/framework/filters/CompositeAuthTest.php | 121 -------------- tests/framework/filters/auth/AuthTest.php | 174 +++++++++++++++++++++ tests/framework/filters/auth/CompositeAuthTest.php | 121 ++++++++++++++ tests/framework/filters/stubs/UserIdentity.php | 66 ++++++++ 8 files changed, 424 insertions(+), 136 deletions(-) delete mode 100644 tests/framework/filters/CompositeAuthTest.php create mode 100644 tests/framework/filters/auth/AuthTest.php create mode 100644 tests/framework/filters/auth/CompositeAuthTest.php create mode 100644 tests/framework/filters/stubs/UserIdentity.php diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index c162998..ad13cfe 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -28,7 +28,7 @@ Yii Framework 2 Change Log - Bug #9791: Fixed endless loop on file creation for non-existing device letters on windows (lukos, cebe) - Bug #9874: Fixed outputting exception stacktrace in non-debug mode when `Response::FORMAT_RAW` is used (nainoon) - Bug #9883: Passing a single `yii\db\Expression` to `Query::select()` or `::addSelect()` was not handled correctly in all cases (cebe) -- Bug #9911: Fixed `yii\helpers\BaseStringHelper::explode()` code so it does not remove items eq to 0 with skip_empty attribute (silverfire, kidol) +- Bug #9911: Fixed `yii\helpers\BaseStringHelper::explode()` code so it doesn't remove items equal to 0 when `skip_empty` is true (silverfire, kidol) - Bug #9915: `yii\helpers\ArrayHelper::getValue()` was erroring instead of returning `null` for non-existing object properties (totaldev, samdark) - Bug #9924: Fixed `yii.js` handleAction corrupted parameter values containing quote (") character (silverfire) - Bug #9984: Fixed wrong captcha color in case Imagick is used (DrDeath72) @@ -41,6 +41,7 @@ Yii Framework 2 Change Log - Enh #3506: Added `\yii\validators\IpValidator` to perform validation of IP addresses and subnets (SilverFire, samdark) - Enh #5146: Added `\yii\i18n\Formatter::asDuration()` method (nineinchnick, SilverFire) - Enh #7341: Client validation now skips disabled inputs (SamMousa) +- Enh #4104: Added `yii\filters\auth\AuthMetod::optional` for optional authentification in all child classes (SilverFire) - Enh #7566: Improved `\yii\validators\CompareValidator` default messages (slinstj) - Enh #7581: Added ability to specify range using anonymous function in `RangeValidator` (RomeroMsk) - Enh #8613: `yii\widgets\FragmentCache` will not store empty content anymore which fixes some problems related to `yii\filters\PageCache` (kidol) diff --git a/framework/base/ActionFilter.php b/framework/base/ActionFilter.php index 30f8402..2c0d780 100644 --- a/framework/base/ActionFilter.php +++ b/framework/base/ActionFilter.php @@ -37,7 +37,6 @@ class ActionFilter extends Behavior */ public $except = []; - /** * @inheritdoc */ @@ -111,14 +110,13 @@ class ActionFilter extends Behavior } /** - * Returns a value indicating whether the filter is active for the given action. - * @param Action $action the action being filtered - * @return boolean whether the filter is active for the given action. + * Returns an $action ID, convert action uniqueId into an ID relative to the module + * @param Action $action + * @return string + * @since 2.0.7 */ - protected function isActive($action) - { + protected function getActionId($action) { if ($this->owner instanceof Module) { - // convert action uniqueId into an ID relative to the module $mid = $this->owner->getUniqueId(); $id = $action->getUniqueId(); if ($mid !== '' && strpos($id, $mid) === 0) { @@ -127,6 +125,18 @@ class ActionFilter extends Behavior } else { $id = $action->id; } + + return $id; + } + + /** + * Returns a value indicating whether the filter is active for the given action. + * @param Action $action the action being filtered + * @return boolean whether the filter is active for the given action. + */ + protected function isActive($action) + { + $id = $this->getActionId($action); return !in_array($id, $this->except, true) && (empty($this->only) || in_array($id, $this->only, true)); } } diff --git a/framework/filters/auth/AuthMethod.php b/framework/filters/auth/AuthMethod.php index 1ec8a60..328369b 100644 --- a/framework/filters/auth/AuthMethod.php +++ b/framework/filters/auth/AuthMethod.php @@ -8,6 +8,7 @@ namespace yii\filters\auth; use Yii; +use yii\base\Action; use yii\base\ActionFilter; use yii\web\UnauthorizedHttpException; use yii\web\User; @@ -34,6 +35,13 @@ abstract class AuthMethod extends ActionFilter implements AuthInterface * @var Response the response to be sent. If not set, the `response` application component will be used. */ public $response; + /** + * @var array list of action IDs that this filter will be applied to, but auth failure will not lead to error. + * It may be used for actions, that are allowed for public, but return some additional data for authenticated users. + * @see isOptional + * @since 2.0.7 + */ + public $optional = []; /** @@ -43,13 +51,21 @@ abstract class AuthMethod extends ActionFilter implements AuthInterface { $response = $this->response ? : Yii::$app->getResponse(); - $identity = $this->authenticate( - $this->user ? : Yii::$app->getUser(), - $this->request ? : Yii::$app->getRequest(), - $response - ); + try { + $identity = $this->authenticate( + $this->user ? : Yii::$app->getUser(), + $this->request ? : Yii::$app->getRequest(), + $response + ); + } catch (UnauthorizedHttpException $e) { + if ($this->isOptional($action)) { + return true; + } + + throw $e; + } - if ($identity !== null) { + if ($identity !== null || $this->isOptional($action)) { return true; } else { $this->challenge($response); @@ -72,4 +88,25 @@ abstract class AuthMethod extends ActionFilter implements AuthInterface { throw new UnauthorizedHttpException('You are requesting with an invalid credential.'); } + + /** + * Checks, whether the $action is optional + * + * @param Action $action + * @return boolean + * @see optional + * @since 2.0.7 + */ + protected function isOptional($action) { + $id = $this->getActionId($action); + return in_array($id, $this->optional, true); + } + + /** + * {@inheritdoc} + */ + protected function isActive($action) + { + return parent::isActive($action) || $this->isOptional($action); + } } diff --git a/framework/filters/auth/HttpBearerAuth.php b/framework/filters/auth/HttpBearerAuth.php index 0aece20..9155c94 100644 --- a/framework/filters/auth/HttpBearerAuth.php +++ b/framework/filters/auth/HttpBearerAuth.php @@ -40,7 +40,7 @@ class HttpBearerAuth extends AuthMethod public function authenticate($user, $request, $response) { $authHeader = $request->getHeaders()->get('Authorization'); - if ($authHeader !== null && preg_match("/^Bearer\\s+(.*?)$/", $authHeader, $matches)) { + if ($authHeader !== null && preg_match('/^Bearer\s+(.*?)$/', $authHeader, $matches)) { $identity = $user->loginByAccessToken($matches[1], get_class($this)); if ($identity === null) { $this->handleFailure($response); diff --git a/tests/framework/filters/CompositeAuthTest.php b/tests/framework/filters/CompositeAuthTest.php deleted file mode 100644 index 0092d2c..0000000 --- a/tests/framework/filters/CompositeAuthTest.php +++ /dev/null @@ -1,121 +0,0 @@ - - */ -class TestAuth extends AuthMethod -{ - public function authenticate($user, $request, $response) - { - return $user; - } -} - -class TestController extends Controller -{ - public function actionA() - { - return 'success'; - } - - public function actionB() - { - /** - * this call will execute the actionA in a same instance of TestController - */ - return $this->runAction('a'); - } - - public function actionC() - { - /** - * this call will execute the actionA in a same instance of TestController - */ - return $this->run('a'); - } - - public function actionD() - { - /** - * this call will execute the actionA in a new instance of TestController - */ - return $this->run('test/a'); - } - - public function behaviors() - { - /** - * the CompositeAuth::authenticate() assumes that it is only executed once per the controller's instance - * i believe this is okay as long as we specify in the documentation that if we want to use the authenticate - * method again(this might even be also true to other behaviors that attaches to the beforeAction event), - * that we will have to forward/run into the other action in a way that it will create a new controller instance - */ - return [ - 'authenticator' => [ - 'class' => CompositeAuth::className(), - 'authMethods' => [ - TestAuth::className() - ] - ], - ]; - } -} - -/** - * @group filters - */ -class CompositeAuthTest extends \yiiunit\TestCase -{ - protected function setUp() - { - parent::setUp(); - - $_SERVER['SCRIPT_FILENAME'] = "/index.php"; - $_SERVER['SCRIPT_NAME'] = "/index.php"; - - $appConfig = [ - 'components' => [ - 'user' => [ - 'identityClass' => UserIdentity::className() - ], - ], - 'controllerMap' => [ - 'test' => TestController::className() - ] - ]; - - $this->mockWebApplication($appConfig); - } - - public function testCallingRunWithCompleteRoute() - { - /** @var TestController $controller */ - $controller = Yii::$app->createController('test')[0]; - $this->assertEquals('success', $controller->run('test/d')); - } - - /** - * reproducing the issue specified in https://github.com/yiisoft/yii2/issues/7409 - */ - public function testRunAction() - { - /** @var TestController $controller */ - $controller = Yii::$app->createController('test')[0]; - $this->assertEquals('success', $controller->run('b')); - } - - public function testRunButWithActionIdOnly() - { - /** @var TestController $controller */ - $controller = Yii::$app->createController('test')[0]; - $this->assertEquals('success', $controller->run('c')); - } -} diff --git a/tests/framework/filters/auth/AuthTest.php b/tests/framework/filters/auth/AuthTest.php new file mode 100644 index 0000000..6983fcf --- /dev/null +++ b/tests/framework/filters/auth/AuthTest.php @@ -0,0 +1,174 @@ + + * @since 2.0.7 + */ +class AuthTest extends \yiiunit\TestCase +{ + protected function setUp() + { + parent::setUp(); + + $_SERVER['SCRIPT_FILENAME'] = "/index.php"; + $_SERVER['SCRIPT_NAME'] = "/index.php"; + + $appConfig = [ + 'components' => [ + 'user' => [ + 'identityClass' => UserIdentity::className() + ], + ], + 'controllerMap' => [ + 'test-auth' => TestAuthController::className() + ] + ]; + + $this->mockWebApplication($appConfig); + } + + public function tokenProvider() + { + return [ + ['token1', 'user1'], + ['token2', 'user2'], + ['token3', 'user3'], + ['unknown', null], + [null, null], + ]; + } + + public function authOnly($token, $login, $filter, $action) + { + /** @var TestAuthController $controller */ + $controller = Yii::$app->createController('test-auth')[0]; + $controller->authenticatorConfig = ArrayHelper::merge($filter, ['only' => [$action]]); + try { + $this->assertEquals($login, $controller->run($action)); + } catch (UnauthorizedHttpException $e) { + + } + } + + public function authOptional($token, $login, $filter, $action) + { + /** @var TestAuthController $controller */ + $controller = Yii::$app->createController('test-auth')[0]; + $controller->authenticatorConfig = ArrayHelper::merge($filter, ['optional' => [$action]]); + try { + $this->assertEquals($login, $controller->run($action)); + } catch (UnauthorizedHttpException $e) { + + } + } + + public function authExcept($token, $login, $filter, $action) + { + /** @var TestAuthController $controller */ + $controller = Yii::$app->createController('test-auth')[0]; + $controller->authenticatorConfig = ArrayHelper::merge($filter, ['except' => ['other']]); + try { + $this->assertEquals($login, $controller->run($action)); + } catch (UnauthorizedHttpException $e) { + + } + } + + /** + * @dataProvider tokenProvider + */ + public function testQueryParamAuth($token, $login) { + $_GET['access-token'] = $token; + $filter = ['class' => QueryParamAuth::className()]; + $this->authOnly($token, $login, $filter, 'query-param-auth'); + $this->authOptional($token, $login, $filter, 'query-param-auth'); + $this->authExcept($token, $login, $filter, 'query-param-auth'); + } + + /** + * @dataProvider tokenProvider + */ + public function testHttpBasicAuth($token, $login) { + $_SERVER['PHP_AUTH_USER'] = $token; + $_SERVER['PHP_AUTH_PW'] = 'whatever, we are testers'; + $filter = ['class' => HttpBasicAuth::className()]; + $this->authOnly($token, $login, $filter, 'basic-auth'); + $this->authOptional($token, $login, $filter, 'basic-auth'); + $this->authExcept($token, $login, $filter, 'basic-auth'); + } + + /** + * @dataProvider tokenProvider + */ + public function testHttpBasicAuthCustom($token, $login) { + $_SERVER['PHP_AUTH_USER'] = $login; + $_SERVER['PHP_AUTH_PW'] = 'whatever, we are testers'; + $filter = [ + 'class' => HttpBasicAuth::className(), + 'auth' => function ($username, $password) { + if (preg_match('/\d$/', $username)) { + return UserIdentity::findIdentity($username); + } + + return null; + } + ]; + $this->authOnly($token, $login, $filter, 'basic-auth'); + $this->authOptional($token, $login, $filter, 'basic-auth'); + $this->authExcept($token, $login, $filter, 'basic-auth'); + } + + /** + * @dataProvider tokenProvider + */ + public function testHttpBearerAuth($token, $login) { + Yii::$app->request->headers->set('Authorization', "Bearer $token"); + $filter = ['class' => HttpBearerAuth::className()]; + $this->authOnly($token, $login, $filter, 'bearer-auth'); + $this->authOptional($token, $login, $filter, 'bearer-auth'); + $this->authExcept($token, $login, $filter, 'bearer-auth'); + } +} + +/** + * Class TestAuthController + * + * @author Dmitry Naumenko + * @since 2.0.7 + */ +class TestAuthController extends Controller +{ + public $authenticatorConfig = []; + + public function behaviors() + { + return ['authenticator' => $this->authenticatorConfig]; + } + + public function actionBasicAuth() + { + return Yii::$app->user->id; + } + + public function actionBearerAuth() + { + return Yii::$app->user->id; + } + + public function actionQueryParamAuth() + { + return Yii::$app->user->id; + } +} \ No newline at end of file diff --git a/tests/framework/filters/auth/CompositeAuthTest.php b/tests/framework/filters/auth/CompositeAuthTest.php new file mode 100644 index 0000000..71a35ec --- /dev/null +++ b/tests/framework/filters/auth/CompositeAuthTest.php @@ -0,0 +1,121 @@ + + */ +class TestAuth extends AuthMethod +{ + public function authenticate($user, $request, $response) + { + return $user; + } +} + +class TestController extends Controller +{ + public function actionA() + { + return 'success'; + } + + public function actionB() + { + /** + * this call will execute the actionA in a same instance of TestController + */ + return $this->runAction('a'); + } + + public function actionC() + { + /** + * this call will execute the actionA in a same instance of TestController + */ + return $this->run('a'); + } + + public function actionD() + { + /** + * this call will execute the actionA in a new instance of TestController + */ + return $this->run('test/a'); + } + + public function behaviors() + { + /** + * the CompositeAuth::authenticate() assumes that it is only executed once per the controller's instance + * i believe this is okay as long as we specify in the documentation that if we want to use the authenticate + * method again(this might even be also true to other behaviors that attaches to the beforeAction event), + * that we will have to forward/run into the other action in a way that it will create a new controller instance + */ + return [ + 'authenticator' => [ + 'class' => CompositeAuth::className(), + 'authMethods' => [ + TestAuth::className() + ], + ], + ]; + } +} + +/** + * @group filters + */ +class CompositeAuthTest extends \yiiunit\TestCase +{ + protected function setUp() + { + parent::setUp(); + + $_SERVER['SCRIPT_FILENAME'] = "/index.php"; + $_SERVER['SCRIPT_NAME'] = "/index.php"; + + $appConfig = [ + 'components' => [ + 'user' => [ + 'identityClass' => UserIdentity::className() + ], + ], + 'controllerMap' => [ + 'test' => TestController::className() + ] + ]; + + $this->mockWebApplication($appConfig); + } + + public function testCallingRunWithCompleteRoute() + { + /** @var TestController $controller */ + $controller = Yii::$app->createController('test')[0]; + $this->assertEquals('success', $controller->run('test/d')); + } + + /** + * reproducing the issue specified in https://github.com/yiisoft/yii2/issues/7409 + */ + public function testRunAction() + { + /** @var TestController $controller */ + $controller = Yii::$app->createController('test')[0]; + $this->assertEquals('success', $controller->run('b')); + } + + public function testRunButWithActionIdOnly() + { + /** @var TestController $controller */ + $controller = Yii::$app->createController('test')[0]; + $this->assertEquals('success', $controller->run('c')); + } +} diff --git a/tests/framework/filters/stubs/UserIdentity.php b/tests/framework/filters/stubs/UserIdentity.php new file mode 100644 index 0000000..18cd026 --- /dev/null +++ b/tests/framework/filters/stubs/UserIdentity.php @@ -0,0 +1,66 @@ + + * @since 2.0.7 + */ +class UserIdentity extends Component implements IdentityInterface +{ + private static $ids = [ + 'user1', + 'user2', + 'user3', + ]; + + private static $tokens = [ + 'token1' => 'user1', + 'token2' => 'user2', + 'token3' => 'user3', + ]; + + private $_id; + + private $_token; + + public static function findIdentity($id) + { + if (in_array($id, static::$ids)) { + $identitiy = new static(); + $identitiy->_id = $id; + return $identitiy; + } + } + + public static function findIdentityByAccessToken($token, $type = null) + { + if (isset(static::$tokens[$token])) { + $id = static::$tokens[$token]; + $identitiy = new static(); + $identitiy->_id = $id; + $identitiy->_token = $token; + return $identitiy; + } + } + + public function getId() + { + return $this->_id; + } + + public function getAuthKey() + { + throw new NotSupportedException(); + } + + public function validateAuthKey($authKey) + { + throw new NotSupportedException(); + } +} \ No newline at end of file