Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit review 6.3: No Agent permissions for shareholders #143

Merged
merged 2 commits into from Sep 2, 2019

Conversation

@ajsantander
Copy link
Contributor

commented Aug 29, 2019

Problem:

The Agent's EXECUTE_ROLE and RUN_SCRIPT_ROLE should be considered "executive" roles, and as so should be assigned only to the board and not the shareholders.

Solution:

Assign the roles only to BoardVoting.

Audit issue:

https://github.com/aragonone/aragon-daotemplates-audit-report-2019-08#63-company-board---inconsistent-permissions-in-agent-application

@ajsantander ajsantander requested review from lkngtn and facuspagnuolo Aug 29, 2019

@ajsantander ajsantander changed the title Audit review 6.3: No Agent permissions for ShareVoting in CompanyBoardTemplate Audit review 6.3: No Agent permissions for shareholders Aug 29, 2019

@ajsantander ajsantander changed the base branch from master to audit Aug 29, 2019

@ajsantander ajsantander force-pushed the audit_6.3 branch from 686e410 to 5534c1f Aug 29, 2019

@facuspagnuolo
Copy link
Member

left a comment

LGTM, given that shareholders are still the managers of these permissions, they will be able to grant these permissions to themselves tho, right?

@lkngtn

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

LGTM, given that shareholders are still the managers of these permissions, they will be able to grant these permissions to themselves tho, right?

With atleast one board member (to create the vote) yes, the shareholders are the root authority and could assign these to themselves if they wanted to.

@lkngtn
lkngtn approved these changes Sep 2, 2019
Copy link
Member

left a comment

👍

@ajsantander ajsantander merged commit 9525f70 into audit Sep 2, 2019

4 checks passed

WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@delete-merged-branch delete-merged-branch bot deleted the audit_6.3 branch Sep 2, 2019

_createPermissions(_acl, grantees, _agent, _agent.EXECUTE_ROLE(), _shareVoting);
_createPermissions(_acl, grantees, _agent, _agent.RUN_SCRIPT_ROLE(), _shareVoting);
_acl.createPermission(_boardVoting, _agent, _agent.EXECUTE_ROLE(), _shareVoting);
_acl.createPermission(_boardVoting, _agent, _agent.RUN_SCRIPT_ROLE(), _shareVoting);

This comment has been minimized.

Copy link
@sohkai

sohkai Sep 2, 2019

Member

Could we just use the default _createAgentPermissions() now?

facuspagnuolo added a commit that referenced this pull request Sep 3, 2019
Audit review 6.3: No Agent permissions for shareholders (#143)
* Do not grant Agent permissions to ShareVoting

* Update docs for Agent permissions
facuspagnuolo added a commit that referenced this pull request Sep 4, 2019
Audit review 6.3: No Agent permissions for shareholders (#143)
* Do not grant Agent permissions to ShareVoting

* Update docs for Agent permissions
facuspagnuolo added a commit that referenced this pull request Sep 4, 2019
Audit review 6.3: No Agent permissions for shareholders (#143)
* Do not grant Agent permissions to ShareVoting

* Update docs for Agent permissions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.