fix: trailing slashes from endpoints, fix check_access for backend ro…

…les (#1893)

* fix(backend): do not redirect trailing slashes on api as it's ambiguous

* fix: replace all instances of api trailing slash with no slash

* fix(backend): check_access correctly check for role hierarchy

* refactor: backend

reorder routes to avoid path param capture

* fix(backend): auth routes and roles, check_access function

.env.example

@@ -19,7 +19,7 @@ OSM_CLIENT_ID=${OSM_CLIENT_ID}
 OSM_CLIENT_SECRET=${OSM_CLIENT_SECRET}
 OSM_URL=${OSM_URL:-"https://www.openstreetmap.org"}
 OSM_SCOPE=${OSM_SCOPE:-'["read_prefs", "send_messages"]'}
-OSM_LOGIN_REDIRECT_URI="http${FMTM_DOMAIN:+s}://${FMTM_DOMAIN:-127.0.0.1:7051}/osmauth/"
+OSM_LOGIN_REDIRECT_URI="http${FMTM_DOMAIN:+s}://${FMTM_DOMAIN:-127.0.0.1:7051}/osmauth"
 OSM_SECRET_KEY=${OSM_SECRET_KEY}
 
 ### S3 File Storage ###

INSTALL.md

@@ -99,8 +99,8 @@ To properly configure your FMTM project, you will need to create keys for OSM.
 
 2. Register your FMTM instance to OAuth 2 applications.
 
-   Put your login redirect url as `http://127.0.0.1:7051/osmauth/` if running locally,
-   or for production replace with https://{YOUR_DOMAIN}/osmauth/
+   Put your login redirect url as `http://127.0.0.1:7051/osmauth` if running locally,
+   or for production replace with https://{YOUR_DOMAIN}/osmauth
 
    > Note: `127.0.0.1` is required for debugging instead of `localhost`
    > due to OSM restrictions.

scripts/gen-env.sh

@@ -305,7 +305,7 @@ set_domains() {
 set_osm_credentials() {
     pretty_echo "OSM OAuth2 Credentials"
 
-    redirect_uri="http${FMTM_DOMAIN:+s}://${FMTM_DOMAIN:-127.0.0.1:7051}/osmauth/"
+    redirect_uri="http${FMTM_DOMAIN:+s}://${FMTM_DOMAIN:-127.0.0.1:7051}/osmauth"
 
     echo "App credentials are generated from your OSM user profile."
     echo

src/backend/app/auth/auth_routes.py

@@ -49,7 +49,7 @@
 )
 
 
-@router.get("/osm-login/")
+@router.get("/osm-login")
 async def login_url(osm_auth=Depends(init_osm_auth)):
     """Get Login URL for OSM Oauth Application.
 
@@ -69,7 +69,7 @@ async def login_url(osm_auth=Depends(init_osm_auth)):
     return JSONResponse(content=login_url, status_code=HTTPStatus.OK)
 
 
-@router.get("/callback/")
+@router.get("/callback")
 async def callback(
     request: Request, osm_auth: Annotated[AuthUser, Depends(init_osm_auth)]
 ) -> JSONResponse:
@@ -135,7 +135,7 @@ async def callback(
         ) from e
 
 
-@router.get("/logout/")
+@router.get("/logout")
 async def logout():
     """Reset httpOnly cookie to sign out user."""
     response = Response(status_code=HTTPStatus.OK)
@@ -178,15 +178,21 @@ async def get_or_create_user(
                     profile_img = EXCLUDED.profile_img
                 RETURNING id, username, profile_img, role
             )
+
             SELECT
                 u.id, u.username, u.profile_img, u.role,
+
+                -- Aggregate the organisation IDs managed by the user
                 array_agg(
                     DISTINCT om.organisation_id
-                ) FILTER (WHERE om.organisation_id IS NOT NULL) as orgs_managed,
+                ) FILTER (WHERE om.organisation_id IS NOT NULL) AS orgs_managed,
+
+                -- Aggregate project roles for the user, as project:role pairs
                 jsonb_object_agg(
                     ur.project_id,
                     COALESCE(ur.role, 'MAPPER')
-                ) FILTER (WHERE ur.project_id IS NOT NULL) as project_roles
+                ) FILTER (WHERE ur.project_id IS NOT NULL) AS project_roles
+
             FROM upserted_user u
             LEFT JOIN user_roles ur ON u.id = ur.user_id
             LEFT JOIN organisation_managers om ON u.id = om.user_id
@@ -229,7 +235,7 @@ async def get_or_create_user(
             ) from e
 
 
-@router.get("/me/", response_model=FMTMUser)
+@router.get("/me", response_model=FMTMUser)
 async def my_data(
     db: Annotated[Connection, Depends(db_conn)],
     current_user: Annotated[AuthUser, Depends(login_required)],

src/backend/app/auth/auth_schemas.py

@@ -16,10 +16,9 @@
 #
 """Pydantic models for Auth."""
 
-from typing import Any, Optional, TypedDict
+from typing import Optional, TypedDict
 
 from pydantic import BaseModel, ConfigDict, PrivateAttr, computed_field
-from pydantic.functional_validators import field_validator
 
 from app.db.enums import ProjectRole, UserRole
 from app.db.models import DbOrganisation, DbProject, DbUser
@@ -80,30 +79,5 @@ class FMTMUser(BaseModel):
     username: str
     profile_img: str
     role: UserRole
-    project_roles: Optional[dict[int, ProjectRole]] = {}
-    orgs_managed: Optional[list[int]] = []
-
-    @field_validator("role", mode="before")
-    @classmethod
-    def convert_user_role_str_to_ints(cls, role: Any) -> Optional[UserRole]:
-        """User role strings returned from db converted to enum integers."""
-        if not role:
-            return None
-        if isinstance(role, str):
-            return UserRole[role]
-        return role
-
-    @field_validator("project_roles", mode="before")
-    @classmethod
-    def convert_project_role_str_to_ints(
-        cls, roles: dict[int, Any]
-    ) -> Optional[dict[int, ProjectRole]]:
-        """User project strings returned from db converted to enum integers."""
-        if not roles:
-            return {}
-
-        first_value = next(iter(roles.values()), None)
-        if isinstance(first_value, str):
-            return {id: ProjectRole[role] for id, role in roles.items()}
-
-        return roles
+    project_roles: Optional[dict[int, ProjectRole]] = None
+    orgs_managed: Optional[list[int]] = None

src/backend/app/auth/roles.py

@@ -86,38 +86,59 @@ async def check_access(
     user_id = await get_uid(user)
 
     sql = """
+        WITH role_hierarchy AS (
+            SELECT 'MAPPER' AS role, 0 AS level
+            UNION ALL SELECT 'VALIDATOR', 1
+            UNION ALL SELECT 'FIELD_MANAGER', 2
+            UNION ALL SELECT 'ASSOCIATE_PROJECT_MANAGER', 3
+            UNION ALL SELECT 'PROJECT_MANAGER', 4
+        )
+
         SELECT *
         FROM users
         WHERE id = %(user_id)s
             AND (
                 CASE
-                    WHEN role = 'ADMIN' THEN true
-                    WHEN role = 'READ_ONLY' THEN false
+                    -- Simple check to see if ADMIN or blocked (READ_ONLY)
+                    WHEN role = 'ADMIN'::public.userrole THEN true
+                    WHEN role = 'READ_ONLY'::public.userrole THEN false
                     ELSE
+
+                        -- Check to see if user is org admin
                         EXISTS (
                             SELECT 1
                             FROM organisation_managers
                             WHERE organisation_managers.user_id = %(user_id)s
-                              AND
-                              organisation_managers.organisation_id = %(org_id)s
+                            AND organisation_managers.organisation_id = %(org_id)s
                         )
+
+                        -- Check to see if user has equal or greater than project role
                         OR EXISTS (
                             SELECT 1
                             FROM user_roles
+                            JOIN role_hierarchy AS user_role_h
+                                ON user_roles.role::public.projectrole
+                                    = user_role_h.role::public.projectrole
+                            JOIN role_hierarchy AS required_role_h
+                                ON %(role)s::public.projectrole
+                                    = required_role_h.role::public.projectrole
                             WHERE user_roles.user_id = %(user_id)s
-                              AND user_roles.project_id = %(project_id)s
-                              AND user_roles.role >= %(role)s
+                            AND user_roles.project_id = %(project_id)s
+                            AND user_role_h.level >= required_role_h.level
                         )
+
+                        -- Extract organisation id from project,
+                        -- then check to see if user is org admin
                         OR (
                             %(org_id)s IS NULL
                             AND EXISTS (
                                 SELECT 1
                                 FROM organisation_managers
-                                JOIN projects ON
-                                  projects.organisation_id
-                                    = organisation_managers.organisation_id
+                                JOIN projects
+                                    ON projects.organisation_id =
+                                        organisation_managers.organisation_id
                                 WHERE organisation_managers.user_id = %(user_id)s
-                                  AND projects.id = %(project_id)s
+                                AND projects.id = %(project_id)s
                             )
                         )
                 END

src/backend/app/config.py

@@ -234,7 +234,7 @@ def assemble_db_connection(cls, v: Optional[str], info: ValidationInfo) -> Any:
     # https://github.com/openstreetmap/operations/issues/951#issuecomment-1748717154
     OSM_URL: HttpUrlStr = "https://www.openstreetmap.org"
     OSM_SCOPE: list[str] = ["read_prefs", "send_messages"]
-    OSM_LOGIN_REDIRECT_URI: str = "http://127.0.0.1:7051/osmauth/"
+    OSM_LOGIN_REDIRECT_URI: str = "http://127.0.0.1:7051/osmauth"
 
     S3_ENDPOINT: str = "http://s3:9000"
     S3_ACCESS_KEY: Optional[str] = ""

src/backend/app/db/database.py

@@ -57,7 +57,7 @@ async def db_conn(request: Request) -> Connection:
     To use the connection in endpoints:
     -----------------------------------
 
-    @app.get("/something/")
+    @app.get("/something")
     async def do_stuff(db = Depends(db_conn)):
         async with db.cursor() as cursor:
             await cursor.execute("SELECT * FROM items")

src/backend/app/db/models.py

@@ -158,7 +158,7 @@ class DbUser(BaseModel):
     registered_at: Optional[AwareDatetime] = None
 
     # Relationships
-    project_roles: Optional[list[DbUserRole]] = None
+    project_roles: Optional[dict[int, ProjectRole]] = None  # project:role pairs
     orgs_managed: Optional[list[int]] = None
 
     @classmethod
@@ -168,13 +168,18 @@ async def one(cls, db: Connection, user_identifier: int | str) -> Self:
             sql = """
                 SELECT
                     u.*,
-                    array_agg(
-                        DISTINCT om.organisation_id
-                    ) FILTER (WHERE om.organisation_id IS NOT NULL) AS orgs_managed,
-                    jsonb_object_agg(
-                        ur.project_id,
-                        COALESCE(ur.role, 'MAPPER')
-                    ) FILTER (WHERE ur.project_id IS NOT NULL) AS project_roles
+
+                -- Aggregate organisation IDs managed by the user
+                array_agg(
+                    DISTINCT om.organisation_id
+                ) FILTER (WHERE om.organisation_id IS NOT NULL) AS orgs_managed,
+
+                -- Aggregate project roles for the user, as project:role pairs
+                jsonb_object_agg(
+                    ur.project_id,
+                    COALESCE(ur.role, 'MAPPER')
+                ) FILTER (WHERE ur.project_id IS NOT NULL) AS project_roles
+
                 FROM users u
                 LEFT JOIN user_roles ur ON u.id = ur.user_id
                 LEFT JOIN organisation_managers om ON u.id = om.user_id

src/backend/app/main.py

@@ -121,6 +121,8 @@ def get_application() -> FastAPI:
         debug=settings.DEBUG,
         lifespan=lifespan,
         root_path=settings.API_PREFIX,
+        # NOTE REST APIs should not have trailing slashes
+        redirect_slashes=False,
     )
 
     # Set custom logger