mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-15 17:25:26 -04:00
fix(models): allow same endpoint URL with different keys (#2758)
* fix(models): allow same endpoint URL with different keys * fix(models): show endpoint key fingerprints
This commit is contained in:
+35
-6
@@ -5,6 +5,7 @@ import re
|
||||
import uuid
|
||||
import json
|
||||
import socket
|
||||
import hashlib
|
||||
import time as _time
|
||||
import logging
|
||||
import httpx
|
||||
@@ -850,6 +851,14 @@ def _visible_models(cached_models, hidden_models, pinned_models=None):
|
||||
return [m for m in merged if m not in hidden]
|
||||
|
||||
|
||||
def _api_key_fingerprint(api_key: Optional[str]) -> str:
|
||||
"""Stable, non-secret label for distinguishing same-URL credentials."""
|
||||
key = (api_key or "").strip()
|
||||
if not key:
|
||||
return ""
|
||||
return hashlib.sha256(key.encode("utf-8")).hexdigest()[:8]
|
||||
|
||||
|
||||
def setup_model_routes(model_discovery):
|
||||
router = APIRouter(prefix="/api")
|
||||
|
||||
@@ -1397,6 +1406,7 @@ def setup_model_routes(model_discovery):
|
||||
"name": r.name,
|
||||
"base_url": r.base_url,
|
||||
"has_key": bool(r.api_key),
|
||||
"api_key_fingerprint": _api_key_fingerprint(r.api_key),
|
||||
"is_enabled": r.is_enabled,
|
||||
"models": visible,
|
||||
"pinned_models": pinned,
|
||||
@@ -1463,21 +1473,34 @@ def setup_model_routes(model_discovery):
|
||||
)
|
||||
explicit_timeout = _explicit_model_list_timeout(base_url, requested_kind, refresh_timeout)
|
||||
|
||||
# Dedupe: if an endpoint with the same base_url already exists and
|
||||
# is reachable by the caller (shared or owned by them), return it
|
||||
# instead of creating a duplicate row. Fixes "Scan for Servers"
|
||||
# re-adding manually-added endpoints under their host:port name.
|
||||
# Dedupe: if an endpoint with the same base_url and compatible
|
||||
# credentials already exists and is reachable by the caller (shared or
|
||||
# owned by them), return it instead of creating a duplicate row. Keep
|
||||
# same-url/different-key rows distinct so users can group the same
|
||||
# provider URL under multiple credentials.
|
||||
from src.auth_helpers import get_current_user as _gcu_dedup
|
||||
_caller = _gcu_dedup(request) or None
|
||||
_incoming_api_key = api_key.strip()
|
||||
_db_dedup = SessionLocal()
|
||||
try:
|
||||
existing = (
|
||||
_same_url_rows = (
|
||||
_db_dedup.query(ModelEndpoint)
|
||||
.filter(ModelEndpoint.base_url == base_url)
|
||||
.filter((ModelEndpoint.owner.is_(None)) | (ModelEndpoint.owner == _caller))
|
||||
.order_by(ModelEndpoint.owner.desc()) # prefer owned over shared
|
||||
.first()
|
||||
.all()
|
||||
)
|
||||
existing = None
|
||||
_empty_key_existing = None
|
||||
for _candidate in _same_url_rows:
|
||||
_candidate_key = (getattr(_candidate, "api_key", None) or "").strip()
|
||||
if _candidate_key == _incoming_api_key:
|
||||
existing = _candidate
|
||||
break
|
||||
if _incoming_api_key and not _candidate_key and _empty_key_existing is None:
|
||||
_empty_key_existing = _candidate
|
||||
if existing is None and _incoming_api_key and _empty_key_existing is not None:
|
||||
existing = _empty_key_existing
|
||||
if existing:
|
||||
changed = False
|
||||
# Persist any incoming pinned IDs onto the existing row. An
|
||||
@@ -1526,6 +1549,8 @@ def setup_model_routes(model_discovery):
|
||||
"id": existing.id,
|
||||
"name": existing.name,
|
||||
"base_url": existing.base_url,
|
||||
"has_key": bool(existing.api_key),
|
||||
"api_key_fingerprint": _api_key_fingerprint(existing.api_key),
|
||||
"models": _visible_models(
|
||||
existing_models,
|
||||
getattr(existing, "hidden_models", None),
|
||||
@@ -1599,6 +1624,8 @@ def setup_model_routes(model_discovery):
|
||||
"id": ep_id,
|
||||
"name": name.strip(),
|
||||
"base_url": base_url,
|
||||
"has_key": bool(api_key.strip()),
|
||||
"api_key_fingerprint": _api_key_fingerprint(api_key),
|
||||
"models": _merge_model_ids(model_ids, _pinned),
|
||||
"pinned_models": _pinned,
|
||||
"online": bool(model_ids) or bool(_pinned) or bool(ping.get("reachable")),
|
||||
@@ -1948,6 +1975,8 @@ def setup_model_routes(model_discovery):
|
||||
"name": ep.name,
|
||||
"model_type": ep.model_type,
|
||||
"base_url": ep.base_url,
|
||||
"has_key": bool(ep.api_key),
|
||||
"api_key_fingerprint": _api_key_fingerprint(ep.api_key),
|
||||
"pinned_models": _normalize_model_ids(getattr(ep, "pinned_models", None)),
|
||||
"endpoint_kind": getattr(ep, "endpoint_kind", None) or "auto",
|
||||
"model_refresh_mode": getattr(ep, "model_refresh_mode", None) or "auto",
|
||||
|
||||
+4
-1
@@ -417,6 +417,9 @@ async function loadEndpoints() {
|
||||
const justAddedClass = (_recentlyAddedEpId && String(ep.id) === _recentlyAddedEpId) ? ' adm-ep-just-added' : '';
|
||||
const category = ep.category || (_isLocalEndpoint(ep.base_url) ? 'local' : 'api');
|
||||
const kindLabel = ep.endpoint_kind && ep.endpoint_kind !== 'auto' ? ep.endpoint_kind.toUpperCase() : '';
|
||||
const keyLabel = ep.has_key
|
||||
? (ep.api_key_fingerprint ? ` (key ${esc(ep.api_key_fingerprint)})` : ' (key set)')
|
||||
: '';
|
||||
return `
|
||||
<div class="admin-user-row${ep.is_enabled ? '' : ' admin-ep-disabled'}${justAddedClass}" data-adm-ep-id="${ep.id}">
|
||||
<div style="display:flex;align-items:center;justify-content:space-between;${hasModels ? 'cursor:pointer;' : ''}padding:4px 0;" data-adm-ep-header="${ep.id}">
|
||||
@@ -434,7 +437,7 @@ async function loadEndpoints() {
|
||||
${hasModels ? '<svg class="admin-user-chevron" width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round" style="opacity:0.3;transition:transform 0.2s,opacity 0.2s;"><polyline points="6 9 12 15 18 9"/></svg>' : ''}
|
||||
</div>
|
||||
</div>
|
||||
<div class="admin-ep-detail">${esc(ep.base_url)}${category === 'local' ? `<button type="button" class="admin-ep-copy-btn" data-adm-copy-url="${esc(ep.base_url)}" title="Copy URL" aria-label="Copy URL" style="background:none;border:none;padding:0 2px;margin-left:6px;cursor:pointer;color:inherit;opacity:0.45;vertical-align:-2px;line-height:1;"><svg width="11" height="11" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect x="9" y="9" width="13" height="13" rx="2"/><path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1"/></svg></button>` : ''}${ep.has_key ? ' (key set)' : ''}</div>
|
||||
<div class="admin-ep-detail">${esc(ep.base_url)}${category === 'local' ? `<button type="button" class="admin-ep-copy-btn" data-adm-copy-url="${esc(ep.base_url)}" title="Copy URL" aria-label="Copy URL" style="background:none;border:none;padding:0 2px;margin-left:6px;cursor:pointer;color:inherit;opacity:0.45;vertical-align:-2px;line-height:1;"><svg width="11" height="11" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect x="9" y="9" width="13" height="13" rx="2"/><path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1"/></svg></button>` : ''}${keyLabel}</div>
|
||||
${hasModels ? `<div class="mcp-tools-panel hidden" data-adm-ep-models-panel="${ep.id}"></div>` : ''}
|
||||
</div>`;
|
||||
});
|
||||
|
||||
@@ -37,6 +37,7 @@ from routes.model_routes import (
|
||||
_curate_models,
|
||||
_visible_models,
|
||||
_normalize_model_ids,
|
||||
_api_key_fingerprint,
|
||||
_is_chat_model,
|
||||
_classify_endpoint,
|
||||
_effective_endpoint_kind,
|
||||
@@ -754,6 +755,16 @@ def test_visible_models_handles_malformed_strings():
|
||||
assert _visible_models("only-cached", None, None) == ["only-cached"]
|
||||
|
||||
|
||||
def test_api_key_fingerprint_is_stable_and_non_secret():
|
||||
fp_one = _api_key_fingerprint("key-one")
|
||||
|
||||
assert _api_key_fingerprint("") == ""
|
||||
assert fp_one == _api_key_fingerprint(" key-one ")
|
||||
assert fp_one != _api_key_fingerprint("key-two")
|
||||
assert len(fp_one) == 8
|
||||
assert "key-one" not in fp_one
|
||||
|
||||
|
||||
def _create_form_kwargs(**overrides):
|
||||
"""Defaults for every Form() param create_model_endpoint reads directly.
|
||||
|
||||
@@ -791,6 +802,29 @@ def _patch_create_deps(monkeypatch, db):
|
||||
monkeypatch.setattr(auth_helpers, "get_current_user", lambda req: None)
|
||||
|
||||
|
||||
def test_list_model_endpoints_returns_key_fingerprint(monkeypatch):
|
||||
endpoint_with_key = _make_endpoint(
|
||||
api_key="key-one",
|
||||
cached_models=json.dumps(["m1"]),
|
||||
)
|
||||
endpoint_without_key = _make_endpoint(
|
||||
id="ep2",
|
||||
api_key=None,
|
||||
cached_models=json.dumps(["m2"]),
|
||||
)
|
||||
db = _PinnedFakeDb([endpoint_with_key, endpoint_without_key])
|
||||
monkeypatch.setattr(model_routes, "SessionLocal", lambda: db)
|
||||
monkeypatch.setattr(model_routes, "require_admin", lambda request: None)
|
||||
endpoint = _get_route("/api/model-endpoints", "GET")
|
||||
|
||||
result = endpoint(_PinnedFakeRequest())
|
||||
|
||||
assert result[0]["has_key"] is True
|
||||
assert result[0]["api_key_fingerprint"] == _api_key_fingerprint("key-one")
|
||||
assert result[1]["has_key"] is False
|
||||
assert result[1]["api_key_fingerprint"] == ""
|
||||
|
||||
|
||||
def test_post_creates_endpoint_with_pinned_models(monkeypatch):
|
||||
db = _PinnedFakeDb([]) # no existing row → fresh create path
|
||||
_patch_create_deps(monkeypatch, db)
|
||||
@@ -856,6 +890,53 @@ def test_post_dedupe_existing_does_not_clobber_pinned_when_omitted(monkeypatch):
|
||||
assert json.loads(existing.pinned_models) == ["keep-me"]
|
||||
assert result["pinned_models"] == ["keep-me"]
|
||||
assert db.committed == 0 # nothing to persist
|
||||
|
||||
|
||||
def test_post_same_base_url_different_api_key_creates_distinct_endpoint(monkeypatch):
|
||||
existing = _make_endpoint(
|
||||
base_url="https://api.example.test/v1",
|
||||
api_key="key-one",
|
||||
)
|
||||
db = _PinnedFakeDb([existing])
|
||||
_patch_create_deps(monkeypatch, db)
|
||||
create = _get_route("/api/model-endpoints", "POST")
|
||||
|
||||
result = create(
|
||||
_PinnedFakeRequest(),
|
||||
base_url="https://api.example.test/v1",
|
||||
**_create_form_kwargs(api_key="key-two"),
|
||||
)
|
||||
|
||||
assert result.get("existing") is not True
|
||||
assert result["has_key"] is True
|
||||
assert result["api_key_fingerprint"] == _api_key_fingerprint("key-two")
|
||||
assert len(db.added) == 1
|
||||
assert db.added[0].base_url == "https://api.example.test/v1"
|
||||
assert db.added[0].api_key == "key-two"
|
||||
|
||||
|
||||
def test_post_same_base_url_same_api_key_still_dedupes(monkeypatch):
|
||||
existing = _make_endpoint(
|
||||
base_url="https://api.example.test/v1",
|
||||
api_key="key-one",
|
||||
)
|
||||
db = _PinnedFakeDb([existing])
|
||||
_patch_create_deps(monkeypatch, db)
|
||||
create = _get_route("/api/model-endpoints", "POST")
|
||||
|
||||
result = create(
|
||||
_PinnedFakeRequest(),
|
||||
base_url="https://api.example.test/v1",
|
||||
**_create_form_kwargs(api_key="key-one"),
|
||||
)
|
||||
|
||||
assert result["existing"] is True
|
||||
assert result["id"] == existing.id
|
||||
assert result["has_key"] is True
|
||||
assert result["api_key_fingerprint"] == _api_key_fingerprint("key-one")
|
||||
assert db.added == []
|
||||
|
||||
|
||||
class _RouteQuery:
|
||||
def __init__(self, rows):
|
||||
self.rows = list(rows)
|
||||
|
||||
Reference in New Issue
Block a user