-
Notifications
You must be signed in to change notification settings - Fork 373
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
feat: p/demo/accesscontrol & p/demo/timelock #2307
base: master
Are you sure you want to change the base?
Changes from 68 commits
9ca3745
0748c45
1569339
1a04741
b439489
e7e2a39
d723750
5c2e768
ac70b14
0f4968d
5a30a47
cc86b90
2f7dbcc
5999935
0528c63
374b4f3
b1bf45d
4414bc9
6746863
eebd5c8
d1930b0
5dca52f
1e2a159
a4ae651
7df946a
91a21ea
84a6e30
36102fc
5b04454
dadcc5d
4e1b98e
5364f67
9a23760
7f6e861
863d1c8
59432e0
d08fc5c
13d7a1c
fca1601
bee7811
35279e0
8a7d710
0035a82
8da6e06
dc30291
2b70215
3c3bb84
2ff3f48
d72594a
2dee17b
30866f5
5a87e5b
c79bf43
d6ee9b5
312730a
fc9128e
923168f
6b4175f
1eee065
8b1e104
f57e5a5
d0f317b
e94a0b4
0de057d
f5a716f
d233452
56ba1f1
ae3a3db
973cbdc
af534a0
ca375b8
7bee06e
f905404
86af787
7998a78
e973d18
877536b
1cca09a
2f78427
555c084
26505de
1593f79
cb919ec
97ba66c
fd49b32
000c615
3b4c63a
0ced75a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see we're using However, I think an equally possible flow is that of having a realm which has an access control list. In this case, actually, we shouldn't do any checks on PrevRealm(); the realm can just use it unexported. But I suggest you have an option for the ACL to not have a "owner"; in which case the PrevRealm checks are simply not performed. Allows someone else to compose other rules on top as well. Btw if Roles is meant to be exposed in a realm, then its fields should be unexported. |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,155 @@ | ||||||||
// import "gno.land/p/demo/accesscontrol" | ||||||||
// | ||||||||
// Create a new role with a specific admin. | ||||||||
// adminAddr := std.Address("admin-address") | ||||||||
// role := accesscontrol.NewRole("ExampleRole", adminRole) | ||||||||
// | ||||||||
// Check if an account has a specific role. | ||||||||
// account := std.Address("user-address") | ||||||||
// hasRole := role.HasRole(account) | ||||||||
// | ||||||||
// Grant a role to a specific account. | ||||||||
// role.GrantRole(account) | ||||||||
// | ||||||||
// Revoke a role from a specific account. | ||||||||
// role.RevokeRole(account) | ||||||||
// | ||||||||
// Renounce a role with caller confirmation. | ||||||||
// role.RenounceRole(std.GetOrigCaller()) | ||||||||
// | ||||||||
// Change the admin role for a specific role. | ||||||||
// newAdmin := std.Address("new-admin-address") | ||||||||
// role.SetRoleAdmin(newAdmin) | ||||||||
package accesscontrol | ||||||||
|
||||||||
import ( | ||||||||
"errors" | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
"std" | ||||||||
|
||||||||
"gno.land/p/demo/avl" | ||||||||
) | ||||||||
|
||||||||
var ErrUnauthorized = errors.New("unauthorized; caller is not admin") | ||||||||
|
||||||||
// Role struct to store role information | ||||||||
type Role struct { | ||||||||
Name string | ||||||||
Holders *avl.Tree // -> std.Address -> bool | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
AdminRole std.Address | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
} | ||||||||
|
||||||||
type Roles struct { | ||||||||
AllRoles []*Role | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
A suggestion. You'd then need to add the code to manage UserToRoles, but I think we should support being able to map a user to the roles they have. (Of course, there should be a related function to modify it.) |
||||||||
Admin std.Address | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
} | ||||||||
|
||||||||
// NewRole creates a new instance of Role | ||||||||
func NewRole(name string, adminRole std.Address) *Role { | ||||||||
return &Role{ | ||||||||
Name: name, | ||||||||
Holders: avl.NewTree(), | ||||||||
AdminRole: adminRole, | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
// Method to check if the caller has the admin role and return an error | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
func (r *Role) CallerIsAdmin() error { | ||||||||
caller := std.PrevRealm().Addr() | ||||||||
|
||||||||
if r.AdminRole != caller { | ||||||||
return ErrUnauthorized | ||||||||
} | ||||||||
|
||||||||
return nil | ||||||||
} | ||||||||
|
||||||||
// Method to assert if the caller has the admin role, panics if not | ||||||||
func (r *Role) AssertCallerIsAdmin() { | ||||||||
if err := r.CallerIsAdmin(); err != nil { | ||||||||
panic(err) | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
} | ||||||||
} | ||||||||
|
||||||||
// Method to create a new role within the realm | ||||||||
func (rs *Roles) CreateRole(name string) *Role { | ||||||||
if rs.Admin != std.PrevRealm().Addr() { | ||||||||
panic("accesscontrol: caller does not have the global admin role") | ||||||||
} | ||||||||
|
||||||||
role := NewRole(name, rs.Admin) | ||||||||
rs.AllRoles = append(rs.AllRoles, role) | ||||||||
|
||||||||
std.Emit( | ||||||||
"RoleCreated", | ||||||||
"roleName", name, | ||||||||
"sender", rs.Admin.String(), | ||||||||
) | ||||||||
|
||||||||
return role | ||||||||
} | ||||||||
|
||||||||
// Method to check if an account has a specific role | ||||||||
func (r *Role) HasRole(account std.Address) bool { | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
return r.Holders.Has(account.String()) | ||||||||
} | ||||||||
|
||||||||
// Method to grant a role to an account | ||||||||
func (r *Role) GrantRole(account std.Address) { | ||||||||
r.AssertCallerIsAdmin() | ||||||||
r.Holders.Set(account.String(), true) | ||||||||
|
||||||||
std.Emit( | ||||||||
"RoleGranted", | ||||||||
"roleName", r.Name, | ||||||||
"account", account.String(), | ||||||||
"sender", std.PrevRealm().Addr().String(), | ||||||||
) | ||||||||
} | ||||||||
|
||||||||
// Method to revoke a role from an account | ||||||||
func (r *Role) RevokeRole(account std.Address) { | ||||||||
r.AssertCallerIsAdmin() | ||||||||
r.Holders.Remove(account.String()) | ||||||||
|
||||||||
std.Emit( | ||||||||
"RoleRevoked", | ||||||||
"roleName", r.Name, | ||||||||
"account", account.String(), | ||||||||
"sender", std.PrevRealm().Addr().String(), | ||||||||
) | ||||||||
} | ||||||||
|
||||||||
// Method to renounce a role with caller confirmation | ||||||||
func (r *Role) RenounceRole(callerConfirmation std.Address) error { | ||||||||
caller := std.PrevRealm().Addr() | ||||||||
|
||||||||
if callerConfirmation != caller { | ||||||||
return errors.New("accesscontrol: caller confirmation does not match account") | ||||||||
} | ||||||||
|
||||||||
r.Holders.Remove(caller.String()) | ||||||||
|
||||||||
std.Emit( | ||||||||
"RoleRenounced", | ||||||||
"roleName", r.Name, | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
"account", caller.String(), | ||||||||
"sender", caller.String(), | ||||||||
) | ||||||||
|
||||||||
return nil | ||||||||
} | ||||||||
|
||||||||
// Method to set the admin role for a specific role | ||||||||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
func (r *Role) SetRoleAdmin(adminRole std.Address) { | ||||||||
r.AssertCallerIsAdmin() | ||||||||
|
||||||||
previousAdminRole := r.AdminRole | ||||||||
r.AdminRole = adminRole | ||||||||
|
||||||||
std.Emit( | ||||||||
"RoleSet", | ||||||||
"roleName", r.Name, | ||||||||
"previousAdminRole", previousAdminRole.String(), | ||||||||
"newAdminRole", adminRole.String(), | ||||||||
) | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,228 @@ | ||
package accesscontrol | ||
|
||
import ( | ||
"std" | ||
"testing" | ||
|
||
"gno.land/p/demo/testutils" | ||
"gno.land/p/demo/uassert" | ||
) | ||
|
||
// TestAccessControl verifies the access control functionality. | ||
func TestAccessControl(t *testing.T) { | ||
admin := testutils.TestAddress("admin") | ||
user1 := testutils.TestAddress("user1") | ||
user2 := testutils.TestAddress("user2") | ||
|
||
// Create new RoleData | ||
roleData := NewRole("admin", admin) | ||
|
||
// Check initial admin role | ||
if roleData.AdminRole != admin { | ||
t.Fatalf("expected admin role to be %s, got %s", admin.String(), roleData.AdminRole.String()) | ||
} | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
!uassert.Equal(t, roleData.AdminRole, admin) | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Grant role to user1 | ||
std.TestSetOrigCaller(admin) | ||
roleData.GrantRole(user1) | ||
if !roleData.HasRole(user1) { | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
t.Fatalf("expected user1 to have role") | ||
} | ||
|
||
// Check that user2 does not have the role | ||
if roleData.HasRole(user2) { | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
t.Fatalf("expected user2 not to have role") | ||
} | ||
|
||
// Revoke role from user1 | ||
roleData.RevokeRole(user1) | ||
if roleData.HasRole(user1) { | ||
t.Fatalf("expected user1 not to have role after revocation") | ||
} | ||
|
||
// Grant role to user1 again | ||
roleData.GrantRole(user1) | ||
|
||
// User1 renounces the role | ||
std.TestSetOrigCaller(user1) | ||
roleData.RenounceRole(user1) | ||
if roleData.HasRole(user1) { | ||
t.Fatalf("expected user1 not to have role after renouncing") | ||
} | ||
|
||
// Change admin role to user2 | ||
std.TestSetOrigCaller(admin) | ||
roleData.SetRoleAdmin(user2) | ||
if roleData.AdminRole != user2 { | ||
t.Fatalf("expected admin role to be %s, got %s", user2.String(), roleData.AdminRole.String()) | ||
} | ||
|
||
// User1 (now not admin) tries to grant role to user2, should panic | ||
std.TestSetOrigCaller(user1) | ||
defer func() { | ||
if r := recover(); r == nil { | ||
t.Fatalf("expected panic when non-admin tries to grant role") | ||
} | ||
}() | ||
roleData.GrantRole(user2) | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// TestCreateRole tests the CreateRole method of the RoleData struct | ||
func TestCreateRole(t *testing.T) { | ||
// Simulate the administrator as the current caller | ||
admin := testutils.TestAddress("admin") | ||
std.TestSetOrigCaller(admin) | ||
|
||
// Create roles with the administrator | ||
roles := &Roles{Admin: admin} | ||
|
||
// Create a new role with a new administrator address | ||
newRoleName := "newRole" | ||
newRole := roles.CreateRole(newRoleName) | ||
|
||
// Check that the new role has been created correctly | ||
if newRole.Name != newRoleName { | ||
t.Fatalf("expected new role name to be '%s', got '%s'", newRoleName, newRole.Name) | ||
} | ||
if newRole.AdminRole != admin { | ||
t.Fatalf("expected new role admin role to be %s, got %s", admin.String(), newRole.AdminRole.String()) | ||
} | ||
|
||
// Simulate newAdmin as the current caller | ||
std.TestSetOrigCaller(admin) | ||
|
||
// Explicitly add the role to the new administrator to check functionality | ||
newRole.GrantRole(admin) | ||
|
||
// Check if the new role has been added to the holder | ||
if !newRole.HasRole(admin) { | ||
t.Fatalf("expected new role to be added to the holder") | ||
} | ||
} | ||
|
||
// TestCallerIsAdmin verifies the CallerIsAdmin method. | ||
func TestCallerIsAdmin(t *testing.T) { | ||
adminRole := testutils.TestAddress("admin-address") | ||
roleData := NewRole("ExampleRole", adminRole) | ||
// Call CallerIsAdmin with admin caller | ||
std.TestSetOrigCaller(adminRole) | ||
defer func() { | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if r := recover(); r != nil { | ||
t.Fatalf("expected no panic, got %v", r) | ||
} | ||
}() | ||
roleData.AssertCallerIsAdmin() | ||
// Call CallerIsAdmin with non-admin caller | ||
user := testutils.TestAddress("user-address") | ||
std.TestSetOrigCaller(user) | ||
defer func() { | ||
if r := recover(); r == nil { | ||
t.Fatalf("expected panic, got nil") | ||
} | ||
}() | ||
roleData.AssertCallerIsAdmin() | ||
} | ||
|
||
// Testing the RevokeRole Method for a Non-Admin | ||
func TestRevokeRoleNonAdmin(t *testing.T) { | ||
admin := testutils.TestAddress("admin") | ||
user1 := testutils.TestAddress("user1") | ||
user2 := testutils.TestAddress("user2") | ||
|
||
// Create role data with the administrator | ||
roleData := NewRole("admin", admin) | ||
|
||
// Grant role to user1 | ||
std.TestSetOrigCaller(admin) | ||
roleData.GrantRole(user1) | ||
if !roleData.HasRole(user1) { | ||
t.Fatalf("expected user1 to have role") | ||
} | ||
|
||
// Simulate user2 as the current caller | ||
std.TestSetOrigCaller(user2) | ||
|
||
// Attempting to revoke user1's role as user2 (non-admin) | ||
defer func() { | ||
if r := recover(); r == nil { | ||
t.Fatalf("expected panic when non-admin tries to revoke role") | ||
} | ||
}() | ||
roleData.RevokeRole(user1) | ||
} | ||
|
||
// Testing the RenounceRole method with Invalid Confirmation | ||
func TestRenounceRole(t *testing.T) { | ||
adminRole := testutils.TestAddress("admin-address") | ||
roleData := NewRole("ExampleRole", adminRole) | ||
account := testutils.TestAddress("user-address") | ||
// Simulate the administrator as the current caller | ||
std.TestSetOrigCaller(adminRole) | ||
// Grant role to the account | ||
roleData.GrantRole(account) | ||
// Simulate the account as the current caller | ||
std.TestSetOrigCaller(account) | ||
// Renounce the role | ||
err := roleData.RenounceRole(account) | ||
kazai777 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
t.Fatalf("expected no error, got %v", err) | ||
} | ||
// Check if the account still has the role | ||
hasRole := roleData.HasRole(account) | ||
if hasRole { | ||
t.Fatalf("expected account not to have role after renouncing") | ||
} | ||
} | ||
|
||
// Testing the SetRoleAdmin method with a New Administrator Address | ||
func TestSetRoleAdmin(t *testing.T) { | ||
admin := testutils.TestAddress("admin") | ||
newAdmin := testutils.TestAddress("newAdmin") | ||
user := testutils.TestAddress("user") | ||
|
||
// Create role data with the administrator | ||
roleData := NewRole("admin", admin) | ||
|
||
// Check that the initial administrator is correct | ||
if roleData.AdminRole != admin { | ||
t.Fatalf("expected initial admin to be %s, got %s", admin.String(), roleData.AdminRole.String()) | ||
} | ||
|
||
// Simulate admin as current caller | ||
std.TestSetOrigCaller(admin) | ||
|
||
// Change administrator | ||
roleData.SetRoleAdmin(newAdmin) | ||
|
||
// Check that the new administrator is correct | ||
if roleData.AdminRole != newAdmin { | ||
t.Fatalf("expected new admin to be %s, got %s", newAdmin.String(), roleData.AdminRole.String()) | ||
} | ||
|
||
// Simulate newAdmin as the current caller | ||
std.TestSetOrigCaller(newAdmin) | ||
defer func() { | ||
if r := recover(); r != nil { | ||
t.Fatalf("expected no panic, got %v", r) | ||
} | ||
}() | ||
roleData.AssertCallerIsAdmin() | ||
|
||
// Add a role to a user | ||
roleData.GrantRole(user) | ||
if !roleData.HasRole(user) { | ||
t.Fatalf("expected user to have role") | ||
} | ||
|
||
// Simulate initial admin as current caller | ||
std.TestSetOrigCaller(admin) | ||
|
||
// Attempting to revoke a user's role by the former administrator should cause panic | ||
defer func() { | ||
if r := recover(); r == nil { | ||
t.Fatalf("expected panic when former admin tries to revoke role") | ||
} | ||
}() | ||
roleData.RevokeRole(user) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module gno.land/p/demo/accesscontrol | ||
|
||
require ( | ||
gno.land/p/demo/avl v0.0.0-latest | ||
gno.land/p/demo/testutils v0.0.0-latest | ||
gno.land/p/demo/uassert v0.0.0-latest | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make a case for how this is different / better than
p/demo/acl
?I'm not saying it's perfect, just that
demo/
should probably contain one preferred ACL implementation. We can decide to move this one top/<name>/accesscontrol
, or that one top/nt/acl
. (cc'ing also @moul for an opinion on what to do here)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although
acl
andaccesscontrol
may seem similar at first glance,accesscontrol
stands out due to its ability to implement role hierarchies as well as dynamic permission optionsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example? Namely, of where this distinction is useful?