From 8af595cb7969fba40af9cfbbfd41681405ff4e50 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 12:55:23 +0200 Subject: [PATCH 01/15] api.schedule: add C3VOCPublishingWebhook --- apps/api/schedule.py | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index f775d8d72..4c8af82f3 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -7,6 +7,7 @@ from . import api from main import db +from models import event_year from models.cfp import Proposal from models.ical import CalendarEvent from models.admin_message import AdminMessage @@ -169,6 +170,56 @@ def post(self, proposal_type): return [t.id for t in res] +class C3VOCPublishingWebhook(Resource): + method_decorators = {"post": [_require_video_api_key]} + + def post(self): + if not request.is_json: + abort(415) + + payload = request.get_json() + + try: + conference = payload["fahrplan"]["conference"] + proposal_id = payload["fahrplan"]["id"] + except KeyError: + abort(422) + + if not payload["is_master"]: + # c3voc *should* only send us information about the master + # encoding getting published. Aborting early ensures we don't + # accidentially delete video information from the database. + abort(406) + + if conference != f"emf{event_year()}": + abort(422) + + proposal = Proposal.query.get_or_404(proposal_id) + + if payload["voctoweb"]["enabled"]: + proposal.c3voc_url = payload["voctoweb"]["frontend_url"] + proposal.video_recording_lost = False + else: + # This allows c3voc to notify us if videos got depublished + # as well. We do not explicitely set 'video_recording_lost' + # here because the video might only need fixing audio or + # such. + proposal.c3voc_url = "" + + if payload["youtube"]["enabled"]: + # c3voc will send us a list, even though we only have one + # video. + proposal.youtube_url = payload["youtube"]["urls"][0] + proposal.video_recording_lost = False + else: + proposal.youtube_url = "" + + db.session.add(proposal) + db.session.commit() + + return "OK", 204 + + def renderScheduleMessage(message): return {"id": message.id, "body": message.message} @@ -186,3 +237,4 @@ def get(self): api.add_resource(FavouriteExternal, "/external//favourite") api.add_resource(ScheduleMessage, "/schedule_messages") api.add_resource(UpdateLotteryPreferences, "/schedule/tickets//preferences") +api.add_resource(C3VOCPublishingWebhook, "/c3voc/publishing-webhook") From 6b9c73aa9844cec6dcc3551cd7ad4708e5c96c0d Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 13:55:19 +0200 Subject: [PATCH 02/15] =?UTF-8?q?api.schedule.C=E2=84=93VOCPublishingWebho?= =?UTF-8?q?ok:=20delete=20urls=20if=20we=20get=20empty=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/api/schedule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 4c8af82f3..99a260d43 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -196,7 +196,7 @@ def post(self): proposal = Proposal.query.get_or_404(proposal_id) - if payload["voctoweb"]["enabled"]: + if payload["voctoweb"]["enabled"] and payload["voctoweb"]["frontend_url"]: proposal.c3voc_url = payload["voctoweb"]["frontend_url"] proposal.video_recording_lost = False else: @@ -206,7 +206,7 @@ def post(self): # such. proposal.c3voc_url = "" - if payload["youtube"]["enabled"]: + if payload["youtube"]["enabled"] and payload["youtube"]["urls"]: # c3voc will send us a list, even though we only have one # video. proposal.youtube_url = payload["youtube"]["urls"][0] From 468a479d7cb3c94cc611cf4f4fd3c74b8b334af7 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 15:06:23 +0200 Subject: [PATCH 03/15] api.schedule: rename ProposalC3VOCPublishingWebhook --- apps/api/schedule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 99a260d43..e99faaeb0 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -170,7 +170,7 @@ def post(self, proposal_type): return [t.id for t in res] -class C3VOCPublishingWebhook(Resource): +class ProposalC3VOCPublishingWebhook(Resource): method_decorators = {"post": [_require_video_api_key]} def post(self): @@ -237,4 +237,4 @@ def get(self): api.add_resource(FavouriteExternal, "/external//favourite") api.add_resource(ScheduleMessage, "/schedule_messages") api.add_resource(UpdateLotteryPreferences, "/schedule/tickets//preferences") -api.add_resource(C3VOCPublishingWebhook, "/c3voc/publishing-webhook") +api.add_resource(ProposalC3VOCPublishingWebhook, "/proposal/c3voc-publishing-webhook") From 22a9e70c15b09f744d080233cfd030161945b591 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 15:27:18 +0200 Subject: [PATCH 04/15] api.schedule.ProposalC3VOCPublishingWebhook: abort with 403 instead of 406 --- apps/api/schedule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index e99faaeb0..e0943e904 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -189,7 +189,7 @@ def post(self): # c3voc *should* only send us information about the master # encoding getting published. Aborting early ensures we don't # accidentially delete video information from the database. - abort(406) + abort(403) if conference != f"emf{event_year()}": abort(422) From 1cd71893727fd3c5c5a908a0759ec835e1bbbf8f Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 18:25:18 +0200 Subject: [PATCH 05/15] api.schedule.ProposalC3VOCPublishingWebhook: better logic on when (not) to overwrite values --- apps/api/schedule.py | 47 ++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index e0943e904..8d9f98df5 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -196,23 +196,36 @@ def post(self): proposal = Proposal.query.get_or_404(proposal_id) - if payload["voctoweb"]["enabled"] and payload["voctoweb"]["frontend_url"]: - proposal.c3voc_url = payload["voctoweb"]["frontend_url"] - proposal.video_recording_lost = False - else: - # This allows c3voc to notify us if videos got depublished - # as well. We do not explicitely set 'video_recording_lost' - # here because the video might only need fixing audio or - # such. - proposal.c3voc_url = "" - - if payload["youtube"]["enabled"] and payload["youtube"]["urls"]: - # c3voc will send us a list, even though we only have one - # video. - proposal.youtube_url = payload["youtube"]["urls"][0] - proposal.video_recording_lost = False - else: - proposal.youtube_url = "" + if payload["voctoweb"]["enabled"]: + if payload["voctoweb"]["frontend_url"]: + proposal.c3voc_url = payload["voctoweb"]["frontend_url"] + proposal.video_recording_lost = False + else: + # This allows c3voc to notify us if videos got depublished + # as well. We do not explicitely set 'video_recording_lost' + # here because the video might only need fixing audio or + # such. + proposal.c3voc_url = "" + + if payload["youtube"]["enabled"]: + if payload["youtube"]["urls"]: + # Please do not overwrite existing youtube urls + if not proposal.youtube_url: + # c3voc will send us a list, even though we only have one + # video. + proposal.youtube_url = payload["youtube"]["urls"][0] + proposal.video_recording_lost = False + elif proposal.youtube_url not in payload["youtube"]["urls"]: + # c3voc sent us some urls, but none of them are matching + # the url we have in our database. + app.logger.warning( + f"C3VOC webhook sent youtube urls {payload['youtube']['urls']!r}, " + f"but we already have {proposal.youtube_url}. NOT " + "overwriting!" + ) + else: + # see comment at c3voc_url above + proposal.youtube_url = "" db.session.add(proposal) db.session.commit() From aa3f9dba6e3f7524144b6db88bb32d0a60ef0984 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 19:49:28 +0200 Subject: [PATCH 06/15] api.schedule.ProposalC3VOCPublishingWebhook: add a bit of input validation --- apps/api/schedule.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 8d9f98df5..31298dee8 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -198,6 +198,8 @@ def post(self): if payload["voctoweb"]["enabled"]: if payload["voctoweb"]["frontend_url"]: + if not payload["voctoweb"]["frontend_url"].startswith('https://media.ccc.de/'): + abort(406, message="voctoweb frontend_url must start with https://media.ccc.de/") proposal.c3voc_url = payload["voctoweb"]["frontend_url"] proposal.video_recording_lost = False else: @@ -211,6 +213,9 @@ def post(self): if payload["youtube"]["urls"]: # Please do not overwrite existing youtube urls if not proposal.youtube_url: + youtube_url = payload["youtube"]["urls"][0] + if not youtube_url.startswith('https://www.youtube.com/watch'): + abort(406, message="youtube url must start with https://www.youtube.com/watch") # c3voc will send us a list, even though we only have one # video. proposal.youtube_url = payload["youtube"]["urls"][0] From 90dcc5d183e74309372c4283ec33879bd00e3d7a Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 19:58:52 +0200 Subject: [PATCH 07/15] api.schedule.ProposalC3VOCPublishingWebhook: add some logging --- apps/api/schedule.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 31298dee8..0b6b9b5db 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -198,15 +198,18 @@ def post(self): if payload["voctoweb"]["enabled"]: if payload["voctoweb"]["frontend_url"]: - if not payload["voctoweb"]["frontend_url"].startswith('https://media.ccc.de/'): + c3voc_url = payload["voctoweb"]["frontend_url"] + if not c3voc_url.startswith('https://media.ccc.de/'): abort(406, message="voctoweb frontend_url must start with https://media.ccc.de/") - proposal.c3voc_url = payload["voctoweb"]["frontend_url"] + app.logger.info(f"C3VOC webhook set c3voc_url for {proposal.id=} to {c3voc_url}") + proposal.c3voc_url = c3voc_url proposal.video_recording_lost = False else: # This allows c3voc to notify us if videos got depublished # as well. We do not explicitely set 'video_recording_lost' # here because the video might only need fixing audio or # such. + app.logger.warning(f"C3VOC webhook cleared c3voc_url for {proposal.id=}, was {proposal.c3voc_url}") proposal.c3voc_url = "" if payload["youtube"]["enabled"]: @@ -218,18 +221,19 @@ def post(self): abort(406, message="youtube url must start with https://www.youtube.com/watch") # c3voc will send us a list, even though we only have one # video. - proposal.youtube_url = payload["youtube"]["urls"][0] + app.logger.info(f"C3VOC webhook set youtube_url for {proposal.id=} to {youtube_url}") + proposal.youtube_url = youtube_url proposal.video_recording_lost = False elif proposal.youtube_url not in payload["youtube"]["urls"]: # c3voc sent us some urls, but none of them are matching # the url we have in our database. app.logger.warning( - f"C3VOC webhook sent youtube urls {payload['youtube']['urls']!r}, " - f"but we already have {proposal.youtube_url}. NOT " - "overwriting!" + "C3VOC webhook sent youtube urls update without referencing the previously stored value. Ignoring." ) + app.logger.debug(f"{proposal.id=} {payload['youtube']['urls']=} {proposal.youtube_url=}") else: # see comment at c3voc_url above + app.logger.warning(f"C3VOC webhook cleared youtube_url for {proposal.id=}, was {proposal.youtube_url}") proposal.youtube_url = "" db.session.add(proposal) From fc246cddbe59571c1cbb034df51c44ca52efede0 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 20:10:09 +0200 Subject: [PATCH 08/15] api.schedule.ProposalC3VOCPublishingWebhook: wrap everything in `try..except` to handle KeyErrors --- apps/api/schedule.py | 102 +++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 0b6b9b5db..4feafeb5b 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -182,62 +182,62 @@ def post(self): try: conference = payload["fahrplan"]["conference"] proposal_id = payload["fahrplan"]["id"] - except KeyError: - abort(422) - if not payload["is_master"]: - # c3voc *should* only send us information about the master - # encoding getting published. Aborting early ensures we don't - # accidentially delete video information from the database. - abort(403) + if not payload["is_master"]: + # c3voc *should* only send us information about the master + # encoding getting published. Aborting early ensures we don't + # accidentially delete video information from the database. + abort(403, message="The request referenced a non-master video edit, and has been denied.") - if conference != f"emf{event_year()}": - abort(422) + if conference != f"emf{event_year()}": + abort(422, message="The request did not reference the current event year, and has not been processed.") - proposal = Proposal.query.get_or_404(proposal_id) + proposal = Proposal.query.get_or_404(proposal_id) - if payload["voctoweb"]["enabled"]: - if payload["voctoweb"]["frontend_url"]: - c3voc_url = payload["voctoweb"]["frontend_url"] - if not c3voc_url.startswith('https://media.ccc.de/'): - abort(406, message="voctoweb frontend_url must start with https://media.ccc.de/") - app.logger.info(f"C3VOC webhook set c3voc_url for {proposal.id=} to {c3voc_url}") - proposal.c3voc_url = c3voc_url - proposal.video_recording_lost = False - else: - # This allows c3voc to notify us if videos got depublished - # as well. We do not explicitely set 'video_recording_lost' - # here because the video might only need fixing audio or - # such. - app.logger.warning(f"C3VOC webhook cleared c3voc_url for {proposal.id=}, was {proposal.c3voc_url}") - proposal.c3voc_url = "" - - if payload["youtube"]["enabled"]: - if payload["youtube"]["urls"]: - # Please do not overwrite existing youtube urls - if not proposal.youtube_url: - youtube_url = payload["youtube"]["urls"][0] - if not youtube_url.startswith('https://www.youtube.com/watch'): - abort(406, message="youtube url must start with https://www.youtube.com/watch") - # c3voc will send us a list, even though we only have one - # video. - app.logger.info(f"C3VOC webhook set youtube_url for {proposal.id=} to {youtube_url}") - proposal.youtube_url = youtube_url + if payload["voctoweb"]["enabled"]: + if payload["voctoweb"]["frontend_url"]: + c3voc_url = payload["voctoweb"]["frontend_url"] + if not c3voc_url.startswith('https://media.ccc.de/'): + abort(406, message="voctoweb frontend_url must start with https://media.ccc.de/") + app.logger.info(f"C3VOC webhook set c3voc_url for {proposal.id=} to {c3voc_url}") + proposal.c3voc_url = c3voc_url proposal.video_recording_lost = False - elif proposal.youtube_url not in payload["youtube"]["urls"]: - # c3voc sent us some urls, but none of them are matching - # the url we have in our database. - app.logger.warning( - "C3VOC webhook sent youtube urls update without referencing the previously stored value. Ignoring." - ) - app.logger.debug(f"{proposal.id=} {payload['youtube']['urls']=} {proposal.youtube_url=}") - else: - # see comment at c3voc_url above - app.logger.warning(f"C3VOC webhook cleared youtube_url for {proposal.id=}, was {proposal.youtube_url}") - proposal.youtube_url = "" - - db.session.add(proposal) - db.session.commit() + else: + # This allows c3voc to notify us if videos got depublished + # as well. We do not explicitely set 'video_recording_lost' + # here because the video might only need fixing audio or + # such. + app.logger.warning(f"C3VOC webhook cleared c3voc_url for {proposal.id=}, was {proposal.c3voc_url}") + proposal.c3voc_url = "" + + if payload["youtube"]["enabled"]: + if payload["youtube"]["urls"]: + # Please do not overwrite existing youtube urls + if not proposal.youtube_url: + youtube_url = payload["youtube"]["urls"][0] + if not youtube_url.startswith('https://www.youtube.com/watch'): + abort(406, message="youtube url must start with https://www.youtube.com/watch") + # c3voc will send us a list, even though we only have one + # video. + app.logger.info(f"C3VOC webhook set youtube_url for {proposal.id=} to {youtube_url}") + proposal.youtube_url = youtube_url + proposal.video_recording_lost = False + elif proposal.youtube_url not in payload["youtube"]["urls"]: + # c3voc sent us some urls, but none of them are matching + # the url we have in our database. + app.logger.warning( + "C3VOC webhook sent youtube urls update without referencing the previously stored value. Ignoring." + ) + app.logger.debug(f"{proposal.id=} {payload['youtube']['urls']=} {proposal.youtube_url=}") + else: + # see comment at c3voc_url above + app.logger.warning(f"C3VOC webhook cleared youtube_url for {proposal.id=}, was {proposal.youtube_url}") + proposal.youtube_url = "" + + db.session.add(proposal) + db.session.commit() + except KeyError as e: + abort(400, message=f"Missing required field: {e}") return "OK", 204 From b85781376d32706611a46f1c75d4d9b7e18990fb Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 21:40:12 +0200 Subject: [PATCH 09/15] apps.api.schedule: reformat using `ruff format` --- apps/api/schedule.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 4feafeb5b..cbed73cd0 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -148,9 +148,7 @@ def post(self, proposal_type): current_tickets = { t.id: t - for t in EventTicket.query.filter_by( - state="entered-lottery", user_id=current_user.id - ).all() + for t in EventTicket.query.filter_by(state="entered-lottery", user_id=current_user.id).all() if t.proposal.type == proposal_type } @@ -190,14 +188,17 @@ def post(self): abort(403, message="The request referenced a non-master video edit, and has been denied.") if conference != f"emf{event_year()}": - abort(422, message="The request did not reference the current event year, and has not been processed.") + abort( + 422, + message="The request did not reference the current event year, and has not been processed.", + ) proposal = Proposal.query.get_or_404(proposal_id) if payload["voctoweb"]["enabled"]: if payload["voctoweb"]["frontend_url"]: c3voc_url = payload["voctoweb"]["frontend_url"] - if not c3voc_url.startswith('https://media.ccc.de/'): + if not c3voc_url.startswith("https://media.ccc.de/"): abort(406, message="voctoweb frontend_url must start with https://media.ccc.de/") app.logger.info(f"C3VOC webhook set c3voc_url for {proposal.id=} to {c3voc_url}") proposal.c3voc_url = c3voc_url @@ -207,7 +208,9 @@ def post(self): # as well. We do not explicitely set 'video_recording_lost' # here because the video might only need fixing audio or # such. - app.logger.warning(f"C3VOC webhook cleared c3voc_url for {proposal.id=}, was {proposal.c3voc_url}") + app.logger.warning( + f"C3VOC webhook cleared c3voc_url for {proposal.id=}, was {proposal.c3voc_url}" + ) proposal.c3voc_url = "" if payload["youtube"]["enabled"]: @@ -215,7 +218,7 @@ def post(self): # Please do not overwrite existing youtube urls if not proposal.youtube_url: youtube_url = payload["youtube"]["urls"][0] - if not youtube_url.startswith('https://www.youtube.com/watch'): + if not youtube_url.startswith("https://www.youtube.com/watch"): abort(406, message="youtube url must start with https://www.youtube.com/watch") # c3voc will send us a list, even though we only have one # video. @@ -228,10 +231,14 @@ def post(self): app.logger.warning( "C3VOC webhook sent youtube urls update without referencing the previously stored value. Ignoring." ) - app.logger.debug(f"{proposal.id=} {payload['youtube']['urls']=} {proposal.youtube_url=}") + app.logger.debug( + f"{proposal.id=} {payload['youtube']['urls']=} {proposal.youtube_url=}" + ) else: # see comment at c3voc_url above - app.logger.warning(f"C3VOC webhook cleared youtube_url for {proposal.id=}, was {proposal.youtube_url}") + app.logger.warning( + f"C3VOC webhook cleared youtube_url for {proposal.id=}, was {proposal.youtube_url}" + ) proposal.youtube_url = "" db.session.add(proposal) From 6ffd133dc7fb7fe02cbd53fdcbd3d1328f8c667d Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sat, 31 Aug 2024 21:48:53 +0200 Subject: [PATCH 10/15] api.schedule.ProposalC3VOCPublishingWebhook: when clearing fields, set to None --- apps/api/schedule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index cbed73cd0..3c3d78cd2 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -211,7 +211,7 @@ def post(self): app.logger.warning( f"C3VOC webhook cleared c3voc_url for {proposal.id=}, was {proposal.c3voc_url}" ) - proposal.c3voc_url = "" + proposal.c3voc_url = None if payload["youtube"]["enabled"]: if payload["youtube"]["urls"]: @@ -239,7 +239,7 @@ def post(self): app.logger.warning( f"C3VOC webhook cleared youtube_url for {proposal.id=}, was {proposal.youtube_url}" ) - proposal.youtube_url = "" + proposal.youtube_url = None db.session.add(proposal) db.session.commit() From 18d6c7ede9f15f7096ca1a32556868778946bb1c Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sun, 1 Sep 2024 09:00:52 +0200 Subject: [PATCH 11/15] add tests/test_api_c3voc.py --- tests/test_api_c3voc.py | 405 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 405 insertions(+) create mode 100644 tests/test_api_c3voc.py diff --git a/tests/test_api_c3voc.py b/tests/test_api_c3voc.py new file mode 100644 index 000000000..3ab335352 --- /dev/null +++ b/tests/test_api_c3voc.py @@ -0,0 +1,405 @@ +import pytest + +from models import event_year +from models.cfp import Proposal, TalkProposal + + +@pytest.fixture(scope="module") +def proposal(db, user): + proposal = TalkProposal() + proposal.title = "Title" + proposal.description = "Description" + proposal.user = user + + db.session.add(proposal) + db.session.commit() + + return proposal + + +def clean_proposal(db, proposal, c3voc_url=None, youtube_url=None, video_recording_lost=True): + proposal.c3voc_url = c3voc_url + proposal.youtube_url = youtube_url + proposal.video_recording_lost = video_recording_lost + db.session.add(proposal) + db.session.commit() + + +def test_denies_request_without_api_key(client, app, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + json={ + "is_master": True, + "fahrplan": { + "conference": "emf1970", + "id": 0, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 401 + + +def test_denies_request_no_master(client, app, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": False, + "fahrplan": { + "conference": "emf1970", + "id": 0, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 403 + + +def test_denies_request_wrong_year(client, app, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": "emf1970", + "id": 0, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 422 + + +def test_request_none_update_none(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.youtube_url is None + assert proposal.c3voc_url is None + + +def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "https://media.ccc.de/", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.c3voc_url == "https://media.ccc.de/" + assert proposal.video_recording_lost == False + assert proposal.youtube_url is None + + +def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, c3voc_url="https://example.com") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "https://example.org", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 406 + + proposal = Proposal.query.get(proposal.id) + # clean_proposal sets this to true, the api should not change that + assert proposal.video_recording_lost == True + assert proposal.c3voc_url == "https://example.com" + + +def test_request_voctoweb_clears_voctoweb(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, c3voc_url="https://example.com") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.c3voc_url is None + + +def test_request_youtube_update_youtube_correct_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": True, + "urls": [ + "https://www.youtube.com/watch", + ], + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.c3voc_url is None + assert proposal.video_recording_lost == False + assert proposal.youtube_url == "https://www.youtube.com/watch" + + +def test_request_youtube_update_youtube_correct_url_but_existing_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, youtube_url="https://example.com") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": True, + "urls": [ + "https://www.youtube.com/watch", + ], + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + # clean_proposal sets this to true, the api should not change that + assert proposal.video_recording_lost == True + assert proposal.youtube_url == "https://example.com" + + +def test_request_youtube_update_youtube_wrong_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, youtube_url="https://example.com") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": True, + "urls": [ + "https://example.org", + ], + }, + }, + ) + assert rv.status_code == 406 + + proposal = Proposal.query.get(proposal.id) + # clean_proposal sets this to true, the api should not change that + assert proposal.video_recording_lost == True + assert proposal.youtube_url == "https://example.com" + + +def test_request_youtube_clears_youtube(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, youtube_url="https://example.com") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": False, + }, + "youtube": { + "enabled": True, + "urls": [], + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.youtube_url is None From f6116cc61bf20c9a714acb5f64a1d3dcea6e6c96 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sun, 1 Sep 2024 09:41:37 +0200 Subject: [PATCH 12/15] api.schedule.ProposalC3VOCPublishingWebhook: fix logic error --- apps/api/schedule.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 3c3d78cd2..29e971f3c 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -216,10 +216,10 @@ def post(self): if payload["youtube"]["enabled"]: if payload["youtube"]["urls"]: # Please do not overwrite existing youtube urls + youtube_url = payload["youtube"]["urls"][0] + if not youtube_url.startswith("https://www.youtube.com/watch"): + abort(406, message="youtube url must start with https://www.youtube.com/watch") if not proposal.youtube_url: - youtube_url = payload["youtube"]["urls"][0] - if not youtube_url.startswith("https://www.youtube.com/watch"): - abort(406, message="youtube url must start with https://www.youtube.com/watch") # c3voc will send us a list, even though we only have one # video. app.logger.info(f"C3VOC webhook set youtube_url for {proposal.id=} to {youtube_url}") From 4e2ed324f88d82657a095f550798efd6e3cc1d48 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sun, 1 Sep 2024 14:01:23 +0200 Subject: [PATCH 13/15] api.schedule: abort early in _require_video_api_key if key is missing/empty in config If we don't have that, the code will happily compare "" against "", which is true, leading to an authentication bypass vulnerability. --- apps/api/schedule.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index 29e971f3c..cf26c7b77 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -17,6 +17,9 @@ def _require_video_api_key(func): @wraps(func) def wrapper(*args, **kwargs): + if not app.config.get("VIDEO_API_KEY"): + abort(401) + auth_header = request.headers.get("authorization", None) if not auth_header or not auth_header.startswith("Bearer "): abort(401) From 7cb8728f3e47fbceeec78e250b5671298e92af14 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sun, 1 Sep 2024 12:59:26 +0200 Subject: [PATCH 14/15] api.schedule.ProposalC3VOCPublishingWebhook: add option to set thumbnail as well --- apps/api/schedule.py | 14 ++++ tests/test_api_c3voc.py | 156 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 168 insertions(+), 2 deletions(-) diff --git a/apps/api/schedule.py b/apps/api/schedule.py index cf26c7b77..4e405c500 100644 --- a/apps/api/schedule.py +++ b/apps/api/schedule.py @@ -216,6 +216,20 @@ def post(self): ) proposal.c3voc_url = None + if payload["voctoweb"]["thumb_path"]: + path = payload["voctoweb"]["thumb_path"] + if path.startswith("/static.media.ccc.de"): + path = "https://static.media.ccc.de/media" + path[len("/static.media.ccc.de"):] + if not path.startswith("https://"): + abort(406, message="voctoweb thumb_path must start with https:// or /static.media.ccc.de") + app.logger.info(f"C3VOC webhook set thumbnail_url for {proposal.id=} to {path}") + proposal.thumbnail_url = path + else: + app.logger.warning( + f"C3VOC webhook cleared thumbnail_url for {proposal.id=}, was {proposal.thumbnail_url}" + ) + proposal.thumbnail_url = None + if payload["youtube"]["enabled"]: if payload["youtube"]["urls"]: # Please do not overwrite existing youtube urls diff --git a/tests/test_api_c3voc.py b/tests/test_api_c3voc.py index 3ab335352..abe41e1fc 100644 --- a/tests/test_api_c3voc.py +++ b/tests/test_api_c3voc.py @@ -17,10 +17,11 @@ def proposal(db, user): return proposal -def clean_proposal(db, proposal, c3voc_url=None, youtube_url=None, video_recording_lost=True): +def clean_proposal(db, proposal, c3voc_url=None, youtube_url=None, thumbnail_url=None, video_recording_lost=True): proposal.c3voc_url = c3voc_url - proposal.youtube_url = youtube_url + proposal.thumbnail_url = thumbnail_url proposal.video_recording_lost = video_recording_lost + proposal.youtube_url = youtube_url db.session.add(proposal) db.session.commit() @@ -167,6 +168,7 @@ def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal) "voctoweb": { "enabled": True, "frontend_url": "https://media.ccc.de/", + "thumb_path": "", }, "youtube": { "enabled": False, @@ -204,6 +206,7 @@ def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal): "voctoweb": { "enabled": True, "frontend_url": "https://example.org", + "thumb_path": "", }, "youtube": { "enabled": False, @@ -241,6 +244,81 @@ def test_request_voctoweb_clears_voctoweb(client, app, db, proposal): "voctoweb": { "enabled": True, "frontend_url": "", + "thumb_path": "", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.c3voc_url is None + + +def test_request_thumbnail_update_thumbnail_correct_path(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "", + "thumb_path": "/static.media.ccc.de/thumb.jpg", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.thumbnail_url == "https://static.media.ccc.de/media/thumb.jpg" + assert proposal.c3voc_url is None + assert proposal.youtube_url is None + + +def test_request_thumbnail_update_thumbnail_correct_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal) + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "", + "thumb_path": "https://example.com/thumb.jpg", }, "youtube": { "enabled": False, @@ -250,7 +328,81 @@ def test_request_voctoweb_clears_voctoweb(client, app, db, proposal): assert rv.status_code == 204 proposal = Proposal.query.get(proposal.id) + assert proposal.thumbnail_url == "https://example.com/thumb.jpg" assert proposal.c3voc_url is None + assert proposal.youtube_url is None + + +def test_request_thumbnail_update_thumbnail_not_url(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, thumbnail_url="https://example.com/thumb.jpg") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "", + "thumb_path": "gopher://example.com/thumb.jpg", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 406 + + proposal = Proposal.query.get(proposal.id) + assert proposal.thumbnail_url == "https://example.com/thumb.jpg" + + +def test_request_thumbnail_clears_thumbnail(client, app, db, proposal): + app.config.update( + { + "VIDEO_API_KEY": "api-key", + } + ) + + clean_proposal(db, proposal, thumbnail_url="https://example.com/thumb.jpg") + + rv = client.post( + f"/api/proposal/c3voc-publishing-webhook", + headers={ + "Authorization": "Bearer api-key", + }, + json={ + "is_master": True, + "fahrplan": { + "conference": f"emf{event_year()}", + "id": proposal.id, + }, + "voctoweb": { + "enabled": True, + "frontend_url": "", + "thumb_path": "", + }, + "youtube": { + "enabled": False, + }, + }, + ) + assert rv.status_code == 204 + + proposal = Proposal.query.get(proposal.id) + assert proposal.thumbnail_url is None def test_request_youtube_update_youtube_correct_url(client, app, db, proposal): From 2d909d21bb3d0995b0dc717dfdbe0676ab392843 Mon Sep 17 00:00:00 2001 From: Franziska Kunsmann Date: Sun, 1 Sep 2024 13:04:16 +0200 Subject: [PATCH 15/15] tests/test_api_c3voc: better test naming --- tests/test_api_c3voc.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/test_api_c3voc.py b/tests/test_api_c3voc.py index abe41e1fc..3a2e99e52 100644 --- a/tests/test_api_c3voc.py +++ b/tests/test_api_c3voc.py @@ -110,7 +110,7 @@ def test_denies_request_wrong_year(client, app, proposal): assert rv.status_code == 422 -def test_request_none_update_none(client, app, db, proposal): +def test_request_none_unchanged(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -145,7 +145,7 @@ def test_request_none_update_none(client, app, db, proposal): assert proposal.c3voc_url is None -def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal): +def test_update_voctoweb_with_correct_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -183,7 +183,7 @@ def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal) assert proposal.youtube_url is None -def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal): +def test_denies_voctoweb_with_wrong_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -221,7 +221,7 @@ def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal): assert proposal.c3voc_url == "https://example.com" -def test_request_voctoweb_clears_voctoweb(client, app, db, proposal): +def test_clears_voctoweb(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -257,7 +257,7 @@ def test_request_voctoweb_clears_voctoweb(client, app, db, proposal): assert proposal.c3voc_url is None -def test_request_thumbnail_update_thumbnail_correct_path(client, app, db, proposal): +def test_update_thumbnail_with_path(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -295,7 +295,7 @@ def test_request_thumbnail_update_thumbnail_correct_path(client, app, db, propos assert proposal.youtube_url is None -def test_request_thumbnail_update_thumbnail_correct_url(client, app, db, proposal): +def test_update_thumbnail_with_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -333,7 +333,7 @@ def test_request_thumbnail_update_thumbnail_correct_url(client, app, db, proposa assert proposal.youtube_url is None -def test_request_thumbnail_update_thumbnail_not_url(client, app, db, proposal): +def test_denies_thumbnail_not_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -356,7 +356,7 @@ def test_request_thumbnail_update_thumbnail_not_url(client, app, db, proposal): "voctoweb": { "enabled": True, "frontend_url": "", - "thumb_path": "gopher://example.com/thumb.jpg", + "thumb_path": "/example.com/thumb.jpg", }, "youtube": { "enabled": False, @@ -369,7 +369,7 @@ def test_request_thumbnail_update_thumbnail_not_url(client, app, db, proposal): assert proposal.thumbnail_url == "https://example.com/thumb.jpg" -def test_request_thumbnail_clears_thumbnail(client, app, db, proposal): +def test_clears_thumbnail(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -405,7 +405,7 @@ def test_request_thumbnail_clears_thumbnail(client, app, db, proposal): assert proposal.thumbnail_url is None -def test_request_youtube_update_youtube_correct_url(client, app, db, proposal): +def test_update_youtube_with_correct_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -444,7 +444,7 @@ def test_request_youtube_update_youtube_correct_url(client, app, db, proposal): assert proposal.youtube_url == "https://www.youtube.com/watch" -def test_request_youtube_update_youtube_correct_url_but_existing_url(client, app, db, proposal): +def test_denies_youtube_update_with_exisiting_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -483,7 +483,7 @@ def test_request_youtube_update_youtube_correct_url_but_existing_url(client, app assert proposal.youtube_url == "https://example.com" -def test_request_youtube_update_youtube_wrong_url(client, app, db, proposal): +def test_denies_youtube_update_with_wrong_url(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key", @@ -522,7 +522,7 @@ def test_request_youtube_update_youtube_wrong_url(client, app, db, proposal): assert proposal.youtube_url == "https://example.com" -def test_request_youtube_clears_youtube(client, app, db, proposal): +def test_clears_youtube(client, app, db, proposal): app.config.update( { "VIDEO_API_KEY": "api-key",