From 0aa41b3f5618315e7a92a9d0b26707346a5d1434 Mon Sep 17 00:00:00 2001 From: Calvin Alkan Date: Sun, 13 Nov 2022 15:08:51 +0100 Subject: [PATCH] feat(wp-nonce-middleware): split middleware into two responsibilities Fix: #167 Fix: #159 --- .../src/EventMapping/EventMapper.php | 2 +- .../tests/wordpress/EventMapperTest.php | 589 +++++++++--------- .../src/Middleware/Middleware.php | 24 +- .../UrlGenerator/UrlGenerationContextTest.php | 11 +- .../Component/signed-url/src/UrlSigner.php | 134 ++-- .../signed-url/tests/UrlSignerTest.php | 4 +- .../wp-nonce/src/Exception/InvalidWPNonce.php | 20 + .../src/Middleware/AddWPNonceToView.php | 37 ++ .../wp-nonce/src/Middleware/CheckWPNonce.php | 45 ++ .../Middleware/wp-nonce/src/VerifyWPNonce.php | 41 +- .../Middleware/wp-nonce/src/WPNonce.php | 5 +- .../wp-nonce/tests/VerifyWPNonceTest.php | 27 +- 12 files changed, 516 insertions(+), 423 deletions(-) create mode 100644 src/Snicco/Middleware/wp-nonce/src/Exception/InvalidWPNonce.php create mode 100644 src/Snicco/Middleware/wp-nonce/src/Middleware/AddWPNonceToView.php create mode 100644 src/Snicco/Middleware/wp-nonce/src/Middleware/CheckWPNonce.php diff --git a/src/Snicco/Component/better-wp-hooks/src/EventMapping/EventMapper.php b/src/Snicco/Component/better-wp-hooks/src/EventMapping/EventMapper.php index 4ca5fdbcb..2469be882 100644 --- a/src/Snicco/Component/better-wp-hooks/src/EventMapping/EventMapper.php +++ b/src/Snicco/Component/better-wp-hooks/src/EventMapping/EventMapper.php @@ -176,7 +176,7 @@ private function dispatchMappedFilter(string $event_class): callable return function (...$args_from_wordpress_hooks) use ($event_class) { $event = $this->event_factory->make($event_class, $args_from_wordpress_hooks); - if (! array_key_exists(0,$args_from_wordpress_hooks)) { + if (! array_key_exists(0, $args_from_wordpress_hooks)) { // @codeCoverageIgnoreStart throw new RuntimeException( sprintf('Event mapper received invalid arguments from WP for mapped hook [%s].', $event_class), diff --git a/src/Snicco/Component/better-wp-hooks/tests/wordpress/EventMapperTest.php b/src/Snicco/Component/better-wp-hooks/tests/wordpress/EventMapperTest.php index a5847768d..f8b68d919 100644 --- a/src/Snicco/Component/better-wp-hooks/tests/wordpress/EventMapperTest.php +++ b/src/Snicco/Component/better-wp-hooks/tests/wordpress/EventMapperTest.php @@ -23,10 +23,8 @@ use function add_filter; use function apply_filters; use function apply_filters_ref_array; -use function cli\render; use function do_action; use function implode; -use function is_null; use function sprintf; use const PHP_INT_MAX; @@ -39,402 +37,401 @@ */ final class EventMapperTest extends WPTestCase { - private EventMapper $event_mapper; - + private BaseEventDispatcher $dispatcher; - - protected function setUp() :void + + protected function setUp(): void { parent::setUp(); $this->dispatcher = new BaseEventDispatcher(new NewableListenerFactory()); $this->event_mapper = new EventMapper($this->dispatcher, new WPHookAPI()); } - + /** * ACTIONS. */ - + /** * @test */ - public function mapped_actions_only_dispatch_for_their_hook() :void + public function mapped_actions_only_dispatch_for_their_hook(): void { $this->event_mapper->map('foo', EmptyActionEvent::class); - + $run = false; - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$run) :void { + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$run): void { $run = true; }); - + do_action('bogus'); - + $this->assertFalse($run); } - + /** * @test */ - public function that_a_wordpress_action_can_be_mapped_to_a_custom_event_and_the_event_will_dispatch() :void + public function that_a_wordpress_action_can_be_mapped_to_a_custom_event_and_the_event_will_dispatch(): void { $this->event_mapper->map('empty', EmptyActionEvent::class); - + $run = false; - $this->dispatcher->listen(EmptyActionEvent::class, function (EmptyActionEvent $event) use (&$run) :void { + $this->dispatcher->listen(EmptyActionEvent::class, function (EmptyActionEvent $event) use (&$run): void { $this->assertSame('foo', $event->value); $run = true; }); - + do_action('empty'); - + $this->assertTrue($run); } - + /** * @test */ - public function that_arguments_from_actions_are_passed_to_the_event() :void + public function that_arguments_from_actions_are_passed_to_the_event(): void { $this->event_mapper->map('foo_action', FooActionEvent::class); - + $run = false; - $this->dispatcher->listen(function (FooActionEvent $event) use (&$run) :void { + $this->dispatcher->listen(function (FooActionEvent $event) use (&$run): void { $this->assertSame('foobarbaz', $event->value()); $run = true; }); - + do_action('foo_action', 'foo', 'bar', 'baz'); - + $this->assertTrue($run); - + $run = false; - + $this->event_mapper->map('foo_action_array', ActionWithArrayArguments::class); - - $this->dispatcher->listen(function (ActionWithArrayArguments $event) use (&$run) :void { + + $this->dispatcher->listen(function (ActionWithArrayArguments $event) use (&$run): void { $this->assertSame('foo|bar:baz', $event->message); $run = true; }); do_action('foo_action_array', ['foo', 'bar'], 'baz'); } - + /** * @test */ - public function that_null_can_be_passed_as_arguments() :void + public function that_null_can_be_passed_as_arguments(): void { $this->event_mapper->map('null_filter', ActionWithNullArguments::class); - + $run = false; - $this->dispatcher->listen(function (ActionWithNullArguments $event) use (&$run) :void { + $this->dispatcher->listen(function (ActionWithNullArguments $event) use (&$run): void { $this->assertSame('baz', $event->filterableAttribute()); $run = true; }); - + $value = apply_filters_ref_array('null_filter', [null, null, 'baz']); - + $this->assertSame('baz', $value); $this->assertTrue($run); } - + /** * @test */ - public function events_mapped_to_a_wordpress_action_are_passed_by_reference_between_listeners() :void + public function events_mapped_to_a_wordpress_action_are_passed_by_reference_between_listeners(): void { $this->event_mapper->map('empty', EmptyActionEvent::class); - + $run = false; - - $this->dispatcher->listen(function (EmptyActionEvent $event) :void { + + $this->dispatcher->listen(function (EmptyActionEvent $event): void { $event->value = 'foobar'; }); - - $this->dispatcher->listen(function (EmptyActionEvent $event) use (&$run) :void { + + $this->dispatcher->listen(function (EmptyActionEvent $event) use (&$run): void { $this->assertSame('foobar', $event->value); $run = true; }); - + do_action('empty'); - + $this->assertTrue($run); } - + /** * @test */ - public function the_mapping_priority_can_be_customized() :void + public function the_mapping_priority_can_be_customized(): void { $count = 0; - add_action('empty', function () use (&$count) :void { + add_action('empty', function () use (&$count): void { ++$count; }, 5); - + $this->event_mapper->map('empty', EmptyActionEvent::class, 4); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(0, $count, 'Priority mapping did not work correctly.'); }); - + do_action('empty'); - + $this->assertSame(1, $count); } - + /** * @test */ - public function two_different_custom_events_can_be_mapped_to_one_action() :void + public function two_different_custom_events_can_be_mapped_to_one_action(): void { $count = 0; - add_action('empty', function () use (&$count) :void { + add_action('empty', function () use (&$count): void { ++$count; }, 5); - + $this->event_mapper->map('empty', EmptyActionEvent::class, 4); $this->event_mapper->map('empty', EmptyActionEvent2::class, 6); - - $this->dispatcher->listen(function (EmptyActionEvent $event) use (&$count) :void { + + $this->dispatcher->listen(function (EmptyActionEvent $event) use (&$count): void { $this->assertSame(0, $count, 'Priority mapping did not work correctly.'); ++$count; }); - - $this->dispatcher->listen(function (EmptyActionEvent2 $event) use (&$count) :void { + + $this->dispatcher->listen(function (EmptyActionEvent2 $event) use (&$count): void { $this->assertSame(2, $count, 'Priority mapping did not work correctly.'); ++$count; }); - + do_action('empty'); - + $this->assertSame(3, $count); } - + /** * @test */ - public function mapped_actions_are_not_accessible_with_wordpress_plugin_functions() :void + public function mapped_actions_are_not_accessible_with_wordpress_plugin_functions(): void { $count = 0; - - add_action(EmptyActionEvent::class, function () use (&$count) :void { + + add_action(EmptyActionEvent::class, function () use (&$count): void { ++$count; }, 1); - + $this->event_mapper->map('action', EmptyActionEvent::class); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(0, $count); ++$count; }); - + do_action('action'); - + $this->assertSame(1, $count); } - + /** * FILTERS. */ - + /** * @test */ - public function mapped_filters_only_dispatch_for_their_hook() :void + public function mapped_filters_only_dispatch_for_their_hook(): void { $this->event_mapper->map('foo', EmptyActionEvent::class); - - $this->dispatcher->listen(EmptyActionEvent::class, function (EmptyActionEvent $event) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function (EmptyActionEvent $event): void { $event->value = 'bar'; }); - + $res = apply_filters('bogus', 'foo'); - + $this->assertSame('foo', $res); } - + /** * @test */ - public function a_wordpress_filter_can_be_mapped_to_a_custom_event() :void + public function a_wordpress_filter_can_be_mapped_to_a_custom_event(): void { $this->event_mapper->map('filter', EventFilterWithNoArgs::class); - - $this->dispatcher->listen(function (EventFilterWithNoArgs $event) :void { + + $this->dispatcher->listen(function (EventFilterWithNoArgs $event): void { $event->filterable_value .= 'bar'; }); - $this->dispatcher->listen(function (EventFilterWithNoArgs $event) :void { + $this->dispatcher->listen(function (EventFilterWithNoArgs $event): void { $event->filterable_value .= 'baz'; }); - + $final_value = apply_filters('filter', 'foo'); - + $this->assertSame('foobarbaz', $final_value); } - + /** * @test */ - public function the_priority_can_be_customized_for_a_mapped_filter() :void + public function the_priority_can_be_customized_for_a_mapped_filter(): void { - add_filter('filter', fn(string $value) :string => $value.'_wp_filtered_1', 4, 1000); - - add_filter('filter', fn(string $value) :string => $value.'_wp_filtered_2', 6, 1000); - + add_filter('filter', fn (string $value): string => $value . '_wp_filtered_1', 4, 1000); + + add_filter('filter', fn (string $value): string => $value . '_wp_filtered_2', 6, 1000); + $this->event_mapper->map('filter', EventFilterWithNoArgs::class, 5); - - $this->dispatcher->listen(function (EventFilterWithNoArgs $event) :void { + + $this->dispatcher->listen(function (EventFilterWithNoArgs $event): void { $event->filterable_value .= 'bar'; }); - $this->dispatcher->listen(function (EventFilterWithNoArgs $event) :void { + $this->dispatcher->listen(function (EventFilterWithNoArgs $event): void { $event->filterable_value .= 'baz'; }); - + $final_value = apply_filters('filter', 'foo'); - + $this->assertSame('foo_wp_filtered_1barbaz_wp_filtered_2', $final_value); } - + /** * @test */ - public function two_different_custom_events_can_be_mapped_to_a_wordpress_filter() :void + public function two_different_custom_events_can_be_mapped_to_a_wordpress_filter(): void { - add_filter('filter', fn(string $value) :string => $value.'_wp_filtered_1_', 4, 1000); - - add_filter('filter', fn(string $value) :string => $value.'_wp_filtered_2_', 6, 1000); - + add_filter('filter', fn (string $value): string => $value . '_wp_filtered_1_', 4, 1000); + + add_filter('filter', fn (string $value): string => $value . '_wp_filtered_2_', 6, 1000); + $this->event_mapper->map('filter', EventFilter1::class, 5); $this->event_mapper->map('filter', EventFilter2::class, 7); - - $this->dispatcher->listen(function (EventFilter1 $event) :void { + + $this->dispatcher->listen(function (EventFilter1 $event): void { $event->foo .= $event->bar; }); - - $this->dispatcher->listen(function (EventFilter2 $event) :void { + + $this->dispatcher->listen(function (EventFilter2 $event): void { $event->foo .= $event->bar; }); - + $final_value = apply_filters('filter', 'foo', 'bar'); - + $this->assertSame('foo_wp_filtered_1_bar_wp_filtered_2_bar', $final_value); } - + /** * MAP_FIRST. */ - + /** * @test */ - public function test_map_first_if_no_other_callback_present() :void + public function test_map_first_if_no_other_callback_present(): void { $count = 0; - + $this->event_mapper->mapFirst('wp_hook', EmptyActionEvent::class); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { ++$count; }, PHP_INT_MIN); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(0, $count, 'Mapped Event did not run first.'); ++$count; }); - + do_action('wp_hook'); - + $this->assertSame(2, $count); } - + /** * @test */ - public function test_map_first_if_another_callback_is_registered_before_with_int_min() :void + public function test_map_first_if_another_callback_is_registered_before_with_int_min(): void { $count = 0; - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { ++$count; }, PHP_INT_MIN, 10); - + $this->event_mapper->mapFirst('wp_hook', EmptyActionEvent::class); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(0, $count, 'Mapped Event did not run first.'); ++$count; }); - + do_action('wp_hook'); - + $this->assertSame(2, $count); } - + /** * @test */ - public function test_map_first_if_another_callback_is_registered_before() :void + public function test_map_first_if_another_callback_is_registered_before(): void { $count = 0; - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { ++$count; }, 10, 1); - + $this->event_mapper->mapFirst('wp_hook', EmptyActionEvent::class); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(0, $count, 'Mapped Event did not run first.'); ++$count; }); - + do_action('wp_hook'); - + $this->assertSame(2, $count); } - + /** * @test */ - public function test_map_first_if_registered_with_php_int_min() :void + public function test_map_first_if_registered_with_php_int_min(): void { $count = 0; - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(1, $count); ++$count; }, PHP_INT_MIN); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(2, $count); ++$count; }, PHP_INT_MIN); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(3, $count); ++$count; }, 20); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(4, $count); ++$count; }, 50); - + $this->event_mapper->mapFirst('wp_hook', EmptyActionEvent::class); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(0, $count, 'Mapped Event did not run first.'); ++$count; }); - + do_action('wp_hook'); - + $this->assertSame(5, $count); } - + /** * @test */ - public function test_exception_if_filtering_during_the_same_filter() :void + public function test_exception_if_filtering_during_the_same_filter(): void { $this->expectException(LogicException::class); $this->expectExceptionMessage( @@ -443,92 +440,92 @@ public function test_exception_if_filtering_during_the_same_filter() :void EmptyActionEvent::class ) ); - - add_action('wp_hook', function () :void { + + add_action('wp_hook', function (): void { $this->event_mapper->mapFirst('wp_hook', EmptyActionEvent::class); }); - + do_action('wp_hook'); } - + /** * MAP_LAST. */ - + /** * @test */ - public function test_map_last_if_no_other_callback_present() :void + public function test_map_last_if_no_other_callback_present(): void { $count = 0; - + $this->event_mapper->mapLast('wp_hook', EmptyActionEvent::class); - - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { ++$count; }); - + do_action('wp_hook'); - + $this->assertSame(1, $count); } - + /** * @test */ - public function test_ensure_last_with_callback_added_after() :void + public function test_ensure_last_with_callback_added_after(): void { $count = 0; - + $this->event_mapper->mapLast('wp_hook', EmptyActionEvent::class); - $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count) :void { + $this->dispatcher->listen(EmptyActionEvent::class, function () use (&$count): void { $this->assertSame(4, $count, 'Mapped Event did not run last.'); ++$count; }); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(0, $count); ++$count; }, 50); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(1, $count); ++$count; }, 100); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(2, $count); ++$count; }, 200); - - add_action('wp_hook', function () use (&$count) :void { + + add_action('wp_hook', function () use (&$count): void { $this->assertSame(3, $count); ++$count; }, PHP_INT_MAX); - + do_action('wp_hook'); - + $this->assertSame(5, $count); } - + /** * VALIDATION. */ - + /** * @test */ - public function a_mapped_event_has_to_have_on_of_the_valid_interfaces() :void + public function a_mapped_event_has_to_have_on_of_the_valid_interfaces(): void { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('has to implement either'); $this->event_mapper->map('foobar', NormalEvent::class); } - + /** * @test */ - public function cant_map_the_same_filter_twice_to_the_same_custom_event() :void + public function cant_map_the_same_filter_twice_to_the_same_custom_event(): void { $this->expectException(LogicException::class); $this->expectExceptionMessage( @@ -537,11 +534,11 @@ public function cant_map_the_same_filter_twice_to_the_same_custom_event() :void $this->event_mapper->map('foobar', EventFilter1::class); $this->event_mapper->map('foobar', EventFilter1::class); } - + /** * @test */ - public function cant_map_the_same_action_twice_to_the_same_custom_event() :void + public function cant_map_the_same_action_twice_to_the_same_custom_event(): void { $this->expectException(LogicException::class); $this->expectExceptionMessage( @@ -550,97 +547,97 @@ public function cant_map_the_same_action_twice_to_the_same_custom_event() :void $this->event_mapper->map('foobar', FooActionEvent::class); $this->event_mapper->map('foobar', FooActionEvent::class); } - + /** * @test * * @psalm-suppress UndefinedClass * @psalm-suppress ArgumentTypeCoercion */ - public function cant_map_to_a_non_existing_class() :void + public function cant_map_to_a_non_existing_class(): void { $this->expectException(LogicException::class); $this->expectExceptionMessage(sprintf('The event class [%s] does not exist.', 'Bogus')); $this->event_mapper->map('foobar', 'Bogus'); } - + /** * @test */ - public function conditional_filters_are_prevented_if_conditions_dont_match() :void + public function conditional_filters_are_prevented_if_conditions_dont_match(): void { $this->event_mapper->map('filter', ConditionalFilter::class); - - $this->dispatcher->listen(ConditionalFilter::class, function (ConditionalFilter $event) :void { + + $this->dispatcher->listen(ConditionalFilter::class, function (ConditionalFilter $event): void { $event->value1 = 'CUSTOM'; }); - - add_filter('filter', function ($value1, $value2) :string { + + add_filter('filter', function ($value1, $value2): string { $this->assertSame('foo', $value1); $this->assertSame('PREVENT', $value2); - - return $value1.':filtered'; + + return $value1 . ':filtered'; }, 10, 2); - + $final_value = apply_filters('filter', 'foo', 'PREVENT'); - + $this->assertSame('foo:filtered', $final_value); } - + /** * @test */ - public function conditional_filter_events_are_dispatched_if_conditions_match() :void + public function conditional_filter_events_are_dispatched_if_conditions_match(): void { $this->event_mapper->map('filter', ConditionalFilter::class); - - $this->dispatcher->listen(ConditionalFilter::class, function (ConditionalFilter $event) :void { + + $this->dispatcher->listen(ConditionalFilter::class, function (ConditionalFilter $event): void { $event->value1 = 'CUSTOM'; }); - - add_filter('filter', function ($value1, $value2) :string { + + add_filter('filter', function ($value1, $value2): string { $this->assertSame('CUSTOM', $value1); $this->assertSame('bar', $value2); - - return $value1.':filtered'; + + return $value1 . ':filtered'; }, 10, 2); - + $final_value = apply_filters('filter', 'foo', 'bar'); - + $this->assertSame('CUSTOM:filtered', $final_value); } - + /** * @test * @psalm-suppress MixedAssignment * @psalm-suppress MixedOperand */ - public function conditional_actions_are_prevented_if_conditions_dont_match() :void + public function conditional_actions_are_prevented_if_conditions_dont_match(): void { $this->event_mapper->map('action', ConditionalAction::class); - + $count = 0; - $this->dispatcher->listen(ConditionalAction::class, function (ConditionalAction $event) :void { + $this->dispatcher->listen(ConditionalAction::class, function (ConditionalAction $event): void { $event->value = 'did run'; }); - - add_action('action', function (string $value) use (&$count) :void { + + add_action('action', function (string $value) use (&$count): void { $this->assertSame('PREVENT', $value); ++$count; }, 10, 2); - + do_action('action', 'PREVENT'); - + $this->assertSame(1, $count); } - + /** * @test */ - public function an_exception_is_thrown_if_the_provided_arguments_are_invalid_for_the_mapped_event() :void + public function an_exception_is_thrown_if_the_provided_arguments_are_invalid_for_the_mapped_event(): void { $this->event_mapper->map('foo_filter', EventFilter1::class); - + $this->expectException(CantCreateMappedEvent::class); $this->expectExceptionMessage( sprintf( @@ -648,289 +645,267 @@ public function an_exception_is_thrown_if_the_provided_arguments_are_invalid_for EventFilter1::class ) ); - + apply_filters_ref_array('foo_filter', ['foo', new stdClass()]); } - + /** * @test */ - public function test_exception_if_current_filter_is_removed_for_some_reason_during_on_the_fly_mapping() :void + public function test_exception_if_current_filter_is_removed_for_some_reason_during_on_the_fly_mapping(): void { $this->expectException(LogicException::class); $this->expectExceptionMessage('$current_filter should never be null during mapping.'); - + $this->event_mapper->mapLast('foo', EmptyActionEvent::class); - add_action('foo', function () :void { + add_action('foo', function (): void { $GLOBALS['wp_current_filter'] = []; }, 10); - + do_action('foo'); } - } final class ConditionalFilter implements MappedFilter { - use ClassAsName; use ClassAsPayload; - + public string $value1; - + private string $value2; - + public function __construct(string $value1, string $value2) { $this->value1 = $value1; $this->value2 = $value2; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return 'PREVENT' !== $this->value2; } - - public function filterableAttribute() :string + + public function filterableAttribute(): string { return $this->value1; } - } final class ConditionalAction implements MappedHook { - use ClassAsName; use ClassAsPayload; - + public string $value; - + public function __construct(string $value) { $this->value = $value; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return 'PREVENT' !== $this->value; } - } final class NormalEvent implements Event { - use ClassAsName; use ClassAsPayload; } final class EventFilterWithNoArgs implements MappedFilter { - use ClassAsName; use ClassAsPayload; - + public string $filterable_value; - + public function __construct(string $filterable_value) { $this->filterable_value = $filterable_value; } - - public function filterableAttribute() :string + + public function filterableAttribute(): string { return $this->filterable_value; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class EventFilter1 implements MappedFilter { - use ClassAsName; use ClassAsPayload; - + public string $foo; - + public string $bar; - + public function __construct(string $foo, string $bar) { $this->foo = $foo; $this->bar = $bar; } - - public function filterableAttribute() :string + + public function filterableAttribute(): string { return $this->foo; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class EventFilter2 implements MappedFilter { - use ClassAsName; use ClassAsPayload; - + public string $foo; - + public string $bar; - + public function __construct(string $foo, string $bar) { $this->foo = $foo; $this->bar = $bar; } - - public function filterableAttribute() :string + + public function filterableAttribute(): string { return $this->foo; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class EmptyActionEvent implements MappedHook { - use ClassAsName; use ClassAsPayload; - + public string $value = 'foo'; - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class EmptyActionEvent2 implements MappedHook { - use ClassAsName; use ClassAsPayload; - + public string $value = 'bar'; - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class FooActionEvent implements MappedHook { - use ClassAsName; use ClassAsPayload; - + private string $foo; - + private string $bar; - + private string $baz; - + public function __construct(string $foo, string $bar, string $baz) { $this->foo = $foo; $this->bar = $bar; $this->baz = $baz; } - - public function value() :string + + public function value(): string { - return $this->foo.$this->bar.$this->baz; + return $this->foo . $this->bar . $this->baz; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class ActionWithArrayArguments implements MappedHook { - use ClassAsName; use ClassAsPayload; - + public string $message; - + /** - * @param string[] $words + * @param string[] $words */ public function __construct(array $words, string $suffix) { - $this->message = implode('|', $words).':'.$suffix; + $this->message = implode('|', $words) . ':' . $suffix; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - } final class ActionWithNullArguments implements MappedFilter { - use ClassAsName; use ClassAsPayload; - + /** * @var null */ private $foo; - + /** * @var null */ private $bar; - + private string $baz; - + /** * @param mixed $foo * @param mixed $bar */ public function __construct($foo, $bar, string $baz) { - if ( ! is_null($foo) || ! is_null($bar)) { + if (null !== $foo || null !== $bar) { throw new InvalidArgumentException('Passed args should be NULL'); } - + $this->foo = $foo; $this->bar = $bar; $this->baz = $baz; } - - public function shouldDispatch() :bool + + public function shouldDispatch(): bool { return true; } - + public function filterableAttribute() { return $this->baz; } - } diff --git a/src/Snicco/Component/http-routing/src/Middleware/Middleware.php b/src/Snicco/Component/http-routing/src/Middleware/Middleware.php index 26eddecfe..b6eea126a 100644 --- a/src/Snicco/Component/http-routing/src/Middleware/Middleware.php +++ b/src/Snicco/Component/http-routing/src/Middleware/Middleware.php @@ -16,7 +16,6 @@ use Snicco\Component\HttpRouting\Http\Psr7\ResponseFactory; use Snicco\Component\HttpRouting\Http\ResponseUtils; use Snicco\Component\HttpRouting\Routing\UrlGenerator\UrlGenerator; - use Webmozart\Assert\Assert; use function sprintf; @@ -28,6 +27,8 @@ abstract class Middleware implements MiddlewareInterface private ?Request $current_request = null; /** + * @interal + * * @psalm-internal Snicco\Component\HttpRouting */ final public function setContainer(ContainerInterface $container): void @@ -35,8 +36,10 @@ final public function setContainer(ContainerInterface $container): void $this->container = $container; } - final public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface - { + final public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): ResponseInterface { $request = Request::fromPsr($request); if (! $handler instanceof NextMiddleware) { @@ -48,6 +51,21 @@ final public function process(ServerRequestInterface $request, RequestHandlerInt return $this->handle($request, $handler); } + /** + * This method can be used to add the container of the current middleware to + * a (private) middleware that is instantiated from within the current one. + * + * @interal + * + * @experimental + * + * @psalm-internal Snicco + */ + final protected function setContainerOnInnerMiddleware(Middleware $other): void + { + $other->setContainer($this->container); + } + abstract protected function handle(Request $request, NextMiddleware $next): ResponseInterface; final protected function url(): UrlGenerator diff --git a/src/Snicco/Component/http-routing/tests/Routing/UrlGenerator/UrlGenerationContextTest.php b/src/Snicco/Component/http-routing/tests/Routing/UrlGenerator/UrlGenerationContextTest.php index aac8e2281..2714112b8 100644 --- a/src/Snicco/Component/http-routing/tests/Routing/UrlGenerator/UrlGenerationContextTest.php +++ b/src/Snicco/Component/http-routing/tests/Routing/UrlGenerator/UrlGenerationContextTest.php @@ -32,7 +32,7 @@ public function test_properties(): void $this->assertSame(4000, $context->httpsPort()); $this->assertFalse($context->httpsByDefault()); } - + /** * @test * @@ -41,21 +41,20 @@ public function test_properties(): void public function test_host_does_not_need_a_dot(): void { $context = new UrlGenerationContext('nginx'); - + $this->assertSame('nginx', $context->host()); $this->assertSame(80, $context->httpPort()); $this->assertSame(443, $context->httpsPort()); $this->assertTrue($context->httpsByDefault()); - + $context = new UrlGenerationContext('nginx', 4000, 8080, false); - + $this->assertSame('nginx', $context->host()); $this->assertSame(8080, $context->httpPort()); $this->assertSame(4000, $context->httpsPort()); $this->assertFalse($context->httpsByDefault()); } - - + /** * @test */ diff --git a/src/Snicco/Component/signed-url/src/UrlSigner.php b/src/Snicco/Component/signed-url/src/UrlSigner.php index 9c59bf578..df158bc21 100644 --- a/src/Snicco/Component/signed-url/src/UrlSigner.php +++ b/src/Snicco/Component/signed-url/src/UrlSigner.php @@ -22,21 +22,20 @@ final class UrlSigner { - private SignedUrlStorage $storage; - + private HMAC $hasher; - + public function __construct(SignedUrlStorage $storage, HMAC $hasher) { $this->storage = $storage; $this->hasher = $hasher; } - + /** - * @param non-empty-string $target - * @param positive-int $lifetime_in_sec - * @param positive-int $max_usage + * @param non-empty-string $target + * @param positive-int $lifetime_in_sec + * @param positive-int $max_usage * * @throws Exception if random_bytes can't be generated * @throws UnavailableStorage @@ -46,157 +45,156 @@ public function sign( int $lifetime_in_sec, int $max_usage = 1, string $request_context = '' - ) :SignedUrl { + ): SignedUrl { Assert::notEmpty($target); - + $target = $this->normalizeTarget($target); - + $parts = $this->parseUrl($target); - + $path = $this->getPath($parts); $query = $this->getQueryString($parts); $expires_at = $this->getExpiryTimestamp($lifetime_in_sec); - - $path_with_query = $query ? ($path.'?'.$query) : $path; - + + $path_with_query = $query ? ($path . '?' . $query) : $path; + // We create a random identifier for storing the signed url usage limit. // We don't store the signature or a hash of it. $identifier = Base64UrlSafe::encode(random_bytes(16)); - + // The signature consists of the identifier, the request context the developer passed // (such as ip address or user agent) and the protected path with an expires_at query parameter. $plain_text_signature = - $identifier. - $request_context. + $identifier . + $request_context . $this->appendExpiryQueryParam($path_with_query, $expires_at); - + $signature = Base64UrlSafe::encode($this->hasher->create($plain_text_signature)); - + // We append the expires_at and signature timestamp to the path, so that it can be easily validated. // If any of the plain text parts have been tampered validation wil fail. $path_with_query = $this->appendExpiryQueryParam($path_with_query, $expires_at) - .'&' - .SignedUrl::SIGNATURE_KEY - .'=' - .$identifier - .$signature; - + . '&' + . SignedUrl::SIGNATURE_KEY + . '=' + . $identifier + . $signature; + $url = ($domain_and_scheme = $this->getHostAndScheme($parts)) - ? $domain_and_scheme.$path_with_query + ? $domain_and_scheme . $path_with_query : $path_with_query; - + $signed_url = SignedUrl::create($url, $target, $identifier, $expires_at, $max_usage); - + $this->storage->store($signed_url); - + return $signed_url; } - - private function normalizeTarget(string $protect) :string + + private function normalizeTarget(string $protect): string { if (0 === strpos($protect, 'http')) { return $protect; } - - return '/'.ltrim($protect, '/'); + + return '/' . ltrim($protect, '/'); } - - private function parseUrl(string $protect) :array + + private function parseUrl(string $protect): array { $parts = ($parts = parse_url($protect)) ? $parts : []; - - if ( ! isset($parts['path']) && isset($parts['host'], $parts['scheme'])) { + + if (! isset($parts['path']) && isset($parts['host'], $parts['scheme'])) { $parts['path'] = '/'; } - + $this->validateUrlParts($parts, $protect); - + return $parts; } - - private function validateUrlParts(array $parsed, string $target) :void + + private function validateUrlParts(array $parsed, string $target): void { if ('/' === $parsed['path'] && '/' !== $target && 0 !== strpos($target, 'http')) { throw new InvalidArgumentException(sprintf('%s is not a valid path.', $target)); } - + /** @var string $qs */ $qs = $parsed['query'] ?? ''; - + parse_str($qs, $query); - + if (isset($query[SignedUrl::EXPIRE_KEY])) { throw new LogicException( "The expires query parameter is reserved.\nPlease rename your query parameter." ); } - + if (isset($query[SignedUrl::SIGNATURE_KEY])) { throw new LogicException( "The signature query parameter is reserved.\nPlease rename your query parameter." ); } } - + /** * @psalm-suppress MixedReturnStatement * @psalm-suppress MixedInferredReturnType */ - private function getPath(array $parts) :string + private function getPath(array $parts): string { if (isset($parts['path'])) { return $parts['path']; } - + // Should not be possible ever. // @codeCoverageIgnoreStart throw new InvalidArgumentException('Invalid path provided.'); // @codeCoverageIgnoreEnd } - + /** * @psalm-suppress MixedArgument */ - private function getQueryString(array $parts) :?string + private function getQueryString(array $parts): ?string { - if ( ! isset($parts['query'])) { + if (! isset($parts['query'])) { return null; } - + return rtrim($parts['query'], '&'); } - - private function getExpiryTimestamp(int $lifetime_in_sec) :int + + private function getExpiryTimestamp(int $lifetime_in_sec): int { return time() + $lifetime_in_sec; } - - private function appendExpiryQueryParam(string $path, int $expires_at) :string + + private function appendExpiryQueryParam(string $path, int $expires_at): string { - if ( ! strpos($path, '?')) { - return $path.'?'.SignedUrl::EXPIRE_KEY.'='.(string)$expires_at; + if (! strpos($path, '?')) { + return $path . '?' . SignedUrl::EXPIRE_KEY . '=' . (string) $expires_at; } - - return rtrim($path, '&').'&'.SignedUrl::EXPIRE_KEY.'='.(string)$expires_at; + + return rtrim($path, '&') . '&' . SignedUrl::EXPIRE_KEY . '=' . (string) $expires_at; } - + /** * @psalm-suppress MixedOperand */ - private function getHostAndScheme(array $parts) :?string + private function getHostAndScheme(array $parts): ?string { if (isset($parts['host'], $parts['scheme'])) { - $scheme_and_domain = $parts['scheme'].'://'.$parts['host']; - + $scheme_and_domain = $parts['scheme'] . '://' . $parts['host']; + if (isset($parts['port'])) { - $scheme_and_domain .= ':'.$parts['port']; + $scheme_and_domain .= ':' . $parts['port']; } - + return $scheme_and_domain; } - + return null; } - } diff --git a/src/Snicco/Component/signed-url/tests/UrlSignerTest.php b/src/Snicco/Component/signed-url/tests/UrlSignerTest.php index 45459c405..d9c3c6c3d 100644 --- a/src/Snicco/Component/signed-url/tests/UrlSignerTest.php +++ b/src/Snicco/Component/signed-url/tests/UrlSignerTest.php @@ -132,7 +132,7 @@ public function a_signed_url_can_be_created_for_a_full_url(): void $this->assertStringContainsString('signature=', $magic_link->asString()); $this->assertSame('https://foo.com/foo/', $magic_link->protects()); } - + /** * @test */ @@ -140,7 +140,7 @@ public function a_signed_url_can_be_created_with_non_standard_ports(): void { $magic_link = $this->url_signer->sign('https://foo.com:8443/foo/', 10); $this->assertInstanceOf(SignedUrl::class, $magic_link); - + $this->assertStringStartsWith('https://foo.com:8443/foo/', $magic_link->asString()); $this->assertStringContainsString('expires=', $magic_link->asString()); $this->assertStringContainsString('signature=', $magic_link->asString()); diff --git a/src/Snicco/Middleware/wp-nonce/src/Exception/InvalidWPNonce.php b/src/Snicco/Middleware/wp-nonce/src/Exception/InvalidWPNonce.php new file mode 100644 index 000000000..855e8a1ab --- /dev/null +++ b/src/Snicco/Middleware/wp-nonce/src/Exception/InvalidWPNonce.php @@ -0,0 +1,20 @@ +wp = $wp ?: new BetterWPAPI(); + } + + protected function handle(Request $request, NextMiddleware $next): ResponseInterface + { + $response = $next($request); + if (! $response instanceof ViewResponse) { + return $response; + } + + return $response->withViewData([ + self::WP_NONCE_VIEW_NAME => new WPNonce($this->url(), $this->wp, $request->path()), + ]); + } +} diff --git a/src/Snicco/Middleware/wp-nonce/src/Middleware/CheckWPNonce.php b/src/Snicco/Middleware/wp-nonce/src/Middleware/CheckWPNonce.php new file mode 100644 index 000000000..f6569b0db --- /dev/null +++ b/src/Snicco/Middleware/wp-nonce/src/Middleware/CheckWPNonce.php @@ -0,0 +1,45 @@ +wp = $wp ?: new BetterWPAPI(); + } + + public static function inputKey(): string + { + return md5(self::class); + } + + protected function handle(Request $request, NextMiddleware $next): ResponseInterface + { + if ($request->isReadVerb()) { + return $next($request); + } + + $current_path = $request->path(); + $nonce = (string) $request->post(self::inputKey(), ''); + + if (! $this->wp->verifyNonce($nonce, $current_path)) { + throw InvalidWPNonce::forPath($current_path); + } + + return $next($request); + } +} diff --git a/src/Snicco/Middleware/wp-nonce/src/VerifyWPNonce.php b/src/Snicco/Middleware/wp-nonce/src/VerifyWPNonce.php index e2cc1dbb3..545e5d623 100644 --- a/src/Snicco/Middleware/wp-nonce/src/VerifyWPNonce.php +++ b/src/Snicco/Middleware/wp-nonce/src/VerifyWPNonce.php @@ -7,15 +7,15 @@ use Psr\Http\Message\ResponseInterface; use Snicco\Component\BetterWPAPI\BetterWPAPI; use Snicco\Component\HttpRouting\Http\Psr7\Request; -use Snicco\Component\HttpRouting\Http\Response\ViewResponse; use Snicco\Component\HttpRouting\Middleware\Middleware; use Snicco\Component\HttpRouting\Middleware\NextMiddleware; -use Snicco\Component\Psr7ErrorHandler\HttpException; - -use function date; -use function sha1; -use function sprintf; +use Snicco\Middleware\WPNonce\Middleware\AddWPNonceToView; +use Snicco\Middleware\WPNonce\Middleware\CheckWPNonce; +/** + * @deprecated This middleware is deprecated in favor of {@see AddWPNonceToView} and {@see CheckWPNonce} as splitting + * these responsibilities into separate middleware has many benefits {@see https://github.com/snicco/snicco/issues/167}. + */ final class VerifyWPNonce extends Middleware { private BetterWPAPI $wp; @@ -27,31 +27,20 @@ public function __construct(BetterWPAPI $wp = null) public static function inputKey(): string { - $month = date('m'); - - return sha1(self::class . $month); + return CheckWPNonce::inputKey(); } protected function handle(Request $request, NextMiddleware $next): ResponseInterface { - $current_path = $request->path(); - - if (! $request->isReadVerb()) { - $nonce = (string) $request->post(self::inputKey(), ''); - - if (! $this->wp->verifyNonce($nonce, $current_path)) { - throw new HttpException(401, sprintf('Nonce check failed for request path [%s].', $current_path)); - } - } - - $response = $next($request); + $check_nonce = new CheckWPNonce($this->wp); + $this->setContainerOnInnerMiddleware($check_nonce); - if (! $response instanceof ViewResponse) { - return $response; - } + $add_wp_nonce = new AddWPNonceToView($this->wp); + $this->setContainerOnInnerMiddleware($add_wp_nonce); - return $response->withViewData([ - 'wp_nonce' => new WPNonce($this->url(), $this->wp, $current_path), - ]); + return $add_wp_nonce->process( + $request, + new NextMiddleware(fn (Request $request) => $check_nonce->process($request, $next)) + ); } } diff --git a/src/Snicco/Middleware/wp-nonce/src/WPNonce.php b/src/Snicco/Middleware/wp-nonce/src/WPNonce.php index c933d1634..c9922f456 100644 --- a/src/Snicco/Middleware/wp-nonce/src/WPNonce.php +++ b/src/Snicco/Middleware/wp-nonce/src/WPNonce.php @@ -45,7 +45,10 @@ public function __invoke(string $route_name = null, array $args = []): string return $this->createNonce($nonce_action); } - + + /** + * @psalm-suppress DeprecatedClass + */ private function createNonce(string $nonce_action): string { $nonce = $this->noHtml($this->wp->createNonce($nonce_action)); diff --git a/src/Snicco/Middleware/wp-nonce/tests/VerifyWPNonceTest.php b/src/Snicco/Middleware/wp-nonce/tests/VerifyWPNonceTest.php index 79bf1fe31..27e4568d7 100644 --- a/src/Snicco/Middleware/wp-nonce/tests/VerifyWPNonceTest.php +++ b/src/Snicco/Middleware/wp-nonce/tests/VerifyWPNonceTest.php @@ -10,11 +10,14 @@ use Snicco\Component\HttpRouting\Routing\Route\Route; use Snicco\Component\HttpRouting\Testing\MiddlewareTestCase; use Snicco\Component\Psr7ErrorHandler\HttpException; +use Snicco\Middleware\WPNonce\Exception\InvalidWPNonce; use Snicco\Middleware\WPNonce\VerifyWPNonce; use Snicco\Middleware\WPNonce\WPNonce; /** * @internal + * + * @psalm-suppress DeprecatedClass */ final class VerifyWPNonceTest extends MiddlewareTestCase { @@ -46,16 +49,20 @@ public function test_with_invalid_nonce_for_same_url_throws_exception(): void VerifyWPNonce::inputKey() => $wp->createNonce('/bar'), ]); - $this->expectException(HttpException::class); - $this->expectExceptionMessage('Nonce check failed for request path [/foo]'); - - $this->runMiddleware($middleware, $request); + try { + $this->runMiddleware($middleware, $request); + $this->fail('Should have thrown exception.'); + } catch (HttpException $e) { + $this->assertSame('Nonce check failed for request path [/foo].', $e->getMessage()); + $this->assertSame(403, $e->statusCode()); + $this->assertInstanceOf(InvalidWPNonce::class, $e); + } } /** * @test */ - public function the_nonce_generator_is_added_for_reading_view_responses(): void + public function the_nonce_generator_is_added_for_view_responses(): void { $middleware = new VerifyWPNonce(new VerifyNonceTestWPApi()); @@ -79,7 +86,7 @@ public function the_nonce_generator_is_added_for_reading_view_responses(): void /** * @test */ - public function that_the_nonce_generator_is_added_to_valid_view_responses_for_post_requests_with_valid_nonces(): void + public function the_nonce_generator_is_added_for_post_requests(): void { $middleware = new VerifyWPNonce($wp = new VerifyNonceTestWPApi()); @@ -87,9 +94,11 @@ public function that_the_nonce_generator_is_added_to_valid_view_responses_for_po fn (Response $response): ViewResponse => new ViewResponse('foo', $response) ); - $request = $this->frontendRequest('/foo', [], 'POST')->withParsedBody([ - VerifyWPNonce::inputKey() => $wp->createNonce('/foo'), - ]); + $request = $this->frontendRequest('/foo') + ->withMethod('POST') + ->withParsedBody([ + VerifyWPNonce::inputKey() => $wp->createNonce('/foo'), + ]); $response = $this->runMiddleware($middleware, $request); $response->assertNextMiddlewareCalled();