fix: do not list superusers while listing platform service users (#861)

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

core/project/service.go

@@ -33,6 +33,7 @@ type UserService interface {
 
 type ServiceuserService interface {
 	GetByIDs(ctx context.Context, ids []string) ([]serviceuser.ServiceUser, error)
+	FilterSudos(ctx context.Context, ids []string) ([]string, error)
 }
 
 type PolicyService interface {
@@ -258,6 +259,17 @@ func (s Service) ListServiceUsers(ctx context.Context, id string, permissionFilt
 		// no users
 		return []serviceuser.ServiceUser{}, nil
 	}
+
+	// filter service users which got access because of SU permission
+	// even if they are from same org, I think it's ideal to not list them
+	userIDs, err = s.suserService.FilterSudos(ctx, userIDs)
+	if err != nil {
+		return nil, err
+	}
+	if len(userIDs) == 0 {
+		// no users
+		return []serviceuser.ServiceUser{}, nil
+	}
 	return s.suserService.GetByIDs(ctx, userIDs)
 }
 

core/project/service_test.go

@@ -644,6 +644,7 @@ func TestService_ListServiceUsers(t *testing.T) {
 					RelationName: "",
 				}).Return([]string{"user-id"}, nil)
 
+				suserService.EXPECT().FilterSudos(ctx, []string{"user-id"}).Return([]string{"user-id"}, nil)
 				suserService.EXPECT().GetByIDs(ctx, []string{"user-id"}).Return([]serviceuser.ServiceUser{
 					{
 						ID: "user-id",

core/serviceuser/service.go

@@ -41,6 +41,7 @@ type RelationService interface {
 	Delete(ctx context.Context, rel relation.Relation) error
 	LookupSubjects(ctx context.Context, rel relation.Relation) ([]string, error)
 	CheckPermission(ctx context.Context, rel relation.Relation) (bool, error)
+	BatchCheckPermission(ctx context.Context, rel []relation.Relation) ([]relation.CheckPair, error)
 }
 
 type Service struct {
@@ -392,6 +393,36 @@ func (s Service) IsSudo(ctx context.Context, id string, permissionName string) (
 	})
 }
 
+// FilterSudos filters serviceusers which have superuser permissions and returns the remaining
+func (s Service) FilterSudos(ctx context.Context, ids []string) ([]string, error) {
+	relations := make([]relation.Relation, 0, len(ids))
+	for _, id := range ids {
+		rel := relation.Relation{
+			Subject: relation.Subject{
+				ID:        id,
+				Namespace: schema.ServiceUserPrincipal,
+			},
+			Object: relation.Object{
+				ID:        schema.PlatformID,
+				Namespace: schema.PlatformNamespace,
+			},
+			RelationName: schema.PlatformSudoPermission,
+		}
+		relations = append(relations, rel)
+	}
+	checkPairs, err := s.relationService.BatchCheckPermission(ctx, relations)
+	if err != nil {
+		return nil, err
+	}
+	sudoIDs := make([]string, 0, len(checkPairs))
+	for i, checkPair := range checkPairs {
+		if !checkPair.Status {
+			sudoIDs = append(sudoIDs, ids[i])
+		}
+	}
+	return sudoIDs, nil
+}
+
 // Sudo add platform permissions to user
 func (s Service) Sudo(ctx context.Context, id string, relationName string) error {
 	currentUser, err := s.Get(ctx, id)

test/e2e/regression/serviceusers_test.go

@@ -873,6 +873,76 @@ func (s *ServiceUsersRegressionTestSuite) TestServiceUserAsPlatformMember() {
 		s.Assert().NoError(err)
 		s.Assert().False(checkResp.GetStatus())
 	})
+	s.Run("5. list super users & service users of platform in non admin APIs", func() {
+		// create a service user
+		createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, &frontierv1beta1.CreateOrganizationRequest{
+			Body: &frontierv1beta1.OrganizationRequestBody{
+				Name: "org-sv-user-pl-5",
+			},
+		})
+		s.Assert().NoError(err)
+
+		createServiceUserResp, err := s.testBench.Client.CreateServiceUser(ctxOrgAdminAuth, &frontierv1beta1.CreateServiceUserRequest{
+			OrgId: createOrgResp.GetOrganization().GetId(),
+		})
+		s.Assert().NoError(err)
+		s.Assert().NotNil(createServiceUserResp)
+
+		// make service user platform su
+		_, err = s.testBench.AdminClient.AddPlatformUser(ctxOrgAdminAuth, &frontierv1beta1.AddPlatformUserRequest{
+			ServiceuserId: createServiceUserResp.GetServiceuser().GetId(),
+			Relation:      schema.AdminRelationName,
+		})
+		s.Assert().NoError(err)
+
+		// create another org to verify su permissions
+		createOrg2Resp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, &frontierv1beta1.CreateOrganizationRequest{
+			Body: &frontierv1beta1.OrganizationRequestBody{
+				Name: "org-sv-user-pl-5a",
+			},
+		})
+		s.Assert().NoError(err)
+
+		// check if su can delete another org
+		checkResp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth,
+			&frontierv1beta1.CheckFederatedResourcePermissionRequest{
+				Subject:    schema.JoinNamespaceAndResourceID(schema.ServiceUserPrincipal, createServiceUserResp.GetServiceuser().GetId()),
+				Resource:   schema.JoinNamespaceAndResourceID(schema.OrganizationNamespace, createOrg2Resp.GetOrganization().GetId()),
+				Permission: schema.DeletePermission,
+			})
+		s.Assert().NoError(err)
+		s.Assert().True(checkResp.GetStatus())
+
+		// check if we have su permissions by listing platform users
+		listUsersResp, err := s.testBench.AdminClient.ListPlatformUsers(ctxOrgAdminAuth, &frontierv1beta1.ListPlatformUsersRequest{})
+		s.Assert().NoError(err)
+		s.Assert().NotNil(listUsersResp)
+		s.Assert().True(utils.ContainsFunc(listUsersResp.GetServiceusers(), func(user *frontierv1beta1.ServiceUser) bool {
+			return user.GetId() == createServiceUserResp.GetServiceuser().GetId()
+		}))
+
+		// superusers shouldn't be listed in non admin calls even if they have access
+		orgServiceUsersResp, err := s.testBench.Client.ListServiceUsers(ctxOrgAdminAuth, &frontierv1beta1.ListServiceUsersRequest{
+			OrgId: createOrg2Resp.GetOrganization().GetId(),
+		})
+		s.Assert().NoError(err)
+		s.Assert().NotNil(orgServiceUsersResp)
+		s.Assert().Len(orgServiceUsersResp.GetServiceusers(), 0)
+		// create a project in it to verify as well
+		createProjectResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, &frontierv1beta1.CreateProjectRequest{
+			Body: &frontierv1beta1.ProjectRequestBody{
+				Name:  "project-sv-user-pl-3a",
+				OrgId: createOrg2Resp.GetOrganization().GetId(),
+			},
+		})
+		s.Assert().NoError(err)
+		projectServiceUsersResp, err := s.testBench.Client.ListProjectServiceUsers(ctxOrgAdminAuth, &frontierv1beta1.ListProjectServiceUsersRequest{
+			Id: createProjectResp.GetProject().GetId(),
+		})
+		s.Assert().NoError(err)
+		s.Assert().NotNil(projectServiceUsersResp)
+		s.Assert().Len(projectServiceUsersResp.GetServiceusers(), 0)
+	})
 }
 
 func (s *ServiceUsersRegressionTestSuite) TestServiceUserWithToken() {