set up pre-commit with black and isort

.editorconfig

@@ -9,4 +9,4 @@ indent_size=2
 trim_trailing_whitespace=true
 
 [*.py]
-indent_size=4
+indent_size=4

.github/workflows/dockerhub_push.yml

@@ -40,7 +40,7 @@ jobs:
           tags: |
             type=ref,event=branch
             type=semver,pattern={{version}}
-      - 
+      -
         name: Echo published tags
         run: |
           echo "Published docker tags: ${{ steps.meta.outputs.tags }}"

.github/workflows/pre-commit.yml

@@ -0,0 +1,14 @@
+name: pre-commit
+
+on:
+  pull_request:
+  push:
+    branches: [master, main]
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - uses: actions/setup-python@v3
+    - uses: pre-commit/action@v2.0.3

.isort.cfg

@@ -0,0 +1,2 @@
+[settings]
+profile=black

.pre-commit-config.yaml

@@ -0,0 +1,15 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.2.0
+    hooks:
+      - id: check-yaml
+      - id: end-of-file-fixer
+      - id: trailing-whitespace
+  - repo: https://github.com/psf/black
+    rev: 22.3.0
+    hooks:
+      - id: black
+  - repo: https://github.com/pycqa/isort
+    rev: 5.10.1
+    hooks:
+      - id: isort

Dockerfile

@@ -72,4 +72,4 @@ EXPOSE 7000
 # expose opa directly
 EXPOSE 8181
 # run gunicorn
-CMD ["/start.sh"]
+CMD ["/start.sh"]

MANIFEST.in

@@ -1 +1 @@
-recursive-include horizon/static *
+recursive-include horizon/static *

README.md

@@ -36,4 +36,4 @@ you must declare the environment variable `READ_ONLY_GITHUB_TOKEN` for this comm
 ```
 DEV_MODE_CLIENT_TOKEN=<CLIENT_TOKEN> make run
 ```
-you must declare the environment variable `DEV_MODE_CLIENT_TOKEN` for this command to work.
+you must declare the environment variable `DEV_MODE_CLIENT_TOKEN` for this command to work.

horizon/authentication.py

@@ -5,7 +5,9 @@
 
 def enforce_pdp_token(authorization=Header(None)):
     if authorization is None:
-        raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Missing Authorization header")
+        raise HTTPException(
+            status.HTTP_401_UNAUTHORIZED, detail="Missing Authorization header"
+        )
     schema, token = authorization.split(" ")
 
     if schema.strip().lower() != "bearer" or token.strip() != sidecar_config.API_KEY:

horizon/config.py

@@ -4,11 +4,19 @@
 
 
 class SidecarConfig(Confi):
-    CONTROL_PLANE = confi.str("CONTROL_PLANE", "http://localhost:8000", description="URL to the control plane that manages this PDP, typically Permit.io cloud (api.permit.io)")
+    CONTROL_PLANE = confi.str(
+        "CONTROL_PLANE",
+        "http://localhost:8000",
+        description="URL to the control plane that manages this PDP, typically Permit.io cloud (api.permit.io)",
+    )
 
     # backend api url, where proxy requests go
-    BACKEND_SERVICE_URL = confi.str("BACKEND_SERVICE_URL", confi.delay("{CONTROL_PLANE}/v1"))
-    BACKEND_LEGACY_URL = confi.str("BACKEND_LEGACY_URL", confi.delay("{CONTROL_PLANE}/sdk"))
+    BACKEND_SERVICE_URL = confi.str(
+        "BACKEND_SERVICE_URL", confi.delay("{CONTROL_PLANE}/v1")
+    )
+    BACKEND_LEGACY_URL = confi.str(
+        "BACKEND_LEGACY_URL", confi.delay("{CONTROL_PLANE}/sdk")
+    )
 
     # backend route to fetch policy data topics
     REMOTE_CONFIG_ENDPOINT = confi.str("REMOTE_CONFIG_ENDPOINT", "pdps/me/config")
@@ -26,55 +34,93 @@ class SidecarConfig(Confi):
     ENABLE_MONITORING = confi.bool("ENABLE_MONITORING", False)
 
     # centralized logging
-    CENTRAL_LOG_DRAIN_URL = confi.str("CENTRAL_LOG_DRAIN_URL", "https://listener.logz.io:8071")
+    CENTRAL_LOG_DRAIN_URL = confi.str(
+        "CENTRAL_LOG_DRAIN_URL", "https://listener.logz.io:8071"
+    )
     CENTRAL_LOG_DRAIN_TIMEOUT = confi.int("CENTRAL_LOG_DRAIN_TIMEOUT", 5)
     CENTRAL_LOG_TOKEN = confi.str("CENTRAL_LOG_TOKEN", None)
     CENTRAL_LOG_ENABLED = confi.bool("CENTRAL_LOG_ENABLED", False)
 
     # internal OPA config
-    OPA_CONFIG_FILE_PATH = confi.str("OPA_CONFIG_FILE_PATH", "~/opa/config.yaml", description="the path on the container for OPA config file")
-    OPA_AUTH_POLICY_FILE_PATH = confi.str("OPA_AUTH_POLICY_FILE_PATH", "~/opa/basic-authz.rego", description="the path on the container for OPA authorization policy (rego file)")
-    OPA_BEARER_TOKEN_REQUIRED = confi.bool("OPA_BEARER_TOKEN_REQUIRED", True, description="if true, all API calls to OPA must provide a bearer token (the value of CLIENT_TOKEN)")
-    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")
-    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)")
-    OPA_DECISION_LOG_INGRESS_ROUTE = confi.str("OPA_DECISION_LOG_INGRESS_ROUTE", "/v1/decision_logs/ingress", description="the route on the backend the decision logs will be uploaded to")
-    OPA_DECISION_LOG_MIN_DELAY = confi.int("OPA_DECISION_LOG_MIN_DELAY", 1, description="min amount of time (in seconds) to wait between decision log uploads")
-    OPA_DECISION_LOG_MAX_DELAY = confi.int("OPA_DECISION_LOG_MAX_DELAY", 10, description="max amount of time (in seconds) to wait between decision log uploads")
+    OPA_CONFIG_FILE_PATH = confi.str(
+        "OPA_CONFIG_FILE_PATH",
+        "~/opa/config.yaml",
+        description="the path on the container for OPA config file",
+    )
+    OPA_AUTH_POLICY_FILE_PATH = confi.str(
+        "OPA_AUTH_POLICY_FILE_PATH",
+        "~/opa/basic-authz.rego",
+        description="the path on the container for OPA authorization policy (rego file)",
+    )
+    OPA_BEARER_TOKEN_REQUIRED = confi.bool(
+        "OPA_BEARER_TOKEN_REQUIRED",
+        True,
+        description="if true, all API calls to OPA must provide a bearer token (the value of CLIENT_TOKEN)",
+    )
+    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",
+    )
+    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)",
+    )
+    OPA_DECISION_LOG_INGRESS_ROUTE = confi.str(
+        "OPA_DECISION_LOG_INGRESS_ROUTE",
+        "/v1/decision_logs/ingress",
+        description="the route on the backend the decision logs will be uploaded to",
+    )
+    OPA_DECISION_LOG_MIN_DELAY = confi.int(
+        "OPA_DECISION_LOG_MIN_DELAY",
+        1,
+        description="min amount of time (in seconds) to wait between decision log uploads",
+    )
+    OPA_DECISION_LOG_MAX_DELAY = confi.int(
+        "OPA_DECISION_LOG_MAX_DELAY",
+        10,
+        description="max amount of time (in seconds) to wait between decision log uploads",
+    )
 
     # temp log format (until cloud config is received)
-    TEMP_LOG_FORMAT = confi.str("TEMP_LOG_FORMAT", "<green>{time}</green> | {process} | <blue>{name: <40}</blue>|<level>{level:^6} | {message}</level>")
+    TEMP_LOG_FORMAT = confi.str(
+        "TEMP_LOG_FORMAT",
+        "<green>{time}</green> | {process} | <blue>{name: <40}</blue>|<level>{level:^6} | {message}</level>",
+    )
 
     # non configurable values -------------------------------------------------
 
     # redoc configuration (openapi schema)
     OPENAPI_TAGS_METADATA = [
         {
             "name": "Authorization API",
-            "description": "Authorization queries to OPA. These queries are answered locally by OPA " + \
-                "and do not require the cloud service. Latency should be very low (< 20ms per query)"
+            "description": "Authorization queries to OPA. These queries are answered locally by OPA "
+            + "and do not require the cloud service. Latency should be very low (< 20ms per query)",
         },
         {
             "name": "Local Queries",
-            "description": "These queries are done locally against the sidecar and do not " + \
-                "involve a network round-trip to Permit.io cloud API. Therefore they are safe " + \
-                "to use with reasonable performance (i.e: with negligible latency) in the context of a user request.",
+            "description": "These queries are done locally against the sidecar and do not "
+            + "involve a network round-trip to Permit.io cloud API. Therefore they are safe "
+            + "to use with reasonable performance (i.e: with negligible latency) in the context of a user request.",
         },
         {
             "name": "Policy Updater",
-            "description": "API to manually trigger and control the local policy caching and refetching."
+            "description": "API to manually trigger and control the local policy caching and refetching.",
         },
         {
             "name": "Cloud API Proxy",
-            "description": "These endpoints proxy the Permit.io cloud api, and therefore **incur high-latency**. " + \
-                "You should not use the cloud API in the standard request flow of users, i.e in places where the incurred " + \
-                "added latency will affect your entire api. A good place to call the cloud API will be in one-time user events " + \
-                "such as user registration (i.e: calling sync user, assigning initial user roles, etc.). " + \
-                "The sidecar will proxy to the cloud every request prefixed with '/sdk'.",
+            "description": "These endpoints proxy the Permit.io cloud api, and therefore **incur high-latency**. "
+            + "You should not use the cloud API in the standard request flow of users, i.e in places where the incurred "
+            + "added latency will affect your entire api. A good place to call the cloud API will be in one-time user events "
+            + "such as user registration (i.e: calling sync user, assigning initial user roles, etc.). "
+            + "The sidecar will proxy to the cloud every request prefixed with '/sdk'.",
             "externalDocs": {
                 "description": "The cloud api complete docs are located here:",
                 "url": "https://api.permit.io/redoc",
             },
-        }
+        },
     ]
 
-sidecar_config = SidecarConfig(prefix="PDP_")
+
+sidecar_config = SidecarConfig(prefix="PDP_")

horizon/enforcer/api.py

@@ -1,17 +1,17 @@
 import json
-from typing import Optional, Dict
+from typing import Dict, Optional
 
-from fastapi import APIRouter, status, Response, Depends
-from opal_client.policy_store import BasePolicyStoreClient, DEFAULT_POLICY_STORE_GETTER
-from opal_client.policy_store.opa_client import fail_silently
+from fastapi import APIRouter, Depends, Response, status
 from opal_client.logger import logger
+from opal_client.policy_store import DEFAULT_POLICY_STORE_GETTER, BasePolicyStoreClient
+from opal_client.policy_store.opa_client import fail_silently
 
 from horizon.authentication import enforce_pdp_token
 from horizon.config import sidecar_config
-
 from horizon.enforcer.schemas import AuthorizationQuery, AuthorizationResult
 
-def init_enforcer_api_router(policy_store:BasePolicyStoreClient=None):
+
+def init_enforcer_api_router(policy_store: BasePolicyStoreClient = None):
     policy_store = policy_store or DEFAULT_POLICY_STORE_GETTER()
     router = APIRouter(dependencies=[Depends(enforce_pdp_token)])
 
@@ -24,12 +24,20 @@ def log_query_and_result(query: AuthorizationQuery, response: Response):
             granting_role = None
             if allowed:
                 granting_permissions = result.get("granting_permission", [])
-                granting_permission = {} if len(granting_permissions) == 0 else granting_permissions[0]
+                granting_permission = (
+                    {} if len(granting_permissions) == 0 else granting_permissions[0]
+                )
                 permission = granting_permission.get("permission", {})
-                granting_role: Optional[Dict] = granting_permission.get("granting_role", None)
+                granting_role: Optional[Dict] = granting_permission.get(
+                    "granting_role", None
+                )
                 if granting_role:
                     role_id = granting_role.get("id", "__NO_ID__")
-                    roles = [r for r in result.get("user_roles", []) if r.get("id", "") == role_id]
+                    roles = [
+                        r
+                        for r in result.get("user_roles", [])
+                        if r.get("id", "") == role_id
+                    ]
                     granting_role = granting_role if not roles else roles[0]
 
             debug = {
@@ -52,53 +60,65 @@ def log_query_and_result(query: AuthorizationQuery, response: Response):
                 allowed=allowed,
                 api_params=params,
                 input=query.dict(),
-                debug=debug
+                debug=debug,
             )
         except:
             try:
                 body = str(response.body, "utf-8")
             except:
                 body = None
-            data = {} if body is None else {"response_body" : body}
-            logger.info("is allowed",
+            data = {} if body is None else {"response_body": body}
+            logger.info(
+                "is allowed",
                 params=params,
                 query=query.dict(),
                 response_status=response.status_code,
                 **data
             )
 
-
-    @router.post("/allowed", response_model=AuthorizationResult, status_code=status.HTTP_200_OK, response_model_exclude_none=True)
+    @router.post(
+        "/allowed",
+        response_model=AuthorizationResult,
+        status_code=status.HTTP_200_OK,
+        response_model_exclude_none=True,
+    )
     async def is_allowed(query: AuthorizationQuery):
         async def _is_allowed():
             return await policy_store.get_data_with_input(path="rbac", input=query)
 
         fallback_response = dict(result=dict(allow=False, debug="OPA not responding"))
-        is_allowed_with_fallback = fail_silently(fallback=fallback_response)(_is_allowed)
+        is_allowed_with_fallback = fail_silently(fallback=fallback_response)(
+            _is_allowed
+        )
         response = await is_allowed_with_fallback()
         log_query_and_result(query, response)
         try:
             raw_result = json.loads(response.body).get("result", {})
             processed_query = raw_result.get("authorization_query", {})
             result = {
                 "allow": raw_result.get("allow", False),
-                "result": raw_result.get("allow", False), # fallback for older sdks (TODO: remove)
+                "result": raw_result.get(
+                    "allow", False
+                ),  # fallback for older sdks (TODO: remove)
                 "query": {
                     "user": processed_query.get("user", {"id": query.user}),
                     "action": processed_query.get("action", query.action),
-                    "resource": processed_query.get("resource", query.resource.dict(exclude_none=True)),
+                    "resource": processed_query.get(
+                        "resource", query.resource.dict(exclude_none=True)
+                    ),
                 },
                 "debug": {
                     "warnings": raw_result.get("debug", []),
                     "user_roles": raw_result.get("user_roles", []),
                     "granting_permission": raw_result.get("granting_permission", []),
                     "user_permissions": raw_result.get("user_permissions", []),
-                }
+                },
             }
         except:
             result = dict(allow=False, result=False)
-            logger.warning("is allowed (fallback response)", reason="cannot decode opa response")
+            logger.warning(
+                "is allowed (fallback response)", reason="cannot decode opa response"
+            )
         return result
 
     return router
-

horizon/enforcer/opa/config_maker.py

@@ -1,7 +1,8 @@
 import os
-import jinja2
 
+import jinja2
 from opal_common.logger import logger
+
 from horizon.config import SidecarConfig
 
 
@@ -17,15 +18,15 @@ def persist_to_file(contents: str, path: str) -> str:
     os.makedirs(os.path.dirname(path), exist_ok=True)
 
     # persist to file
-    with open(path, 'w') as f:
+    with open(path, "w") as f:
         f.write(contents)
 
     return path
 
 
 def get_opa_config_file_path(
     sidecar_config: SidecarConfig,
-    template_path = "config.yaml.template",
+    template_path="config.yaml.template",
 ) -> str:
     """
     renders a template that implements the OPA config file, according to the official spec:
@@ -57,9 +58,10 @@ def get_opa_config_file_path(
 
     return persist_to_file(contents, target_path)
 
+
 def get_opa_authz_policy_file_path(
     sidecar_config: SidecarConfig,
-    template_path = "authz.rego.template",
+    template_path="authz.rego.template",
 ) -> str:
     """
     renders a template that implements a rego policy for OPA authz, as demonstrated here: