From 540fa258aa1b82ada2bae0a1bd42189173cf9932 Mon Sep 17 00:00:00 2001 From: Jorge Lopes Date: Fri, 19 Aug 2022 13:41:05 +0100 Subject: [PATCH 01/14] add filter permission and group --- src/Filters/GroupFilter.php | 61 ++++++++++++++++++++++++++++++++ src/Filters/PermissionFilter.php | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 src/Filters/GroupFilter.php create mode 100644 src/Filters/PermissionFilter.php diff --git a/src/Filters/GroupFilter.php b/src/Filters/GroupFilter.php new file mode 100644 index 000000000..466527302 --- /dev/null +++ b/src/Filters/GroupFilter.php @@ -0,0 +1,61 @@ +loggedIn()) { + return redirect()->to('login'); + } + + if (auth()->user()->inGroup(...$arguments)) { + return; + } + + throw GroupException::forUnauthorized(); + } + + /** + * We don't have anything to do here. + * + * @param Response|ResponseInterface $response + * @param array|null $arguments + */ + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void + { + // Nothing required + } +} diff --git a/src/Filters/PermissionFilter.php b/src/Filters/PermissionFilter.php new file mode 100644 index 000000000..fb36cf3d3 --- /dev/null +++ b/src/Filters/PermissionFilter.php @@ -0,0 +1,61 @@ +loggedIn()) { + return redirect()->to('login'); + } + + foreach ($arguments as $permission) { + if (! auth()->user()->can($permission)) { + throw PermissionException::forUnauthorized(); + } + } + } + + /** + * We don't have anything to do here. + * + * @param Response|ResponseInterface $response + * @param array|null $arguments + */ + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void + { + // Nothing required + } +} From e12c29d05ad1bdcde0809c62611644a0cbdc2393 Mon Sep 17 00:00:00 2001 From: Jorge Lopes Date: Fri, 19 Aug 2022 13:45:49 +0100 Subject: [PATCH 02/14] add exception permission and group --- src/Authorization/AuthorizationException.php | 5 +++++ src/Exceptions/GroupException.php | 11 +++++++++++ src/Exceptions/PermissionException.php | 11 +++++++++++ 3 files changed, 27 insertions(+) create mode 100644 src/Exceptions/GroupException.php create mode 100644 src/Exceptions/PermissionException.php diff --git a/src/Authorization/AuthorizationException.php b/src/Authorization/AuthorizationException.php index c01d2912c..fa85b44b6 100644 --- a/src/Authorization/AuthorizationException.php +++ b/src/Authorization/AuthorizationException.php @@ -19,4 +19,9 @@ public static function forUnknownPermission(string $permission): self { return new self(lang('Auth.unknownPermission', [$permission])); } + + public static function forUnauthorized(): self + { + return new self(lang('Auth.notEnoughPrivilege')); + } } diff --git a/src/Exceptions/GroupException.php b/src/Exceptions/GroupException.php new file mode 100644 index 000000000..bda9ef35c --- /dev/null +++ b/src/Exceptions/GroupException.php @@ -0,0 +1,11 @@ + Date: Fri, 19 Aug 2022 13:46:32 +0100 Subject: [PATCH 03/14] update languages --- src/Language/de/Auth.php | 1 + src/Language/en/Auth.php | 1 + src/Language/es/Auth.php | 3 ++- src/Language/fa/Auth.php | 1 + src/Language/fr/Auth.php | 1 + src/Language/id/Auth.php | 1 + src/Language/ja/Auth.php | 1 + src/Language/sk/Auth.php | 1 + 8 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Language/de/Auth.php b/src/Language/de/Auth.php index e64af7412..855a0cbfe 100644 --- a/src/Language/de/Auth.php +++ b/src/Language/de/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'Es konnte nicht überprüft werden, ob die E-Mail-Adresse mit der gespeicherten übereinstimmt.', 'unableSendEmailToUser' => 'Leider gab es ein Problem beim Senden der E-Mail. Wir konnten keine E-Mail an "{0}" senden.', 'throttled' => 'Es wurden zu viele Anfragen von dieser IP-Adresse gestellt. Sie können es in {0} Sekunden erneut versuchen.', + 'notEnoughPrivilege' => 'Sie haben nicht die erforderliche Berechtigung, um den gewünschten Vorgang auszuführen.', 'email' => 'E-Mail-Adresse', 'username' => 'Benutzername', diff --git a/src/Language/en/Auth.php b/src/Language/en/Auth.php index b20716411..6d9e82cc7 100644 --- a/src/Language/en/Auth.php +++ b/src/Language/en/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'Unable to verify the email address matches the email on record.', 'unableSendEmailToUser' => 'Sorry, there was a problem sending the email. We could not send an email to "{0}".', 'throttled' => 'Too many requests made from this IP address. You may try again in {0} seconds.', + 'notEnoughPrivilege' => 'You do not have the necessary permission to perform the desired operation.', 'email' => 'Email Address', 'username' => 'Username', diff --git a/src/Language/es/Auth.php b/src/Language/es/Auth.php index a7e1de745..d4b5086f0 100644 --- a/src/Language/es/Auth.php +++ b/src/Language/es/Auth.php @@ -18,7 +18,8 @@ 'noUserEntity' => 'Se debe dar una Entidad de Usuario para validar la contraseña.', 'invalidEmail' => 'No podemos verificar que el email coincida con un email registrado.', 'unableSendEmailToUser' => 'Lo sentimaos, ha habido un problema al enviar el email. No podemos enviar un email a "{0}".', - 'throttled' => 'demasiadas peticiones hechas desde esta IP. Puedes intentarlo de nuevo en {0} segundos.', + 'throttled' => 'Demasiadas peticiones hechas desde esta IP. Puedes intentarlo de nuevo en {0} segundos.', + 'notEnoughPrivilege' => 'No tiene los permisos necesarios para realizar la operación deseada.', 'email' => 'Dirección Email', 'username' => 'Usuario', diff --git a/src/Language/fa/Auth.php b/src/Language/fa/Auth.php index a8d6c73fb..f5ee2bcec 100644 --- a/src/Language/fa/Auth.php +++ b/src/Language/fa/Auth.php @@ -28,6 +28,7 @@ 'invalidEmail' => 'امکان تایید ایمیلی که با آدرس ایمیل ثبت شده یکسان نیست، وجود ندارد.', 'unableSendEmailToUser' => 'متاسفانه, در ارسال ایمیل مشکلی پیش آمد. ما نتوانستیم ایمیلی را به "{0}" ارسال کنیم.', 'throttled' => 'درخواست های بسیار زیادی از این آدرس IP انجام شده است. می توانید بعد از {0} ثانیه دوباره امتحان کنید.', + 'notEnoughPrivilege' => 'شما مجوز لازم برای انجام عملیات مورد نظر را ندارید.', 'email' => 'آدرس ایمیل', 'username' => 'نام کاربری', diff --git a/src/Language/fr/Auth.php b/src/Language/fr/Auth.php index f9cd9b684..2bf24a5bb 100644 --- a/src/Language/fr/Auth.php +++ b/src/Language/fr/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'Impossible de vérifier que l\'adresse email existe.', 'unableSendEmailToUser' => 'Désolé, il y a eu un problème lors de l\'envoi de l\'email. Nous ne pouvons pas envoyer un email à "{0}".', 'throttled' => 'Trop de requêtes faites depuis cette adresse IP. Vous pouvez réessayer dans {0} secondes.', + 'notEnoughPrivilege' => 'Vous n\'avez pas l\'autorisation nécessaire pour effectuer l\'opération souhaitée.', 'email' => 'Adresse email', 'username' => 'Identifiant', diff --git a/src/Language/id/Auth.php b/src/Language/id/Auth.php index 19a3ddbce..cc31ed43d 100644 --- a/src/Language/id/Auth.php +++ b/src/Language/id/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'Tidak dapat memverifikasi alamat email yang cocok dengan email yang tercatat.', 'unableSendEmailToUser' => 'Maaf, ada masalah saat mengirim email. Kami tidak dapat mengirim email ke "{0}".', 'throttled' => 'Terlalu banyak permintaan yang dibuat dari alamat IP ini. Anda dapat mencoba lagi dalam {0} detik.', + 'notEnoughPrivilege' => 'Anda tidak memiliki izin yang diperlukan untuk melakukan operasi yang diinginkan.', 'email' => 'Alamat Email', 'username' => 'Nama Pengguna', diff --git a/src/Language/ja/Auth.php b/src/Language/ja/Auth.php index 7f5b3ccd5..70b6e8995 100644 --- a/src/Language/ja/Auth.php +++ b/src/Language/ja/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'メールアドレスが一致しません。', // 'Unable to verify the email address matches the email on record.', 'unableSendEmailToUser' => '申し訳ありませんが、メールの送信に問題がありました。 "{0}"にメールを送信できませんでした。', // 'Sorry, there was a problem sending the email. We could not send an email to "{0}".', 'throttled' => 'このIPアドレスからのリクエストが多すぎます。 {0}秒後に再試行できます。', // Too many requests made from this IP address. You may try again in {0} seconds. + 'notEnoughPrivilege' => '目的の操作を実行するために必要な権限がありません。', // You do not have the necessary permission to perform the desired operation. 'email' => 'メールアドレス', // 'Email Address', 'username' => 'ユーザー名', // 'Username', diff --git a/src/Language/sk/Auth.php b/src/Language/sk/Auth.php index e2f0ead51..5fb83c470 100644 --- a/src/Language/sk/Auth.php +++ b/src/Language/sk/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'Nie je možné overiť, či sa e-mailová adresa zhoduje so zaznamenaným e-mailom.', 'unableSendEmailToUser' => 'Ľutujeme, pri odosielaní e-mailu sa vyskytol problém. Nepodarilo sa nám odoslať e-mail na adresu „{0}".', 'throttled' => 'Z tejto adresy IP bolo odoslaných príliš veľa žiadostí. Môžete to skúsiť znova o {0} sekúnd.', + 'notEnoughPrivilege' => 'Nemáte potrebné povolenie na vykonanie požadovanej operácie.', 'email' => 'Emailová adresa', 'username' => 'Používateľské meno', From 19052f300fe818194a494dacba3b7db6be42721a Mon Sep 17 00:00:00 2001 From: Jorge Lopes Date: Fri, 19 Aug 2022 13:46:45 +0100 Subject: [PATCH 04/14] update install.md --- docs/install.md | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/docs/install.md b/docs/install.md index d18d89f4b..f64ed004d 100644 --- a/docs/install.md +++ b/docs/install.md @@ -135,15 +135,32 @@ your project. 1. Use InnoDB, not MyISAM. ## Controller Filters +The [Controller Filters](https://codeigniter.com/user_guide/incoming/filters.html) you can use to protect your routes the shield provides are: + +```php +public $aliases = [ + // ... + 'session' => \CodeIgniter\Shield\Filters\SessionAuth::class, + 'tokens' => \CodeIgniter\Shield\Filters\TokenAuth::class, + 'chain' => \CodeIgniter\Shield\Filters\ChainAuth::class, + 'auth-rates' => \CodeIgniter\Shield\Filters\AuthRates::class, + 'group' => \CodeIgniter\Shield\Filters\GroupFilter::class, + 'permission' => \CodeIgniter\Shield\Filters\PermissionFilter::class, +]; +``` + +Filters | Description +--- | --- +session and tokens | The `Session` and `AccessTokens` authenticators, respectively. +chained | The filter will check both authenticators in sequence to see if the user is logged in through either of authenticators, allowing a single API endpoint to work for both an SPA using session auth, and a mobile app using access tokens. +auth-rates | Provides a good basis for rate limiting of auth-related routes. +group | Checks if the user is in one of the groups passed in. +permission | Checks if the user has the passed permissions. -Shield provides 4 [Controller Filters](https://codeigniter.com/user_guide/incoming/filters.html) you can -use to protect your routes, `session`, `tokens`, and `chained`. The first two cover the `Session` and -`AccessTokens` authenticators, respectively. The `chained` filter will check both authenticators in sequence -to see if the user is logged in through either of authenticators, allowing a single API endpoint to -work for both an SPA using session auth, and a mobile app using access tokens. The fourth, `auth-rates`, -provides a good basis for rate limiting of auth-related routes. These can be used in any of the [normal filter config settings](https://codeigniter.com/user_guide/incoming/filters.html?highlight=filter#globals), or [within the routes file](https://codeigniter.com/user_guide/incoming/routing.html?highlight=routs#applying-filters). +> **Note** These filters are already loaded for you by the registrar class located at `src/Config/Registrar.php`. + ### Protect All Pages If you want to limit all routes (e.g. `localhost:8080/admin`, `localhost:8080/panel` and ...), you need to add the following code in the `app/Config/Filters.php` file. @@ -158,18 +175,6 @@ public $globals = [ ]; ``` -> **Note** These filters are already loaded for you by the registrar class located at `src/Config/Registrar.php`. - -```php -public $aliases = [ - // ... - 'session' => \CodeIgniter\Shield\Filters\SessionAuth::class, - 'tokens' => \CodeIgniter\Shield\Filters\TokenAuth::class, - 'chain' => \CodeIgniter\Shield\Filters\ChainAuth::class, - 'auth-rates' => \CodeIgniter\Shield\Filters\AuthRates::class, -]; -``` - ### Rate Limiting To help protect your authentication forms from being spammed by bots, it is recommended that you use From f7fbfae6d4e7d476ec4b493b94cd811da80dcef8 Mon Sep 17 00:00:00 2001 From: Jorge Lopes Date: Fri, 19 Aug 2022 14:04:27 +0100 Subject: [PATCH 05/14] update config registrar --- src/Config/Registrar.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Config/Registrar.php b/src/Config/Registrar.php index f212e7962..f05263c9c 100644 --- a/src/Config/Registrar.php +++ b/src/Config/Registrar.php @@ -8,6 +8,8 @@ use CodeIgniter\Shield\Collectors\Auth; use CodeIgniter\Shield\Filters\AuthRates; use CodeIgniter\Shield\Filters\ChainAuth; +use CodeIgniter\Shield\Filters\GroupFilter; +use CodeIgniter\Shield\Filters\PermissionFilter; use CodeIgniter\Shield\Filters\SessionAuth; use CodeIgniter\Shield\Filters\TokenAuth; @@ -24,6 +26,8 @@ public static function Filters(): array 'tokens' => TokenAuth::class, 'chain' => ChainAuth::class, 'auth-rates' => AuthRates::class, + 'group' => GroupFilter::class, + 'permission' => PermissionFilter::class, ], ]; } From 714bdd149c6794c45220146c32e1fd31e6c56235 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Sun, 20 Nov 2022 22:59:48 -0600 Subject: [PATCH 06/14] Add group and permission filters. --- docs/authorization.md | 26 +++++- src/Filters/GroupFilter.php | 21 ++--- src/Filters/PermissionFilter.php | 25 +++--- .../Filters/AbstractFilterTest.php | 9 ++- .../Filters/GroupFilterTest.php | 81 +++++++++++++++++++ .../Filters/PermissionFilterTest.php | 81 +++++++++++++++++++ 6 files changed, 220 insertions(+), 23 deletions(-) create mode 100644 tests/Authentication/Filters/GroupFilterTest.php create mode 100644 tests/Authentication/Filters/PermissionFilterTest.php diff --git a/docs/authorization.md b/docs/authorization.md index 16692e35f..73aae1074 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -9,6 +9,8 @@ - [can()](#can) - [inGroup()](#ingroup) - [hasPermission()](#haspermission) + - [Authorizing via Filters](#authorizing-via-filters) + - [Authorizing via Routes](#authorizing-via-routes) - [Managing User Permissions](#managing-user-permissions) - [addPermission()](#addpermission) - [removePermission()](#removepermission) @@ -128,6 +130,28 @@ if (! $user->hasPermission('users.create')) { } ``` +#### Authorizing via Filters + +You can restrict access to multiple routes through a [Controller Filter](https://codeigniter.com/user_guide/incoming/filters.html). One is provided for both restricting via groups the user belongs to, as well as which permission they need. The filters are automatically registered with the system undeer the `group` and `permission` aliases, respectively. You can define the protectections within `app/Config/Filters.php`: + +```php +public $filters = [ + 'group:admin,superadmin' => ['before' => ['admin/*']], + 'permission:users.manage' => ['before' => ['admin/users/*']], +]; +``` + +#### Authorizing via Routes + +The filters can also be used on a route or route group level: + +```php +$routes->group('admin', ['filter' => 'group:admin,superadmin'], static function ($routes) { + $routes->resource('users'); +}); + +``` + ## Managing User Permissions Permissions can be granted on a user level as well as on a group level. Any user-level permissions granted will @@ -199,7 +223,7 @@ $user->syncGroups('admin', 'beta'); #### getGroups() -Returns all groups this user is a part of. +Returns all groups this user is a part of. ```php $user->getGroups(); diff --git a/src/Filters/GroupFilter.php b/src/Filters/GroupFilter.php index 466527302..23c32dcdf 100644 --- a/src/Filters/GroupFilter.php +++ b/src/Filters/GroupFilter.php @@ -18,14 +18,8 @@ class GroupFilter implements FilterInterface { /** - * Do whatever processing this filter needs to do. - * By default it should not return anything during - * normal execution. However, when an abnormal state - * is found, it should return an instance of - * CodeIgniter\HTTP\Response. If it does, script - * execution will end and that Response will be - * sent back to the client, allowing for error pages, - * redirects, etc. + * Ensures the user is logged in and a member of one or + * more groups as specified in the filter. * * @param array|null $arguments * @@ -38,14 +32,21 @@ public function before(RequestInterface $request, $arguments = null) } if (! auth()->loggedIn()) { - return redirect()->to('login'); + return redirect()->route('login'); } if (auth()->user()->inGroup(...$arguments)) { return; } - throw GroupException::forUnauthorized(); + // If the previous_url is from this site, then + // we can redirect back to it. + if (strpos(previous_url(), site_url()) === 0) { + return redirect()->back()->with('error', lang('Auth.notEnoughPrivilege')); + } + + // Otherwise, we'll just send them to the home page. + return redirect()->to('/')->with('error', lang('Auth.notEnoughPrivilege')); } /** diff --git a/src/Filters/PermissionFilter.php b/src/Filters/PermissionFilter.php index fb36cf3d3..6ec7ae574 100644 --- a/src/Filters/PermissionFilter.php +++ b/src/Filters/PermissionFilter.php @@ -18,14 +18,8 @@ class PermissionFilter implements FilterInterface { /** - * Do whatever processing this filter needs to do. - * By default it should not return anything during - * normal execution. However, when an abnormal state - * is found, it should return an instance of - * CodeIgniter\HTTP\Response. If it does, script - * execution will end and that Response will be - * sent back to the client, allowing for error pages, - * redirects, etc. + * Ensures the user is logged in and has one or + * more permissions as specified in the filter. * * @param array|null $arguments * @@ -38,14 +32,23 @@ public function before(RequestInterface $request, $arguments = null) } if (! auth()->loggedIn()) { - return redirect()->to('login'); + return redirect()->route('login'); } foreach ($arguments as $permission) { - if (! auth()->user()->can($permission)) { - throw PermissionException::forUnauthorized(); + if (auth()->user()->can($permission)) { + return; } } + + // If the previous_url is from this site, then + // we can redirect back to it. + if (strpos(previous_url(), site_url()) === 0) { + return redirect()->back()->with('error', lang('Auth.notEnoughPrivilege')); + } + + // Otherwise, we'll just send them to the home page. + return redirect()->to('/')->with('error', lang('Auth.notEnoughPrivilege')); } /** diff --git a/tests/Authentication/Filters/AbstractFilterTest.php b/tests/Authentication/Filters/AbstractFilterTest.php index d1321e1b5..ada11c976 100644 --- a/tests/Authentication/Filters/AbstractFilterTest.php +++ b/tests/Authentication/Filters/AbstractFilterTest.php @@ -8,6 +8,7 @@ use CodeIgniter\Test\FeatureTestTrait; use Config\Services; use Tests\Support\TestCase; +use CodeIgniter\Shield\Test\AuthenticationTesting; /** * @internal @@ -15,6 +16,7 @@ abstract class AbstractFilterTest extends TestCase { use FeatureTestTrait; + use AuthenticationTesting; protected $namespace; protected string $alias; @@ -25,6 +27,7 @@ protected function setUp(): void $_SESSION = []; Services::reset(true); + helper('test'); parent::setUp(); @@ -48,9 +51,13 @@ private function addRoutes(): void { $routes = service('routes'); + $filterString = ! empty($this->routeFilter) + ? $this->routeFilter + : $this->alias; + $routes->group( '/', - ['filter' => $this->alias], + ['filter' => $filterString], static function ($routes): void { $routes->get('protected-route', static function (): void { echo 'Protected'; diff --git a/tests/Authentication/Filters/GroupFilterTest.php b/tests/Authentication/Filters/GroupFilterTest.php new file mode 100644 index 000000000..6d39c37ea --- /dev/null +++ b/tests/Authentication/Filters/GroupFilterTest.php @@ -0,0 +1,81 @@ +call('get', 'protected-route'); + + $result->assertRedirectTo('/login'); + + $result = $this->get('open-route'); + $result->assertStatus(200); + $result->assertSee('Open'); + } + + public function testFilterSuccess() + { + /** @var User */ + $user = fake(UserModel::class); + $user->addGroup('admin'); + + $result = $this + ->actingAs($user) + ->get('protected-route'); + + $result->assertStatus(200); + $result->assertSee('Protected'); + + $this->assertSame($user->id, auth('session')->id()); + $this->assertSame($user->id, auth('session')->user()->id); + } + + public function testFilterIncorrectGroupNoPrevious() + { + /** @var User */ + $user = fake(UserModel::class); + $user->addGroup('beta'); + + $result = $this + ->actingAs($user) + ->get('protected-route'); + + // Should redirect to home page since previous_url is not set + $result->assertRedirectTo(site_url('/')); + // Should have error message + $result->assertSessionHas('error'); + } + + public function testFilterIncorrectGroupWithPrevious() + { + /** @var User */ + $user = fake(UserModel::class); + $user->addGroup('beta'); + + $result = $this + ->actingAs($user) + ->withSession(['_ci_previous_url' => site_url('open-route')]) + ->get('protected-route'); + + // Should redirect to home page since previous_url is not set + $result->assertRedirectTo(site_url('open-route')); + + $result->assertSessionHas('error'); + } + +} diff --git a/tests/Authentication/Filters/PermissionFilterTest.php b/tests/Authentication/Filters/PermissionFilterTest.php new file mode 100644 index 000000000..c5fbef6de --- /dev/null +++ b/tests/Authentication/Filters/PermissionFilterTest.php @@ -0,0 +1,81 @@ +call('get', 'protected-route'); + + $result->assertRedirectTo('/login'); + + $result = $this->get('open-route'); + $result->assertStatus(200); + $result->assertSee('Open'); + } + + public function testFilterSuccess() + { + /** @var User */ + $user = fake(UserModel::class); + $user->addPermission('admin.access'); + + $result = $this + ->actingAs($user) + ->get('protected-route'); + + $result->assertStatus(200); + $result->assertSee('Protected'); + + $this->assertSame($user->id, auth('session')->id()); + $this->assertSame($user->id, auth('session')->user()->id); + } + + public function testFilterIncorrectGroupNoPrevious() + { + /** @var User */ + $user = fake(UserModel::class); + $user->addPermission('beta.access'); + + $result = $this + ->actingAs($user) + ->get('protected-route'); + + // Should redirect to home page since previous_url is not set + $result->assertRedirectTo(site_url('/')); + // Should have error message + $result->assertSessionHas('error'); + } + + public function testFilterIncorrectGroupWithPrevious() + { + /** @var User */ + $user = fake(UserModel::class); + $user->addPermission('beta.access'); + + $result = $this + ->actingAs($user) + ->withSession(['_ci_previous_url' => site_url('open-route')]) + ->get('protected-route'); + + // Should redirect to home page since previous_url is not set + $result->assertRedirectTo(site_url('open-route')); + + $result->assertSessionHas('error'); + } + +} From 057bc17eed503032544aded396d744835b0e9db8 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Tue, 29 Nov 2022 23:34:37 -0600 Subject: [PATCH 07/14] new phpstan errors fixed --- src/Filters/GroupFilter.php | 2 -- src/Filters/PermissionFilter.php | 2 -- .../Authentication/Filters/AbstractFilterTest.php | 2 +- tests/Authentication/Filters/GroupFilterTest.php | 14 ++++++++------ .../Filters/PermissionFilterTest.php | 14 ++++++++------ 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Filters/GroupFilter.php b/src/Filters/GroupFilter.php index 23c32dcdf..57fb8ab24 100644 --- a/src/Filters/GroupFilter.php +++ b/src/Filters/GroupFilter.php @@ -10,8 +10,6 @@ use CodeIgniter\HTTP\Response; use CodeIgniter\HTTP\ResponseInterface; -use CodeIgniter\Shield\Exceptions\GroupException; - /** * Group Authorization Filter. */ diff --git a/src/Filters/PermissionFilter.php b/src/Filters/PermissionFilter.php index 6ec7ae574..30c1725c2 100644 --- a/src/Filters/PermissionFilter.php +++ b/src/Filters/PermissionFilter.php @@ -10,8 +10,6 @@ use CodeIgniter\HTTP\Response; use CodeIgniter\HTTP\ResponseInterface; -use CodeIgniter\Shield\Exceptions\PermissionException; - /** * Permission Authorization Filter. */ diff --git a/tests/Authentication/Filters/AbstractFilterTest.php b/tests/Authentication/Filters/AbstractFilterTest.php index ada11c976..3a1f97059 100644 --- a/tests/Authentication/Filters/AbstractFilterTest.php +++ b/tests/Authentication/Filters/AbstractFilterTest.php @@ -5,10 +5,10 @@ namespace Tests\Authentication\Filters; use CodeIgniter\Config\Factories; +use CodeIgniter\Shield\Test\AuthenticationTesting; use CodeIgniter\Test\FeatureTestTrait; use Config\Services; use Tests\Support\TestCase; -use CodeIgniter\Shield\Test\AuthenticationTesting; /** * @internal diff --git a/tests/Authentication/Filters/GroupFilterTest.php b/tests/Authentication/Filters/GroupFilterTest.php index 6d39c37ea..df27e7932 100644 --- a/tests/Authentication/Filters/GroupFilterTest.php +++ b/tests/Authentication/Filters/GroupFilterTest.php @@ -9,12 +9,15 @@ use CodeIgniter\Shield\Models\UserModel; use CodeIgniter\Test\DatabaseTestTrait; +/** + * @internal + */ final class GroupFilterTest extends AbstractFilterTest { use DatabaseTestTrait; - protected string $alias = 'group'; - protected string $classname = GroupFilter::class; + protected string $alias = 'group'; + protected string $classname = GroupFilter::class; protected string $routeFilter = 'group:admin'; public function testFilterNotAuthorized(): void @@ -28,7 +31,7 @@ public function testFilterNotAuthorized(): void $result->assertSee('Open'); } - public function testFilterSuccess() + public function testFilterSuccess(): void { /** @var User */ $user = fake(UserModel::class); @@ -45,7 +48,7 @@ public function testFilterSuccess() $this->assertSame($user->id, auth('session')->user()->id); } - public function testFilterIncorrectGroupNoPrevious() + public function testFilterIncorrectGroupNoPrevious(): void { /** @var User */ $user = fake(UserModel::class); @@ -61,7 +64,7 @@ public function testFilterIncorrectGroupNoPrevious() $result->assertSessionHas('error'); } - public function testFilterIncorrectGroupWithPrevious() + public function testFilterIncorrectGroupWithPrevious(): void { /** @var User */ $user = fake(UserModel::class); @@ -77,5 +80,4 @@ public function testFilterIncorrectGroupWithPrevious() $result->assertSessionHas('error'); } - } diff --git a/tests/Authentication/Filters/PermissionFilterTest.php b/tests/Authentication/Filters/PermissionFilterTest.php index c5fbef6de..e46ec6225 100644 --- a/tests/Authentication/Filters/PermissionFilterTest.php +++ b/tests/Authentication/Filters/PermissionFilterTest.php @@ -9,12 +9,15 @@ use CodeIgniter\Shield\Models\UserModel; use CodeIgniter\Test\DatabaseTestTrait; +/** + * @internal + */ final class PermissionFilterTest extends AbstractFilterTest { use DatabaseTestTrait; - protected string $alias = 'permission'; - protected string $classname = PermissionFilter::class; + protected string $alias = 'permission'; + protected string $classname = PermissionFilter::class; protected string $routeFilter = 'permission:admin.access'; public function testFilterNotAuthorized(): void @@ -28,7 +31,7 @@ public function testFilterNotAuthorized(): void $result->assertSee('Open'); } - public function testFilterSuccess() + public function testFilterSuccess(): void { /** @var User */ $user = fake(UserModel::class); @@ -45,7 +48,7 @@ public function testFilterSuccess() $this->assertSame($user->id, auth('session')->user()->id); } - public function testFilterIncorrectGroupNoPrevious() + public function testFilterIncorrectGroupNoPrevious(): void { /** @var User */ $user = fake(UserModel::class); @@ -61,7 +64,7 @@ public function testFilterIncorrectGroupNoPrevious() $result->assertSessionHas('error'); } - public function testFilterIncorrectGroupWithPrevious() + public function testFilterIncorrectGroupWithPrevious(): void { /** @var User */ $user = fake(UserModel::class); @@ -77,5 +80,4 @@ public function testFilterIncorrectGroupWithPrevious() $result->assertSessionHas('error'); } - } From 9bfc68fa46e106c51341c0b01197e449c0519bcb Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Tue, 29 Nov 2022 23:36:52 -0600 Subject: [PATCH 08/14] Appease rector --- tests/Authentication/Filters/GroupFilterTest.php | 6 +++--- tests/Authentication/Filters/PermissionFilterTest.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Authentication/Filters/GroupFilterTest.php b/tests/Authentication/Filters/GroupFilterTest.php index df27e7932..ff653a869 100644 --- a/tests/Authentication/Filters/GroupFilterTest.php +++ b/tests/Authentication/Filters/GroupFilterTest.php @@ -33,7 +33,7 @@ public function testFilterNotAuthorized(): void public function testFilterSuccess(): void { - /** @var User */ + /** @var User user */ $user = fake(UserModel::class); $user->addGroup('admin'); @@ -50,7 +50,7 @@ public function testFilterSuccess(): void public function testFilterIncorrectGroupNoPrevious(): void { - /** @var User */ + /** @var User user */ $user = fake(UserModel::class); $user->addGroup('beta'); @@ -66,7 +66,7 @@ public function testFilterIncorrectGroupNoPrevious(): void public function testFilterIncorrectGroupWithPrevious(): void { - /** @var User */ + /** @var User user */ $user = fake(UserModel::class); $user->addGroup('beta'); diff --git a/tests/Authentication/Filters/PermissionFilterTest.php b/tests/Authentication/Filters/PermissionFilterTest.php index e46ec6225..c67bd1d1a 100644 --- a/tests/Authentication/Filters/PermissionFilterTest.php +++ b/tests/Authentication/Filters/PermissionFilterTest.php @@ -33,7 +33,7 @@ public function testFilterNotAuthorized(): void public function testFilterSuccess(): void { - /** @var User */ + /** @var User user */ $user = fake(UserModel::class); $user->addPermission('admin.access'); @@ -50,7 +50,7 @@ public function testFilterSuccess(): void public function testFilterIncorrectGroupNoPrevious(): void { - /** @var User */ + /** @var User user */ $user = fake(UserModel::class); $user->addPermission('beta.access'); @@ -66,7 +66,7 @@ public function testFilterIncorrectGroupNoPrevious(): void public function testFilterIncorrectGroupWithPrevious(): void { - /** @var User */ + /** @var User user */ $user = fake(UserModel::class); $user->addPermission('beta.access'); From e624cdd41c39c5f2239bad7adf7dfdd2726f6cf8 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Tue, 29 Nov 2022 23:38:35 -0600 Subject: [PATCH 09/14] whoops. Forgot dollar signs --- tests/Authentication/Filters/GroupFilterTest.php | 6 +++--- tests/Authentication/Filters/PermissionFilterTest.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Authentication/Filters/GroupFilterTest.php b/tests/Authentication/Filters/GroupFilterTest.php index ff653a869..a0d925ac9 100644 --- a/tests/Authentication/Filters/GroupFilterTest.php +++ b/tests/Authentication/Filters/GroupFilterTest.php @@ -33,7 +33,7 @@ public function testFilterNotAuthorized(): void public function testFilterSuccess(): void { - /** @var User user */ + /** @var User $user */ $user = fake(UserModel::class); $user->addGroup('admin'); @@ -50,7 +50,7 @@ public function testFilterSuccess(): void public function testFilterIncorrectGroupNoPrevious(): void { - /** @var User user */ + /** @var User $user */ $user = fake(UserModel::class); $user->addGroup('beta'); @@ -66,7 +66,7 @@ public function testFilterIncorrectGroupNoPrevious(): void public function testFilterIncorrectGroupWithPrevious(): void { - /** @var User user */ + /** @var User $user */ $user = fake(UserModel::class); $user->addGroup('beta'); diff --git a/tests/Authentication/Filters/PermissionFilterTest.php b/tests/Authentication/Filters/PermissionFilterTest.php index c67bd1d1a..21284e7a8 100644 --- a/tests/Authentication/Filters/PermissionFilterTest.php +++ b/tests/Authentication/Filters/PermissionFilterTest.php @@ -33,7 +33,7 @@ public function testFilterNotAuthorized(): void public function testFilterSuccess(): void { - /** @var User user */ + /** @var User $user */ $user = fake(UserModel::class); $user->addPermission('admin.access'); @@ -50,7 +50,7 @@ public function testFilterSuccess(): void public function testFilterIncorrectGroupNoPrevious(): void { - /** @var User user */ + /** @var User $user */ $user = fake(UserModel::class); $user->addPermission('beta.access'); @@ -66,7 +66,7 @@ public function testFilterIncorrectGroupNoPrevious(): void public function testFilterIncorrectGroupWithPrevious(): void { - /** @var User user */ + /** @var User $user */ $user = fake(UserModel::class); $user->addPermission('beta.access'); From 02bafb1413a2e2cb086f4528c7c632555fc8ddd6 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Wed, 30 Nov 2022 00:05:09 -0600 Subject: [PATCH 10/14] Apply suggestions from code review Co-authored-by: kenjis --- docs/authorization.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/authorization.md b/docs/authorization.md index 73aae1074..6f4923f96 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -132,7 +132,7 @@ if (! $user->hasPermission('users.create')) { #### Authorizing via Filters -You can restrict access to multiple routes through a [Controller Filter](https://codeigniter.com/user_guide/incoming/filters.html). One is provided for both restricting via groups the user belongs to, as well as which permission they need. The filters are automatically registered with the system undeer the `group` and `permission` aliases, respectively. You can define the protectections within `app/Config/Filters.php`: +You can restrict access to multiple routes through a [Controller Filter](https://codeigniter.com/user_guide/incoming/filters.html). One is provided for both restricting via groups the user belongs to, as well as which permission they need. The filters are automatically registered with the system under the `group` and `permission` aliases, respectively. You can define the protections within `app/Config/Filters.php`: ```php public $filters = [ From 2291828a6a5779f2a5108553bf2bd29de5b0a577 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Fri, 9 Dec 2022 22:59:52 -0600 Subject: [PATCH 11/14] Added missing lang items --- docs/authorization.md | 2 +- src/Language/it/Auth.php | 1 + src/Language/tr/Auth.php | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/authorization.md b/docs/authorization.md index 73aae1074..6f4923f96 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -132,7 +132,7 @@ if (! $user->hasPermission('users.create')) { #### Authorizing via Filters -You can restrict access to multiple routes through a [Controller Filter](https://codeigniter.com/user_guide/incoming/filters.html). One is provided for both restricting via groups the user belongs to, as well as which permission they need. The filters are automatically registered with the system undeer the `group` and `permission` aliases, respectively. You can define the protectections within `app/Config/Filters.php`: +You can restrict access to multiple routes through a [Controller Filter](https://codeigniter.com/user_guide/incoming/filters.html). One is provided for both restricting via groups the user belongs to, as well as which permission they need. The filters are automatically registered with the system under the `group` and `permission` aliases, respectively. You can define the protections within `app/Config/Filters.php`: ```php public $filters = [ diff --git a/src/Language/it/Auth.php b/src/Language/it/Auth.php index b7b5d98e8..4b6425d3a 100644 --- a/src/Language/it/Auth.php +++ b/src/Language/it/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'Impossibile verificare che l\'indirizzo email corrisponda all\'email nel record.', 'unableSendEmailToUser' => 'Spiacente, c\'è stato un problema inviando l\'email. Non possiamo inviare un\'email a "{0}".', 'throttled' => 'Troppe richieste effettuate da questo indirizzo IP. Potrai riprovare tra {0} secondi.', + 'notEnoughPrivilege' => 'Non si dispone dell\'autorizzazione necessaria per eseguire l\'operazione desiderata.', 'email' => 'Indirizzo Email', 'username' => 'Nome Utente', diff --git a/src/Language/tr/Auth.php b/src/Language/tr/Auth.php index e23cf483a..32661726b 100644 --- a/src/Language/tr/Auth.php +++ b/src/Language/tr/Auth.php @@ -19,6 +19,7 @@ 'invalidEmail' => 'E-posta adresinin kayıtlı e-posta ile eşleştiği doğrulanamıyor.', 'unableSendEmailToUser' => 'Üzgünüz, e-posta gönderilirken bir sorun oluştu. "{0}" adresine e-posta gönderemedik.', 'throttled' => 'Bu IP adresinden çok fazla istek yapıldı. {0} saniye sonra tekrar deneyebilirsiniz.', + 'notEnoughPrivilege' => 'İstediğiniz işlemi gerçekleştirmek için gerekli izne sahip değilsiniz.', 'email' => 'E-posta Adresi', 'username' => 'Kullanıcı Adı', From 2bde1aedd83cb925e26bce31f6d3d990d0078d90 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Sat, 10 Dec 2022 12:36:26 -0600 Subject: [PATCH 12/14] Updated test comment per review --- tests/Authentication/Filters/GroupFilterTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Authentication/Filters/GroupFilterTest.php b/tests/Authentication/Filters/GroupFilterTest.php index a0d925ac9..944add41c 100644 --- a/tests/Authentication/Filters/GroupFilterTest.php +++ b/tests/Authentication/Filters/GroupFilterTest.php @@ -75,7 +75,7 @@ public function testFilterIncorrectGroupWithPrevious(): void ->withSession(['_ci_previous_url' => site_url('open-route')]) ->get('protected-route'); - // Should redirect to home page since previous_url is not set + // Should redirect to the previous url (open-route) $result->assertRedirectTo(site_url('open-route')); $result->assertSessionHas('error'); From 96920eaf8dd8d94b3956899853625838f403d175 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Sat, 10 Dec 2022 12:36:52 -0600 Subject: [PATCH 13/14] Updated test comment per review --- tests/Authentication/Filters/PermissionFilterTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Authentication/Filters/PermissionFilterTest.php b/tests/Authentication/Filters/PermissionFilterTest.php index 21284e7a8..8721fc458 100644 --- a/tests/Authentication/Filters/PermissionFilterTest.php +++ b/tests/Authentication/Filters/PermissionFilterTest.php @@ -75,7 +75,7 @@ public function testFilterIncorrectGroupWithPrevious(): void ->withSession(['_ci_previous_url' => site_url('open-route')]) ->get('protected-route'); - // Should redirect to home page since previous_url is not set + // Should redirect to the previous url (open-route) $result->assertRedirectTo(site_url('open-route')); $result->assertSessionHas('error'); From b9955294bdd827c6e1158a9d9d87385b35c499a9 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Sun, 11 Dec 2022 22:35:40 -0600 Subject: [PATCH 14/14] Refactor to a common abstract auth filter. --- src/Filters/AbstractAuthFilter.php | 62 ++++++++++++++++++++++++++++++ src/Filters/GroupFilter.php | 46 ++-------------------- src/Filters/PermissionFilter.php | 48 +++-------------------- 3 files changed, 71 insertions(+), 85 deletions(-) create mode 100644 src/Filters/AbstractAuthFilter.php diff --git a/src/Filters/AbstractAuthFilter.php b/src/Filters/AbstractAuthFilter.php new file mode 100644 index 000000000..99303d8a4 --- /dev/null +++ b/src/Filters/AbstractAuthFilter.php @@ -0,0 +1,62 @@ +loggedIn()) { + return redirect()->route('login'); + } + + if ($this->isAuthorized($arguments)) { + return; + } + + // If the previous_url is from this site, then + // we can redirect back to it. + if (strpos(previous_url(), site_url()) === 0) { + return redirect()->back()->with('error', lang('Auth.notEnoughPrivilege')); + } + + // Otherwise, we'll just send them to the home page. + return redirect()->to('/')->with('error', lang('Auth.notEnoughPrivilege')); + } + + /** + * We don't have anything to do here. + * + * @param Response|ResponseInterface $response + * @param array|null $arguments + */ + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void + { + // Nothing required + } + + abstract protected function isAuthorized(array $arguments): bool; +} diff --git a/src/Filters/GroupFilter.php b/src/Filters/GroupFilter.php index 57fb8ab24..22e73ab4c 100644 --- a/src/Filters/GroupFilter.php +++ b/src/Filters/GroupFilter.php @@ -4,57 +4,17 @@ namespace CodeIgniter\Shield\Filters; -use CodeIgniter\Filters\FilterInterface; -use CodeIgniter\HTTP\RedirectResponse; -use CodeIgniter\HTTP\RequestInterface; -use CodeIgniter\HTTP\Response; -use CodeIgniter\HTTP\ResponseInterface; - /** * Group Authorization Filter. */ -class GroupFilter implements FilterInterface +class GroupFilter extends AbstractAuthFilter { /** * Ensures the user is logged in and a member of one or * more groups as specified in the filter. - * - * @param array|null $arguments - * - * @return RedirectResponse|void - */ - public function before(RequestInterface $request, $arguments = null) - { - if (empty($arguments)) { - return; - } - - if (! auth()->loggedIn()) { - return redirect()->route('login'); - } - - if (auth()->user()->inGroup(...$arguments)) { - return; - } - - // If the previous_url is from this site, then - // we can redirect back to it. - if (strpos(previous_url(), site_url()) === 0) { - return redirect()->back()->with('error', lang('Auth.notEnoughPrivilege')); - } - - // Otherwise, we'll just send them to the home page. - return redirect()->to('/')->with('error', lang('Auth.notEnoughPrivilege')); - } - - /** - * We don't have anything to do here. - * - * @param Response|ResponseInterface $response - * @param array|null $arguments */ - public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void + protected function isAuthorized(array $arguments): bool { - // Nothing required + return auth()->user()->inGroup(...$arguments); } } diff --git a/src/Filters/PermissionFilter.php b/src/Filters/PermissionFilter.php index 30c1725c2..486910818 100644 --- a/src/Filters/PermissionFilter.php +++ b/src/Filters/PermissionFilter.php @@ -4,59 +4,23 @@ namespace CodeIgniter\Shield\Filters; -use CodeIgniter\Filters\FilterInterface; -use CodeIgniter\HTTP\RedirectResponse; -use CodeIgniter\HTTP\RequestInterface; -use CodeIgniter\HTTP\Response; -use CodeIgniter\HTTP\ResponseInterface; - /** * Permission Authorization Filter. */ -class PermissionFilter implements FilterInterface +class PermissionFilter extends AbstractAuthFilter { /** - * Ensures the user is logged in and has one or - * more permissions as specified in the filter. - * - * @param array|null $arguments - * - * @return RedirectResponse|void + * Ensures the user is logged in and has one or more + * of the permissions as specified in the filter. */ - public function before(RequestInterface $request, $arguments = null) + protected function isAuthorized(array $arguments): bool { - if (empty($arguments)) { - return; - } - - if (! auth()->loggedIn()) { - return redirect()->route('login'); - } - foreach ($arguments as $permission) { if (auth()->user()->can($permission)) { - return; + return true; } } - // If the previous_url is from this site, then - // we can redirect back to it. - if (strpos(previous_url(), site_url()) === 0) { - return redirect()->back()->with('error', lang('Auth.notEnoughPrivilege')); - } - - // Otherwise, we'll just send them to the home page. - return redirect()->to('/')->with('error', lang('Auth.notEnoughPrivilege')); - } - - /** - * We don't have anything to do here. - * - * @param Response|ResponseInterface $response - * @param array|null $arguments - */ - public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void - { - // Nothing required + return false; } }