refactor(api): require resource in CheckResourcePermission by AmanGIT07 · Pull Request #1886 · raystack/frontier

AmanGIT07 · 2026-08-14T10:12:42Z

Summary

CheckResourcePermission accepts the object only via the resource field (namespace:id). The deprecated object_id/object_namespace request fields are no longer read; requests sending only those fields now receive InvalidArgument. Namespace aliases keep working inside resource. Part of #1782.

Changes

internal/api/v1beta1connect/permission_check.go: remove the split-field fallback; reject a missing or malformed resource
test/e2e/regression/onboarding_test.go, serviceusers_test.go, api_test.go: check requests send resource
internal/api/v1beta1connect/permission_check_test.go: success cases send resource; add missing-resource case

Test Plan

go test ./internal/api/v1beta1connect/ passes
make lint passes (0 issues)
Onboarding, service-users, and API e2e regression suites pass

🤖 Generated with Claude Code

CheckResourcePermission reads the object only from the resource field ("namespace:id") and returns InvalidArgument when it is missing or malformed. The deprecated object_id/object_namespace request fields are no longer read. E2E tests send the resource form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vercel · 2026-08-14T10:12:43Z

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

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview	Aug 24, 2026 8:05am

coderabbitai · 2026-08-14T10:13:13Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ca87a44-dfcd-47f1-b932-41f0ec1d58dc

📥 Commits

Reviewing files that changed from the base of the PR and between 27ee7b2 and f00e7d3.

📒 Files selected for processing (1)

internal/api/v1beta1connect/permission_check_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Permission checks now require a valid, namespace-qualified resource identifier.
- Requests missing or containing an invalid resource identifier return a clear invalid-argument error.
- Updated permission checks across onboarding, relation, and service-user workflows to use the unified resource format.

Walkthrough

Permission handlers now require namespace-qualified values in Resource. They reject malformed, missing, or incomplete identifiers with InvalidArgument and ErrNamespaceSplitNotation. Unit and end-to-end tests now use the combined field.

Changes

Permission resource identifier migration

Layer / File(s)	Summary
Resource validation and unit coverage `internal/api/v1beta1connect/permission_check.go`, `internal/api/v1beta1connect/permission_check_test.go`	Permission handlers require valid combined `Resource` identifiers and reject deprecated split fields. Tests cover single and batch permission checks.
End-to-end permission request migration `test/e2e/regression/api_test.go`, `test/e2e/regression/onboarding_test.go`, `test/e2e/regression/serviceusers_test.go`	Regression and onboarding permission checks now use namespace-qualified values in `Resource` instead of separate object fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f00e7

The API now requires the documented resource format and updates affected callers and tests; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: whoabhisheksah

🚥 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.

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.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a381cba-390a-4d76-b8c0-f350fcbc32c1

📥 Commits

Reviewing files that changed from the base of the PR and between 7c10c55 and 7105710.

📒 Files selected for processing (5)

internal/api/v1beta1connect/permission_check.go
internal/api/v1beta1connect/permission_check_test.go
test/e2e/regression/api_test.go
test/e2e/regression/onboarding_test.go
test/e2e/regression/serviceusers_test.go

coveralls · 2026-08-14T10:19:37Z

Coverage Report for CI Build 32704499925

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.2%) to 48.897%

Details

Coverage increased (+0.2%) from the base build.
Patch coverage: 2 uncovered changes across 1 file (4 of 6 lines covered, 66.67%).
225 coverage regressions across 10 files.

Uncovered Changes

File	Changed	Covered	%
internal/api/v1beta1connect/permission_check.go	6	4	66.67%

Coverage Regressions

225 previously-covered lines in 10 files lost coverage.

File	Lines Losing Coverage	Coverage
billing/product/service.go	110	68.31%
internal/api/v1beta1connect/permission.go	37	61.22%
cmd/permission.go	17	45.58%
cmd/seed.go	16	7.75%
internal/reconcile/permission_reconciler.go	12	80.43%
internal/bootstrap/schema/schema.go	10	40.88%
internal/reconcile/platformuser_reconciler.go	9	83.7%
internal/reconcile/reconcile.go	9	89.58%
internal/reconcile/billingproduct.go	4	98.21%
internal/reconcile/permission.go	1	98.36%

Coverage Stats


Relevant Lines:	40029
Covered Lines:	19573
Line Coverage:	48.9%
Coverage Strength:	15.76 hits per line

💛 - Coveralls

BatchCheckPermission rejects a resource with an empty namespace or id as InvalidArgument instead of failing at SpiceDB as internal. All three resource guards in the file, and the federated subject guard, return the namespace-notation error so callers see the expected format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 49074c84-636d-440d-a48b-0457f5e47b3a

📥 Commits

Reviewing files that changed from the base of the PR and between 7105710 and 27ee7b2.

📒 Files selected for processing (2)

internal/api/v1beta1connect/permission_check.go
internal/api/v1beta1connect/permission_check_test.go

Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

rohilsurana

Review against main. The change itself is a clean tightening. A couple of behavior and test notes below.

rohilsurana · 2026-08-20T08:55:27Z

 		objectNamespace, objectID, err := schema.SplitNamespaceAndResourceID(body.GetResource())
-		if len(body.GetResource()) == 0 || err != nil {
-			return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest)
+		if err != nil || objectNamespace == "" || objectID == "" {


One malformed or empty-part resource in any body now makes the whole batch return InvalidArgument with zero results. For a batch endpoint that is surprising: 49 valid checks are dropped because of 1 bad body. Consider failing just that item, or returning a per-item error, rather than the whole call.

Keeping whole-batch rejection in this PR: that's the pre-existing contract for a malformed body here — before this change a malformed resource already failed the whole call, and an empty-part one reached SpiceDB and failed the whole call as internal. This PR only turns that into a clean InvalidArgument. Failing just the bad item needs a response-shape change (BatchCheckPermissionResponsePair has no per-item error field), so it's a proto addition rather than a handler tweak. Happy to take that as a follow-up if we want the per-item contract.

rohilsurana · 2026-08-20T08:55:27Z

-	}
-	if objectNamespace == "" || objectID == "" {
-		return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest)
+	if err != nil || objectNamespace == "" || objectID == "" {


This drops the fallback to object_id / object_namespace, so a client sending only those (with no resource) now gets InvalidArgument. That is the intended tightening, but those fields are still in the .proto, so the schema still advertises support the server no longer provides. A clear deprecation note in the proto, or a planned removal, would keep integrations from being surprised at runtime.

Proto will be cleaned up later

Adding to that: the fields already carry deprecated = true in the proto, so the schema does flag it — the generated getters are marked deprecated too. The removal itself rides a later proton sync.

rohilsurana · 2026-08-20T08:55:27Z

-				ObjectNamespace: testRelationV2.Object.Namespace,
-				Permission:      schema.UpdatePermission,
+				Permission: schema.UpdatePermission,
+				Resource:   schema.JoinNamespaceAndResourceID(testRelationV2.Object.Namespace, testRelationV2.Object.ID),


The success cases use canonical namespaces via JoinNamespaceAndResourceID, and none pass a real alias in the resource field. The PR says aliases still work there, so a case with resource set to an alias (e.g. "org:") would lock that promise in and catch any regression in ParseNamespaceAliasIfRequired for this path.

Added in f00e7d3 — a unit case now sends org:<id> and asserts the check runs against app/organization.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vercel Bot deployed to Preview August 14, 2026 10:13 View deployment

coderabbitai Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread internal/api/v1beta1connect/permission_check_test.go

coderabbitai Bot mentioned this pull request Aug 14, 2026

refactor(api): require permission key in create and update requests #1885

Merged

3 tasks

vercel Bot deployed to Preview August 17, 2026 06:44 View deployment

coderabbitai Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread internal/api/v1beta1connect/permission_check_test.go

test(api): cover the empty namespace part in batch check bodies

e5241f5

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vercel Bot deployed to Preview August 17, 2026 16:18 View deployment

AmanGIT07 requested review from rohilsurana and whoAbhishekSah August 18, 2026 05:51

rohilsurana reviewed Aug 20, 2026

View reviewed changes

test(api): cover namespace alias resolution and inject batch test mocks

f00e7d3

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vercel Bot deployed to Preview August 24, 2026 08:05 View deployment

Uh oh!

Conversation

AmanGIT07 commented Aug 14, 2026

Summary

Changes

Test Plan

Uh oh!

vercel Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Coverage Report for CI Build 32704499925

Coverage increased (+0.2%) to 48.897%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rohilsurana left a comment

Choose a reason for hiding this comment

Uh oh!

rohilsurana Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

AmanGIT07 Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

rohilsurana Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

AmanGIT07 Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

AmanGIT07 Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

rohilsurana Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

AmanGIT07 Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel Bot commented Aug 14, 2026 •

edited

Loading

coderabbitai Bot commented Aug 14, 2026 •

edited

Loading

coveralls commented Aug 14, 2026 •

edited

Loading