fix: proper testing of modify permissions

backend/repos/user.py

@@ -56,14 +56,17 @@ def get_subscriptions(self, user: User, team: Team):
  ).filter(TeamTopic.team_id == team.id)
  )
 
- def has_subscribed_to_topic_in_team(self, user: User, team_topic: TeamTopic):
+ def has_subscribed_to_topic_in_team(
+ self, user: User, team_topic: TeamTopic
+ ) -> UserTeamTopic:
  return self._db.scalar(
  select(
+ UserTeamTopic,
  exists().where(
  and_(
  UserTeamTopic.id == team_topic.id,
  UserTeamTopic.user_id == user.id,
  )
- )
+ ),
  )
  )

backend/routes/v1/__init__.py

@@ -8,10 +8,13 @@
 from ... import exceptions
 from ...database import Session, get_session
 from ...models.document import Document
+from ...models.permissions import Permissions
 from ...models.user import User
 from ...repos.access_token import AccessToken, AccessTokenRepo
 from ...repos.document import DocumentRepo
 from ...repos.user import UserRepo
+from ...repos.user_team import UserTeamRepo
+from ...utils.document import Shareables
 
 router = APIRouter(prefix="/v1")
 
@@ -46,7 +49,7 @@ async def get_optional_access_token(
  pass
  # We need this optional autentication to be able to share documents
  # without requiring a login
- return
+ return None
 
 
 async def require_authenticated_user(
@@ -74,23 +77,48 @@ async def get_document(id: UUID, db: Session = Depends(get_session)) -> Document
 
 
 async def get_user_shared_document(
+ id: UUID,
+ permission: Permissions,
+ db: Session,
+ user: User,
+ document: Document,
+) -> Document:
+ # Lets go through the team/topics and check perms individually
+ UserRepo(db)
+ if user and document.user_id == user.id:
+ return document
+ user_team_repo = UserTeamRepo(db)
+ for team_topic in document.team_topics:
+ membership = user_team_repo.get_by_kwargs(
+ user_id=user.id, team_id=team_topic.team_id
+ )
+ if team_topic.team.can(permission, user, membership):
+ return document
+ raise exceptions.NotAllowed("You are not allowed to do what you are trying to do!")
+
+
+async def get_user_shared_document_for_read(
  id: UUID,
  db: Session = Depends(get_session),
  user: User = Depends(optional_authenticated_user),
  document: Document = Depends(get_document),
 ) -> Document:
+ # Everyone can read public documents
  if document.is_public:
  return document
- user_repo = UserRepo(db)
- for team_topic in document.team_topics:
- # TODO: invetigate if we can avoid the loop by a more powerful query
- if user_repo.has_subscribed_to_topic_in_team(user, team_topic):
- return document
- if not user or document.user_id != user.id:
- raise exceptions.NotAllowed(
- "Updating someone else document is not allowed currently!"
- )
- return document
+
+ return await get_user_shared_document(id, Permissions.can_read, db, user, document)
+
+
+async def get_user_shared_document_for_modify(
+ id: UUID,
+ db: Session = Depends(get_session),
+ user: User = Depends(optional_authenticated_user),
+ document: Document = Depends(get_document),
+) -> Document:
+ return await get_user_shared_document(
+ id, Permissions.can_modify, db, user, document
+ )
 
 
 async def get_user_owned_document(
@@ -101,6 +129,19 @@ async def get_user_owned_document(
 ) -> Document:
  if document.user_id != user.id:
  raise exceptions.NotAllowed(
- "Updating someone else document is not allowed currently!"
+ "Deleting someone else's document is not allowed currently!"
  )
  return document
+
+
+def check_document_post_permissions(db: Session, user: User, shareables: Shareables):
+ user_repo = UserRepo(db)
+ for team_topic in shareables.team_topics:
+ team = team_topic.team
+ membership = user_repo.is_member(user, team_topic.team)
+
+ if team.can(Permissions.can_post, user, membership):
+ return
+ # If we have team_topics and we get to this point, we need to raise
+ if shareables.team_topics:
+ raise exceptions.NotAllowed("You are not allowed!")

backend/routes/v1/docs.py

@@ -6,8 +6,8 @@
 import frontmatter
 from fastapi import Depends, Request, Security
 from fastapi.responses import PlainTextResponse
-from backend.repos.team_topic import TeamTopicRepo
 
+from backend.repos.team_topic import TeamTopicRepo
 from backend.repos.user_team_topic import UserTeamTopicRepo
 
 from ... import __version__, exceptions
@@ -17,27 +17,23 @@
 from ...repos.document import DocumentRepo
 from ...repos.document_access import DocumentAccessRepo
 from ...repos.document_body import DocumentBodyRepo
-from ...repos.user import UserRepo
 from ...schema import (
  DocumentFrontMatter,
  DocumentIdentifierResponse,
  DocumentLibraryResponse,
  DocumentResponse,
  DocumentShareType,
  Response,
+ SuccessResponse,
  VersionResponse,
 )
-from ...utils.document import (
- check_document_modify_permissions,
- check_document_post_permissions,
- check_document_read_permissions,
- get_shareables,
- get_title_from_body,
-)
+from ...utils.document import get_shareables, get_title_from_body
 from . import (
+ check_document_post_permissions,
  get_access_token,
  get_user_owned_document,
- get_user_shared_document,
+ get_user_shared_document_for_modify,
+ get_user_shared_document_for_read,
  optional_authenticated_user,
  require_authenticated_user,
  router,
@@ -135,12 +131,10 @@ async def post_doc(
 )
 async def get_doc(
  request: Request,
- document: Document = Depends(get_user_shared_document),
+ document: Document = Depends(get_user_shared_document_for_read),
  db: Session = Depends(get_session),
  user: User = Depends(optional_authenticated_user),
 ):
- if not document.is_public:
- check_document_read_permissions(db, user, document.team_topics)
  document_body_repo = DocumentBodyRepo()
  document_access_repo = DocumentAccessRepo(db)
 
@@ -160,7 +154,9 @@ async def get_doc(
  filesize=document.filesize,
  embeds=document.embeds,
  )
- return Response(result=ret_document).model_dump(by_alias=True)
+ return Response(result=ret_document).model_dump(
+ by_alias=True, exclude_none=True
+ )
  elif content_type == "text/markdown":
  # Add document id to frontmatter
  body = document_body_repo.get_by_id(document.id)
@@ -176,16 +172,21 @@ async def get_doc(
  raise exceptions.BadRequest(f"Unsupported content-type: {content_type}")
 
 
-@router.put("/doc/{id}", tags=["v1"])
+@router.put(
+ "/doc/{id}",
+ tags=["v1"],
+ response_model=Response[DocumentResponse],
+ response_model_exclude_unset=True,
+ response_model_by_alias=True,
+)
 async def put_doc(
  request: Request,
- document: Document = Depends(get_user_owned_document),
+ document: Document = Depends(get_user_shared_document_for_modify),
  user: User = Depends(require_authenticated_user),
  db: Session = Depends(get_session),
 ):
  document_repo = DocumentRepo(db)
  document_body_repo = DocumentBodyRepo()
- UserRepo(db)
 
  body_raw = await request.body()
  body = body_raw.decode("utf-8")
@@ -200,7 +201,6 @@ async def put_doc(
 
  # Parse relay_to as team_topics
  shareables = get_shareables(db, front, user)
- check_document_modify_permissions(db, user, shareables)
 
  # Checksum
  hashing_obj = hashlib.sha256()
@@ -233,6 +233,16 @@ async def put_doc(
  return dict(result=ret_document)
 
 
+@router.delete("/doc/{id}", tags=["v1"], response_model=Response[SuccessResponse])
+async def delete_doc(
+ document: Document = Depends(get_user_owned_document),
+ db: Session = Depends(get_session),
+):
+ document_repo = DocumentRepo(db)
+ document_repo.delete(document)
+ return dict(result=dict(success=True))
+
+
 @router.get(
  "/docs",
  tags=["v1"],

backend/utils/document.py

@@ -1,12 +1,10 @@
 # -*- coding: utf-8 -*-
 import re
 from collections import namedtuple
-from typing import List
 
 from .. import exceptions
 from ..database import Session
 from ..models.permissions import Permissions
-from ..models.team_topic import TeamTopic
 from ..models.user import User
 from ..repos.team_topic import TeamTopicRepo
 from ..repos.user import UserRepo
@@ -36,44 +34,6 @@ def line_allowed_for_headline(line: str) -> bool:
  return "unknown"
 
 
-def check_document_read_permissions(
- db: Session, user: User, team_topics: List[TeamTopic]
-):
- user_repo = UserRepo(db)
- for team_topic in team_topics:
- team = team_topic.team
- membership = user_repo.is_member(user, team_topic.team)
-
- if not team.can(Permissions.can_post, user, membership):
- raise exceptions.NotAllowed(
- f"You are not allowed to read from {team_topic.team}!"
- )
-
-
-def check_document_post_permissions(db: Session, user: User, shareables: Shareables):
- user_repo = UserRepo(db)
- for team_topic in shareables.team_topics:
- team = team_topic.team
- membership = user_repo.is_member(user, team_topic.team)
-
- if not team.can(Permissions.can_post, user, membership):
- raise exceptions.NotAllowed(
- f"You are not allowed to post to {team_topic.team}!"
- )
-
-
-def check_document_modify_permissions(db: Session, user: User, shareables: Shareables):
- user_repo = UserRepo(db)
- for team_topic in shareables.team_topics:
- team = team_topic.team
- membership = user_repo.is_member(user, team_topic.team)
-
- if not team.can(Permissions.can_modify, user, membership):
- raise exceptions.NotAllowed(
- f"You are not allowed to modify posts in {team_topic.team}!"
- )
-
-
 def get_shareables(db: Session, front: DocumentFrontMatter, user: User) -> Shareables:
  """Is used on POST and PUT.
  Converts string version of relay-to into team_topic instances or user

tests/conftest.py

@@ -19,11 +19,6 @@
 # noqa
 from backend import database, models, repos # noqa
 from backend.api import app as api_app
-from backend.repos.document import DocumentRepo
-from backend.repos.team import TeamRepo
-from backend.repos.team_topic import TeamTopicRepo
-from backend.repos.user import UserRepo
-from backend.repos.user_team_topic import UserTeamTopicRepo
 from backend.web import app as web_app
 
 
@@ -183,27 +178,32 @@ def default_team_topics(dbsession, account):
 
 @pytest.fixture()
 def document_repo(dbsession):
- return DocumentRepo(dbsession)
+ return repos.DocumentRepo(dbsession)
 
 
 @pytest.fixture()
 def user_repo(dbsession):
- return UserRepo(dbsession)
+ return repos.UserRepo(dbsession)
 
 
 @pytest.fixture()
 def team_repo(dbsession):
- return TeamRepo(dbsession)
+ return repos.TeamRepo(dbsession)
 
 
 @pytest.fixture()
 def team_topic_repo(dbsession):
- return TeamTopicRepo(dbsession)
+ return repos.TeamTopicRepo(dbsession)
 
 
 @pytest.fixture
 def user_team_topic_repo(dbsession):
- return UserTeamTopicRepo(dbsession)
+ return repos.UserTeamTopicRepo(dbsession)
+
+
+@pytest.fixture
+def user_team_repo(dbsession):
+ return repos.UserTeamRepo(dbsession)
 
 
 @pytest.fixture
@@ -246,7 +246,9 @@ def func(team_topic: str, acc=account):
 @pytest.fixture
 def create_team(team_repo, account):
  def func(name: str, **kwargs):
- return team_repo.create_from_kwargs(name=name, user_id=account.id, **kwargs)
+ if "user_id" not in kwargs:
+ kwargs["user_id"] = account.id
+ return team_repo.create_from_kwargs(name=name, **kwargs)
 
  return func
 
@@ -259,6 +261,16 @@ def func(name: str, from_account=account):
  return func
 
 
+@pytest.fixture
+def join_team(user_team_repo):
+ def func(team: models.Team, user: models.User, **kwargs):
+ return user_team_repo.create_from_kwargs(
+ team_id=team.id, user_id=user.id, **kwargs
+ )
+
+ return func
+
+
 @pytest.fixture
 def upload_document(api_client):
  def func(content: str, headers: dict, relay_to: Optional[str] = None):