From 3130ecd9f56f6b9b0de25b1e832e8d80c50a3c2d Mon Sep 17 00:00:00 2001 From: Jennifer Richards <jennifer@staff.ietf.org> Date: Fri, 25 Oct 2024 10:52:09 -0300 Subject: [PATCH] fix: add recording-name api key endpoint; appauth url fix (#8081) * fix: add endpoint option for recording-name * chore: migration * test: validate PersonalApiKey when used in tests * fix: limit /api/appauth URLs as intended * test: fix tests * chore: fix lint * test: PersonalApiKey create -> factory * chore: remove unused import --- ietf/api/tests.py | 87 ++++++++++--------- ietf/api/urls.py | 2 +- ietf/api/views.py | 4 +- ietf/doc/tests_ballot.py | 6 +- ietf/ietfauth/tests.py | 13 +-- ietf/meeting/tests_views.py | 6 +- ietf/person/factories.py | 16 +++- .../0003_alter_personalapikey_endpoint.py | 42 +++++++++ ietf/person/models.py | 1 + 9 files changed, 118 insertions(+), 59 deletions(-) create mode 100644 ietf/person/migrations/0003_alter_personalapikey_endpoint.py diff --git a/ietf/api/tests.py b/ietf/api/tests.py index 97e928dc5..7dc9c42cc 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -33,9 +33,8 @@ from ietf.meeting.factories import MeetingFactory, SessionFactory from ietf.meeting.models import Session from ietf.nomcom.models import Volunteer from ietf.nomcom.factories import NomComFactory, nomcom_kwargs_for_year -from ietf.person.factories import PersonFactory, random_faker, EmailFactory +from ietf.person.factories import PersonFactory, random_faker, EmailFactory, PersonalApiKeyFactory from ietf.person.models import Email, User -from ietf.person.models import PersonalApiKey from ietf.stats.models import MeetingRegistration from ietf.utils.mail import empty_outbox, outbox, get_payload_text from ietf.utils.models import DumpInfo @@ -71,7 +70,7 @@ class CustomApiTests(TestCase): meeting = MeetingFactory(type_id='ietf') session = SessionFactory(group__type_id='wg', meeting=meeting) group = session.group - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) video = 'https://foo.example.com/bar/beer/' # error cases @@ -79,7 +78,7 @@ class CustomApiTests(TestCase): self.assertContains(r, "Missing apikey parameter", status_code=400) badrole = RoleFactory(group__type_id='ietf', name_id='ad') - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() r = self.client.post(url, {'apikey': badapikey.hash()} ) @@ -151,7 +150,7 @@ class CustomApiTests(TestCase): recman = recmanrole.person meeting = MeetingFactory(type_id="ietf") session = SessionFactory(group__type_id="wg", meeting=meeting) - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) video = "https://foo.example.com/bar/beer/" # error cases @@ -159,7 +158,7 @@ class CustomApiTests(TestCase): self.assertContains(r, "Missing apikey parameter", status_code=400) badrole = RoleFactory(group__type_id="ietf", name_id="ad") - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() r = self.client.post(url, {"apikey": badapikey.hash()}) @@ -228,7 +227,7 @@ class CustomApiTests(TestCase): recman = recmanrole.person meeting = MeetingFactory(type_id="ietf") session = SessionFactory(group__type_id="wg", meeting=meeting) - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) name = "testname" # error cases @@ -236,7 +235,7 @@ class CustomApiTests(TestCase): self.assertContains(r, "Missing apikey parameter", status_code=400) badrole = RoleFactory(group__type_id="ietf", name_id="ad") - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() r = self.client.post(url, {"apikey": badapikey.hash()}) @@ -295,10 +294,10 @@ class CustomApiTests(TestCase): recman = recmanrole.person meeting = MeetingFactory(type_id='ietf') session = SessionFactory(group__type_id='wg', meeting=meeting) - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) badrole = RoleFactory(group__type_id='ietf', name_id='ad') - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() @@ -361,10 +360,10 @@ class CustomApiTests(TestCase): recman = recmanrole.person meeting = MeetingFactory(type_id="ietf") session = SessionFactory(group__type_id="wg", meeting=meeting) - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) badrole = RoleFactory(group__type_id="ietf", name_id="ad") - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() @@ -517,8 +516,8 @@ class CustomApiTests(TestCase): ), ): url = urlreverse(f"ietf.meeting.views.api_upload_{type_id}") - apikey = PersonalApiKey.objects.create(endpoint=url, person=recmanrole.person) - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + apikey = PersonalApiKeyFactory(endpoint=url, person=recmanrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) r = self.client.post(url, {}) self.assertContains(r, "Missing apikey parameter", status_code=400) @@ -562,7 +561,7 @@ class CustomApiTests(TestCase): meeting = MeetingFactory(type_id='ietf') session = SessionFactory(group__type_id='wg', meeting=meeting) group = session.group - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) people = [ {"name": "Andrea Andreotti", "affiliation": "Azienda"}, @@ -579,7 +578,7 @@ class CustomApiTests(TestCase): self.assertContains(r, "Missing apikey parameter", status_code=400) badrole = RoleFactory(group__type_id='ietf', name_id='ad') - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() r = self.client.post(url, {'apikey': badapikey.hash()}) @@ -654,7 +653,7 @@ class CustomApiTests(TestCase): meeting = MeetingFactory(type_id="ietf") session = SessionFactory(group__type_id="wg", meeting=meeting) group = session.group - apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=url, person=recman) people = [ {"name": "Andrea Andreotti", "affiliation": "Azienda"}, @@ -671,7 +670,7 @@ class CustomApiTests(TestCase): self.assertContains(r, "Missing apikey parameter", status_code=400) badrole = RoleFactory(group__type_id="ietf", name_id="ad") - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() r = self.client.post(url, {"apikey": badapikey.hash()}) @@ -781,14 +780,14 @@ class CustomApiTests(TestCase): url = urlreverse('ietf.api.views.ApiV2PersonExportView') robot = PersonFactory(user__is_staff=True) RoleFactory(name_id='robot', person=robot, email=robot.email(), group__acronym='secretariat') - apikey = PersonalApiKey.objects.create(endpoint=url, person=robot) + apikey = PersonalApiKeyFactory(endpoint=url, person=robot) # error cases r = self.client.post(url, {}) self.assertContains(r, "Missing apikey parameter", status_code=400) badrole = RoleFactory(group__type_id='ietf', name_id='ad') - badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badapikey = PersonalApiKeyFactory(endpoint=url, person=badrole.person) badrole.person.user.last_login = timezone.now() badrole.person.user.save() r = self.client.post(url, {'apikey': badapikey.hash()}) @@ -827,7 +826,7 @@ class CustomApiTests(TestCase): oidcp = PersonFactory(user__is_staff=True) # Make sure 'oidcp' has an acceptable role RoleFactory(name_id='robot', person=oidcp, email=oidcp.email(), group__acronym='secretariat') - key = PersonalApiKey.objects.create(person=oidcp, endpoint=url) + key = PersonalApiKeyFactory(person=oidcp, endpoint=url) reg['apikey'] = key.hash() # # Test valid POST @@ -911,7 +910,7 @@ class CustomApiTests(TestCase): oidcp = PersonFactory(user__is_staff=True) # Make sure 'oidcp' has an acceptable role RoleFactory(name_id='robot', person=oidcp, email=oidcp.email(), group__acronym='secretariat') - key = PersonalApiKey.objects.create(person=oidcp, endpoint=url) + key = PersonalApiKeyFactory(person=oidcp, endpoint=url) reg['apikey'] = key.hash() # first test is_nomcom_volunteer False @@ -945,28 +944,30 @@ class CustomApiTests(TestCase): def test_api_appauth(self): - url = urlreverse('ietf.api.views.app_auth') - person = PersonFactory() - apikey = PersonalApiKey.objects.create(endpoint=url, person=person) - - self.client.login(username=person.user.username,password=f'{person.user.username}+password') - self.client.logout() - - # error cases - # missing apikey - r = self.client.post(url, {}) - self.assertContains(r, 'Missing apikey parameter', status_code=400) - - # invalid apikey - r = self.client.post(url, {'apikey': 'foobar'}) - self.assertContains(r, 'Invalid apikey', status_code=403) - - # working case - r = self.client.post(url, {'apikey': apikey.hash()}) - self.assertEqual(r.status_code, 200) - jsondata = r.json() - self.assertEqual(jsondata['success'], True) + for app in ["authortools", "bibxml"]: + url = urlreverse('ietf.api.views.app_auth', kwargs={"app": app}) + person = PersonFactory() + apikey = PersonalApiKeyFactory(endpoint=url, person=person) + self.client.login(username=person.user.username,password=f'{person.user.username}+password') + self.client.logout() + + # error cases + # missing apikey + r = self.client.post(url, {}) + self.assertContains(r, 'Missing apikey parameter', status_code=400) + + # invalid apikey + r = self.client.post(url, {'apikey': 'foobar'}) + self.assertContains(r, 'Invalid apikey', status_code=403) + + # working case + r = self.client.post(url, {'apikey': apikey.hash()}) + self.assertEqual(r.status_code, 200) + jsondata = r.json() + self.assertEqual(jsondata['success'], True) + self.client.logout() + def test_api_get_session_matherials_no_agenda_meeting_url(self): meeting = MeetingFactory(type_id='ietf') session = SessionFactory(meeting=meeting) diff --git a/ietf/api/urls.py b/ietf/api/urls.py index 431ad5c5d..6846e3274 100644 --- a/ietf/api/urls.py +++ b/ietf/api/urls.py @@ -69,7 +69,7 @@ urlpatterns = [ # Datatracker version url(r'^version/?$', api_views.version), # Application authentication API key - url(r'^appauth/[authortools|bibxml]', api_views.app_auth), + url(r'^appauth/(?P<app>authortools|bibxml)$', api_views.app_auth), # latest versions url(r'^rfcdiff-latest-json/%(name)s(?:-%(rev)s)?(\.txt|\.html)?/?$' % settings.URL_REGEXPS, api_views.rfcdiff_latest_json), url(r'^rfcdiff-latest-json/(?P<name>[Rr][Ff][Cc] [0-9]+?)(\.txt|\.html)?/?$', api_views.rfcdiff_latest_json), diff --git a/ietf/api/views.py b/ietf/api/views.py index f8662f9a0..df67cffd5 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -30,7 +30,7 @@ from tastypie.utils import is_valid_jsonp_callback_value from tastypie.utils.mime import determine_format, build_content_type from textwrap import dedent from traceback import format_exception, extract_tb -from typing import Iterable, Optional +from typing import Iterable, Optional, Literal import ietf from ietf.api import _api_list @@ -251,7 +251,7 @@ def version(request): @require_api_key @csrf_exempt -def app_auth(request): +def app_auth(request, app: Literal["authortools", "bibxml"]): return HttpResponse( json.dumps({'success': True}), content_type='application/json') diff --git a/ietf/doc/tests_ballot.py b/ietf/doc/tests_ballot.py index f95ba8812..c7362b58e 100644 --- a/ietf/doc/tests_ballot.py +++ b/ietf/doc/tests_ballot.py @@ -27,8 +27,8 @@ from ietf.group.factories import GroupFactory, RoleFactory, ReviewTeamFactory from ietf.ipr.factories import HolderIprDisclosureFactory from ietf.name.models import BallotPositionName from ietf.iesg.models import TelechatDate -from ietf.person.models import Person, PersonalApiKey -from ietf.person.factories import PersonFactory +from ietf.person.models import Person +from ietf.person.factories import PersonFactory, PersonalApiKeyFactory from ietf.person.utils import get_active_ads from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox, empty_outbox, get_payload_text @@ -111,7 +111,7 @@ class EditPositionTests(TestCase): create_ballot_if_not_open(None, draft, ad, 'approve') ad.user.last_login = timezone.now() ad.user.save() - apikey = PersonalApiKey.objects.create(endpoint=url, person=ad) + apikey = PersonalApiKeyFactory(endpoint=url, person=ad) # vote events_before = draft.docevent_set.count() diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 722c1e8b6..fed764e8b 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -35,7 +35,7 @@ from ietf.ietfauth.utils import has_role from ietf.meeting.factories import MeetingFactory from ietf.nomcom.factories import NomComFactory from ietf.person.factories import PersonFactory, EmailFactory, UserFactory, PersonalApiKeyFactory -from ietf.person.models import Person, Email, PersonalApiKey +from ietf.person.models import Person, Email from ietf.person.tasks import send_apikey_usage_emails_task from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory from ietf.review.models import ReviewWish, UnavailablePeriod @@ -788,9 +788,8 @@ class IetfAuthTests(TestCase): self.assertContains(r, 'Invalid apikey', status_code=403) # invalid apikey (invalidated api key) - unauthorized_url = urlreverse('ietf.api.views.app_auth') - invalidated_apikey = PersonalApiKey.objects.create( - endpoint=unauthorized_url, person=person, valid=False) + unauthorized_url = urlreverse('ietf.api.views.app_auth', kwargs={'app': 'authortools'}) + invalidated_apikey = PersonalApiKeyFactory(endpoint=unauthorized_url, person=person, valid=False) r = self.client.post(unauthorized_url, {'apikey': invalidated_apikey.hash()}) self.assertContains(r, 'Invalid apikey', status_code=403) @@ -803,7 +802,11 @@ class IetfAuthTests(TestCase): person.user.save() # endpoint mismatch - key2 = PersonalApiKey.objects.create(person=person, endpoint='/') + key2 = PersonalApiKeyFactory( + person=person, + endpoint='/', + validate_model=False, # allow invalid endpoint + ) r = self.client.post(key.endpoint, {'apikey':key2.hash(), 'dummy':'dummy',}) self.assertContains(r, 'Apikey endpoint mismatch', status_code=400) key2.delete() diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index 642edcb9b..0d291f12b 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -38,7 +38,7 @@ import debug # pyflakes:ignore from ietf.doc.models import Document, NewRevisionDocEvent from ietf.group.models import Group, Role, GroupFeatures from ietf.group.utils import can_manage_group -from ietf.person.models import Person, PersonalApiKey +from ietf.person.models import Person from ietf.meeting.helpers import can_approve_interim_request, can_request_interim_meeting, can_view_interim_request, preprocess_assignments_for_agenda from ietf.meeting.helpers import send_interim_approval_request, AgendaKeywordTagger from ietf.meeting.helpers import send_interim_meeting_cancellation_notice, send_interim_session_cancellation_notice @@ -56,7 +56,7 @@ from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.test_utils import TestCase, login_testing_unauthorized, unicontent from ietf.utils.timezone import date_today, time_now -from ietf.person.factories import PersonFactory +from ietf.person.factories import PersonFactory, PersonalApiKeyFactory from ietf.group.factories import GroupFactory, GroupEventFactory, RoleFactory from ietf.meeting.factories import (SessionFactory, ScheduleFactory, SessionPresentationFactory, MeetingFactory, FloorPlanFactory, @@ -8743,7 +8743,7 @@ class ProceedingsTests(BaseMeetingTestCase): add_attendees_url = urlreverse('ietf.meeting.views.api_add_session_attendees') recmanrole = RoleFactory(group__type_id='ietf', name_id='recman', person__user__last_login=timezone.now()) recman = recmanrole.person - apikey = PersonalApiKey.objects.create(endpoint=add_attendees_url, person=recman) + apikey = PersonalApiKeyFactory(endpoint=add_attendees_url, person=recman) attendees = [person.user.pk for person in persons] self.client.login(username='recman', password='recman+password') r = self.client.post(add_attendees_url, {'apikey':apikey.hash(), 'attended':f'{{"session_id":{session.pk},"attendees":{attendees}}}'}) diff --git a/ietf/person/factories.py b/ietf/person/factories.py index 2012483c0..45de55476 100644 --- a/ietf/person/factories.py +++ b/ietf/person/factories.py @@ -158,10 +158,22 @@ class EmailFactory(factory.django.DjangoModelFactory): class PersonalApiKeyFactory(factory.django.DjangoModelFactory): person = factory.SubFactory(PersonFactory) - endpoint = FuzzyChoice(PERSON_API_KEY_ENDPOINTS) - + endpoint = FuzzyChoice(v for v, n in PERSON_API_KEY_ENDPOINTS) + class Meta: model = PersonalApiKey + skip_postgeneration_save = True + + @factory.post_generation + def validate_model(obj, create, extracted, **kwargs): + """Validate the model after creation + + Passing validate_model=False will disable the validation. + """ + do_clean = True if extracted is None else extracted + if do_clean: + obj.full_clean() + class PersonApiKeyEventFactory(factory.django.DjangoModelFactory): key = factory.SubFactory(PersonalApiKeyFactory) diff --git a/ietf/person/migrations/0003_alter_personalapikey_endpoint.py b/ietf/person/migrations/0003_alter_personalapikey_endpoint.py new file mode 100644 index 000000000..202af4b10 --- /dev/null +++ b/ietf/person/migrations/0003_alter_personalapikey_endpoint.py @@ -0,0 +1,42 @@ +# Generated by Django 4.2.16 on 2024-10-24 21:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("person", "0002_alter_historicalperson_ascii_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="personalapikey", + name="endpoint", + field=models.CharField( + choices=[ + ("/api/appauth/authortools", "/api/appauth/authortools"), + ("/api/appauth/bibxml", "/api/appauth/bibxml"), + ("/api/iesg/position", "/api/iesg/position"), + ( + "/api/meeting/session/recording-name", + "/api/meeting/session/recording-name", + ), + ( + "/api/meeting/session/video/url", + "/api/meeting/session/video/url", + ), + ("/api/notify/meeting/bluesheet", "/api/notify/meeting/bluesheet"), + ( + "/api/notify/meeting/registration", + "/api/notify/meeting/registration", + ), + ("/api/notify/session/attendees", "/api/notify/session/attendees"), + ("/api/notify/session/chatlog", "/api/notify/session/chatlog"), + ("/api/notify/session/polls", "/api/notify/session/polls"), + ("/api/v2/person/person", "/api/v2/person/person"), + ], + max_length=128, + ), + ), + ] diff --git a/ietf/person/models.py b/ietf/person/models.py index 0c2515236..85989acfc 100644 --- a/ietf/person/models.py +++ b/ietf/person/models.py @@ -376,6 +376,7 @@ PERSON_API_KEY_VALUES = [ ("/api/iesg/position", "/api/iesg/position", "Area Director"), ("/api/v2/person/person", "/api/v2/person/person", "Robot"), ("/api/meeting/session/video/url", "/api/meeting/session/video/url", "Recording Manager"), + ("/api/meeting/session/recording-name", "/api/meeting/session/recording-name", "Recording Manager"), ("/api/notify/meeting/registration", "/api/notify/meeting/registration", "Robot"), ("/api/notify/meeting/bluesheet", "/api/notify/meeting/bluesheet", "Recording Manager"), ("/api/notify/session/attendees", "/api/notify/session/attendees", "Recording Manager"),