Add account remove test

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 {

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")

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,
+		storage:          storage,
-		keyStorage:    keyStorage,
+		keyStorage:       keyStorage,
-		stateBuilder:  newAclStateBuilderWithIdentity(acc),
+		stateBuilder:     newAclStateBuilderWithIdentity(acc),
-		recordBuilder: NewAclRecordBuilder(storage.Id(), keyStorage, 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,
+		storage:          storage,
-		keyStorage:    keyStorage,
+		keyStorage:       keyStorage,
-		stateBuilder:  newAclStateBuilder(),
+		stateBuilder:     newAclStateBuilder(),
-		recordBuilder: NewAclRecordBuilder(storage.Id(), keyStorage, nil),
+		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
 }

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) {
+func (fx *aclFixture) addRec(t *testing.T, rec *aclrecordproto.RawAclRecordWithId) {
-	randomKeys, err := accountdata.NewRandom()
+	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) {
+func (fx *aclFixture) inviteAccount(t *testing.T, perms AclPermissions) {
-	fx := newFixture(t)
 	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)
+	fx.addRec(t, inviteRec)
-	require.NoError(t, err)
-	err = accountAcl.AddRawRecord(inviteRec)
-	require.NoError(t, err)
 
 	// 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)
+	fx.addRec(t, requestJoinRec)
-	require.NoError(t, err)
-	err = accountAcl.AddRawRecord(requestJoinRec)
-	require.NoError(t, err)
 
 	// 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)
+	fx.addRec(t, requestAcceptRec)
-	require.NoError(t, err)
-	err = accountAcl.AddRawRecord(requestAcceptRec)
-	require.NoError(t, err)
 
 	// 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()])
+}

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

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) {
+func (c *contentValidator) validateAccountReadKeys(accountKeys []*aclrecordproto.AclEncryptedReadKey, usersNum int) (err error) {
-	if len(accountKeys) != len(c.aclState.userStates) {
+	if len(accountKeys) != usersNum {
 		return ErrIncorrectNumberOfAccounts
 	}
 	for _, encKeys := range accountKeys {

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,