From d33af419fb7b98b0664a4e50361196be48336454 Mon Sep 17 00:00:00 2001 From: Keith Walsh Date: Wed, 2 Oct 2024 12:52:55 -0400 Subject: [PATCH 1/2] Enable support for read-only mode on APIs This adds an optional `READ_ONLY_API_MODE` environment variable consumed in settings, which defaults to `False`. It can be configured via params in the deploy template to enable/disable with config per environment. This is used in a new `ReadOnlyApiMiddleware` middleware for all RBAC requests, which checks the request method and the configuration setting to determine whether or not to process the request. In the event of a write method + read-only mode being enabled, we'll return a 405 with a message explaining we're in read-only mode, to be used during migration/maintenance windows for the service. Another option would be to use 503, though since we're still accepting GET requests a 405 with explicit message may be the best approach. --- deploy/rbac-clowdapp.yml | 4 +++ rbac/rbac/middleware.py | 13 +++++++++ rbac/rbac/settings.py | 2 ++ tests/rbac/test_middleware.py | 53 ++++++++++++++++++++++++++++++++++- 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/deploy/rbac-clowdapp.yml b/deploy/rbac-clowdapp.yml index 5fa68daf0..9c85c8444 100644 --- a/deploy/rbac-clowdapp.yml +++ b/deploy/rbac-clowdapp.yml @@ -220,6 +220,8 @@ objects: value: ${SA_NAME} - name: RELATION_API_SERVER value: ${RELATION_API_SERVER} + - name: READ_ONLY_API_MODE + value: ${READ_ONLY_API_MODE} - name: scheduler-service minReplicas: ${{MIN_SCHEDULER_REPLICAS}} @@ -920,4 +922,6 @@ parameters: value: "localhost:9000" - name: V2_APIS_ENABLED description: Flag to explicitly enable v2 API endpoints +- name: READ_ONLY_API_MODE + description: Enforce GET only on RBAC APIs value: 'False' diff --git a/rbac/rbac/middleware.py b/rbac/rbac/middleware.py index 5633a9363..318c434b0 100644 --- a/rbac/rbac/middleware.py +++ b/rbac/rbac/middleware.py @@ -420,3 +420,16 @@ def process_request(self, request): # pylint: disable=no-self-use """ setattr(request, "_dont_enforce_csrf_checks", True) + + +class ReadOnlyApiMiddleware(MiddlewareMixin): # pylint: disable=too-few-public-methods + """Middleware to enable read-only on APIs when configured.""" + + def process_request(self, request): # pylint: disable=no-self-use + """Process request ReadOnlyApiMiddleware.""" + if settings.READ_ONLY_API_MODE and request.method in ["POST", "PUT", "PATCH", "DELETE"]: + return HttpResponse( + json.dumps({"error": "This API is currently in read-only mode. Please try again later."}), + content_type="application/json", + status=405, + ) diff --git a/rbac/rbac/settings.py b/rbac/rbac/settings.py index f8205efdc..e4efd54d0 100644 --- a/rbac/rbac/settings.py +++ b/rbac/rbac/settings.py @@ -129,6 +129,7 @@ "django.middleware.clickjacking.XFrameOptionsMiddleware", "whitenoise.middleware.WhiteNoiseMiddleware", "django_prometheus.middleware.PrometheusAfterMiddleware", + "rbac.middleware.ReadOnlyApiMiddleware", ] DEVELOPMENT = ENVIRONMENT.bool("DEVELOPMENT", default=False) @@ -486,3 +487,4 @@ # Versioned API settings V2_APIS_ENABLED = ENVIRONMENT.bool("V2_APIS_ENABLED", default=False) +READ_ONLY_API_MODE = ENVIRONMENT.get_value("READ_ONLY_API_MODE", default=False) diff --git a/tests/rbac/test_middleware.py b/tests/rbac/test_middleware.py index ff6f61315..acdd959c7 100644 --- a/tests/rbac/test_middleware.py +++ b/tests/rbac/test_middleware.py @@ -22,6 +22,7 @@ from unittest.mock import Mock from django.conf import settings from django.http import QueryDict +from django.test.utils import override_settings from django.test import TestCase from django.urls import reverse @@ -32,7 +33,7 @@ from api.models import Tenant, User from api.serializers import create_tenant_name from tests.identity_request import IdentityRequest -from rbac.middleware import HttpResponseUnauthorizedRequest, IdentityHeaderMiddleware +from rbac.middleware import HttpResponseUnauthorizedRequest, IdentityHeaderMiddleware, ReadOnlyApiMiddleware from management.models import Access, Group, Permission, Principal, Policy, ResourceDefinition, Role @@ -574,3 +575,53 @@ def test_principal_with_access_with_wildcard_access(self): "permission": {"read": ["*"], "write": ["*"]}, } self.assertEqual(expected, access) + + +class RBACReadOnlyApiMiddleware(IdentityRequest): + """Tests against the read-only API middleware.""" + + def setUp(self): + """Set up middleware tests.""" + super().setUp() + self.request = Mock() + self.request.path = "/api/rbac/v1/roles/" + self.write_methods = ["POST", "PUT", "PATCH", "DELETE"] + + def assertReadOnlyFailure(self, resp): + resp_body_str = resp.content.decode("utf-8") + self.assertEqual( + json.loads(resp_body_str)["error"], "This API is currently in read-only mode. Please try again later." + ) + self.assertEqual(resp.status_code, 405) + + @override_settings(READ_ONLY_API_MODE=True) + def test_get_read_only_true(self): + """Test GET and READ_ONLY_API_MODE=True.""" + self.request.method = "GET" + middleware = ReadOnlyApiMiddleware(get_response=Mock()) + resp = middleware.process_request(self.request) + self.assertEqual(resp, None) + + @override_settings(READ_ONLY_API_MODE=True) + def test_write_methods_read_only_true(self): + """Test write methods and READ_ONLY_API_MODE=True.""" + for method in self.write_methods: + self.request.method = method + middleware = ReadOnlyApiMiddleware(get_response=Mock()) + resp = middleware.process_request(self.request) + self.assertReadOnlyFailure(resp) + + def test_get_read_only_false(self): + """Test GET and READ_ONLY_API_MODE=False.""" + self.request.method = "GET" + middleware = ReadOnlyApiMiddleware(get_response=Mock()) + resp = middleware.process_request(self.request) + self.assertEqual(resp, None) + + def test_write_methods_read_only_false(self): + """Test write methods and READ_ONLY_API_MODE=False.""" + for method in self.write_methods: + self.request.method = method + middleware = ReadOnlyApiMiddleware(get_response=Mock()) + resp = middleware.process_request(self.request) + self.assertEqual(resp, None) From 623c1010e944a96ece6777f4a7f3d6a3765ba0fb Mon Sep 17 00:00:00 2001 From: Keith Walsh Date: Thu, 3 Oct 2024 07:59:10 -0400 Subject: [PATCH 2/2] Move env for read only to correct service --- deploy/rbac-clowdapp.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy/rbac-clowdapp.yml b/deploy/rbac-clowdapp.yml index 9c85c8444..32a8a2606 100644 --- a/deploy/rbac-clowdapp.yml +++ b/deploy/rbac-clowdapp.yml @@ -220,8 +220,6 @@ objects: value: ${SA_NAME} - name: RELATION_API_SERVER value: ${RELATION_API_SERVER} - - name: READ_ONLY_API_MODE - value: ${READ_ONLY_API_MODE} - name: scheduler-service minReplicas: ${{MIN_SCHEDULER_REPLICAS}} @@ -517,6 +515,8 @@ objects: value: ${IT_TOKEN_JKWS_CACHE_LIFETIME} - name: V2_APIS_ENABLED value: ${V2_APIS_ENABLED} + - name: READ_ONLY_API_MODE + value: ${READ_ONLY_API_MODE} jobs: - name: tenant-org-id-populator