tridentli / trident

Trident is a trusted and secure communication platform for enabling better communication between groups of trusted parties
https://trident.li
Apache License 2.0
21 stars 9 forks source link

Command to set user as sysadmin does not work #79

Open mboggess opened 7 years ago

mboggess commented 7 years ago

When trying to give a user sysadmin rights: $ tcli user get sysadmin trident no $ tcli user set sysadmin trident true Updated sysadmin $ tcli user get sysadmin trident no $ tcli user set sysadmin trident yes Updated sysadmin $ tcli user get sysadmin trident no

When trying to take away a user's sysadmin rights: $ tcli user set sysadmin trident no Value for sysadmin was already the requested value $ tcli user set sysadmin trident false Value for sysadmin was already the requested value

The help says: $ tcli user set help Help for user set ... sysadmin <username> <sysadmin> System Administrator - Wether the user is a System Administrator

(Note: "Wether" should be "Whether".)

What does <sysadmin> mean? Is the tcli command expecting a boolean? A username?

mboggess commented 7 years ago

And/or, and this would be preferable, so maybe a different ticket to be made, is there a way, currently, to obtain a list of system administrators and trust group admins?

massar commented 7 years ago

Typos solved in https://github.com/tridentli/pitchfork/issues/91 (pitchfork) -- to be merged.

massar commented 7 years ago

Note that you should use 'tcli system swapadmin' first, you'll then be a sysadmin instead of a regular user and should be able to change permissions.

But there seems to be a catch in the swapadmin construct we added that is clashing with changing other user's permissions. Looking into it.

massar commented 7 years ago

Should be addressed in the above patch; please await merging and release.

mboggess commented 7 years ago

It isn't that the 'tcli system swapadmin' command does not work. I run 'tcli system swapadmin' and that gives permissions to run trust group, mailing list, etc commands. It's the 'tcli user set sysadmin' command that doesn't work (or I don't know how to use that specific command properly). Perhaps this merge fixes that, I just wanted to be clear about the problem we're experiencing.

massar commented 7 years ago

It's the 'tcli user set sysadmin' command that doesn't work (or I don't know how to use that specific command properly).

You are using it correctly, but it is, as you found, broken, thanks for reporting.

Perhaps this merge fixes that, I just wanted to be clear about the problem we're experiencing.

Yep, that merge fixes your problem. It is awaiting review ;)

Apologies for being unclear; I should have written a bit more detail in the last comment.

In short summary: when we fetch a user, in pre-fix we would split the issysadmin/canbeadmin bit inside the user; but then when you fetch another user, that user would also be split and per default not have the issysadmin bit set even though it should have been. By moving the swapadmin logic into ctx where it belongs, this now is not an issue anymore as any fetched user actually matches the database contents.

The meat of the fix is: https://github.com/tridentli/pitchfork/pull/93/commits/46db7e7d485d400280109c7d328c24ee47d2ead2

btw; this as there are a pile of items in review that build upon each other, hence the merge is larger than it should be.

mboggess commented 7 years ago

Okay, got it! Thanks!

mboggess commented 7 years ago

@massar, I know you said you guys have a whole pile of things to review. I see you have a pull request in, do you have a general estimate of when this fix will be merged in? Thanks!