Add Audit Logging for several endpoints

- Use transactions for DB commits to avoid audit-less logs

Endpoints Supported:
- Manual Link
- Reset Row
- Update Row

postgres/schema.sql

@@ -16,7 +16,7 @@ CREATE TYPE video_range as (
 
 CREATE TYPE video_transition as (
 	type TEXT,
-	duration DOUBLE PRECISON
+	duration DOUBLE PRECISION
 );
 
 CREATE TYPE thumbnail_mode as ENUM (
@@ -33,7 +33,7 @@ CREATE TYPE thumbnail_mode as ENUM (
 CREATE TYPE end_time AS (
 	dashed BOOLEAN,
 	value TIMESTAMP
-)
+);
 
 CREATE TABLE events (
 	id TEXT PRIMARY KEY,
@@ -41,8 +41,8 @@ CREATE TABLE events (
 	sheet_name TEXT NOT NULL,
 	event_start TIMESTAMP,
 	event_end end_time DEFAULT ROW(false, NULL) CHECK (
-		event_end.dashed IS NOT NULL
-		AND (event_end.dashed != TRUE OR event_end.value IS NOT DISTINCT FROM event_start)
+		(event_end).dashed IS NOT NULL
+		AND ((event_end).dashed != TRUE OR (event_end).value IS NOT DISTINCT FROM event_start)
 	),
 	category TEXT NOT NULL DEFAULT '',
 	description TEXT NOT NULL DEFAULT '',
@@ -104,6 +104,18 @@ CREATE TABLE events (
 -- Index on state, since that's almost always what we're querying on besides id
 CREATE INDEX event_state ON events (state);
 
+-- Table for recording each "edit" made to a video, written by thrimshim.
+-- This is mainly a just-in-case thing so we can work out when something was changed,
+-- and change it back if needed. More about accidents than security.
+CREATE TABLE events_edits_audit_log (
+	time TIMESTAMP NOT NULL DEFAULT NOW(),
+	id TEXT NOT NULL,
+	api_action TEXT NOT NULL,
+	editor TEXT NOT NULL,
+	old_data JSONB,
+	new_data JSONB
+);
+
 CREATE TABLE nodes (
 	name TEXT PRIMARY KEY,
 	url TEXT NOT NULL,

thrimshim/thrimshim/main.py

@@ -128,14 +128,14 @@ def get_all_rows():
 		}
 		rows.append(row)
 	logging.info('All rows fetched')
-	return json.dumps(rows)
+	return to_json(rows)
 
 
 @app.route('/thrimshim/defaults')
 @request_stats
 def get_defaults():
 	"""Get default info needed by thrimbletrimmer when not loading a specific row."""
-	return json.dumps({
+	return to_json({
 		"video_channel": app.default_channel,
 		"bustime_start": app.bustime_start,
 		"title_prefix": app.title_header,
@@ -159,6 +159,18 @@ def get_transitions():
 	]
 
 
+def to_json(obj):
+	def convert(value):
+		if isinstance(value, datetime.datetime):
+			return value.isoformat()
+		if isinstance(value, datetime.timedelta):
+			return value.total_seconds()
+		if isinstance(value, memoryview) or isinstance(value, bytes):
+			return base64.b64encode(bytes(value)).decode()
+		raise TypeError(f"Can't convert object of type {value.__class__.__name__} to JSON: {value}")
+	return json.dumps(obj, default=convert)
+
+
 @app.route('/thrimshim/<ident>', methods=['GET'])
 @request_stats
 def get_row(ident):
@@ -247,15 +259,7 @@ def get_row(ident):
 
 	logging.info('Row {} fetched'.format(ident))
 
-	def convert(value):
-		if isinstance(value, datetime.datetime):
-			return value.isoformat()
-		if isinstance(value, datetime.timedelta):
-			return value.total_seconds()
-		if isinstance(value, memoryview) or isinstance(value, bytes):
-			return base64.b64encode(bytes(value)).decode()
-		raise TypeError(f"Can't convert object of type {value.__class__.__name__} to JSON: {value}")
-	return json.dumps(response, default=convert)
+	return to_json(response)
 
 
 @app.route('/thrimshim/<ident>', methods=['POST'])
@@ -297,12 +301,16 @@ def update_row(ident, editor=None):
 	for extra in extras:
 		del new_row[extra]
 
-	# Check a row with id = ident is in the database
-	conn = app.db_manager.get_conn()
+	# Everything that follows happens in a single transaction
+	with app.db_manager.get_conn() as conn:
+
+		# Check a row with id = ident is in the database.
+		# Lock the row to prevent concurrent updates while we check the transition validity.
 		built_query = sql.SQL("""
 			SELECT id, state, {}
 			FROM events
 			WHERE id = %s
+			FOR UPDATE
 		""").format(sql.SQL(', ').join(
 			sql.Identifier(key) for key in sheet_columns
 		))
@@ -431,7 +439,7 @@ def update_row(ident, editor=None):
 			))
 			result = database.query(conn, build_query, id=ident, **new_row)
 			if result.rowcount != 1:
-			return 'Video changed state while we were updating - maybe it was reset?', 403
+				return 'Video changed state while we were updating - maybe it was reset?', 409
 
 		else:
 			# handle state columns
@@ -466,6 +474,8 @@ def update_row(ident, editor=None):
 			if result.rowcount != 1:
 				return 'Video likely already published', 403
 
+		_write_audit_log(conn, ident, "update-row", editor, old_row, new_row)
+
 	logging.info('Row {} updated to state {}'.format(ident, new_row['state']))
 	return ''
 
@@ -490,11 +500,13 @@ def manual_link(ident, editor=None):
 	else:
 		return 'Upload location must be "manual" or "youtube-manual"', 400
 
-	conn = app.db_manager.get_conn()
+	with app.db_manager.get_conn() as conn:
 		results = database.query(conn, """
 			SELECT id, state
 			FROM events
-		WHERE id = %s""", ident)
+			WHERE id = %s
+			FOR UPDATE
+		""", ident)
 		old_row = results.fetchone()
 		if old_row is None:
 			return 'Row {} not found'.format(ident), 404
@@ -503,12 +515,22 @@ def manual_link(ident, editor=None):
 		now = datetime.datetime.utcnow()
 		# note we force thumbnail mode of manual uploads to always be NONE,
 		# since they might not be a video we actually control at all, or might not even be on youtube.
-	results = database.query(conn, """
+		result = database.query(conn, """
 			UPDATE events
 			SET state='DONE', upload_location = %s, video_link = %s, video_id = %s,
 				editor = %s, edit_time = %s, upload_time = %s, thumbnail_mode = 'NONE'
 			WHERE id = %s AND state = 'UNEDITED'
 		""", upload_location, link, video_id, editor, now, now, ident)
+		if result.rowcount != 1:
+			return 'Video changed state while we were updating - maybe it was reset?', 409
+		_write_audit_log(conn, ident, "manual-link", editor, new_row={
+			"state": "DONE",
+			"upload_location": upload_location,
+			"video_link": link,
+			"video_id": video_id,
+			"editor": editor,
+			"thumbnail_mode": None,
+		})
 	logging.info("Row {} video_link set to {}".format(ident, link))
 	return ''
 
@@ -523,7 +545,7 @@ def reset_row(ident, editor=None):
 	(state is UNEDITED, EDITED or CLAIMED)
 	"""
 	force = (flask.request.args.get('force', '').lower() == "true")
-	conn = app.db_manager.get_conn()
+	with app.db_manager.get_conn() as conn:
 		query = """
 			UPDATE events
 			SET state='UNEDITED', error = NULL, video_id = NULL, video_link = NULL,
@@ -536,10 +558,18 @@ def reset_row(ident, editor=None):
 		results = database.query(conn, query, ident)
 		if results.rowcount != 1:
 			return 'Row id = {} not found or not in cancellable state'.format(ident), 404
+		_write_audit_log(conn, ident, "reset-row", editor)
 	logging.info("Row {} reset to 'UNEDITED'".format(ident))
 	return ''
 
 
+def _write_audit_log(conn, ident, api_action, editor, old_row=None, new_row=None):
+	database.query(conn, """
+		INSERT INTO events_edits_audit_log (id, api_action, editor, old_data, new_data)
+		VALUES (%(id)s, %(api_action)s, %(editor)s, %(old_row)s, %(new_row)s)
+	""", id=ident, api_action=api_action, editor=editor, old_row=to_json(old_row), new_row=to_json(new_row))
+
+
 @app.route('/thrimshim/bus/<channel>')
 @request_stats
 def get_odometer(channel):