[meta] Private D8 Port

I have an updated port ready on github, see https://github.com/swentel/private

I've removed the schema and used a boolean base field which means we don't have to worry about storage, views, rendering at all.
The 'key' image is gone for now, but could potentially come back with a dedicated formatter (if wanted).

The private keyword is indeed dramatic because it's kind of silly to have a dedicated submodule only for the actions. I'm also working on D6 to D8 migrate which is also a simple plugin. Same would apply in case we want to have a dedicated formatter. What if at some point access grants hook is removed (probably not in D8 cycle, but maybe in D9) ...

I see two options:

1. create a new project on d.o, something like 'private_content'
2. move all code to a 'private_content' submodule in this project. Directory structure would be a little funky, but oh well.

For now, I went for option two in my repo on github.

Tests, actions and migrate still need porting, coming up today and further this week.

Tests & actions have been ported as well, this can now be reviewed further.

D6 to D8 migration is now available too - should be fine for a first alpha.

Adding credits

@swentel Great news that you have done the port.

I have taken over as a new maintainer to bring some life back into this module. I am fixing some D7 bugs first of all, then will move on to take a look at D8.

@swentel I have now done an initial review of the code and it's looking good. I hope that you are still interested in being involved with the D8 port.

Here are my first thoughts.

1. I understand and agree with the decision to name the module private_content. However I don't think we need the funny directory structure. I moved all the files in private_content to the top level and it worked fine.
2. There is now a 7.x-2.x branch with some major bug fixes. Sorry it's extra work, but the D8 branch needs to be updated to match.
3. The decision to use a field for the private setting is a major design change and I think it needs some thought and discussion....

Private setting as field or pseudo-field

My intuition suggests that private is pseudo-field, which would lead to using hook_entity_extra_field_info, similar to D7.

Using a field for the private setting (hook_entity_base_field_info) has the advantage that it gives a lot of code 'for free'. The key question is whether it is the right code. I think it might become a little more tricky when it comes to integrating the fixes from 7.x-2.x due to challenges with the private field:

• The default value of the field is not necessarily FALSE but rather depends on the content type setting - see private_node_load in 7.x-2.x.
• The value should be displayed in a view even if the storage doesn't exist - with the D8 code as stands I created a view and the value was blank if there was no storage.
• The private setting should not be alterable if the content type setting is PRIVATE_ALWAYS - even by an admin who bypasses normal access checks.
• The private setting should only be alterable if the user has permission 'mark content as private' (could handle with an access hook).

My mind is not yet made up and I'm keen to hear views from anyone else on the thread.

However I don't think we need the funny directory structure

Right, duh, I don't know why I didn't think of that :)

My intuition suggests that private is pseudo-field, which would lead to using hook_entity_extra_field_info, similar to D7

I don't necessarily like pseudo fields, it's a bit clumsy imho. (I hope someday to kill those implementations). We can always expose the field with the base_field info hook too and should be able to use formatters then. The bonus is that the field formatters for private are used in views then automatically too.

Perhaps, we should then also create a new field type which gives more control on the default value that needs to be stored.

Let me think a bit about this some more.

We can always expose the field with the base_field

So funny: this is already exposed (it's been a while since I looked a the manage display configuration in the site). If we write more formatters we can alter the display easily, and in combination with a 'private' field type even target those formatters specific to the private field only.

@swentel I have happy in principal for the D8 version to use a full field. However of course we need to ensure that everything works correctly and is secure.

The SA against 7.x.1.x is now public so I can now be explicit here - there were multiple security bugs in the 1.x version. The 8.x code needs to be reworked to be based on the 7.x.2.x version. Hopefully it won't be too hard - the entire module is fairly small, and it didn't take me all that long to write version 2.x in the first place.

If you are willing to take on the task of finishing off the D8 version then I will happily review and create a D8 release when it's ready. After that you could continue to be a maintainer if you are interested.

A big thank you for creating a working D8 module for this! swentel's code worked fine for me on D8.4

Reading #12 I've tried to make some updates to swentel's code to align 7.x.2.x, but I get the error
Recoverable fatal error: Object of class Drupal\Core\Field\EntityReferenceFieldItemList could not be converted to string in Drupal\Core\Database\Statement->execute() (line 59 of /..../core/lib/Drupal/Core/Database/Statement.php)

When the permissions are rebuilt.

I'm not sure I understand enough to move this any further forward.

@Matt B thanks for getting involved. Please can you explain what your patch file is? Does it apply on top of the GitHub branch created by swentel? If so maybe you could clone that on GitHub and then commit your changes to your clone.

@AdamPS - it applys on top of the GitHub branch created by swentel. You can find the changes here: https://github.com/MattB98/private/tree/rework7.x.2.x/private_content

I have created branches

8.x-1.x is copied from https://github.com/swentel/private (except fixed the directory structure). There will never be a supported release as the code has security bugs inherited from 7.x-1.x.
8.x-2.x is copied from v7.x-2.x so of course doesn't work for D8.

The remaining task is to merge these two.

@Matt B Thanks I'll take a look at your GitHub code next week when I get a chance.

Thank you.

I have just been using what now is the 8.x-1.x branch on a dev site, and noticed that if I grant a role "Edit private content" permission and that role has permissions set under the Node permissions section to Create, Edit,Delete,Revert a nodes of a specific content type (in my case, one I've created called Recording) a user assigned that role can also Edit (but not add) nodes of another content type that they do not have permission to Edit (eg nodes of content type Article), but that have been marked Private. I would expect the "Edit private content" permission to respect the Node permissions.

Is this related to the security issue that affects 8.x-1.x? If not we need to test for it in 8.x-2.x when it is developed further. Happy to raise a separate issue in either branch - please advise!

@Matt B the 1.x branch has many security bugs and that looks like one of them. It would be a good idea to add more tests in 2.x to cover those so please feel free to raise an issue for that.

Am I missing something obvious? 8.x-1.x is not working for me. (I'm not talking about whether it's secure - it just doesn't work.)

• I don't see the private field in "manage form display"
• I don't see a private flag when editing a node
• I do see the private field in "manage display" but don't see any indicator when viewing the page
• Even when I set a content type to be "Hidden (always private)" I can still access it.

I don't see the private field in "manage form display"
-> agreed, but I'm happy with where it is placed (see below)

I don't see a private flag when editing a node
-> look under the Promotion Options tab on the right

I do see the private field in "manage display" but don't see any indicator when viewing the page
-> Agreed - I had no requirement to display it, but I just tested it and cannot see it on the page

Even when I set a content type to be "Hidden (always private)" I can still access it.
-> I've not tested this option as I don't want to hide it on my site.

The flag is also exposed in Drupal 8 views which is handy as well. I think 8.x-1.x has some gaps over and above security, but in general it works for me.

Aha thanks for the explanation. I don't use "Promoted to front page" so had that disabled via "manage form display". It feels like the better Drupal 8 way would be to have a separate "manage form display" for private to match "promoted", "sticky".

As per #9 the field exposed in views is the last value stored (if any) - it doesn't respect the content type defaults (another v1 bug) and will be blank for nodes that have not been saved since private module was enabled.

@swentel are you willing to spend time helping get this module working with fields? Without your extensive Drupal Core knowledge we be unsure in places and could find it easier to go back to pseudo-fields.

@Matt B, I think I can see your current problem. In your branch, in private_content_get_default you have code
$node->private = ...

That's not allowed now it's a field. What is needed here is to create something like this:

namespace Drupal\private_content\Plugin\Field\FieldType;

use Drupal\Core\Field\Plugin\Field\FieldType\BooleanItem

/**
 * @FieldType(
 *   id = "privacy_control_field",
 *   default_formatter = "privacy_indicator_formatter"
 *  FILL IN MORE SETTINGS
 *
 */
class FieldTypePrivacyControl extends BooleanItem

then write code to control the loading of default value - I'm not exactly sure how off the top of my head, but I expect @swentel knows.

I am still unsure over "Private setting as field or pseudo-field". It might be better Drupal architecture, neater, etc, but it might also be more work. With v7.2, I have hopefully got a secure and largely bug free version based on pseudo-fields. If we switch to a real field we may need to solve some of the problems a second time.

@Matt B You have made a good start. How much more time do you have to help with the development?

Hi AdamPS. About an hour a week! Sorry, but too many other commitments around a full time job. I'm also not familiar enough with Drupal 8 yet. I can help with testing though!

Every bit of help is welcome. Yes D8 is quite a jump - it gradually get's easier, but I'm still learning.

For the time being I propose we should just comment out the line $node->private = and set a todo marker. The defaulting isn't the top priority.

OK I created a first alpha release!

This started with v1.x work by @kerasai @swentel merged into v2.x by @Matt B, @AdamPS. Thanks everyone.

It's not yet ready for live use, but please do test. Note there are some @todo in the code - please check those before posting issues, in particular:

• View of private field is not necessarily accurate - may be blank (missing default) and may be ignored (node type has hard-coded setting). This has become less easy to sort out now we are using a field (but otherwise fields have been helpful).
• Can't uninstall.

There are various parts that could be tidied up relating to widget and formatter settings. Probably we can make our own derived field with a formatter and widget.

I get the following error when I go to add or edit content:

TypeError: Argument 1 passed to private_content_is_locked() must implement interface Drupal\node\NodeInterface, array given, called in /drupal/code/modules/contrib/private/private_content.module on line 192 in private_content_is_locked() (line 299 of /drupal/code/modules/contrib/private/private_content.module) 
#0 /drupal/code/modules/contrib/private/private_content.module(192): private_content_is_locked(Array) 
#1 [internal function]: private_content_entity_field_access('edit', Object(Drupal\Core\Field\BaseFieldDefinition), Object(Drupal\Core\Session\AccountProxy), Object(Drupal\Core\Field\FieldItemList)) 
#2 /drupal/code/core/lib/Drupal/Core/Extension/ModuleHandler.php(391): call_user_func_array('private_content...', Array) 
#3 /drupal/code/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php(343): Drupal\Core\Extension\ModuleHandler->invoke('private_content', 'entity_field_ac...', Array) 
#4 /drupal/code/core/lib/Drupal/Core/Field/FieldItemList.php(165): Drupal\Core\Entity\EntityAccessControlHandler->fieldAccess('edit', Object(Drupal\Core\Field\BaseFieldDefinition), Object(Drupal\Core\Session\AccountProxy), Object(Drupal\Core\Field\FieldItemList), false) 
#5 /drupal/code/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php(169): Drupal\Core\Field\FieldItemList->access('edit') 
#6 /drupal/code/core/lib/Drupal/Core/Entity/ContentEntityForm.php(122): Drupal\Core\Entity\Entity\EntityFormDisplay->buildForm(Object(Drupal\node\Entity\Node), Array, Object(Drupal\Core\Form\FormState)) 
#7 /drupal/code/core/modules/node/src/NodeForm.php(113): Drupal\Core\Entity\ContentEntityForm->form(Array, Object(Drupal\Core\Form\FormState)) 
#8 /drupal/code/core/lib/Drupal/Core/Entity/EntityForm.php(117): Drupal\node\NodeForm->form(Array, Object(Drupal\Core\Form\FormState)) 
#9 [internal function]: Drupal\Core\Entity\EntityForm->buildForm(Array, Object(Drupal\Core\Form\FormState)) 
#10 /drupal/code/core/lib/Drupal/Core/Form/FormBuilder.php(514): call_user_func_array(Array, Array) 
#11 /drupal/code/core/lib/Drupal/Core/Form/FormBuilder.php(271): Drupal\Core\Form\FormBuilder->retrieveForm('node_article_ed...', Object(Drupal\Core\Form\FormState)) 
#12 /drupal/code/core/lib/Drupal/Core/Controller/FormController.php(74): Drupal\Core\Form\FormBuilder->buildForm('node_article_ed...', Object(Drupal\Core\Form\FormState)) 
#13 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch)) 
#14 /drupal/code/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array) 
#15 /drupal/code/core/lib/Drupal/Core/Render/Renderer.php(576): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() 
#16 /drupal/code/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) 
#17 /drupal/code/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) 
#18 [internal function]: Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() 
#19 /drupal/code/vendor/symfony/http-kernel/HttpKernel.php(153): call_user_func_array(Object(Closure), Array) 
#20 /drupal/code/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) 
#21 /drupal/code/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#22 /drupal/code/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#23 /drupal/code/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#24 /drupal/code/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#25 /drupal/code/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#26 /drupal/code/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(50): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#27 /drupal/code/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#28 /drupal/code/core/lib/Drupal/Core/DrupalKernel.php(657): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#29 /drupal/code/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) 
#30 {main}.

Ah sorry it looks like a committed a change to dev but didn't make a new alpha release - please can you try dev?

8.x-2.0-alpha1+1-dev works absolutely fine - including my test in #17 - thanks AdamPS!

Great thanks for testing. I just made a new alpha

NB check the open issues.

• Views not fully working
• Module not yet stable: I plan to create a field type and a formatter, which would be a BC problem so don't use on live sites

8.x-2.0-alpha2 tests fine. There is a minor issue with the naming of the module #2939567: Install smoothly with drush. When do you think this will be ready for production use?

I don't have any time at the moment to do more voluntary work on D8. So it needs either sponsors or people to contribute good quality patches. If anyone is interested then please get in touch.

D8 version is now in a separate project "Private content". The module was renamed for D8 because private is a reserved word in PHP so cannot be used as a name.