diff --git a/dev/build/Dockerfile b/dev/build/Dockerfile index c9f91163ce..7af27e7d13 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:20241127T2054 LABEL maintainer="IETF Tools Team " ENV DEBIAN_FRONTEND=noninteractive diff --git a/dev/build/TARGET_BASE b/dev/build/TARGET_BASE index 7bbb32355d..e4b05ed700 100644 --- a/dev/build/TARGET_BASE +++ b/dev/build/TARGET_BASE @@ -1 +1 @@ -20241114T1954 +20241127T2054 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) 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.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/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/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/doc/models.py b/ietf/doc/models.py index 077502db11..03698c80c3 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -4,7 +4,6 @@ import datetime import logging -import io import os import django.db @@ -530,16 +529,27 @@ def replaces(self): def replaced_by(self): return set([ r.document for r in self.related_that("replaces") ]) - def text(self, size = -1): + def _text_path(self): path = self.get_file_name() root, ext = os.path.splitext(path) txtpath = root+'.txt' if ext != '.txt' and os.path.exists(txtpath): path = txtpath + return path + + def text_exists(self): + path = Path(self._text_path()) + return path.exists() + + def text(self, size = -1): + path = Path(self._text_path()) + if not path.exists(): + return None try: - with io.open(path, 'rb') as file: + with path.open('rb') as file: raw = file.read(size) - except IOError: + except IOError as e: + log.log(f"Error reading text for {path}: {e}") return None text = None try: diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index f0c8e30626..0630fcd8d4 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -3318,3 +3318,18 @@ def test_investigate(self): self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertEqual(len(q("#id_name_fragment.is-invalid")), 1) + +class LogIOErrorTests(TestCase): + + def test_doc_text_io_error(self): + + d = IndividualDraftFactory() + + with mock.patch("ietf.doc.models.Path") as path_cls_mock: + with mock.patch("ietf.doc.models.log.log") as log_mock: + path_cls_mock.return_value.exists.return_value = True + path_cls_mock.return_value.open.return_value.__enter__.return_value.read.side_effect = IOError("Bad things happened") + text = d.text() + self.assertIsNone(text) + self.assertTrue(log_mock.called) + self.assertIn("Bad things happened", log_mock.call_args[0][0]) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index a30430829a..b2bc0997b1 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -1081,7 +1081,7 @@ def build_file_urls(doc: Union[Document, DocHistory]): label = "plain text" if t == "txt" else t file_urls.append((label, base + doc.name + "-" + doc.rev + "." + t)) - if doc.text(): + if doc.text_exists(): file_urls.append(("htmlized", urlreverse('ietf.doc.views_doc.document_html', kwargs=dict(name=doc.name, rev=doc.rev)))) file_urls.append(("pdfized", urlreverse('ietf.doc.views_doc.document_pdfized', kwargs=dict(name=doc.name, rev=doc.rev)))) file_urls.append(("bibtex", urlreverse('ietf.doc.views_doc.document_bibtex',kwargs=dict(name=doc.name,rev=doc.rev)))) 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 30f4b367c6..cf8abe9f4d 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" @@ -1076,11 +1149,14 @@ def skip_unreadable_post(record): MARKUP_SETTINGS = { 'restructuredtext': { 'settings_overrides': { + 'report_level': 3, # error (3) or severe (4) only 'initial_header_level': 3, 'doctitle_xform': False, 'footnote_references': 'superscript', 'trim_footnote_reference_space': True, 'default_reference_context': 'view', + 'raw_enabled': False, # critical for security + 'file_insertion_enabled': False, # critical for security 'link_base': '' } } @@ -1273,6 +1349,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 diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index bed87b77c2..183e081242 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -502,7 +502,7 @@ def clean_name(self): return name class ReplacesForm(forms.Form): - replaces = SearchableDocumentsField(required=False, help_text="Any Internet-Drafts that this document replaces (approval required for replacing an Internet-Draft you are not the author of)") + replaces = SearchableDocumentsField(required=False, help_text='Do not enter anything here if you are only submitting the next version of your Internet-Draft. Only enter items here if this submission is intended to replace an I-D with a different name. A typical use of this field is to note what individual I-Ds are replaced by a new -00 Working group I-D. Note that additional approval will be required to replace an I-D for which you are not an author.') def __init__(self, *args, **kwargs): self.name = kwargs.pop("name") 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"]) 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) 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') }) diff --git a/requirements.txt b/requirements.txt index 5c80d5301e..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 @@ -39,7 +42,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