Skip to content

feat(core): object-scoped permission primitives — Permission.scope object, ObjectRole, Collaborator.role (no enforcement) - #11056

Open
MichaelUray wants to merge 3 commits into
hcengineering:developfrom
MichaelUray:feat/object-permission-primitives
Open

MichaelUray wants to merge 3 commits into
hcengineering:developfrom
MichaelUray:feat/object-permission-primitives

Conversation

@MichaelUray

Copy link
Copy Markdown
Contributor

Context

First step of the unified permission system discussed in #10966 (sketch: #10966 (comment), proposal: #10966 (comment), agreed scope: #10966 (comment) / #10966 (comment)).

This PR adds only the four agreed model primitives. It contains declarations only: there is no enforcement, migration or UI change.

Model changes

  1. Permission.scope accepts 'object' (foundations/core/packages/core/src/classes.ts and TPermission in models/core/src/security.ts). It marks a permission that is granted on a single document instead of through a space role.
  2. New class ObjectRole (DOMAIN_MODEL, core.class.ObjectRole, TObjectRole registered in models/core). It is a named set of object-scoped permissions for one class, and apps declare it. It reuses the core.string.Role label, like TRole.
    export interface ObjectRole extends Doc {
      name: IntlString
      description?: IntlString
      objectClass: Ref<Class<Doc>>
      permissions: Ref<Permission>[]
    }
  3. Collaborator.role?: Ref<ObjectRole>. When it is undefined, a collaborator works as it does today (fields, notifications, mentions). On Postgres the field goes to the JSONB data column, because collaboratorSchema has fixed columns only for attachedTo, attachedToClass and collaborator. So there is no ALTER TABLE and no migration.
  4. Tracker declares five Issue permissions in models/tracker/src/permissions.ts: ReadIssue, CommentOnIssue, EditIssue, TransitionIssue and DeleteIssue. Each is scope: 'object' with objectClass: tracker.class.Issue. Labels and descriptions are added in all 14 tracker locales. The non-English strings are short machine-assisted translations, so review by native speakers is welcome.
    builder.createDoc(core.class.Permission, core.space.Model, {
      label: tracker.string.EditIssuePermission,
      description: tracker.string.EditIssuePermissionDescription,
      scope: 'object',
      objectClass: tracker.class.Issue
    }, tracker.permission.EditIssue)

The PR creates no ObjectRole instances. Below is an example of what Tracker would declare in the enforcement follow-up. It is not part of this PR:

builder.createDoc(core.class.ObjectRole, core.space.Model, {
  name: tracker.string.IssueCommenter,
  objectClass: tracker.class.Issue,
  permissions: [tracker.permission.ReadIssue, tracker.permission.CommentOnIssue]
}, tracker.objectRole.IssueCommenter)

No behaviour change: why, and how it is verified

  • No enforcement path reads Permission.scope. Only two UI queries read it, and both exclude 'object':
    • plugins/setting-resources/src/components/spaceTypes/RoleEditor.svelte:48 queries { scope: 'workspace' }.
    • plugins/card-resources/src/components/settings/EditRole.svelte:65 queries { scope: 'space', ... }.
  • The new IDs are not added to availablePermissions or to any ModulePermissionGroup, so they do not appear in the role or guest editors.
  • The declarations have no txClass, txMatch or forbid on purpose. In restricted spaces, SpacePermissionsMiddleware.checkPermission treats every Permission with a matching objectClass and txClass as a restriction for users without a role (fallback after the role loop). Without txClass, isTxClassMatched never matches. This includes its TxMixin branch, which compares against TxUpdateDoc. Write-path matching will come together with enforcement.
  • Tests:
    • models/core: ObjectRole is built into the hierarchy as a Doc-derived DOMAIN_MODEL class, and permissions is typed ArrOf(Ref<Permission>).

    • models/tracker: each of the five permissions is declared exactly once with scope: 'object', objectClass: Issue and no txClass/txMatch/forbid. This test fails if any of those fields is added to a declaration. No ObjectRole instances are created, and ForbidCreateProject is unchanged.

    • server/middleware: a regression matrix for SpacePermissionsMiddleware covers:

      • TxCreateDoc, TxUpdateDoc, TxRemoveDoc and TxMixin (non-empty attributes)
      • a user with no role, and a user whose space role references EditIssue
      • restricted and unrestricted spaces

      The allow/deny results with the five declarations are identical to the results without them, and the baseline is pinned to current behaviour. A negative control shows that the comparison detects a change: the same declarations with txClass: TxCreateDoc change exactly one cell (restricted space, user with no role, TxCreateDoc goes from allow to deny). Not every txClass would show up in this matrix. With TxUpdateDoc, for example, the fixture's existing update permission already decides those cases. The tracker test above is the guard against adding any txClass/txMatch/forbid.

    • server/postgres: isDataField(DOMAIN_COLLABORATOR, 'role') === true.

    • rush bundle --to @hcengineering/model-all + rush show-model: the model contains core:class:ObjectRole (plus its permissions attribute) and tracker:permission:{Read,CommentOn,Edit,Transition,Delete}Issue. It contains no ObjectRole documents.

  • No migration and no model-version bump. The new attributes are optional, and the new classes and permission documents live in DOMAIN_MODEL and arrive with the normal model upgrade. MODEL_VERSION is injected at bundle time from common/scripts/version.txt. Comparable additive model changes did not bump it.

Design notes for the next steps

  • Invariant: read access is a SQL-compilable existence check (a collaborator record exists on the document). Per-action granularity (ObjectRole.permissions) applies on the write path.
  • The object grant is stored on Collaborator.role, so there is no parallel grant document. It is an exception layer under the space scope of the groups → app → spaces model: space roles stay authoritative, and object roles grant access to individual documents.
  • ObjectRole is the app-declared permission set per class (point 4 of the sketch). An admin flow can list it by objectClass without custom code per app.

Follow-ups (separate PRs)

  • Enforcement:
    • write guard using txMatch on the issue permissions, plus an anchor resolver for comments and attachments
    • read predicate in addSecurity() for collaborators with a role
    • guard for writes to Collaborator
  • ObjectRole instances for issues (Viewer / Commenter / Editor)
  • Admin UI for object grants
  • Grant provenance
  • Default role for mention grants (Mention-grants: Collaborator-based cross-space access for @-mentions #10895)

Questions

  1. ObjectRole shape: should the role be bound to a class (objectClass), or carry only permissions? Should the issue roles (Viewer / Commenter / Editor) go into this PR after all? They are 3 model documents plus labels.
  2. Granularity: is Read / Comment / Edit / Transition / Delete right? ReadIssue is used only to compose roles, because readability follows the existence of the grant. Should it still be declared?
  3. Guest groups: should object-scoped permissions become selectable in ModulePermissionGroup.permissions later, or stay separate?
  4. txClass omitted: is it OK to keep the declarations without txClass until enforcement? The alternative is to set it now and exclude scope === 'object' in the restricted-space fallback, but that would change the middleware.
  5. Label: is reusing core.string.Role for ObjectRole fine, or do you want a dedicated core.string.ObjectRole? That would add 14 core locale entries.
  6. Mention-grants: Collaborator-based cross-space access for @-mentions #10895 (mention grants): should it wait for these primitives? It creates Collaborator records without a role and remains compatible either way.

…jectRole class, Collaborator.role

First step of the unified permission model discussed in hcengineering#10966. Model
primitives only - no enforcement path reads them yet:

- Permission.scope accepts 'object' (core interface and TPermission)
- new ObjectRole class (DOMAIN_MODEL): a named, app-declared set of
  object-scoped permissions for documents of a given class; reuses the
  core.string.Role label like TRole
- optional Collaborator.role: Ref<ObjectRole>; undefined keeps today's
  structural collaborator semantics. Stored in the JSONB data column on
  Postgres, so no schema change or migration is needed.

Tests: model-core builds the class into the hierarchy; postgres
isDataField routes Collaborator.role to the data column.

Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
Declare five object-scoped Issue permissions - ReadIssue, CommentOnIssue,
EditIssue, TransitionIssue, DeleteIssue - with scope 'object' and
objectClass tracker.class.Issue, plus labels and descriptions in all 14
tracker locales.

The declarations intentionally carry no txClass, txMatch or forbid:
SpacePermissionsMiddleware treats any Permission with a matching
objectClass and txClass as a restriction in restricted spaces, so these
declarations cannot affect existing access decisions. They are not added
to the project type's availablePermissions or to any
ModulePermissionGroup, so they do not appear in the role editors either.
Write-path matching and ObjectRole instances follow with enforcement.

Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
…sions without txClass never match

Evaluate SpacePermissionsMiddleware over a matrix of TxCreateDoc,
TxUpdateDoc, TxRemoveDoc and TxMixin (non-empty attributes) for a user
without a space role and a user whose role references an object-scoped
permission, in restricted and unrestricted spaces. The decisions against
a model with the object-scoped declarations (scope 'object', no
txClass/txMatch/forbid) must equal the decisions without them, and the
baseline is pinned to the current behaviour.

Covers both the restricted-space fallback and the role path, including
the TxMixin branch of isTxClassMatched.

Negative control: the same declarations with txClass TxCreateDoc must
make the matrix diverge (restricted space, user without a role: create
flips from allow to deny), so the comparison demonstrably detects a
behaviour-changing declaration.

Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

1 participant