Skip to content

Commit

Permalink
feat: add ability to retire users on edX (#1785)
Browse files Browse the repository at this point in the history
* feat: add ability to retire users on edX

* fix tests

* fix tests

* update app-json

* added checks for client id and secret

* tests: add tests

* requirements: updated edx-api-client version

* update dependency

* update dependency
  • Loading branch information
asadali145 authored Aug 17, 2023
1 parent d5961fe commit 3ca0128
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 20 deletions.
8 changes: 8 additions & 0 deletions app.json
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,14 @@
"description": "The 'name' value for the Open edX OAuth Application",
"required": true
},
"OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID": {
"description": "OAuth2 client id for retirement service worker",
"required": false
},
"OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET": {
"description": "OAuth2 client secret for retirement service worker",
"required": false
},
"OPENEDX_SERVICE_WORKER_API_TOKEN": {
"description": "Active access token with staff level permissions to use with OpenEdX API client for service tasks",
"required": false
Expand Down
12 changes: 12 additions & 0 deletions main/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,18 @@
default=None,
description="Username of the user whose token has been set in OPENEDX_SERVICE_WORKER_API_TOKEN",
)

OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID = get_string(
name="OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID",
default=None,
description="OAuth2 client id for retirement service worker",
)
OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET = get_string(
name="OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET",
default=None,
description="OAuth2 client secret for retirement service worker",
)

EDX_API_CLIENT_TIMEOUT = get_int(
name="EDX_API_CLIENT_TIMEOUT",
default=60,
Expand Down
50 changes: 50 additions & 0 deletions openedx/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,40 @@ def get_edx_api_service_client():
return edx_client


def get_edx_retirement_service_client():
"""
Generates a JWT access token for the retirement service worker and returns the edX api client.
"""
if not settings.OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID:
raise ImproperlyConfigured(
"OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID is not set"
)
elif not settings.OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET:
raise ImproperlyConfigured(
"OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET is not set"
)

data = {
"grant_type": "client_credentials",
"client_id": settings.OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID,
"client_secret": settings.OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET,
"token_type": "jwt",
}
resp = requests.post(edx_url(OPENEDX_OAUTH2_ACCESS_TOKEN_PATH), data=data)
resp.raise_for_status()
access_token = resp.json()["access_token"]

edx_client = EdxApi(
{
"access_token": access_token,
},
settings.OPENEDX_API_BASE_URL,
timeout=settings.EDX_API_CLIENT_TIMEOUT,
)

return edx_client


def get_edx_api_course_detail_client():
"""
Gets an edx api client instance for use with the grades api
Expand Down Expand Up @@ -879,3 +913,19 @@ def validate_username_with_edx(username):
raise EdxApiRegistrationValidationException(username, resp)
result = resp.json()
return result["validation_decisions"]["username"]


def bulk_retire_edx_users(usernames):
"""
Bulk retires edX users.
This method calls edX bulk retirement API to initiate the user retirement on edX.
Users will be moved to the pending retirement state and then retirement will be carried out by our
tubular concourse pipeline.
Args:
usernames (str): Comma separated usernames
"""
edx_client = get_edx_retirement_service_client()
response = edx_client.bulk_user_retirement.retire_users({"usernames": usernames})
return response
42 changes: 42 additions & 0 deletions openedx/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@
OPENEDX_AUTH_DEFAULT_TTL_IN_SECONDS,
OPENEDX_REGISTRATION_VALIDATION_PATH,
OPENEDX_UPDATE_USER_PATH,
bulk_retire_edx_users,
create_edx_auth_token,
create_edx_user,
create_user,
enroll_in_edx_course_runs,
existing_edx_enrollment,
get_edx_api_client,
get_edx_retirement_service_client,
get_valid_edx_api_auth,
repair_faulty_edx_user,
repair_faulty_openedx_users,
Expand Down Expand Up @@ -421,6 +423,26 @@ def test_get_edx_api_client(mocker, settings, user):
)


def test_get_edx_retirement_service_client(mocker, settings):
"""Tests that get_edx_retirement_service_client returns an EdxApi client"""

settings.OPENEDX_API_BASE_URL = "http://example.com"
settings.OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID = (
"OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_ID"
)
settings.OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET = (
"OPENEDX_RETIREMENT_SERVICE_WORKER_CLIENT_SECRET"
)
mock_resp = mocker.Mock()
mock_resp.json.return_value = {"access_token": "an_access_token"}
mock_resp.json.status_code = 200
mock_resp.raise_for_status.side_effect = None
mocker.patch("openedx.api.requests.post", return_value=mock_resp)
client = get_edx_retirement_service_client()
assert client.credentials["access_token"] == "an_access_token"
assert client.base_url == settings.OPENEDX_API_BASE_URL


def test_enroll_in_edx_course_runs(settings, mocker, user):
"""Tests that enroll_in_edx_course_runs uses the EdxApi client to enroll in course runs"""
settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "mock_api_token"
Expand Down Expand Up @@ -890,3 +912,23 @@ def test_existing_edx_enrollment(
)
is None
)


def test_bulk_retire_edx_users(mocker):
"""Tests that bulk_retire_edx_users calls the edX bulk retirement api via the edx api client"""
test_usernames = "test_username1,test_username2"
mock_client = mocker.MagicMock()
mock_get_edx_retirement_service_client = mocker.patch(
"openedx.api.get_edx_retirement_service_client", return_value=mock_client
)
mock_client.bulk_user_retirement.retire_users = mocker.Mock(
return_value={
"successful_user_retirements": ["test_username1", "test_username2"]
}
)
resp = bulk_retire_edx_users(test_usernames)
assert resp["successful_user_retirements"] == ["test_username1", "test_username2"]
mock_get_edx_retirement_service_client.assert_called()
mock_client.bulk_user_retirement.retire_users.assert_called_with(
{"usernames": test_usernames}
)
8 changes: 4 additions & 4 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ drf-extensions = "^0.7.1"
django-reversion = "^4.0.1"
django-storages = "^1.11.1"
djoser = "^2.1.0"
edx-api-client = "^1.7.0"
edx-api-client = "^1.8.0"
hubspot-api-client = "^6.1.0"
ipython = "^7.31.1"
mitol-django-common = "2.7.0"
Expand Down
22 changes: 21 additions & 1 deletion users/management/commands/retire_users.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Retire user(s) from MITx Online
Retire user(s) from MITx Online and user initiate retirement on edX
"""
import hashlib
import sys
Expand All @@ -13,6 +13,7 @@

from authentication.utils import block_user_email
from main import settings
from openedx.api import bulk_retire_edx_users
from users.api import fetch_users
from users.models import BlockList

Expand Down Expand Up @@ -106,6 +107,25 @@ def handle(self, *args, **kwargs):
)
continue

resp = bulk_retire_edx_users(user.username)
if user.username not in resp["successful_user_retirements"]:
self.stderr.write(
self.style.ERROR(
"Could not initiate retirement request on edX for user {user}".format(
user=user
)
)
)
continue
else:
self.stdout.write(
self.style.SUCCESS(
"Retirement request initiated on edX for User: '{user}'".format(
user=user
)
)
)

user.is_active = False

# Change user password & email
Expand Down
45 changes: 39 additions & 6 deletions users/management/tests/retire_users_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@


@pytest.mark.django_db
def test_single_success():
def test_single_success(mocker):
"""test retire_users command success with one user"""
test_username = "test_user"

mock_bulk_retire_edx_users = mocker.patch(
"users.management.commands.retire_users.bulk_retire_edx_users",
return_value={"successful_user_retirements": [test_username]},
)

user = UserFactory.create(username=test_username, is_active=True)
UserSocialAuthFactory.create(user=user, provider="edX")

Expand All @@ -32,13 +37,19 @@ def test_single_success():
assert user.is_active is False
assert "retired_email" in user.email
assert UserSocialAuth.objects.filter(user=user).count() == 0
mock_bulk_retire_edx_users.assert_called()


@pytest.mark.django_db
def test_multiple_success():
def test_multiple_success(mocker):
"""test retire_users command success with more than one user"""
test_usernames = ["foo", "bar", "baz"]

mock_bulk_retire_edx_users = mocker.patch(
"users.management.commands.retire_users.bulk_retire_edx_users",
return_value={"successful_user_retirements": test_usernames},
)

for username in test_usernames:
user = UserFactory.create(username=username, is_active=True)
UserSocialAuthFactory.create(user=user, provider="not_edx")
Expand All @@ -54,16 +65,22 @@ def test_multiple_success():
assert user.is_active is False
assert "retired_email" in user.email
assert UserSocialAuth.objects.filter(user=user).count() == 0
mock_bulk_retire_edx_users.assert_called()


@pytest.mark.django_db
def test_retire_user_with_email():
def test_retire_user_with_email(mocker):
"""test retire_users command success with user email"""
test_email = "[email protected]"

user = UserFactory.create(email=test_email, is_active=True)
UserSocialAuthFactory.create(user=user, provider="edX")

mock_bulk_retire_edx_users = mocker.patch(
"users.management.commands.retire_users.bulk_retire_edx_users",
return_value={"successful_user_retirements": [user.username]},
)

assert user.is_active is True
assert "retired_email" not in user.email
assert UserSocialAuth.objects.filter(user=user).count() == 1
Expand All @@ -74,10 +91,11 @@ def test_retire_user_with_email():
assert user.is_active is False
assert "retired_email" in user.email
assert UserSocialAuth.objects.filter(user=user).count() == 0
mock_bulk_retire_edx_users.assert_called_with(user.username)


@pytest.mark.django_db
def test_retire_user_blocking_with_email():
def test_retire_user_blocking_with_email(mocker):
"""test retire_users command success with user email"""
test_email = "[email protected]"

Expand All @@ -90,6 +108,10 @@ def test_retire_user_blocking_with_email():
assert UserSocialAuth.objects.filter(user=user).count() == 1
assert BlockList.objects.all().count() == 0

mock_bulk_retire_edx_users = mocker.patch(
"users.management.commands.retire_users.bulk_retire_edx_users",
return_value={"successful_user_retirements": [user.username]},
)
COMMAND.handle("retire_users", users=[test_email], block_users=True)

user.refresh_from_db()
Expand All @@ -98,12 +120,17 @@ def test_retire_user_blocking_with_email():
assert UserSocialAuth.objects.filter(user=user).count() == 0
assert BlockList.objects.all().count() == 1
assert BlockList.objects.filter(hashed_email=hashed_email).count() == 1
mock_bulk_retire_edx_users.assert_called_with(user.username)


@pytest.mark.django_db
def test_multiple_success_blocking_user():
def test_multiple_success_blocking_user(mocker):
"""test retire_users command blocking emails success with more than one user"""
test_usernames = ["foo", "bar", "baz"]
mock_bulk_retire_edx_users = mocker.patch(
"users.management.commands.retire_users.bulk_retire_edx_users",
return_value={"successful_user_retirements": test_usernames},
)

for username in test_usernames:
user = UserFactory.create(username=username, is_active=True)
Expand All @@ -123,10 +150,11 @@ def test_multiple_success_blocking_user():
assert UserSocialAuth.objects.filter(user=user).count() == 0

assert BlockList.objects.all().count() == 3
mock_bulk_retire_edx_users.assert_called()


@pytest.mark.django_db
def test_user_blocking_if_not_requested():
def test_user_blocking_if_not_requested(mocker):
"""test retire_users command success but it should not block user(s) if not requested"""
test_email = "[email protected]"

Expand All @@ -139,10 +167,15 @@ def test_user_blocking_if_not_requested():
assert UserSocialAuth.objects.filter(user=user).count() == 1
assert BlockList.objects.all().count() == 0

mock_bulk_retire_edx_users = mocker.patch(
"users.management.commands.retire_users.bulk_retire_edx_users",
return_value={"successful_user_retirements": [user.username]},
)
COMMAND.handle("retire_users", users=[test_email])

user.refresh_from_db()
assert user.is_active is False
assert "retired_email" in user.email
assert UserSocialAuth.objects.filter(user=user).count() == 0
assert BlockList.objects.all().count() == 0
mock_bulk_retire_edx_users.assert_called_with(user.username)
Loading

0 comments on commit 3ca0128

Please sign in to comment.