Skip to content

Commit

Permalink
Task/DES-1996 restrict project deletion (#100)
Browse files Browse the repository at this point in the history
* Add check for admin/creator

* Add decorator and use at delete endpoint

* Remove unused methods

* Fix flake8

* Add create/admin info and check before project deletion

* Add logging statement

* Add test

* Fix linting errors

* Add migration

* Use DesignSafe to get users and their admin status

* Add env to gitignore

* Add unit tests and update services

* Fix linting error

* Update refresh_observable_projects

* Fix linting issues

* Add requests-mock to requirements.txt

* Rework migration so that existing project users are admins

* Rework migration

* Use clone to create new namepace model

* Add model for user payload

* Remove some dev logging

* Remove unused import

* Fix user_payload model
  • Loading branch information
nathanfranklin authored Aug 31, 2022
1 parent a54c97d commit 99ef290
Show file tree
Hide file tree
Showing 21 changed files with 552 additions and 200 deletions.
3 changes: 3 additions & 0 deletions geoapi/custom/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from geoapi.custom.designsafe.project_users import get_system_users

custom_system_user_retrieval = {"DESIGNSAFE": get_system_users}
Empty file.
33 changes: 33 additions & 0 deletions geoapi/custom/designsafe/project_users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from urllib.parse import quote
from geoapi.log import logging

logger = logging.getLogger(__name__)


def get_system_users(tenant_id, jwt, system_id: str):
"""
Get systems users based on the DesignSafe project's co-pis and pis.
:param tenant_id: tenant id
:param jwt: jwt of a user
:param system_id: str
:return: list of users with admin status
"""
from geoapi.utils.agave import service_account_client, SystemUser, get_default_system_users

if not system_id.startswith("project-"):
return get_default_system_users(tenant_id, jwt, system_id)

uuid = system_id[len("project-"):]
client = service_account_client(tenant_id=tenant_id)
resp = client.get(quote(f'/projects/v2/{uuid}/'))
resp.raise_for_status()
project = resp.json()["value"]
users = []
if "pi" in project:
users.append(SystemUser(username=project["pi"], admin=True))
for u in project["coPis"]:
users.append(SystemUser(username=u, admin=True))
for u in project["teamMembers"]:
users.append(SystemUser(username=u, admin=False))
return users
38 changes: 38 additions & 0 deletions geoapi/migrations/versions/dc0c2f6ba473_.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
"""empty message
Revision ID: dc0c2f6ba473
Revises: 0a10e4e1ea0c
Create Date: 2022-08-02 02:54:09.836110
"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'dc0c2f6ba473'
down_revision = '0a10e4e1ea0c'
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.add_column('projects_users', sa.Column('admin', sa.Boolean(), nullable=True))
op.add_column('projects_users', sa.Column('creator', sa.Boolean(), nullable=True))

# Update existing rows so that everyone is an admin
op.execute("UPDATE projects_users SET admin = true")
op.execute("UPDATE projects_users SET creator = false")

op.alter_column('projects_users', 'admin', nullable=False)
op.alter_column('projects_users', 'creator', nullable=False)

# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column('projects_users', 'creator')
op.drop_column('projects_users', 'admin')
# ### end Alembic commands ###
11 changes: 9 additions & 2 deletions geoapi/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Column, Integer, String,
ForeignKey, Boolean, DateTime
)
from sqlalchemy.orm import relationship
from sqlalchemy.orm import relationship, backref
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.sql import func
from geoapi.db import Base
Expand All @@ -12,8 +12,15 @@
class ProjectUser(Base):
__tablename__ = 'projects_users'

user_id = Column(Integer, ForeignKey('users.id', ondelete="CASCADE"), primary_key=True, )
user_id = Column(Integer, ForeignKey('users.id', ondelete="CASCADE"), primary_key=True)
project_id = Column(Integer, ForeignKey('projects.id', ondelete="CASCADE"), primary_key=True)
creator = Column(Boolean, nullable=False, default=False)
admin = Column(Boolean, nullable=False, default=False)
project = relationship('Project', backref=backref('project_users', cascade="all, delete-orphan"))
user = relationship('User')

def __repr__(self):
return f'<ProjectUser(user_id={self.user_id}, project_id={self.project_id}, admin={self.admin}, creator={self.creator})>'


class Project(Base):
Expand Down
2 changes: 1 addition & 1 deletion geoapi/models/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ class User(Base):
back_populates='users', lazy="joined")

def __repr__(self):
return '<User(uname={})>'.format(self.username)
return '<User(uname={}, id={})>'.format(self.username, self.id)
39 changes: 26 additions & 13 deletions geoapi/routes/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
from geoapi.services.point_cloud import PointCloudService
from geoapi.services.projects import ProjectsService
from geoapi.tasks import external_data, streetview
from geoapi.utils.decorators import jwt_decoder, project_permissions_allow_public, project_permissions, project_feature_exists, \
project_point_cloud_exists, project_point_cloud_not_processing, check_access_and_get_project, is_anonymous, not_anonymous
from geoapi.utils.decorators import (jwt_decoder, project_permissions_allow_public, project_permissions,
project_feature_exists, project_point_cloud_exists,
project_point_cloud_not_processing, check_access_and_get_project, is_anonymous,
not_anonymous, project_admin_or_creator_permissions)


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -59,9 +61,17 @@
'system_path': fields.String(),
})

project_response = api.clone('Project', project, {
'deletable': fields.Boolean()
})

user = api.model('User', {
'id': fields.Integer(),
'username': fields.String(required=True)
'username': fields.String(required=True),
})

user_payload = api.clone('UserPayload', user, {
'admin': fields.Boolean(default=False)
})

task = api.model('Task', {
Expand Down Expand Up @@ -153,7 +163,7 @@ class ProjectsListing(Resource):
@api.doc(id="getProjects",
description='Get a listing of projects',
parser=parser)
@api.marshal_with(project, as_list=True)
@api.marshal_with(project_response, as_list=True)
def get(self):
u = request.current_user
query = self.parser.parse_args()
Expand All @@ -173,7 +183,7 @@ def get(self):
@api.doc(id="createProject",
description='Create a new project')
@api.expect(project)
@api.marshal_with(project)
@api.marshal_with(project_response)
@not_anonymous
def post(self):
u = request.current_user
Expand All @@ -187,14 +197,16 @@ class ProjectResource(Resource):

@api.doc(id="getProjectById",
description="Get the metadata about a project")
@api.marshal_with(project)
@api.marshal_with(project_response)
@project_permissions_allow_public
def get(self, projectId: int):
return ProjectsService.get(project_id=projectId)
u = request.current_user
logger.info("Get metadata project:{} for user:{}".format(projectId, u.username))
return ProjectsService.get(project_id=projectId, user=u)

@api.doc(id="deleteProject",
description="Delete a project, all associated features and metadata. THIS CANNOT BE UNDONE")
@project_permissions
@project_admin_or_creator_permissions
def delete(self, projectId: int):
u = request.current_user
logger.info("Delete project:{} for user:{}".format(projectId,
Expand All @@ -203,7 +215,7 @@ def delete(self, projectId: int):

@api.doc(id="updateProject",
description="Update metadata about a project")
@api.marshal_with(project)
@api.marshal_with(project_response)
@project_permissions
def put(self, projectId: int):
u = request.current_user
Expand All @@ -222,26 +234,27 @@ def get(self, projectId: int):
return ProjectsService.getUsers(projectId)

@api.doc(id="addUser",
description="Add a user to the project. This allows full access to the project, "
description="Add a user to the project. If `admin` is True then user has full access to the project, "
"including deleting the entire thing so chose carefully")
@api.expect(user)
@api.expect(user_payload)
@project_permissions
def post(self, projectId: int):
payload = request.json
username = payload["username"]
admin = payload.get("admin", False)
logger.info("Add user:{} to project:{} for user:{}".format(
username,
projectId,
request.current_user.username))
ProjectsService.addUserToProject(projectId, username)
ProjectsService.addUserToProject(projectId, username, admin)
return "ok"


@api.route('/<int:projectId>/users/<username>/')
class ProjectUserResource(Resource):
@api.doc(id="removeUser",
description="Remove a user from a project")
@project_permissions
@project_admin_or_creator_permissions
def delete(self, projectId: int, username: str):
logger.info("Delete user:{} from project:{} for user:{}".format(
username,
Expand Down
54 changes: 36 additions & 18 deletions geoapi/services/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
from typing import List, Optional

from sqlalchemy import desc
from geoapi.models import Project, User, ObservableDataProject
from geoapi.models import Project, ProjectUser, User, ObservableDataProject
from geoapi.db import db_session
from sqlalchemy.sql import select, text
from sqlalchemy.exc import IntegrityError
from geoapi.services.users import UserService
from geoapi.utils.agave import AgaveUtils, get_system_users
from geoapi.utils.assets import get_project_asset_dir
from geoapi.utils.users import is_anonymous
from geoapi.tasks.external_data import import_from_agave
from geoapi.log import logging
from geoapi.exceptions import ApiException, ObservableProjectAlreadyExists
Expand Down Expand Up @@ -47,6 +48,10 @@ def create(data: dict, user: User) -> Project:
db_session.add(project)
db_session.commit()

project.project_users[0].creator = True
db_session.add(project)
db_session.commit()

return project

@staticmethod
Expand Down Expand Up @@ -75,7 +80,7 @@ def makeObservable(proj: Project,

users = get_system_users(proj.tenant_id, user.jwt, proj.system_id)
logger.info("Updating project:{} to have the following users: {}".format(name, users))
project_users = [UserService.getOrCreateUser(u, tenant=proj.tenant_id) for u in users]
project_users = [UserService.getOrCreateUser(u.username, tenant=proj.tenant_id) for u in users]
proj.users = project_users

obs.project = proj
Expand All @@ -98,28 +103,39 @@ def list(user: User) -> List[Project]:
:param user: User
:return: List[Project]
"""
projects = db_session.query(Project) \
.join(User.projects)\
.filter(User.username == user.username)\

projects_and_project_user = db_session.query(Project, ProjectUser) \
.join(ProjectUser) \
.filter(User.username == user.username) \
.filter(User.tenant_id == user.tenant_id) \
.order_by(desc(Project.created))\
.filter(ProjectUser.user_id == user.id) \
.order_by(desc(Project.created)) \
.all()

return projects
for p, u in projects_and_project_user:
setattr(p, 'deletable', u.admin or u.creator)

return [p for p, _ in projects_and_project_user]

@staticmethod
def get(project_id: Optional[int] = None, uuid: Optional[str] = None) -> Project:
def get(project_id: Optional[int] = None, uuid: Optional[str] = None, user: User = None) -> Project:
"""
Get the metadata associated with a project
:param project_id: int
:param uid: str
:return: Project
"""
if project_id is not None:
return db_session.query(Project).get(project_id)
project = db_session.query(Project).get(project_id)
elif uuid is not None:
return db_session.query(Project).filter(Project.uuid == uuid).first()
raise ValueError("project_id or uid is required")
project = db_session.query(Project).filter(Project.uuid == uuid).first()
else:
raise ValueError("project_id or uid is required")

if project and user and not is_anonymous(user):
project_user = db_session.query(ProjectUser).filter(Project.id == project.id).filter(User.id == user.id).first()
setattr(project, 'deletable', project_user.admin or project_user.creator)
return project

@staticmethod
def getFeatures(projectId: int, query: dict = None) -> object:
Expand Down Expand Up @@ -257,10 +273,7 @@ def delete(user: User, projectId: int) -> dict:
:param projectId: int
:return:
"""
proj = db_session.query(Project).get(projectId)

db_session.delete(proj)
db_session.commit()
db_session.query(Project).filter(Project.id == projectId).delete()

assets_folder = get_project_asset_dir(projectId)
try:
Expand All @@ -270,20 +283,25 @@ def delete(user: User, projectId: int) -> dict:
return {"status": "ok"}

@staticmethod
def addUserToProject(projectId: int, username: str) -> None:
def addUserToProject(projectId: int, username: str, admin: bool) -> None:
"""
Add a user to a project
:param projectId: int
:param username: string
:param admin: bool
:return:
"""

# TODO: Add TAS integration
proj = db_session.query(Project).get(projectId)
user = UserService.getOrCreateUser(username, proj.tenant_id)
proj.users.append(user)
db_session.commit()

project_user = db_session.query(ProjectUser).filter(Project.id == projectId)\
.filter(User.id == user.id).first()
project_user.admin = admin
db_session.commit()

@staticmethod
def getUsers(projectId: int) -> List[User]:
proj = db_session.query(Project).get(projectId)
Expand All @@ -296,7 +314,7 @@ def getUser(projectId: int, username: str) -> User:
return user

@staticmethod
def removeUserFromProject(projectId: int, username: str, ) -> None:
def removeUserFromProject(projectId: int, username: str) -> None:
"""
Remove a user from a Project.
:param projectId: int
Expand Down
26 changes: 11 additions & 15 deletions geoapi/services/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ def create(username: str, tenant: str, jwt: str = None) -> User:
db_session.commit()
return u

@staticmethod
def checkUser(username: str) -> bool:
# TODO: Add in TAS check
pass

@staticmethod
def getOrCreateUser(username: str, tenant: str) -> User:
user = UserService.getUser(username, tenant)
Expand Down Expand Up @@ -52,16 +47,17 @@ def canAccess(user: User, projectId: int) -> bool:
return False

@staticmethod
def setJWT(user: User, token: str) -> None:
user.jwt = token
db_session.commit()

@staticmethod
def setMapillaryToken(user: User, token: str) -> None:
user.google_jwt = token
db_session.commit()
def is_admin_or_creator(user: User, projectId: int) -> bool:
up = db_session.query(ProjectUser) \
.join(Project) \
.filter(ProjectUser.user_id == user.id) \
.filter(Project.tenant_id == user.tenant_id) \
.filter(ProjectUser.project_id == projectId).first()
if up:
return up.admin or up.creator
return False

@staticmethod
def setGoogleToken(user: User, token: str) -> None:
user.mapillary_jwt = token
def setJWT(user: User, token: str) -> None:
user.jwt = token
db_session.commit()
Loading

0 comments on commit 99ef290

Please sign in to comment.