Consider command execution secure when proper conditions are met.

- If the room is end-to-end encrypted.
 - If only the sending user and the bot are present in the room.

This lays groundwork for sensitive commands like registering a user
account with the bot.

src/bin/dicebot-cmd.rs

@@ -30,7 +30,8 @@ async fn main() -> Result<(), BotError> {
             .expect("Could not create matrix client"),
         room: RoomContext {
             id: &room_id!("!fakeroomid:example.com"),
-            display_name: "fake room",
+            display_name: "fake room".to_owned(),
+            secure: false,
         },
         username: "@localuser:example.com",
         message_body: &input,

src/bot/command_execution.rs

@@ -0,0 +1,122 @@
+use crate::commands::{execute_command, ExecutionError, ExecutionResult, ResponseExtractor};
+use crate::context::{Context, RoomContext};
+use crate::db::sqlite::Database;
+use crate::error::BotError;
+use crate::matrix;
+use futures::stream::{self, StreamExt};
+use log::{error, info};
+use matrix_sdk::{self, identifiers::EventId, room::Joined, Client};
+use std::clone::Clone;
+
+/// Handle responding to a single command being executed. Wil print
+/// out the full result of that command.
+pub(super) async fn handle_single_result(
+    client: &Client,
+    cmd_result: &ExecutionResult,
+    respond_to: &str,
+    room: &Joined,
+    event_id: EventId,
+) {
+    if cmd_result.is_err() {
+        error!(
+            "Command execution error: {}",
+            cmd_result.as_ref().err().unwrap()
+        );
+    }
+
+    let html = cmd_result.message_html(respond_to);
+    matrix::send_message(client, room.room_id(), &html, Some(event_id)).await;
+}
+
+/// Handle responding to multiple commands being executed. Will print
+/// out how many commands succeeded and failed (if any failed).
+pub(super) async fn handle_multiple_results(
+    client: &Client,
+    results: &[(String, ExecutionResult)],
+    respond_to: &str,
+    room: &Joined,
+) {
+    let respond_to = format!(
+        "<a href=\"https://matrix.to/#/{}\">{}</a>",
+        respond_to, respond_to
+    );
+
+    let errors: Vec<(&str, &ExecutionError)> = results
+        .into_iter()
+        .filter_map(|(cmd, result)| match result {
+            Err(e) => Some((cmd.as_ref(), e)),
+            _ => None,
+        })
+        .collect();
+
+    for result in errors.iter() {
+        error!("Command execution error: '{}' - {}", result.0, result.1);
+    }
+
+    let message = if errors.len() == 0 {
+        format!("{}: Executed {} commands", respond_to, results.len())
+    } else {
+        let failures: Vec<String> = errors
+            .iter()
+            .map(|&(cmd, err)| format!("<strong>{}:</strong> {}", cmd, err))
+            .collect();
+
+        format!(
+            "{}: Executed {} commands ({} failed)\n\nFailures:\n{}",
+            respond_to,
+            results.len(),
+            errors.len(),
+            failures.join("\n")
+        )
+        .replace("\n", "<br/>")
+    };
+
+    matrix::send_message(client, room.room_id(), &message, None).await;
+}
+
+/// Create a context for command execution. Can fai if the room
+/// context creation fails.
+async fn create_context<'a>(
+    db: &'a Database,
+    client: &'a Client,
+    room: &'a Joined,
+    sender: &'a str,
+    command: &'a str,
+) -> Result<Context<'a>, BotError> {
+    let room_ctx = RoomContext::new(room, sender).await?;
+    Ok(Context {
+        db: db.clone(),
+        matrix_client: client,
+        room: room_ctx,
+        username: &sender,
+        message_body: &command,
+    })
+}
+
+/// Attempt to execute all commands sent to the bot in a message. This
+/// asynchronously executes all commands given to it. A Vec of all
+/// commands and their execution results are returned.
+pub(super) async fn execute(
+    commands: Vec<&str>,
+    db: &Database,
+    client: &Client,
+    room: &Joined,
+    sender: &str,
+) -> Vec<(String, ExecutionResult)> {
+    stream::iter(commands)
+        .then(|command| async move {
+            match create_context(db, client, room, sender, command).await {
+                Err(e) => (command.to_owned(), Err(ExecutionError(e))),
+                Ok(ctx) => {
+                    let cmd_result = execute_command(&ctx).await;
+                    info!(
+                        "[{}] {} executed: {}",
+                        ctx.room.display_name, sender, command
+                    );
+                    (command.to_owned(), cmd_result)
+                }
+            }
+        })
+        .collect()
+        .await
+}

src/bot/mod.rs

@@ -1,20 +1,18 @@
-use crate::commands::{execute_command, ExecutionError, ExecutionResult, ResponseExtractor};
+use crate::commands::{ExecutionError, ExecutionResult};
 use crate::config::*;
-use crate::context::{Context, RoomContext};
 use crate::db::sqlite::Database;
 use crate::db::DbState;
 use crate::error::BotError;
-use crate::matrix;
 use crate::state::DiceBotState;
 use dirs;
-use futures::stream::{self, StreamExt};
+use log::info;
-use log::{error, info};
 use matrix_sdk::{self, identifiers::EventId, room::Joined, Client, ClientConfig, SyncSettings};
 use std::clone::Clone;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
 use url::Url;
 
+mod command_execution;
 pub mod event_handlers;
 
 /// How many commands can be in one message. If the amount is higher
@@ -53,72 +51,6 @@ fn create_client(config: &Config) -> Result<Client, BotError> {
     Ok(Client::new_with_config(homeserver_url, client_config)?)
 }
 
-/// Handle responding to a single command being executed. Wil print
-/// out the full result of that command.
-async fn handle_single_result(
-    client: &Client,
-    cmd_result: &ExecutionResult,
-    respond_to: &str,
-    room: &Joined,
-    event_id: EventId,
-) {
-    if cmd_result.is_err() {
-        error!(
-            "Command execution error: {}",
-            cmd_result.as_ref().err().unwrap()
-        );
-    }
-
-    let html = cmd_result.message_html(respond_to);
-    matrix::send_message(client, room.room_id(), &html, Some(event_id)).await;
-}
-
-/// Handle responding to multiple commands being executed. Will print
-/// out how many commands succeeded and failed (if any failed).
-async fn handle_multiple_results(
-    client: &Client,
-    results: &[(String, ExecutionResult)],
-    respond_to: &str,
-    room: &Joined,
-) {
-    let respond_to = format!(
-        "<a href=\"https://matrix.to/#/{}\">{}</a>",
-        respond_to, respond_to
-    );
-
-    let errors: Vec<(&str, &ExecutionError)> = results
-        .into_iter()
-        .filter_map(|(cmd, result)| match result {
-            Err(e) => Some((cmd.as_ref(), e)),
-            _ => None,
-        })
-        .collect();
-
-    for result in errors.iter() {
-        error!("Command execution error: '{}' - {}", result.0, result.1);
-    }
-
-    let message = if errors.len() == 0 {
-        format!("{}: Executed {} commands", respond_to, results.len())
-    } else {
-        let failures: Vec<String> = errors
-            .iter()
-            .map(|&(cmd, err)| format!("<strong>{}:</strong> {}", cmd, err))
-            .collect();
-
-        format!(
-            "{}: Executed {} commands ({} failed)\n\nFailures:\n{}",
-            respond_to,
-            results.len(),
-            errors.len(),
-            failures.join("\n")
-        )
-        .replace("\n", "<br/>")
-    };
-
-    matrix::send_message(client, room.room_id(), &message, None).await;
-}
-
 impl DiceBot {
     /// Create a new dicebot with the given configuration and state
     /// actor. This function returns a Result because it is possible
@@ -184,11 +116,9 @@ impl DiceBot {
     async fn execute_commands(
         &self,
         room: &Joined,
-        sender_username: &str,
+        sender: &str,
         msg_body: &str,
     ) -> Vec<(String, ExecutionResult)> {
-        let room_name: &str = &room.display_name().await.ok().unwrap_or_default();
-
         let commands: Vec<&str> = msg_body
             .lines()
             .filter(|line| line.starts_with("!"))
@@ -197,22 +127,7 @@ impl DiceBot {
 
         //Up to 50 commands allowed, otherwise we send back an error.
         let results: Vec<(String, ExecutionResult)> = if commands.len() < MAX_COMMANDS_PER_MESSAGE {
-            stream::iter(commands)
+            command_execution::execute(commands, &self.db, &self.client, room, sender).await
-                .then(|command| async move {
-                    let ctx = Context {
-                        db: self.db.clone(),
-                        matrix_client: &self.client,
-                        room: RoomContext::new_with_name(&room, room_name),
-                        username: &sender_username,
-                        message_body: &command,
-                    };
-
-                    let cmd_result = execute_command(&ctx).await;
-                    info!("[{}] {} executed: {}", room_name, sender_username, command);
-                    (command.to_owned(), cmd_result)
-                })
-                .collect()
-                .await
         } else {
             vec![(
                 "".to_owned(),
@@ -232,7 +147,7 @@ impl DiceBot {
     ) {
         if results.len() >= 1 {
             if results.len() == 1 {
-                handle_single_result(
+                command_execution::handle_single_result(
                     &self.client,
                     &results[0].1,
                     sender_username,
@@ -241,7 +156,13 @@ impl DiceBot {
                 )
                 .await;
             } else if results.len() > 1 {
-                handle_multiple_results(&self.client, &results, sender_username, &room).await;
+                command_execution::handle_multiple_results(
+                    &self.client,
+                    &results,
+                    sender_username,
+                    &room,
+                )
+                .await;
             }
         }
     }

src/cofd/dice.rs

@@ -333,7 +333,8 @@ mod tests {
         () => {
             crate::context::RoomContext {
                 id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
-                display_name: "displayname",
+                display_name: "displayname".to_owned(),
+                secure: false,
             }
         };
     }

src/commands/basic_rolling.rs

@@ -12,6 +12,10 @@ impl Command for RollCommand {
         "roll regular dice"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, _ctx: &Context<'_>) -> ExecutionResult {
         let roll = self.0.roll();
         let html = format!(

src/commands/cofd.rs

@@ -11,6 +11,10 @@ impl Command for PoolRollCommand {
         "roll dice pool"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let pool_with_ctx = DicePoolWithContext(&self.0, ctx);
         let rolled_pool = roll_pool(&pool_with_ctx).await?;

src/commands/cthulhu.rs

@@ -14,6 +14,10 @@ impl Command for CthRoll {
         "roll percentile dice"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let roll_with_ctx = DiceRollWithContext(&self.0, ctx);
         let executed_roll = regular_roll(&roll_with_ctx).await?;
@@ -35,6 +39,10 @@ impl Command for CthAdvanceRoll {
         "roll skill advancement dice"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let roll_with_ctx = AdvancementRollWithContext(&self.0, ctx);
         let executed_roll = advancement_roll(&roll_with_ctx).await?;

src/commands/management.rs

@@ -12,6 +12,10 @@ impl Command for ResyncCommand {
         "resync room information"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let our_username: Option<UserId> = ctx.matrix_client.user_id().await;
         let our_username: &str = our_username.as_ref().map_or("", UserId::as_str);

src/commands/misc.rs

@@ -11,6 +11,10 @@ impl Command for HelpCommand {
         "help information"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, _ctx: &Context<'_>) -> ExecutionResult {
         let help = match &self.0 {
             Some(topic) => topic.message(),

src/commands/mod.rs

@@ -96,6 +96,7 @@ impl ResponseExtractor for ExecutionResult {
 pub trait Command: Send + Sync {
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult;
     fn name(&self) -> &'static str;
+    fn is_secure(&self) -> bool;
 }
 
 /// Attempt to execute a command, and return the content that should
@@ -116,7 +117,8 @@ mod tests {
         () => {
             crate::context::RoomContext {
                 id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
-                display_name: "displayname",
+                display_name: "displayname".to_owned(),
+                secure: false,
             }
         };
     }

src/commands/variables.rs

@@ -12,6 +12,10 @@ impl Command for GetAllVariablesCommand {
         "get all variables"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let variables = ctx
             .db
@@ -43,6 +47,10 @@ impl Command for GetVariableCommand {
         "retrieve variable value"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let name = &self.0;
         let result = ctx
@@ -69,6 +77,10 @@ impl Command for SetVariableCommand {
         "set variable value"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let name = &self.0;
         let value = self.1;
@@ -91,6 +103,10 @@ impl Command for DeleteVariableCommand {
         "delete variable"
     }
 
+    fn is_secure(&self) -> bool {
+        false
+    }
+
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
         let name = &self.0;
         let result = ctx

src/context.rs

@@ -1,7 +1,9 @@
 use crate::db::sqlite::Database;
-use matrix_sdk::identifiers::RoomId;
+use crate::error::BotError;
+use matrix_sdk::identifiers::{RoomId, UserId};
 use matrix_sdk::room::Joined;
 use matrix_sdk::Client;
+use std::convert::TryFrom;
 
 /// A context carried through the system providing access to things
 /// like the database.
@@ -18,19 +20,52 @@ impl Context<'_> {
     pub fn room_id(&self) -> &RoomId {
         self.room.id
     }
+
+    pub fn is_secure(&self) -> bool {
+        self.room.secure
+    }
 }
 
 #[derive(Clone)]
 pub struct RoomContext<'a> {
     pub id: &'a RoomId,
-    pub display_name: &'a str,
+    pub display_name: String,
+    pub secure: bool,
 }
 
 impl RoomContext<'_> {
-    pub fn new_with_name<'a>(room: &'a Joined, display_name: &'a str) -> RoomContext<'a> {
+    pub async fn new_with_name<'a>(
-        RoomContext {
+        room: &'a Joined,
+        display_name: String,
+        sending_user: &str,
+    ) -> Result<RoomContext<'a>, BotError> {
+        // TODO is_direct is a hack; should set rooms to Direct
+        // Message upon joining, if other contact has requested it.
+        // Waiting on SDK support.
+        let sending_user = UserId::try_from(sending_user)?;
+        let user_in_room = room.get_member(&sending_user).await.ok().is_some();
+        let is_direct = room.joined_members().await?.len() == 2;
+
+        Ok(RoomContext {
             id: room.room_id(),
             display_name,
+            secure: room.is_encrypted() && is_direct && user_in_room,
+        })
     }
+
+    pub async fn new<'a>(
+        room: &'a Joined,
+        sending_user: &str,
+    ) -> Result<RoomContext<'a>, BotError> {
+        Self::new_with_name(
+            &room,
+            room.display_name()
+                .await
+                .ok()
+                .unwrap_or_default()
+                .to_string(),
+            sending_user,
+        )
+        .await
     }
 }

src/cthulhu/dice.rs

@@ -427,7 +427,8 @@ mod tests {
         () => {
             crate::context::RoomContext {
                 id: &matrix_sdk::identifiers::room_id!("!fakeroomid:example.com"),
-                display_name: "displayname",
+                display_name: "displayname".to_owned(),
+                secure: false,
             }
         };
     }

src/error.rs

@@ -75,6 +75,9 @@ pub enum BotError {
 
     #[error("could not convert to proper integer type")]
     TryFromIntError(#[from] std::num::TryFromIntError),
+
+    #[error("identifier error: {0}")]
+    IdentifierError(#[from] matrix_sdk::identifiers::Error),
 }
 
 #[derive(Error, Debug)]