From ffc32e312c8b57526556ea802d5ce41a2588aed6 Mon Sep 17 00:00:00 2001 From: Jeremy Dormitzer Date: Tue, 22 Jan 2019 17:36:21 -0500 Subject: [PATCH] Refactor ActivityEvent to also have the actor; implement NonActivityHandler --- src/Activities/ActivityEvent.php | 19 ++++++- src/Activities/InboxActivityEvent.php | 22 -------- src/Activities/NonActivityHandler.php | 28 +++++++++-- src/Activities/OutboxActivityEvent.php | 22 -------- src/Auth/AuthListener.php | 16 +++++- src/Auth/SignatureListener.php | 5 +- src/Controllers/InboxController.php | 21 +++++++- src/Controllers/OutboxController.php | 22 +++++++- src/Crypto/RsaKeypair.php | 2 + src/Entities/ActivityPubObject.php | 36 ++++++++++++++ src/Entities/Field.php | 18 +++++++ src/Http/ControllerResolver.php | 14 ++++-- test/Activities/NonActivityHandlerTest.php | 14 ++++++ test/Auth/AuthListenerTest.php | 58 ++++++++++++++++++---- test/Auth/SignatureListenerTest.php | 50 ++++++++++++++----- test/Controllers/InboxControllerTest.php | 14 ++++++ test/Controllers/OutboxControllerTest.php | 14 ++++++ test/Http/ControllerResolverTest.php | 14 +----- 18 files changed, 294 insertions(+), 95 deletions(-) create mode 100644 test/Activities/NonActivityHandlerTest.php create mode 100644 test/Controllers/InboxControllerTest.php create mode 100644 test/Controllers/OutboxControllerTest.php diff --git a/src/Activities/ActivityEvent.php b/src/Activities/ActivityEvent.php index e1d7b94..7c8c322 100644 --- a/src/Activities/ActivityEvent.php +++ b/src/Activities/ActivityEvent.php @@ -1,6 +1,7 @@ activity = $activity; + $this->actor = $actor; } /** @@ -29,5 +38,13 @@ class ActivityEvent extends Event { $this->activity = $activity; } + + /** + * @return ActivityPubObject The actor + */ + public function getActor() + { + return $this->actor; + } } ?> diff --git a/src/Activities/InboxActivityEvent.php b/src/Activities/InboxActivityEvent.php index 8c90a93..ea0eeb5 100644 --- a/src/Activities/InboxActivityEvent.php +++ b/src/Activities/InboxActivityEvent.php @@ -2,31 +2,9 @@ namespace ActivityPub\Activities; use ActivityPub\Activities\ActivityEvent; -use ActivityPub\Entities\ActivityPubObject; class InboxActivityEvent extends ActivityEvent { const NAME = 'inbox.activity'; - - /** - * The inbox to which the activity was posted - * - * @var ActivityPubObject - */ - protected $inbox; - - public function __construct( array $activity, ActivityPubObject $inbox ) - { - parent::__construct( $activity ); - $this->inbox = $inbox; - } - - /** - * @return ActivityPubObject The inbox - */ - public function getInbox() - { - return $this->inbox; - } } ?> diff --git a/src/Activities/NonActivityHandler.php b/src/Activities/NonActivityHandler.php index f1f6b2f..090cd48 100644 --- a/src/Activities/NonActivityHandler.php +++ b/src/Activities/NonActivityHandler.php @@ -2,6 +2,7 @@ namespace ActivityPub\Activities; use ActivityPub\Activities\OutboxActivityEvent; +use ActivityPub\Objects\IdProvider; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** @@ -13,6 +14,11 @@ class NonActivityHandler implements EventSubscriberInterface * @var ContextProvider */ private $contextProvider; + + /** + * @var IdProvider + */ + private $idProvider; const ACTIVITY_TYPES = array( 'Accept', 'Add', 'Announce', 'Arrive', @@ -44,13 +50,27 @@ class NonActivityHandler implements EventSubscriberInterface /** * Makes a new Create activity with $object as the object * + * @param Request $request The current request + * @param array $object The object + * @param ActivityPubObject $actorId The actor creating the object + * * @return array The Create activity */ - private function makeCreate( array $object ) + private function makeCreate( Request $request, array $object, + ActivityPubObject $actor ) { - // TODO implement me - // if object doesn't have an id, generate one - // generate an id for the Create activity as well + $create = array( + '@context' => $this->contextProvider->getContext(), + 'type' => 'Create', + 'id' => $this->idProvider->getId( $request, "activities" ), + 'actor' => $actor['id'], + 'object' => $object, + ); + foreach ( array( 'to', 'bto', 'cc', 'bcc', 'audience' ) as $field ) { + if ( array_key_exists( $field, $object ) ) { + $create[$field] = $object[$field]; + } + } } } ?> diff --git a/src/Activities/OutboxActivityEvent.php b/src/Activities/OutboxActivityEvent.php index 21f69d6..16be405 100644 --- a/src/Activities/OutboxActivityEvent.php +++ b/src/Activities/OutboxActivityEvent.php @@ -2,31 +2,9 @@ namespace ActivityPub\Activities; use ActivityPub\Activities\ActivityEvent; -use ActivityPub\Entities\ActivityPubObject; class OutboxActivityEvent extends ActivityEvent { const NAME = 'outbox.activity'; - - /** - * The outbox to which the activity was posted - * - * @var ActivityPubObject - */ - protected $outbox; - - public function __construct( array $activity, ActivityPubObject $outbox ) - { - parent::__construct( $activity ); - $this->outbox = $outbox; - } - - /** - * @return ActivityPubObject The outbox - */ - public function getOutbox() - { - return $this->outbox; - } } ?> diff --git a/src/Auth/AuthListener.php b/src/Auth/AuthListener.php index 823b30e..2a1801a 100644 --- a/src/Auth/AuthListener.php +++ b/src/Auth/AuthListener.php @@ -1,6 +1,8 @@ authFunction = $authFunction; + $this->objectsService = $objectsService; } public function checkAuth( GetResponseEvent $event ) @@ -48,7 +56,11 @@ class AuthListener implements EventSubscriberInterface } $actorId = call_user_func( $this->authFunction ); if ( $actorId && ! empty( $actorId ) ) { - $request->attributes->set( 'actor', $actorId ); + $actor = $this->objectsService->dereference( $actorId ); + if ( ! $actor ) { + throw new Exception( "Actor $actorId does not exist" ); + } + $request->attributes->set( 'actor', $actor ); } } } diff --git a/src/Auth/SignatureListener.php b/src/Auth/SignatureListener.php index 4cf01a6..3dd23e1 100644 --- a/src/Auth/SignatureListener.php +++ b/src/Auth/SignatureListener.php @@ -67,8 +67,8 @@ class SignatureListener implements EventSubscriberInterface return; } $owner = $key['owner']; - if ( ! is_string( $owner ) ) { - $owner = $owner['id']; + if ( is_string( $owner ) ) { + $owner = $this->objectsService->dereference( $owner ); } if ( ! $owner ) { return; @@ -77,7 +77,6 @@ class SignatureListener implements EventSubscriberInterface return; } $request->attributes->set( 'signed', true ); - $request->attributes->set( 'signedBy', $owner ); if ( ! $request->attributes->has( 'actor' ) ) { $request->attributes->set( 'actor', $owner ); } diff --git a/src/Controllers/InboxController.php b/src/Controllers/InboxController.php index db00393..0158925 100644 --- a/src/Controllers/InboxController.php +++ b/src/Controllers/InboxController.php @@ -25,10 +25,27 @@ class InboxController */ public function handle( Request $request ) { + if ( ! $request->attributes->has( 'actor' ) ) { + throw new UnauthorizedHttpException(); + } + $actor = $request->attributes->get( 'actor' ); + $inboxId = $this->getUriWithoutQuery( $request ); + if ( ! $actor->hasField( 'inbox' ) || $actor['inbox']['id'] !== $inboxId ) { + throw new UnauthorizedHttpException(); + } $activity = $request->attributes->get( 'activity' ); - $inbox = $request->attributes->get( 'inbox' ); - $event = new InboxActivityEvent( $activity, $inbox ); + $event = new InboxActivityEvent( $activity, $actor ); $this->eventDispatcher->dispatch( InboxActivityEvent::NAME, $event ); } + + private function getUriWithoutQuery( Request $request ) + { + $uri = $request->getUri(); + $queryPos = strpos( $uri, '?' ); + if ( $queryPos !== false ) { + $uri = substr( $uri, 0, $queryPos ); + } + return $uri; + } } ?> diff --git a/src/Controllers/OutboxController.php b/src/Controllers/OutboxController.php index cf4732d..054e463 100644 --- a/src/Controllers/OutboxController.php +++ b/src/Controllers/OutboxController.php @@ -4,6 +4,7 @@ namespace ActivityPub\Controllers; use ActivityPub\Activities\OutboxActivityEvent; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException; class OutboxController { @@ -25,10 +26,27 @@ class OutboxController */ public function handle( Request $request ) { + if ( ! $request->attributes->has( 'actor' ) ) { + throw new UnauthorizedHttpException(); + } + $actor = $request->attributes->get( 'actor' ); + $outboxId = $this->getUriWithoutQuery( $request ); + if ( ! $actor->hasField( 'outbox' ) || $actor['outbox']['id'] !== $outboxId ) { + throw new UnauthorizedHttpException(); + } $activity = $request->attributes->get( 'activity' ); - $outbox = $request->attributes->get( 'outbox' ); - $event = new OutboxActivityEvent( $activity, $outbox ); + $event = new OutboxActivityEvent( $activity, $actor ); $this->eventDispatcher->dispatch( OutboxActivityEvent::NAME, $event ); } + + private function getUriWithoutQuery( Request $request ) + { + $uri = $request->getUri(); + $queryPos = strpos( $uri, '?' ); + if ( $queryPos !== false ) { + $uri = substr( $uri, 0, $queryPos ); + } + return $uri; + } } ?> diff --git a/src/Crypto/RsaKeypair.php b/src/Crypto/RsaKeypair.php index dfe6767..0152c1d 100644 --- a/src/Crypto/RsaKeypair.php +++ b/src/Crypto/RsaKeypair.php @@ -82,6 +82,8 @@ class RsaKeypair */ public function verify( $data, $signature, $hash = 'sha256' ) { + // TODO this throws a "Signature representative out of range" occasionally + // I have no idea what that means or how to fix it $rsa = new RSA(); $rsa->setHash( $hash ); $rsa->setSignatureMode(RSA::SIGNATURE_PKCS1); diff --git a/src/Entities/ActivityPubObject.php b/src/Entities/ActivityPubObject.php index 72e10a1..465586d 100644 --- a/src/Entities/ActivityPubObject.php +++ b/src/Entities/ActivityPubObject.php @@ -149,6 +149,21 @@ class ActivityPubObject implements ArrayAccess return false; } + /** + * Returns the fields named $field, if it exists + * + * @param string $name The name of the field to get + * @return Field|null + */ + public function getField( string $name ) + { + foreach( $this->getFields() as $field ) { + if ( $field->getName() === $name ) { + return $field; + } + } + } + /** * Returns the value of the field with key $name * @@ -286,5 +301,26 @@ class ActivityPubObject implements ArrayAccess 'ActivityPubObject fields cannot be directly unset' ); } + + /** + * Returns true if $other has all the same fields as $this + * + * @param ActivityPubObject $other The other object to compare to + * @return bool Whether or not this object has the same fields and values as + * the other + */ + public function equals( ActivityPubObject $other ) + { + foreach( $other->getFields() as $otherField ) { + $thisField = $this->getField( $otherField->getName() ); + if ( ! $thisField ) { + return false; + } + if ( ! $thisField->equals( $otherField ) ) { + return false; + } + } + return true; + } } ?> diff --git a/src/Entities/Field.php b/src/Entities/Field.php index 67881c3..d1b4cfb 100644 --- a/src/Entities/Field.php +++ b/src/Entities/Field.php @@ -257,5 +257,23 @@ class Field { return $this->lastUpdated; } + + /** + * Returns true if $this is equal to $other + * + * @return bool + */ + public function equals( Field $other ) + { + if ( $this->getName() !== $other->getName() ) { + return false; + } + if ( $this->hasValue() ) { + return $other->hasValue() && $other->getValue() === $this->getValue(); + } else { + return $other->hasTargetObject() && + $this->getTargetObject()->equals( $other->getTargetObject() ); + } + } } ?> diff --git a/src/Http/ControllerResolver.php b/src/Http/ControllerResolver.php index 3efc55e..21cb9da 100644 --- a/src/Http/ControllerResolver.php +++ b/src/Http/ControllerResolver.php @@ -50,7 +50,7 @@ class ControllerResolver implements ControllerResolverInterface if ( $request->getMethod() == Request::METHOD_GET ) { return array( $this->getObjectController, 'handle' ); } else if ( $request->getMethod() == Request::METHOD_POST ) { - $uri = $request->getUri(); + $uri = $this->getUriWithoutQuery( $request ); $actorWithInbox = $this->objectWithField( 'inbox', $uri ); if ( $actorWithInbox ) { $activity = json_decode( $request->getContent(), true ); @@ -58,7 +58,6 @@ class ControllerResolver implements ControllerResolverInterface throw new BadRequestHttpException( '"type" field not found' ); } $request->attributes->set( 'activity', $activity ); - $request->attributes->set( 'inbox', $actorWithInbox->getFieldValue( 'inbox' ) ); return array( $this->inboxController, 'handle' ); } else { $actorWithOutbox = $this->objectWithField( 'outbox', $uri ); @@ -68,7 +67,6 @@ class ControllerResolver implements ControllerResolverInterface throw new BadRequestHttpException( '"type" field not found' ); } $request->attributes->set( 'activity', $activity ); - $request->attributes->set( 'outbox', $actorWithOutbox->getFieldValue( 'outbox' ) ); return array( $this->outboxController, 'handle' ); } else { throw new NotFoundHttpException(); @@ -81,5 +79,15 @@ class ControllerResolver implements ControllerResolverInterface ) ); } } + + private function getUriWithoutQuery( Request $request ) + { + $uri = $request->getUri(); + $queryPos = strpos( $uri, '?' ); + if ( $queryPos !== false ) { + $uri = substr( $uri, 0, $queryPos ); + } + return $uri; + } } ?> diff --git a/test/Activities/NonActivityHandlerTest.php b/test/Activities/NonActivityHandlerTest.php new file mode 100644 index 0000000..b892266 --- /dev/null +++ b/test/Activities/NonActivityHandlerTest.php @@ -0,0 +1,14 @@ +assertTrue( false ); + } +} +?> diff --git a/test/Auth/AuthListenerTest.php b/test/Auth/AuthListenerTest.php index a98ebeb..38086eb 100644 --- a/test/Auth/AuthListenerTest.php +++ b/test/Auth/AuthListenerTest.php @@ -2,6 +2,9 @@ namespace ActivityPub\Test\Auth; use ActivityPub\Auth\AuthListener; +use ActivityPub\Objects\ObjectsService; +use ActivityPub\Entities\ActivityPubObject; +use ActivityPub\Test\TestUtils\TestUtils; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -9,6 +12,20 @@ use PHPUnit\Framework\TestCase; class AuthListenerTest extends TestCase { + private $objectsService; + + public function setUp() + { + $this->objectsService = $this->createMock( ObjectsService::class ); + $this->objectsService->method( 'dereference' )->will( $this->returnValueMap( array( + array( 'https://example.com/actor/1', TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/1', + ) ) ), + array( 'https://example.com/actor/2', TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/2', + ) ) ), + ) ) ); + } public function getEvent() { @@ -28,7 +45,9 @@ class AuthListenerTest extends TestCase return 'https://example.com/actor/1'; }, 'expectedAttributes' => array( - 'actor' => 'https://example.com/actor/1', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/1', + ) ), ), ), array( @@ -37,10 +56,14 @@ class AuthListenerTest extends TestCase return 'https://example.com/actor/1'; }, 'requestAttributes' => array( - 'actor' => 'https://example.com/actor/2', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/2', + ) ), ), 'expectedAttributes' => array( - 'actor' => 'https://example.com/actor/2', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/2', + ) ), ), ), array( @@ -58,13 +81,30 @@ class AuthListenerTest extends TestCase $event->getRequest()->attributes->set( $attribute, $value ); } } - $authListener = new AuthListener( $testCase['authFunction'] ); - $authListener->checkAuth( $event ); - $this->assertEquals( - $testCase['expectedAttributes'], - $event->getRequest()->attributes->all(), - "Error on test $testCase[id]" + $authListener = new AuthListener( + $testCase['authFunction'], $this->objectsService ); + $authListener->checkAuth( $event ); + foreach ( $testCase['expectedAttributes'] as $expectedKey => $expectedValue ) { + $this->assertTrue( + $event->getRequest()->attributes->has( $expectedKey ), + "Error on test $testCase[id]" + ); + if ( $expectedValue instanceof ActivityPubObject ) { + $this->assertTrue( + $expectedValue->equals( + $event->getRequest()->attributes->get( $expectedKey ) + ), + "Error on test $testCase[id]" + ); + } else { + $this->assertEquals( + $expectedValue, + $event->getRequest()->attributes->get( $expectedKey ), + "Error on test $testCase[id]" + ); + } + } } } } diff --git a/test/Auth/SignatureListenerTest.php b/test/Auth/SignatureListenerTest.php index 0ec3590..81a7454 100644 --- a/test/Auth/SignatureListenerTest.php +++ b/test/Auth/SignatureListenerTest.php @@ -16,6 +16,8 @@ use PHPUnit\Framework\TestCase; class SignatureListenerTest extends TestCase { + const ACTOR_ID = 'https://example.com/actor/1'; + const ACTOR = array( 'id' => self::ACTOR_ID ); const KEY_ID = 'https://example.com/actor/1/key'; const PUBLIC_KEY = "-----BEGIN PUBLIC KEY----- MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDCFENGw33yGihy92pDjZQhl0C3 @@ -42,7 +44,8 @@ oYi+1hqp1fIekaxsyQIDAQAB $objectsService = $this->createMock( ObjectsService::class ); $objectsService->method( 'dereference' ) ->will( $this->returnValueMap( array( - array( self::KEY_ID, TestUtils::objectFromArray( self::KEY ) ) + array( self::KEY_ID, TestUtils::objectFromArray( self::KEY ) ), + array( self::ACTOR_ID, TestUtils::objectFromArray( self::ACTOR ) ), ) ) ); $this->signatureListener = new SignatureListener( $httpSignatureService, $objectsService @@ -82,8 +85,9 @@ oYi+1hqp1fIekaxsyQIDAQAB ), 'expectedAttributes' => array( 'signed' => true, - 'signedBy' => 'https://example.com/actor/1', - 'actor' => 'https://example.com/actor/1', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/1', + ) ), ), ), array( @@ -92,12 +96,15 @@ oYi+1hqp1fIekaxsyQIDAQAB 'Authorization' => 'Signature keyId="https://example.com/actor/1/key",algorithm="rsa-sha256",headers="(request-target) host date", signature="qdx+H7PHHDZgy4y/Ahn9Tny9V3GP6YgBPyUXMmoxWtLbHpUnXS2mg2+SbrQDMCJypxBLSPQR2aAjn7ndmw2iicw3HMbe8VfEdKFYRqzic+efkb3nndiv/x1xSHDJWeSWkx3ButlYSuBskLu6kd9Fswtemr3lgdDEmn04swr2Os0="', ), 'requestAttributes' => array( - 'actor' => 'https://example.com/actor/2', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/2', + ) ), ), 'expectedAttributes' => array( 'signed' => true, - 'signedBy' => 'https://example.com/actor/1', - 'actor' => 'https://example.com/actor/2', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/2', + ) ), ), ), array( @@ -107,8 +114,9 @@ oYi+1hqp1fIekaxsyQIDAQAB ), 'expectedAttributes' => array( 'signed' => true, - 'signedBy' => 'https://example.com/actor/1', - 'actor' => 'https://example.com/actor/1', + 'actor' => TestUtils::objectFromArray( array( + 'id' => 'https://example.com/actor/1', + ) ), ), ), array( @@ -129,11 +137,27 @@ oYi+1hqp1fIekaxsyQIDAQAB } } $this->signatureListener->validateHttpSignature( $event ); - $this->assertEquals( - $testCase['expectedAttributes'], - $event->getRequest()->attributes->all(), - "Error on test $testCase[id]" - ); + foreach ( $testCase['expectedAttributes'] as $expectedKey => $expectedValue ) { + $this->assertTrue( + $event->getRequest()->attributes->has( $expectedKey ), + "Error on test $testCase[id]" + ); + xdebug_break(); + if ( $expectedValue instanceof ActivityPubObject ) { + $this->assertTrue( + $expectedValue->equals( + $event->getRequest()->attributes->get( $expectedKey ) + ), + "Error on test $testCase[id]" + ); + } else { + $this->assertEquals( + $expectedValue, + $event->getRequest()->attributes->get( $expectedKey ), + "Error on test $testCase[id]" + ); + } + } } } } diff --git a/test/Controllers/InboxControllerTest.php b/test/Controllers/InboxControllerTest.php new file mode 100644 index 0000000..363e8a6 --- /dev/null +++ b/test/Controllers/InboxControllerTest.php @@ -0,0 +1,14 @@ +assertTrue( false ); + } +} +?> diff --git a/test/Controllers/OutboxControllerTest.php b/test/Controllers/OutboxControllerTest.php new file mode 100644 index 0000000..25b5aaa --- /dev/null +++ b/test/Controllers/OutboxControllerTest.php @@ -0,0 +1,14 @@ +assertTrue( false ); + } +} +?> diff --git a/test/Http/ControllerResolverTest.php b/test/Http/ControllerResolverTest.php index d5ccb05..63209b3 100644 --- a/test/Http/ControllerResolverTest.php +++ b/test/Http/ControllerResolverTest.php @@ -90,13 +90,8 @@ class ControllerResolverTest extends TestCase ); $controller = $this->controllerResolver->getController( $request ); $this->assertTrue( $request->attributes->has( 'activity' ) ); - $this->assertEquals( array( 'type' => 'Foo' ), $request->attributes->get( 'activity' ) ); - $this->assertTrue( $request->attributes->has( 'inbox' ) ); $this->assertEquals( - array( - 'id' => 'https://example.com/actor/1/inbox', - ), - $request->attributes->get( 'inbox' )->asArray() + array( 'type' => 'Foo' ), $request->attributes->get( 'activity' ) ); $this->assertEquals( array( $this->inboxController, 'handle' ), $controller ); } @@ -108,13 +103,8 @@ class ControllerResolverTest extends TestCase ); $controller = $this->controllerResolver->getController( $request ); $this->assertTrue( $request->attributes->has( 'activity' ) ); - $this->assertEquals( array( 'type' => 'Foo' ), $request->attributes->get( 'activity' ) ); - $this->assertTrue( $request->attributes->has( 'outbox' ) ); $this->assertEquals( - array( - 'id' => 'https://example.com/actor/1/outbox', - ), - $request->attributes->get( 'outbox' )->asArray() + array( 'type' => 'Foo' ), $request->attributes->get( 'activity' ) ); $this->assertEquals( array( $this->outboxController, 'handle' ), $controller ); }