diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 6b9002502b..ede63d2752 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -51,8 +51,9 @@ process_submission_xml, process_uploaded_submission, process_and_validate_submission, apply_yang_checker_to_draft, run_all_yang_model_checks) +from ietf.submit.views import access_token_is_valid, auth_token_is_valid from ietf.utils import tool_version -from ietf.utils.accesstoken import generate_access_token +from ietf.utils.accesstoken import generate_access_token, generate_random_key from ietf.utils.mail import outbox, get_payload_text from ietf.utils.test_runner import TestBlobstoreManager from ietf.utils.test_utils import login_testing_unauthorized, TestCase @@ -3500,3 +3501,31 @@ def test_submissionerror(self, mock_sanitize_message): mock_sanitize_message.call_args_list, [mock.call("hi"), mock.call("there")], ) + + +class HelperTests(TestCase): + def test_access_token_is_valid(self): + submission: Submission = SubmissionFactory() # type: ignore + valid_token = submission.access_token() + access_key = submission.access_key # accept this for backwards compat + invalid_token = "not the valid token" + self.assertTrue(access_token_is_valid(submission, valid_token)) + self.assertTrue(access_token_is_valid(submission, access_key)) + self.assertFalse(access_token_is_valid(submission, invalid_token)) + + def test_auth_token_is_valid(self): + auth_key = generate_random_key() + submission: Submission = SubmissionFactory(auth_key = auth_key) # type: ignore + valid_token = generate_access_token(submission.auth_key) + auth_key = submission.auth_key # accept this for backwards compat + invalid_token = "not the valid token" + self.assertTrue(auth_token_is_valid(submission, valid_token)) + self.assertTrue(auth_token_is_valid(submission, auth_key)) + self.assertFalse(auth_token_is_valid(submission, invalid_token)) + + submission.auth_key = "" + submission.save() + self.assertFalse(auth_token_is_valid(submission, valid_token)) + self.assertFalse(auth_token_is_valid(submission, auth_key)) + self.assertFalse(auth_token_is_valid(submission, invalid_token)) + self.assertFalse(auth_token_is_valid(submission, "")) diff --git a/ietf/submit/views.py b/ietf/submit/views.py index 043b613016..8329a312bb 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- import re import datetime +from secrets import compare_digest from typing import Optional, cast # pyflakes:ignore from urllib.parse import urljoin @@ -255,19 +256,48 @@ def search_submission(request): ) -def can_edit_submission(user, submission, access_token): - key_matched = access_token and submission.access_token() == access_token - if not key_matched: key_matched = submission.access_key == access_token # backwards-compat - return key_matched or has_role(user, "Secretariat") +def access_token_is_valid(submission: Submission, access_token: str): + """Check whether access_token is valid for submission, in constant time""" + token_matched = compare_digest(submission.access_token(), access_token) + # also compare key directly for backwards compatibility + key_matched = compare_digest(submission.access_key, access_token) + return token_matched or key_matched + + +def auth_token_is_valid(submission: Submission, auth_token: str): + """Check whether auth_token is valid for submission, in constant time""" + auth_key = submission.auth_key + if not auth_key: + # Make the same calls as the other branch to keep constant time, then + # return False because there is no auth key + compare_digest(generate_access_token("fake"), auth_token) + compare_digest("fake", auth_token) + return False + else: + token_matched = compare_digest(generate_access_token(auth_key), auth_token) + # also compare key directly for backwards compatibility + key_matched = compare_digest(auth_key, auth_token) + return token_matched or key_matched + + +def can_edit_submission(user, submission: Submission, access_token: str | None): + if has_role(user, "Secretariat"): + return True + elif not access_token: + return False + return access_token_is_valid(submission, access_token) + def submission_status(request, submission_id, access_token=None): # type: (HttpRequest, str, Optional[str]) -> HttpResponse submission = get_object_or_404(Submission, pk=submission_id) - key_matched = access_token and submission.access_token() == access_token - if not key_matched: key_matched = submission.access_key == access_token # backwards-compat - if access_token and not key_matched: - raise Http404 + if access_token: + key_matched = access_token_is_valid(submission, access_token) + if not key_matched: + raise Http404 + else: + key_matched = False if submission.state.slug == "cancel": errors = {} @@ -621,8 +651,7 @@ def edit_submission(request, submission_id, access_token=None): def confirm_submission(request, submission_id, auth_token): submission = get_object_or_404(Submission, pk=submission_id) - key_matched = submission.auth_key and auth_token == generate_access_token(submission.auth_key) - if not key_matched: key_matched = auth_token == submission.auth_key # backwards-compat + key_matched = submission.auth_key and auth_token_is_valid(submission, auth_token) if request.method == 'POST' and submission.state_id in ("auth", "aut-appr") and key_matched: # Set a temporary state 'confirmed' to avoid entering this code