Merge commit 'origin/develop~1'

.coveragerc

@@ -0,0 +1,6 @@
+[run]
+branch = True
+parallel = True
+
+source =
+    openconnect_sso

.gitignore

@@ -17,6 +17,7 @@ eggs/
 lib/
 lib64/
 parts/
+pip-wheel-metadata/
 sdist/
 var/
 wheels/

openconnect_sso/__init__.py

@@ -1,2 +1,2 @@
-__version__ = "0.3.3"
+__version__ = "0.3.4"
 __description__ = "Wrapper script for OpenConnect supporting Azure AD (SAMLv2) authentication to Cisco SSL-VPNs"

openconnect_sso/browser/browser.py

@@ -29,17 +29,16 @@ async def spawn(self):
             stdin=asyncio.subprocess.PIPE,
             stdout=asyncio.subprocess.PIPE
         )
+        self.running = True
 
         self.updater = asyncio.ensure_future(self._update_status())
-        self.running = True
 
         def stop(_task):
             self.running = False
 
         asyncio.ensure_future(self.browser_proc.wait()).add_done_callback(stop)
 
     async def _update_status(self):
-        assert self.running
         while self.running:
             logger.debug("Waiting for message from browser process")
 

openconnect_sso/browser/webengine_process.py

@@ -6,7 +6,7 @@
 import pkg_resources
 import structlog
 
-from PyQt5.QtCore import QUrl
+from PyQt5.QtCore import QUrl, QTimer
 from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEngineScript
 from PyQt5.QtWidgets import QApplication
 
@@ -15,16 +15,28 @@
 from openconnect_sso.browser import rpc_types as rpc
 from openconnect_sso.cli import create_argparser
 
+app = None
 logger = structlog.get_logger("webengine")
 
 
 def run_browser_process():
+    # To work around funky GC conflicts with C++ code by ensuring QApplication terminates last
+    global app
     args = create_argparser().parse_known_args()[0]
     configure_logger(logging.getLogger(), args.log_level)
 
     cfg = config.load()
 
     app = QApplication(sys.argv)
+
+    # In order to make Python able to handle signals
+    force_python_execution = QTimer()
+    force_python_execution.start(200)
+
+    def ignore():
+        pass
+
+    force_python_execution.timeout.connect(ignore)
     web = WebBrowser(cfg.auto_fill_rules)
 
     line = sys.stdin.buffer.readline()
@@ -106,7 +118,11 @@ def get_selectors(rules, credentials):
     statements = []
     for i, rule in enumerate(rules):
         selector = json.dumps(rule.selector)
-        if rule.fill:
+        if rule.action == "stop":
+            statements.append(
+                f"""var elem = document.querySelector({selector}); if (elem) {{ return; }}"""
+            )
+        elif rule.fill:
             value = json.dumps(getattr(credentials, rule.fill, None))
             if value:
                 statements.append(
@@ -125,6 +141,12 @@ def get_selectors(rules, credentials):
     return "\n".join(statements)
 
 
+def on_sigterm(signum, frame):
+    logger.info("SIGNAL handler")
+    QApplication.quit()
+
+
 if __name__ == "__main__":
+    signal.signal(signal.SIGTERM, on_sigterm)
     signal.signal(signal.SIGINT, signal.SIG_DFL)
-    run_browser_process()
+    sys.exit(run_browser_process())

openconnect_sso/cli.py

@@ -1,7 +1,10 @@
+#!/usr/bin/env python3
+
 import argparse
 import enum
 import logging
 import os
+import sys
 
 import openconnect_sso
 from openconnect_sso import app, config
@@ -117,4 +120,13 @@ def main():
                 "No Anyconnect profile can be found. One of --profile or --server arguments required."
             )
 
+    if args.use_profile_selector and not args.profile_path:
+        parser.error(
+            "No Anyconnect profile can be found. --profile argument is required."
+        )
+
     return app.run(args)
+
+
+if __name__ == "__main__":
+    sys.exit(main())

openconnect_sso/config.py

@@ -84,6 +84,7 @@ class AutoFillRule(ConfigNode):
 def get_default_auto_fill_rules():
     return {
         "https://*": [
+            AutoFillRule(selector="div[id=passwordError]", action="stop").as_dict(),
             AutoFillRule(selector="input[type=email]", fill="username").as_dict(),
             AutoFillRule(selector="input[type=password]", fill="password").as_dict(),
             AutoFillRule(selector="input[type=submit]", action="click").as_dict(),

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "openconnect-sso"
-version = "0.3.3"
+version = "0.3.4"
 description = "Wrapper script for OpenConnect supporting Azure AD (SAMLv2) authentication to Cisco SSL-VPNs"
 authors = ["László Vaskó <[email protected]>"]
 readme = "README.md"
@@ -31,8 +31,12 @@ structlog = "^19.1"
 toml = "^0.10"
 
 [tool.poetry.dev-dependencies]
+coverage_enable_subprocess = "^1.0"
 pytest = "^3.0"
 black = "=19.3b0"
+pytest-asyncio = "^0.10.0"
+pytest-cov = "^2.7"
+pytest-httpserver = "^0.3.4"
 reno = "^2.11"
 
 [build-system]

releasenotes/notes/browser-stop-if-invalid-creds-6d7031efa2fa691a.yaml

@@ -0,0 +1,15 @@
+---
+prelude: >
+    It is strongly suggested to remove the ``[auto_fill_rules]`` section from
+    the configuration file or delete the entire file located at
+    ``$XDG_CONFIG_HOME/openconnect-sso/config.toml`` (most probably
+    ``~/.config/openconnect-sso/config.toml``). The fix of #4 involves an update
+    of the auto-fill rules but unfortulately they are persisted when the
+    application is first started. Removing them from the configuration forces
+    the updated set of rules to be written in the configuration.
+fixes:
+  - |
+    The embedded browser will now stop and waits for user input when the
+    previously stored credentials are invalid. This still not the proper
+    solution as saved credentials are not updated in that case but at least the
+    infinite login loop is circumvenied.

releasenotes/notes/python-3.6_compatibility-3c6eeb53de22ab20.yaml

@@ -1,6 +1,6 @@
 ---
 fixes:
   - |
-    Python 3.6 compatibilty has been fixedhowever The problem was caused by the
+    Python 3.6 compatibilty has been fixed. The problem was caused by the
     usage of ``asyncio.create_task(coro)``. Its usage was replaced with
     ``asyncio.ensure_future(core)``.

releasenotes/notes/window-close-crash-cd951224e4aedde2.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    A potential crash upon closing the browser window is fixed. The issue was
+    caused by the mismatch between Python's and Qt's memory management model.

tests/conftest.py

@@ -0,0 +1,3 @@
+import os
+
+os.environ["COVERAGE_PROCESS_START"] = ".coveragerc"

tests/test_browser.py

@@ -0,0 +1,43 @@
+import attr
+import pytest
+
+from openconnect_sso.browser import Browser
+
+
+@pytest.mark.asyncio
+async def test_browser_context_manager_should_work_in_empty_context_manager():
+    async with Browser() as browser:
+        pass
+
+
+@pytest.mark.asyncio
+async def test_browser_reports_loaded_url(httpserver):
+    async with Browser() as browser:
+        auth_url = httpserver.url_for("/authenticate")
+
+        await browser.authenticate_at(auth_url, credentials=None)
+
+        assert browser.url is None
+        await browser.page_loaded()
+        assert browser.url == auth_url
+
+
+@pytest.mark.asyncio
+async def test_browser_cookies_accessible(httpserver):
+    async with Browser() as browser:
+        httpserver.expect_request("/authenticate").respond_with_data(
+            "<html><body>Hello</body></html>",
+            headers={"Set-Cookie": "cookie-name=cookie-value"},
+        )
+        auth_url = httpserver.url_for("/authenticate")
+        cred = Credentials("username", "password")
+
+        await browser.authenticate_at(auth_url, cred)
+        await browser.page_loaded()
+        assert browser.cookies == {"cookie-name": "cookie-value"}
+
+
+@attr.s
+class Credentials:
+    username = attr.ib()
+    password = attr.ib()