From b207668440faf8f951dec75ffef8891343b4131b Mon Sep 17 00:00:00 2001 From: Chris Mitchell Date: Wed, 18 May 2022 11:26:44 -0400 Subject: [PATCH] Functionality and tests for OCM integration There's a lot here, and it's a bit of a mess. We needed some additional views for internal users who want to use RBAC's format without necessarily all of the functionality. Originally, attempts were made at using a redirect to get the requests into our normal flow, but this resulted in multiple passes through multiple middleware, so now I'm attempting to just re-use our normal views with the new request data. Signed-off-by: Chris Mitchell --- rbac/internal/integration_views.py | 37 +++++++++----------- rbac/internal/middleware.py | 18 +++------- rbac/internal/urls.py | 8 ++--- rbac/management/group/view.py | 5 +-- rbac/management/querysets.py | 8 ++--- rbac/rbac/dev_middleware.py | 43 ++++++++++++++++-------- rbac/rbac/middleware.py | 10 ++---- tests/identity_request.py | 1 + tests/internal/test_integration_views.py | 31 +++++++++-------- 9 files changed, 81 insertions(+), 80 deletions(-) diff --git a/rbac/internal/integration_views.py b/rbac/internal/integration_views.py index ceed97925..8e8372cf3 100644 --- a/rbac/internal/integration_views.py +++ b/rbac/internal/integration_views.py @@ -19,40 +19,33 @@ import logging -from django.http import HttpResponseBadRequest -from django.shortcuts import redirect, reverse from management.cache import TenantCache +from management.group.view import GroupViewSet logger = logging.getLogger(__name__) TENANTS = TenantCache() -def groups(request, account_number): - """Formant and pass internal groups request to /groups/ API.""" - username = request.GET.get("username") - if username: - base_url = reverse("group-list") - url = f"{base_url}?principals={username}" - return redirect(url) - else: - return HttpResponseBadRequest("Username must be supplied.") +def groups(request, org_id): + """Format and pass internal groups request to /groups/ API.""" + view = GroupViewSet.as_view({"get": "list"}) + return view(request) -def groups_for_principal(request, account_number, username): - """Format and pass internal groups for principal request to /groups/ API.""" - base_url = reverse("group-list") - url = f"{base_url}?principals={username}" - return redirect(url) +def groups_for_principal(request, org_id, principals): + """Format and pass /principal//groups/ request to /groups/ API.""" + view = GroupViewSet.as_view({"get": "list"}) + return view(request, principals=principals) -def roles_from_group(request, account_number, uuid): +def roles_from_group(request, org_id, uuid): """Pass internal /groups//roles/ request to /groups/ API.""" - return redirect("group-roles", uuid=uuid) + view = GroupViewSet.as_view({"get": "roles"}) + return view(request, uuid=uuid) -def roles_for_group_principal(request, account_number, username, uuid): +def roles_for_group_principal(request, org_id, principals, uuid): """Pass internal /principal//groups//roles/ request to /groups/ API.""" - base_url = reverse("group-roles", kwargs={"uuid": uuid}) - url = f"{base_url}?principals={username}" - return redirect(url) + view = GroupViewSet.as_view({"get": "roles"}) + return view(request, uuid=uuid, principals=principals) diff --git a/rbac/internal/middleware.py b/rbac/internal/middleware.py index fb9a6f8df..208bc7fb6 100644 --- a/rbac/internal/middleware.py +++ b/rbac/internal/middleware.py @@ -27,9 +27,9 @@ from django.utils.deprecation import MiddlewareMixin from api.common import RH_IDENTITY_HEADER -from api.models import Tenant, User +from api.models import Tenant from api.serializers import extract_header -from rbac.middleware import IdentityHeaderMiddleware +from .utils import build_internal_user logger = logging.getLogger(__name__) @@ -51,28 +51,20 @@ def process_request(self, request): logger.exception("Invalid X-RH-Identity header.") return HttpResponseForbidden() - user = User() + user = build_internal_user(request, json_rh_auth) try: if not json_rh_auth["identity"]["type"] == "Associate": return HttpResponseForbidden() user.username = json_rh_auth["identity"]["associate"]["email"] user.admin = True + user.account = resolve(request.path).kwargs.get("org_id") + request.tenant = get_object_or_404(Tenant, tenant_name=user.account) except KeyError: logger.error("Malformed X-RH-Identity header.") return HttpResponseForbidden() - target = resolve(request.path) - if target and "integration" in target: - return IdentityHeaderMiddleware.process_request(self, request) - request.user = user def process_response(self, request, response): """Process responses for internal identity middleware.""" return response - - def get_tenant(self, request): - """Ensure internal requests carry proper tenant id.""" - request.tenant = get_object_or_404( - Tenant, tenant_name=self.tenant_re.match(request.path_info).group("tenant_id") - ) diff --git a/rbac/internal/urls.py b/rbac/internal/urls.py index c1a9b10ef..542bf3606 100644 --- a/rbac/internal/urls.py +++ b/rbac/internal/urls.py @@ -26,19 +26,19 @@ path("api/tenant/unmodified/", views.list_unmodified_tenants), path("api/tenant/", views.list_tenants), path("api/tenant//", views.tenant_view), - path("api/tenant//groups/", integration_views.groups, name="integration-groups"), + path("api/tenant//groups/", integration_views.groups, name="integration-groups"), path( - "api/tenant//groups//roles/", + "api/tenant//groups//roles/", integration_views.roles_from_group, name="integration-group-roles", ), path( - "api/tenant//principal//groups/", + "api/tenant//principal//groups/", integration_views.groups_for_principal, name="integration-princ-groups", ), path( - "api/tenant//principal//groups//roles/", + "api/tenant//principal//groups//roles/", integration_views.roles_for_group_principal, name="integration-princ-roles", ), diff --git a/rbac/management/group/view.py b/rbac/management/group/view.py index 26f274190..e7e4bfbef 100644 --- a/rbac/management/group/view.py +++ b/rbac/management/group/view.py @@ -151,7 +151,7 @@ class GroupViewSet( def get_queryset(self): """Obtain queryset for requesting user based on access.""" - return get_group_queryset(self.request) + return get_group_queryset(self.request, self.args, self.kwargs) def get_serializer_class(self): """Get serializer based on route.""" @@ -311,6 +311,7 @@ def destroy(self, request, *args, **kwargs): @apiSuccessExample {json} Success-Response: HTTP/1.1 204 NO CONTENT """ + import pdb; pdb.set_trace() validate_uuid(kwargs.get("uuid"), "group uuid validation") self.protect_default_groups("delete") group = self.get_object() @@ -552,7 +553,7 @@ def principals(self, request, uuid=None): return response @action(detail=True, methods=["get", "post", "delete"]) - def roles(self, request, uuid=None): + def roles(self, request, uuid=None, principals=None): """Get, add or remove roles from a group.""" """ @api {get} /api/v1/groups/:uuid/roles/ Get roles for a group diff --git a/rbac/management/querysets.py b/rbac/management/querysets.py index 997d52877..459f56f6c 100644 --- a/rbac/management/querysets.py +++ b/rbac/management/querysets.py @@ -83,12 +83,12 @@ def has_group_all_access(request): ) -def get_group_queryset(request): +def get_group_queryset(request, args, kwargs): """Obtain the queryset for groups.""" - return _filter_admin_default(request, _gather_group_querysets(request)) + return _filter_admin_default(request, _gather_group_querysets(request, args, kwargs)) -def _gather_group_querysets(request): +def _gather_group_querysets(request, args, kwargs): """Decide which groups to provide for request.""" if settings.AUTHENTICATE_WITH_ORG_ID: scope = request.query_params.get(SCOPE_KEY, ORG_ID_SCOPE) @@ -104,7 +104,7 @@ def _gather_group_querysets(request): tenant=request.tenant ) or Group.platform_default_set().filter(tenant=public_tenant) - username = request.query_params.get("username") + username = request.query_params.get("username") or kwargs.get("principals") if username: principal = get_principal(username, request) if principal.cross_account: diff --git a/rbac/rbac/dev_middleware.py b/rbac/rbac/dev_middleware.py index 80125d30d..c4d099059 100644 --- a/rbac/rbac/dev_middleware.py +++ b/rbac/rbac/dev_middleware.py @@ -37,21 +37,36 @@ def process_request(self, request): # pylint: disable=no-self-use """ if hasattr(request, "META"): - identity_header = { - "identity": { - "account_number": "10001", - "org_id": "11111", - "type": "Associate", - "associate": { - "username": "user_dev", - "email": "user_dev@foo.com", - "is_org_admin": True, - "is_internal": True, - "user_id": "51736777", - }, - "internal": {"cross_access": False}, + user_type = request.headers.get("User-Type") + if user_type and user_type in ["associate", "internal", "turnpike"]: + identity_header = { + "identity": { + "associate": { + "Role": ["role"], + "email": "associate_dev@bar.com", + "givenName": "Associate", + "surname": "dev", + }, + "auth_type": "saml-auth", + "type": "Associate", + } + } + else: + identity_header = { + "identity": { + "account_number": "10001", + "org_id": "11111", + "type": "User", + "user": { + "username": "user_dev", + "email": "user_dev@foo.com", + "is_org_admin": True, + "is_internal": True, + "user_id": "51736777", + }, + "internal": {"cross_access": False}, + } } - } json_identity = json_dumps(identity_header) dev_header = b64encode(json_identity.encode("utf-8")) request.META[self.header] = dev_header diff --git a/rbac/rbac/middleware.py b/rbac/rbac/middleware.py index d9c58d38a..e64911b7c 100644 --- a/rbac/rbac/middleware.py +++ b/rbac/rbac/middleware.py @@ -74,7 +74,7 @@ class IdentityHeaderMiddleware(MiddlewareMixin): header = RH_IDENTITY_HEADER - def get_tenant(self, request): + def get_tenant(self, model, hostname, request): """Override the tenant selection logic.""" if settings.AUTHENTICATE_WITH_ORG_ID: tenant_name = create_tenant_name(request.user.account) @@ -184,11 +184,7 @@ def process_request(self, request): # pylint: disable=R1710 _, json_rh_auth = extract_header(request, self.header) user.account = json_rh_auth.get("identity", {})["account_number"] user.org_id = json_rh_auth.get("identity", {}).get("org_id") - usertype = json_rh_auth.get("identity", {}).get("type") - if usertype.lower() == "associate": - user_info = json_rh_auth.get("identity", {}).get("associate", {}) - else: - user_info = json_rh_auth.get("identity", {}).get("user", {}) + user_info = json_rh_auth.get("identity", {}).get("user") user.username = user_info["username"] user.admin = user_info.get("is_org_admin") user.internal = user_info.get("is_internal") @@ -243,7 +239,7 @@ def process_request(self, request): # pylint: disable=R1710 raise error if user.username and (user.account or user.org_id): request.user = user - request.tenant = self.get_tenant(request=request) + request.tenant = self.get_tenant(model=None, hostname=None, request=request) def process_response(self, request, response): # pylint: disable=no-self-use """Process response for identity middleware. diff --git a/tests/identity_request.py b/tests/identity_request.py index df537c424..644a3140a 100644 --- a/tests/identity_request.py +++ b/tests/identity_request.py @@ -114,6 +114,7 @@ def _create_request_context( mock_header = b64encode(json_identity.encode("utf-8")) request = Mock() request.META = {RH_IDENTITY_HEADER: mock_header} + request.scope = {} request_context = {"request": request} return request_context diff --git a/tests/internal/test_integration_views.py b/tests/internal/test_integration_views.py index e74c9eba9..a744d840d 100644 --- a/tests/internal/test_integration_views.py +++ b/tests/internal/test_integration_views.py @@ -119,21 +119,21 @@ def test_groups_user_filter(self): ) self.assertEqual(response.status_code, status.HTTP_200_OK) # Expecting ["Group All", "Group A"] + expected = [] + expected.append(Group.objects.get(name="Group A")) + expected.append(Group.objects.get(name="Group All")) self.assertEqual(response.data.get("meta").get("count"), 2) + self.assertTrue(expected, response.data) - def test_groups_nonexistent_user(self): + @patch( + "management.principal.proxy.PrincipalProxy.request_filtered_principals", + return_value={"status_code": 200, "data": []}, + ) + def test_groups_nonexistent_user(self, mock_request): """Test that a request for groups of a nonexistent user returns 0.""" response = self.client.get( f"/_private/api/tenant/{self.tenant.tenant_name}/groups/?username=user_x", **self.request.META, follow=True ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data.get("meta").get("count"), 0) - - def test_groups_empty_user(self): - """Test that a request for groups without a ?username param returns as a bad request.""" - response = self.client.get( - f"/_private/api/tenant/{self.tenant.tenant_name}/groups/", **self.request.META, follow=True - ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) def test_groups_for_principal_valid_account(self): @@ -159,7 +159,7 @@ def test_groups_for_principal_invalid_account(self): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_groups_for_principal_filter(self): - """Test that only the groups a user is a member of are returned for a /tenant//principal/user_a/groups/ request.""" + """Test that only the groups a user is a member of are returned for a /tenant//groups/?username= request.""" response = self.client.get( f"/_private/api/tenant/{self.tenant.tenant_name}/groups/?username=user_a", **self.request.META, follow=True ) @@ -167,13 +167,16 @@ def test_groups_for_principal_filter(self): # Expecting ["Group All", "Group A"] self.assertEqual(response.data.get("meta").get("count"), 2) - def test_groups_for_principal_nonexsistant_user(self): - """Test that only the groups a user is a member of are returned for a /tenant//principal/user_x/groups/ request.""" + @patch( + "management.principal.proxy.PrincipalProxy.request_filtered_principals", + return_value={"status_code": 200, "data": []}, + ) + def test_groups_for_principal_nonexistant_user(self, mock_request): + """Test that an error is return for nonexistant .""" response = self.client.get( f"/_private/api/tenant/{self.tenant.tenant_name}/groups/?username=user_x", **self.request.META, follow=True ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data.get("meta").get("count"), 0) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) def test_roles_from_group_valid(self): """Test that a valid request to /tenant//groups//roles/ from an internal account works."""