From e7512686e7442cb112eb7985756480b13107a36d Mon Sep 17 00:00:00 2001 From: Ragnhild Bodsberg Date: Tue, 21 Jun 2022 10:28:39 +0200 Subject: [PATCH 1/9] Fix validation on input fields of maintenance calendar --- python/nav/web/maintenance/forms.py | 10 +++++++++- python/nav/web/maintenance/views.py | 7 +++++-- python/nav/web/templates/maintenance/calendar.html | 7 +++---- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/python/nav/web/maintenance/forms.py b/python/nav/web/maintenance/forms.py index 6b041165d3..3854140355 100644 --- a/python/nav/web/maintenance/forms.py +++ b/python/nav/web/maintenance/forms.py @@ -13,11 +13,12 @@ # details. You should have received a copy of the GNU General Public License # along with NAV. If not, see . # +import datetime from django import forms from crispy_forms.helper import FormHelper from crispy_forms_foundation.layout import Layout, Row, Column, Field - +from django.forms import IntegerField from nav.models.fields import INFINITY @@ -72,3 +73,10 @@ class MaintenanceAddSingleNetbox(forms.Form): a custom variable-checker""" netboxid = forms.IntegerField(required=True) + + +class MaintenanceCalendarForm(forms.Form): + """A form used for displaying a maintenance calendar""" + + month = IntegerField(initial=None, required=True, min_value=1, max_value=12) + year = IntegerField(initial=None, required=True, min_value=2000, max_value=2100) diff --git a/python/nav/web/maintenance/views.py b/python/nav/web/maintenance/views.py index 1b4a02b3c6..01686416ff 100644 --- a/python/nav/web/maintenance/views.py +++ b/python/nav/web/maintenance/views.py @@ -38,7 +38,7 @@ from nav.web.maintenance.utils import structure_component_data from nav.web.maintenance.utils import task_form_initial, infodict_by_state from nav.web.maintenance.utils import MaintenanceCalendar, NAVPATH, TITLE -from nav.web.maintenance.forms import MaintenanceTaskForm +from nav.web.maintenance.forms import MaintenanceTaskForm, MaintenanceCalendarForm from nav.web.maintenance.forms import MaintenanceAddSingleNetbox import nav.maintengine @@ -54,6 +54,9 @@ def redirect_to_calendar(_request): def calendar(request, year=None, month=None): # If the form was used to get here, redirect to the appropriate page + form = MaintenanceCalendarForm() + heading = "Maintenance schedule" + if "year" in request.GET and "month" in request.GET: return redirect( "maintenance-calendar", @@ -61,7 +64,6 @@ def calendar(request, year=None, month=None): month=request.GET.get("month"), ) - heading = "Maintenance schedule" try: year = int(year) month = int(month) @@ -98,6 +100,7 @@ def calendar(request, year=None, month=None): 'maintenance/calendar.html', { 'active': {'calendar': True}, + 'calendarform': form, 'navpath': NAVPATH, 'title': TITLE, 'heading': heading, diff --git a/python/nav/web/templates/maintenance/calendar.html b/python/nav/web/templates/maintenance/calendar.html index 713a27fc0e..005a5eeb2a 100644 --- a/python/nav/web/templates/maintenance/calendar.html +++ b/python/nav/web/templates/maintenance/calendar.html @@ -11,8 +11,8 @@
- - + +
@@ -30,5 +30,4 @@ {{ calendar }} -{% endblock %} - +{% endblock %} \ No newline at end of file From 1a23d4b7e84fcc5e387091ed4ae426d5b1c13071 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 19 Jul 2022 15:44:14 +0200 Subject: [PATCH 2/9] Implement date calc in MaintenanceCalendarForm Let the form do all the heavy lifting of validating input and do date calculations on it. --- python/nav/web/maintenance/forms.py | 60 +++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/python/nav/web/maintenance/forms.py b/python/nav/web/maintenance/forms.py index 3854140355..69b5d68ec4 100644 --- a/python/nav/web/maintenance/forms.py +++ b/python/nav/web/maintenance/forms.py @@ -13,12 +13,11 @@ # details. You should have received a copy of the GNU General Public License # along with NAV. If not, see . # -import datetime +from datetime import date from django import forms from crispy_forms.helper import FormHelper from crispy_forms_foundation.layout import Layout, Row, Column, Field -from django.forms import IntegerField from nav.models.fields import INFINITY @@ -75,8 +74,61 @@ class MaintenanceAddSingleNetbox(forms.Form): netboxid = forms.IntegerField(required=True) +def _get_current_year(): + return date.today().year + + +def _get_current_month(): + return date.today().month + + class MaintenanceCalendarForm(forms.Form): """A form used for displaying a maintenance calendar""" - month = IntegerField(initial=None, required=True, min_value=1, max_value=12) - year = IntegerField(initial=None, required=True, min_value=2000, max_value=2100) + year = forms.IntegerField( + initial=_get_current_year, required=True, min_value=2000, max_value=2100 + ) + month = forms.IntegerField( + initial=_get_current_month, required=True, min_value=1, max_value=12 + ) + + @property + def cleaned_year(self): + """Returns the cleaned year value if valid, current year otherwise""" + return self.cleaned_data['year'] if self.is_valid() else _get_current_year() + + @property + def cleaned_month(self): + """Returns the cleaned month is valid, current month otherwise""" + return self.cleaned_data['month'] if self.is_valid() else _get_current_month() + + @property + def this_month_start(self): + """Returns the first date of the month represented by this form instance""" + return date(self.cleaned_year, self.cleaned_month, 1) + + @property + def next_month_start(self): + """Returns the first date of the month after the one represented by this form + instance + """ + year = self.cleaned_year + month = self.cleaned_month + 1 + + if month > 12: + year += 1 + month = 1 + return date(year, month, 1) + + @property + def previous_month_start(self): + """Returns the first date of the month before the one represented by this form + instance + """ + year = self.cleaned_year + month = self.cleaned_month - 1 + + if month < 1: + year -= 1 + month = 12 + return date(year, month, 1) From aa83c6be80a10c7cda4a191b8f7ce7dc4544f6e2 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 19 Jul 2022 15:45:47 +0200 Subject: [PATCH 3/9] Delegate date logic to MaintenaceCalendarForm This makes the view function simpler, by delegating all input validation and date calculation logic to a MaintenanceCalendarForm instance. --- python/nav/web/maintenance/views.py | 54 ++++++++++------------------- 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/python/nav/web/maintenance/views.py b/python/nav/web/maintenance/views.py index 01686416ff..01d74adb8f 100644 --- a/python/nav/web/maintenance/views.py +++ b/python/nav/web/maintenance/views.py @@ -54,47 +54,29 @@ def redirect_to_calendar(_request): def calendar(request, year=None, month=None): # If the form was used to get here, redirect to the appropriate page - form = MaintenanceCalendarForm() - heading = "Maintenance schedule" - if "year" in request.GET and "month" in request.GET: - return redirect( - "maintenance-calendar", - year=request.GET.get("year"), - month=request.GET.get("month"), + form = MaintenanceCalendarForm(data=request.GET.dict()) + if form.is_valid(): + return redirect( + "maintenance-calendar", + year=request.GET.get("year"), + month=request.GET.get("month"), + ) + elif year and month: + form = MaintenanceCalendarForm( + data={'year': year, 'month': month}, ) + else: + form = MaintenanceCalendarForm() - try: - year = int(year) - month = int(month) - this_month_start = date(year, month, 1) - except (TypeError, ValueError): - year = date.today().year - month = date.today().month - this_month_start = date(year, month, 1) - - next_month = month + 1 - next_year = year - if next_month > 12: - next_year = year + 1 - next_month = 1 - - prev_month = month - 1 - prev_year = year - if prev_month < 1: - prev_year = year - 1 - prev_month = 12 - - prev_month_start = date(prev_year, prev_month, 1) - next_month_start = date(next_year, next_month, 1) tasks = ( MaintenanceTask.objects.filter( - start_time__lt=next_month_start, end_time__gt=this_month_start + start_time__lt=form.next_month_start, end_time__gt=form.this_month_start ) .exclude(state=MaintenanceTask.STATE_CANCELED) .order_by('start_time') ) - cal = MaintenanceCalendar(tasks).formatmonth(year, month) + cal = MaintenanceCalendar(tasks).formatmonth(form.cleaned_year, form.cleaned_month) return render( request, 'maintenance/calendar.html', @@ -103,11 +85,11 @@ def calendar(request, year=None, month=None): 'calendarform': form, 'navpath': NAVPATH, 'title': TITLE, - 'heading': heading, + 'heading': "Maintenance schedule", 'calendar': mark_safe(cal), - 'prev_month': prev_month_start, - 'this_month': this_month_start, - 'next_month': next_month_start, + 'prev_month': form.previous_month_start, + 'this_month': form.this_month_start, + 'next_month': form.next_month_start, 'curr_month': datetime.today(), }, ) From db0f1e5f4218b2c530dc86551cf95a114c333936 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 19 Jul 2022 15:47:15 +0200 Subject: [PATCH 4/9] Get field values/limits from form definition According to the DRY principle. --- python/nav/web/templates/maintenance/calendar.html | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/python/nav/web/templates/maintenance/calendar.html b/python/nav/web/templates/maintenance/calendar.html index 005a5eeb2a..ca58d807ff 100644 --- a/python/nav/web/templates/maintenance/calendar.html +++ b/python/nav/web/templates/maintenance/calendar.html @@ -11,8 +11,18 @@
- - + +
From a01322a5794bd61d9c3630a1e227421f6363eb1e Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 19 Jul 2022 15:48:02 +0200 Subject: [PATCH 5/9] Display form errors instead of calendar If the date form has errors (which shouldn't happen unless you mess with URL manually), display those validation errors rather than a calendar widget. --- python/nav/web/templates/maintenance/calendar.html | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/python/nav/web/templates/maintenance/calendar.html b/python/nav/web/templates/maintenance/calendar.html index ca58d807ff..a1f51f289b 100644 --- a/python/nav/web/templates/maintenance/calendar.html +++ b/python/nav/web/templates/maintenance/calendar.html @@ -36,8 +36,11 @@ - - {{ calendar }} + {% if calendarform.errors %} + {{ calendarform.errors }} + {% else %} + {{ calendar }} + {% endif %} {% endblock %} \ No newline at end of file From e8bc52000d1f8e5f9cb8441a15898b730a052171 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Thu, 21 Jul 2022 09:29:54 +0200 Subject: [PATCH 6/9] Update changelog with PR details --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 291521624c..72c22dfb7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ found in the [HISTORY](HISTORY) file. ## Fixed - Metric values of 0.0 are evaluated correctly by threshold rules [#2447](https://github.com/Uninett/nav/issues/2447) +- Validate maintenance calendar input form to avoid e-mail spam from bots scanning for vulnerabilities [#2420](https://github.com/Uninett/nav/issues/2420) ## [5.4.0] - 2022-05-19 From 01bb10a5beb71c8857c60f91de7475a9467f3550 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Thu, 21 Jul 2022 10:23:37 +0200 Subject: [PATCH 7/9] Unit test MaintenanceCalendarForm --- tests/unittests/web/maintenance/__init__.py | 0 tests/unittests/web/maintenance/forms_test.py | 64 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/unittests/web/maintenance/__init__.py create mode 100644 tests/unittests/web/maintenance/forms_test.py diff --git a/tests/unittests/web/maintenance/__init__.py b/tests/unittests/web/maintenance/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unittests/web/maintenance/forms_test.py b/tests/unittests/web/maintenance/forms_test.py new file mode 100644 index 0000000000..9910562d2b --- /dev/null +++ b/tests/unittests/web/maintenance/forms_test.py @@ -0,0 +1,64 @@ +# +# Copyright (C) 2022 Sikt +# +# This file is part of Network Administration Visualized (NAV). +# +# NAV is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License version 3 as published by +# the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. You should have received a copy of the GNU General Public +# License along with NAV. If not, see . +# +from datetime import date + +import pytest + +from nav.web.maintenance.forms import MaintenanceCalendarForm + + +class TestMaintenanceCalendarForm: + def test_cleaned_year_returns_integer(self, valid_form): + assert valid_form.cleaned_year == 2022 + + def test_cleaned_month_returns_integer(self, valid_form): + assert valid_form.cleaned_month == 7 + + def test_this_month_start_returns_the_first_day_of_the_month(self, valid_form): + assert valid_form.this_month_start == date(2022, 7, 1) + + def test_next_month_start_returns_the_first_day_of_the_next_month(self, valid_form): + assert valid_form.next_month_start == date(2022, 8, 1) + + def test_previous_month_start_returns_the_first_day_of_the_previous_month( + self, valid_form + ): + assert valid_form.previous_month_start == date(2022, 6, 1) + + def test_when_month_is_december_next_month_start_returns_january( + self, december_form + ): + assert december_form.next_month_start == date(2023, 1, 1) + + def test_when_month_is_january_previous_month_start_returns_december( + self, january_form + ): + assert january_form.previous_month_start == date(2021, 12, 1) + + +@pytest.fixture +def valid_form() -> MaintenanceCalendarForm: + return MaintenanceCalendarForm(data={'year': '2022', 'month': '7'}) + + +@pytest.fixture +def december_form() -> MaintenanceCalendarForm: + return MaintenanceCalendarForm(data={'year': '2022', 'month': '12'}) + + +@pytest.fixture +def january_form() -> MaintenanceCalendarForm: + return MaintenanceCalendarForm(data={'year': '2022', 'month': '1'}) From 7a0ed1b58de0fd5501f9e8af873f5fdda0fdc990 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Thu, 21 Jul 2022 10:24:42 +0200 Subject: [PATCH 8/9] Integration test the maintenance calendar view Just test the simple stuff to increase coverage: Does it avoid crashing both with no arguments and with invalid arguments? --- tests/integration/web/maintenance/__init__.py | 0 .../integration/web/maintenance/views_test.py | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/integration/web/maintenance/__init__.py create mode 100644 tests/integration/web/maintenance/views_test.py diff --git a/tests/integration/web/maintenance/__init__.py b/tests/integration/web/maintenance/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/web/maintenance/views_test.py b/tests/integration/web/maintenance/views_test.py new file mode 100644 index 0000000000..78b1dc91ea --- /dev/null +++ b/tests/integration/web/maintenance/views_test.py @@ -0,0 +1,25 @@ +# +# Copyright (C) 2022 Sikt +# +# This file is part of Network Administration Visualized (NAV). +# +# NAV is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License version 3 as published by +# the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. You should have received a copy of the GNU General Public +# License along with NAV. If not, see . +# + + +class TestMaintenanceCalendarView: + def test_calendar_renders_when_no_arguments_given(self, client): + response = client.get('/maintenance/', follow=True) + assert response.status_code == 200 + + def test_calendar_still_renders_when_invalid_arguments_given(self, client): + response = client.get('/maintenance/?year=invalid&month=invalid', follow=True) + assert response.status_code == 200 From eac08e74dfe5c1605980d563ab1718e9d99c7f18 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Fri, 28 Oct 2022 11:21:34 +0200 Subject: [PATCH 9/9] Fix typo --- python/nav/web/maintenance/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/nav/web/maintenance/forms.py b/python/nav/web/maintenance/forms.py index 69b5d68ec4..2ef1961ad9 100644 --- a/python/nav/web/maintenance/forms.py +++ b/python/nav/web/maintenance/forms.py @@ -99,7 +99,7 @@ def cleaned_year(self): @property def cleaned_month(self): - """Returns the cleaned month is valid, current month otherwise""" + """Returns the cleaned month if valid, current month otherwise""" return self.cleaned_data['month'] if self.is_valid() else _get_current_month() @property