Role Re-assignment for Active Users#190
Open
KinshukSS2 wants to merge 5 commits into
Open
Conversation
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.
- Add auth_provider and external_sub_id columns to sensorthings."User"
via idempotent migration (001_identity_linking.sql) with a partial
unique index on (auth_provider, external_sub_id) WHERE auth_provider
IS NOT NULL, so local password users are completely unaffected.
- Introduce PENDING_ROLE sentinel in rbac_roles.py. The 'pending' state
is intentionally absent from VALID_RBAC_ROLES so it can never be
assigned through the public API; existing role validation is unchanged.
- Gate pending accounts in get_current_user() (oauth.py): after the DB
lookup, any user with role='pending' immediately receives HTTP 403
'Account pending admin activation' before any SET ROLE or handler
body is reached.
- Add oidc_user_crud.py with create_pending_oidc_user() and
get_user_by_provider_sub(). The insert function hardcodes role to
PENDING_ROLE and contains zero DDL (no CREATE ROLE / CREATE USER),
giving new OIDC accounts zero PostgreSQL footprint until activation.
- Add POST /Users/{id}/activate endpoint (activate_user.py), restricted
to administrators. Runs UPDATE role, CREATE ROLE NOLOGIN IN ROLE,
GRANT, and RLS policy assignment inside a single transaction so a
failed step leaves the user still 'pending' with no partial state.
- Register activate_user router in api.py inside the AUTHORIZATION guard.
Local password users (POST /Users) are completely unaffected; no changes
were made to create/user.py.
Relates-to: GSoC 2026 Identity Linking architecture
- Add PasswordUpdateRequest Pydantic v2 schema (models/password.py)
enforcing: min 12 chars, at least 1 uppercase, at least 1 digit.
Violations surface as HTTP 422 before any DB is touched.
- Add update_local_password() CRUD function (db/password_crud.py):
1. Fetch user row by ID → 404 if missing.
2. OIDC guard: block auth_provider IS NOT NULL users with HTTP 400
'External identities cannot update passwords locally'.
3. Verify current_password via asyncpg.connect() (PostgreSQL auth layer)
→ 401 on InvalidPasswordError. No Python-side passlib used.
4. Execute ALTER USER <username> WITH ENCRYPTED PASSWORD <new_password>
using pg_quote_ident / pg_quote_literal to prevent injection.
- Add PATCH /Users/{id}/password endpoint (update/password.py):
owner-or-admin guard; returns 204 No Content on success.
- Register update_password router in api.py inside AUTHORIZATION guard.
- Add test_password_update.py (9 tests, all pass, no live DB required):
schema: valid, too-short, no-uppercase, no-digit
crud: 404, 400 OIDC block, 401 wrong password, 204 ALTER USER issued
endpoint: 403 non-owner/non-admin guard
Depends on: feat/identity-linking-jit-provisioning (requires auth_provider column)
…le-JWT fix
- Add RoleUpdateRequest Pydantic v2 schema (models/role.py):
delegates to validate_rbac_role(); blocks 'administrator' (bootstrap-only)
and 'pending' (internal state) with HTTP 422 before any DB is touched.
Docstring explains the security boundary explicitly.
- Add update_user_role() CRUD function (db/role_crud.py):
All mutations run inside a single asyncpg transaction (FOR UPDATE lock):
1. 404 if user not found.
2. 400 if user is in 'pending' waiting room.
3. No-op early return if current_role == new_role (no DDL issued).
4. 409 if demoting the last administrator (lockout guard).
5. UPDATE sensorthings."User" SET role = new_role.
6. REVOKE <old_pg_group_role> / GRANT <new_pg_group_role> only when the
underlying PostgreSQL group role changes (e.g. viewer→obs_manager).
viewer→editor shares the same 'user' PG role — no DDL issued.
pg_quote_ident used for all identifier interpolation.
- Add PATCH /Users/{id}/role endpoint (update/role.py):
administrator-only guard at router layer; returns 204 No Content.
- Register update_role router in api.py inside AUTHORIZATION guard.
- Add comment to get_current_user() in oauth.py documenting that role is
fetched live from the DB on every request (not from the JWT payload),
eliminating stale-JWT vulnerabilities after role changes. No logic change.
- Add test_role_reassignment.py (12 tests, all pass, no live DB needed):
schema: valid, administrator/pending/unknown blocked
crud: 404, 400 pending, no-op, 409 last-admin, REVOKE+GRANT, no-DDL
endpoint: 403 non-admin guard
Depends on: feat/password-updates (stacked)
Member
|
The endpoint to change user role should not be extremely safe: how do you
guarantee very high security access to admin roles?
Maxi
*Massimiliano Cannata*
Professore SUPSI in ingegneria Geomatica
Responsabile settore Geomatica
*Istituto scienze della Terra*
Dipartimento ambiente costruzione e design
Scuola universitaria professionale della Svizzera italiana
Campus Mendrisio, Via Flora Ruchat-Roncati 15
CH – 6850 Mendrisio
Tel. +41 (0)58 666 62 14
Fax +41 (0)58 666 62 09
***@***.***
*www.supsi.ch/ist <http://www.supsi.ch/ist>*
Il lun 8 giu 2026, 20:11 syntax.sculptor ***@***.***> ha
scritto:
… General Role Re-assignment for Active Users
*Stacked on:* feat/password-updates — set that branch as the base, or
merge it first.
Full dependency chain: upstream/main ← #185 (Week 1) ←
feat/identity-linking-jit-provisioning ← feat/password-updates ← *this PR*
Why
Administrators needed a safe API path to change an existing user's
application role (e.g. demoting a viewer to a sensor account, or
promoting an editor to obs_manager). This PR adds PATCH /Users/{id}/role
with four hard guards to ensure the operation can never leave the system in
an inconsistent or locked-out state.
Key architectural decisions
*REVOKE not DROP ROLE*
PostgreSQL's DROP ROLE fails if the user owns any database objects. The
correct approach is REVOKE <old_pg_group_role> FROM <username> followed
by GRANT <new_pg_group_role> TO <username>. This matches the existing
pattern in update/user.py and is safe regardless of object ownership.
*administrator is intentionally blocked as a target role*
administrator is a bootstrap-only role created by the DB init script (
istsos_auth.sql) using the ISTSOS_ADMIN env var. It is absent from
VALID_RBAC_ROLES by design. The API cannot be used to promote a user to
administrator — that is a DBA/infrastructure concern. The schema validator
rejects it with HTTP 422 before any DB is touched.
*Stale-JWT vulnerability is already patched (comment-only change to
oauth.py)*
get_current_user() already fetches the user's role live from the DB on
every authenticated request (get_user_from_db()), not from the JWT
payload. A demoted user's next request will immediately see the new role
without needing token rotation. A clarifying comment was added to lock this
behaviour in and prevent future "optimisation" regressions.
*No unnecessary DDL*
viewer and editor both map to the PostgreSQL group role user; obs_manager
and sensor both map to sensor. When the underlying PG group role is
unchanged (e.g. viewer → editor), only the User.role column is updated —
no REVOKE/GRANT is issued.
What changed
File Change
api/app/models/role.py New RoleUpdateRequest Pydantic v2 schema —
delegates to validate_rbac_role(), blocks administrator and pending with
422
api/app/db/role_crud.py New update_user_role() — atomic transaction with
4 guards + UPDATE + conditional REVOKE/GRANT
api/app/v1/endpoints/update/role.py New PATCH /Users/{id}/role —
admin-only guard, 204 No Content
api/app/v1/api.py Router registered inside the AUTHORIZATION guard
api/app/oauth.py Comment-only change: documents that role is fetched live
from DB (no logic change)
api/tests/test_role_reassignment.py 12 tests — all pass without a live
database Edge cases handled
Case Behaviour
Target role is administrator 422 Unprocessable Entity (Pydantic rejects
before DB)
Target role is pending 422 Unprocessable Entity (Pydantic rejects before
DB)
User not found 404 Not Found
User is in pending waiting room 400 Bad Request — activate first
current_role == new_role Early return, no DB writes, no DDL
Demoting the last administrator 409 Conflict — "Cannot demote the last
administrator"
viewer → obs_manager (different PG group role) UPDATE + REVOKE user +
GRANT sensor
viewer → editor (same PG group role user) UPDATE only — no REVOKE/GRANT
Non-admin caller 403 Forbidden — before any DB contact How to test
# 1. Run unit tests (12 tests, no live DB needed)cd api && .venv/bin/python -m pytest tests/test_role_reassignment.py -v# → 12 passed
# 2. Happy path — admin promotes viewer to obs_manager
USER_ID=<id of a viewer user>
curl -X PATCH http://localhost:8018/v1.1/Users/$USER_ID/role \
-H "Authorization: Bearer $ADMIN_TOKEN" \
-H "Content-Type: application/json" \
-d '{"role": "obs_manager"}'# → 204 No Content
# Verify in postgres:# SELECT rolname FROM pg_auth_members am JOIN pg_roles r ON am.roleid = r.oid# JOIN pg_roles m ON am.member = m.oid WHERE m.rolname = '<username>';# → should now show 'sensor' (the PG group role for obs_manager)
# 3. Same underlying PG role (viewer → editor) — no DDL issued
curl -X PATCH http://localhost:8018/v1.1/Users/$USER_ID/role \
-H "Authorization: Bearer $ADMIN_TOKEN" \
-H "Content-Type: application/json" \
-d '{"role": "editor"}'# → 204 No Content (only User.role column changed)
# 4. Last-admin lockout
curl -X PATCH http://localhost:8018/v1.1/Users/<admin_id>/role \
-H "Authorization: Bearer $ADMIN_TOKEN" \
-H "Content-Type: application/json" \
-d '{"role": "viewer"}'# → 409 Conflict: "Cannot demote the last administrator"
# 5. Pending user block
curl -X PATCH http://localhost:8018/v1.1/Users/<pending_id>/role \
-H "Authorization: Bearer $ADMIN_TOKEN" \
-H "Content-Type: application/json" \
-d '{"role": "viewer"}'# → 400 Bad Request: "Cannot reassign role for a pending user"
# 6. Non-admin caller
curl -X PATCH http://localhost:8018/v1.1/Users/$USER_ID/role \
-H "Authorization: Bearer $VIEWER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"role": "editor"}'# → 403 Forbidden
# 7. Stale-JWT validation — demote a user, then use their old token immediately# After the PATCH succeeds, call any endpoint with the demoted user's token.# → Their role will be the new (demoted) role with no re-login required.
------------------------------
You can view, comment on, or merge this pull request online at:
#190
Commit Summary
- 5cd3222
<5cd3222>
fix(rbac): auto-create RLS policy on user creation
- 1097abe
<1097abe>
feat(auth): implement identity linking & JIT provisioning for OIDC users
- ee2b264
<ee2b264>
feat(auth): password update endpoint with OIDC guard & input validation
- 24ff194
<24ff194>
feat(auth): role re-assignment endpoint with last-admin lockout & stale-JWT
fix
File Changes
(19 files <https://github.com/istSOS/istSOS4/pull/190/files>)
- *A* api/app/db/oidc_user_crud.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-b5cbb84664e71debe1f1a03587e8efa1b3577c7f8cb4b4860f81de691d6c9573>
(131)
- *A* api/app/db/password_crud.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-ffea5e25b398e11ffc2fe0d90209122e8280b96981c75a7a06e20707379bed96>
(149)
- *A* api/app/db/role_crud.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-6f948c9c336abaa8f282a8a57cdcf5978376799aceeb179e504f87890da2203a>
(208)
- *A* api/app/models/password.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-204693f4b3d080133e0a91f31e132998aad3164752d83b53e3785e7074f66a43>
(56)
- *A* api/app/models/role.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-3a3b6e73745163b6e2a4e9f5dbb6f51fc1f6793d7e54ebcef2dae00bc5e70ee8>
(64)
- *M* api/app/oauth.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-45f7727f3c69d2fc63e33c1a7860f7991f96648f832b866c2f734b8bcf6ff4a8>
(23)
- *M* api/app/rbac_roles.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-4f64e921cc910292eb2a34f7e610d6485eee87bafdc8ecc00b453e21fe96e534>
(17)
- *M* api/app/v1/api.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-67aed7ce745d81ed94f970eff70852e72c9e84f62a6b05c0d3515e478a9f78b8>
(6)
- *A* api/app/v1/endpoints/create/activate_user.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-4138a6e3f3f19f80643d4e0f066e4dd84cae38d9b7cd02dbbde74e646a9d30cd>
(235)
- *M* api/app/v1/endpoints/create/data_array_observation.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-9bd33fdf2c1100f98cdd41c28e380ab7ab4098a5a2159ba28803217818529396>
(1)
- *M* api/app/v1/endpoints/create/user.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-732d66cf15425eb39573c8b7c41ba289ea87b2cf6020aec76c49c36495104215>
(23)
- *M* api/app/v1/endpoints/functions.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-15b2f9a554a197c2d240a605a82e04df2b27000bd3508ba77819d5d3c4a7443c>
(14)
- *A* api/app/v1/endpoints/update/password.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-3d9b1d3baacee3169de96d99e6060822a2f67ed5d9b4b096ace18490c76564d7>
(74)
- *A* api/app/v1/endpoints/update/role.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-3f09f30b91e87d73e20a1f3e36b8cf2df9987297de303f9a5c13877d35122d6b>
(70)
- *A* api/tests/test_password_update.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-60a36c66afc728f783f4a3990510926e3ad3367ec6d9ebaa0242657fcfb7d9f7>
(294)
- *A* api/tests/test_rbac_set_role_safety.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-e4b501d51cf28b164ebff8f26f9757666cb9b665d1ef75ccf16d3b47044db492>
(86)
- *A* api/tests/test_rls_policy_creation.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-6e8464e1b0a00e6c5c45b9796ecc4996f12ea078fdfbf09de972e945fe880bd5>
(206)
- *A* api/tests/test_role_reassignment.py
<https://github.com/istSOS/istSOS4/pull/190/files#diff-017564f9008279ff787c0949294e7d15f2f1ecfb83930d117daa4967deb02ea7>
(287)
- *A* database/migrations/001_identity_linking.sql
<https://github.com/istSOS/istSOS4/pull/190/files#diff-ae14d467f66768268152efd47e2a35d1e332eb864fa31d49e572ae761c028e2b>
(58)
Patch Links:
- https://github.com/istSOS/istSOS4/pull/190.patch
- https://github.com/istSOS/istSOS4/pull/190.diff
—
Reply to this email directly, view it on GitHub
<#190?email_source=notifications&email_token=ADC2FTCTDXNIBPNDSXU22JT4636T5A5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTQMRVG43TINJVG6THEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVRTG633UMVZF6Y3MNFRWW>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADC2FTEQUUFIO7OPBIE7B3D4636T5AVCNFSM6AAAAACZ7QZNP2VHI2DSMVQWIX3LMV43ASLTON2WKOZUGYYTKNBSG42TMOI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/ADC2FTCQIPVNDW5P7UWXJL34636T5A5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTQMRVG43TINJVG6THEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVJTG633UMVZF62LPOM>
and Android
<https://github.com/notifications/mobile/android/ADC2FTH277SF4HLPIVXTV234636T5A5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTQMRVG43TINJVG6THEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVZTG633UMVZF6YLOMRZG62LE>.
Download it today!
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
KinshukSS2
added a commit
to KinshukSS2/istSOS4
that referenced
this pull request
Jun 10, 2026
Remove all PostgreSQL DDL (CREATE USER, REVOKE, GRANT, ALTER USER) from the role and user management flows. istSOS users are application-level entities; the backend connects via a single master service account. Changes ------- - rbac_roles.py: add shared POLICY_FN_MAP constant — single source of truth for RLS policy dispatch. Eliminates silent divergence between create/user.py and activate_user.py. - create/user.py: remove CREATE USER … WITH ENCRYPTED PASSWORD and GRANT DDL. Import POLICY_FN_MAP from rbac_roles.py. Passwords are now stored as bcrypt hashes in sensorthings."User".password via a parameterised UPDATE (handled separately in password_crud.py). - create/activate_user.py: remove CREATE ROLE … NOLOGIN and GRANT DDL. Activation is now a pure UPDATE on the role column plus an RLS policy call. Import POLICY_FN_MAP from rbac_roles.py. - role_crud.py (already committed): last-admin lockout now locks ALL administrator rows with SELECT … FOR UPDATE before counting, fixing the race condition where two concurrent demotions could both pass a count of 2 and leave zero administrators. - test_issue7_exception_handling.py: deleted (superseded by the new test_role_reassignment.py test suite which covers all guard paths). No REVOKE, GRANT, CREATE ROLE, or ALTER USER statements remain in any role or user management code path. Refs: istSOS#190
…dentials
Users are strictly application-level entities managed via sensorthings."User".
The backend connects to PostgreSQL through a single master service account;
individual users have no PostgreSQL login roles.
- Add shared POLICY_FN_MAP in rbac_roles.py as single source of truth for
RLS policy dispatch (used by create/user.py and activate_user.py)
- Role reassignment (PATCH /Users/{id}/role) is a pure UPDATE on User.role;
last-admin lockout locks all admin rows via SELECT … FOR UPDATE before
counting to prevent concurrent demotion race condition
- User activation (POST /Users/{id}/activate) updates User.role and applies
the corresponding RLS policy function — no PostgreSQL DDL
- User creation stores bcrypt hash in User.password via parameterised UPDATE
Removed: CREATE USER, CREATE ROLE, REVOKE, GRANT, ALTER USER DDL.
Refs: istSOS#190
f80e262 to
3392cb5
Compare
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Role Re-assignment for Active Users
Summary
Implements administrator-only role reassignment for active istSOS users, along with the supporting OIDC user provisioning, admin activation flow, and password self-service endpoint. This PR is part of the RBAC GSoC project and builds on top of
feat/password-updates.Users in istSOS are application-level entities stored in
sensorthings."User". The backend connects to PostgreSQL via a single master service account; all credential and role management operates on the application layer.New Endpoints
PATCH/Users/{user_id}/rolePATCH/Users/{user_id}/passwordPOST/Users/{user_id}/activateArchitecture
Role reassignment is a pure application-state mutation:
Passwords are stored as bcrypt hashes in
sensorthings."User"."password"and verified withpasslibon the Python side.OIDC users are provisioned in a
pendingwaiting room with zero database footprint until an administrator explicitly activates them with a target role.Security Design
PATCH /Users/{id}/role — Four independent layers
JWT verification —
get_current_user()validates the Bearer token on every request (HTTP 401 on failure).Live role fetch — The caller's role is fetched directly from the database on each request, not read from the JWT payload. A demoted administrator loses access on their next request with no token rotation needed.
Administrator-only guard — Checked at the endpoint layer before any database interaction (HTTP 403 for non-admins).
administratoris not API-assignable — The Pydantic schema rejects it at deserialization time (HTTP 422). Promotion toadministratoris a deploy-time DBA operation viaistsos_auth.sql; there is no application code path that can writerole = 'administrator'.Last-Administrator Lockout (Race-Safe)
When demoting an administrator, all administrator rows are locked inside a single transaction before the count is taken:
This prevents two concurrent demotion requests from both reading a count of 2, both passing the guard, and both succeeding — which would leave the system with zero administrators.
Pending-User Gate
OIDC users in the
pendingstate are blocked globally inget_current_user()(HTTP 403) before reaching any endpoint handler. They cannot access any resource until an administrator activates them.Guards on PATCH /Users/{id}/role
pendingroleis alreadynew_rolerole = 'administrator'in request bodyShared Constants
POLICY_FN_MAP— the mapping from application role to PostgreSQL RLS policy function — lives inapp/rbac_roles.pyas the single source of truth and is imported wherever RLS policies need to be applied.Database Migrations
001_identity_linking.sqlauth_provider,external_sub_idtosensorthings."User"; partial unique index on(auth_provider, external_sub_id)002_user_password_column.sqlpassword VARCHAR(255) DEFAULT NULLtosensorthings."User"for bcrypt hashesFiles
Test Results
All tests run without a live database. Key assertions:
$1argument — never interpolated.REVOKE,GRANT,CREATE,ALTERare asserted absent from every SQL statement issued.Manual Testing
Dependency / Merge Order