From f7d8294a787a51a04a27d1d2403734c29b2bc7c8 Mon Sep 17 00:00:00 2001 From: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:53:39 +0100 Subject: [PATCH] feat: Add `ZABBIX_URL`, prioritize env over config. (#277) --- CHANGELOG | 10 ++- docs/guide/authentication.md | 70 ++++++++++++----- pyproject.toml | 1 + tests/conftest.py | 25 ++++++- tests/pyzabbix/test_client.py | 132 +++++++++++++++++++++++++++++---- tests/test_auth.py | 35 ++++----- uv.lock | 30 +++++++- zabbix_cli/auth.py | 103 +++++++++++++++---------- zabbix_cli/config/constants.py | 5 +- zabbix_cli/pyzabbix/client.py | 41 ++++------ zabbix_cli/utils/fs.py | 1 - 11 files changed, 332 insertions(+), 121 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ae61d6a5..8da3336d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,7 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - +## [Unreleased] + +### Added + +- Environment variable `ZABBIX_URL` to specify the URL for the Zabbix API. + +### Changed + +- Authentication info from environment variables now take priority over the configuration file. ## [3.4.2](https://github.com/unioslo/zabbix-cli/releases/tag/3.4.2) - 2024-12-16 diff --git a/docs/guide/authentication.md b/docs/guide/authentication.md index 5f4e2cd0..1c83e5e9 100644 --- a/docs/guide/authentication.md +++ b/docs/guide/authentication.md @@ -2,12 +2,12 @@ Zabbix-cli provides several ways to authenticate. They are tried in the following order: -1. [Token - Config file](#config-file) 1. [Token - Environment variables](#environment-variables) +1. [Token - Config file](#config-file) 1. [Token - Auth token file](#auth-token-file) +1. [Password - Environment variables](#environment-variables_1) 1. [Password - Config file](#config-file_1) 1. [Password - Auth file](#auth-file) -1. [Password - Environment variables](#environment-variables_1) 1. [Password - Prompt](#prompt) ## Token @@ -17,6 +17,14 @@ The application supports authenticating with an API or session token. API tokens !!! info "Session vs API token" Semantically, a session token and API token are the same thing from an API authentication perspective. They are both sent as the `auth` parameter in the Zabbix API requests. +### Environment variables + +The API token can be set as an environment variable: + +```bash +export ZABBIX_API_TOKEN="API TOKEN" +``` + ### Config file The token can be set directly in the config file: @@ -26,14 +34,6 @@ The token can be set directly in the config file: auth_token = "API_TOKEN" ``` -### Environment variables - -The API token can be set as an environment variable: - -```bash -export ZABBIX_API_TOKEN="API TOKEN" -``` - ### Auth token file The application can store and reuse session tokens between runs. This feature is enabled by default and configurable via the following options: @@ -62,6 +62,15 @@ When `allow_insecure_auth_file` is set to `false`, the application will attempt The application supports authenticating with a username and password. The password can be set in the config file, an auth file, as environment variables, or prompted for when starting the application. +### Environment variables + +The username and password can be set as environment variables: + +```bash +export ZABBIX_USERNAME="Admin" +export ZABBIX_PASSWORD="zabbix" +``` + ### Config file The password can be set directly in the config file: @@ -87,20 +96,45 @@ The location of the auth file file can be changed in the config file: auth_file = "~/.zabbix-cli_auth" ``` -### Environment variables +### Prompt -The username and password can be set as environment variables: +When all other authentication methods fail, the application will prompt for a username and password. The default username in the prompt can be configured: -```bash -export ZABBIX_USERNAME="Admin" -export ZABBIX_PASSWORD="zabbix" +```toml +[api] +username = "Admin" ``` -### Prompt +## URL -When all other authentication methods fail, the application will prompt for a username and password. The default username in the prompt can be configured: +The URL of the Zabbix API can be set in the config file, as an environment variable, or prompted for when starting the application. + +They are processed in the following order: + +1. [Environment variables](#environment-variables_2) +1. [Config file](#config-file_2) +1. [Prompt](#prompt_1) + +The URL should not include `/api_jsonrpc.php`. + +### Config file + +The URL of the Zabbix API can be set in the config file: ```toml + [api] -username = "Admin" +url = "http://zabbix.example.com" +``` + +### Environment variables + +The URL can also be set as an environment variable: + +```bash +export ZABBIX_URL="http://zabbix.example.com" ``` + +### Prompt + +When all other methods fail, the application will prompt for the URL of the Zabbix API. diff --git a/pyproject.toml b/pyproject.toml index d0fadbb3..1943edae 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,6 +61,7 @@ test = [ "pytest-cov>=5.0.0", "inline-snapshot>=0.13.0", "freezegun>=1.5.1", + "pytest-httpserver>=1.1.0", ] docs = [ "mkdocs>=1.5.3", diff --git a/tests/conftest.py b/tests/conftest.py index 8621e863..fce7ade3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,7 @@ import pytest import typer from packaging.version import Version +from pytest_httpserver import HTTPServer from typer.testing import CliRunner from zabbix_cli.app import StatefulApp from zabbix_cli.config.model import Config @@ -98,14 +99,30 @@ def config(tmp_path: Path) -> Iterator[Config]: @pytest.fixture(name="zabbix_client") -def zabbix_client(monkeypatch: pytest.MonkeyPatch) -> Iterator[ZabbixAPI]: +def zabbix_client() -> Iterator[ZabbixAPI]: config = Config.sample_config() client = ZabbixAPI.from_config(config) + yield client - # Patch the version check - monkeypatch.setattr(client, "api_version", lambda: Version("7.0.0")) - yield client +@pytest.fixture(name="zabbix_client_mock_version") +def zabbix_client_mock_version( + zabbix_client: ZabbixAPI, monkeypatch: pytest.MonkeyPatch +) -> Iterator[ZabbixAPI]: + monkeypatch.setattr(zabbix_client, "api_version", lambda: Version("7.0.0")) + yield zabbix_client + + +def add_httpserver_version_endpoint( + httpserver: HTTPServer, version: Version, id: int = 0 +) -> None: + """Add an endpoint emulating the Zabbix apiiinfo.version method.""" + httpserver.expect_oneshot_request( + "/api_jsonrpc.php", + json={"jsonrpc": "2.0", "method": "apiinfo.version", "params": {}, "id": id}, + method="POST", + headers={"Content-Type": "application/json-rpc"}, + ).respond_with_json({"jsonrpc": "2.0", "result": str(version), "id": id}) @pytest.fixture(name="force_color") diff --git a/tests/pyzabbix/test_client.py b/tests/pyzabbix/test_client.py index 88de90a9..12bd6825 100644 --- a/tests/pyzabbix/test_client.py +++ b/tests/pyzabbix/test_client.py @@ -1,15 +1,20 @@ from __future__ import annotations from typing import Any +from typing import Literal import pytest from inline_snapshot import snapshot +from packaging.version import Version +from pytest_httpserver import HTTPServer from zabbix_cli.exceptions import ZabbixAPILoginError from zabbix_cli.exceptions import ZabbixAPILogoutError from zabbix_cli.pyzabbix.client import ZabbixAPI from zabbix_cli.pyzabbix.client import add_param from zabbix_cli.pyzabbix.client import append_param +from tests.conftest import add_httpserver_version_endpoint + @pytest.mark.parametrize( "inp, key, value, expect", @@ -75,14 +80,13 @@ def test_add_param(inp: Any, subkey: str, value: Any, expect: dict[str, Any]) -> # Check in-place modification -def test_login_fails(zabbix_client: ZabbixAPI) -> None: - zabbix_client.set_url("http://some-url-that-will-fail.gg") - assert zabbix_client.url == snapshot( - "http://some-url-that-will-fail.gg/api_jsonrpc.php" - ) +def test_login_fails(zabbix_client_mock_version: ZabbixAPI) -> None: + client = zabbix_client_mock_version + client.set_url("http://some-url-that-will-fail.gg") + assert client.url == snapshot("http://some-url-that-will-fail.gg/api_jsonrpc.php") with pytest.raises(ZabbixAPILoginError) as exc_info: - zabbix_client.login(user="username", password="password") + client.login(user="username", password="password") assert exc_info.exconly() == snapshot( "zabbix_cli.exceptions.ZabbixAPILoginError: Failed to log in to Zabbix: Failed to send request to http://some-url-that-will-fail.gg/api_jsonrpc.php (user.login) with params {'username': 'username', 'password': 'password'}" @@ -92,19 +96,121 @@ def test_login_fails(zabbix_client: ZabbixAPI) -> None: ) -def test_logout_fails(zabbix_client: ZabbixAPI) -> None: +def test_logout_fails(zabbix_client_mock_version: ZabbixAPI) -> None: """Test that we get the correct exception type when login fails due to a connection error.""" - zabbix_client.set_url("http://some-url-that-will-fail.gg") - assert zabbix_client.url == snapshot( - "http://some-url-that-will-fail.gg/api_jsonrpc.php" - ) + client = zabbix_client_mock_version + client.set_url("http://some-url-that-will-fail.gg") + assert client.url == snapshot("http://some-url-that-will-fail.gg/api_jsonrpc.php") - zabbix_client.auth = "authtoken123456789" + client.auth = "authtoken123456789" with pytest.raises(ZabbixAPILogoutError) as exc_info: - zabbix_client.logout() + client.logout() assert exc_info.exconly() == snapshot( "zabbix_cli.exceptions.ZabbixAPILogoutError: Failed to log out of Zabbix: Failed to send request to http://some-url-that-will-fail.gg/api_jsonrpc.php (user.logout) with params {}" ) + + +@pytest.mark.parametrize( + "inp, expect", + [ + pytest.param( + "http://localhost", + "http://localhost/api_jsonrpc.php", + id="localhost-no-slash", + ), + pytest.param( + "http://localhost/", + "http://localhost/api_jsonrpc.php", + id="localhost-with-slash", + ), + pytest.param( + "http://localhost/api_jsonrpc.php", + "http://localhost/api_jsonrpc.php", + id="localhost-full-url", + ), + pytest.param( + "http://localhost/api_jsonrpc.php/", + "http://localhost/api_jsonrpc.php", + id="localhost-full-url-with-slash", + ), + pytest.param( + "http://example.com", + "http://example.com/api_jsonrpc.php", + id="tld-no-slash", + ), + pytest.param( + "http://example.com/", + "http://example.com/api_jsonrpc.php", + id="tld-with-slash", + ), + pytest.param( + "http://example.com/api_jsonrpc.php", + "http://example.com/api_jsonrpc.php", + id="tld-full-url", + ), + pytest.param( + "http://example.com/api_jsonrpc.php/", + "http://example.com/api_jsonrpc.php", + id="tld-full-url-with-slash", + ), + ], +) +def test_client_server_url(inp: str, expect: str) -> None: + zabbix_client = ZabbixAPI(server=inp) + assert zabbix_client.url == expect + + +AuthMethod = Literal["header", "body"] + + +@pytest.mark.parametrize( + "version,expect_method", + [ + pytest.param(Version("5.0.0"), "body", id="5.0.0"), + pytest.param(Version("5.2.0"), "body", id="5.2.0"), + pytest.param(Version("6.0.0"), "body", id="6.0.0"), + pytest.param(Version("6.2.0"), "body", id="6.2.0"), + pytest.param(Version("6.4.0"), "header", id="6.4.0"), + pytest.param(Version("7.0.0"), "header", id="7.0.0"), + pytest.param(Version("7.2.0"), "header", id="7.2.0"), + ], +) +def test_client_auth_method( + zabbix_client: ZabbixAPI, + httpserver: HTTPServer, + version: Version, + expect_method: AuthMethod, +) -> None: + # Add endpoint for version check + zabbix_client.set_url(httpserver.url_for("/api_jsonrpc.php")) + + # Set a mock token we can use for testing + zabbix_client.auth = "token123" + + # Add endpoint that returns the parametrized version + add_httpserver_version_endpoint(httpserver, version, id=0) + + assert zabbix_client.version == version + + data = {"jsonrpc": "2.0", "method": "fake.method", "params": {}, "id": 1} + headers = {} + + # We expect auth token to be in header on >= 6.4.0 + if expect_method == "header": + headers["Authorization"] = f"Bearer {zabbix_client.auth}" + else: + data["auth"] = zabbix_client.auth + + httpserver.expect_oneshot_request( + "/api_jsonrpc.php", + json=data, + headers=headers, + method="POST", + ).respond_with_json({"jsonrpc": "2.0", "result": "authtoken123456789", "id": 1}) + + # Will fail if the auth method is not set correctly + resp = zabbix_client.do_request("fake.method") + assert resp.result == "authtoken123456789" diff --git a/tests/test_auth.py b/tests/test_auth.py index 659dbc57..2f7a4c54 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -2,6 +2,7 @@ from pathlib import Path from typing import TYPE_CHECKING +from typing import Any from typing import Optional import pytest @@ -102,8 +103,8 @@ def _auth_file(tmp_path: Path) -> Path: [ pytest.param( [ - (CredentialsType.AUTH_TOKEN, CredentialsSource.CONFIG), (CredentialsType.AUTH_TOKEN, CredentialsSource.ENV), + (CredentialsType.AUTH_TOKEN, CredentialsSource.CONFIG), (CredentialsType.AUTH_TOKEN, CredentialsSource.FILE), (CredentialsType.PASSWORD, CredentialsSource.CONFIG), (CredentialsType.PASSWORD, CredentialsSource.FILE), @@ -111,12 +112,12 @@ def _auth_file(tmp_path: Path) -> Path: (CredentialsType.PASSWORD, CredentialsSource.PROMPT), ], CredentialsType.AUTH_TOKEN, - CredentialsSource.CONFIG, - id="expect_auth_token_config", + CredentialsSource.ENV, + id="expect_auth_token_env", ), pytest.param( [ - (CredentialsType.AUTH_TOKEN, CredentialsSource.ENV), + (CredentialsType.AUTH_TOKEN, CredentialsSource.CONFIG), (CredentialsType.AUTH_TOKEN, CredentialsSource.FILE), (CredentialsType.PASSWORD, CredentialsSource.CONFIG), (CredentialsType.PASSWORD, CredentialsSource.FILE), @@ -124,8 +125,8 @@ def _auth_file(tmp_path: Path) -> Path: (CredentialsType.PASSWORD, CredentialsSource.PROMPT), ], CredentialsType.AUTH_TOKEN, - CredentialsSource.ENV, - id="expect_auth_token_env", + CredentialsSource.CONFIG, + id="expect_auth_token_config", ), pytest.param( [ @@ -141,33 +142,33 @@ def _auth_file(tmp_path: Path) -> Path: ), pytest.param( [ + (CredentialsType.PASSWORD, CredentialsSource.ENV), (CredentialsType.PASSWORD, CredentialsSource.CONFIG), (CredentialsType.PASSWORD, CredentialsSource.FILE), - (CredentialsType.PASSWORD, CredentialsSource.ENV), (CredentialsType.PASSWORD, CredentialsSource.PROMPT), ], CredentialsType.PASSWORD, - CredentialsSource.CONFIG, - id="expect_password_config", + CredentialsSource.ENV, + id="expect_password_env", ), pytest.param( [ + (CredentialsType.PASSWORD, CredentialsSource.CONFIG), (CredentialsType.PASSWORD, CredentialsSource.FILE), - (CredentialsType.PASSWORD, CredentialsSource.ENV), (CredentialsType.PASSWORD, CredentialsSource.PROMPT), ], CredentialsType.PASSWORD, - CredentialsSource.FILE, - id="expect_password_file", + CredentialsSource.CONFIG, + id="expect_password_config", ), pytest.param( [ - (CredentialsType.PASSWORD, CredentialsSource.ENV), + (CredentialsType.PASSWORD, CredentialsSource.FILE), (CredentialsType.PASSWORD, CredentialsSource.PROMPT), ], CredentialsType.PASSWORD, - CredentialsSource.ENV, - id="expect_password_env", + CredentialsSource.FILE, + id="expect_password_file", ), pytest.param( [ @@ -201,7 +202,7 @@ def test_authenticator_login_with_any( # States reasons for mocking each method # REASON: Makes HTTP calls to the Zabbix API - def mock_login(self: ZabbixAPI, *args, **kwargs): + def mock_login(self: ZabbixAPI, *args: Any, **kwargs: Any) -> str: self.auth = MOCK_TOKEN return self.auth @@ -262,7 +263,7 @@ def mock_get_auth_file_paths(config: Optional[Config] = None) -> list[Path]: auth_file.write_text(f"{MOCK_USER}::{MOCK_PASSWORD}") config.app.auth_file = auth_file - client, info = authenticator.login_with_any() + _, info = authenticator.login_with_any() assert info.credentials.source == expect_source assert info.credentials.type == expect_type assert info.token == MOCK_TOKEN diff --git a/uv.lock b/uv.lock index 29781842..60c8e5f8 100644 --- a/uv.lock +++ b/uv.lock @@ -1096,6 +1096,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/78/3a/af5b4fa5961d9a1e6237b530eb87dd04aea6eb83da09d2a4073d81b54ccf/pytest_cov-5.0.0-py3-none-any.whl", hash = "sha256:4f0764a1219df53214206bf1feea4633c3b558a2925c8b59f144f682861ce652", size = 21990 }, ] +[[package]] +name = "pytest-httpserver" +version = "1.1.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "werkzeug" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/12/53/aa5aa8518246848251243a21440f167b812bd40896546061c1fda814c10c/pytest_httpserver-1.1.0.tar.gz", hash = "sha256:6b1cb0199e2ed551b1b94d43f096863bbf6ae5bcd7c75c2c06845e5ce2dc8701", size = 67210 } +wheels = [ + { url = "https://files.pythonhosted.org/packages/1d/29/c5dfce47d5f575e46e2d4a4257cd43cc199a8014f506d14d808e03d6f8cd/pytest_httpserver-1.1.0-py3-none-any.whl", hash = "sha256:7ef88be8ed3354b6784daa3daa75a422370327c634053cefb124903fa8d73a41", size = 20671 }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" @@ -1531,9 +1543,21 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/fd/84/fd2ba7aafacbad3c4201d395674fc6348826569da3c0937e75505ead3528/wcwidth-0.2.13-py2.py3-none-any.whl", hash = "sha256:3da69048e4540d84af32131829ff948f1e022c1c6bdb8d6102117aac784f6859", size = 34166 }, ] +[[package]] +name = "werkzeug" +version = "3.1.3" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "markupsafe" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/9f/69/83029f1f6300c5fb2471d621ab06f6ec6b3324685a2ce0f9777fd4a8b71e/werkzeug-3.1.3.tar.gz", hash = "sha256:60723ce945c19328679790e3282cc758aa4a6040e4bb330f53d30fa546d44746", size = 806925 } +wheels = [ + { url = "https://files.pythonhosted.org/packages/52/24/ab44c871b0f07f491e5d2ad12c9bd7358e527510618cb1b803a88e986db1/werkzeug-3.1.3-py3-none-any.whl", hash = "sha256:54b78bf3716d19a65be4fceccc0d1d7b89e608834989dfae50ea87564639213e", size = 224498 }, +] + [[package]] name = "zabbix-cli-uio" -version = "3.4.1" +version = "3.4.2" source = { editable = "." } dependencies = [ { name = "httpx", extra = ["socks"] }, @@ -1574,6 +1598,7 @@ dev = [ { name = "pyright" }, { name = "pytest" }, { name = "pytest-cov" }, + { name = "pytest-httpserver" }, { name = "pyyaml" }, { name = "ruff" }, { name = "sanitize-filename" }, @@ -1604,6 +1629,7 @@ test = [ { name = "inline-snapshot" }, { name = "pytest" }, { name = "pytest-cov" }, + { name = "pytest-httpserver" }, ] [package.metadata] @@ -1645,6 +1671,7 @@ dev = [ { name = "pyright" }, { name = "pytest", specifier = ">=8.0.0" }, { name = "pytest-cov", specifier = ">=5.0.0" }, + { name = "pytest-httpserver", specifier = ">=1.1.0" }, { name = "pyyaml" }, { name = "ruff" }, { name = "sanitize-filename" }, @@ -1676,6 +1703,7 @@ test = [ { name = "inline-snapshot", specifier = ">=0.13.0" }, { name = "pytest", specifier = ">=8.0.0" }, { name = "pytest-cov", specifier = ">=5.0.0" }, + { name = "pytest-httpserver", specifier = ">=1.1.0" }, ] [[package]] diff --git a/zabbix_cli/auth.py b/zabbix_cli/auth.py index b519071f..0eb3733b 100644 --- a/zabbix_cli/auth.py +++ b/zabbix_cli/auth.py @@ -48,9 +48,6 @@ logger = logging.getLogger(__name__) -# Auth file location - - SECURE_PERMISSIONS: Final[int] = 0o600 SECURE_PERMISSIONS_STR = format(SECURE_PERMISSIONS, "o") @@ -80,6 +77,13 @@ class Credentials(NamedTuple): password: Optional[str] = None auth_token: Optional[str] = None + def __str__(self) -> str: + if self.type == CredentialsType.PASSWORD: + return f"{self.type} from {self.source}" + if self.type == CredentialsType.AUTH_TOKEN: + return f"{self.type} from {self.source}" + return "no credentials" + @property def type(self) -> Optional[CredentialsType]: if self.auth_token: @@ -100,7 +104,14 @@ class LoginInfo(NamedTuple): class Authenticator: """Encapsulates logic for authenticating with the Zabbix API - using various methods, as well as storing and loading auth tokens.""" + using various methods, as well as storing and loading auth tokens. + + + Exposes methods for logging in with specific credentials, as well as + a method for logging in with any valid credentials. + + Bootstraps application state with login and API info on successful login. + """ config: Config @@ -121,12 +132,12 @@ def login_with_any(self) -> tuple[ZabbixAPI, LoginInfo]: If multiple methods are available, they are tried in the following order: - 1. API token in config file - 2. API token in environment variables + 1. API token in environment variables + 2. API token in config file 3. API token in file (if `use_auth_token_file=true`) - 4. Username and password in config file - 5. Username and password in auth file - 6. Username and password in environment variables + 4. Username and password in environment variables + 5. Username and password in config file + 6. Username and password in auth file 7. Username and password from prompt """ @@ -146,15 +157,17 @@ def _iter_all_credentials( ) -> Generator[Credentials, None, None]: """Generator that yields credentials from all possible sources. + Does not check if credentials are valid. + Finally yields a prompt for username and password if `prompt_password` is True. """ for func in [ - self._get_auth_token_config, self._get_auth_token_env, + self._get_auth_token_config, self._get_auth_token_file, + self._get_username_password_env, self._get_username_password_config, self._get_username_password_auth_file, - self._get_username_password_env, ]: yield func() if prompt_password: @@ -262,7 +275,10 @@ def _update_application_with_login_info(self, info: LoginInfo) -> None: # Write auth token file if info.credentials.username and self.config.app.use_auth_token_file: write_auth_token_file( - info.credentials.username, info.token, self.config.app.auth_token_file + info.credentials.username, + info.token, + self.config.app.auth_token_file, + self.config.app.allow_insecure_auth_file, ) # Log context @@ -306,13 +322,29 @@ def _update_config(self, credentials: Credentials) -> None: ): self.config.api.auth_token = SecretStr(credentials.auth_token) - def get_zabbix_url(self) -> str: - """Get the URL of the Zabbix server from the config, or prompt for it.""" - if not self.config.api.url: + def get_zabbix_url(self, prompt: bool = True) -> str: + """Get the URL of the Zabbix server from env, config, then finally prompt for it..""" + for source in [self._get_zabbix_url_env, self._get_zabbix_url_config]: + url = source() + if url: + return url + if prompt: with self.screen: - return str_prompt("Zabbix URL (without /api_jsonrpc.php)") + return self._get_zabbix_url_prompt() + raise AuthError("No Zabbix URL found in environment or config.") + + def _get_zabbix_url_env(self) -> str: + """Get the URL of the Zabbix server from the environment.""" + return os.environ.get(ConfigEnvVars.URL, "") + + def _get_zabbix_url_config(self) -> str: + """Get the URL of the Zabbix server from the config file.""" return self.config.api.url + def _get_zabbix_url_prompt(self) -> str: + """Prompt for the URL of the Zabbix server.""" + return str_prompt("Zabbix URL (without /api_jsonrpc.php)", empty_ok=False) + def _get_username_password_env(self) -> Credentials: """Get username and password from environment variables.""" return Credentials( @@ -366,12 +398,14 @@ def _get_username_password_prompt( ) def _get_auth_token_config(self) -> Credentials: + """Get auth token from the config file.""" return Credentials( auth_token=self.config.api.auth_token.get_secret_value(), source=CredentialsSource.CONFIG, ) def _get_auth_token_file(self) -> Credentials: + """Get auth token from a file.""" if not self.config.app.use_auth_token_file: logger.debug("Not configured to use auth token file.") return Credentials() @@ -392,6 +426,7 @@ def _get_auth_token_file(self) -> Credentials: ) def load_auth_token_file(self) -> Union[tuple[Path, str], tuple[None, None]]: + """Attempts to load an auth token file.""" paths = get_auth_token_file_paths(self.config) for path in paths: contents = self._do_load_auth_file(path) @@ -448,13 +483,6 @@ def logout(client: ZabbixAPI, config: Config) -> None: clear_auth_token_file(config) -def prompt_username_password(username: str) -> tuple[str, str]: - """Re-useable prompt for username and password.""" - username = str_prompt("Username", default=username, empty_ok=False) - password = str_prompt("Password", password=True, empty_ok=False) - return username, password - - def _parse_auth_file_contents( contents: Optional[str], ) -> tuple[Optional[str], Optional[str]]: @@ -494,7 +522,10 @@ def get_auth_token_file_paths(config: Optional[Config] = None) -> list[Path]: def write_auth_token_file( - username: str, auth_token: str, file: Path = AUTH_TOKEN_FILE + username: str, + auth_token: str, + file: Path = AUTH_TOKEN_FILE, + allow_insecure: bool = False, ) -> Path: """Write a username/auth token pair to the auth token file.""" if not username or not auth_token: @@ -505,16 +536,14 @@ def write_auth_token_file( ) return file - contents = f"{username}::{auth_token}" - if not file.exists(): - try: - file.touch(mode=SECURE_PERMISSIONS) - except OSError as e: - raise AuthTokenFileError( - f"Unable to create auth token file {file}. " - "Change the location or disable auth token file in config." - ) from e - elif not file_has_secure_permissions(file): + try: + file.write_text(f"{username}::{auth_token}") + logger.info(f"Wrote auth token file {file}") + except OSError as e: + raise AuthTokenFileError(f"Unable to write auth token file {file}: {e}") from e + + # Ensure file has secure permissions if configured + if not allow_insecure and not file_has_secure_permissions(file): try: file.chmod(SECURE_PERMISSIONS) except OSError as e: @@ -522,11 +551,7 @@ def write_auth_token_file( f"Unable to set secure permissions ({SECURE_PERMISSIONS_STR}) on {file} when saving auth token. " "Change permissions manually or delete the file." ) from e - try: - file.write_text(contents) - logger.info(f"Wrote auth token file {file}") - except OSError as e: - raise AuthTokenFileError(f"Unable to write auth token file {file}: {e}") from e + return file diff --git a/zabbix_cli/config/constants.py b/zabbix_cli/config/constants.py index c7b34496..2e1885fb 100644 --- a/zabbix_cli/config/constants.py +++ b/zabbix_cli/config/constants.py @@ -36,9 +36,10 @@ # Environment variable names class ConfigEnvVars: - USERNAME = "ZABBIX_USERNAME" - PASSWORD = "ZABBIX_PASSWORD" API_TOKEN = "ZABBIX_API_TOKEN" + PASSWORD = "ZABBIX_PASSWORD" + URL = "ZABBIX_URL" + USERNAME = "ZABBIX_USERNAME" class OutputFormat(StrEnum): diff --git a/zabbix_cli/pyzabbix/client.py b/zabbix_cli/pyzabbix/client.py index 2700fd29..61985949 100644 --- a/zabbix_cli/pyzabbix/client.py +++ b/zabbix_cli/pyzabbix/client.py @@ -26,13 +26,13 @@ from typing import Union from typing import cast +import httpx from packaging.version import InvalidVersion from packaging.version import Version from pydantic import ValidationError from zabbix_cli.__about__ import APP_NAME from zabbix_cli.__about__ import __version__ -from zabbix_cli.cache import ZabbixCache from zabbix_cli.exceptions import ZabbixAPICallError from zabbix_cli.exceptions import ZabbixAPIException from zabbix_cli.exceptions import ZabbixAPILoginError @@ -86,7 +86,6 @@ from zabbix_cli.utils.utils import get_acknowledge_action_value if TYPE_CHECKING: - import httpx from httpx._types import TimeoutTypes from typing_extensions import TypedDict @@ -248,16 +247,13 @@ def __init__( self.use_api_token = False self.id = 0 - server, _, _ = server.partition(RPC_ENDPOINT) self.url = self._get_url(server) logger.info("JSON-RPC Server Endpoint: %s", self.url) - # Cache - self.cache = ZabbixCache(self) - def _get_url(self, server: str) -> str: """Format a URL for the Zabbix API.""" - return f"{server}/api_jsonrpc.php" + server, _, _ = server.partition(RPC_ENDPOINT) + return f"{server.rstrip('/')}{RPC_ENDPOINT}" def set_url(self, server: str) -> str: """Set a new URL for the client.""" @@ -277,8 +273,6 @@ def from_config(cls, config: Config) -> ZabbixAPI: def _get_client( self, verify_ssl: bool, timeout: Union[float, int, None] = None ) -> httpx.Client: - import httpx - kwargs: HTTPXClientKwargs = {} if timeout is not None: kwargs["timeout"] = timeout @@ -310,24 +304,23 @@ def login( """Convenience method for logging into the API and storing the resulting auth token as an instance variable. """ - # Before we do anything, we try to fetch the API version - # Without an API connection, we cannot determine - # the user parameter name to use when logging in. + # By checking the version, we also check if the API is reachable try: - self.version # property + version = self.version # property except ZabbixAPIRequestError as e: raise ZabbixAPIException( f"Failed to connect to Zabbix API at {self.url}" ) from e - logger.debug("Logging in to Zabbix API at %s", self.url) - self.auth = "" + logger.debug("Logging in to Zabbix %s API at %s", version, self.url) + if auth_token: + # TODO: revert this if token is invalid self.use_api_token = True - self.auth = auth_token + auth = auth_token else: params: ParamsType = { - compat.login_user_name(self.version): user, + compat.login_user_name(version): user, "password": password, } try: @@ -339,9 +332,11 @@ def login( "Unknown error when trying to log in to Zabbix" ) from e else: - self.auth = str(auth) if auth else "" + auth = str(auth) if auth else "" self.use_api_token = False + self.auth = auth + # Check if the auth token we obtained or specified is valid # XXX: should revert auth token to what it was before this method # was called if token is invalid. @@ -371,7 +366,8 @@ def logout(self) -> None: ) except ZabbixAPIRequestError as e: raise ZabbixAPILogoutError("Failed to log out of Zabbix") from e - self.auth = "" + else: + self.auth = "" def confimport(self, format: ExportFormat, source: str, rules: ImportRules) -> Any: """Alias for configuration.import because it clashes with @@ -415,7 +411,7 @@ def do_request( # We don't have to pass the auth token if asking for the apiinfo.version # TODO: ensure we have auth token if method requires it - if self.auth and method != "apiinfo.version": + if self.auth and method not in ["apiinfo.version", "user.login"]: if self.version.release >= (6, 4, 0): request_headers["Authorization"] = f"Bearer {self.auth}" else: @@ -505,11 +501,6 @@ def _check_response_errors( response=response, ) - def populate_cache(self) -> None: - """Populates the various caches with data from the Zabbix API.""" - # NOTE: Must be manually invoked. Can we do this in a thread? - self.cache.populate() - def get_hostgroup( self, name_or_id: str, diff --git a/zabbix_cli/utils/fs.py b/zabbix_cli/utils/fs.py index f10fbbb5..0015c4fe 100644 --- a/zabbix_cli/utils/fs.py +++ b/zabbix_cli/utils/fs.py @@ -92,7 +92,6 @@ def sanitize_filename(filename: str) -> str: return re.sub(r"[^\w\-.]", "_", filename) -# NOTE: Move to zabbix_cli.utils.fs? def make_executable(path: Path) -> None: """Make a file executable.""" if sys.platform == "win32":