@Quy Thanks for your great help, this really helps getting better knowledge about
the code. Now everything your mention should be fixed.

Let me now if I forgot something.

Hello,

I solved the change requests of @mbabker.

Now you can use this with options like this:

 php cli/joomla.php user:add --username="Pete" --name="Peter Ganz" --password="dasdasdsdsa" --email="nomail@xxxxx.de" --usergroup=Manager,Administrator

Add user
========

 [OK] User: Pete                                                                         
                                                                                         
      Password: dasdasdsdsa

Or if not you will be ask:

php cli/joomla.php user:add

 Please enter a username:
 > Pete


 Please enter a name:
 > Peter Ganz


 Please enter a email adress:
 > nomail@xxxx.de


 Please enter a password:
 > 


 Please select a usergroup:
  [0] Public
  [1] Registered
  [2] Author
  [3] Editor
  [4] Publisher
  [5] Manager
  [6] Administrator
  [7] Guest
 > 5,6


Add user
========

 [OK] User: Pete                                                                         
                                                                                         
      Password: sdkosadasdpoasdpoasdasodo

Change the password works on the same, but you only have to options:

--username
--pasword

At this moment I exclude Super Users form this cause like @mbabker sait wit no Super User privileges on CLI. So until we find a solution who to this we should do only with the other groups.

Or we use user:add-admin to add user not with the save method of User class,
instead we add Super Users like it was an beginning of the PR.

@Quy One more time thanks for your hints.
New commit should fix that.

@mbabker I also see PR #20059 . Will this effect the CLI.

We are using Symfony from vendor directory.
Like in
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Application/ConsoleApplication.php
and
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/vendor/joomla/console/src/AbstractCommand.php

It affects how the core repo is maintained but not the packaged releases.
So no, it won't affect work here.

On Mon, Apr 9, 2018 at 4:11 PM Felix Hamel notifications@github.com wrote:

@mbabker https://github.com/mbabker I also see PR #20059
#20059 . Will this effect the
CLI.

We are using Symfony from vendor directory.
Like in

https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Application/ConsoleApplication.php
and

https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/vendor/joomla/console/src/AbstractCommand.php

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#20095 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfof7GacOY2dQkW31l_7tOHJQcqJu1ks5tm86cgaJpZM4TJyau
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

I got an idea how could maybe add "super uesers" with the CLI.
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Application/ConsoleApplication.php has following method:

public function isClient($identifier)
	{
		return $identifier === 'cli';
	}

Maybe could use this one to check if we are in CLI context.
The problem is I do not know pass this in to User Class save method, on a secure way.

@mbabker can you take a look on my last changes and what we have so far.
I tried to solve your change request and like to here what you think.

Seems fine to me.

@mbabker I have some good news I thnik. With my last commit we should be able to add super user.

I had to modify the save method of User class a little bit for this.

We are now checking with JFactory the running applications and if its input format is cli the checks will be passed and super user is created

That'll work, but it really, really, really, really needs to be a temporary thing. Changing access related code behaviors based on whether the context is CLI or web is not the way to be doing things (sure it works, but that doesn't mean it's right necessarily).

Hey. Thats right and it's that problem I see too.

So we should only use this untill we have a solution which is not base on checking the context. I'll add a TODO comment to this code and think about a other way to handle that.

I have tested this item ✅ successfully on b30f634

Tested Successfully

@bosunski thanks for testing.

@bosunski still have your request for change because of the Factory::.
Would close/resolve the change request?

Yes of course @f-hamel , that's okay instead of \JFactory

I have tested this item ✅ successfully on 29cab59

@f-hamel this is a great PR! Thanks for working on this, very nice addition.

I do think some small things could be added/improved, but lets get this PR merge first in the 4.0-dev branch to get this included and extend it from there.

Ping @euismod2336, @roland-d, @fastslack you will like this feature as well. Please test!

I have tested this item ✅ successfully on 29cab59

@f-hamel this is a great PR! Thanks for working on this, very nice addition.

I do think some small things could be added/improved, but lets get this PR merge first in the 4.0-dev branch to get this included and extend it from there.

Ping @euismod2336, @roland-d, @fastslack you will like this feature as well. Please test!

@f-hamel Great PR indeed.

As a first test I use and address like test I get an untranslated string [ERROR] JLIB_DATABASE_ERROR_VALID_MAIL

So I was able to delete the last super user from the system as well. Perhaps we need a check not to delete super users or at least leave 1 superuser in place.

I took a backup before doing so and added it back into the database. This user is now not linked to a group, so it cannot login. I decided to add it via the CLI. This is the error I see. This is because the user is not linked to any group. When I add the user to the group, the below issue does not show up.

$ php joomla.php user:addtogroup

 Please enter a username:
 > admin

Add user to group
=================

PHP Notice:  Undefined variable: db in /usr/local/var/www/joomlacms/libraries/src/Console/AddUserToGroupCommand.php on line 156
PHP Stack trace:
PHP   1. {main}() /usr/local/var/www/joomlacms/cli/joomla.php:0
PHP   2. Joomla\CMS\Application\ConsoleApplication->execute() /usr/local/var/www/joomlacms/cli/joomla.php:69
PHP   3. Joomla\CMS\Application\ConsoleApplication->execute() /usr/local/var/www/joomlacms/libraries/src/Application/ConsoleApplication.php:154
PHP   4. Joomla\CMS\Application\ConsoleApplication->doExecute() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Application.php:411
PHP   5. Joomla\CMS\Application\ConsoleApplication->doExecute() /usr/local/var/www/joomlacms/libraries/src/Application/ConsoleApplication.php:114
PHP   6. Joomla\CMS\Application\ConsoleApplication->runCommand() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Application.php:385
PHP   7. Joomla\CMS\Console\AddUserToGroupCommand->execute() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Application.php:906
PHP   8. Joomla\CMS\Console\AddUserToGroupCommand->doExecute() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Command/AbstractCommand.php:235
PHP   9. Joomla\CMS\Console\AddUserToGroupCommand->getGroups() /usr/local/var/www/joomlacms/libraries/src/Console/AddUserToGroupCommand.php:89

Notice: Undefined variable: db in /usr/local/var/www/joomlacms/libraries/src/Console/AddUserToGroupCommand.php on line 156

Call Stack:
    0.0024     401936   1. {main}() /usr/local/var/www/joomlacms/cli/joomla.php:0
    0.0397    5791136   2. Joomla\CMS\Application\ConsoleApplication->execute() /usr/local/var/www/joomlacms/cli/joomla.php:69
    0.0641    8331264   3. Joomla\CMS\Application\ConsoleApplication->execute() /usr/local/var/www/joomlacms/libraries/src/Application/ConsoleApplication.php:154
    0.0706    8338160   4. Joomla\CMS\Application\ConsoleApplication->doExecute() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Application.php:411
    0.0706    8338160   5. Joomla\CMS\Application\ConsoleApplication->doExecute() /usr/local/var/www/joomlacms/libraries/src/Application/ConsoleApplication.php:114
    0.0749    8829248   6. Joomla\CMS\Application\ConsoleApplication->runCommand() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Application.php:385
    0.0755    8846520   7. Joomla\CMS\Console\AddUserToGroupCommand->execute() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Application.php:906
    0.0757    8847536   8. Joomla\CMS\Console\AddUserToGroupCommand->doExecute() /usr/local/var/www/joomlacms/libraries/vendor/joomla/console/src/Command/AbstractCommand.php:235
    2.7753   12045304   9. Joomla\CMS\Console\AddUserToGroupCommand->getGroups() /usr/local/var/www/joomlacms/libraries/src/Console/AddUserToGroupCommand.php:89

The last thing is cosmetic, the poison green is hard to read:

The last thing is cosmetic, the poison green is hard to read:

That bit's not the easiest to change, the color styling is all defined as part of the SymfonyStyle class and IIRC most of it was optimized for "default" light background terminals (and yes, I'm fully aware most of the color combinations don't work as well on dark background terminals, I deal with it daily ?).

@sanderpotjer @roland-d thanks for testing.

So I guess it should be done with checking if a Super User is the last one, so this user can't be deleted.
Then with Super Users we should have no more problems.

@roland-d But I do not understand why the user is not linked to any group after your restore the backup.
I try to get the same result, but I can't reproduce this.
I deleted the last Super User and restore the backup, and I am able to login
But by checking if a Super User is the last one this should be fixed.
I will add this.

Also I try to solve the problem with the untranslated error messages.

@roland-d I fixed the output with untranslated strings and also add a check for deleting super users.
So the last one can't be deleted

As @mbabker mention output styling comes directly form Symfony.
At the moment we need to deal with that.

@f-hamel

But I do not understand why the user is not linked to any group after your restore the backup.
Because I only put the entry back in the #__users table and not in the #__user_usergroup_map.

I found the issue, have a look at this line:
https://github.com/f-hamel/joomla-cms/blob/bbf05bbb2e37315dc0b65cc7e7a1ad9d48fb626e/libraries/src/Console/DeleteUserCommand.php#L122

When it enters this line, $db may not have been defined, as such you get the notices.

The super user and untranslated message is fine now. They work good.

This message is also hard to read :)

@roland-d thanks for your feedback.

I will fix that thing with $db

@roland-d Done now $db is define at the vary beginning of the function.
So now there we should be done with the notice.

What do you mean by

This message is also hard to read :)

Style or spelling? Yes, the style could be better. But it's the same as we have with green box.
Just Symfony.

It you want an other message, what should it be?
Any suggestions?

Style or spelling?
Just the style. Spelling is fine.

The $db issue is solved. Thanks.

Next thing I did is look for consistency in messages.

$ php joomla.php user:delete

 Please enter a username:
 > hamel

Delete users
============

                                                                                                                     
 [ERROR] hamel does not exist! 

$ php joomla.php user:removefromgroup

 Please enter a username:
 > hamel

Remove user from group
======================

$ php joomla.php user:reset-password 

 Please enter a username:
 > hamel

 Please enter a password:
 > 

Change password
===============

$ php joomla.php user:addtogroup    

 Please enter a username:
 > hamel

Add user to group
=================

As you can see only the delete user reports back that the user doesn't exist. I think we need the same for the other three commands. Now we do not know what is happening. At least I think it would be good to have some visual confirmation :)

@roland-d Oh. I guess I was missing this check.
Now we are checking if the user exist.

Sorry guys. Accidentally closed this.
I reopened it ;-)

j4i @brianteeman if you review a line you have a "suggest" button in the github editor, if you use this it copies the code into the editor window and you can change it there. Then the PR author can merge your change with one click

@HLeithner I am well aware of that

or do pr against the pr submitter branch, and yes i know you already know it ?

Or don't waste my time

away from my goals to waste someone time .... sorry ?

sorry for intruding but maybe
if (Access::checkGroup($groupId, 'core.admin'))
{
$query->bind(':groupId', $groupId);
$db->setQuery($query);
$count = $db->loadResult();
if ($count < 2)
{
$this->ioStyle->error("Last super user can't be deleted! At least one super user needs to exist!");
return 1;
}
}
needs an additional check if the only super user that stays is enabled?
I didn't test it just a thought after seeing the code (and i have a site with disabled super users)

@peteruoi Good catch.

@peteruoi Thank you for this important hint.
Now we are checking if there are less than two active super user and if the user you want to delete is the active one you can't delete it.

If you want to delete the inactive super user, this is still possible.
Also deleting the last super user is not possible.

So we should be save of deleting the last or last active super user

@peteruoi Thank you for this important hint.
Now we are checking if there are less than two active super user and if the user you want to delete is the active one you can't delete it.

If you want to delete the inactive super user, this is still possible.
Also deleting the last super user is not possible.

So we should be save of deleting the last or last active super user

I don't know if what i am gonna say makes sense but i 'll say it anyway :)
First with this change the algorithm seems to work so great job!
I don't know if maybe you could move the && $user->block == 0 above the foreach and save some queries?
I mean if the user is disabled just delete it!

With this algorithm you are able to delete a "disabled" user. This works fine.

The user-Object is defined above the foreach statement and no query is executed for this.

What this is doing here is to check all groups of a user and if one of them is core.admin (super user).
This because the could have a different name, should be rare but possibile. Then we counting all active users in this group.

If there is less than two active users in core.admin and the user you want to delete is in core.admin and it's not blocked (means this user active is active one left), it's not possible to delete it.

So this why we need this inside the foreach.

With this algorithm you are able to delete a "disabled" user. This works fine.

The user-Object is defined above the foreach statement and no query is executed for this.

What this is doing here is to check all groups of a user and if one of them is core.admin (super user).
This because the could have a different name, should be rare but possibile. Then we counting all active users in this group.

If there is less than two and the user you want to delete one which is in core.admin and it's not blocked (means active), it's not possible to delete it.

So this why we need this inside the foreach.

I don't think i made myself clear.
if user_to_be_deleted ==enabled
foreach
{
...what you already do without the check if user is enabled cause we know it is...
}
else // the else is optional since he would return 1 in case he couldn't delete it
delete

With this way i think you save the foreach query in case the given user is disabled.

Maybe i am just paranoid :)
Great job!!!

@peteruoi Thank you. Now I understand.

I've fixed that in the way you mention.

Just checking if user is blocked, if yes go ahead and delete it.
If not, check his groups and if one is core.admin, check if there are less than two active user of this group. If so, then do not delete the user.

So at the moment I can't test it because PR #24803 gets the ConsoleApplication broken.
As soon we have a solution I go back on this.

@peteruoi Just pushed it now.
I guess that is way you would like to have the check.
This should really save some queries.

It seems ok now! I am sorry i cannot test it in a practical way.
Good job!

@peteruoi Just pushed it now.
I guess that is way you would like to have the check.
This should really save some queries.

@HLeithner I solved your requested changes.
Could you confirm and approve them?

@wilsonge maybe we could merge this then?

Please, this branch needs an update to be tested.

@twister65 All updated.

Please, note that Joomla also supports PostgreSQL database:
https://github.com/joomla/joomla-cms/pull/20095/files#r300880121

Please, note that Joomla also supports PostgreSQL database:
https://github.com/joomla/joomla-cms/pull/20095/files#r300880121

@twister65 any have you any idea how to solve this on a proper way?

For PostgreSQL, you can use string_agg:
https://www.postgresql.org/docs/9.6/functions-aggregate.html
You need to add it in joomla-framework/database:
https://github.com/joomla-framework/database
See https://github.com/joomla/joomla-cms/pull/20095/files#r300991389

The PR joomla-framework/database#176 has been merged in 2.0-dev branch, but it is not present in https://developer.joomla.org/nightlies/Joomla_4.0.0-alpha11-dev-Development-Full_Package.zip.
Anyone have an idea about it ?

#25498 has to be merged.

sorry guys i will close this PR. Because i destroy the my repo.
I'll open a new one.

Also sorry for mess up this and request unnecessary reviews.
I was trying to fix some commiter-mail in the history cause my git config was wrong sometimes. This destroyed the whole pr. I guess I haved learnd my lessen.