ACL integration into the CLI

[CMCDragonkai]

Specification

• The ACL stores permissions for gestalts and actions
• The CLI should expose the ACL permissions/actions to the pk identities for gestalts and pk vaults for vaults
• This means being able to set permissions and to show/introspect the permissions available
• The permissions are structured as sets of actions
  • Gestalts:
    • notify
    • scan
  • Vaults:
    • pull
    • clone
• Users must be able to set a permission idempotently, and to unset a permission idempotently
• Setting and unsetting of permissions to should map to semantic subcommands
• Or the setting and unsetting of permissions can be its own acl-specific subcommands

Actions

The actions we may want to take are:

• set, Set a specific permission, EG notify, scan.
• del, delete permissions.
• get, get the permission that is set,
• list, List all that have permissions, maybe list specific permissions too?

Command domain.

We can add the commands to the vaults and identities domains. the commands in this case will take the form of.
PK gestalts trust set <ID> <permission>or
PK gestalts trust --action set --ID <ID> --permmission <permission>.

However we can make a new command domain just for trust and have a subcommand for each action.
PK trust setGestalt --ID <ID> --permission <permission>
PK trust listGestalt --ID <ID> --permission <permission>

Tasks

1. Determine if we want to create a trust command domain or add trust commands to vaults and identities.
2. create GRPC commands.
   • identities
     • getGestaltActionsByNode
     • getGestaltActionsByIdentity
     • setGestaltActionByNode
     • setGestaltActionByIdentity
     • unsetGestaltActionByNode
     • unsetGestaltActionByIdentity
   • vaults
     • vaultsShare
     • vaultsPermissions
3. create CLI commands.
   • (subject to change based on decision)
     • pk identities trust: handles set, delete, get
     • pk identities allow: fine grain permissions, set, delete, get
     • pk identities list: list gestalts + permissions.
4. tests
   • grpc
     • getGestaltActionsByNode
     • getGestaltActionsByIdentity
     • setGestaltActionByNode
     • setGestaltActionByIdentity
     • unsetGestaltActionByNode
     • unsetGestaltActionByIdentity
     • vaultsShare
     • vaultsPermissions
   • CLI
     • pk identities trust
     • pk identities allow
     • pk identities list
     • vaults share (set, unset)
     • vaults Permissions (get)

notes:

• pk identities list-trust: list permissions. Might be part of identities list. We'd have to list complete gestalts to have a complete picture anyway.

[CMCDragonkai]

Assigning this to you @tegefaulkes since this should give you a full domain to work into the GUI as well after you get it running in the CLI.

[CMCDragonkai]

Please help spec out the tasks above. @tegefaulkes

[tegefaulkes]

Expanded spec and updated tasks.

For the commands, do we want them inside the vaults and identities domains or keep them all inside its own trust domain?
I'm not quite up to speed with Commander so I'm not sure what would be easier to work with.

[CMCDragonkai]

In the GG we now have:

• getGestaltActionsByNode
• getGestaltActionsByIdentity
• setGestaltActionByNode
• setGestaltActionByIdentity
• unsetGestaltActionByNode
• unsetGestaltActionByIdentity

I believe these 6 functions should be exposed directly to GRPC. So in the client service there should be 6 calls one for one here.

As for trust/untrust. Let's think about this.

First there's the current 2 actions:

type GestaltAction = 'notify' | 'scan';
type GestaltActions = Partial<Record<GestaltAction, null>>;

So trust actually translates to setting and unsetting the notify permission.

I think to be flexible here, we need both a high level subcommand called trust and untrust that just sets the notify, but a lower level acl related subcommands that allow one to set specific action permissions and unset specific action permissions.

We decided before not to have gestalts subcommand in the CLI, and gestalts subcommands are instead under the identities subcommand.

Therefore one can imagine:

# you will need a way to disambiguate between nodeids and providerid + identityid
pk identities trust <NODEID>
pk identities untrust <NODEID>
pk identities trust <PROVIDERID> <IDENTITYID>
pk identities untrust <PROVIDERID><IDENTITYID>

# examples
pk identities trust ds98fjsfjo3eiw4uoiu4dsfsdf
pk identities untrust github.com cmcdragonkai

So then for the specific action setting and unsetting:

pk identities allow <NODEID> notify
pk identities disallow <PROVIDERID> <IDENTITYID> scan

[CMCDragonkai]

@scottmmorris

From the vaults point of view, there's not high level trust/untrust. But there is a high level share/unshare concept. So share/unshare is like trust/untrust in the sense that it's a high level concept that maps to a simpler permission/action setting, which is just about allowing the other side to pull or clone the vault.

pk vaults share ...
pk vaults unshare ...

And the low level commands for ACL manipulation can be the same as before:

pk vaults allow ...
pk vaults disallow ...

Please indicate what:

• parameters are flags - by default they should be false, if set, they are true
• parameters are options - like key value things, but are usually meant to be optional
• parameters are required - should be positional here

[CMCDragonkai]

@tegefaulkes also see (with respect to vault permission actions):

Suggest lifting the permission enforcement to higher level rather than GitManager. I reckon either in VaultManager or in the agent service. Permission enforcement are like business rules. They should be higher level as they change a lot. GitManager should be a low level mechanism.

[tegefaulkes]

Do we want the ability to set and unset multiple permissions at a time?
It should be a simple change since the last argument can be variadic.
eg pk identities trust <nodeId> notify scan

Thoughts? @CMCDragonkai

[CMCDragonkai]

Let's stick with simple first, and see how it works out then we can add extensions subsequently.

…

On 7/11/21 5:10 PM, Brian Botha wrote: Do we want the ability to set and unset multiple permissions at a time? It should be a simple change since the last argument can be variadic. eg |pk identities trust <nodeId> notify scan| Thoughts? @CMCDragonkai <https://github.com/CMCDragonkai> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE4OHJIVY5FSVWGQB7YI73TXE7XPANCNFSM5ACD5W7Q>.

[tegefaulkes]

There is an oddity with types when using the 'as' keyword.
The 'as' keyword doesn't provide any checking, so if we cast a string to an union type of strings, it will not check if the string is valid.
For example...

type GestaltAction = 'notify' | 'scan';

function doThingWithAction(action: GestaltAction) {
    //Do things...
}

doThingWithAction('invalidString'); //Argument of type '"invalidString"' is not assignable to parameter of type 'GestaltAction'.
doThingWithAction('invalidString' as GestaltAction); //no complaint.

Is there a good way to deal with this?
After a quick google search I can't find a quick and easy way to check if a string is valid for such a type.
Right now I can check it with something like if (!["notify", "scan"].includes(action)) throw new ErrorGestaltsInvalidAction(Invalid action: ${action}); but this is a horrible solution.

update:
I've created a utility function to check if the string is valid. However the function needs to be updated when the GestaltAction type changes.
The problem is there is no easy way to get a list of valid strings directly from the type definition. need to look into this more.

Thoughts? @CMCDragonkai

[CMCDragonkai]

This is fine. We only use `as` when using it as a smart constructor or when we need to get around the type checker.

…

On 7/12/21 4:20 PM, Brian Botha wrote: There is an oddity with types when using the 'as' keyword. The 'as' keyword doesn't provide any checking, so if we cast a string to an union type of strings, it will not check if the string is valid. For example... type GestaltAction = 'notify' | 'scan'; function doThingWithAction(action:GestaltAction) { //Do things... } doThingWithAction('invalidString'); //Argument of type '"invalidString"' is not assignable to parameter of type 'GestaltAction'. doThingWithAction('invalidString' as GestaltAction); //no complaint. Is there a good way to deal with this? After a quick google search I can't find a quick and easy way to check if a string is valid for such a type. Right now I can check it with something like |if (!["notify", "scan"].includes(action)) throw new ErrorGestaltsInvalidAction(|Invalid action: ${action}|);| but this is a horrible solution. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE4OHKOCDTHV7NUIQRPCWLTXKCSBANCNFSM5ACD5W7Q>.

[tegefaulkes]

Added the identities trust and allow command with the following format.

pk identities allow <id> <permission>
arguments:
<id>: The NodeID or IdentityID if --identity is set.
<permission>: Permission to be set/unset, either scan or notify.

flags:
'-d, --delete', when set deletes the permission.

options:
'-i --identity <providerId>, Flags that the operation is using an identity and sets the ProviderId.

examples:
pk identities allow <NodeId> scan //Sets scan permission for NodeId
pk identities allow --identity github.com tegefaulkes notify //sets notify permission for the identity tegefaulkes on github.
pk identities allow -d <nodeId> notify //deletes the notify permission for NodeId.

---
pk identities trust <id>
arguments:
<id>: The NodeID or IdentityID if --identity is set.

flags:
'-d, --delete', when set deletes the permission.
'-g, --get', when set, prints permissions. supersedes -d

options:
'-i --identity <providerId>, Flags that the operation is using an identity and sets the ProviderId.

examples:
pk identities trust <NodeId> //Sets notify permission for NodeId
pk identities trust --identity github.com tegefaulkes //sets notify permission for the identity tegefaulkes on github.
pk identities trust -d <nodeId> //deletes the notify permission for NodeId.
pk identities trust -g <nodeID> //Prints the set permissions for nodeId.

This gives us a simple way of setting and unsetting via Node or Identity with one command.

[tegefaulkes]

Updated checklist for vault permissions.

[tegefaulkes]

GRPC functions for

• setVaultPerm
• unsetVaultPerm
• getVaultPermissions

Already exist as

• vaultsShare (set, unset)
• vaultsPermissions (get)

No tests for them exist yet.

[tegefaulkes]

Added tests for the GRPC calls

• vaultsShare
• vaultsPermissions

CLI commands and tests already existed for

• vaults share
• vaults permissions

[tegefaulkes]

I updated the gestalts list command to show permissions per gestalt. Created a test for it too.

[CMCDragonkai]

For @scottmmorris to review as well.

[tegefaulkes]

Changes:

For all commands that took identitiy as an input:
I removed the --identity <providerId> option and identity is now provided as
identities allow github.com:username.
The command will parse that as an identitiy instead of a nodeId.

split up identities allow command into:

• identities allow <id> <permission>
• identities disallow <id> <permission>

split up identities trust command into:

• identities trust <id>
• identities untrust <id>
• identities perms <id>

split up vaults share command into:

• vaults share <vaultId> [nodeIds...]
• vaults unshare <vaultId> [nodeIds...]

[CMCDragonkai]

Things to left to do:

1. Squash and rebase
2. Resolve merge conflicts between session management and bin CLI commands and as well as IdentitisManager construction
3. Resolve testing using pk and using jest-mock-process to test the STDOUT and STDERR
4. Final few commands authenticate and claim
5. GRPC commands - session management

Probably by Monday ready to merge.

[CMCDragonkai]

Are we on track for Monday EOD merge?

[CMCDragonkai]

We can leave the nodes related work to be merged after @joshuakarp merges his gestalts work. Node related identities work/acl/gestalts is going to the Nodes CLI MR.

[tegefaulkes]

Changes here have been merged in with identities-cli.
Closing issue.