Skip to content

Fix group issue - #641

Merged
irees merged 2 commits into
mainfrom
group-tenant-fix
May 29, 2026
Merged

Fix group issue#641
irees merged 2 commits into
mainfrom
group-tenant-fix

Conversation

@irees

@irees irees commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

The admin REST API returned 500 Internal Server Error when adding an org or tenant as a member of another object (for example, a tenant's members as a group's viewers). The permission handlers dropped the request body's ref_relation when building the subject, so userset subjects were written as bare objects the authorization model does not allow.

Root cause

All six permission handlers built the subject as authz.NewEntityKey(er.Type, er.Id), discarding er.RefRelation. The FGA model only permits userset subjects for these relations — an org viewer/editor accepts [user, tenant#member], and a feed_version editor/viewer accepts [user, org#viewer, tenant#member] — never a bare tenant:1 or org:39. OpenFGA rejected the write:

Invalid tuple 'org:46#viewer@tenant:1'. Reason: type 'tenant' is not an allowed type restriction for 'org#viewer'

User subjects have no ref relation, so they were unaffected.

Fix

Carry the ref relation into the subject in all six handlers (POST and DELETE for tenants, groups, and feed versions): authz.NewEntityKey(er.Type, er.Id).WithRefRel(er.RefRelation). WithRefRel(0) is a no-op for user subjects, so they are unchanged; org/tenant subjects are now written as org:39#viewer / tenant:1#member, which the model accepts. This is the server-side counterpart to the frontend, which already sends ref_relation.

Tests

Added AddPermission and RemovePermission subtests to TestServer in server/auth/azchecker. They drive the real admin HTTP API through the real Checker against the canned test database and an in-process in-memory OpenFGA server, so the production model validates the tuple each handler builds. Covered:

  • tenant#member added as a group viewer (the reported case)
  • a user added as a group viewer
  • org#viewer added as a feed version editor
  • adding an org as a group viewer is rejected (the model disallows an org#... subject for an org relation)
  • a non-manager (group viewer) is unauthorized to add members
  • an add/remove round-trip

Reverting the fix turns the two userset cases into 500s, confirming the test guards the regression.

Test plan

  • go test ./server/auth/azchecker/ -run TestServer (requires the canned test DB and FGA, as provisioned in CI by testdata/test_setup.sh)
  • Manual: in the admin UI, add a tenant (as member) and a group (as viewer) to a group's editors/viewers, and to a feed version's editors/viewers; confirm each succeeds.

@irees
irees marked this pull request as ready for review May 29, 2026 01:31
Copilot AI review requested due to automatic review settings May 29, 2026 01:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes admin REST API permission mutations so userset subjects preserve ref_relation when adding/removing permissions, preventing OpenFGA model rejections for valid subjects such as tenant#member and org#viewer.

Changes:

  • Preserves EntityRelation.RefRelation in all tenant, group, and feed version permission add/remove handlers.
  • Adds HTTP-level regression tests covering userset permission additions, invalid userset rejection, unauthorized mutation, and add/remove round-trip behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
server/auth/adminapi/server.go Carries ref_relation into permission subject EntityKeys for all permission mutation routes.
server/auth/azchecker/server_test.go Adds regression coverage for add/remove permission API behavior with userset subjects.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@irees
irees merged commit 382ed0b into main May 29, 2026
8 checks passed
@irees
irees deleted the group-tenant-fix branch May 29, 2026 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants