From e2a3f0b6fa24e844e55d2ddcbf42e59557824a13 Mon Sep 17 00:00:00 2001 From: ducklet Date: Tue, 3 Aug 2021 17:05:25 +0200 Subject: [PATCH] add per user group management Drop the secret from groups, instead set per user access rights to read or write group information. --- unwind/models.py | 7 +- .../sql/20210802-212312--add-group-admins.sql | 45 ++++++++++++ unwind/web.py | 68 +++++++++++++------ 3 files changed, 98 insertions(+), 22 deletions(-) create mode 100644 unwind/sql/20210802-212312--add-group-admins.sql diff --git a/unwind/models.py b/unwind/models.py index ec7f1fb..185b6d5 100644 --- a/unwind/models.py +++ b/unwind/models.py @@ -6,6 +6,7 @@ from typing import ( Annotated, Any, ClassVar, + Literal, Optional, Type, TypeVar, @@ -283,6 +284,11 @@ class User: imdb_id: str = None name: str = None # canonical user name secret: str = None + groups: list[dict[str, str]] = field(default_factory=list) + + def has_access(self, group_id: Union[ULID, str], access: Literal["r", "w"] = "r"): + group_id = group_id if isinstance(group_id, str) else str(group_id) + return any(g["id"] == group_id and access in g["access"] for g in self.groups) @dataclass @@ -291,5 +297,4 @@ class Group: id: ULID = field(default_factory=ULID) name: str = None - secret: str = None users: list[dict[str, str]] = field(default_factory=list) diff --git a/unwind/sql/20210802-212312--add-group-admins.sql b/unwind/sql/20210802-212312--add-group-admins.sql new file mode 100644 index 0000000..13f3105 --- /dev/null +++ b/unwind/sql/20210802-212312--add-group-admins.sql @@ -0,0 +1,45 @@ +-- add group admins + +--- remove secrets from groups +CREATE TABLE _migrate_groups ( + id TEXT PRIMARY KEY NOT NULL, + name TEXT NOT NULL, + users TEXT NOT NULL -- JSON array +);; + +INSERT INTO _migrate_groups +SELECT + id, + name, + users +FROM groups +WHERE true;; + +DROP TABLE groups;; + +ALTER TABLE _migrate_groups +RENAME TO groups;; + +--- add group access to users +CREATE TABLE _migrate_users ( + id TEXT PRIMARY KEY NOT NULL, + imdb_id TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + secret TEXT NOT NULL, + groups TEXT NOT NULL -- JSON array +);; + +INSERT INTO _migrate_users +SELECT + id, + imdb_id, + name, + secret, + '[]' AS groups +FROM users +WHERE true;; + +DROP TABLE users;; + +ALTER TABLE _migrate_users +RENAME TO users;; diff --git a/unwind/web.py b/unwind/web.py index 6f4a054..473654c 100644 --- a/unwind/web.py +++ b/unwind/web.py @@ -37,8 +37,8 @@ log = logging.getLogger(__name__) class AuthedUser(BaseUser): - def __init__(self, username: str, secret: str): - self.username = username + def __init__(self, user_id: str, secret: str): + self.user_id = user_id self.secret = secret @@ -154,10 +154,23 @@ async def json_from_body(request, keys: list[str] = None): raise HTTPException(422, f"Missing data for key: {err.args[0]}") -def auth(request, secret: str = None): - is_admin = "admin" in request.auth.scopes - is_owner = secret and phc_compare(secret=request.user.secret, phc_string=secret) - return is_admin, bool(is_owner) +def is_admin(request): + return "admin" in request.auth.scopes + + +async def auth_user(request) -> Optional[User]: + if not isinstance(request.user, AuthedUser): + return + + user = await db.get(User, id=request.user.user_id) + if not user: + return + + is_authed = phc_compare(secret=request.user.secret, phc_string=user.secret) + if not is_authed: + return + + return user _routes = [] @@ -376,21 +389,32 @@ async def modify_user(request): user_id = as_ulid(request.path_params["user_id"]) - user = await db.get(User, id=str(user_id)) + if is_admin(request): + user = await db.get(User, id=str(user_id)) + + else: + user = await auth_user(request) + if not user: return not_found() - is_admin, is_owner = auth(request, user.secret) - if not (is_admin or is_owner): + is_allowed = user.id == user_id + if not is_allowed: return forbidden() data = await json_from_body(request) - if is_admin and "name" in data: + if "name" in data: + if not is_admin(request): + return forbidden("Changing user name is not allowed.") + # XXX restrict name user.name = data["name"] - if is_admin and "imdb_id" in data: + if "imdb_id" in data: + if not is_admin(request): + return forbidden("Changing IMDb ID is not allowed.") + # XXX check if imdb_id is well-formed user.imdb_id = data["imdb_id"] @@ -447,29 +471,31 @@ async def add_group(request): # XXX restrict name - secret = secrets.token_bytes() - - group = Group(name=name, secret=phc_scrypt(secret)) + group = Group(name=name) await db.add(group) - return JSONResponse( - { - "secret": b64encode(secret), - "group": asplain(group), - } - ) + return JSONResponse(asplain(group)) @route("/groups/{group_id}/users", methods=["POST"]) @requires(["authenticated"]) async def add_user_to_group(request): + group_id = as_ulid(request.path_params["group_id"]) group = await db.get(Group, id=str(group_id)) if not group: return not_found() - is_allowed = any(auth(request, group.secret)) + is_allowed = is_admin(request) + + if not is_allowed: + user = await auth_user(request) + if not user: + return not_found("User not found.") + + is_allowed = user.has_access(group_id, "w") + if not is_allowed: return forbidden()