From a1a30974ea0074a78648e6f74b4a8e0c4ee87a95 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 24 May 2024 11:29:42 -0300 Subject: [PATCH 01/15] fix: compare ext with leading '.' (#7458) This allows an exception to be raised if submission files are missing, leading to a server error. That's not pretty, but is better than ignoring the fail. --- ietf/submit/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index a19c42ecf..770352b4c 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -661,7 +661,7 @@ def move_files_to_repository(submission): os.link(dest, ftp_dest) elif dest.exists(): log.log("Intended to move '%s' to '%s', but found source missing while destination exists.") - elif ext in submission.file_types.split(','): + elif f".{ext}" in submission.file_types.split(','): raise ValueError("Intended to move '%s' to '%s', but found source and destination missing.") From 1a2996e5f6fc9440a0df96dba5d64b9667cbfd04 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 24 May 2024 11:30:01 -0300 Subject: [PATCH 02/15] feat: expire submissions after 14 days (#7461) * feat: expire submissions after 14 days * test: update test_cancel_stale_submissions --- ietf/settings.py | 3 +++ ietf/submit/tasks.py | 19 ++++++++++++++++-- ietf/submit/tests.py | 47 ++++++++++++++++++++++++++++++++++++-------- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/ietf/settings.py b/ietf/settings.py index 8bb264bd6..95a8ad4e4 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -817,6 +817,9 @@ IDSUBMIT_CHECKER_CLASSES = ( # Max time to allow for validation before a submission is subject to cancellation IDSUBMIT_MAX_VALIDATION_TIME = datetime.timedelta(minutes=20) +# Age at which a submission expires if not posted +IDSUBMIT_EXPIRATION_AGE = datetime.timedelta(days=14) + IDSUBMIT_MANUAL_STAGING_DIR = '/tmp/' IDSUBMIT_FILE_TYPES = ( diff --git a/ietf/submit/tasks.py b/ietf/submit/tasks.py index 382bff7fa..9a13268bc 100644 --- a/ietf/submit/tasks.py +++ b/ietf/submit/tasks.py @@ -37,19 +37,34 @@ def process_and_accept_uploaded_submission_task(submission_id): @shared_task def cancel_stale_submissions(): now = timezone.now() - stale_submissions = Submission.objects.filter( + # first check for submissions gone stale awaiting validation + stale_unvalidated_submissions = Submission.objects.filter( state_id='validating', ).annotate( submitted_at=Min('submissionevent__time'), ).filter( submitted_at__lt=now - settings.IDSUBMIT_MAX_VALIDATION_TIME, ) - for subm in stale_submissions: + for subm in stale_unvalidated_submissions: age = now - subm.submitted_at log.log(f'Canceling stale submission (id={subm.id}, age={age})') cancel_submission(subm) create_submission_event(None, subm, 'Submission canceled: validation checks took too long') + # now check for expired submissions + expired_submissions = Submission.objects.exclude( + state_id__in=["posted", "cancel"], + ).annotate( + submitted_at=Min("submissionevent__time"), + ).filter( + submitted_at__lt=now - settings.IDSUBMIT_EXPIRATION_AGE, + ) + for subm in expired_submissions: + age = now - subm.submitted_at + log.log(f'Canceling expired submission (id={subm.id}, age={age})') + cancel_submission(subm) + create_submission_event(None, subm, 'Submission canceled: expired without being posted') + @shared_task(bind=True) def poke(self): diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 58a47aef8..618f237e4 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -42,7 +42,7 @@ from ietf.group.models import Group from ietf.group.utils import setup_default_community_list_for_group from ietf.meeting.models import Meeting from ietf.meeting.factories import MeetingFactory -from ietf.name.models import FormalLanguageName +from ietf.name.models import DraftSubmissionStateName, FormalLanguageName from ietf.person.models import Person from ietf.person.factories import UserFactory, PersonFactory, EmailFactory from ietf.submit.factories import SubmissionFactory, SubmissionExtResourceFactory @@ -3136,28 +3136,59 @@ class AsyncSubmissionTests(BaseSubmitTestCase): self.assertContains(r, s.name) self.assertContains(r, 'This submission is being processed and validated.', status_code=200) - @override_settings(IDSUBMIT_MAX_VALIDATION_TIME=datetime.timedelta(minutes=30)) + @override_settings( + IDSUBMIT_MAX_VALIDATION_TIME=datetime.timedelta(minutes=30), + IDSUBMIT_EXPIRATION_AGE=datetime.timedelta(minutes=90), + ) def test_cancel_stale_submissions(self): + # these will be lists of (Submission, "state_id") pairs + submissions_to_skip = [] + submissions_to_cancel = [] + + # submissions in the validating state fresh_submission = SubmissionFactory(state_id='validating') fresh_submission.submissionevent_set.create( desc='fake created event', time=timezone.now() - datetime.timedelta(minutes=15), ) + submissions_to_skip.append((fresh_submission, "validating")) + stale_submission = SubmissionFactory(state_id='validating') stale_submission.submissionevent_set.create( desc='fake created event', time=timezone.now() - datetime.timedelta(minutes=30, seconds=1), ) + submissions_to_cancel.append((stale_submission, "validating")) + + # submissions in other states + for state in DraftSubmissionStateName.objects.filter(used=True).exclude(slug="validating"): + to_skip = SubmissionFactory(state_id=state.pk) + to_skip.submissionevent_set.create( + desc="fake created event", + time=timezone.now() - datetime.timedelta(minutes=45), # would be canceled if it were "validating" + ) + submissions_to_skip.append((to_skip, state.pk)) + to_expire = SubmissionFactory(state_id=state.pk) + to_expire.submissionevent_set.create( + desc="fake created event", + time=timezone.now() - datetime.timedelta(minutes=90, seconds=1), + ) + if state.pk in ["posted", "cancel"]: + submissions_to_skip.append((to_expire, state.pk)) # these ones should not be expired regardless of age + else: + submissions_to_cancel.append(((to_expire, state.pk))) cancel_stale_submissions() - fresh_submission = Submission.objects.get(pk=fresh_submission.pk) - self.assertEqual(fresh_submission.state_id, 'validating') - self.assertEqual(fresh_submission.submissionevent_set.count(), 1) + for _subm, original_state_id in submissions_to_skip: + subm = Submission.objects.get(pk=_subm.pk) + self.assertEqual(subm.state_id, original_state_id) + self.assertEqual(subm.submissionevent_set.count(), 1) - stale_submission = Submission.objects.get(pk=stale_submission.pk) - self.assertEqual(stale_submission.state_id, 'cancel') - self.assertEqual(stale_submission.submissionevent_set.count(), 2) + for _subm, _ in submissions_to_cancel: + subm = Submission.objects.get(pk=_subm.pk) + self.assertEqual(subm.state_id, "cancel") + self.assertEqual(subm.submissionevent_set.count(), 2) class ApiSubmitTests(BaseSubmitTestCase): From b951c80a4d67801f44485bd4600f915738b87b9f Mon Sep 17 00:00:00 2001 From: Nicolas Giard Date: Fri, 24 May 2024 16:34:41 -0400 Subject: [PATCH 03/15] chore: update app-init.sh with linux host chown check --- docker/scripts/app-init.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docker/scripts/app-init.sh b/docker/scripts/app-init.sh index c8286b242..29fe9a913 100755 --- a/docker/scripts/app-init.sh +++ b/docker/scripts/app-init.sh @@ -2,6 +2,12 @@ WORKSPACEDIR="/workspace" +# Handle Linux host mounting the workspace dir as root +if [ ! -O "$WORKSPACEDIR/ietf" ]; then + sudo chown -R dev:dev $WORKSPACEDIR +fi + +# Start rsyslog service sudo service rsyslog start &>/dev/null # Add /workspace as a safe git directory From 96902bf3b89783606a8037789565e1006f391038 Mon Sep 17 00:00:00 2001 From: Nicolas Giard Date: Fri, 24 May 2024 16:41:19 -0400 Subject: [PATCH 04/15] chore: fix app-init.sh chown check --- docker/scripts/app-init.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/scripts/app-init.sh b/docker/scripts/app-init.sh index 29fe9a913..b96b88f1f 100755 --- a/docker/scripts/app-init.sh +++ b/docker/scripts/app-init.sh @@ -3,7 +3,7 @@ WORKSPACEDIR="/workspace" # Handle Linux host mounting the workspace dir as root -if [ ! -O "$WORKSPACEDIR/ietf" ]; then +if [ ! -O "${WORKSPACEDIR}/ietf" ]; then sudo chown -R dev:dev $WORKSPACEDIR fi From 3c13db45fd6b8eaaf88427d1da01761abab4e5de Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 24 May 2024 18:36:58 -0300 Subject: [PATCH 05/15] fix: validate form in login() (#7435) * fix: validate form in login() * refactor: custom LoginView subclass for logins Preserves old behavior, but avoids some hacks. * test: reverse with strings, not view refs * chore: remove unused imports * fix: restore logout() call --- ietf/ietfauth/tests.py | 53 ++++++++++++------------ ietf/ietfauth/urls.py | 2 +- ietf/ietfauth/views.py | 92 ++++++++++++++++++++++-------------------- 3 files changed, 74 insertions(+), 73 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 29afa56d7..56b4638f6 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -50,8 +50,6 @@ from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.timezone import date_today -import ietf.ietfauth.views - if os.path.exists(settings.HTPASSWD_COMMAND): skip_htpasswd_command = False skip_message = "" @@ -83,30 +81,30 @@ class IetfAuthTests(TestCase): super().tearDown() def test_index(self): - self.assertEqual(self.client.get(urlreverse(ietf.ietfauth.views.index)).status_code, 200) + self.assertEqual(self.client.get(urlreverse("ietf.ietfauth.views.index")).status_code, 200) def test_login_and_logout(self): PersonFactory(user__username='plain') # try logging in without a next - r = self.client.get(urlreverse(ietf.ietfauth.views.login)) + r = self.client.get(urlreverse("ietf.ietfauth.views.login")) self.assertEqual(r.status_code, 200) - r = self.client.post(urlreverse(ietf.ietfauth.views.login), {"username":"plain", "password":"plain+password"}) + r = self.client.post(urlreverse("ietf.ietfauth.views.login"), {"username":"plain", "password":"plain+password"}) self.assertEqual(r.status_code, 302) - self.assertEqual(urlsplit(r["Location"])[2], urlreverse(ietf.ietfauth.views.profile)) + self.assertEqual(urlsplit(r["Location"])[2], urlreverse("ietf.ietfauth.views.profile")) # try logging out r = self.client.post(urlreverse('django.contrib.auth.views.logout'), {}) self.assertEqual(r.status_code, 200) self.assertNotContains(r, "accounts/logout") - r = self.client.get(urlreverse(ietf.ietfauth.views.profile)) + r = self.client.get(urlreverse("ietf.ietfauth.views.profile")) self.assertEqual(r.status_code, 302) - self.assertEqual(urlsplit(r["Location"])[2], urlreverse(ietf.ietfauth.views.login)) + self.assertEqual(urlsplit(r["Location"])[2], urlreverse("ietf.ietfauth.views.login")) # try logging in with a next - r = self.client.post(urlreverse(ietf.ietfauth.views.login) + "?next=/foobar", {"username":"plain", "password":"plain+password"}) + r = self.client.post(urlreverse("ietf.ietfauth.views.login") + "?next=/foobar", {"username":"plain", "password":"plain+password"}) self.assertEqual(r.status_code, 302) self.assertEqual(urlsplit(r["Location"])[2], "/foobar") @@ -137,19 +135,19 @@ class IetfAuthTests(TestCase): # try with a trivial next _test_login("/") # try with a next that requires login - _test_login(urlreverse(ietf.ietfauth.views.profile)) + _test_login(urlreverse("ietf.ietfauth.views.profile")) def test_login_with_different_email(self): person = PersonFactory(user__username='plain') email = EmailFactory(person=person) # try logging in without a next - r = self.client.get(urlreverse(ietf.ietfauth.views.login)) + r = self.client.get(urlreverse("ietf.ietfauth.views.login")) self.assertEqual(r.status_code, 200) - r = self.client.post(urlreverse(ietf.ietfauth.views.login), {"username":email, "password":"plain+password"}) + r = self.client.post(urlreverse("ietf.ietfauth.views.login"), {"username":email, "password":"plain+password"}) self.assertEqual(r.status_code, 302) - self.assertEqual(urlsplit(r["Location"])[2], urlreverse(ietf.ietfauth.views.profile)) + self.assertEqual(urlsplit(r["Location"])[2], urlreverse("ietf.ietfauth.views.profile")) def extract_confirm_url(self, confirm_email): # dig out confirm_email link @@ -176,7 +174,7 @@ class IetfAuthTests(TestCase): # For the lowered barrier to account creation period, we are disabling this kind of failure # def test_create_account_failure(self): - # url = urlreverse(ietf.ietfauth.views.create_account) + # url = urlreverse("ietf.ietfauth.views.create_account") # # get # r = self.client.get(url) @@ -195,7 +193,7 @@ class IetfAuthTests(TestCase): self.assertTrue("Additional Assistance Required" in r) def register(self, email): - url = urlreverse(ietf.ietfauth.views.create_account) + url = urlreverse("ietf.ietfauth.views.create_account") # register email empty_outbox() @@ -240,7 +238,7 @@ class IetfAuthTests(TestCase): note = get_payload_text(outbox[-1]) self.assertIn(email, note) self.assertIn("A datatracker account for that email already exists", note) - self.assertIn(urlreverse(ietf.ietfauth.views.password_reset), note) + self.assertIn(urlreverse("ietf.ietfauth.views.password_reset"), note) def test_ietfauth_profile(self): EmailFactory(person__user__username='plain') @@ -249,7 +247,7 @@ class IetfAuthTests(TestCase): username = "plain" email_address = Email.objects.filter(person__user__username=username).first().address - url = urlreverse(ietf.ietfauth.views.profile) + url = urlreverse("ietf.ietfauth.views.profile") login_testing_unauthorized(self, username, url) @@ -400,7 +398,7 @@ class IetfAuthTests(TestCase): def test_email_case_insensitive_protection(self): EmailFactory(address="TestAddress@example.net") person = PersonFactory() - url = urlreverse(ietf.ietfauth.views.profile) + url = urlreverse("ietf.ietfauth.views.profile") login_testing_unauthorized(self, person.user.username, url) data = { @@ -441,7 +439,7 @@ class IetfAuthTests(TestCase): def test_reset_password(self): - url = urlreverse(ietf.ietfauth.views.password_reset) + url = urlreverse("ietf.ietfauth.views.password_reset") email = 'someone@example.com' password = 'foobar' @@ -507,7 +505,7 @@ class IetfAuthTests(TestCase): self.assertEqual(len(outbox), 1) confirm_url = self.extract_confirm_url(outbox[-1]) - r = self.client.post(urlreverse(ietf.ietfauth.views.login), {'username': email, 'password': password}) + r = self.client.post(urlreverse("ietf.ietfauth.views.login"), {'username': email, 'password': password}) r = self.client.get(confirm_url) self.assertEqual(r.status_code, 404) @@ -589,7 +587,7 @@ class IetfAuthTests(TestCase): availability="unavailable", ) - url = urlreverse(ietf.ietfauth.views.review_overview) + url = urlreverse("ietf.ietfauth.views.review_overview") login_testing_unauthorized(self, reviewer.user.username, url) @@ -633,10 +631,9 @@ class IetfAuthTests(TestCase): def test_change_password(self): - - chpw_url = urlreverse(ietf.ietfauth.views.change_password) - prof_url = urlreverse(ietf.ietfauth.views.profile) - login_url = urlreverse(ietf.ietfauth.views.login) + chpw_url = urlreverse("ietf.ietfauth.views.change_password") + prof_url = urlreverse("ietf.ietfauth.views.profile") + login_url = urlreverse("ietf.ietfauth.views.login") redir_url = '%s?next=%s' % (login_url, chpw_url) # get without logging in @@ -681,9 +678,9 @@ class IetfAuthTests(TestCase): def test_change_username(self): - chun_url = urlreverse(ietf.ietfauth.views.change_username) - prof_url = urlreverse(ietf.ietfauth.views.profile) - login_url = urlreverse(ietf.ietfauth.views.login) + chun_url = urlreverse("ietf.ietfauth.views.change_username") + prof_url = urlreverse("ietf.ietfauth.views.profile") + login_url = urlreverse("ietf.ietfauth.views.login") redir_url = '%s?next=%s' % (login_url, chun_url) # get without logging in diff --git a/ietf/ietfauth/urls.py b/ietf/ietfauth/urls.py index 30e639ad6..7493fe5c9 100644 --- a/ietf/ietfauth/urls.py +++ b/ietf/ietfauth/urls.py @@ -14,7 +14,7 @@ urlpatterns = [ url(r'^confirmnewemail/(?P[^/]+)/$', views.confirm_new_email), url(r'^create/$', views.create_account), url(r'^create/confirm/(?P[^/]+)/$', views.confirm_account), - url(r'^login/$', views.login), + url(r'^login/$', views.AnyEmailLoginView.as_view(), name="ietf.ietfauth.views.login"), url(r'^logout/$', LogoutView.as_view(), name="django.contrib.auth.views.logout"), url(r'^password/$', views.change_password), url(r'^profile/$', views.profile), diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 8c61b8356..7c3f72108 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -45,7 +45,7 @@ import django.core.signing from django import forms from django.contrib import messages from django.conf import settings -from django.contrib.auth import update_session_auth_hash, logout, authenticate +from django.contrib.auth import logout, update_session_auth_hash from django.contrib.auth.decorators import login_required from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.hashers import identify_hasher @@ -752,53 +752,57 @@ def change_username(request): return render(request, 'registration/change_username.html', {'form': form}) - -def login(request, extra_context=None): - """ - This login function is a wrapper around django's login() for the purpose - of providing a notification if the user's password has been cleared. The - warning will be triggered if the password field has been set to something - which is not recognized as a valid password hash. +class AnyEmailAuthenticationForm(AuthenticationForm): + """AuthenticationForm that allows any email address as the username + + Also performs a check for a cleared password field and provides a helpful error message + if that applies to the user attempting to log in. """ + _unauthenticated_user = None - if request.method == "POST": - form = AuthenticationForm(request, data=request.POST) - username = form.data.get('username') - user = User.objects.filter(username__iexact=username).first() # Consider _never_ actually looking for the User username and only looking at Email - if not user: - # try to find user ID from the email address + def clean_username(self): + username = self.cleaned_data.get("username", None) + if username is None: + raise self.get_invalid_login_error() + user = User.objects.filter(username__iexact=username).first() + if user is None: email = Email.objects.filter(address=username).first() - if email and email.person and email.person.user: - u2 = email.person.user - # be conservative, only accept this if login is valid - if u2: - pw = form.data.get('password') - au = authenticate(request, username=u2.username, password=pw) - if au: - # kludge to change the querydict - q2 = request.POST.copy() - q2['username'] = u2.username - request.POST = q2 - user = u2 - # - if user: - try: - identify_hasher(user.password) + if email and email.person: + user = email.person.user # might be None + if user is None: + raise self.get_invalid_login_error() + self._unauthenticated_user = user # remember this for the clean() method + return user.username + + def clean(self): + if self._unauthenticated_user is not None: + try: + identify_hasher(self._unauthenticated_user.password) except ValueError: - extra_context = {"alert": - "Note: Your password has been cleared because " - "of possible password leakage. " - "Please use the password reset link below " - "to set a new password for your account.", - } - response = LoginView.as_view(extra_context=extra_context)(request) - if isinstance(response, HttpResponseRedirect) and user and user.is_authenticated: - try: - user.person - except Person.DoesNotExist: - logout(request) - response = render(request, 'registration/missing_person.html') - return response + self.add_error( + "password", + 'Your password has been cleared because of possible password leakage. ' + 'Please use the "Forgot your password?" button below to set a new password ' + 'for your account.', + ) + return super().clean() + + +class AnyEmailLoginView(LoginView): + """LoginView that allows any email address as the username + + Redirects to the missing_person page instead of logging in if the user does not have a Person + """ + form_class = AnyEmailAuthenticationForm + + def form_valid(self, form): + """Security check complete. Log the user in if they have a Person.""" + user = form.get_user() # user has authenticated at this point + if not hasattr(user, "person"): + logout(self.request) # should not be logged in yet, but just in case... + return render(self.request, "registration/missing_person.html") + return super().form_valid(form) + @login_required @person_required From 79f858b7d753dff438375376de7148c426147419 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 28 May 2024 01:29:35 -0400 Subject: [PATCH 06/15] chore(deps): bump codecov/codecov-action from 4.3.1 to 4.4.1 (#7470) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4.3.1 to 4.4.1. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-action/compare/v4.3.1...v4.4.1) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f6d54b14b..1c44bb6f2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -59,7 +59,7 @@ jobs: path: geckodriver.log - name: Upload Coverage Results to Codecov - uses: codecov/codecov-action@v4.3.1 + uses: codecov/codecov-action@v4.4.1 with: files: coverage.xml From 08e953995a04f8940c75b9bfc2cfad71ee15faee Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 May 2024 12:34:55 -0300 Subject: [PATCH 07/15] feat: better reject null characters in forms (#7472) * feat: subclass ModelMultipleChoiceField to reject nuls * refactor: Use custom ModelMultipleChoiceField * fix: handle value=None --- ietf/doc/views_ballot.py | 3 ++- ietf/doc/views_draft.py | 9 +++++---- ietf/doc/views_review.py | 4 ++-- ietf/doc/views_search.py | 3 ++- ietf/liaisons/forms.py | 8 ++++---- ietf/meeting/forms.py | 12 +++++++++--- ietf/nomcom/forms.py | 9 +++++---- ietf/secr/sreq/forms.py | 5 +++-- ietf/submit/forms.py | 3 ++- ietf/utils/fields.py | 19 ++++++++++++++++++- 10 files changed, 52 insertions(+), 23 deletions(-) diff --git a/ietf/doc/views_ballot.py b/ietf/doc/views_ballot.py index 9b0ccdcea..ff5192156 100644 --- a/ietf/doc/views_ballot.py +++ b/ietf/doc/views_ballot.py @@ -38,6 +38,7 @@ from ietf.mailtrigger.forms import CcSelectForm from ietf.message.utils import infer_message from ietf.name.models import BallotPositionName, DocTypeName from ietf.person.models import Person +from ietf.utils.fields import ModelMultipleChoiceField from ietf.utils.mail import send_mail_text, send_mail_preformatted from ietf.utils.decorators import require_api_key from ietf.utils.response import permission_denied @@ -931,7 +932,7 @@ def approve_ballot(request, name): class ApproveDownrefsForm(forms.Form): - checkboxes = forms.ModelMultipleChoiceField( + checkboxes = ModelMultipleChoiceField( widget = forms.CheckboxSelectMultiple, queryset = RelatedDocument.objects.none(), ) diff --git a/ietf/doc/views_draft.py b/ietf/doc/views_draft.py index 1deca4503..30175491d 100644 --- a/ietf/doc/views_draft.py +++ b/ietf/doc/views_draft.py @@ -52,6 +52,7 @@ from ietf.person.models import Person, Email from ietf.utils.mail import send_mail, send_mail_message, on_behalf_of from ietf.utils.textupload import get_cleaned_text_file_content from ietf.utils import log +from ietf.utils.fields import ModelMultipleChoiceField from ietf.utils.response import permission_denied from ietf.utils.timezone import datetime_today, DEADLINE_TZINFO @@ -390,9 +391,9 @@ def replaces(request, name): )) class SuggestedReplacesForm(forms.Form): - replaces = forms.ModelMultipleChoiceField(queryset=Document.objects.all(), - label="Suggestions", required=False, widget=forms.CheckboxSelectMultiple, - help_text="Select only the documents that are replaced by this document") + replaces = ModelMultipleChoiceField(queryset=Document.objects.all(), + label="Suggestions", required=False, widget=forms.CheckboxSelectMultiple, + help_text="Select only the documents that are replaced by this document") comment = forms.CharField(label="Optional comment", widget=forms.Textarea, required=False, strip=False) def __init__(self, suggested, *args, **kwargs): @@ -1601,7 +1602,7 @@ class ChangeStreamStateForm(forms.Form): new_state = forms.ModelChoiceField(queryset=State.objects.filter(used=True), label='State' ) weeks = forms.IntegerField(label='Expected weeks in state',required=False) comment = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional comment for the document history.", strip=False) - tags = forms.ModelMultipleChoiceField(queryset=DocTagName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False) + tags = ModelMultipleChoiceField(queryset=DocTagName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False) def __init__(self, *args, **kwargs): doc = kwargs.pop("doc") diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 646b51b09..bb9e56742 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -52,7 +52,7 @@ from ietf.utils.text import strip_prefix, xslugify from ietf.utils.textupload import get_cleaned_text_file_content from ietf.utils.mail import send_mail_message from ietf.mailtrigger.utils import gather_address_lists -from ietf.utils.fields import MultiEmailField +from ietf.utils.fields import ModelMultipleChoiceField, MultiEmailField from ietf.utils.http import is_ajax from ietf.utils.response import permission_denied from ietf.utils.timezone import date_today, DEADLINE_TZINFO @@ -68,7 +68,7 @@ def clean_doc_revision(doc, rev): return rev class RequestReviewForm(forms.ModelForm): - team = forms.ModelMultipleChoiceField(queryset=Group.objects.all(), widget=forms.CheckboxSelectMultiple) + team = ModelMultipleChoiceField(queryset=Group.objects.all(), widget=forms.CheckboxSelectMultiple) deadline = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1", "start-date": "+0d" }) class Meta: diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 422e38f7d..2ef4ee83e 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -69,6 +69,7 @@ from ietf.name.models import DocTagName, DocTypeName, StreamName from ietf.person.models import Person from ietf.person.utils import get_active_ads from ietf.utils.draft_search import normalize_draftname +from ietf.utils.fields import ModelMultipleChoiceField from ietf.utils.log import log from ietf.doc.utils_search import prepare_document_table, doc_type, doc_state, doc_type_name, AD_WORKLOAD from ietf.ietfauth.utils import has_role @@ -100,7 +101,7 @@ class SearchForm(forms.Form): ("ad", "AD"), ("-ad", "AD (desc)"), ), required=False, widget=forms.HiddenInput) - doctypes = forms.ModelMultipleChoiceField(queryset=DocTypeName.objects.filter(used=True).exclude(slug__in=('draft', 'rfc', 'bcp', 'std', 'fyi', 'liai-att')).order_by('name'), required=False) + doctypes = ModelMultipleChoiceField(queryset=DocTypeName.objects.filter(used=True).exclude(slug__in=('draft', 'rfc', 'bcp', 'std', 'fyi', 'liai-att')).order_by('name'), required=False) def __init__(self, *args, **kwargs): super(SearchForm, self).__init__(*args, **kwargs) diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py index 0a6974e5b..1d91041b2 100644 --- a/ietf/liaisons/forms.py +++ b/ietf/liaisons/forms.py @@ -32,7 +32,7 @@ from ietf.group.models import Group from ietf.person.models import Email from ietf.person.fields import SearchableEmailField from ietf.doc.models import Document -from ietf.utils.fields import DatepickerDateField +from ietf.utils.fields import DatepickerDateField, ModelMultipleChoiceField from ietf.utils.timezone import date_today, datetime_from_date, DEADLINE_TZINFO from functools import reduce @@ -200,7 +200,7 @@ class SearchLiaisonForm(forms.Form): return results -class CustomModelMultipleChoiceField(forms.ModelMultipleChoiceField): +class CustomModelMultipleChoiceField(ModelMultipleChoiceField): '''If value is a QuerySet, return it as is (for use in widget.render)''' def prepare_value(self, value): if isinstance(value, QuerySetAny): @@ -215,12 +215,12 @@ class CustomModelMultipleChoiceField(forms.ModelMultipleChoiceField): class LiaisonModelForm(forms.ModelForm): '''Specify fields which require a custom widget or that are not part of the model. ''' - from_groups = forms.ModelMultipleChoiceField(queryset=Group.objects.all(),label='Groups',required=False) + from_groups = ModelMultipleChoiceField(queryset=Group.objects.all(),label='Groups',required=False) from_groups.widget.attrs["class"] = "select2-field" from_groups.widget.attrs['data-minimum-input-length'] = 0 from_contact = forms.EmailField() # type: Union[forms.EmailField, SearchableEmailField] to_contacts = forms.CharField(label="Contacts", widget=forms.Textarea(attrs={'rows':'3', }), strip=False) - to_groups = forms.ModelMultipleChoiceField(queryset=Group.objects,label='Groups',required=False) + to_groups = ModelMultipleChoiceField(queryset=Group.objects,label='Groups',required=False) to_groups.widget.attrs["class"] = "select2-field" to_groups.widget.attrs['data-minimum-input-length'] = 0 deadline = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={"autoclose": "1" }, label='Deadline', required=True) diff --git a/ietf/meeting/forms.py b/ietf/meeting/forms.py index 2cec669db..b31ffb6cd 100644 --- a/ietf/meeting/forms.py +++ b/ietf/meeting/forms.py @@ -28,7 +28,13 @@ from ietf.meeting.helpers import is_interim_meeting_approved, get_next_agenda_na from ietf.message.models import Message from ietf.name.models import TimeSlotTypeName, SessionPurposeName from ietf.person.models import Person -from ietf.utils.fields import DatepickerDateField, DurationField, MultiEmailField, DatepickerSplitDateTimeWidget +from ietf.utils.fields import ( + DatepickerDateField, + DatepickerSplitDateTimeWidget, + DurationField, + ModelMultipleChoiceField, + MultiEmailField, +) from ietf.utils.validators import ( validate_file_size, validate_mime_type, validate_file_extension, validate_no_html_frame) @@ -551,7 +557,7 @@ class SwapTimeslotsForm(forms.Form): queryset=TimeSlot.objects.none(), # default to none, fill in when we have a meeting widget=forms.TextInput, ) - rooms = forms.ModelMultipleChoiceField( + rooms = ModelMultipleChoiceField( required=True, queryset=Room.objects.none(), # default to none, fill in when we have a meeting widget=CsvModelPkInput, @@ -617,7 +623,7 @@ class TimeSlotCreateForm(forms.Form): ) duration = TimeSlotDurationField() show_location = forms.BooleanField(required=False, initial=True) - locations = forms.ModelMultipleChoiceField( + locations = ModelMultipleChoiceField( queryset=Room.objects.none(), widget=forms.CheckboxSelectMultiple, ) diff --git a/ietf/nomcom/forms.py b/ietf/nomcom/forms.py index 7db500912..5987b2263 100644 --- a/ietf/nomcom/forms.py +++ b/ietf/nomcom/forms.py @@ -21,6 +21,7 @@ from ietf.nomcom.utils import (NOMINATION_RECEIPT_TEMPLATE, FEEDBACK_RECEIPT_TEM from ietf.person.models import Email from ietf.person.fields import (SearchableEmailField, SearchableEmailsField, SearchablePersonField, SearchablePersonsField ) +from ietf.utils.fields import ModelMultipleChoiceField from ietf.utils.mail import send_mail from ietf.mailtrigger.utils import gather_address_lists @@ -719,9 +720,9 @@ class MutableFeedbackForm(forms.ModelForm): required= self.feedback_type.slug != 'comment', help_text='Hold down "Control", or "Command" on a Mac, to select more than one.') if self.feedback_type.slug == 'comment': - self.fields['topic'] = forms.ModelMultipleChoiceField(queryset=self.nomcom.topic_set.all(), - help_text='Hold down "Control" or "Command" on a Mac, to select more than one.', - required=False,) + self.fields['topic'] = ModelMultipleChoiceField(queryset=self.nomcom.topic_set.all(), + help_text='Hold down "Control" or "Command" on a Mac, to select more than one.', + required=False,) else: self.fields['position'] = forms.ModelChoiceField(queryset=Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True), label="Position") self.fields['searched_email'] = SearchableEmailField(only_users=False,help_text="Try to find the candidate you are classifying with this field first. Only use the name and email fields below if this search does not find the candidate.",label="Candidate",required=False) @@ -847,7 +848,7 @@ class EditNomineeForm(forms.ModelForm): class NominationResponseCommentForm(forms.Form): comments = forms.CharField(widget=forms.Textarea,required=False,help_text="Any comments provided will be encrypted and will only be visible to the NomCom.", strip=False) -class NomcomVolunteerMultipleChoiceField(forms.ModelMultipleChoiceField): +class NomcomVolunteerMultipleChoiceField(ModelMultipleChoiceField): def label_from_instance(self, obj): year = obj.year() return f'Volunteer for the {year}/{year+1} Nominating Committee' diff --git a/ietf/secr/sreq/forms.py b/ietf/secr/sreq/forms.py index 1100bc7c8..4a0f449b2 100644 --- a/ietf/secr/sreq/forms.py +++ b/ietf/secr/sreq/forms.py @@ -13,6 +13,7 @@ from ietf.meeting.forms import sessiondetailsformset_factory from ietf.meeting.models import ResourceAssociation, Constraint from ietf.person.fields import SearchablePersonsField from ietf.person.models import Person +from ietf.utils.fields import ModelMultipleChoiceField from ietf.utils.html import clean_text_field from ietf.utils import log @@ -57,7 +58,7 @@ class GroupSelectForm(forms.Form): self.fields['group'].widget.choices = choices -class NameModelMultipleChoiceField(forms.ModelMultipleChoiceField): +class NameModelMultipleChoiceField(ModelMultipleChoiceField): def label_from_instance(self, name): return name.desc @@ -159,7 +160,7 @@ class SessionForm(forms.Form): self.fields['resources'].widget = forms.MultipleHiddenInput() self.fields['timeranges'].widget = forms.MultipleHiddenInput() # and entirely replace bethere - no need to support searching if input is hidden - self.fields['bethere'] = forms.ModelMultipleChoiceField( + self.fields['bethere'] = ModelMultipleChoiceField( widget=forms.MultipleHiddenInput, required=False, queryset=Person.objects.all(), ) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index f857ac9fd..4e5644b36 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -39,6 +39,7 @@ from ietf.submit.parsers.plain_parser import PlainParser from ietf.submit.parsers.xml_parser import XMLParser from ietf.utils import log from ietf.utils.draft import PlaintextDraft +from ietf.utils.fields import ModelMultipleChoiceField from ietf.utils.text import normalize_text from ietf.utils.timezone import date_today from ietf.utils.xmldraft import InvalidXMLError, XMLDraft, XMLParseError @@ -793,7 +794,7 @@ class EditSubmissionForm(forms.ModelForm): rev = forms.CharField(label='Revision', max_length=2, required=True) document_date = forms.DateField(required=True) pages = forms.IntegerField(required=True) - formal_languages = forms.ModelMultipleChoiceField(queryset=FormalLanguageName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False) + formal_languages = ModelMultipleChoiceField(queryset=FormalLanguageName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False) abstract = forms.CharField(widget=forms.Textarea, required=True, strip=False) note = forms.CharField(label=mark_safe('Comment to the Secretariat'), widget=forms.Textarea, required=False, strip=False) diff --git a/ietf/utils/fields.py b/ietf/utils/fields.py index 95d8a2aa7..3e6f56d45 100644 --- a/ietf/utils/fields.py +++ b/ietf/utils/fields.py @@ -14,7 +14,7 @@ from typing import Optional, Type # pyflakes:ignore from django import forms from django.db import models # pyflakes:ignore -from django.core.validators import validate_email +from django.core.validators import ProhibitNullCharactersValidator, validate_email from django.core.exceptions import ValidationError from django.utils.dateparse import parse_duration @@ -353,3 +353,20 @@ class MissingOkImageField(models.ImageField): super().update_dimension_fields(*args, **kwargs) except FileNotFoundError: pass # don't do anything if the file has gone missing + + +class ModelMultipleChoiceField(forms.ModelMultipleChoiceField): + """ModelMultipleChoiceField that rejects null characters cleanly""" + validate_no_nulls = ProhibitNullCharactersValidator() + + def clean(self, value): + try: + for item in value: + self.validate_no_nulls(item) + except TypeError: + # A TypeError probably means value is not iterable, which most commonly comes up + # with None as a value. If it's something more exotic, we don't know how to test + # for null characters anyway. Either way, trust the superclass clean() method to + # handle it. + pass + return super().clean(value) From 39d471d3ac63f2e90b842847a682ea47be3df4e0 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Tue, 28 May 2024 10:35:29 -0500 Subject: [PATCH 08/15] fix: better chatlog and polls links (#7466) --- ietf/templates/meeting/session_details_panel.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ietf/templates/meeting/session_details_panel.html b/ietf/templates/meeting/session_details_panel.html index df0f57cae..d053ba1c1 100644 --- a/ietf/templates/meeting/session_details_panel.html +++ b/ietf/templates/meeting/session_details_panel.html @@ -137,8 +137,8 @@ {% url 'ietf.doc.views_doc.document_main' name=pres.document.name as url %} - {{ pres.document.title }} - ({{ pres.document.name }}) + {{ pres.document.title }} + ( as json ) {% endfor %} From 1cdfd97937bfa8e2447d8876642749a37ebcf894 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 28 May 2024 15:22:27 -0300 Subject: [PATCH 09/15] fix: abort if output-dir is not a dir (#7478) --- ietf/bin/aliases-from-json.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ietf/bin/aliases-from-json.py b/ietf/bin/aliases-from-json.py index 72fcb469f..a0c383a1a 100644 --- a/ietf/bin/aliases-from-json.py +++ b/ietf/bin/aliases-from-json.py @@ -61,6 +61,13 @@ def generate_files(records, adest, vdest, postconfirm, vdomain): shutil.move(vpath, vdest) +def directory_path(val): + p = Path(val) + if p.is_dir(): + return p + else: + raise argparse.ArgumentTypeError(f"{p} is not a directory") + if __name__ == "__main__": parser = argparse.ArgumentParser( description="Convert a JSON stream of draft alias definitions into alias / virtual alias files." @@ -73,7 +80,7 @@ if __name__ == "__main__": parser.add_argument( "--output-dir", default="./", - type=Path, + type=directory_path, help="Destination for output files.", ) parser.add_argument( @@ -87,8 +94,6 @@ if __name__ == "__main__": help=f"Virtual domain (defaults to {VDOMAIN}_", ) args = parser.parse_args() - if not args.output_dir.is_dir(): - sys.stderr.write("Error: output-dir must be a directory") data = json.load(sys.stdin) generate_files( data["aliases"], From f01ef0c9157988cf62e625da0cad06cb7ddb631a Mon Sep 17 00:00:00 2001 From: Matthew Holloway Date: Wed, 29 May 2024 14:47:49 +1200 Subject: [PATCH 10/15] Adding Linux Docker Desktop install docs --- docker/README.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/docker/README.md b/docker/README.md index bc9af7c21..2e4b50d8f 100644 --- a/docker/README.md +++ b/docker/README.md @@ -6,7 +6,7 @@ > See the [IETF Tools Windows Dev guide](https://github.com/ietf-tools/.github/blob/main/docs/windows-dev.md) on how to get started when using Windows. -2. On Linux, you must also install [Docker Compose](https://docs.docker.com/compose/install/). Docker Desktop for Mac and Windows already include Docker Compose. +2. On Linux, you must [install Docker Compose manually](https://docs.docker.com/compose/install/linux/#install-the-plugin-manually) and not install Docker Desktop. On Mac and Windows install Docker Desktop which already includes Docker Compose. 2. If you have a copy of the datatracker code checked out already, simply `cd` to the top-level directory. @@ -183,3 +183,18 @@ The content of the source files will be copied into the target `.ics` files. Mak ### Missing assets in the data folder Because including all assets in the image would significantly increase the file size, they are not included by default. You can however fetch them by running the **Fetch assets via rsync** task in VS Code or run manually the script `docker/scripts/app-rsync-extras.sh` + + +### Linux file permissions leaking to the host system + +If on the host filesystem you have permissions that look like this, + +```bash +$ ls -la +total 4624 +drwxrwxr-x 2 100999 100999 4096 May 25 07:56 bin +drwxrwxr-x 5 100999 100999 4096 May 25 07:56 client +(etc...) +``` + +Try uninstalling Docker Desktop and installing Docker Compose manually. The Docker Compose bundled with Docker Desktop is incompatible with our software. See also [Rootless Docker: file ownership changes #3343](https://github.com/lando/lando/issues/3343), [Docker context desktop-linux has container permission issues #75](https://github.com/docker/desktop-linux/issues/75). \ No newline at end of file From 020bdeb0585eb65624677951a97392091ce40ac7 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 30 May 2024 10:23:49 -0300 Subject: [PATCH 11/15] feat: purge_personal_api_key_events() task (#7485) * feat: purge_personal_api_key_events() task * feat: log number of events purged * test: test new task * fix: name task properly * chore: create daily PeriodicTask * chore: remove old management command * chore: remove tests of old command * test: finish removing now-empty tests.py --- bin/daily | 3 - .../purge_old_personal_api_key_events.py | 64 --------- ietf/person/management/commands/tests.py | 122 ------------------ ietf/person/tasks.py | 20 +++ ietf/person/tests.py | 19 ++- .../management/commands/periodic_tasks.py | 11 ++ 6 files changed, 48 insertions(+), 191 deletions(-) delete mode 100644 ietf/person/management/commands/purge_old_personal_api_key_events.py delete mode 100644 ietf/person/management/commands/tests.py create mode 100644 ietf/person/tasks.py diff --git a/bin/daily b/bin/daily index 6adb16794..b4ea33995 100755 --- a/bin/daily +++ b/bin/daily @@ -36,6 +36,3 @@ $DTDIR/ietf/manage.py populate_yang_model_dirs -v0 # Re-run yang checks on active documents $DTDIR/ietf/manage.py run_yang_model_checks -v0 - -# Purge older PersonApiKeyEvents -$DTDIR/ietf/manage.py purge_old_personal_api_key_events 14 diff --git a/ietf/person/management/commands/purge_old_personal_api_key_events.py b/ietf/person/management/commands/purge_old_personal_api_key_events.py deleted file mode 100644 index 66b9d2c33..000000000 --- a/ietf/person/management/commands/purge_old_personal_api_key_events.py +++ /dev/null @@ -1,64 +0,0 @@ -# Copyright The IETF Trust 2021, All Rights Reserved -# -*- coding: utf-8 -*- - -from datetime import timedelta -from django.core.management.base import BaseCommand, CommandError -from django.db.models import Max, Min -from django.utils import timezone - -from ietf.person.models import PersonApiKeyEvent - - -class Command(BaseCommand): - help = 'Purge PersonApiKeyEvent instances older than KEEP_DAYS days' - - def add_arguments(self, parser): - parser.add_argument('keep_days', type=int, - help='Delete events older than this many days') - parser.add_argument('-n', '--dry-run', action='store_true', default=False, - help="Don't delete events, just show what would be done") - - - def handle(self, *args, **options): - keep_days = options['keep_days'] - dry_run = options['dry_run'] - verbosity = options.get("verbosity", 1) - - def _format_count(count, unit='day'): - return '{} {}{}'.format(count, unit, ('' if count == 1 else 's')) - - if keep_days < 0: - raise CommandError('Negative keep_days not allowed ({} was specified)'.format(keep_days)) - - if verbosity > 1: - self.stdout.write('purge_old_personal_api_key_events: Finding events older than {}\n'.format(_format_count(keep_days))) - if dry_run: - self.stdout.write('Dry run requested, records will not be deleted\n') - self.stdout.flush() - - now = timezone.now() - old_events = PersonApiKeyEvent.objects.filter( - time__lt=now - timedelta(days=keep_days) - ) - - stats = old_events.aggregate(Min('time'), Max('time')) - old_count = old_events.count() - if old_count == 0: - if verbosity > 1: - self.stdout.write('No events older than {} found\n'.format(_format_count(keep_days))) - return - - oldest_date = stats['time__min'] - oldest_ago = now - oldest_date - newest_date = stats['time__max'] - newest_ago = now - newest_date - - action_fmt = 'Would delete {}\n' if dry_run else 'Deleting {}\n' - if verbosity > 1: - self.stdout.write(action_fmt.format(_format_count(old_count, 'event'))) - self.stdout.write(' Oldest at {} ({} ago)\n'.format(oldest_date, _format_count(oldest_ago.days))) - self.stdout.write(' Most recent at {} ({} ago)\n'.format(newest_date, _format_count(newest_ago.days))) - self.stdout.flush() - - if not dry_run: - old_events.delete() diff --git a/ietf/person/management/commands/tests.py b/ietf/person/management/commands/tests.py deleted file mode 100644 index 38d770a58..000000000 --- a/ietf/person/management/commands/tests.py +++ /dev/null @@ -1,122 +0,0 @@ -# Copyright The IETF Trust 2021, All Rights Reserved -# -*- coding: utf-8 -*- - -import datetime -from io import StringIO - -from django.core.management import call_command, CommandError -from django.utils import timezone - -from ietf.person.factories import PersonApiKeyEventFactory -from ietf.person.models import PersonApiKeyEvent, PersonEvent -from ietf.utils.test_utils import TestCase - - -class CommandTests(TestCase): - @staticmethod - def _call_command(command_name, *args, **options): - out = StringIO() - options['stdout'] = out - call_command(command_name, *args, **options) - return out.getvalue() - - def _assert_purge_results(self, cmd_output, expected_delete_count, expected_kept_events): - self.assertNotIn('Dry run requested', cmd_output) - if expected_delete_count == 0: - delete_text = 'No events older than' - else: - delete_text = 'Deleting {} event'.format(expected_delete_count) - self.assertIn(delete_text, cmd_output) - self.assertCountEqual( - PersonApiKeyEvent.objects.all(), - expected_kept_events, - 'Wrong events were deleted' - ) - - def _assert_purge_dry_run_results(self, cmd_output, expected_delete_count, expected_kept_events): - self.assertIn('Dry run requested', cmd_output) - if expected_delete_count == 0: - delete_text = 'No events older than' - else: - delete_text = 'Would delete {} event'.format(expected_delete_count) - self.assertIn(delete_text, cmd_output) - self.assertCountEqual( - PersonApiKeyEvent.objects.all(), - expected_kept_events, - 'Events were deleted when dry-run option was used' - ) - - def test_purge_old_personal_api_key_events(self): - keep_days = 10 - - # Remember how many PersonEvents were present so we can verify they're cleaned up properly. - personevents_before = PersonEvent.objects.count() - - now = timezone.now() - # The first of these events will be timestamped a fraction of a second more than keep_days - # days ago by the time we call the management command, so will just barely chosen for purge. - old_events = [ - PersonApiKeyEventFactory(time=now - datetime.timedelta(days=n)) - for n in range(keep_days, 2 * keep_days + 1) - ] - num_old_events = len(old_events) - - recent_events = [ - PersonApiKeyEventFactory(time=now - datetime.timedelta(days=n)) - for n in range(0, keep_days) - ] - # We did not create recent_event timestamped exactly keep_days ago because it would - # be treated as an old_event by the management command. Create an event a few seconds - # on the "recent" side of keep_days old to test the threshold. - recent_events.append( - PersonApiKeyEventFactory( - time=now + datetime.timedelta(seconds=3) - datetime.timedelta(days=keep_days) - ) - ) - num_recent_events = len(recent_events) - - # call with dry run - output = self._call_command('purge_old_personal_api_key_events', str(keep_days), '--dry-run', '-v2') - self._assert_purge_dry_run_results(output, num_old_events, old_events + recent_events) - - # call for real - output = self._call_command('purge_old_personal_api_key_events', str(keep_days), '-v2') - self._assert_purge_results(output, num_old_events, recent_events) - self.assertEqual(PersonEvent.objects.count(), personevents_before + num_recent_events, - 'PersonEvents were not cleaned up properly') - - # repeat - there should be nothing left to delete - output = self._call_command('purge_old_personal_api_key_events', '--dry-run', str(keep_days), '-v2') - self._assert_purge_dry_run_results(output, 0, recent_events) - - output = self._call_command('purge_old_personal_api_key_events', str(keep_days), '-v2') - self._assert_purge_results(output, 0, recent_events) - self.assertEqual(PersonEvent.objects.count(), personevents_before + num_recent_events, - 'PersonEvents were not cleaned up properly') - - # and now delete the remaining events - output = self._call_command('purge_old_personal_api_key_events', '0', '-v2') - self._assert_purge_results(output, num_recent_events, []) - self.assertEqual(PersonEvent.objects.count(), personevents_before, - 'PersonEvents were not cleaned up properly') - - def test_purge_old_personal_api_key_events_rejects_invalid_arguments(self): - """The purge_old_personal_api_key_events command should reject invalid arguments""" - event = PersonApiKeyEventFactory(time=timezone.now() - datetime.timedelta(days=30)) - - with self.assertRaises(CommandError): - self._call_command('purge_old_personal_api_key_events') - - with self.assertRaises(CommandError): - self._call_command('purge_old_personal_api_key_events', '-15') - - with self.assertRaises(CommandError): - self._call_command('purge_old_personal_api_key_events', '15.3') - - with self.assertRaises(CommandError): - self._call_command('purge_old_personal_api_key_events', '15', '15') - - with self.assertRaises(CommandError): - self._call_command('purge_old_personal_api_key_events', 'abc', '15') - - self.assertCountEqual(PersonApiKeyEvent.objects.all(), [event]) diff --git a/ietf/person/tasks.py b/ietf/person/tasks.py new file mode 100644 index 000000000..221b718f2 --- /dev/null +++ b/ietf/person/tasks.py @@ -0,0 +1,20 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +# Celery task definitions +# +import datetime + +from celery import shared_task +from django.utils import timezone + +from ietf.utils import log +from .models import PersonApiKeyEvent + + +@shared_task +def purge_personal_api_key_events_task(keep_days): + keep_since = timezone.now() - datetime.timedelta(days=keep_days) + old_events = PersonApiKeyEvent.objects.filter(time__lt=keep_since) + count = len(old_events) + old_events.delete() + log.log(f"Deleted {count} PersonApiKeyEvents older than {keep_since}") diff --git a/ietf/person/tests.py b/ietf/person/tests.py index e5bc855a2..9da201b70 100644 --- a/ietf/person/tests.py +++ b/ietf/person/tests.py @@ -4,6 +4,7 @@ import datetime import json +import mock from io import StringIO, BytesIO from PIL import Image @@ -25,8 +26,9 @@ from ietf.group.models import Group from ietf.nomcom.models import NomCom from ietf.nomcom.test_data import nomcom_test_data from ietf.nomcom.factories import NomComFactory, NomineeFactory, NominationFactory, FeedbackFactory, PositionFactory -from ietf.person.factories import EmailFactory, PersonFactory -from ietf.person.models import Person, Alias +from ietf.person.factories import EmailFactory, PersonFactory, PersonApiKeyEventFactory +from ietf.person.models import Person, Alias, PersonApiKeyEvent +from ietf.person.tasks import purge_personal_api_key_events_task from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification, handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees, handle_reviewer_settings, get_dots) @@ -450,3 +452,16 @@ class PersonUtilsTests(TestCase): self.assertEqual(get_dots(ncmember),['nomcom']) ncchair = RoleFactory(group__acronym='nomcom2020',group__type_id='nomcom',name_id='chair').person self.assertEqual(get_dots(ncchair),['nomcom']) + + +class TaskTests(TestCase): + @mock.patch("ietf.person.tasks.log.log") + def test_purge_personal_api_key_events_task(self, mock_log): + now = timezone.now() + old_event = PersonApiKeyEventFactory(time=now - datetime.timedelta(days=1, minutes=1)) + young_event = PersonApiKeyEventFactory(time=now - datetime.timedelta(days=1, minutes=-1)) + purge_personal_api_key_events_task(keep_days=1) + self.assertFalse(PersonApiKeyEvent.objects.filter(pk=old_event.pk).exists()) + self.assertTrue(PersonApiKeyEvent.objects.filter(pk=young_event.pk).exists()) + self.assertTrue(mock_log.called) + self.assertIn("Deleted 1", mock_log.call_args[0][0]) diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index 792eb0068..5595bbcbf 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -241,6 +241,17 @@ class Command(BaseCommand): ), ) + PeriodicTask.objects.get_or_create( + name="Purge old personal API key events", + task="ietf.person.tasks.purge_personal_api_key_events_task", + kwargs=json.dumps(dict(keep_days=14)), + defaults=dict( + enabled=False, + crontab=self.crontabs["daily"], + description="Purge PersonApiKeyEvent instances older than 14 days", + ), + ) + def show_tasks(self): for label, crontab in self.crontabs.items(): tasks = PeriodicTask.objects.filter(crontab=crontab).order_by( From 2ccc230ce7feaa448f39e7ab1c8d161a0aeeca23 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 30 May 2024 10:31:25 -0300 Subject: [PATCH 12/15] feat: send_apikey_usage_emails_task() (#7486) * feat: send_apikey_usage_emails_task * chore: update test to use task instead of cmd * chore: add PeriodicTask * chore: remove old command + empty management dir * chore: remove now-empty bin/weekly * refactor: only consider keys that might have events --------- Co-authored-by: Robert Sparks --- bin/weekly | 22 -------- ietf/ietfauth/management/__init__.py | 0 ietf/ietfauth/management/commands/__init__.py | 0 .../commands/send_apikey_usage_emails.py | 52 ------------------- ietf/ietfauth/tests.py | 9 ++-- ietf/person/tasks.py | 41 ++++++++++++++- .../management/commands/periodic_tasks.py | 11 ++++ 7 files changed, 54 insertions(+), 81 deletions(-) delete mode 100755 bin/weekly delete mode 100644 ietf/ietfauth/management/__init__.py delete mode 100644 ietf/ietfauth/management/commands/__init__.py delete mode 100644 ietf/ietfauth/management/commands/send_apikey_usage_emails.py diff --git a/bin/weekly b/bin/weekly deleted file mode 100755 index 8e01c273c..000000000 --- a/bin/weekly +++ /dev/null @@ -1,22 +0,0 @@ -#!/bin/bash - -# Weekly datatracker jobs. -# -# This script is expected to be triggered by cron from -# /etc/cron.d/datatracker -export LANG=en_US.UTF-8 -export PYTHONIOENCODING=utf-8 - -DTDIR=/a/www/ietf-datatracker/web -cd $DTDIR/ - -# Set up the virtual environment -source $DTDIR/env/bin/activate - -logger -p user.info -t cron "Running $DTDIR/bin/weekly" - - -# Send out weekly summaries of apikey usage - -$DTDIR/ietf/manage.py send_apikey_usage_emails - diff --git a/ietf/ietfauth/management/__init__.py b/ietf/ietfauth/management/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/ietf/ietfauth/management/commands/__init__.py b/ietf/ietfauth/management/commands/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/ietf/ietfauth/management/commands/send_apikey_usage_emails.py b/ietf/ietfauth/management/commands/send_apikey_usage_emails.py deleted file mode 100644 index d3fce1bcc..000000000 --- a/ietf/ietfauth/management/commands/send_apikey_usage_emails.py +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright The IETF Trust 2017-2020, All Rights Reserved -# -*- coding: utf-8 -*- - - -import datetime - -from textwrap import dedent - -from django.conf import settings -from django.core.management.base import BaseCommand -from django.utils import timezone - -import debug # pyflakes:ignore - -from ietf.person.models import PersonalApiKey, PersonApiKeyEvent -from ietf.utils.mail import send_mail - - -class Command(BaseCommand): - """ - Send out emails to all persons who have personal API keys about usage. - - Usage is show over the given period, where the default period is 7 days. - """ - - help = dedent(__doc__).strip() - - def add_arguments(self, parser): - parser.add_argument('-d', '--days', dest='days', type=int, default=7, - help='The period over which to show usage.') - - def handle(self, *filenames, **options): - """ - """ - - self.verbosity = int(options.get('verbosity')) - days = options.get('days') - - keys = PersonalApiKey.objects.filter(valid=True) - for key in keys: - earliest = timezone.now() - datetime.timedelta(days=days) - events = PersonApiKeyEvent.objects.filter(key=key, time__gt=earliest) - count = events.count() - events = events[:32] - if count: - key_name = key.hash()[:8] - subject = "API key usage for key '%s' for the last %s days" %(key_name, days) - to = key.person.email_address() - frm = settings.DEFAULT_FROM_EMAIL - send_mail(None, to, frm, subject, 'utils/apikey_usage_report.txt', {'person':key.person, - 'days':days, 'key':key, 'key_name':key_name, 'count':count, 'events':events, } ) - diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 56b4638f6..503c091a8 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -41,6 +41,7 @@ from ietf.meeting.factories import MeetingFactory from ietf.nomcom.factories import NomComFactory from ietf.person.factories import PersonFactory, EmailFactory, UserFactory, PersonalApiKeyFactory from ietf.person.models import Person, Email, PersonalApiKey +from ietf.person.tasks import send_apikey_usage_emails_task from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory from ietf.review.models import ReviewWish, UnavailablePeriod from ietf.stats.models import MeetingRegistration @@ -853,9 +854,6 @@ class IetfAuthTests(TestCase): key2.delete() def test_send_apikey_report(self): - from ietf.ietfauth.management.commands.send_apikey_usage_emails import Command - from ietf.utils.mail import outbox, empty_outbox - person = RoleFactory(name_id='secr', group__acronym='secretariat').person url = urlreverse('ietf.ietfauth.views.apikey_create') @@ -880,9 +878,8 @@ class IetfAuthTests(TestCase): date = str(date_today()) empty_outbox() - cmd = Command() - cmd.handle(verbosity=0, days=7) - + send_apikey_usage_emails_task(days=7) + self.assertEqual(len(outbox), len(endpoints)) for mail in outbox: body = get_payload_text(mail) diff --git a/ietf/person/tasks.py b/ietf/person/tasks.py index 221b718f2..f0c979fa2 100644 --- a/ietf/person/tasks.py +++ b/ietf/person/tasks.py @@ -5,12 +5,51 @@ import datetime from celery import shared_task + +from django.conf import settings from django.utils import timezone from ietf.utils import log -from .models import PersonApiKeyEvent +from ietf.utils.mail import send_mail +from .models import PersonalApiKey, PersonApiKeyEvent +@shared_task +def send_apikey_usage_emails_task(days): + """Send usage emails to Persons who have API keys""" + earliest = timezone.now() - datetime.timedelta(days=days) + keys = PersonalApiKey.objects.filter( + valid=True, + personapikeyevent__time__gt=earliest, + ).distinct() + for key in keys: + events = PersonApiKeyEvent.objects.filter(key=key, time__gt=earliest) + count = events.count() + events = events[:32] + if count: + key_name = key.hash()[:8] + subject = "API key usage for key '%s' for the last %s days" % ( + key_name, + days, + ) + to = key.person.email_address() + frm = settings.DEFAULT_FROM_EMAIL + send_mail( + None, + to, + frm, + subject, + "utils/apikey_usage_report.txt", + { + "person": key.person, + "days": days, + "key": key, + "key_name": key_name, + "count": count, + "events": events, + }, + ) + @shared_task def purge_personal_api_key_events_task(keep_days): keep_since = timezone.now() - datetime.timedelta(days=keep_days) diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index 5595bbcbf..7f0c988dc 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -241,6 +241,17 @@ class Command(BaseCommand): ), ) + PeriodicTask.objects.get_or_create( + name="Send personal API key usage emails", + task="ietf.person.tasks.send_apikey_usage_emails_task", + kwargs=json.dumps(dict(days=7)), + defaults=dict( + enabled=False, + crontab=self.crontabs["weekly"], + description="Send personal API key usage summary emails for the past week", + ), + ) + PeriodicTask.objects.get_or_create( name="Purge old personal API key events", task="ietf.person.tasks.purge_personal_api_key_events_task", From 99b852805ba6f8ef9dc728f9f0cf4a8be75258a1 Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Fri, 31 May 2024 08:14:52 -0700 Subject: [PATCH 13/15] fix: handle registration is_nomcom_volunteer = false correctly (#7484) Co-authored-by: Robert Sparks --- ietf/api/tests.py | 16 +++++++++++----- ietf/api/views.py | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ietf/api/tests.py b/ietf/api/tests.py index c4e627c52..66bc7a3be 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -30,7 +30,7 @@ from ietf.doc.factories import IndividualDraftFactory, WgDraftFactory, WgRfcFact from ietf.group.factories import RoleFactory from ietf.meeting.factories import MeetingFactory, SessionFactory from ietf.meeting.models import Session -from ietf.nomcom.models import Volunteer, NomCom +from ietf.nomcom.models import Volunteer from ietf.nomcom.factories import NomComFactory, nomcom_kwargs_for_year from ietf.person.factories import PersonFactory, random_faker, EmailFactory from ietf.person.models import Email, User @@ -828,7 +828,7 @@ class CustomApiTests(TestCase): 'reg_type': 'onsite', 'ticket_type': '', 'checkedin': 'False', - 'is_nomcom_volunteer': 'True', + 'is_nomcom_volunteer': 'False', } person = PersonFactory() reg['email'] = person.email().address @@ -842,16 +842,22 @@ class CustomApiTests(TestCase): # create appropriate group and nomcom objects nomcom = NomComFactory.create(is_accepting_volunteers=True, **nomcom_kwargs_for_year(year)) url = urlreverse('ietf.api.views.api_new_meeting_registration') - r = self.client.post(url, reg) - self.assertContains(r, 'Invalid apikey', status_code=403) oidcp = PersonFactory(user__is_staff=True) # Make sure 'oidcp' has an acceptable role RoleFactory(name_id='robot', person=oidcp, email=oidcp.email(), group__acronym='secretariat') key = PersonalApiKey.objects.create(person=oidcp, endpoint=url) reg['apikey'] = key.hash() + + # first test is_nomcom_volunteer False r = self.client.post(url, reg) - nomcom = NomCom.objects.last() self.assertContains(r, "Accepted, New registration", status_code=202) + # assert no Volunteers exists + self.assertEqual(Volunteer.objects.count(), 0) + + # test is_nomcom_volunteer True + reg['is_nomcom_volunteer'] = 'True' + r = self.client.post(url, reg) + self.assertContains(r, "Accepted, Updated registration", status_code=202) # assert Volunteer exists self.assertEqual(Volunteer.objects.count(), 1) volunteer = Volunteer.objects.last() diff --git a/ietf/api/views.py b/ietf/api/views.py index 6f97efbdb..3c3ea0d5a 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -212,7 +212,7 @@ def api_new_meeting_registration(request): response += ", Email sent" # handle nomcom volunteer - if data['is_nomcom_volunteer'] and object.person: + if request.POST.get('is_nomcom_volunteer', 'false').lower() == 'true' and object.person: try: nomcom = NomCom.objects.get(is_accepting_volunteers=True) except (NomCom.DoesNotExist, NomCom.MultipleObjectsReturned): From ac3813f1af7323f56cb111d6a5db15529079e1fa Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Tue, 4 Jun 2024 12:38:54 -0500 Subject: [PATCH 14/15] fix: improve warnings on ballot issue view. Fixes #7490. (#7491) --- ietf/doc/tests_ballot.py | 35 ++++++++++++++++++++- ietf/doc/views_ballot.py | 3 +- ietf/templates/doc/ballot/writeupnotes.html | 7 ++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/ietf/doc/tests_ballot.py b/ietf/doc/tests_ballot.py index 9c9287dab..e18b2abfd 100644 --- a/ietf/doc/tests_ballot.py +++ b/ietf/doc/tests_ballot.py @@ -32,7 +32,7 @@ from ietf.person.utils import get_active_ads from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.text import unwrap -from ietf.utils.timezone import date_today +from ietf.utils.timezone import date_today, datetime_today class EditPositionTests(TestCase): @@ -529,6 +529,7 @@ class BallotWriteupsTests(TestCase): login_testing_unauthorized(self, "secretary", url) # expect warning about issuing a ballot before IETF Last Call is done + # No last call has yet been issued r = self.client.get(url) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) @@ -536,6 +537,38 @@ class BallotWriteupsTests(TestCase): self.assertTrue(q('[class=text-danger]:contains("not completed IETF Last Call")')) self.assertTrue(q('[type=submit]:contains("Save")')) + # Last call exists but hasn't expired + LastCallDocEvent.objects.create( + doc=draft, + expires=datetime_today()+datetime.timedelta(days=14), + by=Person.objects.get(name="(System)") + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(q('[class=text-danger]:contains("not completed IETF Last Call")')) + + # Last call exists and has expired + LastCallDocEvent.objects.filter(doc=draft).update(expires=datetime_today()-datetime.timedelta(days=2)) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertFalse(q('[class=text-danger]:contains("not completed IETF Last Call")')) + + for state_slug in ["lc", "watching", "ad-eval"]: + draft.set_state(State.objects.get(type="draft-iesg",slug=state_slug)) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(q('[class=text-danger]:contains("It would be unexpected to issue a ballot while in this state.")')) + + draft.set_state(State.objects.get(type="draft-iesg",slug="writeupw")) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertFalse(q('[class=text-danger]:contains("It would be unexpected to issue a ballot while in this state.")')) + + def test_edit_approval_text(self): ad = Person.objects.get(user__username="ad") draft = WgDraftFactory(ad=ad,states=[('draft','active'),('draft-iesg','iesg-eva')],intended_std_level_id='ps',group__parent=Group.objects.get(acronym='farfut')) diff --git a/ietf/doc/views_ballot.py b/ietf/doc/views_ballot.py index ff5192156..02b55249d 100644 --- a/ietf/doc/views_ballot.py +++ b/ietf/doc/views_ballot.py @@ -687,7 +687,8 @@ def ballot_writeupnotes(request, name): dict(doc=doc, back_url=doc.get_absolute_url(), ballot_issued=bool(doc.latest_event(type="sent_ballot_announcement")), - ballot_issue_danger=bool(prev_state.slug in ['ad-eval', 'lc']), + warn_lc = not doc.docevent_set.filter(lastcalldocevent__expires__date__lt=date_today(DEADLINE_TZINFO)).exists(), + warn_unexpected_state= prev_state if bool(prev_state.slug in ['watching', 'ad-eval', 'lc']) else None, ballot_writeup_form=form, need_intended_status=need_intended_status, )) diff --git a/ietf/templates/doc/ballot/writeupnotes.html b/ietf/templates/doc/ballot/writeupnotes.html index 925387d28..8e985c15c 100644 --- a/ietf/templates/doc/ballot/writeupnotes.html +++ b/ietf/templates/doc/ballot/writeupnotes.html @@ -15,11 +15,16 @@ {% bootstrap_form ballot_writeup_form %}
Technical summary, Working Group summary, document quality, personnel, IANA note. This text will be appended to all announcements and messages to the IRTF or RFC Editor. - {% if ballot_issue_danger %} + {% if warn_lc %}

This document has not completed IETF Last Call. Please do not issue the ballot early without good reason.

{% endif %} + {% if warn_unexpected_state %} +

+ This document is in an IESG state of "{{warn_unexpected_state}}". It would be unexpected to issue a ballot while in this state. +

+ {% endif %}