Skip to content

feat: introduce the genric Actor entity - #65987

Open
salmart-dev wants to merge 9 commits into
masterfrom
feat/ncu-actor
Open

salmart-dev wants to merge 9 commits into
masterfrom
feat/ncu-actor

Conversation

@salmart-dev

@salmart-dev salmart-dev commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Introduces an Actor data object to pass around the information of who is performing an action / request, and include information about "privilege escalation" or delegation, so that audit or compliance apps can log a chain, and apps can take decisions losing less context and avoiding mix ups like $userId = null equalling to "all privileges" or "no privileges" depending on the call, which are completely different answers.

The DelegationKind and PrincipalType could be extended in the future as "stable" cases become more prominent.

Example: SystemTagManager

I took the example of the SystemTagManager, where I stumbled on the pattern first-hand.

Before

  public function getTagsByIds($tagIds, ?IUser $user = null): array {
          // ...
          while ($row = $result->fetchAssociative()) {
                  $tag = $this->createSystemTagFromRow($row);
                  if ($user && !$this->canUserSeeTag($tag, $user)) { // null: no filtering
                          continue;
                  }
                  $tags[$row['id']] = $tag;
          }
          // ...
  }

  public function createTag(string $tagName, bool $userVisible, bool $userAssignable, ?IUser $user = null): ISystemTag {
          $user ??= $this->userSession->getUser(); // null: whoever is in the session
          if (!$this->canUserCreateTag($user)) {
                  throw new TagCreationForbiddenException();
          }
          // ...
  }

  public function canUserCreateTag(?IUser $user): bool {
          if ($user === null) {
                  return \OC::$CLI; // null: allowed only if PHP runs from the command line
          }
          // ...
  }

  public function canUserAssignTag(ISystemTag $tag, ?IUser $user): bool {
          if ($user === null) {
                  return false; // null: nobody
          }
          // ...
  }

  public function canUserSeeTag(ISystemTag $tag, ?IUser $user): bool {
          if (!$user && $tag->getAccessLevel() === ISystemTag::ACCESS_LEVEL_PUBLIC) {
                  return true; // null: anonymous visitor
          }
          // ...
  }

After

  public function getTagsByIds($tagIds, Actor $actor): array {
          // ...
          while ($row = $result->fetchAssociative()) {
                  $tag = $this->createSystemTagFromRow($row);
                  if (!$this->canSeeTag($tag, $actor)) {
                          continue;
                  }
                  $tags[$row['id']] = $tag;
          }
          // ...
  }

  public function createTag(string $tagName, bool $userVisible, bool $userAssignable, Actor $actor): ISystemTag {
          if (!$this->canCreateTag($actor)) {
                  throw new TagCreationForbiddenException();
          }
          // ...
  }

  public function canCreateTag(Actor $actor): bool {
          $principal = $actor->principal;
          return match ($principal->type) {
                  PrincipalType::System => true,
                  PrincipalType::User => !$this->appConfig->getValueBool('systemtags', 'restrict_creation_to_admin', false)
                          || $this->groupManager->isAdmin($principal->id),
                  default => false,
          };
  }

  public function canAssignTag(ISystemTag $tag, Actor $actor): bool {
          $principal = $actor->principal;
          return match ($principal->type) {
                  PrincipalType::System => true,
                  PrincipalType::User => $this->canUserAssignTag($tag, $principal->id),
                  default => false,
          };
  }

  public function canSeeTag(ISystemTag $tag, Actor $actor): bool {
          $principal = $actor->principal;
          return match ($principal->type) {
                  PrincipalType::System => true,
                  PrincipalType::User => $tag->isUserVisible() || $this->groupManager->isAdmin($principal->id),
                  PrincipalType::Guest => $tag->getAccessLevel() === ISystemTag::ACCESS_LEVEL_PUBLIC,
                  default => false,
          };
  }

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@salmart-dev salmart-dev added this to the Nextcloud 36 milestone Oct 28, 2026
@salmart-dev salmart-dev added the 3. to review Waiting for reviews label Oct 28, 2026
@salmart-dev salmart-dev self-assigned this Oct 28, 2026
@salmart-dev
salmart-dev marked this pull request as ready for review October 28, 2026 17:40
@salmart-dev
salmart-dev requested a review from a team as a code owner October 28, 2026 17:40
@salmart-dev
salmart-dev requested review from icewind1991, leftybournes, provokateurin and sorbaugh and removed request for a team October 28, 2026 17:40

@provokateurin provokateurin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Despite the amount of comments, this looks really good!

Comment thread lib/unstable/Actor/Actor.php Outdated
* bot "summarizer" for alice: principal=alice, delegate=Actor(bot), via=OnBehalfOf
* version expiry in alice's files: principal=alice, delegate=Actor(system), via=OnBehalfOf
* admin impersonating alice: principal=alice, delegate=Actor(admin), via=Impersonation
* alice's action escalated: principal=system, delegate=Actor(alice), via=SystemElevation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you give a better example? Would this be the case if alice is an admin and would do something only admins can do?

Comment thread lib/unstable/Actor/Actor.php Resolved
Comment thread lib/unstable/Actor/Actor.php Resolved
Comment thread lib/unstable/Actor/Actor.php Resolved
Comment thread lib/unstable/Actor/Actor.php Resolved
Comment thread lib/unstable/Actor/Principal.php Resolved
Comment thread lib/unstable/Actor/Principal.php Resolved
Comment thread lib/unstable/Actor/PrincipalType.php Resolved
Comment thread lib/unstable/Actor/PrincipalType.php Resolved
/**
* @experimental 36.0.0
*/
public function isHuman(): bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if this is a good idea 😅 Maybe it can be removed for now and if there is code that we duplicate to check this, we can add it back?

Comment thread lib/unstable/Actor/Actor.php Resolved
salmart-dev and others added 4 commits November 2, 2026 14:47
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux changed the title feat: introduce Actor data object in NCU feat: introduce the genric Actor entity Nov 2, 2026
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
susnux requested review from GhassenKefi and provokateurin and removed request for sorbaugh November 2, 2026 15:53

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants