From 4d3c7a8503c63dfda6c9d817b6a943acbdd75cd7 Mon Sep 17 00:00:00 2001 From: aman Date: Fri, 13 Nov 2026 14:41:53 +0530 Subject: [PATCH] fix(membership): allow removing a member from a disabled project or group RemoveProjectMember and RemoveGroupMember now find the project or group even when it is disabled, so an admin can take a member's access away without enabling it first. The RemoveGroupUser handler no longer loads the group itself, since that lookup skipped disabled groups. --- core/membership/group.go | 2 +- core/membership/group_test.go | 40 +++++-- core/membership/project.go | 14 ++- core/membership/project_test.go | 41 +++++++- internal/api/v1beta1connect/group.go | 4 - internal/api/v1beta1connect/group_test.go | 9 -- test/e2e/regression/api_test.go | 122 ++++++++++++++++++++++ 7 files changed, 206 insertions(+), 26 deletions(-) diff --git a/core/membership/group.go b/core/membership/group.go index 79003b1579..09fd8a1da2 100644 --- a/core/membership/group.go +++ b/core/membership/group.go @@ -109,7 +109,7 @@ func (s *Service) SetGroupMemberRole(ctx context.Context, groupID, principalID, // the principal has no policies on this group; ErrLastGroupOwnerRole if they // are the sole remaining owner (enforced atomically via the policy guard). func (s *Service) RemoveGroupMember(ctx context.Context, groupID, principalID, principalType string) error { - grp, err := s.groupService.Get(ctx, groupID) + grp, err := s.getGroupIncludingDisabled(ctx, groupID) if err != nil { return err } diff --git a/core/membership/group_test.go b/core/membership/group_test.go index 184ff804a8..36b49f26a2 100644 --- a/core/membership/group_test.go +++ b/core/membership/group_test.go @@ -288,6 +288,10 @@ func TestService_RemoveGroupMember(t *testing.T) { enabledUser := user.User{ID: userID, Title: "test-user", Email: "test@acme.dev", State: user.Enabled} grp := group.Group{ID: groupID, OrganizationID: orgID, Title: "Test Group"} + disabledGrp := grp + disabledGrp.State = group.Disabled + includeDisabled := group.Filter{IncludeDisabled: true} + lookupErr := errors.New("group lookup failed") expectOwnerRoleLookup := func(roleSvc *mocks.RoleService) { roleSvc.EXPECT().Get(ctx, schema.GroupOwnerRole).Return(role.Role{ID: ownerRoleID, Name: schema.GroupOwnerRole}, nil) @@ -303,14 +307,21 @@ func TestService_RemoveGroupMember(t *testing.T) { { name: "should return error if group does not exist", setup: func(_ *mocks.PolicyService, _ *mocks.RelationService, _ *mocks.RoleService, grpSvc *mocks.GroupService, _ *mocks.UserService, _ *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(group.Group{}, group.ErrNotExist) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{}, nil) }, wantErr: group.ErrNotExist, }, + { + name: "should return error if the group lookup fails", + setup: func(_ *mocks.PolicyService, _ *mocks.RelationService, _ *mocks.RoleService, grpSvc *mocks.GroupService, _ *mocks.UserService, _ *mocks.AuditRecordRepository) { + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return(nil, lookupErr) + }, + wantErr: lookupErr, + }, { name: "should return error if principal type is unsupported", setup: func(_ *mocks.PolicyService, _ *mocks.RelationService, _ *mocks.RoleService, grpSvc *mocks.GroupService, _ *mocks.UserService, _ *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{grp}, nil) }, principalType: schema.ServiceUserPrincipal, wantErr: membership.ErrInvalidPrincipalType, @@ -318,7 +329,7 @@ func TestService_RemoveGroupMember(t *testing.T) { { name: "should return ErrNotMember if principal has no group policy", setup: func(policySvc *mocks.PolicyService, _ *mocks.RelationService, _ *mocks.RoleService, grpSvc *mocks.GroupService, userSvc *mocks.UserService, _ *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{grp}, nil) userSvc.EXPECT().GetByID(ctx, userID).Return(enabledUser, nil) policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{}, nil) }, @@ -327,7 +338,7 @@ func TestService_RemoveGroupMember(t *testing.T) { { name: "should return ErrLastGroupOwnerRole when removing last owner (pre-check fires)", setup: func(policySvc *mocks.PolicyService, _ *mocks.RelationService, roleSvc *mocks.RoleService, grpSvc *mocks.GroupService, userSvc *mocks.UserService, _ *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{grp}, nil) userSvc.EXPECT().GetByID(ctx, userID).Return(enabledUser, nil) policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: ownerRoleID}}, nil) expectOwnerRoleLookup(roleSvc) @@ -338,7 +349,7 @@ func TestService_RemoveGroupMember(t *testing.T) { { name: "should surface ErrLastGroupOwnerRole when DeleteWithMinRoleGuard races (TOCTOU)", setup: func(policySvc *mocks.PolicyService, _ *mocks.RelationService, roleSvc *mocks.RoleService, grpSvc *mocks.GroupService, userSvc *mocks.UserService, _ *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{grp}, nil) userSvc.EXPECT().GetByID(ctx, userID).Return(enabledUser, nil) policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: ownerRoleID}}, nil) expectOwnerRoleLookup(roleSvc) @@ -351,7 +362,7 @@ func TestService_RemoveGroupMember(t *testing.T) { { name: "should remove a member (non-owner) and delete the member relation", setup: func(policySvc *mocks.PolicyService, relSvc *mocks.RelationService, roleSvc *mocks.RoleService, grpSvc *mocks.GroupService, userSvc *mocks.UserService, auditRepo *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{grp}, nil) userSvc.EXPECT().GetByID(ctx, userID).Return(enabledUser, nil) policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: memberRoleID}}, nil) expectOwnerRoleLookup(roleSvc) @@ -364,10 +375,25 @@ func TestService_RemoveGroupMember(t *testing.T) { }, wantErr: nil, }, + { + name: "should remove a member from a disabled group and delete the member relation", + setup: func(policySvc *mocks.PolicyService, relSvc *mocks.RelationService, roleSvc *mocks.RoleService, grpSvc *mocks.GroupService, userSvc *mocks.UserService, auditRepo *mocks.AuditRecordRepository) { + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{disabledGrp}, nil) + userSvc.EXPECT().GetByID(ctx, userID).Return(enabledUser, nil) + policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: memberRoleID}}, nil) + expectOwnerRoleLookup(roleSvc) + policySvc.EXPECT().Delete(ctx, "p1").Return(nil) + obj := relation.Object{ID: groupID, Namespace: schema.GroupNamespace} + sub := relation.Subject{ID: userID, Namespace: schema.UserPrincipal} + relSvc.EXPECT().Delete(ctx, relation.Relation{Object: obj, Subject: sub, RelationName: schema.MemberRelationName}).Return(nil) + auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) + }, + wantErr: nil, + }, { name: "should remove an owner via atomic guard when more owners remain", setup: func(policySvc *mocks.PolicyService, relSvc *mocks.RelationService, roleSvc *mocks.RoleService, grpSvc *mocks.GroupService, userSvc *mocks.UserService, auditRepo *mocks.AuditRecordRepository) { - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, includeDisabled).Return([]group.Group{grp}, nil) userSvc.EXPECT().GetByID(ctx, userID).Return(enabledUser, nil) policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "p1", RoleID: ownerRoleID}}, nil) expectOwnerRoleLookup(roleSvc) diff --git a/core/membership/project.go b/core/membership/project.go index 2be87ff947..8bd9a79b5d 100644 --- a/core/membership/project.go +++ b/core/membership/project.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/raystack/frontier/core/policy" + "github.com/raystack/frontier/core/project" "github.com/raystack/frontier/core/relation" "github.com/raystack/frontier/core/role" "github.com/raystack/frontier/internal/bootstrap/schema" @@ -61,7 +62,7 @@ func (s *Service) RemoveProjectMember(ctx context.Context, projectID, principalI return ErrInvalidPrincipalType } - prj, err := s.projectService.Get(ctx, projectID) + prj, err := s.getProjectIncludingDisabled(ctx, projectID) if err != nil { return err } @@ -78,6 +79,17 @@ func (s *Service) RemoveProjectMember(ctx context.Context, projectID, principalI return nil } +func (s *Service) getProjectIncludingDisabled(ctx context.Context, projectID string) (project.Project, error) { + projects, err := s.projectService.List(ctx, project.Filter{ProjectIDs: []string{projectID}, IncludeDisabled: true}) + if err != nil { + return project.Project{}, err + } + if len(projects) == 0 { + return project.Project{}, project.ErrNotExist + } + return projects[0], nil +} + // OnProjectCreated runs right after a project is created. It links the project // to its parent org in SpiceDB and gives the creator the project owner role. // There is no org-membership check on the creator here: platform superusers diff --git a/core/membership/project_test.go b/core/membership/project_test.go index 767789e12a..746d512dd5 100644 --- a/core/membership/project_test.go +++ b/core/membership/project_test.go @@ -170,6 +170,10 @@ func TestService_RemoveProjectMember(t *testing.T) { Title: "Test Project", Organization: organization.Organization{ID: uuid.New().String()}, } + disabledPrj := prj + disabledPrj.State = project.Disabled + lookupFilter := project.Filter{ProjectIDs: []string{projectID}, IncludeDisabled: true} + lookupErr := errors.New("project lookup failed") tests := []struct { name string @@ -184,10 +188,28 @@ func TestService_RemoveProjectMember(t *testing.T) { principalType: "app/invalid", wantErr: membership.ErrInvalidPrincipalType, }, + { + name: "should return ErrNotExist if the project does not exist", + setup: func(_ *mocks.PolicyService, prjSvc *mocks.ProjectService, _ *mocks.AuditRecordRepository) { + prjSvc.EXPECT().List(ctx, lookupFilter).Return([]project.Project{}, nil) + }, + principalID: userID, + principalType: schema.UserPrincipal, + wantErr: project.ErrNotExist, + }, + { + name: "should return error if the project lookup fails", + setup: func(_ *mocks.PolicyService, prjSvc *mocks.ProjectService, _ *mocks.AuditRecordRepository) { + prjSvc.EXPECT().List(ctx, lookupFilter).Return(nil, lookupErr) + }, + principalID: userID, + principalType: schema.UserPrincipal, + wantErr: lookupErr, + }, { name: "should return error if not a member", setup: func(policySvc *mocks.PolicyService, prjSvc *mocks.ProjectService, _ *mocks.AuditRecordRepository) { - prjSvc.EXPECT().Get(ctx, projectID).Return(prj, nil) + prjSvc.EXPECT().List(ctx, lookupFilter).Return([]project.Project{prj}, nil) policySvc.EXPECT().List(ctx, policy.Filter{ProjectID: projectID, PrincipalID: userID, PrincipalType: schema.UserPrincipal, ResourceType: schema.ProjectNamespace}).Return([]policy.Policy{}, nil) }, principalID: userID, @@ -197,7 +219,18 @@ func TestService_RemoveProjectMember(t *testing.T) { { name: "should succeed removing a user", setup: func(policySvc *mocks.PolicyService, prjSvc *mocks.ProjectService, auditRepo *mocks.AuditRecordRepository) { - prjSvc.EXPECT().Get(ctx, projectID).Return(prj, nil) + prjSvc.EXPECT().List(ctx, lookupFilter).Return([]project.Project{prj}, nil) + policySvc.EXPECT().List(ctx, policy.Filter{ProjectID: projectID, PrincipalID: userID, PrincipalType: schema.UserPrincipal, ResourceType: schema.ProjectNamespace}).Return([]policy.Policy{{ID: "p1"}}, nil) + policySvc.EXPECT().Delete(ctx, "p1").Return(nil) + auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) + }, + principalID: userID, + principalType: schema.UserPrincipal, + }, + { + name: "should succeed removing a member from a disabled project", + setup: func(policySvc *mocks.PolicyService, prjSvc *mocks.ProjectService, auditRepo *mocks.AuditRecordRepository) { + prjSvc.EXPECT().List(ctx, lookupFilter).Return([]project.Project{disabledPrj}, nil) policySvc.EXPECT().List(ctx, policy.Filter{ProjectID: projectID, PrincipalID: userID, PrincipalType: schema.UserPrincipal, ResourceType: schema.ProjectNamespace}).Return([]policy.Policy{{ID: "p1"}}, nil) policySvc.EXPECT().Delete(ctx, "p1").Return(nil) auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) @@ -208,7 +241,7 @@ func TestService_RemoveProjectMember(t *testing.T) { { name: "should succeed removing a service user", setup: func(policySvc *mocks.PolicyService, prjSvc *mocks.ProjectService, auditRepo *mocks.AuditRecordRepository) { - prjSvc.EXPECT().Get(ctx, projectID).Return(prj, nil) + prjSvc.EXPECT().List(ctx, lookupFilter).Return([]project.Project{prj}, nil) policySvc.EXPECT().List(ctx, policy.Filter{ProjectID: projectID, PrincipalID: suID, PrincipalType: schema.ServiceUserPrincipal, ResourceType: schema.ProjectNamespace}).Return([]policy.Policy{{ID: "p1"}}, nil) policySvc.EXPECT().Delete(ctx, "p1").Return(nil) auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) @@ -219,7 +252,7 @@ func TestService_RemoveProjectMember(t *testing.T) { { name: "should succeed removing a PAT", setup: func(policySvc *mocks.PolicyService, prjSvc *mocks.ProjectService, auditRepo *mocks.AuditRecordRepository) { - prjSvc.EXPECT().Get(ctx, projectID).Return(prj, nil) + prjSvc.EXPECT().List(ctx, lookupFilter).Return([]project.Project{prj}, nil) policySvc.EXPECT().List(ctx, policy.Filter{ProjectID: projectID, PrincipalID: userID, PrincipalType: schema.PATPrincipal, ResourceType: schema.ProjectNamespace}).Return([]policy.Policy{{ID: "p1"}}, nil) policySvc.EXPECT().Delete(ctx, "p1").Return(nil) auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) diff --git a/internal/api/v1beta1connect/group.go b/internal/api/v1beta1connect/group.go index f6023cbd4f..5397634372 100644 --- a/internal/api/v1beta1connect/group.go +++ b/internal/api/v1beta1connect/group.go @@ -325,10 +325,6 @@ func (h *ConnectHandler) ListGroupUsers(ctx context.Context, request *connect.Re } func (h *ConnectHandler) RemoveGroupUser(ctx context.Context, request *connect.Request[frontierv1beta1.RemoveGroupUserRequest]) (*connect.Response[frontierv1beta1.RemoveGroupUserResponse], error) { - if _, err := h.getGroup(ctx, request.Msg.GetId()); err != nil { - return nil, err - } - if err := h.membershipService.RemoveGroupMember(ctx, request.Msg.GetId(), request.Msg.GetUserId(), schema.UserPrincipal); err != nil { switch { case errors.Is(err, group.ErrNotExist), errors.Is(err, group.ErrInvalidID), errors.Is(err, group.ErrInvalidUUID): diff --git a/internal/api/v1beta1connect/group_test.go b/internal/api/v1beta1connect/group_test.go index 41d7e722c5..c9797fc39f 100644 --- a/internal/api/v1beta1connect/group_test.go +++ b/internal/api/v1beta1connect/group_test.go @@ -1153,7 +1153,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return not found if group does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(group.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1166,7 +1165,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return not found if user does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(user.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1179,7 +1177,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return failed precondition if user is not a group member", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrNotMember) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1192,7 +1189,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return invalid argument if user is the only owner", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrLastGroupOwnerRole) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1205,7 +1201,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return failed precondition if user is disabled", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(user.ErrDisabled) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1218,7 +1213,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return invalid argument if principal type is unsupported", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrInvalidPrincipalType) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1231,7 +1225,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return invalid argument if principal is invalid", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrInvalidPrincipal) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1244,7 +1237,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should return internal error for unknown errors", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(errors.New("unknown")) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1257,7 +1249,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { { name: "should remove user successfully", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index d2463a0114..459d4d3ccd 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -1363,6 +1363,128 @@ func (s *APIRegressionTestSuite) TestGroupAPI() { }) } +func (s *APIRegressionTestSuite) TestMemberRemovalFromDisabledProjectAndGroup() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + _, ownerCtx := signUpUser(s.T(), s.testBench.Client, "disabled-removal-owner@raystack.org") + + orgResp, err := s.testBench.Client.CreateOrganization(ownerCtx, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{Title: "disabled-removal-org", Name: "disabled-removal-org"}, + })) + s.Require().NoError(err) + orgID := orgResp.Msg.GetOrganization().GetId() + createProject := func(name string) string { + resp, err := s.testBench.Client.CreateProject(ownerCtx, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{Name: name, OrgId: orgID}, + })) + s.Require().NoError(err) + return resp.Msg.GetProject().GetId() + } + projectID := createProject("disabled-removal-project") + groupProjectID := createProject("disabled-removal-group-project") + + groupResp, err := s.testBench.Client.CreateGroup(ownerCtx, connect.NewRequest(&frontierv1beta1.CreateGroupRequest{ + OrgId: orgID, + Body: &frontierv1beta1.GroupRequestBody{Name: "disabled-removal-group"}, + })) + s.Require().NoError(err) + groupID := groupResp.Msg.GetGroup().GetId() + + userResp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateUserRequest{ + Body: &frontierv1beta1.UserRequestBody{Email: "disabled-removal-member@raystack.org", Name: "disabled-removal-member"}, + })) + 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: groupID, + PrincipalId: userID, + PrincipalType: schema.UserPrincipal, + RoleId: s.groupMemberRole, + })) + s.Require().NoError(err) + + project := schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, projectID) + group := schema.JoinNamespaceAndResourceID(schema.GroupNamespace, groupID) + groupProject := schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, groupProjectID) + for _, grant := range []struct{ resource, principal string }{ + {project, userSubject}, + {groupProject, schema.JoinNamespaceAndResourceID(schema.GroupPrincipal, groupID)}, + } { + _, err := s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{RoleId: schema.RoleProjectViewer, Resource: grant.resource, Principal: grant.principal}, + })) + s.Require().NoError(err) + } + + 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() + } + s.Require().Equal(1, livePolicies(projectID), "live policy on the project before the removal") + s.Require().Equal(1, livePolicies(groupID), "live policy on the group before the removal") + s.Require().True(canGet(project), "access to the project before the removal") + s.Require().True(canGet(group), "access to the group before the removal") + s.Require().True(canGet(groupProject), "access to a project through the group before the removal") + + _, err = s.testBench.Client.DisableProject(ownerCtx, connect.NewRequest(&frontierv1beta1.DisableProjectRequest{Id: projectID})) + s.Require().NoError(err) + _, err = s.testBench.Client.DisableGroup(ownerCtx, connect.NewRequest(&frontierv1beta1.DisableGroupRequest{Id: groupID})) + s.Require().NoError(err) + + s.Run("the owner can remove the member from the disabled project", func() { + _, err := s.testBench.Client.RemoveProjectMember(ownerCtx, connect.NewRequest(&frontierv1beta1.RemoveProjectMemberRequest{ + ProjectId: projectID, + PrincipalId: userID, + PrincipalType: schema.UserPrincipal, + })) + s.Assert().NoError(err) + }) + + s.Run("the owner can remove the member from the disabled group", func() { + _, err := s.testBench.Client.RemoveGroupUser(ownerCtx, connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ + Id: groupID, + UserId: userID, + })) + s.Assert().NoError(err) + }) + + _, err = s.testBench.Client.EnableProject(ownerCtx, connect.NewRequest(&frontierv1beta1.EnableProjectRequest{Id: projectID})) + s.Require().NoError(err) + _, err = s.testBench.Client.EnableGroup(ownerCtx, connect.NewRequest(&frontierv1beta1.EnableGroupRequest{Id: groupID})) + s.Require().NoError(err) + + s.Run("no live policy is left on them", func() { + s.Assert().Zero(livePolicies(projectID), "live policy on the project") + s.Assert().Zero(livePolicies(groupID), "live policy on the group") + }) + + s.Run("enabling them again gives no access back", func() { + s.Assert().False(canGet(project), "access to the project") + s.Assert().False(canGet(group), "access to the group") + }) + + s.Run("the group no longer gives access", func() { + s.Assert().False(canGet(groupProject)) + }) +} + func (s *APIRegressionTestSuite) TestUserAPI() { var newUser *frontierv1beta1.User