diff --git a/commonspace/object/acl/list/aclrecordbuilder.go b/commonspace/object/acl/list/aclrecordbuilder.go index 3fe15ff2..1c0e0dd3 100644 --- a/commonspace/object/acl/list/aclrecordbuilder.go +++ b/commonspace/object/acl/list/aclrecordbuilder.go @@ -63,14 +63,16 @@ type aclRecordBuilder struct { id string keyStorage crypto.KeyStorage accountKeys *accountdata.AccountKeys + verifier AcceptorVerifier state *AclState } -func NewAclRecordBuilder(id string, keyStorage crypto.KeyStorage, keys *accountdata.AccountKeys) AclRecordBuilder { +func NewAclRecordBuilder(id string, keyStorage crypto.KeyStorage, keys *accountdata.AccountKeys, verifier AcceptorVerifier) AclRecordBuilder { return &aclRecordBuilder{ id: id, keyStorage: keyStorage, accountKeys: keys, + verifier: verifier, } } @@ -420,6 +422,10 @@ func (a *aclRecordBuilder) UnmarshallWithId(rawIdRecord *aclrecordproto.RawAclRe Model: aclRoot, } } else { + err = a.verifier.VerifyAcceptor(rawRec) + if err != nil { + return + } aclRecord := &aclrecordproto.AclRecord{} err = proto.Unmarshal(rawRec.Payload, aclRecord) if err != nil { diff --git a/commonspace/object/acl/list/aclstate.go b/commonspace/object/acl/list/aclstate.go index 0e2a6568..65dfea0a 100644 --- a/commonspace/object/acl/list/aclstate.go +++ b/commonspace/object/acl/list/aclstate.go @@ -25,6 +25,7 @@ var ( ErrInsufficientPermissions = errors.New("insufficient permissions") ErrIsOwner = errors.New("can't be made by owner") ErrIncorrectNumberOfAccounts = errors.New("incorrect number of accounts") + ErrDuplicateAccounts = errors.New("duplicate accounts") ErrNoReadKey = errors.New("acl state doesn't have a read key") ErrIncorrectReadKey = errors.New("incorrect read key") ErrInvalidSignature = errors.New("signature is invalid") diff --git a/commonspace/object/acl/list/list.go b/commonspace/object/acl/list/list.go index 86ee570f..cedf772c 100644 --- a/commonspace/object/acl/list/list.go +++ b/commonspace/object/acl/list/list.go @@ -85,10 +85,11 @@ type internalDeps struct { func BuildAclListWithIdentity(acc *accountdata.AccountKeys, storage liststorage.ListStorage, verifier AcceptorVerifier) (AclList, error) { keyStorage := crypto.NewKeyStorage() deps := internalDeps{ - storage: storage, - keyStorage: keyStorage, - stateBuilder: newAclStateBuilderWithIdentity(acc), - recordBuilder: NewAclRecordBuilder(storage.Id(), keyStorage, acc), + storage: storage, + keyStorage: keyStorage, + stateBuilder: newAclStateBuilderWithIdentity(acc), + recordBuilder: NewAclRecordBuilder(storage.Id(), keyStorage, acc, verifier), + acceptorVerifier: verifier, } return build(deps) } @@ -96,10 +97,11 @@ func BuildAclListWithIdentity(acc *accountdata.AccountKeys, storage liststorage. func BuildAclList(storage liststorage.ListStorage, verifier AcceptorVerifier) (AclList, error) { keyStorage := crypto.NewKeyStorage() deps := internalDeps{ - storage: storage, - keyStorage: keyStorage, - stateBuilder: newAclStateBuilder(), - recordBuilder: NewAclRecordBuilder(storage.Id(), keyStorage, nil), + storage: storage, + keyStorage: keyStorage, + stateBuilder: newAclStateBuilder(), + recordBuilder: NewAclRecordBuilder(storage.Id(), keyStorage, nil, verifier), + acceptorVerifier: verifier, } return build(deps) } @@ -215,15 +217,6 @@ func (a *aclList) AddRawRecord(rawRec *aclrecordproto.RawAclRecordWithId) (err e return } -func (a *aclList) IsValidNext(rawRec *aclrecordproto.RawAclRecordWithId) (err error) { - _, err = a.recordBuilder.UnmarshallWithId(rawRec) - if err != nil { - return - } - // TODO: change state and add "check" method for records - return -} - func (a *aclList) Id() string { return a.id } diff --git a/commonspace/object/acl/list/list_test.go b/commonspace/object/acl/list/list_test.go index 9f1ca77d..070e97d6 100644 --- a/commonspace/object/acl/list/list_test.go +++ b/commonspace/object/acl/list/list_test.go @@ -7,6 +7,7 @@ import ( "github.com/anyproto/any-sync/commonspace/object/accountdata" "github.com/anyproto/any-sync/commonspace/object/acl/aclrecordproto" "github.com/anyproto/any-sync/util/cidutil" + "github.com/anyproto/any-sync/util/crypto" "github.com/stretchr/testify/require" ) @@ -52,16 +53,14 @@ func newFixture(t *testing.T) *aclFixture { } } -func TestAclList_BuildRoot(t *testing.T) { - randomKeys, err := accountdata.NewRandom() +func (fx *aclFixture) addRec(t *testing.T, rec *aclrecordproto.RawAclRecordWithId) { + err := fx.ownerAcl.AddRawRecord(rec) require.NoError(t, err) - randomAcl, err := NewTestDerivedAcl("spaceId", randomKeys) + err = fx.accountAcl.AddRawRecord(rec) require.NoError(t, err) - fmt.Println(randomAcl.Id()) } -func TestAclList_InvitePipeline(t *testing.T) { - fx := newFixture(t) +func (fx *aclFixture) inviteAccount(t *testing.T, perms AclPermissions) { var ( ownerAcl = fx.ownerAcl ownerState = fx.ownerAcl.aclState @@ -72,10 +71,7 @@ func TestAclList_InvitePipeline(t *testing.T) { inv, err := ownerAcl.RecordBuilder().BuildInvite() require.NoError(t, err) inviteRec := wrapRecord(inv.InviteRec) - err = ownerAcl.AddRawRecord(inviteRec) - require.NoError(t, err) - err = accountAcl.AddRawRecord(inviteRec) - require.NoError(t, err) + fx.addRec(t, inviteRec) // building request join requestJoin, err := accountAcl.RecordBuilder().BuildRequestJoin(RequestJoinPayload{ @@ -84,22 +80,16 @@ func TestAclList_InvitePipeline(t *testing.T) { }) require.NoError(t, err) requestJoinRec := wrapRecord(requestJoin) - err = ownerAcl.AddRawRecord(requestJoinRec) - require.NoError(t, err) - err = accountAcl.AddRawRecord(requestJoinRec) - require.NoError(t, err) + fx.addRec(t, requestJoinRec) // building request accept requestAccept, err := ownerAcl.RecordBuilder().BuildRequestAccept(RequestAcceptPayload{ RequestRecordId: requestJoinRec.Id, - Permissions: AclPermissions(aclrecordproto.AclUserPermissions_Writer), + Permissions: perms, }) require.NoError(t, err) requestAcceptRec := wrapRecord(requestAccept) - err = ownerAcl.AddRawRecord(requestAcceptRec) - require.NoError(t, err) - err = accountAcl.AddRawRecord(requestAcceptRec) - require.NoError(t, err) + fx.addRec(t, requestAcceptRec) // checking acl state require.True(t, ownerState.Permissions(ownerState.pubKey).IsOwner()) @@ -109,3 +99,46 @@ func TestAclList_InvitePipeline(t *testing.T) { require.True(t, accountState.Permissions(ownerState.pubKey).IsOwner()) require.True(t, accountState.Permissions(accountState.pubKey).CanWrite()) } + +func TestAclList_BuildRoot(t *testing.T) { + randomKeys, err := accountdata.NewRandom() + require.NoError(t, err) + randomAcl, err := NewTestDerivedAcl("spaceId", randomKeys) + require.NoError(t, err) + fmt.Println(randomAcl.Id()) +} + +func TestAclList_InvitePipeline(t *testing.T) { + fx := newFixture(t) + fx.inviteAccount(t, AclPermissions(aclrecordproto.AclUserPermissions_Writer)) +} + +func TestAclList_Remove(t *testing.T) { + fx := newFixture(t) + var ( + ownerState = fx.ownerAcl.aclState + accountState = fx.accountAcl.aclState + ) + fx.inviteAccount(t, AclPermissions(aclrecordproto.AclUserPermissions_Writer)) + + newReadKey := crypto.NewAES() + remove, err := fx.ownerAcl.RecordBuilder().BuildAccountRemove(AccountRemovePayload{ + Identities: []crypto.PubKey{fx.accountKeys.SignKey.GetPublic()}, + ReadKey: newReadKey, + }) + require.NoError(t, err) + removeRec := wrapRecord(remove) + fx.addRec(t, removeRec) + + // checking acl state + require.True(t, ownerState.Permissions(ownerState.pubKey).IsOwner()) + require.True(t, ownerState.Permissions(accountState.pubKey).NoPermissions()) + require.True(t, ownerState.userReadKeys[removeRec.Id].Equals(newReadKey)) + require.NotNil(t, ownerState.userReadKeys[fx.ownerAcl.Id()]) + require.Equal(t, 0, len(ownerState.pendingRequests)) + require.Equal(t, 0, len(accountState.pendingRequests)) + require.True(t, accountState.Permissions(ownerState.pubKey).IsOwner()) + require.True(t, accountState.Permissions(accountState.pubKey).NoPermissions()) + require.Nil(t, accountState.userReadKeys[removeRec.Id]) + require.NotNil(t, accountState.userReadKeys[fx.ownerAcl.Id()]) +} diff --git a/commonspace/object/acl/list/listutils.go b/commonspace/object/acl/list/listutils.go index 10513ecb..d337dcc5 100644 --- a/commonspace/object/acl/list/listutils.go +++ b/commonspace/object/acl/list/listutils.go @@ -8,7 +8,7 @@ import ( ) func NewTestDerivedAcl(spaceId string, keys *accountdata.AccountKeys) (AclList, error) { - builder := NewAclRecordBuilder("", crypto.NewKeyStorage(), keys) + builder := NewAclRecordBuilder("", crypto.NewKeyStorage(), keys, NoOpAcceptorVerifier{}) masterKey, _, err := crypto.GenerateRandomEd25519KeyPair() if err != nil { return nil, err diff --git a/commonspace/object/acl/list/validator.go b/commonspace/object/acl/list/validator.go index 9b667fda..1296789c 100644 --- a/commonspace/object/acl/list/validator.go +++ b/commonspace/object/acl/list/validator.go @@ -161,11 +161,15 @@ func (c *contentValidator) ValidateAccountRemove(ch *aclrecordproto.AclAccountRe if !c.aclState.Permissions(authorIdentity).CanManageAccounts() { return ErrInsufficientPermissions } + seenIdentities := map[string]struct{}{} for _, rawIdentity := range ch.Identities { identity, err := c.keyStore.PubKeyFromProto(rawIdentity) if err != nil { return err } + if identity.Equals(authorIdentity) { + return ErrInsufficientPermissions + } permissions := c.aclState.Permissions(identity) if permissions.NoPermissions() { return ErrNoSuchAccount @@ -173,8 +177,13 @@ func (c *contentValidator) ValidateAccountRemove(ch *aclrecordproto.AclAccountRe if permissions.IsOwner() { return ErrInsufficientPermissions } + idKey := mapKeyFromPubKey(identity) + if _, exists := seenIdentities[idKey]; exists { + return ErrDuplicateAccounts + } + seenIdentities[mapKeyFromPubKey(identity)] = struct{}{} } - return c.validateAccountReadKeys(ch.AccountKeys) + return c.validateAccountReadKeys(ch.AccountKeys, len(c.aclState.userStates)-len(ch.Identities)) } func (c *contentValidator) ValidateRequestRemove(ch *aclrecordproto.AclAccountRequestRemove, authorIdentity crypto.PubKey) (err error) { @@ -188,11 +197,11 @@ func (c *contentValidator) ValidateRequestRemove(ch *aclrecordproto.AclAccountRe } func (c *contentValidator) ValidateReadKeyChange(ch *aclrecordproto.AclReadKeyChange, authorIdentity crypto.PubKey) (err error) { - return c.validateAccountReadKeys(ch.AccountKeys) + return c.validateAccountReadKeys(ch.AccountKeys, len(c.aclState.userStates)) } -func (c *contentValidator) validateAccountReadKeys(accountKeys []*aclrecordproto.AclEncryptedReadKey) (err error) { - if len(accountKeys) != len(c.aclState.userStates) { +func (c *contentValidator) validateAccountReadKeys(accountKeys []*aclrecordproto.AclEncryptedReadKey, usersNum int) (err error) { + if len(accountKeys) != usersNum { return ErrIncorrectNumberOfAccounts } for _, encKeys := range accountKeys { diff --git a/commonspace/payloads.go b/commonspace/payloads.go index 6cc572be..b3c9835b 100644 --- a/commonspace/payloads.go +++ b/commonspace/payloads.go @@ -72,7 +72,7 @@ func storagePayloadForSpaceCreate(payload SpaceCreatePayload) (storagePayload sp // building acl root keyStorage := crypto.NewKeyStorage() - aclBuilder := list.NewAclRecordBuilder("", keyStorage, nil) + aclBuilder := list.NewAclRecordBuilder("", keyStorage, nil, list.NoOpAcceptorVerifier{}) aclRoot, err := aclBuilder.BuildRoot(list.RootContent{ PrivKey: payload.SigningKey, MasterKey: payload.MasterKey, @@ -159,7 +159,7 @@ func storagePayloadForSpaceDerive(payload SpaceDerivePayload) (storagePayload sp // building acl root keyStorage := crypto.NewKeyStorage() - aclBuilder := list.NewAclRecordBuilder("", keyStorage, nil) + aclBuilder := list.NewAclRecordBuilder("", keyStorage, nil, list.NoOpAcceptorVerifier{}) aclRoot, err := aclBuilder.BuildRoot(list.RootContent{ PrivKey: payload.SigningKey, MasterKey: payload.MasterKey,