From daf3a2e164b146684d389a136d6e33ed67f996eb Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Wed, 8 Jan 2025 09:46:25 -0500 Subject: [PATCH 01/13] add GET method --- rbac/internal/views.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/rbac/internal/views.py b/rbac/internal/views.py index 90af20d3c..ad7abc785 100644 --- a/rbac/internal/views.py +++ b/rbac/internal/views.py @@ -781,3 +781,33 @@ def roles(request, uuid: str) -> HttpResponse: def trigger_error(request): """Trigger an error to confirm Sentry is working.""" raise SentryDiagnosticError + + +def correct_resource_definitions(request): + """Get/Fix resourceDefinitions with incorrect attributeFilters. + + Attribute filters with lists must use 'in' operation. Those with a single string must use 'equal' + + GET /_private/api/utils/resource_definitions + PATCH /_private/api/utils/resource_definitions + """ + if request.method == "GET": + with connection.cursor() as cursor: + query = """SELECT COUNT(*) FROM management_resourcedefinition + WHERE "attributeFilter"->>'operation' = 'equal' + AND jsonb_typeof("attributeFilter"->'value') = 'array';""" + + cursor.execute(query) + count = cursor.fetchone()[0] + + query = """SELECT COUNT(*) from management_resourcedefinition WHERE "attributeFilter"->>'operation' = 'in' + AND jsonb_typeof("attributeFilter"->'value') = 'string';""" + + cursor.execute(query) + count += cursor.fetchone()[0] + + return HttpResponse(f"{count} resource definitions would be corrected", status=200) + elif request.method == "PATCH": + pass + + return HttpResponse('Invalid method, only "GET" or "PATCH" are allowed.', status=405) From 141c470ea7b3e699fd9aa072572c744ffd252d5e Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Wed, 8 Jan 2025 09:48:03 -0500 Subject: [PATCH 02/13] add new resource_definitions view --- rbac/internal/urls.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rbac/internal/urls.py b/rbac/internal/urls.py index 585d38b02..3e796f814 100644 --- a/rbac/internal/urls.py +++ b/rbac/internal/urls.py @@ -84,6 +84,7 @@ path("api/utils/bootstrap_tenant/", views.bootstrap_tenant), path("api/utils/migration_resources/", views.migration_resources), path("api/utils/reset_imported_tenants/", views.reset_imported_tenants), + path("api/utils/resource_definitions/", views.correct_resource_definitions), ] urlpatterns.extend(integration_urlpatterns) From e2918e78e6bdd109e5bdba678f2b11aad02ebaf6 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 13 Jan 2025 07:19:59 -0500 Subject: [PATCH 03/13] change on PATCH --- rbac/internal/views.py | 37 +++++++++++++++++++++++++++---------- rbac/rbac/dev_middleware.py | 2 +- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/rbac/internal/views.py b/rbac/internal/views.py index ad7abc785..2f83ab102 100644 --- a/rbac/internal/views.py +++ b/rbac/internal/views.py @@ -30,7 +30,7 @@ from django.utils.html import escape from internal.utils import delete_bindings from management.cache import TenantCache -from management.models import Group, Permission, Role +from management.models import Group, Permission, Role, ResourceDefinition from management.principal.proxy import ( API_TOKEN_HEADER, CLIENT_ID_HEADER, @@ -791,23 +791,40 @@ def correct_resource_definitions(request): GET /_private/api/utils/resource_definitions PATCH /_private/api/utils/resource_definitions """ + list_query = """ FROM management_resourcedefinition + WHERE "attributeFilter"->>'operation' = 'equal' + AND jsonb_typeof("attributeFilter"->'value') = 'array';""" + + string_query = """ from management_resourcedefinition WHERE "attributeFilter"->>'operation' = 'in' + AND jsonb_typeof("attributeFilter"->'value') = 'string';""" + if request.method == "GET": with connection.cursor() as cursor: - query = """SELECT COUNT(*) FROM management_resourcedefinition - WHERE "attributeFilter"->>'operation' = 'equal' - AND jsonb_typeof("attributeFilter"->'value') = 'array';""" - cursor.execute(query) + cursor.execute("SELECT COUNT(*)" + list_query) count = cursor.fetchone()[0] - query = """SELECT COUNT(*) from management_resourcedefinition WHERE "attributeFilter"->>'operation' = 'in' - AND jsonb_typeof("attributeFilter"->'value') = 'string';""" - - cursor.execute(query) + cursor.execute("SELECT COUNT(*)" + string_query) count += cursor.fetchone()[0] return HttpResponse(f"{count} resource definitions would be corrected", status=200) elif request.method == "PATCH": - pass + with connection.cursor() as cursor: + + cursor.execute("SELECT id " + list_query) + result = cursor.fetchall() + for id in result: + resource_definition = ResourceDefinition.objects.get(id=id) + resource_definition.attributeFilter["operation"] = "in" + resource_definition.save() + + cursor.execute("SELECT id " + string_query) + result = cursor.fetchall() + for id in result: + resource_definition = ResourceDefinition.objects.get(id=id) + resource_definition.attributeFilter["operation"] = "equal" + resource_definition.save() + + return HttpResponse("Updated bad resource definitions", status=200) return HttpResponse('Invalid method, only "GET" or "PATCH" are allowed.', status=405) diff --git a/rbac/rbac/dev_middleware.py b/rbac/rbac/dev_middleware.py index c4d099059..fc5c18fba 100644 --- a/rbac/rbac/dev_middleware.py +++ b/rbac/rbac/dev_middleware.py @@ -38,7 +38,7 @@ def process_request(self, request): # pylint: disable=no-self-use """ if hasattr(request, "META"): user_type = request.headers.get("User-Type") - if user_type and user_type in ["associate", "internal", "turnpike"]: + if True or user_type and user_type in ["associate", "internal", "turnpike"]: identity_header = { "identity": { "associate": { From 9ad73fe815c6121370e32a0e97d8d5e60643b1a1 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 13 Jan 2025 07:20:58 -0500 Subject: [PATCH 04/13] progress on tests --- tests/internal/test_views.py | 97 ++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/tests/internal/test_views.py b/tests/internal/test_views.py index c4dd34824..480206e34 100644 --- a/tests/internal/test_views.py +++ b/tests/internal/test_views.py @@ -21,6 +21,7 @@ from rest_framework.test import APIClient from django.conf import settings from django.test import override_settings +from django.urls import reverse from datetime import datetime, timedelta from unittest.mock import MagicMock from unittest.mock import patch @@ -1294,3 +1295,99 @@ def test_fetch_role(self): self.assertFalse(role["platform_default"]) self.assertFalse(role["admin_default"]) self.assertEqual(response.status_code, 200) + + +class InternalViewsetResourceDefinitionTests(IdentityRequest): + def setUp(self): + """Set up the access view tests.""" + super().setUp() + request = self.request_context["request"] + self.customer = self.customer_data + self.internal_request_context = self._create_request_context(self.customer, self.user_data, is_internal=True) + user = User() + user.username = self.user_data["username"] + user.account = self.customer_data["account_id"] + user.org_id = self.customer_data["org_id"] + request.user = user + public_tenant = Tenant.objects.get(tenant_name="public") + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [{"attributeFilter": {"key": "key1.id", "operation": "equal", "value": "value1"}}], + } + + test_tenant_org_id = "100001" + + # we need to delete old test_tenant's that may exist in cache + TENANTS = TenantCache() + TENANTS.delete_tenant(test_tenant_org_id) + + # items with test_ prefix have hard coded attributes for new BOP requests + self.test_tenant = Tenant( + tenant_name="acct1111111", account_id="1111111", org_id=test_tenant_org_id, ready=True + ) + self.test_tenant.save() + self.test_principal = Principal(username="test_user", tenant=self.test_tenant) + self.test_principal.save() + self.test_group = Group(name="test_groupA", tenant=self.test_tenant) + self.test_group.save() + self.test_group.principals.add(self.test_principal) + self.test_group.save() + self.test_permission = Permission.objects.create(permission="app:test_*:test_*", tenant=self.test_tenant) + Permission.objects.create(permission="app:test_foo:test_bar", tenant=self.test_tenant) + user_data = {"username": "test_user", "email": "test@gmail.com"} + request_context = self._create_request_context( + {"account_id": "1111111", "tenant_name": "acct1111111", "org_id": "100001"}, user_data, is_org_admin=True + ) + request = request_context["request"] + self.test_headers = request.META + test_tenant_root_workspace = Workspace.objects.create( + name="Test Tenant Root Workspace", type=Workspace.Types.ROOT, tenant=self.test_tenant + ) + Workspace.objects.create( + name="Test Tenant Default Workspace", + type=Workspace.Types.DEFAULT, + parent=test_tenant_root_workspace, + tenant=self.test_tenant, + ) + + self.principal = Principal(username=user.username, tenant=self.tenant) + self.principal.save() + self.admin_principal = Principal(username="user_admin", tenant=self.tenant) + self.admin_principal.save() + self.group = Group(name="groupA", tenant=self.tenant) + self.group.save() + self.group.principals.add(self.principal) + self.group.save() + self.permission = Permission.objects.create(permission="app:*:*", tenant=self.tenant) + Permission.objects.create(permission="app:foo:bar", tenant=self.tenant) + + def tearDown(self): + """Tear down access view tests.""" + Group.objects.all().delete() + Principal.objects.all().delete() + Role.objects.all().delete() + Policy.objects.all().delete() + Workspace.objects.filter(parent__isnull=False).delete() + Workspace.objects.filter(parent__isnull=True).delete() + + def create_role(self, role_name, headers, in_access_data=None): + """Create a role.""" + access_data = self.access_data + if in_access_data: + access_data = in_access_data + test_data = {"name": role_name, "access": [access_data]} + + # create a role + url = reverse("v1_management:role-list") + client = APIClient() + response = client.post(url, test_data, format="json", **headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + return response + + def test_correct_string_resource_definition(self): + """Test that a string attributeFilter can have the equal operation""" + + role_name = "roleA" + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) From 8a5152d8e97718de32ebb33989a852c93fc195c9 Mon Sep 17 00:00:00 2001 From: "red-hat-konflux[bot]" <126015336+red-hat-konflux[bot]@users.noreply.github.com> Date: Sat, 18 Jan 2025 08:16:20 +0000 Subject: [PATCH 05/13] Update Konflux references Signed-off-by: red-hat-konflux <126015336+red-hat-konflux[bot]@users.noreply.github.com> --- .tekton/insights-rbac-pull-request.yaml | 16 ++++++++-------- .tekton/insights-rbac-push.yaml | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.tekton/insights-rbac-pull-request.yaml b/.tekton/insights-rbac-pull-request.yaml index 899cd6b3e..8b43435e5 100644 --- a/.tekton/insights-rbac-pull-request.yaml +++ b/.tekton/insights-rbac-pull-request.yaml @@ -127,7 +127,7 @@ spec: - name: name value: init - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-init:0.2@sha256:90dda596d44b3f861889da2fba161dff34c6116fe76c3989e3f84262ea0f29cd + value: quay.io/konflux-ci/tekton-catalog/task-init:0.2@sha256:b76b563510ad9db0049e9b4512a152aef2bb51fb714363b6aa592744a580bcbd - name: kind value: task resolver: bundles @@ -286,7 +286,7 @@ spec: - name: name value: prefetch-dependencies-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-prefetch-dependencies-oci-ta:0.1@sha256:994f816e36ac832f4020647afd69223a015c84c503f925013c573fed52f05420 + value: quay.io/konflux-ci/tekton-catalog/task-prefetch-dependencies-oci-ta:0.1@sha256:8fb092dae7109ac211d8b98413d9bc0c71c14f64644ce239676383576f861a86 - name: kind value: task resolver: bundles @@ -327,7 +327,7 @@ spec: - name: name value: buildah-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:11b9ce26fd2933ccc81ca3f983e094ec54326a2e0aaf8bdcc4c0b8fea1a42c53 + value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:d35ff2504c0f58fd71879c917a1c66d817143cd8586b4bbd6cff0f3f91e5d040 - name: kind value: task resolver: bundles @@ -356,7 +356,7 @@ spec: - name: name value: build-image-index - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-build-image-index:0.1@sha256:479775c8655d815fb515aeb97efc0e64284a8520c452754981970900b937a393 + value: quay.io/konflux-ci/tekton-catalog/task-build-image-index:0.1@sha256:09344e6bda708f48ef759bbe84bce99515549f4cfdcbe89e417f695c19463260 - name: kind value: task resolver: bundles @@ -399,7 +399,7 @@ spec: - name: name value: source-build-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-source-build-oci-ta:0.1@sha256:75e882bf1619dd45a4043060ce42a6ad3ce781264ade5b7f66a1d994ee159126 + value: quay.io/konflux-ci/tekton-catalog/task-source-build-oci-ta:0.1@sha256:bd191e50b2eb7e3996d7f24e38fa44b4552c142920b540fb7f3dc104e508a68f - name: kind value: task resolver: bundles @@ -493,7 +493,7 @@ spec: - name: name value: sast-snyk-check-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-sast-snyk-check-oci-ta:0.3@sha256:af93b35e6e71a6ff7f3785ad8d8497b11204a5c0c33ab1a78b44f9d43f49c7a5 + value: quay.io/konflux-ci/tekton-catalog/task-sast-snyk-check-oci-ta:0.3@sha256:9172196136831a61b9039ea4498fcdc71d6adc86d9694f236bea7b2a85488cd3 - name: kind value: task resolver: bundles @@ -535,7 +535,7 @@ spec: - name: name value: apply-tags - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-apply-tags:0.1@sha256:2c2d88c07623b2d25163994ded6e9f29205ea5bbab090f4c86379739940028b9 + value: quay.io/konflux-ci/tekton-catalog/task-apply-tags:0.1@sha256:46f4471a86130c146a6e14124f175d6ef1ddb4b80ac51e7957d78a3facb6261b - name: kind value: task resolver: bundles @@ -558,7 +558,7 @@ spec: - name: name value: push-dockerfile-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-push-dockerfile-oci-ta:0.1@sha256:98ccae6ac132ab837fc51a70514be5fca656e09d6d4ad93230bd10f0119258aa + value: quay.io/konflux-ci/tekton-catalog/task-push-dockerfile-oci-ta:0.1@sha256:6cfb72a0b5c382a3584af322d11f3ae855d07eb72a6ee2253de7b9512a0c21a5 - name: kind value: task resolver: bundles diff --git a/.tekton/insights-rbac-push.yaml b/.tekton/insights-rbac-push.yaml index dc5019a5b..def4c21f7 100644 --- a/.tekton/insights-rbac-push.yaml +++ b/.tekton/insights-rbac-push.yaml @@ -124,7 +124,7 @@ spec: - name: name value: init - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-init:0.2@sha256:90dda596d44b3f861889da2fba161dff34c6116fe76c3989e3f84262ea0f29cd + value: quay.io/konflux-ci/tekton-catalog/task-init:0.2@sha256:b76b563510ad9db0049e9b4512a152aef2bb51fb714363b6aa592744a580bcbd - name: kind value: task resolver: bundles @@ -174,7 +174,7 @@ spec: - name: name value: prefetch-dependencies-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-prefetch-dependencies-oci-ta:0.1@sha256:994f816e36ac832f4020647afd69223a015c84c503f925013c573fed52f05420 + value: quay.io/konflux-ci/tekton-catalog/task-prefetch-dependencies-oci-ta:0.1@sha256:8fb092dae7109ac211d8b98413d9bc0c71c14f64644ce239676383576f861a86 - name: kind value: task resolver: bundles @@ -215,7 +215,7 @@ spec: - name: name value: buildah-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:11b9ce26fd2933ccc81ca3f983e094ec54326a2e0aaf8bdcc4c0b8fea1a42c53 + value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:d35ff2504c0f58fd71879c917a1c66d817143cd8586b4bbd6cff0f3f91e5d040 - name: kind value: task resolver: bundles @@ -244,7 +244,7 @@ spec: - name: name value: build-image-index - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-build-image-index:0.1@sha256:479775c8655d815fb515aeb97efc0e64284a8520c452754981970900b937a393 + value: quay.io/konflux-ci/tekton-catalog/task-build-image-index:0.1@sha256:09344e6bda708f48ef759bbe84bce99515549f4cfdcbe89e417f695c19463260 - name: kind value: task resolver: bundles @@ -287,7 +287,7 @@ spec: - name: name value: source-build-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-source-build-oci-ta:0.1@sha256:75e882bf1619dd45a4043060ce42a6ad3ce781264ade5b7f66a1d994ee159126 + value: quay.io/konflux-ci/tekton-catalog/task-source-build-oci-ta:0.1@sha256:bd191e50b2eb7e3996d7f24e38fa44b4552c142920b540fb7f3dc104e508a68f - name: kind value: task resolver: bundles @@ -381,7 +381,7 @@ spec: - name: name value: sast-snyk-check-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-sast-snyk-check-oci-ta:0.3@sha256:af93b35e6e71a6ff7f3785ad8d8497b11204a5c0c33ab1a78b44f9d43f49c7a5 + value: quay.io/konflux-ci/tekton-catalog/task-sast-snyk-check-oci-ta:0.3@sha256:9172196136831a61b9039ea4498fcdc71d6adc86d9694f236bea7b2a85488cd3 - name: kind value: task resolver: bundles @@ -423,7 +423,7 @@ spec: - name: name value: apply-tags - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-apply-tags:0.1@sha256:2c2d88c07623b2d25163994ded6e9f29205ea5bbab090f4c86379739940028b9 + value: quay.io/konflux-ci/tekton-catalog/task-apply-tags:0.1@sha256:46f4471a86130c146a6e14124f175d6ef1ddb4b80ac51e7957d78a3facb6261b - name: kind value: task resolver: bundles @@ -446,7 +446,7 @@ spec: - name: name value: push-dockerfile-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-push-dockerfile-oci-ta:0.1@sha256:98ccae6ac132ab837fc51a70514be5fca656e09d6d4ad93230bd10f0119258aa + value: quay.io/konflux-ci/tekton-catalog/task-push-dockerfile-oci-ta:0.1@sha256:6cfb72a0b5c382a3584af322d11f3ae855d07eb72a6ee2253de7b9512a0c21a5 - name: kind value: task resolver: bundles From 319ff64264fa193a1ddb8129b3a9a893570c7a27 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 20 Jan 2025 08:15:54 -0500 Subject: [PATCH 06/13] first test passing --- tests/internal/test_views.py | 52 +++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/internal/test_views.py b/tests/internal/test_views.py index 480206e34..d1bdb669d 100644 --- a/tests/internal/test_views.py +++ b/tests/internal/test_views.py @@ -31,6 +31,7 @@ from api.models import User, Tenant from api.utils import reset_imported_tenants from management.audit_log.model import AuditLog +from management.cache import TenantCache from management.models import BindingMapping, Group, Permission, Policy, Role, Workspace from management.principal.model import Principal from management.relation_replicator.noop_replicator import NoopReplicator @@ -1301,9 +1302,12 @@ class InternalViewsetResourceDefinitionTests(IdentityRequest): def setUp(self): """Set up the access view tests.""" super().setUp() - request = self.request_context["request"] + self.client = APIClient() self.customer = self.customer_data self.internal_request_context = self._create_request_context(self.customer, self.user_data, is_internal=True) + self.internal_request = self.internal_request_context["request"] + + request = self.request_context["request"] user = User() user.username = self.user_data["username"] user.account = self.customer_data["account_id"] @@ -1361,6 +1365,18 @@ def setUp(self): self.group.save() self.permission = Permission.objects.create(permission="app:*:*", tenant=self.tenant) Permission.objects.create(permission="app:foo:bar", tenant=self.tenant) + tenant_root_workspace = Workspace.objects.create( + name="root", + description="Root workspace", + tenant=self.tenant, + type=Workspace.Types.ROOT, + ) + Workspace.objects.create( + name="Tenant Default Workspace", + type=Workspace.Types.DEFAULT, + parent=tenant_root_workspace, + tenant=self.tenant, + ) def tearDown(self): """Tear down access view tests.""" @@ -1385,9 +1401,43 @@ def create_role(self, role_name, headers, in_access_data=None): self.assertEqual(response.status_code, status.HTTP_201_CREATED) return response + def create_policy(self, policy_name, group, roles, tenant): + """Create a policy.""" + # create a policy + policy = Policy.objects.create(name=policy_name, tenant=tenant, system=True) + for role in Role.objects.filter(uuid__in=roles): + policy.roles.add(role) + policy.group = Group.objects.get(uuid=group) + policy.save() + + def create_platform_default_resource(self): + """Setup default group and role.""" + default_permission = Permission.objects.create(permission="default:*:*", tenant=self.tenant) + default_role = Role.objects.create(name="default role", platform_default=True, system=True, tenant=self.tenant) + default_access = Access.objects.create(permission=default_permission, role=default_role, tenant=self.tenant) + default_policy = Policy.objects.create(name="default policy", system=True, tenant=self.tenant) + default_policy.roles.add(default_role) + default_group = Group.objects.create( + name="default group", system=True, platform_default=True, tenant=self.tenant + ) + default_group.policies.add(default_policy) + + def create_role_and_permission(self, role_name, permission): + role = Role.objects.create(name=role_name, tenant=self.tenant) + assigned_permission = Permission.objects.create(permission=permission, tenant=self.tenant) + access = Access.objects.create(role=role, permission=assigned_permission, tenant=self.tenant) + return role + def test_correct_string_resource_definition(self): """Test that a string attributeFilter can have the equal operation""" role_name = "roleA" response = self.create_role(role_name, headers=self.headers) self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) From 76f9583e1c4e735ef70fa2dd36fd3b8f40528786 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 20 Jan 2025 08:23:34 -0500 Subject: [PATCH 07/13] get method tested --- tests/internal/test_views.py | 79 +++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 5 deletions(-) diff --git a/tests/internal/test_views.py b/tests/internal/test_views.py index d1bdb669d..ffbe91e24 100644 --- a/tests/internal/test_views.py +++ b/tests/internal/test_views.py @@ -1315,11 +1315,6 @@ def setUp(self): request.user = user public_tenant = Tenant.objects.get(tenant_name="public") - self.access_data = { - "permission": "app:*:*", - "resourceDefinitions": [{"attributeFilter": {"key": "key1.id", "operation": "equal", "value": "value1"}}], - } - test_tenant_org_id = "100001" # we need to delete old test_tenant's that may exist in cache @@ -1432,6 +1427,79 @@ def test_correct_string_resource_definition(self): """Test that a string attributeFilter can have the equal operation""" role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [{"attributeFilter": {"key": "key1.id", "operation": "equal", "value": "value1"}}], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"0 resource definitions would be corrected") + + def test_incorrect_string_resource_definition(self): + """Test that a string attributeFilter cannot have the in operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [{"attributeFilter": {"key": "key1.id", "operation": "in", "value": "value1"}}], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"1 resource definitions would be corrected") + + def test_correct_list_resource_definition(self): + """Test that a list attributeFilter can have the in operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [ + {"attributeFilter": {"key": "key1.id", "operation": "in", "value": ["value1", "value2"]}} + ], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"0 resource definitions would be corrected") + + def test_correct_list_resource_definition(self): + """Test that a list attributeFilter cannot have the equal operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [ + {"attributeFilter": {"key": "key1.id", "operation": "equal", "value": ["value1", "value2"]}} + ], + } + response = self.create_role(role_name, headers=self.headers) self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -1441,3 +1509,4 @@ def test_correct_string_resource_definition(self): ) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"1 resource definitions would be corrected") From 2a6f8b146a82216399c13cb50b1f51940c0693d9 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 20 Jan 2025 08:35:32 -0500 Subject: [PATCH 08/13] add counter of changed resources in patch --- rbac/internal/views.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/rbac/internal/views.py b/rbac/internal/views.py index 2f83ab102..0bae06ce0 100644 --- a/rbac/internal/views.py +++ b/rbac/internal/views.py @@ -809,22 +809,25 @@ def correct_resource_definitions(request): return HttpResponse(f"{count} resource definitions would be corrected", status=200) elif request.method == "PATCH": + count = 0 with connection.cursor() as cursor: cursor.execute("SELECT id " + list_query) result = cursor.fetchall() for id in result: - resource_definition = ResourceDefinition.objects.get(id=id) + resource_definition = ResourceDefinition.objects.get(id=id[0]) resource_definition.attributeFilter["operation"] = "in" resource_definition.save() + count += 1 cursor.execute("SELECT id " + string_query) result = cursor.fetchall() for id in result: - resource_definition = ResourceDefinition.objects.get(id=id) + resource_definition = ResourceDefinition.objects.get(id=id[0]) resource_definition.attributeFilter["operation"] = "equal" resource_definition.save() + count += 1 - return HttpResponse("Updated bad resource definitions", status=200) + return HttpResponse(f"Updated {count} bad resource definitions", status=200) return HttpResponse('Invalid method, only "GET" or "PATCH" are allowed.', status=405) From ea43ddc84c57033c3b065080a16b8faebcdbcd13 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 20 Jan 2025 08:35:46 -0500 Subject: [PATCH 09/13] finishg tests --- tests/internal/test_views.py | 128 +++++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 4 deletions(-) diff --git a/tests/internal/test_views.py b/tests/internal/test_views.py index ffbe91e24..89738db1a 100644 --- a/tests/internal/test_views.py +++ b/tests/internal/test_views.py @@ -1423,7 +1423,7 @@ def create_role_and_permission(self, role_name, permission): access = Access.objects.create(role=role, permission=assigned_permission, tenant=self.tenant) return role - def test_correct_string_resource_definition(self): + def test_get_correct_string_resource_definition(self): """Test that a string attributeFilter can have the equal operation""" role_name = "roleA" @@ -1444,7 +1444,7 @@ def test_correct_string_resource_definition(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.content, b"0 resource definitions would be corrected") - def test_incorrect_string_resource_definition(self): + def test_get_incorrect_string_resource_definition(self): """Test that a string attributeFilter cannot have the in operation""" role_name = "roleA" @@ -1465,7 +1465,7 @@ def test_incorrect_string_resource_definition(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.content, b"1 resource definitions would be corrected") - def test_correct_list_resource_definition(self): + def test_get_correct_list_resource_definition(self): """Test that a list attributeFilter can have the in operation""" role_name = "roleA" @@ -1488,7 +1488,7 @@ def test_correct_list_resource_definition(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.content, b"0 resource definitions would be corrected") - def test_correct_list_resource_definition(self): + def test_get_incorrect_list_resource_definition(self): """Test that a list attributeFilter cannot have the equal operation""" role_name = "roleA" @@ -1510,3 +1510,123 @@ def test_correct_list_resource_definition(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.content, b"1 resource definitions would be corrected") + + def test_patch_correct_string_resource_definition(self): + """Test patching a string attributeFilter with the equal operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [{"attributeFilter": {"key": "key1.id", "operation": "equal", "value": "value1"}}], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.patch( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"Updated 0 bad resource definitions") + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"0 resource definitions would be corrected") + + def test_patch_incorrect_string_resource_definition(self): + """Test patching a string attributeFilter with the in operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [{"attributeFilter": {"key": "key1.id", "operation": "in", "value": "value1"}}], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.patch( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"Updated 1 bad resource definitions") + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"0 resource definitions would be corrected") + + def test_patch_correct_list_resource_definition(self): + """Test patching a list attributeFilter with the in operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [ + {"attributeFilter": {"key": "key1.id", "operation": "in", "value": ["value1", "value2"]}} + ], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.patch( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"Updated 0 bad resource definitions") + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"0 resource definitions would be corrected") + + def test_patch_incorrect_list_resource_definition(self): + """Test patching a list attributeFilter with the equal operation""" + + role_name = "roleA" + + self.access_data = { + "permission": "app:*:*", + "resourceDefinitions": [ + {"attributeFilter": {"key": "key1.id", "operation": "equal", "value": ["value1", "value2"]}} + ], + } + + response = self.create_role(role_name, headers=self.headers) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.patch( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"Updated 1 bad resource definitions") + + response = self.client.get( + f"/_private/api/utils/resource_definitions/", + **self.internal_request.META, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b"0 resource definitions would be corrected") From e4812c21cd762069a9178c05461434b3be8bff3d Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Mon, 20 Jan 2025 09:37:46 -0500 Subject: [PATCH 10/13] remove testing line --- rbac/rbac/dev_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rbac/rbac/dev_middleware.py b/rbac/rbac/dev_middleware.py index fc5c18fba..c4d099059 100644 --- a/rbac/rbac/dev_middleware.py +++ b/rbac/rbac/dev_middleware.py @@ -38,7 +38,7 @@ def process_request(self, request): # pylint: disable=no-self-use """ if hasattr(request, "META"): user_type = request.headers.get("User-Type") - if True or user_type and user_type in ["associate", "internal", "turnpike"]: + if user_type and user_type in ["associate", "internal", "turnpike"]: identity_header = { "identity": { "associate": { From 6cd05a672674a6023205cb85ce5661815c9886c6 Mon Sep 17 00:00:00 2001 From: Ellen-Yi-Dong Date: Mon, 20 Jan 2025 09:51:31 -0800 Subject: [PATCH 11/13] Updating buildah-oci-ta task with new sha version --- .tekton/insights-rbac-pull-request.yaml | 2 +- .tekton/insights-rbac-push.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.tekton/insights-rbac-pull-request.yaml b/.tekton/insights-rbac-pull-request.yaml index 8b43435e5..626f4b177 100644 --- a/.tekton/insights-rbac-pull-request.yaml +++ b/.tekton/insights-rbac-pull-request.yaml @@ -327,7 +327,7 @@ spec: - name: name value: buildah-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:d35ff2504c0f58fd71879c917a1c66d817143cd8586b4bbd6cff0f3f91e5d040 + value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:b1cdafdac20b33cb6ccbad64fa91abc0aa673f1bdfcf95abd5cf230e0c4ea2b4 - name: kind value: task resolver: bundles diff --git a/.tekton/insights-rbac-push.yaml b/.tekton/insights-rbac-push.yaml index def4c21f7..6cc3adf43 100644 --- a/.tekton/insights-rbac-push.yaml +++ b/.tekton/insights-rbac-push.yaml @@ -215,7 +215,7 @@ spec: - name: name value: buildah-oci-ta - name: bundle - value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:d35ff2504c0f58fd71879c917a1c66d817143cd8586b4bbd6cff0f3f91e5d040 + value: quay.io/konflux-ci/tekton-catalog/task-buildah-oci-ta:0.3@sha256:b1cdafdac20b33cb6ccbad64fa91abc0aa673f1bdfcf95abd5cf230e0c4ea2b4 - name: kind value: task resolver: bundles From 45aa4ca75847c94f39bdc77e7d464fb8d60d7739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Mon, 20 Jan 2025 22:14:45 +0100 Subject: [PATCH 12/13] update django-cors-headers to the latest version --- Pipfile | 2 +- Pipfile.lock | 30 +++++++++++++++--------------- requirements.txt | 4 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Pipfile b/Pipfile index 3a3d4bc1c..7310a4569 100644 --- a/Pipfile +++ b/Pipfile @@ -12,7 +12,7 @@ django = "==4.2.18" django-filter = "==22.1" requests = "==2.32.3" django-tenants = "==3.5.0" -django-cors-headers = "==3.13.0" +django-cors-headers = "==4.6.0" djangorestframework-csv = "==2.1.1" grpcio = "==1.68.1" grpcio-status = "==1.68.1" diff --git a/Pipfile.lock b/Pipfile.lock index 203c0fda5..fe4d44278 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "87e1b28659f134f15595e72474ccc0aa757f8928c50dd901d036892fda8be113" + "sha256": "edb1251f20ecba9af096bbfb97553d405ce0eb2ba702c0477f713291748381ce" }, "pipfile-spec": 6, "requires": { @@ -343,12 +343,12 @@ }, "django-cors-headers": { "hashes": [ - "sha256:37e42883b5f1f2295df6b4bba96eb2417a14a03270cb24b2a07f021cd4487cf4", - "sha256:f9dc6b4e3f611c3199700b3e5f3398c28757dcd559c2f82932687f3d0443cfdf" + "sha256:14d76b4b4c8d39375baeddd89e4f08899051eeaf177cb02a29bd6eae8cf63aa8", + "sha256:8edbc0497e611c24d5150e0055d3b178c6534b8ed826fb6f53b21c63f5d48ba3" ], "index": "pypi", - "markers": "python_version >= '3.7'", - "version": "==3.13.0" + "markers": "python_version >= '3.9'", + "version": "==4.6.0" }, "django-environ": { "hashes": [ @@ -702,11 +702,11 @@ }, "prompt-toolkit": { "hashes": [ - "sha256:d6623ab0477a80df74e646bdbc93621143f5caf104206aa29294d53de1a03d90", - "sha256:f49a827f90062e411f1ce1f854f2aedb3c23353244f8108b89283587397ac10e" + "sha256:544748f3860a2623ca5cd6d2795e7a14f3d0e1c3c9728359013f79877fc89bab", + "sha256:9b6427eb19e479d98acff65196a307c555eb567989e6d88ebbb1b509d9779198" ], - "markers": "python_full_version >= '3.7.0'", - "version": "==3.0.48" + "markers": "python_full_version >= '3.8.0'", + "version": "==3.0.50" }, "protobuf": { "hashes": [ @@ -1479,11 +1479,11 @@ }, "identify": { "hashes": [ - "sha256:14181a47091eb75b337af4c23078c9d09225cd4c48929f521f3bf16b09d02566", - "sha256:c10b33f250e5bba374fae86fb57f3adcebf1161bce7cdf92031915fd480c13bc" + "sha256:7bec12768ed44ea4761efb47806f0a41f86e7c0a5fdf5950d4648c90eca7e251", + "sha256:cbd1810bce79f8b671ecb20f53ee0ae8e86ae84b557de31d89709dc2a48ba881" ], "markers": "python_version >= '3.9'", - "version": "==2.6.5" + "version": "==2.6.6" }, "idna": { "hashes": [ @@ -1911,11 +1911,11 @@ }, "virtualenv": { "hashes": [ - "sha256:6345e1ff19d4b1296954cee076baaf58ff2a12a84a338c62b02eda39f20aa982", - "sha256:c12311863497992dc4b8644f8ea82d3b35bb7ef8ee82e6630d76d0197c39baf9" + "sha256:4e4cb403c0b0da39e13b46b1b2476e505cb0046b25f242bee80f62bf990b2779", + "sha256:b8b8970138d32fb606192cb97f6cd4bb644fa486be9308fb9b63f81091b5dc35" ], "markers": "python_version >= '3.8'", - "version": "==20.29.0" + "version": "==20.29.1" } } } diff --git a/requirements.txt b/requirements.txt index 6a23ef1d0..a948857cb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,7 @@ click-plugins==1.1.1 click-repl==0.3.0; python_version >= '3.6' cryptography==44.0.0; python_version >= '3.7' and python_full_version not in '3.9.0, 3.9.1' django==4.2.18; python_version >= '3.8' -django-cors-headers==3.13.0; python_version >= '3.7' +django-cors-headers==4.6.0; python_version >= '3.9' django-environ==0.10.0; python_version >= '3.5' and python_version < '4' django-extensions==3.2.3; python_version >= '3.6' django-filter==22.1; python_version >= '3.7' @@ -40,7 +40,7 @@ kombu==5.5.0rc2; python_version >= '3.8' markupsafe==3.0.2; python_version >= '3.9' packaging==24.2; python_version >= '3.8' prometheus-client==0.15.0; python_version >= '3.6' -prompt-toolkit==3.0.48; python_full_version >= '3.7.0' +prompt-toolkit==3.0.50; python_full_version >= '3.8.0' protobuf==5.29.3; python_version >= '3.8' protoc-gen-validate==1.0.4; python_version >= '3.6' psycopg2==2.9.10; python_version >= '3.8' From 92b448d857b9d91e4b7967a5d746bacc5c49dfe8 Mon Sep 17 00:00:00 2001 From: Matt Holder Date: Tue, 21 Jan 2025 07:11:35 -0500 Subject: [PATCH 13/13] linting --- rbac/internal/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rbac/internal/views.py b/rbac/internal/views.py index 0bae06ce0..f894854ff 100644 --- a/rbac/internal/views.py +++ b/rbac/internal/views.py @@ -30,7 +30,7 @@ from django.utils.html import escape from internal.utils import delete_bindings from management.cache import TenantCache -from management.models import Group, Permission, Role, ResourceDefinition +from management.models import Group, Permission, ResourceDefinition, Role from management.principal.proxy import ( API_TOKEN_HEADER, CLIENT_ID_HEADER,