mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-15 17:25:26 -04:00
fix(auth): per-user allowed-models checklist ignores cache, [None] doesn't block (#3355)
Three issues combined to make the per-user 'Allowed models' checklist unreliable (#3032): 1. admin.js _loadModelsForUser fetched /api/models, which is backed by cached_models — endpoints that haven't been probed yet (e.g. a freshly-added DeepSeek API endpoint) simply didn't show up in the checklist. Switched to /api/model-endpoints, which always reflects every configured endpoint regardless of cache state. 2. _saveModels sent allowed_models: [] both when the admin clicked [All] (no restriction) and [None] (block everything) — the backend had no way to distinguish the two. 3. _enforce_chat_privileges treated an empty allowed_models list as 'no restriction' (falsy -> skip the check), so [None] had no effect. Added an explicit block_all_models privilege flag (defaulting to False, and forced to False for admins) that admin.js now sets when zero models are checked. _enforce_chat_privileges checks it first and 403s regardless of allowed_models contents.
This commit is contained in:
@@ -31,11 +31,20 @@ DEFAULT_PRIVILEGES = {
|
||||
"max_messages_per_day": 0,
|
||||
"allowed_models": [],
|
||||
"allowed_models_restricted": False,
|
||||
# Explicit "block every model" sentinel. An empty `allowed_models` list is
|
||||
# ambiguous — it's also what gets sent when the admin clicks "[All]" — so
|
||||
# we need a dedicated flag to express "this user may use no models at all"
|
||||
# distinctly from "this user has no restriction".
|
||||
"block_all_models": False,
|
||||
}
|
||||
|
||||
# Admins get everything
|
||||
ADMIN_PRIVILEGES = {k: (True if isinstance(v, bool) else (0 if isinstance(v, int) else [])) for k, v in DEFAULT_PRIVILEGES.items()}
|
||||
ADMIN_PRIVILEGES["allowed_models_restricted"] = False
|
||||
# Admins must never be blocked from using models — the generic dict
|
||||
# comprehension above flips every boolean default to True, which would be
|
||||
# backwards for this sentinel.
|
||||
ADMIN_PRIVILEGES["block_all_models"] = False
|
||||
|
||||
from src.constants import AUTH_FILE
|
||||
DEFAULT_AUTH_PATH = AUTH_FILE
|
||||
|
||||
@@ -88,6 +88,14 @@ def _enforce_chat_privileges(request, sess) -> None:
|
||||
return
|
||||
|
||||
privs = auth_manager.get_privileges(user) or {}
|
||||
|
||||
# Explicit "block everything" sentinel takes precedence over the
|
||||
# allowlist — it's the only way to distinguish "user clicked [None]"
|
||||
# (block all) from "user clicked [All]" (no restriction), since both
|
||||
# otherwise produce an empty `allowed_models` list.
|
||||
if privs.get("block_all_models"):
|
||||
raise HTTPException(403, f"Your account is not allowed to use model '{sess.model}'.")
|
||||
|
||||
allowed_raw = privs.get("allowed_models")
|
||||
allowed = allowed_raw if isinstance(allowed_raw, list) else []
|
||||
restricted = bool(privs.get("allowed_models_restricted")) or bool(allowed)
|
||||
|
||||
+39
-14
@@ -93,6 +93,7 @@ async function loadUsers() {
|
||||
: [];
|
||||
const allowedSet = new Set(allowedModels);
|
||||
const modelsRestricted = !!(u.privileges && u.privileges.allowed_models_restricted);
|
||||
const blockAllModels = !!(u.privileges && u.privileges.block_all_models);
|
||||
html += `<div style="padding:4px 0;">
|
||||
<div style="display:flex;align-items:center;justify-content:space-between;">
|
||||
<span style="font-size:12px;">Allowed models</span>
|
||||
@@ -101,7 +102,7 @@ async function loadUsers() {
|
||||
<a href="#" class="priv-models-none" data-user="${esc(u.username)}" style="font-size:10px;opacity:0.5;">None</a>
|
||||
</div>
|
||||
</div>
|
||||
<div style="font-size:10px;opacity:0.4;margin-bottom:4px;">${!modelsRestricted ? 'All models allowed (no restrictions)' : (allowedSet.size === 0 ? 'No models allowed' : allowedSet.size + ' model(s) allowed')}</div>
|
||||
<div style="font-size:10px;opacity:0.4;margin-bottom:4px;">${blockAllModels ? 'No models allowed' : (!modelsRestricted ? 'All models allowed (no restrictions)' : (allowedSet.size === 0 ? 'No models allowed' : allowedSet.size + ' model(s) allowed'))}</div>
|
||||
<div class="priv-models-list" data-user="${esc(u.username)}">
|
||||
<span style="opacity:0.4;font-size:11px;">Loading models...</span>
|
||||
</div>
|
||||
@@ -123,7 +124,7 @@ async function loadUsers() {
|
||||
// Load models list on first expand
|
||||
if (!_modelsLoaded && !privPanel.classList.contains('hidden')) {
|
||||
_modelsLoaded = true;
|
||||
_loadModelsForUser(u.username, allowedSet, modelsRestricted, privPanel);
|
||||
_loadModelsForUser(u.username, allowedSet, modelsRestricted, blockAllModels, privPanel);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -203,17 +204,22 @@ async function loadUsers() {
|
||||
} catch (e) { list.innerHTML = '<div class="admin-error">Failed to load users</div>'; }
|
||||
}
|
||||
|
||||
async function _loadModelsForUser(username, allowedSet, modelsRestricted, privPanel) {
|
||||
async function _loadModelsForUser(username, allowedSet, modelsRestricted, blockAllModels, privPanel) {
|
||||
const listEl = privPanel.querySelector(`.priv-models-list[data-user="${username}"]`);
|
||||
if (!listEl) return;
|
||||
try {
|
||||
const res = await fetch('/api/models', { credentials: 'same-origin' });
|
||||
// Use /api/model-endpoints rather than /api/models — the latter is
|
||||
// backed by `cached_models`, so endpoints that haven't been probed yet
|
||||
// (e.g. a freshly-added cloud API like DeepSeek) simply don't show up
|
||||
// until some other endpoint happens to trigger a cache refresh. The
|
||||
// endpoints listing always reflects every configured endpoint.
|
||||
const res = await fetch('/api/model-endpoints', { credentials: 'same-origin' });
|
||||
const data = await res.json();
|
||||
const allModels = [];
|
||||
(data.items || []).forEach(item => {
|
||||
if (item.offline) return;
|
||||
(item.models || []).forEach(mid => {
|
||||
allModels.push({ mid, epName: item.endpoint_name || '', display: mid.split('/').pop() });
|
||||
(Array.isArray(data) ? data : []).forEach(ep => {
|
||||
if (!ep.online) return;
|
||||
(ep.models || []).forEach(mid => {
|
||||
allModels.push({ mid, epName: ep.name || '', display: mid.split('/').pop() });
|
||||
});
|
||||
});
|
||||
if (!allModels.length) {
|
||||
@@ -221,8 +227,9 @@ async function _loadModelsForUser(username, allowedSet, modelsRestricted, privPa
|
||||
return;
|
||||
}
|
||||
let restricted = modelsRestricted;
|
||||
let blockAll = blockAllModels;
|
||||
listEl.innerHTML = sortModelObjects(allModels).map(m => {
|
||||
const checked = !restricted || allowedSet.has(m.mid) ? 'checked' : '';
|
||||
const checked = !blockAll && (!restricted || allowedSet.has(m.mid)) ? 'checked' : '';
|
||||
return `<label>
|
||||
<input type="checkbox" class="priv-model-cb" data-mid="${esc(m.mid)}" ${checked}>
|
||||
<span>${esc(m.display)}</span>
|
||||
@@ -236,15 +243,33 @@ async function _loadModelsForUser(username, allowedSet, modelsRestricted, privPa
|
||||
listEl.querySelectorAll('.priv-model-cb').forEach(cb => {
|
||||
if (cb.checked) checked.push(cb.dataset.mid);
|
||||
});
|
||||
// All checked means unrestricted; zero checked means explicitly no models.
|
||||
restricted = checked.length !== allModels.length;
|
||||
const value = restricted ? checked : [];
|
||||
// Three distinct states the backend must be able to tell apart:
|
||||
// - all checked -> no restriction (allowed_models: [], block_all_models: false)
|
||||
// - none checked -> block everything (allowed_models: [], block_all_models: true)
|
||||
// - some checked -> allowlist (allowed_models: checked, block_all_models: false)
|
||||
let value, hintText;
|
||||
if (checked.length === allModels.length) {
|
||||
restricted = false;
|
||||
blockAll = false;
|
||||
value = [];
|
||||
hintText = 'All models allowed (no restrictions)';
|
||||
} else if (checked.length === 0) {
|
||||
restricted = true;
|
||||
blockAll = true;
|
||||
value = [];
|
||||
hintText = 'No models allowed';
|
||||
} else {
|
||||
restricted = true;
|
||||
blockAll = false;
|
||||
value = checked;
|
||||
hintText = value.length + ' model(s) allowed';
|
||||
}
|
||||
const hint = privPanel.querySelector('.priv-models-list[data-user]')?.previousElementSibling?.querySelector('div[style*="opacity"]');
|
||||
if (hint) hint.textContent = !restricted ? 'All models allowed (no restrictions)' : (value.length === 0 ? 'No models allowed' : value.length + ' model(s) allowed');
|
||||
if (hint) hint.textContent = hintText;
|
||||
fetch(`/api/auth/users/${encodeURIComponent(username)}/privileges`, {
|
||||
method: 'PUT', credentials: 'same-origin',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ allowed_models: value, allowed_models_restricted: restricted }),
|
||||
body: JSON.stringify({ allowed_models: value, allowed_models_restricted: restricted, block_all_models: blockAll }),
|
||||
}).catch(() => {});
|
||||
}
|
||||
listEl.querySelectorAll('.priv-model-cb').forEach(cb => cb.addEventListener('change', _saveModels));
|
||||
|
||||
@@ -69,6 +69,64 @@ def test_allowed_models_nonempty_list_still_restricts_without_new_flag(monkeypat
|
||||
)
|
||||
|
||||
|
||||
def test_no_restriction_allows_any_model(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
|
||||
privs = {"allowed_models": [], "block_all_models": False, "max_messages_per_day": 0}
|
||||
_enforce_chat_privileges(_Request(privs), _Session("provider/model-a"))
|
||||
_enforce_chat_privileges(_Request(privs), _Session("provider/model-z"))
|
||||
|
||||
|
||||
def test_specific_allowlist_blocks_models_outside_it(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
|
||||
privs = {
|
||||
"allowed_models": ["gpt-4"],
|
||||
"block_all_models": False,
|
||||
"max_messages_per_day": 0,
|
||||
}
|
||||
_enforce_chat_privileges(_Request(privs), _Session("gpt-4"))
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
_enforce_chat_privileges(_Request(privs), _Session("gpt-3.5"))
|
||||
assert exc.value.status_code == 403
|
||||
|
||||
|
||||
def test_block_all_models_blocks_regardless_of_allowed_models_contents(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
|
||||
# Even if allowed_models contains entries, block_all_models wins.
|
||||
privs = {
|
||||
"allowed_models": ["gpt-4", "gpt-3.5"],
|
||||
"block_all_models": True,
|
||||
"max_messages_per_day": 0,
|
||||
}
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
_enforce_chat_privileges(_Request(privs), _Session("gpt-4"))
|
||||
assert exc.value.status_code == 403
|
||||
|
||||
with pytest.raises(HTTPException):
|
||||
_enforce_chat_privileges(_Request(privs), _Session("anything-else"))
|
||||
|
||||
|
||||
def test_admin_user_is_never_blocked(monkeypatch):
|
||||
from core.auth import ADMIN_PRIVILEGES
|
||||
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "admin")
|
||||
|
||||
class _AdminAuthManager:
|
||||
def get_privileges(self, username):
|
||||
assert username == "admin"
|
||||
return dict(ADMIN_PRIVILEGES)
|
||||
|
||||
class _AdminRequest:
|
||||
def __init__(self):
|
||||
self.app = type("App", (), {})()
|
||||
self.app.state = type("State", (), {"auth_manager": _AdminAuthManager()})()
|
||||
|
||||
_enforce_chat_privileges(_AdminRequest(), _Session("provider/model-a"))
|
||||
_enforce_chat_privileges(_AdminRequest(), _Session("anything-else"))
|
||||
|
||||
|
||||
class _FakeSession:
|
||||
def __init__(self, model="selected-model"):
|
||||
self.model = model
|
||||
|
||||
Reference in New Issue
Block a user