From 390089b5a60baa035a8ba3f07308d75642dd658a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Fri, 19 Jul 2024 17:01:58 +0200 Subject: [PATCH 1/2] rename existing param for destructive operations to be related only with api requests next commit will introduce new param for destructive operations within seeding that will allow us to have a finer control over destructive RBAC operations --- deploy/rbac-clowdapp.yml | 6 +++--- docker-compose.yml | 2 +- rbac/core/utils.py | 9 +++++++-- rbac/internal/views.py | 8 ++++---- rbac/management/role/definer.py | 4 ++-- rbac/rbac/settings.py | 4 +++- tests/core/test_utils.py | 4 ++-- 7 files changed, 22 insertions(+), 15 deletions(-) diff --git a/deploy/rbac-clowdapp.yml b/deploy/rbac-clowdapp.yml index aa1bef24..3a498dc0 100644 --- a/deploy/rbac-clowdapp.yml +++ b/deploy/rbac-clowdapp.yml @@ -455,8 +455,8 @@ objects: value: ${BYPASS_BOP_VERIFICATION} - name: ROLE_CREATE_ALLOW_LIST value: ${ROLE_CREATE_ALLOW_LIST} - - name: RBAC_DESTRUCTIVE_ENABLED_UNTIL - value: ${RBAC_DESTRUCTIVE_ENABLED_UNTIL} + - name: RBAC_DESTRUCTIVE_API_ENABLED_UNTIL + value: ${RBAC_DESTRUCTIVE_API_ENABLED_UNTIL} - name: CLOWDER_ENABLED value: ${CLOWDER_ENABLED} - name: APP_NAMESPACE @@ -727,7 +727,7 @@ parameters: name: ROLE_CREATE_ALLOW_LIST value: cost-management,remediations,inventory,drift,policies,advisor,catalog,approval,vulnerability,compliance,automation-analytics,notifications,patch,integrations,ros,staleness,config-manager,idmsvc - description: Timestamp expiration allowance on destructive actions through the internal RBAC API - name: RBAC_DESTRUCTIVE_ENABLED_UNTIL + name: RBAC_DESTRUCTIVE_API_ENABLED_UNTIL value: '' - description: Image tag name: IMAGE_TAG diff --git a/docker-compose.yml b/docker-compose.yml index 46bfdcbe..f2f4fe11 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -33,7 +33,7 @@ services: - PRINCIPAL_PROXY_SERVICE_PATH=${PRINCIPAL_PROXY_SERVICE_PATH} - PRINCIPAL_PROXY_SERVICE_SOURCE_CERT=${PRINCIPAL_PROXY_SERVICE_SOURCE_CERT-False} - PRINCIPAL_PROXY_SERVICE_SSL_VERIFY=${PRINCIPAL_PROXY_SERVICE_SSL_VERIFY-False} - - RBAC_DESTRUCTIVE_ENABLED_UNTIL=${RBAC_DESTRUCTIVE_ENABLED_UNTIL} + - RBAC_DESTRUCTIVE_API_ENABLED_UNTIL=${RBAC_DESTRUCTIVE_API_ENABLED_UNTIL} privileged: true ports: - 9080:8080 diff --git a/rbac/core/utils.py b/rbac/core/utils.py index 34d570b8..11ad9957 100644 --- a/rbac/core/utils.py +++ b/rbac/core/utils.py @@ -22,7 +22,12 @@ from django.conf import settings -def destructive_ok(): +def destructive_ok(operation_type): """Determine if it's ok to run destructive operations.""" now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC) - return now < settings.INTERNAL_DESTRUCTIVE_API_OK_UNTIL + if operation_type == "api": + return now < settings.INTERNAL_DESTRUCTIVE_API_OK_UNTIL + if operation_type == "seeding": + return False + + return False diff --git a/rbac/internal/views.py b/rbac/internal/views.py index 6b162f0b..b8b9281a 100644 --- a/rbac/internal/views.py +++ b/rbac/internal/views.py @@ -137,7 +137,7 @@ def tenant_view(request, org_id): """ logger.info(f"Tenant view: {request.method} {request.user.username}") if request.method == "DELETE": - if not destructive_ok(): + if not destructive_ok("api"): return HttpResponse("Destructive operations disallowed.", status=400) tenant_obj = get_object_or_404(Tenant, org_id=org_id) @@ -391,7 +391,7 @@ def invalid_default_admin_groups(request): } return HttpResponse(json.dumps(payload), content_type="application/json") if request.method == "DELETE": - if not destructive_ok(): + if not destructive_ok("api"): return HttpResponse("Destructive operations disallowed.", status=400) invalid_default_admin_groups_list.delete() return HttpResponse(status=204) @@ -405,7 +405,7 @@ def role_removal(request): """ logger.info(f"Role removal: {request.method} {request.user.username}") if request.method == "DELETE": - if not destructive_ok(): + if not destructive_ok("api"): return HttpResponse("Destructive operations disallowed.", status=400) role_name = request.GET.get("name") @@ -433,7 +433,7 @@ def permission_removal(request): """ logger.info(f"Permission removal: {request.method} {request.user.username}") if request.method == "DELETE": - if not destructive_ok(): + if not destructive_ok("api"): return HttpResponse("Destructive operations disallowed.", status=400) permission = request.GET.get("permission") diff --git a/rbac/management/role/definer.py b/rbac/management/role/definer.py index 854e2e44..a9954c11 100644 --- a/rbac/management/role/definer.py +++ b/rbac/management/role/definer.py @@ -137,7 +137,7 @@ def seed_roles(): # Find roles in DB but not in config roles_to_delete = Role.objects.filter(system=True).exclude(id__in=current_role_ids) logger.info(f"The following '{roles_to_delete.count()}' roles(s) eligible for removal: {roles_to_delete.values()}") - if destructive_ok(): + if destructive_ok("seeding"): logger.info(f"Removing the following role(s): {roles_to_delete.values()}") # Actually remove roles no longer in config with transaction.atomic(): @@ -202,7 +202,7 @@ def seed_permissions(): logger.info( f"The following '{perms_to_delete.count()}' permission(s) eligible for removal: {perms_to_delete.values()}" ) - if destructive_ok(): + if destructive_ok("seeding"): logger.info(f"Removing the following permissions(s): {perms_to_delete.values()}") # Actually remove perms no longer in DB with transaction.atomic(): diff --git a/rbac/rbac/settings.py b/rbac/rbac/settings.py index 5a9047c3..19c9f74a 100644 --- a/rbac/rbac/settings.py +++ b/rbac/rbac/settings.py @@ -367,7 +367,9 @@ INTERNAL_API_PATH_PREFIXES = ["/_private/"] try: - INTERNAL_DESTRUCTIVE_API_OK_UNTIL = parse_dt(os.environ.get("RBAC_DESTRUCTIVE_ENABLED_UNTIL", "not-a-real-time")) + INTERNAL_DESTRUCTIVE_API_OK_UNTIL = parse_dt( + os.environ.get("RBAC_DESTRUCTIVE_API_ENABLED_UNTIL", "not-a-real-time") + ) if INTERNAL_DESTRUCTIVE_API_OK_UNTIL.tzinfo is None: INTERNAL_DESTRUCTIVE_API_OK_UNTIL = INTERNAL_DESTRUCTIVE_API_OK_UNTIL.replace(tzinfo=pytz.UTC) except ValueError as e: diff --git a/tests/core/test_utils.py b/tests/core/test_utils.py index 6bf3f70e..8b778c7c 100644 --- a/tests/core/test_utils.py +++ b/tests/core/test_utils.py @@ -33,9 +33,9 @@ def invalid_destructive_time(): @override_settings(INTERNAL_DESTRUCTIVE_API_OK_UNTIL=valid_destructive_time()) def test_destructive_ok_true(self): """Test that it's true when within date range.""" - self.assertEqual(destructive_ok(), True) + self.assertEqual(destructive_ok("api"), True) @override_settings(INTERNAL_DESTRUCTIVE_API_OK_UNTIL=invalid_destructive_time()) def test_destructive_ok_false(self): """Test that it's false when not within date range.""" - self.assertEqual(destructive_ok(), False) + self.assertEqual(destructive_ok("api"), False) From a194e941bb8dcb74a1fea4518dd9d52c9010cf5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Fri, 19 Jul 2024 17:15:16 +0200 Subject: [PATCH 2/2] added new param to control enable destructive operations during seeding --- deploy/rbac-clowdapp.yml | 5 +++++ docker-compose.yml | 1 + rbac/core/utils.py | 2 +- rbac/rbac/settings.py | 9 +++++++++ tests/core/test_utils.py | 10 ++++++++++ tests/internal/test_views.py | 2 +- 6 files changed, 27 insertions(+), 2 deletions(-) diff --git a/deploy/rbac-clowdapp.yml b/deploy/rbac-clowdapp.yml index 3a498dc0..3fca384f 100644 --- a/deploy/rbac-clowdapp.yml +++ b/deploy/rbac-clowdapp.yml @@ -457,6 +457,8 @@ objects: value: ${ROLE_CREATE_ALLOW_LIST} - name: RBAC_DESTRUCTIVE_API_ENABLED_UNTIL value: ${RBAC_DESTRUCTIVE_API_ENABLED_UNTIL} + - name: RBAC_DESTRUCTIVE_SEEDING_ENABLED_UNTIL + value: ${RBAC_DESTRUCTIVE_SEEDING_ENABLED_UNTIL} - name: CLOWDER_ENABLED value: ${CLOWDER_ENABLED} - name: APP_NAMESPACE @@ -729,6 +731,9 @@ parameters: - description: Timestamp expiration allowance on destructive actions through the internal RBAC API name: RBAC_DESTRUCTIVE_API_ENABLED_UNTIL value: '' +- description: Timestamp expiration allowance on destructive actions through the seeding job + name: RBAC_DESTRUCTIVE_SEEDING_ENABLED_UNTIL + value: '' - description: Image tag name: IMAGE_TAG required: true diff --git a/docker-compose.yml b/docker-compose.yml index f2f4fe11..65ed49b1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -34,6 +34,7 @@ services: - PRINCIPAL_PROXY_SERVICE_SOURCE_CERT=${PRINCIPAL_PROXY_SERVICE_SOURCE_CERT-False} - PRINCIPAL_PROXY_SERVICE_SSL_VERIFY=${PRINCIPAL_PROXY_SERVICE_SSL_VERIFY-False} - RBAC_DESTRUCTIVE_API_ENABLED_UNTIL=${RBAC_DESTRUCTIVE_API_ENABLED_UNTIL} + - RBAC_DESTRUCTIVE_SEEDING_ENABLED_UNTIL=${RBAC_DESTRUCTIVE_SEEDING_ENABLED_UNTIL} privileged: true ports: - 9080:8080 diff --git a/rbac/core/utils.py b/rbac/core/utils.py index 11ad9957..1ff12dba 100644 --- a/rbac/core/utils.py +++ b/rbac/core/utils.py @@ -28,6 +28,6 @@ def destructive_ok(operation_type): if operation_type == "api": return now < settings.INTERNAL_DESTRUCTIVE_API_OK_UNTIL if operation_type == "seeding": - return False + return now < settings.DESTRUCTIVE_SEEDING_OK_UNTIL return False diff --git a/rbac/rbac/settings.py b/rbac/rbac/settings.py index 19c9f74a..0acfaca4 100644 --- a/rbac/rbac/settings.py +++ b/rbac/rbac/settings.py @@ -355,6 +355,15 @@ GROUP_SEEDING_ENABLED = ENVIRONMENT.bool("GROUP_SEEDING_ENABLED", default=True) MAX_SEED_THREADS = ENVIRONMENT.int("MAX_SEED_THREADS", default=None) +try: + DESTRUCTIVE_SEEDING_OK_UNTIL = parse_dt( + os.environ.get("RBAC_DESTRUCTIVE_SEEDING_ENABLED_UNTIL", "not-a-real-time") + ) + if DESTRUCTIVE_SEEDING_OK_UNTIL.tzinfo is None: + DESTRUCTIVE_SEEDING_OK_UNTIL = DESTRUCTIVE_SEEDING_OK_UNTIL.replace(tzinfo=pytz.UTC) +except ValueError as e: + DESTRUCTIVE_SEEDING_OK_UNTIL = datetime.datetime(1970, 1, 1, tzinfo=pytz.UTC) + # disable log messages less than CRITICAL when running unit tests. if len(sys.argv) > 1 and sys.argv[1] == "test": logging.disable(logging.CRITICAL) diff --git a/tests/core/test_utils.py b/tests/core/test_utils.py index 8b778c7c..f7d9bf95 100644 --- a/tests/core/test_utils.py +++ b/tests/core/test_utils.py @@ -39,3 +39,13 @@ def test_destructive_ok_true(self): def test_destructive_ok_false(self): """Test that it's false when not within date range.""" self.assertEqual(destructive_ok("api"), False) + + @override_settings(DESTRUCTIVE_SEEDING_OK_UNTIL=valid_destructive_time()) + def test_destructive_ok_true(self): + """Test that it's true when within date range.""" + self.assertEqual(destructive_ok("seeding"), True) + + @override_settings(DESTRUCTIVE_SEEDING_OK_UNTIL=invalid_destructive_time()) + def test_destructive_ok_false(self): + """Test that it's false when not within date range.""" + self.assertEqual(destructive_ok("seeding"), False) diff --git a/tests/internal/test_views.py b/tests/internal/test_views.py index 6f34138a..0e510a96 100644 --- a/tests/internal/test_views.py +++ b/tests/internal/test_views.py @@ -410,7 +410,7 @@ def test_delete_selective_roles_disallowed(self): @override_settings(INTERNAL_DESTRUCTIVE_API_OK_UNTIL=valid_destructive_time()) def test_delete_selective_roles(self): """Test that we can delete selective roles when allowed and no roles.""" - # No name speicified + # No name specified response = self.client.delete(f"/_private/api/utils/role/", **self.request.META) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)