From 8de6dd9598b5f4ac3fc6278e671b4458d2ff2992 Mon Sep 17 00:00:00 2001 From: forcodedancing Date: Fri, 15 Sep 2023 17:57:19 -0600 Subject: [PATCH] chore: improve the validations of messages --- x/permission/types/types.go | 13 +++++++------ x/sp/types/message.go | 12 ++++++------ x/sp/types/message_test.go | 4 ++-- x/sp/types/util.go | 2 +- x/storage/types/message.go | 25 ++++++++++++++++++++++--- x/virtualgroup/types/message.go | 2 +- 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/x/permission/types/types.go b/x/permission/types/types.go index e9be026cd..8c641b492 100644 --- a/x/permission/types/types.go +++ b/x/permission/types/types.go @@ -26,12 +26,13 @@ var ( ACTION_UPDATE_BUCKET_INFO: true, ACTION_DELETE_BUCKET: true, - ACTION_CREATE_OBJECT: true, - ACTION_DELETE_OBJECT: true, - ACTION_GET_OBJECT: true, - ACTION_COPY_OBJECT: true, - ACTION_EXECUTE_OBJECT: true, - ACTION_LIST_OBJECT: true, + ACTION_CREATE_OBJECT: true, + ACTION_DELETE_OBJECT: true, + ACTION_GET_OBJECT: true, + ACTION_COPY_OBJECT: true, + ACTION_EXECUTE_OBJECT: true, + ACTION_LIST_OBJECT: true, + ACTION_UPDATE_OBJECT_INFO: true, ACTION_TYPE_ALL: true, } diff --git a/x/sp/types/message.go b/x/sp/types/message.go index 60a5b7409..e9ae11488 100644 --- a/x/sp/types/message.go +++ b/x/sp/types/message.go @@ -105,8 +105,9 @@ func (msg *MsgCreateStorageProvider) ValidateBasic() error { if _, err := sdk.AccAddressFromHexUnsafe(msg.GcAddress); err != nil { return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid gc address (%s)", err) } + //MaintenanceAddress is validated in msg server if !msg.Deposit.IsValid() || !msg.Deposit.Amount.IsPositive() { - return errors.Wrap(sdkerrors.ErrInvalidRequest, "invalid deposit amount") + return errors.Wrap(sdkerrors.ErrInvalidCoins, "invalid deposit amount") } if msg.Description == (Description{}) { return errors.Wrap(sdkerrors.ErrInvalidRequest, "empty description") @@ -114,11 +115,10 @@ func (msg *MsgCreateStorageProvider) ValidateBasic() error { if err := validateBlsKeyAndProof(msg.BlsKey, msg.BlsProof); err != nil { return err } - err := IsValidEndpointURL(msg.Endpoint) - if err != nil { + if err := ValidateEndpointURL(msg.Endpoint); err != nil { return errors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid endpoint (%s)", err) } - if msg.ReadPrice.IsNegative() || msg.StorePrice.IsNegative() { + if msg.ReadPrice.IsNil() || msg.ReadPrice.IsNegative() || msg.StorePrice.IsNil() || msg.StorePrice.IsNegative() { return errors.Wrap(sdkerrors.ErrInvalidRequest, "invalid price") } return nil @@ -177,7 +177,7 @@ func (msg *MsgEditStorageProvider) ValidateBasic() error { } if len(msg.Endpoint) != 0 { - err = IsValidEndpointURL(msg.Endpoint) + err = ValidateEndpointURL(msg.Endpoint) if err != nil { return errors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid endpoint (%s)", err) } @@ -369,7 +369,7 @@ func (msg *MsgUpdateStorageProviderStatus) ValidateBasic() error { return errors.Wrapf(sdkerrors.ErrInvalidRequest, "not allowed to update to status %s", msg.Status) } if msg.Status == STATUS_IN_MAINTENANCE && msg.Duration <= 0 { - return errors.Wrapf(sdkerrors.ErrInvalidRequest, "maintenanceDuration need to be set for %s", msg.Status) + return errors.Wrapf(sdkerrors.ErrInvalidRequest, "maintenance duration need to be set for %s", msg.Status) } return nil } diff --git a/x/sp/types/message_test.go b/x/sp/types/message_test.go index 0d71c20d9..8e67f7443 100644 --- a/x/sp/types/message_test.go +++ b/x/sp/types/message_test.go @@ -30,7 +30,7 @@ func TestMsgCreateStorageProvider_ValidateBasic(t *testing.T) { }{ {"basic", "a", "b", "c", "d", spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, blsPubKey, blsProof, coinPos, nil}, {"basic_empty", "a", "b", "c", "d", sdk.AccAddress{}, spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, blsPubKey, blsProof, coinPos, sdkerrors.ErrInvalidAddress}, - {"zero deposit", "a", "b", "c", "d", spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, blsPubKey, blsProof, coinZero, nil}, + {"zero deposit", "a", "b", "c", "d", spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, spAddr, blsPubKey, blsProof, coinZero, sdkerrors.ErrInvalidCoins}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -48,7 +48,7 @@ func TestMsgCreateStorageProvider_ValidateBasic(t *testing.T) { Endpoint: "http://127.0.0.1:9033", StorePrice: sdk.ZeroDec(), ReadPrice: sdk.ZeroDec(), - Deposit: coinPos, + Deposit: tt.deposit, } err := msg.ValidateBasic() if tt.err != nil { diff --git a/x/sp/types/util.go b/x/sp/types/util.go index 711dbbf50..956b9b104 100644 --- a/x/sp/types/util.go +++ b/x/sp/types/util.go @@ -7,7 +7,7 @@ import ( ) // Verify if input endpoint URL is valid. -func IsValidEndpointURL(endpointURL string) error { +func ValidateEndpointURL(endpointURL string) error { if endpointURL == "" { return errors.Wrap(ErrInvalidEndpointURL, "Endpoint url cannot be empty.") } diff --git a/x/storage/types/message.go b/x/storage/types/message.go index b6282a1ec..bf0563326 100644 --- a/x/storage/types/message.go +++ b/x/storage/types/message.go @@ -970,6 +970,11 @@ func (msg *MsgLeaveGroup) ValidateBasic() error { return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } + _, err = sdk.AccAddressFromHexUnsafe(msg.GroupOwner) + if err != nil { + return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid group owner (%s)", err) + } + err = s3util.CheckValidGroupName(msg.GroupName) if err != nil { return err @@ -1158,6 +1163,10 @@ func (msg *MsgPutPolicy) ValidateBasic() error { return errors.Wrapf(gnfderrors.ErrInvalidGRN, "invalid greenfield resource name (%s)", err) } + if msg.Principal == nil { + return gnfderrors.ErrInvalidPrincipal.Wrapf("principal cannot be empty") + } + if msg.Principal.Type == permtypes.PRINCIPAL_TYPE_GNFD_GROUP && grn.ResourceType() == resource.RESOURCE_TYPE_GROUP { return gnfderrors.ErrInvalidPrincipal.Wrapf("Not allow grant group's permission to another group") } @@ -1218,9 +1227,19 @@ func (msg *MsgDeletePolicy) ValidateBasic() error { return errors.Wrapf(gnfderrors.ErrInvalidGRN, "invalid greenfield resource name (%s)", err) } + if msg.Principal == nil { + return gnfderrors.ErrInvalidPrincipal.Wrapf("principal cannot be empty") + } + if msg.Principal.Type == permtypes.PRINCIPAL_TYPE_GNFD_GROUP && grn.ResourceType() == resource.RESOURCE_TYPE_GROUP { return gnfderrors.ErrInvalidPrincipal.Wrapf("Not allow grant group's permission to another group") } + + err = msg.Principal.ValidateBasic() + if err != nil { + return err + } + return nil } @@ -1266,7 +1285,7 @@ func (msg *MsgMirrorBucket) ValidateBasic() error { return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } - if msg.Id.GT(sdk.NewUint(0)) { + if !msg.Id.IsNil() && msg.Id.GT(sdk.NewUint(0)) { if msg.BucketName != "" { return errors.Wrap(gnfderrors.ErrInvalidBucketName, "Bucket name should be empty") } @@ -1324,7 +1343,7 @@ func (msg *MsgMirrorObject) ValidateBasic() error { return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } - if msg.Id.GT(sdk.NewUint(0)) { + if !msg.Id.IsNil() && msg.Id.GT(sdk.NewUint(0)) { if msg.BucketName != "" { return errors.Wrap(gnfderrors.ErrInvalidBucketName, "Bucket name should be empty") } @@ -1389,7 +1408,7 @@ func (msg *MsgMirrorGroup) ValidateBasic() error { return errors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err) } - if msg.Id.GT(sdk.NewUint(0)) { + if !msg.Id.IsNil() && msg.Id.GT(sdk.NewUint(0)) { if msg.GroupName != "" { return errors.Wrap(gnfderrors.ErrInvalidGroupName, "Group name should be empty") } diff --git a/x/virtualgroup/types/message.go b/x/virtualgroup/types/message.go index 8bc75b594..d56114c3b 100644 --- a/x/virtualgroup/types/message.go +++ b/x/virtualgroup/types/message.go @@ -192,7 +192,7 @@ func (msg *MsgWithdraw) ValidateBasic() error { } if !msg.Withdraw.IsValid() || !msg.Withdraw.Amount.IsPositive() { - return errors.Wrap(sdkerrors.ErrInvalidRequest, "invalid or non-positive deposit amount") + return errors.Wrap(sdkerrors.ErrInvalidRequest, "invalid or non-positive withdraw amount") } return nil }