From 2a0cc44e50df0679533c0525e575281da47c0055 Mon Sep 17 00:00:00 2001 From: natalia Date: Tue, 24 Oct 2023 16:21:29 +0200 Subject: [PATCH 01/17] feat: add forward-auth relation --- .../harness_extensions/v0/capture_events.py | 1 - lib/charms/oathkeeper/v0/forward_auth.py | 577 ++++++++++++++++++ metadata.yaml | 3 + src/charm.py | 45 ++ tests/unit/test_charm.py | 85 ++- 5 files changed, 690 insertions(+), 21 deletions(-) create mode 100644 lib/charms/oathkeeper/v0/forward_auth.py diff --git a/lib/charms/harness_extensions/v0/capture_events.py b/lib/charms/harness_extensions/v0/capture_events.py index 88819159..e5e2fd4e 100644 --- a/lib/charms/harness_extensions/v0/capture_events.py +++ b/lib/charms/harness_extensions/v0/capture_events.py @@ -83,4 +83,3 @@ def capture(charm: CharmBase, typ_: Type[_T] = EventBase) -> Iterator[Captured[_ event = captured[0] assert isinstance(event, typ_), f"expected {typ_}, not {type(event)}" result.event = event - diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py new file mode 100644 index 00000000..d29318df --- /dev/null +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -0,0 +1,577 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Interface library for providing API Gateways with Identity and Access Proxy information. + +It is required to integrate with Oathkeeper (Policy Decision Point). + +## Getting Started + +To get started using the library, you need to fetch the library using `charmcraft`. +**Note that you also need to add `jsonschema` to your charm's `requirements.txt`.** + +```shell +cd some-charm +charmcraft fetch-lib charms.oathkeeper.v0.forward_auth +``` + +To use the library from the requirer side, add the following to the `metadata.yaml` of the charm: + +```yaml +requires: + forward-auth: + interface: forward_auth + limit: 1 +``` + +Then, to initialise the library: +```python +from charms.oathkeeper.v0.forward_auth import ForwardAuthConfigChangedEvent, ForwardAuthRequirer + +class ApiGatewayCharm(CharmBase): + def __init__(self, *args): + # ... + self.forward_auth = ForwardAuthRequirer(self) + self.framework.observe( + self.forward_auth.on.forward_auth_config_changed, + self.some_event_function + ) + + def some_event_function(self, event: ForwardAuthConfigChangedEvent): + if self.forward_auth.is_ready(): + # Fetch the relation info + forward_auth_data = self.forward_auth.get_forward_auth_data() + # update ingress configuration + # ... +``` +""" + +import inspect +import json +import logging +from dataclasses import asdict, dataclass, field +from typing import Dict, List, Mapping, Optional + +import jsonschema +from ops.charm import CharmBase, RelationChangedEvent, RelationCreatedEvent, RelationDepartedEvent +from ops.framework import EventBase, EventSource, Handle, Object, ObjectEvents +from ops.model import Relation, TooManyRelatedAppsError + +# The unique Charmhub library identifier, never change it +LIBID = "3fd31fa89da34d7f9ad9b62d5f7e7b48" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 1 + +RELATION_NAME = "forward-auth" +INTERFACE_NAME = "forward_auth" + +logger = logging.getLogger(__name__) + +FORWARD_AUTH_PROVIDER_JSON_SCHEMA = { + "$schema": "http://json-schema.org/draft-07/schema", + "$id": "https://canonical.github.io/charm-relation-interfaces/docs/json_schemas/forward_auth/v0/provider.json", + "type": "object", + "properties": { + "decisions_address": {"type": "string", "default": None}, + "app_names": {"type": "array", "default": None, "items": {"type": "string"}}, + "headers": {"type": "array", "default": None, "items": {"type": "string"}}, + }, + "required": ["decisions_address", "app_names"], +} + +FORWARD_AUTH_REQUIRER_JSON_SCHEMA = { + "$schema": "http://json-schema.org/draft-07/schema", + "$id": "https://canonical.github.io/charm-relation-interfaces/docs/json_schemas/forward_auth/v0/requirer.json", + "type": "object", + "properties": { + "ingress_app_names": {"type": "array", "default": None, "items": {"type": "string"}}, + }, + "required": ["ingress_app_names"], +} + + +class ForwardAuthConfigError(Exception): + """Emitted when invalid forward auth config is provided.""" + + +class DataValidationError(RuntimeError): + """Raised when data validation fails on relation data.""" + + +def _load_data(data: Mapping, schema: Optional[Dict] = None) -> Dict: + """Parses nested fields and checks whether `data` matches `schema`.""" + ret = {} + for k, v in data.items(): + try: + ret[k] = json.loads(v) + except json.JSONDecodeError: + ret[k] = v + + if schema: + _validate_data(ret, schema) + return ret + + +def _dump_data(data: Dict, schema: Optional[Dict] = None) -> Dict: + if schema: + _validate_data(data, schema) + + ret = {} + for k, v in data.items(): + if isinstance(v, (list, dict)): + try: + ret[k] = json.dumps(v) + except json.JSONDecodeError as e: + raise DataValidationError(f"Failed to encode relation json: {e}") + else: + ret[k] = v + return ret + + +class ForwardAuthRelation(Object): + """A class containing helper methods for forward-auth relation.""" + + def _pop_relation_data(self, relation_id: Relation) -> None: + if not self.model.unit.is_leader(): + return + + if len(self.model.relations) == 0: + return + + relation = self.model.get_relation(self._relation_name, relation_id=relation_id) + if not relation or not relation.app: + return + + try: + for data in list(relation.data[self.model.app]): + relation.data[self.model.app].pop(data, "") + except Exception as e: + logger.info(f"Failed to pop the relation data: {e}") + + +def _validate_data(data: Dict, schema: Dict) -> None: + """Checks whether `data` matches `schema`. + + Will raise DataValidationError if the data is not valid, else return None. + """ + try: + jsonschema.validate(instance=data, schema=schema) + except jsonschema.ValidationError as e: + raise DataValidationError(data, schema) from e + + +@dataclass +class ForwardAuthConfig: + """Helper class containing configuration required by API Gateway to set up the proxy.""" + + decisions_address: str + app_names: List[str] + headers: List[str] = field(default_factory=lambda: []) + + @classmethod + def from_dict(cls, dic: Dict) -> "ForwardAuthConfig": + """Generate ForwardAuthConfig instance from dict.""" + return cls(**{k: v for k, v in dic.items() if k in inspect.signature(cls).parameters}) + + def to_dict(self) -> Dict: + """Convert object to dict.""" + return {k: v for k, v in asdict(self).items() if v is not None} + + +@dataclass +class RequirerConfig: + """Helper class containing configuration required by Oathkeeper. + + Its purpose is to evaluate whether apps can be protected by IAP. + """ + + ingress_app_names: List[str] = field(default_factory=lambda: []) + + def to_dict(self) -> Dict: + """Convert object to dict.""" + return {k: v for k, v in asdict(self).items() if v is not None} + + +class ForwardAuthConfigChangedEvent(EventBase): + """Event to notify the requirer charm that the forward-auth config has changed.""" + + def __init__( + self, + handle: Handle, + decisions_address: str, + app_names: List[str], + headers: List[str], + relation_id: int, + relation_app_name: str, + ) -> None: + super().__init__(handle) + self.decisions_address = decisions_address + self.app_names = app_names + self.headers = headers + self.relation_id = relation_id + self.relation_app_name = relation_app_name + + def snapshot(self) -> Dict: + """Save event.""" + return { + "decisions_address": self.decisions_address, + "app_names": self.app_names, + "headers": self.headers, + "relation_id": self.relation_id, + "relation_app_name": self.relation_app_name, + } + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.decisions_address = snapshot["decisions_address"] + self.app_names = snapshot["app_names"] + self.headers = snapshot["headers"] + self.relation_id = snapshot["relation_id"] + self.relation_app_name = snapshot["relation_app_name"] + + +class ForwardAuthConfigRemovedEvent(EventBase): + """Event to notify the requirer charm that the forward-auth config was removed.""" + + def __init__( + self, + handle: Handle, + relation_id: int, + ) -> None: + super().__init__(handle) + self.relation_id = relation_id + + def snapshot(self) -> Dict: + """Save event.""" + return {"relation_id": self.relation_id} + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.relation_id = snapshot["relation_id"] + + +class ForwardAuthRequirerEvents(ObjectEvents): + """Event descriptor for events raised by `ForwardAuthRequirer`.""" + + forward_auth_config_changed = EventSource(ForwardAuthConfigChangedEvent) + forward_auth_config_removed = EventSource(ForwardAuthConfigRemovedEvent) + + +class ForwardAuthRequirer(ForwardAuthRelation): + """Requirer side of the forward-auth relation.""" + + on = ForwardAuthRequirerEvents() + + def __init__( + self, + charm: CharmBase, + relation_name: str = RELATION_NAME, + forward_auth_requirer_config: Optional[RequirerConfig] = None, + ): + super().__init__(charm, relation_name) + + self._charm = charm + self._relation_name = relation_name + self._forward_auth_requirer_config = forward_auth_requirer_config + + events = self._charm.on[relation_name] + self.framework.observe(events.relation_created, self._on_relation_created_event) + self.framework.observe(events.relation_changed, self._on_relation_changed_event) + self.framework.observe(events.relation_departed, self._on_relation_departed_event) + + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Update the relation with requirer data when a relation is created.""" + if not self.model.unit.is_leader(): + return + + self.update_requirer_relation_data(self._forward_auth_requirer_config, event.relation.id) + + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: + """Get the forward-auth config and emit a custom config-changed event.""" + if not self.model.unit.is_leader(): + return + + data = event.relation.data[event.app] + if not data: + logger.info("No provider relation data available.") + return + + try: + forward_auth_data = _load_data(data, FORWARD_AUTH_PROVIDER_JSON_SCHEMA) + except DataValidationError as e: + logger.error( + f"Received invalid config from the provider: {e}. Config-changed will not be emitted." + ) + return + + decisions_address = forward_auth_data.get("decisions_address") + app_names = forward_auth_data.get("app_names") + headers = forward_auth_data.get("headers") + + relation_id = event.relation.id + relation_app_name = event.relation.app.name + + # Notify Traefik to update the routes + self.on.forward_auth_config_changed.emit( + decisions_address, app_names, headers, relation_id, relation_app_name + ) + + def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: + """Notify the requirer that the relation has departed.""" + self.on.forward_auth_config_removed.emit(event.relation.id) + + def update_requirer_relation_data( + self, ingress_app_names: Optional[RequirerConfig], relation_id: Optional[int] = None + ) -> None: + """Update the relation databag with app names that can get IAP protection.""" + if not self.model.unit.is_leader(): + return + + if not ingress_app_names: + logger.error("Ingress-related app names are missing") + return + + if not isinstance(ingress_app_names, RequirerConfig): + raise ValueError(f"Unexpected type: {type(ingress_app_names)}") + + try: + relation = self.model.get_relation( + relation_name=self._relation_name, relation_id=relation_id + ) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relation is defined. Please provide a relation_id") + + if not relation or not relation.app: + return + + data = _dump_data(ingress_app_names.to_dict(), FORWARD_AUTH_REQUIRER_JSON_SCHEMA) + relation.data[self.model.app].update(data) + + def get_provider_info(self, relation_id: Optional[int] = None) -> Optional[ForwardAuthConfig]: + """Get the provider information from the databag.""" + if len(self.model.relations) == 0: + return None + try: + relation = self.model.get_relation(self._relation_name, relation_id=relation_id) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relation is defined. Please provide a relation_id") + if not relation or not relation.app: + return None + + data = relation.data[relation.app] + if not data: + logger.info("No relation data available.") + return + + data = _load_data(data, FORWARD_AUTH_PROVIDER_JSON_SCHEMA) + forward_auth_config = ForwardAuthConfig.from_dict(data) + logger.info(f"ForwardAuthConfig: {forward_auth_config}") + + return forward_auth_config + + def get_remote_app_name(self, relation_id: Optional[int] = None) -> Optional[str]: + """Get the remote app name.""" + if len(self.model.relations) == 0: + return None + try: + relation = self.model.get_relation(self._relation_name, relation_id=relation_id) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relation is defined. Please provide a relation_id") + if not relation or not relation.app: + return None + + return relation.app.name + + def is_ready(self, relation_id: Optional[int] = None) -> Optional[bool]: + """Checks whether ForwardAuth is ready on this relation. + + Returns True when Oathkeeper shared the config; False otherwise. + """ + if len(self.model.relations) == 0: + return None + try: + relation = self.model.get_relation(self._relation_name, relation_id=relation_id) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relation is defined. Please provide a relation_id") + + if not relation or not relation.app: + return None + + return ( + "decisions_address" in relation.data[relation.app] + and "app_names" in relation.data[relation.app] + ) + + +class ForwardAuthProxySet(EventBase): + """Event to notify the charm that the proxy was set successfully.""" + + def snapshot(self) -> Dict: + """Save event.""" + return {} + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + pass + + +class InvalidForwardAuthConfigEvent(EventBase): + """Event to notify the charm that the forward-auth configuration is invalid.""" + + def __init__(self, handle: Handle, error: str): + super().__init__(handle) + self.error = error + + def snapshot(self) -> Dict: + """Save event.""" + return { + "error": self.error, + } + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.error = snapshot["error"] + + +class ForwardAuthRelationRemovedEvent(EventBase): + """Event to notify the charm that the relation was removed.""" + + def __init__( + self, + handle: Handle, + relation_id: int, + ) -> None: + super().__init__(handle) + self.relation_id = relation_id + + def snapshot(self) -> Dict: + """Save event.""" + return {"relation_id": self.relation_id} + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.relation_id = snapshot["relation_id"] + + +class ForwardAuthProviderEvents(ObjectEvents): + """Event descriptor for events raised by `ForwardAuthProvider`.""" + + forward_auth_proxy_set = EventSource(ForwardAuthProxySet) + invalid_forward_auth_config = EventSource(InvalidForwardAuthConfigEvent) + forward_auth_relation_removed = EventSource(ForwardAuthRelationRemovedEvent) + + +class ForwardAuthProvider(ForwardAuthRelation): + """Provider side of the forward-auth relation.""" + + on = ForwardAuthProviderEvents() + + def __init__( + self, + charm: CharmBase, + relation_name: str = RELATION_NAME, + forward_auth_config: Optional[ForwardAuthConfig] = None, + ) -> None: + super().__init__(charm, relation_name) + self.charm = charm + self._relation_name = relation_name + self.forward_auth_config = forward_auth_config + + events = self.charm.on[relation_name] + self.framework.observe(events.relation_created, self._on_relation_created_event) + self.framework.observe(events.relation_changed, self._on_relation_changed_event) + self.framework.observe(events.relation_departed, self._on_relation_departed_event) + + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Update the relation with provider data when a relation is created.""" + if not self.model.unit.is_leader(): + return + + try: + self._update_relation_data(self.forward_auth_config, event.relation.id) + except ForwardAuthConfigError as e: + self.on.invalid_forward_auth_config.emit(e.args[0]) + + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: + """Update the relation with forward-auth config when a relation is changed.""" + if not self.model.unit.is_leader(): + return + + # Compare ingress-related apps with apps that requested the proxy + self._compare_apps() + + def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: + """Wipe the relation databag and notify the charm that the relation has departed.""" + # Workaround for https://github.com/canonical/operator/issues/888 + self._pop_relation_data(event.relation.id) + + self.on.forward_auth_relation_removed.emit(event.relation.id) + + def _compare_apps(self, relation_id: Optional[int] = None) -> None: + """Compare app names provided by Oathkeeper with apps that are related via ingress. + + The ingress-related app names are provided by the relation requirer. + If an app is not related via ingress-per-app/leader/unit, + emit `InvalidForwardAuthConfigEvent`. + If the app is related via ingress and thus eligible for IAP, emit `ForwardAuthProxySet`. + """ + if len(self.model.relations) == 0: + return None + try: + relation = self.model.get_relation(self._relation_name, relation_id=relation_id) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relation is defined. Please provide a relation_id") + if not relation or not relation.app: + return None + + requirer_data = relation.data[relation.app] + if not requirer_data: + logger.info("No requirer relation data available.") + return + + ingress_apps = requirer_data["ingress_app_names"] + + for app in json.loads(relation.data[self.model.app]["app_names"]): + if app not in ingress_apps: + self.on.invalid_forward_auth_config.emit(error=f"{app} is not related via ingress") + return + self.on.forward_auth_proxy_set.emit() + + def _update_relation_data( + self, forward_auth_config: Optional[ForwardAuthConfig], relation_id: Optional[int] = None + ) -> None: + """Validate the forward-auth config and update the relation databag.""" + if not self.model.unit.is_leader(): + return + + if not forward_auth_config: + logger.info("Forward-auth config is missing") + return + + if not isinstance(forward_auth_config, ForwardAuthConfig): + raise ValueError(f"Unexpected forward_auth_config type: {type(forward_auth_config)}") + + try: + relation = self.model.get_relation( + relation_name=self._relation_name, relation_id=relation_id + ) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relation is defined. Please provide a relation_id") + + if not relation or not relation.app: + return + + data = _dump_data(forward_auth_config.to_dict(), FORWARD_AUTH_PROVIDER_JSON_SCHEMA) + relation.data[self.model.app].update(data) + + def update_forward_auth_config( + self, forward_auth_config: ForwardAuthConfig, relation_id: Optional[int] = None + ) -> None: + """Update the forward-auth config stored in the object.""" + self._update_relation_data(forward_auth_config, relation_id=relation_id) diff --git a/metadata.yaml b/metadata.yaml index e46d1425..29824202 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -72,6 +72,9 @@ requires: limit: 1 description: | Send a CSR to- and obtain a signed certificate from an external CA. + forward-auth: + interface: forward_auth + limit: 1 logging: interface: loki_push_api description: | diff --git a/src/charm.py b/src/charm.py index 8532414f..3cca7fbe 100755 --- a/src/charm.py +++ b/src/charm.py @@ -9,6 +9,8 @@ import json import logging import socket +import typing +from string import Template from typing import Any, Dict, List, Optional, Union from urllib.parse import urlparse @@ -24,6 +26,12 @@ ) from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer +from charms.oathkeeper.v0.forward_auth import ( + ForwardAuthConfigChangedEvent, + ForwardAuthConfigRemovedEvent, + ForwardAuthRequirer, + RequirerConfig, +) from charms.observability_libs.v0.cert_handler import CertHandler from charms.observability_libs.v1.kubernetes_service_patch import ( KubernetesServicePatch, @@ -190,6 +198,11 @@ def __init__(self, *args): self.on.update_status, # type: ignore ], ) + + self.forward_auth = ForwardAuthRequirer( + self, forward_auth_requirer_config=self._forward_auth_requirer_config + ) + observe = self.framework.observe # TODO update init params once auto-renew is implemented @@ -223,6 +236,13 @@ def __init__(self, *args): self._on_recv_ca_cert_removed, ) + observe( + self.forward_auth.on.forward_auth_config_changed, self._on_forward_auth_config_changed + ) + observe( + self.forward_auth.on.forward_auth_config_removed, self._on_forward_auth_config_removed + ) + # observe data_provided and data_removed events for all types of ingress we offer: for ingress in (self.ingress_per_unit, self.ingress_per_appv1, self.ingress_per_appv2): observe(ingress.on.data_provided, self._handle_ingress_data_provided) # type: ignore @@ -250,6 +270,25 @@ def _service_ports(self) -> List[ServicePort]: ServicePort(int(port), name=name) for name, port in self._tcp_entrypoints().items() ] + def _forward_auth_requirer_config(self) -> RequirerConfig: + ingress_app_names = [] + for ingress_relation in ( + self.ingress_per_appv1.relations + + self.ingress_per_appv2.relations + + self.ingress_per_unit.relations + + self.traefik_route.relations + ): + ingress_app_names.append(ingress_relation.app.name) + return RequirerConfig(ingress_app_names) + + def _on_forward_auth_config_changed(self, event: ForwardAuthConfigChangedEvent): + self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + if self.forward_auth.is_ready(): + self._process_status_and_configurations() + + def _on_forward_auth_config_removed(self, event: ForwardAuthConfigRemovedEvent): + self._process_status_and_configurations() + def _on_recv_ca_cert_available(self, event: CertificateTransferAvailableEvent): # Assuming only one cert per relation (this is in line with the original lib design). if not self.container.can_connect(): @@ -551,6 +590,9 @@ def _handle_ingress_data_provided(self, event: RelationEvent): # update-status. self._process_status_and_configurations() + if self.forward_auth.is_ready(): + self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + if isinstance(self.unit.status, MaintenanceStatus): self.unit.status = ActiveStatus() @@ -560,6 +602,9 @@ def _handle_ingress_data_removed(self, event: RelationEvent): event.relation, wipe_rel_data=not isinstance(event, RelationBrokenEvent) ) + if self.forward_auth.is_ready(): + self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + # FIXME? on relation broken, data is still there so cannot simply call # self._process_status_and_configurations(). For this reason, the static config in # traefik.STATIC_CONFIG_PATH will be updated only on update-status. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index ee0aa9d6..5c27e412 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -97,26 +97,6 @@ def _render_middlewares(*, strip_prefix: bool = False, redirect_https: bool = Fa ) -def _render_middlewares(*, strip_prefix: bool = False, redirect_https: bool = False) -> dict: - middlewares = {} - if redirect_https: - middlewares.update({"redirectScheme": {"scheme": "https", "port": 443, "permanent": True}}) - if strip_prefix: - middlewares.update( - { - "stripPrefix": { - "prefixes": ["/test-model-remote-0"], - "forceSlash": False, - } - } - ) - return ( - {"middlewares": {"juju-sidecar-noprefix-test-model-remote-0": middlewares}} - if middlewares - else {} - ) - - class _RequirerMock: local_app: Application = None relation: Relation = None @@ -441,6 +421,71 @@ def test_tcp_config(self): static_config = charm.unit.get_container("traefik").pull(STATIC_CONFIG_PATH).read() assert yaml.safe_load(static_config)["entryPoints"][prefix] == expected_entrypoint + def setup_forward_auth_relation(self) -> int: + relation_id = self.harness.add_relation("forward-auth", "provider") + self.harness.add_relation_unit(relation_id, "provider/0") + self.harness.update_relation_data( + relation_id, + "provider", + { + "decisions_address": "https://oathkeeper.test-model.svc.cluster.local:4456/decisions", + "app_names": '["charmed-app"]', + "headers": '["X-User"]', + }, + ) + + return relation_id + + @patch("charm.KubernetesServicePatch", lambda *_, **__: None) + def test_forward_auth_relation_databag(self): + self.harness.set_leader(True) + self.harness.update_config({"external_hostname": "testhostname"}) + self.harness.begin_with_initial_hooks() + + provider_info = { + "decisions_address": "https://oathkeeper.test-model.svc.cluster.local:4456/decisions", + "app_names": ["charmed-app"], + "headers": ["X-User"], + } + + _ = self.setup_forward_auth_relation() + + self.assertTrue(self.harness.charm.forward_auth.is_ready()) + + expected_provider_info = self.harness.charm.forward_auth.get_provider_info() + + assert expected_provider_info.decisions_address == provider_info["decisions_address"] + assert expected_provider_info.app_names == provider_info["app_names"] + assert expected_provider_info.headers == provider_info["headers"] + + @patch("charm.KubernetesServicePatch", lambda *_, **__: None) + def test_forward_auth_relation_changed(self): + self.harness.set_leader(True) + self.harness.update_config({"external_hostname": "testhostname"}) + self.harness.begin_with_initial_hooks() + + self.harness.charm._on_forward_auth_config_changed = mocked_handle = Mock( + return_value=None + ) + + _ = self.setup_forward_auth_relation() + assert mocked_handle.called + + @patch("charm.KubernetesServicePatch", lambda *_, **__: None) + def test_forward_auth_relation_removed(self): + self.harness.set_leader(True) + self.harness.update_config({"external_hostname": "testhostname"}) + self.harness.begin_with_initial_hooks() + + self.harness.charm._on_forward_auth_config_removed = mocked_handle = Mock( + return_value=None + ) + + relation_id = self.setup_forward_auth_relation() + self.harness.remove_relation(relation_id) + + assert mocked_handle.called + class TestTraefikCertTransferInterface(unittest.TestCase): def setUp(self): From 627adfc4bc667aae5d0f8180781da5880c4a50ca Mon Sep 17 00:00:00 2001 From: natalia Date: Wed, 25 Oct 2023 10:14:42 +0200 Subject: [PATCH 02/17] fix: static check errors --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 3cca7fbe..456dc416 100755 --- a/src/charm.py +++ b/src/charm.py @@ -278,7 +278,7 @@ def _forward_auth_requirer_config(self) -> RequirerConfig: + self.ingress_per_unit.relations + self.traefik_route.relations ): - ingress_app_names.append(ingress_relation.app.name) + ingress_app_names.append(ingress_relation.app.name) # type: ignore return RequirerConfig(ingress_app_names) def _on_forward_auth_config_changed(self, event: ForwardAuthConfigChangedEvent): From e66483c3fa00a6897e11c9fa142a6554438884dd Mon Sep 17 00:00:00 2001 From: natalia Date: Wed, 15 Nov 2023 13:45:44 +0100 Subject: [PATCH 03/17] refactor: address review comments --- lib/charms/oathkeeper/v0/forward_auth.py | 20 +++++------ metadata.yaml | 3 ++ src/charm.py | 45 ++++++++++++------------ 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index d29318df..e6de32b5 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -27,18 +27,18 @@ Then, to initialise the library: ```python -from charms.oathkeeper.v0.forward_auth import ForwardAuthConfigChangedEvent, ForwardAuthRequirer +from charms.oathkeeper.v0.forward_auth import AuthConfigChangedEvent, ForwardAuthRequirer class ApiGatewayCharm(CharmBase): def __init__(self, *args): # ... self.forward_auth = ForwardAuthRequirer(self) self.framework.observe( - self.forward_auth.on.forward_auth_config_changed, + self.forward_auth.on.auth_config_changed, self.some_event_function ) - def some_event_function(self, event: ForwardAuthConfigChangedEvent): + def some_event_function(self, event: AuthConfigChangedEvent): if self.forward_auth.is_ready(): # Fetch the relation info forward_auth_data = self.forward_auth.get_forward_auth_data() @@ -66,7 +66,7 @@ def some_event_function(self, event: ForwardAuthConfigChangedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 RELATION_NAME = "forward-auth" INTERFACE_NAME = "forward_auth" @@ -198,7 +198,7 @@ def to_dict(self) -> Dict: return {k: v for k, v in asdict(self).items() if v is not None} -class ForwardAuthConfigChangedEvent(EventBase): +class AuthConfigChangedEvent(EventBase): """Event to notify the requirer charm that the forward-auth config has changed.""" def __init__( @@ -236,7 +236,7 @@ def restore(self, snapshot: Dict) -> None: self.relation_app_name = snapshot["relation_app_name"] -class ForwardAuthConfigRemovedEvent(EventBase): +class AuthConfigRemovedEvent(EventBase): """Event to notify the requirer charm that the forward-auth config was removed.""" def __init__( @@ -259,8 +259,8 @@ def restore(self, snapshot: Dict) -> None: class ForwardAuthRequirerEvents(ObjectEvents): """Event descriptor for events raised by `ForwardAuthRequirer`.""" - forward_auth_config_changed = EventSource(ForwardAuthConfigChangedEvent) - forward_auth_config_removed = EventSource(ForwardAuthConfigRemovedEvent) + auth_config_changed = EventSource(AuthConfigChangedEvent) + auth_config_removed = EventSource(AuthConfigRemovedEvent) class ForwardAuthRequirer(ForwardAuthRelation): @@ -318,13 +318,13 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: relation_app_name = event.relation.app.name # Notify Traefik to update the routes - self.on.forward_auth_config_changed.emit( + self.on.auth_config_changed.emit( decisions_address, app_names, headers, relation_id, relation_app_name ) def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: """Notify the requirer that the relation has departed.""" - self.on.forward_auth_config_removed.emit(event.relation.id) + self.on.auth_config_removed.emit(event.relation.id) def update_requirer_relation_data( self, ingress_app_names: Optional[RequirerConfig], relation_id: Optional[int] = None diff --git a/metadata.yaml b/metadata.yaml index 29824202..23824eb1 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -75,6 +75,9 @@ requires: forward-auth: interface: forward_auth limit: 1 + description: | + Receives forwardAuth middleware config from Oathkeeper. + The same middleware will be applied to all charms that requested Identity and Access Proxy protection. logging: interface: loki_push_api description: | diff --git a/src/charm.py b/src/charm.py index 456dc416..11acd019 100755 --- a/src/charm.py +++ b/src/charm.py @@ -6,6 +6,8 @@ import contextlib import enum import functools +import ipaddress +import itertools import json import logging import socket @@ -27,8 +29,8 @@ from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer from charms.oathkeeper.v0.forward_auth import ( - ForwardAuthConfigChangedEvent, - ForwardAuthConfigRemovedEvent, + AuthConfigChangedEvent, + AuthConfigRemovedEvent, ForwardAuthRequirer, RequirerConfig, ) @@ -200,7 +202,7 @@ def __init__(self, *args): ) self.forward_auth = ForwardAuthRequirer( - self, forward_auth_requirer_config=self._forward_auth_requirer_config + self, forward_auth_requirer_config=self._forward_auth_config ) observe = self.framework.observe @@ -236,12 +238,8 @@ def __init__(self, *args): self._on_recv_ca_cert_removed, ) - observe( - self.forward_auth.on.forward_auth_config_changed, self._on_forward_auth_config_changed - ) - observe( - self.forward_auth.on.forward_auth_config_removed, self._on_forward_auth_config_removed - ) + observe(self.forward_auth.on.auth_config_changed, self._on_forward_auth_config_changed) + observe(self.forward_auth.on.auth_config_removed, self._on_forward_auth_config_removed) # observe data_provided and data_removed events for all types of ingress we offer: for ingress in (self.ingress_per_unit, self.ingress_per_appv1, self.ingress_per_appv2): @@ -270,23 +268,24 @@ def _service_ports(self) -> List[ServicePort]: ServicePort(int(port), name=name) for name, port in self._tcp_entrypoints().items() ] - def _forward_auth_requirer_config(self) -> RequirerConfig: - ingress_app_names = [] - for ingress_relation in ( - self.ingress_per_appv1.relations - + self.ingress_per_appv2.relations - + self.ingress_per_unit.relations - + self.traefik_route.relations - ): - ingress_app_names.append(ingress_relation.app.name) # type: ignore + def _forward_auth_config(self) -> RequirerConfig: + ingress_app_names = [ + rel.app.name # type: ignore + for rel in itertools.chain( + self.ingress_per_appv1.relations, + self.ingress_per_appv2.relations, + self.ingress_per_unit.relations, + self.traefik_route.relations, + ) + ] return RequirerConfig(ingress_app_names) - def _on_forward_auth_config_changed(self, event: ForwardAuthConfigChangedEvent): - self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) if self.forward_auth.is_ready(): self._process_status_and_configurations() - def _on_forward_auth_config_removed(self, event: ForwardAuthConfigRemovedEvent): + def _on_forward_auth_config_removed(self, event: AuthConfigRemovedEvent): self._process_status_and_configurations() def _on_recv_ca_cert_available(self, event: CertificateTransferAvailableEvent): @@ -591,7 +590,7 @@ def _handle_ingress_data_provided(self, event: RelationEvent): self._process_status_and_configurations() if self.forward_auth.is_ready(): - self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) if isinstance(self.unit.status, MaintenanceStatus): self.unit.status = ActiveStatus() @@ -603,7 +602,7 @@ def _handle_ingress_data_removed(self, event: RelationEvent): ) if self.forward_auth.is_ready(): - self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) # FIXME? on relation broken, data is still there so cannot simply call # self._process_status_and_configurations(). For this reason, the static config in From e62dfc5508a5a2af1241e63d6419c1f0489a04d0 Mon Sep 17 00:00:00 2001 From: natalia Date: Thu, 16 Nov 2023 13:45:42 +0100 Subject: [PATCH 04/17] feat: add experimental flag --- config.yaml | 6 ++++++ metadata.yaml | 3 ++- src/charm.py | 19 +++++++++++++------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/config.yaml b/config.yaml index 0366ee00..5947e1fa 100644 --- a/config.yaml +++ b/config.yaml @@ -2,6 +2,12 @@ # See LICENSE file for licensing details. options: + enable_experimental_forward_auth: + description: | + Enables `forward-auth` middleware capabilities required to set up Identity and Access Proxy. + This feature is experimental and may be unstable. + type: boolean + default: False external_hostname: description: | The DNS name to be used by Traefik ingress. diff --git a/metadata.yaml b/metadata.yaml index 23824eb1..62db66b1 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -72,12 +72,13 @@ requires: limit: 1 description: | Send a CSR to- and obtain a signed certificate from an external CA. - forward-auth: + experimental-forward-auth: interface: forward_auth limit: 1 description: | Receives forwardAuth middleware config from Oathkeeper. The same middleware will be applied to all charms that requested Identity and Access Proxy protection. + This feature is experimental and may be unstable. In order to enable it, run `juju config enable_experimental_forward_auth=True`. logging: interface: loki_push_api description: | diff --git a/src/charm.py b/src/charm.py index 11acd019..c70500e3 100755 --- a/src/charm.py +++ b/src/charm.py @@ -201,9 +201,7 @@ def __init__(self, *args): ], ) - self.forward_auth = ForwardAuthRequirer( - self, forward_auth_requirer_config=self._forward_auth_config - ) + self.forward_auth = ForwardAuthRequirer(self, relation_name="experimental-forward-auth") observe = self.framework.observe @@ -281,9 +279,14 @@ def _forward_auth_config(self) -> RequirerConfig: return RequirerConfig(ingress_app_names) def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): - self.forward_auth.update_requirer_relation_data(self._forward_auth_config) - if self.forward_auth.is_ready(): - self._process_status_and_configurations() + if self.config["enable_experimental_forward_auth"]: + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) + if self.forward_auth.is_ready(): + self._process_status_and_configurations() + else: + logger.info( + "The `enable_experimental_forward_auth` config option is not enabled. Forward-auth relation will not be processed" + ) def _on_forward_auth_config_removed(self, event: AuthConfigRemovedEvent): self._process_status_and_configurations() @@ -472,6 +475,10 @@ def _on_config_changed(self, _: ConfigChangedEvent): # reconsider all data sent over the relations and all configs new_external_host = self._external_host new_routing_mode = self.config["routing_mode"] + + if self.config["enable_experimental_forward_auth"]: + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) + # TODO set BlockedStatus here when compound_status is introduced # https://github.com/canonical/operator/issues/665 From b247e5f9013f5dc0106c7c453269fd4c45390e3d Mon Sep 17 00:00:00 2001 From: natalia Date: Thu, 16 Nov 2023 13:46:39 +0100 Subject: [PATCH 05/17] test: update unit tests --- tests/unit/test_charm.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 5c27e412..16b113b6 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -422,7 +422,7 @@ def test_tcp_config(self): assert yaml.safe_load(static_config)["entryPoints"][prefix] == expected_entrypoint def setup_forward_auth_relation(self) -> int: - relation_id = self.harness.add_relation("forward-auth", "provider") + relation_id = self.harness.add_relation("experimental-forward-auth", "provider") self.harness.add_relation_unit(relation_id, "provider/0") self.harness.update_relation_data( relation_id, @@ -438,6 +438,7 @@ def setup_forward_auth_relation(self) -> int: @patch("charm.KubernetesServicePatch", lambda *_, **__: None) def test_forward_auth_relation_databag(self): + self.harness.update_config({"enable_experimental_forward_auth": True}) self.harness.set_leader(True) self.harness.update_config({"external_hostname": "testhostname"}) self.harness.begin_with_initial_hooks() @@ -460,6 +461,7 @@ def test_forward_auth_relation_databag(self): @patch("charm.KubernetesServicePatch", lambda *_, **__: None) def test_forward_auth_relation_changed(self): + self.harness.update_config({"enable_experimental_forward_auth": True}) self.harness.set_leader(True) self.harness.update_config({"external_hostname": "testhostname"}) self.harness.begin_with_initial_hooks() From a3ed7fb85abefbb9955bbdcf575f194f44f5fb9e Mon Sep 17 00:00:00 2001 From: natalia Date: Thu, 16 Nov 2023 13:47:38 +0100 Subject: [PATCH 06/17] refactor: address review comments on forward-auth lib --- lib/charms/oathkeeper/v0/forward_auth.py | 57 ++++++++++++++---------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index e6de32b5..144fd8cc 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -54,7 +54,13 @@ def some_event_function(self, event: AuthConfigChangedEvent): from typing import Dict, List, Mapping, Optional import jsonschema -from ops.charm import CharmBase, RelationChangedEvent, RelationCreatedEvent, RelationDepartedEvent +from ops.charm import ( + CharmBase, + RelationBrokenEvent, + RelationChangedEvent, + RelationCreatedEvent, + RelationDepartedEvent, +) from ops.framework import EventBase, EventSource, Handle, Object, ObjectEvents from ops.model import Relation, TooManyRelatedAppsError @@ -271,6 +277,7 @@ class ForwardAuthRequirer(ForwardAuthRelation): def __init__( self, charm: CharmBase, + *, relation_name: str = RELATION_NAME, forward_auth_requirer_config: Optional[RequirerConfig] = None, ): @@ -281,16 +288,8 @@ def __init__( self._forward_auth_requirer_config = forward_auth_requirer_config events = self._charm.on[relation_name] - self.framework.observe(events.relation_created, self._on_relation_created_event) self.framework.observe(events.relation_changed, self._on_relation_changed_event) - self.framework.observe(events.relation_departed, self._on_relation_departed_event) - - def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: - """Update the relation with requirer data when a relation is created.""" - if not self.model.unit.is_leader(): - return - - self.update_requirer_relation_data(self._forward_auth_requirer_config, event.relation.id) + self.framework.observe(events.relation_broken, self._on_relation_broken_event) def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Get the forward-auth config and emit a custom config-changed event.""" @@ -299,7 +298,7 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: data = event.relation.data[event.app] if not data: - logger.info("No provider relation data available.") + logger.debug("No provider relation data available.") return try: @@ -322,8 +321,8 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: decisions_address, app_names, headers, relation_id, relation_app_name ) - def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: - """Notify the requirer that the relation has departed.""" + def _on_relation_broken_event(self, event: RelationBrokenEvent) -> None: + """Notify the requirer that the relation was broken.""" self.on.auth_config_removed.emit(event.relation.id) def update_requirer_relation_data( @@ -338,14 +337,16 @@ def update_requirer_relation_data( return if not isinstance(ingress_app_names, RequirerConfig): - raise ValueError(f"Unexpected type: {type(ingress_app_names)}") + raise TypeError(f"Unexpected type: {type(ingress_app_names)}") try: relation = self.model.get_relation( relation_name=self._relation_name, relation_id=relation_id ) except TooManyRelatedAppsError: - raise RuntimeError("More than one relation is defined. Please provide a relation_id") + raise TooManyRelatedAppsError( + "More than one relation is defined. Please provide a relation_id" + ) if not relation or not relation.app: return @@ -360,18 +361,20 @@ def get_provider_info(self, relation_id: Optional[int] = None) -> Optional[Forwa try: relation = self.model.get_relation(self._relation_name, relation_id=relation_id) except TooManyRelatedAppsError: - raise RuntimeError("More than one relation is defined. Please provide a relation_id") + raise TooManyRelatedAppsError( + "More than one relation is defined. Please provide a relation_id" + ) if not relation or not relation.app: return None data = relation.data[relation.app] if not data: - logger.info("No relation data available.") + logger.debug("No relation data available.") return data = _load_data(data, FORWARD_AUTH_PROVIDER_JSON_SCHEMA) forward_auth_config = ForwardAuthConfig.from_dict(data) - logger.info(f"ForwardAuthConfig: {forward_auth_config}") + logger.debug(f"ForwardAuthConfig: {forward_auth_config}") return forward_auth_config @@ -382,7 +385,9 @@ def get_remote_app_name(self, relation_id: Optional[int] = None) -> Optional[str try: relation = self.model.get_relation(self._relation_name, relation_id=relation_id) except TooManyRelatedAppsError: - raise RuntimeError("More than one relation is defined. Please provide a relation_id") + raise TooManyRelatedAppsError( + "More than one relation is defined. Please provide a relation_id" + ) if not relation or not relation.app: return None @@ -398,7 +403,9 @@ def is_ready(self, relation_id: Optional[int] = None) -> Optional[bool]: try: relation = self.model.get_relation(self._relation_name, relation_id=relation_id) except TooManyRelatedAppsError: - raise RuntimeError("More than one relation is defined. Please provide a relation_id") + raise TooManyRelatedAppsError( + "More than one relation is defined. Please provide a relation_id" + ) if not relation or not relation.app: return None @@ -526,7 +533,9 @@ def _compare_apps(self, relation_id: Optional[int] = None) -> None: try: relation = self.model.get_relation(self._relation_name, relation_id=relation_id) except TooManyRelatedAppsError: - raise RuntimeError("More than one relation is defined. Please provide a relation_id") + raise TooManyRelatedAppsError( + "More than one relation is defined. Please provide a relation_id" + ) if not relation or not relation.app: return None @@ -555,14 +564,16 @@ def _update_relation_data( return if not isinstance(forward_auth_config, ForwardAuthConfig): - raise ValueError(f"Unexpected forward_auth_config type: {type(forward_auth_config)}") + raise TypeError(f"Unexpected forward_auth_config type: {type(forward_auth_config)}") try: relation = self.model.get_relation( relation_name=self._relation_name, relation_id=relation_id ) except TooManyRelatedAppsError: - raise RuntimeError("More than one relation is defined. Please provide a relation_id") + raise TooManyRelatedAppsError( + "More than one relation is defined. Please provide a relation_id" + ) if not relation or not relation.app: return From 941e7eed75768be58996e14e2d127bac51c86717 Mon Sep 17 00:00:00 2001 From: natalia Date: Thu, 16 Nov 2023 16:10:52 +0100 Subject: [PATCH 07/17] refactor: address review comments --- lib/charms/oathkeeper/v0/forward_auth.py | 12 +++++++----- metadata.yaml | 5 +++-- src/charm.py | 6 +++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index 144fd8cc..f58845b6 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -191,7 +191,7 @@ def to_dict(self) -> Dict: @dataclass -class RequirerConfig: +class ForwardAuthRequirerConfig: """Helper class containing configuration required by Oathkeeper. Its purpose is to evaluate whether apps can be protected by IAP. @@ -279,13 +279,13 @@ def __init__( charm: CharmBase, *, relation_name: str = RELATION_NAME, - forward_auth_requirer_config: Optional[RequirerConfig] = None, + ingress_app_names: Optional[ForwardAuthRequirerConfig] = None, ): super().__init__(charm, relation_name) self._charm = charm self._relation_name = relation_name - self._forward_auth_requirer_config = forward_auth_requirer_config + self._ingress_app_names = ingress_app_names events = self._charm.on[relation_name] self.framework.observe(events.relation_changed, self._on_relation_changed_event) @@ -326,7 +326,9 @@ def _on_relation_broken_event(self, event: RelationBrokenEvent) -> None: self.on.auth_config_removed.emit(event.relation.id) def update_requirer_relation_data( - self, ingress_app_names: Optional[RequirerConfig], relation_id: Optional[int] = None + self, + ingress_app_names: Optional[ForwardAuthRequirerConfig], + relation_id: Optional[int] = None, ) -> None: """Update the relation databag with app names that can get IAP protection.""" if not self.model.unit.is_leader(): @@ -336,7 +338,7 @@ def update_requirer_relation_data( logger.error("Ingress-related app names are missing") return - if not isinstance(ingress_app_names, RequirerConfig): + if not isinstance(ingress_app_names, ForwardAuthRequirerConfig): raise TypeError(f"Unexpected type: {type(ingress_app_names)}") try: diff --git a/metadata.yaml b/metadata.yaml index 62db66b1..caeb8837 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -76,8 +76,9 @@ requires: interface: forward_auth limit: 1 description: | - Receives forwardAuth middleware config from Oathkeeper. - The same middleware will be applied to all charms that requested Identity and Access Proxy protection. + Receive config from e.g. oathkeeper, for rendering the forwardAuth middleware. + The same middleware is applied to all proxied endpoints that requested Identity and Access Proxy (IAP) protection. + For this reason we set a relation count limit of 1. This feature is experimental and may be unstable. In order to enable it, run `juju config enable_experimental_forward_auth=True`. logging: interface: loki_push_api diff --git a/src/charm.py b/src/charm.py index c70500e3..b2772624 100755 --- a/src/charm.py +++ b/src/charm.py @@ -32,7 +32,7 @@ AuthConfigChangedEvent, AuthConfigRemovedEvent, ForwardAuthRequirer, - RequirerConfig, + ForwardAuthRequirerConfig, ) from charms.observability_libs.v0.cert_handler import CertHandler from charms.observability_libs.v1.kubernetes_service_patch import ( @@ -266,7 +266,7 @@ def _service_ports(self) -> List[ServicePort]: ServicePort(int(port), name=name) for name, port in self._tcp_entrypoints().items() ] - def _forward_auth_config(self) -> RequirerConfig: + def _forward_auth_config(self) -> ForwardAuthRequirerConfig: ingress_app_names = [ rel.app.name # type: ignore for rel in itertools.chain( @@ -276,7 +276,7 @@ def _forward_auth_config(self) -> RequirerConfig: self.traefik_route.relations, ) ] - return RequirerConfig(ingress_app_names) + return ForwardAuthRequirerConfig(ingress_app_names) def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): if self.config["enable_experimental_forward_auth"]: From 7dbb67869fc6a6b96a277a8a820b2d44b95e01d4 Mon Sep 17 00:00:00 2001 From: natalia Date: Fri, 24 Nov 2023 17:45:23 +0100 Subject: [PATCH 08/17] test: add e2e tests for iap --- tests/integration/conftest.py | 10 +- tests/integration/test_forward_auth.py | 187 +++++++ .../testers/forward-auth/charmcraft.yaml | 17 + .../lib/charms/oathkeeper/v0/auth_proxy.py | 477 ++++++++++++++++++ .../testers/forward-auth/metadata.yaml | 22 + .../testers/forward-auth/requirements.txt | 2 + .../testers/forward-auth/src/charm.py | 82 +++ 7 files changed, 796 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_forward_auth.py create mode 100644 tests/integration/testers/forward-auth/charmcraft.yaml create mode 100644 tests/integration/testers/forward-auth/lib/charms/oathkeeper/v0/auth_proxy.py create mode 100644 tests/integration/testers/forward-auth/metadata.yaml create mode 100644 tests/integration/testers/forward-auth/requirements.txt create mode 100755 tests/integration/testers/forward-auth/src/charm.py diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 48e9f222..6f20d3f4 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -74,7 +74,7 @@ def copy_traefik_library_into_tester_charms(ops_test): "observability_libs/v1/kubernetes_service_patch.py", "traefik_route_k8s/v0/traefik_route.py", ] - for tester in ["ipa", "ipu", "tcp", "route"]: + for tester in ["forward-auth", "ipa", "ipu", "tcp", "route"]: for lib in libraries: install_path = f"tests/integration/testers/{tester}/lib/charms/{lib}" os.makedirs(os.path.dirname(install_path), exist_ok=True) @@ -97,6 +97,14 @@ async def traefik_charm(ops_test): raise +@pytest.fixture(scope="module") +@timed_memoizer +async def forward_auth_tester_charm(ops_test): + charm_path = (Path(__file__).parent / "testers" / "forward-auth").absolute() + charm = await ops_test.build_charm(charm_path, verbosity="debug") + return charm + + @pytest.fixture(scope="module") @timed_memoizer async def ipa_tester_charm(ops_test): diff --git a/tests/integration/test_forward_auth.py b/tests/integration/test_forward_auth.py new file mode 100644 index 00000000..720f988f --- /dev/null +++ b/tests/integration/test_forward_auth.py @@ -0,0 +1,187 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +import json +import logging +from os.path import join + +import pytest +import requests +import yaml +from lightkube import Client +from lightkube.resources.core_v1 import ConfigMap +from pytest_operator.plugin import OpsTest +from tenacity import retry, stop_after_attempt, wait_exponential + +from tests.integration.conftest import deploy_traefik_if_not_deployed + +logger = logging.getLogger(__name__) + +OATHKEEPER_CHARM = "oathkeeper" +TRAEFIK_CHARM = "traefik-k8s" +IAP_REQUIRER_CHARM = "iap-requirer" + + +@pytest.fixture(scope="module") +def lightkube_client(ops_test: OpsTest) -> Client: + client = Client(field_manager=OATHKEEPER_CHARM, namespace=ops_test.model.name) + return client + + +async def get_app_address(ops_test: OpsTest, app_name: str) -> str: + """Get address of an app.""" + status = await ops_test.model.get_status() # noqa: F821 + return status["applications"][app_name]["public-address"] + + +async def get_reverse_proxy_app_url( + ops_test: OpsTest, ingress_app_name: str, app_name: str +) -> str: + """Get the ingress address of an app.""" + address = await get_app_address(ops_test, ingress_app_name) + proxy_app_url = f"http://{address}/{ops_test.model.name}-{app_name}/" + logger.debug(f"Retrieved address: {proxy_app_url}") + return proxy_app_url + + +@pytest.mark.skip_if_deployed +@pytest.mark.abort_on_fail +async def test_deployment(ops_test: OpsTest, traefik_charm, forward_auth_tester_charm): + """Deploy the charms and integrations required to set up an Identity and Access Proxy.""" + await deploy_traefik_if_not_deployed(ops_test, traefik_charm) + + # Enable experimental-forward-auth + await ops_test.model.applications[TRAEFIK_CHARM].set_config( + {"enable_experimental_forward_auth": "True"} + ) + + # Deploy oathkeeper + await ops_test.model.deploy( + OATHKEEPER_CHARM, + channel="latest/edge", + trust=True, + ) + + # Deploy the iap-requirer charm with integrations + await ops_test.model.deploy( + application_name=IAP_REQUIRER_CHARM, + entity_url=forward_auth_tester_charm, + resources={"oci-image": "kennethreitz/httpbin"}, + trust=True, + ) + + await ops_test.model.integrate(f"{IAP_REQUIRER_CHARM}:ingress", TRAEFIK_CHARM) + await ops_test.model.integrate(f"{IAP_REQUIRER_CHARM}:auth-proxy", OATHKEEPER_CHARM) + + await ops_test.model.integrate(f"{TRAEFIK_CHARM}:experimental-forward-auth", OATHKEEPER_CHARM) + + await ops_test.model.wait_for_idle( + [TRAEFIK_CHARM, OATHKEEPER_CHARM, IAP_REQUIRER_CHARM], status="active", timeout=1000 + ) + + +@retry( + wait=wait_exponential(multiplier=3, min=1, max=10), + stop=stop_after_attempt(5), + reraise=True, +) +async def test_allowed_forward_auth_url_redirect(ops_test: OpsTest) -> None: + """Test that a request hitting an application protected by IAP is forwarded by traefik to oathkeeper. + + An allowed request should be performed without authentication. + """ + requirer_url = await get_reverse_proxy_app_url(ops_test, TRAEFIK_CHARM, IAP_REQUIRER_CHARM) + + protected_url = join(requirer_url, "anything/allowed") + + resp = requests.get(protected_url, verify=False) + assert resp.status_code == 200 + + +async def test_protected_forward_auth_url_redirect(ops_test: OpsTest) -> None: + """Test that when trying to reach a protected url, the request is forwarded by traefik to oathkeeper. + + An unauthenticated request should then be denied with 401 Unauthorized response. + """ + requirer_url = await get_reverse_proxy_app_url(ops_test, TRAEFIK_CHARM, IAP_REQUIRER_CHARM) + + protected_url = join(requirer_url, "anything/deny") + + resp = requests.get(protected_url, verify=False) + assert resp.status_code == 401 + + +async def test_forward_auth_url_response_headers( + ops_test: OpsTest, lightkube_client: Client +) -> None: + """Test that a response mutated by oathkeeper contains expected custom headers.""" + requirer_url = await get_reverse_proxy_app_url(ops_test, TRAEFIK_CHARM, IAP_REQUIRER_CHARM) + protected_url = join(requirer_url, "anything/anonymous") + + # Push an anonymous access rule as a workaround to avoid deploying identity-platform bundle + anonymous_rule = [ + { + "id": "iap-requirer:anonymous", + "match": { + "url": protected_url, + "methods": ["GET", "POST", "OPTION", "PUT", "PATCH", "DELETE"], + }, + "authenticators": [{"handler": "anonymous"}], + "mutators": [{"handler": "header"}], + "authorizer": {"handler": "allow"}, + "errors": [{"handler": "json"}], + } + ] + + update_access_rules_configmap(ops_test, lightkube_client, rule=anonymous_rule) + update_config_configmap(ops_test, lightkube_client) + + assert_anonymous_response(protected_url) + + +@retry( + wait=wait_exponential(multiplier=3, min=1, max=10), + stop=stop_after_attempt(10), + reraise=True, +) +def assert_anonymous_response(url): + resp = requests.get(url, verify=False) + assert resp.status_code == 200 + + headers = json.loads(resp.content).get("headers") + assert headers["X-User"] == "anonymous" + + +@retry( + wait=wait_exponential(multiplier=3, min=1, max=10), + stop=stop_after_attempt(5), + reraise=True, +) +def update_access_rules_configmap(ops_test: OpsTest, lightkube_client: Client, rule): + cm = lightkube_client.get(ConfigMap, "access-rules", namespace=ops_test.model.name) + data = {"access-rules-iap-requirer-anonymous.json": str(rule)} + cm.data = data + lightkube_client.replace(cm) + + +@retry( + wait=wait_exponential(multiplier=3, min=1, max=10), + stop=stop_after_attempt(5), + reraise=True, +) +def update_config_configmap(ops_test: OpsTest, lightkube_client: Client): + cm = lightkube_client.get(ConfigMap, name="oathkeeper-config", namespace=ops_test.model.name) + cm = yaml.safe_load(cm.data["oathkeeper.yaml"]) + cm["access_rules"]["repositories"] = [ + "file://etc/config/access-rules/access-rules-iap-requirer-anonymous.json" + ] + patch = {"data": {"oathkeeper.yaml": yaml.dump(cm)}} + lightkube_client.patch( + ConfigMap, name="oathkeeper-config", namespace=ops_test.model.name, obj=patch + ) + + +async def test_remove_forward_auth_integration(ops_test: OpsTest): + await ops_test.juju("remove-relation", "oathkeeper", "traefik-k8s:experimental-forward-auth") + await ops_test.model.wait_for_idle( + [TRAEFIK_CHARM, OATHKEEPER_CHARM, IAP_REQUIRER_CHARM], status="active" + ) diff --git a/tests/integration/testers/forward-auth/charmcraft.yaml b/tests/integration/testers/forward-auth/charmcraft.yaml new file mode 100644 index 00000000..ddf3772a --- /dev/null +++ b/tests/integration/testers/forward-auth/charmcraft.yaml @@ -0,0 +1,17 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +type: charm +bases: + - build-on: + - name: "ubuntu" + channel: "20.04" + run-on: + - name: "ubuntu" + channel: "20.04" +parts: + charm: + charm-binary-python-packages: + - jsonschema + - ops + build-packages: + - git diff --git a/tests/integration/testers/forward-auth/lib/charms/oathkeeper/v0/auth_proxy.py b/tests/integration/testers/forward-auth/lib/charms/oathkeeper/v0/auth_proxy.py new file mode 100644 index 00000000..c4a9f2b2 --- /dev/null +++ b/tests/integration/testers/forward-auth/lib/charms/oathkeeper/v0/auth_proxy.py @@ -0,0 +1,477 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Interface library for providing Oathkeeper with downstream charms' auth-proxy information. + +It is required to integrate a charm into an Identity and Access Proxy (IAP). + +## Getting Started + +To get started using the library, you need to fetch the library using `charmcraft`. +**Note that you also need to add `jsonschema` to your charm's `requirements.txt`.** + +```shell +cd some-charm +charmcraft fetch-lib charms.oathkeeper.v0.auth_proxy +``` + +To use the library from the requirer side, add the following to the `metadata.yaml` of the charm: + +```yaml +requires: + auth-proxy: + interface: auth_proxy + limit: 1 +``` + +Then, to initialise the library: +```python +from charms.oathkeeper.v0.auth_proxy import AuthProxyConfig, AuthProxyRequirer + +AUTH_PROXY_ALLOWED_ENDPOINTS = ["welcome", "about/app"] +AUTH_PROXY_HEADERS = ["X-User", "X-Some-Header"] + +class SomeCharm(CharmBase): + def __init__(self, *args): + # ... + self.auth_proxy = AuthProxyRequirer(self, self._auth_proxy_config) + + @property + def external_urls(self) -> list: + # Get ingress-per-unit or externally-configured web urls + # ... + return ["https://example.com/unit-0", "https://example.com/unit-1"] + + @property + def _auth_proxy_config(self) -> AuthProxyConfig: + return AuthProxyConfig( + protected_urls=self.external_urls, + allowed_endpoints=AUTH_PROXY_ALLOWED_ENDPOINTS, + headers=AUTH_PROXY_HEADERS + ) + + def _on_ingress_ready(self, event): + self._configure_auth_proxy() + + def _configure_auth_proxy(self): + self.auth_proxy.update_auth_proxy_config(auth_proxy_config=self._auth_proxy_config) +``` +""" + +import json +import logging +import re +from dataclasses import asdict, dataclass, field +from typing import Dict, List, Mapping, Optional + +import jsonschema +from ops.charm import CharmBase, RelationChangedEvent, RelationCreatedEvent, RelationDepartedEvent +from ops.framework import EventBase, EventSource, Handle, Object, ObjectEvents +from ops.model import Relation, TooManyRelatedAppsError + +# The unique Charmhub library identifier, never change it +LIBID = "0e67a205d1c14d7a86d89f099d19c541" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 2 + +RELATION_NAME = "auth-proxy" +INTERFACE_NAME = "auth_proxy" + +logger = logging.getLogger(__name__) + +ALLOWED_HEADERS = ["X-User"] + +url_regex = re.compile( + r"(^http://)|(^https://)" # http:// or https:// + r"(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|" + r"[A-Z0-9-]{2,}\.?)|" # domain... + r"\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})" # ...or ip + r"(?::\d+)?" # optional port + r"(?:/?|[/?]\S+)$", + re.IGNORECASE, +) + +AUTH_PROXY_REQUIRER_JSON_SCHEMA = { + "$schema": "http://json-schema.org/draft-07/schema", + "$id": "https://canonical.github.io/charm-relation-interfaces/docs/json_schemas/auth_proxy/v0/requirer.json", + "type": "object", + "properties": { + "protected_urls": {"type": "array", "default": None, "items": {"type": "string"}}, + "allowed_endpoints": {"type": "array", "default": [], "items": {"type": "string"}}, + "headers": { + "type": "array", + "default": ["X-User"], + "items": { + "enum": ALLOWED_HEADERS, + "type": "string", + }, + }, + }, + "required": ["protected_urls", "allowed_endpoints", "headers"], +} + + +class AuthProxyConfigError(Exception): + """Emitted when invalid auth proxy config is provided.""" + + +class DataValidationError(RuntimeError): + """Raised when data validation fails on relation data.""" + + +def _load_data(data: Mapping, schema: Optional[Dict] = None) -> Dict: + """Parses nested fields and checks whether `data` matches `schema`.""" + ret = {} + for k, v in data.items(): + try: + ret[k] = json.loads(v) + except json.JSONDecodeError: + ret[k] = v + + if schema: + _validate_data(ret, schema) + return ret + + +def _dump_data(data: Dict, schema: Optional[Dict] = None) -> Dict: + if schema: + _validate_data(data, schema) + + ret = {} + for k, v in data.items(): + if isinstance(v, (list, dict)): + try: + ret[k] = json.dumps(v) + except json.JSONDecodeError as e: + raise DataValidationError(f"Failed to encode relation json: {e}") + else: + ret[k] = v + return ret + + +class AuthProxyRelation(Object): + """A class containing helper methods for auth-proxy relation.""" + + def _pop_relation_data(self, relation_id: Relation) -> None: + if not self.model.unit.is_leader(): + return + + if not self._charm.model.relations[self._relation_name]: + return + + relation = self.model.get_relation(self._relation_name, relation_id=relation_id) + if not relation or not relation.app: + return + + try: + for data in list(relation.data[self.model.app]): + relation.data[self.model.app].pop(data, "") + except Exception as e: + logger.info(f"Failed to pop the relation data: {e}") + + +def _validate_data(data: Dict, schema: Dict) -> None: + """Checks whether `data` matches `schema`. + + Will raise DataValidationError if the data is not valid, else return None. + """ + try: + jsonschema.validate(instance=data, schema=schema) + except jsonschema.ValidationError as e: + raise DataValidationError(data, schema) from e + + +@dataclass +class AuthProxyConfig: + """Helper class containing a configuration for the charm related with Oathkeeper.""" + + protected_urls: List[str] + headers: List[str] + allowed_endpoints: List[str] = field(default_factory=lambda: []) + + def validate(self) -> None: + """Validate the auth proxy configuration.""" + # Validate protected_urls + for url in self.protected_urls: + if not re.match(url_regex, url): + raise AuthProxyConfigError(f"Invalid URL {url}") + + for url in self.protected_urls: + if url.startswith("http://"): + logger.warning( + f"Provided URL {url} uses http scheme. In order to make the Identity Platform work with the Proxy, run kratos in dev mode: `juju config kratos dev=True`. Don't do this in production" + ) + + # Validate headers + for header in self.headers: + if header not in ALLOWED_HEADERS: + raise AuthProxyConfigError( + f"Unsupported header {header}, it must be one of {ALLOWED_HEADERS}" + ) + + def to_dict(self) -> Dict: + """Convert object to dict.""" + return {k: v for k, v in asdict(self).items() if v is not None} + + +class AuthProxyConfigChangedEvent(EventBase): + """Event to notify the Provider charm that the auth proxy config has changed.""" + + def __init__( + self, + handle: Handle, + protected_urls: List[str], + headers: List[str], + allowed_endpoints: List[str], + relation_id: int, + relation_app_name: str, + ) -> None: + super().__init__(handle) + self.protected_urls = protected_urls + self.allowed_endpoints = allowed_endpoints + self.headers = headers + self.relation_id = relation_id + self.relation_app_name = relation_app_name + + def snapshot(self) -> Dict: + """Save event.""" + return { + "protected_urls": self.protected_urls, + "headers": self.headers, + "allowed_endpoints": self.allowed_endpoints, + "relation_id": self.relation_id, + "relation_app_name": self.relation_app_name, + } + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.protected_urls = snapshot["protected_urls"] + self.headers = snapshot["headers"] + self.allowed_endpoints = snapshot["allowed_endpoints"] + self.relation_id = snapshot["relation_id"] + self.relation_app_name = snapshot["relation_app_name"] + + def to_auth_proxy_config(self) -> AuthProxyConfig: + """Convert the event information to an AuthProxyConfig object.""" + return AuthProxyConfig( + self.protected_urls, + self.allowed_endpoints, + self.headers, + ) + + +class AuthProxyConfigRemovedEvent(EventBase): + """Event to notify the provider charm that the auth proxy config was removed.""" + + def __init__( + self, + handle: Handle, + relation_id: int, + ) -> None: + super().__init__(handle) + self.relation_id = relation_id + + def snapshot(self) -> Dict: + """Save event.""" + return {"relation_id": self.relation_id} + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.relation_id = snapshot["relation_id"] + + +class AuthProxyProviderEvents(ObjectEvents): + """Event descriptor for events raised by `AuthProxyProvider`.""" + + proxy_config_changed = EventSource(AuthProxyConfigChangedEvent) + config_removed = EventSource(AuthProxyConfigRemovedEvent) + + +class AuthProxyProvider(AuthProxyRelation): + """Provider side of the auth-proxy relation.""" + + on = AuthProxyProviderEvents() + + def __init__(self, charm: CharmBase, relation_name: str = RELATION_NAME): + super().__init__(charm, relation_name) + + self._charm = charm + self._relation_name = relation_name + + events = self._charm.on[relation_name] + self.framework.observe(events.relation_changed, self._on_relation_changed_event) + self.framework.observe(events.relation_departed, self._on_relation_departed_event) + + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: + """Get the auth-proxy config and emit a custom config-changed event.""" + if not self.model.unit.is_leader(): + return + + data = event.relation.data[event.app] + if not data: + logger.info("No requirer relation data available.") + return + + try: + auth_proxy_data = _load_data(data, AUTH_PROXY_REQUIRER_JSON_SCHEMA) + except DataValidationError as e: + logger.error( + f"Received invalid config from the requirer: {e}. Config-changed will not be emitted" + ) + return + + protected_urls = auth_proxy_data.get("protected_urls") + allowed_endpoints = auth_proxy_data.get("allowed_endpoints") + headers = auth_proxy_data.get("headers") + + relation_id = event.relation.id + relation_app_name = event.relation.app.name + + # Notify Oathkeeper to create access rules + self.on.proxy_config_changed.emit( + protected_urls, headers, allowed_endpoints, relation_id, relation_app_name + ) + + def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: + """Notify Oathkeeper that the relation has departed.""" + self.on.config_removed.emit(event.relation.id) + + def get_headers(self) -> List[str]: + """Returns the list of headers from all relations.""" + if not self._charm.model.relations[self._relation_name]: + return [] + + headers = set() + for relation in self._charm.model.relations[self._relation_name]: + if relation.data[relation.app]: + for header in json.loads(relation.data[relation.app]["headers"]): + headers.add(header) + + return list(headers) + + def get_app_names(self) -> List[str]: + """Returns the list of all related app names.""" + if not self._charm.model.relations[self._relation_name]: + return [] + + app_names = list() + for relation in self._charm.model.relations[self._relation_name]: + app_names.append(relation.app.name) + + return app_names + + +class InvalidAuthProxyConfigEvent(EventBase): + """Event to notify the charm that the auth proxy configuration is invalid.""" + + def __init__(self, handle: Handle, error: str): + super().__init__(handle) + self.error = error + + def snapshot(self) -> Dict: + """Save event.""" + return { + "error": self.error, + } + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + self.error = snapshot["error"] + + +class AuthProxyRelationRemovedEvent(EventBase): + """Custom event to notify the charm that the relation was removed.""" + + def snapshot(self) -> Dict: + """Save event.""" + return {} + + def restore(self, snapshot: Dict) -> None: + """Restore event.""" + pass + + +class AuthProxyRequirerEvents(ObjectEvents): + """Event descriptor for events raised by `AuthProxyRequirer`.""" + + invalid_auth_proxy_config = EventSource(InvalidAuthProxyConfigEvent) + auth_proxy_relation_removed = EventSource(AuthProxyRelationRemovedEvent) + + +class AuthProxyRequirer(AuthProxyRelation): + """Requirer side of the auth-proxy relation.""" + + on = AuthProxyRequirerEvents() + + def __init__( + self, + charm: CharmBase, + auth_proxy_config: Optional[AuthProxyConfig] = None, + relation_name: str = RELATION_NAME, + ) -> None: + super().__init__(charm, relation_name) + self.charm = charm + self._relation_name = relation_name + self._auth_proxy_config = auth_proxy_config + + events = self.charm.on[relation_name] + self.framework.observe(events.relation_created, self._on_relation_created_event) + self.framework.observe(events.relation_departed, self._on_relation_departed_event) + + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Update the relation with auth proxy config when a relation is created.""" + if not self.model.unit.is_leader(): + return + + try: + self._update_relation_data(self._auth_proxy_config, event.relation.id) + except AuthProxyConfigError as e: + self.on.invalid_auth_proxy_config.emit(e.args[0]) + + def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: + """Wipe the relation databag and notify the charm when the relation has departed.""" + # Workaround for https://github.com/canonical/operator/issues/888 + self._pop_relation_data(event.relation.id) + + self.on.auth_proxy_relation_removed.emit() + + def _update_relation_data( + self, auth_proxy_config: Optional[AuthProxyConfig], relation_id: Optional[int] = None + ) -> None: + """Validate the auth-proxy config and update the relation databag.""" + if not self.model.unit.is_leader(): + return + + if not auth_proxy_config: + logger.info("Auth proxy config is missing") + return + + if not isinstance(auth_proxy_config, AuthProxyConfig): + raise ValueError(f"Unexpected auth_proxy_config type: {type(auth_proxy_config)}") + + auth_proxy_config.validate() + + try: + relation = self.model.get_relation( + relation_name=self._relation_name, relation_id=relation_id + ) + except TooManyRelatedAppsError: + raise RuntimeError("More than one relations are defined. Please provide a relation_id") + + if not relation or not relation.app: + return + + data = _dump_data(auth_proxy_config.to_dict(), AUTH_PROXY_REQUIRER_JSON_SCHEMA) + relation.data[self.model.app].update(data) + + def update_auth_proxy_config( + self, auth_proxy_config: AuthProxyConfig, relation_id: Optional[int] = None + ) -> None: + """Update the auth proxy config stored in the object.""" + self._update_relation_data(auth_proxy_config, relation_id=relation_id) diff --git a/tests/integration/testers/forward-auth/metadata.yaml b/tests/integration/testers/forward-auth/metadata.yaml new file mode 100644 index 00000000..d496fd11 --- /dev/null +++ b/tests/integration/testers/forward-auth/metadata.yaml @@ -0,0 +1,22 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +name: iap-requirer +display-name: iap-requirer +description: Identity and Access Proxy Tester +summary: IAP Tester +assumes: + - k8s-api +containers: + httpbin: + resource: oci-image +resources: + oci-image: + type: oci-image + description: OCI image for IAP Tester container + upstream-source: kennethreitz/httpbin +requires: + auth-proxy: + interface: auth_proxy + limit: 1 + ingress: + interface: ingress diff --git a/tests/integration/testers/forward-auth/requirements.txt b/tests/integration/testers/forward-auth/requirements.txt new file mode 100644 index 00000000..a9ad1ee1 --- /dev/null +++ b/tests/integration/testers/forward-auth/requirements.txt @@ -0,0 +1,2 @@ +jsonschema +ops diff --git a/tests/integration/testers/forward-auth/src/charm.py b/tests/integration/testers/forward-auth/src/charm.py new file mode 100755 index 00000000..cfa5074d --- /dev/null +++ b/tests/integration/testers/forward-auth/src/charm.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +import logging + +from charms.oathkeeper.v0.auth_proxy import AuthProxyConfig, AuthProxyRequirer +from charms.traefik_k8s.v2.ingress import IngressPerAppRequirer +from ops.charm import CharmBase +from ops.main import main +from ops.model import ActiveStatus +from ops.pebble import Layer + +AUTH_PROXY_ALLOWED_ENDPOINTS = ["anything/allowed"] +AUTH_PROXY_HEADERS = ["X-User"] +HTTPBIN_PORT = 80 + +logger = logging.getLogger(__name__) + + +class IAPRequirerMock(CharmBase): + def __init__(self, framework): + super().__init__(framework) + self._container = self.unit.get_container("httpbin") + self._service_name = "httpbin" + + self.ingress = IngressPerAppRequirer( + self, + host=f"{self.app.name}.{self.model.name}.svc.cluster.local", + relation_name="ingress", + port=HTTPBIN_PORT, + strip_prefix=True, + ) + + self.auth_proxy_relation_name = "auth-proxy" + self.auth_proxy = AuthProxyRequirer( + self, self._auth_proxy_config, self.auth_proxy_relation_name + ) + + self.framework.observe(self.on.httpbin_pebble_ready, self._on_httpbin_pebble_ready) + self.framework.observe(self.ingress.on.ready, self._on_ingress_ready) + + @property + def _auth_proxy_config(self) -> AuthProxyConfig: + return AuthProxyConfig( + protected_urls=[ + self.ingress.url if self.ingress.url is not None else "https://some-test-url.com" + ], + headers=AUTH_PROXY_HEADERS, + allowed_endpoints=AUTH_PROXY_ALLOWED_ENDPOINTS, + ) + + @property + def _httpbin_pebble_layer(self): + layer_config = { + "summary": "iap-requirer-mock layer", + "description": "pebble config layer for iap-requirer-mock", + "services": { + self._service_name: { + "override": "replace", + "summary": "httpbin layer", + "command": "gunicorn -b 0.0.0.0:80 httpbin:app -k gevent", + "startup": "enabled", + } + }, + } + return Layer(layer_config) + + def _on_httpbin_pebble_ready(self, event): + self._container.add_layer("httpbin", self._httpbin_pebble_layer, combine=True) + self._container.replan() + self.unit.open_port(protocol="tcp", port=HTTPBIN_PORT) + + self.unit.status = ActiveStatus() + + def _on_ingress_ready(self, event): + if self.unit.is_leader(): + logger.info(f"This app's ingress URL: {event.url}") + self.auth_proxy.update_auth_proxy_config(auth_proxy_config=self._auth_proxy_config) + + +if __name__ == "__main__": + main(IAPRequirerMock) From 055a92d966afb6252a971d3ece93f13765c26a4c Mon Sep 17 00:00:00 2001 From: natalia Date: Mon, 27 Nov 2023 10:24:22 +0100 Subject: [PATCH 09/17] test: increase tenacity retries --- tests/integration/test_forward_auth.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_forward_auth.py b/tests/integration/test_forward_auth.py index 720f988f..e4f2fd25 100644 --- a/tests/integration/test_forward_auth.py +++ b/tests/integration/test_forward_auth.py @@ -80,8 +80,8 @@ async def test_deployment(ops_test: OpsTest, traefik_charm, forward_auth_tester_ @retry( - wait=wait_exponential(multiplier=3, min=1, max=10), - stop=stop_after_attempt(5), + wait=wait_exponential(multiplier=3, min=1, max=20), + stop=stop_after_attempt(20), reraise=True, ) async def test_allowed_forward_auth_url_redirect(ops_test: OpsTest) -> None: @@ -110,9 +110,7 @@ async def test_protected_forward_auth_url_redirect(ops_test: OpsTest) -> None: assert resp.status_code == 401 -async def test_forward_auth_url_response_headers( - ops_test: OpsTest, lightkube_client: Client -) -> None: +async def test_forward_auth_url_response_headers(ops_test: OpsTest, lightkube_client: Client) -> None: """Test that a response mutated by oathkeeper contains expected custom headers.""" requirer_url = await get_reverse_proxy_app_url(ops_test, TRAEFIK_CHARM, IAP_REQUIRER_CHARM) protected_url = join(requirer_url, "anything/anonymous") @@ -139,8 +137,8 @@ async def test_forward_auth_url_response_headers( @retry( - wait=wait_exponential(multiplier=3, min=1, max=10), - stop=stop_after_attempt(10), + wait=wait_exponential(multiplier=3, min=1, max=20), + stop=stop_after_attempt(20), reraise=True, ) def assert_anonymous_response(url): From a805c8b23081a05403f7ccaa944304eae6e30064 Mon Sep 17 00:00:00 2001 From: natalia Date: Mon, 27 Nov 2023 10:29:07 +0100 Subject: [PATCH 10/17] style: fix linting --- tests/integration/test_forward_auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_forward_auth.py b/tests/integration/test_forward_auth.py index e4f2fd25..3c3d0a38 100644 --- a/tests/integration/test_forward_auth.py +++ b/tests/integration/test_forward_auth.py @@ -110,7 +110,9 @@ async def test_protected_forward_auth_url_redirect(ops_test: OpsTest) -> None: assert resp.status_code == 401 -async def test_forward_auth_url_response_headers(ops_test: OpsTest, lightkube_client: Client) -> None: +async def test_forward_auth_url_response_headers( + ops_test: OpsTest, lightkube_client: Client +) -> None: """Test that a response mutated by oathkeeper contains expected custom headers.""" requirer_url = await get_reverse_proxy_app_url(ops_test, TRAEFIK_CHARM, IAP_REQUIRER_CHARM) protected_url = join(requirer_url, "anything/anonymous") From 0727d2cca44b3cda2348e3a95a23f9dc11efc77e Mon Sep 17 00:00:00 2001 From: natalia Date: Mon, 27 Nov 2023 11:58:24 +0100 Subject: [PATCH 11/17] test: use http for in-cluster calls to oathkeeper --- tests/integration/test_forward_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_forward_auth.py b/tests/integration/test_forward_auth.py index 3c3d0a38..29e568d4 100644 --- a/tests/integration/test_forward_auth.py +++ b/tests/integration/test_forward_auth.py @@ -58,6 +58,7 @@ async def test_deployment(ops_test: OpsTest, traefik_charm, forward_auth_tester_ await ops_test.model.deploy( OATHKEEPER_CHARM, channel="latest/edge", + config={"dev": "True"}, trust=True, ) From d8c52329b75cdad01d8efbd4b83ccecc65e63e66 Mon Sep 17 00:00:00 2001 From: natalia Date: Mon, 27 Nov 2023 17:42:17 +0100 Subject: [PATCH 12/17] refactor: address review comments --- lib/charms/oathkeeper/v0/forward_auth.py | 8 ++++++++ src/charm.py | 10 +++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index f58845b6..237d014e 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -417,6 +417,14 @@ def is_ready(self, relation_id: Optional[int] = None) -> Optional[bool]: and "app_names" in relation.data[relation.app] ) + def is_protected_app(self, app: str) -> bool: + if self.is_ready(): + forward_auth_config = self.get_provider_info() + if forward_auth_config and app in forward_auth_config.app_names: + return True + return False + + return False class ForwardAuthProxySet(EventBase): """Event to notify the charm that the proxy was set successfully.""" diff --git a/src/charm.py b/src/charm.py index b2772624..8278d747 100755 --- a/src/charm.py +++ b/src/charm.py @@ -280,7 +280,6 @@ def _forward_auth_config(self) -> ForwardAuthRequirerConfig: def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): if self.config["enable_experimental_forward_auth"]: - self.forward_auth.update_requirer_relation_data(self._forward_auth_config) if self.forward_auth.is_ready(): self._process_status_and_configurations() else: @@ -543,6 +542,9 @@ def _update_ingress_configurations(self): errors = False + if self.config["enable_experimental_forward_auth"]: + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) + for ingress_relation in ( self.ingress_per_appv1.relations + self.ingress_per_appv2.relations @@ -596,9 +598,6 @@ def _handle_ingress_data_provided(self, event: RelationEvent): # update-status. self._process_status_and_configurations() - if self.forward_auth.is_ready(): - self.forward_auth.update_requirer_relation_data(self._forward_auth_config) - if isinstance(self.unit.status, MaintenanceStatus): self.unit.status = ActiveStatus() @@ -608,9 +607,6 @@ def _handle_ingress_data_removed(self, event: RelationEvent): event.relation, wipe_rel_data=not isinstance(event, RelationBrokenEvent) ) - if self.forward_auth.is_ready(): - self.forward_auth.update_requirer_relation_data(self._forward_auth_config) - # FIXME? on relation broken, data is still there so cannot simply call # self._process_status_and_configurations(). For this reason, the static config in # traefik.STATIC_CONFIG_PATH will be updated only on update-status. From 4d094f1d2a848adaab2b8863948d1156e7839de1 Mon Sep 17 00:00:00 2001 From: natalia Date: Tue, 28 Nov 2023 17:26:34 +0100 Subject: [PATCH 13/17] refactor: process configs if forward-auth flag changed --- src/charm.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 8278d747..f6164f98 100755 --- a/src/charm.py +++ b/src/charm.py @@ -116,6 +116,7 @@ def __init__(self, *args): self._stored.set_default( current_external_host=None, current_routing_mode=None, + current_forward_auth_mode=self.config["enable_experimental_forward_auth"], ) self.container = self.unit.get_container(_TRAEFIK_CONTAINER_NAME) @@ -474,9 +475,7 @@ def _on_config_changed(self, _: ConfigChangedEvent): # reconsider all data sent over the relations and all configs new_external_host = self._external_host new_routing_mode = self.config["routing_mode"] - - if self.config["enable_experimental_forward_auth"]: - self.forward_auth.update_requirer_relation_data(self._forward_auth_config) + new_forward_auth_mode = self.config["enable_experimental_forward_auth"] # TODO set BlockedStatus here when compound_status is introduced # https://github.com/canonical/operator/issues/665 @@ -484,9 +483,11 @@ def _on_config_changed(self, _: ConfigChangedEvent): 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._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 self._process_status_and_configurations() def _process_status_and_configurations(self): From c17df225f26de11214f7e5774c9676e89d7dccda Mon Sep 17 00:00:00 2001 From: natalia Date: Tue, 28 Nov 2023 17:36:11 +0100 Subject: [PATCH 14/17] style: fix lint in forward-auth lib --- lib/charms/oathkeeper/v0/forward_auth.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index 237d014e..1aa4d0a8 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -418,6 +418,7 @@ def is_ready(self, relation_id: Optional[int] = None) -> Optional[bool]: ) def is_protected_app(self, app: str) -> bool: + """Checks whether a given app requested to be protected by IAP.""" if self.is_ready(): forward_auth_config = self.get_provider_info() if forward_auth_config and app in forward_auth_config.app_names: @@ -426,6 +427,7 @@ def is_protected_app(self, app: str) -> bool: return False + class ForwardAuthProxySet(EventBase): """Event to notify the charm that the proxy was set successfully.""" From f061b1cd213e36db82744e78244ef8c5afe4b55e Mon Sep 17 00:00:00 2001 From: natalia Date: Wed, 29 Nov 2023 14:54:21 +0100 Subject: [PATCH 15/17] refactor: adjust to externalized traefik config mgmt --- lib/charms/oathkeeper/v0/forward_auth.py | 4 +-- src/charm.py | 25 +++++++++++---- src/traefik.py | 40 +++++++++++++++++++++++- 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index 1aa4d0a8..e0954a64 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -372,7 +372,7 @@ def get_provider_info(self, relation_id: Optional[int] = None) -> Optional[Forwa data = relation.data[relation.app] if not data: logger.debug("No relation data available.") - return + return None data = _load_data(data, FORWARD_AUTH_PROVIDER_JSON_SCHEMA) forward_auth_config = ForwardAuthConfig.from_dict(data) @@ -417,7 +417,7 @@ def is_ready(self, relation_id: Optional[int] = None) -> Optional[bool]: and "app_names" in relation.data[relation.app] ) - def is_protected_app(self, app: str) -> bool: + def is_protected_app(self, app: Optional[str]) -> bool: """Checks whether a given app requested to be protected by IAP.""" if self.is_ready(): forward_auth_config = self.get_provider_info() diff --git a/src/charm.py b/src/charm.py index f6164f98..6616416f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -6,13 +6,10 @@ import contextlib import enum import functools -import ipaddress import itertools import json import logging import socket -import typing -from string import Template from typing import Any, Dict, List, Optional, Union from urllib.parse import urlparse @@ -267,6 +264,7 @@ def _service_ports(self) -> List[ServicePort]: ServicePort(int(port), name=name) for name, port in self._tcp_entrypoints().items() ] + @property def _forward_auth_config(self) -> ForwardAuthRequirerConfig: ingress_app_names = [ rel.app.name # type: ignore @@ -279,8 +277,14 @@ def _forward_auth_config(self) -> ForwardAuthRequirerConfig: ] return ForwardAuthRequirerConfig(ingress_app_names) - def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): + @property + def _is_forward_auth_enabled(self) -> bool: if self.config["enable_experimental_forward_auth"]: + return True + return False + + def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): + if self._is_forward_auth_enabled: if self.forward_auth.is_ready(): self._process_status_and_configurations() else: @@ -475,7 +479,7 @@ def _on_config_changed(self, _: ConfigChangedEvent): # 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.config["enable_experimental_forward_auth"] + new_forward_auth_mode = self._is_forward_auth_enabled # TODO set BlockedStatus here when compound_status is introduced # https://github.com/canonical/operator/issues/665 @@ -543,7 +547,7 @@ def _update_ingress_configurations(self): errors = False - if self.config["enable_experimental_forward_auth"]: + if self._is_forward_auth_enabled: self.forward_auth.update_requirer_relation_data(self._forward_auth_config) for ingress_relation in ( @@ -736,6 +740,9 @@ def _get_configs_per_leader(self, relation: Relation) -> Dict[str, Any]: redirect_https=data.get("redirect-https", False), strip_prefix=data.get("strip-prefix", False), external_host=self.external_host, + enable_experimental_forward_auth=self._is_forward_auth_enabled, + forward_auth_app=self.forward_auth.is_protected_app(app=data.get("name")), + forward_auth_config=self.forward_auth.get_provider_info(), ) if self.unit.is_leader(): @@ -767,6 +774,9 @@ def _get_configs_per_app(self, relation: Relation) -> Dict[str, Any]: port=data.app.port, external_host=self.external_host, hosts=[udata.host for udata in data.units], + enable_experimental_forward_auth=self._is_forward_auth_enabled, + forward_auth_app=self.forward_auth.is_protected_app(app=data.app.name), + forward_auth_config=self.forward_auth.get_provider_info(), ) if self.unit.is_leader(): @@ -818,6 +828,9 @@ def _get_configs_per_unit(self, relation: Relation) -> Dict[str, Any]: strip_prefix=data.get("strip-prefix"), redirect_https=data.get("redirect-https"), external_host=self.external_host, + enable_experimental_forward_auth=self._is_forward_auth_enabled, + forward_auth_app=self.forward_auth.is_protected_app(app=data.get("name")), + forward_auth_config=self.forward_auth.get_provider_info(), ) if self.unit.is_leader(): diff --git a/src/traefik.py b/src/traefik.py index baa50064..ce5496ba 100644 --- a/src/traefik.py +++ b/src/traefik.py @@ -14,6 +14,7 @@ from typing import Any, Dict, Iterable, List, Optional, Union, cast import yaml +from charms.oathkeeper.v0.forward_auth import ForwardAuthConfig from ops import Container from ops.pebble import LayerDict, PathError from utils import is_hostname @@ -264,6 +265,9 @@ def get_per_unit_http_config( strip_prefix: Optional[bool], redirect_https: Optional[bool], external_host: str, + enable_experimental_forward_auth: bool, + forward_auth_app: bool, + forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: """Generate a config dict for IngressPerUnit.""" lb_servers = [{"url": f"{scheme or 'http'}://{host}:{port}"}] @@ -274,6 +278,9 @@ def get_per_unit_http_config( strip_prefix=strip_prefix, redirect_https=redirect_https, external_host=external_host, + enable_experimental_forward_auth=enable_experimental_forward_auth, + forward_auth_app=forward_auth_app, + forward_auth_config=forward_auth_config, ) def get_per_app_http_config( @@ -286,6 +293,9 @@ def get_per_app_http_config( strip_prefix: Optional[bool], redirect_https: Optional[bool], external_host: str, + enable_experimental_forward_auth: bool, + forward_auth_app: bool, + forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: """Generate a config dict for Ingress(PerApp).""" # purge potential Nones @@ -298,6 +308,9 @@ def get_per_app_http_config( strip_prefix=strip_prefix, redirect_https=redirect_https, external_host=external_host, + enable_experimental_forward_auth=enable_experimental_forward_auth, + forward_auth_app=forward_auth_app, + forward_auth_config=forward_auth_config, ) def get_per_leader_http_config( @@ -310,6 +323,9 @@ def get_per_leader_http_config( strip_prefix: bool, redirect_https: bool, external_host: str, + enable_experimental_forward_auth: bool, + forward_auth_app: bool, + forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: """Generate a config dict for Ingress v1 (PerLeader).""" lb_servers = [{"url": f"http://{host}:{port}"}] @@ -320,6 +336,9 @@ def get_per_leader_http_config( strip_prefix=strip_prefix, redirect_https=redirect_https, external_host=external_host, + enable_experimental_forward_auth=enable_experimental_forward_auth, + forward_auth_app=forward_auth_app, + forward_auth_config=forward_auth_config, ) def _generate_config_block( @@ -330,6 +349,9 @@ def _generate_config_block( redirect_https: Optional[bool], strip_prefix: Optional[bool], external_host: str, + enable_experimental_forward_auth: bool, + forward_auth_app: bool, + forward_auth_config: Optional[ForwardAuthConfig], ) -> Dict[str, Any]: """Generate a configuration segment. @@ -422,6 +444,9 @@ def _generate_config_block( strip_prefix=strip_prefix_, scheme=scheme_, prefix=prefix, + enable_experimental_forward_auth=enable_experimental_forward_auth, + forward_auth_app=forward_auth_app, + forward_auth_config=forward_auth_config, ) if middlewares: @@ -439,6 +464,9 @@ def _generate_middleware_config( strip_prefix: bool, scheme: str, prefix: str, + enable_experimental_forward_auth: bool, + forward_auth_app: bool, + forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: """Generate a middleware config. @@ -446,6 +474,16 @@ def _generate_middleware_config( "cannot create middleware: multi-types middleware not supported, consider declaring two different pieces of middleware instead" """ + forwardauth_middleware = {} + if enable_experimental_forward_auth: + if forward_auth_app: + forwardauth_middleware[f"juju-sidecar-forward-auth-{prefix}"] = { + "forwardAuth": { + "address": forward_auth_config.decisions_address, # type: ignore + "authResponseHeaders": forward_auth_config.headers, # type: ignore + } + } + no_prefix_middleware = {} # type: Dict[str, Dict[str, Any]] if self._routing_mode is RoutingMode.path and strip_prefix: no_prefix_middleware[f"juju-sidecar-noprefix-{prefix}"] = { @@ -460,7 +498,7 @@ def _generate_middleware_config( "redirectScheme": {"scheme": "https", "port": 443, "permanent": True} } - return {**no_prefix_middleware, **redir_scheme_middleware} + return {**forwardauth_middleware, **no_prefix_middleware, **redir_scheme_middleware} @staticmethod def generate_tls_config_for_route( From f31b24e66d5f9b3e0165658a9a8aec585e74b754 Mon Sep 17 00:00:00 2001 From: natalia Date: Fri, 1 Dec 2023 13:33:37 +0100 Subject: [PATCH 16/17] refactor: add experimental_forward_auth_enabled as an init arg --- src/charm.py | 4 +--- src/traefik.py | 13 +++---------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/charm.py b/src/charm.py index 6616416f..2b564104 100755 --- a/src/charm.py +++ b/src/charm.py @@ -156,6 +156,7 @@ def __init__(self, *args): routing_mode=self._routing_mode, tcp_entrypoints=self._tcp_entrypoints(), tls_enabled=self._is_tls_enabled(), + experimental_forward_auth_enabled=self._is_forward_auth_enabled, ) self.service_patch = KubernetesServicePatch( @@ -740,7 +741,6 @@ def _get_configs_per_leader(self, relation: Relation) -> Dict[str, Any]: redirect_https=data.get("redirect-https", False), strip_prefix=data.get("strip-prefix", False), external_host=self.external_host, - enable_experimental_forward_auth=self._is_forward_auth_enabled, forward_auth_app=self.forward_auth.is_protected_app(app=data.get("name")), forward_auth_config=self.forward_auth.get_provider_info(), ) @@ -774,7 +774,6 @@ def _get_configs_per_app(self, relation: Relation) -> Dict[str, Any]: port=data.app.port, external_host=self.external_host, hosts=[udata.host for udata in data.units], - enable_experimental_forward_auth=self._is_forward_auth_enabled, forward_auth_app=self.forward_auth.is_protected_app(app=data.app.name), forward_auth_config=self.forward_auth.get_provider_info(), ) @@ -828,7 +827,6 @@ def _get_configs_per_unit(self, relation: Relation) -> Dict[str, Any]: strip_prefix=data.get("strip-prefix"), redirect_https=data.get("redirect-https"), external_host=self.external_host, - enable_experimental_forward_auth=self._is_forward_auth_enabled, forward_auth_app=self.forward_auth.is_protected_app(app=data.get("name")), forward_auth_config=self.forward_auth.get_provider_info(), ) diff --git a/src/traefik.py b/src/traefik.py index ce5496ba..c7ee9f27 100644 --- a/src/traefik.py +++ b/src/traefik.py @@ -79,12 +79,14 @@ def __init__( container: Container, routing_mode: RoutingMode, tls_enabled: bool, + experimental_forward_auth_enabled: bool, tcp_entrypoints: Dict[str, int], ): self._container = container self._tcp_entrypoints = tcp_entrypoints self._routing_mode = routing_mode self._tls_enabled = tls_enabled + self._experimental_forward_auth_enabled = experimental_forward_auth_enabled @property def scrape_jobs(self) -> list: @@ -265,7 +267,6 @@ def get_per_unit_http_config( strip_prefix: Optional[bool], redirect_https: Optional[bool], external_host: str, - enable_experimental_forward_auth: bool, forward_auth_app: bool, forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: @@ -278,7 +279,6 @@ def get_per_unit_http_config( strip_prefix=strip_prefix, redirect_https=redirect_https, external_host=external_host, - enable_experimental_forward_auth=enable_experimental_forward_auth, forward_auth_app=forward_auth_app, forward_auth_config=forward_auth_config, ) @@ -293,7 +293,6 @@ def get_per_app_http_config( strip_prefix: Optional[bool], redirect_https: Optional[bool], external_host: str, - enable_experimental_forward_auth: bool, forward_auth_app: bool, forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: @@ -308,7 +307,6 @@ def get_per_app_http_config( strip_prefix=strip_prefix, redirect_https=redirect_https, external_host=external_host, - enable_experimental_forward_auth=enable_experimental_forward_auth, forward_auth_app=forward_auth_app, forward_auth_config=forward_auth_config, ) @@ -323,7 +321,6 @@ def get_per_leader_http_config( strip_prefix: bool, redirect_https: bool, external_host: str, - enable_experimental_forward_auth: bool, forward_auth_app: bool, forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: @@ -336,7 +333,6 @@ def get_per_leader_http_config( strip_prefix=strip_prefix, redirect_https=redirect_https, external_host=external_host, - enable_experimental_forward_auth=enable_experimental_forward_auth, forward_auth_app=forward_auth_app, forward_auth_config=forward_auth_config, ) @@ -349,7 +345,6 @@ def _generate_config_block( redirect_https: Optional[bool], strip_prefix: Optional[bool], external_host: str, - enable_experimental_forward_auth: bool, forward_auth_app: bool, forward_auth_config: Optional[ForwardAuthConfig], ) -> Dict[str, Any]: @@ -444,7 +439,6 @@ def _generate_config_block( strip_prefix=strip_prefix_, scheme=scheme_, prefix=prefix, - enable_experimental_forward_auth=enable_experimental_forward_auth, forward_auth_app=forward_auth_app, forward_auth_config=forward_auth_config, ) @@ -464,7 +458,6 @@ def _generate_middleware_config( strip_prefix: bool, scheme: str, prefix: str, - enable_experimental_forward_auth: bool, forward_auth_app: bool, forward_auth_config: Optional[ForwardAuthConfig], ) -> dict: @@ -475,7 +468,7 @@ def _generate_middleware_config( different pieces of middleware instead" """ forwardauth_middleware = {} - if enable_experimental_forward_auth: + if self._experimental_forward_auth_enabled: if forward_auth_app: forwardauth_middleware[f"juju-sidecar-forward-auth-{prefix}"] = { "forwardAuth": { From e50fc276599a0961dcc8bf9eca9b2d79c4ef5e6e Mon Sep 17 00:00:00 2001 From: natalia Date: Tue, 5 Dec 2023 10:32:04 +0100 Subject: [PATCH 17/17] refactor: address review comments --- src/charm.py | 2 +- tests/integration/test_forward_auth.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 2b564104..6984552e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -490,10 +490,10 @@ def _on_config_changed(self, _: ConfigChangedEvent): 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 - self._process_status_and_configurations() def _process_status_and_configurations(self): routing_mode = self.config["routing_mode"] diff --git a/tests/integration/test_forward_auth.py b/tests/integration/test_forward_auth.py index 29e568d4..7c71d7c0 100644 --- a/tests/integration/test_forward_auth.py +++ b/tests/integration/test_forward_auth.py @@ -89,6 +89,7 @@ async def test_allowed_forward_auth_url_redirect(ops_test: OpsTest) -> None: """Test that a request hitting an application protected by IAP is forwarded by traefik to oathkeeper. An allowed request should be performed without authentication. + Retry the request to ensure the access rules were populated by oathkeeper. """ requirer_url = await get_reverse_proxy_app_url(ops_test, TRAEFIK_CHARM, IAP_REQUIRER_CHARM) @@ -158,6 +159,11 @@ def assert_anonymous_response(url): reraise=True, ) def update_access_rules_configmap(ops_test: OpsTest, lightkube_client: Client, rule): + """Modify the configmap to force access rules update. + + This is a workaround to test response headers without deploying identity-platform bundle. + The anonymous authenticator is used only for testing purposes. + """ cm = lightkube_client.get(ConfigMap, "access-rules", namespace=ops_test.model.name) data = {"access-rules-iap-requirer-anonymous.json": str(rule)} cm.data = data