Skip to content

Commit 67feb73

Browse files
committed
refactor(speaker): scope a request by the tenant it declares, not by an id prefix
A speaker id was minted as `user_{tenant}_...` in three unrelated places, and the WebUI treated that prefix as access control: const userPrefix = `user_${user.id}_` if (!speakerId.startsWith(userPrefix)) { alert('You can only delete your own speakers.') Deciding permission by string-matching a naming convention only holds while the convention does. It already did not: moving the tenant to Chronicle's user id left all 51 live speakers with ids reading `user_1_` and an owner of `69b80e5894aa9ec334a421c9`, so the Speakers page filtered its own gallery down to nothing and every delete was refused. Ids are now opaque (`speaker_{uuid}`) and ownership is only ever `speakers.user_id`. The three by-id endpoints plus `/enroll/append` take the tenant the caller is acting for and 404 on a mismatch — 404 rather than 403 so a refusal does not confirm the speaker exists. The service still has no caller authentication, so this bounds mistakes, not attackers; the WebUI's alert claimed a guarantee that was never enforced anywhere. Existing ids keep their prefix. It is inert now, and renaming them would have to move on-disk enrollment/quarantine directories, relative paths stored in `speaker_audio_segments`, per-speaker manifests, and denormalized copies in four Mongo collections plus `User.wakeword_allowed_speakers` — where a missed rewrite silently disables the wake-word gate rather than failing. The WebUI can no longer mint a tenant from a typed username: this service cannot invent a Chronicle user id, so the selector only picks among tenants that already have speaker data.
1 parent d1b4fb8 commit 67feb73

25 files changed

Lines changed: 214 additions & 241 deletions

File tree

backends/advanced/src/advanced_omi_backend/controllers/guided_enrollment_controller.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ async def decide_clips(user: User, speaker_name: str, decisions: List[dict]):
690690
raise ValueError(f"No enrolled speaker named '{enrollment_target}'")
691691
wav = await reconstruct_audio_segment(conversation_id, start, end)
692692
result = await speaker_client.append_to_speaker(
693-
target_gallery["speaker_id"], wav
693+
target_gallery["speaker_id"], wav, user_id=str(user.user_id)
694694
)
695695
if result.get("error"):
696696
enroll_error = result["error"]
@@ -954,7 +954,7 @@ async def reset_speaker_state(
954954
gallery = await _gallery_stats(speaker_client, speaker_name, str(user.user_id))
955955
if gallery:
956956
result = await speaker_client.delete_speaker(
957-
gallery["speaker_id"], delete_audio=True
957+
gallery["speaker_id"], user_id=str(user.user_id), delete_audio=True
958958
)
959959
if result.get("error"):
960960
return JSONResponse(

backends/advanced/src/advanced_omi_backend/routers/modules/finetuning_routes.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,9 @@ async def enroll_selected_clips(
454454
)
455455
if existing:
456456
result = await speaker_client.append_to_speaker(
457-
speaker_id=existing["id"], audio_data=wav_bytes
457+
speaker_id=existing["id"],
458+
audio_data=wav_bytes,
459+
user_id=str(current_user.user_id),
458460
)
459461
if "error" in result:
460462
failed += 1

backends/advanced/src/advanced_omi_backend/speaker_recognition_client.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,8 +1511,8 @@ async def enroll_new_speaker(
15111511
return {"error": "speaker_recognition_disabled"}
15121512

15131513
try:
1514-
# Generate speaker ID: user_{user_id}_speaker_{random_hex}
1515-
speaker_id = f"user_{user_id}_speaker_{uuid.uuid4().hex[:12]}"
1514+
# Opaque: the tenant is the `user_id` field, not part of the id.
1515+
speaker_id = f"speaker_{uuid.uuid4().hex[:12]}"
15161516

15171517
logger.info(
15181518
f"🎤 Enrolling new speaker '{speaker_name}' with ID: {speaker_id}"
@@ -1558,20 +1558,25 @@ async def enroll_new_speaker(
15581558
logger.error(f"🎤 ❌ Error enrolling speaker: {e}")
15591559
return {"error": "unknown_error", "message": str(e)}
15601560

1561-
async def append_to_speaker(self, speaker_id: str, audio_data: bytes) -> Dict:
1561+
async def append_to_speaker(
1562+
self, speaker_id: str, audio_data: bytes, user_id: str
1563+
) -> Dict:
15621564
"""
15631565
Append audio to existing speaker's embedding (fine-tuning).
15641566
15651567
Args:
15661568
speaker_id: ID of existing speaker
15671569
audio_data: WAV audio bytes
1570+
user_id: Tenant the speaker must belong to
15681571
15691572
Returns:
15701573
Response dict from append endpoint
15711574
"""
15721575
if not self.enabled:
15731576
logger.warning("🎤 Speaker recognition disabled, cannot append to speaker")
15741577
return {"error": "speaker_recognition_disabled"}
1578+
if not user_id:
1579+
raise ValueError("user_id is required to append to a speaker")
15751580

15761581
try:
15771582
logger.info(f"🎤 Appending audio to speaker: {speaker_id}")
@@ -1585,6 +1590,7 @@ async def append_to_speaker(self, speaker_id: str, audio_data: bytes) -> Dict:
15851590
content_type="audio/wav",
15861591
)
15871592
form_data.add_field("speaker_id", speaker_id)
1593+
form_data.add_field("user_id", user_id)
15881594

15891595
async with session.post(
15901596
f"{self.service_url}/enroll/append",
@@ -1731,15 +1737,22 @@ async def delete_enrollment_segment(
17311737
logger.error(f"🎤 Failed to delete enrollment segment: {e}")
17321738
return {"error": "connection_failed", "message": str(e)}
17331739

1734-
async def delete_speaker(self, speaker_id: str, delete_audio: bool = True) -> Dict:
1740+
async def delete_speaker(
1741+
self, speaker_id: str, user_id: str, delete_audio: bool = True
1742+
) -> Dict:
17351743
"""Delete an enrolled speaker (and, by default, their enrollment audio)."""
17361744
if not self.enabled:
17371745
return {"error": "speaker_recognition_disabled"}
1746+
if not user_id:
1747+
raise ValueError("user_id is required to delete a speaker")
17381748
try:
17391749
async with aiohttp.ClientSession() as session:
17401750
async with session.delete(
17411751
f"{self.service_url}/speakers/{speaker_id}",
1742-
params={"delete_audio": "true" if delete_audio else "false"},
1752+
params={
1753+
"user_id": user_id,
1754+
"delete_audio": "true" if delete_audio else "false",
1755+
},
17431756
timeout=aiohttp.ClientTimeout(total=30),
17441757
) as response:
17451758
if response.status != 200:

backends/advanced/src/advanced_omi_backend/workers/finetuning_jobs.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,9 @@ async def run_speaker_finetuning_job() -> dict:
150150

151151
if existing_speaker:
152152
result = await speaker_client.append_to_speaker(
153-
speaker_id=existing_speaker["id"], audio_data=wav_bytes
153+
speaker_id=existing_speaker["id"],
154+
audio_data=wav_bytes,
155+
user_id=conversation.user_id,
154156
)
155157
if "error" in result:
156158
failed += 1

extras/ml-experiments/experiments/pyannote_diarization_validation/enroll_and_identify.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def main():
5858
buf = io.BytesIO()
5959
sf.write(buf, clip, SR, format="WAV", subtype="PCM_16")
6060
buf.seek(0)
61-
sid = f"user_{USER}_{meeting}_{spk}"
61+
sid = f"{meeting}_{spk}"
6262
r = requests.post(
6363
f"{SVC}/enroll/upload",
6464
files={"file": ("enroll.wav", buf, "audio/wav")},

extras/speaker-recognition/laptop_client.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,11 @@ async def list_speakers(self):
182182
result = await self._request("GET", "/speakers")
183183
return result.get("speakers", [])
184184

185-
async def remove_speaker(self, speaker_id: str):
185+
async def remove_speaker(self, speaker_id: str, user_id: str):
186186
"""Remove an enrolled speaker."""
187-
return await self._request("DELETE", f"/speakers/{speaker_id}")
187+
return await self._request(
188+
"DELETE", f"/speakers/{speaker_id}", params={"user_id": user_id}
189+
)
188190

189191

190192
async def record_audio(duration: float, sample_rate: int = 16000) -> str:
@@ -418,7 +420,7 @@ async def cmd_remove(args):
418420
await client.health_check()
419421

420422
# Remove speaker
421-
result = await client.remove_speaker(args.speaker_id)
423+
result = await client.remove_speaker(args.speaker_id, args.user_id)
422424

423425
if result.get("deleted"):
424426
logger.info(f"✅ Successfully removed speaker: {args.speaker_id}")
@@ -584,6 +586,9 @@ def main():
584586
remove_parser.add_argument(
585587
"--speaker-id", required=True, help="Speaker ID to remove"
586588
)
589+
remove_parser.add_argument(
590+
"--user-id", required=True, help="Chronicle user id owning the speaker"
591+
)
587592

588593
# Diarize command
589594
diarize_parser = subparsers.add_parser(

extras/speaker-recognition/scripts/enroll_speaker.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,14 @@ def list_speakers(service_url: str = SPEAKER_SERVICE_URL) -> bool:
288288
return False
289289

290290

291-
def delete_speaker(speaker_id: str, service_url: str = SPEAKER_SERVICE_URL) -> bool:
291+
def delete_speaker(
292+
speaker_id: str, user_id: str, service_url: str = SPEAKER_SERVICE_URL
293+
) -> bool:
292294
"""Delete an enrolled speaker."""
293295
try:
294-
response = requests.delete(f"{service_url}/speakers/{speaker_id}")
296+
response = requests.delete(
297+
f"{service_url}/speakers/{speaker_id}", params={"user_id": user_id}
298+
)
295299
if response.status_code == 200:
296300
logger.info(f"✅ Successfully deleted speaker: {speaker_id}")
297301
return True
@@ -328,7 +332,8 @@ def main():
328332
# Speaker info arguments
329333
parser.add_argument("--id", help="Speaker ID (required for enrollment)")
330334
parser.add_argument(
331-
"--user-id", help="Chronicle user id owning the speaker (required to enroll)"
335+
"--user-id",
336+
help="Chronicle user id owning the speaker (required to enroll or delete)",
332337
)
333338
parser.add_argument("--name", help="Speaker display name (required for enrollment)")
334339

@@ -357,7 +362,10 @@ def main():
357362
return 0 if list_speakers(service_url) else 1
358363

359364
elif args.delete:
360-
return 0 if delete_speaker(args.delete, service_url) else 1
365+
if not args.user_id:
366+
logger.error("❌ --user-id is required to delete a speaker")
367+
return 1
368+
return 0 if delete_speaker(args.delete, args.user_id, service_url) else 1
361369

362370
else:
363371
# Enrollment actions require ID and name

extras/speaker-recognition/src/simple_speaker_recognition/api/core/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from .utils import (
44
get_data_directory,
55
owner_of_speaker,
6+
require_speaker_owner,
67
safe_format_confidence,
78
secure_temp_file,
89
validate_confidence,
@@ -13,5 +14,6 @@
1314
"safe_format_confidence",
1415
"secure_temp_file",
1516
"owner_of_speaker",
17+
"require_speaker_owner",
1618
"validate_confidence",
1719
]

extras/speaker-recognition/src/simple_speaker_recognition/api/core/utils.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,9 @@ def secure_temp_file(suffix: str = ".wav") -> tempfile._TemporaryFileWrapper:
106106

107107

108108
def owner_of_speaker(speaker_id: str) -> str:
109-
"""Return the tenant that owns ``speaker_id``, from the speaker's own record.
110-
111-
``speakers.user_id`` is the ownership record, so it is the only thing consulted.
112-
This used to parse the tenant out of the id — ``int(speaker_id.split("_")[1])`` —
113-
which the audit flags as inferring a tenant from a SpeakerId: the
114-
``user_{user_id}_speaker_{...}`` shape is a naming convention, not a fact, and it
115-
is wrong for a speaker that was imported or created by a client that names ids
116-
differently. It also stopped parsing entirely once a tenant became a Chronicle
117-
ObjectId rather than an integer.
118-
119-
Only for speakers that already exist. Enrolment creates one, so it takes an
120-
explicit ``user_id`` instead of leaving the service to guess.
109+
"""Return the tenant that owns ``speaker_id``, read from ``speakers.user_id``.
110+
111+
A speaker id is opaque. Nothing about the tenant is inferred from its text.
121112
"""
122113

123114
# Imported here rather than at module scope: database.queries imports the API
@@ -135,6 +126,24 @@ def owner_of_speaker(speaker_id: str) -> str:
135126
db.close()
136127

137128

129+
def require_speaker_owner(speaker_id: str, user_id: str) -> str:
130+
"""Return ``user_id`` if it owns ``speaker_id``, else 404.
131+
132+
Scopes a request to the tenant the caller says it is acting for, so a client
133+
holding a stale or wrong tenant cannot reach another tenant's speaker. The
134+
service has no caller authentication, so this bounds mistakes, not attackers.
135+
A mismatch is 404 rather than 403 to avoid confirming the speaker exists.
136+
"""
137+
138+
owner = owner_of_speaker(speaker_id)
139+
if owner != user_id:
140+
log.warning(
141+
"Tenant %s requested speaker %s owned by %s", user_id, speaker_id, owner
142+
)
143+
raise HTTPException(404, f"Speaker not found: {speaker_id}")
144+
return owner
145+
146+
138147
def validate_confidence(confidence: Any, context: str = "") -> float:
139148
"""Validate and sanitize confidence values from speaker identification.
140149

extras/speaker-recognition/src/simple_speaker_recognition/api/routers/enrollment.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from fastapi import APIRouter, Depends, File, Form, HTTPException, UploadFile
1313
from simple_speaker_recognition.api.core.utils import (
1414
get_data_directory,
15-
owner_of_speaker,
15+
require_speaker_owner,
1616
secure_temp_file,
1717
)
1818
from simple_speaker_recognition.core.unified_speaker_db import UnifiedSpeakerDB
@@ -538,11 +538,11 @@ async def enroll_append(
538538
..., description="Multiple audio files to append to existing speaker"
539539
),
540540
speaker_id: str = Form(..., description="Existing speaker identifier"),
541+
user_id: str = Form(..., description="Chronicle user id owning this speaker"),
541542
db: UnifiedSpeakerDB = Depends(get_db),
542543
):
543544
"""Append audio segments to an existing speaker, computing weighted average embedding."""
544-
# The speaker must already exist, so its recorded owner is authoritative.
545-
user_id = owner_of_speaker(speaker_id)
545+
require_speaker_owner(speaker_id, user_id)
546546
log.info(
547547
f"Appending to speaker: {speaker_id} (User: {user_id}) with {len(files)} files"
548548
)

0 commit comments

Comments
 (0)