feat: add filter permission and group
- [x] add filter group
- [x] add filter permission
- [x] update languages
- [ ] add tests (I don't have testing experience)
- [ ] add docs
@jlopes90 Like you, I am a beginner in test writing. However, I have tried hard to learn it. Using tests over time will help you identify errors and problems easily. If you are interested, writing a test is not difficult. Here is some useful information about learning phpunittest. https://forum.codeigniter.com/showthread.php?tid=81830&pid=396218#pid396218
A good start, and some good feedback from the team! Thank you for taking this on @jlopes90 - let us know how we can help.
<?php
namespace CodeIgniter\Shield\Authorization;
use Exception;
class AuthorizationException extends Exception
{
protected $code = 401;
public static function forUnauthorizedGroup(): self
{
return new self(lang('Auth.unauthorizedGroup'));
}
public static function forUnknownGroup(string $group): self
{
return new self(lang('Auth.unknownGroup', [$group]));
}
public static function forUnauthorizedPermission(): self
{
return new self(lang('Auth.unauthorizedPermission'));
}
public static function forUnknownPermission(string $permission): self
{
return new self(lang('Auth.unknownPermission', [$permission]));
}
}
'unauthorizedGroup' => 'You do not have sufficient groups to access that page.',
'unauthorizedPermission' => 'You do not have sufficient permissions to access that page.',
What about?
I'm not good at documentation or explanation in english.
This is from Myth:Auth.
'notEnoughPrivilege' => 'You do not have sufficient permissions to access that page.',
Do we need to change the error message for different filters?
Personal opinion, You do not have the necessary permission to perform the desired operation.
I always prefer a very generic authorization error message; it can be used more globally and it doesn't reveal any information to probe attacks.
<?php
namespace CodeIgniter\Shield\Authorization;
use Exception;
class AuthorizationException extends Exception
{
protected $code = 401;
public static function forUnknownGroup(string $group): self
{
return new self(lang('Auth.unknownGroup', [$group]));
}
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'));
}
}
'notEnoughPrivilege' => 'You do not have sufficient permissions to access that page.',
What about?
Please go with @datamweb's suggestion, then this phrase can be reused for any authorization fault without belying the actual attempt:
You do not have the necessary permission to perform the desired operation.
Please go with @datamweb's suggestion, then this phrase can be reused for any authorization fault without belying the actual attempt:
You do not have the necessary permission to perform the desired operation.
'notEnoughPrivilege' => 'You do not have the necessary permission to perform the desired operation.'
@jlopes90 I know how you feel, things are a bit complicated, let's continue together, we will call our friends whenever necessary.
- Proceed as follows in file
src/Config/Registrar.php:
use CodeIgniter\Shield\Filters\GroupFilter ;
use CodeIgniter\Shield\Filters\PermissionFilter;
/**
* Registers the Shield filters.
*/
public static function Filters(): array
{
return [
'aliases' => [
'session' => SessionAuth::class,
...
'group-filter' => GroupFilter::class,
'permission-filter' => PermissionFilter::class,
],
];
}
src/Config/Registrar.php
I didn't even notice about "registar", it was very helpful. Thanks.
The group-filter or better just group?
$routes->get('/', 'Home::index', ['filter' => 'group-filter:home.view');
or
$routes->get('/', 'Home::index', ['filter' => 'group:home.view');
The
group-filteror better justgroup?
I consider group a common expression and I don't like it.
However, yes group-filter is not interesting either.
Allow other members to decide on this matter.
We need docs for this. At the very least,
install.mdneeds updated to include the new filters. Having instructions added toauthorization.mdin the appropriate section underAuthorizing Useris needed also.
I'm not good at documentation or explanation in english.
Step 2: Writing documents. @jlopes90 , But I understood your comment well.
In fact, since you are not an English speaker, this is a bonus. Because you can express things in simple terms. Don't forget that you are the better person to write the documentation because you know what happened in these PR.
Well, first try to write down everything you need to say in your own language somewhere. After the content is finalized, start translating in any way you are comfortable with. After that I will check here whether it is understandable or not.
Finally, it is reviewed by the members.
So just do it. I am waiting.
We need docs for this. At the very least, install.md needs updated to include the new filters.
https://github.com/codeigniter4/shield/blob/bc85d79599ec7150f9bcad45d3622179919afe74/docs/install.md?plain=1#L157-L176
Controller Filters
The Controller Filters you can use to protect your routes the shield provides are:
| 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-filter | ?? |
| permission-filter | ?? |
These filters are already loaded for you by the registrar class located at src/Config/Registrar.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-filter' => \CodeIgniter\Shield\Filters\GroupFilter::class,
'permission-filter' => \CodeIgniter\Shield\Filters\PermissionFilter::class,
];
I consider
groupa common expression and I don't like it. However, yesgroup-filteris not interesting either.
These are aliases for filters. All of them are filters. So -filter is not needed.
Shorter is preferable.
@jlopes90 The docs seems good. Why don't you proceed?
I have no idea how to resolve the PHPCSFixer error.
I have no idea how to resolve the
PHPCSFixererror.
@jlopes90 We finally decided to use declare_strict_types, please see PR #379 .
And see https://github.com/codeigniter4/shield/runs/7880373539?check_suite_focus=true
@jlopes90 Very easy. Just run composer cs-fix.
See also https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#php-style
LoginTest is not part of here, so I commit?
LoginTest is not part of here, so I commit?
I sent a PR #390
LoginTest was fixed.
Please rebase. See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch
I did git rebase and then git pull and it was like this?
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch
We never use git pull. It is harmful.
and now? make new PR?
Cherry pick the commits that are needed and create a new PR.
Sorry, I can't close pull request
I did wrong again, I'm going to recover and do new PR