From e3828b71b19ba9353448ec22e382a1e30f319679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roger=20Mart=C3=ADnez?= Date: Wed, 15 Jan 2025 19:41:42 +0100 Subject: [PATCH] AAP-38623: Internal RH user check. (#1494) --- ansible_ai_connect/ai/api/tests/test_views.py | 8 +- ansible_ai_connect/ai/api/views.py | 4 +- ansible_ai_connect/main/permissions.py | 8 +- .../main/tests/test_permissions.py | 14 +-- ansible_ai_connect/main/tests/test_views.py | 6 +- ansible_ai_connect/main/views.py | 4 +- .../migrations/0014_user_rh_internal_user.py | 22 +++++ ansible_ai_connect/users/models.py | 2 +- ansible_ai_connect/users/pipeline.py | 13 ++- .../users/templates/users/home.html | 2 +- .../users/templates/users/trial.html | 2 +- .../users/tests/test_pipeline.py | 85 +++++++++++++++++-- ansible_ai_connect/users/views.py | 4 +- 13 files changed, 136 insertions(+), 38 deletions(-) create mode 100644 ansible_ai_connect/users/migrations/0014_user_rh_internal_user.py diff --git a/ansible_ai_connect/ai/api/tests/test_views.py b/ansible_ai_connect/ai/api/tests/test_views.py index 9dc0351a0..5cc1218b4 100644 --- a/ansible_ai_connect/ai/api/tests/test_views.py +++ b/ansible_ai_connect/ai/api/tests/test_views.py @@ -3985,7 +3985,7 @@ def setUp(self): super().setUp() (org, _) = Organization.objects.get_or_create(id=123, telemetry_opt_out=False) self.user.organization = org - self.user.rh_employee = True + self.user.rh_internal = True @staticmethod def mocked_requests_post(*args, **kwargs): @@ -4323,7 +4323,7 @@ def test_chat_rate_limit(self): ) (org, _) = Organization.objects.get_or_create(id=123, telemetry_opt_out=False) self.user2.organization = org - self.user2.rh_employee = True + self.user2.rh_internal = True # Call chart API five times using self.user2 for i in range(5): self.assert_test(TestChatView.VALID_PAYLOAD, user=self.user2) @@ -4333,7 +4333,7 @@ def test_chat_rate_limit(self): if self.user2: self.user2.delete() - def test_not_rh_employee_user(self): + def test_not_rh_internal_user(self): try: username = "u" + "".join(random.choices(string.digits, k=5)) self.user2 = get_user_model().objects.create_user( @@ -4342,7 +4342,7 @@ def test_not_rh_employee_user(self): self.user2.organization = Organization.objects.get_or_create( id=123, telemetry_opt_out=False )[0] - self.user2.rh_employee = False + self.user2.rh_internal = False self.assert_test(TestChatView.VALID_PAYLOAD, expected_status_code=403, user=self.user2) finally: if self.user2: diff --git a/ansible_ai_connect/ai/api/views.py b/ansible_ai_connect/ai/api/views.py index a2f61a327..a5f66d95e 100644 --- a/ansible_ai_connect/ai/api/views.py +++ b/ansible_ai_connect/ai/api/views.py @@ -97,7 +97,7 @@ ) from ansible_ai_connect.users.models import User -from ...main.permissions import IsRHEmployee, IsTestUser +from ...main.permissions import IsRHInternalUser, IsTestUser from ...users.throttling import EndpointRateThrottle from ..feature_flags import FeatureFlags from .data.data_model import ContentMatchPayloadData, ContentMatchResponseDto @@ -950,7 +950,7 @@ class ChatEndpointThrottle(EndpointRateThrottle): permission_classes = [ permissions.IsAuthenticated, IsAuthenticatedOrTokenHasScope, - IsRHEmployee | IsTestUser, + IsRHInternalUser | IsTestUser, ] required_scopes = ["read", "write"] schema1_event = schema1.ChatBotOperationalEvent diff --git a/ansible_ai_connect/main/permissions.py b/ansible_ai_connect/main/permissions.py index 722622dee..de65f133c 100644 --- a/ansible_ai_connect/main/permissions.py +++ b/ansible_ai_connect/main/permissions.py @@ -14,17 +14,17 @@ from rest_framework.permissions import BasePermission -class IsRHEmployee(BasePermission): +class IsRHInternalUser(BasePermission): """ - Allow access only to users who are Red Hat employees + Allow access only to users who are Red Hat internal users. """ - code = "permission_denied__user_not_rh_employee" + code = "permission_denied__user_not_rh_internal" message = "The User is not a Red Hat employee." def has_permission(self, request, view): user = request.user - return user.is_authenticated and user.rh_employee + return user.is_authenticated and user.rh_internal class IsTestUser(BasePermission): diff --git a/ansible_ai_connect/main/tests/test_permissions.py b/ansible_ai_connect/main/tests/test_permissions.py index 7cbc0ae06..84306fb7a 100644 --- a/ansible_ai_connect/main/tests/test_permissions.py +++ b/ansible_ai_connect/main/tests/test_permissions.py @@ -17,14 +17,14 @@ from django.test import RequestFactory, TestCase from django.urls import resolve, reverse -from ansible_ai_connect.main.permissions import IsRHEmployee, IsTestUser +from ansible_ai_connect.main.permissions import IsRHInternalUser, IsTestUser -class TestIsRHEmployee(TestCase): +class TestIsRHInternalUser(TestCase): def setUp(self): super().setUp() - self.permission = IsRHEmployee() + self.permission = IsRHInternalUser() payload = { "query": "Hello", @@ -35,13 +35,13 @@ def setUp(self): username="non-rh-user", password="non-rh-password", email="non-rh-user@email.com", - rh_employee=False, + rh_internal=False, ) self.rh_user = get_user_model().objects.create_user( username="rh-user", password="rh-password", email="rh-user@redhat.com", - rh_employee=True, + rh_internal=True, ) def tearDown(self): @@ -82,7 +82,7 @@ def setUp(self): username="non-rh-user", password="non-rh-password", email="non-rh-user@email.com", - rh_employee=False, + rh_internal=False, ) self.test_group = Group(name="test") self.test_group.save() @@ -90,7 +90,7 @@ def setUp(self): username="non-rh-test-user", password="non-rh-test-password", email="non-rh-test-user@email.com", - rh_employee=False, + rh_internal=False, ) self.non_rh_test_user.groups.add(self.test_group) diff --git a/ansible_ai_connect/main/tests/test_views.py b/ansible_ai_connect/main/tests/test_views.py index 391a7f8c3..a1035ac99 100644 --- a/ansible_ai_connect/main/tests/test_views.py +++ b/ansible_ai_connect/main/tests/test_views.py @@ -255,13 +255,13 @@ def setUp(self): username="non-rh-user", password="non-rh-password", email="non-rh-user@email.com", - rh_employee=False, + rh_internal=False, ) self.rh_user = get_user_model().objects.create_user( username="rh-user", password="rh-password", email="rh-user@redhat.com", - rh_employee=True, + rh_internal=True, ) self.test_group = Group(name="test") self.test_group.save() @@ -269,7 +269,7 @@ def setUp(self): username="non-rh-test-user", password="non-rh-test-password", email="non-rh-test-user@email.com", - rh_employee=False, + rh_internal=False, ) self.non_rh_test_user.groups.add(self.test_group) diff --git a/ansible_ai_connect/main/views.py b/ansible_ai_connect/main/views.py index 6e96fc575..00fe279fa 100644 --- a/ansible_ai_connect/main/views.py +++ b/ansible_ai_connect/main/views.py @@ -32,7 +32,7 @@ IsOrganisationLightspeedSubscriber, ) from ansible_ai_connect.main.base_views import ProtectedTemplateView -from ansible_ai_connect.main.permissions import IsRHEmployee, IsTestUser +from ansible_ai_connect.main.permissions import IsRHInternalUser, IsTestUser from ansible_ai_connect.main.settings.base import SOCIAL_AUTH_OIDC_KEY logger = logging.getLogger(__name__) @@ -116,7 +116,7 @@ class ChatbotView(ProtectedTemplateView): template_name = "chatbot/index.html" permission_classes = [ - IsRHEmployee | IsTestUser, + IsRHInternalUser | IsTestUser, ] def get(self, request): diff --git a/ansible_ai_connect/users/migrations/0014_user_rh_internal_user.py b/ansible_ai_connect/users/migrations/0014_user_rh_internal_user.py new file mode 100644 index 000000000..3bb14afb1 --- /dev/null +++ b/ansible_ai_connect/users/migrations/0014_user_rh_internal_user.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.11 on 2024-07-19 17:55 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0013_user_email_verified_user_family_name_user_given_name_and_more"), + ] + + operations = [ + migrations.RemoveField( + model_name="user", + name="rh_employee", + ), + migrations.AddField( + model_name="user", + name="rh_internal", + field=models.BooleanField(default=False), + ), + ] diff --git a/ansible_ai_connect/users/models.py b/ansible_ai_connect/users/models.py index 8226de537..fd0e37df0 100644 --- a/ansible_ai_connect/users/models.py +++ b/ansible_ai_connect/users/models.py @@ -61,7 +61,7 @@ class User(ExportModelOperationsMixin("user"), AbstractUser): on_delete=models.CASCADE, ) rh_user_is_org_admin = models.BooleanField(default=False) - rh_employee = models.BooleanField(default=False) + rh_internal = models.BooleanField(default=False) external_username = models.CharField(default="", null=False) name = models.CharField(default=None, null=True) given_name = models.CharField(default=None, null=True) diff --git a/ansible_ai_connect/users/pipeline.py b/ansible_ai_connect/users/pipeline.py index 0a0680a50..bc5505b32 100644 --- a/ansible_ai_connect/users/pipeline.py +++ b/ansible_ai_connect/users/pipeline.py @@ -84,6 +84,15 @@ def github_get_username(strategy, details, backend, user=None, *args, **kwargs): return get_username(strategy, details, backend, user, *args, **kwargs) +def is_rh_email_domain(email) -> bool: + email_domain: str = email[email.find("@") + 1 :] if email else None + return email_domain.lower() == "redhat.com" if email_domain else False + + +def is_rh_internal_user(user) -> bool: + return is_rh_email_domain(user.email) and bool(user.email_verified) + + def redhat_organization(backend, user, response, *args, **kwargs): if backend.name != "oidc": return @@ -106,7 +115,7 @@ def redhat_organization(backend, user, response, *args, **kwargs): user.email = payload.get("email") user.email_verified = payload.get("email_verified") user.rh_user_is_org_admin = "admin:org:all" in roles - user.rh_employee = "redhat:employees" in roles + user.rh_internal = is_rh_internal_user(user) if settings.AUTHZ_BACKEND_TYPE == "dummy": if settings.AUTHZ_DUMMY_RH_ORG_ADMINS == "*": @@ -125,7 +134,7 @@ def redhat_organization(backend, user, response, *args, **kwargs): return { "external_username": user.external_username, "organization_id": user.organization.id, - "rh_employee": user.rh_employee, + "rh_internal": user.rh_internal, "rh_user_is_org_admin": user.rh_user_is_org_admin, } diff --git a/ansible_ai_connect/users/templates/users/home.html b/ansible_ai_connect/users/templates/users/home.html index 8f246baaf..9b7407977 100644 --- a/ansible_ai_connect/users/templates/users/home.html +++ b/ansible_ai_connect/users/templates/users/home.html @@ -129,7 +129,7 @@

{{ project_name }}

{% if deployment_mode == 'saas' and user.is_authenticated and user.rh_user_is_org_admin %} Admin Portal {% endif %} - {% if chatbot_enabled and deployment_mode == 'saas' and user.is_authenticated and rh_employee_or_test_user %} + {% if chatbot_enabled and deployment_mode == 'saas' and user.is_authenticated and rh_internal_user_or_test_user %} Chatbot {% endif %} diff --git a/ansible_ai_connect/users/templates/users/trial.html b/ansible_ai_connect/users/templates/users/trial.html index d6f0fc08d..cc8f2686f 100644 --- a/ansible_ai_connect/users/templates/users/trial.html +++ b/ansible_ai_connect/users/templates/users/trial.html @@ -143,7 +143,7 @@

{{ project_name }}

{% if deployment_mode == 'saas' and user.is_authenticated and user.rh_user_is_org_admin %} Admin Portal {% endif %} - {% if chatbot_enabled and deployment_mode == 'saas' and user.is_authenticated and rh_employee_or_test_user %} + {% if chatbot_enabled and deployment_mode == 'saas' and user.is_authenticated and rh_internal_user_or_test_user %} Chatbot {% endif %} diff --git a/ansible_ai_connect/users/tests/test_pipeline.py b/ansible_ai_connect/users/tests/test_pipeline.py index 79a7c189e..800f4536e 100644 --- a/ansible_ai_connect/users/tests/test_pipeline.py +++ b/ansible_ai_connect/users/tests/test_pipeline.py @@ -25,7 +25,11 @@ from ansible_ai_connect.test_utils import WisdomServiceLogAwareTestCase from ansible_ai_connect.users.constants import RHSSO_LIGHTSPEED_SCOPE -from ansible_ai_connect.users.pipeline import load_extra_data, redhat_organization +from ansible_ai_connect.users.pipeline import ( + is_rh_email_domain, + load_extra_data, + redhat_organization, +) def build_access_token(private_key, payload): @@ -174,7 +178,7 @@ def test_redhat_organization_with_rh_admin_user(self): ) assert answer == { "organization_id": 345, - "rh_employee": False, + "rh_internal": False, "rh_user_is_org_admin": True, "external_username": "jean-michel", } @@ -201,7 +205,7 @@ def test_redhat_organization_with_rh_user(self): ) assert answer == { "organization_id": 345, - "rh_employee": False, + "rh_internal": False, "rh_user_is_org_admin": False, "external_username": "yves", } @@ -239,7 +243,7 @@ def test_redhat_organization_with_AUTHZ_DUMMY_parameter(self): ) assert answer == { "organization_id": 345, - "rh_employee": False, + "rh_internal": False, "rh_user_is_org_admin": True, "external_username": "yves", } @@ -267,7 +271,7 @@ def test_redhat_organization_with_AUTHZ_DUMMY_wildcard(self): ) assert answer == { "organization_id": 345, - "rh_employee": False, + "rh_internal": False, "rh_user_is_org_admin": True, "external_username": "yves", } @@ -297,7 +301,7 @@ def test_redhat_organization_with_invalid_AUTHZ_DUMMY_parameter(self): assert answer == { "organization_id": 345, - "rh_employee": False, + "rh_internal": False, "rh_user_is_org_admin": False, "external_username": "yves", } @@ -305,14 +309,15 @@ def test_redhat_organization_with_invalid_AUTHZ_DUMMY_parameter(self): assert self.rh_user.rh_user_is_org_admin is False assert self.rh_user.external_username == "yves" - def test_rh_employee_field(self): + def test_rh_internal_user_field(self): response = { "access_token": build_access_token( self.rsa_private_key, { - "realm_access": {"roles": ["redhat:employees"]}, "preferred_username": "jean-michel", "organization": {"id": "345"}, + "email": "jean-michel@redhat.com", + "email_verified": True, }, ) } @@ -322,7 +327,60 @@ def test_rh_employee_field(self): user=self.rh_user, response=response, ) - self.assertEqual(answer["rh_employee"], True) + self.assertEqual(answer["rh_internal"], True) + + def test_not_rh_internal_user_fields(self): + response = { + "access_token": build_access_token( + self.rsa_private_key, + { + "preferred_username": "jean-michel", + "organization": {"id": "345"}, + }, + ) + } + answer = redhat_organization( + backend=DummyRHBackend(public_key=self.jwk_public_key), + user=self.rh_user, + response=response, + ) + self.assertEqual(answer["rh_internal"], False) + + def test_rh_internal_user_field_not_rh_email(self): + response = { + "access_token": build_access_token( + self.rsa_private_key, + { + "email": "jean-michel@company.com", + "preferred_username": "jean-michel", + "organization": {"id": "345"}, + }, + ) + } + answer = redhat_organization( + backend=DummyRHBackend(public_key=self.jwk_public_key), + user=self.rh_user, + response=response, + ) + self.assertEqual(answer["rh_internal"], False) + + def test_rh_internal_user_field_not_email_verified(self): + response = { + "access_token": build_access_token( + self.rsa_private_key, + { + "email": "jean-michel@redhat.com", + "preferred_username": "jean-michel", + "organization": {"id": "345"}, + }, + ) + } + answer = redhat_organization( + backend=DummyRHBackend(public_key=self.jwk_public_key), + user=self.rh_user, + response=response, + ) + self.assertEqual(answer["rh_internal"], False) def test_rhoss_user_and_email(self): response = { @@ -348,3 +406,12 @@ def test_rhoss_user_and_email(self): self.assertEqual(self.rh_user.email, "francis.drake@example.foo") self.assertEqual(self.rh_user.given_name, "Francis") self.assertEqual(self.rh_user.name, "Francis Drake") + + def test_is_rh_email_domain(self): + self.assertFalse(is_rh_email_domain(None)) + self.assertFalse(is_rh_email_domain("")) + self.assertFalse(is_rh_email_domain("user.com")) + self.assertFalse(is_rh_email_domain("user@domain.com")) + self.assertTrue(is_rh_email_domain("user@redhat.com")) + self.assertTrue(is_rh_email_domain("user@REDHAT.com")) + self.assertTrue(is_rh_email_domain("@REDHAT.com")) diff --git a/ansible_ai_connect/users/views.py b/ansible_ai_connect/users/views.py index f42607a4a..bf0d41fa4 100644 --- a/ansible_ai_connect/users/views.py +++ b/ansible_ai_connect/users/views.py @@ -88,8 +88,8 @@ def get_context_data(self, **kwargs): context["documentation_url"] = settings.COMMERCIAL_DOCUMENTATION_URL user = self.request.user - context["rh_employee_or_test_user"] = user.is_authenticated and ( - user.rh_employee or user.groups.filter(name="test").exists() + context["rh_internal_user_or_test_user"] = user.is_authenticated and ( + user.rh_internal or user.groups.filter(name="test").exists() ) context["chatbot_enabled"] = (