PHPBB3-13264 - Editing an unapproved post as a moderator/admin approves it

[Lady_G]

I am attempting to fix [PHPBB3-13264] Editing an unapproved post as a moderator/admin approves it - phpBB Tracker, as this bug affects my board and is reported by another user in the support forum here: Edit topic that is awaiting approval

Environment: localhost, Linux 64 bit.
Version: cloned and running from the GitHub 3.1.x branch (following the Area 51 wiki for Bug fixing the 3.1.x branch)
Database: postgreSQL, newly created from the repo install

I believe the problem is in includes/functions_posting.php, the section starting at line 1620:

Code: Select all

	// This variable indicates if the user is able to post or put into the queue
	$post_visibility = ITEM_APPROVED;

	// Check the permissions for post approval.
	// Moderators must go through post approval like ordinary users.
	if (!$auth->acl_get('f_noapprove', $data['forum_id']))
	{
		// Post not approved, but in queue
		$post_visibility = ITEM_UNAPPROVED;
		switch ($post_mode)
		{
			case 'edit_first_post':
			case 'edit':
			case 'edit_last_post':
			case 'edit_topic':
				$post_visibility = ITEM_REAPPROVE;
			break;
		}
	}

The if() statement is never executed as !$auth->acl_get('f_noapprove', $data['forum_id'] is always 0. I can not find anywhere in the code that modifies 'f_noapprove'.

Forcing $post_visibility = ITEM_REAPPROVE; at this point appears to fix the problem. The post remains in the moderator queue, waiting for approve / disapprove. However, that is only one test case. I have tried several modifications, but cannot find a solution that works for all conditions.

Permissions are very tricky and require a deep understanding to attempt modifications. I do not have this understanding.

Can someone please provide guidance for solving this problem?

[RMcGirr83]

Try this

Code: Select all

if (!$auth->acl_get('f_noapprove', $data['forum_id']) || ($auth->acl_get('m_approve', $data['forum_id']) && (!$data['post_visibility'] || !$data['topic_visibility'])))

BTW, this has nothing to do with admin permissions. Strictly moderator permissions.

[Lady_G]

Thank you! That worked... for the first edit. Editing the post a second time caused the post to be approved (the bug remains).

I've inserted a trigger_error() after the if() to help debug.

New post - needs moderator approval:

Code: Select all

The post visibility is set to 'ITEM_UNAPPROVED', !(post visibility) = 1, !(topic visibility) = 1.

The post appears in the moderator queue as expected.

Now, the moderator / admin (with global moderator permissions) edits the post:

Code: Select all

The post visibility is set to = 'ITEM_REAPPROVE', !(post visibility) = 1, !(topic visibility) = 1.

The if() statement has executed and the post remains in the moderator queue.

The moderator / admin (with global moderator permissions) edits the post a second time:

Code: Select all

The post visibility is set to = 'ITEM_APPROVED',  !(post visibility) =   , !(topic visibility) =  .

The missing (empty?) visibility fields are causing the if() statement to be skipped. $post_visibility remains in its initial state = ITEM_APPROVED and the post is approved (bug).

There are 2 switch() statements to collect the data for the SQL transaction update:

Line 1653:

Code: Select all

	// Collect Information
	switch ($post_mode)
	{

Line 1754:

Code: Select all

	// And the topic ladies and gentlemen
	switch ($post_mode)
	{

I don't see any post visibility data collected for the switch() conditions:

Code: Select all

			case 'edit_first_post':
			case 'edit':
			case 'edit_last_post':
			case 'edit_topic':

Perhaps post visibility ('ITEM_REAPPROVE') needs to be part of the SQL transaction?

[Marc]

Please take a look at the pull request I've created on GitHub: https://github.com/phpbb/phpbb/pull/4205

The post will stay unapproved even if edited multiple times.

[Lady_G]

Yes, that works. However, the $url construction is broken. The edited post is not appending the post "#p" parameters.

The if() starting on line 2436 is not accounting for the different post visibilities. I tried several test cases and believe this will fix it:

Line 2436 changes:

- if ($post_visibility == ITEM_APPROVED)
+ if (($post_visibility == ITEM_APPROVED) || ($post_visibility == ITEM_UNAPPROVED) || ($post_visibility == ITEM_REAPPROVE))

Code: Select all

	if (($post_visibility == ITEM_APPROVED)  || ($post_visibility == ITEM_UNAPPROVED) || ($post_visibility == ITEM_REAPPROVE))
	{
		$params .= '&t=' . $data['topic_id'];

		if ($mode != 'post')
		{
			$params .= '&p=' . $data['post_id'];
			$add_anchor = '#p' . $data['post_id'];
		}
	}
	else if ($mode != 'post' && $post_mode != 'edit_first_post' && $post_mode != 'edit_topic')
	{
		$params .= '&t=' . $data['topic_id'];
	}

I don't know if this change will have any impact to the "else if" section.

[Marc]

The updated PR should also properly take care of that.

[Lady_G]

Yes, the updated Pull Request has properly taken care of the $url construction.

I have tested the file change on both the GitHub 3.1.x repository and my 3.1.8 production test area.

Moderator actions, logging, and notifications work as expected.

My testing included reapproval:

- A visible (previously approved) post edited by a member in the Newly registered users group will send the post back to the moderator queue for approval. It will no longer be visible to this member.

============
I cannot find any more problems and believe this fix is working (confirmed). Thank you.