Skip to content

Commit

Permalink
Merge pull request from GHSA-hph3-hv3c-7725
Browse files Browse the repository at this point in the history
* test: add reply creation tests

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* fix: access checking being bypassed for post creation when first post is deleted

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* chore: recover tests

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* chore: make provider public

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
  • Loading branch information
SychO9 committed Jan 10, 2023
1 parent 248a71d commit 12dfcc5
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function handle(StartDiscussion $command)
// We will do this by running the PostReply command.
try {
$post = $this->bus->dispatch(
new PostReply($discussion->id, $actor, $data, $ipAddress)
new PostReply($discussion->id, $actor, $data, $ipAddress, true)
);
} catch (Exception $e) {
$discussion->delete();
Expand Down
8 changes: 7 additions & 1 deletion framework/core/src/Post/Command/PostReply.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,23 @@ class PostReply
*/
public $ipAddress;

/**
* @var bool
*/
public $isFirstPost;

/**
* @param int $discussionId The ID of the discussion to post the reply to.
* @param User $actor The user who is performing the action.
* @param array $data The attributes to assign to the new post.
* @param string $ipAddress The IP address of the actor.
*/
public function __construct($discussionId, User $actor, array $data, $ipAddress = null)
public function __construct($discussionId, User $actor, array $data, $ipAddress = null, bool $isFirstPost = false)
{
$this->discussionId = $discussionId;
$this->actor = $actor;
$this->data = $data;
$this->ipAddress = $ipAddress;
$this->isFirstPost = $isFirstPost;
}
}
2 changes: 1 addition & 1 deletion framework/core/src/Post/Command/PostReplyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function handle(PostReply $command)

// If this is the first post in the discussion, it's technically not a
// "reply", so we won't check for that permission.
if ($discussion->first_post_id !== null) {
if (! $command->isFirstPost) {
$actor->assertCan('reply', $discussion);
}

Expand Down
47 changes: 41 additions & 6 deletions framework/core/tests/integration/api/posts/CreateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace Flarum\Tests\integration\api\posts;

use Carbon\Carbon;
use Flarum\Group\Group;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;

Expand All @@ -26,36 +27,70 @@ protected function setUp(): void

$this->prepareDatabase([
'discussions' => [
['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2],
['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1],
// Discussion with deleted first post.
['id' => 2, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => null],
],
'posts' => [
['id' => 1, 'discussion_id' => 1, 'number' => 1, 'created_at' => Carbon::now()->subDay()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '<t></t>'],
],
'users' => [
$this->normalUser(),
]
['id' => 3, 'username' => 'restricted', 'email' => 'restricted@machine.local', 'is_email_confirmed' => 1],
],
'groups' => [
['id' => 40, 'name_singular' => 'tess', 'name_plural' => 'tess'],
],
'group_user' => [
['group_id' => 40, 'user_id' => 3],
],
'group_permission' => [
['group_id' => 40, 'permission' => 'discussion.reply'],
],
]);
}

/**
* @dataProvider discussionRepliesPrvider
* @test
*/
public function can_create_reply()
public function can_create_reply_if_allowed(int $actorId, int $discussionId, int $responseStatus)
{
// Reset permissions for normal users group.
$this->database()
->table('group_permission')
->where('permission', 'discussion.reply')
->where('group_id', Group::MEMBER_ID)
->delete();

$response = $this->send(
$this->request('POST', '/api/posts', [
'authenticatedAs' => 2,
'authenticatedAs' => $actorId,
'json' => [
'data' => [
'attributes' => [
'content' => 'reply with predetermined content for automated testing - too-obscure',
],
'relationships' => [
'discussion' => ['data' => ['id' => 1]],
'discussion' => ['data' => ['id' => $discussionId]],
],
],
],
])
);

$this->assertEquals(201, $response->getStatusCode());
$this->assertEquals($responseStatus, $response->getStatusCode());
}

public function discussionRepliesPrvider(): array
{
return [
// [$actorId, $discussionId, $responseStatus]
'can_create_reply_with_ability' => [3, 1, 201],
'cannot_create_reply_without_ability' => [2, 1, 403],
'can_create_reply_with_ability_when_first_post_is_deleted' => [3, 2, 201],
'cannot_create_reply_without_ability_when_first_post_is_deleted' => [2, 2, 403],
];
}

/**
Expand Down

0 comments on commit 12dfcc5

Please sign in to comment.