Skip to content

feat(auth): public self-registration endpoint for restricted dataset access - #199

Open
KinshukSS2 wants to merge 18 commits into
istSOS:mainfrom
KinshukSS2:feat/restricted-registration
Open

feat(auth): public self-registration endpoint for restricted dataset access#199
KinshukSS2 wants to merge 18 commits into
istSOS:mainfrom
KinshukSS2:feat/restricted-registration

Conversation

@KinshukSS2

Copy link
Copy Markdown
Contributor

feat(auth): public self-registration endpoint for restricted dataset access

Summary

Adds the public POST /Register endpoint so external users can self-register for restricted STAC dataset access. Accounts land in role='pending' with zero privileges — an admin must explicitly activate them via the existing POST /Users/{id}/activate. This closes the gap in the ODRL pipeline: the RESTRICTED_REQUEST audit action type existed in the schema but had no producer.


Files Changed

File Change
api/app/models/register_request.py New. ContactInfo (6 optional strings) + RestrictedRegistrationRequest Pydantic schema
api/app/v1/endpoints/create/register_request.py New. POST /Register handler — hashing, JSONB merge, atomic DB transaction
api/app/v1/api.py Modified. Import + include_router under AUTHORIZATION guard

No schema changes. No new dependencies. No DDL issued.


Key Implementation Details (for reviewer)

No auth required — endpoint is intentionally public. Security boundary is the DB: role='pending' has no RLS privileges.

Non-blocking bcryptpwd_context.hash is offloaded via asyncio.to_thread to avoid stalling the event loop.

Single atomic transaction — all three writes share one conn.transaction():

  1. INSERT INTO "User" … RETURNING id
  2. UPDATE "User" SET uri = '/Users(' || id || ')'
  3. INSERT INTO "AuditLog" via log_audit_event()

If the audit INSERT fails, the user row rolls back too — no orphaned accounts without a paper trail.

JSONB serialisationcontact_info.model_dump() + explanation merged into one dict → json.dumps()$N::jsonb cast. Follows the pattern in audit_crud.py (asyncpg does not auto-serialise dicts).


Test Evidence

Assertion Result
POST /Register → HTTP 201, returns {id, status: "pending"}
User.role = 'pending', status = 'active'
User.uri = '/Users(3)' (RETURNING id)
User.password is bcrypt hash ($2b$12$…), not plain text
User.contact JSONB contains all contact_info fields + explanation
AuditLog row: action_type=RESTRICTED_REQUEST, correct actor_id, dataset_id, odrl_policy_id
Duplicate username → HTTP 409
Missing required fields → HTTP 422 (Pydantic)

Full DB output in pr3_test_report.md.


How to Test Locally

curl -s -X POST http://localhost:8018/istsos4/v1.1/Register \
  -H "Content-Type: application/json" \
  -d '{
    "username": "reviewer_test",
    "password": "testpass123",
    "dataset_id": "stac-collection-xyz",
    "odrl_policy_id": "policy-456",
    "explanation": "Reviewer smoke test.",
    "contact_info": {"company": "SUPSI"}
  }'
# Expect: HTTP 201  {"id": ..., "status": "pending"}

Then verify in DB:

SELECT id, role, uri, password, contact FROM sensorthings."User" WHERE username = 'reviewer_test';
SELECT action_type, dataset_id, odrl_policy_id FROM sensorthings."AuditLog" WHERE actor_id = <id>;

@KinshukSS2 KinshukSS2 changed the title eat(auth): public self-registration endpoint for restricted dataset access feat(auth): public self-registration endpoint for restricted dataset access Jul 16, 2026
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)
…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
BREAKING: set_role() now maps app-layer roles to PG group roles via
SET LOCAL ROLE instead of SET ROLE <username>. All 72 RESET ROLE
instances deleted across 41 endpoint files.

Refactor 1 — functions.py::set_role():
  - Maps viewer/editor → 'user', sensor/obs_manager → 'sensor', etc.
  - SET LOCAL ROLE (transaction-scoped, auto-reverts on COMMIT/ROLLBACK)
  - Removed inner connection.transaction() nested savepoint
  - Eliminated entire RESET ROLE call class (pool leak prevention)

Refactor 2 — password_crud.py:
  - Removed asyncpg.connect() credential verification
  - Removed ALTER USER … WITH ENCRYPTED PASSWORD DDL
  - Modern path: passlib/bcrypt verify + UPDATE User.password
  - Legacy fallback: get_auth_connection() for NULL password JIT migration

Refactor 3 — role_crud.py:
  - Removed REVOKE/GRANT DDL block (lines 167-196)
  - Role reassignment is pure UPDATE sensorthings."User" SET role
  - Removed _ADMIN_PG_ROLE, DB_ROLE_BY_RBAC_ROLE, pg_quote_ident imports

Refactor 4 — Test suite alignment:
  - test_set_role_sql_safety: asserts SET LOCAL ROLE + group role mapping
  - test_rbac_set_role_safety: parametrised 5 app roles → PG group roles
  - test_password_update: asserts UPDATE + bcrypt hash (not ALTER USER)
  - test_role_reassignment: asserts UPDATE only (not REVOKE/GRANT)
  - test_policy_role_switch: asserts SET LOCAL ROLE + zero RESET ROLE

49 files changed, 298 insertions(+), 442 deletions(-)
63/63 tests passing.
update/user.py lines 119-135 contained live REVOKE/GRANT statements
that were missed in the PR5 cleanup pass. These operated on individual
usernames as PostgreSQL role identifiers, which no longer exist under
the app-layer credential model.

Remove the entire DDL block. Role changes are reflected in the
sensorthings."User".role UPDATE above; set_role() maps the new role
to its PG group role dynamically at request time.
## Summary
Completes the authentication foundation required before building any
new access-control features.  Two self-contained changes:

  1. database/migrations/002_add_password_status.sql
  2. api/app/oauth.py — rewrite authenticate_user()

---

## 1. Migration: 002_add_password_status.sql

Adds two columns to sensorthings."User" inside the existing
custom.authorization guard block (mirrors migration 001):

  • password VARCHAR(255) DEFAULT NULL
      Stores the passlib/bcrypt hash of the user's local credential.
      NULL signals a legacy (pre-migration) account; on next login the
      pg_authid JIT fallback fires and backfills this column so
      subsequent logins never touch pg_authid again.

  • status VARCHAR(50) DEFAULT 'active'
      Account lifecycle flag.  'active' is the default so all existing
      rows are completely unaffected.  Future values: 'suspended',
      'deleted'.  Application-layer enforcement is a follow-up task.

Both ALTER TABLE statements use ADD COLUMN IF NOT EXISTS so the
migration is idempotent and safe to re-run against instances that
already received the columns via a prior manual hotfix.

---

## 2. authenticate_user() — bcrypt-first with JIT pg_authid fallback

Replaces the legacy pg_authid-only authentication flow with a
three-step process:

  Step 1 — Fetch from sensorthings."User"
    SELECT id, username, role, password WHERE username = $1.
    Unknown users return None immediately; we never attempt pg_authid
    for users not present in the application-layer table.

  Step 2 — Bcrypt verify (modern path, password IS NOT NULL)
    pwd_context.verify() is dispatched via asyncio.to_thread() to
    avoid blocking the event loop with bcrypt's intentional CPU cost.
    Correct hash → return user dict.  Wrong hash → return None.

  Step 3 — pg_authid JIT fallback (legacy path, password IS NULL)
    get_auth_connection() attempts a raw asyncpg.connect() to let
    PostgreSQL validate via pg_authid.
    • Failure → return None.
    • Success → asyncio.to_thread(pwd_context.hash, password) computes
      the bcrypt hash then writes it via the write pool
      (POSTGRES_PORT_WRITE pattern from role_crud / password_crud).
      Backfill failure is caught, logged, and swallowed — login still
      succeeds (best-effort JIT migration, never blocks the user).

  Circular import resolution
    pwd_context lives in password_crud.py which already imports
    get_auth_connection from oauth.py.  Both symbols are imported
    lazily inside the function body to break the cycle, following the
    identical pattern already used in password_crud.py line 115.

---

Breaking changes: none.
Existing users with password IS NULL continue to log in as before;
they are transparently migrated on first login.
Users with a bcrypt hash no longer require a pg_authid LOGIN role.
…ation errors

passlib 1.7.4 is incompatible with bcrypt >= 4.1 due to a wrap-bug
detection test that passes a >72-byte secret, which bcrypt 4+ rejects
with ValueError.  Pin bcrypt==4.0.1 — the last version that works with
passlib 1.7.4 without triggering the 72-byte guard.

A previous refactor left dangling 'if current_user is not None:' guards
with no body before the return statement, causing Python to raise
IndentationError at import time and crashing the entire API process.

Removed the dead guard in each case — the 404 / not-found response
should always be returned regardless of auth context.  The outer
exception handler already enforces auth context where needed.

Files fixed:
  api/app/v1/endpoints/create/bulk_observation.py  (ValueError catch)
  api/app/v1/endpoints/delete/observation.py        (404 guard)
  api/app/v1/endpoints/update/historical_location.py (404 guard)
  api/app/v1/endpoints/update/location.py           (404 guard)
  api/app/v1/endpoints/update/observation.py        (404 guard)
  api/app/v1/endpoints/update/observed_property.py  (404 guard)
  api/app/v1/endpoints/update/thing.py              (404 guard)
BREAKING: set_role() now maps app-layer roles to PG group roles via
SET LOCAL ROLE instead of SET ROLE <username>. All 72 RESET ROLE
instances deleted across 41 endpoint files.

Refactor 1 — functions.py::set_role():
  - Maps viewer/editor → 'user', sensor/obs_manager → 'sensor', etc.
  - SET LOCAL ROLE (transaction-scoped, auto-reverts on COMMIT/ROLLBACK)
  - Removed inner connection.transaction() nested savepoint
  - Eliminated entire RESET ROLE call class (pool leak prevention)

Refactor 2 — password_crud.py:
  - Removed asyncpg.connect() credential verification
  - Removed ALTER USER … WITH ENCRYPTED PASSWORD DDL
  - Modern path: passlib/bcrypt verify + UPDATE User.password
  - Legacy fallback: get_auth_connection() for NULL password JIT migration

Refactor 3 — role_crud.py:
  - Removed REVOKE/GRANT DDL block (lines 167-196)
  - Role reassignment is pure UPDATE sensorthings."User" SET role
  - Removed _ADMIN_PG_ROLE, DB_ROLE_BY_RBAC_ROLE, pg_quote_ident imports

Refactor 4 — Test suite alignment:
  - test_set_role_sql_safety: asserts SET LOCAL ROLE + group role mapping
  - test_rbac_set_role_safety: parametrised 5 app roles → PG group roles
  - test_password_update: asserts UPDATE + bcrypt hash (not ALTER USER)
  - test_role_reassignment: asserts UPDATE only (not REVOKE/GRANT)
  - test_policy_role_switch: asserts SET LOCAL ROLE + zero RESET ROLE

49 files changed, 298 insertions(+), 442 deletions(-)
63/63 tests passing.
## Summary
Implements the audit trail foundation for STAC/ODRL access governance.
Two self-contained additions stacked on feat/auth-foundation-phase0:

  1. database/migrations/003_audit_log.sql
  2. api/app/db/audit_crud.py

---

## 1. Migration: 003_audit_log.sql

Creates sensorthings."AuditLog" inside the custom.authorization guard
block (SET ROLE "administrator" for DDL, RESET ROLE before privileges).

Schema:
  id             UUID PRIMARY KEY DEFAULT gen_random_uuid()
                 gen_random_uuid() provided by pgcrypto (already loaded)
  actor_id       BIGINT FK → sensorthings."User"(id) ON DELETE SET NULL
                 Nullable for anonymous events (e.g. PUBLIC_READ)
  action_type    VARCHAR(50) NOT NULL CHECK IN (
                   'PUBLIC_READ', 'RESTRICTED_REQUEST', 'ADMIN_APPROVAL')
  dataset_id     TEXT  — STAC dataset identifier (nullable)
  odrl_policy_id TEXT  — ODRL policy reference (nullable)
  payload        JSONB DEFAULT NULL — arbitrary event metadata
  created_at     TIMESTAMPTZ NOT NULL DEFAULT NOW()

Indexes:
  idx_auditlog_action_type  (btree) — filter by event category
  idx_auditlog_actor_id     (btree) — filter by user

Append-only enforcement (after RESET ROLE, following istsos_auth.sql
lines 398-444 pattern):
  REVOKE UPDATE, DELETE from: administrator, user, sensor, guest, qc
  GRANT INSERT to: user, sensor

---

## 2. Helper: audit_crud.py

async def log_audit_event(conn, action_type, actor_id, dataset_id,
                           odrl_policy_id, payload) -> None

  * Accepts a pre-acquired asyncpg connection so callers can include
    the audit INSERT in the same transaction as the action being logged.
  * Serialises payload dict via json.dumps() + $5::jsonb cast to avoid
    asyncpg TypeError on dict → JSONB (oidc_user_crud.py pattern).
  * None payload passes through as SQL NULL unchanged.
  * Exports three action-type constants (AUDIT_ACTION_PUBLIC_READ,
    AUDIT_ACTION_RESTRICTED_REQUEST, AUDIT_ACTION_ADMIN_APPROVAL)
    that match the DB CHECK constraint exactly.

---

Breaking changes: none.
No existing tables or application code are modified.
….sql

The 'qc' role only exists when AUTHORIZATION was enabled at DB init time.
Replace flat REVOKE statements with a DO block that checks pg_roles
before each REVOKE/GRANT so the migration is idempotent and safe on
any deployment configuration.
Implements POST /Register (Path B, Step 4 of the architecture plan).

Changes
-------
* api/app/models/register_request.py
  - ContactInfo BaseModel: 6 optional string fields (domain, company,
    address, telephone, telegram, linkedin).
  - RestrictedRegistrationRequest BaseModel: username, password,
    dataset_id, odrl_policy_id, explanation, contact_info.

* api/app/v1/endpoints/create/register_request.py
  - Public POST /Register handler (no auth dependency).
  - bcrypt hash via asyncio.to_thread (non-blocking).
  - Merges explanation into contact JSONB blob.
  - Single atomic transaction: INSERT User (pending/active),
    UPDATE uri, INSERT AuditLog (RESTRICTED_REQUEST).
  - Structured error handling: 409 UniqueViolation, 503, 504, 500.

* api/app/v1/api.py
  - Import + include_router for register_request.v1 under the
    AUTHORIZATION guard alongside other user-management routers.
@KinshukSS2
KinshukSS2 force-pushed the feat/restricted-registration branch from fc5595e to f887118 Compare July 21, 2026 13:36
RestrictedRegistrationRequest.password had no validation at all — any
string, including a single character, passed. Adds a shared
validate_password_strength() helper (app/validators.py) enforcing:
  - at least 8 characters
  - at least 1 digit
  - at least 1 symbol

Kept in a separate shared module rather than inline in this model so
the same rule can be applied to PasswordUpdateRequest.new_password
(feat/password-updates) without duplicating the check in two places
and letting them silently drift apart, which is what had happened
previously (password-change enforced 12 chars/1 upper/1 digit;
registration enforced nothing).

Verified live: 422 on <8 chars, 422 on no digit, 422 on no symbol,
201 on a password satisfying all three rules.
RestrictedRegistrationRequest.username had zero validation — any
string passed, including empty, whitespace, or special characters.
The admin-created-user path (create/user.py) already enforces
3-63 chars, letters/digits/underscores only, via validate_username().
Reuses that same function here instead of duplicating the pattern,
so the two entry points can't drift apart.

Verified live: empty string, "bad user" (space), "bad@user",
"bad-user", and "ab" (too short) all -> 422. A valid username
("valid_user_123") -> 201, unaffected.
…ymbol rule

Stale from today's password-policy change (shared validate_password_strength:
8 chars, 1 digit, 1 symbol; no uppercase requirement) -- these two tests
still asserted the old 12-char/uppercase rule and were failing:
  - test_too_short_raises_422 checked for "12 characters" in the error
  - test_no_uppercase_raises_422 asserted an uppercase requirement that
    no longer exists; its own test password (18 chars, has digit+symbol)
    now correctly passes validation, so the test never even raised

Renamed the latter to test_no_symbol_raises_422 to cover the new third
rule instead. test_no_digit_raises_422 and test_valid_payload_passes
needed no changes -- their fixtures already satisfy both the old and
new rule. Full file: 9/9 passing.
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