search

cyberark / cyberark-conjur-cli

CyberArk Conjur command line interface written in Python
https://www.conjur.org
Apache License 2.0
17 stars 16 forks source link

Role command #217

Open InbalZilberman opened 3 years ago

InbalZilberman commented 3 years ago

Feature Overview & Customer Need

As a Conjur user I would like to use the Conjur cli In order to understand which roles are members of a specific role

A new command will be called role We will use 1 sub command

For example

conjur role list-members -i demo:policy:server <- returns array of roles [ ...]

Out of scope

The commands will be used as follow:

TBD

Args description:

-i --id - provide the role identifier

Expected behavior should not change from the corresponding command in Ruby CLI @sharonr78 should provide the right help screen Help is according to https://ljfz3b.axshare.com/#id=s9nycf&p=conjur_help__policy&g=1

Quality

Make sure we have test coverage of the role command. Create test plan and execute accordingly.

Process logic and Demo

Conjur user with a machine that the Python CLI already been installed and conjur init & login run against a Conjur/ Conjur Enterprise machine with the following: Given the following roles when performing 'conjur list' [ "MyAccount:policy:root", "MyAccount:user:alice", "MyAccount:user:bob", "MyAccount:group:conjur-root-admins", "MyAccount:group:ops-admin", "MyAccount:host:www-01", "MyAccount:layer:app-layer", "MyAccount:host:bob_machine" ] If the user performs

conjur role list-members -i MyAccount:group:conjur-root-admins [ "MyAccount:user:admin", "MyAccount:user:alice" ]

Demo each option that was implemented Show that the role has members

Delete a role by using !delete in a policy Show that the option of running members is failing.

User messages

All user messages regarding resource actions should be reviewed

Especially error messages

if an argument is missing we need to return the help of the command

Documentation

Please provide enhance documentation in online help and readme

DOD

sharonr78 commented 3 years ago

Hi @InbalZilberman Regarding the subcommands:

  1. exists - Determines whether a role exists - why shouldn't we call the subcommand validate? all our subcommands are actually verbs, and "exists" is not.
  2. members - Lists all direct members of the role - should also be changed to a verb. We can use list.
  3. The flag -r, --role should also be changed to -i, --id like we use in variable, host or user for identification.

The syntax would then be: conjur role validate -i demo:policy:server conjur role list -i demo:policy:server

I don't think we need the quotes, unless there's a technical limitation here.

WDYT?

InbalZilberman commented 3 years ago
  1. @sharonr78 exists is a verb. Validate does not have the same meaning as exist... Maybe we can consult TW here.
  2. @sharonr78 we cant use simply list how about members list? it does become a bit long conjur role members list -i demo:policy:server 3 I agree thanks! changed
  3. quotes should be supported for all cases as I believe there are special chars that will require this.
szamir1 commented 3 years ago

@InbalZilberman

  1. Why list membership is out of scope?
  2. added this under the logic section: given the following roles when performing 'conjur list' [ "MyAccount:policy:root", "MyAccount:user:alice", "MyAccount:user:bob", "MyAccount:group:conjur-root-admins", "MyAccount:group:ops-admin", "MyAccount:host:www-01", "MyAccount:layer:app-layer", "MyAccount:host:bob_machine" ] If the user performs conjur role exists -i MyAccount:group:conjur-root-admins true conjur role members -i MyAccount:user:conjur-root-admins false conjur role members -i MyAccount:group:conjur-root-admins [ "MyAccount:user:admin", "MyAccount:user:alice" ] conjur role membership -i MyAccount:group:conjur-root-admins [ "MyAccount:host:sapirs_machine", "MyAccount:group:conjur-root-admins" ]
InbalZilberman commented 3 years ago

@szamir1 Thanks! Ill add this to the logic! membership is out of scope as in the customer research we conducted it seems that it is not used.

sharonr78 commented 3 years ago
  1. @sharonr78 exists is a verb. Validate does not have the same meaning as exist... Maybe we can consult TW here.
  2. @sharonr78 we cant use simply list how about members list? it does become a bit long conjur role members list -i demo:policy:server 3 I agree thanks! changed
  3. quotes should be supported for all cases as I believe there are special chars that will require this.

@InbalZilberman How about:

  1. role check -i ... or role verify -i ...? since we're checking/querying/verifying whether that role exists. The true/false response still works here.
  2. role list-members -i ... I'm trying to find a verb that will be a clear sub-command after role.

Does DavidP need an invite to this repo?

InbalZilberman commented 3 years ago

Same as in issue https://github.com/cyberark/conjur-api-python3/issues/215 list-members fine i think exists provide better readability and is a verb

sharonr78 commented 3 years ago

I suggest we consult @pdavid2 or @shulifink in this matter. David/Shuli, we would like to have a subcommand for the commands "Role" and "Resource" which determines whether a role exists or not (so the output would no True/False). Since a subcommand should also be a verb (and I think it should be the infinitive form), should we use "exist" or "exists" or maybe a different word to represent this action. For instance: conjur role exists -i "demo:policy:server" (where -i is the role's identifier)

sigalsax commented 3 years ago

@InbalZilberman I am not really understanding the significance of exists because the user can just run conjur list and see that the resource there

also for your examples, it is enough to provide resource: so policy:server

sigalsax commented 3 years ago

for list-members will we support flags (limit, count, etc)? See REST here https://docs.conjur.org/Latest/en/Content/Developer/Conjur_API_List_Role_Members.htm?tocpath=Developer%7CREST%C2%A0APIs%7C_____16

sigalsax commented 3 years ago

I think the language is very confusing. Are we listing the entities associated with groupings? For example, if the group mygroup, there are two entities sigal and inbal? or are we talking about ownership? as in mygroup owns sigal and inbal? could very well be both but it should be explicit and clear

InbalZilberman commented 3 years ago

for list-members will we support flags (limit, count, etc)? See REST here https://docs.conjur.org/Latest/en/Content/Developer/Conjur_API_List_Role_Members.htm?tocpath=Developer%7CREST%C2%A0APIs%7C_____16

No

InbalZilberman commented 3 years ago

I think the language is very confusing. Are we listing the entities associated with groupings? For example, if the group mygroup, there are two entities sigal and inbal? or are we talking about ownership? as in mygroup owns sigal and inbal? could very well be both but it should be explicit and clear

I will go over the language with @shulifink and make sure it is fixed in help and in our documentation

InbalZilberman commented 3 years ago

@InbalZilberman I am not really understanding the significance of exists because the user can just run conjur list and see that the resource there

also for your examples, it is enough to provide resource: so policy:server

Thank you for challenging! We decided to remove the exists option