Export content as user 1 when running in a CLI context

Hm, probably it makes sense, the win is that no access restrictions will apply on export

@Berdir any thoughts?

IMO absolutely. Yes. I have found that in order to implement #2698425: Do not re-import existing entities this is needed because in order to find out whether an entity exists we use need to look them up as user 1. If we look up as any other user with limited access and there are nodes that have access restrictions then they won't be found and we'll get a db error.

Next Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1062
Duplicate entry '2c24a28a-fdb4-4c98-8118-7c845e6b0f3d' for key 'node_field__uuid__value': INSERT INTO
{node} (vid, type, uuid, langcode) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1,
:db_insert_placeholder_2, :db_insert_placeholder_3); Array
(
    [:db_insert_placeholder_0] => 
    [:db_insert_placeholder_1] => page
    [:db_insert_placeholder_2] => 2c24a28a-fdb4-4c98-8118-7c845e6b0f3d
    [:db_insert_placeholder_3] => en
)
 in /vagrant/repos/subscriptions/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:770

I have been considering removing --user from drush since I thought this sort os masquerading is not needed anymore. Isn't there a lower level API that default content can use to bypass access control?

Yeah, we can use the account switcher service in core.

here is a patch that uses the account switcher service for the task

Hmm... I tried using the patch in #6—applied it to default_content module successfully, did a drush cr, then ran:

drush dcer user 2 --folder=modules/custom/mysite_default_content/content/

But the exported user JSON still doesn't contain status, email, or roles.

Found that the changes in the patch only apply to the Drush < 9 versions, and I'm running Drush 9. Testing another version of the patch now which updates the Drush services and DefaultContentCommands.php as well.

Attached patch also applies to Drush 9 commands. Seems to work well in my case—now I'm seeing roles, email, etc. (all the things available to user 1).

Btw \Drupal\default_content\Importer::importContent() already does switch to user 1

I think better to make it in exporter because it's hard to explain to module's users that they must user `--user` drush's argument (nobody read docs)
Also it will be minimize code base if someone will provide drupal console integration

My understanding is that Command would not consult a --user option because the module doesn't work without a non-priveleged user. It would simply switch to uid=1.

FWIW I dont especially care where the switch happens.

Right now I don't have any cycles to devote to working on this patch, with it being end of year, holiday season and all that :(

Here's a patch that switches the user in Exporter rather than the drush commands themselves.

Fixed the tests.

+++ b/src/Exporter.php
@@ -127,6 +142,9 @@ class Exporter implements ExporterInterface {
   public function exportContentWithReferences($entity_type_id, $entity_id) {
+    $root_user = $this->entityTypeManager->getStorage('user')->load(1);
+    $this->accountSwitcher->switchTo($root_user);

not sure it is the best place, suppose better to add to \Drupal\default_content\Commands\DefaultContentCommands because drush 9 no longer have --user argument

This way we will keep compatibility with drush8 and will not affect all export methods

I think it's best to put this directly in the exporter class precisely because it's agnostic to the Drush version :)

+++ b/src/Exporter.php
@@ -127,6 +142,9 @@ class Exporter implements ExporterInterface {
+    $root_user = $this->entityTypeManager->getStorage('user')->load(1);
+    $this->accountSwitcher->switchTo($root_user);

we throw two execptions here, we should switch back before throwing for safety sake

In addition, I think we should check we're running in a cli context before we switch, in case someone has built UI for this - we don't want there to be an access bypass

other than that, looks good to me - thanks

@larowlan, good points :)

we throw two execptions here, we should switch back before throwing for safety sake

Since the problematic part is the serialization process, not entity loading, I've moved the account switching after those exceptions are thrown. Also cleaned up the code a bit and added the exceptions to exportContent() as well.

In addition, I think we should check we're running in a cli context before we switch, in case someone has built UI for this - we don't want there to be an access bypass

Done :)

Looks good to me