diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index fd1378bc..860f45a5 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -1,23 +1,34 @@ # actionlint configuration. # -# One suppression, and it is a gap in the linter rather than in the workflow. +# Two suppressions, and both are gaps in the linter rather than in the workflows. # -# `vulnerability-alerts: read` is the ONLY GITHUB_TOKEN permission that grants +# 1. `vulnerability-alerts: read` is the ONLY GITHUB_TOKEN permission that grants # GET /repos/{owner}/{repo}/dependabot/alerts. GitHub documents it under the # `permissions` key in the workflow syntax reference ("For Dependabot alerts, use the # vulnerability-alerts permission"), but actionlint 1.7.12 still validates permission # scopes against a hard-coded list that predates it, so it reports the real, working key # as `unknown permission scope`. # -# Removal gate: drop this block once actionlint's `AllPermissionScopes` includes +# Removal gate: drop this entry once actionlint's `AllPermissionScopes` includes # `vulnerability-alerts` — check with # actionlint .github/workflows/scheduled-security-scan.yml -# after an actionlint upgrade. If it passes, delete this file. +# after an actionlint upgrade. # # Deliberately scoped to the one file and the one message. A blanket `permissions` ignore # would also hide a genuinely misspelled scope, which fails OPEN: an unrecognised scope is # not granted, the API call 403s, and the watcher reports zero alerts. +# +# 2. `concurrency.queue` (GitHub changelog, 2026-05-07) is what keeps pdp-tester runs waiting +# instead of cancelling each other in tests.yml. actionlint 1.7.12 only knows `group` and +# `cancel-in-progress` (rhysd/actionlint#657), so it reports the documented key as a syntax +# error. Scoped to that file and that message, so a misspelled concurrency key still fails. +# +# Removal gate: drop this entry once `actionlint .github/workflows/tests.yml` passes without +# it after an actionlint upgrade. With both entries gone, delete this file. paths: .github/workflows/scheduled-security-scan.yml: ignore: - 'unknown permission scope "vulnerability-alerts"' + .github/workflows/tests.yml: + ignore: + - 'unexpected key "queue" for "concurrency" section' diff --git a/.github/dependabot.yml b/.github/dependabot.yml index ecef8efb..7a89fab1 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -83,6 +83,16 @@ updates: # exercises end to end. Majors stay manual; minors and patches are fine. - dependency-name: "websockets" update-types: ["version-update:semver-major"] + # opal-common and opal-client 0.9.9, the first final release after the 0.9.6 pinned in + # pyproject.toml, declare Requires-Python <3.13 and cannot run on the 3.13 image (see the + # note at that pin). uv ignores Requires-Python upper bounds when it locks, so Dependabot + # would still open the PR and only the pytests job's pip check would catch it. 0.10 stays + # in, so a release that lifts the bound still arrives as a PR. Drop these two once an + # OPAL 0.9.x release supports Python 3.13. + - dependency-name: "opal-common" + versions: [">=0.9.7, <0.10"] + - dependency-name: "opal-client" + versions: [">=0.9.7, <0.10"] commit-message: prefix: "deps" prefix-development: "deps-dev" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 88442af5..ff96e96d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -82,7 +82,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: permitio/permit-opa - ref: ff2356d560e3c7152bcb6c6ab8ebd5d4eb691306 # permit-opa 0.0.23 (#52; go 1.26, x/crypto v0.57.0) + ref: 2f411da9b486226a0bfe4c1e39344eaa885553a1 # permit-opa main (permitio/permit-opa#53-#57) + ReBAC derivation fixes (PER-16921); re-pin to the merge commit path: './permit-opa' token: ${{ steps.permit-opa-token.outputs.token }} # Nothing after the clone needs the token; don't leave it in .git/config. @@ -347,7 +347,7 @@ jobs: - name: Docker Scout scan the release image id: scout if: ${{ !cancelled() }} - uses: docker/scout-action@7c6b6c3f7844478ace1ffd4e7aef649053d1f87d # v1.24.0 + uses: docker/scout-action@221e7f4860634eeb1579e3bd7ca232e577bd1864 # v1.25.0 with: command: cves image: local://permitio/pdp-v2:release-scan diff --git a/.github/workflows/scheduled-security-scan.yml b/.github/workflows/scheduled-security-scan.yml index 4b381da9..21b7ed7b 100644 --- a/.github/workflows/scheduled-security-scan.yml +++ b/.github/workflows/scheduled-security-scan.yml @@ -269,7 +269,7 @@ jobs: # `if:` is skipped once anything earlier in the job has failed, and a skipped report # leaves code scanning showing yesterday's answer for this image. if: ${{ !cancelled() }} - uses: docker/scout-action@7c6b6c3f7844478ace1ffd4e7aef649053d1f87d # v1.24.0 + uses: docker/scout-action@221e7f4860634eeb1579e3bd7ca232e577bd1864 # v1.25.0 with: command: cves # `registry://`, NOT the `local://` that tests.yml uses. There is no local image @@ -292,7 +292,7 @@ jobs: # Load-bearing, not defensive. Without it a failed report step skips this one, and # the report job would have nothing from Scout to read. if: ${{ !cancelled() }} - uses: docker/scout-action@7c6b6c3f7844478ace1ffd4e7aef649053d1f87d # v1.24.0 + uses: docker/scout-action@221e7f4860634eeb1579e3bd7ca232e577bd1864 # v1.25.0 with: command: cves image: registry://permitio/pdp-v2:latest diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8c1326ec..3a6d0fdd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -104,6 +104,14 @@ jobs: python -m pip install --dry-run --quiet --disable-pip-version-check \ --require-hashes --no-deps --requirement "$RUNNER_TEMP/lock.txt" + # For horizon/tests/test_helm_chart.py, which renders and lints charts/pdp. Pinned so a + # Helm release cannot change the result on its own; the tests fail, not skip, when CI is set + # and helm is missing. + - name: Install Helm + uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5.0.1 + with: + version: v4.3.0 + - name: Run Pytests run: uv run --frozen pytest -s --cache-clear horizon/tests/ @@ -163,7 +171,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: permitio/permit-opa - ref: ff2356d560e3c7152bcb6c6ab8ebd5d4eb691306 # permit-opa 0.0.23 (#52; go 1.26, x/crypto v0.57.0) + ref: 2f411da9b486226a0bfe4c1e39344eaa885553a1 # permit-opa main (permitio/permit-opa#53-#57) + ReBAC derivation fixes (PER-16921); re-pin to the merge commit path: './permit-opa' token: ${{ steps.permit-opa-token.outputs.token }} # Nothing after the clone needs the token; don't leave it in .git/config. @@ -263,6 +271,15 @@ jobs: pdp-tester: runs-on: ubuntu-24.04 needs: build-pdp-image + # One run at a time across the repository - PRs, pushes and releases, which reach this + # job through release.yml - because every run writes to the same staging environment and + # overlapping runs failed each other's sync cases (PER-16929). `queue: max` lets up to 100 + # runs wait their turn. The default queue holds one, and a newer run cancels the one already + # waiting, which would leave another PR with a cancelled required check. GitHub rejects + # `queue: max` together with cancel-in-progress, so a running test always finishes. + concurrency: + group: pdp-tester-staging + queue: max # Cap the run so a hung tester fails fast instead of holding the runner # until GitHub's 360-minute default. The old k3d path was bounded by # `kubectl wait --timeout=600s`; the tester's own max_running_time only @@ -447,7 +464,7 @@ jobs: username: ${{ vars.DOCKERHUB_ORGANIZATION }} - name: Docker Scout CVE report (all severities) - uses: docker/scout-action@7c6b6c3f7844478ace1ffd4e7aef649053d1f87d # v1.24.0 + uses: docker/scout-action@221e7f4860634eeb1579e3bd7ca232e577bd1864 # v1.25.0 with: command: cves image: local://permitio/pdp-v2:next @@ -473,7 +490,7 @@ jobs: - name: Docker Scout CVE gate (high and critical) id: scout-gate - uses: docker/scout-action@7c6b6c3f7844478ace1ffd4e7aef649053d1f87d # v1.24.0 + uses: docker/scout-action@221e7f4860634eeb1579e3bd7ca232e577bd1864 # v1.25.0 with: command: cves image: local://permitio/pdp-v2:next diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cc2854a6..5624f757 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: cef0300fd0fc4d2a87a85fa2093c6b283ea36f4b # frozen: v5.0.0 + rev: 3e8a8703264a2f4a69428a0aa4dcb512790b2c8c # frozen: v6.0.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer diff --git a/Cargo.lock b/Cargo.lock index 853316ac..eff89b9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1453,6 +1453,7 @@ dependencies = [ "serde_json", "serde_yaml", "sha2", + "subtle", "tempfile", "thiserror", "tokio", diff --git a/README.md b/README.md index de3e9ddd..52321956 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,9 @@ PDP_CONTROL_PLANE=https://api.permit.io PDP_API_KEY= uv run uvicor ``` ## Building a Custom PDP Docker image +The build compiles Permit's OPA build from the private `permitio/permit-opa` repository, which +`build_opal_bundle.sh` clones over SSH into `../permit-opa`. + For ARM architecture: ``` VERSION= make build-arm64 @@ -64,6 +67,16 @@ For AMD64 architecture: VERSION= make build-amd64 ``` +### Building without access to permit-opa +`PDP_VANILLA=true` builds the image with upstream OPA instead (`OPA_BUILD=vanilla`), for +development without access to `permitio/permit-opa`. It works with every build target: +``` +PDP_VANILLA=true VERSION= make build +``` +Permit-generated policies call builtins that exist only in Permit's OPA build, so an image built +this way cannot evaluate them. To evaluate Permit policies, use the published `permitio/pdp-v2` +image. + ### Running the image in development mode ``` VERSION= API_KEY= make run diff --git a/charts/pdp/Chart.yaml b/charts/pdp/Chart.yaml index ac9815e2..63a8ae85 100644 --- a/charts/pdp/Chart.yaml +++ b/charts/pdp/Chart.yaml @@ -1,7 +1,7 @@ apiVersion: v2 name: pdp description: An official Helm chart for Permit.io PDP (Policy Decision Point) with OpenShift support and configurable ports -version: 0.0.6 +version: 0.0.7 keywords: - policy - authorization diff --git a/charts/pdp/templates/_helpers.tpl b/charts/pdp/templates/_helpers.tpl index b65ae0ce..437cca0f 100644 --- a/charts/pdp/templates/_helpers.tpl +++ b/charts/pdp/templates/_helpers.tpl @@ -15,6 +15,19 @@ Common labels {{- end }} {{- end }} +{{/* +Fail on API key settings that contradict each other or that a template would misread +*/}} +{{- define "pdp.validateApiKeySource" -}} +{{- $userProvidedSecret := .Values.pdp.userProvidedSecret | default false -}} +{{- if not (kindIs "bool" $userProvidedSecret) -}} +{{- fail (printf "pdp.userProvidedSecret must be true or false, got the %s %q. A quoted \"false\" counts as set and would drop PDP_API_KEY." (kindOf $userProvidedSecret) (toString $userProvidedSecret)) -}} +{{- end -}} +{{- if and $userProvidedSecret .Values.pdp.existingApiKeySecret -}} +{{- fail "pdp.userProvidedSecret and pdp.existingApiKeySecret cannot both be set: existingApiKeySecret makes the chart read PDP_API_KEY from that Secret, userProvidedSecret makes it read no Secret at all. Unset one of them." -}} +{{- end -}} +{{- end }} + {{/* Get the secret name for the API key */}} diff --git a/charts/pdp/templates/deployment.yaml b/charts/pdp/templates/deployment.yaml index 214cea11..4dfb8226 100644 --- a/charts/pdp/templates/deployment.yaml +++ b/charts/pdp/templates/deployment.yaml @@ -1,3 +1,4 @@ +{{- include "pdp.validateApiKeySource" . }} apiVersion: apps/v1 kind: Deployment metadata: @@ -50,11 +51,13 @@ spec: containerPort: {{ .targetPort }} {{- end }} env: + {{- if not .Values.pdp.userProvidedSecret }} - name: PDP_API_KEY valueFrom: secretKeyRef: name: {{ include "pdp.secretName" . }} key: {{ include "pdp.secretKey" . }} + {{- end }} {{- if .Values.pdp.pdpEnvs }} {{- range .Values.pdp.pdpEnvs }} - name: {{ .name }} @@ -155,3 +158,15 @@ spec: - name: opa-volume emptyDir: {} {{- end }} + {{- with .Values.pdp.nodeSelector }} + nodeSelector: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.pdp.affinity }} + affinity: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.pdp.tolerations }} + tolerations: + {{- toYaml . | nindent 8 }} + {{- end }} diff --git a/charts/pdp/templates/secret.yaml b/charts/pdp/templates/secret.yaml index b3b1f650..69c8f705 100644 --- a/charts/pdp/templates/secret.yaml +++ b/charts/pdp/templates/secret.yaml @@ -1,4 +1,4 @@ -{{- if not .Values.pdp.existingApiKeySecret }} +{{- if not (or .Values.pdp.existingApiKeySecret .Values.pdp.userProvidedSecret) }} apiVersion: v1 kind: Secret metadata: diff --git a/charts/pdp/values.yaml b/charts/pdp/values.yaml index 49aebdf7..c4588662 100644 --- a/charts/pdp/values.yaml +++ b/charts/pdp/values.yaml @@ -10,6 +10,10 @@ pdp: # Example - enable Envoy gRPC ext_authz on port 9191 (requires PDP >= 0.9.10): # - name: PDP_OPA_PLUGINS # value: '{"permit_graph":{},"envoy_ext_authz_grpc":{"addr":":9191","path":"permit/root"}}' + # The PDP's API key. By default the chart stores ApiKey in a Secret it creates + # (permitio-pdp-secret) and passes it to the PDP as PDP_API_KEY. existingApiKeySecret and + # userProvidedSecret below change that. Set at most one of them: with both set, the chart + # fails to render. ApiKey: "" # Use an existing secret for the API key instead of creating one @@ -17,6 +21,16 @@ pdp: # existingApiKeySecret: # name: "my-existing-secret" # key: "api-key" + + # Set to true when something other than the chart supplies PDP_API_KEY, for example a + # mutating webhook such as bank-vaults. The chart then creates no Secret, sets no PDP_API_KEY + # and ignores ApiKey. Set PDP_API_KEY in pdpEnvs above, or have the webhook add it to the pod. + # pdpEnvs values are stored in the Deployment as plain text, so put a reference there that + # the webhook resolves when the container starts (for example a bank-vaults `vault:` path), + # and keep a literal key in ApiKey or existingApiKeySecret. Must be a boolean: a quoted + # "false" fails the render. Without PDP_API_KEY the PDP does not start: its health checks + # fail and Kubernetes keeps restarting the container. + userProvidedSecret: false port: 7766 # Example - expose Envoy gRPC ext_authz port (requires PDP_OPA_PLUGINS env var above): # additionalPorts: @@ -43,6 +57,28 @@ pdp: index: "" debug_mode: false + # Pod scheduling, copied into the PDP pod spec as given. Empty values render nothing. + # Example - run only on a dedicated, tainted node pool: + # nodeSelector: + # pool: pdp + # tolerations: + # - key: "dedicated" + # operator: "Equal" + # value: "pdp" + # effect: "NoSchedule" + # affinity: + # podAntiAffinity: + # preferredDuringSchedulingIgnoredDuringExecution: + # - weight: 100 + # podAffinityTerm: + # topologyKey: kubernetes.io/hostname + # labelSelector: + # matchLabels: + # app: permitio-pdp + nodeSelector: {} + tolerations: [] + affinity: {} + podDisruptionBudget: # Automatically enabled when replicas > 1 # Set minAvailable OR maxUnavailable (not both) diff --git a/horizon/config.py b/horizon/config.py index 3e86f271..addcdffd 100644 --- a/horizon/config.py +++ b/horizon/config.py @@ -245,13 +245,15 @@ def __new__(cls, *, prefix=None, is_model=True): # noqa: ARG004 OPA_DECISION_LOG_ENABLED = confi.bool( "OPA_DECISION_LOG_ENABLED", True, - description="if true, OPA decision logs will be uploaded to the Permit.io cloud console", + description="if true, OPA decision logs will be uploaded to the Permit.io cloud console. " + "The control plane also sends this setting; a value set in the PDP's environment takes " + "precedence over it", ) OPA_DECISION_LOG_CONSOLE = confi.bool( "OPA_DECISION_LOG_CONSOLE", False, - description="if true, OPA decision logs will also be printed to console " - "(only relevant if `OPA_DECISION_LOG_ENABLED` is true)", + description="if true, OPA decision logs will be printed to console, " + "whether or not `OPA_DECISION_LOG_ENABLED` uploads them", ) OPA_DECISION_LOG_INGRESS_ROUTE = confi.str( "OPA_DECISION_LOG_INGRESS_ROUTE", diff --git a/horizon/enforcer/api.py b/horizon/enforcer/api.py index fed6894c..0fa6c761 100644 --- a/horizon/enforcer/api.py +++ b/horizon/enforcer/api.py @@ -42,7 +42,7 @@ KongWrappedAuthorizationQuery, ) from horizon.enforcer.schemas_v1 import AuthorizationQueryV1 -from horizon.enforcer.utils.mapping_rules_utils import MappingRulesUtils +from horizon.enforcer.utils.mapping_rules_utils import ConflictingQueryParameterError, MappingRulesUtils from horizon.enforcer.utils.statistics_utils import StatisticsManager from horizon.state import PersistentStateHandler @@ -67,16 +67,19 @@ def extract_pdp_api_key(request: Request) -> str: authorization: str = request.headers.get(AUTHZ_HEADER, "") parts = authorization.split(" ") if len(parts) != AUTHZ_HEADER_PARTS: - raise HTTPException( - status.HTTP_401_UNAUTHORIZED, - detail=f"bad authz header: {authorization}", - ) + raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="bad authz header") schema, token = parts if schema.strip().lower() != "bearer": raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Invalid PDP token") return token +def _url_not_allowed(query: UrlAuthorizationQuery, error: ConflictingQueryParameterError) -> dict: + """The /allowed_url answer for a URL whose mapping rule cannot be applied unambiguously.""" + logger.debug("allowed_url: {reason}; not allowed", reason=str(error), url=query.url) + return {"allow": False, "result": False, "query": query.dict(), "debug": {"reason": str(error)}} + + def transform_headers(request: Request) -> dict: token = extract_pdp_api_key(request) return { @@ -341,9 +344,12 @@ async def is_allowed_url( data_result = json.loads(data.body).get("result") or {} mapping_rules_json = data_result.get("all") or [] mapping_rules = [parse_obj_as(MappingRuleData, mapping_rule) for mapping_rule in mapping_rules_json] - matched_mapping_rule = MappingRulesUtils.extract_mapping_rule_by_request( - mapping_rules, query.http_method, query.url - ) + try: + matched_mapping_rule = MappingRulesUtils.extract_mapping_rule_by_request( + mapping_rules, query.http_method, query.url + ) + except ConflictingQueryParameterError as e: + return _url_not_allowed(query, e) if matched_mapping_rule is None: return { "allow": False, @@ -365,9 +371,12 @@ async def is_allowed_url( path_attributes = MappingRulesUtils.extract_attributes_from_url(matched_mapping_rule.url, query.url) # Query params handling remains the same for both types - query_params_attributes = MappingRulesUtils.extract_attributes_from_query_params( - matched_mapping_rule.url, query.url - ) + try: + query_params_attributes = MappingRulesUtils.extract_attributes_from_query_params( + matched_mapping_rule.url, query.url + ) + except ConflictingQueryParameterError as e: + return _url_not_allowed(query, e) attributes = {**path_attributes, **query_params_attributes} allowed_query = AuthorizationQuery( user=query.user, diff --git a/horizon/enforcer/opa/config_maker.py b/horizon/enforcer/opa/config_maker.py index 76052a0a..478bac21 100644 --- a/horizon/enforcer/opa/config_maker.py +++ b/horizon/enforcer/opa/config_maker.py @@ -31,23 +31,31 @@ def get_opa_config_file_path( puts the rendered contents in a file and returns the path to that file. - NOTE: Not all features of the config are implemented - only decision logs for now. + NOTE: Not all features of the config are implemented - only decision logs (upload and + console) and plugins. The permit_io service and the API key OPA sends to it are written only + when decision logs are enabled: uploading them is the service's only use. Console decision + logs do not depend on the upload. """ env = get_jinja_environment() target_path = sidecar_config.OPA_CONFIG_FILE_PATH + decision_logs_enabled = sidecar_config.OPA_DECISION_LOG_ENABLED decision_logs_backend_tier = ( sidecar_config.OPA_DECISION_LOG_INGRESS_BACKEND_TIER_URL or sidecar_config.CONTROL_PLANE ) logger.info( - "Uploading decision logs to backend tier: {tier}", - tier=decision_logs_backend_tier, + "Writing the OPA config file: decision log upload {upload}; console decision logs {console}; " + "plugins: {plugins}", + upload=f"to {decision_logs_backend_tier}" if decision_logs_enabled else "disabled", + console="enabled" if sidecar_config.OPA_DECISION_LOG_CONSOLE else "disabled", + plugins=", ".join(sidecar_config.OPA_PLUGINS) or "none", ) try: template = env.get_template(template_path) contents = template.render( + decision_logs_enabled=decision_logs_enabled, cloud_service_url=decision_logs_backend_tier, - bearer_token=get_env_api_key(), + bearer_token=get_env_api_key() if decision_logs_enabled else None, log_ingress_endpoint=sidecar_config.OPA_DECISION_LOG_INGRESS_ROUTE, min_delay_seconds=sidecar_config.OPA_DECISION_LOG_MIN_DELAY, max_delay_seconds=sidecar_config.OPA_DECISION_LOG_MAX_DELAY, diff --git a/horizon/enforcer/utils/mapping_rules_utils.py b/horizon/enforcer/utils/mapping_rules_utils.py index bdab75d6..5b237aff 100644 --- a/horizon/enforcer/utils/mapping_rules_utils.py +++ b/horizon/enforcer/utils/mapping_rules_utils.py @@ -1,6 +1,7 @@ # TODO: change to use re2 in the future, currently not supported in alpine due to c++ library issues # import re2 as re # use re2 instead of re for regex matching because it's simiplier and safer for user inputted regexes # noqa: ERA001,E501 import re +from urllib.parse import parse_qsl from loguru import logger from pydantic import AnyHttpUrl @@ -9,6 +10,19 @@ from horizon.enforcer.schemas import MappingRuleData, UrlTypes +class ConflictingQueryParameterError(ValueError): + """A query parameter a mapping rule reads has more than one distinct value in the requested URL. + + Which value the protected app reads for such a parameter is up to the app, so the PDP cannot + tell which rule applies or which attribute value to check. For a regex rule, the parameter is one + whose value can change whether the rule matches or what its named groups capture. + """ + + def __init__(self, key: str): + super().__init__(f"Query parameter '{key}' has more than one distinct value in the requested URL") + self.key = key + + class MappingRulesUtils: @staticmethod def _compare_httpurls(mapping_rule_url: str, request_url: str) -> bool: @@ -40,20 +54,32 @@ def _compare_url_path(mapping_rule_url: str | None, request_url: str | None) -> @staticmethod def _compare_query_params(mapping_rule_query_string: str, request_url_query_string: str) -> bool: + """Whether the request's query satisfies every parameter of the mapping rule's query. + + A parameter repeated with one value counts as given once. + + Raises: + ConflictingQueryParameterError: the request could satisfy the rule, but a parameter the + rule reads has more than one distinct value, so whether it does depends on the value read. + """ mapping_rule_query_params = QueryParams(mapping_rule_query_string) request_query_params = QueryParams(request_url_query_string) + conflicting_key = None for key in mapping_rule_query_params: - if key not in request_query_params: + request_values = set(request_query_params.getlist(key)) + if not request_values: return False - if mapping_rule_query_params[key].startswith("{") and mapping_rule_query_params[key].endswith("}"): - # if the value is an attribute - # we just need to make sure the attribute is in the request query params - continue - if mapping_rule_query_params[key] != request_query_params[key]: - # if the value is not an attribute, verify that the values are the same + rule_value = mapping_rule_query_params[key] + is_attribute = rule_value.startswith("{") and rule_value.endswith("}") + if not is_attribute and rule_value not in request_values: return False + if len(request_values) > 1: + conflicting_key = key + + if conflicting_key is not None: + raise ConflictingQueryParameterError(conflicting_key) return True @staticmethod @@ -70,20 +96,37 @@ def extract_attributes_from_url(rule_url: str, request_url: str) -> dict: @staticmethod def extract_attributes_from_query_params(rule_url: str, request_url: str) -> dict: + """The attributes a mapping rule's query reads from the request URL, e.g. ``{"id": "7"}`` for + a rule with ``?id={id}`` and a request with ``?id=7``. + + Raises: + ConflictingQueryParameterError: a parameter the rule reads an attribute from has more than + one distinct value in the request URL. + """ if "?" not in rule_url or "?" not in request_url: return {} - rule_query_params = QueryParams(rule_url.split("?")[1]) - request_query_params = QueryParams(request_url.split("?")[1]) + rule_query_params = QueryParams(rule_url.split("?", 1)[1]) + request_query_params = QueryParams(request_url.split("?", 1)[1]) attributes = {} for key in rule_query_params: if rule_query_params[key].startswith("{") and rule_query_params[key].endswith("}"): + if len(set(request_query_params.getlist(key))) > 1: + raise ConflictingQueryParameterError(key) attributes[rule_query_params[key][1:-1]] = request_query_params[key] return attributes @classmethod def _compare_urls(cls, mapping_rule_url: str, request_url: str, *, is_regex: bool = False) -> bool: - """ - Compare a mapping rule URL against a request URL. + """Whether the request URL matches the mapping rule URL. + + A regex rule is matched against the raw URL, so which query parameters it reads is unknown. When + a parameter has more than one distinct value, the rule is also matched against the URL as an app + reading the first value and as one reading the last value of each such parameter. + + Raises: + ConflictingQueryParameterError: the rule could match, but whether it does, or what it reads, + depends on which value of a repeated query parameter is read. For a regex rule the error + names the first parameter with more than one distinct value. """ # If the mapping rule is a regex pattern if is_regex: @@ -92,12 +135,57 @@ def _compare_urls(cls, mapping_rule_url: str, request_url: str, *, is_regex: boo except re.error as e: logger.warning("regex pattern compilation failed", pattern=mapping_rule_url, error=str(e)) return False - match_result = bool(pattern.match(request_url)) - logger.debug("regex url comparison", pattern=mapping_rule_url, url=request_url, matched=match_result) - return match_result + groups = cls._regex_match_groups(pattern, request_url) + logger.debug("regex url comparison", pattern=mapping_rule_url, url=request_url, matched=groups is not None) + readings = cls._first_and_last_value_readings(request_url) + if readings is not None: + key, reading_urls = readings + if any(cls._regex_match_groups(pattern, reading_url) != groups for reading_url in reading_urls): + raise ConflictingQueryParameterError(key) + return groups is not None return cls._compare_httpurls(mapping_rule_url, request_url) + @staticmethod + def _regex_match_groups(pattern: re.Pattern[str], url: str) -> dict[str, str | None] | None: + """The named groups of the pattern's match at the start of the URL, or None if it does not match.""" + match = pattern.match(url) + return match.groupdict() if match else None + + @staticmethod + def _first_and_last_value_readings(request_url: str) -> tuple[str, tuple[str, str]] | None: + """The request URL as an app reading the first, and as one reading the last, value of each query + parameter that has more than one distinct value. + + Each reading drops the occurrences of such a parameter that carry another value and keeps the + rest of the URL as it was, so a pattern sees the same characters as in the original. + + Returns: + The first parameter with more than one distinct value and the two readings, or None if no + parameter has more than one distinct value. + """ + path, separator, query = request_url.partition("?") + if not separator: + return None + segments = [(segment, parse_qsl(segment, keep_blank_values=True)) for segment in query.split("&")] + values_by_key: dict[str, list[str]] = {} + for _, pairs in segments: + for key, value in pairs: + values_by_key.setdefault(key, []).append(value) + kept_values = {key: (values[0], values[-1]) for key, values in values_by_key.items() if len(set(values)) > 1} + if not kept_values: + return None + + def reading(position: int) -> str: + kept_segments = [ + segment + for segment, pairs in segments + if all(key not in kept_values or value == kept_values[key][position] for key, value in pairs) + ] + return f"{path}?{'&'.join(kept_segments)}" + + return next(iter(kept_values)), (reading(0), reading(1)) + @classmethod def extract_mapping_rule_by_request( cls, @@ -105,7 +193,18 @@ def extract_mapping_rule_by_request( http_method: str, url: AnyHttpUrl, ) -> MappingRuleData | None: - matched_mapping_rules = [] + """The highest-priority mapping rule for the request's method and URL, or None if none matches. + + Rules of equal priority keep their order in ``mapping_rules``. + + Raises: + ConflictingQueryParameterError: the rule that would come first could match, but a query + parameter it reads has more than one distinct value. The request then gets no rule at + all, rather than a lower-priority one that skips the parameter. A conflict in a rule + that a matching rule outranks does not matter: that rule comes first whatever value + is read. + """ + candidates: list[tuple[MappingRuleData, ConflictingQueryParameterError | None]] = [] http_method = http_method.lower() # Convert once instead of in each iteration for mapping_rule in mapping_rules: @@ -126,14 +225,19 @@ def extract_mapping_rule_by_request( # if the method is not the same, we don't need to check the url continue - if not cls._compare_urls(mapping_rule.url, url, is_regex=is_regex): - continue - - matched_mapping_rules.append(mapping_rule) - - # most priority first - matched_mapping_rules.sort(key=lambda rule: rule.priority or 0, reverse=True) - if len(matched_mapping_rules) > 0: - return matched_mapping_rules[0] - - return None + try: + if not cls._compare_urls(mapping_rule.url, url, is_regex=is_regex): + continue + except ConflictingQueryParameterError as conflict: + candidates.append((mapping_rule, conflict)) + else: + candidates.append((mapping_rule, None)) + + if not candidates: + return None + # most priority first; the sort is stable, so equal priorities keep the rules' order + candidates.sort(key=lambda candidate: candidate[0].priority or 0, reverse=True) + first_rule, conflict = candidates[0] + if conflict is not None: + raise conflict + return first_rule diff --git a/horizon/facts/client.py b/horizon/facts/client.py index 7ac1799f..7bf7e1e2 100644 --- a/horizon/facts/client.py +++ b/horizon/facts/client.py @@ -48,6 +48,9 @@ async def build_forward_request( Build an HTTPX request from a FastAPI request to forward to the facts service. :param request: FastAPI request :param path: Backend facts service path to forward to + :param query_params: query parameters to set on the forwarded request. Each replaces every value + the request has for the same key; every other request query parameter is forwarded with all + of its values, in order. :param is_consistent_update: if True, marks the request as a consistent update so the backend skips the control-plane delta update (the PDP handles propagation locally). :return: HTTPX request @@ -67,11 +70,15 @@ async def build_forward_request( ) full_path = urljoin(f"/v2/facts/{project_id}/{environment_id}/", path.removeprefix("/")) - _query_params = {**request.query_params, **(query_params or {})} + overrides = query_params or {} + forward_params: list[tuple[str, Any]] = [ + (key, value) for key, value in request.query_params.multi_items() if key not in overrides + ] + forward_params.extend(overrides.items()) return self.client.build_request( method=request.method, url=full_path, - params=_query_params, + params=forward_params, headers=forward_headers, content=request.stream(), ) diff --git a/horizon/pdp.py b/horizon/pdp.py index 6e3dea99..b64117c6 100644 --- a/horizon/pdp.py +++ b/horizon/pdp.py @@ -21,7 +21,7 @@ opal_common_config, ) from opal_client.engine.options import OpaServerOptions -from opal_common.confi import Confi +from opal_common.confi import Confi, UndefinedValueError, cast_boolean from opal_common.fetcher.providers.http_fetch_provider import ( HttpFetcherConfig, HttpMethods, @@ -164,6 +164,54 @@ def apply_config(overrides_dict: dict, config_object: Confi): logger.warning(f"Ignored non-existing config key: {prefixed_key}") +DECISION_LOG_ENABLED_KEY = "OPA_DECISION_LOG_ENABLED" + + +def apply_pdp_overrides(overrides_dict: dict) -> None: + """Apply the control plane's PDP config overrides to ``sidecar_config``. + + The control plane sends the same ``OPA_DECISION_LOG_ENABLED`` to every PDP. When + ``PDP_OPA_DECISION_LOG_ENABLED`` is set to a boolean in this PDP's environment, the local + value is kept and that one override is skipped. An empty or non-boolean value is logged as a + warning and the control plane's value is used. Which of the two values is used is logged. + + Args: + overrides_dict: the ``pdp`` section of the remote config. + """ + overrides = dict(overrides_dict) + if DECISION_LOG_ENABLED_KEY in overrides: + env_name = sidecar_config._prefix_key(DECISION_LOG_ENABLED_KEY) + remote_value = overrides[DECISION_LOG_ENABLED_KEY] + local_text = os.environ.get(env_name) + if local_text is None: + logger.info( + "{env_name} is not set for this PDP; using the control plane's value ({remote}).", + env_name=env_name, + remote=remote_value, + ) + else: + try: + local_value = cast_boolean(local_text) + except UndefinedValueError: + logger.warning( + "{env_name} is set to {local!r}, which is not true, false, 1 or 0; " + "using the control plane's value ({remote}).", + env_name=env_name, + local=local_text, + remote=remote_value, + ) + else: + del overrides[DECISION_LOG_ENABLED_KEY] + logger.info( + "{env_name} is set for this PDP ({local}); " + "using it instead of the control plane's value ({remote}).", + env_name=env_name, + local=local_value, + remote=remote_value, + ) + apply_config(overrides, sidecar_config) + + # Declared as a ``response_model`` (rather than left as a bare dict) because the trigger routes' # customer-facing OpenAPI description tells integrators to branch on ``triggered``: without one, # FastAPI publishes an empty 200 schema, so the prose would reference a field the machine-readable @@ -295,13 +343,11 @@ def __init__(self): apply_config(remote_config.opal_common or {}, opal_common_config) apply_config(remote_config.opal_client or {}, opal_client_config) - apply_config(remote_config.pdp or {}, sidecar_config) + apply_pdp_overrides(remote_config.pdp or {}) self._log_environment(remote_config.context) - if sidecar_config.OPA_BEARER_TOKEN_REQUIRED or sidecar_config.OPA_DECISION_LOG_ENABLED: - # we need to pass to OPAL a custom inline OPA config to enable these features - self._configure_inline_opa_config() + self._configure_inline_opa_config() self._configure_opal_data_updater() self._configure_opal_offline_mode() @@ -419,16 +465,40 @@ def _configure_cloud_logging(self, remote_context: dict | None = None): catch=True, # if sink throws exceptions, swallow them as not critical ) - def _configure_inline_opa_config(self): + @staticmethod + def _configure_inline_opa_config(): + """Pass OPAL the inline OPA config that decision logs, plugins and bearer auth need. + + OPA reads decision log (upload or console) and plugin settings only from its config file, + so the file is written whenever any of them is on. A different ``config_file`` already in + the inline config is replaced, with a warning. With none of them on and no bearer token + required, the inline config is left as it is. + """ + config_file_needed = ( + sidecar_config.OPA_DECISION_LOG_ENABLED + or sidecar_config.OPA_DECISION_LOG_CONSOLE + or bool(sidecar_config.OPA_PLUGINS) + ) + if not (config_file_needed or sidecar_config.OPA_BEARER_TOKEN_REQUIRED): + return + # Start from the existing config inline_opa_config = opal_client_config.INLINE_OPA_CONFIG.dict() logger.debug(f"existing OPAL_INLINE_OPA_CONFIG={inline_opa_config}") - if sidecar_config.OPA_DECISION_LOG_ENABLED: - # decision logs needs to be configured via the config file + if config_file_needed: config_file_path = get_opa_config_file_path(sidecar_config) + existing_config_file = inline_opa_config.get("config_file") + if existing_config_file and Path(existing_config_file).expanduser() != Path(config_file_path): + logger.warning( + "OPAL_INLINE_OPA_CONFIG sets config_file={existing}; OPA uses the PDP's config file " + "{path} instead, which holds its decision log and plugin settings.", + existing=existing_config_file, + path=config_file_path, + ) + # append the config file to inline OPA config inline_opa_config.update({"config_file": config_file_path}) diff --git a/horizon/proxy/api.py b/horizon/proxy/api.py index fd0adb68..3b4636c9 100644 --- a/horizon/proxy/api.py +++ b/horizon/proxy/api.py @@ -182,7 +182,7 @@ async def proxy_request_to_cloud_service( headers={"WWW-Authenticate": "Bearer"}, ) path = f"{cloud_service_url}/{path}" - params = dict(request.query_params) or {} + params = request.query_params.multi_items() original_headers = {k.lower(): v for k, v in iter(dict(request.headers).items())} headers = additional_headers diff --git a/horizon/static/templates/config.yaml.template b/horizon/static/templates/config.yaml.template index 51a901f1..c57f3d2e 100644 --- a/horizon/static/templates/config.yaml.template +++ b/horizon/static/templates/config.yaml.template @@ -1,22 +1,26 @@ +{% if decision_logs_enabled %} services: permit_io: url: {{ cloud_service_url }} - {% if bearer_token is defined %} credentials: bearer: token: "{{ bearer_token }}" - {% endif %} +{% endif %} +{% if decision_logs_enabled or log_to_console %} decision_logs: {% if log_to_console %} console: true {% endif %} + {% if decision_logs_enabled %} service: permit_io resource: {{ log_ingress_endpoint }} reporting: min_delay_seconds: {{ min_delay_seconds }} max_delay_seconds: {{ max_delay_seconds }} upload_size_limit_bytes: {{ upload_size_limit_bytes }} + {% endif %} +{% endif %} {% if plugins %} plugins: diff --git a/horizon/tests/test_decision_log_setting.py b/horizon/tests/test_decision_log_setting.py new file mode 100644 index 00000000..5c010939 --- /dev/null +++ b/horizon/tests/test_decision_log_setting.py @@ -0,0 +1,97 @@ +"""Which PDP_OPA_DECISION_LOG_ENABLED a PDP ends up with, its own or the control plane's. + +The control plane sends OPA_DECISION_LOG_ENABLED to every PDP with its config overrides. A boolean +set in the PDP's environment is kept; without one, or with an empty or non-boolean value, the +control plane's value is used. + +Each case runs in a new interpreter, because horizon.config reads the environment once, at import. +""" + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +ENV_NAME = "PDP_OPA_DECISION_LOG_ENABLED" +ROUTE = "/v1/decision_logs/custom-ingress" + + +def _apply_overrides(env_value: str | None, overrides: dict) -> tuple[dict, str]: + """Start with ENV_NAME set to env_value (unset for None), apply the overrides as the PDP does. + + Returns the resulting decision log settings and everything the interpreter logged. + """ + env = {name: value for name, value in os.environ.items() if name != ENV_NAME} + if env_value is not None: + env[ENV_NAME] = env_value + code = ( + "import json, sys\n" + "from horizon.config import sidecar_config\n" + "from horizon.pdp import apply_pdp_overrides\n" + "apply_pdp_overrides(json.loads(sys.argv[1]))\n" + "print(json.dumps({'enabled': sidecar_config.OPA_DECISION_LOG_ENABLED," + " 'route': sidecar_config.OPA_DECISION_LOG_INGRESS_ROUTE}))\n" + ) + result = subprocess.run( + [sys.executable, "-c", code, json.dumps(overrides)], + cwd=REPO_ROOT, + env=env, + capture_output=True, + text=True, + check=False, + timeout=120, + ) + assert result.returncode == 0, result.stderr + return json.loads(result.stdout.splitlines()[-1]), result.stderr + + +def test_false_set_for_the_pdp_is_kept_over_the_control_plane_true(): + settings, logged = _apply_overrides( + "false", {"OPA_DECISION_LOG_ENABLED": True, "OPA_DECISION_LOG_INGRESS_ROUTE": ROUTE} + ) + + assert settings == {"enabled": False, "route": ROUTE} + assert f"{ENV_NAME} is set for this PDP (False); using it instead of the control plane's value (True)." in logged + + +def test_true_set_for_the_pdp_is_kept_over_the_control_plane_false(): + settings, logged = _apply_overrides( + "true", {"OPA_DECISION_LOG_ENABLED": False, "OPA_DECISION_LOG_INGRESS_ROUTE": ROUTE} + ) + + assert settings == {"enabled": True, "route": ROUTE} + assert f"{ENV_NAME} is set for this PDP (True); using it instead of the control plane's value (False)." in logged + + +def test_without_a_value_set_for_the_pdp_the_control_plane_value_is_used(): + settings, logged = _apply_overrides( + None, {"OPA_DECISION_LOG_ENABLED": False, "OPA_DECISION_LOG_INGRESS_ROUTE": ROUTE} + ) + + assert settings == {"enabled": False, "route": ROUTE} + assert f"{ENV_NAME} is not set for this PDP; using the control plane's value (False)." in logged + + +@pytest.mark.parametrize("env_value", ["", "maybe"], ids=["empty", "not-a-boolean"]) +def test_a_pdp_value_that_is_not_a_boolean_is_ignored_with_a_warning(env_value: str): + settings, logged = _apply_overrides( + env_value, {"OPA_DECISION_LOG_ENABLED": False, "OPA_DECISION_LOG_INGRESS_ROUTE": ROUTE} + ) + + assert settings == {"enabled": False, "route": ROUTE} + message = ( + f"{ENV_NAME} is set to {env_value!r}, which is not true, false, 1 or 0; " + "using the control plane's value (False)." + ) + assert [line for line in logged.splitlines() if message in line and "WARNING" in line] + + +def test_a_control_plane_that_sends_no_value_leaves_the_pdp_value_alone(): + settings, logged = _apply_overrides("false", {"OPA_DECISION_LOG_INGRESS_ROUTE": ROUTE}) + + assert settings == {"enabled": False, "route": ROUTE} + assert ENV_NAME not in logged diff --git a/horizon/tests/test_enforcer_api.py b/horizon/tests/test_enforcer_api.py index b90167a8..4c012693 100644 --- a/horizon/tests/test_enforcer_api.py +++ b/horizon/tests/test_enforcer_api.py @@ -6,9 +6,10 @@ from typing import TYPE_CHECKING import aiohttp +import httpx import pytest from aioresponses import aioresponses -from fastapi import FastAPI, Response +from fastapi import FastAPI, HTTPException, Request, Response from fastapi.testclient import TestClient from loguru import logger from opal_client.client import OpalClient @@ -16,7 +17,7 @@ from starlette import status from horizon.config import sidecar_config -from horizon.enforcer.api import log_query_result, log_query_result_kong, stats_manager +from horizon.enforcer.api import extract_pdp_api_key, log_query_result, log_query_result_kong, stats_manager from horizon.enforcer.schemas import ( AuthorizationQuery, Resource, @@ -133,6 +134,22 @@ def test_enforcer_endpoint_malformed_header_is_401_not_500(endpoint, value): client = TestClient(sidecar._app) response = client.post(endpoint, headers={"authorization": value}, json={}) assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert value.strip() not in response.text + + +@pytest.mark.parametrize( + "authorization", + [f"Bearer {sidecar_config.API_KEY} trailing-part", sidecar_config.API_KEY], + ids=["three-parts", "no-scheme"], +) +def test_extract_pdp_api_key_401_does_not_repeat_the_header(authorization: str): + request = Request({"type": "http", "headers": [(b"authorization", authorization.encode())]}) + + with pytest.raises(HTTPException) as raised: + extract_pdp_api_key(request) + + assert raised.value.status_code == status.HTTP_401_UNAUTHORIZED + assert raised.value.detail == "bad authz header" def test_health_endpoint_is_public(monkeypatch): @@ -366,6 +383,119 @@ def test_nginx_allowed_without_a_required_header_is_422_and_never_asks_opa(monke assert [error["loc"] for error in response.json()["detail"]] == [["header", missing]] +DOCUMENTS_URL = "https://api.example.com/documents" +DOC_ID_RULE = {"url": DOCUMENTS_URL + "?id={doc_id}", "http_method": "get", "action": "read", "resource": "document"} +DOC_7_RULE_OVER_CATCH_ALL = [ + {"url": DOCUMENTS_URL + "?id=7", "http_method": "get", "action": "edit", "resource": "document", "priority": 10}, + {"url": DOCUMENTS_URL, "http_method": "get", "action": "read", "resource": "document", "priority": 1}, +] + + +def _post_allowed_url(monkeypatch, url: str, mapping_rules: list[dict]) -> tuple[httpx.Response, list[dict]]: + """POST /allowed_url for ``url`` with OPA holding ``mapping_rules`` and allowing every check. + + Returns the response and the input of each check the PDP asked OPA to decide. + """ + monkeypatch.setattr(stats_manager, "_messages", asyncio.Queue()) + opa_data_url = f"{opal_client_config.POLICY_STORE_URL}/v1/data" + query = UrlAuthorizationQuery(user=User(key="user1"), http_method="GET", url=url, tenant="default") + with aioresponses() as m: + m.post(f"{opa_data_url}/mapping_rules", payload={"result": {"all": mapping_rules}}) + m.post(f"{opa_data_url}/permit/root", payload={"result": {"allow": True}}, repeat=True) + response = TestClient(sidecar._app).post( + "/allowed_url", headers={"authorization": f"Bearer {sidecar_config.API_KEY}"}, json=query.dict() + ) + checks = [ + json.loads(call.kwargs["data"])["input"] + for (_, requested_url), calls in m.requests.items() + if requested_url.path.endswith("/permit/root") + for call in calls + ] + return response, checks + + +@pytest.mark.parametrize( + "query_string", + ["?id=7", "?id=7&id=7", "?z=1?id=5&id=7"], + ids=["single", "repeated-same-value", "question-mark-in-another-value"], +) +def test_allowed_url_checks_the_value_of_the_query_parameter_the_rule_reads(monkeypatch, query_string: str): + response, checks = _post_allowed_url(monkeypatch, DOCUMENTS_URL + query_string, [DOC_ID_RULE]) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["allow"] is True + (check,) = checks + assert check["resource"]["attributes"] == {"doc_id": "7"} + + +@pytest.mark.parametrize("query_string", ["?id=7&id=8", "?id=8&id=7"], ids=["rule-value-first", "rule-value-last"]) +@pytest.mark.parametrize( + "mapping_rules", [[DOC_ID_RULE], DOC_7_RULE_OVER_CATCH_ALL], ids=["attribute-rule", "literal-rule-over-catch-all"] +) +def test_allowed_url_with_conflicting_values_for_a_rule_query_parameter_is_not_allowed( + monkeypatch, query_string: str, mapping_rules: list[dict] +): + """The app behind the URL may read either value, so no single check covers the request.""" + response, checks = _post_allowed_url(monkeypatch, DOCUMENTS_URL + query_string, mapping_rules) + + assert response.status_code == status.HTTP_200_OK + body = response.json() + assert body["allow"] is False + assert body["result"] is False + assert body["debug"] == {"reason": "Query parameter 'id' has more than one distinct value in the requested URL"} + assert checks == [] + + +@pytest.mark.parametrize("tag_rule_first", [True, False], ids=["tag-rule-listed-first", "tag-rule-listed-last"]) +def test_allowed_url_checks_a_higher_priority_rule_that_reads_no_conflicting_parameter( + monkeypatch, *, tag_rule_first: bool +): + """tag=x&tag=y could select the lower-priority tag rule, but the id rule comes first either way.""" + tag_rule = {"url": DOCUMENTS_URL + "?tag=x", "http_method": "get", "action": "tag", "resource": "document"} + mapping_rules = [{**DOC_ID_RULE, "priority": 10}, {**tag_rule, "priority": 1}] + if tag_rule_first: + mapping_rules.reverse() + + response, checks = _post_allowed_url(monkeypatch, DOCUMENTS_URL + "?id=7&tag=x&tag=y", mapping_rules) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["allow"] is True + (check,) = checks + assert check["action"] == "read" + assert check["resource"]["attributes"] == {"doc_id": "7"} + + +DOC_ID_REGEX_RULE = { + "url": r"^https://api\.example\.com/documents\?id=(?P\d+)", + "url_type": "regex", + "http_method": "get", + "action": "read", + "resource": "document", +} + + +def test_allowed_url_checks_the_value_a_regex_rule_captures(monkeypatch): + response, checks = _post_allowed_url(monkeypatch, DOCUMENTS_URL + "?id=7&tag=a&tag=b", [DOC_ID_REGEX_RULE]) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["allow"] is True + (check,) = checks + assert check["resource"]["attributes"] == {"doc_id": "7"} + + +@pytest.mark.parametrize( + "query_string", ["?id=7&id=8", "?id=8&id=7"], ids=["captured-value-first", "captured-value-last"] +) +def test_allowed_url_with_conflicting_values_for_a_regex_rule_is_not_allowed(monkeypatch, query_string: str): + response, checks = _post_allowed_url(monkeypatch, DOCUMENTS_URL + query_string, [DOC_ID_REGEX_RULE]) + + assert response.status_code == status.HTTP_200_OK + body = response.json() + assert body["allow"] is False + assert body["debug"] == {"reason": "Query parameter 'id' has more than one distinct value in the requested URL"} + assert checks == [] + + ALLOWED_ENDPOINTS = [ ( "/allowed", diff --git a/horizon/tests/test_facts_client.py b/horizon/tests/test_facts_client.py index d1e2a370..ab125657 100644 --- a/horizon/tests/test_facts_client.py +++ b/horizon/tests/test_facts_client.py @@ -1,5 +1,5 @@ from collections.abc import Iterator -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -14,13 +14,13 @@ from loguru import Record -def _make_request(headers: dict[str, str] | None = None) -> FastApiRequest: +def _make_request(headers: dict[str, str] | None = None, query_string: bytes = b"") -> FastApiRequest: scope = { "type": "http", "method": "POST", "path": "/facts/users", "raw_path": b"/facts/users", - "query_string": b"", + "query_string": query_string, "headers": [(k.lower().encode(), v.encode()) for k, v in (headers or {}).items()], } @@ -90,6 +90,80 @@ async def test_send_forward_request_propagates_consistent_update_kwarg(): assert sent_request.headers.get("X-Permit-Consistent-Update") == "true" +@pytest.fixture +def facts_environment() -> Iterator[None]: + """The remote config and API key build_forward_request reads, as the PDP has them once started.""" + remote_config = MagicMock() + remote_config.context = {"project_id": "proj1", "env_id": "env1"} + with ( + patch("horizon.facts.client.get_remote_config", return_value=remote_config), + patch("horizon.facts.client.get_env_api_key", return_value="test_api_key"), + ): + yield + + +def _values_by_key(items: list[tuple[str, str]]) -> dict[str, list[str]]: + values: dict[str, list[str]] = {} + for key, value in items: + values.setdefault(key, []).append(value) + return values + + +async def _forwarded_query(query_string: bytes, query_params: dict[str, Any] | None = None) -> dict[str, list[str]]: + """Each query parameter of the request the PDP forwards for one with ``query_string``, with its decoded + values in order.""" + request = _make_request(query_string=query_string) + forward_request = await FactsClient().build_forward_request(request, "/role_assignments", query_params=query_params) + return _values_by_key(forward_request.url.params.multi_items()) + + +@pytest.mark.usefixtures("facts_environment") +@pytest.mark.parametrize( + "query_string", + [ + b"", + b"user=u1", + b"user=u1&user=u2", + b"user=u2&role=r1&user=u1&tenant=t1&role=r2&search=b&tenant=t2&search=a", + b"user=u1&user=u1", + b"search=a%20b&search=c%26d&search=e%3Df&search=&search=%C3%A9", + ], + ids=["none", "single", "repeated", "interleaved-unsorted", "repeated-same-value", "encoded-values"], +) +@pytest.mark.asyncio +async def test_build_forward_request_forwards_every_query_parameter_value_in_order(query_string: bytes): + incoming = _make_request(query_string=query_string).query_params.multi_items() + + assert await _forwarded_query(query_string) == _values_by_key(incoming) + + +@pytest.mark.usefixtures("facts_environment") +@pytest.mark.asyncio +async def test_build_forward_request_keeps_each_repeated_value_and_its_order(): + """The generic test above compares against the request's own parsing; pin the decoded values once.""" + forwarded = await _forwarded_query(b"user=u2&role=r1&user=u1&search=c%26d&search=&search=%C3%A9") + + assert forwarded == {"user": ["u2", "u1"], "role": ["r1"], "search": ["c&d", "", "\u00e9"]} + + +@pytest.mark.usefixtures("facts_environment") +@pytest.mark.parametrize( + ("query_string", "expected"), + [ + (b"", {"return_deleted": ["true"]}), + (b"user=u1&user=u2", {"user": ["u1", "u2"], "return_deleted": ["true"]}), + (b"return_deleted=false&user=u1&return_deleted=0&user=u2", {"user": ["u1", "u2"], "return_deleted": ["true"]}), + ], + ids=["no-request-params", "request-without-the-key", "request-sets-the-key-twice"], +) +@pytest.mark.asyncio +async def test_build_forward_request_query_params_replace_every_request_value_for_their_key( + query_string: bytes, expected: dict[str, list[str]] +): + """The router's return_deleted=True must win over whatever the caller sent for return_deleted.""" + assert await _forwarded_query(query_string, query_params={"return_deleted": True}) == expected + + @pytest.fixture def logged_errors() -> Iterator[list["Record"]]: """Every loguru record at ERROR or above emitted during the test.""" diff --git a/horizon/tests/test_facts_router.py b/horizon/tests/test_facts_router.py index 430313c6..4737b8c4 100644 --- a/horizon/tests/test_facts_router.py +++ b/horizon/tests/test_facts_router.py @@ -219,6 +219,40 @@ def test_facts_write_with_an_invalid_wait_header_is_400_before_forwarding(facts, facts.subscriber.publish_and_wait.assert_not_awaited() +def test_facts_read_forwards_every_value_of_a_repeated_query_parameter(facts): + """GET /facts/role_assignments?user=a&user=b lists both users' assignments: permitio/PDP#299.""" + response = facts.client.get("/facts/role_assignments?user=u2&tenant=t1&user=u1&role=r1&role=r2", headers=AUTH) + + assert response.status_code == status.HTTP_200_OK + (backend_call,) = facts.backend_calls + assert backend_call.url.path == "/v2/facts/proj/env/role_assignments" + params = backend_call.url.params + assert sorted(params.keys()) == ["role", "tenant", "user"] + assert params.get_list("user") == ["u2", "u1"] + assert params.get_list("tenant") == ["t1"] + assert params.get_list("role") == ["r1", "r2"] + + +@pytest.mark.parametrize( + ("method", "path"), + [("DELETE", "/facts/role_assignments"), ("DELETE", "/facts/users/user-1/roles")], + ids=["role-assignments", "user-roles"], +) +def test_facts_unassign_sends_return_deleted_true_whatever_the_caller_sent(facts, method, path): + facts.client.request( + method, + f"{path}?return_deleted=false&tenant=t1&tenant=t2&return_deleted=0", + headers=AUTH, + json={"user": "user-1", "role": "viewer", "tenant": "t1"}, + ) + + (backend_call,) = facts.backend_calls + params = backend_call.url.params + assert sorted(params.keys()) == ["return_deleted", "tenant"] + assert params.get_list("return_deleted") == ["true"] + assert params.get_list("tenant") == ["t1", "t2"] + + @pytest.fixture def logged_warnings() -> Iterator[list["Record"]]: """Every loguru record at WARNING or above emitted during the test.""" diff --git a/horizon/tests/test_helm_chart.py b/horizon/tests/test_helm_chart.py new file mode 100644 index 00000000..36dcea98 --- /dev/null +++ b/horizon/tests/test_helm_chart.py @@ -0,0 +1,174 @@ +"""Render tests for the Helm chart in charts/pdp. + +They live here because the required `pytests` job runs exactly +`pytest -s --cache-clear horizon/tests/`, and that job installs Helm for them. Without Helm on +PATH they skip locally and fail in CI, so a runner that lost Helm cannot pass them silently. +""" + +import os +import shutil +import subprocess +from pathlib import Path +from typing import Any + +import pytest +import yaml + +CHART = Path(__file__).resolve().parents[2] / "charts" / "pdp" +TEMPLATE = ["template", "pdp", str(CHART), "--namespace", "pdp"] +EXISTING_SECRET = {"existingApiKeySecret": {"name": "pdp-api-key", "key": "api-key"}} +USER_PROVIDED_SECRET = { + "userProvidedSecret": True, + "pdpEnvs": [{"name": "PDP_API_KEY", "value": "vault:secret/data/pdp#api-key"}], +} +SCHEDULING_KEYS = {"nodeSelector", "tolerations", "affinity"} +SCHEDULING = { + "nodeSelector": {"pool": "pdp"}, + "tolerations": [{"key": "dedicated", "operator": "Equal", "value": "pdp", "effect": "NoSchedule"}], + "affinity": { + "nodeAffinity": { + "requiredDuringSchedulingIgnoredDuringExecution": { + "nodeSelectorTerms": [{"matchExpressions": [{"key": "pool", "operator": "In", "values": ["pdp"]}]}] + } + } + }, +} + + +def _helm() -> str: + helm = shutil.which("helm") + if helm is None: + message = "helm is not on PATH: install Helm to run the chart tests" + if os.environ.get("CI"): + pytest.fail(message) + pytest.skip(message) + return helm + + +def _helm_run(tmp_path: Path, command: list[str], values: dict[str, Any] | None) -> subprocess.CompletedProcess[str]: + args = [_helm(), *command] + if values is not None: + values_file = tmp_path / "values.yaml" + values_file.write_text(yaml.safe_dump(values), encoding="utf-8") + args += ["--values", str(values_file)] + return subprocess.run(args, capture_output=True, text=True, check=False) + + +def _render(tmp_path: Path, values: dict[str, Any] | None = None) -> list[dict[str, Any]]: + result = _helm_run(tmp_path, TEMPLATE, values) + assert result.returncode == 0, result.stderr + return [doc for doc in yaml.safe_load_all(result.stdout) if doc] + + +def _of_kind(docs: list[dict[str, Any]], kind: str) -> list[dict[str, Any]]: + return [doc for doc in docs if doc["kind"] == kind] + + +def _pod_spec(docs: list[dict[str, Any]]) -> dict[str, Any]: + [deployment] = _of_kind(docs, "Deployment") + return deployment["spec"]["template"]["spec"] + + +def _pdp_env(docs: list[dict[str, Any]], *names: str) -> list[dict[str, Any]]: + """The PDP container's env entries with the given names, in render order.""" + [container] = [c for c in _pod_spec(docs)["containers"] if c["name"] == "permitio-pdp"] + return [entry for entry in container.get("env") or [] if entry["name"] in names] + + +@pytest.mark.parametrize( + "values", + [ + pytest.param(None, id="defaults"), + pytest.param({"pdp": SCHEDULING}, id="scheduling"), + pytest.param({"pdp": EXISTING_SECRET}, id="existing-secret"), + pytest.param({"pdp": USER_PROVIDED_SECRET}, id="user-provided-secret"), + pytest.param({"pdp": {"userProvidedSecret": True}}, id="user-provided-secret-without-envs"), + ], +) +def test_chart_lints_clean(tmp_path, values): + result = _helm_run(tmp_path, ["lint", "--strict", str(CHART)], values) + assert result.returncode == 0, result.stdout + result.stderr + + +def test_default_render_sets_no_scheduling_constraints(tmp_path): + assert not SCHEDULING_KEYS & _pod_spec(_render(tmp_path)).keys() + + +def test_scheduling_values_reach_the_pod_spec(tmp_path): + pod = _pod_spec(_render(tmp_path, {"pdp": SCHEDULING})) + assert {key: pod[key] for key in SCHEDULING_KEYS} == SCHEDULING + + +@pytest.mark.parametrize("key", sorted(SCHEDULING_KEYS)) +def test_each_scheduling_value_renders_on_its_own(tmp_path, key): + pod = _pod_spec(_render(tmp_path, {"pdp": {key: SCHEDULING[key]}})) + assert pod[key] == SCHEDULING[key] + assert not (SCHEDULING_KEYS - {key}) & pod.keys() + + +@pytest.mark.parametrize( + ("values", "volumes"), + [ + pytest.param( + {"pdp": {"logs_forwarder": {"enabled": True}}}, ["fluent-bit-config", "logs"], id="logs-forwarder" + ), + pytest.param({"openshift": {"enabled": True}}, ["tmp-volume", "opa-volume"], id="openshift"), + ], +) +def test_scheduling_values_sit_beside_the_optional_volumes(tmp_path, values, volumes): + pod = _pod_spec(_render(tmp_path, {**values, "pdp": {**values.get("pdp", {}), **SCHEDULING}})) + assert {key: pod[key] for key in SCHEDULING_KEYS} == SCHEDULING + assert [volume["name"] for volume in pod["volumes"]] == volumes + + +@pytest.mark.parametrize( + "values", + [ + pytest.param(None, id="defaults"), + pytest.param({"pdp": {"userProvidedSecret": False}}, id="user-provided-secret-off"), + # A null in a values file removes the key, which must read as false. + pytest.param({"pdp": {"userProvidedSecret": None}}, id="user-provided-secret-null"), + ], +) +def test_api_key_comes_from_the_chart_secret_by_default(tmp_path, values): + docs = _render(tmp_path, values) + [secret] = _of_kind(docs, "Secret") + assert secret["metadata"]["name"] == "permitio-pdp-secret" + assert set(secret["data"]) == {"ApiKey"} + assert _pdp_env(docs, "PDP_API_KEY") == [ + {"name": "PDP_API_KEY", "valueFrom": {"secretKeyRef": {"name": "permitio-pdp-secret", "key": "ApiKey"}}} + ] + + +def test_existing_secret_replaces_the_chart_secret(tmp_path): + docs = _render(tmp_path, {"pdp": EXISTING_SECRET}) + assert not _of_kind(docs, "Secret") + assert _pdp_env(docs, "PDP_API_KEY") == [ + {"name": "PDP_API_KEY", "valueFrom": {"secretKeyRef": {"name": "pdp-api-key", "key": "api-key"}}} + ] + + +def test_user_provided_secret_leaves_the_api_key_to_pdp_envs(tmp_path): + docs = _render(tmp_path, {"pdp": USER_PROVIDED_SECRET}) + assert not _of_kind(docs, "Secret") + assert _pdp_env(docs, "PDP_API_KEY") == [{"name": "PDP_API_KEY", "value": "vault:secret/data/pdp#api-key"}] + + +def test_user_provided_secret_without_pdp_envs_sets_no_api_key(tmp_path): + docs = _render(tmp_path, {"pdp": {"userProvidedSecret": True}}) + assert not _of_kind(docs, "Secret") + assert not _pdp_env(docs, "PDP_API_KEY") + + +def test_user_provided_secret_and_existing_secret_together_fail_to_render(tmp_path): + result = _helm_run(tmp_path, TEMPLATE, {"pdp": {**EXISTING_SECRET, **USER_PROVIDED_SECRET}}) + assert result.returncode != 0 + assert "pdp.userProvidedSecret and pdp.existingApiKeySecret cannot both be set" in result.stderr + + +@pytest.mark.parametrize("value", ["false", "true"]) +def test_user_provided_secret_given_as_a_string_fails_to_render(tmp_path, value): + # A quoted "false" is truthy in a template, so without this check it would drop PDP_API_KEY. + result = _helm_run(tmp_path, TEMPLATE, {"pdp": {"userProvidedSecret": value}}) + assert result.returncode != 0 + assert f'pdp.userProvidedSecret must be true or false, got the string "{value}"' in result.stderr diff --git a/horizon/tests/test_mapping_rules_utils.py b/horizon/tests/test_mapping_rules_utils.py index 85e3f810..b86b9770 100644 --- a/horizon/tests/test_mapping_rules_utils.py +++ b/horizon/tests/test_mapping_rules_utils.py @@ -4,7 +4,7 @@ from pydantic import AnyHttpUrl, parse_obj_as from horizon.enforcer.schemas import MappingRuleData, UrlTypes -from horizon.enforcer.utils.mapping_rules_utils import MappingRulesUtils +from horizon.enforcer.utils.mapping_rules_utils import ConflictingQueryParameterError, MappingRulesUtils BASE = "https://api.example.com/documents" @@ -66,3 +66,246 @@ def test_a_request_matches_a_rule_whose_query_it_satisfies(rule_query: str, requ ) def test_a_request_does_not_match_a_rule_whose_query_it_lacks(rule_query: str, request_query: str): assert not _matches(BASE + rule_query, BASE + request_query) + + +@pytest.mark.parametrize( + ("rule_query", "request_query"), + [ + ("?id=1", "?id=1&id=1"), + ("?id=1", "?id=%31&id=1"), + ("?id={id}", "?id=7&id=7"), + ("?id=1", "?id=1&page=2&page=3"), + ("", "?id=1&id=2"), + ], + ids=[ + "literal-repeated-same-value", + "literal-same-value-once-decoded", + "attribute-repeated-same-value", + "repeated-param-the-rule-does-not-read", + "rule-reads-no-query", + ], +) +def test_a_query_parameter_repeated_with_one_value_matches_as_if_given_once(rule_query: str, request_query: str): + assert _matches(BASE + rule_query, BASE + request_query) + + +@pytest.mark.parametrize( + ("rule_query", "request_query"), + [ + ("?id=1", "?id=2&id=1"), + ("?id=1", "?id=1&id=2"), + ("?id=1", "?id=1&id="), + ("?id={id}", "?id=1&id=2"), + ("?team=a&id=1", "?team=a&id=1&id=2"), + ], + ids=["last-value-is-the-rules", "first-value-is-the-rules", "other-value-blank", "attribute", "with-other-params"], +) +def test_a_rule_reading_a_query_parameter_with_conflicting_values_raises(rule_query: str, request_query: str): + """The answer is the same whichever order the values come in: the app behind the URL may read + either one, so neither reading decides whether the rule applies.""" + with pytest.raises(ConflictingQueryParameterError) as raised: + _matches(BASE + rule_query, BASE + request_query) + assert raised.value.key == "id" + + +@pytest.mark.parametrize( + ("rule_query", "request_query"), + [ + ("?id=3", "?id=1&id=2"), + ("?team=a&id=1", "?team=b&id=1&id=2"), + ("?team=a&id={id}", "?id=1&id=2"), + ], + ids=["no-value-is-the-rules", "another-param-differs", "another-param-missing"], +) +def test_a_rule_no_value_could_satisfy_does_not_match_even_with_conflicting_values(rule_query: str, request_query: str): + assert not _matches(BASE + rule_query, BASE + request_query) + + +def _rule(url: str, priority: int | None = None) -> MappingRuleData: + return MappingRuleData(url=url, http_method="get", resource="document", action="read", priority=priority) + + +@pytest.mark.parametrize("request_query", ["?admin=true&admin=false", "?admin=false&admin=true"]) +def test_conflicting_values_do_not_fall_through_to_a_rule_that_ignores_the_parameter(request_query: str): + """Skipping only the rule on ?admin=true would hand the request to the catch-all rule, whatever + value the app goes on to read.""" + rules = [_rule(BASE + "?admin=true", priority=10), _rule(BASE, priority=1)] + url = parse_obj_as(AnyHttpUrl, BASE + request_query) + + with pytest.raises(ConflictingQueryParameterError): + MappingRulesUtils.extract_mapping_rule_by_request(rules, "GET", url) + + +TAG_X_RULE = _rule(BASE + "?tag=x") +ID_RULE = _rule(BASE + "?id={id}") +TAGGED_REQUEST = BASE + "?id=1&tag=x&tag=y" + + +@pytest.mark.parametrize( + ("rules", "expected_rule"), + [ + ([_rule(ID_RULE.url, priority=10), _rule(TAG_X_RULE.url, priority=1)], 0), + ([_rule(TAG_X_RULE.url, priority=1), _rule(ID_RULE.url, priority=10)], 1), + ([ID_RULE, TAG_X_RULE], 0), + ], + ids=[ + "clean-rule-ranks-higher-listed-first", + "clean-rule-ranks-higher-listed-last", + "equal-priority-clean-listed-first", + ], +) +def test_a_rule_without_conflicts_that_comes_first_decides_whatever_value_a_lower_rule_reads( + rules: list[MappingRuleData], expected_rule: int +): + """tag=x&tag=y could select the tag rule, but the id rule comes first under either reading.""" + url = parse_obj_as(AnyHttpUrl, TAGGED_REQUEST) + + assert MappingRulesUtils.extract_mapping_rule_by_request(rules, "GET", url) is rules[expected_rule] + + +@pytest.mark.parametrize( + "rules", + [ + [_rule(TAG_X_RULE.url, priority=10), _rule(ID_RULE.url, priority=1)], + [_rule(ID_RULE.url, priority=1), _rule(TAG_X_RULE.url, priority=10)], + [TAG_X_RULE, ID_RULE], + ], + ids=["conflicting-rule-ranks-higher-listed-first", "conflicting-rule-ranks-higher-listed-last", "equal-priority"], +) +def test_a_rule_with_conflicting_values_that_comes_first_raises_over_a_rule_without_conflicts( + rules: list[MappingRuleData], +): + """Read as tag=x the tag rule wins, read as tag=y the id rule does: the reading decides.""" + url = parse_obj_as(AnyHttpUrl, TAGGED_REQUEST) + + with pytest.raises(ConflictingQueryParameterError) as raised: + MappingRulesUtils.extract_mapping_rule_by_request(rules, "GET", url) + assert raised.value.key == "tag" + + +def test_conflicting_values_matter_only_for_rules_on_the_requested_path(): + rules = [_rule("https://api.example.com/other?id=1"), _rule(BASE)] + url = parse_obj_as(AnyHttpUrl, BASE + "?id=1&id=2") + + assert MappingRulesUtils.extract_mapping_rule_by_request(rules, "GET", url) is rules[1] + + +REGEX_BASE = r"^https://api\.example\.com/documents" + + +@pytest.mark.parametrize( + ("rule_regex", "request_query", "key"), + [ + (REGEX_BASE + r"\?id=1", "?id=1&id=2", "id"), + (REGEX_BASE + r"\?id=1", "?id=2&id=1", "id"), + (REGEX_BASE + r"\?id=(?P\d+)", "?id=1&id=2", "id"), + (REGEX_BASE + r".*[?&]admin=true", "?admin=false&admin=true", "admin"), + (REGEX_BASE + r".*[?&]admin=true", "?admin=true&page=1&admin=false", "admin"), + (REGEX_BASE + r".*[?&]id=(?P\d+)", "?tag=a&tag=b&id=1&id=2", "tag"), + ], + ids=[ + "literal-value-first", + "literal-value-last", + "named-group", + "anywhere-value-last", + "anywhere-value-first", + "names-the-first-repeated-param", + ], +) +def test_a_regex_rule_whose_answer_depends_on_the_value_read_raises(rule_regex: str, request_query: str, key: str): + """A pattern is matched against the raw URL, so the PDP cannot tell which parameters it reads. It + tries the readings of an app taking the first and one taking the last value of each repeated + parameter, and a difference in the match or in a named group is a conflict.""" + with pytest.raises(ConflictingQueryParameterError) as raised: + _matches(rule_regex, BASE + request_query, UrlTypes.REGEX) + assert raised.value.key == key + + +@pytest.mark.parametrize( + ("rule_regex", "request_query", "expected"), + [ + (REGEX_BASE, "?tag=a&tag=b", True), + (REGEX_BASE + r"\?id=1", "?id=1&id=1", True), + (REGEX_BASE + r"\?id=(?P\d+)", "?id=1&tag=a&tag=b", True), + (REGEX_BASE + r"\?id=1", "?id=1&id=2&id=1", True), + (REGEX_BASE + "$", "?tag=a&tag=b", False), + (r"^https://api\.example\.com/other", "?id=1&id=2", False), + ], + ids=[ + "catch-all", + "repeated-same-value", + "repeated-param-the-pattern-does-not-read", + "first-and-last-values-agree", + "pattern-ends-before-the-query", + "another-path", + ], +) +def test_a_regex_rule_whose_answer_is_the_same_for_every_value_read_matches_as_before( + rule_regex: str, request_query: str, *, expected: bool +): + assert _matches(rule_regex, BASE + request_query, UrlTypes.REGEX) is expected + + +@pytest.mark.parametrize(("regex_priority", "conflict"), [(1, False), (20, True)], ids=["ranks-lower", "ranks-higher"]) +def test_a_regex_rule_with_conflicting_values_matters_only_when_it_comes_first(regex_priority: int, *, conflict: bool): + rules = [ + MappingRuleData( + url=REGEX_BASE + r".*[?&]tag=x", + http_method="get", + resource="document", + action="tag", + url_type=UrlTypes.REGEX, + priority=regex_priority, + ), + _rule(ID_RULE.url, priority=10), + ] + url = parse_obj_as(AnyHttpUrl, TAGGED_REQUEST) + + if conflict: + with pytest.raises(ConflictingQueryParameterError): + MappingRulesUtils.extract_mapping_rule_by_request(rules, "GET", url) + else: + assert MappingRulesUtils.extract_mapping_rule_by_request(rules, "GET", url) is rules[1] + + +@pytest.mark.parametrize( + ("rule_url", "request_url", "expected"), + [ + (BASE + "?id={doc_id}", BASE + "?id=7", {"doc_id": "7"}), + (BASE + "?id={doc_id}", BASE + "?id=7&id=7", {"doc_id": "7"}), + (BASE + "?id={doc_id}", BASE + "?id=7&page=1&page=2", {"doc_id": "7"}), + (BASE + "?id=7", BASE + "?id=7&id=8", {}), + (BASE, BASE + "?id=7&id=8", {}), + (BASE + "?id={doc_id}", BASE, {}), + ], + ids=[ + "single", + "repeated-same-value", + "repeated-param-the-rule-does-not-read", + "rule-reads-no-attribute", + "rule-without-query", + "request-without-query", + ], +) +def test_query_attributes_come_from_the_parameters_value(rule_url: str, request_url: str, expected: dict): + assert MappingRulesUtils.extract_attributes_from_query_params(rule_url, request_url) == expected + + +@pytest.mark.parametrize( + ("request_query", "expected"), + [("?z=1?id=5&id=7", {"doc_id": "7"}), ("?id=1?id=2", {"doc_id": "1?id=2"})], + ids=["question-mark-in-another-value", "question-mark-in-the-value"], +) +def test_query_attributes_come_from_the_query_the_rule_matched(request_query: str, expected: dict): + """Everything after the first "?" is the query, both for matching a rule and for reading its attributes.""" + rule_url = BASE + "?id={doc_id}" + + assert _matches(rule_url, BASE + request_query) + assert MappingRulesUtils.extract_attributes_from_query_params(rule_url, BASE + request_query) == expected + + +@pytest.mark.parametrize("request_query", ["?id=7&id=8", "?id=8&id=7", "?id=7&id="]) +def test_query_attributes_raise_for_a_parameter_with_conflicting_values(request_query: str): + with pytest.raises(ConflictingQueryParameterError) as raised: + MappingRulesUtils.extract_attributes_from_query_params(BASE + "?id={doc_id}", BASE + request_query) + assert raised.value.key == "id" diff --git a/horizon/tests/test_opa_config.py b/horizon/tests/test_opa_config.py new file mode 100644 index 00000000..4d06ce51 --- /dev/null +++ b/horizon/tests/test_opa_config.py @@ -0,0 +1,220 @@ +"""What the PDP hands OPA: the OPA config file, and the inline OPA config OPAL starts OPA with. + +OPA reads decision log and plugin settings only from its config file. The file used to be written +only with decision logs on, so a PDP with them off never loaded its plugins (permit_graph among +them) and its ReBAC user permissions came back empty. +""" + +import itertools +from collections.abc import Iterator +from pathlib import Path +from types import SimpleNamespace +from typing import NamedTuple + +import pytest +import yaml +from loguru import logger +from opal_client.config import EngineLogFormat, opal_client_config, opal_common_config +from opal_client.engine.options import OpaServerOptions + +from horizon.config import sidecar_config +from horizon.enforcer.opa import config_maker +from horizon.enforcer.opa.config_maker import get_opa_config_file_path +from horizon.pdp import OPA_LOGGER_MODULE, PermitPDP + +API_KEY = "test-pdp-api-key" +BACKEND_TIER = "https://logs.example.test" +PLUGINS: dict[str, dict[str, int | bool | str]] = { + "permit_graph": {}, + "envoy_ext_authz_grpc": {"addr": ":9191", "path": "permit/root"}, +} + + +class OpaSettings(NamedTuple): + decision_logs: bool + plugins: dict[str, dict[str, int | bool | str]] + bearer_required: bool + console: bool = False + + @property + def id(self) -> str: + return "-".join( + [ + "decision-logs" if self.decision_logs else "no-decision-logs", + "plugins" if self.plugins else "no-plugins", + "bearer-required" if self.bearer_required else "bearer-optional", + "console" if self.console else "no-console", + ] + ) + + +ALL_SETTINGS = [ + OpaSettings(decision_logs, plugins, bearer_required, console) + for decision_logs, plugins, bearer_required, console in itertools.product( + [True, False], [PLUGINS, {}], [True, False], [True, False] + ) +] +CONFIG_FILE_SETTINGS = [ + settings for settings in ALL_SETTINGS if settings.decision_logs or settings.plugins or settings.console +] + + +@pytest.fixture +def opa_files(monkeypatch, tmp_path) -> SimpleNamespace: + """Write the OPA files under tmp_path, and answer the API key lookup without a control plane.""" + files = SimpleNamespace(config=tmp_path / "opa" / "config.yaml", authz=tmp_path / "opa" / "basic-authz.rego") + monkeypatch.setattr(sidecar_config, "OPA_CONFIG_FILE_PATH", str(files.config)) + monkeypatch.setattr(sidecar_config, "OPA_AUTH_POLICY_FILE_PATH", str(files.authz)) + monkeypatch.setattr(sidecar_config, "OPA_DECISION_LOG_INGRESS_BACKEND_TIER_URL", BACKEND_TIER) + monkeypatch.setattr(config_maker, "get_env_api_key", lambda: API_KEY) + monkeypatch.setattr("horizon.pdp.get_env_api_key", lambda: API_KEY) + return files + + +@pytest.fixture +def inline_opa_config(monkeypatch) -> OpaServerOptions: + """The image's OPAL_INLINE_OPA_CONFIG. The OPAL settings the PDP may change are restored after.""" + shipped = OpaServerOptions(v0_compatible=True) + monkeypatch.setattr(opal_client_config, "INLINE_OPA_CONFIG", shipped) + monkeypatch.setattr(opal_client_config, "POLICY_STORE_AUTH_TOKEN", opal_client_config.POLICY_STORE_AUTH_TOKEN) + monkeypatch.setattr(opal_client_config, "POLICY_STORE_AUTH_TYPE", opal_client_config.POLICY_STORE_AUTH_TYPE) + monkeypatch.setattr(opal_client_config, "INLINE_OPA_LOG_FORMAT", EngineLogFormat.NONE) + monkeypatch.setattr(opal_common_config, "LOG_MODULE_EXCLUDE_LIST", ["uvicorn", OPA_LOGGER_MODULE]) + return shipped + + +@pytest.fixture +def logged_warnings() -> Iterator[list[str]]: + """Every loguru message at WARNING or above emitted during the test.""" + messages: list[str] = [] + sink_id = logger.add(lambda message: messages.append(message.record["message"]), level="WARNING") + yield messages + logger.remove(sink_id) + + +def _configure(monkeypatch, settings: OpaSettings) -> None: + monkeypatch.setattr(sidecar_config, "OPA_DECISION_LOG_ENABLED", settings.decision_logs) + monkeypatch.setattr(sidecar_config, "OPA_PLUGINS", settings.plugins) + monkeypatch.setattr(sidecar_config, "OPA_BEARER_TOKEN_REQUIRED", settings.bearer_required) + monkeypatch.setattr(sidecar_config, "OPA_DECISION_LOG_CONSOLE", settings.console) + + +def _expected_config(settings: OpaSettings) -> dict: + expected: dict = {} + if settings.decision_logs: + expected["services"] = { + "permit_io": {"url": BACKEND_TIER, "credentials": {"bearer": {"token": API_KEY}}}, + } + if settings.decision_logs or settings.console: + expected["decision_logs"] = {} + if settings.console: + expected["decision_logs"]["console"] = True + if settings.decision_logs: + expected["decision_logs"] |= { + "service": "permit_io", + "resource": sidecar_config.OPA_DECISION_LOG_INGRESS_ROUTE, + "reporting": { + "min_delay_seconds": sidecar_config.OPA_DECISION_LOG_MIN_DELAY, + "max_delay_seconds": sidecar_config.OPA_DECISION_LOG_MAX_DELAY, + "upload_size_limit_bytes": sidecar_config.OPA_DECISION_LOG_UPLOAD_SIZE_LIMIT, + }, + } + if settings.plugins: + # A plugin with an empty config is written as `permit_graph:`, which YAML reads as null. + expected["plugins"] = { + plugin_id: plugin_config or None for plugin_id, plugin_config in settings.plugins.items() + } + return expected + + +def _configure_inline_opa_config() -> OpaServerOptions: + PermitPDP._configure_inline_opa_config() + return opal_client_config.INLINE_OPA_CONFIG + + +@pytest.mark.parametrize("settings", ALL_SETTINGS, ids=lambda settings: settings.id) +def test_the_config_file_holds_what_is_enabled(monkeypatch, opa_files, settings: OpaSettings): + # Whether callers of OPA need a bearer token does not change the file: the permit_io + # credentials are what OPA uploads decision logs with. + _configure(monkeypatch, settings) + + path = get_opa_config_file_path(sidecar_config) + + assert path == str(opa_files.config) + contents = Path(path).read_text() + assert (yaml.safe_load(contents) or {}) == _expected_config(settings) + assert (API_KEY in contents) == settings.decision_logs + + +@pytest.mark.usefixtures("inline_opa_config") +@pytest.mark.parametrize("settings", CONFIG_FILE_SETTINGS, ids=lambda settings: settings.id) +def test_opa_gets_the_config_file_when_decision_logs_console_or_plugins_are_on( + monkeypatch, opa_files, settings: OpaSettings +): + _configure(monkeypatch, settings) + + configured = _configure_inline_opa_config() + + assert configured.config_file == str(opa_files.config) + assert yaml.safe_load(opa_files.config.read_text()) == _expected_config(settings) + assert configured.authentication == ("token" if settings.bearer_required else "off") + assert configured.v0_compatible is True # the image's own setting is kept + # Console decision logs reach the PDP's output only with OPA's log lines shown. + shown = opal_client_config.INLINE_OPA_LOG_FORMAT == EngineLogFormat.FULL + assert shown == settings.console + assert (OPA_LOGGER_MODULE not in opal_common_config.LOG_MODULE_EXCLUDE_LIST) == settings.console + + +@pytest.mark.usefixtures("inline_opa_config") +def test_a_required_bearer_token_alone_writes_no_config_file(monkeypatch, opa_files): + _configure(monkeypatch, OpaSettings(decision_logs=False, plugins={}, bearer_required=True)) + + configured = _configure_inline_opa_config() + + assert configured.config_file is None + assert not opa_files.config.exists() + assert configured.authentication == "token" + assert configured.files == [str(opa_files.authz)] + assert configured.v0_compatible is True # the image's own setting is kept + + +def test_with_nothing_to_add_the_inline_config_is_left_as_it_is(monkeypatch, opa_files, inline_opa_config): + _configure(monkeypatch, OpaSettings(decision_logs=False, plugins={}, bearer_required=False)) + + configured = _configure_inline_opa_config() + + assert configured is inline_opa_config + assert not opa_files.config.exists() + + +@pytest.mark.usefixtures("inline_opa_config") +def test_a_different_config_file_in_the_inline_config_is_replaced_with_a_warning( + monkeypatch, opa_files, logged_warnings +): + user_config_file = "/etc/opa/user-config.yaml" + monkeypatch.setattr( + opal_client_config, "INLINE_OPA_CONFIG", OpaServerOptions(v0_compatible=True, config_file=user_config_file) + ) + _configure(monkeypatch, OpaSettings(decision_logs=False, plugins=PLUGINS, bearer_required=False)) + + configured = _configure_inline_opa_config() + + assert configured.config_file == str(opa_files.config) + assert len(logged_warnings) == 1 + assert user_config_file in logged_warnings[0] + assert str(opa_files.config) in logged_warnings[0] + + +@pytest.mark.usefixtures("inline_opa_config") +def test_the_pdp_config_file_already_in_the_inline_config_is_kept_without_a_warning( + monkeypatch, opa_files, logged_warnings +): + monkeypatch.setattr( + opal_client_config, "INLINE_OPA_CONFIG", OpaServerOptions(v0_compatible=True, config_file=str(opa_files.config)) + ) + _configure(monkeypatch, OpaSettings(decision_logs=False, plugins=PLUGINS, bearer_required=False)) + + configured = _configure_inline_opa_config() + + assert configured.config_file == str(opa_files.config) + assert logged_warnings == [] diff --git a/horizon/tests/test_proxy_api.py b/horizon/tests/test_proxy_api.py new file mode 100644 index 00000000..f9c10be5 --- /dev/null +++ b/horizon/tests/test_proxy_api.py @@ -0,0 +1,57 @@ +"""Query parameters the /cloud and /sdk proxy routes forward to the backend.""" + +import re + +import pytest +from aioresponses import aioresponses +from fastapi import FastAPI +from fastapi.testclient import TestClient +from starlette import status +from yarl import URL + +from horizon.config import sidecar_config +from horizon.proxy.api import router + +CLOUD_BACKEND = "http://backend.test/cloud-api" +LEGACY_BACKEND = "http://backend.test/legacy-api" +AUTH = {"Authorization": "Bearer proxy-test-token"} + + +@pytest.fixture +def proxy_client(monkeypatch) -> TestClient: + """The proxy routes alone, pointed at a backend that aioresponses answers.""" + monkeypatch.setattr(sidecar_config, "BACKEND_SERVICE_URL", CLOUD_BACKEND) + monkeypatch.setattr(sidecar_config, "BACKEND_LEGACY_URL", LEGACY_BACKEND) + app = FastAPI() + app.include_router(router) + return TestClient(app) + + +@pytest.mark.parametrize(("route", "backend"), [("/cloud", CLOUD_BACKEND), ("/sdk", LEGACY_BACKEND)]) +@pytest.mark.parametrize( + ("query_string", "expected"), + [ + ("", []), + ("?user=u1", [("user", "u1")]), + ( + "?user=u2&tenant=t1&user=u1&search=b%26c&search=a", + [("user", "u2"), ("tenant", "t1"), ("user", "u1"), ("search", "b&c"), ("search", "a")], + ), + ], + ids=["none", "single", "repeated"], +) +def test_proxy_forwards_every_query_parameter_value_in_order( + proxy_client: TestClient, route: str, backend: str, query_string: str, expected: list[tuple[str, str]] +): + with aioresponses() as backend_mock: + backend_mock.get(re.compile(rf"^{re.escape(backend)}/v2/role_assignments"), payload=[]) + + response = proxy_client.get(f"{route}/v2/role_assignments{query_string}", headers=AUTH) + + assert response.status_code == status.HTTP_200_OK + ((_, recorded_url), calls) = next(iter(backend_mock.requests.items())) + # aioresponses records the URL with its query sorted, so it shows that no value was dropped. + # aiohttp builds the query it sends from the params it was given, which show the order. + assert sorted(recorded_url.query.items()) == sorted(expected) + sent_query = URL(backend).with_query(calls[0].kwargs["params"]).query + assert list(sent_query.items()) == expected diff --git a/pdp-server/Cargo.toml b/pdp-server/Cargo.toml index 9bf1017e..7dcc1f35 100644 --- a/pdp-server/Cargo.toml +++ b/pdp-server/Cargo.toml @@ -22,6 +22,7 @@ serde = { version = "1.0.219", features = ["derive"] } serde_json = "1.0.140" serde_yaml = "0.9" sha2 = "0.11.0" +subtle = "2.6.1" thiserror = "2.0.12" tokio = { version = "1.44.1", features = ["full"] } url = "2.5.4" diff --git a/pdp-server/src/api/authn_middleware.rs b/pdp-server/src/api/authn_middleware.rs index 83cbf7b8..1d7f7b16 100644 --- a/pdp-server/src/api/authn_middleware.rs +++ b/pdp-server/src/api/authn_middleware.rs @@ -7,6 +7,7 @@ use axum::{ response::Response, }; use log::warn; +use subtle::ConstantTimeEq; pub(super) async fn authentication_middleware( State(state): State, @@ -32,8 +33,8 @@ pub(super) async fn authentication_middleware( // Remove the "Bearer " prefix header_str[7..].to_string() } - Ok(header_str) => { - warn!("Invalid Authorization header format, missing 'Bearer ' prefix: {header_str}"); + Ok(_) => { + warn!("Invalid Authorization header format, missing 'Bearer ' prefix"); return Response::builder() .status(StatusCode::FORBIDDEN) .body( @@ -55,7 +56,11 @@ pub(super) async fn authentication_middleware( }; // Verify the API key - if api_key != state.config.api_key { + let key_matches: bool = api_key + .as_bytes() + .ct_eq(state.config.api_key.as_bytes()) + .into(); + if !key_matches { warn!("Authentication failed: Invalid API key"); return Response::builder() .status(StatusCode::FORBIDDEN) @@ -70,9 +75,10 @@ pub(super) async fn authentication_middleware( #[cfg(test)] mod tests { use super::*; - use crate::test_utils::TestFixture; + use crate::test_utils::{LogCapture, TestFixture}; use axum::routing::get; use axum::Router; + use http::HeaderValue; use http_body_util::BodyExt; use tower::ServiceExt; @@ -97,6 +103,15 @@ mod tests { /// Helper function to build a request with optional authorization header async fn send_request(app: &Router, auth_header: Option<&str>) -> (StatusCode, String) { + let auth_header = + auth_header.map(|auth| HeaderValue::from_str(auth).expect("Invalid header value")); + send_request_with_header(app, auth_header).await + } + + async fn send_request_with_header( + app: &Router, + auth_header: Option, + ) -> (StatusCode, String) { let mut request_builder = Request::builder().uri(TEST_ROUTE); if let Some(auth) = auth_header { @@ -168,4 +183,79 @@ mod tests { "You are not authorized to access this resource, please check your API key." ); } + + #[tokio::test] + async fn test_lowercase_bearer_scheme_is_accepted() { + let app = setup_authn_mock_app("test_api_key").await; + let (status, body) = send_request(&app, Some("bearer test_api_key")).await; + + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "Authenticated"); + } + + #[tokio::test] + async fn test_right_key_with_another_scheme_is_rejected() { + let app = setup_authn_mock_app("test_api_key").await; + let (status, _) = send_request(&app, Some("Basic test_api_key")).await; + + assert_eq!(status, StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn test_key_that_differs_only_in_length_is_rejected() { + let app = setup_authn_mock_app("test_api_key").await; + + for header in [ + "Bearer ", + "Bearer test_api_ke", + "Bearer test_api_key2", + "Bearer test_api_key ", + ] { + let (status, _) = send_request(&app, Some(header)).await; + assert_eq!(status, StatusCode::FORBIDDEN, "header {header:?}"); + } + } + + /// The Authorization header the caller sent appears in no log line, at any level, whatever + /// the reason the middleware rejects it for. + #[tokio::test] + async fn test_rejected_authorization_header_is_not_logged() { + const PRESENTED: &str = "presented_key_7f3a9c"; + let app = setup_authn_mock_app("test_api_key").await; + let cases = [ + ( + HeaderValue::from_str(&format!("Basic {PRESENTED}")).expect("Invalid header value"), + "Invalid Authorization header format", + ), + ( + HeaderValue::from_str(&format!("Bearer {PRESENTED}")) + .expect("Invalid header value"), + "Authentication failed", + ), + ( + HeaderValue::from_bytes(format!("Bearer {PRESENTED}\u{e9}").as_bytes()) + .expect("Invalid header value"), + "Failed to parse Authorization header", + ), + ]; + + for (header, expected_warning) in cases { + let logs = LogCapture::start(); + + let (status, _) = send_request_with_header(&app, Some(header)).await; + + assert_eq!(status, StatusCode::FORBIDDEN); + let lines = logs.lines(); + assert!( + lines + .iter() + .any(|line| line.starts_with("WARN") && line.contains(expected_warning)), + "expected a WARN line containing {expected_warning:?}, got {lines:?}" + ); + assert!( + lines.iter().all(|line| !line.contains(PRESENTED)), + "a log line repeats the Authorization header: {lines:?}" + ); + } + } } diff --git a/pdp-server/src/test_utils.rs b/pdp-server/src/test_utils.rs index 5bc60e4e..6e66bac7 100644 --- a/pdp-server/src/test_utils.rs +++ b/pdp-server/src/test_utils.rs @@ -7,6 +7,7 @@ use http::{Method, Request, StatusCode}; use http_body_util::BodyExt; use log::LevelFilter; use serde::{de::DeserializeOwned, Serialize}; +use std::cell::RefCell; use tower::ServiceExt; use wiremock::matchers; use wiremock::Mock; @@ -82,11 +83,7 @@ impl TestFixture { /// } /// ``` pub async fn new() -> Self { - // Initialize test logger - let _ = env_logger::builder() - .filter_level(LevelFilter::Debug) - .is_test(true) - .try_init(); + Self::setup_logger(LevelFilter::Debug); // Create mock servers let opa_mock = MockServer::start().await; @@ -239,10 +236,14 @@ impl TestFixture { /// } /// ``` pub fn setup_logger(level: LevelFilter) { - let _ = env_logger::builder() + let inner = env_logger::builder() .filter_level(level) .is_test(true) - .try_init(); + .build(); + let max_level = inner.filter(); + if log::set_boxed_logger(Box::new(TestLogger { inner })).is_ok() { + log::set_max_level(max_level); + } } /// Creates a request builder with pre-configured headers. @@ -707,3 +708,80 @@ impl TestResponse { String::from_utf8_lossy(&self.body).to_string() } } + +thread_local! { + static CAPTURED_LOG_LINES: RefCell>> = const { RefCell::new(None) }; +} + +/// The logger [`TestFixture::setup_logger`] installs: env_logger, which prints through the test +/// harness's output capture, plus the per-thread collection behind [`LogCapture`]. +struct TestLogger { + inner: env_logger::Logger, +} + +impl log::Log for TestLogger { + fn enabled(&self, metadata: &log::Metadata) -> bool { + self.inner.enabled(metadata) + } + + fn log(&self, record: &log::Record) { + CAPTURED_LOG_LINES.with_borrow_mut(|lines| { + if let Some(lines) = lines { + lines.push(format!("{} {}", record.level(), record.args())); + } + }); + self.inner.log(record); + } + + fn flush(&self) { + self.inner.flush(); + } +} + +/// Collects the log lines emitted on the current thread, from [`LogCapture::start`] until it is +/// dropped. +/// +/// `#[tokio::test]` runs on a single-threaded runtime, so a request a test sends through a router +/// with `oneshot` is handled on the test's thread and its log lines are the test's alone. +pub struct LogCapture; + +impl LogCapture { + /// Starts collecting at every level, whatever level the logger was installed with. That level + /// still filters what env_logger prints, so other tests' output does not change. + pub fn start() -> Self { + TestFixture::setup_logger(LevelFilter::Debug); + log::set_max_level(LevelFilter::Trace); + CAPTURED_LOG_LINES.set(Some(Vec::new())); + Self + } + + /// Each line collected so far, as "LEVEL message". + pub fn lines(&self) -> Vec { + CAPTURED_LOG_LINES.with_borrow(|lines| lines.clone().unwrap_or_default()) + } +} + +impl Drop for LogCapture { + fn drop(&mut self) { + CAPTURED_LOG_LINES.set(None); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Another test may have installed the logger first with a higher level, as the health + /// handler tests do with `Info`; a capture still sees every level. + #[test] + fn test_log_capture_sees_every_level_whatever_logger_came_first() { + TestFixture::setup_logger(LevelFilter::Info); + log::set_max_level(LevelFilter::Info); + + let logs = LogCapture::start(); + log::debug!("debug line"); + log::trace!("trace line"); + + assert_eq!(logs.lines(), ["DEBUG debug line", "TRACE trace line"]); + } +} diff --git a/pyproject.toml b/pyproject.toml index e7ab4c7d..9530a211 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,8 @@ [project] -name = "horizon" +# The distribution name; the import package is still `horizon` (horizon/). Not "horizon": GitHub's +# dependency graph matched that name to an unrelated PyPI project and raised its alerts here +# (PER-16930). Nothing reads this name: the image and CI export the lock with --no-emit-project. +name = "permit-pdp" version = "0.2.0" description = "The Python side of the Permit.io PDP: OPAL client, policy-store sync and the PDP API." # uv.lock is resolved for the interpreter the image ships (Dockerfile: python:3.13-alpine3.23) @@ -81,8 +84,8 @@ dev = [ "pytest-timeout", "aioresponses", # Imported by .github/scripts/check_waiver_parity.py, which horizon/tests/test_ci_scripts.py - # loads at collection time, so a missing PyYAML fails the whole pytests job. A test and - # CI-tooling dependency only; nothing under horizon/ imports it. + # loads at collection time, and by horizon/tests/test_helm_chart.py, so a missing PyYAML fails + # the whole pytests job. A test and CI-tooling dependency only; no runtime code imports it. "PyYAML", # Here, unlike ruff, because it types third-party imports from the project's .venv; the # `ty-check` hook runs it with `uv run`. Pinned exactly: every ty release adds checks, so a diff --git a/uv.lock b/uv.lock index 02610e4b..d56f3071 100644 --- a/uv.lock +++ b/uv.lock @@ -559,83 +559,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/04/4b/29cac41a4d98d144bf5f6d33995617b185d14b22401f75ca86f384e87ff1/h11-0.16.0-py3-none-any.whl", hash = "sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86", size = 37515, upload-time = "2025-04-24T03:35:24.344Z" }, ] -[[package]] -name = "horizon" -version = "0.2.0" -source = { virtual = "." } -dependencies = [ - { name = "aiohttp" }, - { name = "cryptography" }, - { name = "ddtrace" }, - { name = "fastapi" }, - { name = "fastapi-websocket-pubsub" }, - { name = "fastapi-websocket-rpc" }, - { name = "gitpython" }, - { name = "gunicorn" }, - { name = "httpx" }, - { name = "jinja2" }, - { name = "logzio-python-handler" }, - { name = "opal-client" }, - { name = "opal-common" }, - { name = "protobuf" }, - { name = "pydantic", extra = ["email"] }, - { name = "requests" }, - { name = "scalar-fastapi" }, - { name = "starlette" }, - { name = "tenacity" }, - { name = "typer" }, - { name = "urllib3" }, - { name = "uvicorn", extra = ["standard"] }, - { name = "websockets" }, -] - -[package.dev-dependencies] -dev = [ - { name = "aioresponses" }, - { name = "pytest" }, - { name = "pytest-asyncio" }, - { name = "pytest-timeout" }, - { name = "pyyaml" }, - { name = "ty" }, -] - -[package.metadata] -requires-dist = [ - { name = "aiohttp", specifier = ">=3.14.3,<4" }, - { name = "cryptography", specifier = ">=50.0.0,<51" }, - { name = "ddtrace", specifier = "==3.19.8" }, - { name = "fastapi", specifier = ">=0.124.0,<1" }, - { name = "fastapi-websocket-pubsub", specifier = "==1.0.1" }, - { name = "fastapi-websocket-rpc", specifier = "==0.1.29" }, - { name = "gitpython", specifier = ">=3.1.59,<4" }, - { name = "gunicorn", specifier = ">=23.0.0,<24" }, - { name = "httpx", specifier = ">=0.27.0,<1" }, - { name = "jinja2", specifier = ">=3.1.2,<4" }, - { name = "logzio-python-handler" }, - { name = "opal-client", specifier = "==0.9.6" }, - { name = "opal-common", specifier = "==0.9.6" }, - { name = "protobuf", specifier = ">=7.36.2" }, - { name = "pydantic", extras = ["email"], specifier = ">=1.9.1,<2" }, - { name = "requests", specifier = ">=2.32.4,<3" }, - { name = "scalar-fastapi", specifier = "==1.8.2" }, - { name = "starlette", specifier = "==0.50.0" }, - { name = "tenacity", specifier = ">=8.0.1,<9" }, - { name = "typer", specifier = ">=0.27.2,<1" }, - { name = "urllib3", specifier = ">=2.8.0,<3" }, - { name = "uvicorn", extras = ["standard"], specifier = ">=0.54.0,<1" }, - { name = "websockets", specifier = "==17.0" }, -] - -[package.metadata.requires-dev] -dev = [ - { name = "aioresponses" }, - { name = "pytest" }, - { name = "pytest-asyncio" }, - { name = "pytest-timeout" }, - { name = "pyyaml" }, - { name = "ty", specifier = "==0.0.84" }, -] - [[package]] name = "httpcore" version = "1.0.9" @@ -947,6 +870,83 @@ redis = [ { name = "asyncio-redis" }, ] +[[package]] +name = "permit-pdp" +version = "0.2.0" +source = { virtual = "." } +dependencies = [ + { name = "aiohttp" }, + { name = "cryptography" }, + { name = "ddtrace" }, + { name = "fastapi" }, + { name = "fastapi-websocket-pubsub" }, + { name = "fastapi-websocket-rpc" }, + { name = "gitpython" }, + { name = "gunicorn" }, + { name = "httpx" }, + { name = "jinja2" }, + { name = "logzio-python-handler" }, + { name = "opal-client" }, + { name = "opal-common" }, + { name = "protobuf" }, + { name = "pydantic", extra = ["email"] }, + { name = "requests" }, + { name = "scalar-fastapi" }, + { name = "starlette" }, + { name = "tenacity" }, + { name = "typer" }, + { name = "urllib3" }, + { name = "uvicorn", extra = ["standard"] }, + { name = "websockets" }, +] + +[package.dev-dependencies] +dev = [ + { name = "aioresponses" }, + { name = "pytest" }, + { name = "pytest-asyncio" }, + { name = "pytest-timeout" }, + { name = "pyyaml" }, + { name = "ty" }, +] + +[package.metadata] +requires-dist = [ + { name = "aiohttp", specifier = ">=3.14.3,<4" }, + { name = "cryptography", specifier = ">=50.0.0,<51" }, + { name = "ddtrace", specifier = "==3.19.8" }, + { name = "fastapi", specifier = ">=0.124.0,<1" }, + { name = "fastapi-websocket-pubsub", specifier = "==1.0.1" }, + { name = "fastapi-websocket-rpc", specifier = "==0.1.29" }, + { name = "gitpython", specifier = ">=3.1.59,<4" }, + { name = "gunicorn", specifier = ">=23.0.0,<24" }, + { name = "httpx", specifier = ">=0.27.0,<1" }, + { name = "jinja2", specifier = ">=3.1.2,<4" }, + { name = "logzio-python-handler" }, + { name = "opal-client", specifier = "==0.9.6" }, + { name = "opal-common", specifier = "==0.9.6" }, + { name = "protobuf", specifier = ">=7.36.2" }, + { name = "pydantic", extras = ["email"], specifier = ">=1.9.1,<2" }, + { name = "requests", specifier = ">=2.32.4,<3" }, + { name = "scalar-fastapi", specifier = "==1.8.2" }, + { name = "starlette", specifier = "==0.50.0" }, + { name = "tenacity", specifier = ">=8.0.1,<9" }, + { name = "typer", specifier = ">=0.27.2,<1" }, + { name = "urllib3", specifier = ">=2.8.0,<3" }, + { name = "uvicorn", extras = ["standard"], specifier = ">=0.54.0,<1" }, + { name = "websockets", specifier = "==17.0" }, +] + +[package.metadata.requires-dev] +dev = [ + { name = "aioresponses" }, + { name = "pytest" }, + { name = "pytest-asyncio" }, + { name = "pytest-timeout" }, + { name = "pyyaml" }, + { name = "ty", specifier = "==0.0.84" }, +] + [[package]] name = "pluggy" version = "1.6.0"