Skip to content

Commit

Permalink
feat: organisation approval and admin endpoints (#1126)
Browse files Browse the repository at this point in the history
* dependencies to cehck if user_exists

* endpoint to add organisation admin

* added approved field in organisation model

* fix: table name in migration file for organisations

* feat: organisations list api updated according to role

* feat: endpoint to approve organisations

* update: get organisation endpoint for filtering approval

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* added docstrings

* fix: pre-commit linting errors

* added docstring in user_deps file

* build: add default odk credentials to organisations

* build: merge migrations to organisations table

* refactor: fix linting errors

* refactor: remove subscription_tier field for orgs

* build: add public.organisation.approved field to base schema

* refactor: remove extra url field from DbOrganisation

* refactor: fix organizationModel dir --> organisation for import

* fix: remove router get_db global dependency (on routes)

* fix: use separate super_admin + check_super_admin deps

* fix: update org_admin to also allow super_admin

* refactor: remove missed log.warning from organisations endpoint

* fix: separate Depends from logic, working org approval

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: spwoodcock <[email protected]>
  • Loading branch information
3 people authored Jan 23, 2024
1 parent 67a099f commit 36ae5d4
Show file tree
Hide file tree
Showing 19 changed files with 281 additions and 48 deletions.
70 changes: 53 additions & 17 deletions src/backend/app/auth/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
and always return an AuthUser object in a standard format.
"""

from typing import Optional

from fastapi import Depends, HTTPException
from loguru import logger as log
from sqlalchemy.orm import Session
Expand All @@ -30,6 +32,7 @@
from app.db.database import get_db
from app.db.db_models import DbProject, DbUser, DbUserRoles, organisation_managers
from app.models.enums import HTTPStatus, ProjectRole, UserRole
from app.organisations.organisation_deps import check_org_exists
from app.projects.project_deps import get_project_by_id


Expand All @@ -45,24 +48,66 @@ async def get_uid(user_data: AuthUser) -> int:
)


async def check_super_admin(
db: Session,
user: [AuthUser, int],
) -> DbUser:
"""Database check to determine if super admin role."""
if isinstance(user, int):
user_id = user
else:
user_id = await get_uid(user)
return db.query(DbUser).filter_by(id=user_id, role=UserRole.ADMIN).first()


async def super_admin(
db: Session = Depends(get_db),
user_data: AuthUser = Depends(login_required),
db: Session = Depends(get_db),
) -> AuthUser:
"""Super admin role, with access to all endpoints."""
user_id = await get_uid(user_data)
super_admin = await check_super_admin(db, user_data)

match = db.query(DbUser).filter_by(id=user_id, role=UserRole.ADMIN).first()

if not match:
log.error(f"User ID {user_id} requested an admin endpoint, but is not admin")
if not super_admin:
log.error(
f"User {user_data.get('username')} requested an admin endpoint, "
"but is not admin"
)
raise HTTPException(
status_code=HTTPStatus.FORBIDDEN, detail="User must be an administrator"
)

return user_data


async def check_org_admin(
db: Session,
user: [AuthUser, int],
project: Optional[DbProject],
org_id: Optional[int],
) -> DbUser:
"""Database check to determine if org admin role."""
if isinstance(user, int):
user_id = user
else:
user_id = await get_uid(user)

if project:
org_id = db.query(DbProject).filter_by(id=project.id).first().organisation_id

# Check org exists
await check_org_exists(db, org_id)

# If user is admin, skip checks
if await check_super_admin(db, user):
return user

return (
db.query(organisation_managers)
.filter_by(organisation_id=org_id, user_id=user_id)
.first()
)


async def org_admin(
project: DbProject = Depends(get_project_by_id),
org_id: int = None,
Expand All @@ -77,19 +122,10 @@ async def org_admin(
detail="Both org_id and project_id cannot be passed at the same time",
)

user_id = await get_uid(user_data)

if project:
org_id = db.query(DbProject).filter_by(id=project.id).first().organisation_id

org_admin = (
db.query(organisation_managers)
.filter_by(organisation_id=org_id, user_id=user_id)
.first()
)
org_admin = await check_org_admin(db, user_data, project, org_id)

if not org_admin:
log.error(f"User ID {user_id} is not an admin for organisation {org_id}")
log.error(f"User {user_data} is not an admin for organisation {org_id}")
raise HTTPException(
status_code=HTTPStatus.FORBIDDEN,
detail="User is not organisation admin",
Expand Down
1 change: 0 additions & 1 deletion src/backend/app/central/central_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
router = APIRouter(
prefix="/central",
tags=["central"],
dependencies=[Depends(database.get_db)],
responses={404: {"description": "Not found"}},
)

Expand Down
2 changes: 1 addition & 1 deletion src/backend/app/db/db_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class DbOrganisation(Base):
description = Column(String)
url = Column(String)
type = Column(Enum(OrganisationType), default=OrganisationType.FREE, nullable=False)
# subscription_tier = Column(Integer)
approved = Column(Boolean, default=False)

managers = relationship(
DbUser,
Expand Down
51 changes: 47 additions & 4 deletions src/backend/app/organisations/organisation_crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
from sqlalchemy import update
from sqlalchemy.orm import Session

from app.auth.osm import AuthUser
from app.auth.roles import check_super_admin
from app.config import settings
from app.db import db_models
from app.models.enums import HTTPStatus
Expand All @@ -34,11 +36,15 @@
from app.s3 import add_obj_to_bucket


def get_organisations(
db: Session,
):
async def get_organisations(db: Session, current_user: AuthUser, is_approved: bool):
"""Get all orgs."""
return db.query(db_models.DbOrganisation).all()
super_admin = await check_super_admin(db, current_user)

if super_admin:
return db.query(db_models.DbOrganisation).filter_by(approved=is_approved).all()

# If user not admin, only show approved orgs
return db.query(db_models.DbOrganisation).filter_by(approved=True).all()


async def upload_logo_to_s3(
Expand Down Expand Up @@ -186,3 +192,40 @@ async def delete_organisation(
db.commit()

return Response(status_code=HTTPStatus.NO_CONTENT)


async def add_organisation_admin(
db: Session, user: db_models.DbUser, organisation: db_models.DbOrganisation
):
"""Adds a user as an admin to the specified organisation.
Args:
db (Session): The database session.
user (DbUser): The user model instance.
organisation (DbOrganisation): The organisation model instance.
Returns:
Response: The HTTP response with status code 200.
"""
log.info(f"Adding user ({user.id}) as org ({organisation.id}) admin")
# add data to the managers field in organisation model
organisation.managers.append(user)
db.commit()

return Response(status_code=HTTPStatus.OK)


async def approve_organisation(db, organisation):
"""Approves an oranisation request made by the user .
Args:
db: The database session.
organisation (DbOrganisation): The organisation model instance.
Returns:
Response: An HTTP response with the status code 200.
"""
log.info(f"Approving organisation ID {organisation.id}")
organisation.approved = True
db.commit()
return Response(status_code=HTTPStatus.OK)
46 changes: 38 additions & 8 deletions src/backend/app/organisations/organisation_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,58 @@
from app.models.enums import HTTPStatus


async def get_organisation_by_name(db: Session, org_name: str) -> DbOrganisation:
async def get_organisation_by_name(
db: Session, org_name: str, check_approved: bool = True
) -> DbOrganisation:
"""Get an organisation from the db by name.
Args:
db (Session): database session
org_name (int): id of the organisation
check_approved (bool): first check if the organisation is approved
Returns:
DbOrganisation: organisation with the given id
"""
return (
org_obj = (
db.query(DbOrganisation)
.filter(func.lower(DbOrganisation.name).like(func.lower(f"%{org_name}%")))
.first()
)
if org_obj and check_approved and org_obj.approved is False:
raise HTTPException(
status_code=HTTPStatus.NOT_FOUND,
detail=f"Organisation ({org_obj.id}) is not approved yet",
)
return org_obj


async def get_organisation_by_id(db: Session, org_id: int) -> DbOrganisation:
async def get_organisation_by_id(
db: Session, org_id: int, check_approved: bool = True
) -> DbOrganisation:
"""Get an organisation from the db by id.
Args:
db (Session): database session
org_id (int): id of the organisation
check_approved (bool): first check if the organisation is approved
Returns:
DbOrganisation: organisation with the given id
"""
return db.query(DbOrganisation).filter(DbOrganisation.id == org_id).first()
org_obj = db.query(DbOrganisation).filter_by(id=org_id).first()
if org_obj and check_approved and org_obj.approved is False:
raise HTTPException(
status_code=HTTPStatus.NOT_FOUND,
detail=f"Organisation {org_id} is not approved yet",
)
return org_obj


async def org_exists(
async def check_org_exists(
db: Session,
org_id: Union[str, int],
db: Session = Depends(get_db),
check_approved: bool = True,
) -> DbOrganisation:
"""Check if organisation name exists, else error.
Expand All @@ -76,11 +95,11 @@ async def org_exists(

if isinstance(org_id, int):
log.debug(f"Getting organisation by id: {org_id}")
db_organisation = await get_organisation_by_id(db, org_id)
db_organisation = await get_organisation_by_id(db, org_id, check_approved)

if isinstance(org_id, str):
log.debug(f"Getting organisation by name: {org_id}")
db_organisation = await get_organisation_by_name(db, org_id)
db_organisation = await get_organisation_by_name(db, org_id, check_approved)

if not db_organisation:
raise HTTPException(
Expand All @@ -90,3 +109,14 @@ async def org_exists(

log.debug(f"Organisation match: {db_organisation}")
return db_organisation


async def org_exists(
org_id: Union[str, int],
db: Session = Depends(get_db),
) -> DbOrganisation:
"""Wrapper for check_org_exists to be used as a route dependency.
Requires Depends from a route.
"""
return await check_org_exists(db, org_id)
42 changes: 37 additions & 5 deletions src/backend/app/organisations/organisation_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,29 @@
)
from sqlalchemy.orm import Session

from app.auth.osm import AuthUser, login_required
from app.auth.roles import org_admin, super_admin
from app.db import database
from app.db.db_models import DbOrganisation
from app.db.db_models import DbOrganisation, DbUser
from app.organisations import organisation_crud, organisation_schemas
from app.organisations.organisation_deps import org_exists
from app.organisations.organisation_deps import check_org_exists, org_exists
from app.users.user_deps import user_exists_in_db

router = APIRouter(
prefix="/organisation",
tags=["organisation"],
dependencies=[Depends(database.get_db)],
responses={404: {"description": "Not found"}},
)


@router.get("/", response_model=list[organisation_schemas.OrganisationOut])
def get_organisations(
async def get_organisations(
db: Session = Depends(database.get_db),
current_user: AuthUser = Depends(login_required),
approved: bool = True,
) -> list[organisation_schemas.OrganisationOut]:
"""Get a list of all organisations."""
return organisation_crud.get_organisations(db)
return await organisation_crud.get_organisations(db, current_user, approved)


@router.get("/{org_id}", response_model=organisation_schemas.OrganisationOut)
Expand Down Expand Up @@ -85,3 +89,31 @@ async def delete_organisations(
):
"""Delete an organisation."""
return await organisation_crud.delete_organisation(db, organisation)


@router.post("/approve/")
async def approve_organisation(
org_id: int,
db: Session = Depends(database.get_db),
current_user: AuthUser = Depends(super_admin),
):
"""Approve the organisation request made by the user.
The logged in user must be super admin to perform this action .
"""
org_obj = await check_org_exists(db, org_id, check_approved=False)
return await organisation_crud.approve_organisation(db, org_obj)


@router.post("/add_admin/")
async def add_new_organisation_admin(
db: Session = Depends(database.get_db),
organisation: DbOrganisation = Depends(org_exists),
user: DbUser = Depends(user_exists_in_db),
current_user: AuthUser = Depends(org_admin),
):
"""Add a new organisation admin.
The logged in user must be either the owner of the organisation or a super admin.
"""
return await organisation_crud.add_organisation_admin(db, user, organisation)
1 change: 0 additions & 1 deletion src/backend/app/projects/project_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
router = APIRouter(
prefix="/projects",
tags=["projects"],
dependencies=[Depends(database.get_db)],
responses={404: {"description": "Not found"}},
)

Expand Down
1 change: 0 additions & 1 deletion src/backend/app/submissions/submission_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
router = APIRouter(
prefix="/submission",
tags=["submission"],
dependencies=[Depends(database.get_db)],
responses={404: {"description": "Not found"}},
)

Expand Down
1 change: 0 additions & 1 deletion src/backend/app/tasks/tasks_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
router = APIRouter(
prefix="/tasks",
tags=["tasks"],
dependencies=[Depends(database.get_db)],
responses={404: {"description": "Not found"}},
)

Expand Down
Loading

0 comments on commit 36ae5d4

Please sign in to comment.