Skip to content

Commit b207668

Browse files
committed
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 <[email protected]>
1 parent 1853e7c commit b207668

File tree

9 files changed

+81
-80
lines changed

9 files changed

+81
-80
lines changed

Diff for: rbac/internal/integration_views.py

+15-22
Original file line numberDiff line numberDiff line change
@@ -19,40 +19,33 @@
1919

2020
import logging
2121

22-
from django.http import HttpResponseBadRequest
23-
from django.shortcuts import redirect, reverse
2422
from management.cache import TenantCache
23+
from management.group.view import GroupViewSet
2524

2625

2726
logger = logging.getLogger(__name__)
2827
TENANTS = TenantCache()
2928

3029

31-
def groups(request, account_number):
32-
"""Formant and pass internal groups request to /groups/ API."""
33-
username = request.GET.get("username")
34-
if username:
35-
base_url = reverse("group-list")
36-
url = f"{base_url}?principals={username}"
37-
return redirect(url)
38-
else:
39-
return HttpResponseBadRequest("Username must be supplied.")
30+
def groups(request, org_id):
31+
"""Format and pass internal groups request to /groups/ API."""
32+
view = GroupViewSet.as_view({"get": "list"})
33+
return view(request)
4034

4135

42-
def groups_for_principal(request, account_number, username):
43-
"""Format and pass internal groups for principal request to /groups/ API."""
44-
base_url = reverse("group-list")
45-
url = f"{base_url}?principals={username}"
46-
return redirect(url)
36+
def groups_for_principal(request, org_id, principals):
37+
"""Format and pass /principal/<username>/groups/ request to /groups/ API."""
38+
view = GroupViewSet.as_view({"get": "list"})
39+
return view(request, principals=principals)
4740

4841

49-
def roles_from_group(request, account_number, uuid):
42+
def roles_from_group(request, org_id, uuid):
5043
"""Pass internal /groups/<uuid>/roles/ request to /groups/ API."""
51-
return redirect("group-roles", uuid=uuid)
44+
view = GroupViewSet.as_view({"get": "roles"})
45+
return view(request, uuid=uuid)
5246

5347

54-
def roles_for_group_principal(request, account_number, username, uuid):
48+
def roles_for_group_principal(request, org_id, principals, uuid):
5549
"""Pass internal /principal/<username>/groups/<uuid>/roles/ request to /groups/ API."""
56-
base_url = reverse("group-roles", kwargs={"uuid": uuid})
57-
url = f"{base_url}?principals={username}"
58-
return redirect(url)
50+
view = GroupViewSet.as_view({"get": "roles"})
51+
return view(request, uuid=uuid, principals=principals)

Diff for: rbac/internal/middleware.py

+5-13
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
from django.utils.deprecation import MiddlewareMixin
2828

2929
from api.common import RH_IDENTITY_HEADER
30-
from api.models import Tenant, User
30+
from api.models import Tenant
3131
from api.serializers import extract_header
32-
from rbac.middleware import IdentityHeaderMiddleware
32+
from .utils import build_internal_user
3333

3434

3535
logger = logging.getLogger(__name__)
@@ -51,28 +51,20 @@ def process_request(self, request):
5151
logger.exception("Invalid X-RH-Identity header.")
5252
return HttpResponseForbidden()
5353

54-
user = User()
54+
user = build_internal_user(request, json_rh_auth)
5555
try:
5656
if not json_rh_auth["identity"]["type"] == "Associate":
5757
return HttpResponseForbidden()
5858
user.username = json_rh_auth["identity"]["associate"]["email"]
5959
user.admin = True
60+
user.account = resolve(request.path).kwargs.get("org_id")
61+
request.tenant = get_object_or_404(Tenant, tenant_name=user.account)
6062
except KeyError:
6163
logger.error("Malformed X-RH-Identity header.")
6264
return HttpResponseForbidden()
6365

64-
target = resolve(request.path)
65-
if target and "integration" in target:
66-
return IdentityHeaderMiddleware.process_request(self, request)
67-
6866
request.user = user
6967

7068
def process_response(self, request, response):
7169
"""Process responses for internal identity middleware."""
7270
return response
73-
74-
def get_tenant(self, request):
75-
"""Ensure internal requests carry proper tenant id."""
76-
request.tenant = get_object_or_404(
77-
Tenant, tenant_name=self.tenant_re.match(request.path_info).group("tenant_id")
78-
)

Diff for: rbac/internal/urls.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,19 @@
2626
path("api/tenant/unmodified/", views.list_unmodified_tenants),
2727
path("api/tenant/", views.list_tenants),
2828
path("api/tenant/<str:tenant_name>/", views.tenant_view),
29-
path("api/tenant/<str:account_number>/groups/", integration_views.groups, name="integration-groups"),
29+
path("api/tenant/<str:org_id>/groups/", integration_views.groups, name="integration-groups"),
3030
path(
31-
"api/tenant/<str:account_number>/groups/<str:uuid>/roles/",
31+
"api/tenant/<str:org_id>/groups/<str:uuid>/roles/",
3232
integration_views.roles_from_group,
3333
name="integration-group-roles",
3434
),
3535
path(
36-
"api/tenant/<str:account_number>/principal/<str:username>/groups/",
36+
"api/tenant/<str:org_id>/principal/<str:principals>/groups/",
3737
integration_views.groups_for_principal,
3838
name="integration-princ-groups",
3939
),
4040
path(
41-
"api/tenant/<str:account_number>/principal/<str:username>/groups/<str:uuid>/roles/",
41+
"api/tenant/<str:org_id>/principal/<str:principals>/groups/<str:uuid>/roles/",
4242
integration_views.roles_for_group_principal,
4343
name="integration-princ-roles",
4444
),

Diff for: rbac/management/group/view.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class GroupViewSet(
151151

152152
def get_queryset(self):
153153
"""Obtain queryset for requesting user based on access."""
154-
return get_group_queryset(self.request)
154+
return get_group_queryset(self.request, self.args, self.kwargs)
155155

156156
def get_serializer_class(self):
157157
"""Get serializer based on route."""
@@ -311,6 +311,7 @@ def destroy(self, request, *args, **kwargs):
311311
@apiSuccessExample {json} Success-Response:
312312
HTTP/1.1 204 NO CONTENT
313313
"""
314+
import pdb; pdb.set_trace()
314315
validate_uuid(kwargs.get("uuid"), "group uuid validation")
315316
self.protect_default_groups("delete")
316317
group = self.get_object()
@@ -552,7 +553,7 @@ def principals(self, request, uuid=None):
552553
return response
553554

554555
@action(detail=True, methods=["get", "post", "delete"])
555-
def roles(self, request, uuid=None):
556+
def roles(self, request, uuid=None, principals=None):
556557
"""Get, add or remove roles from a group."""
557558
"""
558559
@api {get} /api/v1/groups/:uuid/roles/ Get roles for a group

Diff for: rbac/management/querysets.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ def has_group_all_access(request):
8383
)
8484

8585

86-
def get_group_queryset(request):
86+
def get_group_queryset(request, args, kwargs):
8787
"""Obtain the queryset for groups."""
88-
return _filter_admin_default(request, _gather_group_querysets(request))
88+
return _filter_admin_default(request, _gather_group_querysets(request, args, kwargs))
8989

9090

91-
def _gather_group_querysets(request):
91+
def _gather_group_querysets(request, args, kwargs):
9292
"""Decide which groups to provide for request."""
9393
if settings.AUTHENTICATE_WITH_ORG_ID:
9494
scope = request.query_params.get(SCOPE_KEY, ORG_ID_SCOPE)
@@ -104,7 +104,7 @@ def _gather_group_querysets(request):
104104
tenant=request.tenant
105105
) or Group.platform_default_set().filter(tenant=public_tenant)
106106

107-
username = request.query_params.get("username")
107+
username = request.query_params.get("username") or kwargs.get("principals")
108108
if username:
109109
principal = get_principal(username, request)
110110
if principal.cross_account:

Diff for: rbac/rbac/dev_middleware.py

+29-14
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,36 @@ def process_request(self, request): # pylint: disable=no-self-use
3737
3838
"""
3939
if hasattr(request, "META"):
40-
identity_header = {
41-
"identity": {
42-
"account_number": "10001",
43-
"org_id": "11111",
44-
"type": "Associate",
45-
"associate": {
46-
"username": "user_dev",
47-
"email": "[email protected]",
48-
"is_org_admin": True,
49-
"is_internal": True,
50-
"user_id": "51736777",
51-
},
52-
"internal": {"cross_access": False},
40+
user_type = request.headers.get("User-Type")
41+
if user_type and user_type in ["associate", "internal", "turnpike"]:
42+
identity_header = {
43+
"identity": {
44+
"associate": {
45+
"Role": ["role"],
46+
"email": "[email protected]",
47+
"givenName": "Associate",
48+
"surname": "dev",
49+
},
50+
"auth_type": "saml-auth",
51+
"type": "Associate",
52+
}
53+
}
54+
else:
55+
identity_header = {
56+
"identity": {
57+
"account_number": "10001",
58+
"org_id": "11111",
59+
"type": "User",
60+
"user": {
61+
"username": "user_dev",
62+
"email": "[email protected]",
63+
"is_org_admin": True,
64+
"is_internal": True,
65+
"user_id": "51736777",
66+
},
67+
"internal": {"cross_access": False},
68+
}
5369
}
54-
}
5570
json_identity = json_dumps(identity_header)
5671
dev_header = b64encode(json_identity.encode("utf-8"))
5772
request.META[self.header] = dev_header

Diff for: rbac/rbac/middleware.py

+3-7
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class IdentityHeaderMiddleware(MiddlewareMixin):
7474

7575
header = RH_IDENTITY_HEADER
7676

77-
def get_tenant(self, request):
77+
def get_tenant(self, model, hostname, request):
7878
"""Override the tenant selection logic."""
7979
if settings.AUTHENTICATE_WITH_ORG_ID:
8080
tenant_name = create_tenant_name(request.user.account)
@@ -184,11 +184,7 @@ def process_request(self, request): # pylint: disable=R1710
184184
_, json_rh_auth = extract_header(request, self.header)
185185
user.account = json_rh_auth.get("identity", {})["account_number"]
186186
user.org_id = json_rh_auth.get("identity", {}).get("org_id")
187-
usertype = json_rh_auth.get("identity", {}).get("type")
188-
if usertype.lower() == "associate":
189-
user_info = json_rh_auth.get("identity", {}).get("associate", {})
190-
else:
191-
user_info = json_rh_auth.get("identity", {}).get("user", {})
187+
user_info = json_rh_auth.get("identity", {}).get("user")
192188
user.username = user_info["username"]
193189
user.admin = user_info.get("is_org_admin")
194190
user.internal = user_info.get("is_internal")
@@ -243,7 +239,7 @@ def process_request(self, request): # pylint: disable=R1710
243239
raise error
244240
if user.username and (user.account or user.org_id):
245241
request.user = user
246-
request.tenant = self.get_tenant(request=request)
242+
request.tenant = self.get_tenant(model=None, hostname=None, request=request)
247243

248244
def process_response(self, request, response): # pylint: disable=no-self-use
249245
"""Process response for identity middleware.

Diff for: tests/identity_request.py

+1
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ def _create_request_context(
114114
mock_header = b64encode(json_identity.encode("utf-8"))
115115
request = Mock()
116116
request.META = {RH_IDENTITY_HEADER: mock_header}
117+
request.scope = {}
117118
request_context = {"request": request}
118119
return request_context
119120

Diff for: tests/internal/test_integration_views.py

+17-14
Original file line numberDiff line numberDiff line change
@@ -119,21 +119,21 @@ def test_groups_user_filter(self):
119119
)
120120
self.assertEqual(response.status_code, status.HTTP_200_OK)
121121
# Expecting ["Group All", "Group A"]
122+
expected = []
123+
expected.append(Group.objects.get(name="Group A"))
124+
expected.append(Group.objects.get(name="Group All"))
122125
self.assertEqual(response.data.get("meta").get("count"), 2)
126+
self.assertTrue(expected, response.data)
123127

124-
def test_groups_nonexistent_user(self):
128+
@patch(
129+
"management.principal.proxy.PrincipalProxy.request_filtered_principals",
130+
return_value={"status_code": 200, "data": []},
131+
)
132+
def test_groups_nonexistent_user(self, mock_request):
125133
"""Test that a request for groups of a nonexistent user returns 0."""
126134
response = self.client.get(
127135
f"/_private/api/tenant/{self.tenant.tenant_name}/groups/?username=user_x", **self.request.META, follow=True
128136
)
129-
self.assertEqual(response.status_code, status.HTTP_200_OK)
130-
self.assertEqual(response.data.get("meta").get("count"), 0)
131-
132-
def test_groups_empty_user(self):
133-
"""Test that a request for groups without a ?username param returns as a bad request."""
134-
response = self.client.get(
135-
f"/_private/api/tenant/{self.tenant.tenant_name}/groups/", **self.request.META, follow=True
136-
)
137137
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
138138

139139
def test_groups_for_principal_valid_account(self):
@@ -159,21 +159,24 @@ def test_groups_for_principal_invalid_account(self):
159159
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
160160

161161
def test_groups_for_principal_filter(self):
162-
"""Test that only the groups a user is a member of are returned for a /tenant/<id>/principal/user_a/groups/ request."""
162+
"""Test that only the groups a user is a member of are returned for a /tenant/<id>/groups/?username= request."""
163163
response = self.client.get(
164164
f"/_private/api/tenant/{self.tenant.tenant_name}/groups/?username=user_a", **self.request.META, follow=True
165165
)
166166
self.assertEqual(response.status_code, status.HTTP_200_OK)
167167
# Expecting ["Group All", "Group A"]
168168
self.assertEqual(response.data.get("meta").get("count"), 2)
169169

170-
def test_groups_for_principal_nonexsistant_user(self):
171-
"""Test that only the groups a user is a member of are returned for a /tenant/<id>/principal/user_x/groups/ request."""
170+
@patch(
171+
"management.principal.proxy.PrincipalProxy.request_filtered_principals",
172+
return_value={"status_code": 200, "data": []},
173+
)
174+
def test_groups_for_principal_nonexistant_user(self, mock_request):
175+
"""Test that an error is return for nonexistant ."""
172176
response = self.client.get(
173177
f"/_private/api/tenant/{self.tenant.tenant_name}/groups/?username=user_x", **self.request.META, follow=True
174178
)
175-
self.assertEqual(response.status_code, status.HTTP_200_OK)
176-
self.assertEqual(response.data.get("meta").get("count"), 0)
179+
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
177180

178181
def test_roles_from_group_valid(self):
179182
"""Test that a valid request to /tenant/<id>/groups/<uuid>/roles/ from an internal account works."""

0 commit comments

Comments
 (0)