Make management interface connect return error on missing account token

3 files changed, 45 insertions, 13 deletions

diff --git a/mullvad-cli/src/cmds/connect.rs b/mullvad-cli/src/cmds/connect.rs
index b7cf7cd8c5..40dbe5a5a0 100644
--- a/mullvad-cli/src/cmds/connect.rs
+++ b/mullvad-cli/src/cmds/connect.rs
@@ -1,9 +1,9 @@
 use clap;
+use error_chain::ChainedError;
 use new_rpc_client;
 use Command;
 use Result;
 
-
 pub struct Connect;
 
 impl Command for Connect {
@@ -18,7 +18,9 @@ impl Command for Connect {
 
     fn run(&self, _matches: &clap::ArgMatches) -> Result<()> {
         let mut rpc = new_rpc_client()?;
-        rpc.connect()?;
+        if let Err(e) = rpc.connect() {
+            eprintln!("{}", e.display_chain());
+        }
         Ok(())
     }
 }
diff --git a/mullvad-daemon/src/main.rs b/mullvad-daemon/src/main.rs
index df8aac4bfd..4470a86e30 100644
--- a/mullvad-daemon/src/main.rs
+++ b/mullvad-daemon/src/main.rs
@@ -296,9 +296,11 @@ impl Daemon {
     /// Consume the `Daemon` and run the main event loop. Blocks until an error happens or a
     /// shutdown event is received.
     pub fn run(mut self) -> Result<()> {
-        if self.settings.get_auto_connect() && self.settings.get_account_token().is_some() {
+        if self.settings.get_auto_connect() {
             info!("Automatically connecting since auto-connect is turned on");
-            self.set_target_state(TargetState::Secured);
+            if self.set_target_state(TargetState::Secured).is_err() {
+                warn!("Aborting auto-connect since no account token is set");
+            }
         }
         while let Ok(event) = self.rx.recv() {
             self.handle_event(event)?;
@@ -347,7 +349,7 @@ impl Daemon {
     fn handle_management_interface_event(&mut self, event: ManagementCommand) -> Result<()> {
         use ManagementCommand::*;
         match event {
-            SetTargetState(state) => Ok(self.on_set_target_state(state)),
+            SetTargetState(tx, state) => Ok(self.on_set_target_state(tx, state)),
             GetState(tx) => Ok(self.on_get_state(tx)),
             GetCurrentLocation(tx) => Ok(self.on_get_current_location(tx)),
             GetAccountData(tx, account_token) => Ok(self.on_get_account_data(tx, account_token)),
@@ -369,11 +371,16 @@ impl Daemon {
         }
     }
 
-    fn on_set_target_state(&mut self, new_target_state: TargetState) {
+    fn on_set_target_state(
+        &mut self,
+        tx: OneshotSender<::std::result::Result<(), ()>>,
+        new_target_state: TargetState,
+    ) {
         if self.state.is_running() {
-            self.set_target_state(new_target_state);
+            Self::oneshot_send(tx, self.set_target_state(new_target_state), "targe state");
         } else {
             warn!("Ignoring target state change request due to shutdown");
+            Self::oneshot_send(tx, Ok(()), "targe state");
         }
     }
 
@@ -436,7 +443,7 @@ impl Daemon {
                 if account_changed {
                     if account_token_cleared {
                         info!("Disconnecting because account token was cleared");
-                        self.set_target_state(TargetState::Unsecured);
+                        let _ = self.set_target_state(TargetState::Unsecured);
                     } else {
                         info!("Initiating tunnel restart because the account token changed");
                         self.reconnect_tunnel();
@@ -592,18 +599,23 @@ impl Daemon {
 
     /// Set the target state of the client. If it changed trigger the operations needed to
     /// progress towards that state.
-    fn set_target_state(&mut self, new_state: TargetState) {
+    /// Returns an error if trying to set secured state, but no account token is present.
+    fn set_target_state(&mut self, new_state: TargetState) -> ::std::result::Result<(), ()> {
         if new_state != self.target_state {
             debug!("Target state {:?} => {:?}", self.target_state, new_state);
             self.target_state = new_state;
             match self.target_state {
                 TargetState::Secured => match self.settings.get_account_token() {
                     Some(account_token) => self.connect_tunnel(account_token),
-                    None => self.set_target_state(TargetState::Unsecured),
+                    None => {
+                        self.set_target_state(TargetState::Unsecured)?;
+                        return Err(());
+                    }
                 },
                 TargetState::Unsecured => self.disconnect_tunnel(),
             }
         }
+        Ok(())
     }
 
     fn connect_tunnel(&mut self, account_token: AccountToken) {
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index ed4a4a56cf..383693bb27 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -156,7 +156,7 @@ build_rpc_trait! {
 /// Enum representing commands coming in on the management interface.
 pub enum ManagementCommand {
     /// Change target state.
-    SetTargetState(TargetState),
+    SetTargetState(OneshotSender<Result<(), ()>>, TargetState),
     /// Request the current state.
     GetState(OneshotSender<TunnelStateTransition>),
     /// Get the current geographical location.
@@ -477,12 +477,30 @@ impl<T: From<ManagementCommand> + 'static + Send> ManagementInterfaceApi
 
     fn connect(&self, _: Self::Metadata) -> BoxFuture<(), Error> {
         debug!("connect");
-        self.send_command_to_daemon(ManagementCommand::SetTargetState(TargetState::Secured))
+        let (tx, rx) = sync::oneshot::channel();
+        let future = self
+            .send_command_to_daemon(ManagementCommand::SetTargetState(tx, TargetState::Secured))
+            .and_then(|_| rx.map_err(|_| Error::internal_error()))
+            .and_then(|result| match result {
+                Ok(()) => future::ok(()),
+                Err(()) => future::err(Error {
+                    code: ErrorCode::ServerError(-900),
+                    message: "No account token configured".to_owned(),
+                    data: None,
+                }),
+            });
+        Box::new(future)
     }
 
     fn disconnect(&self, _: Self::Metadata) -> BoxFuture<(), Error> {
         debug!("disconnect");
-        self.send_command_to_daemon(ManagementCommand::SetTargetState(TargetState::Unsecured))
+        let (tx, _) = sync::oneshot::channel();
+        let future = self
+            .send_command_to_daemon(ManagementCommand::SetTargetState(
+                tx,
+                TargetState::Unsecured,
+            )).then(|_| future::ok(()));
+        Box::new(future)
     }
 
     fn get_state(&self, _: Self::Metadata) -> BoxFuture<TunnelStateTransition, Error> {