LaconicNetwork/cosmos-sdk
10
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix(circuit): Reset only doable by authorized account (backport #17511) (#17581)

Browse Source 
Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
This commit is contained in:
mergify[bot] 2023-08-30 14:18:05 +00:00 committed by GitHub
parent 6e68abc2ae
commit ac64612dc2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 119 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

112
x/circuit/keeper/msg_server.go
View File

@ -86,51 +86,33 @@ func (srv msgServer) TripCircuitBreaker(ctx context.Context, msg *types.MsgTripC
return nil, err
}
switch {
case perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || bytes.Equal(address, srv.GetAuthority()):
for _, msgTypeURL := range msg.MsgTypeUrls {
// check if the message is in the list of allowed messages
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
if err != nil {
return nil, err
}
if !isAllowed {
return nil, fmt.Errorf("message %s is already disabled", msgTypeURL)
}
if err = srv.DisableList.Set(ctx, msgTypeURL); err != nil {
return nil, err
}
for _, msgTypeURL := range msg.MsgTypeUrls {
// check if the message is in the list of allowed messages
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
if err != nil {
return nil, err
}
case perms.Level == types.Permissions_LEVEL_SOME_MSGS:
for _, msgTypeURL := range msg.MsgTypeUrls {
// check if the message is in the list of allowed messages
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
if err != nil {
return nil, err
}
if !isAllowed {
return nil, fmt.Errorf("message %s is already disabled", msgTypeURL)
}
permExists := false
for _, msgurl := range perms.LimitTypeUrls {
if msgTypeURL == msgurl {
permExists = true
}
}
if !isAllowed {
return nil, fmt.Errorf("message %s is already disabled", msgTypeURL)
}
if !permExists {
switch {
case perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || bytes.Equal(address, srv.GetAuthority()):
// if the sender is a super admin or the module authority, no need to check perms
case perms.Level == types.Permissions_LEVEL_SOME_MSGS:
// if the sender has permission for some messages, check if the sender has permission for this specific message
if !hasPermissionForMsg(perms, msgTypeURL) {
return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "account does not have permission to trip circuit breaker for message %s", msgTypeURL)
}
if err = srv.DisableList.Set(ctx, msgTypeURL); err != nil {
return nil, err
}
default:
return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to trip circuit breaker")
}
default:
return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to trip circuit breaker")
if err = srv.DisableList.Set(ctx, msgTypeURL); err != nil {
return nil, err
}
}
urls := strings.Join(msg.GetMsgTypeUrls(), ",")
@ -164,24 +146,32 @@ func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgRese
return nil, err
}
if perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || perms.Level == types.Permissions_LEVEL_SOME_MSGS || bytes.Equal(address, srv.GetAuthority()) {
// add all msg type urls to the disable list
for _, msgTypeURL := range msg.MsgTypeUrls {
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
if err != nil {
return nil, err
}
if isAllowed {
return nil, fmt.Errorf("message %s is not disabled", msgTypeURL)
}
if err = srv.DisableList.Remove(ctx, msgTypeURL); err != nil {
return nil, err
}
for _, msgTypeURL := range msg.MsgTypeUrls {
// check if the message is in the list of allowed messages
isAllowed, err := srv.IsAllowed(ctx, msgTypeURL)
if err != nil {
return nil, err
}
if isAllowed {
return nil, fmt.Errorf("message %s is not disabled", msgTypeURL)
}
switch {
case perms.Level == types.Permissions_LEVEL_SUPER_ADMIN || perms.Level == types.Permissions_LEVEL_ALL_MSGS || bytes.Equal(address, srv.GetAuthority()):
// if the sender is a super admin or the module authority, no need to check perms
case perms.Level == types.Permissions_LEVEL_SOME_MSGS:
// if the sender has permission for some messages, check if the sender has permission for this specific message
if !hasPermissionForMsg(perms, msgTypeURL) {
return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "account does not have permission to reset circuit breaker for message %s", msgTypeURL)
}
default:
return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to reset circuit breaker")
}
if err = srv.DisableList.Remove(ctx, msgTypeURL); err != nil {
return nil, err
}
} else {
return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "account does not have permission to reset circuit breaker")
}
urls := strings.Join(msg.GetMsgTypeUrls(), ",")
@ -197,3 +187,13 @@ func (srv msgServer) ResetCircuitBreaker(ctx context.Context, msg *types.MsgRese
return &types.MsgResetCircuitBreakerResponse{Success: true}, nil
}
// hasPermissionForMsg returns true if the account can trip or reset the message.
func hasPermissionForMsg(perms types.Permissions, msg string) bool {
for _, msgurl := range perms.LimitTypeUrls {
if msg == msgurl {
return true
}
}
return false
}

69
x/circuit/keeper/msg_server_test.go
View File

@ -194,7 +194,7 @@ func TestTripCircuitBreaker(t *testing.T) {
require.ErrorContains(t, err, "MsgEditValidator: unauthorized")
// user tries to trip an already tripped circuit breaker
alreadyTripped := "cosmos.bank.v1beta1.MsgSend"
alreadyTripped := msgSend
twoTrip := &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{alreadyTripped}}
_, err = srv.TripCircuitBreaker(ft.ctx, twoTrip)
require.ErrorContains(t, err, "already disabled")
@ -208,7 +208,7 @@ func TestResetCircuitBreaker(t *testing.T) {
srv := keeper.NewMsgServerImpl(ft.keeper)
// admin resets circuit breaker
url := "cosmos.bank.v1beta1.MsgSend"
url := msgSend
// admin trips circuit breaker
admintrip := &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url}}
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
@ -291,7 +291,7 @@ func TestResetCircuitBreaker(t *testing.T) {
require.NoError(t, err)
// user with some messages resets circuit breaker
someMsgsReset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}}
someMsgsReset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url2}}
_, err = srv.ResetCircuitBreaker(ft.ctx, someMsgsReset)
require.NoError(t, err)
require.Equal(
@ -299,14 +299,14 @@ func TestResetCircuitBreaker(t *testing.T) {
sdk.NewEvent(
"reset_circuit_breaker",
sdk.NewAttribute("authority", addresses[2]),
sdk.NewAttribute("msg_url", url),
sdk.NewAttribute("msg_url", url2),
),
lastEvent(ft.ctx),
)
// user tries to reset an already reset circuit breaker
admintrip = &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url2}}
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
someMsgsReset = &types.MsgResetCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url2}}
_, err = srv.ResetCircuitBreaker(ft.ctx, someMsgsReset)
require.Error(t, err)
}
@ -316,3 +316,60 @@ func lastEvent(ctx context.Context) sdk.Event {
return events[len(events)-1]
}
func TestResetCircuitBreakerSomeMsgs(t *testing.T) {
ft := initFixture(t)
authority, err := ft.ac.BytesToString(ft.mockAddr)
require.NoError(t, err)
srv := keeper.NewMsgServerImpl(ft.keeper)
// admin resets circuit breaker
url := msgSend
url2 := "the_only_message_acc2_can_trip_and_reset"
// add acc2 as an authorized account for only url2
authmsg := &types.MsgAuthorizeCircuitBreaker{
Granter: authority,
Grantee: addresses[2],
Permissions: &types.Permissions{
Level: types.Permissions_LEVEL_SOME_MSGS,
LimitTypeUrls: []string{url2},
},
}
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, authmsg)
require.NoError(t, err)
// admin trips circuit breaker
admintrip := &types.MsgTripCircuitBreaker{Authority: authority, MsgTypeUrls: []string{url, url2}}
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
require.NoError(t, err)
// sanity check, both messages should be tripped
allowed, err := ft.keeper.IsAllowed(ft.ctx, url)
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")
allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")
// now let's try to reset url using acc2 (should fail)
acc2Reset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}}
_, err = srv.ResetCircuitBreaker(ft.ctx, acc2Reset)
require.Error(t, err)
// now let's try to reset url2 using acc2 (should pass)
acc2Reset = &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url2}}
_, err = srv.ResetCircuitBreaker(ft.ctx, acc2Reset)
require.NoError(t, err)
// Only url2 should be reset, url should still be tripped
allowed, err = ft.keeper.IsAllowed(ft.ctx, url)
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")
allowed, err = ft.keeper.IsAllowed(ft.ctx, url2)
require.NoError(t, err)
require.True(t, allowed, "circuit breaker should be reset")
}