Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions api/app/rbac_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,24 @@
}


# WARNING: This static RBAC dictionary is legacy scaffolding. Data policies
# and role grants are intended to be dynamically generated via the ODRL
# engine. Do not extend this map for new use cases — treat it as a
# placeholder pending that migration, and update it here (not by
# redeclaring it in individual endpoint files) if it must change before
# then.
#
# Maps the application-layer role to its sensorthings RLS policy function.
# Administrator is intentionally absent — admins bypass RLS by privilege,
# not by policy.
POLICY_FN_MAP = {
"viewer": "sensorthings.viewer_policy",
"editor": "sensorthings.editor_policy",
"obs_manager": "sensorthings.obs_manager_policy",
"sensor": "sensorthings.sensor_policy",
}


def validate_rbac_role(role: str) -> str:
clean_role = role.strip().lower()
if clean_role not in VALID_RBAC_ROLES:
Expand Down
1 change: 1 addition & 0 deletions api/app/v1/endpoints/create/data_array_observation.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
set_commit,
update_datastream_last_foi_id,
)
from app.v1.endpoints.functions import set_role

v1 = APIRouter()

Expand Down
19 changes: 18 additions & 1 deletion api/app/v1/endpoints/create/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from app import HOSTNAME, POSTGRES_PORT_WRITE, SUBPATH, VERSION
from app.db.asyncpg_db import get_pool, get_pool_w
from app.oauth import get_current_user
from app.rbac_roles import get_db_role_for_rbac, validate_rbac_role
from app.rbac_roles import POLICY_FN_MAP, get_db_role_for_rbac, validate_rbac_role
from app.utils.utils import pg_quote_ident, pg_quote_literal, validate_username
from app.v1.endpoints.functions import insert_commit, set_role
from asyncpg.exceptions import (
Expand All @@ -41,6 +41,8 @@
"role": "viewer", # viewer, editor, obs_manager, sensor, qc, custom
}

# POLICY_FN_MAP is the single source of truth — imported from rbac_roles.py.
# Do not redeclare it here; update rbac_roles.POLICY_FN_MAP instead.

@v1.api_route(
"/Users",
Expand Down Expand Up @@ -150,6 +152,9 @@ async def create_user(
if current_user is not None:
await connection.execute("RESET ROLE;")

# Capture app_role before get_db_role_for_rbac() to use
# for RLS policy dispatch below (fixes Issue #28).
app_role = payload["role"]
db_role = get_db_role_for_rbac(payload["role"])

await connection.execute(
Expand All @@ -167,6 +172,18 @@ async def create_user(
)
)

# Auto-create the default RLS policy for the new user.
# Policy functions already exist in the DB (istsos_auth.sql).
# Administrator role bypasses RLS by privilege, not policy.
policy_fn = POLICY_FN_MAP.get(app_role)
if policy_fn:
policyname = f"{user['username']}_default"
await connection.execute(
f"SELECT {policy_fn}($1, $2);",
[user["username"]],
policyname,
)

return Response(status_code=status.HTTP_201_CREATED)

except UniqueViolationError:
Expand Down
16 changes: 15 additions & 1 deletion api/app/v1/endpoints/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,27 @@
_PG_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")


def validate_role_identifier(username: str) -> str:
def _validate_role_identifier(username: str) -> str:
"""Validate that *username* is a safe PostgreSQL identifier.

asyncpg does not support $1 placeholders for SET ROLE identifiers —
PostgreSQL's SET ROLE only accepts a literal role name. We therefore
validate the identifier before interpolating it into the query string.

Raises:
ValueError: if the username does not match a plain PG identifier.
"""
if not isinstance(username, str) or not _PG_IDENTIFIER_RE.match(username):
raise ValueError("Invalid role identifier")
return username


async def set_role(connection, current_user):
"""Switch the current session role to *current_user['username']*.

The username is validated against ``_PG_IDENTIFIER_RE`` before use.
Uses ``pg_quote_ident`` to safely quote the identifier for the query.
"""
async with connection.transaction():
username = validate_role_identifier(current_user["username"])
query = f"SET ROLE {pg_quote_ident(username)};"
Expand Down
86 changes: 86 additions & 0 deletions api/tests/test_rbac_set_role_safety.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import asyncio
import inspect
import os
import sys
from pathlib import Path

import pytest

API_DIR = str(Path(__file__).resolve().parents[1])
if API_DIR not in sys.path:
sys.path.insert(0, API_DIR)

os.environ.setdefault("ISTSOS_ADMIN", "admin")
os.environ.setdefault("ISTSOS_ADMIN_PASSWORD", "secret")
os.environ.setdefault("POSTGRES_HOST", "localhost")
os.environ.setdefault("POSTGRES_PORT", "5432")
os.environ.setdefault("POSTGRES_DB", "istsos")
os.environ.setdefault("POSTGRES_USER", "admin")
os.environ.setdefault("SECRET_KEY", "test_secret_key_1234567890")
os.environ.setdefault("ALGORITHM", "HS256")

import app.v1.endpoints.create.data_array_observation as dao
from app.v1.endpoints.functions import set_role


class _Tx:
async def __aenter__(self):
return self

async def __aexit__(self, exc_type, exc, tb):
return False


class _Conn:
def __init__(self):
self.executed = []

def transaction(self):
return _Tx()

async def execute(self, query):
self.executed.append(query)


@pytest.mark.parametrize(
"username, expected_query",
[
("alice", 'SET ROLE "alice";'),
("user_1", 'SET ROLE "user_1";'),
],
)
def test_set_role_allows_safe_identifiers(username, expected_query):
conn = _Conn()

async def _run():
await set_role(conn, {"username": username})

asyncio.run(_run())
assert conn.executed == [expected_query]


@pytest.mark.parametrize(
"username",
[
'attacker"; RESET ROLE; --',
"bad-user",
"1starts_with_digit",
"user space",
"",
],
)
def test_set_role_rejects_unsafe_identifiers(username):
conn = _Conn()

async def _run():
await set_role(conn, {"username": username})

with pytest.raises(ValueError, match="Invalid role identifier"):
asyncio.run(_run())
assert conn.executed == []


def test_data_array_observation_uses_shared_set_role_helper():
src = inspect.getsource(dao.data_array_observation)
assert 'query.format(username=current_user["username"])' not in src
assert "await set_role(conn, current_user)" in src
Loading