diff --git a/openedx/core/djangoapps/xblock/tests/test_utils.py b/openedx/core/djangoapps/xblock/tests/test_utils.py index a3510d965a..5f5e4f6d21 100644 --- a/openedx/core/djangoapps/xblock/tests/test_utils.py +++ b/openedx/core/djangoapps/xblock/tests/test_utils.py @@ -9,6 +9,8 @@ from freezegun import freeze_time from openedx.core.djangoapps.xblock.utils import ( get_secure_token_for_xblock_handler, + validate_secure_token_for_xblock_handler, + _get_secure_token_for_xblock_handler, validate_secure_token_for_xblock_handler ) @@ -35,15 +37,49 @@ REFERENCE_PARAMS = { ({}, True), # Ensure token still valid in 1 hour ({"validation_time_delta_s": 3600}, True), - # Ensure token still valid in 1 day, this should be true but isn't due - # to the bug that will be fixed in ARCHBOM-1677 - ({"validation_time_delta_s": 86400}, False), + # Ensure token still valid in 1 day + ({"validation_time_delta_s": 86400}, True), # Ensure token is invalid after 5 days ({"validation_time_delta_s": 86400 * 5}, False), # Ensure token is not valid 5 days before generation # In the case where the validating server is really skewed # from the generating server. ({"validation_time_delta_s": 86400 * -5}, False), + # Setting reference_time to 20 seconds after start of a 2 day time period(UTC) + # Demonstrating maximum possible validity period is just below 4 days + # This passes because validation time is just below the cutoff point + ( + {"reference_time": datetime.datetime(2021, 1, 27, 0, 0, 20, tzinfo=datetime.timezone.utc), + "validation_time_delta_s": (86400 * 4) - 21 + }, + True, + ), + # Setting reference_time to 20 seconds after start of a 2 day time period(UTC) + # Demonstrating maximum possible validity period is just below 4 days + # This does not pass because validation time is just above the cutoff point + ( + {"reference_time": datetime.datetime(2021, 1, 27, 0, 0, 20, tzinfo=datetime.timezone.utc), + "validation_time_delta_s": (86400 * 4) - 19 + }, + False, + ), + # Setting reference_time to 20 seconds before end of a 2 day time period(UTC) + # Demonstrating minimum possible validity period is just above 2 days + # This passes because validation time is just below the cutoff point + ( + {"reference_time": datetime.datetime(2021, 1, 28, 23, 59, 40, tzinfo=datetime.timezone.utc), + "validation_time_delta_s": (86400 * 2) + 19 + }, + True, + ), + # Setting reference_time to 20 seconds before end of a 2 day time period(UTC) + # Demonstrating minimum possible validity period is just above 2 days + # This fails because validation time is just above the cutoff point + ( + {"reference_time": datetime.datetime(2021, 1, 28, 23, 59, 40, tzinfo=datetime.timezone.utc), + "validation_time_delta_s": (86400 * 2) + 21 + }, + False), # Different user tries to use your token. ({"validation_user_id": 54321}, False), # Access a different block. @@ -114,6 +150,18 @@ def test_secure_token(param_delta: dict, expected_validation: bool): assert ( validate_secure_token_for_xblock_handler( params["validation_user_id"], params["validation_block_key_str"], token - ) - == expected_validation + ) == + expected_validation ) + + +def test_private_get_secure_token_for_xblock_handler(): + """ + Confirms function behaviour has not changed and is giving the same token for same inputs + If these tokens change, this will invalidate some or all of the tokens handed out to learners + """ + with freeze_time(datetime.datetime(2021, 1, 27, 0, 0, 0, tzinfo=datetime.timezone.utc)): + token_now = _get_secure_token_for_xblock_handler("12345", "some_block_key", 0, "some_hashing_key") + token_previous = _get_secure_token_for_xblock_handler("12345", "some_block_key", -1, "some_hashing_key") + assert token_now == "de8399a51fef6aa7584a" + assert token_previous == "f3b5d92b8ca2934009e4" diff --git a/openedx/core/djangoapps/xblock/utils.py b/openedx/core/djangoapps/xblock/utils.py index 6eb504437d..877325a86c 100644 --- a/openedx/core/djangoapps/xblock/utils.py +++ b/openedx/core/djangoapps/xblock/utils.py @@ -97,21 +97,10 @@ def _get_secure_token_for_xblock_handler(user_id, block_key_str, time_idx: int, Internal function to extract repeating hashing steps which we call multiple times with different time_idx and hashing key. """ - # This was supposed to be an interval boundary number that matches - # the current 2 day chunk(midnight UTC of every other day). - # Instead it's a number that will always be consistent but - # otherwise does not have a semantic meaning. - # Right now the math is missing a multiplication by TOKEN_PERIOD after - # the math.floor which means the unit is days which is then added to - # 0 or -TOKEN_PERIOD seconds. - # Right now this is only valid for 0-2 days instead of the intended - # 2-4 days because of the units issue above. Correcting the math should - # be possible but we are not going to do it in the middle of the other - # change we're making. We've made https://openedx.atlassian.net/browse/ARCHBOM-1677 - # to come back and deal with it. TOKEN_PERIOD = 24 * 60 * 60 * 2 # These URLs are valid for 2-4 days + # time_token is the number of time periods since unix epoch time_token = math.floor(time.time() / TOKEN_PERIOD) - time_token += TOKEN_PERIOD * time_idx + time_token += time_idx check_string = str(time_token) + ':' + str(user_id) + ':' + block_key_str secure_key = hmac.new(hashing_key.encode('utf-8'), check_string.encode('utf-8'), hashlib.sha256).hexdigest() return secure_key[:20]