Add functions to manage Acl (super-)admins

[mooori]

This PR builds on top of #5 and adds Acl functions related to (super-)admin permissions. A super-admin may act as admin for every role, these tests provide an overview of super-admin permissions.

Overview

• Trait methods are added to AccessControllable.
• The attribute #[access_controllable] adds implementations of these methods.
• The new methods are tested in near-plugins/tests/access_controllable.rs

# Command to execute the tests:
cargo test --test access_controllable

# This makes `workspaces` write more than 1G to /tmp (on my machine).
# Depending on the size of your /tmp, a cleanup might be required afterwards.
du --summarize --total --human-readable /tmp/sandbox-*
rm -r /tmp/sandbox-*

Notes on super-admin

Currently only acl_init_super_admin is added to the public interface, i.e. only the contract itself may add a super-admin. Potential further additions could be:

Allow super-admin to add/revoke other accounts as super-admin.
- acl_add_super_admin(account_id)
- acl_revoke_super_admin(account_id)

Allow a super-admin to renounce their super-admin permission.
- acl_renounce_super_admin()

All of the above are basically just wrappers for
acl_{add, revoke}_super_admin_unchecked which verify the permission of the
caller. Similar to how `acl_add_admin` is a simple wrapper around
`acl_add_admin_unchecked`.

Next up

A follow-up PR to add some missing functions related to roles:

acl_grant_role
acl_revoke_role[_unchecked]
acl_renounce_role

[mfornet]

FYI: project doesn't compile for me when doing cargo test

[mooori]

FYI: project doesn't compile for me when doing cargo test

Hm on my machine it compiles and all tests are passing. I'm having near-sdk version 4.0.0 in Cargo.lock, is it the same for you? There might be issues when locally 4.0.0.-pre* is installed.

If it's not related to the version of near-sdk, could you please post more infos about the compilation error?

[mooori]

I'm in the process of removing the *_unchecked methods and noticed that:

Tests rely on calling *_unchecked methods

They have been since the first Acl PR. An easy fix is manually exporting *_unchecked methods in the test contract.

// near-plugins/tests/contracts/access_controllable/src/lib.rs

// Helpers to let integration tests call *_unchecked methods.
#[near_bindgen]
impl StatusMessage {
    #[private]
    pub fn acl_add_admin_unchecked(&mut self, role: Role, account_id: AccountId) -> bool {
        self.__acl.add_admin_unchecked(role, &account_id)
    }

    // same for the other *_unchecked methods
}

Developers can no longer use *_unchecked for initialization

Hence they would have to call internal methods, for instance:

#[near_bindgen]
impl Contract {
    #[init]
    pub fn new() -> Self {
        self.__acl.add_super_admin_unchecked(&env::currenct_account_id());
        // ...
    }
}

To avoid that, a workaround could be having some method like this in the trait:

/// Makes the contract a super-admin. It is #[private] in the implementation
/// provided by this trait, i.e. only the contract itself may call this method.
fn acl_make_self_super_admin();

Once the contract is super-admin, it can call all checked methods like acl_add_admin and acl_grant_role.

---

Side note: OpenZeppelin Acl has unchecked methods, e.g. _grant_role().

[mfornet]

IMO, unchecked methods are an internal detail about our default implementation and not mandatory for every other implementation out there. It is conceivable that other implementations use different mechanisms internally. Arguably unchecked-like methods will be present one way or another, but we should try to keep the main interface as simple as possible.

Side note: OpenZeppelin Acl has unchecked methods, e.g. _grant_role().

This is a great example of what I try to convey. _grantRole is only part of the default implementation provided by OpenZeppelin, but it is not callable or present in the global Access Control Interface (IAccessControl).

Tests rely on calling *_unchecked methods

We can keep this method behind a testing feature flag.

Developers can no longer use *_unchecked for initialization Proposal: fn acl_make_self_super_admin();

I like the new proposal, even we can take it further and have the method:

fn acl_init_super_admin(accountId: AccountId);

This method can only be called once and will add the account passed as an argument as super admin. In particular, it is mandatory (or strongly recommended) to developers that this is called during initialization.

[mfornet]

Hm on my machine it compiles and all tests are passing. I'm having near-sdk version 4.0.0 in Cargo.lock, is it the same for you? There might be issues when locally 4.0.0.-pre* is installed.

If it's not related to the version of near-sdk, could you please post more infos about the compilation error?

Check compilation-error-logs and Cargo.lock.

[mooori]

fn acl_init_super_admin(accountId: AccountId);

This method can only be called once and will add the account passed as an argument as super admin.

Do we want it to be callable strictly once? I think that would require a new field like initiated: bool on the Acl struct.

Alternatively we might relax it a bit and say acl_init_super_admin succeeds only if there are no super-admins. This condition is met during initialization and additionally when a contract has (by mistake) removed all super-admins. So it's kind of a recovery mechanism.

[mooori]

Regarding the compilation errors: After removing Cargo.lock in addition to cargo clean I could reproduce the errors. Skimming over them, looks like they're all in dependencies. I'm looking into it...

[mooori]

Regarding the compilation errors: After removing Cargo.lock in addition to cargo clean I could reproduce the errors. Skimming over them, looks like they're all in dependencies. I'm looking into it...

Running cargo test in a fresh directory pulled in different, inconsistent versions of near* dependencies. Updating the workspaces dev-dependency to the latest version should fix it (done in the latest commit).