feat: implement SetProjectMemberRole RPC by whoAbhishekSah · Pull Request #1481 · raystack/frontier

whoAbhishekSah · 2026-03-27T04:21:34Z

Summary

Add SetProjectMemberRole RPC for atomic role assignment on project members
Supports user, service user, and group principals via principal_id + principal_type
Validates principal exists and belongs to the project's org
Validates role has project scope (app/project)
Authorization checks update permission on the project (consistent with CreatePolicyForProject)
No minimum-owner constraint — project access can come from org-level permissions

Closes #1461 (partially — RemoveProjectMember will be a separate PR)

Proton PR: raystack/proton#456

Test plan

Unit tests for service layer (12 cases)
E2E tests — 29/29 pass on clean database
Authorization verified for org owner, org admin, org member, project owner, project viewer, non-member, unauthenticated

🤖 Generated with Claude Code

vercel · 2026-03-27T04:21:39Z

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

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	Apr 2, 2026 3:21am

coderabbitai · 2026-03-27T04:21:47Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a server-side RPC to set a project member's role and implements end-to-end support: service logic, Connect handler, interface/mocks, tests, domain errors, authorization entry, and updates Makefile PROTON_COMMIT.

Changes

Cohort / File(s)	Summary
Build config `Makefile`	Updated `PROTON_COMMIT` hash used by `make proto`.
Server wiring `cmd/serve.go`	Passed new `roleService` into `project.NewService(...)` when assembling API dependencies.
Domain errors `core/project/errors.go`, `internal/api/v1beta1connect/errors.go`	Added errors: `ErrInvalidProjectRole`, `ErrNotOrgMember`, `ErrInvalidPrincipalType` (API errors file adds `ErrInvalidProjectRole`).
Project service implementation `core/project/service.go`	Added `RoleService` interface and `roleService` dependency; implemented `Service.SetMemberRole(...)` plus helpers `validatePrincipal` and `validateProjectRole`; added required service method signatures (Get/Delete/Get).
Service tests `core/project/service_test.go`	Added `mocks.RoleService` to test harness, adapted existing NewService calls, and added `TestService_SetMemberRole` with table-driven scenarios and mock expectations.
Core mocks `core/project/mocks/...` `role_service.go`, `policy_service.go`, `group_service.go`, `serviceuser_service.go`	Added autogenerated `RoleService` mock and typed mock methods/expecters: `PolicyService.Delete`, `GroupService.Get`, `ServiceuserService.Get`, plus helpers for configuring calls/returns.
API interface & mocks `internal/api/v1beta1connect/interfaces.go`, `internal/api/v1beta1connect/mocks/project_service.go`	Added `ProjectService.SetMemberRole(...)` to Connect interface and corresponding mock expecter/call helpers.
API handler `internal/api/v1beta1connect/project.go`	Added `ConnectHandler.SetProjectMemberRole(...)` with request validation, service invocation, structured logging, and domain→Connect error mapping.
Authorization interceptor `pkg/server/connect_interceptors/authorization.go`	Added authorization validation entry for `FrontierService/SetProjectMemberRole`, checking `update` on the project resource.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

feat: implement SearchOrganizationPATs admin RPC #1482: also updates the PROTON_COMMIT Makefile variable (same Build/proto input change).
feat: implement UpdateCurrentUserPAT RPC #1467: also changes PROTON_COMMIT used by make proto.
feat: implement CheckCurrentUserPATTitle RPC #1469: also updates PROTON_COMMIT hash for protobuf generation.

Suggested reviewers

rsbh

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❓ Inconclusive	Proton commit update in Makefile appears related to protobuf generation for the RPC, but PR description references proton PR `#456` without explicit confirmation of what changes are needed.	Verify that the Proton commit hash f51485d9a328b15f0a8d0dc14fcb405438e7ba93 includes the SetProjectMemberRole RPC proto definition from raystack/proton#456.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All core requirements from issue `#1461` have been implemented: SetProjectMemberRole RPC handles both add and role change as atomic operations, validates project role scopes, enforces authorization with update permission, and includes unit tests.

_{✏️ 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

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

coderabbitai

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

core/project/service_test.go (1)

23-35: ⚠️ Potential issue | 🟠 Major

Add direct unit tests for SetMemberRole behavior (currently missing).

This update wires new dependencies, but the new project-member role mutation path is not covered here. Please add focused tests for role validation, non-member handling, and last-owner protection to reduce regression risk in the new RPC path.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 56252940-859b-4c10-83dd-9b50d88a457e

📥 Commits

Reviewing files that changed from the base of the PR and between d05d9e5 and b32bbf7.

⛔ Files ignored due to path filters (3)

proto/v1beta1/frontier.pb.go is excluded by !**/*.pb.go, !proto/**
proto/v1beta1/frontier.pb.validate.go is excluded by !proto/**
proto/v1beta1/frontierv1beta1connect/frontier.connect.go is excluded by !proto/**

📒 Files selected for processing (12)

Makefile
cmd/serve.go
core/project/errors.go
core/project/mocks/policy_service.go
core/project/mocks/role_service.go
core/project/service.go
core/project/service_test.go
internal/api/v1beta1connect/errors.go
internal/api/v1beta1connect/interfaces.go
internal/api/v1beta1connect/mocks/project_service.go
internal/api/v1beta1connect/project.go
pkg/server/connect_interceptors/authorization.go

core/project/service.go

internal/api/v1beta1connect/project.go

coveralls · 2026-03-27T05:10:49Z

Pull Request Test Coverage Report for Build 23882133908

Details

72 of 120 (60.0%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.06%) to 41.249%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cmd/serve.go	0	1	0.0%
pkg/server/connect_interceptors/authorization.go	0	4	0.0%
core/project/service.go	72	78	92.31%
internal/api/v1beta1connect/project.go	0	37	0.0%

Totals
Change from base Build 23839671857:	0.06%
Covered Lines:	14910
Relevant Lines:	36146

💛 - Coveralls

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (1)

core/project/service.go (1)

360-363: ⚠️ Potential issue | 🟠 Major

The last-owner invariant is gone.

Demoting the only direct owner now succeeds and leaves the project with zero project-scoped owners. That regresses the owner-constraint behavior this RPC is supposed to preserve; if org-level fallback is intentionally replacing that invariant, the contract/tests need to change too.

Also applies to: 377-390

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7a3f72ab-6cdf-4e25-beb2-3c66806bc560

📥 Commits

Reviewing files that changed from the base of the PR and between b32bbf7 and fc069d7.

📒 Files selected for processing (3)

core/project/errors.go
core/project/service.go
internal/api/v1beta1connect/project.go

🚧 Files skipped from review as they are similar to previous changes (2)

core/project/errors.go
internal/api/v1beta1connect/project.go

core/project/service.go

coderabbitai

♻️ Duplicate comments (1)

core/project/service.go (1)
362-402: ⚠️ Potential issue | 🟠 Major

Use the resolved project ID (prj.ID) consistently.

Lines 380 and 396 use the raw projectID parameter instead of prj.ID. Since s.Get() at line 366 resolves both UUIDs and names, passing a project name will cause the policy filter and create operations to use the name string instead of the canonical UUID.
🐛 Proposed fix
 	existingPolicies, err := s.policyService.List(ctx, policy.Filter{
-		ProjectID:     projectID,
+		ProjectID:     prj.ID,
 		PrincipalID:   principalID,
 		PrincipalType: principalType,
 	})
@@
 	_, err = s.policyService.Create(ctx, policy.Policy{
 		RoleID:        newRoleID,
-		ResourceID:    projectID,
+		ResourceID:    prj.ID,
 		ResourceType:  schema.ProjectNamespace,
 		PrincipalID:   principalID,
 		PrincipalType: principalType,
 	})

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c2d62ec5-29b5-4805-9f36-2c26ab203e72

📥 Commits

Reviewing files that changed from the base of the PR and between fc069d7 and 1d15a38.

⛔ Files ignored due to path filters (2)

proto/v1beta1/frontier.pb.go is excluded by !**/*.pb.go, !proto/**
proto/v1beta1/frontier.pb.validate.go is excluded by !proto/**

📒 Files selected for processing (9)

Makefile
core/project/errors.go
core/project/mocks/group_service.go
core/project/mocks/serviceuser_service.go
core/project/service.go
core/project/service_test.go
internal/api/v1beta1connect/interfaces.go
internal/api/v1beta1connect/mocks/project_service.go
internal/api/v1beta1connect/project.go

🚧 Files skipped from review as they are similar to previous changes (3)

Makefile
internal/api/v1beta1connect/project.go
core/project/errors.go

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

core/project/service_test.go (1)

59-60: Remove redundant _ = roleService no-op assignments.

These lines add noise and can be dropped since roleService is passed to project.NewService(...) in the same scope.

Also applies to: 77-78, 122-123, 140-141

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d83355dc-3728-44ea-b279-794bb4b06969

📥 Commits

Reviewing files that changed from the base of the PR and between 1d15a38 and cdfc3a9.

📒 Files selected for processing (3)

core/project/service.go
core/project/service_test.go
pkg/server/connect_interceptors/authorization.go

✅ Files skipped from review due to trivial changes (1)

pkg/server/connect_interceptors/authorization.go

🚧 Files skipped from review as they are similar to previous changes (1)

core/project/service.go

core/project/service_test.go

whoAbhishekSah · 2026-03-30T08:49:33Z

E2E Test Results — 29/29 PASS

Tested on a clean database (fresh Frontier + SpiceDB) with users at different permission levels.

Happy Path — 9/9 PASS

#	Test	Principal	Action	Result
H1	Add user as project viewer	app/user	assign app_project_viewer	PASS
H2	Change user role (viewer → owner)	app/user	assign app_project_owner	PASS
H3	Downgrade user role (owner → viewer)	app/user	assign app_project_viewer	PASS
H4	Idempotency (set same role again)	app/user	assign app_project_viewer again	PASS
H5	Add service user as project viewer	app/serviceuser	assign app_project_viewer	PASS
H6	Change service user role (viewer → owner)	app/serviceuser	assign app_project_owner	PASS
H7	Add group as project viewer	app/group	assign app_project_viewer	PASS
H8	Change group role (viewer → owner)	app/group	assign app_project_owner	PASS
H9	Add second user as project viewer	app/user	assign app_project_viewer	PASS

Authorization — 7/7 PASS

#	Caller	Caller's Roles	Result	Verdict
A1	user1	org owner + project owner	{} success	PASS
A2	user2	org admin, no project role	{} success	PASS
A3	user3	org member + project viewer	permission_denied	PASS
A4	user4	not in org	permission_denied	PASS
A5	(no cookie)	unauthenticated	unauthenticated	PASS
A6	user3	org member + project viewer	permission_denied	PASS
A7	user5	org member + project owner	{} success	PASS

A2: Org admin (app_organization_manager) is correctly allowed — the role has cascading project update permission via the authz model.

A3/A6: Project viewer correctly denied on a clean SpiceDB instance. Previous local testing showed false positives due to stale SpiceDB permission relations from the additive-only role.Service.Upsert.

Validation — 13/13 PASS

#	Test	Expected	Actual	Verdict
V1	Invalid project_id (not UUID)	error	not_found	PASS
V2	Non-existent project_id	error	permission_denied	PASS
V3	Invalid principal_id (not UUID)	invalid_argument	invalid_argument	PASS
V4	Non-existent user principal	not_found	not_found	PASS
V5	Non-existent service user principal	not_found	not_found	PASS
V6	Non-existent group principal	not_found	not_found	PASS
V7	Invalid principal_type	invalid_argument	invalid_argument	PASS
V8	Empty principal_type	invalid_argument	invalid_argument	PASS
V9	Invalid role_id (not UUID)	invalid_argument	invalid_argument	PASS
V10	Non-existent role	not_found	not_found	PASS
V11	Wrong scope role (org role on project)	invalid_argument	invalid_argument	PASS
V12	User not in org	failed_precondition	failed_precondition	PASS
V13	Empty body	invalid_argument	invalid_argument	PASS

pkg/server/connect_interceptors/authorization.go

core/project/service.go

internal/api/v1beta1connect/project.go

AmanGIT07

LGTM

Add handler, service, and authorization for atomically setting a principal's role in a project. Supports user, service user, and group principals. - Validates principal exists and belongs to the project's org - Validates role has project scope (app/project) - Authorization checks update permission on the project - No minimum-owner constraint (project access can come from org-level) - 14 unit tests covering all validation and success paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vercel bot deployed to Preview March 27, 2026 04:22 View deployment

coderabbitai bot reviewed Mar 27, 2026

View reviewed changes

vercel bot deployed to Preview March 27, 2026 05:05 View deployment

vercel bot deployed to Preview March 27, 2026 05:07 View deployment

vercel bot deployed to Preview March 27, 2026 05:18 View deployment

vercel bot deployed to Preview March 27, 2026 05:28 View deployment

coderabbitai bot reviewed Mar 27, 2026

View reviewed changes

core/project/service.go Outdated Show resolved Hide resolved

vercel bot deployed to Preview March 27, 2026 05:33 View deployment

vercel bot deployed to Preview March 27, 2026 05:40 View deployment

vercel bot deployed to Preview March 27, 2026 05:41 View deployment

vercel bot deployed to Preview March 27, 2026 05:47 View deployment

coderabbitai bot reviewed Mar 27, 2026

View reviewed changes

vercel bot deployed to Preview March 30, 2026 04:06 View deployment

coderabbitai bot reviewed Mar 30, 2026

View reviewed changes

core/project/service_test.go Show resolved Hide resolved

vercel bot deployed to Preview March 30, 2026 04:18 View deployment

vercel bot deployed to Preview March 30, 2026 09:19 View deployment

whoAbhishekSah force-pushed the feat/set-project-member-role branch from d48a848 to 606a863 Compare March 30, 2026 09:20

vercel bot deployed to Preview March 30, 2026 09:21 View deployment

AmanGIT07 reviewed Mar 31, 2026

View reviewed changes

pkg/server/connect_interceptors/authorization.go Show resolved Hide resolved

AmanGIT07 reviewed Mar 31, 2026

View reviewed changes

core/project/service.go Show resolved Hide resolved

rohilsurana reviewed Mar 31, 2026

View reviewed changes

internal/api/v1beta1connect/project.go Outdated Show resolved Hide resolved

vercel bot deployed to Preview April 1, 2026 04:01 View deployment

whoAbhishekSah requested review from AmanGIT07 and rohilsurana April 1, 2026 04:02

whoAbhishekSah force-pushed the feat/set-project-member-role branch from c0191e0 to 790192f Compare April 1, 2026 04:03

vercel bot deployed to Preview April 1, 2026 04:04 View deployment

whoAbhishekSah force-pushed the feat/set-project-member-role branch from 790192f to 2d16d2d Compare April 1, 2026 04:28

vercel bot deployed to Preview April 1, 2026 04:29 View deployment

AmanGIT07 approved these changes Apr 1, 2026

View reviewed changes

whoAbhishekSah force-pushed the feat/set-project-member-role branch from 2d16d2d to 273767e Compare April 1, 2026 07:10

vercel bot deployed to Preview April 1, 2026 07:11 View deployment

whoAbhishekSah force-pushed the feat/set-project-member-role branch from 273767e to ff832b5 Compare April 1, 2026 07:12

vercel bot deployed to Preview April 1, 2026 07:13 View deployment

whoAbhishekSah force-pushed the feat/set-project-member-role branch from ff832b5 to d2048ff Compare April 2, 2026 03:21

vercel bot deployed to Preview April 2, 2026 03:21 View deployment

whoAbhishekSah merged commit 7bb0650 into main Apr 2, 2026
8 checks passed

whoAbhishekSah deleted the feat/set-project-member-role branch April 2, 2026 04:49

coderabbitai bot mentioned this pull request Apr 2, 2026

feat: implement RemoveProjectMember RPC #1504

Merged

2 tasks

Conversation

whoAbhishekSah commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

vercel bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 23882133908

Details

💛 - Coveralls

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

whoAbhishekSah commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Test Results — 29/29 PASS

Happy Path — 9/9 PASS

Authorization — 7/7 PASS

Validation — 13/13 PASS

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AmanGIT07 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

whoAbhishekSah commented Mar 27, 2026 •

edited

Loading

vercel bot commented Mar 27, 2026 •

edited

Loading

coderabbitai bot commented Mar 27, 2026 •

edited

Loading

coveralls commented Mar 27, 2026 •

edited

Loading

whoAbhishekSah commented Mar 30, 2026 •

edited

Loading