Skip to content

Commit

Permalink
Merge pull request #574 from ubc/#573-xapi-limit-user-input
Browse files Browse the repository at this point in the history
Limit user input in xAPI statements
  • Loading branch information
xcompass authored Aug 30, 2017
2 parents 3eeca86 + 5bc0940 commit e6c3546
Show file tree
Hide file tree
Showing 9 changed files with 300 additions and 38 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ xAPI statements require an actor (currently logged in user) account information.

`LRS_ACTOR_ACCOUNT_CAS_IDENTIFIER`: Optionally set a param to set as the actor's unique key for the CAS account. (uses CAS username by default)

`LRS_USER_INPUT_FIELD_SIZE_LIMIT`: Set the byte limit on xAPI statement fields containing user input. Set this in order to prevent sending large statements to the LRS that it can't handle (1048576 by default or 1MB)

Restart server after making any changes to settings

(Optional) Setting up Background Tasks
Expand Down
2 changes: 1 addition & 1 deletion compair/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@

env_int_overridables = [
'SQLALCHEMY_POOL_RECYCLE',
'ATTACHMENT_UPLOAD_LIMIT',
'ATTACHMENT_UPLOAD_LIMIT', 'LRS_USER_INPUT_FIELD_SIZE_LIMIT',
'MAIL_PORT', 'MAIL_MAX_EMAILS'
]

Expand Down
2 changes: 2 additions & 0 deletions compair/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
LRS_AUTH = None
LRS_USERNAME = None
LRS_PASSWORD = None
# limit user generated content field text size limit
LRS_USER_INPUT_FIELD_SIZE_LIMIT = 1048576 #1024 * 1024 -> max 1MB

LRS_ACTOR_ACCOUNT_USE_CAS = False # set to True to use CAS account information if available
LRS_ACTOR_ACCOUNT_CAS_IDENTIFIER = None # set to a param field value to use or None to use unique_identifier
Expand Down
2 changes: 1 addition & 1 deletion compair/tasks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
from .lti_outcomes import update_lti_course_grades, update_lti_assignment_grades
from .send_mail import send_message, send_messages
from .user_password import set_passwords
from .xapi_statement import send_lrs_statements
from .xapi_statement import send_lrs_statement
5 changes: 2 additions & 3 deletions compair/tasks/xapi_statement.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@

@celery.task(bind=True, autoretry_for=(Exception,),
ignore_result=True, store_errors_even_if_ignored=True)
def send_lrs_statements(self, statements):
def send_lrs_statement(self, statement_string):
try:
statements = [json.loads(statement) for statement in statements]
XAPI._send_lrs_statements(statements)
XAPI._send_lrs_statement(json.loads(statement_string))
except socket.error as error:
# don't raise connection refused error when in eager mode
if error.errno != socket.errno.ECONNREFUSED:
Expand Down
31 changes: 17 additions & 14 deletions compair/tests/test_compair.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,25 +117,28 @@ def generate_tracking(self, with_duration=False, **kargs):

return tracking

def _validate_and_cleanup_statement(self, statement):
# check categories
categories = statement['context']['contextActivities']['category']
self.assertIn(self.compair_source_category, categories)
categories.remove(self.compair_source_category)

if len(categories) == 0:
del statement['context']['contextActivities']['category']
if len(statement['context']['contextActivities']) == 0:
del statement['context']['contextActivities']
if len(statement['context']) == 0:
del statement['context']

# check timestamp
self.assertIsNotNone(statement['timestamp'])

def get_and_clear_statement_log(self, has_request=False):
statements = []
for xapi_log in XAPILog.query.all():
statement = json.loads(xapi_log.statement)

# check categories
categories = statement['context']['contextActivities']['category']
self.assertIn(self.compair_source_category, categories)
categories.remove(self.compair_source_category)

if len(categories) == 0:
del statement['context']['contextActivities']['category']
if len(statement['context']['contextActivities']) == 0:
del statement['context']['contextActivities']
if len(statement['context']) == 0:
del statement['context']

# check timestamp
self.assertIsNotNone(statement['timestamp'])
self._validate_and_cleanup_statement(statement)

statements.append(statement)
XAPILog.query.delete()
Expand Down
232 changes: 232 additions & 0 deletions compair/tests/xapi/test_remote.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
import json
import mock
from six import text_type

from data.fixtures.test_data import SimpleAnswersTestData
from compair.tests.test_compair import ComPAIRXAPITestCase


from compair.core import db
from flask_login import current_app
from compair.xapi import XAPIStatement, XAPIVerb, XAPIObject, \
XAPIContext, XAPIResult, XAPI
from tincan import LRSResponse

class RemoteXAPITests(ComPAIRXAPITestCase):

def setUp(self):
super(ComPAIRXAPITestCase, self).setUp()
self.app.config['LRS_STATEMENT_ENDPOINT'] = 'http://example.com'
self.app.config['LRS_USERNAME'] = 'lrs_username'
self.app.config['LRS_PASSWORD'] = 'lrs_password'
self.app.config['LRS_USER_INPUT_FIELD_SIZE_LIMIT'] = 200 # 200 bytes

self.data = SimpleAnswersTestData()
self.user = self.data.authorized_student
self.course = self.data.main_course
self.assignment = self.data.assignments[0]
self.answer = self.data.create_answer(self.assignment, self.user)
self.sent_statement = None
self.character_limit = int(current_app.config.get('LRS_USER_INPUT_FIELD_SIZE_LIMIT') / len("c".encode('utf-8')))

@mock.patch('tincan.RemoteLRS.save_statement')
def test_send_remote_statement(self, mocked_save_statement):

def save_statement_override(statement):
self.sent_statement = json.loads(statement.to_json(XAPI._version))

return LRSResponse(
success=True,
request=None,
response=None,
data=json.dumps(["123"]),
)
mocked_save_statement.side_effect = save_statement_override


# test with answer normal content
statement = XAPIStatement.generate(
user=self.user,
verb=XAPIVerb.generate('submitted'),
object=XAPIObject.answer(self.answer),
context=XAPIContext.answer(self.answer),
result=XAPIResult.answer(self.answer, includeAttachment=True, success=True)
)

XAPI._send_lrs_statement(json.loads(statement.to_json(XAPI._version)))

self._validate_and_cleanup_statement(self.sent_statement)

self.assertEqual(self.sent_statement['actor'], self.get_compair_actor(self.user))
self.assertEqual(self.sent_statement['verb'], {
'id': 'http://activitystrea.ms/schema/1.0/submit',
'display': {'en-US': 'submitted'}
})
self.assertEqual(self.sent_statement['object'], {
'id': 'https://localhost:8888/app/xapi/answer/'+self.answer.uuid,
'definition': {'type': 'http://id.tincanapi.com/activitytype/solution', 'name': {'en-US': 'Assignment answer'}},
'objectType': 'Activity'
})
self.assertEqual(self.sent_statement['result'], {
'extensions': {
'http://xapi.learninganalytics.ubc.ca/extension/character-count': len(self.answer.content),
'http://xapi.learninganalytics.ubc.ca/extension/word-count': len(self.answer.content.split(" "))
},
'response': self.answer.content,
'success': True
})
self.assertEqual(self.sent_statement['context'], {
'contextActivities': {
'grouping': [{
'id': 'https://localhost:8888/app/xapi/course/'+self.course.uuid,
'objectType': 'Activity'
},{
'id': 'https://localhost:8888/app/xapi/assignment/'+self.assignment.uuid,
'objectType': 'Activity'
}],
'parent': [{
'id': 'https://localhost:8888/app/xapi/assignment/'+self.assignment.uuid+'/question',
'objectType': 'Activity'
}]
}
})

# test with extremely long answer content

# content should be ~ LRS_USER_INPUT_FIELD_SIZE_LIMIT bytes long + 100 characters
content = "c" * (self.character_limit + 100)
# expected_result_response should be <= LRS_USER_INPUT_FIELD_SIZE_LIMIT bytes long + " [TEXT TRIMMED]..."
expected_result_response = ("c" * self.character_limit) + " [TEXT TRIMMED]..."

self.answer.content = content
db.session.commit()

statement = XAPIStatement.generate(
user=self.user,
verb=XAPIVerb.generate('submitted'),
object=XAPIObject.answer(self.answer),
context=XAPIContext.answer(self.answer),
result=XAPIResult.answer(self.answer, includeAttachment=True, success=True)
)

XAPI._send_lrs_statement(json.loads(statement.to_json(XAPI._version)))

self._validate_and_cleanup_statement(self.sent_statement)

self.assertEqual(self.sent_statement['actor'], self.get_compair_actor(self.user))
self.assertEqual(self.sent_statement['verb'], {
'id': 'http://activitystrea.ms/schema/1.0/submit',
'display': {'en-US': 'submitted'}
})
self.assertEqual(self.sent_statement['object'], {
'id': 'https://localhost:8888/app/xapi/answer/'+self.answer.uuid,
'definition': {'type': 'http://id.tincanapi.com/activitytype/solution', 'name': {'en-US': 'Assignment answer'}},
'objectType': 'Activity'
})
self.assertEqual(self.sent_statement['result'], {
'extensions': {
'http://xapi.learninganalytics.ubc.ca/extension/character-count': len(self.answer.content),
'http://xapi.learninganalytics.ubc.ca/extension/word-count': len(self.answer.content.split(" "))
},
'response': expected_result_response,
'success': True
})
self.assertEqual(self.sent_statement['context'], {
'contextActivities': {
'grouping': [{
'id': 'https://localhost:8888/app/xapi/course/'+self.course.uuid,
'objectType': 'Activity'
},{
'id': 'https://localhost:8888/app/xapi/assignment/'+self.assignment.uuid,
'objectType': 'Activity'
},],
'parent': [{
'id': 'https://localhost:8888/app/xapi/assignment/'+self.assignment.uuid+'/question',
'objectType': 'Activity'
}]
}
})

# test with assignment normal content
statement = XAPIStatement.generate(
user=self.user,
verb=XAPIVerb.generate('updated'),
object=XAPIObject.assignment(self.assignment),
context=XAPIContext.assignment(self.assignment)
)

XAPI._send_lrs_statement(json.loads(statement.to_json(XAPI._version)))

self._validate_and_cleanup_statement(self.sent_statement)

self.assertEqual(self.sent_statement['actor'], self.get_compair_actor(self.user))
self.assertEqual(self.sent_statement['verb'], {
'id': 'http://activitystrea.ms/schema/1.0/update',
'display': {'en-US': 'updated'}
})
self.assertEqual(self.sent_statement['object'], {
'id': 'https://localhost:8888/app/xapi/assignment/'+self.assignment.uuid,
'definition': {
'type': 'http://adlnet.gov/expapi/activities/assessment',
'name': {'en-US': self.assignment.name },
'description': {'en-US': self.assignment.description }
},
'objectType': 'Activity'
})
self.assertNotIn('result', self.sent_statement)
self.assertEqual(self.sent_statement['context'], {
'contextActivities': {
'parent': [{
'id': 'https://localhost:8888/app/xapi/course/'+self.course.uuid,
'objectType': 'Activity'
}]
}
})

# test with extremely long answer content

# content should be ~ LRS_USER_INPUT_FIELD_SIZE_LIMIT bytes long + 100 characters
name = "a" * (self.character_limit + 100)
description = "b" * (self.character_limit + 100)
# expected_result_response should be <= LRS_USER_INPUT_FIELD_SIZE_LIMIT bytes long + " [TEXT TRIMMED]..."
expected_object_name = ("a" * self.character_limit) + " [TEXT TRIMMED]..."
expected_object_description = ("b" * self.character_limit) + " [TEXT TRIMMED]..."

self.assignment.name = name
self.assignment.description = description
db.session.commit()

statement = XAPIStatement.generate(
user=self.user,
verb=XAPIVerb.generate('updated'),
object=XAPIObject.assignment(self.assignment),
context=XAPIContext.assignment(self.assignment)
)

XAPI._send_lrs_statement(json.loads(statement.to_json(XAPI._version)))

self._validate_and_cleanup_statement(self.sent_statement)

self.assertEqual(self.sent_statement['actor'], self.get_compair_actor(self.user))
self.assertEqual(self.sent_statement['verb'], {
'id': 'http://activitystrea.ms/schema/1.0/update',
'display': {'en-US': 'updated'}
})
self.assertEqual(self.sent_statement['object'], {
'id': 'https://localhost:8888/app/xapi/assignment/'+self.assignment.uuid,
'definition': {
'type': 'http://adlnet.gov/expapi/activities/assessment',
'name': {'en-US': expected_object_name },
'description': {'en-US': expected_object_description }
},
'objectType': 'Activity'
})
self.assertNotIn('result', self.sent_statement)
self.assertEqual(self.sent_statement['context'], {
'contextActivities': {
'parent': [{
'id': 'https://localhost:8888/app/xapi/course/'+self.course.uuid,
'objectType': 'Activity'
}]
}
})
9 changes: 6 additions & 3 deletions compair/xapi/capture_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ def xapi_on_answer_comment_modified(sender, user, **extra):
)
statements.append(statement)

XAPI.send_statements(statements)
for statement in statements:
XAPI.send_statement(statement)
else:
# (public or private)
statement = XAPIStatement.generate(
Expand Down Expand Up @@ -300,7 +301,8 @@ def xapi_on_answer_modified(sender, user, **extra):
)
statements.append(statement)

XAPI.send_statements(statements)
for statement in statements:
XAPI.send_statement(statement)

# on_answer_delete
# deleted answer_solution
Expand Down Expand Up @@ -494,7 +496,8 @@ def xapi_on_comparison_update(sender, user, **extra):
)
statements.append(statement)

XAPI.send_statements(statements)
for statement in statements:
XAPI.send_statement(statement)


# on_course_create
Expand Down
Loading

0 comments on commit e6c3546

Please sign in to comment.