User able to edit title of locked post (still/again possible)

This topic has been closed:

But on our install (very latest Discourse), this is still/again possible for a TL3+ user.

TL3 can alter OP title, category, and tags even on locked posts.

Admin locks post:

TL3 can still edit/alter the locked post:

Is this working as intended (hope not) or an old bug crawling around the code again?

1 Like

If I read this correctly, this is working as “intended” by the code at least:

https://github.com/discourse/discourse/blob/master/lib/guardian/topic_guardian.rb#L101

TL3 cannot edit only if topic is archived or topic is a PM.

If think you can change trusted_users_can_edit_others if you want to disable this behavior. AFAIK, the default for trusted_users_can_edit_others is not false to keep consistent behavior with how it was before this setting was introduced.

3 Likes

What is the purpose of locking posts then? There is none. Or I have not found one, thus the feature is either obsolete, dysfunctional, or at least confusing.

image

The text clearly says “a staff member has locked this post from being edited”. If locking has a completely different purpose than protecting the post/topic OP from being edited this text should be changed.

Locked posts should NOT be editable by non-staff users, despite the setting that high trust level posters can edit posts (which we like in general, but not for locked posts of course).

As it seems from the code below “locking” a post currently means that ONLY the OP of the post can’t edit the post anymore. TL3, TL4 (and staff) are still able to edit a locked post (if enabled in the settings).

This is very much limiting the locking feature to an extremely limited use case.

How do I protect a very important official admin announcement from one rogue TL3 user to be changed into “This is a Micky Mouse post” and being tagged the wrong way and moved into a non-relevant category, but still want to have TL3/TL4 user have the ability to edit posts in general?

Locked should mean locked (e.g. uneditable) for all besides staff.

  def can_edit_topic?(topic)
    return false if Discourse.static_doc_topic_ids.include?(topic.id) && !is_admin?
    return false unless can_see?(topic)

    return true if is_admin?
    return true if is_moderator? && can_create_post?(topic)

    # can't edit topics in secured categories where you don't have permission to create topics
    # except for a tiny edge case where the topic is uncategorized and you are trying
    # to fix it but uncategorized is disabled
    if (
      SiteSetting.allow_uncategorized_topics ||
      topic.category_id != SiteSetting.uncategorized_category_id
    )
      return false if !can_create_topic_on_category?(topic.category)
    end

    # TL4 users can edit archived topics, but can not edit private messages
    return true if (
      SiteSetting.trusted_users_can_edit_others? &&
      topic.archived &&
      !topic.private_message? &&
      user.has_trust_level?(TrustLevel[4]) &&
      can_create_post?(topic)
    )

    # TL3 users can not edit archived topics and private messages
    return true if (
      SiteSetting.trusted_users_can_edit_others? &&
      !topic.archived &&
      !topic.private_message? &&
      user.has_trust_level?(TrustLevel[3]) &&
      can_create_post?(topic)
    )

    return false if topic.archived
    is_my_own?(topic) &&
      !topic.edit_time_limit_expired?(user) &&
      **!Post.where(topic_id: topic.id, post_number: 1).where.not(locked_by_id: nil).exists?**
  end
2 Likes

And it gets more confusing…

The topic guardian allows TL3 and TL4 users to edit a locked topic/post (number 1 post), but the post guardian (see code below) disallows TL3 and TL4 users to edit a locked non-OP post:

def can_edit_post?(post)
if Discourse.static_doc_topic_ids.include?(post.topic_id) && !is_admin?
  return false
end

return true if is_admin?

# Must be staff to edit a locked post
return false if post.locked? && !is_staff?

@eviltrout is this how it is intended? More logical (and valuable as a locked feature) would be if the topic guardian would work the same way the post guardian respects the locked state IMHO. As said, I’m happy for TL3 and TL4 to edit non-locked posts, but not locked ones (this includes the first post).

1 Like

Worth looking at the rspec files.

1 Like

I did already. But just looking at how different both guardians treat the locked state is confusing in itself.

It should be the way how the post guardian is handling the locked state i.e. # Must be staff to edit a locked post

I mean currently locked non-first posts are really locked (TL3+ can’t edit/alter), and locked OP posts are not locked at all (besides for the OP, TL3+ can still edit/alter). This does not make sense at all to me, in particular because neither TL3 or TL4 have the permission to lock a post or topic.

In light of this, also locked OPs should be really locked from being editable by TL3+ users, despite the setting to allow them to edit (non-locked) posts and topics.

2 Likes

This does feel like a bug @joffreyjaffeux – a locked post should not be editable by anyone but staff, otherwise, what’s the point?

5 Likes

Thanks for acknowledging this @codinghorror

Yes, it’s a bug and both guardians should really lock a post regardless of the position and the setting that allows TL3 and TL4 to edit posts. I really gave my best to explain it in all the details.

1 Like

Can you assign this one @eviltrout? The current behavior doesn’t seem right to me.

1 Like

I opened a PR here

https://github.com/discourse/discourse/pull/10729

Merged, closing this topic.

5 Likes