WIP: FEATURE: Content Repository Privileges #4185

bwaidelich · 2023-04-12T13:46:31Z

Related: #3732

bwaidelich · 2023-04-12T13:47:22Z

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php

+            $privileges = $this->privilegeProvider->getPrivileges($visibilityConstraints);
+            if (!$privileges->isContentStreamAllowed($contentStreamId)) {
+                throw new \RuntimeException(sprintf('No access to content stream "%s"', $contentStreamId->value), 1681306937);
+            }


does it make sense to check the privileges here already?

I would say yes (if we can, like it is done here). Ofc people could work around this if they manually instanciated ContentSubgraph; but we could never prevent this.

rename to sth like $this->privilegeProvider->isFetchingOfNodesAllowedInSubgraph($contentStreamId, $dimensionSpacePoint) (consider using result object for more context)

bwaidelich · 2023-04-12T13:48:05Z

Neos.ContentRepository.Core/Classes/SharedModel/Privilege/Privileges.php

+/**
+ * @api
+ */
+final class Privileges


This is missing the "restriction edge attributes" still

(see #4557 -> but they will be called node tags as discussed)

Rename to sth like ReadPrivileges

bwaidelich · 2023-04-12T13:49:33Z

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php

+        if (!$privilegeProviderFactory instanceof PrivilegeProviderFactoryInterface) {
+            throw InvalidConfigurationException::fromMessage('privilegeProvider.factoryObjectName for content repository "%s" is not an instance of %s but %s.', $contentRepositoryId->value, PrivilegeProviderFactoryInterface::class, get_debug_type($privilegeProviderFactory));
+        }
+        return $privilegeProviderFactory->build($contentRepositoryId, $contentRepositorySettings['userIdProvider']['options'] ?? [], $userIdProvider, $contentRepositoryRegistry);


The PrivilegeProvider currently has a dependency to...

the UserIdProvider to load policies of the authenticated user

the ContentRepositoryRegistry to retrieve ContentStreams for the user (via WorkspaceFinder)..

This is nasty

UserIdProvider is fine IMHO, ContentRepositoryRegistry should go away. What about making the $contentRepository available to the provider at runtime? would IMHO be easier and more consistent.

@skurfuerst can you clarify what you mean by "available [...] at runtime"? A parameter of the PrivilegeProvider::getPrivileges() method? But than the ContentGraph would need to pass it

yep, that's what I thought - similar to what we did in the command handlers. But you are right, the projection does not know it yet...

bwaidelich · 2023-04-12T13:50:25Z

Neos.ContentRepositoryRegistry/Classes/Factory/PrivilegeProvider/FakePrivilegeProvider.php

+/**
+ * @internal
+ */
+final class FakePrivilegeProvider implements PrivilegeProviderInterface


Not really a FakePrivilegeProvider but a first implementation to test content stream access

bwaidelich · 2023-04-12T13:51:38Z

Neos.ContentRepositoryRegistry/Classes/Factory/PrivilegeProvider/FakePrivilegeProvider.php

+
+        $privileges = Privileges::create();
+
+        $userWorkspace = $contentRepository->getWorkspaceFinder()->findOneByWorkspaceOwner($userId->value);


This won't necessarily return the actual user workspace, but any shared workspace with the same owner.
Also this won't include base workspaces currently

dangerous code - people without user workspace will automatically be admins. let's discuss :)

hehe, good catch. That can't stay like this ofc

skurfuerst

really really cool ❤️ ❤️ ❤️ ❤️

skurfuerst · 2023-04-12T17:05:28Z

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php

+            $privileges = $this->privilegeProvider->getPrivileges($visibilityConstraints);
+            if (!$privileges->isContentStreamAllowed($contentStreamId)) {
+                throw new \RuntimeException(sprintf('No access to content stream "%s"', $contentStreamId->value), 1681306937);
+            }


I would say yes (if we can, like it is done here). Ofc people could work around this if they manually instanciated ContentSubgraph; but we could never prevent this.

skurfuerst · 2023-04-12T17:06:23Z

Neos.ContentRepository.Core/Classes/SharedModel/Privilege/ContentStreamPrivilege.php

+{
+    private function __construct(
+        public readonly ?ContentStreamIds $allowedContentStreamIds,
+        public readonly ?ContentStreamIds $disallowedContentStreamIds,


what is the semantics of having both of these lists? :) I don't fully understand this yet.

Me neither :)
But I thought, that might need both. an allow list and a deny list..

Looking at this after a break, I think the terms "allowed" and "disallowed" are not helpful.
i.e. what does $privileges->isContentStreamAllowed($contentStreamId) exactly mean?

Also we should re-evaluate whether we really need allow- and deny list

skurfuerst · 2023-04-12T17:07:52Z

Neos.ContentRepository.Core/Classes/SharedModel/Privilege/Privileges.php

+        if ($this->contentStreamPrivilege === null) {
+            return true;
+        }
+        return $this->contentStreamPrivilege->allowedContentStreamIds->contain($contentStreamId);


Hm, I am not so sure about this one - IMHO this must be done lazily (otherwise we could under some scenarios have REALLY long lists of contentStreamIds; where only a single one will be relevant here)

isn't it just the currently active Content Streams of the user workspace (and its base workspace(s))?

rename to sth. that makes clearer that this is about filtering nodes

skurfuerst · 2023-04-12T17:09:03Z

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php

+        if (!$privilegeProviderFactory instanceof PrivilegeProviderFactoryInterface) {
+            throw InvalidConfigurationException::fromMessage('privilegeProvider.factoryObjectName for content repository "%s" is not an instance of %s but %s.', $contentRepositoryId->value, PrivilegeProviderFactoryInterface::class, get_debug_type($privilegeProviderFactory));
+        }
+        return $privilegeProviderFactory->build($contentRepositoryId, $contentRepositorySettings['userIdProvider']['options'] ?? [], $userIdProvider, $contentRepositoryRegistry);


UserIdProvider is fine IMHO, ContentRepositoryRegistry should go away. What about making the $contentRepository available to the provider at runtime? would IMHO be easier and more consistent.

skurfuerst · 2023-04-12T17:09:44Z

Neos.ContentRepositoryRegistry/Classes/Factory/PrivilegeProvider/FakePrivilegeProvider.php

+
+        $privileges = Privileges::create();
+
+        $userWorkspace = $contentRepository->getWorkspaceFinder()->findOneByWorkspaceOwner($userId->value);


dangerous code - people without user workspace will automatically be admins. let's discuss :)

bwaidelich · 2023-10-24T14:50:22Z

A couple of ideas @skurfuerst and I just came up with:

We should change the PrivilegeProviderInterface to a more functional interface like:

interface PrivilegeProviderInterface {

    public function isFetchingOfNodesAllowedInSubgraph(ContentStreamId $contentStreamId, DimensionSpacePoint $dimensionSpacePoint): PrivilegeResult;

    public function isCommandAllowed(CommandInterface $command): PrivilegeResult;

}

(potentially split into two individual interfaces)
The PrivilegeResult (better name to be found) is basically a boolean + x (e.g. some context on why s.th. is not allowed).

This should make it possible to deal with all 4 security related categories we came up with:

1. Prevent certain commands to be handled

This could be enforced with a single call to the PrivilegeProviderInterface::isCommandAllowed() inside ContentRepository::handle()

Examples

Prevent anonymous user from altering nodes
Prevent user X from altering nodes in workspace of user Y
Allow role X alter only content underneath nodes with a certain tag (e.g. NewsArea)

1a. Determine whether a certain command is allowed

This would not be part of the core itself – Instead the surrounding framework (e.g. Neos) could call their implementation of the isCommandAllowed() method.
That approach might not be feasible – it's up to the implementation to decide

Examples

Can Node X moved to Node Y (might be dependent on the node tags)

2. Hide certain nodes depending on the context

This will be enforced via "Subtree Tags" (#4550).
Deciding which tags are allowed in the VisibilityConstraints would not be a responsibility of the CR

Examples

Hide disabled nodes in frontend
Hide nodes within some "member" area tag in frontend

3. Hide UI elements depending on the context

Like above this would not be a concern of the CR

Examples

Hide "disable" button if the corresponding command is not allowed for the authenticated user

bwaidelich · 2024-10-17T16:11:25Z

Closing in favor of #5298

WIP: FEATURE: Content Repository Privileges

69a9c33

Related: #3732

github-actions bot added the 9.0 label Apr 12, 2023

bwaidelich commented Apr 12, 2023

View reviewed changes

skurfuerst reviewed Apr 12, 2023

View reviewed changes

bwaidelich mentioned this pull request Oct 13, 2023

Policies / Security #3732

Open

5 tasks

bwaidelich mentioned this pull request Nov 3, 2023

Refer to WorkspaceName instead of ContentStreamId in commands #4694

Closed

bwaidelich mentioned this pull request Nov 10, 2023

Odd things about (user) workspaces #4566

Open

9 tasks

bwaidelich self-assigned this Sep 18, 2024

bwaidelich closed this Oct 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: FEATURE: Content Repository Privileges #4185

WIP: FEATURE: Content Repository Privileges #4185

bwaidelich commented Apr 12, 2023

bwaidelich Apr 12, 2023

skurfuerst Apr 12, 2023

bwaidelich Oct 24, 2023 •

edited

Loading

bwaidelich Apr 12, 2023

bwaidelich Oct 13, 2023

bwaidelich Oct 24, 2023

bwaidelich Apr 12, 2023

skurfuerst Apr 12, 2023

bwaidelich Apr 12, 2023

skurfuerst Apr 13, 2023

bwaidelich Apr 12, 2023

bwaidelich Apr 12, 2023

skurfuerst Apr 12, 2023

bwaidelich Apr 12, 2023

skurfuerst left a comment

skurfuerst Apr 12, 2023

skurfuerst Apr 12, 2023

bwaidelich Apr 12, 2023

bwaidelich Oct 13, 2023

skurfuerst Apr 12, 2023

bwaidelich Apr 12, 2023

bwaidelich Oct 24, 2023

skurfuerst Apr 12, 2023

skurfuerst Apr 12, 2023

bwaidelich commented Oct 24, 2023 •

edited

Loading

bwaidelich commented Oct 17, 2024


		$privileges = Privileges::create();

		$userWorkspace = $contentRepository->getWorkspaceFinder()->findOneByWorkspaceOwner($userId->value);

WIP: FEATURE: Content Repository Privileges #4185

WIP: FEATURE: Content Repository Privileges #4185

Conversation

bwaidelich commented Apr 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwaidelich Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skurfuerst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwaidelich commented Oct 24, 2023 • edited Loading

1. Prevent certain commands to be handled

Examples

1a. Determine whether a certain command is allowed

Examples

2. Hide certain nodes depending on the context

Examples

3. Hide UI elements depending on the context

Examples

bwaidelich commented Oct 17, 2024

bwaidelich Oct 24, 2023 •

edited

Loading

bwaidelich commented Oct 24, 2023 •

edited

Loading