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: Email option in occ user:add command #40726

Merged
merged 4 commits into from
Feb 26, 2024
Merged

feat: Email option in occ user:add command #40726

merged 4 commits into from
Feb 26, 2024

Conversation

kyteinsky
Copy link
Contributor

@kyteinsky kyteinsky commented Sep 30, 2023 •
edited
Loading

Summary

Builds upon #29368

This adds an occ command option (email) during the addition of a user to auto-generate a password and send a link to the email address to set a new password if:

  1. password-from-env option was not passed
  2. email is given and valid

Docs

Checklist

@kyteinsky kyteinsky mentioned this pull request Sep 30, 2023
Merged
@kyteinsky kyteinsky requested a review from come-nc September 30, 2023 06:36
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 30, 2023
View reviewed changes
core/register_command.php Fixed Show fixed Hide fixed
core/register_command.php Fixed Show fixed Hide fixed
core/Command/User/Add.php Fixed Show fixed Hide fixed
@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Sep 30, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Sep 30, 2023
@szaimen szaimen requested review from nickvergessen, a team, icewind1991 and nfebe and removed request for a team September 30, 2023 07:35
nickvergessen
nickvergessen requested changes Sep 30, 2023
View reviewed changes
core/Command/User/Add.php Outdated Show resolved Hide resolved
tests/Core/Command/User/AddTest.php Outdated Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 30, 2023
View reviewed changes
core/register_command.php Fixed Show fixed Hide fixed
@kyteinsky kyteinsky force-pushed the feat/new-user-email branch 2 times, most recently from fd4822d to b9e4d89 Compare September 30, 2023 21:10
come-nc
come-nc reviewed Oct 2, 2023
View reviewed changes
core/Command/User/Add.php Outdated Show resolved Hide resolved
core/register_command.php Outdated Show resolved Hide resolved
core/Command/User/Add.php Outdated Show resolved Hide resolved
@kyteinsky kyteinsky force-pushed the feat/new-user-email branch from b9e4d89 to f295fde Compare October 2, 2023 08:34
@solracsf solracsf changed the title feat: Email option in occ use:add command feat: Email option in occ user:add command Oct 2, 2023
nickvergessen
nickvergessen approved these changes Oct 4, 2023
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Not blocking, but with the comments the code would be simpler/easier to read in the future

core/Command/User/Add.php Outdated Show resolved Hide resolved
core/Command/User/Add.php Outdated Show resolved Hide resolved
@kyteinsky kyteinsky force-pushed the feat/new-user-email branch from f295fde to ef6691c Compare October 4, 2023 10:00
@kyteinsky kyteinsky requested a review from come-nc October 9, 2023 14:48
come-nc
come-nc requested changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is still not good for when newUser.sendEmail is off, this will generate a random password but not send it.
Using --email should not change how password is handled, unless newUser.sendEmail is on and password-from-env was not given.

This needs to work and set email:
occ user:add --password-from-env --email bob@example.com bob
This needs to work and set email, if newUser.sendEmail is off it should ask for password same as without --email:
occ user:add --email bob@example.com bob

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
This was referenced Nov 6, 2023
Merged
Merged
This was referenced Nov 14, 2023
Merged
Merged
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@helmo helmo mentioned this pull request Feb 1, 2024
Closed
4 tasks
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 23, 2024
@kyteinsky kyteinsky requested a review from come-nc February 23, 2024 23:26
philipreinken and others added 4 commits February 24, 2024 04:56
@philipreinken @kyteinsky
Enable adding E-Mail addresses to new user accounts using the CLI
b587ec3 
Signed-off-by: Philip Gatzka <philip.gatzka@mailbox.org>
@kyteinsky
new user password email option, improved on #29368
a92c507 
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
@helmo @kyteinsky
Allow combining --email option with --password-from-env
bf7e84f 
Signed-off-by: Herman van Rink <rink@initfour.nl>
@kyteinsky
add generate-password option and flow fixes
ce24923 
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
@github-actions GitHub Actions
Copy link
Contributor

Possible performance regression detected

Show Output 
564 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test/new_file.txt
+ /remote.php/dav/files/test added with 45 queries
+ /remote.php/dav/files/test/test.txt added with 33 queries
+ /remote.php/dav/files/test/many_files added with 46 queries
+ /remote.php/dav/files/test/new_file.txt added with 67 queries
+ /remote.php/dav/files/test/new_file.txt added with 91 queries
+ /remote.php/dav/files/test added with 45 queries
+ /remote.php/dav/files/test/test.txt added with 33 queries
+ /remote.php/dav/files/test/many_files added with 46 queries
+ /remote.php/dav/files/test/new_file.txt added with 67 queries
+ /remote.php/dav/files/test/new_file.txt added with 91 queries

@kyteinsky
Copy link
Contributor Author

is performance regression something to worry about? Seems unrelated to this PR.

@come-nc come-nc merged commit 102773a into master Feb 26, 2024
158 of 159 checks passed
@come-nc come-nc deleted the feat/new-user-email branch February 26, 2024 10:50
@come-nc
Copy link
Contributor

come-nc commented Feb 26, 2024

is performance regression something to worry about? Seems unrelated to this PR.

It is unrelated.

@b3nks
Copy link

b3nks commented Feb 26, 2024

Great! Thanks a lot for that! 💙

@blizzz blizzz mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@nickvergessen nickvergessen nickvergessen approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 was automatically assigned from nextcloud/server-backend

@nfebe nfebe Awaiting requested review from nfebe nfebe was automatically assigned from nextcloud/server-backend

Assignees

@kyteinsky kyteinsky

Labels
2. developing Work in progress enhancement
Projects
None yet
Milestone
Nextcloud 29
Development

Successfully merging this pull request may close these issues.

No "Welcome" email sent when using occ.
9 participants
@kyteinsky @come-nc @b3nks @nickvergessen @blizzz @skjnldsv @szaimen @philipreinken @helmo