mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-07-09 12:41:05 +02:00
fix: capture Auth0 post-submit diagnostics (#1183)
## ℹ️ Description - Link to the related issue(s): Issue #1120 - Capture diagnostics when Auth0 post-submit verification remains inconclusive, so future failures include a DOM/screenshot bundle plus classified metadata. ## 📋 Changes Summary - Reuse existing login-detection diagnostics for the final Auth0 post-password-submit timeout. - Add sanitized JSON metadata with `event`, `classification`, and `page_url`. - Keep diagnostics best-effort so capture failures do not mask the original classified timeout. - Add focused tests for enabled/disabled capture, privacy sanitization, failure preservation, and diagnostics argument forwarding. - No new configuration, schema, translation, captcha, or identifier-flow changes. ### ⚙️ Type of Change Select the type(s) of change(s) included in this pull request: - [x] 🐞 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (adds new functionality without breaking existing usage) - [ ] 💥 Breaking change (changes that might break existing user setups, scripts, or configurations) ## ✅ Checklist Before requesting a review, confirm the following: - [x] I have reviewed my changes to ensure they meet the project's standards. - [x] I have tested my changes and ensured that all tests pass (`pdm run test tests/unit/test_login_flow.py tests/unit/test_translations.py`). - [x] I have formatted the code (`pdm run format`). - [x] I have verified that linting passes (`pdm run lint`). - [x] I have updated documentation where necessary. (Not needed.) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved login timeout handling with clearer error messages and safer URL reporting. * Added fallback diagnostics during failed post-login verification, while preserving the original timeout if diagnostics capture fails. * Prevented sensitive URL parts from appearing in error reports. * **Tests** * Expanded coverage for login diagnostics forwarding and timeout behavior with diagnostics enabled and disabled. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -182,6 +182,9 @@ async def login(
|
|||||||
username = username,
|
username = username,
|
||||||
password = password,
|
password = password,
|
||||||
captcha_config = captcha_config,
|
captcha_config = captcha_config,
|
||||||
|
diagnostics_config = diagnostics_config,
|
||||||
|
diagnostics_output_dir_fn = diagnostics_output_dir_fn,
|
||||||
|
log_file_path = log_file_path,
|
||||||
)
|
)
|
||||||
await handle_after_login_logic(web)
|
await handle_after_login_logic(web)
|
||||||
except (AssertionError, TimeoutError):
|
except (AssertionError, TimeoutError):
|
||||||
@@ -408,7 +411,14 @@ async def _classify_post_submit_state(web:WebScrapingMixin) -> str:
|
|||||||
return f"UNKNOWN (url={url})"
|
return f"UNKNOWN (url={url})"
|
||||||
|
|
||||||
|
|
||||||
async def wait_for_post_auth0_submit_transition(web:WebScrapingMixin, *, username:str) -> None:
|
async def wait_for_post_auth0_submit_transition(
|
||||||
|
web:WebScrapingMixin,
|
||||||
|
*,
|
||||||
|
username:str,
|
||||||
|
diagnostics_config:DiagnosticsConfig | None = None,
|
||||||
|
diagnostics_output_dir_fn:Callable[[], Path] | None = None,
|
||||||
|
log_file_path:str | None = None,
|
||||||
|
) -> None:
|
||||||
post_submit_timeout = web.timeout("login_detection")
|
post_submit_timeout = web.timeout("login_detection")
|
||||||
quick_dom_timeout = web.timeout("quick_dom")
|
quick_dom_timeout = web.timeout("quick_dom")
|
||||||
fallback_max_ms = max(700, int(quick_dom_timeout * 1_000))
|
fallback_max_ms = max(700, int(quick_dom_timeout * 1_000))
|
||||||
@@ -445,10 +455,30 @@ async def wait_for_post_auth0_submit_transition(web:WebScrapingMixin, *, usernam
|
|||||||
LOG.debug("Final post-submit login confirmation did not complete within %.1fs", quick_dom_timeout)
|
LOG.debug("Final post-submit login confirmation did not complete within %.1fs", quick_dom_timeout)
|
||||||
|
|
||||||
classification = await _classify_post_submit_state(web)
|
classification = await _classify_post_submit_state(web)
|
||||||
|
sanitized_url = _diagnostic_url(web)
|
||||||
LOG.warning("Auth0 post-submit verification remained inconclusive")
|
LOG.warning("Auth0 post-submit verification remained inconclusive")
|
||||||
|
|
||||||
|
# Best-effort diagnostics capture — never mask the original TimeoutError.
|
||||||
|
try:
|
||||||
|
await capture_login_detection_diagnostics_if_enabled(
|
||||||
|
web,
|
||||||
|
base_prefix = "login_detection_auth0_post_submit_inconclusive",
|
||||||
|
pause_banner_message = "# Auth0 post-submit verification remained inconclusive. Browser is paused for manual inspection.",
|
||||||
|
diagnostics_config = diagnostics_config,
|
||||||
|
diagnostics_output_dir_fn = diagnostics_output_dir_fn,
|
||||||
|
log_file_path = log_file_path,
|
||||||
|
json_payload = {
|
||||||
|
"event": "auth0_post_submit_inconclusive",
|
||||||
|
"classification": classification,
|
||||||
|
"page_url": sanitized_url,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
except Exception: # noqa: S110, BLE001
|
||||||
|
pass
|
||||||
|
|
||||||
raise TimeoutError(
|
raise TimeoutError(
|
||||||
_("Auth0 post-submit verification remained inconclusive: %s (url=%s)")
|
_("Auth0 post-submit verification remained inconclusive: %s (url=%s)")
|
||||||
% (classification, _diagnostic_url(web))
|
% (classification, sanitized_url)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -570,6 +600,9 @@ async def fill_login_data_and_send(
|
|||||||
username:str,
|
username:str,
|
||||||
password:str,
|
password:str,
|
||||||
captcha_config:CaptchaConfig,
|
captcha_config:CaptchaConfig,
|
||||||
|
diagnostics_config:DiagnosticsConfig | None = None,
|
||||||
|
diagnostics_output_dir_fn:Callable[[], Path] | None = None,
|
||||||
|
log_file_path:str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Auth0 2-step login via m-einloggen-sso.html (server-side redirect, no JS needed).
|
"""Auth0 2-step login via m-einloggen-sso.html (server-side redirect, no JS needed).
|
||||||
|
|
||||||
@@ -597,7 +630,13 @@ async def fill_login_data_and_send(
|
|||||||
await web.web_input(By.CSS_SELECTOR, "input[type='password']", password)
|
await web.web_input(By.CSS_SELECTOR, "input[type='password']", password)
|
||||||
await captcha_flow.check_and_wait_for_captcha(web, captcha_config, is_login_page = True)
|
await captcha_flow.check_and_wait_for_captcha(web, captcha_config, is_login_page = True)
|
||||||
await _click_auth0_submit(web)
|
await _click_auth0_submit(web)
|
||||||
await wait_for_post_auth0_submit_transition(web, username = username)
|
await wait_for_post_auth0_submit_transition(
|
||||||
|
web,
|
||||||
|
username = username,
|
||||||
|
diagnostics_config = diagnostics_config,
|
||||||
|
diagnostics_output_dir_fn = diagnostics_output_dir_fn,
|
||||||
|
log_file_path = log_file_path,
|
||||||
|
)
|
||||||
LOG.debug("Auth0 login submitted.")
|
LOG.debug("Auth0 login submitted.")
|
||||||
|
|
||||||
|
|
||||||
@@ -696,6 +735,7 @@ async def capture_login_detection_diagnostics_if_enabled(
|
|||||||
diagnostics_config:DiagnosticsConfig | None,
|
diagnostics_config:DiagnosticsConfig | None,
|
||||||
diagnostics_output_dir_fn:Callable[[], Path] | None,
|
diagnostics_output_dir_fn:Callable[[], Path] | None,
|
||||||
log_file_path:str | None,
|
log_file_path:str | None,
|
||||||
|
json_payload:dict[str, str] | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
cfg = diagnostics_config
|
cfg = diagnostics_config
|
||||||
if cfg is None or not getattr(getattr(cfg, "capture_on", None), "login_detection", False):
|
if cfg is None or not getattr(getattr(cfg, "capture_on", None), "login_detection", False):
|
||||||
@@ -720,6 +760,7 @@ async def capture_login_detection_diagnostics_if_enabled(
|
|||||||
output_dir = output_dir,
|
output_dir = output_dir,
|
||||||
base_prefix = base_prefix,
|
base_prefix = base_prefix,
|
||||||
page = page,
|
page = page,
|
||||||
|
json_payload = json_payload,
|
||||||
log_file_path = log_file_path,
|
log_file_path = log_file_path,
|
||||||
copy_log = getattr(cfg, "capture_log_copy", False),
|
copy_log = getattr(cfg, "capture_log_copy", False),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -570,6 +570,40 @@ class TestKleinanzeigenBotAuthentication:
|
|||||||
]
|
]
|
||||||
assert mock_submit.await_count == 2 # identifier submit + password submit
|
assert mock_submit.await_count == 2 # identifier submit + password submit
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_fill_login_data_and_send_forwards_diagnostics_args(self, test_bot:KleinanzeigenBot, tmp_path:Path) -> None:
|
||||||
|
"""Diagnostics config, output_dir_fn, and log_file_path are forwarded to wait_for_post_auth0_submit_transition."""
|
||||||
|
test_bot.config.diagnostics = DiagnosticsConfig.model_validate(
|
||||||
|
{
|
||||||
|
"capture_on": {"login_detection": True},
|
||||||
|
"output_dir": str(tmp_path),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
with (
|
||||||
|
patch("kleinanzeigen_bot.login_flow.wait_for_auth0_login_context", new_callable = AsyncMock),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.handle_identifier_captcha_state", new_callable = AsyncMock),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.wait_for_auth0_password_step", new_callable = AsyncMock),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.wait_for_post_auth0_submit_transition", new_callable = AsyncMock) as wait_transition,
|
||||||
|
patch.object(test_bot, "web_input"),
|
||||||
|
patch("kleinanzeigen_bot.login_flow._click_auth0_submit", new_callable = AsyncMock),
|
||||||
|
patch("kleinanzeigen_bot.captcha_flow.check_and_wait_for_captcha", new_callable = AsyncMock),
|
||||||
|
):
|
||||||
|
await fill_login_data_and_send(
|
||||||
|
test_bot,
|
||||||
|
username = test_bot.config.login.username,
|
||||||
|
password = test_bot.config.login.password,
|
||||||
|
captcha_config = test_bot.config.captcha,
|
||||||
|
diagnostics_config = test_bot.config.diagnostics,
|
||||||
|
diagnostics_output_dir_fn = test_bot._diagnostics_output_dir,
|
||||||
|
log_file_path = test_bot.log_file_path,
|
||||||
|
)
|
||||||
|
|
||||||
|
wait_transition.assert_awaited_once()
|
||||||
|
assert wait_transition.await_args is not None
|
||||||
|
assert wait_transition.await_args.kwargs.get("diagnostics_config") is test_bot.config.diagnostics
|
||||||
|
assert wait_transition.await_args.kwargs.get("diagnostics_output_dir_fn") == test_bot._diagnostics_output_dir
|
||||||
|
assert wait_transition.await_args.kwargs.get("log_file_path") == test_bot.log_file_path
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_fill_login_data_and_send_fails_when_password_step_missing(self, test_bot:KleinanzeigenBot) -> None:
|
async def test_fill_login_data_and_send_fails_when_password_step_missing(self, test_bot:KleinanzeigenBot) -> None:
|
||||||
"""Missing Auth0 password step should fail fast."""
|
"""Missing Auth0 password step should fail fast."""
|
||||||
@@ -1296,3 +1330,159 @@ class TestClassifyPostSubmitState:
|
|||||||
pytest.raises(TimeoutError, match = "Auth0 post-submit verification remained inconclusive"),
|
pytest.raises(TimeoutError, match = "Auth0 post-submit verification remained inconclusive"),
|
||||||
):
|
):
|
||||||
await wait_for_post_auth0_submit_transition(test_bot, username = test_bot.config.login.username)
|
await wait_for_post_auth0_submit_transition(test_bot, username = test_bot.config.login.username)
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_wait_for_post_auth0_submit_transition_capture_enabled(
|
||||||
|
self, test_bot:KleinanzeigenBot, tmp_path:Path,
|
||||||
|
) -> None:
|
||||||
|
"""Enabled capture_on.login_detection triggers diagnostics capture with sanitised JSON payload."""
|
||||||
|
test_bot.config.diagnostics = DiagnosticsConfig.model_validate(
|
||||||
|
{
|
||||||
|
"capture_on": {"login_detection": True},
|
||||||
|
"output_dir": str(tmp_path),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
test_bot._login_detection_diagnostics_captured = False
|
||||||
|
test_bot.page = MagicMock()
|
||||||
|
# Raw URL with userinfo, query params (state, code), and fragment
|
||||||
|
test_bot.page.url = (
|
||||||
|
"https://user:secret@login.kleinanzeigen.de/u/login/password"
|
||||||
|
"?state=abc123&code=secret456&other=keep#frag"
|
||||||
|
)
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = [TimeoutError()]),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.is_logged_in", new_callable = AsyncMock, side_effect = asyncio.TimeoutError),
|
||||||
|
patch.object(test_bot, "web_sleep", new_callable = AsyncMock),
|
||||||
|
patch(
|
||||||
|
"kleinanzeigen_bot.login_flow._classify_post_submit_state",
|
||||||
|
new_callable = AsyncMock,
|
||||||
|
return_value = "STILL_ON_PASSWORD_PAGE + AUTH0_INLINE_ERROR",
|
||||||
|
),
|
||||||
|
patch("kleinanzeigen_bot.utils.diagnostics.capture_diagnostics", new_callable = AsyncMock) as mock_capture,
|
||||||
|
pytest.raises(TimeoutError, match = "STILL_ON_PASSWORD_PAGE"),
|
||||||
|
):
|
||||||
|
await wait_for_post_auth0_submit_transition(
|
||||||
|
test_bot,
|
||||||
|
username = test_bot.config.login.username,
|
||||||
|
diagnostics_config = test_bot.config.diagnostics,
|
||||||
|
diagnostics_output_dir_fn = test_bot._diagnostics_output_dir,
|
||||||
|
log_file_path = test_bot.log_file_path,
|
||||||
|
)
|
||||||
|
|
||||||
|
mock_capture.assert_awaited_once()
|
||||||
|
assert mock_capture.await_args is not None
|
||||||
|
assert mock_capture.await_args.kwargs.get("base_prefix") == "login_detection_auth0_post_submit_inconclusive"
|
||||||
|
payload = mock_capture.await_args.kwargs.get("json_payload")
|
||||||
|
assert isinstance(payload, dict)
|
||||||
|
assert payload["event"] == "auth0_post_submit_inconclusive"
|
||||||
|
assert payload["classification"] == "STILL_ON_PASSWORD_PAGE + AUTH0_INLINE_ERROR"
|
||||||
|
|
||||||
|
# page_url must be sanitised — no userinfo, query/fragment stripped
|
||||||
|
sanitised = str(payload.get("page_url", ""))
|
||||||
|
assert "user:" not in sanitised
|
||||||
|
assert "secret" not in sanitised
|
||||||
|
assert "state=" not in sanitised
|
||||||
|
assert "code=" not in sanitised
|
||||||
|
assert "#frag" not in sanitised
|
||||||
|
# Host and path preserved
|
||||||
|
assert "login.kleinanzeigen.de/u/login/password" in sanitised
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_wait_for_post_auth0_submit_transition_capture_disabled_config_none(
|
||||||
|
self, test_bot:KleinanzeigenBot,
|
||||||
|
) -> None:
|
||||||
|
"""Disabled diagnostics config (None) skips capture and raises original TimeoutError."""
|
||||||
|
with (
|
||||||
|
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = [TimeoutError()]),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.is_logged_in", new_callable = AsyncMock, side_effect = asyncio.TimeoutError),
|
||||||
|
patch.object(test_bot, "web_sleep", new_callable = AsyncMock),
|
||||||
|
patch(
|
||||||
|
"kleinanzeigen_bot.login_flow._classify_post_submit_state",
|
||||||
|
new_callable = AsyncMock,
|
||||||
|
return_value = "STILL_ON_PASSWORD_PAGE",
|
||||||
|
),
|
||||||
|
patch("kleinanzeigen_bot.utils.diagnostics.capture_diagnostics", new_callable = AsyncMock) as mock_capture,
|
||||||
|
pytest.raises(TimeoutError, match = "Auth0 post-submit verification remained inconclusive"),
|
||||||
|
):
|
||||||
|
await wait_for_post_auth0_submit_transition(
|
||||||
|
test_bot,
|
||||||
|
username = test_bot.config.login.username,
|
||||||
|
)
|
||||||
|
|
||||||
|
mock_capture.assert_not_awaited()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_wait_for_post_auth0_submit_transition_capture_disabled_login_detection_false(
|
||||||
|
self, test_bot:KleinanzeigenBot, tmp_path:Path,
|
||||||
|
) -> None:
|
||||||
|
"""Disabled capture_on.login_detection skips capture and raises original TimeoutError."""
|
||||||
|
test_bot.config.diagnostics = DiagnosticsConfig.model_validate(
|
||||||
|
{
|
||||||
|
"capture_on": {"login_detection": False},
|
||||||
|
"output_dir": str(tmp_path),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = [TimeoutError()]),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.is_logged_in", new_callable = AsyncMock, side_effect = asyncio.TimeoutError),
|
||||||
|
patch.object(test_bot, "web_sleep", new_callable = AsyncMock),
|
||||||
|
patch(
|
||||||
|
"kleinanzeigen_bot.login_flow._classify_post_submit_state",
|
||||||
|
new_callable = AsyncMock,
|
||||||
|
return_value = "STILL_ON_PASSWORD_PAGE",
|
||||||
|
),
|
||||||
|
patch("kleinanzeigen_bot.utils.diagnostics.capture_diagnostics", new_callable = AsyncMock) as mock_capture,
|
||||||
|
pytest.raises(TimeoutError, match = "Auth0 post-submit verification remained inconclusive"),
|
||||||
|
):
|
||||||
|
await wait_for_post_auth0_submit_transition(
|
||||||
|
test_bot,
|
||||||
|
username = test_bot.config.login.username,
|
||||||
|
diagnostics_config = test_bot.config.diagnostics,
|
||||||
|
)
|
||||||
|
|
||||||
|
mock_capture.assert_not_awaited()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_wait_for_post_auth0_submit_transition_capture_failure_preserves_error(
|
||||||
|
self, test_bot:KleinanzeigenBot, tmp_path:Path,
|
||||||
|
) -> None:
|
||||||
|
"""Capture exception does not mask the original classified TimeoutError.
|
||||||
|
|
||||||
|
Patches ``capture_login_detection_diagnostics_if_enabled`` itself (not
|
||||||
|
the lower-level ``capture_diagnostics``) to prove the local
|
||||||
|
``try/except`` guard in ``wait_for_post_auth0_submit_transition`` works.
|
||||||
|
"""
|
||||||
|
test_bot.config.diagnostics = DiagnosticsConfig.model_validate(
|
||||||
|
{
|
||||||
|
"capture_on": {"login_detection": True},
|
||||||
|
"output_dir": str(tmp_path),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
test_bot._login_detection_diagnostics_captured = False
|
||||||
|
test_bot.page = MagicMock()
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = [TimeoutError()]),
|
||||||
|
patch("kleinanzeigen_bot.login_flow.is_logged_in", new_callable = AsyncMock, side_effect = asyncio.TimeoutError),
|
||||||
|
patch.object(test_bot, "web_sleep", new_callable = AsyncMock),
|
||||||
|
patch(
|
||||||
|
"kleinanzeigen_bot.login_flow._classify_post_submit_state",
|
||||||
|
new_callable = AsyncMock,
|
||||||
|
return_value = "STILL_ON_PASSWORD_PAGE",
|
||||||
|
),
|
||||||
|
patch(
|
||||||
|
"kleinanzeigen_bot.login_flow.capture_login_detection_diagnostics_if_enabled",
|
||||||
|
new_callable = AsyncMock,
|
||||||
|
side_effect = RuntimeError("capture guard boom"),
|
||||||
|
),
|
||||||
|
pytest.raises(TimeoutError, match = "STILL_ON_PASSWORD_PAGE"),
|
||||||
|
):
|
||||||
|
await wait_for_post_auth0_submit_transition(
|
||||||
|
test_bot,
|
||||||
|
username = test_bot.config.login.username,
|
||||||
|
diagnostics_config = test_bot.config.diagnostics,
|
||||||
|
diagnostics_output_dir_fn = test_bot._diagnostics_output_dir,
|
||||||
|
log_file_path = test_bot.log_file_path,
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user