Change admins from Vec<String> into Hashset<Uuid>, add commands to server-cli-bin for adding and removing admins

common/src/comp/admin.rs

@@ -1,5 +1,4 @@
 use specs::{Component, NullStorage};
-use std::ops::Deref;
 
 #[derive(Clone, Copy, Default)]
 pub struct Admin;
@@ -7,12 +6,3 @@ pub struct Admin;
 impl Component for Admin {
     type Storage = NullStorage<Self>;
 }
-
-/// List of admin usernames. This is stored as a specs resource so that the list
-/// can be read by specs systems.
-pub struct AdminList(pub Vec<String>);
-impl Deref for AdminList {
-    type Target = Vec<String>;
-
-    fn deref(&self) -> &Vec<String> { &self.0 }
-}

common/src/comp/mod.rs

@@ -24,7 +24,7 @@ pub mod visual;
 
 // Reexports
 pub use ability::{CharacterAbility, CharacterAbilityType, ItemConfig, Loadout};
-pub use admin::{Admin, AdminList};
+pub use admin::Admin;
 pub use agent::{Agent, Alignment};
 pub use beam::{Beam, BeamSegment};
 pub use body::{

server-cli/src/admin.rs

@@ -0,0 +1,28 @@
+pub fn admin_subcommand(
+    sub_m: &clap::ArgMatches,
+    server_settings: &server::Settings,
+    editable_settings: &mut server::EditableSettings,
+    data_dir: &std::path::Path,
+) {
+    let login_provider =
+        server::login_provider::LoginProvider::new(server_settings.auth_server_address.clone());
+
+    match sub_m.subcommand() {
+        ("add", Some(sub_m)) => {
+            if let Some(username) = sub_m.value_of("username") {
+                server::add_admin(username, &login_provider, editable_settings, data_dir)
+            }
+        },
+        ("remove", Some(sub_m)) => {
+            if let Some(username) = sub_m.value_of("username") {
+                server::remove_admin(username, &login_provider, editable_settings, data_dir)
+            }
+        },
+        // TODO: can clap enforce this?
+        // or make this list current admins or something
+        _ => tracing::error!(
+            "Invalid input, use one of the subcommands listed using: \nveloren-server-cli help \
+             admin"
+        ),
+    }
+}

server-cli/src/main.rs

@@ -2,6 +2,7 @@
 #![deny(clippy::clone_on_ref_ptr)]
 #![feature(bool_to_option)]
 
+mod admin;
 mod logging;
 mod settings;
 mod shutdown_coordinator;
@@ -12,11 +13,9 @@ use crate::{
     shutdown_coordinator::ShutdownCoordinator,
     tui_runner::{Message, Tui},
 };
-use clap::{App, Arg};
+use clap::{App, Arg, SubCommand};
 use common::clock::Clock;
 use server::{Event, Input, Server};
-#[cfg(any(target_os = "linux", target_os = "macos"))]
-use signal_hook::SIGUSR1;
 use std::{
     io,
     sync::{atomic::AtomicBool, mpsc, Arc},
@@ -35,48 +34,54 @@ fn main() -> io::Result<()> {
             Arg::with_name("basic")
                 .short("b")
                 .long("basic")
-                .help("Disables the tui")
+                .help("Disables the tui"),
-                .takes_value(false),
             Arg::with_name("interactive")
                 .short("i")
                 .long("interactive")
-                .help("Enables command input for basic mode")
+                .help("Enables command input for basic mode"),
-                .takes_value(false),
             Arg::with_name("no-auth")
                 .long("no-auth")
                 .help("Runs without auth enabled"),
         ])
+        .subcommand(
+            SubCommand::with_name("admin")
+                .about("Add or remove admins")
+                .subcommands(vec![
+                    SubCommand::with_name("add").about("Adds an admin").arg(
+                        Arg::with_name("username")
+                            .help("Name of the admin to add")
+                            .required(true),
+                    ),
+                    SubCommand::with_name("remove")
+                        .about("Removes an admin")
+                        .arg(
+                            Arg::with_name("username")
+                                .help("Name of the admin to remove")
+                                .required(true),
+                        ),
+                ]),
+        )
         .get_matches();
 
-    let basic = matches.is_present("basic");
+    let basic = matches.is_present("basic")
+        // Default to basic with these subcommands
+        || matches
+            .subcommand_name()
+            .filter(|name| ["admin"].contains(name))
+            .is_some();
     let interactive = matches.is_present("interactive");
     let no_auth = matches.is_present("no-auth");
 
     let sigusr1_signal = Arc::new(AtomicBool::new(false));
 
     #[cfg(any(target_os = "linux", target_os = "macos"))]
-    let _ = signal_hook::flag::register(SIGUSR1, Arc::clone(&sigusr1_signal));
+    let _ = signal_hook::flag::register(signal_hook::SIGUSR1, Arc::clone(&sigusr1_signal));
 
     logging::init(basic);
 
     // Load settings
     let settings = settings::Settings::load();
 
-    // Panic hook to ensure that console mode is set back correctly if in non-basic
-    // mode
-    let hook = std::panic::take_hook();
-    std::panic::set_hook(Box::new(move |info| {
-        Tui::shutdown(basic);
-        hook(info);
-    }));
-
-    let tui = (!basic || interactive).then(|| Tui::run(basic));
-
-    info!("Starting server...");
-
-    // Set up an fps clock
-    let mut clock = Clock::start();
-
     // Determine folder to save server data in
     let server_data_dir = {
         let mut path = common::userdata_dir_workspace!();
@@ -86,7 +91,33 @@ fn main() -> io::Result<()> {
 
     // Load server settings
     let mut server_settings = server::Settings::load(&server_data_dir);
-    let editable_settings = server::EditableSettings::load(&server_data_dir);
+    let mut editable_settings = server::EditableSettings::load(&server_data_dir);
+    match matches.subcommand() {
+        ("admin", Some(sub_m)) => {
+            admin::admin_subcommand(
+                sub_m,
+                &server_settings,
+                &mut editable_settings,
+                &server_data_dir,
+            );
+            return Ok(());
+        },
+        _ => {},
+    }
+
+    // Panic hook to ensure that console mode is set back correctly if in non-basic
+    // mode
+    if !basic {
+        let hook = std::panic::take_hook();
+        std::panic::set_hook(Box::new(move |info| {
+            Tui::shutdown(basic);
+            hook(info);
+        }));
+    }
+
+    let tui = (!basic || interactive).then(|| Tui::run(basic));
+
+    info!("Starting server...");
 
     if no_auth {
         server_settings.auth_server_address = None;
@@ -106,6 +137,12 @@ fn main() -> io::Result<()> {
 
     let mut shutdown_coordinator = ShutdownCoordinator::new(Arc::clone(&sigusr1_signal));
 
+    // Set up an fps clock
+    let mut clock = Clock::start();
+    // Wait for a tick so we don't start with a zero dt
+    // TODO: consider integrating this into Clock::start?
+    clock.tick(Duration::from_millis(1000 / TPS));
+
     loop {
         // Terminate the server if instructed to do so by the shutdown coordinator
         if shutdown_coordinator.check(&mut server, &settings) {
@@ -144,6 +181,12 @@ fn main() -> io::Result<()> {
                         info!("Closing the server");
                         break;
                     },
+                    Message::AddAdmin(username) => {
+                        server.add_admin(&username);
+                    },
+                    Message::RemoveAdmin(username) => {
+                        server.remove_admin(&username);
+                    },
                 },
                 Err(mpsc::TryRecvError::Empty) | Err(mpsc::TryRecvError::Disconnected) => {},
             }

server-cli/src/tui_runner.rs

@@ -26,6 +26,8 @@ pub enum Message {
     AbortShutdown,
     Shutdown { grace_period: Duration },
     Quit,
+    AddAdmin(String),
+    RemoveAdmin(String),
 }
 
 pub struct Command<'a> {
@@ -37,7 +39,8 @@ pub struct Command<'a> {
     pub cmd: fn(Vec<String>, &mut mpsc::Sender<Message>),
 }
 
-pub const COMMANDS: [Command; 4] = [
+// TODO: mabye we could be using clap here?
+pub const COMMANDS: [Command; 5] = [
     Command {
         name: "quit",
         description: "Closes the server",
@@ -70,6 +73,22 @@ pub const COMMANDS: [Command; 4] = [
         args: 0,
         cmd: |_, sender| sender.send(Message::AbortShutdown).unwrap(),
     },
+    Command {
+        name: "admin",
+        description: "Add or remove an admin via \'admin add/remove <username>\'",
+        split_spaces: true,
+        args: 2,
+        cmd: |args, sender| match args.get(..2) {
+            Some([op, username]) if op == "add" => {
+                sender.send(Message::AddAdmin(username.clone())).unwrap()
+            },
+            Some([op, username]) if op == "remove" => {
+                sender.send(Message::RemoveAdmin(username.clone())).unwrap()
+            },
+            Some(_) => error!("First arg must be add or remove"),
+            _ => error!("Not enough args, should be unreachable"),
+        },
+    },
     Command {
         name: "help",
         description: "List all command available",

server/src/lib.rs

@@ -156,9 +156,6 @@ impl Server {
         state
             .ecs_mut()
             .insert(CharacterLoader::new(&persistence_db_dir)?);
-        state
-            .ecs_mut()
-            .insert(comp::AdminList(settings.admins.clone()));
         state.ecs_mut().insert(Vec::<Outcome>::new());
 
         // System timers for performance monitoring
@@ -935,6 +932,20 @@ impl Server {
     pub fn number_of_players(&self) -> i64 {
         self.state.ecs().read_storage::<Client>().join().count() as i64
     }
+
+    pub fn add_admin(&self, username: &str) {
+        let mut editable_settings = self.editable_settings_mut();
+        let login_provider = self.state.ecs().fetch::<LoginProvider>();
+        let data_dir = self.data_dir();
+        add_admin(username, &login_provider, &mut editable_settings, &data_dir.path);
+    }
+
+    pub fn remove_admin(&self, username: &str) {
+        let mut editable_settings = self.editable_settings_mut();
+        let login_provider = self.state.ecs().fetch::<LoginProvider>();
+        let data_dir = self.data_dir();
+        remove_admin(username, &login_provider, &mut editable_settings, &data_dir.path);
+    }
 }
 
 impl Drop for Server {
@@ -943,3 +954,52 @@ impl Drop for Server {
             .notify_registered_clients(ServerMsg::Disconnect(DisconnectReason::Shutdown));
     }
 }
+
+pub fn add_admin(
+    username: &str,
+    login_provider: &LoginProvider, 
+    editable_settings: &mut EditableSettings,
+    data_dir: &std::path::Path,
+) {
+    use crate::settings::EditableSetting;
+    match login_provider.username_to_uuid(username) {
+        Ok(uuid) => editable_settings.admins.edit(data_dir, |admins| {
+            if admins.insert(uuid.clone()) {
+                info!("Successfully added {} ({}) as an admin!", username, uuid);
+            } else {
+                info!("{} ({}) is already an admin!", username, uuid);
+            }
+        }),
+        Err(err) => error!(
+            ?err,
+            "Could not find uuid for this name either the user does not exist or \
+             there was an error communicating with the auth server."
+        ),
+    }
+}
+
+pub fn remove_admin(
+    username: &str,
+    login_provider: &LoginProvider, 
+    editable_settings: &mut EditableSettings,
+    data_dir: &std::path::Path,
+) {
+    use crate::settings::EditableSetting;
+    match login_provider.username_to_uuid(username) {
+        Ok(uuid) => editable_settings.admins.edit(data_dir, |admins| {
+            if admins.remove(&uuid) {
+                info!(
+                    "Successfully removed {} ({}) from the admins",
+                    username, uuid
+                );
+            } else {
+                info!("{} ({}) is not an admin!", username, uuid);
+            }
+        }),
+        Err(err) => error!(
+            ?err,
+            "Could not find uuid for this name either the user does not exist or \
+             there was an error communicating with the auth server."
+        ),
+    }
+}

server/src/login_provider.rs

@@ -53,7 +53,7 @@ impl LoginProvider {
     pub fn try_login(
         &mut self,
         username_or_token: &str,
-        admins: &[String],
+        admins: &HashSet<Uuid>,
         whitelist: &HashSet<Uuid>,
         banlist: &HashMap<Uuid, BanRecord>,
     ) -> Result<(String, Uuid), RegisterError> {
@@ -70,7 +70,7 @@ impl LoginProvider {
 
                 // user can only join if he is admin, the whitelist is empty (everyone can join)
                 // or his name is in the whitelist
-                if !whitelist.is_empty() && !whitelist.contains(&uuid) && !admins.contains(&username) {
+                if !whitelist.is_empty() && !whitelist.contains(&uuid) && !admins.contains(&uuid) {
                     return Err(RegisterError::NotOnWhitelist);
                 }
 

server/src/settings.rs

@@ -22,6 +22,7 @@ const SETTINGS_FILENAME: &str = "settings.ron";
 const WHITELIST_FILENAME: &str = "whitelist.ron";
 const BANLIST_FILENAME: &str = "banlist.ron";
 const SERVER_DESCRIPTION_FILENAME: &str = "description.ron";
+const ADMINS_FILENAME: &str = "admins.ron";
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(default)]
@@ -34,7 +35,6 @@ pub struct Settings {
     //pub pvp_enabled: bool,
     pub server_name: String,
     pub start_time: f64,
-    pub admins: Vec<String>,
     /// When set to None, loads the default map file (if available); otherwise,
     /// uses the value of the file options to decide how to proceed.
     pub map_file: Option<FileOpts>,
@@ -57,7 +57,6 @@ impl Default for Settings {
             max_players: 100,
             start_time: 9.0 * 3600.0,
             map_file: None,
-            admins: Vec::new(),
             persistence_db_dir: "saves".into(),
             max_view_distance: Some(30),
             banned_words_files: Vec::new(),
@@ -137,8 +136,6 @@ impl Settings {
             server_name: "Singleplayer".to_owned(),
             max_players: 100,
             start_time: 9.0 * 3600.0,
-            admins: vec!["singleplayer".to_string()], /* TODO: Let the player choose if they want
-                                                       * to use admin commands or not */
             max_view_distance: None,
             client_timeout: Duration::from_secs(180),
             ..load // Fill in remaining fields from server_settings.ron.
@@ -179,18 +176,28 @@ pub struct BanRecord {
 #[derive(Deserialize, Serialize, Default)]
 #[serde(transparent)]
 pub struct Whitelist(HashSet<Uuid>);
+
 #[derive(Deserialize, Serialize, Default)]
 #[serde(transparent)]
 pub struct Banlist(HashMap<Uuid, BanRecord>);
+
 #[derive(Deserialize, Serialize)]
 #[serde(transparent)]
 pub struct ServerDescription(String);
+impl Default for ServerDescription {
+    fn default() -> Self { Self("This is the best Veloren server".into()) }
+}
+
+#[derive(Deserialize, Serialize, Default)]
+#[serde(transparent)]
+pub struct Admins(HashSet<Uuid>);
 
 /// Combines all the editable settings into one struct that is stored in the ecs
 pub struct EditableSettings {
     pub whitelist: Whitelist,
     pub banlist: Banlist,
     pub server_description: ServerDescription,
+    pub admins: Admins,
 }
 
 impl EditableSettings {
@@ -199,6 +206,7 @@ impl EditableSettings {
             whitelist: Whitelist::load(data_dir),
             banlist: Banlist::load(data_dir),
             server_description: ServerDescription::load(data_dir),
+            admins: Admins::load(data_dir),
         }
     }
 
@@ -206,15 +214,21 @@ impl EditableSettings {
         let load = Self::load(data_dir);
         Self {
             server_description: ServerDescription("Who needs friends anyway?".into()),
+            // TODO: Let the player choose if they want to use admin commands or not
+            admins: Admins(
+                std::iter::once(
+                    // TODO: hacky
+                    crate::login_provider::LoginProvider::new(None)
+                        .username_to_uuid("singleplayer")
+                        .unwrap(),
+                )
+                .collect(),
+            ),
             ..load
         }
     }
 }
 
-impl Default for ServerDescription {
-    fn default() -> Self { Self("This is the best Veloren server".into()) }
-}
-
 impl EditableSetting for Whitelist {
     const FILENAME: &'static str = WHITELIST_FILENAME;
 }
@@ -227,6 +241,10 @@ impl EditableSetting for ServerDescription {
     const FILENAME: &'static str = SERVER_DESCRIPTION_FILENAME;
 }
 
+impl EditableSetting for Admins {
+    const FILENAME: &'static str = ADMINS_FILENAME;
+}
+
 impl Deref for Whitelist {
     type Target = HashSet<Uuid>;
 
@@ -256,3 +274,13 @@ impl Deref for ServerDescription {
 impl DerefMut for ServerDescription {
     fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 }
 }
+
+impl Deref for Admins {
+    type Target = HashSet<Uuid>;
+
+    fn deref(&self) -> &Self::Target { &self.0 }
+}
+
+impl DerefMut for Admins {
+    fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 }
+}

server/src/sys/message.rs

@@ -10,7 +10,7 @@ use crate::{
 };
 use common::{
     comp::{
-        Admin, AdminList, CanBuild, ChatMode, ChatType, ControlEvent, Controller, ForceUpdate, Ori,
+        Admin, CanBuild, ChatMode, ChatType, ControlEvent, Controller, ForceUpdate, Ori,
         Player, Pos, Stats, UnresolvedChatMsg, Vel,
     },
     event::{EventBus, ServerEvent},
@@ -57,7 +57,6 @@ impl Sys {
         chat_modes: &ReadStorage<'_, ChatMode>,
         login_provider: &mut WriteExpect<'_, LoginProvider>,
         block_changes: &mut Write<'_, BlockChange>,
-        admin_list: &ReadExpect<'_, AdminList>,
         admins: &mut WriteStorage<'_, Admin>,
         positions: &mut WriteStorage<'_, Pos>,
         velocities: &mut WriteStorage<'_, Vel>,
@@ -99,7 +98,7 @@ impl Sys {
                 } => {
                     let (username, uuid) = match login_provider.try_login(
                         &token_or_username,
-                        &settings.admins,
+                        &*editable_settings.admins,
                         &*editable_settings.whitelist,
                         &*editable_settings.banlist,
                     ) {
@@ -112,8 +111,8 @@ impl Sys {
 
                     let vd =
                         view_distance.map(|vd| vd.min(settings.max_view_distance.unwrap_or(vd)));
+                    let is_admin = editable_settings.admins.contains(&uuid);
                     let player = Player::new(username.clone(), None, vd, uuid);
-                    let is_admin = admin_list.contains(&username);
 
                     if !player.is_valid() {
                         // Invalid player
@@ -441,7 +440,6 @@ impl<'a> System<'a> for Sys {
         ReadStorage<'a, ChatMode>,
         WriteExpect<'a, LoginProvider>,
         Write<'a, BlockChange>,
-        ReadExpect<'a, AdminList>,
         WriteStorage<'a, Admin>,
         WriteStorage<'a, Pos>,
         WriteStorage<'a, Vel>,
@@ -475,7 +473,6 @@ impl<'a> System<'a> for Sys {
             chat_modes,
             mut accounts,
             mut block_changes,
-            admin_list,
             mut admins,
             mut positions,
             mut velocities,
@@ -537,7 +534,6 @@ impl<'a> System<'a> for Sys {
                     &chat_modes,
                     &mut accounts,
                     &mut block_changes,
-                    &admin_list,
                     &mut admins,
                     &mut positions,
                     &mut velocities,