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.

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

Add new commands for user management #109

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Dec 6, 2022

  • Added deleteUser for deleting existing users on the Nextcloud instance
  • Added listUsers for querying registered user IDs
  • Added modifyUser to change user metadata
  • Added userData to query user metadata (added mainly to test modifyUser)

This commands are currently used at least by the text app, but they are commonly useful also for other apps.
So I guess it would be best to integrate them into this library as a part of nextcloud/text#2201 .

@susnux susnux added 3. to review Waiting for reviews enhancement New feature request labels Dec 6, 2022
@susnux susnux requested a review from skjnldsv December 6, 2022 10:51
lib/commands/users.ts Outdated Show resolved Hide resolved
lib/commands/users.ts Outdated Show resolved Hide resolved
lib/commands/users.ts Outdated Show resolved Hide resolved
lib/commands/users.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

See comments :)

@susnux susnux force-pushed the feat/user-management branch 2 times, most recently from d59a4ab to f54ff6d Compare December 6, 2022 15:45
@susnux susnux requested a review from skjnldsv December 6, 2022 15:45
Copy link
Contributor

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

A few nitpicks

lib/index.ts Outdated Show resolved Hide resolved
lib/commands/users.ts Outdated Show resolved Hide resolved
@skjnldsv
Copy link
Contributor

skjnldsv commented Dec 6, 2022

On another note, i see some commands clear the session, some don't.
Some might even log in as a user and not reset it afterwards.

Can we make some ground rules?

  1. If the current session will be reset, have a dedicated warning jsdoc comment on the method
  2. If the command sends a request as an admin make sure we reset the cookies/session after to prevent re-using it or mishaps
  3. Anything else?

cc @max-nextcloud @juliushaertl

* Added `deleteUser` for deleting existing users on the Nextcloud instance
* Added `listUsers` for querying registered user IDs
* Added `modifyUser` to change user metadata
* Added `userData` to query user metadata (added mainly to test `modifyUser`)

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux
Copy link
Contributor Author

susnux commented Dec 6, 2022

Fixed the remaining comments, and also

1. If the current session will be reset, have a dedicated warning jsdoc comment on the method

Added such a warning for all methods which reset the cookies

2. If the command sends a request as an admin make sure we reset the cookies/session after to prevent re-using it or mishaps

This is already done

@skjnldsv
Copy link
Contributor

skjnldsv commented Dec 6, 2022

@susnux I meant more generally speaking, adding some documentation in the readme maybe when we have time :)

But thanks for doing this right now!! You're awesome! Great addition!! 🚀 🪅

@skjnldsv skjnldsv merged commit c2141c4 into master Dec 6, 2022
@skjnldsv skjnldsv deleted the feat/user-management branch December 6, 2022 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants