Skip to content

Feat/requestroleaccess

Lakshy Yarlagadda requested to merge feat/requestroleaccess into develop 
## Merge Request Overview

This MR adds a backend role request workflow so authenticated users can request additional roles, while role assignment, approval, and removal remain admin-controlled.

Problem solved:
- users previously had no approval workflow for requesting new access roles
- direct role changes needed to stay restricted to admins
- self-role escalation through profile update needed to be blocked

Feature added:
- users can create role requests for additional roles
- users can view their own role requests
- admins can list, filter, approve, or reject role requests
- API responses now include reviewer details for reviewed requests

Why this change is needed:
- it introduces a proper approval flow instead of relying only on direct admin role assignment
- it preserves RBAC control by keeping final approval/removal with admins
- it closes the gap where a user could otherwise try to change roles through profile update payloads

Expand on the commit message with more context:
- motivation: add a safe, auditable role-access workflow without weakening existing admin controls
- approach: introduce a dedicated `role_requests` domain with its own table, model, schema, service, and API routes
- approach: keep `status` as a DB enum because it is a stable internal workflow state
- approach: move `requested_role` validation to schema/service and store it as a string so future role additions do not require enum churn in this table
- approach: eager-load reviewer details so the frontend can show who reviewed a request without extra API calls
- trade-off: `requested_role` is no longer DB-enforced as an enum, so correctness now relies on API/schema/service validation rather than a DB enum constraint

## Changes Made

Files added/modified/removed:
- added [8f6c2a4d1b30_create_role_requests_table.py](/home/lakshy/Projects/ehrs-fastapi/alembic/versions/8f6c2a4d1b30_create_role_requests_table.py)
- added [role_request.py](/home/lakshy/Projects/ehrs-fastapi/app/models/role_request.py)
- added [role_request.py](/home/lakshy/Projects/ehrs-fastapi/app/schemas/role_request.py)
- added [role_request_service.py](/home/lakshy/Projects/ehrs-fastapi/app/services/role_request_service.py)
- modified [users_routes.py](/home/lakshy/Projects/ehrs-fastapi/app/api/v1/routes/users_routes.py)
- modified [user.py](/home/lakshy/Projects/ehrs-fastapi/app/models/user.py)
- modified [__init__.py](/home/lakshy/Projects/ehrs-fastapi/app/models/__init__.py)
- modified [user.py](/home/lakshy/Projects/ehrs-fastapi/app/schemas/user.py)
- modified [user_service.py](/home/lakshy/Projects/ehrs-fastapi/app/services/user_service.py)
- added/updated [test_users_routes.py](/home/lakshy/Projects/ehrs-fastapi/tests/test_api/test_users_routes.py)
- added/updated [test_role_request_service.py](/home/lakshy/Projects/ehrs-fastapi/tests/test_services/test_role_request_service.py)

New models, services, or utilities introduced:
- `RoleRequest` SQLAlchemy model
- `RoleRequestStatusEnum`
- `RoleRequestCreate`, `RoleRequestReview`, `RoleRequestResponse`, `RoleRequestListResponse`
- `RoleRequestReviewer` response schema
- `role_request_service` for create/list/review flows

API endpoint changes:
- `POST /api/v1/users/role-requests`
- `GET /api/v1/users/role-requests/me`
- `GET /api/v1/users/role-requests`
- `PUT /api/v1/users/role-requests/{request_id}/review`

Configuration changes:
- none in this branch

## Technical Details

Architecture decisions made:
- role request workflow is separated into model, schema, service, and route layers
- admin remains the only actor allowed to review requests or directly assign/remove roles
- reviewer details are included in the response model to support audit visibility in the frontend
- `requested_role` is string-backed in the DB for easier future role additions
- `status` remains enum-backed because it is a stable workflow state

Database schema changes:
- new `role_requests` table
- columns include `id`, `user_id`, `requested_role`, `status`, `review_note`, `reviewed_by`, `reviewed_at`, `created_at`, `updated_at`
- foreign keys:
  - `user_id -> users.id` with `CASCADE`
  - `reviewed_by -> users.id` with `SET NULL`
- indexes added for `id`, `user_id`, `requested_role`, `status`, `reviewed_by`

Service layer structure and data flow:
- `create_role_request()` validates requestable roles, blocks duplicate pending requests, and creates a pending request
- `get_role_requests_by_user()` returns the authenticated user’s requests with optional filtering
- `get_role_requests()` returns admin-view request lists with optional filtering
- `review_role_request()` approves or rejects a pending request and assigns the role on approval
- reviewer relation is loaded with `selectinload` to avoid extra lookups when serializing responses

Root cause addressed:
- there was no dedicated backend approval workflow for users requesting new roles
- role changes needed stronger separation between request creation and admin approval
- role changes also required immediate RBAC cache invalidation after approval/assign/remove

How this addresses it:
- adds a dedicated persistence and API flow for role requests
- keeps approval/removal under admin control
- removes `roles` from `UserUpdate` to prevent self-escalation
- clears permission cache after effective role changes

## Type of Change

- [x] ✨ New feature
- [ ] 🐛 Bug fix
- [ ] 💥 Breaking change
- [ ] 📝 Documentation update
- [x] ♻️ Refactor
- [ ] ⚡ Performance improvement
- [x] 🧪 Test update
- [ ] 🔧 Configuration change
- [x] 🚨 Security fix
- [ ] 🗑️ Deprecation

## Related Issues / References

- No GitLab issue/MR reference was provided for this branch
- Add `Closes #...` or `Related to !...` if you have linked tracker items

## Screenshots or Screen Recordings

For API changes, attach:
- Swagger `/docs` screenshot showing the new role-request endpoints
- pytest result screenshot for the focused backend suites
- curl/Postman examples for create request, list requests, and review request

## How to Validate Locally

Previous behaviour:
- users could not raise role requests through a dedicated backend flow
- admins could assign/remove roles directly, but there was no request-review workflow
- profile update payloads still needed protection from self-role edits

Changes made:
- added role request table, model, schemas, service, and routes
- added admin review flow for pending requests
- added reviewer details in API responses
- moved `requested_role` validation to schema/service instead of a DB enum in the new table
- blocked self-role updates in profile edit flow

New behaviour:
- authenticated users can submit role requests
- authenticated users can list their own requests
- admins can filter and review requests
- approved requests update the user’s roles and clear cached permissions
- reviewed responses can include reviewer identity details

Step-by-step validation:
1. Run migrations for the backend database.
2. Start the API server.
3. Log in as a normal user and create a role request with `POST /api/v1/users/role-requests`.
4. Confirm the request appears in `GET /api/v1/users/role-requests/me`.
5. Log in as an admin and confirm the request appears in `GET /api/v1/users/role-requests`.
6. Approve or reject it using `PUT /api/v1/users/role-requests/{request_id}/review`.
7. If approved, verify the target user now has the new role and permissions take effect after cache clear.
8. Verify that profile update does not allow direct role editing.

## Testing Done

Unit tests added/updated:
- role request service tests updated
- user route tests updated

API endpoint tests passing:
- yes, focused route and service suites are passing

Test Cases Covered:

| Scenario | Expected Result | Status |
|---|---|---|
| User requests a valid role | Pending role request is created | ✅ |
| User requests a non-requestable role | Request is rejected with validation error | ✅ |
| User requests a role they already have | Request is rejected | ✅ |
| Duplicate pending request for same role | Request is rejected | ✅ |
| User lists own requests with filters | Filtered requests are returned | ✅ |
| Admin lists all requests with filters | Filtered requests are returned | ✅ |
| Admin approves pending request | Request is reviewed and role is assigned | ✅ |
| Admin rejects reviewed/non-pending request | Proper error response is returned | ✅ |
| Reviewer details included in response | Reviewer metadata is serialized | ✅ |
| Self profile update tries to modify roles | Direct self-role escalation is blocked | ✅ |

Test Commands Run:
```bash
uv run ruff check alembic/versions/8f6c2a4d1b30_create_role_requests_table.py app/models/role_request.py app/services/role_request_service.py tests/test_services/test_role_request_service.py
uv run mypy app/models/role_request.py app/services/role_request_service.py app/api/v1/routes/users_routes.py
uv run pytest tests/test_services/test_role_request_service.py tests/test_api/test_users_routes.py

Test result:

  • 34 passed

Code Quality Checklist

Code Standards:

  • Code follows project conventions
  • No debug statements or commented-out code left
  • No unused imports, variables, or functions
  • No duplicate code introduced
  • Type hints are defined and focused mypy checks pass
  • Ruff checks pass on the touched backend files

Python & FastAPI Best Practices:

  • Functions are separated by responsibility
  • Dependency injection is used for DB/auth access
  • Pydantic models are used for request/response validation
  • SQLAlchemy queries avoid extra reviewer lookups via selectinload
  • Error handling returns appropriate HTTP exceptions

API Design:

  • RESTful conventions followed
  • Proper HTTP status codes returned
  • Input validation implemented
  • Authentication/authorization enforced
  • Role-based access control used for admin-only operations
  • API docstrings updated for the new endpoints

Database & Migrations:

  • Database migration created
  • Migration contains downgrade script
  • Indexes added for frequently queried fields
  • ORM used instead of raw SQL
  • Data integrity constraints maintained
  • Migration-head topology across the whole repo is not addressed by this MR

Security:

  • No sensitive data logged by this change
  • ORM usage avoids SQL injection concerns
  • Authentication tokens are handled through existing auth flow
  • Self-role escalation through profile update is blocked
  • bandit -r app/ was not rerun in this focused validation pass

Documentation:

  • API documentation is updated through route/schema changes
  • README update not required for this feature
  • .env.example update not required
  • CHANGELOG update not included in this branch

Known Limitations / Technical Debt

  • requested_role is now validated at the schema/service layer rather than enforced by a DB enum in the role_requests table
  • if the existing migration revision was already applied in a local DB before this refactor, the local database may need to be recreated or aligned with a follow-up migration
  • repo-wide Alembic head unification is not addressed in this MR

Additional Notes

  • status intentionally remains enum-backed because the review workflow states are stable and internal
  • reviewer details were added to the response model so the frontend can display who approved/rejected a request without extra API calls
  • direct admin role assignment/removal remains available and separate from the request workflow

MR Acceptance Checklist

Quality & Correctness:

  • Code works as intended and solves the stated problem
  • No known regressions introduced in the focused validated paths
  • Edge cases covered in route/service tests

Maintainability:

  • Code is readable and organized by layer
  • Code is testable and tested
  • Follows existing FastAPI/SQLAlchemy project patterns

Acceptance Review:

  • Reviewed by at least 1 teammate
  • Reviewed by product owner
  • /assign me
Edited by Lakshy Yarlagadda

Merge request reports