fix: org members should be allowed to get org details (#353)

Signed-off-by: Kush Sharma <[email protected]>

cmd/serve.go

@@ -297,7 +297,7 @@ func buildAPIDependencies(
 	invitationService := invitation.NewService(mailDialer, postgres.NewInvitationRepository(logger, dbc),
 		organizationService, groupService, userService, relationService, policyService, cfg.App.Invite)
 	cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService,
-		groupService, policyService, roleService, invitationService)
+		groupService, policyService, roleService, invitationService, userService)
 
 	// we should default it with a stdout logger repository as postgres can start to bloat really fast
 	var auditRepository audit.Repository

core/deleter/service.go

@@ -29,6 +29,7 @@ type OrganizationService interface {
 	Get(ctx context.Context, id string) (organization.Organization, error)
 	DeleteModel(ctx context.Context, id string) error
 	RemoveUsers(ctx context.Context, orgID string, userIDs []string) error
+	ListByUser(ctx context.Context, userID string) ([]organization.Organization, error)
 }
 
 type RoleService interface {
@@ -56,6 +57,10 @@ type InvitationService interface {
 	Delete(ctx context.Context, id uuid.UUID) error
 }
 
+type UserService interface {
+	Delete(ctx context.Context, id string) error
+}
+
 type Service struct {
 	projService       ProjectService
 	orgService        OrganizationService
@@ -64,12 +69,13 @@ type Service struct {
 	policyService     PolicyService
 	roleService       RoleService
 	invitationService InvitationService
+	userService       UserService
 }
 
 func NewCascadeDeleter(orgService OrganizationService, projService ProjectService,
 	resService ResourceService, groupService GroupService,
 	policyService PolicyService, roleService RoleService,
-	invitationService InvitationService) *Service {
+	invitationService InvitationService, userService UserService) *Service {
 	return &Service{
 		projService:       projService,
 		orgService:        orgService,
@@ -78,6 +84,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic
 		policyService:     policyService,
 		roleService:       roleService,
 		invitationService: invitationService,
+		userService:       userService,
 	}
 }
 
@@ -229,3 +236,16 @@ func (d Service) RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs [
 	// remove user from org
 	return d.orgService.RemoveUsers(ctx, orgID, userIDs)
 }
+
+func (d Service) DeleteUser(ctx context.Context, userID string) error {
+	userOrgs, err := d.orgService.ListByUser(ctx, userID)
+	if err != nil {
+		return err
+	}
+	for _, org := range userOrgs {
+		if err = d.RemoveUsersFromOrg(ctx, org.ID, []string{userID}); err != nil {
+			return fmt.Errorf("failed to delete user from org[%s]: %w", org.Name, err)
+		}
+	}
+	return d.userService.Delete(ctx, userID)
+}

core/organization/service.go

@@ -199,7 +199,7 @@ func (s Service) AddUsers(ctx context.Context, orgID string, userIDs []string) e
 
 // RemoveUsers removes users from an organization as members
 // it doesn't remove user access to projects or other resources provided
-// by policies
+// by policies, don't call directly, use cascade deleter
 func (s Service) RemoveUsers(ctx context.Context, orgID string, userIDs []string) error {
 	var err error
 	for _, userID := range userIDs {

core/serviceuser/service.go

@@ -141,7 +141,7 @@ func (s Service) Delete(ctx context.Context, id string) error {
 	// delete all of serviceuser relationships
 	// before deleting the serviceuser
 	if err := s.relService.Delete(ctx, relation.Relation{
-		Object: relation.Object{
+		Subject: relation.Subject{
 			ID:        id,
 			Namespace: schema.ServiceUserPrincipal,
 		},

core/user/service.go

@@ -108,10 +108,12 @@ func (s Service) Disable(ctx context.Context, id string) error {
 	return s.repository.SetState(ctx, id, Disabled)
 }
 
+// Delete by user uuid
+// don't call this directly, use cascade deleter
 func (s Service) Delete(ctx context.Context, id string) error {
-	if err := s.relationService.Delete(ctx, relation.Relation{Object: relation.Object{
+	if err := s.relationService.Delete(ctx, relation.Relation{Subject: relation.Subject{
 		ID:        id,
-		Namespace: schema.ProjectNamespace,
+		Namespace: schema.UserPrincipal,
 	}}); err != nil {
 		return err
 	}

internal/api/v1beta1/audit_test.go

@@ -26,8 +26,8 @@ func TestHandler_ListOrganizationAuditLogs(t *testing.T) {
 		{
 			name: "should return list of audit logs",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
-				as.EXPECT().List(mock.AnythingOfType("*context.emptyCtx"), audit.Filter{
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
+				as.EXPECT().List(mock.Anything, audit.Filter{
 					OrgID:     testOrgMap[testOrgID].ID,
 					Source:    "guardian-service",
 					Action:    "project.create",
@@ -84,8 +84,8 @@ func TestHandler_ListOrganizationAuditLogs(t *testing.T) {
 		{
 			name: "should return error when audit service returns error",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
-				as.EXPECT().List(mock.AnythingOfType("*context.emptyCtx"), audit.Filter{
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
+				as.EXPECT().List(mock.Anything, audit.Filter{
 					OrgID:     testOrgMap[testOrgID].ID,
 					Source:    "guardian-service",
 					Action:    "project.create",
@@ -106,7 +106,7 @@ func TestHandler_ListOrganizationAuditLogs(t *testing.T) {
 		{
 			name: "should return error when org is disabled",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(organization.Organization{}, organization.ErrDisabled)
+				os.EXPECT().Get(mock.Anything, "org-id").Return(organization.Organization{}, organization.ErrDisabled)
 			},
 			request: &frontierv1beta1.ListOrganizationAuditLogsRequest{
 				OrgId:     "org-id",
@@ -146,8 +146,8 @@ func TestHandler_CreateOrganizationAuditLogs(t *testing.T) {
 		{
 			name: "should create audit logs on success and return nil error",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
-				as.EXPECT().Create(mock.AnythingOfType("*context.emptyCtx"), &audit.Log{
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
+				as.EXPECT().Create(mock.Anything, &audit.Log{
 					ID:    "test-id",
 					OrgID: testOrgMap[testOrgID].ID,
 
@@ -193,7 +193,7 @@ func TestHandler_CreateOrganizationAuditLogs(t *testing.T) {
 		{
 			name: "should return error when log source and action is empty",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
 			},
 			req: &frontierv1beta1.CreateOrganizationAuditLogsRequest{
 				OrgId: "org-id",
@@ -222,8 +222,8 @@ func TestHandler_CreateOrganizationAuditLogs(t *testing.T) {
 		{
 			name: "should return error when audit service returns error",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
-				as.EXPECT().Create(mock.AnythingOfType("*context.emptyCtx"), &audit.Log{
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
+				as.EXPECT().Create(mock.Anything, &audit.Log{
 					ID:        "test-id",
 					OrgID:     testOrgMap[testOrgID].ID,
 					Source:    "guardian-service",
@@ -279,8 +279,8 @@ func TestHandler_GetOrganizationAuditLog(t *testing.T) {
 		{
 			name: "should return audit log on success",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
-				as.EXPECT().GetByID(mock.AnythingOfType("*context.emptyCtx"), "test-id").Return(audit.Log{
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
+				as.EXPECT().GetByID(mock.Anything, "test-id").Return(audit.Log{
 					ID:        "test-id",
 					OrgID:     testOrgMap[testOrgID].ID,
 					Source:    "guardian-service",
@@ -331,8 +331,8 @@ func TestHandler_GetOrganizationAuditLog(t *testing.T) {
 		{
 			name: "should return error when audit service returns error",
 			setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
-				os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
-				as.EXPECT().GetByID(mock.AnythingOfType("*context.emptyCtx"), "test-id").Return(audit.Log{}, errors.New("test-error"))
+				os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
+				as.EXPECT().GetByID(mock.Anything, "test-id").Return(audit.Log{}, errors.New("test-error"))
 			},
 			req: &frontierv1beta1.GetOrganizationAuditLogRequest{
 				Id:    "test-id",

internal/api/v1beta1/deleter.go

@@ -13,6 +13,7 @@ type CascadeDeleter interface {
 	DeleteProject(ctx context.Context, id string) error
 	DeleteOrganization(ctx context.Context, id string) error
 	RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs []string) error
+	DeleteUser(ctx context.Context, userID string) error
 }
 
 func (h Handler) DeleteProject(ctx context.Context, request *frontierv1beta1.DeleteProjectRequest) (*frontierv1beta1.DeleteProjectResponse, error) {

internal/api/v1beta1/user.go

@@ -44,7 +44,6 @@ type UserService interface {
 	Update(ctx context.Context, toUpdate user.User) (user.User, error)
 	Enable(ctx context.Context, id string) error
 	Disable(ctx context.Context, id string) error
-	Delete(ctx context.Context, id string) error
 	IsSudo(ctx context.Context, id string) (bool, error)
 }
 
@@ -664,7 +663,7 @@ func (h Handler) DisableUser(ctx context.Context, request *frontierv1beta1.Disab
 
 func (h Handler) DeleteUser(ctx context.Context, request *frontierv1beta1.DeleteUserRequest) (*frontierv1beta1.DeleteUserResponse, error) {
 	logger := grpczap.Extract(ctx)
-	if err := h.userService.Delete(ctx, request.GetId()); err != nil {
+	if err := h.deleterService.DeleteUser(ctx, request.GetId()); err != nil {
 		logger.Error(err.Error())
 		return nil, status.Errorf(codes.Internal, err.Error())
 	}

internal/bootstrap/schema/base_schema.zed

@@ -32,7 +32,7 @@ definition app/organization {
 
     permission delete = platform->superuser + granted->app_organization_administer + granted->app_organization_delete + owner
     permission update = platform->superuser + granted->app_organization_administer + granted->app_organization_update + owner
-    permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner
+    permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner + member
     permission rolemanage = platform->superuser + granted->app_organization_administer + granted->app_organization_rolemanage + owner
     permission policymanage = platform->superuser + granted->app_organization_administer + granted->app_organization_policymanage + owner
     permission projectlist = platform->superuser + granted->app_organization_administer + granted->app_organization_projectlist + owner

internal/bootstrap/testdata/compiled_schema.zed

@@ -31,7 +31,7 @@ definition app/organization {
 	permission compute_receipt_get = owner + platform->superuser + granted->app_organization_administer + granted->compute_receipt_get
 	permission compute_receipt_update = owner + platform->superuser + granted->app_organization_administer + granted->compute_receipt_update
 	permission delete = platform->superuser + granted->app_organization_administer + granted->app_organization_delete + owner
-	permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner
+	permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner + member
 	relation granted: app/rolebinding
 
 	// synthetic permissions - group

test/e2e/regression/api_test.go

@@ -694,7 +694,7 @@ func (s *APIRegressionTestSuite) TestUserAPI() {
 			PermissionFilter: schema.GetPermission,
 		})
 		s.Assert().NoError(err)
-		s.Assert().Equal(2, len(orgUsersRespAfterRelation.GetUsers()))
+		s.Assert().Equal(3, len(orgUsersRespAfterRelation.GetUsers()))
 		groupUsersResp, err := s.testBench.Client.ListGroupUsers(ctxOrgAdminAuth, &frontierv1beta1.ListGroupUsersRequest{
 			Id:    existingGroup.Id,
 			OrgId: existingOrg.GetOrganization().GetId(),
@@ -735,7 +735,7 @@ func (s *APIRegressionTestSuite) TestUserAPI() {
 			PermissionFilter: schema.GetPermission,
 		})
 		s.Assert().NoError(err)
-		s.Assert().Equal(1, len(orgUsersRespAfterDeletion.GetUsers()))
+		s.Assert().Equal(2, len(orgUsersRespAfterDeletion.GetUsers()))
 
 		// check its relations with group
 		groupUsersRespAfterDeletion, err := s.testBench.Client.ListGroupUsers(ctxOrgAdminAuth, &frontierv1beta1.ListGroupUsersRequest{