Skip to content

fix(rbac): auto-create RLS policy on user creation#185

Open
KinshukSS2 wants to merge 1 commit into
istSOS:mainfrom
KinshukSS2:feat/week1-rbac-auto-policy-safe-setrole
Open

fix(rbac): auto-create RLS policy on user creation#185
KinshukSS2 wants to merge 1 commit into
istSOS:mainfrom
KinshukSS2:feat/week1-rbac-auto-policy-safe-setrole

Conversation

@KinshukSS2

Copy link
Copy Markdown
Contributor

PR Description

Summary

POST /Users now automatically attaches the user's default RLS policy in the same transaction as user creation. No separate POST /Policies call is required.


What changed

api/app/v1/endpoints/create/user.py

Added _POLICY_FN_MAP (module-level constant) and a policy dispatch block after the GRANT statement:

_POLICY_FN_MAP = {
    "viewer":      "sensorthings.viewer_policy",
    "editor":      "sensorthings.editor_policy",
    "obs_manager": "sensorthings.obs_manager_policy",
    "sensor":      "sensorthings.sensor_policy",
}

After GRANT, app_role is captured before get_db_role_for_rbac() remaps it, then:

policy_fn = _POLICY_FN_MAP.get(app_role)

if policy_fn:
    await connection.execute(
        f"SELECT {policy_fn}($1, $2);",
        [user["username"]],
        f"{user['username']}_default",
    )

Notes

  • Administrator role is intentionally skipped — admins bypass RLS by privilege.
  • Policy functions already exist in the DB (istsos_auth.sql). No migration needed.
  • The call is inside the existing transaction, so failure rolls back atomically.

api/app/v1/endpoints/functions.py

Docstrings added to:

  • _validate_role_identifier()
  • set_role()

api/tests/test_rls_policy_creation.py (new, 12 tests)

Covers:

  • Correct function dispatch per role
  • Administrator exclusion
  • {username}_default naming convention
  • users_ passed as list[str] (maps to text[])

api/tests/test_rbac_set_role_safety.py (new, 8 tests)

Covers:

  • set_role() identifier validation
  • Safe identifiers pass
  • Injection payloads raise ValueError
  • data_array_observation confirmed to use shared helper

Context

The set_role() SQL injection fix was already merged upstream (cbbcd78) before this PR.

The unique change here is the auto-policy dispatch in create_user().


Testing

python -m pytest api/tests/test_rls_policy_creation.py api/tests/test_rbac_set_role_safety.py -v

Result

20 passed, 0 failed

Closes Issue istSOS#28 — eliminates two-step user provisioning.

After POST /Users, a newly created user had no RLS policy and could not
access any data until an administrator separately called POST /Policies.
This commit fixes that by automatically calling the appropriate policy
function inside the same transaction as user creation.

Changes:
- api/app/v1/endpoints/create/user.py
  * Add module-level _POLICY_FN_MAP (viewer/editor/obs_manager/sensor).
  * Capture app_role before get_db_role_for_rbac() remaps it, so the
    correct policy function can be dispatched.
  * After GRANT, call sensorthings.<role>_policy([username], policyname)
    with policyname = '{username}_default'. Administrator is skipped —
    admins bypass RLS by privilege, not by policy.
  * Policy functions already exist in the DB (istsos_auth.sql); no
    migration required.

- api/app/v1/endpoints/functions.py
  * Add docstrings to _validate_role_identifier() and set_role().

- api/app/v1/endpoints/create/data_array_observation.py
  * Import shared set_role() helper (was already using the correct
    upstream version; this import makes the dependency explicit).

- api/tests/test_rls_policy_creation.py (new)
  * Tests: correct policy function per role, administrator exclusion,
    naming convention, users_ as text[].

- api/tests/test_rbac_set_role_safety.py (new)
  * Tests: identifier validation, injection rejection, shared helper
    usage in data_array_observation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant