Project-listing handlers: duplicated access_pairs and 500s on invalid input

[AmanGIT07]

Summary

Three pre-existing issues in the project-listing handlers surfaced while testing the ListByUser → List(Filter{Principal}) migration. They're independent of the migration but live on the same handlers and have a consistent shape: invalid input maps to Internal instead of InvalidArgument, and the with_permissions access-pair output is malformed.

1. access_pairs is duplicated when more than one permission is requested. The handlers emit one entry per CheckPair instead of grouping per resource — N projects × M permissions → N*M entries, each project repeated M times.
2. Unknown permission name returns 5xx/404 instead of an empty access_pairs block alongside the projects list. The two RPCs even disagree on the code (Internal vs NotFound).
3. Malformed / empty id returns Internal instead of InvalidArgument on ListProjectsByUser and ListServiceUserProjects.

1. access_pairs duplication

Reproduction

Authenticate as a user with at least one accessible project, then:

curl -s -X POST "http://localhost:8002/raystack.frontier.v1beta1.FrontierService/ListProjectsByCurrentUser" \
  -H "content-type: application/json" -H "connect-protocol-version: 1" \
  -H "Cookie: sid=<sid>" \
  -d '{"orgId":"<org>","withPermissions":["get","delete"]}'

For 3 visible projects the response carries 6 access_pairs entries — each project_id appears twice, each entry redundantly listing ["get","delete"].

Root cause

internal/api/v1beta1connect/user.go:949-964 (ListProjectsByCurrentUser) and internal/api/v1beta1connect/serviceuser.go:501-516 (ListServiceUserProjects) iterate over every CheckPair returned by fetchAccessPairsOnResource, then for each one re-filter the full pair slice by resID and append an AccessPair{ProjectId: resID, Permissions: <all perms for that resID>}. With M permissions, every resource is visited M times and a fully populated AccessPair is emitted on each visit.

Suggested fix

Group successCheckPairs by Object.ID once, then emit a single AccessPair per group:

byResource := map[string][]string{}
order := []string{}
for _, cp := range successCheckPairs {
    rid := cp.Relation.Object.ID
    if _, ok := byResource[rid]; !ok {
        order = append(order, rid)
    }
    byResource[rid] = append(byResource[rid], cp.Relation.RelationName)
}
for _, rid := range order {
    accessPairsPb = append(accessPairsPb, &frontierv1beta1.ListProjectsByCurrentUserResponse_AccessPair{
        ProjectId:   rid,
        Permissions: byResource[rid],
    })
}

Same shape for the ListServiceUserProjectsResponse_AccessPair type.

2. Unknown permission name → 5xx / 404

Reproduction

curl -s -X POST ".../ListProjectsByCurrentUser" \
  -H "Cookie: sid=<sid>" \
  -d '{"orgId":"<org>","withPermissions":["bogus_perm"]}'
# {"code":"internal","message":"internal server error"}

curl -s -X POST ".../ListServiceUserProjects" \
  -H "Cookie: sid=<sa-sid>" \
  -d '{"id":"<su>","orgId":"<org>","withPermissions":["bogus_perm"]}'
# {"code":"not_found","message":"not found"}

Root cause

fetchAccessPairsOnResource at internal/api/v1beta1connect/permission_check.go:96-112 calls getPermissionName per (resource, permission) pair. getPermissionName at permission_check.go:30-46 translates permission.ErrNotExist into connect.CodeNotFound.

• ListServiceUserProjects at serviceuser.go:499 does return nil, err — the NotFound propagates unchanged.
• ListProjectsByCurrentUser at user.go:947 wraps everything into connect.CodeInternal — so the same condition surfaces as internal.

Neither response is right. The caller asked: "for each of these projects, which of these permissions does the principal hold?" The answer for an unknown permission is "none", not "the request blew up."

Suggested fix

Filter unknown permission names out inside fetchAccessPairsOnResource (or earlier in the handler) and return an empty access_pairs set for them rather than erroring. Concretely: treat permission.ErrNotExist as "drop this permission" instead of return nil, err. Keep the Internal path only for genuine repository failures.

Optional: if the desire is to be strict and reject unknown perms, do so explicitly with InvalidArgument + a message naming the offending permission — and align both handlers on the same code.

3. Malformed / empty id → 5xx instead of 400

Reproduction

# ListProjectsByUser, garbage uuid
curl -s -X POST ".../ListProjectsByUser" -H "Cookie: sid=<admin-sid>" \
  -d '{"id":"not-a-uuid"}'
# {"code":"internal","message":"internal server error"}

# ListServiceUserProjects, empty id but valid org_id
curl -s -X POST ".../ListServiceUserProjects" -H "Cookie: sid=<admin-sid>" \
  -d '{"id":"","orgId":"<org>"}'
# {"code":"internal","message":"internal server error"}

Root cause

• ListProjectsByUser handler (internal/api/v1beta1connect/user.go:864-892) maps user.ErrInvalidUUID to InvalidArgument, but project.Service.List no longer surfaces that error. With the new Filter{Principal: {ID: userID}} path, the malformed UUID flows down to policyService.List and fails at the SQL layer as a generic error → the handler's default branch returns CodeInternal.
• ListServiceUserProjects empty id is rejected by the authorization interceptor at pkg/server/connect_interceptors/authorization.go:274-286, which calls GetServiceUser with the empty id. That returns an error which is propagated as Internal rather than InvalidArgument.

id has no min_len/uuid validation in the proto (ListProjectsByUserRequest.id, ListServiceUserProjectsRequest.id). The handler is the only line of defence and it doesn't validate before the call chain.

Suggested fix

Three options, any one of which is enough:

1. Add (buf.validate.field).string.uuid = true (and REQUIRED) on id in both request protos. This makes the response shape correct without any handler change.
2. Validate id at the top of each handler with utils.IsValidUUID(id) and return CodeInvalidArgument early.
3. In project.Service.List, when Filter.Principal != nil, reject empty or non-UUID Principal.ID with a typed error the handlers can map.

Option 1 is the smallest change and makes the behaviour consistent across both RPCs.

Scope notes

• All three issues are independent of the ListByUser migration. They became more visible because that change consolidated the inheritance + group + direct paths onto the same handler surface.