From 9eb3c3c7decdd32adc0c9ee14e0639f18cd437b9 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 10 Jul 2017 13:50:11 +0200 Subject: [PATCH 01/14] Start specifying the roles module --- modules/coin/errors.go | 16 ++++----- modules/roles/error.go | 60 +++++++++++++++++++++++++++++++ modules/roles/handler.go | 4 +++ modules/roles/middleware.go | 1 + modules/roles/store.go | 44 +++++++++++++++++++++++ modules/roles/tx.go | 70 +++++++++++++++++++++++++++++++++++++ 6 files changed, 187 insertions(+), 8 deletions(-) create mode 100644 modules/roles/error.go create mode 100644 modules/roles/handler.go create mode 100644 modules/roles/middleware.go create mode 100644 modules/roles/store.go create mode 100644 modules/roles/tx.go diff --git a/modules/coin/errors.go b/modules/coin/errors.go index 7b86f6343f..bc3d7c5b78 100644 --- a/modules/coin/errors.go +++ b/modules/coin/errors.go @@ -2,20 +2,20 @@ package coin import ( - rawerr "errors" + "fmt" abci "github.com/tendermint/abci/types" "github.com/tendermint/basecoin/errors" ) var ( - errNoAccount = rawerr.New("No such account") - errInsufficientFunds = rawerr.New("Insufficient Funds") - errNoInputs = rawerr.New("No Input Coins") - errNoOutputs = rawerr.New("No Output Coins") - errInvalidAddress = rawerr.New("Invalid Address") - errInvalidCoins = rawerr.New("Invalid Coins") - errInvalidSequence = rawerr.New("Invalid Sequence") + errNoAccount = fmt.Errorf("No such account") + errInsufficientFunds = fmt.Errorf("Insufficient Funds") + errNoInputs = fmt.Errorf("No Input Coins") + errNoOutputs = fmt.Errorf("No Output Coins") + errInvalidAddress = fmt.Errorf("Invalid Address") + errInvalidCoins = fmt.Errorf("Invalid Coins") + errInvalidSequence = fmt.Errorf("Invalid Sequence") ) var ( diff --git a/modules/roles/error.go b/modules/roles/error.go new file mode 100644 index 0000000000..52041a252a --- /dev/null +++ b/modules/roles/error.go @@ -0,0 +1,60 @@ +//nolint +package roles + +import ( + "fmt" + + abci "github.com/tendermint/abci/types" + "github.com/tendermint/basecoin/errors" +) + +var ( + errNoRole = fmt.Errorf("No such role") + errRoleExists = fmt.Errorf("Role already exists") + errNotMember = fmt.Errorf("Not a member") + errInsufficientSigs = fmt.Errorf("Not enough signatures") + errNoMembers = fmt.Errorf("No members specified") + errTooManyMembers = fmt.Errorf("Too many members specified") +) + +func ErrNoRole() errors.TMError { + return errors.WithCode(errNoRole, abci.CodeType_Unauthorized) +} +func IsNoRoleErr(err error) bool { + return errors.IsSameError(errNoRole, err) +} + +func ErrRoleExists() errors.TMError { + return errors.WithCode(errRoleExists, abci.CodeType_Unauthorized) +} +func IsRoleExistsErr(err error) bool { + return errors.IsSameError(errRoleExists, err) +} + +func ErrNotMember() errors.TMError { + return errors.WithCode(errNotMember, abci.CodeType_Unauthorized) +} +func IsNotMemberErr(err error) bool { + return errors.IsSameError(errNotMember, err) +} + +func ErrInsufficientSigs() errors.TMError { + return errors.WithCode(errInsufficientSigs, abci.CodeType_Unauthorized) +} +func IsInsufficientSigsErr(err error) bool { + return errors.IsSameError(errInsufficientSigs, err) +} + +func ErrNoMembers() errors.TMError { + return errors.WithCode(errNoMembers, abci.CodeType_Unauthorized) +} +func IsNoMembersErr(err error) bool { + return errors.IsSameError(errNoMembers, err) +} + +func ErrTooManyMembers() errors.TMError { + return errors.WithCode(errTooManyMembers, abci.CodeType_Unauthorized) +} +func IsTooManyMembersErr(err error) bool { + return errors.IsSameError(errTooManyMembers, err) +} diff --git a/modules/roles/handler.go b/modules/roles/handler.go new file mode 100644 index 0000000000..a20d4f27ac --- /dev/null +++ b/modules/roles/handler.go @@ -0,0 +1,4 @@ +package roles + +//NameRole - name space of the roles module +const NameRole = "role" diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go new file mode 100644 index 0000000000..3258a668dc --- /dev/null +++ b/modules/roles/middleware.go @@ -0,0 +1 @@ +package roles diff --git a/modules/roles/store.go b/modules/roles/store.go new file mode 100644 index 0000000000..fe83a74166 --- /dev/null +++ b/modules/roles/store.go @@ -0,0 +1,44 @@ +package roles + +import ( + "fmt" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/state" + wire "github.com/tendermint/go-wire" +) + +// Role - structure to hold permissioning +type Role struct { + MinSigs uint32 `json:"min_sigs"` + Signers []basecoin.Actor `json:"signers"` +} + +// MakeKey creates the lookup key for a role +func MakeKey(role []byte) []byte { + prefix := []byte(NameRole + "/") + return append(prefix, role...) +} + +func loadRole(store state.KVStore, key []byte) (role Role, err error) { + data := store.Get(key) + if len(data) == 0 { + return role, ErrNoRole() + } + err = wire.ReadBinaryBytes(data, &role) + if err != nil { + msg := fmt.Sprintf("Error reading role %X", key) + return role, errors.ErrInternal(msg) + } + return role, nil +} + +func createRole(store state.KVStore, key []byte, role Role) error { + if _, err := loadRole(store, key); !IsNoRoleErr(err) { + return ErrRoleExists() + } + bin := wire.BinaryBytes(role) + store.Set(key, bin) + return nil // real stores can return error... +} diff --git a/modules/roles/tx.go b/modules/roles/tx.go new file mode 100644 index 0000000000..270c43027d --- /dev/null +++ b/modules/roles/tx.go @@ -0,0 +1,70 @@ +package roles + +import ( + "github.com/tendermint/go-wire/data" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" +) + +// AssumeRoleTx is a layered tx that can wrap your normal tx to give it +// the authority to use a given role. +type AssumeRoleTx struct { + Role data.Bytes `json:"role"` + Tx basecoin.Tx `json:"tx"` +} + +// NewAssumeRoleTx creates a new wrapper to add a role to a tx execution +func NewAssumeRoleTx(role []byte, tx basecoin.Tx) basecoin.Tx { + return AssumeRoleTx{Role: role, Tx: tx}.Wrap() +} + +// ValidateBasic - validate nothing is empty +func (tx AssumeRoleTx) ValidateBasic() error { + if len(tx.Role) == 0 { + return ErrNoRole() + } + if tx.Tx.Empty() { + return errors.ErrUnknownTxType(tx.Tx) + } + return nil +} + +// Wrap - used to satisfy TxInner +func (tx AssumeRoleTx) Wrap() basecoin.Tx { + return basecoin.Tx{tx} +} + +// CreateRoleTx is used to construct a new role +// +// TODO: add ability to update signers on a role... but that adds a lot +// more complexity to the permissions +type CreateRoleTx struct { + Role data.Bytes `json:"role"` + MinSigs uint32 `json:"min_sigs"` + Signers []basecoin.Actor `json:"signers"` +} + +// NewCreateRoleTx creates a new role, which we can later use +func NewCreateRoleTx(role []byte, minSigs uint32, signers []basecoin.Actor) basecoin.Tx { + return CreateRoleTx{Role: role, MinSigs: minSigs, Signers: signers}.Wrap() +} + +// ValidateBasic - validate nothing is empty +func (tx CreateRoleTx) ValidateBasic() error { + if len(tx.Role) == 0 { + return ErrNoRole() + } + if tx.MinSigs == 0 { + return ErrNoMembers() + } + if len(tx.Signers) == 0 { + return ErrNoMembers() + } + return nil +} + +// Wrap - used to satisfy TxInner +func (tx CreateRoleTx) Wrap() basecoin.Tx { + return basecoin.Tx{tx} +} From 3e52e6b959f7960ac178f6f8813268f25a6bcc08 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 10 Jul 2017 18:35:38 +0200 Subject: [PATCH 02/14] Add role struct, storage, better tx validation --- context.go | 10 +++++++ modules/roles/error.go | 10 +++++++ modules/roles/store.go | 32 ++++++++++++++++++++ modules/roles/store_test.go | 60 +++++++++++++++++++++++++++++++++++++ modules/roles/tx.go | 10 +++++++ stack/mock.go | 3 +- 6 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 modules/roles/store_test.go diff --git a/context.go b/context.go index b82aa042dc..d67fa0d9ae 100644 --- a/context.go +++ b/context.go @@ -1,6 +1,8 @@ package basecoin import ( + "bytes" + wire "github.com/tendermint/go-wire" "github.com/tendermint/go-wire/data" "github.com/tendermint/tmlibs/log" @@ -21,10 +23,18 @@ func NewActor(app string, addr []byte) Actor { return Actor{App: app, Address: addr} } +// Bytes makes a binary coding, useful for turning this into a key in the store func (a Actor) Bytes() []byte { return wire.BinaryBytes(a) } +// Equals checks if two actors are the same +func (a Actor) Equals(b Actor) bool { + return a.ChainID == b.ChainID && + a.App == b.App && + bytes.Equal(a.Address, b.Address) +} + // Context is an interface, so we can implement "secure" variants that // rely on private fields to control the actions type Context interface { diff --git a/modules/roles/error.go b/modules/roles/error.go index 52041a252a..1fb8333a27 100644 --- a/modules/roles/error.go +++ b/modules/roles/error.go @@ -15,8 +15,11 @@ var ( errInsufficientSigs = fmt.Errorf("Not enough signatures") errNoMembers = fmt.Errorf("No members specified") errTooManyMembers = fmt.Errorf("Too many members specified") + errNotEnoughMembers = fmt.Errorf("Not enough members specified") ) +// TODO: codegen? +// ex: err-gen NoRole,"No such role",CodeType_Unauthorized func ErrNoRole() errors.TMError { return errors.WithCode(errNoRole, abci.CodeType_Unauthorized) } @@ -58,3 +61,10 @@ func ErrTooManyMembers() errors.TMError { func IsTooManyMembersErr(err error) bool { return errors.IsSameError(errTooManyMembers, err) } + +func ErrNotEnoughMembers() errors.TMError { + return errors.WithCode(errNotEnoughMembers, abci.CodeType_Unauthorized) +} +func IsNotEnoughMembersErr(err error) bool { + return errors.IsSameError(errNotEnoughMembers, err) +} diff --git a/modules/roles/store.go b/modules/roles/store.go index fe83a74166..61ca16e61c 100644 --- a/modules/roles/store.go +++ b/modules/roles/store.go @@ -15,6 +15,37 @@ type Role struct { Signers []basecoin.Actor `json:"signers"` } +func NewRole(min uint32, signers []basecoin.Actor) Role { + return Role{ + MinSigs: min, + Signers: signers, + } +} + +// IsSigner checks if the given Actor is allowed to sign this role +func (r Role) IsSigner(a basecoin.Actor) bool { + for _, s := range r.Signers { + if a.Equals(s) { + return true + } + } + return false +} + +// IsAuthorized checks if the context has permission to assume the role +func (r Role) IsAuthorized(ctx basecoin.Context) bool { + needed := r.MinSigs + for _, s := range r.Signers { + if ctx.HasPermission(s) { + needed-- + if needed <= 0 { + return true + } + } + } + return false +} + // MakeKey creates the lookup key for a role func MakeKey(role []byte) []byte { prefix := []byte(NameRole + "/") @@ -34,6 +65,7 @@ func loadRole(store state.KVStore, key []byte) (role Role, err error) { return role, nil } +// we only have create here, no update, since we don't allow update yet func createRole(store state.KVStore, key []byte, role Role) error { if _, err := loadRole(store, key); !IsNoRoleErr(err) { return ErrRoleExists() diff --git a/modules/roles/store_test.go b/modules/roles/store_test.go new file mode 100644 index 0000000000..a38592cf65 --- /dev/null +++ b/modules/roles/store_test.go @@ -0,0 +1,60 @@ +package roles_test + +import ( + "strconv" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/modules/roles" + "github.com/tendermint/basecoin/stack" +) + +func TestRole(t *testing.T) { + assert := assert.New(t) + + // prepare some actors... + a := basecoin.Actor{App: "foo", Address: []byte("bar")} + b := basecoin.Actor{ChainID: "eth", App: "foo", Address: []byte("bar")} + c := basecoin.Actor{App: "foo", Address: []byte("baz")} + d := basecoin.Actor{App: "si-ly", Address: []byte("bar")} + e := basecoin.Actor{App: "si-ly", Address: []byte("big")} + f := basecoin.Actor{App: "sig", Address: []byte{1}} + g := basecoin.Actor{App: "sig", Address: []byte{2, 3, 4}} + + cases := []struct { + sigs uint32 + allowed []basecoin.Actor + signers []basecoin.Actor + valid bool + }{ + // make sure simple compare is correct + {1, []basecoin.Actor{a}, []basecoin.Actor{a}, true}, + {1, []basecoin.Actor{a}, []basecoin.Actor{b}, false}, + {1, []basecoin.Actor{a}, []basecoin.Actor{c}, false}, + {1, []basecoin.Actor{a}, []basecoin.Actor{d}, false}, + // make sure multi-sig counts to 1 + {1, []basecoin.Actor{a, b, c}, []basecoin.Actor{d, e, a, f}, true}, + {1, []basecoin.Actor{a, b, c}, []basecoin.Actor{a, b, c, d}, true}, + {1, []basecoin.Actor{a, b, c}, []basecoin.Actor{d, e, f}, false}, + // make sure multi-sig counts higher + {2, []basecoin.Actor{b, e, g}, []basecoin.Actor{g, c, a, d, b}, true}, + {2, []basecoin.Actor{b, e, g}, []basecoin.Actor{c, a, d, b}, false}, + {3, []basecoin.Actor{a, b, c}, []basecoin.Actor{g}, false}, + } + + for idx, tc := range cases { + i := strconv.Itoa(idx) + // make sure IsSigner works + role := roles.NewRole(tc.sigs, tc.allowed) + for _, a := range tc.allowed { + assert.True(role.IsSigner(a), i) + } + // make sure IsAuthorized works + ctx := stack.MockContext("chain-id").WithPermissions(tc.signers...) + allowed := role.IsAuthorized(ctx) + assert.Equal(tc.valid, allowed, i) + } + +} diff --git a/modules/roles/tx.go b/modules/roles/tx.go index 270c43027d..c4f47d1e8a 100644 --- a/modules/roles/tx.go +++ b/modules/roles/tx.go @@ -7,6 +7,10 @@ import ( "github.com/tendermint/basecoin/errors" ) +const ( + MaxMembers = 10 +) + // AssumeRoleTx is a layered tx that can wrap your normal tx to give it // the authority to use a given role. type AssumeRoleTx struct { @@ -61,6 +65,12 @@ func (tx CreateRoleTx) ValidateBasic() error { if len(tx.Signers) == 0 { return ErrNoMembers() } + if len(tx.Signers) < int(tx.MinSigs) { + return ErrNotEnoughMembers() + } + if len(tx.Signers) > MaxMembers { + return ErrTooManyMembers() + } return nil } diff --git a/stack/mock.go b/stack/mock.go index 2f6ef9a5de..b64f4e12f1 100644 --- a/stack/mock.go +++ b/stack/mock.go @@ -1,7 +1,6 @@ package stack import ( - "bytes" "math/rand" "github.com/tendermint/tmlibs/log" @@ -52,7 +51,7 @@ func (c naiveContext) WithPermissions(perms ...basecoin.Actor) basecoin.Context func (c naiveContext) HasPermission(perm basecoin.Actor) bool { for _, p := range c.perms { - if perm.App == p.App && bytes.Equal(perm.Address, p.Address) { + if p.Equals(perm) { return true } } From 11a8c0d9a2a067c19ac14d21671eef0bad50b9c6 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 10 Jul 2017 19:41:17 +0200 Subject: [PATCH 03/14] Add roles middleware and handler --- errors/common.go | 9 +++++ modules/coin/handler.go | 5 +-- modules/roles/handler.go | 59 +++++++++++++++++++++++++++ modules/roles/middleware.go | 80 +++++++++++++++++++++++++++++++++++++ modules/roles/store.go | 7 ++++ 5 files changed, 156 insertions(+), 4 deletions(-) diff --git a/errors/common.go b/errors/common.go index c04af0984d..c793a6953e 100644 --- a/errors/common.go +++ b/errors/common.go @@ -22,6 +22,7 @@ var ( errInvalidFormat = fmt.Errorf("Invalid format") errUnknownModule = fmt.Errorf("Unknown module") errExpired = fmt.Errorf("Tx expired") + errUnknownKey = fmt.Errorf("Unknown key") ) // some crazy reflection to unwrap any generated struct. @@ -63,6 +64,14 @@ func IsUnknownModuleErr(err error) bool { return IsSameError(errUnknownModule, err) } +func ErrUnknownKey(mod string) TMError { + w := errors.Wrap(errUnknownKey, mod) + return WithCode(w, abci.CodeType_UnknownRequest) +} +func IsUnknownKeyErr(err error) bool { + return IsSameError(errUnknownKey, err) +} + func ErrInternal(msg string) TMError { return New(msg, abci.CodeType_InternalError) } diff --git a/modules/coin/handler.go b/modules/coin/handler.go index cd45910f38..590362045f 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -1,8 +1,6 @@ package coin import ( - "fmt" - "github.com/tendermint/go-wire/data" "github.com/tendermint/tmlibs/log" @@ -106,8 +104,7 @@ func (h Handler) SetOption(l log.Logger, store state.KVStore, module, key, value return "Success", nil } - msg := fmt.Sprintf("Unknown key: %s", key) - return "", errors.ErrInternal(msg) + return errors.ErrUnknownKey(key) } func checkTx(ctx basecoin.Context, tx basecoin.Tx) (send SendTx, err error) { diff --git a/modules/roles/handler.go b/modules/roles/handler.go index a20d4f27ac..53c7607c77 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -1,4 +1,63 @@ package roles +import ( + "github.com/tendermint/tmlibs/log" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/state" +) + //NameRole - name space of the roles module const NameRole = "role" + +type Handler struct{} + +var _ basecoin.Handler = Handler{} + +// NewHandler makes a role handler to create roles +func NewHandler() Handler { + return Handler{} +} + +// Name - return name space +func (Handler) Name() string { + return NameRole +} + +// CheckTx checks if there is enough money in the account +func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + _, err = checkTx(ctx, tx) + return res, err +} + +// DeliverTx moves the money +func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + create, err := checkTx(ctx, tx) + if err != nil { + return res, err + } + + // lets try... + role := NewRole(create.MinSigs, create.Signers) + err = createRole(store, MakeKey(create.Role), role) + return res, err +} + +// SetOption - sets the genesis account balance +func (h Handler) SetOption(l log.Logger, store state.KVStore, module, key, value string) (log string, err error) { + if module != NameRole { + return "", errors.ErrUnknownModule(module) + } + return "", errors.ErrUnknownKey(key) +} + +func checkTx(ctx basecoin.Context, tx basecoin.Tx) (create CreateRoleTx, err error) { + // check if the tx is proper type and valid + create, ok := tx.Unwrap().(CreateRoleTx) + if !ok { + return create, errors.ErrInvalidFormat(tx) + } + err = create.ValidateBasic() + return create, err +} diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go index 3258a668dc..8568bab2b2 100644 --- a/modules/roles/middleware.go +++ b/modules/roles/middleware.go @@ -1 +1,81 @@ package roles + +import ( + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/state" +) + +type Middleware struct { + stack.PassOption +} + +var _ stack.Middleware = Middleware{} + +func NewMiddleware() Middleware { + return Middleware{} +} + +// Name - return name space +func (Middleware) Name() string { + return NameRole +} + +// CheckTx checks if this is valid +func (m Middleware) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { + assume, err := checkMiddleTx(ctx, tx) + if err != nil { + return res, err + } + + ctx, err = assumeRole(ctx, store, assume) + if err != nil { + return res, err + } + + return next.CheckTx(ctx, store, assume.Tx) +} + +// DeliverTx moves the money +func (m Middleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { + assume, err := checkMiddleTx(ctx, tx) + if err != nil { + return res, err + } + + ctx, err = assumeRole(ctx, store, assume) + if err != nil { + return res, err + } + + return next.DeliverTx(ctx, store, assume.Tx) +} + +func checkMiddleTx(ctx basecoin.Context, tx basecoin.Tx) (assume AssumeRoleTx, err error) { + // check if the tx is proper type and valid + assume, ok := tx.Unwrap().(AssumeRoleTx) + if !ok { + return assume, errors.ErrInvalidFormat(tx) + } + err = assume.ValidateBasic() + if err != nil { + return assume, err + } + + // load it up and check it out... + return assume, err +} + +func assumeRole(ctx basecoin.Context, store state.KVStore, assume AssumeRoleTx) (basecoin.Context, error) { + role, err := loadRole(store, MakeKey(assume.Role)) + if err != nil { + return nil, err + } + + if !role.IsAuthorized(ctx) { + return nil, ErrInsufficientSigs() + } + ctx = ctx.WithPermissions(NewPerm(assume.Role)) + return ctx, nil +} diff --git a/modules/roles/store.go b/modules/roles/store.go index 61ca16e61c..a576b0e1a9 100644 --- a/modules/roles/store.go +++ b/modules/roles/store.go @@ -9,6 +9,13 @@ import ( wire "github.com/tendermint/go-wire" ) +func NewPerm(role []byte) basecoin.Actor { + return basecoin.Actor{ + App: NameRole, + Address: role, + } +} + // Role - structure to hold permissioning type Role struct { MinSigs uint32 `json:"min_sigs"` From fbdec34ea6d7b5a1677c94da171a5bcf123268f3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 10 Jul 2017 20:31:48 +0200 Subject: [PATCH 04/14] fix typo --- modules/coin/handler.go | 2 +- modules/roles/store_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/coin/handler.go b/modules/coin/handler.go index 590362045f..e0b51b89ff 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -104,7 +104,7 @@ func (h Handler) SetOption(l log.Logger, store state.KVStore, module, key, value return "Success", nil } - return errors.ErrUnknownKey(key) + return "", errors.ErrUnknownKey(key) } func checkTx(ctx basecoin.Context, tx basecoin.Tx) (send SendTx, err error) { diff --git a/modules/roles/store_test.go b/modules/roles/store_test.go index a38592cf65..fb69abb8d6 100644 --- a/modules/roles/store_test.go +++ b/modules/roles/store_test.go @@ -52,7 +52,7 @@ func TestRole(t *testing.T) { assert.True(role.IsSigner(a), i) } // make sure IsAuthorized works - ctx := stack.MockContext("chain-id").WithPermissions(tc.signers...) + ctx := stack.MockContext("chain-id", 100).WithPermissions(tc.signers...) allowed := role.IsAuthorized(ctx) assert.Equal(tc.valid, allowed, i) } From 2f0cb2569ffe989cd0afcc85ed124c305f173515 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 10 Jul 2017 22:57:18 +0200 Subject: [PATCH 05/14] Add prefixStore (state space) --- stack/prefixstore.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 stack/prefixstore.go diff --git a/stack/prefixstore.go b/stack/prefixstore.go new file mode 100644 index 0000000000..8592ba321d --- /dev/null +++ b/stack/prefixstore.go @@ -0,0 +1,31 @@ +package stack + +import "github.com/tendermint/basecoin/state" + +type prefixStore struct { + prefix []byte + store state.KVStore +} + +var _ state.KVStore = prefixStore{} + +func (p prefixStore) Set(key, value []byte) { + key = append(key, p.prefix...) + p.store.Set(key, value) +} + +func (p prefixStore) Get(key []byte) (value []byte) { + key = append(key, p.prefix...) + return p.store.Get(key) +} + +// stateSpace will unwrap any prefixStore and then add the prefix +func stateSpace(store state.KVStore, app string) state.KVStore { + // unwrap one-level if wrapped + if pstore, ok := store.(prefixStore); ok { + store = pstore.store + } + // wrap it with the prefix + prefix := append([]byte(app), byte(0)) + return prefixStore{prefix, store} +} From 2a358010dd8077d041f21fbf7b31a064e91489f9 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 11 Jul 2017 14:10:40 +0200 Subject: [PATCH 06/14] Clean up roles module with Rigels comments --- modules/roles/handler.go | 21 ++++------ modules/roles/middleware.go | 80 ++++++++++++++++++------------------- modules/roles/store.go | 2 + modules/roles/tx.go | 5 ++- 4 files changed, 54 insertions(+), 54 deletions(-) diff --git a/modules/roles/handler.go b/modules/roles/handler.go index 53c7607c77..99621ceccb 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -1,8 +1,6 @@ package roles import ( - "github.com/tendermint/tmlibs/log" - "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/errors" "github.com/tendermint/basecoin/state" @@ -11,7 +9,10 @@ import ( //NameRole - name space of the roles module const NameRole = "role" -type Handler struct{} +// Handler allows us to create new roles +type Handler struct { + basecoin.NopOption +} var _ basecoin.Handler = Handler{} @@ -25,13 +26,15 @@ func (Handler) Name() string { return NameRole } -// CheckTx checks if there is enough money in the account +// CheckTx verifies if the transaction is properly formated func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { _, err = checkTx(ctx, tx) return res, err } -// DeliverTx moves the money +// DeliverTx tries to create a new role. +// +// Returns an error if the role already exists func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { create, err := checkTx(ctx, tx) if err != nil { @@ -44,14 +47,6 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi return res, err } -// SetOption - sets the genesis account balance -func (h Handler) SetOption(l log.Logger, store state.KVStore, module, key, value string) (log string, err error) { - if module != NameRole { - return "", errors.ErrUnknownModule(module) - } - return "", errors.ErrUnknownKey(key) -} - func checkTx(ctx basecoin.Context, tx basecoin.Tx) (create CreateRoleTx, err error) { // check if the tx is proper type and valid create, ok := tx.Unwrap().(CreateRoleTx) diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go index 8568bab2b2..c2d5352525 100644 --- a/modules/roles/middleware.go +++ b/modules/roles/middleware.go @@ -2,17 +2,19 @@ package roles import ( "github.com/tendermint/basecoin" - "github.com/tendermint/basecoin/errors" "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/state" ) +// Middleware allows us to add a requested role as a permission +// if the tx requests it and has sufficient authority type Middleware struct { stack.PassOption } var _ stack.Middleware = Middleware{} +// NewMiddleware creates a role-checking middleware func NewMiddleware() Middleware { return Middleware{} } @@ -22,52 +24,50 @@ func (Middleware) Name() string { return NameRole } -// CheckTx checks if this is valid +// CheckTx tries to assume the named role if requested. +// If no role is requested, do nothing. +// If insufficient authority to assume the role, return error. func (m Middleware) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { - assume, err := checkMiddleTx(ctx, tx) - if err != nil { - return res, err - } - - ctx, err = assumeRole(ctx, store, assume) - if err != nil { - return res, err - } - - return next.CheckTx(ctx, store, assume.Tx) -} - -// DeliverTx moves the money -func (m Middleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { - assume, err := checkMiddleTx(ctx, tx) - if err != nil { - return res, err - } - - ctx, err = assumeRole(ctx, store, assume) - if err != nil { - return res, err - } - - return next.DeliverTx(ctx, store, assume.Tx) -} - -func checkMiddleTx(ctx basecoin.Context, tx basecoin.Tx) (assume AssumeRoleTx, err error) { - // check if the tx is proper type and valid + // if this is not an AssumeRoleTx, then continue assume, ok := tx.Unwrap().(AssumeRoleTx) - if !ok { - return assume, errors.ErrInvalidFormat(tx) - } - err = assume.ValidateBasic() - if err != nil { - return assume, err + if !ok { // this also breaks the recursion below + return next.CheckTx(ctx, store, tx) } - // load it up and check it out... - return assume, err + ctx, err = assumeRole(ctx, store, assume) + if err != nil { + return res, err + } + + // one could add multiple role statements, repeat as needed + return m.CheckTx(ctx, store, assume.Tx, next) +} + +// DeliverTx tries to assume the named role if requested. +// If no role is requested, do nothing. +// If insufficient authority to assume the role, return error. +func (m Middleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { + // if this is not an AssumeRoleTx, then continue + assume, ok := tx.Unwrap().(AssumeRoleTx) + if !ok { // this also breaks the recursion below + return next.DeliverTx(ctx, store, tx) + } + + ctx, err = assumeRole(ctx, store, assume) + if err != nil { + return res, err + } + + // one could add multiple role statements, repeat as needed + return m.DeliverTx(ctx, store, assume.Tx, next) } func assumeRole(ctx basecoin.Context, store state.KVStore, assume AssumeRoleTx) (basecoin.Context, error) { + err := assume.ValidateBasic() + if err != nil { + return nil, err + } + role, err := loadRole(store, MakeKey(assume.Role)) if err != nil { return nil, err diff --git a/modules/roles/store.go b/modules/roles/store.go index a576b0e1a9..bfde718339 100644 --- a/modules/roles/store.go +++ b/modules/roles/store.go @@ -9,6 +9,7 @@ import ( wire "github.com/tendermint/go-wire" ) +// NewPerm creates a role permission with the given label func NewPerm(role []byte) basecoin.Actor { return basecoin.Actor{ App: NameRole, @@ -22,6 +23,7 @@ type Role struct { Signers []basecoin.Actor `json:"signers"` } +// NewRole creates a Role structure to store the permissioning func NewRole(min uint32, signers []basecoin.Actor) Role { return Role{ MinSigs: min, diff --git a/modules/roles/tx.go b/modules/roles/tx.go index c4f47d1e8a..6215f44e69 100644 --- a/modules/roles/tx.go +++ b/modules/roles/tx.go @@ -8,7 +8,10 @@ import ( ) const ( - MaxMembers = 10 + // MaxMembers it the maximum number of members in a Role. Used to avoid + // extremely large roles. + // Value is arbitrary, please adjust as needed + MaxMembers = 20 ) // AssumeRoleTx is a layered tx that can wrap your normal tx to give it From 5cc7406a00897585f82485419931d23767c946a4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 11 Jul 2017 15:05:37 +0200 Subject: [PATCH 07/14] Add state space to the modules and test --- stack/dispatcher.go | 26 +++++-- stack/middleware.go | 7 ++ stack/prefixstore.go | 4 +- stack/state_space_test.go | 139 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 stack/state_space_test.go diff --git a/stack/dispatcher.go b/stack/dispatcher.go index b8850b89b0..11090fafa0 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -69,8 +69,14 @@ func (d *Dispatcher) CheckTx(ctx basecoin.Context, store state.KVStore, tx basec if err != nil { return res, err } - // TODO: check on callback - cb := d + + // make sure no monkey business with the context + cb := secureCheck(d, ctx) + + // and isolate the permissions and the data store for this app + ctx = withApp(ctx, r.Name()) + store = stateSpace(store, r.Name()) + return r.CheckTx(ctx, store, tx, cb) } @@ -84,8 +90,14 @@ func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store state.KVStore, tx bas if err != nil { return res, err } - // TODO: check on callback - cb := d + + // make sure no monkey business with the context + cb := secureDeliver(d, ctx) + + // and isolate the permissions and the data store for this app + ctx = withApp(ctx, r.Name()) + store = stateSpace(store, r.Name()) + return r.DeliverTx(ctx, store, tx, cb) } @@ -98,8 +110,12 @@ func (d *Dispatcher) SetOption(l log.Logger, store state.KVStore, module, key, v if err != nil { return "", err } - // TODO: check on callback + + // no ctx, so secureCheck not needed cb := d + // but isolate data space + store = stateSpace(store, r.Name()) + return r.SetOption(l, store, module, key, value, cb) } diff --git a/stack/middleware.go b/stack/middleware.go index b7e1e58dd3..9e1b8e75a1 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -27,6 +27,8 @@ func (m *middleware) CheckTx(ctx basecoin.Context, store state.KVStore, tx basec next := secureCheck(m.next, ctx) // set the permissions for this app ctx = withApp(ctx, m.Name()) + store = stateSpace(store, m.Name()) + return m.middleware.CheckTx(ctx, store, tx, next) } @@ -36,10 +38,15 @@ func (m *middleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx bas next := secureDeliver(m.next, ctx) // set the permissions for this app ctx = withApp(ctx, m.Name()) + store = stateSpace(store, m.Name()) + return m.middleware.DeliverTx(ctx, store, tx, next) } func (m *middleware) SetOption(l log.Logger, store state.KVStore, module, key, value string) (string, error) { + // set the namespace for the app + store = stateSpace(store, m.Name()) + return m.middleware.SetOption(l, store, module, key, value, m.next) } diff --git a/stack/prefixstore.go b/stack/prefixstore.go index 8592ba321d..e9616ad521 100644 --- a/stack/prefixstore.go +++ b/stack/prefixstore.go @@ -10,12 +10,12 @@ type prefixStore struct { var _ state.KVStore = prefixStore{} func (p prefixStore) Set(key, value []byte) { - key = append(key, p.prefix...) + key = append(p.prefix, key...) p.store.Set(key, value) } func (p prefixStore) Get(key []byte) (value []byte) { - key = append(key, p.prefix...) + key = append(p.prefix, key...) return p.store.Get(key) } diff --git a/stack/state_space_test.go b/stack/state_space_test.go new file mode 100644 index 0000000000..2722ad6a3c --- /dev/null +++ b/stack/state_space_test.go @@ -0,0 +1,139 @@ +package stack + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/tendermint/tmlibs/log" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/state" + "github.com/tendermint/go-wire/data" +) + +// writerMid is a middleware that writes the given bytes on CheckTx and DeliverTx +type writerMid struct { + name string + key, value []byte +} + +var _ Middleware = writerMid{} + +func (w writerMid) Name() string { return w.name } + +func (w writerMid) CheckTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx, next basecoin.Checker) (basecoin.Result, error) { + store.Set(w.key, w.value) + return next.CheckTx(ctx, store, tx) +} + +func (w writerMid) DeliverTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx, next basecoin.Deliver) (basecoin.Result, error) { + store.Set(w.key, w.value) + return next.DeliverTx(ctx, store, tx) +} + +func (w writerMid) SetOption(l log.Logger, store state.KVStore, module, + key, value string, next basecoin.SetOptioner) (string, error) { + store.Set([]byte(key), []byte(value)) + return next.SetOption(l, store, module, key, value) +} + +// writerHand is a middleware that writes the given bytes on CheckTx and DeliverTx +type writerHand struct { + name string + key, value []byte +} + +var _ basecoin.Handler = writerHand{} + +func (w writerHand) Name() string { return w.name } + +func (w writerHand) CheckTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx) (basecoin.Result, error) { + store.Set(w.key, w.value) + return basecoin.Result{}, nil +} + +func (w writerHand) DeliverTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx) (basecoin.Result, error) { + store.Set(w.key, w.value) + return basecoin.Result{}, nil +} + +func (w writerHand) SetOption(l log.Logger, store state.KVStore, module, + key, value string) (string, error) { + store.Set([]byte(key), []byte(value)) + return "Success", nil +} + +func TestStateSpace(t *testing.T) { + cases := []struct { + h basecoin.Handler + m []Middleware + expected []data.Bytes + }{ + { + writerHand{"foo", []byte{1, 2}, []byte("bar")}, + []Middleware{ + writerMid{"bing", []byte{1, 2}, []byte("bang")}, + }, + []data.Bytes{ + {'f', 'o', 'o', 0, 1, 2}, + {'b', 'i', 'n', 'g', 0, 1, 2}, + }, + }, + } + + for i, tc := range cases { + // make an app with this setup + d := NewDispatcher(WrapHandler(tc.h)) + app := New(tc.m...).Use(d) + + // register so RawTx is routed to this handler + basecoin.TxMapper.RegisterImplementation(RawTx{}, tc.h.Name(), byte(50+i)) + + // run various tests on this setup + spaceCheck(t, i, app, tc.expected) + spaceDeliver(t, i, app, tc.expected) + // spaceOption(t, i, app, keys) + } +} + +func spaceCheck(t *testing.T, i int, app basecoin.Handler, keys []data.Bytes) { + assert := assert.New(t) + require := require.New(t) + + ctx := MockContext("chain", 100) + store := state.NewMemKVStore() + + // run a tx + _, err := app.CheckTx(ctx, store, NewRawTx([]byte{77})) + require.Nil(err, "%d: %+v", i, err) + + // verify that the data was writen + for j, k := range keys { + v := store.Get(k) + assert.NotEmpty(v, "%d / %d", i, j) + } +} + +func spaceDeliver(t *testing.T, i int, app basecoin.Handler, keys []data.Bytes) { + assert := assert.New(t) + require := require.New(t) + + ctx := MockContext("chain", 100) + store := state.NewMemKVStore() + + // run a tx + _, err := app.DeliverTx(ctx, store, NewRawTx([]byte{1, 56})) + require.Nil(err, "%d: %+v", i, err) + + // verify that the data was writen + for j, k := range keys { + v := store.Get(k) + assert.NotEmpty(v, "%d / %d", i, j) + } +} From 771c08483ee91f42ee922090e4152672eb5f1f91 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 11 Jul 2017 15:11:10 +0200 Subject: [PATCH 08/14] Remove prefix space in modules, done with state space --- modules/coin/handler.go | 16 +++++-------- modules/coin/handler_test.go | 6 ++--- modules/coin/store.go | 44 ++++++++---------------------------- modules/roles/handler.go | 2 +- modules/roles/middleware.go | 2 +- modules/roles/store.go | 6 ----- 6 files changed, 20 insertions(+), 56 deletions(-) diff --git a/modules/coin/handler.go b/modules/coin/handler.go index e0b51b89ff..bd047f8eb4 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -14,17 +14,13 @@ import ( const NameCoin = "coin" // Handler includes an accountant -type Handler struct { - Accountant -} +type Handler struct{} var _ basecoin.Handler = Handler{} // NewHandler - new accountant handler for the coin module func NewHandler() Handler { - return Handler{ - Accountant: NewAccountant(""), - } + return Handler{} } // Name - return name space @@ -41,7 +37,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin. // now make sure there is money for _, in := range send.Inputs { - _, err = h.CheckCoins(store, in.Address, in.Coins.Negative(), in.Sequence) + _, err = CheckCoins(store, in.Address, in.Coins.Negative(), in.Sequence) if err != nil { return res, err } @@ -60,7 +56,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // deduct from all input accounts for _, in := range send.Inputs { - _, err = h.ChangeCoins(store, in.Address, in.Coins.Negative(), in.Sequence) + _, err = ChangeCoins(store, in.Address, in.Coins.Negative(), in.Sequence) if err != nil { return res, err } @@ -69,7 +65,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // add to all output accounts for _, out := range send.Outputs { // note: sequence number is ignored when adding coins, only checked for subtracting - _, err = h.ChangeCoins(store, out.Address, out.Coins, 0) + _, err = ChangeCoins(store, out.Address, out.Coins, 0) if err != nil { return res, err } @@ -97,7 +93,7 @@ func (h Handler) SetOption(l log.Logger, store state.KVStore, module, key, value } // this sets the permission for a public key signature, use that app actor := auth.SigPerm(addr) - err = storeAccount(store, h.MakeKey(actor), acc.ToAccount()) + err = storeAccount(store, actor.Bytes(), acc.ToAccount()) if err != nil { return "", err } diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 50a4bbed75..716ad7ed7f 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -144,7 +144,7 @@ func TestDeliverTx(t *testing.T) { store := state.NewMemKVStore() for _, m := range tc.init { acct := Account{Coins: m.coins} - err := storeAccount(store, h.MakeKey(m.addr), acct) + err := storeAccount(store, m.addr.Bytes(), acct) require.Nil(err, "%d: %+v", i, err) } @@ -154,7 +154,7 @@ func TestDeliverTx(t *testing.T) { assert.Nil(err, "%d: %+v", i, err) // make sure the final balances are correct for _, f := range tc.final { - acct, err := loadAccount(store, h.MakeKey(f.addr)) + acct, err := loadAccount(store, f.addr.Bytes()) assert.Nil(err, "%d: %+v", i, err) assert.Equal(f.coins, acct.Coins) } @@ -210,7 +210,7 @@ func TestSetOption(t *testing.T) { // check state is proper for _, f := range tc.expected { - acct, err := loadAccount(store, h.MakeKey(f.addr)) + acct, err := loadAccount(store, f.addr.Bytes()) assert.Nil(err, "%d: %+v", i, err) assert.Equal(f.coins, acct.Coins) } diff --git a/modules/coin/store.go b/modules/coin/store.go index 01032cf1d3..94c086f1de 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -10,25 +10,9 @@ import ( "github.com/tendermint/basecoin/state" ) -// Accountant - custom object to manage coins for the coin module -// TODO prefix should be post-fix if maintaining the same key space -type Accountant struct { - Prefix []byte -} - -// NewAccountant - create the new accountant with prefix information -func NewAccountant(prefix string) Accountant { - if prefix == "" { - prefix = NameCoin - } - return Accountant{ - Prefix: []byte(prefix + "/"), - } -} - // GetAccount - Get account from store and address -func (a Accountant) GetAccount(store state.KVStore, addr basecoin.Actor) (Account, error) { - acct, err := loadAccount(store, a.MakeKey(addr)) +func GetAccount(store state.KVStore, addr basecoin.Actor) (Account, error) { + acct, err := loadAccount(store, addr.Bytes()) // for empty accounts, don't return an error, but rather an empty account if IsNoAccountErr(err) { @@ -38,27 +22,27 @@ func (a Accountant) GetAccount(store state.KVStore, addr basecoin.Actor) (Accoun } // CheckCoins makes sure there are funds, but doesn't change anything -func (a Accountant) CheckCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { - acct, err := a.updateCoins(store, addr, coins, seq) +func CheckCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { + acct, err := updateCoins(store, addr, coins, seq) return acct.Coins, err } // ChangeCoins changes the money, returns error if it would be negative -func (a Accountant) ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { - acct, err := a.updateCoins(store, addr, coins, seq) +func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { + acct, err := updateCoins(store, addr, coins, seq) if err != nil { return acct.Coins, err } - err = storeAccount(store, a.MakeKey(addr), acct) + err = storeAccount(store, addr.Bytes(), acct) return acct.Coins, err } // updateCoins will load the account, make all checks, and return the updated account. // // it doesn't save anything, that is up to you to decide (Check/Change Coins) -func (a Accountant) updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (acct Account, err error) { - acct, err = loadAccount(store, a.MakeKey(addr)) +func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (acct Account, err error) { + acct, err = loadAccount(store, addr.Bytes()) // we can increase an empty account... if IsNoAccountErr(err) && coins.IsPositive() { err = nil @@ -85,16 +69,6 @@ func (a Accountant) updateCoins(store state.KVStore, addr basecoin.Actor, coins return acct, nil } -// MakeKey - generate key bytes from address using accountant prefix -// TODO Prefix -> PostFix for consistent namespace -func (a Accountant) MakeKey(addr basecoin.Actor) []byte { - key := addr.Bytes() - if len(a.Prefix) > 0 { - key = append(a.Prefix, key...) - } - return key -} - // Account - coin account structure type Account struct { Coins Coins `json:"coins"` diff --git a/modules/roles/handler.go b/modules/roles/handler.go index 99621ceccb..ce1ab137d2 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -43,7 +43,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // lets try... role := NewRole(create.MinSigs, create.Signers) - err = createRole(store, MakeKey(create.Role), role) + err = createRole(store, create.Role, role) return res, err } diff --git a/modules/roles/middleware.go b/modules/roles/middleware.go index c2d5352525..78bc4b1b30 100644 --- a/modules/roles/middleware.go +++ b/modules/roles/middleware.go @@ -68,7 +68,7 @@ func assumeRole(ctx basecoin.Context, store state.KVStore, assume AssumeRoleTx) return nil, err } - role, err := loadRole(store, MakeKey(assume.Role)) + role, err := loadRole(store, assume.Role) if err != nil { return nil, err } diff --git a/modules/roles/store.go b/modules/roles/store.go index bfde718339..4b3d4f3d14 100644 --- a/modules/roles/store.go +++ b/modules/roles/store.go @@ -55,12 +55,6 @@ func (r Role) IsAuthorized(ctx basecoin.Context) bool { return false } -// MakeKey creates the lookup key for a role -func MakeKey(role []byte) []byte { - prefix := []byte(NameRole + "/") - return append(prefix, role...) -} - func loadRole(store state.KVStore, key []byte) (role Role, err error) { data := store.Get(key) if len(data) == 0 { From 66d1f86098d76c948e8712496d35d4a20d1ce3bf Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 11 Jul 2017 15:21:02 +0200 Subject: [PATCH 09/14] Fixed app tests for state spaces --- app/app_test.go | 14 ++++++++++++-- stack/prefixstore.go | 16 +++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 7889a31b0e..38aa990cda 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,6 +14,7 @@ import ( "github.com/tendermint/basecoin/modules/auth" "github.com/tendermint/basecoin/modules/base" "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/state" wire "github.com/tendermint/go-wire" eyes "github.com/tendermint/merkleeyes/client" @@ -82,8 +84,16 @@ func (at *appTest) reset() { } func getBalance(key basecoin.Actor, state state.KVStore) (coin.Coins, error) { - acct, err := coin.NewAccountant("").GetAccount(state, key) - return acct.Coins, err + var acct coin.Account + k := stack.PrefixedKey(coin.NameCoin, key.Bytes()) + v := state.Get(k) + // empty if no data + if len(v) == 0 { + return nil, nil + } + // otherwise read it + err := wire.ReadBinaryBytes(v, &acct) + return acct.Coins, errors.WithStack(err) } func getAddr(addr []byte, state state.KVStore) (coin.Coins, error) { diff --git a/stack/prefixstore.go b/stack/prefixstore.go index e9616ad521..cc0c5807ff 100644 --- a/stack/prefixstore.go +++ b/stack/prefixstore.go @@ -26,6 +26,20 @@ func stateSpace(store state.KVStore, app string) state.KVStore { store = pstore.store } // wrap it with the prefix - prefix := append([]byte(app), byte(0)) + prefix := makePrefix(app) return prefixStore{prefix, store} } + +func makePrefix(app string) []byte { + return append([]byte(app), byte(0)) +} + +// PrefixedKey gives us the absolute path to a key that is embedded in an +// application-specific state-space. +// +// This is useful for tests or utilities that have access to the global +// state to check individual app spaces. Individual apps should not be able +// to use this to read each other's space +func PrefixedKey(app string, key []byte) []byte { + return append(makePrefix(app), key...) +} From bb61b9fca300018ee3176cd60088207164ea7283 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 11 Jul 2017 15:35:43 +0200 Subject: [PATCH 10/14] Update cli to properly query into app state-space --- app/app_test.go | 16 +++-------- cmd/basecli/commands/query.go | 3 ++- .../counter/cmd/countercli/commands/query.go | 3 ++- docs/guide/counter/plugins/counter/counter.go | 2 +- stack/prefixstore.go | 27 ++++++++++++------- 5 files changed, 27 insertions(+), 24 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 38aa990cda..9656190c8d 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -5,7 +5,6 @@ import ( "os" "testing" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -83,17 +82,10 @@ func (at *appTest) reset() { require.True(at.t, resabci.IsOK(), resabci) } -func getBalance(key basecoin.Actor, state state.KVStore) (coin.Coins, error) { - var acct coin.Account - k := stack.PrefixedKey(coin.NameCoin, key.Bytes()) - v := state.Get(k) - // empty if no data - if len(v) == 0 { - return nil, nil - } - // otherwise read it - err := wire.ReadBinaryBytes(v, &acct) - return acct.Coins, errors.WithStack(err) +func getBalance(key basecoin.Actor, store state.KVStore) (coin.Coins, error) { + cspace := stack.PrefixedStore(coin.NameCoin, store) + acct, err := coin.GetAccount(cspace, key) + return acct.Coins, err } func getAddr(addr []byte, state state.KVStore) (coin.Coins, error) { diff --git a/cmd/basecli/commands/query.go b/cmd/basecli/commands/query.go index aba5fbf550..3d5b4fb066 100644 --- a/cmd/basecli/commands/query.go +++ b/cmd/basecli/commands/query.go @@ -13,6 +13,7 @@ import ( "github.com/tendermint/basecoin/modules/auth" "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" ) // AccountQueryCmd - command to query an account @@ -27,7 +28,7 @@ func doAccountQuery(cmd *cobra.Command, args []string) error { if err != nil { return err } - key := coin.NewAccountant("").MakeKey(auth.SigPerm(addr)) + key := stack.PrefixedKey(coin.NameCoin, auth.SigPerm(addr).Bytes()) acc := coin.Account{} proof, err := proofcmd.GetAndParseAppProof(key, &acc) diff --git a/docs/guide/counter/cmd/countercli/commands/query.go b/docs/guide/counter/cmd/countercli/commands/query.go index be0c502652..489a8a01d5 100644 --- a/docs/guide/counter/cmd/countercli/commands/query.go +++ b/docs/guide/counter/cmd/countercli/commands/query.go @@ -6,6 +6,7 @@ import ( proofcmd "github.com/tendermint/light-client/commands/proofs" "github.com/tendermint/basecoin/docs/guide/counter/plugins/counter" + "github.com/tendermint/basecoin/stack" ) //CounterQueryCmd - CLI command to query the counter state @@ -16,7 +17,7 @@ var CounterQueryCmd = &cobra.Command{ } func counterQueryCmd(cmd *cobra.Command, args []string) error { - key := counter.StateKey() + key := stack.PrefixedKey(counter.NameCounter, counter.StateKey()) var cp counter.State proof, err := proofcmd.GetAndParseAppProof(key, &cp) diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 0bf38f362d..168886160b 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -195,7 +195,7 @@ type State struct { // StateKey - store key for the counter state func StateKey() []byte { - return []byte(NameCounter + "/state") + return []byte("state") } // LoadState - retrieve the counter state from the store diff --git a/stack/prefixstore.go b/stack/prefixstore.go index cc0c5807ff..f6934d6e73 100644 --- a/stack/prefixstore.go +++ b/stack/prefixstore.go @@ -20,26 +20,35 @@ func (p prefixStore) Get(key []byte) (value []byte) { } // stateSpace will unwrap any prefixStore and then add the prefix +// +// this can be used by the middleware and dispatcher to isolate one space, +// then unwrap and isolate another space func stateSpace(store state.KVStore, app string) state.KVStore { // unwrap one-level if wrapped if pstore, ok := store.(prefixStore); ok { store = pstore.store } - // wrap it with the prefix - prefix := makePrefix(app) + return PrefixedStore(app, store) +} + +// PrefixedStore allows one to create an isolated state-space for a given +// app prefix, but it cannot easily be unwrapped +// +// This is useful for tests or utilities that have access to the global +// state to check individual app spaces. Individual apps should not be able +// to use this to read each other's space +func PrefixedStore(app string, store state.KVStore) state.KVStore { + prefix := append([]byte(app), byte(0)) return prefixStore{prefix, store} } -func makePrefix(app string) []byte { - return append([]byte(app), byte(0)) -} - -// PrefixedKey gives us the absolute path to a key that is embedded in an -// application-specific state-space. +// PrefixedKey returns the absolute path to a given key in a particular +// app's state-space // // This is useful for tests or utilities that have access to the global // state to check individual app spaces. Individual apps should not be able // to use this to read each other's space func PrefixedKey(app string, key []byte) []byte { - return append(makePrefix(app), key...) + prefix := append([]byte(app), byte(0)) + return append(prefix, key...) } From 7e7f124bc92fc0e580b98c63b8bef1ba64b0fd53 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 12 Jul 2017 16:22:17 +0200 Subject: [PATCH 11/14] Test create-role and fix CheckTx --- modules/roles/handler.go | 9 +++++-- modules/roles/handler_test.go | 50 +++++++++++++++++++++++++++++++++++ modules/roles/store.go | 11 ++++++-- modules/roles/tx.go | 2 +- 4 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 modules/roles/handler_test.go diff --git a/modules/roles/handler.go b/modules/roles/handler.go index ce1ab137d2..2ade96ee1c 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -28,8 +28,13 @@ func (Handler) Name() string { // CheckTx verifies if the transaction is properly formated func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { - _, err = checkTx(ctx, tx) - return res, err + var cr CreateRoleTx + cr, err = checkTx(ctx, tx) + if err != nil { + return + } + err = checkNoRole(store, cr.Role) + return } // DeliverTx tries to create a new role. diff --git a/modules/roles/handler_test.go b/modules/roles/handler_test.go new file mode 100644 index 0000000000..e5c744bbc8 --- /dev/null +++ b/modules/roles/handler_test.go @@ -0,0 +1,50 @@ +package roles_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/modules/roles" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/state" +) + +func TestCreateRole(t *testing.T) { + assert := assert.New(t) + + a := basecoin.Actor{App: "foo", Address: []byte("bar")} + b := basecoin.Actor{ChainID: "eth", App: "foo", Address: []byte("bar")} + c := basecoin.Actor{App: "foo", Address: []byte("baz")} + d := basecoin.Actor{App: "si-ly", Address: []byte("bar")} + + cases := []struct { + valid bool + role string + min uint32 + sigs []basecoin.Actor + }{ + {true, "awesome", 1, []basecoin.Actor{a}}, + {true, "cool", 2, []basecoin.Actor{b, c, d}}, + {false, "oops", 3, []basecoin.Actor{a, d}}, // too many + {false, "ugh", 0, []basecoin.Actor{a, d}}, // too few + {false, "phew", 1, []basecoin.Actor{}}, // none + {false, "cool", 1, []basecoin.Actor{c, d}}, // duplicate of existing one + } + + h := roles.NewHandler() + ctx := stack.MockContext("role-chain", 123) + store := state.NewMemKVStore() + for i, tc := range cases { + tx := roles.NewCreateRoleTx([]byte(tc.role), tc.min, tc.sigs) + _, err := h.CheckTx(ctx, store, tx) + _, err2 := h.DeliverTx(ctx, store, tx) + if tc.valid { + assert.Nil(err, "%d/%s: %+v", i, tc.role, err) + assert.Nil(err2, "%d/%s: %+v", i, tc.role, err2) + } else { + assert.NotNil(err, "%d/%s", i, tc.role) + assert.NotNil(err2, "%d/%s", i, tc.role) + } + } +} diff --git a/modules/roles/store.go b/modules/roles/store.go index 4b3d4f3d14..c29d93f0f8 100644 --- a/modules/roles/store.go +++ b/modules/roles/store.go @@ -68,11 +68,18 @@ func loadRole(store state.KVStore, key []byte) (role Role, err error) { return role, nil } -// we only have create here, no update, since we don't allow update yet -func createRole(store state.KVStore, key []byte, role Role) error { +func checkNoRole(store state.KVStore, key []byte) error { if _, err := loadRole(store, key); !IsNoRoleErr(err) { return ErrRoleExists() } + return nil +} + +// we only have create here, no update, since we don't allow update yet +func createRole(store state.KVStore, key []byte, role Role) error { + if err := checkNoRole(store, key); err != nil { + return err + } bin := wire.BinaryBytes(role) store.Set(key, bin) return nil // real stores can return error... diff --git a/modules/roles/tx.go b/modules/roles/tx.go index 6215f44e69..156b13a89c 100644 --- a/modules/roles/tx.go +++ b/modules/roles/tx.go @@ -7,7 +7,7 @@ import ( "github.com/tendermint/basecoin/errors" ) -const ( +var ( // MaxMembers it the maximum number of members in a Role. Used to avoid // extremely large roles. // Value is arbitrary, please adjust as needed From 9ea34e9c8f4459ea477128ec8d62af91973406be Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 12 Jul 2017 16:53:07 +0200 Subject: [PATCH 12/14] Add CheckHandler as a helper --- stack/helpers.go | 60 ++++++++++++++++++++++++++++++++++++++++--- stack/helpers_test.go | 35 +++++++++++++++++++++++++ stack/helperware.go | 4 +-- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/stack/helpers.go b/stack/helpers.go index a3dd296e5f..a21c867fa3 100644 --- a/stack/helpers.go +++ b/stack/helpers.go @@ -18,14 +18,19 @@ const ( //nolint const ( - ByteRawTx = 0x1 - TypeRawTx = "raw" + ByteRawTx = 0xF0 + ByteCheckTx = 0xF1 + + TypeRawTx = "raw" + TypeCheckTx = NameCheck + "/tx" + rawMaxSize = 2000 * 1000 ) func init() { basecoin.TxMapper. - RegisterImplementation(RawTx{}, TypeRawTx, ByteRawTx) + RegisterImplementation(RawTx{}, TypeRawTx, ByteRawTx). + RegisterImplementation(CheckTx{}, TypeCheckTx, ByteCheckTx) } // RawTx just contains bytes that can be hex-ified @@ -49,6 +54,24 @@ func (r RawTx) ValidateBasic() error { return nil } +// CheckTx contains a list of permissions to be tested +type CheckTx struct { + Required []basecoin.Actor +} + +var _ basecoin.TxInner = CheckTx{} + +// nolint +func NewCheckTx(req []basecoin.Actor) basecoin.Tx { + return CheckTx{req}.Wrap() +} +func (c CheckTx) Wrap() basecoin.Tx { + return basecoin.Tx{c} +} +func (CheckTx) ValidateBasic() error { + return nil +} + // OKHandler just used to return okay to everything type OKHandler struct { Log string @@ -148,3 +171,34 @@ func (p PanicHandler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx ba } panic(p.Msg) } + +// CheckHandler accepts CheckTx and verifies the permissions +type CheckHandler struct { + PassOption +} + +// Name - return handler's name +func (CheckHandler) Name() string { + return NameCheck +} + +// CheckTx verifies the permissions +func (c CheckHandler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + check, ok := tx.Unwrap().(CheckTx) + if !ok { + return res, errors.ErrUnknownTxType(tx) + } + + for _, perm := range check.Required { + if !ctx.HasPermission(perm) { + return res, errors.ErrUnauthorized() + } + } + return res, nil +} + +// DeliverTx verifies the permissions +func (c CheckHandler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + // until something changes, just do the same as check + return c.CheckTx(ctx, store, tx) +} diff --git a/stack/helpers_test.go b/stack/helpers_test.go index 58748e054d..a45732c16b 100644 --- a/stack/helpers_test.go +++ b/stack/helpers_test.go @@ -62,3 +62,38 @@ func TestPanic(t *testing.T) { assert.Panics(func() { fail.CheckTx(ctx, store, tx) }) assert.Panics(func() { fail.DeliverTx(ctx, store, tx) }) } + +func TestCheck(t *testing.T) { + assert := assert.New(t) + + ctx := MockContext("check-chain", 123) + store := state.NewMemKVStore() + h := CheckHandler{} + + a := basecoin.Actor{App: "foo", Address: []byte("baz")} + b := basecoin.Actor{App: "si-ly", Address: []byte("bar")} + + cases := []struct { + valid bool + signers, required []basecoin.Actor + }{ + {true, []basecoin.Actor{a}, []basecoin.Actor{a}}, + {true, []basecoin.Actor{a, b}, []basecoin.Actor{a}}, + {false, []basecoin.Actor{a}, []basecoin.Actor{a, b}}, + {false, []basecoin.Actor{a}, []basecoin.Actor{b}}, + } + + for i, tc := range cases { + tx := CheckTx{tc.required}.Wrap() + myCtx := ctx.WithPermissions(tc.signers...) + _, err := h.CheckTx(myCtx, store, tx) + _, err2 := h.DeliverTx(myCtx, store, tx) + if tc.valid { + assert.Nil(err, "%d: %+v", i, err) + assert.Nil(err2, "%d: %+v", i, err2) + } else { + assert.NotNil(err, "%d", i) + assert.NotNil(err2, "%d", i) + } + } +} diff --git a/stack/helperware.go b/stack/helperware.go index adca481c6d..46cdfaa8cc 100644 --- a/stack/helperware.go +++ b/stack/helperware.go @@ -8,8 +8,8 @@ import ( ) const ( - NameCheck = "chck" - NameGrant = "grnt" + NameCheck = "check" + NameGrant = "grant" ) // CheckMiddleware returns an error if the tx doesn't have auth of this From 33c9aa96f39cb370b1d9bdd465c54b85180ad0ae Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 12 Jul 2017 17:24:12 +0200 Subject: [PATCH 13/14] First end-to-end role tests --- modules/roles/middleware_test.go | 95 ++++++++++++++++++++++++++++++++ modules/roles/tx.go | 15 +++++ stack/helpers.go | 4 +- stack/helpers_test.go | 1 + 4 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 modules/roles/middleware_test.go diff --git a/modules/roles/middleware_test.go b/modules/roles/middleware_test.go new file mode 100644 index 0000000000..ecf83b267f --- /dev/null +++ b/modules/roles/middleware_test.go @@ -0,0 +1,95 @@ +package roles_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/tendermint/go-wire/data" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/modules/roles" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/state" +) + +// shortcut for the lazy +type ba []basecoin.Actor + +func createRole(app basecoin.Handler, store state.KVStore, + name []byte, min uint32, sigs ...basecoin.Actor) (basecoin.Actor, error) { + tx := roles.NewCreateRoleTx(name, min, sigs) + ctx := stack.MockContext("foo", 1) + _, err := app.DeliverTx(ctx, store, tx) + return roles.NewPerm(name), err +} + +func TestAssumeRole(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + // one handle to add a role, another to check permissions + disp := stack.NewDispatcher( + stack.WrapHandler(roles.NewHandler()), + stack.WrapHandler(stack.CheckHandler{}), + ) + // and wrap with the roles middleware + app := stack.New(roles.NewMiddleware()).Use(disp) + + // basic state for the app + ctx := stack.MockContext("role-chain", 123) + store := state.NewMemKVStore() + + // potential actors + a := basecoin.Actor{App: "sig", Address: []byte("jae")} + b := basecoin.Actor{App: "sig", Address: []byte("bucky")} + c := basecoin.Actor{App: "sig", Address: []byte("ethan")} + d := basecoin.Actor{App: "tracko", Address: []byte("rigel")} + + // devs is a 2-of-3 multisig + devs := data.Bytes{0, 1, 0, 1} + pdev, err := createRole(app, store, devs, 2, b, c, d) + require.Nil(err) + + // deploy requires a dev role, or supreme authority + deploy := data.Bytes("deploy") + _, err = createRole(app, store, deploy, 1, a, pdev) + require.Nil(err) + + // now, let's test the roles are set properly + cases := []struct { + valid bool + roles []data.Bytes // which roles we try to assume (can be multiple!) + signers []basecoin.Actor // which people sign the tx + required []basecoin.Actor // which permission we require to succeed + }{ + // basic checks to see logic works + {true, nil, nil, nil}, + {true, nil, ba{b, c}, ba{b}}, + {false, nil, ba{b}, ba{b, c}}, + + // simple role check + } + + for i, tc := range cases { + // set the signers, the required check + myCtx := ctx.WithPermissions(tc.signers...) + tx := stack.NewCheckTx(tc.required) + // and the roles we attempt to assume + for _, r := range tc.roles { + tx = roles.NewAssumeRoleTx(r, tx) + } + + // try CheckTx and DeliverTx and make sure they both assert permissions + _, err := app.CheckTx(myCtx, store, tx) + _, err2 := app.DeliverTx(myCtx, store, tx) + if tc.valid { + assert.Nil(err, "%d: %+v", i, err) + assert.Nil(err2, "%d: %+v", i, err2) + } else { + assert.NotNil(err, "%d", i) + assert.NotNil(err2, "%d", i) + } + } +} diff --git a/modules/roles/tx.go b/modules/roles/tx.go index 156b13a89c..48f5d5392a 100644 --- a/modules/roles/tx.go +++ b/modules/roles/tx.go @@ -14,6 +14,21 @@ var ( MaxMembers = 20 ) +//nolint +const ( + ByteAssumeRoleTx = 0x23 + ByteCreateRoleTx = 0x24 + + TypeAssumeRoleTx = NameRole + "/assume" // no prefix needed as it is middleware + TypeCreateRoleTx = NameRole + "/create" // prefix needed for dispatcher +) + +func init() { + basecoin.TxMapper. + RegisterImplementation(AssumeRoleTx{}, TypeAssumeRoleTx, ByteAssumeRoleTx). + RegisterImplementation(CreateRoleTx{}, TypeCreateRoleTx, ByteCreateRoleTx) +} + // AssumeRoleTx is a layered tx that can wrap your normal tx to give it // the authority to use a given role. type AssumeRoleTx struct { diff --git a/stack/helpers.go b/stack/helpers.go index a21c867fa3..5dce066075 100644 --- a/stack/helpers.go +++ b/stack/helpers.go @@ -174,9 +174,11 @@ func (p PanicHandler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx ba // CheckHandler accepts CheckTx and verifies the permissions type CheckHandler struct { - PassOption + basecoin.NopOption } +var _ basecoin.Handler = CheckHandler{} + // Name - return handler's name func (CheckHandler) Name() string { return NameCheck diff --git a/stack/helpers_test.go b/stack/helpers_test.go index a45732c16b..888bb790f2 100644 --- a/stack/helpers_test.go +++ b/stack/helpers_test.go @@ -77,6 +77,7 @@ func TestCheck(t *testing.T) { valid bool signers, required []basecoin.Actor }{ + {true, nil, nil}, {true, []basecoin.Actor{a}, []basecoin.Actor{a}}, {true, []basecoin.Actor{a, b}, []basecoin.Actor{a}}, {false, []basecoin.Actor{a}, []basecoin.Actor{a, b}}, From 60f3ecd9b1b09c315258fb013e790feb8a0ac044 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 12 Jul 2017 17:30:48 +0200 Subject: [PATCH 14/14] Test multiple nested roles --- modules/roles/middleware_test.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/modules/roles/middleware_test.go b/modules/roles/middleware_test.go index ecf83b267f..69f60a1ca7 100644 --- a/modules/roles/middleware_test.go +++ b/modules/roles/middleware_test.go @@ -53,14 +53,17 @@ func TestAssumeRole(t *testing.T) { require.Nil(err) // deploy requires a dev role, or supreme authority + // shows how we can build larger constructs, eg. (A and B) OR C deploy := data.Bytes("deploy") - _, err = createRole(app, store, deploy, 1, a, pdev) + pdeploy, err := createRole(app, store, deploy, 1, a, pdev) require.Nil(err) // now, let's test the roles are set properly cases := []struct { - valid bool - roles []data.Bytes // which roles we try to assume (can be multiple!) + valid bool + // which roles we try to assume (can be multiple!) + // note: that wrapping is FILO, so tries to assume last role first + roles []data.Bytes signers []basecoin.Actor // which people sign the tx required []basecoin.Actor // which permission we require to succeed }{ @@ -70,6 +73,14 @@ func TestAssumeRole(t *testing.T) { {false, nil, ba{b}, ba{b, c}}, // simple role check + {false, []data.Bytes{devs}, ba{a, b}, ba{pdev}}, // not enough sigs + {false, nil, ba{b, c}, ba{pdev}}, // must explicitly request group status + {true, []data.Bytes{devs}, ba{b, c}, ba{pdev}}, // ahh... better + {true, []data.Bytes{deploy}, ba{a, b}, ba{b, pdeploy}}, // deploy also works + + // multiple levels of roles - must be in correct order - assume dev, then deploy + {false, []data.Bytes{devs, deploy}, ba{c, d}, ba{pdeploy}}, + {true, []data.Bytes{deploy, devs}, ba{c, d}, ba{pdev, pdeploy}}, } for i, tc := range cases {