fix(permissions): grant create:user_api_keys to the Basic group and let admins toggle it (#15810)

This commit is contained in:
Nikolas Garza authored and GitHub committed 2026-10-11 17:31:36 +00:00
1 parent 9091cc227d
commit d6f2ee08f9
34 files changed
+593 -133

No files matched your search

@@ -155,7 +155,10 @@ OpenSearch chunk document
- `UserGroup` (`onyx/db/models.py`, EE-managed via `ee/onyx/db/user_group.py`):
a named group. Two are seeded and protected as "default": Admin and Basic
(`onyx/db/user_group.py:assert_group_config_is_editable`,
`assert_not_shared_with_default_group`). Sharing a resource with the Basic
`assert_not_shared_with_default_group`). The one write a default group takes
besides membership is the permission toggle on Basic, limited to
`auth/permissions.py:DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS`
(`onyx/db/user_group.py:assert_group_permissions_editable`). Sharing a resource with the Basic
default group is refused outright: use the resource's own `is_public` flag
instead, since Basic holds every user. The Community downgrade
(`ee/onyx/db/community_downgrade.py:remove_custom_user_groups__no_commit`,
@@ -110,7 +110,7 @@ the coding-agent sandbox).
|---|---|---|---|
| CRUD | `/admin/api-key*` | `server/api_key/api.py` | `Permission.MANAGE_SERVICE_ACCOUNT_API_KEYS`. Creating or updating a key is **admin-equivalent by design**: `group_ids` is uncapped, so a holder can add a key to the Admin group (`api.py:create_api_key` comment). |
| GET | `/user/pats/scopes` | `server/pat/api.py:list_selectable_scopes` | `Permission.BASIC_ACCESS`. Drops `use:llm_gateway` from the list while the workspace `llm_gateway_enabled` setting is off. |
| GET/POST/DELETE | `/user/pats` | `server/pat/api.py` | List/create needs `CREATE_USER_API_KEYS` (create) or `BASIC_ACCESS` (list/delete); a user only ever sees/revokes their own. Minting a PAT with `use:llm_gateway` while the setting is off is rejected with `INVALID_INPUT` (`server/pat/api.py:_validate_assignable_scopes`). |
| GET/POST/DELETE | `/user/pats` | `server/pat/api.py` | Create needs `CREATE_USER_API_KEYS`. The seeded Basic group grants it by default, and a full admin can switch it off or on from the Basic group's page (`auth/permissions.py:DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS`). The same permission gates MCP OAuth consent (`server/oauth_provider/api.py`), so the toggle covers both. List and delete need `BASIC_ACCESS`, and a user only ever sees or revokes their own. Minting a PAT with `use:llm_gateway` while the setting is off is rejected with `INVALID_INPUT` (`server/pat/api.py:_validate_assignable_scopes`). |
| GET/PUT | `/admin/security`, GET `/admin/security/pinned-fields` | `server/security/api.py` | `FULL_ADMIN_PANEL_ACCESS`. Workspace security overrides: password policy, `password_auth_enabled`, `valid_email_domains`, JWT validation fields, SSRF level, incognito availability, and `allow_connector_group_restrictions`. Env-pinned fields win over stored values. Turning on `allow_connector_group_restrictions` needs the Business tier. |
| CRUD | `/admin/user-group*` | `ee/onyx/server/user_group/api.py` | See [[access-control]] §2; this is where group membership, and thus `PermissionGrant`-derived capability, is actually assigned. |
| GET/POST/PATCH | `/admin/sso-providers*`, `/admin/sso-providers/{id}/domains` | `server/manage/sso/api.py` | `FULL_ADMIN_PANEL_ACCESS` (exact gating per route). Domain-routing SSO providers beyond the first require business tier (`_require_business_tier_for_additional_enabled_provider`). |
@@ -0,0 +1,146 @@
"""grant create:user_api_keys to the Basic group
Every member of the seeded Basic group may mint personal access tokens by default. The
grant is written as a SYSTEM row on the group and appended to its members'
stored permissions, since those are only recomputed on membership and grant
edits.
Revision ID: b6d920407aa6
Revises: c4d8e2f1a9b3
"""
from uuid import UUID
import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql
from sqlalchemy.dialects.postgresql import insert as pg_insert
# revision identifiers, used by Alembic.
revision = "b6d920407aa6"
down_revision = "c4d8e2f1a9b3"
branch_labels = None
depends_on = None
BASIC_GROUP_NAME = "Basic"
PERMISSION = "create:user_api_keys"
GRANT_SOURCE = "SYSTEM"
user_group_table: sa.TableClause = sa.table(
"user_group",
sa.column("id", sa.Integer),
sa.column("name", sa.String),
sa.column("is_default", sa.Boolean),
)
permission_grant_table: sa.TableClause = sa.table(
"permission_grant",
sa.column("group_id", sa.Integer),
sa.column("permission", sa.String),
sa.column("grant_source", sa.String),
sa.column("is_deleted", sa.Boolean),
)
user__user_group_table: sa.TableClause = sa.table(
"user__user_group",
sa.column("user_group_id", sa.Integer),
sa.column("user_id", sa.Uuid),
)
user_table: sa.TableClause = sa.table(
"user",
sa.column("id", sa.Uuid),
sa.column("effective_permissions", postgresql.JSONB),
)
permission_json: sa.BindParameter[list[str]] = sa.literal(
[PERMISSION], type_=postgresql.JSONB
)
basic_group_id: sa.ScalarSelect[int] = (
sa.select(user_group_table.c.id)
.where(
user_group_table.c.name == BASIC_GROUP_NAME,
user_group_table.c.is_default.is_(True),
)
.scalar_subquery()
)
basic_member_ids: sa.Select[tuple[UUID]] = sa.select(
user__user_group_table.c.user_id
).where(user__user_group_table.c.user_group_id == basic_group_id)
def upgrade() -> None:
conn: sa.engine.Connection = op.get_bind()
# A soft-deleted row would block the insert on the unique key, so revive it.
conn.execute(
pg_insert(permission_grant_table)
.values(
group_id=basic_group_id,
permission=PERMISSION,
grant_source=GRANT_SOURCE,
is_deleted=False,
)
.on_conflict_do_update(
index_elements=["group_id", "permission"],
set_={"is_deleted": False},
)
)
conn.execute(
user_table.update()
.where(
user_table.c.id.in_(basic_member_ids),
sa.not_(user_table.c.effective_permissions.contains(permission_json)),
)
.values(
effective_permissions=user_table.c.effective_permissions.op("||")(
permission_json
)
)
)
def downgrade() -> None:
conn: sa.engine.Connection = op.get_bind()
conn.execute(
sa.delete(permission_grant_table).where(
permission_grant_table.c.group_id == basic_group_id,
permission_grant_table.c.permission == PERMISSION,
)
)
# Members who still hold the permission through another group keep it.
other_grant_holders = (
sa.select(user__user_group_table.c.user_id)
.select_from(
user__user_group_table.join(
permission_grant_table,
permission_grant_table.c.group_id
== user__user_group_table.c.user_group_id,
)
)
.where(
user__user_group_table.c.user_id.is_not(None),
permission_grant_table.c.permission == PERMISSION,
permission_grant_table.c.is_deleted.is_(False),
)
)
conn.execute(
user_table.update()
.where(
user_table.c.id.in_(basic_member_ids),
user_table.c.id.not_in(other_grant_holders),
)
.values(
# jsonb - text drops the element. A JSONB bind would compile to the
# undefined jsonb - jsonb.
effective_permissions=user_table.c.effective_permissions.op("-")(
sa.literal(PERMISSION, type_=sa.Text)
)
)
)
+8 -4
View File
@@ -75,7 +75,11 @@ from onyx.db.engine.sql_engine import get_session
from onyx.db.enums import ConnectorManageRole, Permission, PermissionAuthority
from onyx.db.models import User
from onyx.db.persona import fetch_persona_by_id_for_user, get_personas_by_ids
from onyx.db.user_group import assert_group_config_is_editable
from onyx.db.user_group import (
assert_group_config_is_editable,
assert_group_permissions_editable,
group_permission_toggles_allowed,
)
from onyx.error_handling.error_codes import OnyxErrorCode
from onyx.error_handling.exceptions import OnyxError
from onyx.server.security.store import get_security_settings
@@ -141,6 +145,7 @@ def list_user_groups(
is_user_groups_admin=is_user_groups_admin,
is_full_admin=is_full_admin,
is_default=user_group.is_default,
permission_toggles_allowed=group_permission_toggles_allowed(user_group),
),
)
for user_group in user_groups
@@ -178,6 +183,7 @@ def get_user_group(
user, Permission.FULL_ADMIN_PANEL_ACCESS
),
is_default=user_group.is_default,
permission_toggles_allowed=group_permission_toggles_allowed(user_group),
),
)
@@ -242,9 +248,7 @@ def set_user_group_permissions(
group = fetch_user_group(db_session, user_group_id)
if group is None:
raise OnyxError(OnyxErrorCode.NOT_FOUND, "User group not found")
assert_group_config_is_editable(
db_session, user_group_id, "change the permissions of"
)
assert_group_permissions_editable(group, request.permissions)
non_toggleable = [p for p in request.permissions if p in NON_TOGGLEABLE_PERMISSIONS]
if non_toggleable:
+7 -5
View File
@@ -209,23 +209,25 @@ def user_group_permissions(
is_user_groups_admin: bool,
is_full_admin: bool,
is_default: bool,
permission_toggles_allowed: bool,
) -> dict[str, bool]:
"""User group affordance map. ``manage`` (rename, assign agents/document sets, set
manager) and ``edit_token_limits`` are the per-group ``manages_group`` decision — group
in the manager's managed set, or global MANAGE_USER_GROUPS. A scoped manager gets full
token-limit CRUD for groups they manage: every token route (read/create/update/delete)
now admits scope. ``delete`` needs global MANAGE_USER_GROUPS (its route has no
``allow_scope``). ``edit_permissions`` is FULL_ADMIN (the permission-toggle route).
``allow_scope``). ``edit_permissions`` is FULL_ADMIN plus ``permission_toggles_allowed``
(the permission-toggle route).
A seeded default group (Admin/Basic) has members and nothing else, so it keeps only
``manage_members``, and only for a full admin — mirroring the routes, where every other
write raises CONFLICT."""
A seeded default group (Admin/Basic) holds members and nothing else, so a full admin
gets ``manage_members`` and, on Basic only, ``edit_permissions`` for its allowlisted
toggles. Every other write on a default group raises CONFLICT at the route."""
is_editable = can_manage and not is_default
result: UserGroupPermissions = {
"manage": is_editable,
"manage_members": is_full_admin if is_default else can_manage,
"delete": is_user_groups_admin and not is_default,
"edit_permissions": is_full_admin and not is_default,
"edit_permissions": is_full_admin and permission_toggles_allowed,
"edit_token_limits": is_editable,
}
return cast(dict[str, bool], result)
+13 -1
View File
@@ -10,7 +10,7 @@ from collections.abc import Callable, Coroutine
from typing import Any
from fastapi import Depends, Request
from pydantic import BaseModel, field_validator
from pydantic import BaseModel, computed_field, field_validator
from onyx.db.enums import AccountType, Permission, PermissionAuthority
from onyx.db.models import User
@@ -98,6 +98,12 @@ NON_TOGGLEABLE_PERMISSIONS: frozenset[Permission] = frozenset(
| Permission.IMPLIED
)
# The only grants an admin may switch on or off for the seeded Basic group, which
# holds the whole org. Everything else on a default group stays fixed by migrations.
DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS: frozenset[Permission] = frozenset(
{Permission.CREATE_USER_API_KEYS}
)
# Permissions auto-granted to all users in Community Edition.
# In CE there is no group-permission UI, so these capabilities must be
# available without explicit grants. In EE they are controlled normally
@@ -138,6 +144,12 @@ class PermissionRegistryEntry(BaseModel):
permissions: list[Permission]
group: int
@computed_field
@property
def default_group_toggleable(self) -> bool:
"""Whether the row is offered on the Basic group's page."""
return all(p in DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS for p in self.permissions)
@field_validator("permissions")
@classmethod
def must_be_toggleable(cls, v: list[Permission]) -> list[Permission]:
+34 -2
View File
@@ -1,7 +1,9 @@
"""Invariants for the seeded default groups (Admin/Basic).
A default group holds members and nothing else: no connectors, document sets, agents,
skills, providers or limits, and its name and permission grant are fixed at seed time.
skills, providers or limits, and its name is fixed. Its permission grants come from
migrations, except the few an admin may switch on the Basic group
(``DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS``).
Lives in Community Edition even though group management is Enterprise: the groups are
seeded by a core migration and the resource-sharing paths that must refuse them are core
@@ -13,15 +15,23 @@ from collections.abc import Collection
from sqlalchemy import select
from sqlalchemy.orm import Session
from onyx.auth.permissions import DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS
from onyx.db.enums import Permission
from onyx.db.models import UserGroup
from onyx.db.users import DEFAULT_BASIC_GROUP_NAME
from onyx.error_handling.error_codes import OnyxErrorCode
from onyx.error_handling.exceptions import OnyxError
def group_permission_toggles_allowed(user_group: UserGroup) -> bool:
"""Custom groups take any toggleable grant, Basic its allowlist, Admin nothing."""
return not user_group.is_default or user_group.name == DEFAULT_BASIC_GROUP_NAME
def assert_group_config_is_editable(
db_session: Session, user_group_id: int, action: str
) -> None:
"""Membership is the only thing a default group has, so every other write is refused.
"""A default group holds only members, so the config writes this guards are refused.
``action`` completes "Cannot {action} a default system group.". A missing group falls
through to the caller's own NOT_FOUND."""
assert_groups_config_are_editable(db_session, [user_group_id], action)
@@ -70,3 +80,25 @@ def assert_not_shared_with_default_group(
+ ", ".join(sorted(default_names))
+ ". Make the resource public instead.",
)
def assert_group_permissions_editable(
user_group: UserGroup, permissions: Collection[Permission]
) -> None:
"""A custom group takes any toggleable grant. Basic takes only the allowlisted
ones, since it holds the whole org, and Admin takes none."""
if not user_group.is_default:
return
if not group_permission_toggles_allowed(user_group):
raise OnyxError(
OnyxErrorCode.CONFLICT,
"Cannot change the permissions of a default system group.",
)
refused: list[str] = sorted(
p.value for p in permissions if p not in DEFAULT_GROUP_TOGGLEABLE_PERMISSIONS
)
if refused:
raise OnyxError(
OnyxErrorCode.CONFLICT,
f"Permissions {refused} cannot be toggled on the Basic group.",
)
@@ -1270,6 +1270,7 @@ def test_user_group_projection_matches_gates(db_session: Session) -> None:
actor, Permission.FULL_ADMIN_PANEL_ACCESS
),
is_default=False,
permission_toggles_allowed=True,
)
assert tags["manage"] == manage_enforced, actor.email
assert tags["manage_members"] == manage_enforced, actor.email
@@ -1295,6 +1296,7 @@ def test_user_group_projection_matches_gates(db_session: Session) -> None:
in_scope, Permission.FULL_ADMIN_PANEL_ACCESS
),
is_default=False,
permission_toggles_allowed=True,
) == {
"manage": True,
"manage_members": True,
@@ -1315,6 +1317,7 @@ def test_user_group_projection_matches_gates(db_session: Session) -> None:
groups_admin, Permission.FULL_ADMIN_PANEL_ACCESS
),
is_default=False,
permission_toggles_allowed=True,
) == {
"manage": True,
"manage_members": True,
@@ -1346,6 +1349,7 @@ def test_default_user_group_projection_is_membership_only(db_session: Session) -
actor, Permission.FULL_ADMIN_PANEL_ACCESS
),
is_default=True,
permission_toggles_allowed=False,
)
assert _tags(admin) == {
@@ -1364,6 +1368,22 @@ def test_default_user_group_projection_is_membership_only(db_session: Session) -
"edit_token_limits": False,
}
# Basic is the one default group that offers permission toggles, full admin only.
assert user_group_permissions(
can_manage=False,
is_user_groups_admin=True,
is_full_admin=True,
is_default=True,
permission_toggles_allowed=True,
)["edit_permissions"]
assert not user_group_permissions(
can_manage=False,
is_user_groups_admin=True,
is_full_admin=False,
is_default=True,
permission_toggles_allowed=True,
)["edit_permissions"]
def test_user_group_key_coverage() -> None:
assert set(
@@ -1372,6 +1392,7 @@ def test_user_group_key_coverage() -> None:
is_user_groups_admin=True,
is_full_admin=True,
is_default=False,
permission_toggles_allowed=True,
)
) == set(USER_GROUP_ACTIONS)
@@ -0,0 +1,138 @@
"""Migration b6d920407aa6 grants create:user_api_keys to the Basic group and
backfills its members' stored permissions. The test database already sits at
head, so the test runs downgrade then upgrade inside its transaction and
rolls back."""
import importlib.util
from importlib.machinery import ModuleSpec
from pathlib import Path
from types import ModuleType
from uuid import uuid4
from alembic.migration import MigrationContext
from alembic.operations import Operations
from sqlalchemy import select
from sqlalchemy.orm import Session
from onyx.db.enums import GrantSource, Permission
from onyx.db.models import PermissionGrant, User, User__UserGroup, UserGroup
from onyx.db.users import DEFAULT_BASIC_GROUP_NAME
_MIGRATION_PATH = (
Path(__file__).parents[3]
/ "alembic/versions/b6d920407aa6_grant_create_pat_to_basic_group.py"
)
PAT_PERMISSION = Permission.CREATE_USER_API_KEYS.value
BASIC_PERMISSION = Permission.BASIC_ACCESS.value
def _load_migration() -> ModuleType:
spec: ModuleSpec | None = importlib.util.spec_from_file_location(
"pat_grant_migration", _MIGRATION_PATH
)
assert spec is not None and spec.loader is not None
migration: ModuleType = importlib.util.module_from_spec(spec)
spec.loader.exec_module(migration)
return migration
def _basic_grant(db_session: Session, basic: UserGroup) -> PermissionGrant | None:
return db_session.scalar(
select(PermissionGrant).where(
PermissionGrant.group_id == basic.id,
PermissionGrant.permission == Permission.CREATE_USER_API_KEYS,
)
)
def _stored_permissions(db_session: Session, user: User) -> set[str]:
db_session.expire(user)
return set(user.effective_permissions)
def _make_user(db_session: Session, permissions: list[str]) -> User:
user: User = User(
id=uuid4(),
email=f"pat-migration-{uuid4().hex[:8]}@example.com",
hashed_password="unused",
effective_permissions=permissions,
)
db_session.add(user)
return user
def test_basic_members_lose_and_regain_the_grant(
db_session: Session,
tenant_context: None, # noqa: ARG001
) -> None:
try:
basic: UserGroup | None = db_session.scalar(
select(UserGroup).where(
UserGroup.name == DEFAULT_BASIC_GROUP_NAME,
UserGroup.is_default.is_(True),
)
)
assert basic is not None
other_group: UserGroup = UserGroup(name=f"pat-migration-{uuid4().hex[:8]}")
db_session.add(other_group)
db_session.flush()
db_session.add(
PermissionGrant(
group_id=other_group.id,
permission=Permission.CREATE_USER_API_KEYS,
grant_source=GrantSource.USER,
)
)
member: User = _make_user(db_session, [BASIC_PERMISSION, PAT_PERMISSION])
dual: User = _make_user(db_session, [BASIC_PERMISSION, PAT_PERMISSION])
outsider: User = _make_user(db_session, [])
db_session.flush()
db_session.add_all(
[
User__UserGroup(user_group_id=basic.id, user_id=member.id),
User__UserGroup(user_group_id=basic.id, user_id=dual.id),
User__UserGroup(user_group_id=other_group.id, user_id=dual.id),
]
)
db_session.flush()
migration: ModuleType = _load_migration()
with Operations.context(MigrationContext.configure(db_session.connection())):
migration.downgrade()
assert _basic_grant(db_session, basic) is None
assert _stored_permissions(db_session, member) == {BASIC_PERMISSION}
# Held through the other group, so the downgrade leaves it alone.
assert _stored_permissions(db_session, dual) == {
BASIC_PERMISSION,
PAT_PERMISSION,
}
assert _stored_permissions(db_session, outsider) == set()
migration.upgrade()
grant: PermissionGrant | None = _basic_grant(db_session, basic)
assert grant is not None
assert grant.grant_source == GrantSource.SYSTEM
assert not grant.is_deleted
assert _stored_permissions(db_session, member) == {
BASIC_PERMISSION,
PAT_PERMISSION,
}
db_session.expire(dual)
assert sorted(dual.effective_permissions) == [
BASIC_PERMISSION,
PAT_PERMISSION,
]
assert _stored_permissions(db_session, outsider) == set()
# A soft-deleted Basic grant is revived rather than duplicated.
grant.is_deleted = True
member.effective_permissions = [BASIC_PERMISSION]
db_session.flush()
migration.upgrade()
db_session.expire(grant)
assert not grant.is_deleted
assert PAT_PERMISSION in _stored_permissions(db_session, member)
finally:
db_session.rollback()
@@ -110,9 +110,22 @@ class UserGroupManager:
user_group: DATestUserGroup,
user_performing_action: DATestUser,
include_non_toggleable: bool = False,
) -> list[str]:
return UserGroupManager.get_permissions_by_id(
user_group.id, user_performing_action, include_non_toggleable
)
# *_by_id variants: SCIM tests hold only the group id the SCIM API returns, and
# default-group tests hold the API model from get_default.
@staticmethod
def get_permissions_by_id(
user_group_id: int | str,
user_performing_action: DATestUser,
include_non_toggleable: bool = False,
) -> list[str]:
response = client.get(
f"{API_SERVER_URL}/manage/admin/user-group/{user_group.id}/permissions",
f"{API_SERVER_URL}/manage/admin/user-group/{user_group_id}/permissions",
params=(
{"include_non_toggleable": "true"} if include_non_toggleable else None
),
@@ -121,8 +134,6 @@ class UserGroupManager:
response.raise_for_status()
return response.json()
# *_by_id variants: SCIM tests hold only the group id the SCIM API returns.
@staticmethod
def set_permissions_by_id(
user_group_id: int | str,
@@ -1,14 +1,5 @@
"""
Integration tests for Personal Access Token (PAT) API.
Test Suite:
1. test_pat_lifecycle_happy_path - Complete PAT lifecycle (create, auth, revoke)
2. test_pat_user_isolation_and_authentication - User authentication and multi-user isolation
3. test_pat_expiration_flow - Expiration logic (end-of-day UTC, never-expiring)
4. test_pat_validation_errors - Input validation and error handling
5. test_pat_sorting_and_last_used - Sorting and last_used_at tracking
6. test_pat_role_based_access_control - Admin vs Basic permissions
"""
"""Integration tests for the personal access token API: lifecycle, isolation,
expiration, validation, sorting, and permission inheritance."""
import os
import time
@@ -25,30 +16,6 @@ from tests.integration.common_utils.managers.user_group import UserGroupManager
from tests.integration.common_utils.test_models import DATestUser
def _grant_create_pat_permission(
user: DATestUser,
admin_user: DATestUser,
) -> None:
"""Grant CREATE_USER_API_KEYS to a non-admin user via a temp group.
PAT creation is gated by Permission.CREATE_USER_API_KEYS; non-admin test
users must be placed in a group that grants it before they can call
POST /user/pats.
"""
group = UserGroupManager.create(
name=f"pat_creator_{user.id[:8]}",
user_ids=[user.id],
cc_pair_ids=[],
user_performing_action=admin_user,
)
response = UserGroupManager.set_permissions(
user_group=group,
permissions=[Permission.CREATE_USER_API_KEYS.value],
user_performing_action=admin_user,
)
response.raise_for_status()
def test_pat_lifecycle_happy_path(reset: None) -> None: # noqa: ARG001
"""Complete PAT lifecycle: create, authenticate, revoke."""
user: DATestUser = UserManager.create(name="pat_user")
@@ -109,11 +76,6 @@ def test_pat_user_isolation_and_authentication(
user_a: DATestUser = UserManager.create(name="user_a")
user_b: DATestUser = UserManager.create(name="user_b")
# user_a is the first registered user and lands in Admin → has
# FULL_ADMIN_PANEL_ACCESS which short-circuits the PAT-creation gate.
# user_b is basic and needs CREATE_USER_API_KEYS via a group grant.
_grant_create_pat_permission(user_b, user_a)
# Create tokens for both users
user_a_pats = []
for i in range(2):
@@ -350,8 +312,6 @@ def test_pat_role_based_access_control(reset: None) -> None: # noqa: ARG001
basic_user: DATestUser = UserManager.create(name="basic_user")
assert not basic_user.is_admin
_grant_create_pat_permission(basic_user, admin_user)
admin_pat = PATManager.create(
name="Admin Token",
expiration_days=7,
@@ -448,7 +408,6 @@ def test_pat_group_permission_access_control(reset: None) -> None: # noqa: ARG0
permissions=[
Permission.MANAGE_CONNECTORS.value,
Permission.MANAGE_USER_GROUPS.value,
Permission.CREATE_USER_API_KEYS.value,
],
user_performing_action=admin_user,
)
@@ -483,7 +442,6 @@ def test_pat_group_permission_access_control(reset: None) -> None: # noqa: ARG0
# Sanity: a plain basic user's PAT still cannot reach that endpoint
plain_basic: DATestUser = UserManager.create(name="plain_basic")
_grant_create_pat_permission(plain_basic, admin_user)
plain_pat = PATManager.create(
name="Plain Basic Token",
expiration_days=7,
@@ -83,7 +83,7 @@ def permission_admin_user(module_reset: None) -> DATestUser: # noqa: ARG001
def permission_basic_user(
permission_admin_user: DATestUser, # noqa: ARG001
) -> DATestUser:
"""Second registered user — joins the default Basic user group with only basic permissions."""
"""Second registered user. Joins the default Basic group and holds only its grants."""
return UserManager.create(name="perm_basic")
@@ -1,24 +1,13 @@
"""Integration tests for assigning scopes when minting a PAT via the user API.
Minting needs CREATE_USER_API_KEYS, so those tests use ``pat_creator``. Listing
scopes is BASIC_ACCESS and stays on the basic user, proving it needs no creation
rights.
Minting needs CREATE_USER_API_KEYS, which the seeded Basic group grants, so every
test runs as the basic user.
"""
from typing import Any
import pytest
from onyx.db.enums import Permission
from tests.integration.common_utils.managers.pat import PATManager
from tests.integration.common_utils.test_models import DATestUser
@pytest.fixture(scope="module")
def pat_creator(permission_holder_user_factory: Any) -> DATestUser:
return permission_holder_user_factory(Permission.CREATE_USER_API_KEYS.value)
# The scopes a user may assign today (admin scopes are intentionally excluded).
EXPECTED_SELECTABLE_SCOPES = {
Permission.READ_SEARCH.value,
@@ -48,30 +37,36 @@ def test_scope_implications(permission_basic_user: DATestUser) -> None:
assert by_scope[Permission.USE_LLM_GATEWAY.value]["min_tier"] == "business"
def test_scopes_round_trip(pat_creator: DATestUser) -> None:
def test_scopes_round_trip(permission_basic_user: DATestUser) -> None:
scopes = [Permission.READ_SEARCH, Permission.READ_CHAT]
expected = {s.value for s in scopes}
created = PATManager.create("scoped-token", None, pat_creator, scopes=scopes)
created = PATManager.create(
"scoped-token", None, permission_basic_user, scopes=scopes
)
assert created.scopes is not None and set(created.scopes) == expected
listed = next(pat for pat in PATManager.list(pat_creator) if pat.id == created.id)
listed = next(
pat for pat in PATManager.list(permission_basic_user) if pat.id == created.id
)
assert listed.scopes is not None and set(listed.scopes) == expected
def test_unscoped_token_is_unrestricted(pat_creator: DATestUser) -> None:
created = PATManager.create("full-token", None, pat_creator)
def test_unscoped_token_is_unrestricted(permission_basic_user: DATestUser) -> None:
created = PATManager.create("full-token", None, permission_basic_user)
assert created.scopes is None
def test_empty_scopes_rejected(pat_creator: DATestUser) -> None:
response = PATManager.create_response("empty-token", None, pat_creator, scopes=[])
assert response.status_code == 400
assert response.json()["error_code"] == "INVALID_INPUT"
def test_non_selectable_scope_rejected(pat_creator: DATestUser) -> None:
def test_empty_scopes_rejected(permission_basic_user: DATestUser) -> None:
response = PATManager.create_response(
"admin-token", None, pat_creator, scopes=[Permission.READ_ADMIN]
"empty-token", None, permission_basic_user, scopes=[]
)
assert response.status_code == 400
assert response.json()["error_code"] == "INVALID_INPUT"
def test_non_selectable_scope_rejected(permission_basic_user: DATestUser) -> None:
response = PATManager.create_response(
"admin-token", None, permission_basic_user, scopes=[Permission.READ_ADMIN]
)
assert response.status_code == 400
assert response.json()["error_code"] == "INVALID_INPUT"
@@ -4,6 +4,9 @@ Covers the personal-access-token creation endpoint in
``backend/onyx/server/pat/api.py`` (router prefix ``/user/pats``).
The basic-access GET endpoints on the same router are covered by
``test_basic_access.py`` — this file only exercises the CREATE token.
The seeded Basic group grants this permission, so every standard user is a
holder. The matrix below replaces the shared USER_KINDS to expect that.
"""
import os
@@ -14,7 +17,6 @@ import pytest
from onyx.db.enums import Permission
from tests.integration.common_utils.test_models import DATestAPIKey, DATestUser
from tests.integration.tests.permissions._access_matrix import (
USER_KINDS,
Endpoint,
assert_response,
call_endpoint,
@@ -28,6 +30,17 @@ pytestmark = pytest.mark.skipif(
PERMISSION = Permission.CREATE_USER_API_KEYS.value
USER_KINDS: list[tuple[str, str]] = [
("admin", "allowed"),
("holder", "allowed"),
("service_account_holder", "allowed"),
("basic", "allowed"),
("service_account", "denied"),
("bot", "denied"),
("ext_perm", "denied"),
("anonymous", "anon_denied"),
]
_CREATE_PAT_BODY: dict[str, Any] = {
"name": "perm-test-pat",
"expiration_days": 7,
@@ -328,9 +328,10 @@ def test_holder_cannot_touch_the_basic_group(
permission_admin_user: DATestUser,
holder_user: DATestUser,
) -> None:
"""Basic grants only BASIC_ACCESS, so the amplification check above can't be what
stops the holder — the default-group gate is. The affordance map agrees, which is
what keeps the groups page from listing Admin/Basic for them."""
"""The holder is a Basic member and already holds every Basic grant, so the
amplification check above can't be what stops them. The default-group gate is. The
affordance map agrees, which is what keeps the groups page from listing Admin/Basic
for them."""
basic = UserGroupManager.get_default(
user_performing_action=permission_admin_user, name="Basic"
)
@@ -0,0 +1,68 @@
"""The seeded Basic group grants create:user_api_keys by default. An admin can switch
it off and back on, and nothing else can be toggled there."""
import os
import httpx
import pytest
from ee.onyx.server.user_group.models import UserGroup
from onyx.db.enums import Permission
from tests.integration.common_utils.managers.pat import PATManager
from tests.integration.common_utils.managers.user import UserManager
from tests.integration.common_utils.managers.user_group import UserGroupManager
from tests.integration.common_utils.test_models import DATestPAT, DATestUser
PAT_PERMISSION = Permission.CREATE_USER_API_KEYS.value
@pytest.mark.skipif(
os.environ.get("RUN_EE_TESTS", "").lower() != "true",
reason="User group tests are enterprise only",
)
def test_basic_group_pat_toggle(new_admin_user: DATestUser) -> None:
basic: UserGroup = UserGroupManager.get_default(
user_performing_action=new_admin_user, name="Basic"
)
member: DATestUser = UserManager.create(name="basic_pat_toggle_member")
# On by default, so a fresh basic user mints a token.
assert PAT_PERMISSION in UserGroupManager.get_permissions_by_id(
basic.id, new_admin_user
)
minted: DATestPAT = PATManager.create(
name="before-toggle", expiration_days=7, user_performing_action=member
)
assert minted.token is not None
off: httpx.Response = UserGroupManager.set_permissions_by_id(
basic.id, [], new_admin_user
)
off.raise_for_status()
assert PAT_PERMISSION not in UserGroupManager.get_permissions_by_id(
basic.id, new_admin_user
)
denied: httpx.Response = PATManager.create_response("after-toggle", 7, member)
assert denied.status_code == 403
on: httpx.Response = UserGroupManager.set_permissions_by_id(
basic.id, [PAT_PERMISSION], new_admin_user
)
on.raise_for_status()
restored: DATestPAT = PATManager.create(
name="after-restore", expiration_days=7, user_performing_action=member
)
assert restored.token is not None
# Only the allowlisted toggles exist on Basic, and Admin takes none.
refused: httpx.Response = UserGroupManager.set_permissions_by_id(
basic.id, [PAT_PERMISSION, Permission.MANAGE_LLMS.value], new_admin_user
)
assert refused.status_code == 409
admin_group: UserGroup = UserGroupManager.get_default(
user_performing_action=new_admin_user, name="Admin"
)
refused_on_admin: httpx.Response = UserGroupManager.set_permissions_by_id(
admin_group.id, [PAT_PERMISSION], new_admin_user
)
assert refused_on_admin.status_code == 409
@@ -1,5 +1,6 @@
"""The seeded default groups (Admin/Basic) are visible, hold members and nothing else,
and only a full admin may change that membership.
"""The seeded default groups (Admin/Basic) are visible and hold members and nothing
else. Only a full admin may change that membership or switch Basic's allowlisted
permission toggles.
The list endpoint still returns them to any READ_USER_GROUPS holder that asks — the
service-account picker relies on that — so "full admins only" rides on the ``permissions``
@@ -44,8 +45,9 @@ def test_default_groups_are_listed_only_when_requested(admin_user: DATestUser) -
@ENTERPRISE_SKIP
def test_default_group_affordances_are_membership_only(admin_user: DATestUser) -> None:
"""Whole-map equality: a new action defaulting to True must fail here rather than
quietly render a control the routes reject."""
for name in ("Admin", "Basic"):
quietly render a control the routes reject. Basic alone offers its allowlisted
permission toggles."""
for name, edit_permissions in (("Admin", False), ("Basic", True)):
group = UserGroupManager.get_default(
user_performing_action=admin_user, name=name
)
@@ -53,7 +55,7 @@ def test_default_group_affordances_are_membership_only(admin_user: DATestUser) -
"manage": False,
"manage_members": True,
"delete": False,
"edit_permissions": False,
"edit_permissions": edit_permissions,
"edit_token_limits": False,
}, name
@@ -73,6 +75,7 @@ def test_default_group_rejects_every_non_membership_write(
json={"id": group.id, "name": "Renamed"},
headers=headers,
),
# outside the Basic allowlist, so still refused
"permissions": client.put(
f"{GROUP_URL}/{group.id}/permissions",
json={"permissions": [Permission.MANAGE_LLMS.value]},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "يجري تحديث تضمينات المستندات بسبب تغييرات حديثة على هذه المجموعة."
},
"systemGroup": {
"basicDescription": "يدير {appName} هذه المجموعة. الأعضاء هم الشيء الوحيد الذي تحتويه، والأذونات أدناه تنطبق على الجميع فيها.",
"description": "يدير {appName} هذه المجموعة. الأعضاء هم الشيء الوحيد الذي تحتويه، والشيء الوحيد الذي يمكنك تغييره.",
"title": "مجموعة نظام"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "Die Dokument-Embeddings werden aufgrund kürzlicher Änderungen an dieser Gruppe aktualisiert."
},
"systemGroup": {
"basicDescription": "{appName} verwaltet diese Gruppe. Sie enthält nur Mitglieder, und die Berechtigungen unten gelten für alle darin.",
"description": "{appName} verwaltet diese Gruppe. Sie enthält nur Mitglieder, und nur diese können Sie ändern.",
"title": "Systemgruppe"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "Document embeddings are being updated due to recent changes to this group."
},
"systemGroup": {
"basicDescription": "{appName} manages this group. Members are the only thing it holds, and the permissions below apply to everyone in it.",
"description": "{appName} manages this group. Members are the only thing it holds, and the only thing you can change.",
"title": "System group"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "Los embeddings de los documentos se están actualizando por los cambios recientes en este grupo."
},
"systemGroup": {
"basicDescription": "{appName} gestiona este grupo. Solo contiene miembros, y los permisos de abajo se aplican a todos ellos.",
"description": "{appName} gestiona este grupo. Solo contiene miembros y es lo único que puedes cambiar.",
"title": "Grupo del sistema"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "Les embeddings des documents sont en cours de mise à jour à la suite de modifications récentes de ce groupe."
},
"systemGroup": {
"basicDescription": "{appName} gère ce groupe. Il ne contient que des membres, et les permissions ci-dessous s'appliquent à tous.",
"description": "{appName} gère ce groupe. Il ne contient que des membres, et c'est la seule chose que vous pouvez modifier.",
"title": "Groupe système"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "このグループの最近の変更により、ドキュメントの embedding を更新しています。"
},
"systemGroup": {
"basicDescription": "このグループは {appName} が管理します。保持するのはメンバーのみで、以下の権限は全員に適用されます。",
"description": "このグループは {appName} が管理します。保持するのはメンバーのみで、変更できるのもメンバーだけです。",
"title": "システムグループ"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "이 그룹의 최근 변경 사항으로 인해 문서 embedding을 업데이트하는 중입니다."
},
"systemGroup": {
"basicDescription": "{appName}가 이 그룹을 관리합니다. 이 그룹에는 멤버만 포함되며, 아래 권한은 모든 멤버에게 적용됩니다.",
"description": "{appName}가 이 그룹을 관리합니다. 이 그룹에는 멤버만 포함되며, 변경할 수 있는 항목도 멤버뿐입니다.",
"title": "시스템 그룹"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "Os embeddings dos documentos estão sendo atualizados por causa de alterações recentes neste grupo."
},
"systemGroup": {
"basicDescription": "O {appName} gerencia este grupo. Ele contém apenas membros, e as permissões abaixo se aplicam a todos eles.",
"description": "O {appName} gerencia este grupo. Ele contém apenas membros, e isso é a única coisa que você pode alterar.",
"title": "Grupo do sistema"
},
+1
View File
@@ -4426,6 +4426,7 @@
"tooltip": "由于此用户组最近发生更改,正在更新文档 embedding。"
},
"systemGroup": {
"basicDescription": "此用户组由 {appName} 管理。它只包含成员,下方的权限适用于其中的所有人。",
"description": "此用户组由 {appName} 管理。它只包含成员,您也只能更改成员。",
"title": "系统用户组"
},
@@ -38,7 +38,7 @@ import { useUser } from "@/providers/UserProvider";
import { useSettings } from "@/lib/settings/hooks";
import { Tier } from "@/lib/settings/types";
import { tierAtLeast } from "@/lib/tiers";
import type { MemberRow, TokenRateLimitDisplay } from "./interfaces";
import type { MemberRow, TokenRateLimitDisplay } from "./types";
import {
makeBaseColumns,
makeMemberTableColumns,
@@ -629,9 +629,12 @@ function EditGroupPage({ groupId }: EditGroupPageProps) {
<MessageCard
variant="info"
title={t("edit.systemGroup.title")}
description={t("edit.systemGroup.description", {
appName: settings.appName,
})}
description={t(
canEditPermissions
? "edit.systemGroup.basicDescription"
: "edit.systemGroup.description",
{ appName: settings.appName }
)}
/>
)}
@@ -744,6 +747,7 @@ function EditGroupPage({ groupId }: EditGroupPageProps) {
<GroupPermissionsSection
enabledPermissions={enabledPermissions}
onPermissionsChange={setEnabledPermissions}
isDefaultGroup={isDefaultGroup}
/>
)}
@@ -29,7 +29,7 @@ import Card from "@/refresh-components/cards/Card";
import { Collapsible } from "@opal/components";
import { errorHandlingFetcher } from "@/lib/fetcher";
import { SWR_KEYS } from "@/lib/swr-keys";
import type { PermissionRegistryEntry } from "@/views/admin/GroupsPage/interfaces";
import type { PermissionRegistryEntry } from "@/views/admin/GroupsPage/types";
// Icon mapping — the only permission metadata kept in the frontend.
// `id` keys must match the backend PERMISSION_REGISTRY entries.
@@ -50,17 +50,23 @@ const ICON_MAP: Record<string, IconFunctionComponent> = {
interface GroupPermissionsSectionProps {
enabledPermissions: Set<string>;
onPermissionsChange: (permissions: Set<string>) => void;
// A seeded default group only offers the rows the backend marks for it.
isDefaultGroup?: boolean;
}
function GroupPermissionsSection({
enabledPermissions,
onPermissionsChange,
isDefaultGroup = false,
}: GroupPermissionsSectionProps) {
const t = useTranslations("admin.groups");
const { data: registry, isLoading } = useSWR<PermissionRegistryEntry[]>(
const { data: fullRegistry, isLoading } = useSWR<PermissionRegistryEntry[]>(
SWR_KEYS.permissionRegistry,
errorHandlingFetcher
);
const registry = isDefaultGroup
? fullRegistry?.filter((entry) => entry.default_group_toggleable)
: fullRegistry;
function isRowEnabled(entry: PermissionRegistryEntry): boolean {
return entry.permissions.every((p) => enabledPermissions.has(p));
+1 -1
View File
@@ -6,7 +6,7 @@ import { SvgSlack } from "@opal/logos";
import type { IconFunctionComponent } from "@opal/types";
import Text from "@/refresh-components/texts/Text";
import { AccountType, UserStatus } from "@/lib/types";
import type { ApiKeyDescriptor, MemberRow } from "./interfaces";
import type { ApiKeyDescriptor, MemberRow } from "./types";
// ---------------------------------------------------------------------------
// Constants
@@ -29,4 +29,5 @@ export interface PermissionRegistryEntry {
description: string;
permissions: string[];
group: number;
default_group_toggleable: boolean;
}
@@ -7,7 +7,7 @@ import { SWR_KEYS } from "@/lib/swr-keys";
import { useUser } from "@/providers/UserProvider";
import { AccountType, UserStatus } from "@/lib/types";
import type { FullUserSnapshot } from "@/views/admin/UsersPage/types";
import type { ApiKeyDescriptor, MemberRow } from "./interfaces";
import type { ApiKeyDescriptor, MemberRow } from "./types";
interface ManageUsersResponse {
accepted: FullUserSnapshot[];
+11 -1
View File
@@ -165,11 +165,21 @@ export class GroupsAdminPage {
return this.page.getByText("System group", { exact: true });
}
/** Header of the group-permissions section (full admin, custom groups only). */
/** Header of the group-permissions section (full admin, custom groups and Basic). */
get permissionsSectionHeader(): Locator {
return this.page.getByText("Group Permissions", { exact: true });
}
/** One permission row in that section, matched by its display name. */
permissionRow(displayName: string): Locator {
return this.page.getByText(displayName, { exact: true });
}
/** Every permission switch on the edit page. */
get permissionSwitches(): Locator {
return this.page.getByRole("switch");
}
/** Header of the shared-resources section (custom groups only). */
get sharedResourcesSectionHeader(): Locator {
return this.page.getByText("Shared with This Group", { exact: true });
+7 -1
View File
@@ -103,7 +103,13 @@ test.describe("Groups page — layout", () => {
await expect(groupsPage.saveButton).toBeEnabled();
await expect(groupsPage.deleteGroupButton).toHaveCount(0);
await expect(groupsPage.permissionsSectionHeader).toHaveCount(0);
// Basic offers its allowlisted toggle and nothing else. The switch state is
// left alone because the gating spec flips it from another worker.
await expect(groupsPage.permissionsSectionHeader).toBeVisible();
await expect(
groupsPage.permissionRow("Create User Access Token")
).toBeVisible();
await expect(groupsPage.permissionSwitches).toHaveCount(1);
await expect(groupsPage.sharedResourcesSectionHeader).toHaveCount(0);
});
@@ -879,8 +879,9 @@ test.describe("Permission gating — READ_QUERY_HISTORY", () => {
});
test.describe("Permission gating — CREATE_USER_API_KEYS", () => {
test("Access Tokens section and New Access Token button are gated behind CREATE_USER_API_KEYS", async ({
test("The Basic group grants Access Tokens by default and an admin can switch it off", async ({
page,
browser,
adminClient,
testUserContext,
}) => {
@@ -892,30 +893,16 @@ test.describe("Permission gating — CREATE_USER_API_KEYS", () => {
"Permission registry unavailable (CE environment)"
);
const { groupId, email, password } = testUserContext;
const { email, password } = testUserContext;
const basicGroup = (await adminClient.getUserGroups()).find(
(g) => g.is_default && g.name === "Basic"
);
expect(basicGroup).toBeDefined();
const basicGroupId = basicGroup!.id;
let createdPatId: number | undefined;
try {
// Phase 1: Without permission and no tokens — section should be hidden entirely
await page.context().clearCookies();
await apiLogin(page, email, password);
await page.goto("/app/settings/accounts-access");
await page.waitForLoadState("networkidle");
await expect(
page.getByText("Access Tokens", { exact: true })
).not.toBeVisible();
await expect(
page.getByRole("button", { name: "New Access Token" })
).not.toBeVisible();
// Phase 2: Grant CREATE_USER_API_KEYS — section visible, button enabled
await page.context().clearCookies();
await loginAs(page, "admin");
await adminClient.setUserGroupPermissions(groupId, [
Permission.CREATE_USER_API_KEYS,
]);
// Phase 1: Basic grants it by default, so the section shows and the button is enabled
await page.context().clearCookies();
await apiLogin(page, email, password);
await page.goto("/app/settings/accounts-access");
@@ -931,7 +918,7 @@ test.describe("Permission gating — CREATE_USER_API_KEYS", () => {
await expect(newTokenButton).toBeVisible({ timeout: 10000 });
await expect(newTokenButton).toBeEnabled();
// Create a PAT via API so Phase 3 can test "token exists but no permission"
// Create a PAT via API so Phase 2 can test "token exists but no permission"
const createResponse = await page.request.post("/api/user/pats", {
data: {
name: `E2E PAT ${Date.now()}`,
@@ -942,10 +929,10 @@ test.describe("Permission gating — CREATE_USER_API_KEYS", () => {
const patData = await createResponse.json();
createdPatId = patData.id;
// Phase 3: Revoke permission — section stays (token exists) but button disabled
// Phase 2: Switch it off on Basic. The section stays (a token exists) but the button is disabled
await page.context().clearCookies();
await loginAs(page, "admin");
await adminClient.setUserGroupPermissions(groupId, []);
await adminClient.setUserGroupPermissions(basicGroupId, []);
await page.context().clearCookies();
await apiLogin(page, email, password);
@@ -958,9 +945,39 @@ test.describe("Permission gating — CREATE_USER_API_KEYS", () => {
await expect(newTokenButton).toBeVisible({ timeout: 10000 });
await expect(newTokenButton).toBeDisabled();
// Phase 3: Switch it back on and the button is enabled again
await page.context().clearCookies();
await loginAs(page, "admin");
await adminClient.setUserGroupPermissions(basicGroupId, [
Permission.CREATE_USER_API_KEYS,
]);
await page.context().clearCookies();
await apiLogin(page, email, password);
await page.goto("/app/settings/accounts-access");
await page.waitForLoadState("networkidle");
await expect(newTokenButton).toBeVisible({ timeout: 10000 });
await expect(newTokenButton).toBeEnabled();
} finally {
// Basic is shared with every other spec, so restore it from a context
// that outlives this test's page if the test timed out.
await cleanup(async () => {
const restoreContext = await browser.newContext({
storageState: "admin_auth.json",
});
try {
await new OnyxApiClient(
restoreContext.request
).setUserGroupPermissions(basicGroupId, [
Permission.CREATE_USER_API_KEYS,
]);
} finally {
await restoreContext.close();
}
});
await cleanup(async () => {
// Delete the PAT (only requires BASIC_ACCESS)
if (createdPatId !== undefined) {
await page.context().clearCookies();
await apiLogin(page, email, password);