Skip to content

Commit 70f40d5

Browse files
committed
feat: give the identity an ordered role instead of a single is_admin boolean
Add `UserRole`, an ordered `StrEnum` (none < viewer < editor < admin < super_admin) whose members compare by rank rather than alphabetically, and a required `role` field on `BaseUser`. `is_admin` becomes a computed property equal to `role >= UserRole.ADMIN`, so every consumer keeps reading the same boolean and admits the same identities. Each of the five construction paths sets `role` explicitly: the two Grafana record mappings flatten org memberships by rank (server-admin flag outranking them), the Casdoor payload derives it from the admin flag, and the service principal takes `VIEWER`. Both inbound legacy-claim paths reconstruct the role through one shared helper that validates the flag with Pydantic's boolean semantics, so a payload spelling the flag "false" or "0" still means non-admin. The minted assertion is unchanged, so tokens issued before this change keep decoding; Editor and Viewer therefore stay indistinguishable on bearer requests until the assertion carries a real role claim. `role` is required with no default so a construction path that forgets it fails loudly instead of silently yielding the lowest role. This breaks out-of-tree `CUSTOM` providers that build identities from `is_admin` alone, which ships as a breaking changelog fragment.
1 parent c3e1d57 commit 70f40d5

19 files changed

Lines changed: 826 additions & 75 deletions

File tree

.github/instructions/tests.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Tests mirror app structure exactly. `app/sep/snippets/config.py` → `tests/app/
1212

1313
## Factories — never manual dicts
1414

15-
Test data MUST come from a factory, never a hand-rolled `dict`. Core, cross-app factories live in `tests/app/factories.py` (`SchemaWriteFactory`, `TableWriteFactory`, `TaskFactory` / `GeneratedTaskFactory`, `PeriodicTaskFactory`, …). Build with `.build()`, customise inline: `CasdoorUserFactory.build(is_admin=True)`. Use mock ID constants from `tests/app/factories.py`.
15+
Test data MUST come from a factory, never a hand-rolled `dict`. Core, cross-app factories live in `tests/app/factories.py` (`SchemaWriteFactory`, `TableWriteFactory`, `TaskFactory` / `GeneratedTaskFactory`, `PeriodicTaskFactory`, …). Build with `.build()`, customise inline: `CasdoorUserFactory.build(role=UserRole.ADMIN)`. Use mock ID constants from `tests/app/factories.py`.
1616

1717
**A factory for an app's model belongs in `tests/app/sep/apps/<app>/factories.py`, not the shared file.** `tests/app/factories.py` sits at the root of the test tree, so every subtree imports it — an app-specific factory there couples the shared file to that app and has to be found and deleted by hand when the app goes. Adding an app means adding `tests/app/sep/apps/<app>/factories.py`, never editing the shared module; the app test packages already have `__init__.py`, so `tests.app.sep.apps.<app>.factories` resolves as-is. `tests/app/test_factories_boundary.py` enforces this: any module directly under `tests/app/` that imports `app.sep.apps.*` (or re-exports from `tests.app.sep.apps.*`) fails.
1818

app/api/deps.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
HTTPUnauthorizedException,
3131
InactiveUserException,
3232
)
33+
from app.core.auth.models import UserRole
3334
from app.core.auth.utils import get_user_model
3435
from app.core.config import settings
3536
from app.core.log import set_log_context
@@ -53,6 +54,7 @@
5354
username="sep-service",
5455
first_name="SEP",
5556
last_name="Service",
57+
role=UserRole.VIEWER,
5658
)
5759

5860

@@ -166,8 +168,9 @@ async def require_admin_for_unsafe_methods(request: Request) -> None:
166168
167169
``SEP_INTERNAL_TOKEN``'s service principal is admitted by identity so
168170
scheduled inventory sync and scheduled execution keep working. The bypass is
169-
scoped to this gate: the principal keeps ``is_admin=False`` and every
170-
pre-existing ``IsApiAdmin`` / ``IsAdminDep`` check refuses it as before.
171+
scoped to this gate: the principal holds ``UserRole.VIEWER``, so it keeps
172+
``is_admin=False`` and every pre-existing ``IsApiAdmin`` / ``IsAdminDep``
173+
check refuses it as before.
171174
172175
:param request: The incoming HTTP request.
173176
:raises HTTPUnauthorizedException: When the method is unsafe and the request

app/core/auth/models.py

Lines changed: 103 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,28 @@
1818
from abc import ABC, abstractmethod
1919
from collections.abc import Sequence
2020
from datetime import datetime
21+
from enum import StrEnum
2122
from functools import cached_property
22-
from typing import Any, Self
23+
from typing import Any, Final, Self
2324

2425
from pydantic import (
2526
BaseModel,
2627
computed_field,
2728
Field,
2829
FutureDatetime,
2930
PastDatetime,
31+
TypeAdapter,
3032
UUID4,
33+
ValidationError,
3134
)
3235

3336
from app.core.utils.date_time import utc_now
34-
from app.core.utils.fields import EmptyStrToNone, NonEmptyStr, TimedeltaSeconds
37+
from app.core.utils.fields import (
38+
EmptyStrToNone,
39+
EnumFieldMixin,
40+
NonEmptyStr,
41+
TimedeltaSeconds,
42+
)
3543

3644

3745
class OAuthToken(BaseModel):
@@ -125,6 +133,62 @@ async def from_jwt(cls, token: str) -> Self:
125133
"""
126134

127135

136+
class UserRole(EnumFieldMixin, StrEnum):
137+
"""Enumerate an identity's access level, lowest to highest.
138+
139+
Members and ordering mirror PMM's own authorization vocabulary so the two
140+
products stay semantically aligned; ``SUPER_ADMIN`` is SEP's
141+
provider-neutral name for the rank PMM calls ``grafanaAdmin``.
142+
143+
Members compare by rank rather than by name: ``EDITOR < ADMIN`` even
144+
though ``"editor" > "admin"`` lexicographically. All four comparisons are
145+
spelled out, and each refuses a non-member, because ``str`` supplies its own
146+
alphabetical implementations. Whatever this class leaves to ``str`` — an
147+
operator it does not override, or an operand it declines — silently ranks
148+
``EDITOR`` above ``ADMIN``. Equality is untouched, so ``ADMIN == "admin"``
149+
still holds.
150+
"""
151+
152+
NONE = "none"
153+
VIEWER = "viewer"
154+
EDITOR = "editor"
155+
ADMIN = "admin"
156+
SUPER_ADMIN = "super_admin"
157+
158+
def __lt__(self, other: object) -> bool:
159+
return _rank(self) < _rank(other)
160+
161+
def __le__(self, other: object) -> bool:
162+
return _rank(self) <= _rank(other)
163+
164+
def __gt__(self, other: object) -> bool:
165+
return _rank(self) > _rank(other)
166+
167+
def __ge__(self, other: object) -> bool:
168+
return _rank(self) >= _rank(other)
169+
170+
171+
_USER_ROLE_ORDER: Final = tuple(UserRole)
172+
173+
174+
def _rank(role: object) -> int:
175+
"""Return a role's position in the declared order, lowest first.
176+
177+
:param role: The value to rank.
178+
:return: The role's rank.
179+
:raises TypeError: If ``role`` is not a :class:`UserRole`. Returning
180+
``NotImplemented`` instead would hand the comparison back to ``str``,
181+
which answers it alphabetically, making ``UserRole.EDITOR >= "admin"``
182+
True.
183+
"""
184+
if not isinstance(role, UserRole):
185+
raise TypeError(f"cannot order UserRole against {type(role).__name__}")
186+
return _USER_ROLE_ORDER.index(role)
187+
188+
189+
_ADMIN_FLAG_ADAPTER: Final = TypeAdapter(bool)
190+
191+
128192
class BaseUser(BaseModel, ABC):
129193
"""Represent the abstract base for a user.
130194
@@ -133,8 +197,7 @@ class BaseUser(BaseModel, ABC):
133197
:param email: The email address of the user.
134198
:param first_name: The first name of the user.
135199
:param last_name: The last name of the user.
136-
:param is_admin: Whether the user has administrative privileges. Defaults
137-
to False.
200+
:param role: The user's access level. ``is_admin`` is derived from it.
138201
:param created_time: The datetime when the user was created. Defaults to
139202
current datetime.
140203
:param updated_time: The datetime when the user was last updated. Defaults
@@ -146,7 +209,7 @@ class BaseUser(BaseModel, ABC):
146209
email: str = ""
147210
first_name: str = ""
148211
last_name: str = ""
149-
is_admin: bool = False
212+
role: UserRole
150213
created_time: datetime | EmptyStrToNone = Field(default_factory=utc_now)
151214
updated_time: datetime | EmptyStrToNone = Field(default_factory=utc_now)
152215
_access_token: str = ""
@@ -171,6 +234,38 @@ def is_active(self) -> bool:
171234
"""
172235
return True
173236

237+
@computed_field
238+
@property
239+
def is_admin(self) -> bool:
240+
"""Indicate whether the user holds administrative privileges.
241+
242+
:return: True from ``ADMIN`` upwards, False below it.
243+
"""
244+
return self.role >= UserRole.ADMIN
245+
246+
@staticmethod
247+
def _role_from_admin_flag(flag: Any) -> UserRole:
248+
"""Map a legacy admin flag onto the ordered role.
249+
250+
The flag carries one bit, so it resolves only to the two roles the
251+
admin gates already distinguish: ``ADMIN`` preserves the access the flag
252+
granted, ``VIEWER`` the reads a non-admin has today.
253+
254+
The flag is validated rather than tested for truthiness, so a payload
255+
spelling the boolean as ``"false"`` or ``"0"`` keeps the meaning the
256+
removed ``is_admin`` field gave it instead of resolving to ``ADMIN``.
257+
258+
:param flag: The raw admin flag carried by a provider payload.
259+
:return: The role the flag maps to.
260+
:raises ValueError: If the flag is not a value Pydantic accepts as a
261+
boolean, matching the error the removed field raised.
262+
"""
263+
try:
264+
is_admin = _ADMIN_FLAG_ADAPTER.validate_python(flag)
265+
except ValidationError as exc:
266+
raise ValueError(f"invalid admin flag: {flag!r}") from exc
267+
return UserRole.ADMIN if is_admin else UserRole.VIEWER
268+
174269
@property
175270
def access_token(self) -> str:
176271
"""Get the user's access token.
@@ -195,10 +290,10 @@ def build_service_principal(
195290
*,
196291
user_id: UUID4,
197292
username: NonEmptyStr,
293+
role: UserRole,
198294
email: str = "",
199295
first_name: str = "",
200296
last_name: str = "",
201-
is_admin: bool = False,
202297
**provider_fields: Any,
203298
) -> Self:
204299
"""Build the synthetic service-principal user for SEP-internal auth.
@@ -209,11 +304,10 @@ def build_service_principal(
209304
210305
:param user_id: The service principal's stable unique identifier.
211306
:param username: The service principal's username.
307+
:param role: The service principal's access level.
212308
:param email: The service principal's email address; empty by default.
213309
:param first_name: The service principal's first name; empty by default.
214310
:param last_name: The service principal's last name; empty by default.
215-
:param is_admin: Whether the principal has administrative privileges;
216-
``False`` by default.
217311
:return: A user instance representing the service principal.
218312
"""
219313
return cls(
@@ -222,7 +316,7 @@ def build_service_principal(
222316
email=email,
223317
first_name=first_name,
224318
last_name=last_name,
225-
is_admin=is_admin,
319+
role=role,
226320
**provider_fields,
227321
)
228322

app/core/auth/providers/casdoor/models.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,17 @@
1818
from collections.abc import Sequence
1919
from typing import Annotated, Any, cast, Literal, Self
2020

21-
from pydantic import AliasChoices, computed_field, ConfigDict, Field, field_validator
21+
from pydantic import (
22+
AliasChoices,
23+
computed_field,
24+
ConfigDict,
25+
Field,
26+
field_validator,
27+
model_validator,
28+
)
2229
from pydantic.alias_generators import to_camel
2330

24-
from app.core.auth.models import BaseTokenPayload, BaseUser, OAuthToken
31+
from app.core.auth.models import BaseTokenPayload, BaseUser, OAuthToken, UserRole
2532
from app.core.auth.providers.casdoor.sdk import CasdoorSDK
2633

2734
CasdoorUsernameField = Annotated[
@@ -107,7 +114,8 @@ class CasdoorUser(BaseUser):
107114
:param email: The email address of the user.
108115
:param first_name: The first name of the user.
109116
:param last_name: The last name of the user.
110-
:param is_admin: Whether the user has administrative privileges. Defaults to False.
117+
:param role: The user's access level, derived from Casdoor's admin flag when
118+
the payload carries no role of its own.
111119
:param created_time: The datetime when the user was created. Defaults to current
112120
datetime.
113121
:param updated_time: The datetime when the user was last updated. Defaults to
@@ -126,6 +134,32 @@ class CasdoorUser(BaseUser):
126134
is_forbidden: bool = False
127135
is_deleted: bool = False
128136

137+
@model_validator(mode="before")
138+
@classmethod
139+
def _derive_role(cls, data: Any) -> Any:
140+
"""Derive the ordered role from Casdoor's admin flag.
141+
142+
Casdoor exposes no role concept, so the payload's admin boolean is the
143+
only signal. An unset flag resolves to ``VIEWER`` rather than the lowest
144+
role, keeping the read access a non-admin Casdoor user has today; a flag
145+
that is present keeps the boolean semantics it had as a model field,
146+
including its rejection of a non-boolean.
147+
148+
Both wire spellings are read because ``model_config`` accepts either:
149+
Casdoor's API sends ``isAdmin``, SEP's own fixtures ``is_admin``.
150+
151+
:param data: The raw model input.
152+
:return: The input with a ``role`` filled in, or ``data`` unchanged.
153+
:raises ValueError: If the admin flag is not a value Pydantic accepts as
154+
a boolean.
155+
"""
156+
if not isinstance(data, dict) or "role" in data:
157+
return data
158+
for key in ("isAdmin", "is_admin"):
159+
if key in data:
160+
return {**data, "role": cls._role_from_admin_flag(data[key])}
161+
return {**data, "role": UserRole.VIEWER}
162+
129163
@computed_field
130164
@property
131165
def is_active(self) -> bool:

0 commit comments

Comments
 (0)