From 394e32e022aa99e07c93823f7ad53679815a6124 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 22 Nov 2024 10:13:53 -0400 Subject: [PATCH 01/12] fix: lock session requests for new meetings (#8251) * fix: lock session requests for new meetings * fix: lint --- ietf/secr/meetings/tests.py | 4 ++++ ietf/secr/meetings/views.py | 7 +++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ietf/secr/meetings/tests.py b/ietf/secr/meetings/tests.py index a241d2b5eb..08c792ce1e 100644 --- a/ietf/secr/meetings/tests.py +++ b/ietf/secr/meetings/tests.py @@ -82,6 +82,10 @@ def test_add_meeting(self): [cn.slug for cn in new_meeting.group_conflict_types.all()], post_data['group_conflict_types'], ) + self.assertEqual( + new_meeting.session_request_lock_message, + "Session requests for this meeting have not yet opened.", + ) def test_add_meeting_default_conflict_types(self): """Add meeting should default to same conflict types as previous meeting""" diff --git a/ietf/secr/meetings/views.py b/ietf/secr/meetings/views.py index 8afcf5a11e..47f7b7ffa5 100644 --- a/ietf/secr/meetings/views.py +++ b/ietf/secr/meetings/views.py @@ -17,7 +17,7 @@ from ietf.ietfauth.utils import role_required from ietf.utils.mail import send_mail from ietf.meeting.forms import duration_string -from ietf.meeting.helpers import get_meeting, make_materials_directories, populate_important_dates +from ietf.meeting.helpers import make_materials_directories, populate_important_dates from ietf.meeting.models import Meeting, Session, Room, TimeSlot, SchedTimeSessAssignment, Schedule, SchedulingEvent from ietf.meeting.utils import add_event_info_to_session_qs from ietf.name.models import SessionStatusName @@ -223,9 +223,8 @@ def add(request): ) meeting.schedule = schedule - # we want to carry session request lock status over from previous meeting - previous_meeting = get_meeting( int(meeting.number) - 1 ) - meeting.session_request_lock_message = previous_meeting.session_request_lock_message + # Create meeting with session requests locked + meeting.session_request_lock_message = "Session requests for this meeting have not yet opened." meeting.save() populate_important_dates(meeting) From 622ded5d2b4db9d4bf948af0e1d0890edaa8bb33 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 22 Nov 2024 12:38:36 -0400 Subject: [PATCH 02/12] fix: ensure csrf cookie for searches (#8260) Needed on views that include search_form.html in their responses. --- ietf/doc/views_search.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 0f1937efb3..f4ad247ff0 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -56,6 +56,7 @@ from django.utils.html import strip_tags from django.utils.cache import _generate_cache_key # type: ignore from django.utils.text import slugify +from django.views.decorators.csrf import ensure_csrf_cookie from django_stubs_ext import QuerySetAny import debug # pyflakes:ignore @@ -283,6 +284,7 @@ def retrieve_search_results(form, all_types=False): return docs +@ensure_csrf_cookie def search(request): """Search for a draft""" # defaults for results / meta @@ -335,6 +337,7 @@ def search(request): ) +@ensure_csrf_cookie def frontpage(request): form = SearchForm() return render(request, 'doc/frontpage.html', {'form':form}) From 9681d9f9786772e3b83a27661444b0c4af0b54f0 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 22 Nov 2024 15:30:54 -0400 Subject: [PATCH 03/12] fix: revert POST for document search requests (#8263) * Revert "fix: ensure csrf cookie for searches (#8260)" This reverts commit 622ded5d2b4db9d4bf948af0e1d0890edaa8bb33. * Revert "refactor: eliminate single-use helper (#8226)" This reverts commit 6608c9d530b62d10c88d637f949a00fb5fea4526. * Revert "feat: POST for document search requests (#8206)" This reverts commit b65a37b6e8f6cfe105cd89c61c7046fff9d21523. * test: add back test fix * refactor: eliminate single-use helper --- ietf/api/serializer.py | 1 - ietf/doc/tests.py | 155 ++++++--------------- ietf/doc/views_search.py | 120 ++++++---------- ietf/liaisons/forms.py | 1 - ietf/liaisons/widgets.py | 1 - ietf/templates/doc/search/search_form.html | 2 - 6 files changed, 84 insertions(+), 196 deletions(-) diff --git a/ietf/api/serializer.py b/ietf/api/serializer.py index ca34ea649e..27f194c5b5 100644 --- a/ietf/api/serializer.py +++ b/ietf/api/serializer.py @@ -146,7 +146,6 @@ def end_object(self, obj): field_value = None else: field_value = field - # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if isinstance(field_value, QuerySetAny) or isinstance(field_value, list): self._current[name] = dict([ (rel.pk, self.expand_related(rel, name)) for rel in field_value ]) else: diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index abac10a5e9..f0c8e30626 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -73,163 +73,96 @@ class SearchTests(TestCase): - def test_search_handles_querystring_parameters(self): - """Search parameters via querystring should not actually search""" - url = urlreverse("ietf.doc.views_search.search") - r = self.client.get(url + "?name=some-document-name&oldDrafts=on") - # Check that we got a valid response and that the warning about query string parameters is shown. - self.assertContains( - r, - "Searching via the URL query string is no longer supported.", - status_code=200, - ) - # Check that the form was filled in correctly (not an exhaustive check, but different from the - # form defaults) - pq = PyQuery(r.content) - self.assertEqual( - pq("form#search_form input#id_name").attr("value"), - "some-document-name", - "The name field should be set in the SearchForm", - ) - self.assertEqual( - pq("form#search_form input#id_olddrafts").attr("checked"), - "checked", - "The old drafts checkbox should be selected in the SearchForm", - ) - self.assertIsNone( - pq("form#search_form input#id_rfcs").attr("checked"), - "The RFCs checkbox should not be selected in the SearchForm", - ) - self.assertIsNone( - pq("form#search_form input#id_activedrafts").attr("checked"), - "The active drafts checkbox should not be selected in the SearchForm", - ) - def test_search(self): - draft = WgDraftFactory( - name="draft-ietf-mars-test", - group=GroupFactory(acronym="mars", parent=Group.objects.get(acronym="farfut")), - authors=[PersonFactory()], - ad=PersonFactory(), - ) + + draft = WgDraftFactory(name='draft-ietf-mars-test',group=GroupFactory(acronym='mars',parent=Group.objects.get(acronym='farfut')),authors=[PersonFactory()],ad=PersonFactory()) rfc = WgRfcFactory() draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="pub-req")) - old_draft = IndividualDraftFactory( - name="draft-foo-mars-test", - authors=[PersonFactory()], - title="Optimizing Martian Network Topologies", - ) + old_draft = IndividualDraftFactory(name='draft-foo-mars-test',authors=[PersonFactory()],title="Optimizing Martian Network Topologies") old_draft.set_state(State.objects.get(used=True, type="draft", slug="expired")) - - url = urlreverse("ietf.doc.views_search.search") - + + base_url = urlreverse('ietf.doc.views_search.search') + # only show form, no search yet - r = self.client.get(url) + r = self.client.get(base_url) self.assertEqual(r.status_code, 200) - + # no match - r = self.client.post(url, {"activedrafts": "on", "name": "thisisnotadocumentname"}) + r = self.client.get(base_url + "?activedrafts=on&name=thisisnotadocumentname") self.assertEqual(r.status_code, 200) self.assertContains(r, "No documents match") - - r = self.client.post(url, {"rfcs": "on", "name": "xyzzy"}) + + r = self.client.get(base_url + "?rfcs=on&name=xyzzy") self.assertEqual(r.status_code, 200) self.assertContains(r, "No documents match") - - r = self.client.post(url, {"olddrafts": "on", "name": "bar"}) + + r = self.client.get(base_url + "?olddrafts=on&name=bar") self.assertEqual(r.status_code, 200) self.assertContains(r, "No documents match") - - r = self.client.post(url, {"olddrafts": "on", "name": "foo"}) + + r = self.client.get(base_url + "?olddrafts=on&name=foo") self.assertEqual(r.status_code, 200) self.assertContains(r, "draft-foo-mars-test") - - r = self.client.post(url, {"olddrafts": "on", "name": "FoO"}) # mixed case + + r = self.client.get(base_url + "?olddrafts=on&name=FoO") # mixed case self.assertEqual(r.status_code, 200) self.assertContains(r, "draft-foo-mars-test") - + # find by RFC - r = self.client.post(url, {"rfcs": "on", "name": rfc.name}) + r = self.client.get(base_url + "?rfcs=on&name=%s" % rfc.name) self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) - + # find by active/inactive - + draft.set_state(State.objects.get(type="draft", slug="active")) - r = self.client.post(url, {"activedrafts": "on", "name": draft.name}) + r = self.client.get(base_url + "?activedrafts=on&name=%s" % draft.name) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + draft.set_state(State.objects.get(type="draft", slug="expired")) - r = self.client.post(url, {"olddrafts": "on", "name": draft.name}) + r = self.client.get(base_url + "?olddrafts=on&name=%s" % draft.name) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + draft.set_state(State.objects.get(type="draft", slug="active")) - + # find by title - r = self.client.post(url, {"activedrafts": "on", "name": draft.title.split()[0]}) + r = self.client.get(base_url + "?activedrafts=on&name=%s" % draft.title.split()[0]) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by author - r = self.client.post( - url, - { - "activedrafts": "on", - "by": "author", - "author": draft.documentauthor_set.first().person.name_parts()[1], - }, - ) + r = self.client.get(base_url + "?activedrafts=on&by=author&author=%s" % draft.documentauthor_set.first().person.name_parts()[1]) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by group - r = self.client.post( - url, - {"activedrafts": "on", "by": "group", "group": draft.group.acronym}, - ) + r = self.client.get(base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - - r = self.client.post( - url, - {"activedrafts": "on", "by": "group", "group": draft.group.acronym.swapcase()}, - ) + + r = self.client.get(base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym.swapcase()) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by area - r = self.client.post( - url, - {"activedrafts": "on", "by": "area", "area": draft.group.parent_id}, - ) + r = self.client.get(base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by area - r = self.client.post( - url, - {"activedrafts": "on", "by": "area", "area": draft.group.parent_id}, - ) + r = self.client.get(base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by AD - r = self.client.post(url, {"activedrafts": "on", "by": "ad", "ad": draft.ad_id}) + r = self.client.get(base_url + "?activedrafts=on&by=ad&ad=%s" % draft.ad_id) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by IESG state - r = self.client.post( - url, - { - "activedrafts": "on", - "by": "state", - "state": draft.get_state("draft-iesg").pk, - "substate": "", - }, - ) + r = self.client.get(base_url + "?activedrafts=on&by=state&state=%s&substate=" % draft.get_state("draft-iesg").pk) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) @@ -238,15 +171,15 @@ def test_search_became_rfc(self): rfc = WgRfcFactory() draft.set_state(State.objects.get(type="draft", slug="rfc")) draft.relateddocument_set.create(relationship_id="became_rfc", target=rfc) - url = urlreverse("ietf.doc.views_search.search") + base_url = urlreverse('ietf.doc.views_search.search') # find by RFC - r = self.client.post(url, {"rfcs": "on", "name": rfc.name}) + r = self.client.get(base_url + f"?rfcs=on&name={rfc.name}") self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) # find by draft - r = self.client.post(url, {"activedrafts": "on", "rfcs": "on", "name": draft.name}) + r = self.client.get(base_url + f"?activedrafts=on&rfcs=on&name={draft.name}") self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index f4ad247ff0..9e9b5e88dd 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -33,12 +33,11 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - +import hashlib +import json import re import datetime import copy -import hashlib -import json import operator from collections import defaultdict @@ -46,18 +45,16 @@ from django import forms from django.conf import settings -from django.contrib import messages from django.core.cache import cache, caches from django.urls import reverse as urlreverse from django.db.models import Q -from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect +from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect, QueryDict from django.shortcuts import render from django.utils import timezone from django.utils.html import strip_tags from django.utils.cache import _generate_cache_key # type: ignore from django.utils.text import slugify -from django.views.decorators.csrf import ensure_csrf_cookie -from django_stubs_ext import QuerySetAny + import debug # pyflakes:ignore @@ -149,29 +146,6 @@ def clean(self): q['irtfstate'] = None return q - def cache_key_fragment(self): - """Hash a bound form to get a value for use in a cache key - - Raises a ValueError if the form is not valid. - """ - def _serialize_value(val): - # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 - if isinstance(val, QuerySetAny): - return [item.pk for item in val] - else: - return getattr(val, "pk", val) # use pk if present, else value - - if not self.is_valid(): - raise ValueError(f"SearchForm invalid: {self.errors}") - contents = { - field_name: _serialize_value(field_value) - for field_name, field_value in self.cleaned_data.items() - if field_name != "sort" and field_value is not None - } - contents_json = json.dumps(contents, sort_keys=True) - return hashlib.sha512(contents_json.encode("utf-8")).hexdigest() - - def retrieve_search_results(form, all_types=False): """Takes a validated SearchForm and return the results.""" @@ -284,65 +258,51 @@ def retrieve_search_results(form, all_types=False): return docs -@ensure_csrf_cookie def search(request): - """Search for a draft""" - # defaults for results / meta - results = [] - meta = {"by": None, "searching": False} - - if request.method == "POST": - form = SearchForm(data=request.POST) - if form.is_valid(): - cache_key = f"doc:document:search:{form.cache_key_fragment()}" - cached_val = cache.get(cache_key) - if cached_val: - [results, meta] = cached_val - else: - results = retrieve_search_results(form) - results, meta = prepare_document_table( - request, results, form.cleaned_data - ) - cache.set( - cache_key, [results, meta] - ) # for settings.CACHE_MIDDLEWARE_SECONDS - log(f"Search results computed for {form.cleaned_data}") - meta["searching"] = True - else: - if request.GET: - # backwards compatibility - fill in the form - get_params = request.GET.copy() - if "activeDrafts" in request.GET: - get_params["activedrafts"] = request.GET["activeDrafts"] - if "oldDrafts" in request.GET: - get_params["olddrafts"] = request.GET["oldDrafts"] - if "subState" in request.GET: - get_params["substate"] = request.GET["subState"] - form = SearchForm(data=get_params) - messages.error( - request, - ( - "Searching via the URL query string is no longer supported. " - "The form below has been filled in with the parameters from your request. " - 'To execute your search, please click "Search".' - ), - ) + def _get_cache_key(params): + fields = set(SearchForm.base_fields) - {'sort'} + kwargs = dict([(k, v) for (k, v) in list(params.items()) if k in fields]) + key = "doc:document:search:" + hashlib.sha512(json.dumps(kwargs, sort_keys=True).encode('utf-8')).hexdigest() + return key + + if request.GET: + # backwards compatibility + get_params = request.GET.copy() + if 'activeDrafts' in request.GET: + get_params['activedrafts'] = request.GET['activeDrafts'] + if 'oldDrafts' in request.GET: + get_params['olddrafts'] = request.GET['oldDrafts'] + if 'subState' in request.GET: + get_params['substate'] = request.GET['subState'] + + form = SearchForm(get_params) + if not form.is_valid(): + return HttpResponseBadRequest("form not valid: %s" % form.errors) + + cache_key = _get_cache_key(get_params) + cached_val = cache.get(cache_key) + if cached_val: + [results, meta] = cached_val else: - form = SearchForm() + results = retrieve_search_results(form) + results, meta = prepare_document_table(request, results, get_params) + cache.set(cache_key, [results, meta]) # for settings.CACHE_MIDDLEWARE_SECONDS + log(f"Search results computed for {get_params}") + meta['searching'] = True + else: + form = SearchForm() + results = [] + meta = { 'by': None, 'searching': False } + get_params = QueryDict('') - return render( - request, - "doc/search/search.html", - context={"form": form, "docs": results, "meta": meta}, + return render(request, 'doc/search/search.html', { + 'form':form, 'docs':results, 'meta':meta, 'queryargs':get_params.urlencode() }, ) - -@ensure_csrf_cookie def frontpage(request): form = SearchForm() return render(request, 'doc/frontpage.html', {'form':form}) - def search_for_name(request, name): def find_unique(n): exact = Document.objects.filter(name__iexact=n).first() diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py index a75028bf79..1d91041b25 100644 --- a/ietf/liaisons/forms.py +++ b/ietf/liaisons/forms.py @@ -203,7 +203,6 @@ def get_results(self): class CustomModelMultipleChoiceField(ModelMultipleChoiceField): '''If value is a QuerySet, return it as is (for use in widget.render)''' def prepare_value(self, value): - # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if isinstance(value, QuerySetAny): return value if (hasattr(value, '__iter__') and diff --git a/ietf/liaisons/widgets.py b/ietf/liaisons/widgets.py index 3d4f2d13a5..74368e83f2 100644 --- a/ietf/liaisons/widgets.py +++ b/ietf/liaisons/widgets.py @@ -35,7 +35,6 @@ def render(self, name, value, **kwargs): html = '
' % name html += 'No files attached' html += '
' - # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if value and isinstance(value, QuerySetAny): for attachment in value: html += '%s ' % (conditional_escape(attachment.document.get_href()), conditional_escape(attachment.document.title)) diff --git a/ietf/templates/doc/search/search_form.html b/ietf/templates/doc/search/search_form.html index 6c91894c8c..d4f463ec66 100644 --- a/ietf/templates/doc/search/search_form.html +++ b/ietf/templates/doc/search/search_form.html @@ -4,10 +4,8 @@ {% load widget_tweaks %} {% load ietf_filters %}
- {% csrf_token %}
{{ form.name|add_class:"form-control"|attr:"placeholder:Document name/title/RFC number"|attr:"aria-label:Document name/title/RFC number" }} From ff5b2e12f390162796135c7b9cd17c80934fb55a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 22 Nov 2024 18:54:15 -0400 Subject: [PATCH 04/12] chore: fix is_authenticated logging (#8266) "o" instead of "i" to check response headers --- ietf/utils/jsonlogger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/utils/jsonlogger.py b/ietf/utils/jsonlogger.py index c383ba310f..b02cd7af2b 100644 --- a/ietf/utils/jsonlogger.py +++ b/ietf/utils/jsonlogger.py @@ -31,4 +31,4 @@ def add_fields(self, log_record, record, message_dict): log_record.setdefault("cf_connecting_ip", record.args["{cf-connecting-ip}i"]) log_record.setdefault("cf_connecting_ipv6", record.args["{cf-connecting-ipv6}i"]) log_record.setdefault("cf_ray", record.args["{cf-ray}i"]) - log_record.setdefault("is_authenticated", record.args["{x-datatracker-is-authenticated}i"]) + log_record.setdefault("is_authenticated", record.args["{x-datatracker-is-authenticated}o"]) From a45f10c0ce6cbfbc6061049532056892605fa036 Mon Sep 17 00:00:00 2001 From: Matthew Holloway Date: Mon, 25 Nov 2024 15:42:05 +0000 Subject: [PATCH 05/12] fix: increase timeout from 5s to 15s for playwright status tests (#8267) --- playwright/tests/status/status.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/playwright/tests/status/status.spec.js b/playwright/tests/status/status.spec.js index daef7a88f1..c70617e2fd 100644 --- a/playwright/tests/status/status.spec.js +++ b/playwright/tests/status/status.spec.js @@ -20,7 +20,8 @@ test.describe('site status', () => { by: 'Exile is a cool Amiga game' } - test.beforeEach(({ browserName }) => { + test.beforeEach(({ page, browserName }) => { + page.setDefaultTimeout(15 * 1000) // increase default timeout test.skip(browserName === 'firefox', 'bypassing flaky tests on Firefox') }) From 4b5760240417cac57801b5dc7b47296f94cf9ac3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 25 Nov 2024 14:51:44 -0400 Subject: [PATCH 06/12] chore: log in-flight request lists on worker term (#8272) * chore: log in-flight request lists on worker term * style: Black * chore: suppress empty in-flight logs * chore: use list consistently --- dev/build/gunicorn.conf.py | 70 ++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 11 deletions(-) diff --git a/dev/build/gunicorn.conf.py b/dev/build/gunicorn.conf.py index 89d3dd58d3..cabbee0b1e 100644 --- a/dev/build/gunicorn.conf.py +++ b/dev/build/gunicorn.conf.py @@ -12,26 +12,25 @@ "level": "INFO", "handlers": ["console"], "propagate": False, - "qualname": "gunicorn.error" + "qualname": "gunicorn.error", }, - "gunicorn.access": { "level": "INFO", "handlers": ["access_console"], "propagate": False, - "qualname": "gunicorn.access" - } + "qualname": "gunicorn.access", + }, }, "handlers": { "console": { "class": "logging.StreamHandler", "formatter": "json", - "stream": "ext://sys.stdout" + "stream": "ext://sys.stdout", }, "access_console": { "class": "logging.StreamHandler", "formatter": "access_json", - "stream": "ext://sys.stdout" + "stream": "ext://sys.stdout", }, }, "formatters": { @@ -44,14 +43,29 @@ "class": "ietf.utils.jsonlogger.GunicornRequestJsonFormatter", "style": "{", "format": "{asctime}{levelname}{message}{name}{process}", - } - } + }, + }, } -def pre_request(worker, req): +# Track in-flight requests and emit a list of what was happeningwhen a worker is terminated. +# For the default sync worker, there will only be one request per PID, but allow for the +# possibility of multiple requests in case we switch to a different worker class. +# +# This dict is only visible within a single worker, but key by pid to guarantee no conflicts. +# +# Use a list rather than a set to allow for the possibility of overlapping identical requests. +in_flight_by_pid: dict[str, list[str]] = {} # pid -> list of in-flight requests + + +def _describe_request(req): + """Generate a consistent description of a request + + The return value is used identify in-flight requests, so it must not vary between the + start and end of handling a request. E.g., do not include a timestamp. + """ client_ip = "-" cf_ray = "-" - for (header, value) in req.headers: + for header, value in req.headers: header = header.lower() if header == "cf-connecting-ip": client_ip = value @@ -61,4 +75,38 @@ def pre_request(worker, req): path = f"{req.path}?{req.query}" else: path = req.path - worker.log.info(f"gunicorn starting to process {req.method} {path} (client_ip={client_ip}, cf_ray={cf_ray})") + return f"{req.method} {path} (client_ip={client_ip}, cf_ray={cf_ray})" + + +def pre_request(worker, req): + """Log the start of a request and add it to the in-flight list""" + request_description = _describe_request(req) + worker.log.info(f"gunicorn starting to process {request_description}") + in_flight = in_flight_by_pid.setdefault(worker.pid, []) + in_flight.append(request_description) + + +def worker_abort(worker): + """Emit an error log if any requests were in-flight""" + in_flight = in_flight_by_pid.get(worker.pid, []) + if len(in_flight) > 0: + worker.log.error( + f"Aborted worker {worker.pid} with in-flight requests: {', '.join(in_flight)}" + ) + + +def worker_int(worker): + """Emit an error log if any requests were in-flight""" + in_flight = in_flight_by_pid.get(worker.pid, []) + if len(in_flight) > 0: + worker.log.error( + f"Interrupted worker {worker.pid} with in-flight requests: {', '.join(in_flight)}" + ) + + +def post_request(worker, req, environ, resp): + """Remove request from in-flight list when we finish handling it""" + request_description = _describe_request(req) + in_flight = in_flight_by_pid.get(worker.pid, []) + if request_description in in_flight: + in_flight.remove(request_description) From 5bb79bb7ca05d4660a5f21c704cb58c0e1668b37 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 26 Nov 2024 09:23:30 -0400 Subject: [PATCH 07/12] ci: fix comment in settings_local.py --- k8s/settings_local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/settings_local.py b/k8s/settings_local.py index 33ac4f1e38..a63a02e536 100644 --- a/k8s/settings_local.py +++ b/k8s/settings_local.py @@ -82,7 +82,7 @@ def _multiline_to_list(s): # Set DEBUG if DATATRACKER_DEBUG env var is the word "true" DEBUG = os.environ.get("DATATRACKER_DEBUG", "false").lower() == "true" -# DATATRACKER_ALLOWED_HOSTS env var is a comma-separated list of allowed hosts +# DATATRACKER_ALLOWED_HOSTS env var is a newline-separated list of allowed hosts _allowed_hosts_str = os.environ.get("DATATRACKER_ALLOWED_HOSTS", None) if _allowed_hosts_str is not None: ALLOWED_HOSTS = _multiline_to_list(_allowed_hosts_str) From 35e3433c2a75fa6452161dd8028df943b3e8167b Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Tue, 26 Nov 2024 19:15:50 -0600 Subject: [PATCH 08/12] fix: use lxml 5 (#8279) --- requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 5c80d5301e..91647729be 100644 --- a/requirements.txt +++ b/requirements.txt @@ -39,7 +39,8 @@ jsonfield>=3.1.0 # for SubmissionCheck. This is https://github.com/bradjaspe jsonschema[format]>=4.2.1 jwcrypto>=1.2 # for signed notifications - this is aspirational, and is not really used. logging_tree>=1.9 # Used only by the showloggers management command -lxml>=4.8.0,<5 +lxml>=5.3.0 # lxml[html_clean] fails on some architectures +lxml_html_clean>=0.4.1 markdown>=3.3.6 types-markdown>=3.3.6 mock>=4.0.3 # Used only by tests, of course From 15f3ebd2d5092a91e2feb6673212744384325b8b Mon Sep 17 00:00:00 2001 From: rjsparks Date: Wed, 27 Nov 2024 01:27:15 +0000 Subject: [PATCH 09/12] ci: update base image target version to 20241127T0116 --- dev/build/Dockerfile | 2 +- dev/build/TARGET_BASE | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/build/Dockerfile b/dev/build/Dockerfile index c9f91163ce..ef77fef58f 100644 --- a/dev/build/Dockerfile +++ b/dev/build/Dockerfile @@ -1,4 +1,4 @@ -FROM ghcr.io/ietf-tools/datatracker-app-base:20241114T1954 +FROM ghcr.io/ietf-tools/datatracker-app-base:20241127T0116 LABEL maintainer="IETF Tools Team " ENV DEBIAN_FRONTEND=noninteractive diff --git a/dev/build/TARGET_BASE b/dev/build/TARGET_BASE index 7bbb32355d..6420933c22 100644 --- a/dev/build/TARGET_BASE +++ b/dev/build/TARGET_BASE @@ -1 +1 @@ -20241114T1954 +20241127T0116 From c18900a8e693e5704ae6a0336dba25388ced5896 Mon Sep 17 00:00:00 2001 From: Kesara Rathnayake Date: Wed, 27 Nov 2024 14:20:24 +1100 Subject: [PATCH 10/12] feat: Expose important library versions (#7713) * feat: Expose important library versions Update `/api/version` to include ``` "other": { "xml2rfc": "", "weasyprint": "" }, ``` Fixes #3415 * fix: Use importlib * chore: Reomve additional newline * fix: Expose libraries that are important for document submission * fix: Rename IMPORTANT_LIBRARIES as ADVERTISE_VERSIONS --- ietf/api/tests.py | 2 ++ ietf/api/views.py | 8 ++++++++ ietf/settings.py | 2 ++ 3 files changed, 12 insertions(+) diff --git a/ietf/api/tests.py b/ietf/api/tests.py index 7dc9c42cc8..a8d6ac4e57 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -936,6 +936,8 @@ def test_api_version(self): r = self.client.get(url) data = r.json() self.assertEqual(data['version'], ietf.__version__+ietf.__patch__) + for lib in settings.ADVERTISE_VERSIONS: + self.assertIn(lib, data['other']) self.assertEqual(data['dumptime'], "2022-08-31 07:10:01 +0000") DumpInfo.objects.update(tz='PST8PDT') r = self.client.get(url) diff --git a/ietf/api/views.py b/ietf/api/views.py index df67cffd56..3e56757528 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -23,6 +23,7 @@ from django.views.decorators.gzip import gzip_page from django.views.generic.detail import DetailView from email.message import EmailMessage +from importlib.metadata import version as metadata_version from jwcrypto.jwk import JWK from tastypie.exceptions import BadRequest from tastypie.serializers import Serializer @@ -240,9 +241,16 @@ def version(request): if dumpinfo.tz != "UTC": dumpdate = pytz.timezone(dumpinfo.tz).localize(dumpinfo.date.replace(tzinfo=None)) dumptime = dumpdate.strftime('%Y-%m-%d %H:%M:%S %z') if dumpinfo else None + + # important libraries + __version_extra__ = {} + for lib in settings.ADVERTISE_VERSIONS: + __version_extra__[lib] = metadata_version(lib) + return HttpResponse( json.dumps({ 'version': ietf.__version__+ietf.__patch__, + 'other': __version_extra__, 'dumptime': dumptime, }), content_type='application/json', diff --git a/ietf/settings.py b/ietf/settings.py index 30f4b367c6..368f61c6e6 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -1273,6 +1273,8 @@ def skip_unreadable_post(record): PUBLISH_IPR_STATES = ['posted', 'removed', 'removed_objfalse'] +ADVERTISE_VERSIONS = ["markdown", "pyang", "rfc2html", "xml2rfc"] + # We provide a secret key only for test and development modes. It's # absolutely vital that django fails to start in production mode unless a # secret key has been provided elsewhere, not in this file which is From c58490bb36ed6c548e064ce3ab4e7c42fd48f0a5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 27 Nov 2024 16:54:28 -0400 Subject: [PATCH 11/12] feat: django-rest-framework + Person/Email API (#8256) * feat: django-rest-framework + Person/Email API (#8233) * chore: djangorestframework -> requirements.txt * chore: auth/perm/schema classes for drf * chore: settings for drf and friends * chore: comment that api/serializer.py is not DRF * feat: URL router for DRF * feat: simple api/v3/person/{id} endpoint * fix: actually working demo endpoint * chore: no auth for PersonViewSet * ci: params in ci-run-tests.yml * Revert "ci: params in ci-run-tests.yml" This reverts commit 03808ddf94afe42b7382ddd3730959987389612b. * feat: email addresses for person API * feat: email update api (WIP) * fix: working Email API endpoint * chore: annotate address format in api schema * chore: api adjustments * feat: expose SpectacularAPIView At least for now... * chore: better schema_path_prefix * feat: permissions for DRF API * refactor: use permissions classes * refactor: extract NewEmailForm validation for reuse * refactor: ietfauth.validators module * refactor: send new email conf req via helper * feat: API call to issue new address request * chore: move datatracker DRF api to /api/core/ * fix: unused import * fix: lint * test: drf URL names + API tests (#8248) * refactor: better drf URL naming * test: test person-detail view * test: permissions * test: add_email tests + stubs * test: test email update * test: test 404 vs 403 * fix: fix permissions * test: test email partial update * test: assert we have a nonexistent PK * chore: disable DRF api for now * chore: fix git inanity * fix: lint * test: disable tests of disabled code * test: more lint --- ietf/api/apps.py | 4 + ietf/api/authentication.py | 19 +++ ietf/api/permissions.py | 39 +++++ ietf/api/routers.py | 16 ++ ietf/api/schema.py | 20 +++ ietf/api/serializer.py | 5 +- ietf/api/tests_core.py | 289 ++++++++++++++++++++++++++++++++++++ ietf/api/urls.py | 14 +- ietf/ietfauth/forms.py | 27 +--- ietf/ietfauth/utils.py | 49 +++++- ietf/ietfauth/validators.py | 34 +++++ ietf/ietfauth/views.py | 27 +--- ietf/person/api.py | 45 ++++++ ietf/person/serializers.py | 39 +++++ ietf/settings.py | 73 +++++++++ requirements.txt | 3 + 16 files changed, 650 insertions(+), 53 deletions(-) create mode 100644 ietf/api/authentication.py create mode 100644 ietf/api/permissions.py create mode 100644 ietf/api/routers.py create mode 100644 ietf/api/schema.py create mode 100644 ietf/api/tests_core.py create mode 100644 ietf/ietfauth/validators.py create mode 100644 ietf/person/api.py create mode 100644 ietf/person/serializers.py diff --git a/ietf/api/apps.py b/ietf/api/apps.py index 7eca094a62..4549e0d7f2 100644 --- a/ietf/api/apps.py +++ b/ietf/api/apps.py @@ -12,4 +12,8 @@ def ready(self): interact with the database. See https://docs.djangoproject.com/en/4.2/ref/applications/#django.apps.AppConfig.ready """ + # Populate our API list now that the app registry is set up populate_api_list() + + # Import drf-spectacular extensions + import ietf.api.schema # pyflakes: ignore diff --git a/ietf/api/authentication.py b/ietf/api/authentication.py new file mode 100644 index 0000000000..dfab0d72b8 --- /dev/null +++ b/ietf/api/authentication.py @@ -0,0 +1,19 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +from rest_framework import authentication +from django.contrib.auth.models import AnonymousUser + + +class ApiKeyAuthentication(authentication.BaseAuthentication): + """API-Key header authentication""" + + def authenticate(self, request): + """Extract the authentication token, if present + + This does not validate the token, it just arranges for it to be available in request.auth. + It's up to a Permissions class to validate it for the appropriate endpoint. + """ + token = request.META.get("HTTP_X_API_KEY", None) + if token is None: + return None + return AnonymousUser(), token # available as request.user and request.auth diff --git a/ietf/api/permissions.py b/ietf/api/permissions.py new file mode 100644 index 0000000000..8f7fdd026f --- /dev/null +++ b/ietf/api/permissions.py @@ -0,0 +1,39 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +from rest_framework import permissions +from ietf.api.ietf_utils import is_valid_token + + +class HasApiKey(permissions.BasePermission): + """Permissions class that validates a token using is_valid_token + + The view class must indicate the relevant endpoint by setting `api_key_endpoint`. + Must be used with an Authentication class that puts a token in request.auth. + """ + def has_permission(self, request, view): + endpoint = getattr(view, "api_key_endpoint", None) + auth_token = getattr(request, "auth", None) + if endpoint is not None and auth_token is not None: + return is_valid_token(endpoint, auth_token) + return False + + +class IsOwnPerson(permissions.BasePermission): + """Permission to access own Person object""" + def has_object_permission(self, request, view, obj): + if not (request.user.is_authenticated and hasattr(request.user, "person")): + return False + return obj == request.user.person + + +class BelongsToOwnPerson(permissions.BasePermission): + """Permission to access objects associated with own Person + + Requires that the object have a "person" field that indicates ownership. + """ + def has_object_permission(self, request, view, obj): + if not (request.user.is_authenticated and hasattr(request.user, "person")): + return False + return ( + hasattr(obj, "person") and obj.person == request.user.person + ) diff --git a/ietf/api/routers.py b/ietf/api/routers.py new file mode 100644 index 0000000000..745ddaa811 --- /dev/null +++ b/ietf/api/routers.py @@ -0,0 +1,16 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +"""Custom django-rest-framework routers""" +from django.core.exceptions import ImproperlyConfigured +from rest_framework import routers + +class PrefixedSimpleRouter(routers.SimpleRouter): + """SimpleRouter that adds a dot-separated prefix to its basename""" + def __init__(self, name_prefix="", *args, **kwargs): + self.name_prefix = name_prefix + if len(self.name_prefix) == 0 or self.name_prefix[-1] == ".": + raise ImproperlyConfigured("Cannot use a name_prefix that is empty or ends with '.'") + super().__init__(*args, **kwargs) + + def get_default_basename(self, viewset): + basename = super().get_default_basename(viewset) + return f"{self.name_prefix}.{basename}" diff --git a/ietf/api/schema.py b/ietf/api/schema.py new file mode 100644 index 0000000000..7340149685 --- /dev/null +++ b/ietf/api/schema.py @@ -0,0 +1,20 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +from drf_spectacular.extensions import OpenApiAuthenticationExtension + + +class ApiKeyAuthenticationScheme(OpenApiAuthenticationExtension): + """Authentication scheme extension for the ApiKeyAuthentication + + Used by drf-spectacular when rendering the OpenAPI schema + """ + target_class = "ietf.api.authentication.ApiKeyAuthentication" + name = "apiKeyAuth" + + def get_security_definition(self, auto_schema): + return { + "type": "apiKey", + "description": "Shared secret in the X-Api-Key header", + "name": "X-Api-Key", + "in": "header", + } diff --git a/ietf/api/serializer.py b/ietf/api/serializer.py index 27f194c5b5..d5bca430e0 100644 --- a/ietf/api/serializer.py +++ b/ietf/api/serializer.py @@ -1,6 +1,9 @@ -# Copyright The IETF Trust 2018-2020, All Rights Reserved +# Copyright The IETF Trust 2018-2024, All Rights Reserved # -*- coding: utf-8 -*- +"""Serialization utilities +This is _not_ for django-rest-framework! +""" import hashlib import json diff --git a/ietf/api/tests_core.py b/ietf/api/tests_core.py new file mode 100644 index 0000000000..7e45deac8a --- /dev/null +++ b/ietf/api/tests_core.py @@ -0,0 +1,289 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +"""Core API tests""" +from unittest.mock import patch +# from unittest.mock import patch, call + +from django.urls import reverse as urlreverse, NoReverseMatch +from rest_framework.test import APIClient + +# from ietf.person.factories import PersonFactory, EmailFactory +# from ietf.person.models import Person +from ietf.utils.test_utils import TestCase + + +class CoreApiTestCase(TestCase): + client_class = APIClient + + +class PersonTests(CoreApiTestCase): + # Tests disabled until we activate the DRF URLs in api/urls.py + + def test_person_detail(self): + with self.assertRaises(NoReverseMatch, msg="Re-enable test when this view is enabled"): + urlreverse("ietf.api.core_api.person-detail", kwargs={"pk": 1}) + + # person = PersonFactory() + # other_person = PersonFactory() + # url = urlreverse("ietf.api.core_api.person-detail", kwargs={"pk": person.pk}) + # bad_pk = person.pk + 10000 + # if Person.objects.filter(pk=bad_pk).exists(): + # bad_pk += 10000 # if this doesn't get us clear, something is wrong... + # self.assertFalse( + # Person.objects.filter(pk=bad_pk).exists(), + # "Failed to find a non-existent person pk", + # ) + # bad_url = urlreverse("ietf.api.core_api.person-detail", kwargs={"pk": bad_pk}) + # r = self.client.get(bad_url, format="json") + # self.assertEqual(r.status_code, 403, "Must be logged in preferred to 404") + # r = self.client.get(url, format="json") + # self.assertEqual(r.status_code, 403, "Must be logged in") + # self.client.login( + # username=other_person.user.username, + # password=other_person.user.username + "+password", + # ) + # r = self.client.get(bad_url, format="json") + # self.assertEqual(r.status_code, 404) + # r = self.client.get(url, format="json") + # self.assertEqual(r.status_code, 403, "Can only retrieve self") + # self.client.login( + # username=person.user.username, password=person.user.username + "+password" + # ) + # r = self.client.get(url, format="json") + # self.assertEqual(r.status_code, 200) + # self.assertEqual( + # r.data, + # { + # "id": person.pk, + # "name": person.name, + # "emails": [ + # { + # "person": person.pk, + # "address": email.address, + # "primary": email.primary, + # "active": email.active, + # "origin": email.origin, + # } + # for email in person.email_set.all() + # ], + # }, + # ) + + @patch("ietf.person.api.send_new_email_confirmation_request") + def test_add_email(self, send_confirmation_mock): + with self.assertRaises(NoReverseMatch, msg="Re-enable this test when this view is enabled"): + urlreverse("ietf.api.core_api.person-email", kwargs={"pk": 1}) + + # email = EmailFactory(address="old@example.org") + # person = email.person + # other_person = PersonFactory() + # url = urlreverse("ietf.api.core_api.person-email", kwargs={"pk": person.pk}) + # post_data = {"address": "new@example.org"} + # + # r = self.client.post(url, data=post_data, format="json") + # self.assertEqual(r.status_code, 403, "Must be logged in") + # self.assertFalse(send_confirmation_mock.called) + # + # self.client.login( + # username=other_person.user.username, + # password=other_person.user.username + "+password", + # ) + # r = self.client.post(url, data=post_data, format="json") + # self.assertEqual(r.status_code, 403, "Can only retrieve self") + # self.assertFalse(send_confirmation_mock.called) + # + # self.client.login( + # username=person.user.username, password=person.user.username + "+password" + # ) + # r = self.client.post(url, data=post_data, format="json") + # self.assertEqual(r.status_code, 200) + # self.assertEqual(r.data, {"address": "new@example.org"}) + # self.assertTrue(send_confirmation_mock.called) + # self.assertEqual( + # send_confirmation_mock.call_args, call(person, "new@example.org") + # ) + + +class EmailTests(CoreApiTestCase): + def test_email_update(self): + with self.assertRaises(NoReverseMatch, msg="Re-enable this test when the view is enabled"): + urlreverse( + "ietf.api.core_api.email-detail", kwargs={"pk": "original@example.org"} + ) + + # email = EmailFactory( + # address="original@example.org", primary=False, active=True, origin="factory" + # ) + # person = email.person + # other_person = PersonFactory() + # url = urlreverse( + # "ietf.api.core_api.email-detail", kwargs={"pk": "original@example.org"} + # ) + # bad_url = urlreverse( + # "ietf.api.core_api.email-detail", + # kwargs={"pk": "not-original@example.org"}, + # ) + # + # r = self.client.put( + # bad_url, data={"primary": True, "active": False}, format="json" + # ) + # self.assertEqual(r.status_code, 403, "Must be logged in preferred to 404") + # r = self.client.put(url, data={"primary": True, "active": False}, format="json") + # self.assertEqual(r.status_code, 403, "Must be logged in") + # + # self.client.login( + # username=other_person.user.username, + # password=other_person.user.username + "+password", + # ) + # r = self.client.put( + # bad_url, data={"primary": True, "active": False}, format="json" + # ) + # self.assertEqual(r.status_code, 404, "No such address") + # r = self.client.put(url, data={"primary": True, "active": False}, format="json") + # self.assertEqual(r.status_code, 403, "Can only access own addresses") + # + # self.client.login( + # username=person.user.username, password=person.user.username + "+password" + # ) + # r = self.client.put(url, data={"primary": True, "active": False}, format="json") + # self.assertEqual(r.status_code, 200) + # self.assertEqual( + # r.data, + # { + # "person": person.pk, + # "address": "original@example.org", + # "primary": True, + # "active": False, + # "origin": "factory", + # }, + # ) + # email.refresh_from_db() + # self.assertEqual(email.person, person) + # self.assertEqual(email.address, "original@example.org") + # self.assertTrue(email.primary) + # self.assertFalse(email.active) + # self.assertEqual(email.origin, "factory") + # + # # address / origin should be immutable + # r = self.client.put( + # url, + # data={ + # "address": "modified@example.org", + # "primary": True, + # "active": False, + # "origin": "hacker", + # }, + # format="json", + # ) + # self.assertEqual(r.status_code, 200) + # self.assertEqual( + # r.data, + # { + # "person": person.pk, + # "address": "original@example.org", + # "primary": True, + # "active": False, + # "origin": "factory", + # }, + # ) + # email.refresh_from_db() + # self.assertEqual(email.person, person) + # self.assertEqual(email.address, "original@example.org") + # self.assertTrue(email.primary) + # self.assertFalse(email.active) + # self.assertEqual(email.origin, "factory") + + def test_email_partial_update(self): + with self.assertRaises(NoReverseMatch, msg="Re-enable this test when the view is enabled"): + urlreverse( + "ietf.api.core_api.email-detail", kwargs={"pk": "original@example.org"} + ) + + # email = EmailFactory( + # address="original@example.org", primary=False, active=True, origin="factory" + # ) + # person = email.person + # other_person = PersonFactory() + # url = urlreverse( + # "ietf.api.core_api.email-detail", kwargs={"pk": "original@example.org"} + # ) + # bad_url = urlreverse( + # "ietf.api.core_api.email-detail", + # kwargs={"pk": "not-original@example.org"}, + # ) + # + # r = self.client.patch( + # bad_url, data={"primary": True}, format="json" + # ) + # self.assertEqual(r.status_code, 403, "Must be logged in preferred to 404") + # r = self.client.patch(url, data={"primary": True}, format="json") + # self.assertEqual(r.status_code, 403, "Must be logged in") + # + # self.client.login( + # username=other_person.user.username, + # password=other_person.user.username + "+password", + # ) + # r = self.client.patch( + # bad_url, data={"primary": True}, format="json" + # ) + # self.assertEqual(r.status_code, 404, "No such address") + # r = self.client.patch(url, data={"primary": True}, format="json") + # self.assertEqual(r.status_code, 403, "Can only access own addresses") + # + # self.client.login( + # username=person.user.username, password=person.user.username + "+password" + # ) + # r = self.client.patch(url, data={"primary": True}, format="json") + # self.assertEqual(r.status_code, 200) + # self.assertEqual( + # r.data, + # { + # "person": person.pk, + # "address": "original@example.org", + # "primary": True, + # "active": True, + # "origin": "factory", + # }, + # ) + # email.refresh_from_db() + # self.assertEqual(email.person, person) + # self.assertEqual(email.address, "original@example.org") + # self.assertTrue(email.primary) + # self.assertTrue(email.active) + # self.assertEqual(email.origin, "factory") + # + # r = self.client.patch(url, data={"active": False}, format="json") + # self.assertEqual(r.status_code, 200) + # self.assertEqual( + # r.data, + # { + # "person": person.pk, + # "address": "original@example.org", + # "primary": True, + # "active": False, + # "origin": "factory", + # }, + # ) + # email.refresh_from_db() + # self.assertEqual(email.person, person) + # self.assertEqual(email.address, "original@example.org") + # self.assertTrue(email.primary) + # self.assertFalse(email.active) + # self.assertEqual(email.origin, "factory") + # + # r = self.client.patch(url, data={"address": "modified@example.org"}, format="json") + # self.assertEqual(r.status_code, 200) # extra fields allowed, but ignored + # email.refresh_from_db() + # self.assertEqual(email.person, person) + # self.assertEqual(email.address, "original@example.org") + # self.assertTrue(email.primary) + # self.assertFalse(email.active) + # self.assertEqual(email.origin, "factory") + # + # r = self.client.patch(url, data={"origin": "hacker"}, format="json") + # self.assertEqual(r.status_code, 200) # extra fields allowed, but ignored + # email.refresh_from_db() + # self.assertEqual(email.person, person) + # self.assertEqual(email.address, "original@example.org") + # self.assertTrue(email.primary) + # self.assertFalse(email.active) + # self.assertEqual(email.origin, "factory") diff --git a/ietf/api/urls.py b/ietf/api/urls.py index 6846e32747..a9aaaf5805 100644 --- a/ietf/api/urls.py +++ b/ietf/api/urls.py @@ -5,12 +5,21 @@ from django.views.generic import TemplateView from ietf import api -from ietf.api import views as api_views from ietf.doc import views_ballot from ietf.meeting import views as meeting_views from ietf.submit import views as submit_views from ietf.utils.urls import url +from . import views as api_views + +# DRF API routing - disabled until we plan to use it +# from drf_spectacular.views import SpectacularAPIView +# from django.urls import path +# from ietf.person import api as person_api +# from .routers import PrefixedSimpleRouter +# core_router = PrefixedSimpleRouter(name_prefix="ietf.api.core_api") # core api router +# core_router.register("email", person_api.EmailViewSet) +# core_router.register("person", person_api.PersonViewSet) api.autodiscover() @@ -21,6 +30,9 @@ url(r'^v1/?$', api_views.top_level), # For mailarchive use, requires secretariat role url(r'^v2/person/person', api_views.ApiV2PersonExportView.as_view()), + # --- DRF API --- + # path("core/", include(core_router.urls)), + # path("schema/", SpectacularAPIView.as_view()), # # --- Custom API endpoints, sorted alphabetically --- # Email alias information for drafts diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index 9b8ee22e0b..a70f7b6ca1 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -6,17 +6,15 @@ from unidecode import unidecode from django import forms -from django.conf import settings from django.core.exceptions import ValidationError from django.db import models from django.contrib.auth.models import User -import debug # pyflakes:ignore - from ietf.person.models import Person, Email from ietf.mailinglists.models import Allowlisted from ietf.utils.text import isascii +from .validators import prevent_at_symbol, prevent_system_name, prevent_anonymous_name, is_allowed_address from .widgets import PasswordStrengthInput, PasswordConfirmationInput @@ -53,19 +51,6 @@ def ascii_cleaner(supposedly_ascii): raise forms.ValidationError("Only unaccented Latin characters are allowed.") return supposedly_ascii -def prevent_at_symbol(name): - if "@" in name: - raise forms.ValidationError("Please fill in name - this looks like an email address (@ is not allowed in names).") - -def prevent_system_name(name): - name_without_spaces = name.replace(" ", "").replace("\t", "") - if "(system)" in name_without_spaces.lower(): - raise forms.ValidationError("Please pick another name - this name is reserved.") - -def prevent_anonymous_name(name): - name_without_spaces = name.replace(" ", "").replace("\t", "") - if "anonymous" in name_without_spaces.lower(): - raise forms.ValidationError("Please pick another name - this name is reserved.") class PersonPasswordForm(forms.ModelForm, PasswordForm): @@ -156,15 +141,7 @@ def clean(self): class NewEmailForm(forms.Form): - new_email = forms.EmailField(label="New email address", required=False) - - def clean_new_email(self): - email = self.cleaned_data.get("new_email", "") - for pat in settings.EXCLUDED_PERSONAL_EMAIL_REGEX_PATTERNS: - if re.search(pat, email): - raise ValidationError("This email address is not valid in a datatracker account") - - return email + new_email = forms.EmailField(label="New email address", required=False, validators=[is_allowed_address]) class RoleEmailForm(forms.Form): diff --git a/ietf/ietfauth/utils.py b/ietf/ietfauth/utils.py index d8bd67a4e0..b4c6da14ea 100644 --- a/ietf/ietfauth/utils.py +++ b/ietf/ietfauth/utils.py @@ -12,6 +12,8 @@ from django.conf import settings from django.contrib.auth import REDIRECT_FIELD_NAME +from django.contrib.sites.models import Site +from django.core import signing from django.core.exceptions import PermissionDenied from django.db.models import Q from django.http import HttpResponseRedirect @@ -20,9 +22,10 @@ import debug # pyflakes:ignore from ietf.group.models import Role, GroupFeatures -from ietf.person.models import Person +from ietf.person.models import Email, Person from ietf.person.utils import get_dots from ietf.doc.utils_bofreq import bofreq_editors +from ietf.utils.mail import send_mail def user_is_person(user, person): """Test whether user is associated with person.""" @@ -394,3 +397,47 @@ def can_request_rfc_publication(user, doc): return False # See the docstring else: return False + + +def send_new_email_confirmation_request(person: Person, address: str): + """Request confirmation of a new email address + + If the email address is already in use, sends an alert to it. If not, sends a confirmation request. + By design, does not indicate which was sent. This is intended to make it a bit harder to scrape addresses + with a mindless bot. + """ + auth = signing.dumps([person.user.username, address], salt="add_email") + domain = Site.objects.get_current().domain + from_email = settings.DEFAULT_FROM_EMAIL + + existing = Email.objects.filter(address=address).first() + if existing: + subject = f"Attempt to add your email address by {person.name}" + send_mail( + None, + address, + from_email, + subject, + "registration/add_email_exists_email.txt", + { + "domain": domain, + "email": address, + "person": person, + }, + ) + else: + subject = f"Confirm email address for {person.name}" + send_mail( + None, + address, + from_email, + subject, + "registration/add_email_email.txt", + { + "domain": domain, + "auth": auth, + "email": address, + "person": person, + "expire": settings.DAYS_TO_EXPIRE_REGISTRATION_LINK, + }, + ) diff --git a/ietf/ietfauth/validators.py b/ietf/ietfauth/validators.py new file mode 100644 index 0000000000..84684f34d5 --- /dev/null +++ b/ietf/ietfauth/validators.py @@ -0,0 +1,34 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +import re + +from django import forms +from django.conf import settings +from django.core.exceptions import ValidationError + + +def prevent_at_symbol(name): + if "@" in name: + raise forms.ValidationError( + "Please fill in name - this looks like an email address (@ is not allowed in names)." + ) + + +def prevent_system_name(name): + name_without_spaces = name.replace(" ", "").replace("\t", "") + if "(system)" in name_without_spaces.lower(): + raise forms.ValidationError("Please pick another name - this name is reserved.") + + +def prevent_anonymous_name(name): + name_without_spaces = name.replace(" ", "").replace("\t", "") + if "anonymous" in name_without_spaces.lower(): + raise forms.ValidationError("Please pick another name - this name is reserved.") + + +def is_allowed_address(value): + """Validate that an address complies with datatracker requirements""" + for pat in settings.EXCLUDED_PERSONAL_EMAIL_REGEX_PATTERNS: + if re.search(pat, value): + raise ValidationError( + "This email address is not valid in a datatracker account" + ) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 32bb5c92be..23f66ce824 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -65,7 +65,7 @@ from ietf.ietfauth.forms import ( RegistrationForm, PasswordForm, ResetPasswordForm, TestEmailForm, ChangePasswordForm, get_person_form, RoleEmailForm, NewEmailForm, ChangeUsernameForm, PersonPasswordForm) -from ietf.ietfauth.utils import has_role +from ietf.ietfauth.utils import has_role, send_new_email_confirmation_request from ietf.name.models import ExtResourceName from ietf.nomcom.models import NomCom from ietf.person.models import Person, Email, Alias, PersonalApiKey, PERSON_API_KEY_VALUES @@ -297,31 +297,8 @@ def profile(request): to_email = f.cleaned_data["new_email"] if not to_email: continue - email_confirmations.append(to_email) - - auth = django.core.signing.dumps([person.user.username, to_email], salt="add_email") - - domain = Site.objects.get_current().domain - from_email = settings.DEFAULT_FROM_EMAIL - - existing = Email.objects.filter(address=to_email).first() - if existing: - subject = 'Attempt to add your email address by %s' % person.name - send_mail(request, to_email, from_email, subject, 'registration/add_email_exists_email.txt', { - 'domain': domain, - 'email': to_email, - 'person': person, - }) - else: - subject = 'Confirm email address for %s' % person.name - send_mail(request, to_email, from_email, subject, 'registration/add_email_email.txt', { - 'domain': domain, - 'auth': auth, - 'email': to_email, - 'person': person, - 'expire': settings.DAYS_TO_EXPIRE_REGISTRATION_LINK, - }) + send_new_email_confirmation_request(person, to_email) for r in roles: e = r.email_form.cleaned_data["email"] diff --git a/ietf/person/api.py b/ietf/person/api.py new file mode 100644 index 0000000000..960785a3d4 --- /dev/null +++ b/ietf/person/api.py @@ -0,0 +1,45 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +"""DRF API Views""" +from rest_framework import mixins, viewsets +from rest_framework.decorators import action +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response + +from ietf.api.permissions import BelongsToOwnPerson, IsOwnPerson +from ietf.ietfauth.utils import send_new_email_confirmation_request + +from .models import Email, Person +from .serializers import NewEmailSerializer, EmailSerializer, PersonSerializer + + +class EmailViewSet(mixins.UpdateModelMixin, viewsets.GenericViewSet): + """Email viewset + + Only allows updating an existing email for now. + """ + permission_classes = [IsAuthenticated & BelongsToOwnPerson] + queryset = Email.objects.all() + serializer_class = EmailSerializer + lookup_value_regex = '.+@.+' # allow @-sign in the pk + + +class PersonViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet): + """Person viewset""" + permission_classes = [IsAuthenticated & IsOwnPerson] + queryset = Person.objects.all() + serializer_class = PersonSerializer + + @action(detail=True, methods=["post"], serializer_class=NewEmailSerializer) + def email(self, request, pk=None): + """Add an email address for this Person + + Always succeeds if the email address is valid. Causes a confirmation email to be sent to the + requested address and completion of that handshake will actually add the email address. If the + address already exists, an alert will be sent instead of the confirmation email. + """ + person = self.get_object() + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + # This may or may not actually send a confirmation, but doesn't reveal that to the user. + send_new_email_confirmation_request(person, serializer.validated_data["address"]) + return Response(serializer.data) diff --git a/ietf/person/serializers.py b/ietf/person/serializers.py new file mode 100644 index 0000000000..023d77d4bc --- /dev/null +++ b/ietf/person/serializers.py @@ -0,0 +1,39 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +"""DRF Serializers""" + +from rest_framework import serializers + +from ietf.ietfauth.validators import is_allowed_address + +from .models import Email, Person + + +class EmailSerializer(serializers.ModelSerializer): + """Email serializer for read/update""" + + address = serializers.EmailField(read_only=True) + + class Meta: + model = Email + fields = [ + "person", + "address", + "primary", + "active", + "origin", + ] + read_only_fields = ["person", "address", "origin"] + + +class NewEmailSerializer(serializers.Serializer): + """Serialize a new email address request""" + address = serializers.EmailField(validators=[is_allowed_address]) + + +class PersonSerializer(serializers.ModelSerializer): + """Person serializer""" + emails = EmailSerializer(many=True, source="email_set") + + class Meta: + model = Person + fields = ["id", "name", "emails"] diff --git a/ietf/settings.py b/ietf/settings.py index 368f61c6e6..6990037585 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -455,6 +455,9 @@ def skip_unreadable_post(record): 'corsheaders', 'django_markup', 'oidc_provider', + 'drf_spectacular', + 'drf_standardized_errors', + 'rest_framework', 'simple_history', 'tastypie', 'widget_tweaks', @@ -550,6 +553,76 @@ def skip_unreadable_post(record): '::1', ) +# django-rest-framework configuration +REST_FRAMEWORK = { + "DEFAULT_AUTHENTICATION_CLASSES": [ + "ietf.api.authentication.ApiKeyAuthentication", + "rest_framework.authentication.SessionAuthentication", + ], + "DEFAULT_PERMISSION_CLASSES": [ + "ietf.api.permissions.HasApiKey", + ], + "DEFAULT_RENDERER_CLASSES": [ + "rest_framework.renderers.JSONRenderer", + ], + "DEFAULT_PARSER_CLASSES": [ + "rest_framework.parsers.JSONParser", + ], + "DEFAULT_SCHEMA_CLASS": "drf_standardized_errors.openapi.AutoSchema", + "EXCEPTION_HANDLER": "drf_standardized_errors.handler.exception_handler", +} + +# DRF OpenApi schema settings +SPECTACULAR_SETTINGS = { + "TITLE": "Datatracker API", + "DESCRIPTION": "Datatracker API", + "VERSION": "1.0.0", + "SCHEMA_PATH_PREFIX": "/api/", + "COMPONENT_SPLIT_REQUEST": True, + "COMPONENT_NO_READ_ONLY_REQUIRED": True, + "SERVERS": [ + {"url": "http://localhost:8000", "description": "local dev server"}, + {"url": "https://datatracker.ietf.org", "description": "production server"}, + ], + # The following settings are needed for drf-standardized-errors + "ENUM_NAME_OVERRIDES": { + "ValidationErrorEnum": "drf_standardized_errors.openapi_serializers.ValidationErrorEnum.choices", + "ClientErrorEnum": "drf_standardized_errors.openapi_serializers.ClientErrorEnum.choices", + "ServerErrorEnum": "drf_standardized_errors.openapi_serializers.ServerErrorEnum.choices", + "ErrorCode401Enum": "drf_standardized_errors.openapi_serializers.ErrorCode401Enum.choices", + "ErrorCode403Enum": "drf_standardized_errors.openapi_serializers.ErrorCode403Enum.choices", + "ErrorCode404Enum": "drf_standardized_errors.openapi_serializers.ErrorCode404Enum.choices", + "ErrorCode405Enum": "drf_standardized_errors.openapi_serializers.ErrorCode405Enum.choices", + "ErrorCode406Enum": "drf_standardized_errors.openapi_serializers.ErrorCode406Enum.choices", + "ErrorCode415Enum": "drf_standardized_errors.openapi_serializers.ErrorCode415Enum.choices", + "ErrorCode429Enum": "drf_standardized_errors.openapi_serializers.ErrorCode429Enum.choices", + "ErrorCode500Enum": "drf_standardized_errors.openapi_serializers.ErrorCode500Enum.choices", + }, + "POSTPROCESSING_HOOKS": ["drf_standardized_errors.openapi_hooks.postprocess_schema_enums"], +} + +# DRF Standardized Errors settings +DRF_STANDARDIZED_ERRORS = { + # enable the standardized errors when DEBUG=True for unhandled exceptions. + # By default, this is set to False so you're able to view the traceback in + # the terminal and get more information about the exception. + "ENABLE_IN_DEBUG_FOR_UNHANDLED_EXCEPTIONS": False, + # ONLY the responses that correspond to these status codes will appear + # in the API schema. + "ALLOWED_ERROR_STATUS_CODES": [ + "400", + # "401", + # "403", + "404", + # "405", + # "406", + # "415", + # "429", + # "500", + ], + +} + # no slash at end IDTRACKER_BASE_URL = "https://datatracker.ietf.org" RFCDIFF_BASE_URL = "https://author-tools.ietf.org/iddiff" diff --git a/requirements.txt b/requirements.txt index 91647729be..f974113d8f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -24,8 +24,11 @@ django-stubs>=4.2.7,<5 # The django-stubs version used determines the the myp django-tastypie>=0.14.7,<0.15.0 # Version must be locked in sync with version of Django django-vite>=2.0.2,<3 django-widget-tweaks>=1.4.12 +djangorestframework>=3.15,<4 djlint>=1.0.0 # To auto-indent templates via "djlint --profile django --reformat" docutils>=0.18.1 # Used only by dbtemplates for RestructuredText +drf-spectacular>=0.27 +drf-standardized-errors[openapi] >= 0.14 types-docutils>=0.18.1 factory-boy>=3.3 github3.py>=3.2.0 From e5b6e330b18ad24f029266715186b4f8efc3c25d Mon Sep 17 00:00:00 2001 From: rjsparks Date: Wed, 27 Nov 2024 21:05:52 +0000 Subject: [PATCH 12/12] ci: update base image target version to 20241127T2054 --- dev/build/Dockerfile | 2 +- dev/build/TARGET_BASE | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/build/Dockerfile b/dev/build/Dockerfile index ef77fef58f..7af27e7d13 100644 --- a/dev/build/Dockerfile +++ b/dev/build/Dockerfile @@ -1,4 +1,4 @@ -FROM ghcr.io/ietf-tools/datatracker-app-base:20241127T0116 +FROM ghcr.io/ietf-tools/datatracker-app-base:20241127T2054 LABEL maintainer="IETF Tools Team " ENV DEBIAN_FRONTEND=noninteractive diff --git a/dev/build/TARGET_BASE b/dev/build/TARGET_BASE index 6420933c22..e4b05ed700 100644 --- a/dev/build/TARGET_BASE +++ b/dev/build/TARGET_BASE @@ -1 +1 @@ -20241127T0116 +20241127T2054