Merge pull request #15 from bento-platform/feat/new-evaluate-api

feat!: new evaluation API for returning a matrix of resources/permissions

.github/workflows/build.yml

@@ -19,12 +19,12 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           submodules: "recursive"
 
       - name: Run Bento build action
-        uses: bento-platform/[email protected].0
+        uses: bento-platform/[email protected].1
         with:
           registry: ghcr.io
           registry-username: ${{ github.actor }}

.github/workflows/lint.yml

@@ -11,7 +11,7 @@ jobs:
       matrix:
         python-version: [ "3.11" ]
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: actions/setup-python@v4
         name: Set up Python
         with:

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
         run: |
           docker exec ${{ job.services.postgres.id }} sh -c 'echo "max_connections=200" >> /var/lib/postgresql/data/postgresql.conf'
           docker kill --signal=SIGHUP ${{ job.services.postgres.id }}
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: actions/setup-python@v4
         name: Set up Python
         with:

Dockerfile

@@ -1,8 +1,8 @@
-FROM ghcr.io/bento-platform/bento_base_image:python-debian-2023.07.17
+FROM ghcr.io/bento-platform/bento_base_image:python-debian-2023.11.10
 
 # Use uvicorn (instead of hypercorn) in production since I've found
 # multiple benchmarks showing it to be faster - David L
-RUN pip install --no-cache-dir -U pip && pip install --no-cache-dir "uvicorn[standard]==0.20.0"
+RUN pip install --no-cache-dir -U pip && pip install --no-cache-dir "uvicorn[standard]==0.24.0"
 
 WORKDIR /authorization
 

README.md

@@ -73,36 +73,83 @@ the user will be treated as `{"anonymous": true}`.
 
 #### `POST /policy/evaluate` - The main evaluation endpoint
 
-Implementers MUST use this when making *binary* authorization decisions, e.g., does User A have the 
-`query:data` permission for Resource B.
+Implementers MUST use either this or the scalar version of this endpoint when making *binary* authorization decisions, 
+e.g., does User A have the `query:data` permission for Resource B.
 
-Implementers SHOULD use this when making graceful-fallback policy decisions, via a multiple-requests approach, e.g.:
+Implementers SHOULD use this when making graceful-fallback policy decisions, via the matrix-based approach and 
+additional logic in the implementers' own code, e.g.:
 
 * "does User A have the `query:data` permission for Resource B"? 
-* If not, "do they have the `dataset_level_counts` permission for Resource B?"
+* If not, "do they have the `query:dataset_level_counts` permission for Resource B?"
 * *et cetera.*
 
-##### Request body example (JSON)
+##### Request example
+
+###### Headers
+
+```
+Authorization: Bearer ...
+```
+
+###### Body (JSON)
 
 ```json
 {
-  "requested_resource": {"everything": true},
-  "required_permissions": ["query:data"]
+  "resources": [{"project": "project-1"}, {"project": "project-2"}, {"project": "project-3"}],
+  "permissions": ["query:data", "query:dataset_level_counts"]
 }
 ```
 
-The `requested_resource` field can also be an **array** of resources.
-
 ##### Response (JSON)
 
 ```json
 {
-  "result": true
+  "result": [
+    [false, true],
+    [false, false],
+    [true, true]
+  ]
 }
 ```
 
-If `requested_resource` is an array of resources, `result` would instead be returned as a **list of booleans**.
+Here, `result` is a matrix of evaluation results, with rows being resources and columns being permissions.
 
+In this case, the response can be interpreted as:
+
+* For the provided bearer token:
+  * They have the `query:dataset_level_counts` permission for `project-1`, but NOT `query:data`
+  * They have none of the queried permissions for `project-2`
+  * They have both of the queried permissions for `project-3`
+
+
+#### `POST /policy/evaluate_one` - The evaluation endpoint for one resource and one 
+
+Equivalent to the above endpoint, but with a request body that looks like:
+
+```json
+{
+  "resource": {"project": "project-1"},
+  "permission": "query:data"
+}
+```
+
+... and a response that looks like:
+
+```json
+{
+  "result": false
+}
+```
+
+We added this endpoint to prevent slip-ups when checking just one permission, since if the implementer accidentally
+checks the falsiness of a list, they'll accidentally grant access incorrectly, since the following holds:
+
+```python
+if [[False]]:
+    print("this will print")
+```
+
+
 #### `POST /policy/permissions` - a secondary evaluation endpoint
 
 This endpoint lists permissions that apply to a particular token/resource pair.
@@ -117,22 +164,19 @@ which the user does not have the permissions to use.
 
 ```json
 {
-  "requested_resource": {"everything": true}
+  "resources": [{"everything": true}]
 }
 ```
 
-The `requested_resource` field can also be an **array** of resources.
-
 ##### Response (JSON)
 
 ```json
 {
-  "result": ["query:data"]
+  "result": [["query:data"]]
 }
 ```
 
-If `requested_resource` is an array of resources, `result` would instead be returned as a 
-**list of lists of permissions**.
+The `result` value is returned as a **list of lists of permissions**; one list of permissions for each resource queried.
 
 
 ### Group endpoints

bento_authorization_service/cli.py

@@ -4,13 +4,13 @@
 import sys
 import types
 
+from bento_lib.auth.permissions import PERMISSIONS, PERMISSIONS_BY_STRING
 from typing import Any, Callable, Coroutine
 
 from . import __version__
 from .config import Config, get_config
 from .db import Database, get_db
 from .models import GrantModel, GroupModel, SubjectModel, ResourceModel, RESOURCE_EVERYTHING
-from .policy_engine.permissions import PERMISSIONS, PERMISSIONS_BY_STRING
 from .utils import json_model_dump_kwargs
 
 
@@ -50,7 +50,7 @@ async def list_cmd(_config: Config, db: Database, args):
 
 
 async def create_grant(_config: Config, db: Database, args) -> int:
-    g, created = await db.create_grant(
+    g = await db.create_grant(
         GrantModel(
             subject=SubjectModel.model_validate_json(getattr(args, "subject", "null")),
             resource=ResourceModel.model_validate_json(getattr(args, "resource", "null")),
@@ -60,7 +60,7 @@ async def create_grant(_config: Config, db: Database, args) -> int:
         )
     )
 
-    if created:
+    if g:
         print(f"Grant successfully created: {g}")
         return 0
 
@@ -154,7 +154,7 @@ async def delete_cmd(_config: Config, db: Database, args) -> int:
 
 
 async def assign_all_cmd(_config: Config, db: Database, args) -> int:
-    g, created = await db.create_grant(
+    g = await db.create_grant(
         GrantModel(
             subject=SubjectModel.model_validate({"iss": args.iss, "sub": args.sub}),
             resource=RESOURCE_EVERYTHING,
@@ -164,7 +164,7 @@ async def assign_all_cmd(_config: Config, db: Database, args) -> int:
         )
     )
 
-    if created:
+    if g:
         print(f"Grant successfully created: {g}")
         return 0
 

bento_authorization_service/config.py

@@ -18,7 +18,7 @@
 class CorsOriginsParsingSource(EnvSettingsSource):
     def prepare_field_value(self, field_name: str, field: FieldInfo, value: Any, value_is_complex: bool) -> Any:
         if field_name == "cors_origins":
-            return tuple(x.strip() for x in value.split(";"))
+            return tuple(x.strip() for x in value.split(";")) if value is not None else ()
         return json.loads(value) if value_is_complex else value
 
 

bento_authorization_service/db.py

@@ -202,7 +202,7 @@ async def get_grants(self) -> tuple[StoredGrantModel, ...]:
             )
             return tuple(grant_db_deserialize(r) for r in res)
 
-    async def create_grant(self, grant: GrantModel) -> tuple[int | None, bool]:  # id, created
+    async def create_grant(self, grant: GrantModel) -> int | None:
         conn: asyncpg.Connection
         async with self.connect() as conn:
             async with conn.transaction():
@@ -229,9 +229,9 @@ async def create_grant(self, grant: GrantModel) -> tuple[int | None, bool]:  # i
                         )
 
                 except AssertionError:  # Failed for some reason
-                    return None, False
+                    return None
 
-                return res, res is not None
+                return res
 
     async def delete_grant(self, grant_id: int) -> None:
         conn: asyncpg.Connection

bento_authorization_service/json_schemas.py

@@ -1,4 +1,3 @@
-import jsonschema
 from bento_lib.search import queries as q
 
 from .config import get_config

bento_authorization_service/main.py

@@ -1,31 +1,39 @@
 import asyncio
 
+from bento_lib.responses.fastapi_errors import http_exception_handler_factory, validation_exception_handler_factory
+from bento_lib.types import BentoExtraServiceInfo
 from fastapi import FastAPI, Request, Response, status
+from fastapi.exceptions import RequestValidationError
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.responses import JSONResponse
+from starlette.exceptions import HTTPException as StarletteHTTPException
 
-from bento_authorization_service import __version__
-from bento_lib.types import BentoExtraServiceInfo
-
+from . import __version__
 from .config import ConfigDependency, get_config
 from .constants import BENTO_SERVICE_KIND, SERVICE_TYPE
 from .logger import logger
 from .routers.grants import grants_router
 from .routers.groups import groups_router
 from .routers.policy import policy_router
 from .routers.schemas import schema_router
-from .routers.utils import public_endpoint_dependency
+from .routers.utils import MarkAuthzDone, public_endpoint_dependency
 
 
+# TODO: Find a way to DI this
+config_for_setup = get_config()
+
 app = FastAPI()
 app.add_middleware(
     CORSMiddleware,
-    allow_origins=get_config().cors_origins,
+    allow_origins=config_for_setup.cors_origins,
     allow_headers=["Authorization"],
     allow_credentials=True,
     allow_methods=["*"],
 )
 
+app.exception_handler(StarletteHTTPException)(http_exception_handler_factory(logger, MarkAuthzDone))
+app.exception_handler(RequestValidationError)(validation_exception_handler_factory(MarkAuthzDone))
+
 app.include_router(grants_router)
 app.include_router(groups_router)
 app.include_router(policy_router)
@@ -50,6 +58,9 @@ async def permissions_enforcement(request: Request, call_next) -> Response:
 
     if not request.state.determined_authz:
         # Next in response chain didn't properly think about auth; return 403
+        logger.warning(
+            f"Masking true response with 403 since determined_authz was not set: {request.url} {response.status_code}"
+        )
         return JSONResponse(status_code=status.HTTP_403_FORBIDDEN, content={"detail": "Forbidden"})
 
     # Otherwise, return the response as normal