diff --git a/core/acl/aclservice.go b/core/acl/aclservice.go index 30df9d2705..275c6c6a86 100644 --- a/core/acl/aclservice.go +++ b/core/acl/aclservice.go @@ -24,6 +24,7 @@ import ( "github.com/anyproto/anytype-heart/pkg/lib/logging" "github.com/anyproto/anytype-heart/pkg/lib/pb/model" "github.com/anyproto/anytype-heart/space" + space2 "github.com/anyproto/anytype-heart/space/spacecore" "github.com/anyproto/anytype-heart/space/spaceinfo" "github.com/anyproto/anytype-heart/space/techspace" ) @@ -78,6 +79,7 @@ type aclService struct { nodeConfigGetter NodeConfGetter joiningClient aclclient.AclJoiningClient spaceService space.Service + spaceCoreService space2.SpaceCoreService inviteService inviteservice.InviteService accountService account.Service coordClient coordinatorclient.CoordinatorClient @@ -88,6 +90,7 @@ func (a *aclService) Init(ap *app.App) (err error) { a.nodeConfigGetter = app.MustComponent[NodeConfGetter](ap) a.joiningClient = app.MustComponent[aclclient.AclJoiningClient](ap) a.spaceService = app.MustComponent[space.Service](ap) + a.spaceCoreService = app.MustComponent[space2.SpaceCoreService](ap) a.accountService = app.MustComponent[account.Service](ap) a.inviteService = app.MustComponent[inviteservice.InviteService](ap) a.coordClient = app.MustComponent[coordinatorclient.CoordinatorClient](ap) @@ -295,7 +298,14 @@ func (a *aclService) ApproveLeave(ctx context.Context, spaceId string, identitie } func (a *aclService) Leave(ctx context.Context, spaceId string) error { - removeSpace, err := a.spaceService.Get(ctx, spaceId) + status, err := a.spaceService.SpacePersistentStatus(ctx, spaceId) + if err != nil { + return err + } + if status != spaceinfo.AccountStatusActive && status != spaceinfo.AccountStatusUnknown { + return fmt.Errorf("space status not active or unknown") + } + removeSpace, err := a.spaceCoreService.Get(ctx, spaceId) if err != nil { // space storage missing can occur only in case of missing space if errors.Is(err, space.ErrSpaceStorageMissig) || errors.Is(err, space.ErrSpaceDeleted) { @@ -303,14 +313,8 @@ func (a *aclService) Leave(ctx context.Context, spaceId string) error { } return convertedOrSpaceErr(err) } - identity := a.accountService.Keys().SignKey.GetPublic() - if !removeSpace.GetAclIdentity().Equals(identity) { - // this is a streamable space - // we exist there under ephemeral guest identity and should not remove it - return nil - } - cl := removeSpace.CommonSpace().AclClient() + cl := removeSpace.AclClient() err = cl.RequestSelfRemove(ctx) if err != nil { errs := []error{ diff --git a/core/acl/aclservice_test.go b/core/acl/aclservice_test.go index 627997f98c..bcf1493194 100644 --- a/core/acl/aclservice_test.go +++ b/core/acl/aclservice_test.go @@ -39,6 +39,8 @@ import ( "github.com/anyproto/anytype-heart/space/clientspace" "github.com/anyproto/anytype-heart/space/clientspace/mock_clientspace" "github.com/anyproto/anytype-heart/space/mock_space" + "github.com/anyproto/anytype-heart/space/spacecore" + "github.com/anyproto/anytype-heart/space/spacecore/mock_spacecore" "github.com/anyproto/anytype-heart/space/spaceinfo" "github.com/anyproto/anytype-heart/space/techspace" "github.com/anyproto/anytype-heart/space/techspace/mock_techspace" @@ -69,6 +71,7 @@ type fixture struct { ctrl *gomock.Controller mockJoiningClient *mock_aclclient.MockAclJoiningClient mockSpaceService *mock_space.MockService + mockSpaceCoreService *mock_spacecore.MockSpaceCoreService mockAccountService *mock_account.MockService mockInviteService *mock_inviteservice.MockInviteService mockCoordinatorClient *mock_coordinatorclient.MockCoordinatorClient @@ -89,6 +92,7 @@ func newFixture(t *testing.T) *fixture { ctrl: ctrl, mockJoiningClient: mock_aclclient.NewMockAclJoiningClient(ctrl), mockSpaceService: mock_space.NewMockService(t), + mockSpaceCoreService: mock_spacecore.NewMockSpaceCoreService(t), mockAccountService: mock_account.NewMockService(t), mockInviteService: mock_inviteservice.NewMockInviteService(t), mockCoordinatorClient: mock_coordinatorclient.NewMockCoordinatorClient(ctrl), @@ -103,6 +107,7 @@ func newFixture(t *testing.T) *fixture { fx.a.Register(testutil.PrepareMock(ctx, fx.a, fx.mockAccountService)). Register(testutil.PrepareMock(ctx, fx.a, fx.mockJoiningClient)). Register(testutil.PrepareMock(ctx, fx.a, fx.mockSpaceService)). + Register(testutil.PrepareMock(ctx, fx.a, fx.mockSpaceCoreService)). Register(testutil.PrepareMock(ctx, fx.a, fx.mockInviteService)). Register(testutil.PrepareMock(ctx, fx.a, fx.mockCoordinatorClient)). Register(fx.mockConfig). @@ -272,9 +277,8 @@ func TestService_ApproveLeave(t *testing.T) { identityC := exec.ActualAccounts()["c"].Keys.SignKey.GetPublic() acl := mockSyncAcl{exec.ActualAccounts()["a"].Acl} mockCommonSpace.EXPECT().Acl().Return(acl) - aclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl) - mockCommonSpace.EXPECT().AclClient().Return(aclClient) - aclClient.EXPECT().RemoveAccounts(ctx, gomock.Any()).DoAndReturn(func(ctx context.Context, payload list.AccountRemovePayload) error { + mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient) + fx.mockSpaceClient.EXPECT().RemoveAccounts(ctx, gomock.Any()).DoAndReturn(func(ctx context.Context, payload list.AccountRemovePayload) error { require.Equal(t, []crypto.PubKey{identityB, identityC}, payload.Identities) return nil }).Return(nil) @@ -487,19 +491,13 @@ func TestService_Leave(t *testing.T) { fx := newFixture(t) defer fx.finish(t) spaceId := "spaceId" - mockSpace := mock_clientspace.NewMockSpace(t) - mockCommonSpace := mock_commonspace.NewMockSpace(fx.ctrl) - mockAclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl) - fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil) - keys, err := accountdata.NewRandom() - require.NoError(t, err) - fx.mockAccountService.EXPECT().Keys().Return(keys) - mockSpace.EXPECT().GetAclIdentity().Return(keys.SignKey.GetPublic()) - - mockSpace.EXPECT().CommonSpace().Return(mockCommonSpace) - mockCommonSpace.EXPECT().AclClient().Return(mockAclClient) - mockAclClient.EXPECT().RequestSelfRemove(ctx).Return(nil) - err = fx.Leave(ctx, spaceId) + anySpace := &spacecore.AnySpace{} + anySpace.Space = fx.mockCommonSpace + fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusUnknown, nil) + fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(anySpace, nil) + fx.mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient) + fx.mockSpaceClient.EXPECT().RequestSelfRemove(ctx).Return(nil) + err := fx.Leave(ctx, spaceId) require.NoError(t, err) }) t.Run("leave success if space service error is known", func(t *testing.T) { @@ -512,7 +510,8 @@ func TestService_Leave(t *testing.T) { fx := newFixture(t) defer fx.finish(t) spaceId := "spaceId" - fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(nil, err) + fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusActive, nil) + fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(nil, err) err = fx.Leave(ctx, spaceId) require.NoError(t, err) }) @@ -522,7 +521,8 @@ func TestService_Leave(t *testing.T) { fx := newFixture(t) defer fx.finish(t) spaceId := "spaceId" - fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(nil, fmt.Errorf("error")) + fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusActive, nil) + fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(nil, fmt.Errorf("error")) err := fx.Leave(ctx, spaceId) require.True(t, errors.Is(err, ErrInternal)) }) @@ -539,17 +539,12 @@ func TestService_Leave(t *testing.T) { fx := newFixture(t) defer fx.finish(t) spaceId := "spaceId" - mockSpace := mock_clientspace.NewMockSpace(t) - mockCommonSpace := mock_commonspace.NewMockSpace(fx.ctrl) - mockAclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl) - fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil) - keys, err := accountdata.NewRandom() - require.NoError(t, err) - fx.mockAccountService.EXPECT().Keys().Return(keys) - mockSpace.EXPECT().GetAclIdentity().Return(keys.SignKey.GetPublic()) - mockSpace.EXPECT().CommonSpace().Return(mockCommonSpace) - mockCommonSpace.EXPECT().AclClient().Return(mockAclClient) - mockAclClient.EXPECT().RequestSelfRemove(ctx).Return(err) + anySpace := &spacecore.AnySpace{} + anySpace.Space = fx.mockCommonSpace + fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusUnknown, nil) + fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(anySpace, nil) + fx.mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient) + fx.mockSpaceClient.EXPECT().RequestSelfRemove(ctx).Return(err) err = fx.Leave(ctx, spaceId) require.NoError(t, err) }) @@ -559,37 +554,13 @@ func TestService_Leave(t *testing.T) { fx := newFixture(t) defer fx.finish(t) spaceId := "spaceId" - mockSpace := mock_clientspace.NewMockSpace(t) - mockCommonSpace := mock_commonspace.NewMockSpace(fx.ctrl) - mockAclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl) - fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil) - mockSpace.EXPECT().CommonSpace().Return(mockCommonSpace) - mockCommonSpace.EXPECT().AclClient().Return(mockAclClient) - keys, err := accountdata.NewRandom() - require.NoError(t, err) - fx.mockAccountService.EXPECT().Keys().Return(keys) - mockSpace.EXPECT().GetAclIdentity().Return(keys.SignKey.GetPublic()) - mockAclClient.EXPECT().RequestSelfRemove(ctx).Return(fmt.Errorf("error")) - err = fx.Leave(ctx, spaceId) + anySpace := &spacecore.AnySpace{} + anySpace.Space = fx.mockCommonSpace + fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusUnknown, nil) + fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(anySpace, nil) + fx.mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient) + fx.mockSpaceClient.EXPECT().RequestSelfRemove(ctx).Return(fmt.Errorf("error")) + err := fx.Leave(ctx, spaceId) require.True(t, errors.Is(err, ErrAclRequestFailed)) }) - - t.Run("leave if acl key is not account key", func(t *testing.T) { - // this is a case of guest user trying to leave the space - fx := newFixture(t) - defer fx.finish(t) - spaceId := "spaceId" - mockSpace := mock_clientspace.NewMockSpace(t) - - fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil) - accountKeys, err := accountdata.NewRandom() - require.NoError(t, err) - aclKeys, err := accountdata.NewRandom() - require.NoError(t, err) - fx.mockAccountService.EXPECT().Keys().Return(accountKeys) - mockSpace.EXPECT().GetAclIdentity().Return(aclKeys.SignKey.GetPublic()) - - err = fx.Leave(ctx, spaceId) - require.NoError(t, err) - }) } diff --git a/space/mock_space/mock_Service.go b/space/mock_space/mock_Service.go index c98769e695..ad0c6a957b 100644 --- a/space/mock_space/mock_Service.go +++ b/space/mock_space/mock_Service.go @@ -11,6 +11,8 @@ import ( crypto "github.com/anyproto/any-sync/util/crypto" mock "github.com/stretchr/testify/mock" + + spaceinfo "github.com/anyproto/anytype-heart/space/spaceinfo" ) // MockService is an autogenerated mock type for the Service type @@ -816,6 +818,63 @@ func (_c *MockService_Run_Call) RunAndReturn(run func(context.Context) error) *M return _c } +// SpacePersistentStatus provides a mock function with given fields: ctx, spaceId +func (_m *MockService) SpacePersistentStatus(ctx context.Context, spaceId string) (spaceinfo.AccountStatus, error) { + ret := _m.Called(ctx, spaceId) + + if len(ret) == 0 { + panic("no return value specified for SpacePersistentStatus") + } + + var r0 spaceinfo.AccountStatus + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (spaceinfo.AccountStatus, error)); ok { + return rf(ctx, spaceId) + } + if rf, ok := ret.Get(0).(func(context.Context, string) spaceinfo.AccountStatus); ok { + r0 = rf(ctx, spaceId) + } else { + r0 = ret.Get(0).(spaceinfo.AccountStatus) + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, spaceId) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockService_SpacePersistentStatus_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SpacePersistentStatus' +type MockService_SpacePersistentStatus_Call struct { + *mock.Call +} + +// SpacePersistentStatus is a helper method to define mock.On call +// - ctx context.Context +// - spaceId string +func (_e *MockService_Expecter) SpacePersistentStatus(ctx interface{}, spaceId interface{}) *MockService_SpacePersistentStatus_Call { + return &MockService_SpacePersistentStatus_Call{Call: _e.mock.On("SpacePersistentStatus", ctx, spaceId)} +} + +func (_c *MockService_SpacePersistentStatus_Call) Run(run func(ctx context.Context, spaceId string)) *MockService_SpacePersistentStatus_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *MockService_SpacePersistentStatus_Call) Return(_a0 spaceinfo.AccountStatus, _a1 error) *MockService_SpacePersistentStatus_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockService_SpacePersistentStatus_Call) RunAndReturn(run func(context.Context, string) (spaceinfo.AccountStatus, error)) *MockService_SpacePersistentStatus_Call { + _c.Call.Return(run) + return _c +} + // SpaceViewId provides a mock function with given fields: spaceId func (_m *MockService) SpaceViewId(spaceId string) (string, error) { ret := _m.Called(spaceId) diff --git a/space/service.go b/space/service.go index afb4983beb..3a89599d5d 100644 --- a/space/service.go +++ b/space/service.go @@ -67,6 +67,7 @@ type Service interface { AddStreamable(ctx context.Context, id string, guestKey crypto.PrivKey) (err error) Delete(ctx context.Context, id string) (err error) TechSpaceId() string + SpacePersistentStatus(ctx context.Context, spaceId string) (spaceinfo.AccountStatus, error) PersonalSpaceId() string FirstCreatedSpaceId() string TechSpace() *clientspace.TechSpace @@ -473,6 +474,14 @@ func (s *service) TechSpaceId() string { return s.techSpaceId } +func (s *service) SpacePersistentStatus(ctx context.Context, spaceId string) (spaceinfo.AccountStatus, error) { + controller, isFound := s.spaceControllers[spaceId] + if !isFound { + return spaceinfo.AccountStatusUnknown, fmt.Errorf("controller not found") + } + return controller.GetStatus(), nil +} + func (s *service) PersonalSpaceId() string { return s.personalSpaceId }