Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add -g option to shield:user create #1164

Merged
merged 7 commits into from
Aug 21, 2024
Merged

feat: add -g option to shield:user create #1164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
56662ca
feat: add -g option to `shield:user create`
kenjis Aug 12, 2024
4cac2d8
refactor: extract isValidGroup()
kenjis Aug 20, 2024
e79bf78
feat: add GroupModel::isValidGroup()
kenjis Aug 20, 2024
7ae3ff5
refactor: use GroupModel::isValidGroup()
kenjis Aug 20, 2024
641c573
feat: check invalid group name
kenjis Aug 20, 2024
0fe1251
refactor: remove unneeded (bool)
kenjis Aug 20, 2024
2e7c54b
chore: vendor/bin/phpstan analyze --generate-baseline phpstan-baselin…
kenjis Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion phpstan-baseline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@
'count' => 1,
'path' => __DIR__ . '/src/Collectors/Auth.php',
];
$ignoreErrors[] = [
// identifier: codeigniter.factoriesClassConstFetch
'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\GroupModel\\:\\:class is discouraged\\.$#',
'count' => 1,
'path' => __DIR__ . '/src/Commands/User.php',
];
$ignoreErrors[] = [
// identifier: codeigniter.factoriesClassConstFetch
'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\UserModel\\:\\:class is discouraged\\.$#',
Expand Down Expand Up @@ -259,7 +265,7 @@
$ignoreErrors[] = [
// identifier: codeigniter.factoriesClassConstFetch
'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\GroupModel\\:\\:class is discouraged\\.$#',
'count' => 2,
'count' => 4,
'path' => __DIR__ . '/src/Entities/User.php',
];
$ignoreErrors[] = [
Expand Down Expand Up @@ -330,6 +336,12 @@
'count' => 1,
'path' => __DIR__ . '/src/Models/UserIdentityModel.php',
];
$ignoreErrors[] = [
// identifier: codeigniter.factoriesClassConstFetch
'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\GroupModel\\:\\:class is discouraged\\.$#',
'count' => 1,
'path' => __DIR__ . '/src/Models/UserModel.php',
];
$ignoreErrors[] = [
// identifier: codeigniter.factoriesClassConstFetch
'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\UserIdentityModel\\:\\:class is discouraged\\.$#',
Expand Down
20 changes: 7 additions & 13 deletions src/Authorization/Traits/Authorizable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ public function addGroup(string ...$groups): self
{
$this->populateGroups();

$configGroups = $this->getConfigGroups();

$groupCount = count($this->groupCache);

foreach ($groups as $group) {
Expand All @@ -45,8 +43,11 @@ public function addGroup(string ...$groups): self
continue;
}

/** @var GroupModel $groupModel */
$groupModel = model(GroupModel::class);

// make sure it's a valid group
if (! in_array($group, $configGroups, true)) {
if (! $groupModel->isValidGroup($group)) {
throw AuthorizationException::forUnknownGroup($group);
}

Expand Down Expand Up @@ -96,10 +97,11 @@ public function syncGroups(string ...$groups): self
{
$this->populateGroups();

$configGroups = $this->getConfigGroups();
/** @var GroupModel $groupModel */
$groupModel = model(GroupModel::class);

foreach ($groups as $group) {
if (! in_array($group, $configGroups, true)) {
if (! $groupModel->isValidGroup($group)) {
throw AuthorizationException::forUnknownGroup($group);
}
}
Expand Down Expand Up @@ -398,14 +400,6 @@ private function saveGroupsOrPermissions(string $type, $model, array $cache): vo
}
}

/**
* @return list<string>
*/
private function getConfigGroups(): array
{
return array_keys(setting('AuthGroups.groups'));
}

/**
* @return list<string>
*/
Expand Down
33 changes: 28 additions & 5 deletions src/Commands/User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use CodeIgniter\Shield\Config\Auth;
use CodeIgniter\Shield\Entities\User as UserEntity;
use CodeIgniter\Shield\Exceptions\UserNotFoundException;
use CodeIgniter\Shield\Models\GroupModel;
use CodeIgniter\Shield\Models\UserModel;
use CodeIgniter\Shield\Validation\ValidationRules;
use Config\Services;
Expand Down Expand Up @@ -53,6 +54,7 @@ class User extends BaseCommand
shield:user <action> options

shield:user create -n newusername -e [email protected]
shield:user create -n newusername -e [email protected] -g mygroup
kenjis marked this conversation as resolved.
Show resolved Hide resolved

shield:user activate -n username
shield:user activate -e [email protected]
Expand Down Expand Up @@ -159,7 +161,7 @@ public function run(array $params): int
try {
switch ($action) {
case 'create':
$this->create($username, $email);
$this->create($username, $email, $group);
break;

case 'activate':
Expand Down Expand Up @@ -252,8 +254,9 @@ private function setValidationRules(): void
*
* @param string|null $username User name to create (optional)
* @param string|null $email User email to create (optional)
* @param string|null $group Group to add user to (optional)
*/
private function create(?string $username = null, ?string $email = null): void
private function create(?string $username = null, ?string $email = null, ?string $group = null): void
{
$data = [];

Expand Down Expand Up @@ -303,6 +306,11 @@ private function create(?string $username = null, ?string $email = null): void

$user = new UserEntity($data);

// Validate the group
if ($group !== null && ! $this->validateGroup($group)) {
throw new CancelException('Invalid group: "' . $group . '"');
}

if ($username === null) {
$userModel->allowEmptyInserts()->save($user);
$this->write('New User created', 'green');
Expand All @@ -311,11 +319,26 @@ private function create(?string $username = null, ?string $email = null): void
$this->write('User "' . $username . '" created', 'green');
}

// Add to default group
$user = $userModel->findById($userModel->getInsertID());
$userModel->addToDefaultGroup($user);

$this->write('The user is added to the default group.', 'green');
if ($group === null) {
// Add to default group
$userModel->addToDefaultGroup($user);

$this->write('The user is added to the default group.', 'green');
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if an invalid group is set?
shield:user create -n user1 -e [email protected] -g xxx

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

$ ./spark shield:user create -n user2 -e [email protected] -g xxx

CodeIgniter v4.5.3 Command Line Tool - Server Time: 2024-08-20 04:13:43 UTC+00:00

Password : password
Password confirmation : password
User "user2" created
[CodeIgniter\Shield\Authorization\AuthorizationException]
xxx is not a valid group.
at VENDORPATH/codeigniter4/shield/src/Authorization/AuthorizationException.php:24

Backtrace:
  1    VENDORPATH/codeigniter4/shield/src/Authorization/Traits/Authorizable.php:50
       CodeIgniter\Shield\Authorization\AuthorizationException::forUnknownGroup('xxx')

  2    VENDORPATH/codeigniter4/shield/src/Commands/User.php:324
       CodeIgniter\Shield\Entities\User()->addGroup('xxx')

  3    VENDORPATH/codeigniter4/shield/src/Commands/User.php:163
       CodeIgniter\Shield\Commands\User()->create('user2', '[email protected]', 'xxx')

  4    SYSTEMPATH/CLI/Commands.php:70
       CodeIgniter\Shield\Commands\User()->run([...])

  5    SYSTEMPATH/CLI/Console.php:48
       CodeIgniter\CLI\Commands()->run('shield:user', [...])

  6    SYSTEMPATH/Boot.php:351
       CodeIgniter\CLI\Console()->run()

  7    SYSTEMPATH/Boot.php:104
       CodeIgniter\Boot::runCommand(Object(CodeIgniter\CLI\Console))

  8    ROOTPATH/spark:84
       CodeIgniter\Boot::bootSpark(Object(Config\Paths))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above is not good, but the same as addgroup.

$ ./spark shield:user addgroup -n user1 -g mygroup

CodeIgniter v4.5.3 Command Line Tool - Server Time: 2024-08-20 04:12:13 UTC+00:00

Add the user "user1" to the group "mygroup" ? [y, n]: y
[CodeIgniter\Shield\Authorization\AuthorizationException]
mygroup is not a valid group.
at VENDORPATH/codeigniter4/shield/src/Authorization/AuthorizationException.php:24

Backtrace:
  1    VENDORPATH/codeigniter4/shield/src/Authorization/Traits/Authorizable.php:50
       CodeIgniter\Shield\Authorization\AuthorizationException::forUnknownGroup('mygroup')

  2    VENDORPATH/codeigniter4/shield/src/Commands/User.php:600
       CodeIgniter\Shield\Entities\User()->addGroup('mygroup')

  3    VENDORPATH/codeigniter4/shield/src/Commands/User.php:195
       CodeIgniter\Shield\Commands\User()->addgroup('mygroup', 'user1', null)

  4    SYSTEMPATH/CLI/Commands.php:70
       CodeIgniter\Shield\Commands\User()->run([...])

  5    SYSTEMPATH/CLI/Console.php:48
       CodeIgniter\CLI\Commands()->run('shield:user', [...])

  6    SYSTEMPATH/Boot.php:351
       CodeIgniter\CLI\Console()->run()

  7    SYSTEMPATH/Boot.php:104
       CodeIgniter\Boot::runCommand(Object(CodeIgniter\CLI\Console))

  8    ROOTPATH/spark:84
       CodeIgniter\Boot::bootSpark(Object(Config\Paths))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added checking the group name.

$ ./spark shield:user create -n user2 -e [email protected] -g xxx

CodeIgniter v4.5.4 Command Line Tool - Server Time: 2024-08-20 06:02:07 UTC+00:00

Password : password
Password confirmation : password
Invalid group: "xxx"

$user->addGroup($group);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to be able to set up multiple groups. For example:

$user->addGroup('admin', 'beta');
shield:user create -n user1 -e [email protected] -g admin,beta

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another enhancement.
If we add this, it is better to change all -g option.

    shield:user create -n newusername -e [email protected] -g mygroup
    shield:user addgroup -n username -g mygroup
    shield:user removegroup -n username -g mygroup


$this->write('The user is added to group "' . $group . '".', 'green');
}
}

private function validateGroup(string $group): bool
{
/** @var GroupModel $groupModel */
$groupModel = model(GroupModel::class);

return $groupModel->isValidGroup($group);
}

/**
Expand Down
10 changes: 10 additions & 0 deletions src/Models/GroupModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,14 @@ public function deleteNotIn($userId, $cache): void

$this->checkQueryReturn($return);
}

/**
* @param non-empty-string $group Group name
*/
public function isValidGroup(string $group): bool
{
$allowedGroups = array_keys(setting('AuthGroups.groups'));

return in_array($group, $allowedGroups, true);
}
}
8 changes: 5 additions & 3 deletions src/Models/UserModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,12 @@ private function assignIdentities(array $data, array $identities): array
*/
public function addToDefaultGroup(User $user): void
{
$defaultGroup = setting('AuthGroups.defaultGroup');
$allowedGroups = array_keys(setting('AuthGroups.groups'));
$defaultGroup = setting('AuthGroups.defaultGroup');

if (empty($defaultGroup) || ! in_array($defaultGroup, $allowedGroups, true)) {
/** @var GroupModel $groupModel */
$groupModel = model(GroupModel::class);

if (empty($defaultGroup) || ! $groupModel->isValidGroup($defaultGroup)) {
throw new InvalidArgumentException(lang('Auth.unknownGroup', [$defaultGroup ?? '--not found--']));
}

Expand Down
53 changes: 53 additions & 0 deletions tests/Commands/UserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,59 @@ public function testCreate(): void
]);
}

public function testCreateWithGroupBeta(): void
{
$this->setMockIo([
'Secret Passw0rd!',
'Secret Passw0rd!',
]);

command('shield:user create -n user1 -e [email protected] -g beta');

$this->assertStringContainsString(
'User "user1" created',
$this->io->getFirstOutput()
);
$this->assertStringContainsString(
'The user is added to group "beta"',
$this->io->getFirstOutput()
);

$users = model(UserModel::class);
$user = $users->findByCredentials(['email' => '[email protected]']);
$this->seeInDatabase($this->tables['identities'], [
'user_id' => $user->id,
'secret' => '[email protected]',
]);
$this->seeInDatabase($this->tables['users'], [
'id' => $user->id,
'active' => 0,
]);
$this->seeInDatabase($this->tables['groups_users'], [
'user_id' => $user->id,
'group' => 'beta',
]);
}

public function testCreateWithInvalidGroup(): void
{
$this->setMockIo([
'Secret Passw0rd!',
'Secret Passw0rd!',
]);

command('shield:user create -n user1 -e [email protected] -g invalid');

$this->assertStringContainsString(
'Invalid group: "invalid"',
$this->io->getFirstOutput()
);

$users = model(UserModel::class);
$user = $users->findByCredentials(['email' => '[email protected]']);
$this->assertNull($user);
}

public function testCreateNotUniqueName(): void
{
$user = $this->createUser([
Expand Down
Loading