diff --git a/server/mergin/app.py b/server/mergin/app.py index 629037ea..d025b6e2 100644 --- a/server/mergin/app.py +++ b/server/mergin/app.py @@ -180,7 +180,6 @@ def create_app(public_keys: List[str] = None) -> Flask: arguments={"title": "Mergin"}, options={"swagger_ui": Configuration.SWAGGER_UI}, validate_responses=True, - pythonic_params=True, ) app.add_api( "sync/private_api.yaml", diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index 4cee2238..39c92946 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -10,6 +10,8 @@ tags: description: Mergin user - name: admin description: For mergin admin + - name: public + description: Public API paths: /app/auth/user/search: get: @@ -642,6 +644,57 @@ paths: $ref: "#/components/responses/UnauthorizedError" "403": $ref: "#/components/responses/Forbidden" + /v2/users: + post: + tags: + - user + - public + summary: Create user + operationId: create_user + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - email + - password + - workspace_id + - role + properties: + email: + type: string + format: email + example: john.doe@example.com + username: + type: string + example: john.doe + password: + type: string + format: password + example: topsecret + workspace_id: + type: integer + example: 1 + role: + $ref: "#/components/schemas/WorkspaceRole" + notify_user: + type: boolean + responses: + "201": + description: User info + content: + application/json: + schema: + $ref: "#/components/schemas/UserInfo" + "401": + $ref: "#/components/responses/UnauthorizedError" + "404": + $ref: "#/components/responses/NotFoundResp" + "422": + $ref: "#/components/responses/UnprocessableEntity" + x-openapi-router-controller: mergin.auth.controller components: responses: UnauthorizedError: diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index 1a3caba4..cfba8d98 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -12,7 +12,6 @@ from .commands import add_commands from .config import Configuration from .models import User, UserProfile -from ..app import db # signal for other versions to listen to user_account_closed = signal("user_account_closed") @@ -97,12 +96,18 @@ def confirm_token(token, expiration=3600 * 24 * 3): return email -def send_confirmation_email(app, user, url, template, header): +def send_confirmation_email(app, user, url, template, header, **kwargs): + """ + Send confirmation email from selected template with customizable email subject and confirmation URL. + Optional kwargs are passed to render_template method if needed for particular template. + """ from ..celery import send_email_async token = generate_confirmation_token(app, user.email) confirm_url = f"{url}/{token}" - html = render_template(template, subject=header, confirm_url=confirm_url, user=user) + html = render_template( + template, subject=header, confirm_url=confirm_url, user=user, **kwargs + ) email_data = { "subject": header, "html": html, @@ -110,11 +115,3 @@ def send_confirmation_email(app, user, url, template, header): "sender": app.config["MAIL_DEFAULT_SENDER"], } send_email_async.delay(**email_data) - - -def do_register_user(username, email, password): - user = User(username.strip(), email.strip(), password, False) - user.profile = UserProfile() - db.session.add(user) - db.session.commit() - return user diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 42612709..6e0e9dd5 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -15,7 +15,6 @@ from .app import ( auth_required, authenticate, - do_register_user, send_confirmation_email, confirm_token, generate_confirmation_token, @@ -92,7 +91,7 @@ def get_user_public(username=None): # noqa: E501 "storage": user_workspace.storage if user_workspace else 104857600, "storage_limit": user_workspace.storage if user_workspace else 104857600, "organisations": { - ws.name: ws.get_user_role(current_user) for ws in all_workspaces + ws.name: ws.get_user_role(current_user).value for ws in all_workspaces }, } return user_info, 200 @@ -374,7 +373,7 @@ def register_user(): # pylint: disable=W0613,W0612 form = UserRegistrationForm() if form.validate(): - user = do_register_user(form.username.data, form.email.data, form.password.data) + user = User.create(form.username.data, form.email.data, form.password.data) user_created.send(user, source="admin") token = generate_confirmation_token(current_app, user.email) confirm_url = f"confirm-email/{token}" @@ -486,7 +485,7 @@ def get_user_info(): user_info = UserInfoSchema().dump(current_user) workspaces = current_app.ws_handler.list_user_workspaces(current_user.username) user_info["workspaces"] = [ - {"id": ws.id, "name": ws.name, "role": ws.get_user_role(current_user)} + {"id": ws.id, "name": ws.name, "role": ws.get_user_role(current_user).value} for ws in workspaces ] preferred_workspace = current_app.ws_handler.get_preferred(current_user) @@ -501,6 +500,48 @@ def get_user_info(): return user_info, 200 +@auth_required +def create_user(): + """Create new user""" + workspace = current_app.ws_handler.get(request.json.get("workspace_id")) + if not (workspace and workspace.can_add_users(current_user)): + abort(403) + + username = request.json.get( + "username", User.generate_username(request.json["email"]) + ) + form = UserRegistrationForm() + form.confirm.data = form.password.data + form.username.data = username + if not form.validate(): + return jsonify(form.errors), 400 + + user = User.create( + form.username.data, + form.email.data, + form.password.data, + request.json.get("notify_user", False), + ) + user_created.send( + user, + source="api", + workspace_id=request.json["workspace_id"], + workspace_role=request.json["role"], + ) + + if user.profile.receive_notifications: + send_confirmation_email( + current_app, + user, + "confirm-email", + "email/user_created.html", + "Invitation to Mergin Maps", + password=form.password.data, + ) + + return jsonify(UserInfoSchema().dump(user)), 201 + + @auth_required(permissions=["admin"]) def get_server_usage(): data = { diff --git a/server/mergin/auth/models.py b/server/mergin/auth/models.py index 58c706ad..c16c21f6 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -7,9 +7,10 @@ from typing import List, Optional import bcrypt from flask import current_app, request -from sqlalchemy import or_, func +from sqlalchemy import or_, func, text from ..app import db +from ..sync.models import ProjectUser from ..sync.utils import get_user_agent, get_ip, get_device_id @@ -149,24 +150,10 @@ def inactivate(self) -> None: """Inactivate user account and remove explicitly shared projects as well clean references to created projects. User is then safe to be removed. """ - from ..sync.models import Project, ProjectAccess, AccessRequest, RequestStatus + from ..sync.models import AccessRequest, RequestStatus - shared_projects = Project.query.filter( - or_( - Project.access.has(ProjectAccess.owners.contains([self.id])), - Project.access.has(ProjectAccess.writers.contains([self.id])), - Project.access.has(ProjectAccess.editors.contains([self.id])), - Project.access.has(ProjectAccess.readers.contains([self.id])), - ) - ).all() - - for p in shared_projects: - for key in ("owners", "writers", "editors", "readers"): - value = set(getattr(p.access, key)) - if self.id in value: - value.remove(self.id) - setattr(p.access, key, list(value)) - db.session.add(p) + # remove explicit permissions + ProjectUser.query.filter(ProjectUser.user_id == self.id).delete() # decline all access requests for req in ( @@ -192,6 +179,51 @@ def anonymize(self): self.profile.last_name = None db.session.commit() + @classmethod + def get_by_login(cls, login: str) -> Optional[User]: + """Find user by its login which can be either username or email""" + login = login.strip().lower() + return cls.query.filter( + or_( + func.lower(User.email) == login, + func.lower(User.username) == login, + ) + ).first() + + @classmethod + def generate_username(cls, email: str) -> Optional[str]: + """Autogenerate username from email""" + if not "@" in email: + return + username = email.split("@")[0].strip().lower() + # check if we already do not have existing usernames + suffix = db.session.execute( + text( + """ + SELECT + replace(username, :username, '0')::int AS suffix + FROM "user" + WHERE + username = :username OR + username SIMILAR TO :username'\d+' + ORDER BY replace(username, :username, '0')::int DESC + LIMIT 1; + """ + ), + {"username": username}, + ).scalar() + return username if suffix is None else username + str(int(suffix) + 1) + + @classmethod + def create( + cls, username: str, email: str, password: str, notifications: bool = True + ) -> User: + user = cls(username.strip(), email.strip(), password, False) + user.profile = UserProfile(receive_notifications=notifications) + db.session.add(user) + db.session.commit() + return user + class UserProfile(db.Model): user_id = db.Column( diff --git a/server/mergin/auth/schemas.py b/server/mergin/auth/schemas.py index ea0bcd71..62d0bd1e 100644 --- a/server/mergin/auth/schemas.py +++ b/server/mergin/auth/schemas.py @@ -32,17 +32,16 @@ def get_disk_usage(self, obj): def _has_project(self, obj): # DEPRECATED functionality - kept for the backward-compatibility - from ..sync.models import Project, ProjectAccess + from ..sync.models import ProjectUser, Project ws = current_app.ws_handler.get_by_name(obj.user.username) if ws: projects_count = ( - Project.query.filter(Project.creator_id == obj.user.id) + Project.query.join(ProjectUser) + .filter(Project.creator_id == obj.user.id) .filter(Project.removed_at.is_(None)) - .filter_by(workspace_id=ws.id) - .filter( - Project.access.has(ProjectAccess.readers.contains([obj.user.id])) - ) + .filter(Project.workspace_id == ws.id) + .filter(ProjectUser.user_id == obj.user.id) .count() ) return projects_count > 0 diff --git a/server/mergin/auth/tasks.py b/server/mergin/auth/tasks.py index a09848a8..e18cc5d4 100644 --- a/server/mergin/auth/tasks.py +++ b/server/mergin/auth/tasks.py @@ -6,7 +6,7 @@ from sqlalchemy.sql.operators import isnot from ..celery import celery -from .app import db +from ..app import db from .models import User from .config import Configuration diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index a71be8fe..97e85981 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -11,7 +11,7 @@ from .files import UploadChanges from ..app import db -from .models import Project, ProjectAccess, ProjectVersion +from .models import Project, ProjectVersion from .utils import split_project_path from ..auth.models import User @@ -52,7 +52,6 @@ def create(name, namespace, username): # pylint: disable=W0612 p = Project(**project_params) p.updated = datetime.utcnow() db.session.add(p) - p.access = ProjectAccess(p, public=False) changes = UploadChanges(added=[], updated=[], removed=[]) pv = ProjectVersion(p, 0, user.id, changes, "127.0.0.1") pv.project = p diff --git a/server/mergin/sync/interfaces.py b/server/mergin/sync/interfaces.py index 4cf12650..3961ee37 100644 --- a/server/mergin/sync/interfaces.py +++ b/server/mergin/sync/interfaces.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial from abc import ABC, abstractmethod +from enum import Enum class AbstractWorkspace: @@ -74,6 +75,16 @@ def project_count(self): """Return number of workspace projects""" pass + @abstractmethod + def members(self): + """Return workspace members""" + pass + + @abstractmethod + def can_add_users(self, user): + """Check if user can add another user to workspace""" + pass + class WorkspaceHandler(ABC): """ @@ -169,3 +180,21 @@ def get_push_permission(self, changes: dict): Return project permission for user to push data to project """ pass + + +class WorkspaceRole(Enum): + GUEST = "guest" + READER = "reader" + EDITOR = "editor" + WRITER = "writer" + ADMIN = "admin" + OWNER = "owner" + + @classmethod + def values(cls): + return [member.value for member in cls.__members__.values()] + + def __ge__(self, other): + """Compare roles""" + members = list(WorkspaceRole.__members__) + return members.index(self.name) >= members.index(other.name) diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index cff88229..de81676a 100644 --- a/server/mergin/sync/models.py +++ b/server/mergin/sync/models.py @@ -28,6 +28,7 @@ ChangesSchema, ProjectFile, ) +from .interfaces import WorkspaceRole from .storages.disk import move_to_tmp from ..app import db from .storages import DiskStorage @@ -66,11 +67,16 @@ class Project(db.Model): removed_by = db.Column( db.Integer, db.ForeignKey("user.id"), nullable=True, index=True ) + public = db.Column(db.Boolean, default=False, index=True, nullable=False) creator = db.relationship( "User", uselist=False, backref=db.backref("projects"), foreign_keys=[creator_id] ) + project_users = db.relationship( + "ProjectUser", cascade="all, delete-orphan", back_populates="project" + ) + __table_args__ = (db.UniqueConstraint("name", "workspace_id"),) def __init__( @@ -81,8 +87,10 @@ def __init__( self.workspace_id = workspace.id self.creator = creator self.latest_version = 0 + self.public = kwargs.get("public", False) latest_files = LatestProjectFiles(project=self) db.session.add(latest_files) + self.set_role(creator.id, ProjectRole.OWNER) @property def storage(self): @@ -238,9 +246,7 @@ def delete(self, removed_by: int = None): db.session.execute( upload_table.delete().where(upload_table.c.project_id == self.id) ) - self.access.owners = self.access.writers = self.access.editors = ( - self.access.readers - ) = [] + self.project_users.clear() access_requests = ( AccessRequest.query.filter_by(project_id=self.id) .filter(AccessRequest.status.is_(None)) @@ -251,24 +257,99 @@ def delete(self, removed_by: int = None): db.session.commit() project_deleted.send(self) + def _member(self, user_id: int) -> Optional[ProjectUser]: + """Return association object for user_id""" + return next((u for u in self.project_users if u.user_id == user_id), None) + + def get_role(self, user_id: int) -> Optional[ProjectRole]: + """Get user role""" + member = self._member(user_id) + if member: + return ProjectRole(member.role) + + def set_role(self, user_id: int, role: ProjectRole) -> None: + """Set user role""" + member = self._member(user_id) + if member: + member.role = role.value + else: + self.project_users.append(ProjectUser(user_id=user_id, role=role.value)) + + def unset_role(self, user_id: int) -> None: + """Remove user's role""" + member = self._member(user_id) + if member: + self.project_users.remove(member) + + def get_member(self, user_id: int) -> Optional[ProjectMember]: + """Get project member""" + member = self._member(user_id) + if member: + return ProjectMember( + id=user_id, + username=member.user.username, + email=member.user.email, + project_role=ProjectRole(member.role), + workspace_role=self.workspace.get_user_role(member.user), + ) + + def members_by_role(self, role: ProjectRole) -> List[int]: + """Project members' ids with at least required role (or higher)""" + return [u.user_id for u in self.project_users if ProjectRole(u.role) >= role] + + def bulk_roles_update(self, access: Dict) -> Set[int]: + """Update roles from access lists and return users ids of those affected by any action""" + id_diffs = [] + for role in list(ProjectRole.__reversed__()): + # we might not want to modify all roles + if role not in access: + continue + + for user_id in access.get(role): + if self.get_role(user_id) != role: + self.set_role(user_id, role) + id_diffs.append(user_id) + + # make sure we do not have other user ids than in the list at this role + for user in self.project_users: + if ProjectRole(user.role) == role and user.user_id not in access.get( + role + ): + self.unset_role(user.user_id) + id_diffs.append(user.user_id) + + return set(id_diffs) + class ProjectRole(Enum): - OWNER = "owner" - WRITER = "writer" - EDITOR = "editor" + """Project roles ordered by rank (do not change)""" + READER = "reader" + EDITOR = "editor" + WRITER = "writer" + OWNER = "owner" + + def __ge__(self, other): + """Compare project roles""" + members = list(ProjectRole.__members__) + return members.index(self.name) >= members.index(other.name) def __gt__(self, other): - """ - Compare project roles + members = list(ProjectRole.__members__) + return members.index(self.name) > members.index(other.name) - https://docs.python.org/3/library/enum.html#enum.EnumType.__members__ - """ + def __lt__(self, other): members = list(ProjectRole.__members__) - if members.index(self.name) < members.index(other.name): - return True - else: - return False + return members.index(self.name) < members.index(other.name) + + +@dataclass +class ProjectMember: + id: int + email: str + username: str + workspace_role: WorkspaceRole + project_role: Optional[ProjectRole] @dataclass @@ -282,113 +363,6 @@ class ProjectAccessDetail: type: str -class ProjectAccess(db.Model): - project_id = db.Column( - UUID(as_uuid=True), - db.ForeignKey("project.id", ondelete="CASCADE"), - primary_key=True, - index=True, - ) - public = db.Column(db.Boolean, default=False, index=True) - owners = db.Column(ARRAY(db.Integer), server_default="{}") - readers = db.Column(ARRAY(db.Integer), server_default="{}") - writers = db.Column(ARRAY(db.Integer), server_default="{}") - editors = db.Column(ARRAY(db.Integer), server_default="{}") - - project = db.relationship( - "Project", - uselist=False, - backref=db.backref( - "access", - single_parent=True, - uselist=False, - cascade="all,delete", - lazy="joined", - ), - ) - - __table_args__ = ( - db.Index("ix_project_access_owners", owners, postgresql_using="gin"), - db.Index("ix_project_access_readers", readers, postgresql_using="gin"), - db.Index("ix_project_access_writers", writers, postgresql_using="gin"), - db.Index("ix_project_access_editors", editors, postgresql_using="gin"), - ) - - def __init__(self, project, public=False): - self.project = project - self.owners = [project.creator.id] - self.writers = [project.creator.id] - self.readers = [project.creator.id] - self.editors = [project.creator.id] - self.project_id = project.id - self.public = public - - def get_role(self, user_id: int) -> Optional[ProjectRole]: - """Get user role based on mapping to DB ACL""" - if user_id in self.owners: - return ProjectRole.OWNER - elif user_id in self.writers: - return ProjectRole.WRITER - elif user_id in self.editors: - return ProjectRole.EDITOR - elif user_id in self.readers: - return ProjectRole.READER - else: - return None - - @staticmethod - def _permission_attrs(role: ProjectRole) -> List[str]: - """Return db attributes list related to permission""" - # because roles do not inherit, they must be un/set explicitly in db ACLs - perm_list = { - ProjectRole.READER: ["readers"], - ProjectRole.EDITOR: ["editors", "readers"], - ProjectRole.WRITER: ["writers", "editors", "readers"], - ProjectRole.OWNER: ["owners", "writers", "editors", "readers"], - } - return perm_list[role] - - def set_role(self, user_id: int, role: ProjectRole) -> None: - """Set user role""" - self.unset_role(user_id) - for attr in self._permission_attrs(role): - ids = getattr(self, attr) - if user_id not in ids: - ids.append(user_id) - setattr(self, attr, ids) - flag_modified(self, attr) - - def unset_role(self, user_id: int) -> None: - """Remove user's role""" - role = self.get_role(user_id) - if not role: - return - - for attr in self._permission_attrs(role): - ids = getattr(self, attr) - if user_id in ids: - ids.remove(user_id) - setattr(self, attr, ids) - flag_modified(self, attr) - - def bulk_update(self, new_access: Dict) -> Set[int]: - """From new access lists do bulk update and return ids with any change applied""" - diff = set() - for key in ("owners", "writers", "editors", "readers"): - new_value = new_access.get(key, None) - if not new_value: - continue - old_value = set(getattr(self, key)) - diff = diff.union(set(new_value).symmetric_difference(old_value)) - setattr(self, key, list(new_value)) - - # make sure lists are consistent (they inherit from each other) - self.writers = list(set(self.writers).union(set(self.owners))) - self.editors = list(set(self.editors).union(set(self.writers))) - self.readers = list(set(self.readers).union(set(self.editors))) - return diff - - class ProjectFilePath(db.Model): """Files (paths) within Project""" @@ -1110,7 +1084,6 @@ def expire(self): def accept(self, permissions): """Accept project access request""" - project_access = self.project.access PERMISSION_PROJECT_ROLE = { "read": ProjectRole.READER, "edit": ProjectRole.EDITOR, @@ -1118,7 +1091,7 @@ def accept(self, permissions): "owner": ProjectRole.OWNER, } - project_access.set_role( + self.project.set_role( self.requested_by, PERMISSION_PROJECT_ROLE.get(permissions) ) self.resolve(RequestStatus.ACCEPTED, current_user.id) @@ -1203,3 +1176,29 @@ def __init__( if os.path.exists(diff_path): self.diff_size = os.path.getsize(diff_path) self.changes = GeoDiff().changes_count(diff_path) + + +class ProjectUser(db.Model): + """Association table for project membership""" + + __tablename__ = "project_member" + + project_id = db.Column( + UUID(as_uuid=True), + db.ForeignKey("project.id", ondelete="CASCADE"), + primary_key=True, + ) + user_id = db.Column( + db.Integer, + db.ForeignKey("user.id", ondelete="CASCADE"), + primary_key=True, + ) + role = db.Column( + ENUM( + *[member.value for member in ProjectRole.__members__.values()], + name="project_role", + ), + nullable=False, + ) + project = db.relationship("Project", back_populates="project_users") + user = db.relationship("User") diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 9ec1f981..4e3901d5 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -10,8 +10,9 @@ from sqlalchemy import or_ from .utils import is_valid_uuid +from ..app import db from ..auth.models import User -from .models import ProjectAccess, Project, Upload, ProjectRole +from .models import Project, Upload, ProjectRole, ProjectUser def _is_superuser(f): @@ -48,11 +49,12 @@ class Read(Base): @_is_superuser def check(cls, project, user): # public active projects can be access by anyone - if project.access.public and not project.removed_at: + if project.public and not project.removed_at: return True + project_role = project.get_role(user.id) if user.is_authenticated else None return super().check(project, user) and ( - (user.id in project.access.readers) + (project_role and project_role >= ProjectRole.READER) or (check_project_workspace_permissions(project, user, "read")) ) @@ -61,10 +63,8 @@ def query(cls, user, as_admin=True, public=True): if user.is_authenticated and user.is_admin and as_admin: return Project.query - query = ( - Project.query.join(ProjectAccess) - .filter(Project.storage_params.isnot(None)) - .filter(Project.removed_at.is_(None)) + query = Project.query.filter(Project.storage_params.isnot(None)).filter( + Project.removed_at.is_(None) ) if user.is_authenticated and user.active: all_workspaces = current_app.ws_handler.list_user_workspaces( @@ -75,23 +75,28 @@ def query(cls, user, as_admin=True, public=True): for ws in all_workspaces if ws.user_has_permissions(user, "read") ] + subquery = ( + db.session.query(ProjectUser.project_id) + .filter(ProjectUser.user_id == user.id) + .subquery() + ) if public: query = query.filter( or_( - ProjectAccess.public.is_(True), + Project.public.is_(True), Project.workspace_id.in_(user_workspace_ids), - ProjectAccess.readers.contains([user.id]), + Project.id.in_(subquery), ) ) else: query = query.filter( or_( Project.workspace_id.in_(user_workspace_ids), - ProjectAccess.readers.contains([user.id]), + Project.id.in_(subquery), ) ) else: - query = query.filter(ProjectAccess.public.is_(True)) + query = query.filter(Project.public.is_(True)) return query @@ -99,9 +104,10 @@ class Edit(Base): @classmethod @_is_superuser def check(self, project, user): + project_role = project.get_role(user.id) if user.is_authenticated else None return super().check(project, user) and ( ( - user.id in project.access.editors + (project_role and project_role >= ProjectRole.EDITOR) or check_project_workspace_permissions(project, user, "edit") ) ) @@ -110,9 +116,10 @@ class Upload(Base): @classmethod @_is_superuser def check(cls, project, user): + project_role = project.get_role(user.id) if user.is_authenticated else None return super().check(project, user) and ( ( - user.id in project.access.writers + (project_role and project_role >= ProjectRole.WRITER) or check_project_workspace_permissions(project, user, "write") ) ) @@ -121,9 +128,10 @@ class Update(Base): @classmethod @_is_superuser def check(cls, project, user): + project_role = project.get_role(user.id) if user.is_authenticated else None return super().check(project, user) and ( ( - user.id in project.access.owners + project_role is ProjectRole.OWNER or check_project_workspace_permissions(project, user, "admin") ) ) @@ -132,9 +140,10 @@ class Delete(Base): @classmethod @_is_superuser def check(cls, project, user): + project_role = project.get_role(user.id) if user.is_authenticated else None return super().check(project, user) and ( ( - user.id in project.access.owners + project_role is ProjectRole.OWNER or check_project_workspace_permissions(project, user, "admin") ) ) @@ -143,9 +152,10 @@ class All(Base): @classmethod @_is_superuser def check(cls, project, user): + project_role = project.get_role(user.id) if user.is_authenticated else None return super().check(project, user) and ( ( - user.id in project.access.owners + project_role is ProjectRole.OWNER or check_project_workspace_permissions(project, user, "admin") ) ) diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index 01c5af6e..616ac69d 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/server/mergin/sync/private_api_controller.py @@ -12,7 +12,14 @@ from ..auth import auth_required from ..auth.models import User, UserProfile from .forms import AccessPermissionForm -from .models import Project, AccessRequest, ProjectRole, RequestStatus, ProjectVersion +from .models import ( + Project, + AccessRequest, + ProjectRole, + RequestStatus, + ProjectVersion, + ProjectUser, +) from .schemas import ( ProjectListSchema, ProjectAccessRequestSchema, @@ -41,7 +48,7 @@ def create_project_access_request(namespace, project_name): # noqa: E501 Project.workspace_id == workspace.id, Project.removed_at.is_(None), ).first_or_404() - if current_user.id in project.access.readers: + if project.get_role(current_user.id): abort(409, "You already have access to project") access_request = ( @@ -59,9 +66,13 @@ def create_project_access_request(namespace, project_name): # noqa: E501 db.session.commit() # notify project owners owners = ( - User.query.join(UserProfile) - .filter(User.verified_email, User.id.in_(project.access.owners)) - .filter(UserProfile.receive_notifications) + User.query.join(UserProfile, ProjectUser) + .filter( + ProjectUser.project_id == project.id, + ProjectUser.role == ProjectRole.OWNER.value, + User.verified_email, + UserProfile.receive_notifications, + ) .all() ) for owner in owners: @@ -124,10 +135,10 @@ def accept_project_access_request(request_id): project = access_request.project project_role = ProjectPermissions.get_user_project_role(project, current_user) if project_role == ProjectRole.OWNER: + access_request.accept(permission) project_access_granted.send( access_request.project, user_id=access_request.requested_by ) - access_request.accept(permission) return "", 200 abort(403, "You don't have permissions to accept project access request") @@ -278,7 +289,7 @@ def unsubscribe_project(id): # pylint: disable=W0612 from ..celery import send_email_async project = require_project_by_uuid(id, ProjectPermissions.Read) - current_role = project.access.get_role(current_user.id) + current_role = project.get_role(current_user.id) if not current_role: return NoContent, 200 # nothing to do so request is idempotent @@ -286,7 +297,7 @@ def unsubscribe_project(id): # pylint: disable=W0612 if current_role == ProjectRole.OWNER: abort(400, "Owner cannot leave a project") - project.access.unset_role(current_user.id) + project.unset_role(current_user.id) db.session.add(project) db.session.commit() return NoContent, 200 @@ -301,7 +312,7 @@ def update_project_access(id: str): project = require_project_by_uuid(id, ProjectPermissions.Update) if "public" in request.json: - project.access.public = request.json["public"] + project.public = request.json["public"] if "user_id" in request.json and "role" in request.json: user = User.query.filter_by( @@ -309,12 +320,12 @@ def update_project_access(id: str): ).first_or_404("User does not exist") if request.json["role"] == "none": - project.access.unset_role(user.id) + project.unset_role(user.id) else: - project.access.set_role(user.id, ProjectRole(request.json["role"])) + project.set_role(user.id, ProjectRole(request.json["role"])) project_access_granted.send(project, user_id=user.id) db.session.commit() - return ProjectAccessSchema().dump(project.access), 200 + return ProjectAccessSchema().dump(project), 200 @auth_required diff --git a/server/mergin/sync/public_api.yaml b/server/mergin/sync/public_api.yaml index c066b400..48d39c13 100644 --- a/server/mergin/sync/public_api.yaml +++ b/server/mergin/sync/public_api.yaml @@ -232,6 +232,7 @@ paths: summary: Update an existing project description: Updates 'public' flag and access list for project operationId: update_project + deprecated: true requestBody: description: Data to be updated required: true diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 485bc173..77faeec2 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -25,23 +25,28 @@ ) from pygeodiff import GeoDiffLibError from flask_login import current_user -from sqlalchemy import and_, desc, asc, text +from sqlalchemy import and_, desc, asc, text, func, select from sqlalchemy.exc import IntegrityError from binaryornot.check import is_binary from gevent import sleep import base64 + +from sqlalchemy.orm import load_only from werkzeug.exceptions import HTTPException + +from .interfaces import WorkspaceRole from ..app import db from ..auth import auth_required from ..auth.models import User from .models import ( Project, - ProjectAccess, ProjectVersion, Upload, PushChangeType, FileHistory, ProjectFilePath, + ProjectUser, + ProjectRole, ) from .files import ( UploadChanges, @@ -93,14 +98,19 @@ def parse_project_access_update_request(access: Dict) -> Dict: """Parse raw project access update request and filter out invalid entries. New access can be specified either by list of usernames or ids -> convert only to ids fur further processing. + Converted lists are flattened, e.g. user id is unique within all keys. Bear in mind roles keys are optional, + if missing, it means that we do not want to do any changes there. + + Deprecated. Used only in legacy PUT /v1/project endpoint for project access replacement. :Example: >>> parse_project_access_update_request({"writersnames": ["john"], "readersnames": ["john, jack, bob.inactive"]}) - {"writers": [1], "readers": [1,2], "invalid_usernames": ["bob.inactive"], "invalid_ids":[]} + {"ProjectRole.WRITER": [1], "ProjectRole.READER": [2], "invalid_usernames": ["bob.inactive"], "invalid_ids":[]} >>> parse_project_access_update_request({"writers": [1], "readers": [1,2,3]}) - {"writers": [1], "readers": [1,2], "invalid_usernames": [], "invalid_ids":[3]"} + {"ProjectRole.WRITER": [1], "ProjectRole.READER": [2], "invalid_usernames": [], "invalid_ids":[3]"} """ + resp = {} parsed_access = {} names = set( access.get("ownersnames", []) @@ -137,9 +147,23 @@ def parse_project_access_update_request(access: Dict) -> Dict: # use legacy option elif key in access: parsed_access[key] = [id for id in access.get(key) if id in valid_ids] - parsed_access["invalid_usernames"] = list(names.difference(valid_usernames)) - parsed_access["invalid_ids"] = list(ids.difference(valid_ids)) - return parsed_access + + # remove 'inheritance', prepare final map for direct assignments + processed_ids = [] + for key in ("owners", "writers", "editors", "readers"): + # we might not want to modify all roles + if key not in parsed_access: + continue + role = ProjectRole(key[:-1]) + resp[role] = [] + for user_id in parsed_access.get(key): + if user_id not in processed_ids: + resp[role].append(user_id) + processed_ids.append(user_id) + + resp["invalid_usernames"] = list(names.difference(valid_usernames)) + resp["invalid_ids"] = list(ids.difference(valid_ids)) + return resp @auth_required @@ -193,10 +217,12 @@ def add_project(namespace): # noqa: E501 "location": generate_location(), } - p = Project(**request.json, creator=current_user, workspace=workspace) + p = Project( + **request.json, + creator=current_user, + workspace=workspace, + ) p.updated = datetime.utcnow() - db.session.add(p) - pa = ProjectAccess(p, public=request.json.get("public", False)) template_name = request.json.get("template", None) if template_name: @@ -231,7 +257,7 @@ def add_project(namespace): # noqa: E501 get_device_id(request), ) - db.session.add(pa) + db.session.add(p) db.session.add(version) db.session.commit() project_version_created.send(version) @@ -498,12 +524,12 @@ def get_projects_by_names(): # noqa: E501 Project.workspace_id == workspace.id, Project.name == name ).first() if result: - user_ids = ( - result.access.owners + result.access.writers + result.access.readers - ) users_map = { u.id: u.username - for u in User.query.filter(User.id.in_(set(user_ids))).all() + for u in User.query.select_from(ProjectUser) + .join(User) + .filter(ProjectUser.project_id == result.id) + .all() } workspaces_map = {workspace.id: workspace.name} ctx = {"users_map": users_map, "workspaces_map": workspaces_map} @@ -530,18 +556,19 @@ def get_projects_by_uuids(uuids): # noqa: E501 if len(proj_ids) > 10: abort(400, "Too many projects") - user_ids = [] - ws_ids = [] projects = ( projects_query(ProjectPermissions.Read, as_admin=False) .filter(Project.id.in_(proj_ids)) .all() ) - for p in projects: - user_ids.extend(p.access.owners + p.access.writers + p.access.readers) - ws_ids.append(p.workspace_id) + ws_ids = set([p.workspace_id for p in projects]) + projects_ids = [p.id for p in projects] users_map = { - u.id: u.username for u in User.query.filter(User.id.in_(set(user_ids))).all() + u.id: u.username + for u in User.query.select_from(ProjectUser) + .join(User) + .filter(ProjectUser.project_id.in_(projects_ids)) + .all() } workspaces_map = {w.id: w.name for w in current_app.ws_handler.get_by_ids(ws_ids)} ctx = {"users_map": users_map, "workspaces_map": workspaces_map} @@ -616,15 +643,16 @@ def get_paginated_projects( only_public, ) result = projects.paginate(page, per_page).items - total = projects.paginate(page, per_page).total + total = projects.paginate().total # create user map id:username passed to project schema to minimize queries to db - user_ids = [] - for p in result: - user_ids.extend(p.access.owners + p.access.writers + p.access.readers) - + projects_ids = [p.id for p in result] users_map = { - u.id: u.username for u in User.query.filter(User.id.in_(set(user_ids))).all() + u.id: u.username + for u in User.query.select_from(ProjectUser) + .join(User) + .filter(ProjectUser.project_id.in_(projects_ids)) + .all() } ws_ids = [p.workspace_id for p in projects] workspaces_map = {w.id: w.name for w in current_app.ws_handler.get_by_ids(ws_ids)} @@ -651,16 +679,18 @@ def update_project(namespace, project_name): # noqa: E501 # pylint: disable=W0 :rtype: ProjectDetail """ project = require_project(namespace, project_name, ProjectPermissions.Update) - access = request.json.get("access", {}) - - id_diffs, error = current_app.ws_handler.update_project_members(project, access) + parsed_access = parse_project_access_update_request(request.json.get("access", {})) + # get set of modified user_ids and possible (custom) errors + id_diffs, error = current_app.ws_handler.update_project_members( + project, parsed_access + ) if not id_diffs and error: # nothing was done but there are errors return jsonify(error.to_dict()), 422 if "public" in request.json["access"]: - project.access.public = request.json["access"]["public"] + project.public = request.json["access"]["public"] db.session.add(project) db.session.commit() @@ -1188,7 +1218,6 @@ def clone_project(namespace, project_name): # noqa: E501 ) p.updated = datetime.utcnow() db.session.add(p) - pa = ProjectAccess(p, public=False) try: p.storage.initialize(template_project=cloned_project) @@ -1213,7 +1242,6 @@ def clone_project(namespace, project_name): # noqa: E501 user_agent, device_id, ) - db.session.add(pa) db.session.add(project_version) db.session.commit() project_version_created.send(project_version) @@ -1399,7 +1427,7 @@ def get_workspace_by_id(id): if not ( ws.user_has_permissions(current_user, "read") - or ws.get_user_role(current_user) == "guest" + or ws.get_user_role(current_user) == WorkspaceRole.GUEST ): abort(403, f"You do not have permissions to workspace") diff --git a/server/mergin/sync/public_api_v2.yaml b/server/mergin/sync/public_api_v2.yaml index 4973696a..a167cb26 100644 --- a/server/mergin/sync/public_api_v2.yaml +++ b/server/mergin/sync/public_api_v2.yaml @@ -98,6 +98,129 @@ paths: "404": $ref: "#/components/responses/NotFound" x-openapi-router-controller: mergin.sync.public_api_v2_controller + /projects/{id}/collaborators: + parameters: + - $ref: "#/components/parameters/ProjectId" + get: + tags: + - project + summary: Get project collaborators + operationId: get_project_collaborators + responses: + "200": + description: Project members + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/ProjectMember" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" + x-openapi-router-controller: mergin.sync.public_api_v2_controller + post: + tags: + - project + summary: Add project collaborator + operationId: add_project_collaborator + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - username + - role + properties: + username: + type: string + example: john.doe + description: username or email + role: + $ref: "#/components/schemas/ProjectRole" + responses: + "201": + description: New project member + content: + application/json: + schema: + $ref: "#/components/schemas/ProjectMember" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" + x-openapi-router-controller: mergin.sync.public_api_v2_controller + /projects/{id}/collaborators/{user_id}: + parameters: + - $ref: "#/components/parameters/ProjectId" + - name: user_id + in: path + description: user id + required: true + schema: + type: integer + patch: + tags: + - project + summary: Update project collaborator + operationId: update_project_collaborator + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - role + properties: + role: + $ref: "#/components/schemas/ProjectRole" + responses: + "200": + description: Updated project member + content: + application/json: + schema: + $ref: "#/components/schemas/ProjectMember" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" + x-openapi-router-controller: mergin.sync.public_api_v2_controller + delete: + tags: + - project + summary: Remove project collaborator + operationId: remove_project_collaborator + parameters: + - $ref: "#/components/parameters/ProjectId" + responses: + "204": + $ref: "#/components/responses/NoContent" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" + x-openapi-router-controller: mergin.sync.public_api_v2_controller components: responses: NoContent: @@ -122,3 +245,40 @@ components: type: string format: uuid pattern: \b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b + schemas: + ProjectRole: + type: string + nullable: true + enum: + - owner + - writer + - editor + - reader + example: reader + WorkspaceRole: + type: string + enum: + - owner + - admin + - writer + - editor + - reader + - guest + example: writer + ProjectMember: + type: object + properties: + id: + type: integer + example: 1 + username: + type: string + example: john.doe + email: + type: string + format: email + example: john.doe@example.com + workspace_role: + $ref: '#/components/schemas/WorkspaceRole' + project_role: + $ref: '#/components/schemas/ProjectRole' diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 59cdbb2d..d2076910 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -3,28 +3,28 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial from datetime import datetime - from connexion import NoContent, request from flask import abort, jsonify from flask_login import current_user -from mergin.app import db -from mergin.auth import auth_required -from mergin.sync.models import Project -from mergin.sync.permissions import ProjectPermissions, require_project_by_uuid -from mergin.sync.utils import is_name_allowed +from .schemas import ProjectMemberSchema +from .workspace import WorkspaceRole +from ..app import db +from ..auth import auth_required +from ..auth.models import User +from .models import Project, ProjectRole, ProjectMember +from .permissions import ProjectPermissions, require_project_by_uuid +from .private_api_controller import project_access_granted +from .utils import is_name_allowed @auth_required -def schedule_delete_project(id_): +def schedule_delete_project(id): """Schedule deletion of the project. Celery job `mergin.sync.tasks.remove_projects_backups` does the rest. - - :param id_: Project id - :type id_: str """ - project = require_project_by_uuid(id_, ProjectPermissions.Delete) + project = require_project_by_uuid(id, ProjectPermissions.Delete) project.removed_at = datetime.utcnow() project.removed_by = current_user.id db.session.commit() @@ -33,26 +33,18 @@ def schedule_delete_project(id_): @auth_required -def delete_project_now(id_): - """Delete the project immediately. - - :param id_: Project id - :type id_: str - """ - project = require_project_by_uuid(id_, ProjectPermissions.Delete, scheduled=True) +def delete_project_now(id): + """Delete the project immediately""" + project = require_project_by_uuid(id, ProjectPermissions.Delete, scheduled=True) project.delete() return NoContent, 204 @auth_required -def update_project(id_): - """Rename project - - :param id_: Project_id - :type id_: str - """ - project = require_project_by_uuid(id_, ProjectPermissions.Update) +def update_project(id): + """Rename project""" + project = require_project_by_uuid(id, ProjectPermissions.Update) new_name = request.json["name"] if not is_name_allowed(new_name): @@ -72,3 +64,69 @@ def update_project(id_): db.session.commit() return NoContent, 204 + + +@auth_required +def get_project_collaborators(id): + """Get project collaborators, with both direct role and inherited role from workspace""" + project = require_project_by_uuid(id, ProjectPermissions.Update) + project_members = [] + for user, workspace_role in project.workspace.members(): + project_role = project.get_role(user.id) + if workspace_role != WorkspaceRole.GUEST or project_role is not None: + project_members.append( + ProjectMember( + id=user.id, + username=user.username, + email=user.email, + project_role=project_role, + workspace_role=workspace_role, + ) + ) + + data = ProjectMemberSchema(many=True).dump(project_members) + return data, 200 + + +@auth_required +def add_project_collaborator(id): + """Add project collaborator""" + project = require_project_by_uuid(id, ProjectPermissions.Update) + user = User.get_by_login(request.json["username"]) + if not user: + abort(404) + + if project.get_role(user.id): + abort(409, "User is already a project member") + + project.set_role(user.id, ProjectRole(request.json["role"])) + db.session.commit() + project_access_granted.send(project, user_id=user.id) + data = ProjectMemberSchema().dump(project.get_member(user.id)) + return data, 201 + + +@auth_required +def update_project_collaborator(id, user_id): + """Update project collaborator""" + project = require_project_by_uuid(id, ProjectPermissions.Update) + user = User.query.filter_by(id=user_id, active=True).first_or_404() + if not project.get_role(user_id): + abort(404, "User is not a project member") + + project.set_role(user.id, ProjectRole(request.json["role"])) + db.session.commit() + data = ProjectMemberSchema().dump(project.get_member(user.id)) + return data, 200 + + +@auth_required +def remove_project_collaborator(id, user_id): + """Remove project collaborator""" + project = require_project_by_uuid(id, ProjectPermissions.Update) + if not project.get_role(user_id): + abort(404, "User is not a project member") + + project.unset_role(user_id) + db.session.commit() + return NoContent, 204 diff --git a/server/mergin/sync/schemas.py b/server/mergin/sync/schemas.py index d8c766aa..c782c5ca 100644 --- a/server/mergin/sync/schemas.py +++ b/server/mergin/sync/schemas.py @@ -9,16 +9,27 @@ from .files import ProjectFileSchema, FileSchema from .permissions import ProjectPermissions -from .models import Project, ProjectVersion, AccessRequest, FileHistory, PushChangeType +from .models import ( + Project, + ProjectVersion, + AccessRequest, + FileHistory, + PushChangeType, + ProjectRole, + ProjectUser, +) +from .workspace import WorkspaceRole from ..app import DateTimeWithZ, ma from ..auth.models import User class ProjectAccessSchema(ma.SQLAlchemyAutoSchema): - owners = fields.List(fields.Integer()) - writers = fields.List(fields.Integer()) - editors = fields.List(fields.Integer()) - readers = fields.List(fields.Integer()) + """Schema for legacy response with user arrays""" + + owners = fields.Function(lambda obj: obj.members_by_role(ProjectRole.OWNER)) + writers = fields.Function(lambda obj: obj.members_by_role(ProjectRole.WRITER)) + editors = fields.Function(lambda obj: obj.members_by_role(ProjectRole.EDITOR)) + readers = fields.Function(lambda obj: obj.members_by_role(ProjectRole.READER)) public = fields.Boolean() @post_dump @@ -106,7 +117,7 @@ class ProjectSchemaForVersion(ma.SQLAlchemyAutoSchema): uploads = fields.Method("_uploads") name = fields.Function(lambda obj: obj.project.name) namespace = fields.Function(lambda obj: obj.project.workspace.name) - access = fields.Method("_access") + access = fields.Function(lambda obj: ProjectAccessSchema().dump(obj.project)) permissions = fields.Method("_permissions") disk_usage = fields.Method("_disk_usage") files = fields.Nested(ProjectFileSchema(), many=True) @@ -124,9 +135,6 @@ def _role(self, obj): def _uploads(self, obj): return [u.id for u in obj.project.uploads.all()] - def _access(self, obj): - return ProjectAccessSchema().dump(obj.project.access) - def _permissions(self, obj): return project_user_permissions(obj.project) @@ -156,7 +164,7 @@ class Meta: class ProjectSchema(ma.SQLAlchemyAutoSchema): id = fields.UUID() files = fields.Nested(ProjectFileSchema(), many=True) - access = fields.Nested(ProjectAccessSchema()) + access = fields.Function(lambda obj: ProjectAccessSchema().dump(obj)) permissions = fields.Function(project_user_permissions) version = fields.Function(lambda obj: ProjectVersion.to_v_name(obj.latest_version)) namespace = fields.Function(lambda obj: obj.workspace.name) @@ -185,7 +193,7 @@ class ProjectListSchema(ma.SQLAlchemyAutoSchema): id = fields.UUID() name = fields.Str() namespace = fields.Method("get_workspace_name") - access = fields.Nested(ProjectAccessSchema()) + access = fields.Function(lambda obj: ProjectAccessSchema().dump(obj)) permissions = fields.Function(project_user_permissions) version = fields.Function(lambda obj: ProjectVersion.to_v_name(obj.latest_version)) updated = fields.Method("get_updated") @@ -329,7 +337,8 @@ class UserWorkspaceSchema(ma.SQLAlchemyAutoSchema): def _user_role(self, obj): if not self.context.get("user"): return - return obj.get_user_role(self.context.get("user")) + role = obj.get_user_role(self.context.get("user")) + return role and role.value class ProjectInvitationAccessSchema(Schema): @@ -387,3 +396,11 @@ class Meta: "user_agent", ] load_instance = True + + +class ProjectMemberSchema(Schema): + id = fields.Integer() + username = fields.String() + email = fields.Email() + project_role = fields.Enum(enum=ProjectRole, by_value=True) + workspace_role = fields.Enum(enum=WorkspaceRole, by_value=True) diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 0cd835e1..db7865cf 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -5,23 +5,21 @@ from datetime import datetime, timedelta, timezone from typing import Dict, Tuple, Optional, Set, List from flask_login import current_user -from sqlalchemy import or_, and_, Column, literal, extract -from sqlalchemy.orm import joinedload +from sqlalchemy import Column, literal, extract from .errors import UpdateProjectAccessError from .models import ( Project, - ProjectAccess, AccessRequest, ProjectAccessDetail, ProjectVersion, + ProjectUser, ) from .permissions import projects_query, ProjectPermissions -from .public_api_controller import parse_project_access_update_request from ..app import db from ..auth.models import User from ..config import Configuration -from .interfaces import AbstractWorkspace, WorkspaceHandler +from .interfaces import AbstractWorkspace, WorkspaceHandler, WorkspaceRole class GlobalWorkspace(AbstractWorkspace): @@ -74,33 +72,33 @@ def disk_usage(self): def user_has_permissions(self, user, permissions): role = self.get_user_role(user) # mergin super-user has all permissions - if role == "owner": + if role is WorkspaceRole.OWNER: return True if permissions == "read": - return role in ["admin", "writer", "editor", "reader"] + return role >= WorkspaceRole.READER elif permissions == "edit": - return role in ["admin", "writer", "editor"] + return role >= WorkspaceRole.EDITOR elif permissions == "write": - return role in ["admin", "writer"] + return role >= WorkspaceRole.WRITER elif permissions == "admin": - return role == "admin" + return role >= WorkspaceRole.ADMIN else: return False def user_is_member(self, user): return True - def get_user_role(self, user): + def get_user_role(self, user) -> WorkspaceRole: if user.is_admin: - return "owner" + return WorkspaceRole.OWNER if Configuration.GLOBAL_ADMIN: - return "admin" + return WorkspaceRole.ADMIN if Configuration.GLOBAL_WRITE: - return "writer" + return WorkspaceRole.WRITER if Configuration.GLOBAL_READ: - return "reader" - return "guest" + return WorkspaceRole.READER + return WorkspaceRole.GUEST def project_count(self): from .models import Project @@ -112,6 +110,17 @@ def project_count(self): .count() ) + def members(self): + return [ + (user, self.get_user_role(user)) + for user in User.query.filter(User.active.is_(True)) + .order_by(User.email) + .all() + ] + + def can_add_users(self, user: User) -> bool: + return user.is_admin + class GlobalWorkspaceHandler(WorkspaceHandler): """Implements handler for GlobalWorkspace objects""" @@ -166,10 +175,9 @@ def filter_projects( ): if only_public: projects = ( - Project.query.join(ProjectAccess) - .filter(Project.storage_params.isnot(None)) + Project.query.filter(Project.storage_params.isnot(None)) .filter(Project.removed_at.is_(None)) - .filter(ProjectAccess.public.is_(True)) + .filter(Project.public.is_(True)) ) else: projects = projects_query( @@ -183,23 +191,17 @@ def filter_projects( if flag == "created": projects = projects.filter(Project.creator_id == user.id) if flag == "shared": - # check global read permissions + projects = projects.filter(Project.creator_id != user.id) + # check global read permissions or direct project permissions if workspace.user_has_permissions(user, "read"): - read_access_workspace_id = workspace.id + projects = projects.filter(Project.workspace_id == workspace.id) else: - read_access_workspace_id = None - projects = projects.filter( - or_( - and_( - ProjectAccess.readers.contains([user.id]), - Project.creator_id != user.id, - ), - and_( - Project.workspace_id == read_access_workspace_id, - Project.creator_id != user.id, - ), + subquery = ( + db.session.query(ProjectUser.project_id) + .filter(ProjectUser.user_id == user.id) + .subquery() ) - ) + projects = projects.filter(Project.id.in_(subquery)) if name: projects = projects.filter(Project.name.ilike("%{}%".format(name))) @@ -288,13 +290,13 @@ def update_project_members( ) -> Tuple[Set[int], Optional[UpdateProjectAccessError]]: """Update project members doing bulk access update""" error = None - parsed_access = parse_project_access_update_request(access) - id_diffs = project.access.bulk_update(parsed_access) + id_diffs = project.bulk_roles_update(access) db.session.add(project) db.session.commit() - if parsed_access.get("invalid_usernames") or parsed_access.get("invalid_ids"): + + if access.get("invalid_usernames") or access.get("invalid_ids"): error = UpdateProjectAccessError( - parsed_access["invalid_usernames"], parsed_access["invalid_ids"] + access["invalid_usernames"], access["invalid_ids"] ) return id_diffs, error @@ -317,12 +319,7 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: elif Configuration.GLOBAL_READ: global_role = "reader" - direct_members_ids = set( - project.access.readers - + project.access.editors - + project.access.writers - + project.access.owners - ) + direct_members_ids = [u.user_id for u in project.project_users] users = User.query.filter(User.active.is_(True)).order_by(User.email) direct_members = users.filter(User.id.in_(direct_members_ids)).all() @@ -331,7 +328,7 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: member = ProjectAccessDetail( id=dm.id, username=dm.username, - role=ws.get_user_role(dm), + role=ws.get_user_role(dm).value, name=dm.profile.name(), email=dm.email, project_permission=project_role and project_role.value, diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 41f1065d..2613c68f 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -13,7 +13,7 @@ from ..auth.models import User, UserProfile, LoginHistory from ..auth.tasks import anonymize_removed_users from ..app import db -from ..sync.models import Project +from ..sync.models import Project, ProjectRole from . import ( test_workspace_id, json_headers, @@ -667,7 +667,7 @@ def test_close_user_account(client): user = User.query.filter_by(username=DEFAULT_USER[0]).first() # check default project project = Project.query.filter_by(workspace_id=test_workspace_id).first() - assert user.id in project.access.owners + assert project.get_role(user.id) is ProjectRole.OWNER resp = client.delete("/v1/user") assert resp.status_code == 204 @@ -677,7 +677,7 @@ def test_close_user_account(client): assert not user.active assert user.inactive_since assert ( - project.access.owners == [] + project.project_users == [] ) # project will be removed by cron job, but is not accessible # login should fail now @@ -782,3 +782,59 @@ def test_update_project_v2(client): login_as_admin(client) resp = client.patch(f"v2/projects/{project.id}", json=data) assert resp.status_code == 204 + + +@patch("mergin.celery.send_email_async.apply_async") +def test_add_user(send_email_mock, client): + """Test adding new user via public endpoint""" + login_as_admin(client) + data = { + "username": "test_user", + "email": "test@test.com", + "password": "#pwd1234", + "role": "reader", + "workspace_id": test_workspace_id, + } + response = client.post("/v2/users", json=data) + assert response.status_code == 201 + assert response.json["username"] == "test_user" + assert response.json["email"] == "test@test.com" + assert response.json["receive_notifications"] is False + assert not send_email_mock.called + + # drop required attribute + data.pop("email") + assert client.post("/v2/users", json=data).status_code == 400 + + # try register with the same account again + data["email"] = "test@test.com" + assert client.post("/v2/users", json=data).status_code == 400 + + # register with optional flag sent but username missing + data["email"] = "test2@test.com" + data.pop("username") + data["notify_user"] = True + response = client.post("/v2/users", json=data) + assert response.status_code == 201 + assert response.json["receive_notifications"] is True + assert response.json["username"] == "test2" # autogenerated from email + assert send_email_mock.called + assert send_email_mock.call_args.args[1]["subject"] == "Invitation to Mergin Maps" + + # ordinary user cannot add others + login(client, "test2", "#pwd1234") + assert client.post("/v2/users", json=data).status_code == 403 + + +def test_username_generation(client): + """Test generation of username from email""" + assert User.generate_username("tralala@example.com") == "tralala" + assert User.generate_username("TraLala@example.com") == "tralala" + assert User.generate_username("tralala") is None + + # add user to test autoincrement + user = add_user("user", "user") + assert User.generate_username(user.email) == user.username + "1" + + user = add_user("user25", "user") + assert User.generate_username(user.email) == user.username + "1" diff --git a/server/mergin/tests/test_celery.py b/server/mergin/tests/test_celery.py index 09b118c6..e8346973 100644 --- a/server/mergin/tests/test_celery.py +++ b/server/mergin/tests/test_celery.py @@ -126,7 +126,7 @@ def test_remove_deleted_project_backups(client): rm_project.removed_at and not rm_project.storage_params and not rm_project.files - and rm_project.access.owners == [] + and rm_project.project_users == [] ) assert ( not Project.query.filter_by(id=rm_project.id) diff --git a/server/mergin/tests/test_db_hooks.py b/server/mergin/tests/test_db_hooks.py index 43186c18..e7f9e270 100644 --- a/server/mergin/tests/test_db_hooks.py +++ b/server/mergin/tests/test_db_hooks.py @@ -4,19 +4,19 @@ import os from pathlib import Path -from sqlalchemy.orm.attributes import flag_modified from ..sync.models import ( Project, ProjectVersion, Upload, - ProjectAccess, SyncFailuresHistory, AccessRequest, RequestStatus, FileHistory, ProjectFilePath, LatestProjectFiles, + ProjectRole, + ProjectUser, ) from ..sync.files import UploadChanges from ..auth.models import User @@ -38,8 +38,7 @@ def test_close_user_account(client, diff_project): user_id = user.id # user has access to mergin user diff_project - diff_project.access.writers.append(user.id) - flag_modified(diff_project.access, "writers") + diff_project.set_role(user.id, ProjectRole.WRITER) # user contributed to another user project so he is listed in projects history changes = UploadChanges(added=[], updated=[], removed=[]) pv = ProjectVersion(diff_project, 11, user.id, changes, "127.0.0.1") @@ -51,7 +50,7 @@ def test_close_user_account(client, diff_project): test_workspace = create_workspace() p = create_project(user_project, test_workspace, user) db.session.commit() - assert user.id in diff_project.access.writers + assert diff_project.get_role(user.id) is ProjectRole.WRITER proj_resp = client.get( f"/v1/project/{diff_project.workspace.name}/{diff_project.name}" ) @@ -67,7 +66,7 @@ def test_close_user_account(client, diff_project): # deactivate user first (direct hack in db to mimic inconsistency) user.active = False db.session.commit() - assert user.id in diff_project.access.writers # not yet removed + assert diff_project.get_role(user.id) is ProjectRole.WRITER # not yet removed proj_resp = client.get( f"/v1/project/{diff_project.workspace.name}/{diff_project.name}" ) @@ -96,7 +95,7 @@ def test_close_user_account(client, diff_project): ).count() == 1 ) - assert user_id not in diff_project.access.writers + assert not diff_project.get_role(user_id) # user remains referenced in existing project version he created (as read-only ref) assert diff_project.get_latest_version().author_id == user_id sync_fail_history = SyncFailuresHistory.query.filter( @@ -143,7 +142,7 @@ def test_remove_project(client, diff_project): assert Project.query.filter_by(id=project_id).count() assert not Upload.query.filter_by(project_id=project_id).count() assert ProjectVersion.query.filter_by(project_id=project_id).count() - assert ProjectAccess.query.filter_by(project_id=project_id).count() + assert not ProjectUser.query.filter_by(project_id=project_id).count() cleanup(client, [project_dir]) assert access_request.status == RequestStatus.DECLINED.value # after removal cached information in project table remains and project versions, but not files details diff --git a/server/mergin/tests/test_permissions.py b/server/mergin/tests/test_permissions.py index cfabbdc2..230961f0 100644 --- a/server/mergin/tests/test_permissions.py +++ b/server/mergin/tests/test_permissions.py @@ -17,6 +17,7 @@ def test_project_permissions(client): owner = add_user("owner", "pwd") test_workspace = create_workspace() project = create_project("test_permissions", test_workspace, owner) + project.set_role(owner.id, ProjectRole.OWNER) # testing owner permissions -> has all of them assert ProjectPermissions.Read.check(project, owner) @@ -47,7 +48,7 @@ def test_project_permissions(client): # tests user with editor access -> has read access - project.access.set_role(user.id, ProjectRole.EDITOR) + project.set_role(user.id, ProjectRole.EDITOR) db.session.commit() assert ProjectPermissions.Read.check(project, user) assert ProjectPermissions.Edit.check(project, user) @@ -73,7 +74,7 @@ def test_project_permissions(client): # deactivate user -> no permissions user.active = False - project.access.unset_role(user.id) + project.unset_role(user.id) db.session.commit() assert not ProjectPermissions.Upload.check(project, user) assert not ProjectPermissions.Delete.check(project, user) @@ -86,7 +87,7 @@ def test_project_permissions(client): # tests anonymous user -> only has read access if project is public user = AnonymousUserMixin() assert not ProjectPermissions.Read.check(project, user) - project.access.public = True + project.public = True db.session.commit() assert ProjectPermissions.Read.check(project, user) assert not ProjectPermissions.Edit.check(project, user) diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index d8c3145c..c8aabd9e 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/server/mergin/tests/test_private_project_api.py @@ -24,7 +24,7 @@ def test_project_unsubscribe(client, diff_project): # create user and grant him write access user = add_user("reader", "reader") - diff_project.access.set_role(user.id, ProjectRole.WRITER) + diff_project.set_role(user.id, ProjectRole.WRITER) db.session.commit() # project owner is logged in @@ -46,9 +46,7 @@ def test_project_unsubscribe(client, diff_project): ) ) assert resp.status_code == 200 - assert not diff_project.access.get_role(user.id) - assert user.id not in diff_project.access.readers - assert user.id not in diff_project.access.writers + assert not diff_project.get_role(user.id) def test_project_access_request(client): @@ -155,9 +153,7 @@ def test_project_access_request(client): project = Project.query.filter( Project.name == "testx", Project.workspace_id == test_workspace.id ).first() - assert user2.id in project.access.readers - assert user2.id in project.access.writers - assert user2.id not in project.access.owners + assert project.get_role(user2.id) is ProjectRole.WRITER # no request listed resp = client.get( @@ -341,52 +337,49 @@ def test_update_project_access(client, diff_project): original_creator_id = diff_project.creator.id # create user and grant him write access user = add_user("reader", "reader") - assert user.id not in diff_project.access.readers + assert not diff_project.get_role(user.id) data = {"user_id": user.id, "role": "none"} # nothing happens resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert user.id not in diff_project.access.readers + assert not diff_project.get_role(user.id) # grant read access data["role"] = "reader" resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert user.id in diff_project.access.readers + assert diff_project.get_role(user.id) is ProjectRole.READER # grant editor access data["role"] = "editor" resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert user.id in diff_project.access.editors + assert diff_project.get_role(user.id) is ProjectRole.EDITOR # change to write access data["role"] = "writer" resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert user.id in diff_project.access.readers - assert user.id in diff_project.access.writers + assert diff_project.get_role(user.id) is ProjectRole.WRITER # downgrade to read access data["role"] = "reader" resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert user.id in diff_project.access.readers - assert user.id not in diff_project.access.writers + assert diff_project.get_role(user.id) is ProjectRole.READER # remove access data["role"] = "none" resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert user.id not in diff_project.access.readers - assert user.id not in diff_project.access.writers + assert not diff_project.get_role(user.id) # update public parameter => public: True data["public"] = True resp = client.patch(url, headers=json_headers, data=json.dumps(data)) assert resp.status_code == 200 - assert diff_project.access.public == True + assert diff_project.public == True # access of project creator can be removed data["user_id"] = diff_project.creator_id @@ -397,8 +390,7 @@ def test_update_project_access(client, diff_project): ) assert resp.status_code == 200 db.session.rollback() - assert user.id not in diff_project.access.owners - assert user.id not in diff_project.access.readers + assert not diff_project.get_role(user.id) assert diff_project.creator_id == original_creator_id # try to grant access to inaccessible user @@ -499,10 +491,9 @@ def test_get_project_access(client): assert resp.status_code == 200 assert len(resp.json) == 1 assert resp.json[0]["project_permission"] == "owner" - project.access.set_role(users[0].id, ProjectRole.OWNER) - project.access.set_role(users[1].id, ProjectRole.WRITER) - project.access.set_role(users[2].id, ProjectRole.READER) - db.session.add(project.access) + project.set_role(users[0].id, ProjectRole.OWNER) + project.set_role(users[1].id, ProjectRole.WRITER) + project.set_role(users[2].id, ProjectRole.READER) db.session.commit() resp = client.get(url) assert resp.status_code == 200 diff --git a/server/mergin/tests/test_project_controller.py b/server/mergin/tests/test_project_controller.py index 4371ab8b..76c94d89 100644 --- a/server/mergin/tests/test_project_controller.py +++ b/server/mergin/tests/test_project_controller.py @@ -18,7 +18,6 @@ import re from flask_login import current_user -from unittest.mock import patch from pygeodiff import GeoDiff from flask import url_for, current_app import tempfile @@ -29,7 +28,6 @@ Project, Upload, ProjectVersion, - ProjectAccess, SyncFailuresHistory, GeodiffActionHistory, ProjectRole, @@ -38,7 +36,8 @@ ProjectFilePath, ) from ..sync.files import ChangesSchema -from ..sync.schemas import ProjectListSchema +from ..sync.permissions import projects_query +from ..sync.schemas import ProjectListSchema, ProjectSchema from ..sync.utils import generate_checksum, is_versioned_file from ..auth.models import User, UserProfile @@ -177,7 +176,7 @@ def test_get_paginated_projects(client): resp_data = json.loads(resp.data) assert len(resp_data.get("projects")) == 10 assert resp_data.get("count") == 15 - assert "foo8" in resp_data.get("projects")[9]["name"] + assert "foo7" in resp_data.get("projects")[9]["name"] assert "v0" == resp_data.get("projects")[9]["version"] resp = client.get( @@ -200,7 +199,7 @@ def test_get_paginated_projects(client): ) resp_data = json.loads(resp.data) assert len(resp_data.get("projects")) == 5 - assert "foo13" in resp_data.get("projects")[-1]["name"] + assert "foo12" in resp_data.get("projects")[-1]["name"] # tests backward compatibility sort resp_alt = client.get( "/v1/project/paginated?page=2&per_page=10&order_by=namespace&descending=false" @@ -290,14 +289,14 @@ def test_get_paginated_projects(client): assert resp_data.get("count") == 1 assert not resp_data.get("projects")[0].get("has_conflict") - project.access.public = True + project.public = True db.session.commit() # reset permissions so new user would be only a guest Configuration.GLOBAL_READ = False Configuration.GLOBAL_WRITE = False Configuration.GLOBAL_ADMIN = False - # add new user and let him create one project + # add new user and let him create one project (total 15+1) user2 = add_user("user2", "ilovemergin") assert not test_workspace.user_has_permissions(user2, "read") create_project("created", test_workspace, user2) @@ -310,7 +309,7 @@ def test_get_paginated_projects(client): assert resp.json["count"] == 0 # share project explicitly p = Project.query.filter_by(name="foo1").first() - p.access.set_role(user2.id, ProjectRole.READER) + p.set_role(user2.id, ProjectRole.READER) db.session.commit() resp = client.get("/v1/project/paginated?page=1&per_page=10&flag=shared") assert resp.json["count"] == 1 @@ -318,6 +317,8 @@ def test_get_paginated_projects(client): # make user reader of all projects Configuration.GLOBAL_READ = True + resp = client.get("/v1/project/paginated?page=1&per_page=10&flag=created") + assert resp.json["count"] == 1 resp = client.get("/v1/project/paginated?page=1&per_page=20&flag=shared") resp_data = json.loads(resp.data) assert resp.status_code == 200 @@ -448,6 +449,26 @@ def test_add_project(client, app, data, expected): ) +project_data = [ + ( + {"name": "project1", "public": True}, + True, + ), # project accessibility follows the request argument + ({"name": "project1"}, False), # by default the project is private +] + + +@pytest.mark.parametrize("add_project_data,public", project_data) +def test_add_public_project(client, add_project_data, public): + resp = client.post( + "/v1/project/{}".format(test_workspace_name), + data=json.dumps(add_project_data), + headers=json_headers, + ) + p = Project.query.filter_by(name=add_project_data["name"]).first() + assert p.public == public + + def test_versioning(client): # tests if blank project has version set up to v0 resp = client.post( @@ -629,8 +650,9 @@ def test_update_project(client): project = Project.query.filter_by( name=test_project, workspace_id=test_workspace_id ).first() + creator = User.query.get(project.creator_id) # need for private project - project.access.public = False + project.public = False db.session.add(project) # add some tester test_user = User( @@ -642,27 +664,25 @@ def test_update_project(client): db.session.commit() # add tests user as reader to project - data = {"access": {"readers": project.access.readers + [test_user.id]}} + data = {"access": {"readers": [test_user.id]}} resp = client.put( "/v1/project/{}/{}".format(test_workspace_name, test_project), data=json.dumps(data), headers=json_headers, ) assert resp.status_code == 200 - assert test_user.id in project.access.readers + assert project.get_role(test_user.id) is ProjectRole.READER # add tests user as writer to project - writers = [ - u.username for u in User.query.filter(User.id.in_(project.access.writers)).all() - ] - data = {"access": {"writersnames": writers + [test_user.username]}} + data = {"access": {"writersnames": [test_user.username]}} resp = client.put( "/v1/project/{}/{}".format(test_workspace_name, test_project), data=json.dumps(data), headers=json_headers, ) assert resp.status_code == 200 - assert test_user.id in project.access.writers + assert project.get_role(test_user.id) is ProjectRole.WRITER + assert project.get_role(creator.id) is ProjectRole.OWNER # try to remove project creator from owners data = {"access": {"owners": [test_user.id]}} @@ -672,13 +692,10 @@ def test_update_project(client): headers=json_headers, ) assert resp.status_code == 200 + assert not project.get_role(creator.id) # try to add non-existing user - readers = [ - user.username - for user in User.query.filter(User.id.in_(project.access.readers)).all() - ] - data = {"access": {"readersnames": readers + ["not-found-user"]}} + data = {"access": {"readersnames": ["not-found-user"]}} resp = client.put( f"/v1/project/{test_workspace_name}/{test_project}", data=json.dumps(data), @@ -690,16 +707,16 @@ def test_update_project(client): assert resp.json["invalid_usernames"] == ["not-found-user"] # try to add non-existing user plus make some valid update -> only partial success - readers = [ - user.username - for user in User.query.filter(User.id.in_(project.access.readers)).all() + current_users = [u.user_id for u in project.project_users] + usernames = [ + user.username for user in User.query.filter(User.id.in_(current_users)).all() ] data = { "access": { - "readersnames": readers + ["not-found-user"], - "editorsnames": readers, - "writersnames": readers, - "ownersnames": readers, + "readersnames": ["not-found-user"], + "editorsnames": usernames + [creator.username], + "writersnames": usernames + [creator.username], + "ownersnames": usernames, } } resp = client.put( @@ -709,6 +726,8 @@ def test_update_project(client): ) assert resp.status_code == 207 assert resp.json["code"] == "UpdateProjectAccessError" + assert project.get_role(test_user.id) is ProjectRole.OWNER + assert project.get_role(creator.id) is ProjectRole.WRITER # login as a new project owner and check permissions login(client, test_user.username, "tester") @@ -1131,10 +1150,10 @@ def test_push_to_new_project(client): p = Project.query.filter_by( name=test_project, workspace_id=test_workspace_id ).first() - project = Project("blank", p.storage_params, p.creator, p.workspace, files=[]) + project = Project( + "blank", p.storage_params, p.creator, p.workspace, files=[], public=True + ) db.session.add(project) - pa = ProjectAccess(project, True) - db.session.add(pa) db.session.commit() current_app.config["BLACKLIST"] = ["test4"] @@ -1433,7 +1452,7 @@ def test_push_finish(client): project = Project.query.filter_by( name=test_project, workspace_id=test_workspace_id ).first() - project.access.owners.append(user.id) + project.set_role(user.id, ProjectRole.OWNER) db.session.commit() upload, upload_dir = create_transaction(user.username, changes) @@ -1777,7 +1796,7 @@ def test_clone_project(client, data, username, expected): assert os.path.exists( os.path.join(project.storage.project_dir, project.files[0].location) ) - assert not project.access.public + assert not project.public # check if there is no diffs in cloned files assert not any(file.diff for file in project.files) pv = project.get_latest_version() @@ -2115,7 +2134,7 @@ def test_orphan_project(client): test_workspace = create_workspace() project = create_project("orphan", test_workspace, user) assert project.creator_id == user_id - assert project.access.owners == [user_id] + assert project.get_role(user_id) is ProjectRole.OWNER # user is removed by superuser login_as_admin(client) @@ -2128,7 +2147,7 @@ def test_orphan_project(client): # project still exists (it belongs to workspace) p = Project.query.filter_by(name="orphan").first() assert p.creator_id - assert p.access.owners == [] + assert len(p.project_users) == 0 # superuser as workspace owner has access to project and can assign new writer/owner resp = client.get(f"/v1/project/{test_workspace.name}/{p.name}") @@ -2173,7 +2192,7 @@ def test_orphan_project(client): def test_inactive_project(client, diff_project): """Project set for removal is not listed and can not be updated""" user = add_user("tests", "tests") - diff_project.access.set_role(user.id, ProjectRole.OWNER) + diff_project.set_role(user.id, ProjectRole.OWNER) diff_project.removed_at = datetime.datetime.utcnow() db.session.commit() project_path = get_project_path(diff_project) @@ -2212,7 +2231,7 @@ def test_inactive_project(client, diff_project): assert resp.status_code == 404 # modify project - data = {"access": {"readers": diff_project.access.readers + [user.id]}} + data = {"access": {"readers": [user.id]}} resp = client.put( f"/v1/project/{project_path}", data=json.dumps(data), headers=json_headers ) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 4bdb65a2..6dc96610 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -1,11 +1,14 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial - +from .utils import add_user from ..app import db from mergin.sync.models import Project from tests import test_project, test_workspace_id +from ..config import Configuration +from ..sync.models import ProjectRole + def test_schedule_delete_project(client): project = Project.query.filter_by( @@ -65,3 +68,61 @@ def test_rename_project(client): ) assert response.status_code == 400 assert response.json["code"] == "InvalidProjectName" + + +def test_project_members(client): + """Test CRUD endpoints for direct project members""" + project = Project.query.filter_by( + workspace_id=test_workspace_id, name=test_project + ).first() + url = f"v2/projects/{project.id}/collaborators" + response = client.get(url) + assert response.status_code == 200 + assert len(response.json) == 1 + + user = add_user("user", "password") + # user not a member yet + response = client.delete(url + f"/{user.id}") + assert response.status_code == 404 + + role = ProjectRole.READER.value + response = client.patch(url + f"/{user.id}", json={"role": role}) + assert response.status_code == 404 + + # add direct access + response = client.post(url, json={"role": role, "username": user.email}) + assert response.status_code == 201 + assert response.json["id"] == user.id + assert response.json["project_role"] == role + + Configuration.GLOBAL_READ = 0 + Configuration.GLOBAL_WRITE = 0 + Configuration.GLOBAL_ADMIN = 0 + response = client.get(url) + assert len(response.json) == 2 + member = next(u for u in response.json if u["id"] == user.id) + assert member["project_role"] == role + assert member["workspace_role"] == "guest" + + response = client.patch( + url + f"/{user.id}", json={"role": ProjectRole.WRITER.value} + ) + assert response.status_code == 200 + assert response.json["project_role"] == ProjectRole.WRITER.value + + response = client.delete(url + f"/{user.id}") + assert response.status_code == 204 + + response = client.get(url) + assert len(response.json) == 1 + + # test access only by workspace role + Configuration.GLOBAL_READ = 1 + response = client.get(url) + member = next(u for u in response.json if u["id"] == user.id) + assert not member["project_role"] + assert member["workspace_role"] == "reader" + + # access provided by workspace role cannot be removed directly + response = client.delete(url + f"/{user.id}") + assert response.status_code == 404 diff --git a/server/mergin/tests/test_workspace.py b/server/mergin/tests/test_workspace.py index a10eadb9..29ed6c5f 100644 --- a/server/mergin/tests/test_workspace.py +++ b/server/mergin/tests/test_workspace.py @@ -8,6 +8,7 @@ from ..app import db from ..config import Configuration +from ..sync.interfaces import WorkspaceRole from ..sync.models import FileHistory, ProjectVersion, PushChangeType, ProjectFilePath from ..sync.workspace import GlobalWorkspaceHandler from .utils import add_user, login, create_project @@ -25,7 +26,7 @@ def test_workspace_implementation(client): Configuration.GLOBAL_READ = False Configuration.GLOBAL_WRITE = False Configuration.GLOBAL_ADMIN = False - assert ws.get_user_role(user) == "guest" + assert ws.get_user_role(user) == WorkspaceRole.GUEST assert not ws.user_has_permissions(user, "owner") assert not ws.user_has_permissions(user, "read") assert not ws.user_has_permissions(user, "write") diff --git a/server/mergin/tests/utils.py b/server/mergin/tests/utils.py index debc096e..52fec093 100644 --- a/server/mergin/tests/utils.py +++ b/server/mergin/tests/utils.py @@ -19,7 +19,7 @@ from ..auth.models import User, UserProfile from ..sync.utils import generate_location, generate_checksum -from ..sync.models import Project, ProjectAccess, ProjectVersion, FileHistory +from ..sync.models import Project, ProjectVersion, FileHistory, ProjectRole from ..sync.files import UploadChanges, ChangesSchema from ..sync.workspace import GlobalWorkspace from ..app import db @@ -80,11 +80,9 @@ def create_project(name, workspace, user, **kwargs): p = Project(**project_params, **kwargs) p.updated = datetime.utcnow() + p.set_role(user.id, ProjectRole.OWNER) + db.session.add(p) db.session.flush() - - public = kwargs.get("public", False) - pa = ProjectAccess(p, public) - db.session.add(pa) changes = UploadChanges(added=[], updated=[], removed=[]) pv = ProjectVersion(p, 0, user.id, changes, "127.0.0.1") db.session.add(pv) @@ -170,12 +168,10 @@ def initialize(): } ) p.latest_version = 1 + p.public = True + p.set_role(user.id, ProjectRole.OWNER) p.updated = datetime.utcnow() db.session.add(p) - - # add default project permissions - pa = ProjectAccess(p, True) - db.session.add(pa) db.session.commit() upload_changes = ChangesSchema(context={"version": 1}).load( diff --git a/server/migrations/community/d02961c7416c_add_project_member_table.py b/server/migrations/community/d02961c7416c_add_project_member_table.py new file mode 100644 index 00000000..fe13279b --- /dev/null +++ b/server/migrations/community/d02961c7416c_add_project_member_table.py @@ -0,0 +1,242 @@ +"""Add project member table + +Revision ID: d02961c7416c +Revises: 1ab5b02ce532 +Create Date: 2024-10-31 15:20:52.833051 + +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "d02961c7416c" +down_revision = "1ab5b02ce532" +branch_labels = None +depends_on = None + + +def upgrade(): + conn = op.get_bind() + conn.execute(sa.text("CREATE EXTENSION IF NOT EXISTS intarray;")) + + op.create_table( + "project_member", + sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column( + "role", + postgresql.ENUM("reader", "editor", "writer", "owner", name="project_role"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["project_id"], + ["project.id"], + name=op.f("fk_project_member_project_id_project"), + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["user_id"], + ["user.id"], + name=op.f("fk_project_member_user_id_user"), + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint( + "project_id", "user_id", name=op.f("pk_project_member") + ), + ) + + op.add_column("project", sa.Column("public", sa.Boolean(), nullable=True)) + + data_upgrade() + + op.alter_column("project", "public", nullable=False) + op.create_index(op.f("ix_project_public"), "project", ["public"], unique=False) + op.drop_table("project_access") + + +def downgrade(): + op.create_table( + "project_access", + sa.Column("public", sa.BOOLEAN(), autoincrement=False, nullable=True), + sa.Column( + "owners", + postgresql.ARRAY(sa.INTEGER()), + server_default=sa.text("'{}'::integer[]"), + autoincrement=False, + nullable=True, + ), + sa.Column( + "readers", + postgresql.ARRAY(sa.INTEGER()), + server_default=sa.text("'{}'::integer[]"), + autoincrement=False, + nullable=True, + ), + sa.Column( + "writers", + postgresql.ARRAY(sa.INTEGER()), + server_default=sa.text("'{}'::integer[]"), + autoincrement=False, + nullable=True, + ), + sa.Column("project_id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column( + "editors", + postgresql.ARRAY(sa.INTEGER()), + server_default=sa.text("'{}'::integer[]"), + autoincrement=False, + nullable=True, + ), + sa.ForeignKeyConstraint( + ["project_id"], + ["project.id"], + name="fk_project_access_project_id_project", + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("project_id", name="pk_project_access"), + ) + + data_downgrade() + + op.create_index( + "ix_project_access_writers", + "project_access", + ["writers"], + unique=False, + postgresql_using="gin", + ) + op.create_index( + "ix_project_access_readers", + "project_access", + ["readers"], + unique=False, + postgresql_using="gin", + ) + op.create_index( + "ix_project_access_public", "project_access", ["public"], unique=False + ) + op.create_index( + "ix_project_access_project_id", "project_access", ["project_id"], unique=False + ) + op.create_index( + "ix_project_access_owners", + "project_access", + ["owners"], + unique=False, + postgresql_using="gin", + ) + op.create_index( + "ix_project_access_editors", + "project_access", + ["editors"], + unique=False, + postgresql_using="gin", + ) + + op.drop_index(op.f("ix_project_public"), table_name="project") + op.drop_column("project", "public") + op.drop_table("project_member") + conn = op.get_bind() + conn.execute(sa.text("DROP TYPE project_role;")) + + +def data_upgrade(): + conn = op.get_bind() + conn.execute( + sa.text( + """ + WITH members AS ( + SELECT + project_id AS project_id, + unnest(owners) AS user_id, + 'owner' AS role + FROM project_access + UNION + SELECT + project_id AS project_id, + unnest(writers - owners) AS user_id, + 'writer' AS role + FROM project_access + UNION + SELECT + project_id AS project_id, + unnest(editors - writers - owners) AS user_id, + 'editor' AS role + FROM project_access + UNION + SELECT + project_id AS project_id, + unnest(readers - editors - writers - owners) AS user_id, + 'reader' AS role + FROM project_access + ) + INSERT INTO project_member (project_id, user_id, role) + SELECT project_id, user_id, role::project_role FROM members; + """ + ) + ) + + conn.execute( + sa.text( + """ + UPDATE project p + SET + public = coalesce(pa.public, 'false') + FROM project_access pa + WHERE pa.project_id = p.id; + """ + ) + ) + + +def data_downgrade(): + conn = op.get_bind() + # recreate 1:1 relationship for project + conn.execute( + sa.text( + """ + INSERT INTO project_access (project_id, public) + SELECT id, coalesce(public, 'false') FROM project; + """ + ) + ) + + # update access fields from assigned roles + conn.execute( + sa.text( + """ + WITH members AS ( + WITH agg AS ( + SELECT + project_id, + array_agg(user_id) AS users_ids, + role + FROM + project_member + GROUP BY project_id, role + ) + SELECT + p.id AS project_id, + COALESCE(o.users_ids, '{}'::INT[]) AS owners, + COALESCE(o.users_ids || w.users_ids, '{}'::INT[]) AS writers, + COALESCE(o.users_ids || w.users_ids || e.users_ids, '{}'::INT[]) AS editors, + COALESCE(o.users_ids || w.users_ids || e.users_ids || r.users_ids, '{}'::INT[]) AS readers + FROM project p + LEFT OUTER JOIN (SELECT * FROM agg WHERE role = 'owner') AS o ON p.id = o.project_id + LEFT OUTER JOIN (SELECT * FROM agg WHERE role = 'reader') AS r ON p.id = r.project_id + LEFT OUTER JOIN (SELECT * FROM agg WHERE role = 'editor') AS e ON p.id = e.project_id + LEFT OUTER JOIN (SELECT * FROM agg WHERE role = 'writer') AS w ON p.id = w.project_id + ) + UPDATE project_access pa + SET + owners = m.owners, + writers = m.writers, + editors = m.editors, + readers = m.readers + FROM members m + WHERE m.project_id = pa.project_id; + """ + ) + )