From af3eaa33d462a30b983c43c4e1dc6c095b81661d Mon Sep 17 00:00:00 2001 From: Justin Myers Date: Sat, 22 Feb 2025 14:10:34 -0800 Subject: [PATCH 1/3] Secrets cleanup --- adafruit_portalbase/network.py | 102 +++++++++++-------- adafruit_portalbase/wifi_coprocessor.py | 13 +-- examples/portalbase_simpletest.py | 8 +- optional_requirements.txt | 2 + tests/test_get_settings.py | 127 ++++++++++++++++++++++++ 5 files changed, 194 insertions(+), 58 deletions(-) create mode 100644 tests/test_get_settings.py diff --git a/adafruit_portalbase/network.py b/adafruit_portalbase/network.py index 9d4390d..fe63ae6 100755 --- a/adafruit_portalbase/network.py +++ b/adafruit_portalbase/network.py @@ -24,6 +24,7 @@ import gc import os import time +import warnings from adafruit_fakerequests import Fake_Requests from adafruit_io.adafruit_io import IO_HTTP, AdafruitIO_RequestError @@ -57,11 +58,18 @@ CONTENT_JSON = const(2) CONTENT_IMAGE = const(3) -OLD_SETTINGS = { +OLD_SECRETS = { + "ADAFRUIT_AIO_KEY": "aio_key", + "ADAFRUIT_AIO_USERNAME": "aio_username", + "AIO_KEY": "aio_key", + "AIO_USERNAME": "aio_username", "CIRCUITPY_WIFI_SSID": "ssid", "CIRCUITPY_WIFI_PASSWORD": "password", - "AIO_USERNAME": "aio_username", - "AIO_KEY": "aio_key", +} + +OLD_SETTINGS = { + "ADAFRUIT_AIO_KEY": "AIO_KEY", + "ADAFRUIT_AIO_USERNAME": "AIO_USERNAME", } @@ -83,12 +91,11 @@ class NetworkBase: :param bool extract_values: If true, single-length fetched values are automatically extracted from lists and tuples. Defaults to ``True``. :param debug: Turn on debug print outs. Defaults to False. - :param list secrets_data: An optional list in place of the data contained in the secrets.py file """ def __init__( # noqa: PLR0912,PLR0913 Too many branches,Too many arguments in function definition - self, wifi_module, *, extract_values=True, debug=False, secrets_data=None + self, wifi_module, *, extract_values=True, debug=False ): self._wifi = wifi_module self._debug = debug @@ -101,11 +108,6 @@ def __init__( # noqa: PLR0912,PLR0913 Too many branches,Too many arguments in f ] self._settings = {} - if secrets_data is not None: - for key, value in secrets_data.items(): - if key in OLD_SETTINGS: - key = OLD_SETTINGS.get(key) # noqa: PLW2901 `for` loop variable `value` overwritten by assignment target - self._settings[key] = value self._wifi_credentials = None self.requests = None @@ -120,31 +122,44 @@ def __init__( # noqa: PLR0912,PLR0913 Too many branches,Too many arguments in f gc.collect() - def _get_setting(self, setting_name, show_error=True): + def _get_setting(self, setting_name): + # if setting is has already been found, return it if setting_name in self._settings: return self._settings[setting_name] - old_setting_name = setting_name + # if setting is in settings.toml return it + env_value = os.getenv(setting_name) + if env_value is not None: + self._settings[setting_name] = env_value + return env_value + + # if setting old name is in settings.toml return it if setting_name in OLD_SETTINGS: old_setting_name = OLD_SETTINGS.get(setting_name) - if os.getenv(setting_name) is not None: - return os.getenv(setting_name) + env_value = os.getenv(old_setting_name) + if env_value is not None: + self._settings[setting_name] = env_value + return env_value + try: from secrets import secrets except ImportError: - secrets = {} - if old_setting_name in secrets.keys(): - self._settings[setting_name] = secrets[old_setting_name] - return self._settings[setting_name] - if show_error: - if setting_name in ("CIRCUITPY_WIFI_SSID", "CIRCUITPY_WIFI_PASSWORD"): - print( - """WiFi settings are kept in settings.toml, please add them there! - the secrets dictionary must contain 'CIRCUITPY_WIFI_SSID' and 'CIRCUITPY_WIFI_PASSWORD' - at a minimum in order to use network related features""" - ) - else: - print(f"{setting_name} not found. Please add this setting to settings.toml.") + return None + + # if setting is in legacy secrets.py return it + secrets_setting_name = setting_name + if setting_name in OLD_SECRETS: + # translate common names + secrets_setting_name = OLD_SECRETS.get(setting_name) + env_value = secrets.get(secrets_setting_name) + if env_value is not None: + warnings.warn( + "The using of `secrets`, is deprecated. Please put your settings in " + "settings.toml" + ) + self._settings[setting_name] = env_value + return env_value + return None def neo_status(self, value): @@ -206,17 +221,17 @@ def get_strftime(self, time_format, location=None, max_attempts=10): api_url = None reply = None try: - aio_username = self._get_setting("AIO_USERNAME") - aio_key = self._get_setting("AIO_KEY") + aio_username = self._get_setting("ADAFRUIT_AIO_USERNAME") + aio_key = self._get_setting("ADAFRUIT_AIO_KEY") except KeyError: raise KeyError( "\n\nOur time service requires a login/password to rate-limit. " - "Please register for a free adafruit.io account and place the user/key " - "in your secrets file under 'AIO_USERNAME' and 'AIO_KEY'" + "Please register for a free adafruit.io account and place the user/key in " + "your settings.toml file under 'ADAFRUIT_AIO_USERNAME' and 'ADAFRUIT_AIO_KEY'" ) from KeyError if location is None: - location = self._get_setting("timezone", False) + location = self._get_setting("timezone") if location: print("Getting time for timezone", location) api_url = (TIME_SERVICE + "&tz=%s") % (aio_username, aio_key, location) @@ -242,7 +257,8 @@ def get_strftime(self, time_format, location=None, max_attempts=10): reply = response.text except KeyError: raise KeyError( - "Was unable to lookup the time, try setting secrets['timezone'] according to http://worldtimeapi.org/timezones" + "Was unable to lookup the time, try setting 'timezone' in your settings.toml" + "according to http://worldtimeapi.org/timezones" ) from KeyError # now clean up response.close() @@ -346,7 +362,7 @@ def wget(self, url, filename, *, chunk_size=12000, headers=None): def connect(self, max_attempts=10): """ - Connect to WiFi using the settings found in secrets.py + Connect to WiFi using the settings found in settings.toml :param max_attempts: The maximum number of attempts to connect to WiFi before failing or use None to disable. Defaults to 10. @@ -361,7 +377,7 @@ def connect(self, max_attempts=10): } ] - networks = self._get_setting("networks", False) + networks = self._get_setting("networks") if networks is not None: if isinstance(networks, (list, tuple)): self._wifi_credentials = networks @@ -370,14 +386,14 @@ def connect(self, max_attempts=10): "'networks' must be a list/tuple of dicts of 'ssid' and 'password'" ) - for secret_entry in self._wifi_credentials: + for credentials in self._wifi_credentials: self._wifi.neo_status(STATUS_CONNECTING) attempt = 1 while not self._wifi.is_connected: - # secrets dictionary must contain 'ssid' and 'password' at a minimum - print("Connecting to AP", secret_entry["ssid"]) - if secret_entry["ssid"] == "CHANGE ME" or secret_entry["password"] == "CHANGE ME": + # credentials must contain 'CIRCUITPY_WIFI_SSID' and 'CIRCUITPY_WIFI_PASSWORD' + print("Connecting to AP", credentials["ssid"]) + if credentials["ssid"] == "CHANGE ME" or credentials["password"] == "CHANGE ME": change_me = "\n" + "*" * 45 change_me += "\nPlease update the 'settings.toml' file on your\n" change_me += "CIRCUITPY drive to include your local WiFi\n" @@ -387,7 +403,7 @@ def connect(self, max_attempts=10): raise OSError(change_me) self._wifi.neo_status(STATUS_NO_CONNECTION) # red = not connected try: - self._wifi.connect(secret_entry["ssid"], secret_entry["password"]) + self._wifi.connect(credentials["ssid"], credentials["password"]) self.requests = self._wifi.requests self._wifi.neo_status(STATUS_CONNECTED) break @@ -412,11 +428,11 @@ def _get_io_client(self): self.connect() try: - aio_username = self._get_setting("AIO_USERNAME") - aio_key = self._get_setting("AIO_KEY") + aio_username = self._get_setting("ADAFRUIT_AIO_USERNAME") + aio_key = self._get_setting("ADAFRUIT_AIO_KEY") except KeyError: raise KeyError( - "Adafruit IO secrets are kept in secrets.py, please add them there!\n\n" + "Adafruit IO settings are kept in settings.toml, please add them there!\n\n" ) from KeyError self._io_client = IO_HTTP(aio_username, aio_key, self._wifi.requests) diff --git a/adafruit_portalbase/wifi_coprocessor.py b/adafruit_portalbase/wifi_coprocessor.py index ab3b912..f0b6aa7 100644 --- a/adafruit_portalbase/wifi_coprocessor.py +++ b/adafruit_portalbase/wifi_coprocessor.py @@ -70,7 +70,6 @@ def __init__(self, *, status_led=None, esp=None, external_spi=None): if self.esp.is_connected: self._set_requests() - self._manager = None gc.collect() @@ -81,9 +80,9 @@ def _set_requests(self): def connect(self, ssid, password): """ - Connect to WiFi using the settings found in secrets.py + Connect to WiFi using the settings found in settings.toml """ - self.esp.connect({"ssid": ssid, "password": password}) + self.esp.connect(ssid, password) self._set_requests() def neo_status(self, value): @@ -95,14 +94,6 @@ def neo_status(self, value): if self.neopix: self.neopix.fill(value) - def manager(self, secrets): - """Initialize the WiFi Manager if it hasn't been cached and return it""" - if self._manager is None: - self._manager = adafruit_esp32spi_wifimanager.ESPSPI_WiFiManager( - self.esp, secrets, None - ) - return self._manager - @property def is_connected(self): """Return whether we are connected.""" diff --git a/examples/portalbase_simpletest.py b/examples/portalbase_simpletest.py index 20b126c..d1cc4af 100644 --- a/examples/portalbase_simpletest.py +++ b/examples/portalbase_simpletest.py @@ -6,12 +6,12 @@ used directly by end users. This example shows one such usage with the PyPortal library. See MatrixPortal, MagTag, and PyPortal libraries for more examples. """ -# NOTE: Make sure you've created your secrets.py file before running this example -# https://learn.adafruit.com/adafruit-pyportal/internet-connect#whats-a-secrets-file-17-2 +# NOTE: Make sure you've created your settings.toml file before running this example +# https://learn.adafruit.com/adafruit-pyportal/create-your-settings-toml-file import board -import displayio from adafruit_pyportal import PyPortal +from displayio import CIRCUITPYTHON_TERMINAL # Set a data source URL TEXT_URL = "http://wifitest.adafruit.com/testwifi/index.html" @@ -20,7 +20,7 @@ pyportal = PyPortal(url=TEXT_URL, status_neopixel=board.NEOPIXEL) # Set display to show REPL -board.DISPLAY.root_group = displayio.CIRCUITPYTHON_TERMINAL +board.DISPLAY.root_group = CIRCUITPYTHON_TERMINAL # Go get that data print("Fetching text from", TEXT_URL) diff --git a/optional_requirements.txt b/optional_requirements.txt index d4e27c4..b4e7041 100644 --- a/optional_requirements.txt +++ b/optional_requirements.txt @@ -1,3 +1,5 @@ # SPDX-FileCopyrightText: 2022 Alec Delaney, for Adafruit Industries # # SPDX-License-Identifier: Unlicense + +pytest diff --git a/tests/test_get_settings.py b/tests/test_get_settings.py new file mode 100644 index 0000000..d316cd0 --- /dev/null +++ b/tests/test_get_settings.py @@ -0,0 +1,127 @@ +# SPDX-FileCopyrightText: 2025 Justin Myers +# +# SPDX-License-Identifier: Unlicense + +import os +import sys +from unittest import mock + +import pytest + +from adafruit_portalbase.network import NetworkBase + + +@pytest.fixture +def secrets(): + sys.modules["secrets.secrets"] = { + "aio_key": "secret_aio_key", + "aio_username": "secret_aio_username", + "password": "secret_password", + "ssid": "secret_ssid", + "timezone": "secret_timezone", + "fallback_test": "secret_fallback_test", + } + yield + del sys.modules["secrets.secrets"] + + +@pytest.fixture +def settings_toml_current(monkeypatch): + monkeypatch.setenv("ADAFRUIT_AIO_KEY", "settings_current_aio_key") + monkeypatch.setenv("ADAFRUIT_AIO_USERNAME", "settings_current_aio_username") + monkeypatch.setenv("CIRCUITPY_WIFI_PASSWORD", "settings_current_password") + monkeypatch.setenv("CIRCUITPY_WIFI_SSID", "settings_current_ssid") + monkeypatch.setenv("timezone", "settings_current_timezone") + + +@pytest.fixture +def settings_toml_old(monkeypatch): + monkeypatch.setenv("AIO_KEY", "settings_old_aio_key") + monkeypatch.setenv("AIO_USERNAME", "settings_old_aio_username") + monkeypatch.setenv("CIRCUITPY_WIFI_PASSWORD", "settings_old_password") + monkeypatch.setenv("CIRCUITPY_WIFI_SSID", "settings_old_ssid") + monkeypatch.setenv("timezone", "settings_old_timezone") + + +def test_get_setting_does_not_exist(): + network = NetworkBase(None) + assert network._get_setting("test") == None + + +@pytest.mark.parametrize( + ("key", "value"), + ( + ("ADAFRUIT_AIO_KEY", "secret_aio_key"), + ("ADAFRUIT_AIO_USERNAME", "secret_aio_username"), + ("AIO_KEY", "secret_aio_key"), + ("AIO_USERNAME", "secret_aio_username"), + ("CIRCUITPY_WIFI_PASSWORD", "secret_password"), + ("CIRCUITPY_WIFI_SSID", "secret_ssid"), + ("timezone", "secret_timezone"), + ("not_found", None), + ), +) +def test_get_setting_in_secrets(secrets, key, value): + network = NetworkBase(None) + with mock.patch("adafruit_portalbase.network.warnings.warn") as mock_warnings: + assert network._get_setting(key) == value + if value: + mock_warnings.assert_called() + + +@pytest.mark.parametrize( + ("key", "value"), + ( + ("ADAFRUIT_AIO_KEY", "settings_current_aio_key"), + ("ADAFRUIT_AIO_USERNAME", "settings_current_aio_username"), + ("CIRCUITPY_WIFI_PASSWORD", "settings_current_password"), + ("CIRCUITPY_WIFI_SSID", "settings_current_ssid"), + ("timezone", "settings_current_timezone"), + ("not_found", None), + ), +) +def test_get_setting_in_settings_current(settings_toml_current, key, value): + network = NetworkBase(None) + with mock.patch("adafruit_portalbase.network.warnings.warn") as mock_warnings: + assert network._get_setting(key) == value + mock_warnings.assert_not_called() + + +@pytest.mark.parametrize( + ("key", "value"), + ( + ("ADAFRUIT_AIO_KEY", "settings_old_aio_key"), + ("ADAFRUIT_AIO_USERNAME", "settings_old_aio_username"), + ("CIRCUITPY_WIFI_PASSWORD", "settings_old_password"), + ("CIRCUITPY_WIFI_SSID", "settings_old_ssid"), + ("timezone", "settings_old_timezone"), + ("not_found", None), + ), +) +def test_get_setting_in_settings_old(settings_toml_old, key, value): + network = NetworkBase(None) + with mock.patch("adafruit_portalbase.network.warnings.warn") as mock_warnings: + assert network._get_setting(key) == value + mock_warnings.assert_not_called() + if key in ["ADAFRUIT_AIO_KEY", "ADAFRUIT_AIO_USERNAME"]: + assert os.getenv(key) is None + + +def test_fallback(secrets, settings_toml_current): + network = NetworkBase(None) + with mock.patch("adafruit_portalbase.network.warnings.warn") as mock_warnings: + assert network._get_setting("ADAFRUIT_AIO_KEY") == "settings_current_aio_key" + mock_warnings.assert_not_called() + with mock.patch("adafruit_portalbase.network.warnings.warn") as mock_warnings: + assert network._get_setting("fallback_test") == "secret_fallback_test" + mock_warnings.assert_called() + + +def test_value_stored(settings_toml_current): + network = NetworkBase(None) + with mock.patch("os.getenv", return_value="test") as mock_getenv: + assert network._get_setting("ADAFRUIT_AIO_KEY") == "test" + mock_getenv.assert_called() + with mock.patch("os.getenv", return_value="test") as mock_getenv: + assert network._get_setting("ADAFRUIT_AIO_KEY") == "test" + mock_getenv.assert_not_called() From 6c353a2d78e8e4951dc3fc2cdedb44a6a56a41cc Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 23 Feb 2025 09:57:30 -0500 Subject: [PATCH 2/3] Copyedit error messages --- adafruit_portalbase/network.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/adafruit_portalbase/network.py b/adafruit_portalbase/network.py index fe63ae6..d0a7a90 100755 --- a/adafruit_portalbase/network.py +++ b/adafruit_portalbase/network.py @@ -154,8 +154,7 @@ def _get_setting(self, setting_name): env_value = secrets.get(secrets_setting_name) if env_value is not None: warnings.warn( - "The using of `secrets`, is deprecated. Please put your settings in " - "settings.toml" + "Using secrets.py for network settings is deprecated. Put your settings in settings.toml." ) self._settings[setting_name] = env_value return env_value @@ -225,8 +224,8 @@ def get_strftime(self, time_format, location=None, max_attempts=10): aio_key = self._get_setting("ADAFRUIT_AIO_KEY") except KeyError: raise KeyError( - "\n\nOur time service requires a login/password to rate-limit. " - "Please register for a free adafruit.io account and place the user/key in " + "\nThe Adafruit IO time service requires a login and password. " + "Rgister for a free adafruit.io account and put the username and key in " "your settings.toml file under 'ADAFRUIT_AIO_USERNAME' and 'ADAFRUIT_AIO_KEY'" ) from KeyError @@ -257,7 +256,7 @@ def get_strftime(self, time_format, location=None, max_attempts=10): reply = response.text except KeyError: raise KeyError( - "Was unable to lookup the time, try setting 'timezone' in your settings.toml" + "Unable to lookup the time, try setting 'timezone' in your settings.toml" "according to http://worldtimeapi.org/timezones" ) from KeyError # now clean up From 57f092fb19e797aedf322de7cf051ae9b17a1b23 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 23 Feb 2025 10:00:05 -0500 Subject: [PATCH 3/3] ruff --- adafruit_portalbase/network.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/adafruit_portalbase/network.py b/adafruit_portalbase/network.py index d0a7a90..d183bf8 100755 --- a/adafruit_portalbase/network.py +++ b/adafruit_portalbase/network.py @@ -154,7 +154,8 @@ def _get_setting(self, setting_name): env_value = secrets.get(secrets_setting_name) if env_value is not None: warnings.warn( - "Using secrets.py for network settings is deprecated. Put your settings in settings.toml." + "Using secrets.py for network settings is deprecated." + " Put your settings in settings.toml." ) self._settings[setting_name] = env_value return env_value