From 8a27de6c7d92fbadbb9083dc3a2dcd6ab8cdcb92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Thu, 14 Nov 2019 17:58:31 +0100 Subject: [PATCH] Remove all settings which are not implemented (#73) * Remove all settings which are not implemented * Add support for migrating config files from version 0 -> 1 Add basic unittests for the config file migration * Update the config file used by travis * Fix typos in console log messages * Improve the comment * Fix type of limit * Set informed consent to true in the travis config --- config/parser.go | 39 +++++------ config/parser_test.go | 97 ++++++++++++++++++++++++++- config/settings.go | 92 ++----------------------- config/testdata/config-v0.json | 63 +++++++++++++++++ config/testdata/valid-config.json | 57 ++-------------- data/default-config.json | 52 +------------- nettests/websites/web_connectivity.go | 6 +- ooni.go | 3 + testdata/testing-config.json | 52 ++------------ 9 files changed, 197 insertions(+), 264 deletions(-) create mode 100644 config/testdata/config-v0.json diff --git a/config/parser.go b/config/parser.go index 5b5e15e..7a36ea6 100644 --- a/config/parser.go +++ b/config/parser.go @@ -11,6 +11,9 @@ import ( "github.com/pkg/errors" ) +// ConfigVersion is the current version of the config +const ConfigVersion = 1 + // ReadConfig reads the configuration from the path func ReadConfig(path string) (*Config, error) { b, err := ioutil.ReadFile(path) @@ -34,13 +37,12 @@ func ParseConfig(b []byte) (*Config, error) { return nil, errors.Wrap(err, "parsing json") } - if err := c.Default(); err != nil { - return nil, errors.Wrap(err, "defaulting") + home, err := utils.GetOONIHome() + if err != nil { + return nil, err } + c.path = utils.ConfigPath(home) - if err := c.Validate(); err != nil { - return nil, errors.Wrap(err, "validating") - } if c.Advanced.SendCrashReports == false { log.Info("Disabling crash reporting.") crashreport.Disabled = true @@ -55,14 +57,10 @@ type Config struct { Comment string `json:"_"` Version int64 `json:"_version"` InformedConsent bool `json:"_informed_consent"` - IsBeta bool `json:"_is_beta"` // This is a boolean flag used to indicate this installation of OONI Probe was a beta install. These installations will have their data deleted across releases. - AutoUpdate bool `json:"auto_update"` - Sharing Sharing `json:"sharing"` - Notifications Notifications `json:"notifications"` - AutomatedTesting AutomatedTesting `json:"automated_testing"` - NettestGroups NettestGroups `json:"test_settings"` - Advanced Advanced `json:"advanced"` + Sharing Sharing `json:"sharing"` + Nettests Nettests `json:"nettests"` + Advanced Advanced `json:"advanced"` mutex sync.Mutex path string @@ -92,18 +90,11 @@ func (c *Config) Unlock() { c.mutex.Unlock() } -// Default config settings -func (c *Config) Default() error { - home, err := utils.GetOONIHome() - if err != nil { - return err +// MaybeMigrate checks the current config version and the config file on disk +// and if necessary performs and upgrade of the configuration file. +func (c *Config) MaybeMigrate() error { + if c.Version < ConfigVersion { + return c.Write() } - - c.path = utils.ConfigPath(home) - return nil -} - -// Validate the config file -func (c *Config) Validate() error { return nil } diff --git a/config/parser_test.go b/config/parser_test.go index e26a569..3caf366 100644 --- a/config/parser_test.go +++ b/config/parser_test.go @@ -1,19 +1,110 @@ package config import ( + "crypto/sha256" + "encoding/hex" + "io" + "io/ioutil" + "os" "testing" ) +func getShasum(path string) (string, error) { + hasher := sha256.New() + + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + if _, err := io.Copy(hasher, f); err != nil { + return "", err + } + return hex.EncodeToString(hasher.Sum(nil)), nil +} + func TestParseConfig(t *testing.T) { config, err := ReadConfig("testdata/valid-config.json") if err != nil { t.Error(err) } - if len(config.NettestGroups.Middlebox.EnabledTests) < 0 { - t.Error("at least one middlebox test should be enabled") - } if config.Sharing.IncludeCountry == false { t.Error("country should be included") } } + +func TestUpdateConfig(t *testing.T) { + tmpFile, err := ioutil.TempFile(os.TempDir(), "ooniconfig-") + if err != nil { + t.Error(err) + } + configPath := tmpFile.Name() + defer os.Remove(configPath) + + data, err := ioutil.ReadFile("testdata/config-v0.json") + if err != nil { + t.Error(err) + } + err = ioutil.WriteFile(configPath, data, 0644) + if err != nil { + t.Error(err) + } + + origShasum, err := getShasum(configPath) + if err != nil { + t.Error(err) + } + config, err := ReadConfig(configPath) + if err != nil { + t.Error(err) + } + origIncludeIP := config.Sharing.IncludeIP + origIncludeASN := config.Sharing.IncludeASN + origIncludeCountry := config.Sharing.IncludeCountry + origUploadResults := config.Sharing.UploadResults + origInformedConsent := config.InformedConsent + if err != nil { + t.Error(err) + } + + config.MaybeMigrate() + migratedShasum, err := getShasum(configPath) + if err != nil { + t.Error(err) + } + if migratedShasum == origShasum { + t.Fatal("the config was not migrated") + } + + newConfig, err := ReadConfig(configPath) + if err != nil { + t.Error(err) + } + if newConfig.Sharing.IncludeIP != origIncludeIP { + t.Error("includeIP differs") + } + if newConfig.Sharing.IncludeASN != origIncludeASN { + t.Error("includeASN differs") + } + if newConfig.Sharing.IncludeCountry != origIncludeCountry { + t.Error("includeCountry differs") + } + if newConfig.Sharing.UploadResults != origUploadResults { + t.Error("UploadResults differs") + } + if newConfig.InformedConsent != origInformedConsent { + t.Error("InformedConsent differs") + } + + // Check that the config file stays the same if it's already the most up to + // date version + config.MaybeMigrate() + finalShasum, err := getShasum(configPath) + if err != nil { + t.Error(err) + } + if migratedShasum != finalShasum { + t.Fatal("the config was migrated again") + } +} diff --git a/config/settings.go b/config/settings.go index 5edbaee..09051f6 100644 --- a/config/settings.go +++ b/config/settings.go @@ -33,102 +33,22 @@ var websiteCategories = []string{ "XED", } -// NettestConfig represents the configuration for a particular nettest -type NettestConfig struct { - Name string - Options string -} - -// Websites test group -type Websites struct { - EnabledCategories []string `json:"enabled_categories"` - Limit int `json:"limit"` -} - -// NettestConfigs returns a list configured enabled tests for the group -func (s *Websites) NettestConfigs() []NettestConfig { - var nts []NettestConfig - nts = append(nts, NettestConfig{"web_connectivity", "options"}) - return nts -} - -// InstantMessaging nettest group -type InstantMessaging struct { - EnabledTests []string `json:"enabled_tests"` -} - -func (s *InstantMessaging) isEnabled(nt string) bool { - for _, v := range s.EnabledTests { - if v == nt { - return true - } - } - return false -} - -// NettestConfigs returns a list configured enabled tests for the group -func (s *InstantMessaging) NettestConfigs() []NettestConfig { - var nts []NettestConfig - if s.isEnabled("facebook_messenger") { - nts = append(nts, NettestConfig{"facebook_messenger", "options"}) - } - if s.isEnabled("telegram") { - nts = append(nts, NettestConfig{"telegram", "options"}) - } - if s.isEnabled("whatsapp") { - nts = append(nts, NettestConfig{"whatsapp", "options"}) - } - return nts -} - -// Performance nettest group -type Performance struct { - NDTServer string `json:"ndt_server"` - NDTServerPort string `json:"ndt_server_port"` - DashServer string `json:"dash_server"` - DashServerPort string `json:"dash_server_port"` -} - -// Middlebox nettest group -type Middlebox struct { - EnabledTests []string `json:"enabled_tests"` -} - -// NettestGroups related settings -type NettestGroups struct { - Websites Websites `json:"websites"` - InstantMessaging InstantMessaging `json:"instant_messaging"` - Performance Performance `json:"performance"` - Middlebox Middlebox `json:"middlebox"` -} - -// Notifications settings -type Notifications struct { - Enabled bool `json:"enabled"` - NotifyOnTestCompletion bool `json:"notify_on_test_completion"` - NotifyOnNews bool `json:"notify_on_news"` -} - // Sharing settings type Sharing struct { IncludeIP bool `json:"include_ip"` IncludeASN bool `json:"include_asn"` IncludeCountry bool `json:"include_country"` - IncludeGPS bool `json:"include_gps"` UploadResults bool `json:"upload_results"` } // Advanced settings type Advanced struct { - UseDomainFronting bool `json:"use_domain_fronting"` - SendCrashReports bool `json:"send_crash_reports"` - CollectorURL string `json:"collector_url"` - BouncerURL string `json:"bouncer_url"` + SendCrashReports bool `json:"send_crash_reports"` + CollectorURL string `json:"collector_url"` + BouncerURL string `json:"bouncer_url"` } -// AutomatedTesting settings -type AutomatedTesting struct { - Enabled bool `json:"enabled"` - EnabledTests []string `json:"enabled_tests"` - MonthlyAllowance string `json:"monthly_allowance"` +// Nettests related settings +type Nettests struct { + WebsitesURLLimit int64 `json:"websites_url_limit"` } diff --git a/config/testdata/config-v0.json b/config/testdata/config-v0.json new file mode 100644 index 0000000..0d95dac --- /dev/null +++ b/config/testdata/config-v0.json @@ -0,0 +1,63 @@ +{ + "_": "This is your OONI Probe config file. See https://ooni.io/help/probe-cli for help", + "_version": 0, + "_informed_consent": true, + "_is_beta": true, + "auto_update": true, + "sharing": { + "include_ip": false, + "include_asn": true, + "include_country": true, + "include_gps": true, + "upload_results": true + }, + "notifications": { + "enabled": true, + "notify_on_test_completion": true, + "notify_on_news": false + }, + "automated_testing": { + "enabled": false, + "enabled_tests": [ + "web-connectivity", + "facebook-messenger", + "whatsapp", + "telegram", + "dash", + "ndt", + "http-invalid-request-line", + "http-header-field-manipulation" + ], + "monthly_allowance": "300MB" + }, + "test_settings": { + "websites": { + "enabled_categories": [] + }, + "instant_messaging": { + "enabled_tests": [ + "facebook-messenger", + "whatsapp", + "telegram" + ] + }, + "performance": { + "ndt_server": "auto", + "ndt_server_port": "auto", + "dash_server": "auto", + "dash_server_port": "auto" + }, + "middlebox": { + "enabled_tests": [ + "http-invalid-request-line", + "http-header-field-manipulation" + ] + } + }, + "advanced": { + "use_domain_fronting": false, + "send_crash_reports": true, + "collector_url": "", + "bouncer_url": "https://bouncer.ooni.io" + } +} diff --git a/config/testdata/valid-config.json b/config/testdata/valid-config.json index 4a44336..725b07f 100644 --- a/config/testdata/valid-config.json +++ b/config/testdata/valid-config.json @@ -1,65 +1,20 @@ { "_": "This is your OONI Probe config file. See https://ooni.io/help/probe-cli for help", - "_version": 0, + "_version": 1, "_informed_consent": false, - "auto_update": true, "sharing": { "include_ip": false, - "include_country": true, "include_asn": true, - "include_gps": true, + "include_country": true, "upload_results": true }, - "notifications": { - "enabled": true, - "notify_on_test_completion": true, - "notify_on_news": false - }, - "automated_testing": { - "enabled": false, - "enabled_tests": [ - "web-connectivity", - "facebook-messenger", - "whatsapp", - "telegram", - "dash", - "ndt", - "http-invalid-request-line", - "http-header-field-manipulation" - ], - "monthly_allowance": "300MB" - }, - "test_settings": { - "websites": { - "enabled_categories": [] - }, - "instant_messaging": { - "enabled_tests": [ - "facebook-messenger", - "whatsapp", - "telegram" - ] - }, - "performance": { - "enabled_tests": [ - "ndt" - ], - "ndt_server": "auto", - "ndt_server_port": "auto", - "dash_server": "auto", - "dash_server_port": "auto" - }, - "middlebox": { - "enabled_tests": [ - "http-invalid-request-line", - "http-header-field-manipulation" - ] - } + "nettests": { + "websites_url_limit": 0 }, "advanced": { "use_domain_fronting": false, "send_crash_reports": true, - "bouncer_url": "https://bouncer.ooni.io", - "collector_url": "https://c.collector.ooni.io" + "collector_url": "", + "bouncer_url": "https://bouncer.ooni.io" } } diff --git a/data/default-config.json b/data/default-config.json index d7d1e7a..725b07f 100644 --- a/data/default-config.json +++ b/data/default-config.json @@ -1,61 +1,15 @@ { "_": "This is your OONI Probe config file. See https://ooni.io/help/probe-cli for help", - "_version": 0, + "_version": 1, "_informed_consent": false, - "_is_beta": true, - "auto_update": true, "sharing": { "include_ip": false, "include_asn": true, "include_country": true, - "include_gps": true, "upload_results": true }, - "notifications": { - "enabled": true, - "notify_on_test_completion": true, - "notify_on_news": false - }, - "automated_testing": { - "enabled": false, - "enabled_tests": [ - "web-connectivity", - "facebook-messenger", - "whatsapp", - "telegram", - "dash", - "ndt", - "http-invalid-request-line", - "http-header-field-manipulation" - ], - "monthly_allowance": "300MB" - }, - "test_settings": { - "websites": { - "enabled_categories": [] - }, - "instant_messaging": { - "enabled_tests": [ - "facebook-messenger", - "whatsapp", - "telegram" - ] - }, - "performance": { - "enabled_tests": [ - "ndt" - ], - "ndt_server": "auto", - "ndt_server_port": "auto", - "dash_server": "auto", - "dash_server_port": "auto" - }, - "middlebox": { - "enabled_tests": [ - "http-invalid-request-line", - "http-header-field-manipulation" - ] - } + "nettests": { + "websites_url_limit": 0 }, "advanced": { "use_domain_fronting": false, diff --git a/nettests/websites/web_connectivity.go b/nettests/websites/web_connectivity.go index ce25b96..63a457a 100644 --- a/nettests/websites/web_connectivity.go +++ b/nettests/websites/web_connectivity.go @@ -6,11 +6,11 @@ import ( "github.com/ooni/probe-cli/nettests" ) -func lookupURLs(ctl *nettests.Controller, limit int) ([]string, map[int64]int64, error) { +func lookupURLs(ctl *nettests.Controller, limit int64) ([]string, map[int64]int64, error) { var urls []string urlIDMap := make(map[int64]int64) config := ctl.Ctx.Session.NewTestListsConfig() - config.Limit = limit + config.Limit = int(limit) client := ctl.Ctx.Session.NewTestListsClient() testlist, err := client.Fetch(config) if err != nil { @@ -38,7 +38,7 @@ type WebConnectivity struct { // Run starts the test func (n WebConnectivity) Run(ctl *nettests.Controller) error { - urls, urlIDMap, err := lookupURLs(ctl, ctl.Ctx.Config.NettestGroups.Websites.Limit) + urls, urlIDMap, err := lookupURLs(ctl, ctl.Ctx.Config.Nettests.WebsitesURLLimit) if err != nil { return err } diff --git a/ooni.go b/ooni.go index 3aed806..58d20eb 100644 --- a/ooni.go +++ b/ooni.go @@ -73,6 +73,9 @@ func (c *Context) Init() error { if err != nil { return err } + if err = c.Config.MaybeMigrate(); err != nil { + return errors.Wrap(err, "migrating config") + } c.dbPath = utils.DBDir(c.Home, "main") log.Debugf("Connecting to database sqlite3://%s", c.dbPath) diff --git a/testdata/testing-config.json b/testdata/testing-config.json index 749d58f..621ff90 100644 --- a/testdata/testing-config.json +++ b/testdata/testing-config.json @@ -1,59 +1,15 @@ { "_": "This is your OONI Probe config file. See https://ooni.io/help/probe-cli for help", - "_version": 0, + "_version": 1, "_informed_consent": true, - "_is_beta": true, - "auto_update": true, "sharing": { - "include_ip": false, + "include_ip": true, "include_asn": true, "include_country": true, - "include_gps": true, "upload_results": true }, - "notifications": { - "enabled": true, - "notify_on_test_completion": true, - "notify_on_news": false - }, - "automated_testing": { - "enabled": false, - "enabled_tests": [ - "web-connectivity", - "facebook-messenger", - "whatsapp", - "telegram", - "dash", - "ndt", - "http-invalid-request-line", - "http-header-field-manipulation" - ], - "monthly_allowance": "300MB" - }, - "test_settings": { - "websites": { - "enabled_categories": [], - "limit": 10 - }, - "instant_messaging": { - "enabled_tests": [ - "facebook-messenger", - "whatsapp", - "telegram" - ] - }, - "performance": { - "ndt_server": "auto", - "ndt_server_port": "auto", - "dash_server": "auto", - "dash_server_port": "auto" - }, - "middlebox": { - "enabled_tests": [ - "http-invalid-request-line", - "http-header-field-manipulation" - ] - } + "nettests": { + "websites_url_limit": 10 }, "advanced": { "use_domain_fronting": false,