basic auth support (#394)

* basic auth implemented

* static

* simplified retry wrapper

* simplified retry some more

* refactored get url assertions

* lint

* fixed itest

config.yaml

@@ -8,6 +8,15 @@ options:
       This feature is experimental and may be unstable.
     type: boolean
     default: False
+  basic_auth_user:
+    description: |
+      Enables the `basicAuth` middleware for **all** routes on this proxy. 
+      The format of this string must be: `name:hashed-password`, generated with e.g. htpasswd.
+      Supported hashing algorithms are: MD5, SHA1, BCrypt.
+      For more documentation see https://doc.traefik.io/traefik/middlewares/http/basicauth/
+      Once this config option is set, the username/password pair will be required to authenticate 
+      http requests on all routes proxied by this traefik app.
+    type: string
   external_hostname:
     description: |
       The DNS name to be used by Traefik ingress.

src/charm.py

@@ -127,9 +127,7 @@ def __init__(self, *args):
         super().__init__(*args)
 
         self._stored.set_default(
-            current_external_host=None,
-            current_routing_mode=None,
-            current_forward_auth_mode=self.config["enable_experimental_forward_auth"],
+            config_hash=None,
         )
 
         self.container = self.unit.get_container(_TRAEFIK_CONTAINER_NAME)
@@ -173,6 +171,7 @@ def __init__(self, *args):
             tls_enabled=self._is_tls_enabled(),
             experimental_forward_auth_enabled=self._is_forward_auth_enabled,
             traefik_route_static_configs=self._traefik_route_static_configs(),
+            basic_auth_user=self._basic_auth_user,
         )
 
         self.service_patch = KubernetesServicePatch(
@@ -299,6 +298,14 @@ def _is_forward_auth_enabled(self) -> bool:
             return True
         return False
 
+    @property
+    def _basic_auth_user(self) -> Optional[str]:
+        """A single user for the global basic auth configuration.
+
+        As we can't reject it, we assume it's correctly formatted.
+        """
+        return cast(Optional[str], self.config.get("basic_auth_user", None))
+
     def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent):
         if self._is_forward_auth_enabled:
             if self.forward_auth.is_ready():
@@ -535,29 +542,41 @@ def _on_update_status(self, _: UpdateStatusEvent):
         self._process_status_and_configurations()
         self._set_workload_version()
 
+    @property
+    def _config_hash(self) -> int:
+        """A hash of the config of this application.
+
+        Only include here the config options that, should they change, should trigger a recalculation of
+        the traefik config files.
+        The main goal of this logic is to avoid recalculating status and configs on each event,
+        since it can be quite expensive.
+        """
+        return hash(
+            (
+                self._external_host,
+                self.config["routing_mode"],
+                self._is_forward_auth_enabled,
+                self._basic_auth_user,
+                self._is_tls_enabled(),
+            )
+        )
+
     def _on_config_changed(self, _: ConfigChangedEvent):
-        # If the external hostname is changed since we last processed it, we need to
-        # reconsider all data sent over the relations and all configs
-        new_external_host = self._external_host
-        new_routing_mode = self.config["routing_mode"]
-        new_forward_auth_mode = self._is_forward_auth_enabled
+        """Handle the ops.ConfigChanged event."""
+        # that we're processing a config-changed event, doesn't necessarily mean that our config has changed (duh!)
 
-        # TODO set BlockedStatus here when compound_status is introduced
-        #  https://github.com/canonical/operator/issues/665
+        # If the config hash has changed since we last calculated it, we need to
+        # recompute our state from scratch, based on all data sent over the relations and all configs
+        new_config_hash = self._config_hash
+        if self._stored.config_hash != new_config_hash:
+            self._stored.config_hash = new_config_hash
 
-        if (
-            self._stored.current_external_host != new_external_host  # type: ignore
-            or self._stored.current_routing_mode != new_routing_mode  # type: ignore
-            or self._stored.current_forward_auth_mode != new_forward_auth_mode  # type: ignore
-        ):
-            self._process_status_and_configurations()
-            self._stored.current_external_host = new_external_host  # type: ignore
-            self._stored.current_routing_mode = new_routing_mode  # type: ignore
-            self._stored.current_forward_auth_mode = new_forward_auth_mode  # type: ignore
+            if self._is_tls_enabled():
+                # we keep this nested under the hash-check because, unless the tls config has
+                # changed, we don't need to redo this.
+                self._update_cert_configs()
+                self._configure_traefik()
 
-        if self._is_tls_enabled():
-            self._update_cert_configs()
-            self._configure_traefik()
             self._process_status_and_configurations()
 
     def _process_status_and_configurations(self):

src/traefik.py

@@ -104,13 +104,15 @@ def __init__(
         experimental_forward_auth_enabled: bool,
         tcp_entrypoints: Dict[str, int],
         traefik_route_static_configs: Iterable[Dict[str, Any]],
+        basic_auth_user: Optional[str] = None,
     ):
         self._container = container
         self._tcp_entrypoints = tcp_entrypoints
         self._traefik_route_static_configs = traefik_route_static_configs
         self._routing_mode = routing_mode
         self._tls_enabled = tls_enabled
         self._experimental_forward_auth_enabled = experimental_forward_auth_enabled
+        self._basic_auth_user = basic_auth_user
 
     @property
     def scrape_jobs(self) -> list:
@@ -536,6 +538,14 @@ def _generate_middleware_config(
           "cannot create middleware: multi-types middleware not supported, consider declaring two
           different pieces of middleware instead"
         """
+        basicauth_middleware = {}
+        if basicauth_user := self._basic_auth_user:
+            basicauth_middleware[f"juju-basic-auth-{prefix}"] = {
+                "basicAuth": {
+                    "users": [basicauth_user],
+                }
+            }
+
         forwardauth_middleware = {}
         if self._experimental_forward_auth_enabled:
             if forward_auth_app:
@@ -560,7 +570,12 @@ def _generate_middleware_config(
                 "redirectScheme": {"scheme": "https", "port": 443, "permanent": True}
             }
 
-        return {**forwardauth_middleware, **no_prefix_middleware, **redir_scheme_middleware}
+        return {
+            **forwardauth_middleware,
+            **no_prefix_middleware,
+            **redir_scheme_middleware,
+            **basicauth_middleware,
+        }
 
     @staticmethod
     def generate_tls_config_for_route(

tests/integration/test_basic_auth.py

@@ -0,0 +1,133 @@
+# Copyright 2022 Canonical Ltd.
+# See LICENSE file for licensing details.
+import asyncio
+import subprocess
+import time
+import urllib.request
+from urllib.error import HTTPError
+
+import pytest
+import yaml
+from pytest_operator.plugin import OpsTest
+from tenacity import retry, stop_after_delay
+
+from tests.integration.conftest import (
+    get_relation_data,
+    trfk_resources,
+)
+
+USERNAME = "admin"
+PASSWORD = "admin"
+
+# we don't expect a 200 because ipa-tester has no real server listening
+SUCCESS_EXIT_CODE = 502
+
+# user:hashed-password pair generated via https://www.transip.nl/htpasswd/
+TEST_AUTH_USER = r"admin:$2a$13$XOHdzKdVS4mPKT0LvOfXru4LqyLbwcEvFlssXGS3laC6d/i6cKrLS"
+APP_NAME = "traefik"
+IPA = "ipa-tester"
+
+
+@pytest.mark.abort_on_fail
+@pytest.mark.skip_on_deployed
+async def test_deployment(ops_test: OpsTest, traefik_charm, ipa_tester_charm):
+    await asyncio.gather(
+        ops_test.model.deploy(traefik_charm, application_name=APP_NAME, resources=trfk_resources),
+        ops_test.model.deploy(ipa_tester_charm, IPA),
+    )
+
+    await ops_test.model.wait_for_idle([APP_NAME, IPA], status="active", timeout=1000)
+
+
+@pytest.mark.abort_on_fail
+@pytest.mark.skip_on_deployed
+async def test_relate(ops_test: OpsTest):
+    await ops_test.model.add_relation("ipa-tester:ingress", f"{APP_NAME}:ingress")
+    await ops_test.model.wait_for_idle([APP_NAME, IPA])
+
+
+def get_tester_url(model):
+    data = get_relation_data(
+        requirer_endpoint=f"{IPA}/0:ingress",
+        provider_endpoint=f"{APP_NAME}/0:ingress",
+        model=model,
+    )
+    provider_app_data = yaml.safe_load(data.provider.application_data["ingress"])
+    return provider_app_data["url"]
+
+
+@retry(stop=stop_after_delay(60 * 1))  # 5 minutes
+def assert_get_url_returns(url: str, expected: int, auth: str = None):
+    print(f"attempting to curl {url} (with auth? {'yes' if auth else 'no'})")
+    if auth:
+        passman = urllib.request.HTTPPasswordMgrWithDefaultRealm()
+        passman.add_password(None, url, USERNAME, PASSWORD)
+        authhandler = urllib.request.HTTPBasicAuthHandler(passman)
+        opener = urllib.request.build_opener(authhandler)
+        urllib.request.install_opener(opener)
+
+    try:
+        urllib.request.urlopen(url, timeout=1)
+    except HTTPError as e:
+        if e.code == expected:
+            return True
+
+        print(f"unexpected exit code {e.code}")
+        time.sleep(0.1)
+        raise AssertionError
+
+    if expected == 200:
+        return True
+
+    print("unexpected 200")
+    time.sleep(0.1)
+    raise AssertionError
+
+
+@pytest.fixture
+def model(ops_test):
+    return ops_test.model_full_name
+
+
+def set_basic_auth(model: str, user: str):
+    print(f"setting basic auth to {user!r}")
+    option = f"basic_auth_user={user}" if user else "basic_auth_user="
+    subprocess.run(["juju", "config", "-m", model, APP_NAME, option])
+
+
+def test_ipa_charm_ingress_noauth(model):
+    # GIVEN basic auth is disabled (initial condition)
+    set_basic_auth(model, "")
+    tester_url = get_tester_url(model)
+
+    # WHEN we GET the tester url
+    # THEN we get it fine
+    assert_get_url_returns(tester_url, expected=SUCCESS_EXIT_CODE)
+
+
+def test_ipa_charm_ingress_auth(model):
+    # GIVEN basic auth is disabled (previous test)
+    tester_url = get_tester_url(model)
+
+    # WHEN we enable basic auth
+    set_basic_auth(model, TEST_AUTH_USER)
+
+    # THEN we can't GET the tester url
+    # might take a little bit to apply the new config
+    # 401 unauthorized
+    assert_get_url_returns(tester_url, expected=401)
+
+    # UNLESS we use auth
+    assert_get_url_returns(tester_url, expected=SUCCESS_EXIT_CODE, auth=TEST_AUTH_USER)
+
+
+def test_ipa_charm_ingress_auth_disable(model):
+    # GIVEN auth is enabled (previous test)
+    tester_url = get_tester_url(model)
+
+    # WHEN we disable it again
+    set_basic_auth(model, "")
+
+    # THEN we eventually can GET the endpoint without auth
+    # might take a little bit to apply the new config
+    assert_get_url_returns(tester_url, expected=SUCCESS_EXIT_CODE)

tests/scenario/test_middlewares.py

@@ -2,15 +2,17 @@
 # See LICENSE file for licensing details.
 
 import tempfile
-import unittest
+from pathlib import Path
 from unittest.mock import PropertyMock, patch
 
 import ops
 import pytest
+import scenario
 import yaml
 from scenario import Container, ExecOutput, Mount, Relation, State
 
 from tests.scenario._utils import _render_config, create_ingress_relation
+from traefik import DYNAMIC_CONFIG_DIR
 
 
 def _create_relation(
@@ -127,5 +129,42 @@ def test_middleware_config(
     assert yaml.safe_load(config_file) == expected
 
 
-if __name__ == "__main__":
-    unittest.main()
+@patch("charm.TraefikIngressCharm.version", PropertyMock(return_value="0.0.0"))
+def test_basicauth_config(traefik_ctx: scenario.Context):
+    # GIVEN traefik is configured with a sample basicauth user
+    ingress = create_ingress_relation()
+    basicauth_user = "user:hashed-password"
+    state = State(
+        config={"basic_auth_user": basicauth_user},
+        relations=[ingress],
+        containers=[
+            Container(
+                name="traefik",
+                can_connect=True,
+                exec_mock={
+                    ("find", "/opt/traefik/juju", "-name", "*.yaml", "-delete"): ExecOutput()
+                },
+                layers={
+                    "traefik": ops.pebble.Layer({"services": {"traefik": {"startup": "enabled"}}})
+                },
+                service_status={"traefik": ops.pebble.ServiceStatus.ACTIVE},
+            )
+        ],
+    )
+
+    # WHEN we process a config-changed event
+    state_out = traefik_ctx.run("config_changed", state)
+
+    # THEN traefik writes a dynamic config file with the expected basicauth middleware
+    traefik_fs = state_out.get_container("traefik").get_filesystem(traefik_ctx)
+    dynamic_config_path = (
+        Path(str(traefik_fs) + DYNAMIC_CONFIG_DIR)
+        / f"juju_ingress_{ingress.endpoint}_{ingress.relation_id}_{ingress.remote_app_name}.yaml"
+    )
+    assert dynamic_config_path.exists()
+    http_cfg = yaml.safe_load(dynamic_config_path.read_text())["http"]
+    assert http_cfg["middlewares"]["juju-basic-auth-test-model-remote-0"] == {
+        "basicAuth": {"users": [basicauth_user]}
+    }
+    for router in http_cfg["routers"].values():
+        assert "juju-basic-auth-test-model-remote-0" in router["middlewares"]