From cad5c3cd5f97cfa0d072b5f313c78c684aa51cc7 Mon Sep 17 00:00:00 2001 From: aman Date: Wed, 11 Nov 2026 13:50:16 +0530 Subject: [PATCH] fix(membership): member removal covers disabled projects and groups cascadeRemovePrincipal now lists the org's projects and groups with IncludeDisabled, so a removed member's policies there, their custom-resource policies in disabled projects, and their group member relations are removed too. --- core/membership/org.go | 4 +- core/membership/org_test.go | 62 +++++++-------- test/e2e/regression/api_test.go | 130 ++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 33 deletions(-) diff --git a/core/membership/org.go b/core/membership/org.go index 90b0ac5d9..632379758 100644 --- a/core/membership/org.go +++ b/core/membership/org.go @@ -283,7 +283,7 @@ func (s *Service) removeOrganizationMember(ctx context.Context, orgID, principal func (s *Service) cascadeRemovePrincipal(ctx context.Context, org organization.Organization, principalID, principalType, ownerRoleID string) error { orgID := org.ID - orgProjects, err := s.projectService.List(ctx, project.Filter{OrgID: orgID}) + orgProjects, err := s.projectService.List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}) if err != nil { return fmt.Errorf("list org projects: %w", err) } @@ -294,7 +294,7 @@ func (s *Service) cascadeRemovePrincipal(ctx context.Context, org organization.O orgProjectIDSet[p.ID] = struct{}{} } - orgGroups, err := s.groupService.List(ctx, group.Filter{OrganizationID: orgID}) + orgGroups, err := s.groupService.List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}) if err != nil { return fmt.Errorf("list org groups: %w", err) } diff --git a/core/membership/org_test.go b/core/membership/org_test.go index c38de21e8..cae97007b 100644 --- a/core/membership/org_test.go +++ b/core/membership/org_test.go @@ -810,7 +810,7 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return(nil, errors.New("db error")) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return(nil, errors.New("db error")) }, wantErrContain: "list org projects", }, @@ -820,8 +820,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return(nil, errors.New("db error")) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return(nil, errors.New("db error")) }, wantErrContain: "list org groups", }, @@ -831,8 +831,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) @@ -850,8 +850,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: ownerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, RoleID: ownerRoleID}).Return([]policy.Policy{{ID: "org-p1"}, {ID: "other-owner-p"}}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: projectID}}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{{ID: groupID}}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{{ID: projectID}}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{{ID: groupID}}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, {ID: "proj-p1", ResourceType: schema.ProjectNamespace, ResourceID: projectID}, @@ -876,8 +876,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, {ID: "other-org-p", ResourceType: schema.OrganizationNamespace, ResourceID: otherOrgID}, @@ -896,8 +896,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: projectID}}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{{ID: projectID}}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "proj-p1", ResourceType: schema.ProjectNamespace, ResourceID: projectID}, }, nil) @@ -912,8 +912,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{{ID: groupID}}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{{ID: groupID}}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: viewerRoleID}, }, nil) @@ -932,8 +932,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: projectID}, {ID: secondProjectID}}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{{ID: projectID}, {ID: secondProjectID}}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) @@ -950,8 +950,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: projectID}}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{{ID: projectID}}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) @@ -967,8 +967,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: ownerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, RoleID: ownerRoleID}).Return([]policy.Policy{{ID: "org-p1"}, {ID: "other-owner-p"}}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: projectID}}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{{ID: projectID}}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: ownerRoleID}, }, nil) @@ -984,8 +984,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) @@ -1002,8 +1002,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { setup: func(d testDeps) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.PATPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.PATPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) @@ -1020,8 +1020,8 @@ func TestService_RemoveOrganizationMember(t *testing.T) { d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) @@ -1104,8 +1104,8 @@ func TestService_RemoveOrganizationMemberOnDelete(t *testing.T) { name: "removes an owner from a disabled org with the owner guard", setup: func(d testDeps) { expectDisabledOrgOwner(d, []policy.Policy{{ID: "org-p1"}, {ID: "other-owner-p"}}) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: ownerRoleID}, }, nil) @@ -1118,8 +1118,8 @@ func TestService_RemoveOrganizationMemberOnDelete(t *testing.T) { name: "skips policies that are already gone", setup: func(d testDeps) { expectDisabledOrgOwner(d, []policy.Policy{{ID: "org-p1"}, {ID: "other-owner-p"}}) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: "proj-1"}}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{{ID: "proj-1"}}, nil) + d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: ownerRoleID}, {ID: "proj-p1", ResourceType: schema.ProjectNamespace, ResourceID: "proj-1"}, @@ -1201,8 +1201,8 @@ func TestService_RemoveOrganizationMember_WithoutResourceService(t *testing.T) { orgSvc.EXPECT().Get(ctx, orgID).Return(organization.Organization{ID: orgID, Title: "Test Org"}, nil) policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: viewerRoleID}}, nil) roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) - projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) + projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}).Return([]project.Project{}, nil) + grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}).Return([]group.Group{}, nil) policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID}, }, nil) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index d2463a011..1c619ef26 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -1915,6 +1915,136 @@ func (s *APIRegressionTestSuite) TestUserDeleteLeavesNoAccessAndNoLivePolicy() { }) } +func (s *APIRegressionTestSuite) TestOrgMemberRemovalLeavesNoAccessOnDisabledProjectsAndGroups() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + + orgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{Title: "member-removal-disabled-org", Name: "member-removal-disabled-org"}, + })) + s.Require().NoError(err) + orgID := orgResp.Msg.GetOrganization().GetId() + createProject := func(name string) string { + resp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{Name: name, OrgId: orgID}, + })) + s.Require().NoError(err) + return resp.Msg.GetProject().GetId() + } + disabledProjectID := createProject("member-removal-disabled-project") + groupProjectID := createProject("member-removal-group-project") + + orderResp, err := s.testBench.Client.CreateProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectResourceRequest{ + ProjectId: disabledProjectID, + Body: &frontierv1beta1.ResourceRequestBody{Name: "member-removal-order", Namespace: computeOrderNamespace}, + })) + s.Require().NoError(err) + orderID := orderResp.Msg.GetResource().GetId() + + groupResp, err := s.testBench.Client.CreateGroup(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateGroupRequest{ + OrgId: orgID, + Body: &frontierv1beta1.GroupRequestBody{Name: "member-removal-disabled-group"}, + })) + s.Require().NoError(err) + disabledGroupID := groupResp.Msg.GetGroup().GetId() + + userResp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateUserRequest{ + Body: &frontierv1beta1.UserRequestBody{Email: "member-removal-disabled@raystack.org", Name: "member-removal-disabled"}, + })) + s.Require().NoError(err) + userID := userResp.Msg.GetUser().GetId() + userSubject := schema.JoinNamespaceAndResourceID(schema.UserPrincipal, userID) + + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: orgID, + Members: []*frontierv1beta1.OrgMemberEntry{{UserId: userID, RoleId: s.orgViewerRole}}, + })) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) + _, err = s.testBench.Client.SetGroupMemberRole(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.SetGroupMemberRoleRequest{ + GroupId: disabledGroupID, + PrincipalId: userID, + PrincipalType: schema.UserPrincipal, + RoleId: s.groupMemberRole, + })) + s.Require().NoError(err) + + disabledProject := schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, disabledProjectID) + groupProject := schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, groupProjectID) + order := schema.JoinNamespaceAndResourceID(computeOrderNamespace, orderID) + for _, grant := range []struct{ role, resource, principal string }{ + {schema.RoleProjectViewer, disabledProject, userSubject}, + {computeViewerRoleName, order, userSubject}, + {schema.RoleProjectViewer, groupProject, schema.JoinNamespaceAndResourceID(schema.GroupPrincipal, disabledGroupID)}, + } { + _, err := s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{RoleId: grant.role, Resource: grant.resource, Principal: grant.principal}, + })) + s.Require().NoError(err) + } + + resources := []struct { + name string + id string + resource string + }{ + {"the disabled project", disabledProjectID, disabledProject}, + {"the disabled group", disabledGroupID, schema.JoinNamespaceAndResourceID(schema.GroupNamespace, disabledGroupID)}, + {"an order in the disabled project", orderID, order}, + } + livePolicies := func(resourceID string) int { + var count int + s.Require().NoError(s.dbc.GetContext(context.Background(), &count, + "SELECT count(*) FROM policies WHERE principal_id = $1 AND resource_id = $2 AND deleted_at IS NULL", userID, resourceID)) + return count + } + canGet := func(resource string) bool { + resp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: resource, + Permission: schema.GetPermission, + Subject: userSubject, + })) + s.Require().NoError(err) + return resp.Msg.GetStatus() + } + for _, r := range resources { + s.Require().Equalf(1, livePolicies(r.id), "live policy on %s before the removal", r.name) + s.Require().Truef(canGet(r.resource), "access to %s before the removal", r.name) + } + s.Require().True(canGet(groupProject), "access to a project through the group before the removal") + + _, err = s.testBench.Client.DisableProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DisableProjectRequest{Id: disabledProjectID})) + s.Require().NoError(err) + _, err = s.testBench.Client.DisableGroup(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DisableGroupRequest{Id: disabledGroupID})) + s.Require().NoError(err) + + _, err = s.testBench.Client.RemoveOrganizationMember(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.RemoveOrganizationMemberRequest{ + OrgId: orgID, + PrincipalId: userID, + PrincipalType: schema.UserPrincipal, + })) + s.Require().NoError(err) + + _, err = s.testBench.Client.EnableProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.EnableProjectRequest{Id: disabledProjectID})) + s.Require().NoError(err) + _, err = s.testBench.Client.EnableGroup(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.EnableGroupRequest{Id: disabledGroupID})) + s.Require().NoError(err) + + s.Run("no live policy is left on them", func() { + for _, r := range resources { + s.Assert().Zerof(livePolicies(r.id), "live policy on %s", r.name) + } + }) + + s.Run("enabling them again gives no access back", func() { + for _, r := range resources { + s.Assert().Falsef(canGet(r.resource), "access to %s", r.name) + } + }) + + s.Run("the group no longer gives access", func() { + s.Assert().False(canGet(groupProject)) + }) +} + func (s *APIRegressionTestSuite) TestUserDeleteWithOrgs() { ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie)