Allow Clients to connect to Server anonymously

Hello,

Yes currently, you can't use Entity Share with anonymous user. Because there is a permission to access the channel list, but the exposed channels are then filtered depending on the authorized user.

If you can provide a patch that will help getting this feature.

A workaround would be to configure a role that will have the same permissions as the anonymous user, and create a dedicated user with this role.

Tagging for Drupal Global Contribution Weekend

Hello @id.aleks,

Did you have time to work on this issue during the contribution weekend?

@Grimreaper Unfortunately no. I'm removing the LutskGCW20 tag.

Hello,

i think it's raw patch, but i want to hear your thoughts

Cheers

Hello,

I think this is the right direction but RemoteManager::getHttpClient method needs to be updated too.

Regards,

Hello,

Patch with updated getHttpClient method.

Cheers

Hello,

Thanks for the patch.

Do not forget to update the status to "Needs review" to trigger automated tests :).

Patch is ok for me.

Can you please provide a test with 2 contents, one published one not?

Import as admin, check that 2 nodes had been imported and then import as anonymous and check that only one node had been imported because "normally" anonymous users don't see unpublished content.

I think you can start creating the test and we will see when #3157110: Additional/refactor tests for the new architecture will be done for a last update of the test.

Hello,

added new patch with tests.

Cheers

1. +++ b/modules/entity_share_client/src/Form/RemoteForm.php
   @@ -53,19 +53,18 @@ class RemoteForm extends EntityForm {
   +      '#description' => $this->t('Leave empty if on Server website allowed Anonymous user.')
   

   Leave empty to request the server website as the anonymous user.

2. +++ b/modules/entity_share_client/src/Service/RemoteManager.php
   @@ -135,13 +135,15 @@ class RemoteManager implements RemoteManagerInterface {
   +      if ($remote->get('basic_auth_username') || $remote->get('basic_auth_password')) {
   
   @@ -155,16 +157,23 @@ class RemoteManager implements RemoteManagerInterface {
   +      if ($remote->get('basic_auth_username') || $remote->get('basic_auth_password')) {
   

   && instead of ||, no?

3. +++ b/modules/entity_share_client/tests/src/Functional/AdminContentImportTest.php
   @@ -0,0 +1,125 @@
   +    $this->entityTypeManager->getStorage('jsonapi_resource_config')->create([
   +      'id' => 'node--es_test',
   +      'disabled' => FALSE,
   +      'path' => 'node/es_test',
   +      'resourceType' => 'node--es_test',
   +      'resourceFields' => [
   +        'title' => [
   +          'fieldName' => 'title',
   +          'publicName' => $this->randomMachineName(),
   +          'enhancer' => [
   +            'id' => '',
   +          ],
   +          'disabled' => FALSE,
   +        ],
   +        'langcode' => [
   +          'fieldName' => 'langcode',
   +          'publicName' => $this->randomMachineName(),
   +          'enhancer' => [
   +            'id' => '',
   +          ],
   +          'disabled' => FALSE,
   +        ],
   +      ],
   +    ])->save();
   

   I think this is not needed.

4. +++ b/modules/entity_share_client/tests/src/Functional/AnonymousContentImportTest.php
   @@ -0,0 +1,152 @@
   +    $this->entityTypeManager->getStorage('jsonapi_resource_config')->create([
   +      'id' => 'node--es_test',
   +      'disabled' => FALSE,
   +      'path' => 'node/es_test',
   +      'resourceType' => 'node--es_test',
   +      'resourceFields' => [
   +        'title' => [
   +          'fieldName' => 'title',
   +          'publicName' => $this->randomMachineName(),
   +          'enhancer' => [
   +            'id' => '',
   +          ],
   +          'disabled' => FALSE,
   +        ],
   +        'langcode' => [
   +          'fieldName' => 'langcode',
   +          'publicName' => $this->randomMachineName(),
   +          'enhancer' => [
   +            'id' => '',
   +          ],
   +          'disabled' => FALSE,
   +        ],
   +      ],
   +    ])->save();
   

   I think this is not needed.

5. +++ b/modules/entity_share_client/tests/src/Functional/AnonymousContentImportTest.php
   @@ -0,0 +1,152 @@
   +    Role::load(AccountInterface::ANONYMOUS_ROLE)
   +      ->grantPermission('entity_share_server_access_channels')
   +      ->save();
   

   Nice!

6. +++ b/modules/entity_share_client/tests/src/Functional/EntityShareClientFunctionalTestBase.php
   @@ -227,6 +227,8 @@ abstract class EntityShareClientFunctionalTestBase extends BrowserTestBase {
   +      'bypass node access',
   +      'entity_share_server_access_channels',
   

   I understand why now we need "bypass node access". But I don't understand why it works without "entity_share_server_access_channels".

   Ok, after re-reading the codebase. It seems that adminUser is unused. and it was the channelUser that is in fact used and it has the "entity_share_server_access_channels".

7. +++ b/modules/entity_share_server/src/Controller/EntryPoint.php
   @@ -64,52 +64,49 @@ class EntryPoint extends ControllerBase {
   -    if ($current_user instanceof UserInterface) {
   

   UserInterface is no more used in the import at the beginning of the class.

8. +++ b/modules/entity_share_server/src/Form/ChannelForm.php
   @@ -786,7 +786,9 @@ class ChannelForm extends EntityForm implements ContainerInjectionInterface {
   +      'anonymous' => 'Anonymous',
   

   $this->t('Anonymous')

Ok after reading several times the new tests, I understand why it is difficult to make things differently.

So I would have said: add the anonymous user in the main channel and this would be ok. But because of the testRemoteManager::doRequest that stores per URL, this would require to do in the same test to reset it like in testPathautoFieldEnhancer()

Also, methods like "pullChannel()" use $this->remote, so it is hard to provide several remotes in the same class.

I was about to make the small fixes and merge, but I think to prepare tests for #2856713: Authentication plugins and HTTP authentication, we should try to have one class dedicated to authentication tests.

Can you please try to do like Ivan had done in PathautoTest.php please? I think one channel with all the required users can be enough and this would avoid the need to update it. "Just" update the remote with the user you want to use. And no problem to use the admin user, this will give it one reason to exist (you can remove the previous permissions as it were not used).

Hello,

Here is the patch which contains (mostly) the rework of the test (ie. merges two tests into one).

I used two channels and one remote, which I changed in the middle of the tests.
Admin user is not used, but permission bypass node access was granted to the channel user.

Thanks a lot for the patch.

Here is my review:

+++ b/modules/entity_share_client/tests/src/Functional/AuthenticationImportTest.php
@@ -0,0 +1,183 @@
+    return [
+      'entity_share_server_access_channels',
+      // This user will act an administrative user, so allow them to access
+      // unpublished nodes.
+      'bypass node access',
+    ];

return [
// This user will act an administrative user, so allow them to access
// unpublished nodes.
'bypass node access',
] + parent::getChannelUserPermissions();

But that is perfectly ok. The rest of the code is perfect. So I will merge now!