Skip to content

Commit f8a3016

Browse files
committed
feat: implement course authoring migration functionality
1 parent a18df56 commit f8a3016

9 files changed

Lines changed: 623 additions & 30 deletions

File tree

openedx_authz/admin.py

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,24 @@
11
"""Admin configuration for openedx_authz."""
22

3+
import json
4+
35
from casbin_adapter.models import CasbinRule
46
from django import forms
57
from django.contrib import admin
8+
from django.utils.html import format_html
9+
10+
from openedx_authz.models import AuthzCourseAuthoringMigrationRun, ExtendedCasbinRule
611

7-
from openedx_authz.models import ExtendedCasbinRule
12+
13+
def pretty_json(value) -> str:
14+
"""Return an indented JSON representation of a value."""
15+
if value is None:
16+
return "-"
17+
try:
18+
formatted = json.dumps(value, indent=2, ensure_ascii=False)
19+
except (TypeError, ValueError):
20+
return str(value)
21+
return format_html("<pre>{}</pre>", formatted)
822

923

1024
class CasbinRuleForm(forms.ModelForm):
@@ -48,3 +62,28 @@ class CasbinRuleAdmin(admin.ModelAdmin):
4862
# TODO: In a future, possibly we should only show an inline for the rules that
4963
# have an extended rule, and show the subject and scope information in detail.
5064
inlines = [ExtendedCasbinRuleInline]
65+
66+
67+
@admin.register(AuthzCourseAuthoringMigrationRun)
68+
class AuthzCourseAuthoringMigrationRunAdmin(admin.ModelAdmin):
69+
"""Admin for AuthzCourseAuthoringMigrationRun to display additional metadata."""
70+
71+
list_display = ("id", "scope_type", "scope_key", "migration_type", "status", "created_at", "updated_at")
72+
search_fields = ("scope_type", "scope_key", "migration_type", "status")
73+
list_filter = ("scope_type", "migration_type", "status")
74+
readonly_fields = (
75+
"scope_type",
76+
"scope_key",
77+
"migration_type",
78+
"status",
79+
"pretty_metadata",
80+
"completed_at",
81+
"created_at",
82+
"updated_at",
83+
)
84+
fields = readonly_fields
85+
86+
@admin.display(description="Metadata")
87+
def pretty_metadata(self, obj):
88+
"""Return formatted JSON for the metadata field."""
89+
return pretty_json(obj.metadata)

openedx_authz/engine/utils.py

Lines changed: 199 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
import logging
88
from collections import defaultdict
9+
from dataclasses import dataclass, field
910

1011
from casbin import Enforcer
12+
from django.db import IntegrityError, transaction
1113
from django.db.models import Q
1214
from opaque_keys.edx.django.models import CourseKeyField
1315

@@ -24,6 +26,7 @@
2426
LIBRARY_AUTHOR,
2527
LIBRARY_USER,
2628
)
29+
from openedx_authz.models.migrations import AuthzCourseAuthoringMigrationRun, MigrationType, ScopeType
2730

2831
logger = logging.getLogger(__name__)
2932

@@ -34,6 +37,46 @@
3437
COURSE_ROLE_EQUIVALENCES = {v: k for k, v in LEGACY_COURSE_ROLE_EQUIVALENCES.items()}
3538

3639

40+
class MigrationErrorReason:
41+
"""String constants for categorising why a single role assignment failed during migration."""
42+
43+
# Forward (legacy → authz) reasons
44+
UNKNOWN_ROLE = "unknown_role"
45+
NO_SCOPE = "no_scope"
46+
ASSIGNMENT_FAILED = "assignment_failed"
47+
48+
# Rollback (authz → legacy) reasons
49+
UNEXPECTED_SCOPE_TYPE = "unexpected_scope_type"
50+
NO_LEGACY_EQUIVALENT = "no_legacy_equivalent"
51+
UNEXPECTED_ERROR = "unexpected_error"
52+
53+
54+
@dataclass
55+
class MigrationMetadata:
56+
"""Normalised representation of a single role-assignment outcome during migration.
57+
58+
Can represent both successful and failed assignments. Populate ``reason`` /
59+
``details`` only for failures; leave them empty for successes.
60+
61+
Attributes:
62+
subject: External key of the user whose assignment was attempted.
63+
role: Role external key (new-style for rollback, legacy key for forward).
64+
scope: Scope external key, or empty string when not yet determined.
65+
reason: One of the ``MigrationErrorReason`` constants; empty for successes.
66+
details: Optional human-readable extra context (e.g. exception message).
67+
"""
68+
69+
subject: str
70+
role: str
71+
scope: str = field(default="")
72+
reason: str = field(default="")
73+
details: str = field(default="")
74+
75+
def to_dict(self) -> dict:
76+
"""Convert the migration metadata to a dictionary."""
77+
return {k: v for k, v in self.__dict__.items() if v}
78+
79+
3780
def migrate_policy_between_enforcers(
3881
source_enforcer: Enforcer,
3982
target_enforcer: Enforcer,
@@ -187,7 +230,9 @@ def _validate_migration_input(course_id_list, org_id):
187230
)
188231

189232

190-
def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_list, org_id, delete_after_migration):
233+
def migrate_legacy_course_roles_to_authz(
234+
course_access_role_model, course_id_list, org_id, delete_after_migration
235+
) -> tuple[list[MigrationMetadata], list[MigrationMetadata]]:
191236
"""
192237
Migrate legacy course role data to the new Casbin-based authorization model.
193238
This function reads legacy permissions from the CourseAccessRole model
@@ -235,19 +280,22 @@ def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_lis
235280
.select_related("user")
236281
)
237282

238-
# List to keep track of any permissions that could not be migrated
239-
permissions_with_errors = []
240-
permissions_with_no_errors = []
283+
permissions_with_errors: list[MigrationMetadata] = []
284+
permissions_with_no_errors: list[MigrationMetadata] = []
285+
permission_ids: list[int] = []
241286

242287
for permission in legacy_permissions:
243288
# Migrate the permission to the new model
289+
migration_metadata = MigrationMetadata(subject=permission.user.username, role=permission.role)
244290

245291
role = LEGACY_COURSE_ROLE_EQUIVALENCES.get(permission.role)
246292
if role is None:
247293
# This should not happen as there are no more access_levels defined
248294
# in CourseAccessRole, log and skip
249295
logger.error(f"Unknown access level: {permission.role} for User: {permission.user}")
250-
permissions_with_errors.append(permission)
296+
migration_metadata.reason = MigrationErrorReason.UNKNOWN_ROLE
297+
migration_metadata.details = f"Unknown access level: {permission.role} for User: {permission.user.username}"
298+
permissions_with_errors.append(migration_metadata)
251299
continue
252300

253301
if permission.course_id:
@@ -259,7 +307,9 @@ def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_lis
259307
logger.error(
260308
f"Permission for User: {permission.user.username} has neither course_id nor org defined, skipping."
261309
)
262-
permissions_with_errors.append(permission)
310+
migration_metadata.reason = MigrationErrorReason.NO_SCOPE
311+
migration_metadata.details = f"User '{permission.user.username}' has neither course_id nor org defined"
312+
permissions_with_errors.append(migration_metadata)
263313
continue
264314

265315
# Permission applied to individual user
@@ -279,23 +329,30 @@ def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_lis
279329
f"to Role: {role} in Scope: {permission.course_id} "
280330
"user may already have this permission assigned"
281331
)
282-
permissions_with_errors.append(permission)
332+
migration_metadata.scope = scope_external_key
333+
migration_metadata.reason = MigrationErrorReason.ASSIGNMENT_FAILED
334+
migration_metadata.details = f"User '{permission.user.username}' may already have this permission assigned"
335+
permissions_with_errors.append(migration_metadata)
283336
continue
284337

285-
permissions_with_no_errors.append(permission)
338+
migration_metadata.scope = scope_external_key
339+
permissions_with_no_errors.append(migration_metadata)
340+
341+
permission_ids.append(permission.id)
286342

287343
if delete_after_migration:
288344
# Only delete permissions that were successfully migrated to avoid data loss.
289-
course_access_role_model.objects.filter(id__in=[p.id for p in permissions_with_no_errors]).delete()
345+
course_access_role_model.objects.filter(id__in=permission_ids).delete()
290346
logger.info(f"Deleted {len(permissions_with_no_errors)} legacy permissions after successful migration.")
291347
logger.info(f"Retained {len(permissions_with_errors)} legacy permissions that had errors during migration.")
292348

293349
return permissions_with_errors, permissions_with_no_errors
294350

295351

352+
# pylint: disable=too-many-statements
296353
def migrate_authz_to_legacy_course_roles(
297354
course_access_role_model, user_subject_model, course_id_list, org_id, delete_after_migration
298-
):
355+
) -> tuple[list[MigrationMetadata], list[MigrationMetadata]]:
299356
"""
300357
Migrate permissions from the new Casbin-based authorization model back to the legacy CourseAccessRole model.
301358
This function reads permissions from the Casbin enforcer and creates equivalent entries in the
@@ -321,10 +378,7 @@ def migrate_authz_to_legacy_course_roles(
321378
_validate_migration_input(course_id_list, org_id)
322379

323380
role_assignments = get_all_role_assignments_per_scope_type(
324-
scope_types=(
325-
CourseOverviewData,
326-
OrgCourseOverviewGlobData,
327-
)
381+
scope_types=(CourseOverviewData, OrgCourseOverviewGlobData)
328382
)
329383

330384
# Two cases here:
@@ -343,8 +397,8 @@ def migrate_authz_to_legacy_course_roles(
343397
and role_assignment.scope.course_id in course_id_list
344398
]
345399

346-
roles_with_errors = []
347-
roles_with_no_errors = []
400+
roles_with_errors: list[MigrationMetadata] = []
401+
roles_with_no_errors: list[MigrationMetadata] = []
348402
unassignments = defaultdict(list)
349403

350404
user_external_keys = {assignment.subject.external_key for assignment in role_assignments}
@@ -361,9 +415,24 @@ def migrate_authz_to_legacy_course_roles(
361415
role_external_key = role_assignment.roles[0].external_key
362416
scope_external_key = role_assignment.scope.external_key
363417

418+
migration_metadata = MigrationMetadata(
419+
subject=user_external_key, role=role_external_key, scope=scope_external_key
420+
)
421+
422+
legacy_role = COURSE_ROLE_EQUIVALENCES.get(role_external_key)
423+
if legacy_role is None:
424+
logger.error(
425+
f"No legacy equivalent found for role: {role_external_key}, "
426+
f"user: {user_external_key}, scope: {scope_external_key}. Skipping."
427+
)
428+
migration_metadata.reason = MigrationErrorReason.NO_LEGACY_EQUIVALENT
429+
migration_metadata.details = f"Role '{role_external_key}' has no legacy equivalent."
430+
roles_with_errors.append(migration_metadata)
431+
continue
432+
364433
course_access_role_kwargs = {
365434
"user": users_by_username[user_external_key],
366-
"role": COURSE_ROLE_EQUIVALENCES[role_external_key],
435+
"role": legacy_role,
367436
}
368437

369438
if isinstance(role_assignment.scope, CourseOverviewData):
@@ -378,11 +447,13 @@ def migrate_authz_to_legacy_course_roles(
378447
f"Unexpected scope type: {type(role_assignment.scope)} for RoleAssignment with "
379448
f"scope: {scope_external_key}, user: {user_external_key} and role: {role_external_key}, skipping."
380449
)
381-
roles_with_errors.append(role_assignment)
450+
migration_metadata.reason = MigrationErrorReason.UNEXPECTED_SCOPE_TYPE
451+
migration_metadata.details = f"Unexpected scope type: {type(role_assignment.scope).__name__}"
452+
roles_with_errors.append(migration_metadata)
382453
continue
383454

384455
course_access_role_model.objects.get_or_create(**course_access_role_kwargs)
385-
roles_with_no_errors.append(role_assignment)
456+
roles_with_no_errors.append(migration_metadata)
386457

387458
logger.info(
388459
f"Successfully rolled back RoleAssignment for User: {user_external_key} "
@@ -398,7 +469,9 @@ def migrate_authz_to_legacy_course_roles(
398469
f"Error rolling back RoleAssignment for User: {role_assignment.subject.external_key} "
399470
f"in Role: {role_assignment.roles[0].external_key} and Scope: {role_assignment.scope.external_key}: {e}"
400471
)
401-
roles_with_errors.append(role_assignment)
472+
migration_metadata.reason = MigrationErrorReason.UNEXPECTED_ERROR
473+
migration_metadata.details = str(e)
474+
roles_with_errors.append(migration_metadata)
402475

403476
# Once the loop is done, we can log summary of unassignments
404477
# and perform batch unassignment if delete_after_migration is True
@@ -407,7 +480,7 @@ def migrate_authz_to_legacy_course_roles(
407480
logger.info(f"Total of {total_unassignments} role assignments unassigned after successful rollback migration.")
408481
for (role_external_key, scope), users in unassignments.items():
409482
logger.info(
410-
f"Unassigned Role: {role_external_key} from {len(users)} users \n"
483+
f"Unassigned Role: {role_external_key} from {len(users)} users "
411484
f"in Scope: {scope} after successful rollback migration."
412485
)
413486
batch_unassign_role_from_users(
@@ -417,3 +490,108 @@ def migrate_authz_to_legacy_course_roles(
417490
)
418491

419492
return roles_with_errors, roles_with_no_errors
493+
494+
495+
# pylint: disable=too-many-positional-arguments
496+
def run_course_authoring_migration(
497+
migration_type: MigrationType,
498+
scope_type: ScopeType,
499+
scope_key: str,
500+
course_access_role_model,
501+
user_subject_model=None,
502+
course_id_list=None,
503+
org_id=None,
504+
delete_after_migration=True,
505+
) -> None:
506+
"""
507+
Orchestrate a course authoring role migration with concurrency protection and lifecycle tracking.
508+
509+
Wraps either :func:`migrate_legacy_course_roles_to_authz` (``FORWARD``) or
510+
:func:`migrate_authz_to_legacy_course_roles` (``ROLLBACK``) with three guarantees:
511+
512+
1. Concurrency guard: an ``AuthzCourseAuthoringMigrationRun`` record is
513+
created atomically before work begins. If an active run already exists for the
514+
same ``(scope_type, scope_key)``, the call is skipped immediately to prevent
515+
duplicate parallel runs.
516+
2. Lifecycle tracking: the run record is updated to ``COMPLETED``,
517+
``PARTIAL_SUCCESS``, or ``FAILED`` regardless of the outcome, with per-role
518+
error details persisted in the record's ``metadata`` field.
519+
3. Transactional safety: data-migration work runs inside an inner
520+
``atomic()`` block so that an unexpected exception rolls back all data changes
521+
while the tracking record (updated outside that block) is always persisted.
522+
523+
Args:
524+
migration_type (MigrationType): Direction of the migration (``FORWARD`` or ``ROLLBACK``).
525+
scope_type (ScopeType): Granularity key dimension for the tracking record (e.g. course or org).
526+
scope_key (str): Concrete identifier — a course-v1 key or an org name.
527+
course_access_role_model: The ``CourseAccessRole`` model class.
528+
user_subject_model: The ``UserSubject`` model class; required for ``ROLLBACK``, ignored otherwise.
529+
course_id_list (list[str] | None): Restrict migration to these course-v1 keys.
530+
org_id (str | None): Restrict migration to this org; takes precedence over ``course_id_list``.
531+
delete_after_migration (bool): Remove successfully migrated entries from the source system.
532+
"""
533+
try:
534+
with transaction.atomic():
535+
run = AuthzCourseAuthoringMigrationRun.create_running(migration_type, scope_type, scope_key)
536+
except IntegrityError:
537+
logger.warning(
538+
"Skipping %s migration for %s:%s — an active run already exists.", migration_type, scope_type, scope_key
539+
)
540+
AuthzCourseAuthoringMigrationRun.create_skipped(migration_type, scope_type, scope_key)
541+
return
542+
543+
logger.info("Started %s migration run [%s] for %s:%s", migration_type, run.id, scope_type, scope_key)
544+
545+
try:
546+
with transaction.atomic():
547+
if migration_type == MigrationType.FORWARD:
548+
errors, successes = migrate_legacy_course_roles_to_authz(
549+
course_access_role_model, course_id_list, org_id, delete_after_migration
550+
)
551+
else:
552+
errors, successes = migrate_authz_to_legacy_course_roles(
553+
course_access_role_model, user_subject_model, course_id_list, org_id, delete_after_migration
554+
)
555+
except Exception as exc: # pylint: disable=broad-exception-caught
556+
# The inner atomic block is rolled back on exception; mark_failed() runs
557+
# outside it so the tracking record is always persisted.
558+
logger.exception(
559+
"Unexpected error in migration run [%s] for %s:%s", run.id, scope_type, scope_key, exc_info=exc
560+
)
561+
run.mark_failed(exception=exc)
562+
return
563+
564+
errors_by_reason: dict = defaultdict(list)
565+
for entry in errors:
566+
entry_dict = entry.to_dict()
567+
errors_by_reason[entry_dict.pop("reason")].append(entry_dict)
568+
569+
metadata_updates = {
570+
"total": len(successes) + len(errors),
571+
"success_count": len(successes),
572+
"error_count": len(errors),
573+
"successes": [entry.to_dict() for entry in successes],
574+
"errors": dict(errors_by_reason),
575+
}
576+
577+
if errors:
578+
run.mark_partial_success(metadata_updates=metadata_updates)
579+
logger.warning(
580+
"Partial success in %s migration run [%s] for %s:%s — successes=%d, errors=%d",
581+
migration_type,
582+
run.id,
583+
scope_type,
584+
scope_key,
585+
len(successes),
586+
len(errors),
587+
)
588+
else:
589+
run.mark_completed(metadata_updates=metadata_updates)
590+
logger.info(
591+
"Completed %s migration run [%s] for %s:%s — successes=%d",
592+
migration_type,
593+
run.id,
594+
scope_type,
595+
scope_key,
596+
len(successes),
597+
)

0 commit comments

Comments
 (0)