Permissions are inaccurate

[kernelwhisperer]

from @lkngtn:

After installing an app from the CLI the application’s permissions indicate both the applications Roles as having Any Entity set as their Allowed Entities and their Manager.

This is confusing (and inaccurate) as any entity cannot manage the role, and any entity cannot perform the action.

This will be fixed by #287 , #288 or #289

[kernelwhisperer]

Short-term solution:

#289 and #288 proposes we don't set permissions, possibly check the roles and throw when the app does not have any roles. This would be the safest and easiest, but not the nicest.

This is already possible with the --set-permissions false flag, introduced in #270
E.g.:

• dao create new -> Created DAO: 0x5b6a3301a67A4bfda9D3a528CaD34cac6e7F8070
• dao install 0x5b6a3301a67A4bfda9D3a528CaD34cac6e7F8070 foo.aragonpm.eth --set-permissions false this outputs:

 Successfully executed: "Create a new upgradeable instance of 0x3c681e874ad40688fa93222f494450207493d525619db9035f2c0d608c1b1b1e app linked to the Kernel, setting its code to 0x6Dc9E61aed9c8018db22e77730e515D42AD1375a ."
 ⚠ After the app instance is created, you will need to assign permissions to it for it appear as an app in the DAO

This assumes you deployed the counter app previously:

• aragon init foo
• cd foo
• aragon apm publish minor -> Contract address: 0x6Dc9E61aed9c8018db22e77730e515D42AD1375a

The only problem with this is that you don't know the new instance address to set permissions.
What you need to do is check the NewAppProxy event from the kernel:

• with a block explorer, e.g.: https://rinkeby.etherscan.io/address/DAO_ADDRESS#events
• running this task: Fetching deployed app

Assuming you know the instance address, you can proceed:

• dao acl create 0x5b6a3301a67A4bfda9D3a528CaD34cac6e7F8070 0x634FAA183ba1f5f968cB96656D24dFf66021f5A2 INCREMENT_ROLE 0xb4124cEB3451635DAcedd11767f004d8a28c6eE7 0xb4124cEB3451635DAcedd11767f004d8a28c6eE7

Now the app shows in the dapp, with the main account 0xb41.. being manager and allowed for incrementing.

I think the only improvement we can make here is finding the instance address easier.

[kernelwhisperer]

Long-term solution:

Installing the app and setting the "correct" permissions:

Current situation:
dao install 0x5b6a3301a67A4bfda9D3a528CaD34cac6e7F8070 foo.aragonpm.eth
results in:

• any account can increment/decrement
• any account is manager (cannot be changed, also dangerous for production)

#287 proposes that we install apps with "open" permissions and no manager.
E.g.: If you install the counter-app:

• any account can increment/decrement
• only the account with the "Create permissions" role from the ACL app can change this (by initializing the role with a manager)

Even though this is very nice for developing, using this in production I suppose is undesirable as anyone has a short window of using the app in a malicious way. (is this why you preferred #289 @bingen ?)

Suggestion:

• set permissions for all roles to the entity installing the app.
• set managers for all roles to the entity installing the app.

E.g.: If you install the counter-app with entity X:

• x can increment/decrement
• x is the manager for all roles

This is nice because if the app is installed by the Voting app, then increments/decrements would go through voting.

This could be a bit more complicated to implement, or as easy as x = ctx.transactionPath[0].from (not sure 😅 )

I would propose we merge #287 since it's an improvement and better than having the manager unchangeable. But this keep issue open ? 🤷‍♂️

[lkngtn]

I think the only improvement we can make here is finding the instance address easier.

If it's possible to retrieve the instance address from the CLI as a minor (and quick improvement), the fact that it is two separate steps is not a huge deal as it adds negligible complexity when scripting (or providing a user with a sequence of commands to run).

I would propose we merge #287 since it's an improvement and better than having the manager unchangeable. But this keep issue open ?

Since #287 could be merged now and would unblock the permissions issue (without needing to workaround the issue using etherscan), I agree we may as well merge. Optimizing the dev experience is more important than optimizing the main-net deployment experience, imo. Until there is a better solution, the best practice for a main-net deployment would be to use --set-permissions false.

Suggestion:

set permissions for all roles to the entity installing the app.
set managers for all roles to the entity installing the app.
E.g.: If you install the counter-app with entity X:

x can increment/decrement
x is the manager for all roles

Agree that this would be ideal!

[bingen]

Yes, I agree too!

using this in production I suppose is undesirable as anyone has a short window of using the app in a malicious way. (is this why you preferred #289 @bingen ?)

Yes, indeed.

[sohkai]

Wondering if we should use a safe default (--set-permissions false), log that it has potentially undesirable effects (e.g. not showing up on the client), and make the option more powerful:

• --set-permissions open: as we have it now, everything is ANY_ENTITY
• --set-permissions installer: the executor of the installation (e.g. voting app)
• --set-permissions interactive: CLI prompts for each role for an address to use

If you're scripting for development, you could simply use --set-permissions open.

[kernelwhisperer]

@sohkai awesome idea!

One question though: are the role parameters something to be concerned about?

    {
      "name": "Modify support",
      "id": "MODIFY_SUPPORT_ROLE",
      "params": [
        "New support",
        "Current support"
      ]
   }

Are these set when creating permissions?

[sohkai]

Are the role parameters something to be concerned about

Eventually, but right now I wouldn't worry about supporting them in a first go. The "interactive" mode (and maybe "installer") could both be extended to also ask for any parameter-based limitations for the permissions it assigns (see parameter docs).

[kernelwhisperer]

I created #337 for the installer and interactive options.
The safe default and open option is implemented in #334.