Skip to content
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/tanlang/add user isolation / 增加用户数据隔离 #293

Merged
merged 21 commits into from
Dec 28, 2022

Conversation

LinZexiao
Copy link
Collaborator

@LinZexiao LinZexiao commented Dec 1, 2022

关联的Issues (Related Issues)

close filecoin-project/venus#5609
ref filecoin-project/venus#5384

改动 (Proposed Changes)

附注 (Additional Info)

自查清单 (Checklist)

在你认为本 PR 满足被审阅的标准之前,需要确保 / Before you mark the PR ready for review, please make sure that:

  • 符合Venus项目管理规范中关于PR的相关标准 / The PR follows the PR standards set out in the Venus project management guidelines
  • 具有清晰明确的commit message / All commits have a clear commit message.
  • 包含相关的的测试用例或者不需要新增测试用例 / This PR has tests for new functionality or change in behaviour or not need to add new tests.
  • 包含相关的的指南以及文档或者不需要新增文档 / This PR has updated usage guidelines and documentation or not need
  • 通过必要的检查项 / All checks are green

实现细节 (Detail)

不需要用户隔离的接口

SetSharedParams(ctx context.Context, params *mtypes.SharedSpec) //perm:admin
SetLogLevel(ctx context.Context, subsystem, level string) //perm:admin
SaveNode(ctx context.Context, node *mtypes.Node) //perm:admin
ListNode(ctx context.Context) //perm:admin
HasNode(ctx context.Context, name string) //perm:admin
GetSharedParams(ctx context.Context) //perm:admin
GetNode(ctx context.Context, name string) //perm:admin
DeleteNode(ctx context.Context, name string) //perm:admin
ListMessageByAddress(ctx context.Context, addr address.Address) //perm:admin
ListMessageByFromState(ctx context.Context, from address.Address, state mtypes.MessageState, isAsc bool, pageIndex, pageSize int) //perm:admin
UpdateAllFilledMessage(ctx context.Context) //perm:admin
RepublishMessage(ctx context.Context, id string) //perm:admin
LogList(context.Context) //perm:write
NetPeers //perm:read
NetFindPeer //perm:read
NetAddrsListen //perm:read
NetConnect //perm:admin
UpdateNonce(ctx context.Context, addr address.Address, nonce uint64) //perm:admin
HasMessageByUid(ctx context.Context, id string) //perm:read

已经实现用户隔离的接口

HasAddress(ctx context.Context, addr address.Address) //perm:read
GetMessageByUnsignedCid(ctx context.Context, cid cid.Cid) //perm:read
GetMessageByUid(ctx context.Context, id string) //perm:read
GetMessageBySignedCid(ctx context.Context, cid cid.Cid) //perm:read
GetMessageByFromAndNonce(ctx context.Context, from address.Address, nonce uint64) //perm:read
PushMessageWithId(ctx context.Context, id string, msg *types.Message, meta *mtypes.SendSpec) //perm:write
PushMessage(ctx context.Context, msg *types.Message, meta *mtypes.SendSpec) //perm:write
WalletHas(ctx context.Context, addr address.Address) //perm:read
WaitMessage(ctx context.Context, id string, confidence uint64) //perm:read
UpdateMessageStateByID(ctx context.Context, id string, state mtypes.MessageState) //perm:admin
UpdateFilledMessageByID(ctx context.Context, id string) //perm:admin
SetFeeParams(ctx context.Context, params *mtypes.AddressSpec) //perm:admin
ReplaceMessage(ctx context.Context, params *mtypes.ReplacMessageParams) //perm:admin
RecoverFailedMsg(ctx context.Context, addr address.Address) //perm:admin
MarkBadMessage(ctx context.Context, id string) //perm:admin
ListMessage(ctx context.Context , p *types.MsgQueryParams) //perm:admin change params
ListFailedMessage(ctx context.Context) //perm:admin
ListBlockedMessage(ctx context.Context, addr address.Address, d time.Duration) //perm:admin
ListAddress(ctx context.Context) //perm:admin
GetAddress(ctx context.Context, addr address.Address) //perm:admin
ForbiddenAddress(ctx context.Context, addr address.Address) //perm:admin
DeleteAddress(ctx context.Context, addr address.Address) //perm:admin
ClearUnFillMessage(ctx context.Context, addr address.Address) //perm:admin
ActiveAddress(ctx context.Context, addr address.Address) //perm:admin
Send(ctx context.Context, params mtypes.QuickSendParams) //perm:sign
SetSelectMsgNum(ctx context.Context, addr address.Address, num uint64) //perm:admin

接口变动

参数变动

ListMessage(ctx context.Context ) -> ListMessage(ctx context.Context , p *types.MsgQueryParams)

接口权限变动

LogList(context.Context) //perm:write -> admin
UpdateMessageStateByID(ctx context.Context, id string, state mtypes.MessageState) //perm:admin -> write
UpdateFilledMessageByID(ctx context.Context, id string) //perm:admin -> write
SetFeeParams(ctx context.Context, params *mtypes.AddressSpec) //perm:admin -> write
ReplaceMessage(ctx context.Context, params *mtypes.ReplacMessageParams) //perm:admin -> write
RecoverFailedMsg(ctx context.Context, addr address.Address) //perm:admin -> write
MarkBadMessage(ctx context.Context, id string) //perm:admin -> write
ListMessage(ctx context.Context , p *types.MsgQueryParams) //perm:admin -> read
ListFailedMessage(ctx context.Context) //perm:admin -> read
ListBlockedMessage(ctx context.Context, addr address.Address, d time.Duration) //perm:admin -> read
ListAddress(ctx context.Context) //perm:admin -> read
GetAddress(ctx context.Context, addr address.Address) //perm:admin -> read
ForbiddenAddress(ctx context.Context, addr address.Address) //perm:admin -> write
DeleteAddress(ctx context.Context, addr address.Address) //perm:admin -> write
ClearUnFillMessage(ctx context.Context, addr address.Address) //perm:admin ->write
ActiveAddress(ctx context.Context, addr address.Address) //perm:admin ->write
Send(ctx context.Context, params mtypes.QuickSendParams) //perm:admin -> sign
SetSelectMsgNum(ctx context.Context, addr address.Address, num uint64) //perm:admin -> write
NetPeers //perm:read -> admin
NetFindPeer //perm:read -> admin
NetAddrsListen //perm:read -> admin

接口测试

与本PR关联的测试主要有两个部分

  • 接口变动测试(权限变动,参数变动)
  • 用户数据隔离测试

第一部分直接根据变动的细节构造测试用例即可

用户数据隔离测试

第二部分需要实现在 msg 中生成至少两个用户的不同数据(包括 消息 ,和地址 )
判断数据是否能被某个用户访问的唯一标准是, 该用户是否具有关联地址的签名权, 也就是该签名地址是否支持测试用户

部分测试用例示例:

测试数据

用户1: admin 权限
用户2: sign权限
用户3: sign权限

消息1: 由地址1 发送
消息2: 由地址2 发送
消息3: 由地址3 发送

地址1: 支持用户2
地址2: 支持用户2
地址3: 支持用户3

预期结果

用户1 可以访问 所有的数据
用户2 可以访问 地址1,和地址2的关联所有数据,包括发消息的参数和已发送的消息,以及打算发送的消息
用户3 只能访问地址3的关联数据

举例说明:

  • GetMessageByUid 时,
    用户1 可以拿到三条消息,
    用户2 能够拿到消息1和消息2,
    用户3只能拿到消息3

@LinZexiao LinZexiao force-pushed the feat/tanlang/add-user-isolation branch from 534acb4 to 873b206 Compare December 1, 2022 04:04
@LinZexiao LinZexiao marked this pull request as draft December 2, 2022 06:12
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
@LinZexiao LinZexiao changed the base branch from master to dev/v1.10 December 7, 2022 02:54
@LinZexiao LinZexiao force-pushed the feat/tanlang/add-user-isolation branch from 2c7c4bb to d4dc617 Compare December 7, 2022 03:10
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
api/messager_impl.go Outdated Show resolved Hide resolved
@hunjixin
Copy link
Contributor

hunjixin commented Dec 8, 2022

UpdateFilledMessageByID实现中,如果message的状态已经是Onchain或者是replace, 这不需要再去更新状态了。防止用户,反复点击更新状态。

@LinZexiao
Copy link
Collaborator Author

LinZexiao commented Dec 8, 2022

UpdateFilledMessageByID实现中,如果message的状态已经是Onchain或者是replace, 这不需要再去更新状态了。防止用户,反复点击更新状态。

已经修改

@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2022

Codecov Report

❗ No coverage uploaded for pull request base (dev/v1.10.0@8d4a584). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff               @@
##             dev/v1.10.0     #293   +/-   ##
==============================================
  Coverage               ?   55.09%           
==============================================
  Files                  ?       69           
  Lines                  ?     7226           
  Branches               ?        0           
==============================================
  Hits                   ?     3981           
  Misses                 ?     2923           
  Partials               ?      322           
Flag Coverage Δ
unittests 55.09% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

@LinZexiao LinZexiao marked this pull request as ready for review December 13, 2022 09:25
api/mesager_impl_test.go Outdated Show resolved Hide resolved
@LinZexiao LinZexiao force-pushed the feat/tanlang/add-user-isolation branch 2 times, most recently from c10dc3f to 784a1e7 Compare December 16, 2022 09:26
Copy link
Collaborator

@simlecode simlecode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fix test case
migrate utils to check permission
@LinZexiao LinZexiao force-pushed the feat/tanlang/add-user-isolation branch from 46fb62b to b1e273c Compare December 20, 2022 06:38
utils/util.go Outdated Show resolved Hide resolved
Copy link
Contributor

@hunjixin hunjixin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hunjixin
Copy link
Contributor

hunjixin commented Dec 22, 2022

@LinZexiao 麻烦增加一个测试说明并给issue打needtesting标签

@LinZexiao LinZexiao force-pushed the feat/tanlang/add-user-isolation branch from 0d8e2ff to 762da0f Compare December 27, 2022 05:21
@LinZexiao LinZexiao merged commit 4181c8c into dev/v1.10.0 Dec 28, 2022
@LinZexiao LinZexiao deleted the feat/tanlang/add-user-isolation branch December 28, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

4 participants