feat(membership): add group membership management

[whoAbhishekSah]

Summary

Adds the SetGroupMemberRole RPC and three new service methods on the membership package that fix the long-standing leaky-relation issue for groups by managing policy and SpiceDB relation in sync.

• AddGroupMember(groupID, principalID, principalType, roleID) — service-only (no proto). Validates the principal is a member of the group's parent org, rejects existing members with ErrAlreadyMember, creates policy + matching group#owner/group#member relation, falling back to delete the policy if relation creation fails.
• SetGroupMemberRole(...) — service + RPC. Rejects non-members with ErrNotMember, enforces min-owner constraint (ErrLastGroupOwnerRole) on demotion, replaces both policy and relation atomically (matches the org pattern in Add SetOrganizationMemberRole RPC to replace client-side policy manipulation #1459).
• OnGroupCreated(groupID, orgID, creatorID, creatorType) — service-only. Bundles group#org@organization + organization#member@group#member hierarchy relations with the initial owner add, so future group.Create can wire SpiceDB through one call.

Design notes

• Principal type is restricted to app/user for groups today. The switch in validateGroupPrincipal is structured so adding app/serviceuser later is a one-case change with no API impact.
• Audit events for added/role-changed cases were added under pkg/auditrecord and core/audit.
• No existing call sites are migrated in this PR — group.Create, AddGroupUsers, and the deletion of group.addOwner/AddMember/AddUsers/addMemberPolicy follow in subsequent PRs to keep each diff reviewable.

Why this is safe to land alone

• All new methods; nothing else calls them yet.
• Existing org/project membership flows and AddGroupUsers/group creation behavior are untouched.

Test plan

• go build ./...
• go test ./core/membership/... ./internal/api/v1beta1connect/... ./core/audit/... ./pkg/auditrecord/...
• New membership tests cover: not-found, principal-type rejection, disabled user, invalid group role, not-in-org, already-member, member add (member + owner role), policy cleanup on relation failure, role change (member↔owner with relation flip), skip-if-same, last-owner constraint, hierarchy linking on OnGroupCreated.
• New handler tests cover the full error-mapping matrix for SetGroupMemberRole.
• gofmt -l clean on changed files.

Follow-ups (separate PRs)

• Migrate group.Create to OnGroupCreated; delete group.addOwner/addAsOrgMember/addOrgToGroup.
• Migrate AddGroupUsers handler to loop AddGroupMember; delete group.AddMember/AddUsers/addMemberPolicy.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	May 13, 2026 3:34am

[coderabbitai]

Warning

Rate limit exceeded

@whoAbhishekSah has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 17 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 03be7ece-ac8d-4815-852a-43b323899f2e

📥 Commits

Reviewing files that changed from the base of the PR and between 291b0fc and f4d66d2.

📒 Files selected for processing (3)

• core/membership/service.go
• core/membership/service_test.go
• internal/api/v1beta1connect/group_test.go

📝 Walkthrough

Walkthrough

This PR implements group membership management, extending the membership service with methods to add members to groups, change member roles with ownership constraints, and wire groups to their parent organization. The changes include domain logic, API handler, error types, audit events, and authorization rules.

Changes

Group Membership Management

Layer / File(s)	Summary
Audit events and error constants core/membership/errors.go, internal/api/v1beta1connect/errors.go, core/audit/audit.go, pkg/auditrecord/consts.go	New error types for group role validation and last-owner constraints; audit event constants for group lifecycle and member operations.
Service interface and test mocks internal/api/v1beta1connect/interfaces.go, internal/api/v1beta1connect/mocks/membership_service.go	MembershipService interface declares AddGroupMember, SetGroupMemberRole, OnGroupCreated; autogenerated mocks provide testing infrastructure.
Core group membership service implementation core/membership/service.go	Implements member addition with eligibility validation (principal type, group role scope, org membership), role changes with last-owner constraint enforcement, group creation wiring to parent org, SpiceDB relation management, and audit event emission.
Service layer tests core/membership/service_test.go	Table-driven test suites for AddGroupMember, SetGroupMemberRole, and OnGroupCreated verify error handling, policy/relation creation, ownership constraint enforcement, compensating cleanup on failures, and audit trail generation.
API handler with error mapping internal/api/v1beta1connect/group.go, internal/api/v1beta1connect/group_test.go	SetGroupMemberRole handler extracts request fields, validates organization, delegates to membership service, and maps domain errors to Connect error codes (NotFound, InvalidArgument, FailedPrecondition); comprehensive tests cover all error scenarios and success path.
Authorization enforcement pkg/server/connect_interceptors/authorization.go	Adds authorization rule requiring Update permission on target group for SetGroupMemberRole RPC invocation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The PR introduces coherent group membership logic across service and API layers with moderate complexity: domain-level policy/relation management and constraint validation, familiar error-mapping patterns in the handler, and standard mock/authorization wiring. The main review work is understanding the group-member lifecycle (addition with org-membership check, role changes with owner constraint, relation management via SpiceDB) and verifying test coverage.

Possibly related issues

• raystack/frontier#1558: Proposes resourceSpec/Member refactoring to centralize role-to-relation and upsert/remove logic that would directly encompass the new group membership APIs added in this PR.
• raystack/frontier#1478: Goals to centralize membership logic in the membership package align with this PR's addition of AddGroupMember, SetGroupMemberRole, and OnGroupCreated methods to core/membership.Service.

Possibly related PRs

• raystack/frontier#1537: Extends the same core/membership/service.go and core/membership/errors.go files, directly building membership service infrastructure for org-member policy and relation management.

Suggested reviewers

• rohilsurana

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coveralls]

Coverage Report for CI Build 25776634343

Coverage increased (+0.3%) to 42.356%

Details

• Coverage increased (+0.3%) from the base build.
• Patch coverage: 70 uncovered changes across 3 files (260 of 330 lines covered, 78.79%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
core/membership/service.go	280	220	78.57%
internal/api/v1beta1connect/group.go	46	40	86.96%
pkg/server/connect_interceptors/authorization.go	4	0	0.0%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	37607
Covered Lines:	15929
Line Coverage:	42.36%
Coverage Strength:	11.88 hits per line

---

💛 - Coveralls

[whoAbhishekSah]

Manual Testing Results - SetGroupMemberRole RPC

Tested with multiple users and groups using the frontier-test CLI.

Test Environment

• Org: pr1596-b557d6
• Group1: Alice (owner), Bob (member), Charlie (member)
• Group2: Alice (owner only)
• Users: Alice, Bob, Charlie, Dave (in org), Eve (not in org)

Happy Path Tests

#	Test Case	Expected	Actual	Status
1	Promote Bob from member to owner	{} (success)	{}	✅ Pass
2	Demote Bob from owner to member (with Alice as owner)	{} (success)	{}	✅ Pass
3	Set same role (idempotent no-op)	{} (success)	{}	✅ Pass

Unhappy Path Tests

#	Test Case	Expected	Actual	Status
4	Invalid group_id (non-existent)	error	permission_denied	⚠️ Info
5	Invalid org_id (non-existent)	not_found	not_found: org doesn't exist	✅ Pass
6	Invalid principal_id (non-existent user)	not_found	not_found: user doesn't exist	✅ Pass
7	Invalid principal_type (app/serviceuser)	invalid_argument	invalid_argument: unsupported principal type	✅ Pass
8	User not in group (Dave)	failed_precondition	failed_precondition: principal is not a member of the resource	✅ Pass
9	Invalid role (org role for group)	invalid_argument	invalid_argument: role is not valid for group scope	✅ Pass
10	Demote last owner (Alice in group2)	failed_precondition	failed_precondition: group must have at least one owner...	✅ Pass

Authorization Tests

#	Test Case	Expected	Actual	Status
11	Non-owner (Bob) tries to change Charlie's role	permission_denied	permission_denied: not authorized	✅ Pass
12	Non-org-member (Eve) tries to change role	permission_denied	permission_denied: not authorized	✅ Pass
13	Set role for user not in org (Eve)	error	failed_precondition: principal is not a member of the resource	✅ Pass

Validation Tests

#	Test Case	Expected	Actual	Status
14	Invalid UUID format for group_id	validation error	validation error	✅ Pass
15	Empty principal_type	validation error	validation error	✅ Pass
16	Non-existent role_id	not_found	not_found: role id is invalid	✅ Pass

SpiceDB Relation Sync Tests

#	Test Case	Expected	Actual	Status
17	After promoting Bob to owner	Bob can update group	Bob can update group	✅ Pass
18	After demoting Bob to member	Bob cannot update group	permission_denied	✅ Pass

Summary

Category	Passed	Total
Happy Path	3	3
Unhappy Path	6	7
Authorization	3	3
Validation	3	3
Relation Sync	2	2
Total	17	18

Note: Test 4 returns permission_denied instead of not_found for invalid group_id. This is expected behavior - authorization is checked before resource existence.

The SpiceDB relation sync tests confirm that both policy and relation are updated atomically when roles change, fixing the leaky-relation issue.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

core/membership/service_test.go (1)

1587-1597: ⚡ Quick win

Add regression tests for hierarchy compensation paths.

Please add OnGroupCreated cases for: (1) second relation create failure with rollback of the first relation, and (2) AddGroupMember failure after successful hierarchy links with rollback of both links.

internal/api/v1beta1connect/group.go (1)

487-513: ⚡ Quick win

Only log the unexpected branch here.

LogServiceError currently runs for every mapped client error (not_found, invalid_argument, failed_precondition) before the switch. That will turn normal validation/membership failures into error-log noise and make real server faults harder to spot. Move the log call into the default branch, or otherwise limit it to the paths you truly treat as internal failures.

♻️ Suggested change

 	if err := h.membershipService.SetGroupMemberRole(ctx, groupID, principalID, principalType, roleID); err != nil {
-		errorLogger.LogServiceError(ctx, request, "SetGroupMemberRole", err,
-			"group_id", groupID,
-			"principal_id", principalID,
-			"principal_type", principalType,
-			"role_id", roleID)
-
 		switch {
 		case errors.Is(err, group.ErrNotExist), errors.Is(err, group.ErrInvalidID), errors.Is(err, group.ErrInvalidUUID):
 			return nil, connect.NewError(connect.CodeNotFound, ErrGroupNotFound)
@@
 		case errors.Is(err, membership.ErrLastGroupOwnerRole):
 			return nil, connect.NewError(connect.CodeFailedPrecondition, ErrLastGroupOwnerRole)
 		default:
+			errorLogger.LogServiceError(ctx, request, "SetGroupMemberRole", err,
+				"group_id", groupID,
+				"principal_id", principalID,
+				"principal_type", principalType,
+				"role_id", roleID)
 			return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError)
 		}
 	}

pkg/server/connect_interceptors/authorization.go (1)

477-480: ⚡ Quick win

Please add an interceptor regression test for this route.

This fix lives entirely in authorizationValidationMap, and the original failure mode was the interceptor returning permission_denied before the handler ran. A handler-only test suite won't catch that regression if this entry is removed or wired to the wrong permission later.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d40d9e97-70c8-43db-affb-591942055065

📥 Commits

Reviewing files that changed from the base of the PR and between f4d3398 and 291b0fc.

📒 Files selected for processing (11)

• core/audit/audit.go
• core/membership/errors.go
• core/membership/service.go
• core/membership/service_test.go
• internal/api/v1beta1connect/errors.go
• internal/api/v1beta1connect/group.go
• internal/api/v1beta1connect/group_test.go
• internal/api/v1beta1connect/interfaces.go
• internal/api/v1beta1connect/mocks/membership_service.go
• pkg/auditrecord/consts.go
• pkg/server/connect_interceptors/authorization.go