From 6c998888af2581d12bf5e876d11831a88f989c56 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Thu, 25 Feb 2021 16:03:17 -0500 Subject: [PATCH] Require authorization for upload and publish APIs [RHELDST-4729] Users must now possess the appropriate role to request uploads or publishes to a particular environment, e.g., "stage-blob-uploader" to upload to the "stage" environment. The development environment was updated to automatically authorizate users for the "test" environment. --- exodus_gw/auth.py | 10 +++++++--- exodus_gw/routers/publish.py | 14 +++++++++++-- exodus_gw/routers/upload.py | 17 ++++++++++++++-- scripts/systemd/exodus-gw-sidecar.service | 8 ++++++++ scripts/systemd/install | 6 ++++++ tests/conftest.py | 24 ++++++++++++++++++++++- tests/routers/test_publish.py | 22 ++++++++++++++------- 7 files changed, 86 insertions(+), 15 deletions(-) diff --git a/exodus_gw/auth.py b/exodus_gw/auth.py index f851cf3b..0ab15a96 100644 --- a/exodus_gw/auth.py +++ b/exodus_gw/auth.py @@ -78,10 +78,14 @@ def needs_role(rolename): > "If caller does not have role xyz, they will never get here." """ - async def check_roles(roles: Set[str] = Depends(caller_roles)): - if rolename not in roles: + async def check_roles( + env: Optional[str] = None, roles: Set[str] = Depends(caller_roles) + ): + role = env + "-" + rolename if env else rolename + + if role not in roles: raise HTTPException( - 403, "this operation requires role '%s'" % rolename + 403, "this operation requires role '%s'" % role ) return Depends(check_roles) diff --git a/exodus_gw/routers/publish.py b/exodus_gw/routers/publish.py index 2baed29f..45d7eb01 100644 --- a/exodus_gw/routers/publish.py +++ b/exodus_gw/routers/publish.py @@ -66,7 +66,7 @@ from fastapi import APIRouter, Body, HTTPException from sqlalchemy.orm import Session -from .. import deps, models, schemas, worker +from .. import auth, deps, models, schemas, worker from ..aws.util import validate_object_key from ..settings import Environment, Settings @@ -100,11 +100,15 @@ }, } }, + dependencies=[auth.needs_role("publisher")], ) def publish( env: Environment = deps.env, db: Session = deps.db ) -> models.Publish: - """Creates and returns a new publish object.""" + """Creates and returns a new publish object. + + **Required roles**: `{env}-publisher` + """ db_publish = models.Publish(id=uuid4(), env=env.name, state="PENDING") db.add(db_publish) @@ -116,6 +120,7 @@ def publish( "/{env}/publish/{publish_id}", status_code=200, response_model=schemas.EmptyResponse, + dependencies=[auth.needs_role("publisher")], ) def update_publish_items( items: Union[schemas.ItemBase, List[schemas.ItemBase]] = Body( @@ -137,6 +142,8 @@ def update_publish_items( ) -> Dict[None, None]: """Add publish items to an existing publish object. + **Required roles**: `{env}-publisher` + Publish items primarily are a mapping between a URI relative to the root of the CDN, and the key of a binary object which should be exposed from that URI. @@ -176,6 +183,7 @@ def update_publish_items( "/{env}/publish/{publish_id}/commit", status_code=200, response_model=schemas.Task, + dependencies=[auth.needs_role("publisher")], ) def commit_publish( publish_id: UUID = schemas.PathPublishId, @@ -185,6 +193,8 @@ def commit_publish( ) -> models.Task: """Commit an existing publish object. + **Required roles**: `{env}-publisher` + Committing a publish has the following effects: - All URIs contained within the publish become accessible from the CDN, diff --git a/exodus_gw/routers/upload.py b/exodus_gw/routers/upload.py index bd03ac2a..d8cf1142 100644 --- a/exodus_gw/routers/upload.py +++ b/exodus_gw/routers/upload.py @@ -67,7 +67,7 @@ from botocore.exceptions import ClientError from fastapi import APIRouter, HTTPException, Path, Query, Request, Response -from .. import deps +from .. import auth, deps from ..aws.client import S3ClientWrapper as s3_client from ..aws.util import ( RequestReader, @@ -96,6 +96,7 @@ "/upload/{env}/{key}", summary="Create/complete multipart upload", response_class=Response, + dependencies=[auth.needs_role("blob-uploader")], ) async def multipart_upload( request: Request, @@ -126,6 +127,8 @@ async def multipart_upload( ): """Create or complete a multi-part upload. + **Required roles**: `{env}-blob-uploader` + To create a multi-part upload: - include ``uploads`` in query string, with no value (e.g. ``POST /upload/{env}/{key}?uploads``) - see also: https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateMultipartUpload.html @@ -156,6 +159,7 @@ async def multipart_upload( "/upload/{env}/{key}", summary="Upload bytes", response_class=Response, + dependencies=[auth.needs_role("blob-uploader")], ) async def upload( request: Request, @@ -170,6 +174,8 @@ async def upload( ): """Write to an object, either as a standalone operation or within a multi-part upload. + **Required roles**: `{env}-blob-uploader` + To upload an entire object: - include all object bytes in request body - see also: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html @@ -283,6 +289,7 @@ async def multipart_put( summary="Abort multipart upload", response_description="Empty response", response_class=Response, + dependencies=[auth.needs_role("blob-uploader")], ) async def abort_multipart_upload( env: Environment = deps.env, @@ -291,6 +298,8 @@ async def abort_multipart_upload( ): """Abort a multipart upload. + **Required roles**: `{env}-blob-uploader` + If an upload cannot be completed, explicitly aborting it is recommended in order to free up resources as early as possible, although this is not mandatory. @@ -312,12 +321,16 @@ async def abort_multipart_upload( "/upload/{env}/{key}", summary="Request head object", response_class=Response, + dependencies=[auth.needs_role("blob-uploader")], ) async def head( env: Environment = deps.env, key: str = Path(..., description="S3 object key"), ): - """Retrieve metadata from an S3 object.""" + """Retrieve metadata from an S3 object. + + **Required roles**: `{env}-blob-uploader` + """ validate_object_key(key) diff --git a/scripts/systemd/exodus-gw-sidecar.service b/scripts/systemd/exodus-gw-sidecar.service index 6f0a3790..8bd0f2d2 100644 --- a/scripts/systemd/exodus-gw-sidecar.service +++ b/scripts/systemd/exodus-gw-sidecar.service @@ -37,6 +37,14 @@ com.redhat.api.platform:\n\ |\n$(sed -r -e 's|^| |' %S/exodus-gw-dev/service.pem)\n\ private-key-pkcs8-pem:\ |\n$(sed -r -e 's|^| |' %S/exodus-gw-dev/service-key.pem)\n\ + security:\n\ + roles:\n\ + test-blob-uploader:\n\ + users:\n\ + byInternalUsername: [${USER}]\n\ + test-publisher:\n\ + users:\n\ + byInternalUsername: [${USER}]\n\ \nEND\n\ " diff --git a/scripts/systemd/install b/scripts/systemd/install index 98f81f3c..8431808c 100755 --- a/scripts/systemd/install +++ b/scripts/systemd/install @@ -66,6 +66,12 @@ EXODUS_GW_SRC_PATH=$(realpath -L ../..) # You'll have to change this too if you change the localstack port. #EXODUS_GW_S3_ENDPOINT_URL=https://localhost:3377 + +# Disable migrations during development +#EXODUS_GW_DB_MIGRATION_MODE=model + +# Drop and recreate tables on restart +#EXODUS_GW_DB_RESET=true END fi diff --git a/tests/conftest.py b/tests/conftest.py index ec1cb3cd..421a0829 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,12 +1,15 @@ +import base64 +import json import os import uuid +from typing import List import mock import pytest from fastapi.testclient import TestClient from sqlalchemy.orm.session import Session -from exodus_gw import database, main, models, schemas, settings # noqa +from exodus_gw import auth, database, main, models, schemas, settings # noqa from .async_utils import BlockDetector @@ -139,3 +142,22 @@ def fake_publish(): ), ] yield publish + + +@pytest.fixture +def auth_header(): + def _auth_header(roles: List[str] = []): + raw_context = { + "user": { + "authenticated": True, + "internalUsername": "fake-user", + "roles": roles, + } + } + + json_context = json.dumps(raw_context).encode("utf-8") + b64_context = base64.b64encode(json_context) + + return {"X-RhApiPlatform-CallContext": b64_context.decode("utf-8")} + + return _auth_header diff --git a/tests/routers/test_publish.py b/tests/routers/test_publish.py index 5d399220..15fdaa66 100644 --- a/tests/routers/test_publish.py +++ b/tests/routers/test_publish.py @@ -19,9 +19,12 @@ "test3", ], ) -def test_publish_env_exists(env, db): +def test_publish_env_exists(env, db, auth_header): with TestClient(app) as client: - r = client.post("/%s/publish" % env) + r = client.post( + "/%s/publish" % env, + headers=auth_header(roles=["%s-publisher" % env]), + ) # Should succeed assert r.ok @@ -33,9 +36,11 @@ def test_publish_env_exists(env, db): assert publishes.count() == 1 -def test_publish_env_doesnt_exist(): +def test_publish_env_doesnt_exist(auth_header): with TestClient(app) as client: - r = client.post("/foo/publish") + r = client.post( + "/foo/publish", headers=auth_header(roles=["foo-publisher"]) + ) # It should fail assert r.status_code == 404 @@ -58,7 +63,7 @@ def test_publish_links(mock_db_session): } -def test_update_publish_items_typical(db): +def test_update_publish_items_typical(db, auth_header): """PUTting some items on a publish creates expected objects in DB.""" publish_id = "11224567-e89b-12d3-a456-426614174000" @@ -85,6 +90,7 @@ def test_update_publish_items_typical(db): "object_key": "2" * 64, }, ], + headers=auth_header(roles=["test-publisher"]), ) # It should have succeeded @@ -106,7 +112,7 @@ def test_update_publish_items_typical(db): ] -def test_update_publish_items_single_item(db): +def test_update_publish_items_single_item(db, auth_header): """PUTting a single item on a publish creates expected object in DB.""" publish_id = "11224567-e89b-12d3-a456-426614174000" @@ -127,6 +133,7 @@ def test_update_publish_items_single_item(db): "web_uri": "/uri1", "object_key": "1" * 64, }, + headers=auth_header(roles=["test-publisher"]), ) # It should have succeeded @@ -144,7 +151,7 @@ def test_update_publish_items_single_item(db): assert item_dicts == [{"web_uri": "/uri1", "object_key": "1" * 64}] -def test_update_pubish_items_invalid_publish(db): +def test_update_pubish_items_invalid_publish(db, auth_header): """PUTting items on a completed publish fails with code 409.""" publish_id = "11224567-e89b-12d3-a456-426614174000" @@ -171,6 +178,7 @@ def test_update_pubish_items_invalid_publish(db): "object_key": "2" * 64, }, ], + headers=auth_header(roles=["test-publisher"]), ) # It should have failed with 409