[refactor] some renaming, preliminar refactor

5 files changed, 105 insertions, 90 deletions

diff --git a/pkg/vpn/bonafide/bonafide.go b/pkg/vpn/bonafide/bonafide.go
index 28cfe44..4426da6 100644
--- a/pkg/vpn/bonafide/bonafide.go
+++ b/pkg/vpn/bonafide/bonafide.go
@@ -203,22 +203,13 @@ func (b *Bonafide) GetGateways(transport string) ([]Gateway, error) {
 	return b.eip.getGateways(transport), nil
 }
 
-func (b *Bonafide) SetDefaultGateway(name string) {
-	b.eip.setDefaultGateway(name)
-	b.sortGateways()
+func (b *Bonafide) SetManualGateway(name string) {
+	/* TODO use gateway-id instead - a location-id is probably more useful than
+	* the gateway hostname */
+	b.eip.setManualGateway(name)
 }
 
-func (b *Bonafide) GetOpenvpnArgs() ([]string, error) {
-	if b.eip == nil {
-		err := b.fetchEipJSON()
-		if err != nil {
-			return nil, err
-		}
-	}
-	return b.eip.getOpenvpnArgs(), nil
-}
-
-func (b *Bonafide) fetchGeolocation() ([]string, error) {
+func (b *Bonafide) requestBestGatewaysFromService() ([]string, error) {
 	/* FIXME in float deployments, geolocation is served on gemyip.domain/json, with a LE certificate, but in riseup is served behind the api certificate.
 	So this is a workaround until we streamline that behavior */
 	resp, err := b.client.Post(config.GeolocationAPI, "", nil)
@@ -254,12 +245,22 @@ func (b *Bonafide) fetchGeolocation() ([]string, error) {
 }
 
 func (b *Bonafide) sortGateways() {
-	geolocatedGateways, _ := b.fetchGeolocation()
+	serviceSelection, _ := b.requestBestGatewaysFromService()
 
-	if len(geolocatedGateways) > 0 {
-		b.eip.sortGatewaysByGeolocation(geolocatedGateways)
+	if len(serviceSelection) > 0 {
+		b.eip.autoSortGateways(serviceSelection)
 	} else {
 		log.Printf("Falling back to timezone heuristic for gateway selection")
 		b.eip.sortGatewaysByTimezone(b.tzOffsetHours)
 	}
 }
+
+func (b *Bonafide) GetOpenvpnArgs() ([]string, error) {
+	if b.eip == nil {
+		err := b.fetchEipJSON()
+		if err != nil {
+			return nil, err
+		}
+	}
+	return b.eip.getOpenvpnArgs(), nil
+}
diff --git a/pkg/vpn/bonafide/bonafide_test.go b/pkg/vpn/bonafide/bonafide_test.go
index ee54fdb..481c079 100644
--- a/pkg/vpn/bonafide/bonafide_test.go
+++ b/pkg/vpn/bonafide/bonafide_test.go
@@ -28,21 +28,37 @@ import (
 
 const (
 	certPath   = "testdata/cert"
+	geoPath    = "testdata/geoloc.json"
 	eip1Path   = "testdata/eip-service.json"
 	eipPath    = "testdata/eip-service3.json"
 	eipPathSip = "testdata/eip-service3-sip.json"
 )
 
+type nopCloser struct {
+	io.Reader
+}
+
+func (nopCloser) Close() error { return nil }
+
 type client struct {
 	path string
+	geo  string
 }
 
 func (c client) Post(url, contentType string, body io.Reader) (resp *http.Response, err error) {
-	f, err := os.Open(c.path)
-	return &http.Response{
-		Body:       f,
-		StatusCode: 200,
-	}, err
+	if strings.Contains(url, "api.black.riseup.net:9001/json") {
+		f, err := os.Open(c.geo)
+		return &http.Response{
+			Body:       f,
+			StatusCode: 200,
+		}, err
+	} else {
+		f, err := os.Open(c.path)
+		return &http.Response{
+			Body:       f,
+			StatusCode: 200,
+		}, err
+	}
 }
 
 func (c client) Do(req *http.Request) (*http.Response, error) {
@@ -54,7 +70,7 @@ func (c client) Do(req *http.Request) (*http.Response, error) {
 }
 
 func TestAnonGetCert(t *testing.T) {
-	b := Bonafide{client: client{certPath}}
+	b := Bonafide{client: client{certPath, geoPath}}
 	b.auth = &anonymousAuthentication{}
 	cert, err := b.GetPemCertificate()
 	if err != nil {
@@ -93,10 +109,11 @@ func TestGatewayTzLocation(t *testing.T) {
 
 	for tzOffset, location := range values {
 		b := Bonafide{
-			client:        client{eipPath},
+			client:        client{eipPath, geoPath},
 			tzOffsetHours: tzOffset,
 		}
 		gateways, err := b.GetGateways("openvpn")
+
 		if err != nil {
 			t.Errorf("getGateways returned an error: %v", err)
 			continue
@@ -114,7 +131,7 @@ func TestGatewayTzLocation(t *testing.T) {
 
 func TestOpenvpnGateways(t *testing.T) {
 	b := Bonafide{
-		client: client{eipPath},
+		client: client{eipPath, geoPath},
 	}
 	gateways, err := b.GetGateways("openvpn")
 	if err != nil {
@@ -150,7 +167,7 @@ func TestOpenvpnGateways(t *testing.T) {
 
 func TestObfs4Gateways(t *testing.T) {
 	b := Bonafide{
-		client: client{eipPath},
+		client: client{eipPath, geoPath},
 	}
 	gateways, err := b.GetGateways("obfs4")
 	if err != nil {
@@ -192,13 +209,11 @@ func TestObfs4Gateways(t *testing.T) {
 	}
 }
 
-/* TODO -- failClient instead? */
-
-type fallClient struct {
+type failingClient struct {
 	path string
 }
 
-func (c fallClient) Post(url, contentType string, body io.Reader) (resp *http.Response, err error) {
+func (c failingClient) Post(url, contentType string, body io.Reader) (resp *http.Response, err error) {
 	statusCode := 200
 	if strings.Contains(url, "3/config/eip-service.json") {
 		statusCode = 404
@@ -210,7 +225,7 @@ func (c fallClient) Post(url, contentType string, body io.Reader) (resp *http.Re
 	}, err
 }
 
-func (c fallClient) Do(req *http.Request) (*http.Response, error) {
+func (c failingClient) Do(req *http.Request) (*http.Response, error) {
 	f, err := os.Open(c.path)
 	return &http.Response{
 		Body:       f,
@@ -220,7 +235,7 @@ func (c fallClient) Do(req *http.Request) (*http.Response, error) {
 
 func TestEipServiceV1Fallback(t *testing.T) {
 	b := Bonafide{
-		client: fallClient{eip1Path},
+		client: failingClient{eip1Path},
 	}
 	gateways, err := b.GetGateways("obfs4")
 	if err != nil {
diff --git a/pkg/vpn/bonafide/eip_service.go b/pkg/vpn/bonafide/eip_service.go
index 21cc8e8..9c8dc66 100644
--- a/pkg/vpn/bonafide/eip_service.go
+++ b/pkg/vpn/bonafide/eip_service.go
@@ -16,14 +16,16 @@ import (
 
 type eipService struct {
 	Gateways             []gatewayV3
+	SelectedGateways     []gatewayV3
 	Locations            map[string]location
+	defaultGateway       string
 	OpenvpnConfiguration openvpnConfig `json:"openvpn_configuration"`
 	auth                 string
-	defaultGateway       string
 }
 
 type eipServiceV1 struct {
 	Gateways             []gatewayV1
+	SelectedGateways     []gatewayV1
 	Locations            map[string]location
 	OpenvpnConfiguration openvpnConfig `json:"openvpn_configuration"`
 }
@@ -161,7 +163,8 @@ func decodeEIP1(body io.Reader) (*eipService, error) {
 
 func (eip eipService) getGateways(transport string) []Gateway {
 	gws := []Gateway{}
-	for _, g := range eip.Gateways {
+	// TODO check that len(selected) != 0
+	for _, g := range eip.SelectedGateways {
 		for _, t := range g.Capabilities.Transport {
 			if t.Type != transport {
 				continue
@@ -178,47 +181,27 @@ func (eip eipService) getGateways(transport string) []Gateway {
 			gws = append(gws, gateway)
 		}
 	}
+	// TODO return only top 3, at least for openvpn
 	return gws
 }
 
-func (eip *eipService) setDefaultGateway(name string) {
+func (eip *eipService) setManualGateway(name string) {
 	eip.defaultGateway = name
-}
 
-func (eip eipService) getOpenvpnArgs() []string {
-	args := []string{}
-	for arg, value := range eip.OpenvpnConfiguration {
-		switch v := value.(type) {
-		case string:
-			args = append(args, "--"+arg)
-			args = append(args, strings.Split(v, " ")...)
-		case bool:
-			if v {
-				args = append(args, "--"+arg)
-			}
-		default:
-			log.Printf("Unknown openvpn argument type: %s - %v", arg, value)
+	gws := make([]gatewayV3, 0)
+	for _, gw := range eip.Gateways {
+		if gw.Location == eip.defaultGateway {
+			gws = append(gws, gw)
+			break
 		}
 	}
-	return args
+	eip.SelectedGateways = gws
 }
 
-func (eip *eipService) sortGatewaysByGeolocation(geolocatedGateways []string) {
+func (eip *eipService) autoSortGateways(serviceSelection []string) {
 	gws := make([]gatewayV3, 0)
 
-	/* TODO this probably should be moved out of this method */
-	if eip.defaultGateway != "" {
-		for _, gw := range eip.Gateways {
-			if gw.Location == eip.defaultGateway {
-				gws = append(gws, gw)
-				break
-			}
-		}
-		// a manually selected gateway means we do want exactly one remote
-		return
-	}
-
-	for _, host := range geolocatedGateways {
+	for _, host := range serviceSelection {
 		for _, gw := range eip.Gateways {
 			if gw.Host == host {
 				gws = append(gws, gw)
@@ -229,22 +212,13 @@ func (eip *eipService) sortGatewaysByGeolocation(geolocatedGateways []string) {
 	if len(gws) == 0 {
 		// this can happen if a misconfigured geoip service does not match the
 		// providers list we got.
-		log.Println("ERROR: avoiding to nullify eip.Gateways. Is the geolocation service properly configured?")
+		log.Println("ERROR: did not get any useful selection. Is the geolocation service properly configured?")
+		eip.SelectedGateways = eip.Gateways
 	} else {
-		if len(gws) > 2 {
-			eip.Gateways = gws[:3]
-		} else {
-			eip.Gateways = gws
-		}
-		log.Println("Picked best gateways for location:", eip.Gateways)
+		eip.SelectedGateways = gws
 	}
 }
 
-type gatewayDistance struct {
-	gateway  gatewayV3
-	distance int
-}
-
 func (eip *eipService) sortGatewaysByTimezone(tzOffsetHours int) {
 	gws := []gatewayDistance{}
 
@@ -271,9 +245,33 @@ func (eip *eipService) sortGatewaysByTimezone(tzOffsetHours int) {
 	}
 	sort.Slice(gws, cmp)
 
+	eip.SelectedGateways = make([]gatewayV3, len(eip.Gateways))
 	for i, gw := range gws {
-		eip.Gateways[i] = gw.gateway
+		eip.SelectedGateways[i] = gw.gateway
+	}
+}
+
+func (eip eipService) getOpenvpnArgs() []string {
+	args := []string{}
+	for arg, value := range eip.OpenvpnConfiguration {
+		switch v := value.(type) {
+		case string:
+			args = append(args, "--"+arg)
+			args = append(args, strings.Split(v, " ")...)
+		case bool:
+			if v {
+				args = append(args, "--"+arg)
+			}
+		default:
+			log.Printf("Unknown openvpn argument type: %s - %v", arg, value)
+		}
 	}
+	return args
+}
+
+type gatewayDistance struct {
+	gateway  gatewayV3
+	distance int
 }
 
 func tzDistance(offset1, offset2 int) int {
diff --git a/pkg/vpn/bonafide/testdata/eip-service3.json b/pkg/vpn/bonafide/testdata/eip-service3.json
index cefd6c0..31a2a3b 100644
--- a/pkg/vpn/bonafide/testdata/eip-service3.json
+++ b/pkg/vpn/bonafide/testdata/eip-service3.json
@@ -14,7 +14,7 @@
             "protocols": [
               "tcp"
             ]
-	  },
+          },
           {
             "type": "obfs4",
             "ports": [
@@ -23,11 +23,11 @@
             "protocols": [
               "tcp"
             ],
-	    "options": {
+            "options": {
               "cert": "obfs-cert",
               "iat-mode": "0"
-	    }
-	  }
+            }
+          }
         ],
         "user_ips": false
       },
@@ -49,7 +49,7 @@
             "protocols": [
               "tcp"
             ]
-	  }
+          }
         ],
         "user_ips": false
       },
@@ -71,7 +71,7 @@
             "protocols": [
               "tcp"
             ]
-	  },
+          },
           {
             "type": "obfs4",
             "ports": [
@@ -80,11 +80,11 @@
             "protocols": [
               "tcp"
             ],
-	    "options": {
+            "options": {
               "cert": "obfs-cert",
               "iat-mode": "0"
-	    }
-	  }
+            }
+          }
         ],
         "user_ips": false
       },
@@ -106,7 +106,7 @@
             "protocols": [
               "tcp"
             ]
-	  }
+          }
         ],
         "user_ips": false
       },
@@ -128,11 +128,11 @@
             "protocols": [
               "tcp"
             ],
-	    "options": {
+            "options": {
               "cert": "obfs-cert",
               "iat-mode": "0"
-	    }
-	  }
+            }
+          }
         ],
         "user_ips": false
       },
diff --git a/pkg/vpn/bonafide/testdata/geoloc.json b/pkg/vpn/bonafide/testdata/geoloc.json
new file mode 100644
index 0000000..3f984c6
--- /dev/null
+++ b/pkg/vpn/bonafide/testdata/geoloc.json
@@ -0,0 +1 @@
+{"ip":"1.1.1.1","cc":"UK","city":"London","lat":0,"lon":0,"gateways":[]}