From 272ae800083f07b152c3e10845d69de598787ab7 Mon Sep 17 00:00:00 2001 From: Caleigh Runge-Hottman Date: Fri, 12 Apr 2024 16:52:21 -0400 Subject: [PATCH] Introduce per-user path restrictions [RHELDST-23442] Previously, exodus-gw's publish APIs allowed any authorized user to publish to any paths within a given CDN environment. Now, it is possible to restrict individual users to publishing to certain paths in a given CDN environment using the publish_paths setting, or the EXODUS_GW_PUBLISH_PATHS environment variable. An example of the publish paths config: { "pre": { "fake-user": [ "^(/content)?/origin/files/sha256/[0-f]{2}/[0-f]{64}/[^/]{1,300}$" ], }, "live": { "fake-user": [ "^(/content)?/origin/files/sha256/[0-f]{2}/[0-f]{64}/[^/]{1,300}$" ], } } Any clients identified in the config are authorized to publish to any path which matches the defined regex. When a client attempts to publish to a path to which does not match the defined regex, they will get a 403 response. Any client which is not included in the publish_paths config will be authorized to publish to any path (assuming they have the necessary publish roles). This should reduce the risk of conflicts and other issues between exodus-gw users. --- exodus_gw/routers/publish.py | 43 +++++++++++++++ exodus_gw/settings.py | 13 +++++ tests/routers/test_publish.py | 100 ++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+) diff --git a/exodus_gw/routers/publish.py b/exodus_gw/routers/publish.py index d862718d..0e38bebc 100644 --- a/exodus_gw/routers/publish.py +++ b/exodus_gw/routers/publish.py @@ -112,6 +112,7 @@ import logging import os +import re from datetime import datetime from uuid import uuid4 @@ -208,6 +209,7 @@ def update_publish_items( env: Environment = deps.env, db: Session = deps.db, settings: Settings = deps.settings, + call_context: auth.CallContext = deps.call_context, ) -> dict[None, None]: """Add publish items to an existing publish object. @@ -276,6 +278,47 @@ def update_publish_items( db_publish.resolve_links(ln_items=resolvable) + # Prevent unauthorized users from publishing to restricted paths within + # a particular CDN environment. + # + # Some users only need to publish to certain paths. Allowing those + # users to publish to other paths increases the risk of conflicts + # between clients, or of accidents with a large impact. + username = str( + call_context.client.serviceAccountId + or call_context.user.internalUsername + ) + path_restrictions = (settings.publish_paths.get(env.name) or {}).get( + username + ) or [".*"] + path_patterns = [re.compile(path) for path in path_restrictions] + + for i in items: + # Determine whether the client is authorized to publish to this URI. + authorized = False + for pattern in path_patterns: + if re.match(pattern, i.web_uri): + authorized = True + break + if not authorized: + # The URI did not match one of the client's permitted patterns in publish_paths. + LOG.error( + "User '%s' is not authorized to publish to path '%s'", + username, + i.web_uri, + extra={ + "publish_id": publish_id, + "event": "publish", + "success": False, + }, + ) + + raise HTTPException( + 403, + detail="User '%s' is not authorized to publish to path '%s'" + % (username, i.web_uri), + ) + # Convert the list into dict and update each dict with a publish_id. # Each item's 'dirty' and 'updated' are refreshed to ensure it's # written to DynamoDB with the current update, even if it was already diff --git a/exodus_gw/settings.py b/exodus_gw/settings.py index bf0f469c..0310eb57 100644 --- a/exodus_gw/settings.py +++ b/exodus_gw/settings.py @@ -118,6 +118,19 @@ class Settings(BaseSettings): for validation. E.g., "exodus-migration-md5": "^[0-9a-f]{32}$" """ + publish_paths: dict[str, dict[str, list[str]]] = {} + """A set of user or service accounts which are only authorized to publish to a + particular set of path(s) in a given CDN environment and the regex(es) describing + the paths to which the user or service account is authorized to publish. The user or + service account will be prevented from publishing to any paths that do not match the + defined regular expression(s). + E.g., '{"pre": {"fake-user": + ["^(/content)?/origin/files/sha256/[0-f]{2}/[0-f]{64}/[^/]{1,300}$"]}}' + + Any user or service account not included in this configuration is considered to have + unrestricted publish access (i.e., can publish to any path). + """ + log_config: dict[str, Any] = { "version": 1, "incremental": True, diff --git a/tests/routers/test_publish.py b/tests/routers/test_publish.py index 52c5c00d..5f18ce15 100644 --- a/tests/routers/test_publish.py +++ b/tests/routers/test_publish.py @@ -1357,3 +1357,103 @@ def test_get_publish_not_found(auth_header, fake_publish): assert r.json() == { "detail": "No publish found for ID %s" % fake_publish.id } + + +def test_update_user_authorized_publish_paths(db, auth_header, monkeypatch): + """Ensure that a user can successfully publish content to any paths to + which they are authorized to publish.""" + + monkeypatch.setenv( + "EXODUS_GW_PUBLISH_PATHS", + json.dumps( + { + "test": { + "fake-user": [ + "^(/content)?/origin/files/sha256/[0-f]{2}/[0-f]{64}/[^/]{1,300}$", + "^/fake-path/[0-f]{64}/[^/]{1,300}$", + ] + } + } + ), + ) + + publish_id = "11224567-e89b-12d3-a456-426614174000" + + publish = Publish(id=publish_id, env="test", state="PENDING") + + db.add(publish) + db.commit() + + with TestClient(app) as client: + # Try to add some items to it + r = client.put( + "/test/publish/%s" % publish_id, + json=[ + { + "web_uri": "/content/origin/files/sha256/00/0044062dca731c0d5c24148722537e181d752ca8cda0097005f9268a51658b0a/test.rpm", + "object_key": "1" * 64, + "content_type": "application/octet-stream", + }, + { + "web_uri": "/fake-path/0097062dca731c0d5c24148722537e181d752ca8cda0097005f9268a51658b0a/other-test.rpm", + "object_key": "2" * 64, + "content_type": "application/octet-stream", + }, + ], + headers=auth_header(roles=["test-publisher"]), + ) + + # It should have succeeded + assert r.status_code == 200 + + +def test_update_user_unauthorized_publish_paths(db, auth_header, monkeypatch): + """When a user is only authorized to publish to certain paths in a given + CDN environment, ensure that the user is prevented from publishing to any + paths to which they are unauthorized to publish.""" + + monkeypatch.setenv( + "EXODUS_GW_PUBLISH_PATHS", + json.dumps( + { + "test": { + "fake-user": [ + "^(/content)?/origin/files/sha256/[0-f]{2}/[0-f]{64}/[^/]{1,300}$", + "/fake-path/[0-f]{64}/[^/]{1,300}$", + ] + } + } + ), + ) + + publish_id = "11224567-e89b-12d3-a456-426614174000" + + publish = Publish(id=publish_id, env="test", state="PENDING") + + db.add(publish) + db.commit() + + with TestClient(app) as client: + # Try to add some items to it + r = client.put( + "/test/publish/%s" % publish_id, + json=[ + { + "web_uri": "/content/origin/files/sha256/00/0044062dca731c0d5c24148722537e181d752ca8cda0097005f9268a51658b0a/test.rpm", + "object_key": "1" * 64, + "content_type": "application/octet-stream", + }, + { + "web_uri": "/content/origin/uri1", + "object_key": "2" * 64, + "content_type": "application/octet-stream", + }, + ], + headers=auth_header(roles=["test-publisher"]), + ) + + # It should have failed + assert r.status_code == 403 + assert r.json() == { + "detail": "User 'fake-user' is not authorized to publish to path '/content/origin/uri1'" + }