diff --git a/integrated_channels/xapi/admin/__init__.py b/integrated_channels/xapi/admin/__init__.py index d3c3d551fa..6ba204fefd 100644 --- a/integrated_channels/xapi/admin/__init__.py +++ b/integrated_channels/xapi/admin/__init__.py @@ -19,6 +19,10 @@ class XAPILRSConfigurationAdmin(admin.ModelAdmin): 'version', 'key', 'secret', + 'auth_method', + 'auth_url', + 'oauth_scope', + 'plain_course_id_in_statements' ) list_display = ( diff --git a/integrated_channels/xapi/constants.py b/integrated_channels/xapi/constants.py index 9e49e81dfc..d63d43caa6 100644 --- a/integrated_channels/xapi/constants.py +++ b/integrated_channels/xapi/constants.py @@ -12,3 +12,7 @@ # Constants for edX grades MIN_SCORE = 0 MAX_SCORE = 100 + +# Constants for xAPI Object ID types +OBJECT_ID_URI = 'uri' +OBJECT_ID_PLAIN = 'plain' diff --git a/integrated_channels/xapi/migrations/0008_adds_auth_method_scope_and_url.py b/integrated_channels/xapi/migrations/0008_adds_auth_method_scope_and_url.py new file mode 100644 index 0000000000..a13becf0dc --- /dev/null +++ b/integrated_channels/xapi/migrations/0008_adds_auth_method_scope_and_url.py @@ -0,0 +1,28 @@ +# Generated by Django 3.2.12 on 2023-09-01 07:26 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('xapi', '0007_auto_20220405_2311'), + ] + + operations = [ + migrations.AddField( + model_name='xapilrsconfiguration', + name='auth_method', + field=models.CharField(choices=[('BASIC', 'HTTP Basic'), ('OAUTH2_CC', 'OAuth 2.0 Client Credentials')], default='BASIC', help_text='The Authentication Method to use when sending the xAPI data to the endpoint.', max_length=16, verbose_name='xAPI POST Authentication Method'), + ), + migrations.AddField( + model_name='xapilrsconfiguration', + name='auth_url', + field=models.URLField(blank=True, help_text='URL to use for authentication. Eg., Token URL for OAuth', null=True), + ), + migrations.AddField( + model_name='xapilrsconfiguration', + name='oauth_scope', + field=models.CharField(blank=True, help_text='The "scope" to pass for OAuth authentication.', max_length=255, null=True, verbose_name='OAuth scope'), + ), + ] diff --git a/integrated_channels/xapi/migrations/0009_adds_plain_course_id_in_statements_flag_to_xapi_config.py b/integrated_channels/xapi/migrations/0009_adds_plain_course_id_in_statements_flag_to_xapi_config.py new file mode 100644 index 0000000000..16fbc01677 --- /dev/null +++ b/integrated_channels/xapi/migrations/0009_adds_plain_course_id_in_statements_flag_to_xapi_config.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.12 on 2023-09-13 05:33 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('xapi', '0008_adds_auth_method_scope_and_url'), + ] + + operations = [ + migrations.AddField( + model_name='xapilrsconfiguration', + name='plain_course_id_in_statements', + field=models.BooleanField(default=False, help_text='Uses the plain course ID (eg., course-v1:X+Y+Z) instead of the URI format in xAPI statements.', verbose_name='Use plain Course ID in xAPI Statements'), + ), + ] diff --git a/integrated_channels/xapi/models.py b/integrated_channels/xapi/models.py index 7275d88b2f..193f97075c 100644 --- a/integrated_channels/xapi/models.py +++ b/integrated_channels/xapi/models.py @@ -3,8 +3,12 @@ """ import base64 +from functools import cached_property + +import requests from django.contrib import auth +from django.core.exceptions import ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ @@ -16,6 +20,11 @@ User = auth.get_user_model() +class XAPIAuthMethods(models.TextChoices): + HTTP_BASIC = 'BASIC', _('HTTP Basic') + OAUTH2_CLIENT_CREDS = 'OAUTH2_CC', _('OAuth 2.0 Client Credentials') + + class XAPILRSConfiguration(TimeStampedModel): """ xAPI LRS configurations. @@ -39,6 +48,32 @@ class XAPILRSConfiguration(TimeStampedModel): null=False, help_text=_('Is this configuration active?'), ) + auth_method = models.CharField( + max_length=16, + verbose_name="xAPI POST Authentication Method", + choices=XAPIAuthMethods.choices, + default=XAPIAuthMethods.HTTP_BASIC, + help_text=_('The Authentication Method to use when sending the xAPI data to the endpoint.') + ) + auth_url = models.URLField( + blank=True, + null=True, + help_text=_("URL to use for authentication. Eg., Token URL for OAuth") + ) + oauth_scope = models.CharField( + max_length=255, + verbose_name=_('OAuth scope'), + blank=True, + null=True, + help_text=_('The "scope" to pass for OAuth authentication.') + ) + plain_course_id_in_statements = models.BooleanField( + default=False, + blank=False, + null=False, + verbose_name=_("Use plain Course ID in xAPI Statements"), + help_text=_('Uses the plain course ID (eg., course-v1:X+Y+Z) instead of the URI format in xAPI statements.') + ) class Meta: app_label = 'xapi' @@ -62,10 +97,43 @@ def authorization_header(self): """ Authorization header for authenticating requests to LRS. """ + if self.auth_method == XAPIAuthMethods.OAUTH2_CLIENT_CREDS: + return f'Bearer {self.access_token}' + return 'Basic {}'.format( base64.b64encode('{key}:{secret}'.format(key=self.key, secret=self.secret).encode()).decode() ) + def clean(self): + errors = {} + # Don't allow OAuth2 Client Credentials method to be set without auth_url, or oauth_scope + if self.auth_method == XAPIAuthMethods.OAUTH2_CLIENT_CREDS: + if not self.auth_url: + errors['auth_url'] = _("Authentication URL is required for the OAuth2 authentication method.") + if not self.oauth_scope: + errors['oauth_scope'] = _("OAuth scope is required for the OAuth2 authentication method.") + + if errors: + raise ValidationError(errors) + + @cached_property + def access_token(self): + """ + Gets the access token from the OAuth2 Authentication endpoint return it. + """ + if self.auth_method != XAPIAuthMethods.OAUTH2_CLIENT_CREDS: + raise RuntimeError("Access Token can be fetched only for OAuth2 Client Credentials authenication method.") + + data = { + "grant_type": "client_credentials", + "scope": self.oauth_scope, + "client_id": self.key, + "client_secret": self.secret + } + response = requests.post(self.auth_url, data=data) + response.raise_for_status() + return response.json()["access_token"] + class XAPILearnerDataTransmissionAudit(LearnerDataTransmissionAudit): """ diff --git a/integrated_channels/xapi/statements/base.py b/integrated_channels/xapi/statements/base.py index f2cee0b81e..61d88f8d5f 100644 --- a/integrated_channels/xapi/statements/base.py +++ b/integrated_channels/xapi/statements/base.py @@ -4,7 +4,7 @@ from tincan import Activity, ActivityDefinition, Agent, LanguageMap, Statement -from integrated_channels.xapi.constants import X_API_ACTIVITY_COURSE +from integrated_channels.xapi.constants import OBJECT_ID_PLAIN, X_API_ACTIVITY_COURSE class EnterpriseStatement(Statement): @@ -38,12 +38,13 @@ def get_actor(self, user, user_social_auth): mbox='mailto:{email}'.format(email=user.email), ) - def get_object(self, domain, course_overview, object_type): + def get_object(self, domain, course_overview, object_type, object_id_type): """ Returns the object (activity) component of the Enterprise xAPI statement. Arguments: course_overview (CourseOverview): CourseOverview. object_type (string): Object type for activity. + object_id_type (string): The format to use for the Object ID. OBJECT_ID_URI or OBJECT_ID_PLAIN """ name = (course_overview.display_name or '').encode("ascii", "ignore").decode('ascii') @@ -53,11 +54,14 @@ def get_object(self, domain, course_overview, object_type): if object_type is not None and object_type == 'course': activity_id = course_overview.course_key - xapi_activity_id = 'https://{domain}/xapi/activities/{object_type}/{activity_id}'.format( - domain=domain, - object_type=object_type, - activity_id=activity_id - ) + if object_id_type == OBJECT_ID_PLAIN: + xapi_activity_id = course_overview.id + else: + xapi_activity_id = 'https://{domain}/xapi/activities/{object_type}/{activity_id}'.format( + domain=domain, + object_type=object_type, + activity_id=activity_id + ) xapi_object_extensions = {} diff --git a/integrated_channels/xapi/statements/learner_course_completion.py b/integrated_channels/xapi/statements/learner_course_completion.py index 804d6902b1..a4457312b0 100644 --- a/integrated_channels/xapi/statements/learner_course_completion.py +++ b/integrated_channels/xapi/statements/learner_course_completion.py @@ -13,7 +13,18 @@ class LearnerCourseCompletionStatement(EnterpriseStatement): xAPI Statement to serialize data related to course completion. """ - def __init__(self, site, user, user_social_auth, course_overview, course_grade, object_type, *args, **kwargs): + def __init__( + self, + site, + user, + user_social_auth, + course_overview, + course_grade, + object_type, + object_id_type, + *args, + **kwargs + ): """ Initialize and populate statement with learner info and course info. @@ -22,11 +33,12 @@ def __init__(self, site, user, user_social_auth, course_overview, course_grade, user_social_auth (UserSocialAuth): UserSocialAuth object for learner course_overview (CourseOverview): course overview object containing course details. course_grade (CourseGrade): User grade in the course. + object_id_type (string): Either OBJECT_ID_PLAIN or OBJECT_ID_URI. """ kwargs.update( actor=self.get_actor(user, user_social_auth), verb=self.get_verb(), - object=self.get_object(site.domain, course_overview, object_type), + object=self.get_object(site.domain, course_overview, object_type, object_id_type), result=self.get_result(course_grade), ) super().__init__(*args, **kwargs) diff --git a/integrated_channels/xapi/statements/learner_course_enrollment.py b/integrated_channels/xapi/statements/learner_course_enrollment.py index 7e3254df43..6bf2d36a4b 100644 --- a/integrated_channels/xapi/statements/learner_course_enrollment.py +++ b/integrated_channels/xapi/statements/learner_course_enrollment.py @@ -13,7 +13,7 @@ class LearnerCourseEnrollmentStatement(EnterpriseStatement): xAPI statement to serialize data related to course registration. """ - def __init__(self, site, user, user_social_auth, course_overview, object_type, *args, **kwargs): + def __init__(self, site, user, user_social_auth, course_overview, object_type, object_id_type, *args, **kwargs): """ Initialize and populate statement with learner info and course info. @@ -22,11 +22,12 @@ def __init__(self, site, user, user_social_auth, course_overview, object_type, * user_social_auth (UserSocialAuth): UserSocialAuth object of learner for the enterprise if learner has a linked third party auth account course_overview (CourseOverview): course overview object containing course details. + object_id_type (string): Either OBJECT_ID_PLAIN or OBJECT_ID_URI. """ kwargs.update( actor=self.get_actor(user, user_social_auth), verb=self.get_verb(), - object=self.get_object(site.domain, course_overview, object_type) + object=self.get_object(site.domain, course_overview, object_type, object_id_type) ) super().__init__(*args, **kwargs) diff --git a/integrated_channels/xapi/utils.py b/integrated_channels/xapi/utils.py index d2af0ad551..8fbfe1821d 100644 --- a/integrated_channels/xapi/utils.py +++ b/integrated_channels/xapi/utils.py @@ -7,6 +7,7 @@ from enterprise.tpa_pipeline import get_user_social_auth from integrated_channels.exceptions import ClientError from integrated_channels.xapi.client import EnterpriseXAPIClient +from integrated_channels.xapi.constants import OBJECT_ID_PLAIN, OBJECT_ID_URI from integrated_channels.xapi.statements.learner_course_completion import LearnerCourseCompletionStatement from integrated_channels.xapi.statements.learner_course_enrollment import LearnerCourseEnrollmentStatement @@ -85,6 +86,7 @@ def send_course_enrollment_statement(lrs_configuration, user, course_overview, o user_social_auth, course_overview, object_type, + OBJECT_ID_PLAIN if lrs_configuration.plain_course_id_in_statements else OBJECT_ID_URI ) response_fields = _send_statement( @@ -124,6 +126,7 @@ def send_course_completion_statement(lrs_configuration, course_overview, course_grade, object_type, + OBJECT_ID_PLAIN if lrs_configuration.plain_course_id_in_statements else OBJECT_ID_URI ) response_fields = _send_statement( @@ -147,6 +150,6 @@ def is_success_response(response_fields): Returns: Boolean """ success_response = False - if response_fields['status'] == 200: + if 200 <= response_fields['status'] <= 299: success_response = True return success_response diff --git a/test_utils/factories.py b/test_utils/factories.py index a883ab948b..92fa1e3876 100644 --- a/test_utils/factories.py +++ b/test_utils/factories.py @@ -60,7 +60,7 @@ SAPSuccessFactorsGlobalConfiguration, SapSuccessFactorsLearnerDataTransmissionAudit, ) -from integrated_channels.xapi.models import XAPILearnerDataTransmissionAudit, XAPILRSConfiguration +from integrated_channels.xapi.models import XAPIAuthMethods, XAPILearnerDataTransmissionAudit, XAPILRSConfiguration FAKER = FakerFactory.create() User = auth.get_user_model() @@ -786,6 +786,9 @@ class Meta: key = factory.LazyAttribute(lambda x: FAKER.slug()) secret = factory.LazyAttribute(lambda x: FAKER.uuid4()) active = True + auth_method = XAPIAuthMethods.HTTP_BASIC + auth_url = factory.LazyAttribute(lambda x: FAKER.url()) + oauth_scope = factory.LazyAttribute(lambda x: FAKER.slug()) class XAPILearnerDataTransmissionAuditFactory(factory.django.DjangoModelFactory): diff --git a/tests/test_integrated_channels/test_xapi/test_commands/test_send_course_enrollments.py b/tests/test_integrated_channels/test_xapi/test_commands/test_send_course_enrollments.py index 2e339480c5..a02be9cd62 100644 --- a/tests/test_integrated_channels/test_xapi/test_commands/test_send_course_enrollments.py +++ b/tests/test_integrated_channels/test_xapi/test_commands/test_send_course_enrollments.py @@ -287,10 +287,7 @@ def test_is_already_transmitted(self): Command.is_already_transmitted(xapi_transmissions, user_id, course_id) - @mock.patch( - 'integrated_channels.xapi.utils.is_success_response', - mock.MagicMock(return_value=False) - ) + @mock.patch(MODULE_PATH + 'is_success_response', mock.MagicMock(return_value=False)) @mock.patch(MODULE_PATH + 'send_course_enrollment_statement') def test_transmit_course_enrollments_transmit_fail_skip(self, mock_send_statement): # pylint: disable=import-outside-toplevel diff --git a/tests/test_integrated_channels/test_xapi/test_models.py b/tests/test_integrated_channels/test_xapi/test_models.py index 3496bb9b61..842ffc32df 100644 --- a/tests/test_integrated_channels/test_xapi/test_models.py +++ b/tests/test_integrated_channels/test_xapi/test_models.py @@ -3,10 +3,15 @@ """ import base64 +import json import unittest +import responses from pytest import mark +from django.core.exceptions import ValidationError + +from integrated_channels.xapi.models import XAPIAuthMethods, XAPILRSConfiguration from test_utils import factories @@ -19,6 +24,9 @@ class TestXAPILRSConfiguration(unittest.TestCase): def setUp(self): super().setUp() self.x_api_lrs_config = factories.XAPILRSConfigurationFactory() + self.x_api_oauth_lrs_config = factories.XAPILRSConfigurationFactory( + auth_method=XAPIAuthMethods.OAUTH2_CLIENT_CREDS + ) def test_string_representation(self): """ @@ -41,6 +49,52 @@ def test_authorization_header(self): ) assert expected_header == self.x_api_lrs_config.authorization_header + with responses.RequestsMock() as rsps: + rsps.add( + responses.POST, + self.x_api_oauth_lrs_config.auth_url, + body=json.dumps({ + 'access_token': 'test_token', + 'token_type': 'bearer', + 'expires_in': 3600 + }), + status=200, + content_type="application/json" + ) + expected_header = 'Bearer test_token' + + assert expected_header == self.x_api_oauth_lrs_config.authorization_header + + def test_auth_url_and_scope_are_required_oauth2_auth_method(self): + """ + Test that a validation error is raised when the auth_url or scope is not set for auth method OAUTH2_CC. + """ + conf = XAPILRSConfiguration( + enterprise_customer=factories.EnterpriseCustomerFactory(), + endpoint="https://xapi.endpoint", + key="key", + secret="secret", + active=True, + auth_method="OAUTH2_CC", + ) + with self.assertRaises(ValidationError) as context: + conf.full_clean() + self.assertIn('auth_url', context.exception.message) + self.assertIn('oauth_scope', context.exception.message) + + conf.auth_url = "https://auth.url" + + with self.assertRaises(ValidationError) as context: + conf.full_clean() + self.assertIn('oauth_scope', context.exception.message) + + conf.auth_url = None + conf.oauth_scope = "xapi:write" + + with self.assertRaises(ValidationError) as context: + conf.full_clean() + self.assertIn('auth_url', context.exception.message) + @mark.django_db class TestXAPILearnerDataTransmissionAudit(unittest.TestCase): diff --git a/tests/test_integrated_channels/test_xapi/test_statements/test_course_completion.py b/tests/test_integrated_channels/test_xapi/test_statements/test_course_completion.py index f643e41706..6034fd2a1c 100644 --- a/tests/test_integrated_channels/test_xapi/test_statements/test_course_completion.py +++ b/tests/test_integrated_channels/test_xapi/test_statements/test_course_completion.py @@ -9,7 +9,12 @@ from faker import Factory as FakerFactory from pytest import mark -from integrated_channels.xapi.constants import X_API_ACTIVITY_COURSE, X_API_VERB_COMPLETED +from integrated_channels.xapi.constants import ( + OBJECT_ID_PLAIN, + OBJECT_ID_URI, + X_API_ACTIVITY_COURSE, + X_API_VERB_COMPLETED, +) from integrated_channels.xapi.statements.learner_course_completion import LearnerCourseCompletionStatement from test_utils import factories @@ -41,6 +46,7 @@ def setUp(self): self.object_id_course = 'https://{domain}/xapi/activities/course/{activity_id}'.format( domain=self.site.domain, activity_id=self.course_overview.course_key) + self.plain_course_id = self.course_overview.id self.object_id_courserun = 'https://{domain}/xapi/activities/courserun/{activity_id}'.format( domain=self.site.domain, @@ -150,7 +156,8 @@ def test_statement_course(self): self.mock_social_auth, self.course_overview, self.course_grade, - 'course' + 'course', + OBJECT_ID_URI, ) self.assertDictEqual(json.loads(statement.to_json()), self.expected_course) @@ -164,7 +171,8 @@ def test_statement_courserun(self): self.mock_social_auth, self.course_overview, self.course_grade, - 'courserun' + 'courserun', + OBJECT_ID_URI, ) self.assertDictEqual(json.loads(statement.to_json()), self.expected_courserun) @@ -178,6 +186,35 @@ def test_statement_notpassed(self): self.mock_social_auth, self.course_overview, self.course_grade_notpassed, - 'course' + 'course', + OBJECT_ID_URI, ) self.assertDictEqual(json.loads(statement.to_json()), self.expected_notpassed) + + def test_statement_with_different_course_id_formats(self): + """ + Validate statement contains Object IDs in the specified format. + """ + statement = LearnerCourseCompletionStatement( + self.site, + self.user, + self.mock_social_auth, + self.course_overview, + self.course_grade, + 'course', + OBJECT_ID_URI, + ) + + self.assertEqual(json.loads(statement.to_json())["object"]["id"], self.object_id_course) + + statement = LearnerCourseCompletionStatement( + self.site, + self.user, + self.mock_social_auth, + self.course_overview, + self.course_grade, + 'course', + OBJECT_ID_PLAIN, + ) + + self.assertEqual(json.loads(statement.to_json())["object"]["id"], self.plain_course_id) diff --git a/tests/test_integrated_channels/test_xapi/test_statements/test_learner_course_enrollment.py b/tests/test_integrated_channels/test_xapi/test_statements/test_learner_course_enrollment.py index ff50654ff0..608abc1056 100644 --- a/tests/test_integrated_channels/test_xapi/test_statements/test_learner_course_enrollment.py +++ b/tests/test_integrated_channels/test_xapi/test_statements/test_learner_course_enrollment.py @@ -9,7 +9,12 @@ from faker import Factory as FakerFactory from pytest import mark -from integrated_channels.xapi.constants import X_API_ACTIVITY_COURSE, X_API_VERB_REGISTERED +from integrated_channels.xapi.constants import ( + OBJECT_ID_PLAIN, + OBJECT_ID_URI, + X_API_ACTIVITY_COURSE, + X_API_VERB_REGISTERED, +) from integrated_channels.xapi.statements.learner_course_enrollment import LearnerCourseEnrollmentStatement from test_utils import factories @@ -39,6 +44,7 @@ def setUp(self): self.object_id_course = 'https://{domain}/xapi/activities/course/{activity_id}'.format( domain=self.site.domain, activity_id=self.course_overview.course_key) + self.plain_course_id = self.course_overview.id self.object_id_courserun = 'https://{domain}/xapi/activities/courserun/{activity_id}'.format( domain=self.site.domain, @@ -123,6 +129,7 @@ def test_statement_course(self): self.mock_social_auth, self.course_overview, 'course', + OBJECT_ID_URI, ) self.assertDictEqual(json.loads(statement.to_json()), self.expected_course) @@ -136,5 +143,32 @@ def test_statement_courserun(self): self.mock_social_auth, self.course_overview, 'courserun', + OBJECT_ID_URI, ) self.assertDictEqual(json.loads(statement.to_json()), self.expected_courserun) + + def test_statement_with_different_course_id_formats(self): + """ + Validate statement contains Object IDs in the specified format. + """ + statement = LearnerCourseEnrollmentStatement( + self.site, + self.user, + self.mock_social_auth, + self.course_overview, + 'course', + OBJECT_ID_URI, + ) + + self.assertEqual(json.loads(statement.to_json())["object"]["id"], self.object_id_course) + + statement = LearnerCourseEnrollmentStatement( + self.site, + self.user, + self.mock_social_auth, + self.course_overview, + 'course', + OBJECT_ID_PLAIN, + ) + + self.assertEqual(json.loads(statement.to_json())["object"]["id"], self.plain_course_id) diff --git a/tests/test_integrated_channels/test_xapi/test_utils.py b/tests/test_integrated_channels/test_xapi/test_utils.py index d25c25c6b4..36f82c5814 100644 --- a/tests/test_integrated_channels/test_xapi/test_utils.py +++ b/tests/test_integrated_channels/test_xapi/test_utils.py @@ -151,6 +151,12 @@ def test_is_success_response(self): response_fields = {'status': 200, 'error_message': None} self.assertTrue(is_success_response(response_fields)) + response_fields = {'status': 201, 'error_message': None} + self.assertTrue(is_success_response(response_fields)) + + response_fields = {'status': 202, 'error_message': None} + self.assertTrue(is_success_response(response_fields)) + response_fields = {'status': 400, 'error_message': None} self.assertFalse(is_success_response(response_fields))