From d80e716822b9d8d773edee8d5dc71e1a9d3364f0 Mon Sep 17 00:00:00 2001 From: Tom Hubrecht Date: Fri, 24 Feb 2023 16:54:58 +0100 Subject: [PATCH 1/3] Add authentication to the ntfy hook --- borgmatic/config/schema.yaml | 10 ++++++++++ borgmatic/hooks/ntfy.py | 11 ++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 02bd5d6a3..d7acc1c1d 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -1029,6 +1029,16 @@ properties: description: | The address of your self-hosted ntfy.sh instance. example: https://ntfy.your-domain.com + username: + type: string + description: | + The username used for authentication. + example: testuser + password: + type: string + description: | + The password used for authentication. + example: fakepassword start: type: object properties: diff --git a/borgmatic/hooks/ntfy.py b/borgmatic/hooks/ntfy.py index c62b51103..fd3b9881b 100644 --- a/borgmatic/hooks/ntfy.py +++ b/borgmatic/hooks/ntfy.py @@ -56,10 +56,19 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ 'X-Tags': state_config.get('tags'), } + username = hook_config.get('username') + password = hook_config.get('password') + + auth = ( + requests.auth.HTTPBasicAuth(username, password) + if (username and password) is not None + else None + ) + if not dry_run: logging.getLogger('urllib3').setLevel(logging.ERROR) try: - response = requests.post(f'{base_url}/{topic}', headers=headers) + response = requests.post(f'{base_url}/{topic}', headers=headers, auth=auth) if not response.ok: response.raise_for_status() except requests.exceptions.RequestException as error: From 9b071ff92f12c72df5f9cd265ccb53731bb03981 Mon Sep 17 00:00:00 2001 From: Tom Hubrecht Date: Sat, 25 Feb 2023 20:03:37 +0100 Subject: [PATCH 2/3] Make the auth logic more explicit and warnings if necessary --- borgmatic/hooks/ntfy.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/borgmatic/hooks/ntfy.py b/borgmatic/hooks/ntfy.py index fd3b9881b..3120500f3 100644 --- a/borgmatic/hooks/ntfy.py +++ b/borgmatic/hooks/ntfy.py @@ -59,11 +59,18 @@ def ping_monitor(hook_config, config_filename, state, monitoring_log_level, dry_ username = hook_config.get('username') password = hook_config.get('password') - auth = ( - requests.auth.HTTPBasicAuth(username, password) - if (username and password) is not None - else None - ) + auth = None + if (username and password) is not None: + auth = requests.auth.HTTPBasicAuth(username, password) + logger.info(f'{config_filename}: Using basic auth with user {username} for Ntfy') + elif username is not None: + logger.warn( + f'{config_filename}: Password missing for Ntfy authentication, defaulting to no auth' + ) + elif password is not None: + logger.warn( + f'{config_filename}: Username missing for Ntfy authentication, defaulting to no auth' + ) if not dry_run: logging.getLogger('urllib3').setLevel(logging.ERROR) From 95575c3450db99659e9312fb22c1fe081605cde6 Mon Sep 17 00:00:00 2001 From: Tom Hubrecht Date: Sat, 25 Feb 2023 20:03:51 +0100 Subject: [PATCH 3/3] Add auth test for the ntfy hook --- tests/unit/hooks/test_ntfy.py | 52 ++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/unit/hooks/test_ntfy.py b/tests/unit/hooks/test_ntfy.py index 3867cee1d..ea3f3c1da 100644 --- a/tests/unit/hooks/test_ntfy.py +++ b/tests/unit/hooks/test_ntfy.py @@ -38,6 +38,7 @@ def test_ping_monitor_minimal_config_hits_hosted_ntfy_on_fail(): flexmock(module.requests).should_receive('post').with_args( f'{default_base_url}/{topic}', headers=return_default_message_headers(module.monitor.State.FAIL), + auth=None, ).and_return(flexmock(ok=True)).once() module.ping_monitor( @@ -45,6 +46,51 @@ def test_ping_monitor_minimal_config_hits_hosted_ntfy_on_fail(): ) +def test_ping_monitor_with_auth_hits_hosted_ntfy_on_fail(): + hook_config = { + 'topic': topic, + 'username': 'testuser', + 'password': 'fakepassword', + } + flexmock(module.requests).should_receive('post').with_args( + f'{default_base_url}/{topic}', + headers=return_default_message_headers(module.monitor.State.FAIL), + auth=module.requests.auth.HTTPBasicAuth('testuser', 'fakepassword'), + ).and_return(flexmock(ok=True)).once() + + module.ping_monitor( + hook_config, 'config.yaml', module.monitor.State.FAIL, monitoring_log_level=1, dry_run=False + ) + + +def test_ping_monitor_auth_with_no_username_warning(): + hook_config = {'topic': topic, 'password': 'fakepassword'} + flexmock(module.requests).should_receive('post').with_args( + f'{default_base_url}/{topic}', + headers=return_default_message_headers(module.monitor.State.FAIL), + auth=None, + ).and_return(flexmock(ok=True)).once() + flexmock(module.logger).should_receive('warning').once() + + module.ping_monitor( + hook_config, 'config.yaml', module.monitor.State.FAIL, monitoring_log_level=1, dry_run=False + ) + + +def test_ping_monitor_auth_with_no_password_warning(): + hook_config = {'topic': topic, 'username': 'testuser'} + flexmock(module.requests).should_receive('post').with_args( + f'{default_base_url}/{topic}', + headers=return_default_message_headers(module.monitor.State.FAIL), + auth=None, + ).and_return(flexmock(ok=True)).once() + flexmock(module.logger).should_receive('warning').once() + + module.ping_monitor( + hook_config, 'config.yaml', module.monitor.State.FAIL, monitoring_log_level=1, dry_run=False + ) + + def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_start(): hook_config = {'topic': topic} flexmock(module.requests).should_receive('post').never() @@ -76,6 +122,7 @@ def test_ping_monitor_minimal_config_hits_selfhosted_ntfy_on_fail(): flexmock(module.requests).should_receive('post').with_args( f'{custom_base_url}/{topic}', headers=return_default_message_headers(module.monitor.State.FAIL), + auth=None, ).and_return(flexmock(ok=True)).once() module.ping_monitor( @@ -95,7 +142,7 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_fail_dry_run(): def test_ping_monitor_custom_message_hits_hosted_ntfy_on_fail(): hook_config = {'topic': topic, 'fail': custom_message_config} flexmock(module.requests).should_receive('post').with_args( - f'{default_base_url}/{topic}', headers=custom_message_headers, + f'{default_base_url}/{topic}', headers=custom_message_headers, auth=None ).and_return(flexmock(ok=True)).once() module.ping_monitor( @@ -108,6 +155,7 @@ def test_ping_monitor_custom_state_hits_hosted_ntfy_on_start(): flexmock(module.requests).should_receive('post').with_args( f'{default_base_url}/{topic}', headers=return_default_message_headers(module.monitor.State.START), + auth=None, ).and_return(flexmock(ok=True)).once() module.ping_monitor( @@ -124,6 +172,7 @@ def test_ping_monitor_with_connection_error_logs_warning(): flexmock(module.requests).should_receive('post').with_args( f'{default_base_url}/{topic}', headers=return_default_message_headers(module.monitor.State.FAIL), + auth=None, ).and_raise(module.requests.exceptions.ConnectionError) flexmock(module.logger).should_receive('warning').once() @@ -145,6 +194,7 @@ def test_ping_monitor_with_other_error_logs_warning(): flexmock(module.requests).should_receive('post').with_args( f'{default_base_url}/{topic}', headers=return_default_message_headers(module.monitor.State.FAIL), + auth=None, ).and_return(response) flexmock(module.logger).should_receive('warning').once()