From 69812a6794f227b0b2455c148965c379ec0e81f1 Mon Sep 17 00:00:00 2001 From: Trevor Bergeron Date: Sat, 11 Nov 2023 21:15:48 -0500 Subject: [PATCH] squashme: move tests, address comments --- Makefile | 2 +- pyproject.toml | 9 ++ setup.cfg | 4 - sopel/coretasks.py | 26 ++-- test/coretasks/test_coretasks_sasl.py | 207 +++++++++++++++++++++++++ test/test_coretasks.py | 210 -------------------------- 6 files changed, 231 insertions(+), 227 deletions(-) diff --git a/Makefile b/Makefile index 35decee8c..a2c10f3c4 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ lint-style: flake8 lint-type: - mypy --check-untyped-defs sopel + mypy sopel .PHONY: test test_norecord test_novcr vcr_rerecord test: diff --git a/pyproject.toml b/pyproject.toml index aeb62999e..d8cf3e93b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -73,6 +73,15 @@ sopel-plugins = "sopel.cli.plugins:main" [project.entry-points.pytest11] pytest-sopel = "sopel.tests.pytest_plugin" +[tool.mypy] +check_untyped_defs = true +plugins = "sqlalchemy.ext.mypy.plugin" +show_error_codes = true + +[[tool.mypy.overrides]] +module = 'scramp.*' +ignore_missing_imports = true + [tool.pytest.ini_options] # NOTE: sopel/ is included here to include dynamically-generated tests testpaths = ["test", "sopel"] diff --git a/setup.cfg b/setup.cfg index 9d5eb9c20..bf53c04a3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,7 +26,3 @@ exclude = contrib/*, conftest.py no-accept-encodings = True - -[mypy] -plugins = sqlalchemy.ext.mypy.plugin -show_error_codes = True diff --git a/sopel/coretasks.py b/sopel/coretasks.py index dbee7fec0..22f7fe1fd 100644 --- a/sopel/coretasks.py +++ b/sopel/coretasks.py @@ -98,10 +98,8 @@ def _handle_sasl_capability( # Manage CAP REQ :sasl auth_method = bot.settings.core.auth_method server_auth_method = bot.settings.core.server_auth_method - auth_target = bot.settings.core.auth_target is_required = 'sasl' in (auth_method, server_auth_method) - LOGGER.critical("FOO. BAR!") if not is_required: # not required: we are fine, available or not return plugin.CapabilityNegotiation.DONE @@ -112,13 +110,6 @@ def _handle_sasl_capability( 'cannot authenticate with SASL.', ) return plugin.CapabilityNegotiation.ERROR - elif auth_target not in ["PLAIN", "EXTERNAL", "SCRAM-SHA-256"]: - LOGGER.error( - 'Configured SASL method %r is not supported by Sopel.', - auth_target, - ) - return plugin.CapabilityNegotiation.ERROR - # Check SASL configuration (password is required) password, mech = _get_sasl_pass_and_mech(bot) if not password: @@ -130,9 +121,18 @@ def _handle_sasl_capability( cap_info = bot.capabilities.get_capability_info('sasl') cap_params = cap_info.params - available_mechs = cap_params.split(',') if cap_params else [] + server_mechs = cap_params.split(',') if cap_params else [] - if available_mechs and mech not in available_mechs: + sopel_mechs = ["PLAIN", "EXTERNAL", "SCRAM-SHA-256"] + if mech not in sopel_mechs: + raise config.ConfigurationError( + 'SASL mechanism "{mech}" is not supported by Sopel; ' + 'available mechanisms are: {available}.'.format( + mech=mech, + available=', '.join(sopel_mechs), + ) + ) + if server_mechs and mech not in server_mechs: # Raise an error if configured to use an unsupported SASL mechanism, # but only if the server actually advertised supported mechanisms, # i.e. this network supports SASL 3.2 @@ -141,11 +141,13 @@ def _handle_sasl_capability( # by the sasl_mechs() function # See https://github.com/sopel-irc/sopel/issues/1780 for background + + common_mechs = set(sopel_mechs) & set(server_mechs) raise config.ConfigurationError( 'SASL mechanism "{mech}" is not advertised by this server; ' 'available mechanisms are: {available}.'.format( mech=mech, - available=', '.join(available_mechs), + available=', '.join(common_mechs), ) ) diff --git a/test/coretasks/test_coretasks_sasl.py b/test/coretasks/test_coretasks_sasl.py index 39da87b22..19030e35a 100644 --- a/test/coretasks/test_coretasks_sasl.py +++ b/test/coretasks/test_coretasks_sasl.py @@ -1,14 +1,18 @@ """Test behavior of SASL by ``sopel.coretasks``""" from __future__ import annotations +from base64 import b64decode, b64encode +from logging import ERROR from typing import TYPE_CHECKING import pytest +from scramp import ScramMechanism from sopel import coretasks from sopel.tests import rawlist if TYPE_CHECKING: + from sopel.bot import Sopel from sopel.config import Config from sopel.tests.factories import BotFactory, ConfigFactory @@ -65,6 +69,11 @@ def tmpconfig(configfactory: ConfigFactory) -> Config: return configfactory('conf.ini', TMP_CONFIG_SASL_DEFAULT) +@pytest.fixture +def mockbot(tmpconfig, botfactory): + return botfactory.preloaded(tmpconfig) + + def test_sasl_plain_token_generation() -> None: """Make sure SASL PLAIN tokens match the expected format.""" assert ( @@ -344,3 +353,201 @@ def test_sasl_nak(botfactory: BotFactory, tmpconfig) -> None: 'CAP END', 'QUIT :Error negotiating capabilities.', ) + + +def test_sasl_bad_method(mockbot: Sopel, caplog: pytest.LogCaptureFixture): + """Verify the bot behaves when configured with an unsupported SASL method.""" + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "SCRAM-MD4" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + assert mockbot.backend.message_sent == rawlist( + "CAP REQ :sasl", + "CAP END", + ) + with caplog.at_level(ERROR): + mockbot.on_message("AUTHENTICATE +") + assert '"SCRAM-MD4" is not supported' in caplog.text + + +def test_sasl_plain_auth(mockbot: Sopel): + """Verify the bot performs SASL PLAIN auth correctly.""" + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "PLAIN" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + assert mockbot.backend.message_sent == rawlist( + "CAP REQ :sasl", + "AUTHENTICATE PLAIN", + ) + mockbot.on_message("AUTHENTICATE +") + assert ( + len(mockbot.backend.message_sent) == 3 + and mockbot.backend.message_sent[-1] + == rawlist("AUTHENTICATE VGVzdEJvdABUZXN0Qm90AHNlY3JldA==")[0] + ) + mockbot.on_message( + "900 TestBot test!test@test TestBot :You are now logged in as TestBot" + ) + mockbot.on_message("903 TestBot :SASL authentication succeeded") + assert ( + len(mockbot.backend.message_sent) == 4 + and mockbot.backend.message_sent[-1] == rawlist("CAP END")[0] + ) + + +def test_sasl_scram_sha_256_auth(mockbot: Sopel): + """Verify the bot performs SASL SCRAM-SHA-256 auth correctly.""" + mech = ScramMechanism() + salt, stored_key, server_key, iter_count = mech.make_auth_info( + "secret", iteration_count=5000 + ) + scram_server = mech.make_server( + lambda x: (salt, stored_key, server_key, iter_count) + ) + + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "SCRAM-SHA-256" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + assert mockbot.backend.message_sent == rawlist( + "CAP REQ :sasl", + "AUTHENTICATE SCRAM-SHA-256", + ) + mockbot.on_message("AUTHENTICATE +") + + scram_server.set_client_first( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message( + "AUTHENTICATE " + + b64encode(scram_server.get_server_first().encode("utf-8")).decode("utf-8") + ) + scram_server.set_client_final( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message( + "AUTHENTICATE " + + b64encode(scram_server.get_server_final().encode("utf-8")).decode("utf-8") + ) + assert ( + len(mockbot.backend.message_sent) == 5 + and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE +")[0] + ) + + mockbot.on_message( + "900 TestBot test!test@test TestBot :You are now logged in as TestBot" + ) + mockbot.on_message("903 TestBot :SASL authentication succeeded") + assert ( + len(mockbot.backend.message_sent) == 6 + and mockbot.backend.message_sent[-1] == rawlist("CAP END")[0] + ) + + +def test_sasl_scram_sha_256_nonsense_server_first(mockbot: Sopel): + """Verify the bot handles a nonsense SCRAM-SHA-256 server_first correctly.""" + mech = ScramMechanism() + salt, stored_key, server_key, iter_count = mech.make_auth_info( + "secret", iteration_count=5000 + ) + scram_server = mech.make_server( + lambda x: (salt, stored_key, server_key, iter_count) + ) + + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "SCRAM-SHA-256" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + mockbot.on_message("AUTHENTICATE +") + + scram_server.set_client_first( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message("AUTHENTICATE " + b64encode(b"junk").decode("utf-8")) + assert ( + len(mockbot.backend.message_sent) == 4 + and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] + ) + + +def test_sasl_scram_sha_256_nonsense_server_final(mockbot: Sopel): + """Verify the bot handles a nonsense SCRAM-SHA-256 server_final correctly.""" + mech = ScramMechanism() + salt, stored_key, server_key, iter_count = mech.make_auth_info( + "secret", iteration_count=5000 + ) + scram_server = mech.make_server( + lambda x: (salt, stored_key, server_key, iter_count) + ) + + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "SCRAM-SHA-256" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + mockbot.on_message("AUTHENTICATE +") + + scram_server.set_client_first( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message( + "AUTHENTICATE " + + b64encode(scram_server.get_server_first().encode("utf-8")).decode("utf-8") + ) + scram_server.set_client_final( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message("AUTHENTICATE " + b64encode(b"junk").decode("utf-8")) + assert ( + len(mockbot.backend.message_sent) == 5 + and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] + ) + + +def test_sasl_scram_sha_256_error_server_first(mockbot: Sopel): + """Verify the bot handles an error SCRAM-SHA-256 server_first correctly.""" + + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "SCRAM-SHA-256" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + mockbot.on_message("AUTHENTICATE +") + + mockbot.on_message("AUTHENTICATE " + b64encode(b"e=some-error").decode("utf-8")) + assert ( + len(mockbot.backend.message_sent) == 4 + and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] + ) + + +def test_sasl_scram_sha_256_error_server_final(mockbot: Sopel): + """Verify the bot handles an error SCRAM-SHA-256 server_final correctly.""" + mech = ScramMechanism() + salt, stored_key, server_key, iter_count = mech.make_auth_info( + "secret", iteration_count=5000 + ) + scram_server = mech.make_server( + lambda x: (salt, stored_key, server_key, iter_count) + ) + + mockbot.settings.core.auth_method = "sasl" + mockbot.settings.core.auth_target = "SCRAM-SHA-256" + mockbot.on_message("CAP * LS :sasl") + mockbot.on_message("CAP TestBot ACK :sasl") + mockbot.on_message("AUTHENTICATE +") + + scram_server.set_client_first( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message( + "AUTHENTICATE " + + b64encode(scram_server.get_server_first().encode("utf-8")).decode("utf-8") + ) + scram_server.set_client_final( + b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") + ) + mockbot.on_message("AUTHENTICATE " + b64encode(b"e=some-error").decode("utf-8")) + assert ( + len(mockbot.backend.message_sent) == 5 + and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] + ) diff --git a/test/test_coretasks.py b/test/test_coretasks.py index 8d0efae40..0680309fb 100644 --- a/test/test_coretasks.py +++ b/test/test_coretasks.py @@ -1,13 +1,10 @@ """coretasks.py tests""" from __future__ import annotations -from base64 import b64decode, b64encode from datetime import datetime, timezone import logging -from typing import TYPE_CHECKING import pytest -from scramp import ScramMechanism from sopel import coretasks from sopel.irc import isupport @@ -15,9 +12,6 @@ from sopel.tests import rawlist from sopel.tools import Identifier -if TYPE_CHECKING: - from sopel.bot import Sopel - TMP_CONFIG = """ [core] @@ -509,210 +503,6 @@ def test_handle_rpl_myinfo(mockbot): assert mockbot.myinfo.version == 'example-1.2.3' -def test_sasl_plain_token_generation(): - """Make sure SASL PLAIN tokens match the expected format.""" - assert ( - coretasks._make_sasl_plain_token('sopel', 'sasliscool') == - 'sopel\x00sopel\x00sasliscool') - - -def test_sasl_bad_method(mockbot: Sopel): - """Verify the bot behaves when configured with an unsupported SASL method.""" - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "SCRAM-MD4" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - assert mockbot.backend.message_sent == rawlist( - "CAP REQ :sasl", - "CAP END", - ) - #with pytest.raises(Exception): - mockbot.on_message("AUTHENTICATE +") - - -def test_sasl_plain_auth(mockbot: Sopel): - """Verify the bot performs SASL PLAIN auth correctly.""" - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "PLAIN" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - assert mockbot.backend.message_sent == rawlist( - "CAP REQ :sasl", - "AUTHENTICATE PLAIN", - ) - mockbot.on_message("AUTHENTICATE +") - assert ( - len(mockbot.backend.message_sent) == 3 - and mockbot.backend.message_sent[-1] - == rawlist("AUTHENTICATE VGVzdEJvdABUZXN0Qm90AGh1bnRlcjI=")[0] - ) - mockbot.on_message( - "900 TestBot test!test@test TestBot :You are now logged in as TestBot" - ) - mockbot.on_message("903 TestBot :SASL authentication succeeded") - assert ( - len(mockbot.backend.message_sent) == 4 - and mockbot.backend.message_sent[-1] == rawlist("CAP END")[0] - ) - - -def test_sasl_scram_sha_256_auth(mockbot: Sopel): - """Verify the bot performs SASL SCRAM-SHA-256 auth correctly.""" - mech = ScramMechanism() - salt, stored_key, server_key, iter_count = mech.make_auth_info( - "hunter2", iteration_count=5000 - ) - scram_server = mech.make_server( - lambda x: (salt, stored_key, server_key, iter_count) - ) - - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "SCRAM-SHA-256" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - assert mockbot.backend.message_sent == rawlist( - "CAP REQ :sasl", - "AUTHENTICATE SCRAM-SHA-256", - ) - mockbot.on_message("AUTHENTICATE +") - - scram_server.set_client_first( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message( - "AUTHENTICATE " - + b64encode(scram_server.get_server_first().encode("utf-8")).decode("utf-8") - ) - scram_server.set_client_final( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message( - "AUTHENTICATE " - + b64encode(scram_server.get_server_final().encode("utf-8")).decode("utf-8") - ) - assert ( - len(mockbot.backend.message_sent) == 5 - and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE +")[0] - ) - - mockbot.on_message( - "900 TestBot test!test@test TestBot :You are now logged in as TestBot" - ) - mockbot.on_message("903 TestBot :SASL authentication succeeded") - assert ( - len(mockbot.backend.message_sent) == 6 - and mockbot.backend.message_sent[-1] == rawlist("CAP END")[0] - ) - - -def test_sasl_scram_sha_256_nonsense_server_first(mockbot: Sopel): - """Verify the bot handles a nonsense SCRAM-SHA-256 server_first correctly.""" - mech = ScramMechanism() - salt, stored_key, server_key, iter_count = mech.make_auth_info( - "hunter2", iteration_count=5000 - ) - scram_server = mech.make_server( - lambda x: (salt, stored_key, server_key, iter_count) - ) - - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "SCRAM-SHA-256" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - mockbot.on_message("AUTHENTICATE +") - - scram_server.set_client_first( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message("AUTHENTICATE " + b64encode(b"junk").decode("utf-8")) - assert ( - len(mockbot.backend.message_sent) == 4 - and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] - ) - - -def test_sasl_scram_sha_256_nonsense_server_final(mockbot: Sopel): - """Verify the bot handles a nonsense SCRAM-SHA-256 server_final correctly.""" - mech = ScramMechanism() - salt, stored_key, server_key, iter_count = mech.make_auth_info( - "hunter2", iteration_count=5000 - ) - scram_server = mech.make_server( - lambda x: (salt, stored_key, server_key, iter_count) - ) - - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "SCRAM-SHA-256" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - mockbot.on_message("AUTHENTICATE +") - - scram_server.set_client_first( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message( - "AUTHENTICATE " - + b64encode(scram_server.get_server_first().encode("utf-8")).decode("utf-8") - ) - scram_server.set_client_final( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message("AUTHENTICATE " + b64encode(b"junk").decode("utf-8")) - assert ( - len(mockbot.backend.message_sent) == 5 - and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] - ) - - -def test_sasl_scram_sha_256_error_server_first(mockbot: Sopel): - """Verify the bot handles an error SCRAM-SHA-256 server_first correctly.""" - - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "SCRAM-SHA-256" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - mockbot.on_message("AUTHENTICATE +") - - mockbot.on_message("AUTHENTICATE " + b64encode(b"e=some-error").decode("utf-8")) - assert ( - len(mockbot.backend.message_sent) == 4 - and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] - ) - - -def test_sasl_scram_sha_256_error_server_final(mockbot: Sopel): - """Verify the bot handles an error SCRAM-SHA-256 server_final correctly.""" - mech = ScramMechanism() - salt, stored_key, server_key, iter_count = mech.make_auth_info( - "hunter2", iteration_count=5000 - ) - scram_server = mech.make_server( - lambda x: (salt, stored_key, server_key, iter_count) - ) - - mockbot.settings.core.auth_method = "sasl" - mockbot.settings.core.auth_target = "SCRAM-SHA-256" - mockbot.on_message("CAP * LS :sasl") - mockbot.on_message("CAP TestBot ACK :sasl") - mockbot.on_message("AUTHENTICATE +") - - scram_server.set_client_first( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message( - "AUTHENTICATE " - + b64encode(scram_server.get_server_first().encode("utf-8")).decode("utf-8") - ) - scram_server.set_client_final( - b64decode(mockbot.backend.message_sent[-1].split(b" ")[-1]).decode("utf-8") - ) - mockbot.on_message("AUTHENTICATE " + b64encode(b"e=some-error").decode("utf-8")) - assert ( - len(mockbot.backend.message_sent) == 5 - and mockbot.backend.message_sent[-1] == rawlist("AUTHENTICATE *")[0] - ) - - def test_recv_chghost(mockbot, ircfactory): """Ensure that CHGHOST messages are correctly handled.""" irc = ircfactory(mockbot)