mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-21 10:15:04 +02:00
Let API clients resolve user ids to names (#1894)
Archives, the queue and statistics report ownership as a numeric created_by_id, and statistics accept it as a filter, but nothing let an API key discover whose id was whose -- the only user listing returns emails, roles, group membership and full permission sets, so it is administrative and rejects keys. Add GET /users/slim returning id + username only, gated on a new users:read_slim permission mapped to can_read_status. That grants no data a key could not already reach: for API-keyed requests the permission deps return None as current_user, so the stats:filter_by_user guard short-circuits and ?created_by_id=N is already honoured for every N. What was missing was the ability to address the filter, not permission to use it. The full listing stays unmapped = admin-only. Also fix /auth/me, which answered an API key with a synthetic administrator: id 0, role admin, is_admin true and every permission in the enum. A key cannot reach an administrative route at all, so clients building their UI from that response rendered actions that 403 on use. It now reports the key owner's identity, is_admin false, and the permissions the key's scopes actually admit. Ownerless legacy keys keep id 0 but no longer claim admin. --- Source user names from the slim listing where only names are needed (#1894) Stats filter-by-user, the Archives print log filter, the File Manager username autocomplete, the camera-token owner column and the Finance member picker all render nothing but a username, but all of them read the full user listing, which is gated on the admin-level users:read. An operator granted stats:filter_by_user but not users:read got an empty filter with no indication why. Point them at /users/slim under a separate react-query key, since the full listing shares the 'users' key and the two shapes would clobber each other in the cache.
This commit is contained in:
@@ -21,6 +21,7 @@ from backend.app.core.auth import (
|
||||
RequirePermissionIfAuthEnabled,
|
||||
_is_token_fresh,
|
||||
_validate_api_key,
|
||||
apikey_effective_permissions,
|
||||
authenticate_user,
|
||||
authenticate_user_by_email,
|
||||
create_access_token,
|
||||
@@ -30,13 +31,13 @@ from backend.app.core.auth import (
|
||||
get_user_by_email,
|
||||
get_user_by_username,
|
||||
is_jti_revoked,
|
||||
resolve_apikey_owner,
|
||||
resolve_session_max_minutes,
|
||||
revoke_jti,
|
||||
security,
|
||||
)
|
||||
from backend.app.core.database import async_session, get_db
|
||||
from backend.app.core.oidc_env import env_bool
|
||||
from backend.app.core.permissions import ALL_PERMISSIONS
|
||||
from backend.app.models.auth_ephemeral import AuthEphemeralToken, AuthRateLimitEvent, EventType, TokenType
|
||||
from backend.app.models.group import Group
|
||||
from backend.app.models.settings import Settings
|
||||
@@ -89,17 +90,47 @@ def _user_to_response(user: User) -> UserResponse:
|
||||
)
|
||||
|
||||
|
||||
def _api_key_to_user_response(api_key) -> UserResponse:
|
||||
"""Create a synthetic admin UserResponse for a valid API key."""
|
||||
async def _api_key_to_user_response(db: AsyncSession, api_key) -> UserResponse:
|
||||
"""Describe a valid API key as the identity it actually carries (#1894).
|
||||
|
||||
Until 0.2.5 this returned a synthetic admin: ``id=0``, ``role="admin"``,
|
||||
``is_admin=True`` and every permission in the enum. That was wrong in both
|
||||
directions. A key cannot perform administrative operations at all --
|
||||
``_check_apikey_permissions`` denies every permission that is not in the
|
||||
scope allowlist -- so a client that builds its UI from this response (which
|
||||
is exactly what a native client does) rendered admin actions that 403 on
|
||||
use, and had no way to learn the id its own prints are filed under.
|
||||
|
||||
Now: identity comes from the key's owner, and ``permissions`` is the set the
|
||||
key can genuinely exercise. ``is_admin`` is always False because no key can
|
||||
reach an administrative route regardless of who owns it.
|
||||
|
||||
Legacy keys predating per-user ownership (``user_id IS NULL``) have no
|
||||
identity to report, so they keep ``id=0`` and the ``api-key:`` username --
|
||||
but they stop claiming admin. ``created_at`` describes the credential in
|
||||
both branches, unchanged.
|
||||
"""
|
||||
# Same resolution the permission gate uses, so what is reported here and
|
||||
# what is enforced there cannot drift -- including the 403 when the owner
|
||||
# has been deactivated, which makes the key dead rather than anonymous.
|
||||
owner = await resolve_apikey_owner(db, api_key)
|
||||
return UserResponse(
|
||||
id=0,
|
||||
username=f"api-key:{api_key.key_prefix}",
|
||||
id=owner.id if owner else 0,
|
||||
username=owner.username if owner else f"api-key:{api_key.key_prefix}",
|
||||
# Withheld on purpose: the owner's email is not needed to resolve
|
||||
# identity, and this response is reachable by anyone holding the key.
|
||||
email=None,
|
||||
role="admin",
|
||||
# Deprecated free-text field; "user" is the existing value meaning
|
||||
# "not an admin". Inventing an "api_key" role here would put a third
|
||||
# value into a field callers compare against string literals.
|
||||
role="user",
|
||||
is_active=True,
|
||||
is_admin=True,
|
||||
is_admin=False,
|
||||
auth_source=getattr(owner, "auth_source", "local") if owner else "local",
|
||||
# The key is not a group member -- listing the owner's groups would
|
||||
# imply capabilities the key does not inherit.
|
||||
groups=[],
|
||||
permissions=sorted(ALL_PERMISSIONS),
|
||||
permissions=apikey_effective_permissions(api_key, owner),
|
||||
created_at=api_key.created_at.isoformat(),
|
||||
)
|
||||
|
||||
@@ -637,8 +668,9 @@ async def get_current_user_info(
|
||||
"""Get current user information.
|
||||
|
||||
Accepts JWT tokens (via Authorization: Bearer header) and API keys
|
||||
(via X-API-Key header or Authorization: Bearer bb_xxx).
|
||||
API keys return a synthetic admin user with all permissions.
|
||||
(via X-API-Key header or Authorization: Bearer bb_xxx). API keys report
|
||||
their owner's identity and the permissions the key can actually exercise
|
||||
-- see ``_api_key_to_user_response``.
|
||||
"""
|
||||
import jwt
|
||||
from jwt.exceptions import PyJWTError as JWTError
|
||||
@@ -647,7 +679,7 @@ async def get_current_user_info(
|
||||
if x_api_key:
|
||||
api_key = await _validate_api_key(db, x_api_key)
|
||||
if api_key:
|
||||
return _api_key_to_user_response(api_key)
|
||||
return await _api_key_to_user_response(db, api_key)
|
||||
|
||||
# Check for Bearer token (could be JWT or API key)
|
||||
if credentials is not None:
|
||||
@@ -656,7 +688,7 @@ async def get_current_user_info(
|
||||
if token.startswith("bb_"):
|
||||
api_key = await _validate_api_key(db, token)
|
||||
if api_key:
|
||||
return _api_key_to_user_response(api_key)
|
||||
return await _api_key_to_user_response(db, api_key)
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid API key",
|
||||
|
||||
@@ -28,8 +28,19 @@ from backend.app.schemas.group import (
|
||||
router = APIRouter(prefix="/groups", tags=["groups"])
|
||||
|
||||
|
||||
# Permissions whose derived label would misdescribe what is being granted.
|
||||
# The derived form for USERS_READ_SLIM is "Read Slim Users", which reads as a
|
||||
# property of the users rather than of the response -- and an admin ticking a
|
||||
# box in the group editor has nothing else to go on (#1894).
|
||||
_PERMISSION_LABEL_OVERRIDES: dict[Permission, str] = {
|
||||
Permission.USERS_READ_SLIM: "List User Names (id + username only)",
|
||||
}
|
||||
|
||||
|
||||
def _permission_label(perm: Permission) -> str:
|
||||
"""Convert permission enum to human-readable label."""
|
||||
if perm in _PERMISSION_LABEL_OVERRIDES:
|
||||
return _PERMISSION_LABEL_OVERRIDES[perm]
|
||||
# e.g., "printers:read" -> "Read Printers"
|
||||
parts = perm.value.split(":")
|
||||
if len(parts) == 2:
|
||||
|
||||
@@ -13,6 +13,7 @@ from backend.app.core.auth import (
|
||||
ALGORITHM,
|
||||
SECRET_KEY,
|
||||
RequireAdminIfAuthEnabled,
|
||||
RequireAnyPermissionIfAuthEnabled,
|
||||
RequirePermissionIfAuthEnabled,
|
||||
get_current_user_optional,
|
||||
get_password_hash,
|
||||
@@ -34,7 +35,14 @@ from backend.app.models.settings import Settings
|
||||
from backend.app.models.user import User
|
||||
from backend.app.models.user_otp_code import UserOTPCode
|
||||
from backend.app.models.user_totp import UserTOTP
|
||||
from backend.app.schemas.auth import ChangePasswordRequest, GroupBrief, UserCreate, UserResponse, UserUpdate
|
||||
from backend.app.schemas.auth import (
|
||||
ChangePasswordRequest,
|
||||
GroupBrief,
|
||||
UserCreate,
|
||||
UserResponse,
|
||||
UserSlim,
|
||||
UserUpdate,
|
||||
)
|
||||
from backend.app.services.email_service import (
|
||||
create_welcome_email_from_template,
|
||||
generate_secure_password,
|
||||
@@ -190,6 +198,31 @@ async def create_user(
|
||||
return _user_to_response(new_user)
|
||||
|
||||
|
||||
@router.get("/slim", response_model=list[UserSlim])
|
||||
async def list_users_slim(
|
||||
_: User | None = RequireAnyPermissionIfAuthEnabled(Permission.USERS_READ_SLIM, Permission.USERS_READ),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""List users as ``{id, username}`` only (#1894).
|
||||
|
||||
Exists so an API key -- or a group that should not see emails, roles and
|
||||
permission sets -- can turn the ``created_by_id`` values it already gets
|
||||
back from archives, stats and the queue into names.
|
||||
|
||||
``USERS_READ`` is accepted alongside ``USERS_READ_SLIM`` because it is
|
||||
strictly broader; groups that already hold it keep working without a
|
||||
permission backfill. For API keys only the slim permission resolves (the
|
||||
full one is unmapped = administrative), so a key reaches this and not the
|
||||
listing above.
|
||||
|
||||
Declared before ``/{user_id}`` on purpose: FastAPI matches in declaration
|
||||
order, and the reverse order would parse "slim" as the int path parameter
|
||||
and answer 422.
|
||||
"""
|
||||
result = await db.execute(select(User.id, User.username).order_by(User.username))
|
||||
return [UserSlim(id=row.id, username=row.username) for row in result.all()]
|
||||
|
||||
|
||||
@router.get("/{user_id}", response_model=UserResponse)
|
||||
async def get_user(
|
||||
user_id: int,
|
||||
|
||||
@@ -5,7 +5,7 @@ from pydantic import BaseModel
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from backend.app.core.auth import check_permission, check_printer_access, get_api_key
|
||||
from backend.app.core.auth import check_printer_access, check_webhook_permission, get_api_key
|
||||
from backend.app.core.database import get_db
|
||||
from backend.app.models.api_key import APIKey
|
||||
from backend.app.models.archive import PrintArchive
|
||||
@@ -68,7 +68,7 @@ async def webhook_add_to_queue(
|
||||
|
||||
Requires 'can_queue' permission.
|
||||
"""
|
||||
check_permission(api_key, "queue")
|
||||
await check_webhook_permission(db, api_key, "queue")
|
||||
check_printer_access(api_key, data.printer_id)
|
||||
|
||||
# Verify archive exists
|
||||
@@ -153,7 +153,7 @@ async def webhook_start_print(
|
||||
|
||||
Requires 'can_control_printer' permission.
|
||||
"""
|
||||
check_permission(api_key, "control_printer")
|
||||
await check_webhook_permission(db, api_key, "control_printer")
|
||||
check_printer_access(api_key, printer_id)
|
||||
|
||||
# Get printer
|
||||
@@ -191,12 +191,13 @@ async def webhook_start_print(
|
||||
async def webhook_stop_print(
|
||||
printer_id: int,
|
||||
api_key: APIKey = Depends(get_api_key),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Stop the current print on a printer.
|
||||
|
||||
Requires 'can_control_printer' permission.
|
||||
"""
|
||||
check_permission(api_key, "control_printer")
|
||||
await check_webhook_permission(db, api_key, "control_printer")
|
||||
check_printer_access(api_key, printer_id)
|
||||
|
||||
status = printer_manager.get_status(printer_id)
|
||||
@@ -222,12 +223,13 @@ async def webhook_stop_print(
|
||||
async def webhook_cancel_print(
|
||||
printer_id: int,
|
||||
api_key: APIKey = Depends(get_api_key),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Cancel the current print on a printer.
|
||||
|
||||
Requires 'can_control_printer' permission.
|
||||
"""
|
||||
check_permission(api_key, "control_printer")
|
||||
await check_webhook_permission(db, api_key, "control_printer")
|
||||
check_printer_access(api_key, printer_id)
|
||||
|
||||
status = printer_manager.get_status(printer_id)
|
||||
@@ -257,7 +259,7 @@ async def webhook_get_printer_status(
|
||||
|
||||
Requires 'can_read_status' permission.
|
||||
"""
|
||||
check_permission(api_key, "read_status")
|
||||
await check_webhook_permission(db, api_key, "read_status")
|
||||
check_printer_access(api_key, printer_id)
|
||||
|
||||
# Get printer
|
||||
@@ -293,7 +295,7 @@ async def webhook_get_queue_status(
|
||||
|
||||
Requires 'can_read_status' permission.
|
||||
"""
|
||||
check_permission(api_key, "read_status")
|
||||
await check_webhook_permission(db, api_key, "read_status")
|
||||
|
||||
# Get printers
|
||||
if printer_id:
|
||||
|
||||
+146
-10
@@ -49,8 +49,15 @@ logger = logging.getLogger(__name__)
|
||||
# The denylist is retained for documentation / drift-detection only — its
|
||||
# entries also satisfy "not in the allowlist", so they fail closed regardless.
|
||||
#
|
||||
# #1894 follow-on: the allowlist is a ceiling, not a grant. A key is also
|
||||
# narrowed to what its owner may do, so a user who can create keys cannot mint
|
||||
# themselves authority they do not have, and deactivating a user disables their
|
||||
# keys. Legacy ownerless keys (``user_id IS NULL``) have no owner to narrow
|
||||
# against and remain governed by the scope flags alone.
|
||||
#
|
||||
# Mapping rationale (see wiki/features/api-keys.md):
|
||||
# can_read_status → every ``*_READ`` + camera + stats + system + websocket
|
||||
# + the slim id/username user listing (NOT ``users:read``)
|
||||
# can_queue → queue write ops + archive reprint
|
||||
# can_control_printer → physical printer + smart-plug control
|
||||
# can_manage_library → library upload/own + MakerWorld import (separate
|
||||
@@ -94,6 +101,14 @@ _APIKEY_SCOPE_BY_PERMISSION: dict[Permission, str] = {
|
||||
Permission.PRINTER_SENSOR_HISTORY_READ: "can_read_status",
|
||||
Permission.STATS_READ: "can_read_status",
|
||||
Permission.STATS_FILTER_BY_USER: "can_read_status",
|
||||
# USERS_READ_SLIM grants no data an API key could not already reach (#1894):
|
||||
# for API-keyed requests the permission deps return None as ``current_user``,
|
||||
# so ``_validate_user_filter_permission`` in routes/archives.py short-circuits
|
||||
# and ``?created_by_id=N`` is already honoured for every N. Without a way to
|
||||
# discover the ids, that filter is only addressable by brute force. The slim
|
||||
# listing makes it usable; the full USERS_READ listing (emails, roles, group
|
||||
# membership, permission sets) stays unmapped = admin-only.
|
||||
Permission.USERS_READ_SLIM: "can_read_status",
|
||||
Permission.SYSTEM_READ: "can_read_status",
|
||||
# SETTINGS_READ stays allowed via read-status so SpoolBuddy kiosks keep
|
||||
# working (they need the UI-language setting via API key).
|
||||
@@ -293,13 +308,89 @@ def _resolve_apikey_scope(perm_string: str) -> str | None:
|
||||
return _APIKEY_SCOPE_BY_PERMISSION.get(perm)
|
||||
|
||||
|
||||
def _check_apikey_permissions(api_key: APIKey, perm_strings: list[str], *, require_any: bool = False) -> None:
|
||||
def apikey_effective_permissions(api_key: APIKey, owner: User | None = None) -> list[str]:
|
||||
"""Return the permissions ``api_key`` can actually exercise, sorted.
|
||||
|
||||
This is the exact set ``_check_apikey_permissions`` will let through: every
|
||||
mapped permission whose scope flag is True on the key, further narrowed to
|
||||
what ``owner`` may do. Unmapped permissions are administrative and never
|
||||
resolve for a key, so they are absent.
|
||||
|
||||
``owner=None`` means a legacy ownerless key, where the scope flags are the
|
||||
whole of the key's authority -- not "skip the owner check". Callers holding
|
||||
an owned key must pass the owner, or ``/auth/me`` will over-report and drift
|
||||
from the gate, which is the defect #1894 was about.
|
||||
"""
|
||||
return sorted(
|
||||
perm.value
|
||||
for perm, scope_attr in _APIKEY_SCOPE_BY_PERMISSION.items()
|
||||
if getattr(api_key, scope_attr, False) and (owner is None or owner.has_permission(perm.value))
|
||||
)
|
||||
|
||||
|
||||
async def resolve_apikey_owner(db: AsyncSession, api_key: APIKey) -> User | None:
|
||||
"""Load the owner of ``api_key`` for an authorization decision.
|
||||
|
||||
Distinct from ``_user_from_api_key``, which answers "who is this, if
|
||||
anyone" and returns None for both the legacy and the broken case. Here
|
||||
those two must not be conflated:
|
||||
|
||||
- ``user_id IS NULL`` -- a key predating per-user ownership. There is no
|
||||
owner to narrow against, so the scope flags stand alone. Returns None.
|
||||
- ``user_id`` set but the row is missing or deactivated -- the key's
|
||||
authority came from a user who no longer has any. Raises 403 rather than
|
||||
returning None, because returning None here would fail open: deactivating
|
||||
a user would leave their keys working with full scope authority.
|
||||
|
||||
Groups are eager-loaded because ``has_permission`` walks them, and a lazy
|
||||
load inside the permission check would raise MissingGreenlet.
|
||||
"""
|
||||
if api_key.user_id is None:
|
||||
return None
|
||||
result = await db.execute(select(User).where(User.id == api_key.user_id).options(selectinload(User.groups)))
|
||||
owner = result.scalar_one_or_none()
|
||||
if owner is None or not owner.is_active:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="API key owner is deactivated or no longer exists",
|
||||
)
|
||||
return owner
|
||||
|
||||
|
||||
async def authorize_api_key(
|
||||
db: AsyncSession,
|
||||
api_key: APIKey,
|
||||
perm_strings: list[str],
|
||||
*,
|
||||
require_any: bool = False,
|
||||
) -> None:
|
||||
"""Resolve the key's owner and run the full permission gate. Raises 403."""
|
||||
owner = await resolve_apikey_owner(db, api_key)
|
||||
_check_apikey_permissions(api_key, perm_strings, owner=owner, require_any=require_any)
|
||||
|
||||
|
||||
def _check_apikey_permissions(
|
||||
api_key: APIKey,
|
||||
perm_strings: list[str],
|
||||
*,
|
||||
owner: User | None = None,
|
||||
require_any: bool = False,
|
||||
) -> None:
|
||||
"""Raise 403 unless ``api_key`` is allowed to use ``perm_strings``.
|
||||
|
||||
Allowlist semantics: every requested permission MUST be present in
|
||||
``_APIKEY_SCOPE_BY_PERMISSION`` AND its scope flag must be True on
|
||||
``api_key``. Unmapped permissions = administrative = 403.
|
||||
|
||||
A key must not out-rank the user it belongs to, so when ``owner`` is given
|
||||
the permission must additionally be one the owner holds. Scope flags are
|
||||
chosen at creation time by whoever holds ``api_keys:create``; that is
|
||||
admin-only in the default groups, but a custom group can grant it, and
|
||||
without this check such a user could mint themselves a key with
|
||||
``can_control_printer`` and act through it beyond their own permissions.
|
||||
``owner=None`` is only correct for legacy ownerless keys -- see
|
||||
``resolve_apikey_owner``.
|
||||
|
||||
By default ALL requested permissions must pass (mirrors
|
||||
``require_permission`` / ``require_permission_if_auth_enabled``).
|
||||
When ``require_any=True``, only one needs to pass (mirrors
|
||||
@@ -327,6 +418,11 @@ def _check_apikey_permissions(api_key: APIKey, perm_strings: list[str], *, requi
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail=f"API key does not have '{scope_attr}' permission",
|
||||
)
|
||||
elif owner is not None and not owner.has_permission(perm_str):
|
||||
failure = HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail=f"API key owner does not have '{perm_str}' permission",
|
||||
)
|
||||
else:
|
||||
failure = None
|
||||
|
||||
@@ -384,6 +480,12 @@ def require_energy_cost_update():
|
||||
detail="Invalid API key",
|
||||
headers={"WWW-Authenticate": "Bearer"},
|
||||
)
|
||||
# Fails closed if the owner has been deactivated. The scope
|
||||
# flag itself is not narrowed against the owner's permissions
|
||||
# the way the general gate is: this door exists precisely
|
||||
# because no user permission maps to it (SETTINGS_UPDATE stays
|
||||
# denied for keys even when the owner is an administrator).
|
||||
await resolve_apikey_owner(db, api_key)
|
||||
if not api_key.can_update_energy_cost:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
@@ -1133,10 +1235,14 @@ async def require_auth_if_enabled(
|
||||
if not auth_enabled:
|
||||
return None
|
||||
|
||||
# Check for API key first (X-API-Key header)
|
||||
# Check for API key first (X-API-Key header). The owner is resolved
|
||||
# purely for its side effect: a key whose owner has been deactivated
|
||||
# must be dead everywhere, not just on the permission-gated routes.
|
||||
# There is no permission to check here -- this dep is auth-only.
|
||||
if x_api_key:
|
||||
api_key = await _validate_api_key(db, x_api_key)
|
||||
if api_key:
|
||||
await resolve_apikey_owner(db, api_key)
|
||||
return None # API key valid, allow access
|
||||
|
||||
# Check for Bearer token (could be JWT or API key)
|
||||
@@ -1146,6 +1252,7 @@ async def require_auth_if_enabled(
|
||||
if token.startswith("bb_"):
|
||||
api_key = await _validate_api_key(db, token)
|
||||
if api_key:
|
||||
await resolve_apikey_owner(db, api_key)
|
||||
return None # API key valid, allow access
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
@@ -1424,6 +1531,35 @@ def check_permission(api_key: APIKey, permission: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
# The coarse webhook permission names predate the Permission enum. Each maps to
|
||||
# the enum member that best represents it, so the owner can be held to the same
|
||||
# standard here as on the modern routes.
|
||||
_WEBHOOK_PERMISSION_EQUIVALENT: dict[str, Permission] = {
|
||||
"queue": Permission.QUEUE_CREATE,
|
||||
"control_printer": Permission.PRINTERS_CONTROL,
|
||||
"read_status": Permission.PRINTERS_READ,
|
||||
}
|
||||
|
||||
|
||||
async def check_webhook_permission(db: AsyncSession, api_key: APIKey, permission: str) -> None:
|
||||
"""``check_permission`` plus the owner checks the modern routes apply.
|
||||
|
||||
``/webhook/*`` reaches its scope flags through ``check_permission`` rather
|
||||
than ``_check_apikey_permissions``, so it does not pick up the owner
|
||||
narrowing automatically. Without this it would be the way around the gate:
|
||||
the same key that is refused printer control on ``/printers/{id}/print/stop``
|
||||
could stop the print through ``/webhook/printer/{id}/stop``.
|
||||
"""
|
||||
check_permission(api_key, permission)
|
||||
owner = await resolve_apikey_owner(db, api_key)
|
||||
equivalent = _WEBHOOK_PERMISSION_EQUIVALENT.get(permission)
|
||||
if owner is not None and equivalent is not None and not owner.has_permission(equivalent.value):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail=f"API key owner does not have '{equivalent.value}' permission",
|
||||
)
|
||||
|
||||
|
||||
def check_printer_access(api_key: APIKey, printer_id: int) -> None:
|
||||
"""Check if API key has access to the specified printer.
|
||||
|
||||
@@ -1481,7 +1617,7 @@ def require_permission(*permissions: str | Permission):
|
||||
if x_api_key:
|
||||
api_key = await _validate_api_key(db, x_api_key)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, perm_strings)
|
||||
await authorize_api_key(db, api_key, perm_strings)
|
||||
return None # API key valid, allow access
|
||||
|
||||
credentials_exception = HTTPException(
|
||||
@@ -1498,7 +1634,7 @@ def require_permission(*permissions: str | Permission):
|
||||
if token.startswith("bb_"):
|
||||
api_key = await _validate_api_key(db, token)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, perm_strings)
|
||||
await authorize_api_key(db, api_key, perm_strings)
|
||||
return None # API key valid, allow access
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
@@ -1571,7 +1707,7 @@ def require_permission_if_auth_enabled(*permissions: str | Permission):
|
||||
if x_api_key:
|
||||
api_key = await _validate_api_key(db, x_api_key)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, perm_strings)
|
||||
await authorize_api_key(db, api_key, perm_strings)
|
||||
return None # API key valid, allow access
|
||||
|
||||
# Check for Bearer token (could be JWT or API key)
|
||||
@@ -1581,7 +1717,7 @@ def require_permission_if_auth_enabled(*permissions: str | Permission):
|
||||
if token.startswith("bb_"):
|
||||
api_key = await _validate_api_key(db, token)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, perm_strings)
|
||||
await authorize_api_key(db, api_key, perm_strings)
|
||||
return None # API key valid, allow access
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
@@ -1674,7 +1810,7 @@ def require_any_permission_if_auth_enabled(*permissions: str | Permission):
|
||||
# GHSA-r2qv-8222-hqg3: previously returned None unconditionally,
|
||||
# letting any valid API key satisfy admin "any-of" route
|
||||
# dependencies. require_any → at-least-one must pass the scope check.
|
||||
_check_apikey_permissions(api_key, perm_strings, require_any=True)
|
||||
await authorize_api_key(db, api_key, perm_strings, require_any=True)
|
||||
return None
|
||||
|
||||
if credentials is not None:
|
||||
@@ -1682,7 +1818,7 @@ def require_any_permission_if_auth_enabled(*permissions: str | Permission):
|
||||
if token.startswith("bb_"):
|
||||
api_key = await _validate_api_key(db, token)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, perm_strings, require_any=True)
|
||||
await authorize_api_key(db, api_key, perm_strings, require_any=True)
|
||||
return None
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
@@ -1873,7 +2009,7 @@ def require_ownership_permission(
|
||||
if x_api_key:
|
||||
api_key = await _validate_api_key(db, x_api_key)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, [all_perm])
|
||||
await authorize_api_key(db, api_key, [all_perm])
|
||||
return None, True
|
||||
|
||||
# Check for Bearer token (could be JWT or API key)
|
||||
@@ -1883,7 +2019,7 @@ def require_ownership_permission(
|
||||
if token.startswith("bb_"):
|
||||
api_key = await _validate_api_key(db, token)
|
||||
if api_key:
|
||||
_check_apikey_permissions(api_key, [all_perm])
|
||||
await authorize_api_key(db, api_key, [all_perm])
|
||||
return None, True
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
|
||||
@@ -174,6 +174,11 @@ class Permission(StrEnum):
|
||||
|
||||
# Users (admin-level)
|
||||
USERS_READ = "users:read"
|
||||
# Narrow read: id + username only, no emails/roles/groups/permissions (#1894).
|
||||
# Exists so an id -> name mapping can be resolved without handing out the
|
||||
# full user objects. Pairs with STATS_FILTER_BY_USER, which is useless
|
||||
# without a way to discover the ids it filters on.
|
||||
USERS_READ_SLIM = "users:read_slim"
|
||||
USERS_CREATE = "users:create"
|
||||
USERS_UPDATE = "users:update"
|
||||
USERS_DELETE = "users:delete"
|
||||
@@ -346,6 +351,7 @@ PERMISSION_CATEGORIES = {
|
||||
],
|
||||
"User Management": [
|
||||
Permission.USERS_READ,
|
||||
Permission.USERS_READ_SLIM,
|
||||
Permission.USERS_CREATE,
|
||||
Permission.USERS_UPDATE,
|
||||
Permission.USERS_DELETE,
|
||||
|
||||
@@ -93,6 +93,22 @@ class UserResponse(BaseModel):
|
||||
from_attributes = True
|
||||
|
||||
|
||||
class UserSlim(BaseModel):
|
||||
"""Just enough to resolve a user id to a display name (#1894).
|
||||
|
||||
Deliberately narrower than ``UserResponse``: no email, role, auth source,
|
||||
group membership or permission set. Adding a field here widens what every
|
||||
``can_read_status`` API key can read about every account, so treat this
|
||||
shape as the contract rather than a starting point.
|
||||
"""
|
||||
|
||||
id: int
|
||||
username: str
|
||||
|
||||
class Config:
|
||||
from_attributes = True
|
||||
|
||||
|
||||
class LDAPSearchResultResponse(BaseModel):
|
||||
"""One match from GET /auth/ldap/search — surfaced in the admin UI."""
|
||||
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
"""An API key must not out-rank the user it belongs to (#1894 follow-on).
|
||||
|
||||
``_check_apikey_permissions`` gated purely on the scope flags stored on the key
|
||||
row and never looked at the owner. Scope flags are chosen at creation time by
|
||||
whoever holds ``api_keys:create`` -- admin-only in the default groups, but a
|
||||
custom group can grant it -- so a user with, say, queue permissions could mint
|
||||
themselves a key with ``can_control_printer`` and stop other people's prints
|
||||
through it. Deactivating that user did not help either: their keys kept working
|
||||
with full scope authority, because nothing re-checked the owner.
|
||||
|
||||
The gate now narrows the scope flags to what the owner may do. Two cases must
|
||||
NOT be conflated, and each has a test below:
|
||||
|
||||
- ``user_id IS NULL`` -- legacy key from before per-user ownership. No owner
|
||||
exists to narrow against, so the flags stand alone and the key keeps working.
|
||||
- ``user_id`` set but the row is gone or deactivated -- the key's authority came
|
||||
from a user who has none. Fails closed.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
from sqlalchemy import select
|
||||
|
||||
from backend.app.core.auth import generate_api_key, get_password_hash
|
||||
from backend.app.models.api_key import APIKey
|
||||
from backend.app.models.group import Group
|
||||
from backend.app.models.user import User
|
||||
|
||||
# A route gated on PRINTERS_READ (can_read_status) and one gated on
|
||||
# PRINTERS_CONTROL (can_control_printer). Both scope flags are set on every key
|
||||
# built below, so any denial comes from the owner check rather than the flags.
|
||||
READ_ROUTE = "/api/v1/printers/"
|
||||
CONTROL_ROUTE = "/api/v1/printers/1/print/stop"
|
||||
|
||||
|
||||
async def _setup(async_client: AsyncClient) -> None:
|
||||
await async_client.post(
|
||||
"/api/v1/auth/setup",
|
||||
json={"auth_enabled": True, "admin_username": "owneradmin", "admin_password": "OwnerPass1!"},
|
||||
)
|
||||
|
||||
|
||||
async def _key_for(db_session, owner: User | None, **scopes) -> str:
|
||||
defaults = {"can_read_status": True, "can_control_printer": True, "can_queue": True}
|
||||
defaults.update(scopes)
|
||||
full_key, key_hash, key_prefix = generate_api_key()
|
||||
db_session.add(
|
||||
APIKey(
|
||||
name=f"key-{owner.username if owner else 'legacy'}",
|
||||
key_hash=key_hash,
|
||||
key_prefix=key_prefix,
|
||||
enabled=True,
|
||||
user_id=owner.id if owner else None,
|
||||
**defaults,
|
||||
)
|
||||
)
|
||||
await db_session.commit()
|
||||
return full_key
|
||||
|
||||
|
||||
async def _user(db_session, username: str, permissions: list[str], *, is_active: bool = True) -> User:
|
||||
group = Group(name=f"grp-{username}", description="t", permissions=permissions, is_system=False)
|
||||
db_session.add(group)
|
||||
await db_session.flush()
|
||||
user = User(
|
||||
username=username,
|
||||
password_hash=get_password_hash("Whatever1!"), # noqa: S106
|
||||
role="user",
|
||||
is_active=is_active,
|
||||
groups=[group],
|
||||
)
|
||||
db_session.add(user)
|
||||
await db_session.commit()
|
||||
return user
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_admin_owned_key_keeps_full_scope_authority(async_client: AsyncClient, db_session):
|
||||
"""The common case must not regress -- almost every key is admin-owned."""
|
||||
await _setup(async_client)
|
||||
admin = (await db_session.execute(select(User).where(User.username == "owneradmin"))).scalar_one()
|
||||
key = await _key_for(db_session, admin)
|
||||
|
||||
response = await async_client.get(READ_ROUTE, headers={"X-API-Key": key})
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_legacy_ownerless_key_still_works(async_client: AsyncClient, db_session):
|
||||
"""No owner to narrow against is not the same as a failed owner lookup."""
|
||||
await _setup(async_client)
|
||||
key = await _key_for(db_session, None)
|
||||
|
||||
response = await async_client.get(READ_ROUTE, headers={"X-API-Key": key})
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_key_cannot_exceed_its_owners_permissions(async_client: AsyncClient, db_session):
|
||||
"""The escalation: control flags ticked, owner who may not control."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "readonly", ["printers:read"])
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
allowed = await async_client.get(READ_ROUTE, headers={"X-API-Key": key})
|
||||
denied = await async_client.post(CONTROL_ROUTE, headers={"X-API-Key": key})
|
||||
|
||||
assert allowed.status_code == 200
|
||||
assert denied.status_code == 403
|
||||
assert "owner does not have" in denied.json()["detail"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_deactivating_the_owner_disables_the_key(async_client: AsyncClient, db_session):
|
||||
"""Previously the key kept working -- nothing re-checked the owner."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "gone", ["printers:read"], is_active=False)
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
response = await async_client.get(READ_ROUTE, headers={"X-API-Key": key})
|
||||
|
||||
assert response.status_code == 403
|
||||
assert "deactivated" in response.json()["detail"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_deleted_owner_does_not_fall_back_to_anonymous(async_client: AsyncClient, db_session):
|
||||
"""The dangling-row case fails closed rather than reverting to flags-only.
|
||||
|
||||
CASCADE should prevent this, but "should" is not a gate -- if the row is
|
||||
ever orphaned the key must not silently regain full scope authority.
|
||||
"""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "doomed", ["printers:read"])
|
||||
key = await _key_for(db_session, owner)
|
||||
api_key = (await db_session.execute(select(APIKey).where(APIKey.user_id == owner.id))).scalar_one()
|
||||
api_key.user_id = 999999 # owner row that does not exist
|
||||
await db_session.commit()
|
||||
|
||||
response = await async_client.get(READ_ROUTE, headers={"X-API-Key": key})
|
||||
|
||||
assert response.status_code == 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_bearer_path_is_gated_the_same_as_the_header(async_client: AsyncClient, db_session):
|
||||
"""Both credential paths run the same gate; only one was ever tested."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "bearer-readonly", ["printers:read"])
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
denied = await async_client.post(CONTROL_ROUTE, headers={"Authorization": f"Bearer {key}"})
|
||||
|
||||
assert denied.status_code == 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_webhook_routes_are_not_a_way_around_the_owner_check(async_client: AsyncClient, db_session):
|
||||
"""/webhook/* reaches its scope flags by a different route than the rest.
|
||||
|
||||
It gates on ``check_permission``, not ``_check_apikey_permissions``, so it
|
||||
does not inherit the owner narrowing for free. If it is missed, the same key
|
||||
that is refused on /printers/{id}/print/stop simply stops the print here
|
||||
instead, and the whole gate is decorative.
|
||||
"""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "webhook-readonly", ["printers:read"])
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
denied = await async_client.post("/api/v1/webhook/printer/1/stop", headers={"X-API-Key": key})
|
||||
|
||||
assert denied.status_code == 403
|
||||
assert "owner does not have" in denied.json()["detail"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_webhook_still_works_for_a_permitted_owner(async_client: AsyncClient, db_session):
|
||||
"""The narrowing must not simply break every webhook caller."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "webhook-operator", ["printers:read", "printers:control"])
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
response = await async_client.post("/api/v1/webhook/printer/1/stop", headers={"X-API-Key": key})
|
||||
|
||||
# There is no connected printer 1, so the handler itself fails. What
|
||||
# matters is that the request got that far: neither gate rejected it.
|
||||
assert response.status_code != 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_webhook_rejects_a_deactivated_owner(async_client: AsyncClient, db_session):
|
||||
"""Fail-closed applies on this path too."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "webhook-gone", ["printers:read", "printers:control"], is_active=False)
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
response = await async_client.post("/api/v1/webhook/printer/1/stop", headers={"X-API-Key": key})
|
||||
|
||||
assert response.status_code == 403
|
||||
assert "deactivated" in response.json()["detail"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_reports_the_narrowed_set(async_client: AsyncClient, db_session):
|
||||
"""/auth/me and the gate must agree, including about the owner."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "narrow", ["printers:read"])
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
result = (await async_client.get("/api/v1/auth/me", headers={"X-API-Key": key})).json()
|
||||
|
||||
assert result["id"] == owner.id
|
||||
assert "printers:read" in result["permissions"]
|
||||
# can_control_printer is ticked on the key, but the owner cannot control.
|
||||
assert "printers:control" not in result["permissions"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_is_rejected_once_the_owner_is_deactivated(async_client: AsyncClient, db_session):
|
||||
"""A dead key identifies as nothing, rather than as an anonymous key."""
|
||||
await _setup(async_client)
|
||||
owner = await _user(db_session, "me-gone", ["printers:read"], is_active=False)
|
||||
key = await _key_for(db_session, owner)
|
||||
|
||||
response = await async_client.get("/api/v1/auth/me", headers={"X-API-Key": key})
|
||||
|
||||
assert response.status_code == 403
|
||||
@@ -232,8 +232,8 @@ class TestAuthMeAPI:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_with_api_key_bearer(self, async_client: AsyncClient, db_session):
|
||||
"""Verify /me returns synthetic admin user when using API key via Bearer token."""
|
||||
async def test_me_with_ownerless_api_key_bearer(self, async_client: AsyncClient, db_session):
|
||||
"""A legacy key has no identity to report, but no longer claims admin (#1894)."""
|
||||
from backend.app.core.auth import generate_api_key
|
||||
from backend.app.models.api_key import APIKey
|
||||
|
||||
@@ -253,15 +253,18 @@ class TestAuthMeAPI:
|
||||
result = response.json()
|
||||
assert result["id"] == 0
|
||||
assert result["username"].startswith("api-key:")
|
||||
assert result["role"] == "admin"
|
||||
assert result["is_admin"] is True
|
||||
assert result["role"] != "admin"
|
||||
assert result["is_admin"] is False
|
||||
assert result["is_active"] is True
|
||||
# can_read_status defaults True, so the scope-derived set is non-empty
|
||||
# -- but it is a set, not "every permission there is".
|
||||
assert len(result["permissions"]) > 0
|
||||
assert "users:create" not in result["permissions"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_with_api_key_header(self, async_client: AsyncClient, db_session):
|
||||
"""Verify /me returns synthetic admin user when using X-API-Key header."""
|
||||
async def test_me_with_ownerless_api_key_header(self, async_client: AsyncClient, db_session):
|
||||
"""Same as above via the X-API-Key header rather than Bearer."""
|
||||
from backend.app.core.auth import generate_api_key
|
||||
from backend.app.models.api_key import APIKey
|
||||
|
||||
@@ -279,7 +282,7 @@ class TestAuthMeAPI:
|
||||
result = response.json()
|
||||
assert result["id"] == 0
|
||||
assert result["username"].startswith("api-key:")
|
||||
assert result["is_admin"] is True
|
||||
assert result["is_admin"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
@@ -292,6 +295,127 @@ class TestAuthMeAPI:
|
||||
|
||||
assert response.status_code == 401
|
||||
|
||||
async def _owned_key(self, async_client: AsyncClient, db_session, **scopes):
|
||||
"""Set up auth and return (owner, full_key) for a key with ``scopes``.
|
||||
|
||||
The owner is given an email and a group explicitly rather than relying
|
||||
on what /auth/setup happens to seed, so the assertions about what /me
|
||||
withholds cannot pass vacuously.
|
||||
"""
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.orm import selectinload
|
||||
|
||||
from backend.app.core.auth import generate_api_key
|
||||
from backend.app.models.api_key import APIKey
|
||||
from backend.app.models.group import Group
|
||||
from backend.app.models.user import User
|
||||
|
||||
await async_client.post(
|
||||
"/api/v1/auth/setup",
|
||||
json={
|
||||
"auth_enabled": True,
|
||||
"admin_username": "keyowner",
|
||||
"admin_password": "KeyPass1!",
|
||||
},
|
||||
)
|
||||
owner = (
|
||||
await db_session.execute(select(User).where(User.username == "keyowner").options(selectinload(User.groups)))
|
||||
).scalar_one()
|
||||
owner.email = "keyowner@example.invalid"
|
||||
group = Group(name="key-owner-group", description="t", permissions=["printers:read"], is_system=False)
|
||||
db_session.add(group)
|
||||
await db_session.flush()
|
||||
owner.groups.append(group)
|
||||
|
||||
full_key, key_hash, key_prefix = generate_api_key()
|
||||
db_session.add(
|
||||
APIKey(name="owned", key_hash=key_hash, key_prefix=key_prefix, enabled=True, user_id=owner.id, **scopes)
|
||||
)
|
||||
await db_session.commit()
|
||||
return owner, full_key
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_reports_the_key_owner_not_a_synthetic_admin(self, async_client: AsyncClient, db_session):
|
||||
"""The id is the point of #1894 -- it is what created_by_id filters on."""
|
||||
owner, full_key = await self._owned_key(async_client, db_session)
|
||||
|
||||
response = await async_client.get("/api/v1/auth/me", headers={"X-API-Key": full_key})
|
||||
|
||||
assert response.status_code == 200
|
||||
result = response.json()
|
||||
assert result["id"] == owner.id
|
||||
assert result["username"] == "keyowner"
|
||||
# The owner is an admin; the key still is not, because no key reaches
|
||||
# an administrative route regardless of who owns it.
|
||||
assert result["is_admin"] is False
|
||||
assert result["role"] != "admin"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_withholds_owner_email_and_groups(self, async_client: AsyncClient, db_session):
|
||||
"""Identity, not the owner's profile -- anyone holding the key sees this."""
|
||||
owner, full_key = await self._owned_key(async_client, db_session)
|
||||
assert owner.email is not None and owner.groups # the helper made both non-empty
|
||||
|
||||
result = (await async_client.get("/api/v1/auth/me", headers={"X-API-Key": full_key})).json()
|
||||
|
||||
assert result["email"] is None
|
||||
assert result["groups"] == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_permissions_track_the_key_scopes_not_the_owner(self, async_client: AsyncClient, db_session):
|
||||
"""A key owned by an admin still reports only what its flags allow."""
|
||||
_, full_key = await self._owned_key(
|
||||
async_client,
|
||||
db_session,
|
||||
can_read_status=True,
|
||||
can_control_printer=False,
|
||||
can_queue=False,
|
||||
)
|
||||
|
||||
perms = (await async_client.get("/api/v1/auth/me", headers={"X-API-Key": full_key})).json()["permissions"]
|
||||
|
||||
assert "printers:read" in perms # can_read_status
|
||||
assert "printers:control" not in perms # can_control_printer is off
|
||||
assert "queue:create" not in perms # can_queue is off
|
||||
assert "users:create" not in perms # administrative: unmapped for keys
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_me_permissions_are_exactly_what_the_gate_admits(self, async_client: AsyncClient, db_session):
|
||||
"""/me must not drift from _check_apikey_permissions.
|
||||
|
||||
The whole defect in #1894 was a /me response that described a different
|
||||
credential than the one the gate enforces, so pin them to each other
|
||||
rather than to a hand-written list that can rot. The owner is threaded
|
||||
through both sides for the same reason -- the gate narrows to the
|
||||
owner's permissions, so a check that skipped the owner would stop
|
||||
catching drift the moment the owner is not an administrator.
|
||||
"""
|
||||
from fastapi import HTTPException
|
||||
from sqlalchemy import select
|
||||
|
||||
from backend.app.core.auth import _check_apikey_permissions, resolve_apikey_owner
|
||||
from backend.app.core.permissions import ALL_PERMISSIONS
|
||||
from backend.app.models.api_key import APIKey
|
||||
|
||||
_, full_key = await self._owned_key(async_client, db_session, can_read_status=True, can_control_printer=False)
|
||||
|
||||
response = await async_client.get("/api/v1/auth/me", headers={"X-API-Key": full_key})
|
||||
reported = set(response.json()["permissions"])
|
||||
|
||||
key = (await db_session.execute(select(APIKey).where(APIKey.name == "owned"))).scalar_one()
|
||||
owner = await resolve_apikey_owner(db_session, key)
|
||||
for perm in ALL_PERMISSIONS:
|
||||
try:
|
||||
_check_apikey_permissions(key, [perm], owner=owner)
|
||||
except HTTPException:
|
||||
assert perm not in reported, f"/me reports '{perm}' but the gate denies it"
|
||||
else:
|
||||
assert perm in reported, f"the gate admits '{perm}' but /me omits it"
|
||||
|
||||
|
||||
class TestUsersAPI:
|
||||
"""Integration tests for /api/v1/users/ endpoints."""
|
||||
|
||||
@@ -30,6 +30,7 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
from backend.app.core.auth import generate_api_key
|
||||
from backend.app.core.config import settings as app_settings
|
||||
from backend.app.models.api_key import APIKey
|
||||
from backend.app.models.group import Group
|
||||
from backend.app.models.print_queue import PrintQueueItem
|
||||
from backend.app.models.user import User
|
||||
|
||||
@@ -173,7 +174,14 @@ class TestWebhookQueueAddAttribution:
|
||||
async def test_credits_the_key_owner(self, async_client: AsyncClient, db_session, test_engine, printer_and_archive):
|
||||
printer, archive = printer_and_archive
|
||||
|
||||
owner = User(username="keyowner", password_hash="x", is_active=True)
|
||||
# The owner needs queue:create in their own right: a key is capped by
|
||||
# its owner's permissions (#1894), so a bare account with no groups
|
||||
# cannot queue through a key however its scope flags are set. This test
|
||||
# is about who the row is credited to, not about the gate.
|
||||
group = Group(name="queue-writers", description="t", permissions=["queue:create"], is_system=False)
|
||||
db_session.add(group)
|
||||
await db_session.flush()
|
||||
owner = User(username="keyowner", password_hash="x", is_active=True, groups=[group])
|
||||
db_session.add(owner)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(owner)
|
||||
|
||||
@@ -0,0 +1,180 @@
|
||||
"""GET /api/v1/users/slim -- the id -> username mapping for API clients (#1894).
|
||||
|
||||
An API key could already read global archive stats and filter them by
|
||||
``created_by_id`` (for API-keyed requests the permission deps return None as
|
||||
``current_user``, so the ``stats:filter_by_user`` guard short-circuits), but
|
||||
had no way to discover which id belonged to whom: the full listing is gated on
|
||||
``users:read``, which is unmapped in the API-key scope allowlist and therefore
|
||||
administrative.
|
||||
|
||||
The slim listing closes that gap without handing keys the full user objects.
|
||||
These tests pin both halves: that it answers for a key, and that it stays
|
||||
narrow while the full listing stays admin-only.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
from sqlalchemy import select
|
||||
|
||||
from backend.app.core.auth import generate_api_key
|
||||
from backend.app.models.api_key import APIKey
|
||||
from backend.app.models.group import Group
|
||||
from backend.app.models.user import User
|
||||
|
||||
|
||||
async def _setup_and_login(async_client: AsyncClient) -> str:
|
||||
await async_client.post(
|
||||
"/api/v1/auth/setup",
|
||||
json={"auth_enabled": True, "admin_username": "slimadmin", "admin_password": "SlimPass1!"},
|
||||
)
|
||||
login = await async_client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"username": "slimadmin", "password": "SlimPass1!"},
|
||||
)
|
||||
return login.json()["access_token"]
|
||||
|
||||
|
||||
async def _add_key(db_session, *, user_id: int | None = None, **scopes) -> str:
|
||||
full_key, key_hash, key_prefix = generate_api_key()
|
||||
db_session.add(
|
||||
APIKey(name="slim-test", key_hash=key_hash, key_prefix=key_prefix, enabled=True, user_id=user_id, **scopes)
|
||||
)
|
||||
await db_session.commit()
|
||||
return full_key
|
||||
|
||||
|
||||
async def _add_user(db_session, username: str, **kwargs) -> User:
|
||||
from backend.app.core.auth import get_password_hash
|
||||
|
||||
user = User(
|
||||
username=username,
|
||||
password_hash=get_password_hash("Whatever1!"),
|
||||
email=f"{username}@example.invalid",
|
||||
role="user",
|
||||
is_active=True,
|
||||
**kwargs,
|
||||
)
|
||||
db_session.add(user)
|
||||
await db_session.commit()
|
||||
return user
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_slim_returns_only_id_and_username(async_client: AsyncClient, db_session):
|
||||
"""The response shape is the contract -- no emails, roles, or permissions."""
|
||||
token = await _setup_and_login(async_client)
|
||||
await _add_user(db_session, "bob")
|
||||
|
||||
response = await async_client.get("/api/v1/users/slim", headers={"Authorization": f"Bearer {token}"})
|
||||
|
||||
assert response.status_code == 200
|
||||
rows = response.json()
|
||||
assert rows, "expected at least the admin created by setup"
|
||||
for row in rows:
|
||||
assert set(row) == {"id", "username"}
|
||||
assert "bob" in [row["username"] for row in rows]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_slim_is_reachable_with_an_api_key(async_client: AsyncClient, db_session):
|
||||
"""The point of the issue: a key can resolve the ids it already filters on."""
|
||||
await _setup_and_login(async_client)
|
||||
owner = (await db_session.execute(select(User).where(User.username == "slimadmin"))).scalar_one()
|
||||
full_key = await _add_key(db_session, user_id=owner.id, can_read_status=True)
|
||||
|
||||
header = await async_client.get("/api/v1/users/slim", headers={"X-API-Key": full_key})
|
||||
bearer = await async_client.get("/api/v1/users/slim", headers={"Authorization": f"Bearer {full_key}"})
|
||||
|
||||
assert header.status_code == 200
|
||||
assert bearer.status_code == 200
|
||||
assert {row["username"] for row in header.json()} == {"slimadmin"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_slim_needs_can_read_status(async_client: AsyncClient, db_session):
|
||||
"""A key without the read scope gets nothing, same as any other read route."""
|
||||
await _setup_and_login(async_client)
|
||||
owner = (await db_session.execute(select(User).where(User.username == "slimadmin"))).scalar_one()
|
||||
full_key = await _add_key(db_session, user_id=owner.id, can_read_status=False)
|
||||
|
||||
response = await async_client.get("/api/v1/users/slim", headers={"X-API-Key": full_key})
|
||||
|
||||
assert response.status_code == 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_full_listing_stays_admin_only_for_api_keys(async_client: AsyncClient, db_session):
|
||||
"""Regression guard: widening the slim route must not widen the full one.
|
||||
|
||||
``users:read`` returns emails, group membership and the complete permission
|
||||
set for every account. It has to stay unmapped in the scope allowlist.
|
||||
"""
|
||||
await _setup_and_login(async_client)
|
||||
owner = (await db_session.execute(select(User).where(User.username == "slimadmin"))).scalar_one()
|
||||
full_key = await _add_key(db_session, user_id=owner.id, can_read_status=True)
|
||||
|
||||
response = await async_client.get("/api/v1/users", headers={"X-API-Key": full_key})
|
||||
|
||||
assert response.status_code == 403
|
||||
assert "administrative" in response.json()["detail"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_slim_is_not_parsed_as_a_user_id(async_client: AsyncClient, db_session):
|
||||
"""Route ordering. Declared after /{user_id}, "slim" would 422 as an int."""
|
||||
token = await _setup_and_login(async_client)
|
||||
|
||||
response = await async_client.get("/api/v1/users/slim", headers={"Authorization": f"Bearer {token}"})
|
||||
|
||||
assert response.status_code != 422
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_group_with_only_users_read_still_reaches_slim(async_client: AsyncClient, db_session):
|
||||
"""``users:read`` is strictly broader, so it must pass the any-of gate.
|
||||
|
||||
Without this, every existing custom group holding ``users:read`` would need
|
||||
a permission backfill before the frontend could ever move to this route.
|
||||
"""
|
||||
await _setup_and_login(async_client)
|
||||
group = Group(name="readers", description="t", permissions=["users:read"], is_system=False)
|
||||
db_session.add(group)
|
||||
await db_session.flush()
|
||||
await _add_user(db_session, "reader", groups=[group])
|
||||
|
||||
login = await async_client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"username": "reader", "password": "Whatever1!"},
|
||||
)
|
||||
token = login.json()["access_token"]
|
||||
|
||||
response = await async_client.get("/api/v1/users/slim", headers={"Authorization": f"Bearer {token}"})
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_group_with_only_slim_cannot_read_the_full_listing(async_client: AsyncClient, db_session):
|
||||
"""The narrow grant has to actually be narrower for JWT users too."""
|
||||
await _setup_and_login(async_client)
|
||||
group = Group(name="slim-only", description="t", permissions=["users:read_slim"], is_system=False)
|
||||
db_session.add(group)
|
||||
await db_session.flush()
|
||||
await _add_user(db_session, "slimonly", groups=[group])
|
||||
|
||||
login = await async_client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"username": "slimonly", "password": "Whatever1!"},
|
||||
)
|
||||
token = login.json()["access_token"]
|
||||
headers = {"Authorization": f"Bearer {token}"}
|
||||
|
||||
assert (await async_client.get("/api/v1/users/slim", headers=headers)).status_code == 200
|
||||
assert (await async_client.get("/api/v1/users", headers=headers)).status_code == 403
|
||||
Reference in New Issue
Block a user