fix(api): dedupe access_pairs and map invalid-id/unknown-perm to 4xx

[AmanGIT07]

Summary

Three independent bugs in the project / service-user / current-user-groups listing handlers, and a bundled fix for the same access_pairs duplication in ListCurrentUserGroups.

Fixes #1647.

1. access_pairs duplicated per resource

ListProjectsByCurrentUser, ListServiceUserProjects, and ListCurrentUserGroups emitted one AccessPair per (resource, permission) CheckPair and inside each iteration filtered the full slice for the same resource id. With M permissions per resource that produced M copies of the same row.

Fix: group successCheckPairs by resource id once and emit one entry per resource.

2. Unknown permission name returned 5xx / 404

fetchAccessPairsOnResource called getPermissionName per (resource, permission) pair, which mapped permission.ErrNotExist to CodeNotFound. ListProjectsByCurrentUser then re-wrapped everything as Internal; ListServiceUserProjects passed NotFound through. Both responses were wrong: the caller asked which of these permissions the principal holds; the answer for an unknown permission is "none".

Fix: new resolvePermissionName helper returns (name, ok, err). fetchAccessPairsOnResource resolves each requested permission once, drops unknown names and duplicate inputs, and reserves Internal for genuine repository failures. getPermissionName is kept as a thin wrapper for callers that should reject unknown permissions (CheckResourcePermission, CheckFederatedResourcePermission, BatchCheckPermission).

3. Empty / malformed id returned 5xx instead of 400

After the ListByUser → List(Filter{Principal}) migration, project.Service.List returned an untyped fmt.Errorf for invalid principals; non-UUID ids flowed to the SQL layer and surfaced as generic DB errors. Both produced Internal in the handlers. ListServiceUserProjects had the same shape via the interceptor's GetServiceUser call.

Fix:

• project.Service.List returns project.ErrInvalidUUID for empty/non-UUID Principal.ID and project.ErrInvalidPrincipalType for empty Principal.Type.
• serviceuser.Service.Get rejects non-UUID ids with serviceuser.ErrInvalidID (consistent with user.Service.Enable/Disable). The GetServiceUser handler maps ErrInvalidID to InvalidArgument, which propagates through the authorization interceptor.
• ListProjectsByUser, ListServiceUserProjects map project.ErrInvalidUUID / project.ErrInvalidPrincipalType to InvalidArgument. The dead user.ErrInvalidUUID case in ListProjectsByUser is removed.

RPCs and status-code changes

RPC	Input	Before	After
ListProjectsByCurrentUser	withPermissions=["update","delete"], N projects	2N duplicate access_pairs	N rows, one per project
ListProjectsByCurrentUser	unknown permission name	500	200, drops that permission
ListServiceUserProjects	unknown permission name	404	200, drops that permission
ListServiceUserProjects	multi-permission	2N duplicate access_pairs	N rows
ListServiceUserProjects	non-UUID id	500	400
ListProjectsByUser	non-UUID id	500	400
GetServiceUser	empty / non-UUID id	500	400
ListCurrentUserGroups	multi-permission	2N duplicate access_pairs	N rows
ListCurrentUserGroups	unknown permission name	500	200, drops that permission

Frontend impact

None. useOrganizationProjects and useOrganizationTeams already collapse the response via acc[id] = permissions (last-write-wins). The pre-fix duplicate rows were identical, so old and new responses produce the same final map. ListServiceUserProjects frontend callers don't read access_pairs.

Test plan

• go test ./core/project/... ./core/serviceuser/... ./internal/api/v1beta1connect/...
• go vet ./...
• Manual verification against staging with the request from the bug report — confirm one access_pairs row per project for withPermissions=["update","delete"]
• Manual verification: ListServiceUserProjects with non-UUID id returns 400
• Manual verification: ListProjectsByCurrentUser with withPermissions=["bogus_perm"] returns 200 with empty access_pairs

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	May 27, 2026 1:12pm

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c04bf5a5-2014-4631-a27b-286a0a1b47c9

📥 Commits

Reviewing files that changed from the base of the PR and between e399d9c and 97a5317.

📒 Files selected for processing (9)

• core/project/service.go
• core/project/service_test.go
• core/serviceuser/service.go
• core/serviceuser/service_test.go
• internal/api/v1beta1connect/permission_check.go
• internal/api/v1beta1connect/serviceuser.go
• internal/api/v1beta1connect/serviceuser_test.go
• internal/api/v1beta1connect/user.go
• internal/api/v1beta1connect/user_test.go

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Enhanced input validation for identifiers, now returning specific error codes for invalid formats.
  • Improved error responses in API endpoints to distinguish between invalid arguments and other failures.
  • Fixed permission handling to deduplicate and filter unknown permissions instead of failing requests; multiple permissions per resource are now properly aggregated.

Walkthrough

This PR tightens input validation across project and service-user services, adds permission resolution to filter unknown permissions, and updates API handler error mapping and access pair aggregation. Validation errors now return specific sentinel types instead of generic messages and map to InvalidArgument in gRPC responses.

Changes

Input Validation and Permission Handling

Layer / File(s)	Summary
Core service input validation core/project/service.go, core/project/service_test.go, core/serviceuser/service.go, core/serviceuser/service_test.go	project.Service.List validates f.Principal.ID as UUID and f.Principal.Type as non-empty, returning ErrInvalidUUID or ErrInvalidPrincipalType respectively. serviceuser.Service.Get validates id as UUID before repository query, returning ErrInvalidID. Tests verify specific error types using errors.Is.
Permission resolution and filtering internal/api/v1beta1connect/permission_check.go	New resolvePermissionName helper resolves permissions and returns a flag indicating existence; getPermissionName maps unknown permissions to connect.CodeNotFound; fetchAccessPairsOnResource resolves permissions up front, deduplicates, filters out unknown ones, and short-circuits when no valid permissions remain.
ServiceUser handler error mapping and access pair aggregation internal/api/v1beta1connect/serviceuser.go, internal/api/v1beta1connect/serviceuser_test.go	GetServiceUser maps ErrInvalidID to InvalidArgument; ListServiceUserProjects maps ErrInvalidUUID and ErrInvalidPrincipalType to InvalidArgument; permission access pairs are aggregated per project with first-seen ordering instead of per-permission filtering. Tests cover error mappings and multi-permission aggregation.
User handler error mapping and access pair aggregation internal/api/v1beta1connect/user.go, internal/api/v1beta1connect/user_test.go	ListCurrentUserGroups and ListProjectsByCurrentUser refactored to group permissions per resource and emit one AccessPair per group/project with aggregated permissions; ListProjectsByUser updated to map validation errors to InvalidArgument; unused relation import removed. New tests validate access pair generation with multiple permissions and unknown permission filtering.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Project-listing handlers: duplicated access_pairs and 500s on invalid input #1647: This PR directly addresses the same validation and error-handling improvements described in the issue: tightened principal UUID/type validation, specific error types, filtering/ignoring unknown permissions, and per-resource access pair aggregation.

Possibly related PRs

• raystack/frontier#1620: Overlaps on error mapping improvements in GetServiceUser within the same handler file.
• raystack/frontier#1648: Both PRs enhance project.Service.List principal-filter validation; this PR further tightens it by validating ID as UUID and Type as non-empty.

Suggested reviewers

• whoAbhishekSah
• 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 26513272720

Coverage increased (+0.2%) to 43.055%

Details

• Coverage increased (+0.2%) from the base build.
• Patch coverage: 13 uncovered changes across 2 files (77 of 90 lines covered, 85.56%).
• 50 coverage regressions across 1 file.

Uncovered Changes

File	Changed	Covered	%
internal/api/v1beta1connect/permission_check.go	37	28	75.68%
internal/api/v1beta1connect/user.go	23	19	82.61%
Total (5 files)	90	77	85.56%

Coverage Regressions

50 previously-covered lines in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
internal/api/v1beta1connect/user.go	50	65.31%

---

Coverage Stats

Relevant Lines:	37884
Covered Lines:	16311
Line Coverage:	43.06%
Coverage Strength:	12.07 hits per line

---

💛 - Coveralls

[AmanGIT07]

Sandbox RPC test results

Exercised the changed RPCs against a local Frontier built from this branch. Scaffold: one org with 3 projects, 1 group, 1 service user (app_project_owner on 2 of 3 projects); a second user with app_project_viewer on 1 project for filter-correctness.

Listed deltas

RPC	Case	Expected	Got
ListProjectsByCurrentUser	[get,update] (multi-perm dedup)	3 pairs each [get,update]	✅
	[bogus] (was 500)	200, 0 pairs	✅
	[get,bogus] (was 500)	3 pairs each [get]	✅
	[get,get] (input dedup)	3 pairs each [get]	✅
ListCurrentUserGroups	[get,update]	1 pair [get,update]	✅
	[bogus] (was 500)	200, 0 pairs	✅
	[get,bogus]	1 pair [get]	✅
	[get,get]	1 pair [get]	✅
ListServiceUserProjects	[get,update]	2 pairs each [get,update]	✅
	[bogus] (was 404)	200, 0 pairs	✅
	[get,bogus]	2 pairs each [get]	✅
	[get,get]	2 pairs each [get]	✅
	non-UUID id (was 500)	400	✅
	empty id	400	✅
ListProjectsByUser	non-UUID id (was 500)	400	✅
	empty id	400	✅
	random valid UUID, no user	200, empty projects	✅
GetServiceUser (direct)	empty id	400	✅
	non-UUID id	400 expected	403 — see note
	valid UUID, not found	404 expected	403 — see note
GetServiceUser (indirect via DeleteServiceUser / ListServiceUserProjects interceptor)	non-UUID id	400	✅
	empty id	400	✅
	valid UUID, not found	404	✅

Note on direct GetServiceUser: the interceptor at pkg/server/connect_interceptors/authorization.go:257 calls IsAuthorized(serviceuser:<id>, manage) directly — it doesn't route through handler.GetServiceUser. So a non-UUID or missing-UUID id never reaches the new ErrInvalidID / ErrNotExist mapping; it falls into IsAuthorized → CheckAuthz → handleAuthErr and returns 403. The handler-level fix is wire-observable through every other interceptor that does call handler.GetServiceUser (DeleteServiceUser, ListServiceUserProjects), both verified above.

Additional behavioral checks

Test	Expected	Got
Permission-filter correctness — viewer-only user asks [get,delete]	1 pair with [get] only (failed delete excluded)	✅
Empty with_permissions across all 3 list RPCs (omit + [])	access_pairs absent from response	✅
Ordering stability — 5 sequential calls	identical order each time; access_pairs order matches projects order	✅
CheckResourcePermission known perm	200 {status: true}	✅
CheckResourcePermission unknown perm	404 (unchanged)	✅
BatchCheckPermission mixed known + unknown	404 (unchanged)	✅
BatchCheckPermission all known	200, one pair per body	✅