Merge branch 'fix-unsecured-flash-when-changing-location'

10 files changed, 135 insertions, 60 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 777c629dc5..9f06a9c301 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,9 @@ Line wrap the file at 100 chars.                                              Th
 #### Linux
 - Add support for DNS configuration using resolvconf.
 
+### Fixed
+- Don't temporarily show the unsecured state in the GUI when the app is reconnecting or blocking.
+
 
 ## [2018.3] - 2018-09-17
 ### Changed
diff --git a/gui/packages/desktop/src/renderer/app.js b/gui/packages/desktop/src/renderer/app.js
index fa1c1b0d21..00d3950f3a 100644
--- a/gui/packages/desktop/src/renderer/app.js
+++ b/gui/packages/desktop/src/renderer/app.js
@@ -547,7 +547,7 @@ export default class AppRenderer {
         break;
 
       case 'disconnecting':
-        actions.connection.disconnecting();
+        actions.connection.disconnecting(stateTransition.details);
         break;
 
       case 'disconnected':
diff --git a/gui/packages/desktop/src/renderer/components/Connect.js b/gui/packages/desktop/src/renderer/components/Connect.js
index 832e805bb8..0ce31864d1 100644
--- a/gui/packages/desktop/src/renderer/components/Connect.js
+++ b/gui/packages/desktop/src/renderer/components/Connect.js
@@ -13,7 +13,7 @@ import Map from './Map';
 import styles from './ConnectStyles';
 import { NoCreditError, NoInternetError } from '../errors';
 import WindowStateObserver from '../lib/window-state-observer';
-import type { BlockReason, TunnelState } from '../lib/daemon-rpc';
+import type { BlockReason, TunnelStateTransition } from '../lib/daemon-rpc';
 
 import type { HeaderBarStyle } from './HeaderBar';
 import type { ConnectionReduxState } from '../redux/connection/reducers';
@@ -77,7 +77,7 @@ export default class Connect extends Component<Props, State> {
     const connection = props.connection;
     this.state = {
       ...this.state,
-      banner: this.getBannerState(connection.status, connection.blockReason),
+      banner: this.getBannerState(connection.status),
     };
   }
 
@@ -102,11 +102,11 @@ export default class Connect extends Component<Props, State> {
     const newConnection = this.props.connection;
 
     if (
-      oldConnection.status !== newConnection.status ||
-      oldConnection.blockReason !== newConnection.blockReason
+      oldConnection.status.state !== newConnection.status.state ||
+      oldConnection.status.details !== newConnection.status.details
     ) {
       this.setState({
-        banner: this.getBannerState(newConnection.status, newConnection.blockReason),
+        banner: this.getBannerState(newConnection.status),
       });
     }
   }
@@ -126,11 +126,8 @@ export default class Connect extends Component<Props, State> {
     );
   }
 
-  getBannerState(
-    tunnelState: TunnelState,
-    blockReason: ?BlockReason,
-  ): $PropertyType<State, 'banner'> {
-    switch (tunnelState) {
+  getBannerState(tunnelState: TunnelStateTransition): $PropertyType<State, 'banner'> {
+    switch (tunnelState.state) {
       case 'connecting':
         return {
           visible: true,
@@ -142,7 +139,7 @@ export default class Connect extends Component<Props, State> {
         return {
           visible: true,
           title: 'BLOCKING INTERNET',
-          subtitle: blockReason ? getBlockReasonMessage(blockReason) : '',
+          subtitle: getBlockReasonMessage(tunnelState.details),
         };
 
       default:
@@ -190,16 +187,17 @@ export default class Connect extends Component<Props, State> {
 
   _getMapProps() {
     const { longitude, latitude, status } = this.props.connection;
+    const state = status.state;
 
     // when the user location is known
     if (typeof longitude === 'number' && typeof latitude === 'number') {
       return {
         center: [longitude, latitude],
         // do not show the marker when connecting
-        showMarker: status !== 'connecting',
-        markerStyle: status === 'connected' || status === 'blocked' ? 'secure' : 'unsecure',
+        showMarker: state !== 'connecting',
+        markerStyle: this._getMarkerStyle(),
         // zoom in when connected
-        zoomLevel: status === 'connected' ? 'low' : 'medium',
+        zoomLevel: state === 'connected' ? 'low' : 'medium',
         // a magic offset to align marker with spinner
         offset: [0, 123],
       };
@@ -216,6 +214,31 @@ export default class Connect extends Component<Props, State> {
     }
   }
 
+  _getMarkerStyle() {
+    const { status } = this.props.connection;
+
+    switch (status.state) {
+      case 'connecting':
+      case 'connected':
+      case 'blocked':
+        return 'secure';
+      case 'disconnected':
+        return 'unsecure';
+      case 'disconnecting':
+        switch (status.details) {
+          case 'block':
+          case 'reconnect':
+            return 'secure';
+          case 'nothing':
+            return 'unsecure';
+          default:
+            throw new Error(`Invalid action after disconnection: $(status.details: empty)}`);
+        }
+      default:
+        throw new Error(`Invalid connection status: ${(status.state: empty)}`);
+    }
+  }
+
   renderMap() {
     let [isConnecting, isConnected, isDisconnected, isDisconnecting, isBlocked] = [
       false,
@@ -224,7 +247,7 @@ export default class Connect extends Component<Props, State> {
       false,
       false,
     ];
-    switch (this.props.connection.status) {
+    switch (this.props.connection.status.state) {
       case 'connecting':
         isConnecting = true;
         break;
@@ -393,32 +416,41 @@ export default class Connect extends Component<Props, State> {
 
   headerBarStyle(): HeaderBarStyle {
     const { status } = this.props.connection;
-    switch (status) {
-      case 'disconnecting':
+    switch (status.state) {
       case 'disconnected':
         return 'error';
       case 'connecting':
       case 'connected':
       case 'blocked':
         return 'success';
+      case 'disconnecting':
+        switch (status.details) {
+          case 'block':
+          case 'reconnect':
+            return 'success';
+          case 'nothing':
+            return 'error';
+          default:
+            throw new Error(`Invalid action after disconnection: ${(status.details: empty)}`);
+        }
       default:
-        throw new Error(`Invalid TunnelState: ${(status: empty)}`);
+        throw new Error(`Invalid TunnelState: ${(status.state: empty)}`);
     }
   }
 
   networkSecurityStyle(): Types.Style {
     const classes = [styles.status_security];
-    const { status } = this.props.connection;
-    if (status === 'connected' || status === 'blocked') {
+    const { state } = this.props.connection.status;
+    if (state === 'connected' || state === 'blocked') {
       classes.push(styles.status_security__secure);
-    } else if (status === 'disconnected' || status === 'disconnecting') {
+    } else if (state === 'disconnected' || state === 'disconnecting') {
       classes.push(styles.status_security__unsecured);
     }
     return classes;
   }
 
   networkSecurityMessage(): string {
-    switch (this.props.connection.status) {
+    switch (this.props.connection.status.state) {
       case 'connected':
         return 'SECURE CONNECTION';
       case 'blocked':
@@ -432,7 +464,7 @@ export default class Connect extends Component<Props, State> {
 
   ipAddressStyle(): Types.Style {
     var classes = [styles.status_ipaddress];
-    if (this.props.connection.status === 'connecting') {
+    if (this.props.connection.status.state === 'connecting') {
       classes.push(styles.status_ipaddress__invisible);
     }
     return classes;
diff --git a/gui/packages/desktop/src/renderer/lib/daemon-rpc.js b/gui/packages/desktop/src/renderer/lib/daemon-rpc.js
index d984bfbf76..cece79f783 100644
--- a/gui/packages/desktop/src/renderer/lib/daemon-rpc.js
+++ b/gui/packages/desktop/src/renderer/lib/daemon-rpc.js
@@ -51,10 +51,13 @@ export type BlockReason =
     }
   | { reason: 'auth_failed', details: ?string };
 
+export type AfterDisconnect = 'nothing' | 'block' | 'reconnect';
+
 export type TunnelState = 'connecting' | 'connected' | 'disconnecting' | 'disconnected' | 'blocked';
 
 export type TunnelStateTransition =
-  | { state: 'disconnecting' | 'disconnected' | 'connecting' | 'connected' }
+  | { state: 'disconnected' | 'connecting' | 'connected' }
+  | { state: 'disconnecting', details: AfterDisconnect }
   | { state: 'blocked', details: BlockReason };
 
 export type RelayProtocol = 'tcp' | 'udp';
@@ -235,6 +238,10 @@ const AccountDataSchema = object({
 
 const TunnelStateTransitionSchema = oneOf(
   object({
+    state: enumeration('disconnecting'),
+    details: enumeration('nothing', 'block', 'reconnect'),
+  }),
+  object({
     state: enumeration('blocked'),
     details: oneOf(
       object({
@@ -249,7 +256,7 @@ const TunnelStateTransitionSchema = oneOf(
     ),
   }),
   object({
-    state: enumeration('connected', 'connecting', 'disconnected', 'disconnecting'),
+    state: enumeration('connected', 'connecting', 'disconnected'),
   }),
 );
 
diff --git a/gui/packages/desktop/src/renderer/redux/connection/actions.js b/gui/packages/desktop/src/renderer/redux/connection/actions.js
index 6a54a8926b..5b4793f1cc 100644
--- a/gui/packages/desktop/src/renderer/redux/connection/actions.js
+++ b/gui/packages/desktop/src/renderer/redux/connection/actions.js
@@ -1,6 +1,6 @@
 // @flow
 
-import type { BlockReason, Ip } from '../../lib/daemon-rpc';
+import type { AfterDisconnect, BlockReason, Ip } from '../../lib/daemon-rpc';
 
 type ConnectingAction = {
   type: 'CONNECTING',
@@ -16,6 +16,7 @@ type DisconnectedAction = {
 
 type DisconnectingAction = {
   type: 'DISCONNECTING',
+  afterDisconnect: AfterDisconnect,
 };
 
 type BlockedAction = {
@@ -71,9 +72,10 @@ function disconnected(): DisconnectedAction {
   };
 }
 
-function disconnecting(): DisconnectingAction {
+function disconnecting(afterDisconnect: AfterDisconnect): DisconnectingAction {
   return {
     type: 'DISCONNECTING',
+    afterDisconnect,
   };
 }
 
diff --git a/gui/packages/desktop/src/renderer/redux/connection/reducers.js b/gui/packages/desktop/src/renderer/redux/connection/reducers.js
index 82374dbb2c..b90f285e4f 100644
--- a/gui/packages/desktop/src/renderer/redux/connection/reducers.js
+++ b/gui/packages/desktop/src/renderer/redux/connection/reducers.js
@@ -1,28 +1,26 @@
 // @flow
 
 import type { ReduxAction } from '../store';
-import type { BlockReason, TunnelState, Ip } from '../../lib/daemon-rpc';
+import type { TunnelStateTransition, Ip } from '../../lib/daemon-rpc';
 
 export type ConnectionReduxState = {
-  status: TunnelState,
+  status: TunnelStateTransition,
   isOnline: boolean,
   ip: ?Ip,
   latitude: ?number,
   longitude: ?number,
   country: ?string,
   city: ?string,
-  blockReason: ?BlockReason,
 };
 
 const initialState: ConnectionReduxState = {
-  status: 'disconnected',
+  status: { state: 'disconnected' },
   isOnline: true,
   ip: null,
   latitude: null,
   longitude: null,
   country: null,
   city: null,
-  blockReason: null,
 };
 
 export default function(
@@ -34,25 +32,25 @@ export default function(
       return { ...state, ...action.newLocation };
 
     case 'CONNECTING':
-      return { ...state, ...{ status: 'connecting', blockReason: null } };
+      return { ...state, status: { state: 'connecting' } };
 
     case 'CONNECTED':
-      return { ...state, ...{ status: 'connected', blockReason: null } };
+      return { ...state, status: { state: 'connected' } };
 
     case 'DISCONNECTED':
-      return { ...state, ...{ status: 'disconnected', blockReason: null } };
+      return { ...state, status: { state: 'disconnected' } };
 
     case 'DISCONNECTING':
-      return { ...state, ...{ status: 'disconnecting', blockReason: null } };
+      return { ...state, status: { state: 'disconnecting', details: action.afterDisconnect } };
 
     case 'BLOCKED':
-      return { ...state, ...{ status: 'blocked', blockReason: action.reason } };
+      return { ...state, status: { state: 'blocked', details: action.reason } };
 
     case 'ONLINE':
-      return { ...state, ...{ isOnline: true } };
+      return { ...state, isOnline: true };
 
     case 'OFFLINE':
-      return { ...state, ...{ isOnline: false } };
+      return { ...state, isOnline: false };
 
     default:
       return state;
diff --git a/gui/packages/desktop/test/components/Connect.spec.js b/gui/packages/desktop/test/components/Connect.spec.js
index 567518b0ce..f144b249ee 100644
--- a/gui/packages/desktop/test/components/Connect.spec.js
+++ b/gui/packages/desktop/test/components/Connect.spec.js
@@ -12,7 +12,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'disconnected',
+        status: { state: 'disconnected' },
       },
     });
 
@@ -28,7 +28,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'connected',
+        status: { state: 'connected' },
       },
     });
 
@@ -44,8 +44,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'blocked',
-        blockReason: { reason: 'no_matching_relay' },
+        status: { state: 'blocked', details: { reason: 'no_matching_relay' } },
       },
     });
 
@@ -61,7 +60,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'connecting',
+        status: { state: 'connecting' },
         country: 'Norway',
         city: 'Oslo',
         ip: '4.3.2.1',
@@ -79,7 +78,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'connected',
+        status: { state: 'connected' },
         country: 'Norway',
         city: 'Oslo',
         ip: '4.3.2.1',
@@ -97,7 +96,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'disconnected',
+        status: { state: 'disconnected' },
         country: 'Norway',
         city: 'Oslo',
         ip: '4.3.2.1',
@@ -116,7 +115,7 @@ describe('components/Connect', () => {
       onConnect: () => done(),
       connection: {
         ...defaultProps.connection,
-        status: 'disconnected',
+        status: { state: 'disconnected' },
       },
     });
     const connectButton = getComponent(component, 'secureConnection');
@@ -124,12 +123,26 @@ describe('components/Connect', () => {
     connectButton.prop('onPress')();
   });
 
-  it('hides the blocking internet message when connected, disconnecting or disconnected', () => {
-    for (const status of ['connected', 'disconnecting', 'disconnected']) {
+  it('hides the blocking internet message when connected or disconnected', () => {
+    for (const state of ['connected', 'disconnected']) {
       const component = renderWithProps({
         connection: {
           ...defaultProps.connection,
-          status,
+          status: { state },
+        },
+      });
+      const blockingAccordion = getComponent(component, 'blockingAccordion');
+
+      expect(blockingAccordion.prop('height')).to.equal(0);
+    }
+  });
+
+  it('hides the blocking internet message when disconnecting', () => {
+    for (const afterDisconnect of ['nothing', 'block', 'reconnect']) {
+      const component = renderWithProps({
+        connection: {
+          ...defaultProps.connection,
+          status: { state: 'disconnecting', details: afterDisconnect },
         },
       });
       const blockingAccordion = getComponent(component, 'blockingAccordion');
@@ -142,7 +155,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'connecting',
+        status: { state: 'connecting' },
         country: 'Norway',
         city: 'Oslo',
       },
@@ -156,8 +169,7 @@ describe('components/Connect', () => {
     const component = renderWithProps({
       connection: {
         ...defaultProps.connection,
-        status: 'blocked',
-        blockReason: { reason: 'no_matching_relay' },
+        status: { state: 'blocked', details: { reason: 'no_matching_relay' } },
       },
     });
     const blockingAccordion = getComponent(component, 'blockingAccordion');
@@ -175,14 +187,13 @@ const defaultProps: ConnectProps = {
   accountExpiry: '',
   selectedRelayName: '',
   connection: {
-    status: 'disconnected',
+    status: { state: 'disconnected' },
     isOnline: true,
     ip: null,
     latitude: null,
     longitude: null,
     country: null,
     city: null,
-    blockReason: null,
   },
   updateAccountExpiry: () => Promise.resolve(),
 };
diff --git a/mullvad-cli/src/cmds/status.rs b/mullvad-cli/src/cmds/status.rs
index fbf4e23e1a..06d75380aa 100644
--- a/mullvad-cli/src/cmds/status.rs
+++ b/mullvad-cli/src/cmds/status.rs
@@ -47,7 +47,7 @@ fn print_state(state: &TunnelStateTransition) {
         Connected => println!("Connected"),
         Connecting => println!("Connecting..."),
         Disconnected => println!("Disconnected"),
-        Disconnecting => println!("Disconnecting..."),
+        Disconnecting(_) => println!("Disconnecting..."),
     }
 }
 
diff --git a/talpid-core/src/tunnel_state_machine/disconnecting_state.rs b/talpid-core/src/tunnel_state_machine/disconnecting_state.rs
index a7f20ca5ea..47467e3090 100644
--- a/talpid-core/src/tunnel_state_machine/disconnecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/disconnecting_state.rs
@@ -4,7 +4,7 @@ use error_chain::ChainedError;
 use futures::sync::{mpsc, oneshot};
 use futures::{Async, Future, Stream};
 
-use talpid_types::tunnel::BlockReason;
+use talpid_types::tunnel::{ActionAfterDisconnect, BlockReason};
 
 use super::{
     BlockedState, ConnectingState, DisconnectedState, EventConsequence, ResultExt,
@@ -103,12 +103,14 @@ impl TunnelState for DisconnectingState {
             }
         });
 
+        let action_after_disconnect = after_disconnect.action();
+
         (
             TunnelStateWrapper::from(DisconnectingState {
                 exited,
                 after_disconnect,
             }),
-            TunnelStateTransition::Disconnecting,
+            TunnelStateTransition::Disconnecting(action_after_disconnect),
         )
     }
 
@@ -128,3 +130,14 @@ pub enum AfterDisconnect {
     Block(BlockReason, bool),
     Reconnect(TunnelParameters),
 }
+
+impl AfterDisconnect {
+    /// Build event representation of the action that will be taken after the disconnection.
+    pub fn action(&self) -> ActionAfterDisconnect {
+        match self {
+            AfterDisconnect::Nothing => ActionAfterDisconnect::Nothing,
+            AfterDisconnect::Block(..) => ActionAfterDisconnect::Block,
+            AfterDisconnect::Reconnect(_) => ActionAfterDisconnect::Reconnect,
+        }
+    }
+}
diff --git a/talpid-types/src/tunnel.rs b/talpid-types/src/tunnel.rs
index 6a1c7a55ed..3d911f640a 100644
--- a/talpid-types/src/tunnel.rs
+++ b/talpid-types/src/tunnel.rs
@@ -12,11 +12,20 @@ pub enum TunnelStateTransition {
     /// Tunnel is connected.
     Connected,
     /// Disconnecting tunnel.
-    Disconnecting,
+    Disconnecting(ActionAfterDisconnect),
     /// Tunnel is disconnected but secured by blocking all connections.
     Blocked(BlockReason),
 }
 
+/// Action that will be taken after disconnection is complete.
+#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum ActionAfterDisconnect {
+    Nothing,
+    Block,
+    Reconnect,
+}
+
 impl TunnelStateTransition {
     pub fn is_blocked(&self) -> bool {
         match self {