Refactor redaction permission checks

src/service/pdu.rs

@@ -20,7 +20,7 @@ use serde_json::{
 };
 use tracing::warn;
 
-use crate::Error;
+use crate::{utils::room_version::RoomVersion, Error};
 
 /// Content hashes of a PDU.
 #[derive(Clone, Debug, Deserialize, Serialize)]
@@ -104,6 +104,35 @@ impl PduEvent {
         unsigned.redacted_because.is_some()
     }
 
+    /// Returns the redaction target of an event, if present.
+    ///
+    /// For versions <v11, the redaction target is a top-level property. For
+    /// versions >=v11, it is part of the room content.
+    // Allowed because we don't use state_res::Event anywhere in grapevine, it's
+    // just implemented so we can pass Pdu to ruma stateres.
+    #[expect(clippy::same_name_method)]
+    pub(crate) fn redacts(
+        &self,
+        room_version: &RoomVersion,
+    ) -> Option<Cow<'_, EventId>> {
+        if room_version.redaction_event_redacts_in_content {
+            #[derive(Deserialize)]
+            struct ExtractRedacts<'a> {
+                #[serde(borrow)]
+                redacts: Option<Cow<'a, EventId>>,
+            }
+            let extract =
+                serde_json::from_str::<ExtractRedacts<'_>>(self.content.get())
+                    .map_err(|_| {
+                        Error::bad_database("Invalid content in redaction pdu.")
+                    })
+                    .ok()?;
+            extract.redacts
+        } else {
+            self.redacts.as_ref().map(|e| Cow::Borrowed(&**e))
+        }
+    }
+
     pub(crate) fn remove_transaction_id(&mut self) -> crate::Result<()> {
         if let Some(unsigned) = &self.unsigned {
             let mut unsigned: BTreeMap<String, Box<RawJsonValue>> =

src/service/rooms/event_handler.rs

@@ -19,11 +19,8 @@ use ruma::{
         },
     },
     events::{
-        room::{
-            redaction::RoomRedactionEventContent,
-            server_acl::RoomServerAclEventContent,
-        },
-        StateEventType, TimelineEventType,
+        room::server_acl::RoomServerAclEventContent, StateEventType,
+        TimelineEventType,
     },
     int,
     state_res::{self, StateMap},
@@ -43,7 +40,7 @@ use super::{
 use crate::{
     service::{globals::SigningKeys, pdu, rooms::state::ExtractVersion},
     services,
-    utils::{debug_slice_truncated, room_version::RoomVersion},
+    utils::debug_slice_truncated,
     Error, PduEvent, Result,
 };
 
@@ -568,7 +565,6 @@ impl Service {
             "Upgrading event to timeline pdu",
         );
 
-        let room_version = RoomVersion::try_from(room_version_id)?;
         let ruma_room_version = state_res::RoomVersion::new(room_version_id)
             .map_err(|_| {
                 Error::UnsupportedRoomVersion(room_version_id.clone())
@@ -925,33 +921,10 @@ impl Service {
             Error::BadRequest(ErrorKind::InvalidParam, "Auth check failed.")
         })? || incoming_pdu.kind
             == TimelineEventType::RoomRedaction
-            && if room_version.redaction_event_redacts_in_content {
-                let content =
-                    serde_json::from_str::<RoomRedactionEventContent>(
-                        incoming_pdu.content.get(),
-                    )
-                    .map_err(|_| {
-                        Error::bad_database("Invalid content in redaction pdu.")
-                    })?;
-
-                if let Some(redact_id) = &content.redacts {
-                    !services().rooms.state_accessor.user_can_redact(
-                        redact_id,
-                        &incoming_pdu.sender,
-                        &incoming_pdu.room_id,
-                    )?
-                } else {
-                    false
-                }
-            } else if let Some(redact_id) = &incoming_pdu.redacts {
-                !services().rooms.state_accessor.user_can_redact(
-                    redact_id,
-                    &incoming_pdu.sender,
-                    &incoming_pdu.room_id,
-                )?
-            } else {
-                false
-            };
+            && services()
+                .rooms
+                .state_accessor
+                .redaction_event_allowed(&incoming_pdu)?;
 
         // 13. Use state resolution to find new room state
 

src/service/rooms/state_accessor.rs

@@ -14,7 +14,7 @@ use ruma::{
             name::RoomNameEventContent,
             power_levels::{RoomPowerLevels, RoomPowerLevelsEventContent},
         },
-        StateEventType,
+        StateEventType, TimelineEventType,
     },
     state_res::Event,
     EventId, OwnedRoomId, OwnedServerName, OwnedUserId, RoomId, ServerName,
@@ -26,9 +26,9 @@ use tracing::{error, warn};
 use super::short::{ShortStateHash, ShortStateKey};
 use crate::{
     observability::{FoundIn, Lookup, METRICS},
-    service::{globals::marker, pdu::PduBuilder},
+    service::{globals::marker, pdu::PduBuilder, rooms::state::ExtractVersion},
     services,
-    utils::on_demand_hashmap::KeyToken,
+    utils::{on_demand_hashmap::KeyToken, room_version::RoomVersion},
     Error, PduEvent, Result,
 };
 
@@ -521,7 +521,7 @@ impl Service {
                 .expect("Event content always serializes");
 
         let new_event = PduBuilder {
-            event_type: ruma::events::TimelineEventType::RoomMember,
+            event_type: TimelineEventType::RoomMember,
             content,
             unsigned: None,
             state_key: Some(target_user.into()),
@@ -592,4 +592,31 @@ impl Service {
                 },
             )
     }
+
+    /// Checks whether a redaction event is authorized against the current state
+    /// of it's room.
+    ///
+    /// Panics if `event` is not a `m.room.redaction` event.
+    pub(crate) fn redaction_event_allowed(
+        &self,
+        event: &PduEvent,
+    ) -> Result<bool> {
+        assert_eq!(
+            event.kind,
+            TimelineEventType::RoomRedaction,
+            "event must be a redaction event"
+        );
+
+        let room_version_id = services()
+            .rooms
+            .state
+            .get_create_content::<ExtractVersion>(&event.room_id)?;
+        let room_version = RoomVersion::try_from(&room_version_id)?;
+
+        let Some(redacted_id) = event.redacts(&room_version) else {
+            return Ok(false);
+        };
+
+        self.user_can_redact(&redacted_id, &event.sender, &event.room_id)
+    }
 }

src/service/rooms/timeline.rs

@@ -14,7 +14,6 @@ use ruma::{
             create::RoomCreateEventContent, encrypted::Relation,
             member::MembershipState, message::RoomMessageEventContent,
             power_levels::RoomPowerLevelsEventContent,
-            redaction::RoomRedactionEventContent,
         },
         StateEventType, TimelineEventType,
     },
@@ -472,29 +471,14 @@ impl Service {
                     .state
                     .get_create_content::<ExtractVersion>(&pdu.room_id)?;
                 let room_version = RoomVersion::try_from(&room_version_id)?;
-                if room_version.redaction_event_redacts_in_content {
-                    let content = serde_json::from_str::<
-                        RoomRedactionEventContent,
-                    >(pdu.content.get())
-                    .map_err(|_| {
-                        Error::bad_database("Invalid content in redaction pdu.")
-                    })?;
-                    if let Some(redact_id) = &content.redacts {
-                        if services().rooms.state_accessor.user_can_redact(
-                            redact_id,
-                            &pdu.sender,
-                            &pdu.room_id,
-                        )? {
-                            self.redact_pdu(redact_id, pdu, shortroomid)?;
-                        }
-                    }
-                } else if let Some(redact_id) = &pdu.redacts {
-                    if services().rooms.state_accessor.user_can_redact(
-                        redact_id,
-                        &pdu.sender,
-                        &pdu.room_id,
-                    )? {
-                        self.redact_pdu(redact_id, pdu, shortroomid)?;
+
+                if let Some(redact_id) = pdu.redacts(&room_version) {
+                    if services()
+                        .rooms
+                        .state_accessor
+                        .redaction_event_allowed(pdu)?
+                    {
+                        self.redact_pdu(&redact_id, pdu, shortroomid)?;
                     }
                 }
             }
@@ -1056,46 +1040,14 @@ impl Service {
 
         // If redaction event is not authorized, do not append it to the
         // timeline
-        if pdu.kind == TimelineEventType::RoomRedaction {
-            let room_version_id = services()
-                .rooms
-                .state
-                .get_create_content::<ExtractVersion>(&pdu.room_id)?;
-            let room_version = RoomVersion::try_from(&room_version_id)?;
-            if room_version.redaction_event_redacts_in_content {
-                let content =
-                    serde_json::from_str::<RoomRedactionEventContent>(
-                        pdu.content.get(),
-                    )
-                    .map_err(|_| {
-                        Error::bad_database("Invalid content in redaction pdu.")
-                    })?;
-
-                if let Some(redact_id) = &content.redacts {
-                    if !services().rooms.state_accessor.user_can_redact(
-                        redact_id,
-                        &pdu.sender,
-                        &pdu.room_id,
-                    )? {
+        if pdu.kind == TimelineEventType::RoomRedaction
+            && !services().rooms.state_accessor.redaction_event_allowed(&pdu)?
+        {
             return Err(Error::BadRequest(
                 ErrorKind::forbidden(),
                 "User cannot redact this event.",
             ));
         }
-                }
-            } else if let Some(redact_id) = &pdu.redacts {
-                if !services().rooms.state_accessor.user_can_redact(
-                    redact_id,
-                    &pdu.sender,
-                    &pdu.room_id,
-                )? {
-                    return Err(Error::BadRequest(
-                        ErrorKind::forbidden(),
-                        "User cannot redact this event.",
-                    ));
-                }
-            }
-        }
 
         // We append to state before appending the pdu, so we don't have a
         // moment in time with the pdu without it's state. This is okay