diff --git a/api/services/namespace.go b/api/services/namespace.go index 2ea1b294dff..9a8c3c3a3f8 100644 --- a/api/services/namespace.go +++ b/api/services/namespace.go @@ -256,27 +256,42 @@ func (s *service) AddNamespaceMember(ctx context.Context, req *requests.Namespac AddedAt: addedAt, ExpiresAt: expiresAt, Role: req.MemberRole, - Status: models.MemberStatusAccepted, } - // In cloud instances, the member must accept the invite before enter in the namespace. - if envs.IsCloud() { - member.Status = models.MemberStatusPending - if err := s.client.InviteMember(ctx, req.TenantID, member.ID, req.FowardedHost); err != nil { - return nil, err - } + if err := s.store.WithTransaction(ctx, s.addMember(req.TenantID, req.FowardedHost, member)); err != nil { + return nil, err } - if err := s.store.NamespaceAddMember(ctx, req.TenantID, member); err != nil { - switch { - case errors.Is(err, mongo.ErrNamespaceDuplicatedMember): - return nil, NewErrNamespaceMemberDuplicated(passiveUser.ID, err) - default: - return nil, err + return s.store.NamespaceGet(ctx, req.TenantID, true) +} + +// addMember returns a transaction callback that adds a member and sends an invite if the instance is cloud. +func (s *service) addMember(tenantID, fowardedHost string, member *models.Member) store.TransactionCb { + return func(ctx context.Context) error { + // In cloud instances, the member must accept the invite before enter in the namespace. + if envs.IsCloud() { + member.Status = models.MemberStatusPending + } else { + member.Status = models.MemberStatusAccepted } - } - return s.store.NamespaceGet(ctx, req.TenantID, true) + if err := s.store.NamespaceAddMember(ctx, tenantID, member); err != nil { + switch { + case errors.Is(err, mongo.ErrNamespaceDuplicatedMember): + return NewErrNamespaceMemberDuplicated(member.ID, err) + default: + return err + } + } + + if envs.IsCloud() { + if err := s.client.InviteMember(ctx, tenantID, member.ID, fowardedHost); err != nil { + return err + } + } + + return nil + } } func (s *service) UpdateNamespaceMember(ctx context.Context, req *requests.NamespaceUpdateMember) error { diff --git a/api/services/namespace_test.go b/api/services/namespace_test.go index 6e71fb860ea..000ff533cd6 100644 --- a/api/services/namespace_test.go +++ b/api/services/namespace_test.go @@ -9,16 +9,20 @@ import ( "github.com/shellhub-io/shellhub/api/store" "github.com/shellhub-io/shellhub/api/store/mocks" + "github.com/shellhub-io/shellhub/api/store/mongo" "github.com/shellhub-io/shellhub/pkg/api/authorizer" "github.com/shellhub-io/shellhub/pkg/api/query" "github.com/shellhub-io/shellhub/pkg/api/requests" storecache "github.com/shellhub-io/shellhub/pkg/cache" "github.com/shellhub-io/shellhub/pkg/clock" clockmock "github.com/shellhub-io/shellhub/pkg/clock/mocks" + "github.com/shellhub-io/shellhub/pkg/envs" + envmock "github.com/shellhub-io/shellhub/pkg/envs/mocks" "github.com/shellhub-io/shellhub/pkg/models" "github.com/shellhub-io/shellhub/pkg/uuid" uuid_mocks "github.com/shellhub-io/shellhub/pkg/uuid/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestListNamespaces(t *testing.T) { @@ -1321,7 +1325,7 @@ func TestAddNamespaceMember(t *testing.T) { Return("false"). Once() storeMock. - On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000001", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + On("WithTransaction", ctx, mock.Anything). Return(errors.New("error")). Once() }, @@ -1374,7 +1378,7 @@ func TestAddNamespaceMember(t *testing.T) { Return("false"). Once() storeMock. - On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000001", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + On("WithTransaction", ctx, mock.Anything). Return(nil). Once() storeMock. @@ -1419,162 +1423,192 @@ func TestAddNamespaceMember(t *testing.T) { err: nil, }, }, + } + + s := NewService(store.Store(storeMock), privateKey, publicKey, storecache.NewNullCache(), clientMock) + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + ctx := context.TODO() + tc.requiredMocks(ctx) + ns, err := s.AddNamespaceMember(ctx, tc.req) + assert.Equal(t, tc.expected, Expected{ns, err}) + }) + } + + storeMock.AssertExpectations(t) +} + +func TestService_addMember(t *testing.T) { + envMock = new(envmock.Backend) + storeMock := new(mocks.Store) + + now := time.Now() + envs.DefaultBackend = envMock + + cases := []struct { + description string + tenantID string + fowardedHost string + member *models.Member + requiredMocks func(context.Context) + expected error + }{ { - description: "fails when instance is cloud and cannot send the invite", - req: &requests.NamespaceAddMember{ - FowardedHost: "localhost", - UserID: "000000000000000000000000", - TenantID: "00000000-0000-4000-0000-000000000000", - MemberEmail: "john.doe@test.com", - MemberRole: authorizer.RoleObserver, - }, + description: "fails cannot add the member", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, requiredMocks: func(ctx context.Context) { + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("false"). + Once() storeMock. - On("NamespaceGet", ctx, "00000000-0000-4000-0000-000000000000", true). - Return(&models.Namespace{ - TenantID: "00000000-0000-4000-0000-000000000000", - Name: "namespace", - Owner: "000000000000000000000000", - Members: []models.Member{ - { - ID: "000000000000000000000000", - Role: authorizer.RoleOwner, - Status: models.MemberStatusAccepted, - }, - }, - }, nil). + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + Return(errors.New("error")). + Once() + }, + expected: errors.New("error"), + }, + { + description: "fails when member is duplicated", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, + requiredMocks: func(ctx context.Context) { + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("false"). Once() storeMock. - On("UserGetByID", ctx, "000000000000000000000000", false). - Return(&models.User{ - ID: "000000000000000000000000", - UserData: models.UserData{Username: "jane_doe"}, - }, 0, nil). + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + Return(mongo.ErrNamespaceDuplicatedMember). + Once() + }, + expected: NewErrNamespaceMemberDuplicated("000000000000000000000000", mongo.ErrNamespaceDuplicatedMember), + }, + { + description: "succeeds", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, + requiredMocks: func(ctx context.Context) { + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("false"). Once() storeMock. - On("UserGetByEmail", ctx, "john.doe@test.com"). - Return(&models.User{ID: "000000000000000000000001"}, nil). + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + Return(nil). + Once() + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("false"). Once() + }, + expected: nil, + }, + { + description: "[cloud] fails cannot add the member", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, + requiredMocks: func(ctx context.Context) { envMock. On("Get", "SHELLHUB_CLOUD"). Return("true"). Once() - clientMock. - On("InviteMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000001", "localhost"). + storeMock. + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). Return(errors.New("error")). Once() }, - expected: Expected{ - namespace: nil, - err: errors.New("error"), - }, + expected: errors.New("error"), }, { - description: "succeeds when the instance is cloud", - req: &requests.NamespaceAddMember{ - UserID: "000000000000000000000000", - TenantID: "00000000-0000-4000-0000-000000000000", - MemberEmail: "john.doe@test.com", - MemberRole: authorizer.RoleObserver, - FowardedHost: "localhost", - }, + description: "[cloud] fails when member is duplicated", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, requiredMocks: func(ctx context.Context) { - storeMock. - On("NamespaceGet", ctx, "00000000-0000-4000-0000-000000000000", true). - Return(&models.Namespace{ - TenantID: "00000000-0000-4000-0000-000000000000", - Name: "namespace", - Owner: "000000000000000000000000", - Members: []models.Member{ - { - ID: "000000000000000000000000", - Role: authorizer.RoleOwner, - Status: models.MemberStatusAccepted, - }, - }, - }, nil). + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("true"). Once() storeMock. - On("UserGetByID", ctx, "000000000000000000000000", false). - Return(&models.User{ - ID: "000000000000000000000000", - UserData: models.UserData{Username: "jane_doe"}, - }, 0, nil). + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + Return(mongo.ErrNamespaceDuplicatedMember). + Once() + }, + expected: NewErrNamespaceMemberDuplicated("000000000000000000000000", mongo.ErrNamespaceDuplicatedMember), + }, + { + description: "[cloud] fails cannot send the invite", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, + requiredMocks: func(ctx context.Context) { + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("true"). Once() storeMock. - On("UserGetByEmail", ctx, "john.doe@test.com"). - Return(&models.User{ - ID: "000000000000000000000001", - }, nil). + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + Return(nil). Once() envMock. On("Get", "SHELLHUB_CLOUD"). Return("true"). Once() clientMock. - On("InviteMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000001", "localhost"). - Return(nil). + On("InviteMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000", "localhost"). + Return(errors.New("error")). + Once() + }, + expected: errors.New("error"), + }, + { + description: "[cloud] succeeds", + tenantID: "00000000-0000-4000-0000-000000000000", + fowardedHost: "localhost", + member: &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}, + requiredMocks: func(ctx context.Context) { + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("true"). Once() storeMock. - On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000001", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). + On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}). Return(nil). Once() - storeMock. - On("NamespaceGet", ctx, "00000000-0000-4000-0000-000000000000", true). - Return(&models.Namespace{ - TenantID: "00000000-0000-4000-0000-000000000000", - Name: "namespace", - Owner: "000000000000000000000000", - Members: []models.Member{ - { - ID: "000000000000000000000000", - Role: authorizer.RoleOwner, - Status: models.MemberStatusAccepted, - }, - { - ID: "000000000000000000000001", - Role: authorizer.RoleObserver, - Status: models.MemberStatusPending, - }, - }, - }, nil). + envMock. + On("Get", "SHELLHUB_CLOUD"). + Return("true"). + Once() + clientMock. + On("InviteMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000", "localhost"). + Return(nil). Once() }, - expected: Expected{ - namespace: &models.Namespace{ - TenantID: "00000000-0000-4000-0000-000000000000", - Name: "namespace", - Owner: "000000000000000000000000", - Members: []models.Member{ - { - ID: "000000000000000000000000", - Role: authorizer.RoleOwner, - Status: models.MemberStatusAccepted, - }, - { - ID: "000000000000000000000001", - Role: authorizer.RoleObserver, - Status: models.MemberStatusPending, - }, - }, - }, - err: nil, - }, + expected: nil, }, } - s := NewService(store.Store(storeMock), privateKey, publicKey, storecache.NewNullCache(), clientMock) + s := NewService(storeMock, privateKey, publicKey, storecache.NewNullCache(), clientMock) for _, tc := range cases { - t.Run(tc.description, func(t *testing.T) { - ctx := context.TODO() + t.Run(tc.description, func(tt *testing.T) { + ctx := context.Background() tc.requiredMocks(ctx) - ns, err := s.AddNamespaceMember(ctx, tc.req) - assert.Equal(t, tc.expected, Expected{ns, err}) + + cb := s.addMember(tc.tenantID, tc.fowardedHost, tc.member) + assert.Equal(tt, tc.expected, cb(ctx)) + + storeMock.AssertExpectations(tt) + envMock.AssertExpectations(tt) }) } - - storeMock.AssertExpectations(t) } func TestUpdateNamespaceMember(t *testing.T) {