refactor: casl factory - attachments

[HayenNico]

Description

Subsection of PR #2748 for attchments. PR depends on #2759 to be merged first.

This unifies the attachmentEndpointAccess and attachmentInstanceAccess functions in CaslAbilityFactory, removes instance-level Action elements and adjusts the affected controller to accommodate the change. The attachment-specific code is extracted into a separate module.

Changes:

• Replace CaslAbilityFactory.attachmentInstanceAccess and CaslAbilityFactory.attachmentEndpointAccess with one function CaslAbilityFactory.attachmentAccess
• Code for CaslAbilityFactory.attachmentAccessis factored out into new module AttachmentAbility
• Remove all instance-level attachment Action elements, rename endpoint-level actions to cover both
• Adjust endpoint and instance auth logic in attachment controller

Tests included

• Included for each change/fix?
• Passing?

Documentation

• swagger documentation updated (required for API changes)
• official documentation updated

Summary by Sourcery

Unify attachment authorization into a single CASL ability and simplify attachment actions used across the API.

Enhancements:

• Replace separate attachment endpoint and instance accessors with a unified attachmentAccess ability in CaslAbilityFactory.
• Consolidate attachment endpoint and instance actions into a single set of attachment CRUD actions in the Action enum.
• Update the attachments v4 controller to rely on the unified attachmentAccess ability and new attachment actions for all route and instance-level permission checks.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In attachmentAccess, the AttachmentRead rule for published attachments is granted twice (before and after the unauthenticated branch); consider keeping this in a single place to avoid confusion about which rules apply to anonymous vs authenticated users.
• The repeated user.currentGroups.some((g) => this.accessGroups?.<group>.includes(g)) checks in attachmentAccess could be extracted into a small helper (e.g. hasGroup(user, this.accessGroups.attachment)) to reduce duplication and make the group-based branching easier to read and maintain.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `attachmentAccess`, the `AttachmentRead` rule for published attachments is granted twice (before and after the unauthenticated branch); consider keeping this in a single place to avoid confusion about which rules apply to anonymous vs authenticated users.
- The repeated `user.currentGroups.some((g) => this.accessGroups?.<group>.includes(g))` checks in `attachmentAccess` could be extracted into a small helper (e.g. `hasGroup(user, this.accessGroups.attachment)`) to reduce duplication and make the group-based branching easier to read and maintain.

## Individual Comments

### Comment 1
<location path="src/attachments/attachments.v4.controller.ts" line_range="208-209" />
<code_context>
   // GET /attachments
   @UseGuards(PoliciesGuard)
   @CheckPolicies("attachments", (ability: AppAbility) =>
-    ability.can(Action.AttachmentReadEndpoint, Attachment),
+    ability.can(Action.AttachmentRead, Attachment),
   )
   @ApiOperation({
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `AttachmentRead` with a class subject while the rule has instance-level conditions may cause unexpected guard behavior for unauthenticated requests.

In `attachmentAccess`, unauthenticated users are granted `can(Action.AttachmentRead, Attachment, { isPublished: true })`, but `CheckPolicies` is now calling `ability.can(Action.AttachmentRead, Attachment)` with the *class* as subject. Depending on how `PoliciesGuard`/CASL treat class subjects, the `isPublished` condition may never be applied, which could either unintentionally block previously allowed unauthenticated reads or, worse, bypass the condition and allow more than intended. To avoid changing behavior, either:

- Add an unconditional `can(Action.AttachmentRead, Attachment)` rule specifically for endpoint-level checks, and keep `{ isPublished: true }` for instance-level filtering, or
- Update `PoliciesGuard` to evaluate this permission against a representative `Attachment` instance instead of the class.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Using AttachmentRead with a class subject while the rule has instance-level conditions may cause unexpected guard behavior for unauthenticated requests.

In attachmentAccess, unauthenticated users are granted can(Action.AttachmentRead, Attachment, { isPublished: true }), but CheckPolicies is now calling ability.can(Action.AttachmentRead, Attachment) with the class as subject. Depending on how PoliciesGuard/CASL treat class subjects, the isPublished condition may never be applied, which could either unintentionally block previously allowed unauthenticated reads or, worse, bypass the condition and allow more than intended. To avoid changing behavior, either:

• Add an unconditional can(Action.AttachmentRead, Attachment) rule specifically for endpoint-level checks, and keep { isPublished: true } for instance-level filtering, or
• Update PoliciesGuard to evaluate this permission against a representative Attachment instance instead of the class.