Add reviewed duplicate account consolidation and prevent duplicate imports

backend/tests/test_duplicate_accounts.py

@@ -0,0 +1,168 @@
+import json
+import sqlite3
+import unittest
+from unittest.mock import AsyncMock, patch
+from types import SimpleNamespace
+
+from fastapi import FastAPI, HTTPException
+from fastapi.testclient import TestClient
+from backend.app import db
+from backend.app.auth import get_current_user
+from backend.app.feature_access import permissions, update_permissions
+from backend.app.routers import identities
+from backend.app.services import duplicate_accounts as duplicates, identity_review as review
+from backend.app.services.jellyfin_identity import link_user
+from backend.tests.test_backend_quality import TempDatabaseMixin
+
+JF, SERVER = 'a' * 32, 'b' * 32
+
+
+class DuplicateAccountTests(TempDatabaseMixin, unittest.IsolatedAsyncioTestCase):
+    def setUp(self):
+        super().setUp()
+        db.create_user('Viewer', 'Password-123456!', auth_provider='jellyfin', jellyseerr_user_id=42)
+        self.keep = db.get_user_by_username('Viewer')['id']
+        with db._connect() as conn:
+            self.extra = conn.execute("""INSERT INTO users(username,password_hash,role,auth_provider,
+                jellyseerr_user_id,created_at) VALUES('viewer ','old-hash','user','jellyfin',42,'2026-01-01')""").lastrowid
+        self.runtime = SimpleNamespace(jellyfin_base_url='http://jf', jellyfin_api_key='test',
+            jellyseerr_base_url='http://seerr', jellyseerr_api_key='test', jellystat_base_url='http://stats', jellystat_api_key='test')
+        link_user('Viewer', JF, 'http://jf')
+        self.jf = {'state': 'available', 'server_id': SERVER, 'users': [{'id': JF, 'name': 'Viewer'}]}
+        self.seerr = {'state': 'available', 'users': [{'id': 42, 'name': 'Viewer', 'jellyfin_id': JF}]}
+        for name, value in [('get_runtime_settings', self.runtime), ('jellyfin_directory', self.jf), ('seerr_directory', self.seerr)]:
+            mocked = patch.object(review, name, return_value=value)
+            mocked.start(); self.addCleanup(mocked.stop)
+        mocked = patch.object(review.JellystatClient, 'check_user_ids', new_callable=AsyncMock,
+                              return_value={JF: {'state': 'matched', 'id': JF}})
+        mocked.start(); self.addCleanup(mocked.stop)
+
+    async def test_consolidation_preserves_history_and_restrictive_access(self):
+        with db._connect() as conn:
+            conn.execute('UPDATE users SET auto_search_enabled=0,expires_at=? WHERE id=?', ('2026-01-01T00:00:00+00:00', self.extra))
+            conn.execute('INSERT INTO user_feature_permissions VALUES(?,?,?)', (self.extra, 'issues', 0))
+        db.upsert_user_activity('Viewer', '127.0.0.1', 'test')
+        db.upsert_user_activity('viewer ', '127.0.0.1', 'test')
+        item = db.create_portal_item(kind='issue', title='Issue', description='History', created_by_username='viewer ', created_by_id=42)
+        before = review.read_snapshot()
+        preview = await duplicates.repair_duplicates(self.extra)
+        self.assertEqual(review.read_snapshot(), before, 'Preview must not mutate accounts')
+        self.assertTrue(preview['can_confirm'], preview['issues'])
+        self.assertEqual(preview['keep_id'], self.keep)
+        self.assertNotIn('old-hash', json.dumps(preview))
+        result = await duplicates.repair_duplicates(self.extra, self.keep, preview['revision'], {'username': 'admin'})
+        self.assertEqual(result['consolidated'], 1)
+        self.assertIsNone(db.get_user_by_id(self.extra))
+        user = db.get_user_by_username('Viewer')
+        self.assertEqual(user['id'], self.keep)
+        self.assertFalse(user['auto_search_enabled'])
+        self.assertFalse(permissions(user)['issues'])
+        self.assertTrue(user['is_expired'])
+        self.assertEqual(db.get_portal_item(item['id'])['created_by_username'], 'Viewer')
+        self.assertEqual(db.get_portal_item(item['id'])['created_by_id'], 42, 'IDs here belong to Seerr')
+        with db._connect() as conn:
+            self.assertEqual(conn.execute('SELECT SUM(hit_count) FROM user_activity').fetchone()[0], 2)
+            archive = json.loads(conn.execute('SELECT archive_json FROM user_duplicate_repairs').fetchone()[0])
+            self.assertEqual(len(archive['users']), 2)
+            self.assertEqual(conn.execute('SELECT local_user_id FROM jellyfin_user_links').fetchone()[0], self.keep)
+        report, _, _ = await review.review_identities()
+        self.assertEqual(next(row for row in report['rows'] if row['user']['id'] == self.keep)['state'], 'confirmed')
+        self.assertFalse(db.create_user_if_missing('VIEWER ', 'unused', auth_provider='jellyfin'))
+
+    async def test_choose_other_row_retains_its_settings_and_moves_link(self):
+        with db._connect() as conn:
+            conn.execute('UPDATE users SET email=? WHERE id=?', ('chosen@example.test', self.extra))
+        preview = await duplicates.repair_duplicates(self.keep, self.extra)
+        self.assertEqual(preview['proposed']['email'], 'chosen@example.test')
+        await duplicates.repair_duplicates(self.keep, self.extra, preview['revision'], {'username': 'admin'})
+        self.assertEqual(db.get_user_by_username('Viewer')['id'], self.extra)
+        self.assertEqual(db.get_user_by_id(self.extra)['username'], 'Viewer')
+
+    async def test_changed_permission_or_identity_rejects_stale_preview(self):
+        preview, report, local, runtime, state = await duplicates.prepare(self.keep)
+        update_permissions({'stats': False}, 'Viewer')
+        with self.assertRaises(HTTPException) as caught:
+            duplicates.consolidate(preview, report, local, runtime, state, {'username': 'admin'})
+        self.assertEqual(caught.exception.status_code, 409)
+        self.assertIsNotNone(db.get_user_by_id(self.extra))
+        self.seerr['users'][0]['jellyfin_id'] = 'c' * 32
+        with self.assertRaises(HTTPException):
+            await duplicates.repair_duplicates(self.keep, self.keep, preview['revision'], {'username': 'admin'})
+
+    async def test_conflicting_identities_admins_and_other_owners_are_blocked(self):
+        with db._connect() as conn:
+            conn.execute("UPDATE users SET role='admin' WHERE id=?", (self.extra,))
+        self.assertFalse((await duplicates.repair_duplicates(self.keep))['can_confirm'])
+        with db._connect() as conn:
+            conn.execute("UPDATE users SET role='user',jellyseerr_user_id=99 WHERE id=?", (self.extra,))
+        self.assertFalse((await duplicates.repair_duplicates(self.keep))['can_confirm'])
+        with db._connect() as conn:
+            conn.execute('UPDATE users SET jellyseerr_user_id=42 WHERE id=?', (self.extra,))
+        db.create_user('Other', 'Password-123456!', auth_provider='jellyfin', jellyseerr_user_id=42)
+        self.assertFalse((await duplicates.repair_duplicates(self.keep))['can_confirm'])
+
+    async def test_transaction_rolls_back_archive_and_history_on_failure(self):
+        preview, report, local, runtime, state = await duplicates.prepare(self.keep)
+        with db._connect() as conn:
+            conn.execute("CREATE TRIGGER prevent_test_delete BEFORE DELETE ON users BEGIN SELECT RAISE(ABORT,'fixture failure'); END")
+        with self.assertRaises(sqlite3.IntegrityError):
+            duplicates.consolidate(preview, report, local, runtime, state, {'username': 'admin'})
+        self.assertIsNotNone(db.get_user_by_id(self.extra))
+        with db._connect() as conn:
+            self.assertEqual(conn.execute('SELECT COUNT(*) FROM user_duplicate_repairs').fetchone()[0], 0)
+
+    async def test_creation_rejects_case_and_whitespace_variants(self):
+        for name in ('viewer', 'VIEWER', ' Viewer '):
+            self.assertFalse(db.create_user_if_missing(name, 'unused'))
+            with self.assertRaises(sqlite3.IntegrityError):
+                db.create_user(name, 'unused')
+
+    async def test_unresolved_whitespace_accounts_keep_distinct_lookup(self):
+        self.assertEqual(db.get_user_by_username('Viewer')['id'], self.keep)
+        self.assertEqual(db.get_user_by_username('viewer ')['id'], self.extra)
+        self.assertIsNone(db.get_user_by_username(' Viewer '), 'Do not guess between unresolved identities')
+
+    async def test_concurrent_imports_create_only_one_normalized_account(self):
+        from concurrent.futures import ThreadPoolExecutor
+        with ThreadPoolExecutor(max_workers=2) as pool:
+            results = list(pool.map(lambda name: db.create_user_if_missing(name, 'Password-123456!'), ['New viewer', 'NEW VIEWER ']))
+        self.assertEqual(sorted(results), [False, True])
+
+    def seed_delivery(self, state='queued'):
+        with db._connect() as conn:
+            for prefix in ('email_recap', 'newsletter'):
+                for identity in (self.keep, self.extra):
+                    conn.execute(f'''INSERT INTO {prefix}_subscriptions(user_id,state,email,identity_source,identity_id,
+                        version,requested_at,unsubscribe_token) VALUES(?,?,?,?,?,?,?,?)''',
+                        (identity, 'enabled', 'viewer@example.test', review.source_key('http://jf'), JF, str(identity), 1, prefix + str(identity)))
+                period = {'month': '2026-08'} if prefix == 'email_recap' else {'edition_id': 'edition', 'edition_revision': 1}
+                values = {'id': prefix, 'dedupe_key': prefix, 'user_id': self.extra, **period, 'kind': 'test',
+                          'email': 'viewer@example.test', 'subscription_version': str(self.extra), 'public_url': 'https://example.test',
+                          'state': state, 'created_at': 1, 'updated_at': 1, 'next_attempt_at': 1}
+                conn.execute(f"INSERT INTO {prefix}_deliveries({','.join(values)}) VALUES({','.join('?' for _ in values)})", tuple(values.values()))
+
+    async def test_email_history_retained_pending_cancelled_and_consent_not_inherited(self):
+        self.seed_delivery()
+        preview = await duplicates.repair_duplicates(self.extra)
+        await duplicates.repair_duplicates(self.extra, self.keep, preview['revision'], {'username': 'admin'})
+        with db._connect() as conn:
+            for prefix in ('email_recap', 'newsletter'):
+                delivery = conn.execute(f'SELECT user_id,state FROM {prefix}_deliveries').fetchone()
+                self.assertEqual(delivery, (self.keep, 'cancelled'))
+                subs = conn.execute(f'SELECT user_id,state FROM {prefix}_subscriptions').fetchall()
+                self.assertEqual(subs, [(self.keep, 'enabled')])
+
+    async def test_sending_email_blocks_repair_without_removing_accounts(self):
+        self.seed_delivery('sending')
+        preview = await duplicates.repair_duplicates(self.extra)
+        with self.assertRaises(HTTPException) as caught:
+            await duplicates.repair_duplicates(self.extra, self.keep, preview['revision'], {'username': 'admin'})
+        self.assertEqual(caught.exception.status_code, 409)
+        self.assertIsNotNone(db.get_user_by_id(self.extra))
+
+    async def test_duplicate_endpoints_are_admin_only(self):
+        app = FastAPI(); app.include_router(identities.router)
+        app.dependency_overrides[get_current_user] = lambda: {'username': 'viewer', 'role': 'user'}
+        with TestClient(app) as client:
+            for path in ('check', 'confirm'):
+                self.assertEqual(client.post('/admin/identities/duplicates/' + path, json={'user_id': self.keep}).status_code, 403)

backend/tests/test_identity_review.py

@@ -281,7 +281,9 @@ class IdentityReviewTests(TempDatabaseMixin, unittest.IsolatedAsyncioTestCase):
         self.assertFalse(self.row(report)["can_confirm"])
 
     async def test_case_duplicates_in_magent_remain_visible_and_blocked(self):
-        db.create_user("georgia", "jellyfin-user", auth_provider="jellyfin")
+        # Legacy duplicate predates the normalized-name creation guard.
+        with db._connect() as conn:
+            conn.execute("INSERT INTO users(username,password_hash,role,auth_provider,created_at) VALUES('georgia','unused','user','jellyfin','2026-01-01')")
         report, _ = self.build()
         self.assertEqual(report["counts"]["conflict"], 2)
         self.assertEqual(report["counts"]["ready"], 0)