- Issue created by @MegaKeegMan
- πΊπΈUnited States MegaKeegMan
I have marked this issue as related to https://www.drupal.org/project/activitypub/issues/3318782 β (allow any entity to be an actor), which I think would be excellent as well, and more fitting for our end goal (which will probably involved using taxonomy terms under a specified vocabulary as our actors). However, I am not sure if this request would be simpler. Please share any thoughts about this or pointers on getting this contributed. I am happy to do some work here, but wanted to check in about the plausibility or feasibility of this request, especially in relation to the other. My hope is that this might be a lower hanging fruit that others could use as well.
- π§πͺBelgium swentel
Hmm, very interesting idea. Let me think about this one for a couple of days. It's been a while since I've looked at the code, and I want to check where/how the code should be changed to achieve this, because this could help out many people as a first intermediate step since #3318782: Allow any entity to be actor β is a major overhaul of the code in general :)
- π§πͺBelgium swentel
Hi, looked at the places in code which would need some changes for this to work.
I'm been thinking about this scenario: A Drupal user has the ability to view the 'ActivityPub outbox' fieldset on a node edit form, even if this Drupal user has no ActivityPub profile.
That fieldset is added in ActivityPubFormAlter::addActivityPubOutboxFormElement(). The first thing it does is checking whether the current user has an actor. If not, the code completely stops. There's also some code in ActivityPubOutbox::createActivity which checks for the uid, which, looking back now, doesn't even make sense. so lines 80 to 83 can be deleted and we get the $uid from the $actor.
To get the fieldset to show I'm thinking of this: add a setting at /admin/config/services/activitypub which allows you to configure the uid of the site-wide actor and a permission 'publish activities to the site-wide actor' (or something like this).
The code in the form alter would become something like this:
// Check if a site-wide actor is configured and the current user has the permission to publish. if (!$actor) { if ($site_wide_uid = $this->config('settings')->get('site_wide_uid') && $this->currentUser->hasPermission('publish to site-wide actor')) { $actor = $this->actorStorage->loadActorByEntityIdAndType(site_wide_uid, 'person'); } } if (!$actor) { return; }
Haven't fully scanned the code, but I think this should do it. We should definitely add a test for this. Looking at the tests, I think we can take a copy more or less from OutboxTest::testTo adding a third user which does not have an actor, creates a node and that test it goes to the outbox from user 1 to user 2.
What do you think?
- @megakeegman opened merge request.
- Status changed to Needs review
almost 2 years ago 11:45pm 9 March 2023 - πΊπΈUnited States MegaKeegMan
Made some slight fixes, and now I think all of the necessary changes are in. Trying to test though, and expecting to see some content from @Portside@test.portside.org and not seeing anything at the moment. We did have this working before, and I am not sure whether the failures are user-related, or I broke something in the code, or if social.coop needs to process their inbox. I couldn't say. I will try to test this a little more, but if anyone else is able to test the changes in the issue fork that would be great.
- πΊπΈUnited States MegaKeegMan
This doesn't seem like it should be related, but test last week worked and test this week is doing this. So maybe related to a failure to set up properly or something I brokeβI am seeing 2 errors in my log:
1) Error creating the signature: load() should not be called from final classes (phpseclib3\Crypt\RSA\PrivateKey)
2) activitypub Outbox exception to /inbox to /inbox: Client error: `POST /inbox` resulted in a `401 Unauthorized` response: {"error":"Incompatible request signature. keyId and signature are required"}The second error looks similar to another issue I found: https://www.drupal.org/project/activitypub/issues/3319068 β
Holding off on opening a new issue in case there is some existing knowledge of these, but I may be blocked on reviewing my own code
- πΊπΈUnited States MegaKeegMan
The above errors (same as test failures) are related to another new issue which I have just opened:
π Error creating the signature: load() should not be called from final classes (phpseclib3\Crypt\RSA\PrivateKey) FixedSo let's get back to testing this (rerun the pipeline and review) once there is a fix for this other issue.
- π§πͺBelgium swentel
It's indeed a problem with phpseclib, the patch in π Error creating the signature: load() should not be called from final classes (phpseclib3\Crypt\RSA\PrivateKey) Fixed locking it to 3.0.18 in our composer fixes it, will commit that - it's a one line patch to merge in with our branch then.
- πΊπΈUnited States MegaKeegMan
Ok awesome. So this whole process reminded me of your comment earlier that a new test is still needed for this feature. Will try to get around to this early in the week.
- Status changed to Needs work
almost 2 years ago 7:06pm 11 March 2023 - πΊπΈUnited States MegaKeegMan
Ran into some hiccups, since I needed to rebase in order to have passing tests. Some conflicting code, in particular the change from activitypub_type to activitypub_create. I accidentally included changes to the config schema in my last commit (which was just adding a test), but I am not going to worry about it for now. Otherwise, we could consider whether the full copy of testTo is somewhat excessive. I am leaving it for now.
- Status changed to Needs review
almost 2 years ago 11:52pm 13 March 2023 - Status changed to RTBC
almost 2 years ago 9:17pm 16 March 2023 - πΊπΈUnited States mlncn Minneapolis, MN, USA
The functionality works! For something of a workaround, the code is clean. And testbot is happy now.
- π§πͺBelgium swentel
Hmm the tests are still failing no? I can have a look if you want to get this in, would be good for a new release.
- πΊπΈUnited States MegaKeegMan
No the tests are passing. The pipeline link is hidden under the the "show more commits" button
-
swentel β
committed 9aaad695 on 1.0.x authored by
MegaKeegMan β
Issue #3344797 by MegaKeegMan: Syndicate all content from a dedicated...
-
swentel β
committed 9aaad695 on 1.0.x authored by
MegaKeegMan β
- Status changed to Fixed
almost 2 years ago 3:25pm 17 March 2023 - π§πͺBelgium swentel
Ooh, missed that, sorry, merged, thanks!
Will tag a new release too.
Automatically closed - issue fixed for 2 weeks with no activity.