diff --git a/.github/workflows/test-functional-microshift.yaml b/.github/workflows/test-functional-microshift.yaml index 110e91a..1f0af2a 100644 --- a/.github/workflows/test-functional-microshift.yaml +++ b/.github/workflows/test-functional-microshift.yaml @@ -35,6 +35,10 @@ jobs: run: | bash ./ci/setup-oc-client.sh + - name: Install Keycloak + run: | + bash ./ci/setup-keycloak.sh + - name: Install Microshift run: | ./ci/microshift.sh diff --git a/.github/workflows/test-functional-microstack.yaml b/.github/workflows/test-functional-microstack.yaml index 594fdf9..1ad6034 100644 --- a/.github/workflows/test-functional-microstack.yaml +++ b/.github/workflows/test-functional-microstack.yaml @@ -18,6 +18,10 @@ jobs: with: python-version: 3.12 + - name: Install Keycloak + run: | + bash ./ci/setup-keycloak.sh + - name: Install ColdFront and plugin run: | python -m pip install --upgrade pip diff --git a/ci/run_functional_tests_openshift.sh b/ci/run_functional_tests_openshift.sh index 2aa9125..bef9219 100755 --- a/ci/run_functional_tests_openshift.sh +++ b/ci/run_functional_tests_openshift.sh @@ -5,6 +5,12 @@ # Tests expect the resource to be name Devstack set -xe +export KEYCLOAK_BASE_URL="http://localhost:8080" +export KEYCLOAK_REALM="master" +export KEYCLOAK_CLIENT_ID="admin-cli" +export KEYCLOAK_ADMIN_USER="admin" +export KEYCLOAK_ADMIN_PASSWORD="nomoresecret" + export OPENSHIFT_MICROSHIFT_TOKEN="$(oc create token -n onboarding onboarding-serviceaccount)" export OPENSHIFT_MICROSHIFT_VERIFY="false" diff --git a/ci/run_functional_tests_openstack.sh b/ci/run_functional_tests_openstack.sh index b6aa1c6..a60cd1d 100755 --- a/ci/run_functional_tests_openstack.sh +++ b/ci/run_functional_tests_openstack.sh @@ -5,6 +5,12 @@ # Tests expect the resource to be name Devstack set -xe +export KEYCLOAK_BASE_URL="http://localhost:8080" +export KEYCLOAK_REALM="master" +export KEYCLOAK_CLIENT_ID="admin-cli" +export KEYCLOAK_ADMIN_USER="admin" +export KEYCLOAK_ADMIN_PASSWORD="nomoresecret" + export CREDENTIAL_NAME=$(openssl rand -base64 12) export OPENSTACK_DEVSTACK_APPLICATION_CREDENTIAL_SECRET=$( diff --git a/ci/setup-keycloak.sh b/ci/setup-keycloak.sh new file mode 100644 index 0000000..c1524f5 --- /dev/null +++ b/ci/setup-keycloak.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +set -xe + +sudo docker run -d --name keycloak \ + -e KEYCLOAK_ADMIN=admin \ + -e KEYCLOAK_ADMIN_PASSWORD=nomoresecret \ + -p 8080:8080 \ + -p 8443:8443 \ + quay.io/keycloak/keycloak:25.0 start-dev diff --git a/requirements.txt b/requirements.txt index 181a262..a52194f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,3 +8,4 @@ python-keystoneclient python-novaclient python-neutronclient python-swiftclient +requests diff --git a/src/coldfront_plugin_cloud/base.py b/src/coldfront_plugin_cloud/base.py index 00a4807..656f539 100644 --- a/src/coldfront_plugin_cloud/base.py +++ b/src/coldfront_plugin_cloud/base.py @@ -1,11 +1,14 @@ import abc import functools from typing import NamedTuple +import logging from coldfront.core.allocation import models as allocation_models from coldfront.core.resource import models as resource_models -from coldfront_plugin_cloud import attributes +from coldfront_plugin_cloud import attributes, kc_client + +logger = logging.getLogger(__name__) class ResourceAllocator(abc.ABC): @@ -25,11 +28,30 @@ def __init__( self.resource = resource self.allocation = allocation + @functools.cached_property + def kc_admin_client(self): + return kc_client.KeyCloakAPIClient() + def get_or_create_federated_user(self, username): if not (user := self.get_federated_user(username)): user = self.create_federated_user(username) return user + def assign_role_on_user(self, username, project_id): + self.kc_admin_client.create_group(project_id) + if user_id := self.kc_admin_client.get_user_id(username): + group_id = self.kc_admin_client.get_group_id(project_id) + self.kc_admin_client.add_user_to_group(user_id, group_id) + else: + logger.warning( + f"User {username} not found in Keycloak, cannot add to group." + ) + + def remove_role_from_user(self, username, project_id): + user_id = self.kc_admin_client.get_user_id(username) + group_id = self.kc_admin_client.get_group_id(project_id) + self.kc_admin_client.remove_user_from_group(user_id, group_id) + @functools.cached_property def auth_url(self): return self.resource.get_attribute(attributes.RESOURCE_AUTH_URL).rstrip("/") @@ -69,11 +91,3 @@ def create_federated_user(self, unique_id): @abc.abstractmethod def get_federated_user(self, unique_id): pass - - @abc.abstractmethod - def assign_role_on_user(self, username, project_id): - pass - - @abc.abstractmethod - def remove_role_from_user(self, username, project_id): - pass diff --git a/src/coldfront_plugin_cloud/kc_client.py b/src/coldfront_plugin_cloud/kc_client.py new file mode 100644 index 0000000..574ad2c --- /dev/null +++ b/src/coldfront_plugin_cloud/kc_client.py @@ -0,0 +1,86 @@ +import os +import functools + +import requests + + +class KeyCloakAPIClient: + def __init__(self): + self.base_url = os.getenv("KEYCLOAK_BASE_URL") + self.realm = os.getenv("KEYCLOAK_REALM") + self.admin_user = os.getenv("KEYCLOAK_ADMIN_USER") + self.admin_password = os.getenv("KEYCLOAK_ADMIN_PASSWORD") + self.client_id = os.getenv("KEYCLOAK_CLIENT_ID", "admin-cli") + + self.token_url = ( + f"{self.base_url}/realms/{self.realm}/protocol/openid-connect/token" + ) + + @functools.cached_property + def api_client(self): + params = { + "grant_type": "password", + "client_id": self.client_id, + "username": self.admin_user, + "password": self.admin_password, + "scope": "openid", + } + r = requests.post(self.token_url, data=params).json() + headers = { + "Authorization": ("Bearer %s" % r["access_token"]), + "Content-Type": "application/json", + } + session = requests.session() + session.headers.update(headers) + return session + + def create_group(self, group_name): + url = f"{self.base_url}/admin/realms/{self.realm}/groups" + payload = {"name": group_name} + response = self.api_client.post(url, json=payload) + + # If group already exists, ignore and move on + if response.status_code not in (201, 409): + response.raise_for_status() + + def create_user(self, cf_username): + """Helper function to create user in Keycloak, for testing purposes only""" + url = f"{self.base_url}/admin/realms/{self.realm}/users" + payload = { + "username": cf_username, + "enabled": True, + "email": cf_username, + } + r = self.api_client.post(url, json=payload) + r.raise_for_status() + + def get_group_id(self, group_name) -> str | None: + """Return None if group not found""" + query = f"search={group_name}&exact=true" + url = f"{self.base_url}/admin/realms/{self.realm}/groups?{query}" + r = self.api_client.get(url).json() + return r[0]["id"] if r else None + + def get_user_id(self, cf_username) -> str | None: + """Return None if user not found""" + # TODO (Quan): Confirm that Coldfront usernames map to Keycloak emails, not email, or something else? + query = f"email={cf_username}&exact=true" + url = f"{self.base_url}/admin/realms/{self.realm}/users?{query}" + r = self.api_client.get(url).json() + return r[0]["id"] if r else None + + def add_user_to_group(self, user_id, group_id): + url = f"{self.base_url}/admin/realms/{self.realm}/users/{user_id}/groups/{group_id}" + r = self.api_client.put(url) + r.raise_for_status() + + def remove_user_from_group(self, user_id, group_id): + url = f"{self.base_url}/admin/realms/{self.realm}/users/{user_id}/groups/{group_id}" + r = self.api_client.delete(url) + r.raise_for_status() + + def get_user_groups(self, user_id) -> list[str]: + url = f"{self.base_url}/admin/realms/{self.realm}/users/{user_id}/groups" + r = self.api_client.get(url) + r.raise_for_status() + return [group["name"] for group in r.json()] diff --git a/src/coldfront_plugin_cloud/management/commands/validate_allocations.py b/src/coldfront_plugin_cloud/management/commands/validate_allocations.py index ef5ea1f..7fbeb45 100644 --- a/src/coldfront_plugin_cloud/management/commands/validate_allocations.py +++ b/src/coldfront_plugin_cloud/management/commands/validate_allocations.py @@ -50,7 +50,7 @@ def sync_users(project_id, allocation, allocator, apply): if apply: tasks.add_user_to_allocation(coldfront_user.pk) - # remove users that are in the resource but not in coldfront + # remove users that are in the resource but not in coldfront allocation users = set( [coldfront_user.user.username for coldfront_user in coldfront_users] ) @@ -58,7 +58,7 @@ def sync_users(project_id, allocation, allocator, apply): if allocation_user not in users: failed_validation = True logger.warn( - f"{allocation_user} exists in the resource {project_id} but not in coldfront" + f"{allocation_user} exists in the resource {project_id} but not in coldfront allocation" ) if apply: allocator.remove_role_from_user(allocation_user, project_id) diff --git a/src/coldfront_plugin_cloud/openshift.py b/src/coldfront_plugin_cloud/openshift.py index 91b39cd..e2780a1 100644 --- a/src/coldfront_plugin_cloud/openshift.py +++ b/src/coldfront_plugin_cloud/openshift.py @@ -253,6 +253,8 @@ def assign_role_on_user(self, username, project_id): # Role already exists, ignore pass + super().assign_role_on_user(username, project_id) + def remove_role_from_user(self, username, project_id): """Remove a role from a user in a project using direct OpenShift API calls""" try: @@ -275,6 +277,8 @@ def remove_role_from_user(self, username, project_id): # Rolebinding doesn't exist, nothing to remove pass + super().remove_role_from_user(username, project_id) + def _create_project(self, project_name, project_id): pi_username = self.allocation.project.pi.username diff --git a/src/coldfront_plugin_cloud/openstack.py b/src/coldfront_plugin_cloud/openstack.py index b47ec84..514a2c6 100644 --- a/src/coldfront_plugin_cloud/openstack.py +++ b/src/coldfront_plugin_cloud/openstack.py @@ -344,12 +344,14 @@ def assign_role_on_user(self, username, project_id): user = self.get_federated_user(username) self.identity.roles.grant(user=user["id"], project=project_id, role=role) + super().assign_role_on_user(username, project_id) def remove_role_from_user(self, username, project_id): role = self.identity.roles.find(name=self.member_role_name) if user := self.get_federated_user(username): self.identity.roles.revoke(user=user["id"], project=project_id, role=role) + super().remove_role_from_user(username, project_id) def create_default_network(self, project_id): neutron = neutronclient.Client(session=get_session_for_resource(self.resource)) diff --git a/src/coldfront_plugin_cloud/tests/base.py b/src/coldfront_plugin_cloud/tests/base.py index 03d35d6..44113ae 100644 --- a/src/coldfront_plugin_cloud/tests/base.py +++ b/src/coldfront_plugin_cloud/tests/base.py @@ -24,6 +24,8 @@ from coldfront.core.field_of_science.models import FieldOfScience from django.core.management import call_command +from coldfront_plugin_cloud import kc_client + class TestBase(TestCase): def setUp(self) -> None: @@ -37,11 +39,7 @@ def setUp(self) -> None: # For testing we can validate allocations with this status AllocationStatusChoice.objects.get_or_create(name="Active (Needs Renewal)") - @staticmethod - def new_user(username=None) -> User: - username = username or f"{uuid.uuid4().hex}@example.com" - User.objects.create(username=username, email=username) - return User.objects.get(username=username) + self.kc_admin_client = kc_client.KeyCloakAPIClient() @staticmethod def new_esi_resource(name=None, auth_url=None) -> Resource: @@ -101,6 +99,15 @@ def new_openshift_resource( ) return Resource.objects.get(name=resource_name) + def new_user(self, username=None, add_to_keycloak=True) -> User: + username = username or f"{uuid.uuid4().hex}@example.com" + User.objects.create(username=username, email=username) + + if add_to_keycloak: + self.kc_admin_client.create_user(username) + + return User.objects.get(username=username) + def new_project(self, title=None, pi=None) -> Project: title = title or uuid.uuid4().hex pi = pi or self.new_user() diff --git a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py index a417779..3c501ad 100644 --- a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py +++ b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py @@ -2,7 +2,6 @@ import time import unittest from unittest import mock -import uuid from coldfront_plugin_cloud import attributes, openshift, tasks, utils from coldfront_plugin_cloud.tests import base @@ -52,7 +51,13 @@ def test_new_allocation(self): allocator._get_role(user.username, project_id) + # Check Keycloak group and user membership + self.kc_admin_client.get_group_id(project_id) + user_id = self.kc_admin_client.get_user_id(user.username) + assert project_id in self.kc_admin_client.get_user_groups(user_id) + allocator.remove_role_from_user(user.username, project_id) + assert project_id not in self.kc_admin_client.get_user_groups(user_id) with self.assertRaises(openshift.NotFound): allocator._get_role(user.username, project_id) @@ -109,7 +114,7 @@ def test_add_remove_user(self): # directly add a user to openshift which should then be # deleted when validate_allocations is called - non_coldfront_user = uuid.uuid4().hex + non_coldfront_user = self.new_user(add_to_keycloak=True).username allocator.get_or_create_federated_user(non_coldfront_user) allocator.assign_role_on_user(non_coldfront_user, project_id) assert non_coldfront_user in allocator.get_users(project_id) diff --git a/src/coldfront_plugin_cloud/tests/functional/openstack/test_allocation.py b/src/coldfront_plugin_cloud/tests/functional/openstack/test_allocation.py index 20a24a6..6b83c7f 100644 --- a/src/coldfront_plugin_cloud/tests/functional/openstack/test_allocation.py +++ b/src/coldfront_plugin_cloud/tests/functional/openstack/test_allocation.py @@ -1,6 +1,5 @@ import os import unittest -import uuid import time from coldfront_plugin_cloud import attributes, openstack, tasks, utils @@ -57,6 +56,11 @@ def test_new_allocation(self): self.assertEqual(len(roles), 1) self.assertEqual(roles[0].role["id"], self.role_member.id) + # Check Keycloak group and user membership + self.kc_admin_client.get_group_id(project_id) + user_id = self.kc_admin_client.get_user_id(user.username) + assert project_id in self.kc_admin_client.get_user_groups(user_id) + # Check default network # Port build-up time is not instant time.sleep(5) @@ -297,6 +301,11 @@ def test_add_remove_user(self): tasks.add_user_to_allocation(allocation_user2.pk) + # Check Keycloak group and user membership + self.kc_admin_client.get_group_id(project_id) + user2_id = self.kc_admin_client.get_user_id(user2.username) + assert project_id in self.kc_admin_client.get_user_groups(user2_id) + openstack_user = allocator.get_federated_user(user2.username) openstack_user = self.identity.users.get(openstack_user["id"]) @@ -310,6 +319,8 @@ def test_add_remove_user(self): tasks.remove_user_from_allocation(allocation_user2.pk) + assert project_id not in self.kc_admin_client.get_user_groups(user2_id) + roles = self.identity.role_assignments.list( user=openstack_user.id, project=openstack_project.id ) @@ -326,7 +337,7 @@ def test_add_remove_user(self): # directly add a user to openstack which should then be # deleted when validate_allocations is called - non_coldfront_user = uuid.uuid4().hex + non_coldfront_user = self.new_user(add_to_keycloak=True).username allocator.get_or_create_federated_user(non_coldfront_user) allocator.assign_role_on_user(non_coldfront_user, project_id) assert non_coldfront_user in allocator.get_users(project_id) diff --git a/src/coldfront_plugin_cloud/tests/unit/openshift/test_rbac.py b/src/coldfront_plugin_cloud/tests/unit/openshift/test_rbac.py index b7898f7..e29c91b 100644 --- a/src/coldfront_plugin_cloud/tests/unit/openshift/test_rbac.py +++ b/src/coldfront_plugin_cloud/tests/unit/openshift/test_rbac.py @@ -13,6 +13,7 @@ def setUp(self) -> None: self.allocator = OpenShiftResourceAllocator(mock_resource, mock_allocation) self.allocator.id_provider = "fake_idp" self.allocator.k8_client = mock.Mock() + self.allocator.kc_admin_client = mock.Mock() self.allocator.member_role_name = "admin" def test_user_in_rolebindings_false(self): diff --git a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py index f704cde..eba3f32 100644 --- a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py +++ b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py @@ -22,7 +22,7 @@ def test_new_allocation_quota(self): ) with freezegun.freeze_time("2020-03-15 00:01:00"): - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) utils.set_attribute_on_allocation( @@ -92,7 +92,7 @@ def test_new_allocation_quota_expired(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) allocation.status = allocation_models.AllocationStatusChoice.objects.get( @@ -125,7 +125,7 @@ def test_new_allocation_quota_denied(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -155,7 +155,7 @@ def test_new_allocation_quota_last_revoked(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -200,7 +200,7 @@ def test_new_allocation_quota_new(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -218,7 +218,7 @@ def test_new_allocation_quota_never_approved(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -240,7 +240,7 @@ def test_change_request_decrease(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -286,7 +286,7 @@ def test_change_request_increase(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -332,7 +332,7 @@ def test_change_request_decrease_multiple(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) @@ -395,7 +395,7 @@ def test_new_allocation_quota_change_request(self): self.resource = self.new_openshift_resource( name="", ) - user = self.new_user() + user = self.new_user(add_to_keycloak=False) project = self.new_project(pi=user) allocation = self.new_allocation(project, self.resource, 2) diff --git a/src/coldfront_plugin_cloud/tests/unit/test_migrate_field_of_science.py b/src/coldfront_plugin_cloud/tests/unit/test_migrate_field_of_science.py index 8bc11f1..56f5b0e 100644 --- a/src/coldfront_plugin_cloud/tests/unit/test_migrate_field_of_science.py +++ b/src/coldfront_plugin_cloud/tests/unit/test_migrate_field_of_science.py @@ -18,9 +18,12 @@ def test_command_output(self): new_fos_1_des = uuid.uuid4().hex # Migrate to new fos new_fos_2_des = old_fos_4.description # Migrate to existing fos - fake_project_1 = self.new_project() - fake_project_2 = self.new_project() - fake_project_3 = self.new_project() + fake_user = self.new_user( + add_to_keycloak=False + ) # To avoid Keycloak dependency in unit test + fake_project_1 = self.new_project(pi=fake_user) + fake_project_2 = self.new_project(pi=fake_user) + fake_project_3 = self.new_project(pi=fake_user) fake_project_1.field_of_science = old_fos_1 fake_project_2.field_of_science = old_fos_2 fake_project_3.field_of_science = old_fos_3