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

Resource command #215

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 and members have what privileges on Conjur's resources

==================== Initial suggestion ==================== A new command will be called resource We will use sub command

The commands will be used as follow:

conjur resource list-roles -i demo:host:ansible/ansible-01 -p read [ "demo:user:admin", "demo:policy:ansible" ] <- returns a list in the same structure list command will

Args description:

-i --id - provide the resource identifier -p --privilege - provide privilege name

==================== New suggestion ====================

Process logic and Demo

A new command with the name 'list', with the following options:

  1. List the members of the group or layer.
  2. List the permitted members on a certain resource according to the permissions provided.

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 resources: MyAccount:group:conjur-root-admins and the following roles -

[
"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"
]
  1. Members of - will return the members of the group or layer. Permissions are not relevant here.

conjur list --membersof <Group/Layer ID>

Args: --membersof, -m - the group/layer ID the user wants to get its members.

For example: conjur list --membersof MyAccount:group:conjur-root-admins

will return the members of the group conjur-root-admins The output will be:

[
MyAccount:user:Moshe,
MyAccount:user:Vika
]
  1. Permitted members of of - will return a list of permitted members on a certain resource according to the permissions provided.

Conjur list --permitted-members-of <Resource ID> –p <Permission type>

Args: --permitted-members-of, -pm - the permitted members on a certain resource. --permission, -p - the permissions on the certain resource.

For example: Conjur list --permitted-members-of cucumber:variable:secrets/test-variable – p read Will return the roles that has read permissions on test-variable The output will be:

[
"MyAccount:user:admin"
]

Expected behaviour should not change from the corresponding command in Ruby CLI Help is according to XXXXXXXXXX

Failure scenarios

Quality

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

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

Demo

Documentation

Please provide enhance documentation in online help and readme

DOD

szamir1 commented 3 years ago

@InbalZilberman added the following under the logic section:

given The following resources are configured: MyAccount:group:conjur-root-admins

The user wants to check if the resource exists, conjur resource exists -r MyAccount:group:test-group false

conjur resource permitted_roles - i MyAccount:group:conjur-root-admins -p read [ "MyAccount:user:admin" ]

If the user enters a non-existing privileged role, we need to return the help of the command For example: conjur resource permitted_roles -r MyAccount:group:conjur-root-admins -p delete

InbalZilberman commented 3 years ago

Done thank you!!

sharonr78 commented 3 years ago

@InbalZilberman @szamir1

  1. Same as in "role" command, I suggest to replace "exists" with "check" or "verify" since we're checking/querying/verifying whether that resource exists. The true/false response still works here.
  2. As for "permitted_roles", I also suggest to change this sub-command to "list_roles" since it's a verb. I'm not sure we need to specifically use "permitted". We'll have the explanation in the help screen. WDYT?
InbalZilberman commented 3 years ago

@InbalZilberman @szamir1

  1. Same as in "role" command, I suggest to replace "exists" with "check" or "verify" since we're checking/querying/verifying whether that resource exists. The true/false response still works here.
  2. As for "permitted_roles", I also suggest to change this sub-command to "list_roles" since it's a verb. I'm not sure we need to specifically use "permitted". We'll have the explanation in the help screen. WDYT?

I agree with # 2 but regarding exists this is a verb and is much more intuitive than verify.

sharonr78 commented 3 years ago

I agree with # 2 but regarding exists this is a verb and is much more intuitive than verify.

OK thanks Inbal, then we're agreeing that the subcommand for both "Role" and "Resource" commands will be "exist", e.g., conjur resource exist -i "demo:host:ansible/ansible-01"

InbalZilberman commented 3 years ago

I think it should be exists as resource is an it

sigalsax commented 3 years ago

I am not sure why we even need exists because the user can just run conjur list and see the actual item in the server

conjur resource permitted_roles should be conjur resource permitted-roles to follow suite with the rest of the commands like rotate-api-key and change-password

@InbalZilberman

sigalsax commented 3 years ago

@InbalZilberman above it says

list-roles - List roles with a specified privilege on the resource

but in the following section it references permitted_roles. Which one is it?

sigalsax commented 3 years ago

Would be helpful to also link the supporting REST for more detailed info. For example this command's REST is https://docs.conjur.org/Latest/en/Content/Developer/Conjur_API_Check_Permission.htm?tocpath=Developer%7CREST%C2%A0APIs%7C_____21

InbalZilberman commented 3 years ago

@InbalZilberman above it says

list-roles - List roles with a specified privilege on the resource

but in the following section it references permitted_roles. Which one is it?

Fixed, thanks

InbalZilberman commented 3 years ago

I am not sure why we even need exists because the user can just run conjur list and see the actual item in the server

conjur resource permitted_roles should be conjur resource permitted-roles to follow suite with the rest of the commands like rotate-api-key and change-password

@InbalZilberman

@sigalsax I have fixed the doc to list-roles. But now wonder if list-permitted-roles isnt better WDYT? it is long but maybe more comprehensive?

sigalsax commented 3 years ago

I like the idea but I do think it is too long. I think list-roles or show-roles (@sharonr78 ) would suffice here because we have the command and a follow up read execute write flag so it is clear that together we want to see all roles that have read/execute/write on the variable we pass in