Skip to content

Commit a234c6e

Browse files
mariajgrimaldiMaferMazu
authored andcommitted
refactor: do not inherit from BaseRolesTestCase to favor CL setup methods
If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior.
1 parent 96ecd3f commit a234c6e

1 file changed

Lines changed: 134 additions & 129 deletions

File tree

openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py

Lines changed: 134 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
from openedx.core.djangolib.testing.utils import skip_unless_cms
4343

4444
from ..models import ContentLibrary
45+
from openedx_authz.engine.enforcer import AuthzEnforcer
4546

4647

4748
@skip_unless_cms
@@ -1252,7 +1253,7 @@ def test_xblock_handler_invalid_key(self):
12521253

12531254

12541255
@skip_unless_cms
1255-
class ContentLibrariesRestAPIAuthzIntegrationTestCase(BaseRolesTestCase, ContentLibrariesRestApiTest):
1256+
class ContentLibrariesRestAPIAuthzIntegrationTestCase(ContentLibrariesRestApiTest):
12561257
"""
12571258
Test that Content Libraries REST API endpoints respect AuthZ roles and permissions.
12581259
@@ -1262,8 +1263,11 @@ class ContentLibrariesRestAPIAuthzIntegrationTestCase(BaseRolesTestCase, Content
12621263
3. Library Contributor: Can view and edit library content, but cannot delete or publish the library.
12631264
4. Library User: Can only view library content.
12641265
"""
1266+
12651267
def setUp(self):
12661268
super().setUp()
1269+
self._seed_database_with_policies()
1270+
12671271
self.library_admin = UserFactory.create(
12681272
username="library_admin",
12691273
@@ -1279,12 +1283,29 @@ def setUp(self):
12791283
self.random_user = UserFactory.create(
12801284
username="random_user",
12811285
1286+
1287+
# Define user groups by permission level
1288+
self.list_of_all_users = [
1289+
self.library_admin,
1290+
self.library_author,
1291+
self.library_contributor,
1292+
self.library_user,
1293+
self.random_user,
1294+
]
1295+
self.library_viewers = [self.library_admin, self.library_author, self.library_contributor, self.library_user]
1296+
self.library_editors = [self.library_admin, self.library_author, self.library_contributor]
1297+
self.library_publishers = [self.library_admin, self.library_author]
1298+
self.library_collection_editors = [self.library_admin, self.library_author, self.library_contributor]
1299+
self.library_deleters = [self.library_admin]
1300+
1301+
# Create library and assign roles
12821302
library = self._create_library(
12831303
slug="authzlib",
12841304
title="AuthZ Test Library",
12851305
description="Testing AuthZ",
12861306
)
12871307
self.lib_id = library["id"]
1308+
12881309
authz_api.assign_role_to_user_in_scope(
12891310
self.library_admin.username,
12901311
roles.LIBRARY_ADMIN.external_key, self.lib_id)
@@ -1297,68 +1318,69 @@ def setUp(self):
12971318
authz_api.assign_role_to_user_in_scope(
12981319
self.library_user.username,
12991320
roles.LIBRARY_USER.external_key, self.lib_id)
1321+
AuthzEnforcer.get_enforcer().load_policy() # Load policies to simulate fresh start
13001322

1301-
# Define user groups by permission level
1302-
self.list_of_all_users = [
1303-
self.library_admin,
1304-
self.library_author,
1305-
self.library_contributor,
1306-
self.library_user,
1307-
self.random_user,
1308-
]
1309-
self.library_viewers = [self.library_admin, self.library_author, self.library_contributor, self.library_user]
1310-
self.library_editors = [self.library_admin, self.library_author, self.library_contributor]
1311-
self.library_publishers = [self.library_admin, self.library_author]
1312-
self.library_collection_editors = [self.library_admin, self.library_author, self.library_contributor]
1313-
self.library_deleters = [self.library_admin]
1323+
def tearDown(self):
1324+
"""Clean up after each test to ensure isolation."""
1325+
super().tearDown()
1326+
AuthzEnforcer.get_enforcer().clear_policy() # Clear policies after each test to ensure isolation
13141327

1315-
def _all_users_excluding(self, excluded_users):
1316-
return set(self.list_of_all_users) - set(excluded_users)
1328+
@classmethod
1329+
def _seed_database_with_policies(cls):
1330+
"""Seed the database with policies from the policy file.
13171331
1318-
def test_library_permissions(self):
1319-
"""
1320-
Verify that the rest api endpoints takes into account the openedx-authz roles and permissions.
1332+
This simulates the one-time database seeding that would happen
1333+
during application deployment, separate from the runtime policy loading.
13211334
"""
1322-
self._test_view_permissions()
1323-
self._test_edit_permissions()
1324-
self._test_publish_permissions()
1325-
self._test_collection_permissions()
1326-
self._test_delete_library_permissions()
1335+
import pkg_resources
1336+
from openedx_authz.engine.utils import migrate_policy_between_enforcers
1337+
import casbin
1338+
1339+
global_enforcer = AuthzEnforcer.get_enforcer()
1340+
global_enforcer.load_policy()
1341+
model_path = pkg_resources.resource_filename("openedx_authz.engine", "config/model.conf")
1342+
policy_path = pkg_resources.resource_filename("openedx_authz.engine", "config/authz.policy")
1343+
1344+
migrate_policy_between_enforcers(
1345+
source_enforcer=casbin.Enforcer(model_path, policy_path),
1346+
target_enforcer=global_enforcer,
1347+
)
1348+
global_enforcer.clear_policy() # Clear to simulate fresh start for each test
1349+
1350+
def _all_users_excluding(self, excluded_users):
1351+
return set(self.list_of_all_users) - set(excluded_users)
13271352

1328-
def _test_view_permissions(self):
1353+
def test_view_permissions(self):
13291354
"""
13301355
Verify that only users with view permissions can view.
13311356
"""
13321357
# Test library view access
13331358
for user in self.library_viewers:
1334-
with transaction.atomic():
1335-
with self.as_user(user):
1336-
self._get_library(self.lib_id, expect_response=status.HTTP_200_OK)
1359+
with self.as_user(user):
1360+
self._get_library(self.lib_id, expect_response=status.HTTP_200_OK)
13371361
for user in self._all_users_excluding(self.library_viewers):
1338-
with transaction.atomic():
1339-
with self.as_user(user):
1340-
self._get_library(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN)
1362+
with self.as_user(user):
1363+
self._get_library(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN)
13411364

1342-
def _test_edit_permissions(self):
1365+
def test_edit_permissions(self):
13431366
"""
13441367
Verify that only users with edit permissions can edit.
13451368
"""
13461369
# Test library edit access
13471370
for user in self.library_editors:
1348-
with transaction.atomic():
1349-
with self.as_user(user):
1350-
self._update_library(
1351-
self.lib_id,
1352-
description=f"Description by {user.username}",
1353-
expect_response=status.HTTP_200_OK)
1354-
#Verify the permitted changes were made
1355-
data = self._get_library(self.lib_id)
1356-
assert data['description'] == f"Description by {user.username}"
1371+
with self.as_user(user):
1372+
self._update_library(
1373+
self.lib_id,
1374+
description=f"Description by {user.username}",
1375+
expect_response=status.HTTP_200_OK,
1376+
)
1377+
#Verify the permitted changes were made
1378+
data = self._get_library(self.lib_id)
1379+
assert data['description'] == f"Description by {user.username}"
13571380

13581381
for user in self._all_users_excluding(self.library_editors):
1359-
with transaction.atomic():
1360-
with self.as_user(user):
1361-
self._update_library(
1382+
with self.as_user(user):
1383+
self._update_library(
13621384
self.lib_id,
13631385
description="I can't edit this.", expect_response=status.HTTP_403_FORBIDDEN)
13641386

@@ -1369,90 +1391,79 @@ def _test_edit_permissions(self):
13691391
# Library XBlock editing
13701392
for user in self.library_editors:
13711393
with self.as_user(user):
1372-
with transaction.atomic():
1373-
# They can create blocks
1374-
block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}")
1375-
# They can modify blocks
1376-
self._set_library_block_olx(
1377-
block_data["id"],
1378-
"<problem/>",
1379-
expect_response=status.HTTP_200_OK)
1380-
self._set_library_block_fields(
1381-
block_data["id"],
1382-
{"data": "<problem />", "metadata": {}},
1383-
expect_response=status.HTTP_200_OK)
1384-
self._set_library_block_asset(
1385-
block_data["id"],
1386-
"static/test.txt",
1387-
b"data",
1388-
expect_response=status.HTTP_200_OK)
1389-
# They can remove blocks
1390-
self._delete_library_block(block_data["id"], expect_response=status.HTTP_200_OK)
1391-
# Verify deletion
1392-
self._get_library_block(block_data["id"], expect_response=404)
1394+
# They can create blocks
1395+
block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}")
1396+
# They can modify blocks
1397+
self._set_library_block_olx(
1398+
block_data["id"],
1399+
"<problem/>",
1400+
expect_response=status.HTTP_200_OK)
1401+
self._set_library_block_fields(
1402+
block_data["id"],
1403+
{"data": "<problem />", "metadata": {}},
1404+
expect_response=status.HTTP_200_OK)
1405+
self._set_library_block_asset(
1406+
block_data["id"],
1407+
"static/test.txt",
1408+
b"data",
1409+
expect_response=status.HTTP_200_OK)
1410+
# They can remove blocks
1411+
self._delete_library_block(block_data["id"], expect_response=status.HTTP_200_OK)
1412+
# Verify deletion
1413+
self._get_library_block(block_data["id"], expect_response=404)
13931414

13941415
# Recreate blocks for further tests
13951416
block_data = self._add_block_to_library(self.lib_id, "problem", "new_problem")
13961417

13971418
for user in self._all_users_excluding(self.library_editors):
13981419
with self.as_user(user):
1399-
# They can't create blocks
1400-
with transaction.atomic():
1401-
self._add_block_to_library(
1402-
self.lib_id,
1403-
"problem",
1404-
"problem1",
1405-
expect_response=status.HTTP_403_FORBIDDEN)
1420+
self._add_block_to_library(
1421+
self.lib_id,
1422+
"problem",
1423+
"problem1",
1424+
expect_response=status.HTTP_403_FORBIDDEN)
14061425
# They can't modify blocks
1407-
with transaction.atomic():
1408-
self._set_library_block_olx(
1409-
block_data["id"],
1410-
"<problem/>",
1411-
expect_response=status.HTTP_403_FORBIDDEN)
1412-
with transaction.atomic():
1413-
self._set_library_block_fields(
1414-
block_data["id"],
1415-
{"data": "<problem />", "metadata": {}},
1416-
expect_response=status.HTTP_403_FORBIDDEN)
1417-
with transaction.atomic():
1418-
self._set_library_block_asset(
1419-
block_data["id"],
1420-
"static/test.txt",
1421-
b"data",
1422-
expect_response=status.HTTP_403_FORBIDDEN)
1426+
self._set_library_block_olx(
1427+
block_data["id"],
1428+
"<problem/>",
1429+
expect_response=status.HTTP_403_FORBIDDEN)
1430+
self._set_library_block_fields(
1431+
block_data["id"],
1432+
{"data": "<problem />", "metadata": {}},
1433+
expect_response=status.HTTP_403_FORBIDDEN)
1434+
self._set_library_block_asset(
1435+
block_data["id"],
1436+
"static/test.txt",
1437+
b"data",
1438+
expect_response=status.HTTP_403_FORBIDDEN)
14231439
# They can't remove blocks
1424-
with transaction.atomic():
1425-
self._delete_library_block(block_data["id"], expect_response=status.HTTP_403_FORBIDDEN)
1440+
self._delete_library_block(block_data["id"], expect_response=status.HTTP_403_FORBIDDEN)
14261441

1427-
def _test_publish_permissions(self):
1442+
def test_publish_permissions(self):
14281443
"""
14291444
Verify that only users with publish permissions can publish.
14301445
"""
14311446
# Test publish access
14321447
for user in self.library_publishers:
14331448
with self.as_user(user):
1434-
with transaction.atomic():
1435-
block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}_1")
1436-
self._publish_library_block(block_data["id"], expect_response=status.HTTP_200_OK)
1437-
with transaction.atomic():
1438-
block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}_2")
1439-
assert self._get_library(self.lib_id)['has_unpublished_changes'] is True
1440-
self._commit_library_changes(self.lib_id, expect_response=status.HTTP_200_OK)
1441-
assert self._get_library(self.lib_id)['has_unpublished_changes'] is False
1449+
block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}_1")
1450+
self._publish_library_block(block_data["id"], expect_response=status.HTTP_200_OK)
1451+
block_data = self._add_block_to_library(self.lib_id, "problem", f"problem_{user.username}_2")
1452+
assert self._get_library(self.lib_id)['has_unpublished_changes'] is True
1453+
self._commit_library_changes(self.lib_id, expect_response=status.HTTP_200_OK)
1454+
assert self._get_library(self.lib_id)['has_unpublished_changes'] is False
14421455

14431456
block_data = self._add_block_to_library(self.lib_id, "problem", "draft_problem")
14441457
assert self._get_library(self.lib_id)['has_unpublished_changes'] is True
14451458

14461459
for user in self._all_users_excluding(self.library_publishers):
14471460
with self.as_user(user):
1448-
with transaction.atomic():
1449-
self._publish_library_block(block_data["id"], expect_response=status.HTTP_403_FORBIDDEN)
1450-
with transaction.atomic():
1451-
self._commit_library_changes(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN)
1461+
self._publish_library_block(block_data["id"], expect_response=status.HTTP_403_FORBIDDEN)
1462+
self._commit_library_changes(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN)
14521463
# Verify that no changes were published
14531464
assert self._get_library(self.lib_id)['has_unpublished_changes'] is True
14541465

1455-
def _test_collection_permissions(self):
1466+
def test_collection_permissions(self):
14561467
"""
14571468
Verify that only users with collection permissions can perform collection actions.
14581469
"""
@@ -1464,20 +1475,18 @@ def _test_collection_permissions(self):
14641475
# Create collection
14651476
collection_data = self._create_collection(
14661477
self.lib_id,
1467-
title="Temp Collection",
1478+
title=f"Temp Collection {user.username}",
14681479
expect_response=status.HTTP_200_OK)
14691480
collection_id = collection_data["key"]
14701481
collection_key = LibraryCollectionLocator(lib_key=library_key, collection_id=collection_id)
14711482
# Update collection
14721483
self._update_collection(collection_key, title="Updated Collection", expect_response=status.HTTP_200_OK)
1473-
with transaction.atomic():
1474-
self._add_items_to_collection(
1475-
collection_key,
1476-
item_keys=[block_data["id"]],
1477-
expect_response=status.HTTP_200_OK)
1484+
self._add_items_to_collection(
1485+
collection_key,
1486+
item_keys=[block_data["id"]],
1487+
expect_response=status.HTTP_200_OK)
14781488
# Delete collection
1479-
with transaction.atomic():
1480-
self._soft_delete_collection(collection_key, expect_response=status.HTTP_204_NO_CONTENT)
1489+
self._soft_delete_collection(collection_key, expect_response=status.HTTP_204_NO_CONTENT)
14811490

14821491
collection_data = self._create_collection(
14831492
self.lib_id,
@@ -1498,29 +1507,25 @@ def _test_collection_permissions(self):
14981507
collection_key,
14991508
title="Unauthorized Change",
15001509
expect_response=status.HTTP_403_FORBIDDEN)
1501-
with transaction.atomic():
1502-
self._add_items_to_collection(
1503-
collection_key,
1504-
item_keys=[block_data["id"]],
1505-
expect_response=status.HTTP_403_FORBIDDEN)
1510+
self._add_items_to_collection(
1511+
collection_key,
1512+
item_keys=[block_data["id"]],
1513+
expect_response=status.HTTP_403_FORBIDDEN)
15061514
# Attempt to delete collection
1507-
with transaction.atomic():
1508-
self._soft_delete_collection(collection_key, expect_response=status.HTTP_403_FORBIDDEN)
1515+
self._soft_delete_collection(collection_key, expect_response=status.HTTP_403_FORBIDDEN)
15091516

1510-
def _test_delete_library_permissions(self):
1517+
def test_delete_library_permissions(self):
15111518
"""
15121519
Verify that only users with delete permissions can delete a library.
15131520
"""
15141521
# Test library delete access
15151522
for user in self._all_users_excluding(self.library_deleters):
1516-
with transaction.atomic():
1517-
with self.as_user(user):
1518-
result = self._delete_library(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN)
1519-
assert 'detail' in result # Error message
1520-
assert 'permission' in result['detail'].lower()
1523+
with self.as_user(user):
1524+
result = self._delete_library(self.lib_id, expect_response=status.HTTP_403_FORBIDDEN)
1525+
assert 'detail' in result # Error message
1526+
assert 'permission' in result['detail'].lower()
15211527

15221528
for user in self.library_deleters:
1523-
with transaction.atomic():
1524-
with self.as_user(user):
1525-
result = self._delete_library(self.lib_id, expect_response=status.HTTP_200_OK)
1526-
assert result == {}
1529+
with self.as_user(user):
1530+
result = self._delete_library(self.lib_id, expect_response=status.HTTP_200_OK)
1531+
assert result == {}

0 commit comments

Comments
 (0)