Revisión de Control de Acceso e IDOR

🔍 Fase 1 — Modelo de Autorización

El codebase implementa autenticación JWT estándar pero no existe un modelo de autorización coherente a nivel de objeto. La única barrera es IsAuthenticated, que verifica login pero no propiedad ni pertenencia a organización.

Decoradores / Clases

IsAuthenticated únicamente has_object_permission ausente

Scoping de queries

MealPlan.objects.all() ProgressLog: param opcional

Modelo de propiedad

patient FK → User organization FK Sin enforcement

🗺️ Fase 2 — Superficie de Ataque

Método	Endpoint	Recurso	Quién fija el owner	Riesgo IDOR
GET	`/api/meal-plans/`	MealPlan (lista)	—	Alto — devuelve TODOS
GET	`/api/meal-plans/{pk}/`	MealPlan (detalle)	—	Alto — sin chequeo
POST	`/api/meal-plans/`	MealPlan (create)	request.data (cliente)	Alto — org_id inyectable
PUT	`/api/meal-plans/{pk}/`	MealPlan (update)	—	Alto — puede editar ajenos
DELETE	`/api/meal-plans/{pk}/`	MealPlan (delete)	—	Alto — puede eliminar ajenos
GET	`/api/progress-logs/?patient_id=X`	ProgressLog (lista)	—	Alto — param no validado
POST	`/api/progress-logs/`	ProgressLog (create)	request.data completo	Crítico — mass assignment

🐛 Fases 3 & 4 — Hallazgos Confirmados

IDOR-001

Acceso irrestricto a planes de alimentación ajenos (GET/PUT/DELETE)

api/views.py — MealPlanViewSet · Confianza: Alta — flujo trazado completamente

Severidad CRÍTICA

La pregunta clave

¿Puede el Paciente A leer, editar o eliminar el plan de alimentación del Paciente B conociendo su ID?

Respuesta confirmada

SÍ — sin ninguna restricción

Trazado del flujo — GET /api/meal-plans/42/

1

La URL llega a MealPlanViewSet.retrieve() con pk=42.

2

MealPlanViewSet hereda de viewsets.ModelViewSet sin clase base personalizada.

3

permission_classes = [IsAuthenticated] — sólo valida que el usuario esté logueado, no verifica propiedad del objeto.

4

get_queryset() retorna MealPlan.objects.all() — todos los planes de todas las clínicas.

5

has_object_permission() no está implementado. DRF lo omite silenciosamente.

6

Confirmado: get_object() usa el queryset sin filtrar → devuelve el plan pk=42 a cualquier usuario autenticado.

            api/views.py — Código vulnerable
            Python
          

class MealPlanViewSet(viewsets.ModelViewSet):
    serializer_class = MealPlanSerializer
    permission_classes = [IsAuthenticated]  # ← solo autenticacion, SIN ownership check

    def get_queryset(self):
        return MealPlan.objects.all()  # ← sin filtro por usuario/organización

    def perform_create(self, serializer):
        serializer.save(
            nutritionist=self.request.user,
            organization_id=self.request.data.get('organization_id')  # ← cliente decide org
        )

Impacto confirmado

Cualquier paciente/nutricionista autenticado puede: (1) listar planes de todas las clínicas, (2) leer planes de pacientes de la competencia, (3) editar o eliminar datos clínicos ajenos, (4) crear planes asignándolos a otra organización cambiando organization_id en el body.

            api/views.py — Corrección sugerida (ENFORCEMENT real)
            Python
          

class IsPatientOrAssignedNutritionist(BasePermission):
    def has_object_permission(self, request, view, obj):
        # El paciente propietario puede leer; el nutricionista asignado puede leer+escribir
        if request.method in SAFE_METHODS:
            return obj.patient == request.user or obj.nutritionist == request.user
        return obj.nutritionist == request.user  # solo nutricionista asignado puede mutar


class MealPlanViewSet(viewsets.ModelViewSet):
    serializer_class = MealPlanSerializer
    permission_classes = [IsAuthenticated, IsPatientOrAssignedNutritionist]

    def get_queryset(self):
        # Scoping a la organización del usuario autenticado
        user = self.request.user
        return MealPlan.objects.filter(organization=user.organization)

    def perform_create(self, serializer):
        # Servidor fija organización; cliente NO puede inyectarla
        serializer.save(
            nutritionist=self.request.user,
            organization=self.request.user.organization,
        )

IDOR-002

Mass Assignment en creación de ProgressLog + enumeración de pacientes

api/views.py — ProgressLogViewSet · Confianza: Alta — flujo trazado

Severidad CRÍTICA

La pregunta clave

¿Puede el Nutricionista A leer logs del Paciente B de otra clínica? ¿Puede un paciente crear logs en nombre de otro?

Respuesta confirmada

SÍ en ambos casos

Trazado del flujo — GET /api/progress-logs/?patient_id=99

1

get_queryset() extrae patient_id de request.query_params sin validar que pertenezca al usuario autenticado.

2

Cualquier usuario autenticado puede sustituir el patient_id por el ID de cualquier otro paciente.

3

Trazado POST: perform_create llama serializer.save(**self.request.data) → el cliente controla TODOS los campos, incluyendo patient y meal_plan.

4

Un atacante puede crear un ProgressLog con patient_id=999 (otra persona) o alterar el meal_plan al que apunta.

            api/views.py — Código vulnerable
            Python
          

class ProgressLogViewSet(viewsets.ModelViewSet):
    permission_classes = [IsAuthenticated]

    def get_queryset(self):
        patient_id = self.request.query_params.get('patient_id')
        if patient_id:
            return ProgressLog.objects.filter(patient_id=patient_id)  # sin validar ownership
        return ProgressLog.objects.filter(patient=self.request.user)

    def perform_create(self, serializer):
        serializer.save(**self.request.data)  # mass assignment total

Impacto confirmado

Un nutricionista malintencionado puede enumerar el historial de peso e ingesta de pacientes de cualquier clínica competidora. Un paciente puede falsificar registros de evolución en nombre de otro. Esto expone datos de salud de carácter especialmente protegido (RGPD Art. 9).

            api/views.py — Corrección sugerida
            Python
          

class ProgressLogViewSet(viewsets.ModelViewSet):
    permission_classes = [IsAuthenticated]
    serializer_class = ProgressLogSerializer

    def get_queryset(self):
        user = self.request.user
        patient_id = self.request.query_params.get('patient_id')
        # Sólo el propio paciente o nutricionista de la misma org puede ver logs
        base_qs = ProgressLog.objects.filter(
            meal_plan__organization=user.organization
        )
        if patient_id:
            # Verificar que el patient_id pertenece a la misma org
            if not User.objects.filter(pk=patient_id, organization=user.organization).exists():
                raise PermissionDenied("Acceso denegado")
            return base_qs.filter(patient_id=patient_id)
        return base_qs.filter(patient=user)

    def perform_create(self, serializer):
        # Servidor fija el paciente; no se acepta del cliente
        serializer.save(patient=self.request.user)

IDOR-003

Permission class `IsOwnerOrNutritionist` sin `has_object_permission`

api/permissions.py · Confianza: Alta — clase declarada pero nunca usada con enforcement

Severidad MEDIA

La pregunta clave

¿La clase de permiso que parece proteger objetos realmente lo hace?

Respuesta

NO — comentario en docstring, sin código que valide

            api/permissions.py — Clase engañosa
            Python
          

class IsOwnerOrNutritionist(BasePermission):
    """Permite acceso si eres el paciente propietario o el nutricionista asignado."""
    def has_permission(self, request, view):
        return request.user and request.user.is_authenticated  # = IsAuthenticated
    # has_object_permission() ausente → DRF da acceso por defecto

Corrección

La clase debe implementar has_object_permission() con lógica real o eliminarse. Su existencia crea una falsa sensación de seguridad para futuros desarrolladores que asuman que ya está protegido.

🔎 Fase 5 — Requieren Verificación Manual

⚠️

Middleware de tenant

No se encontró ningún TenantMiddleware ni OrganizationMiddleware. Confirmar si existe a nivel de settings/router antes de asumir que no hay scoping global.

⚠️

Permisos de admin de clínica (`clinic_admin`)

El rol clinic_admin no tiene endpoints dedicados revisados. Si puede gestionar todos los usuarios de su organización, verificar que no puede escalar a otras orgs cambiando organization_id en operaciones bulk.

🛠️ Plan de Remediación Priorizado

ID	Hallazgo	Prioridad	Esfuerzo	Acción inmediata
IDOR-001	Acceso total a MealPlans ajenos	P1 — Esta sprint	Bajo	Añadir `IsPatientOrAssignedNutritionist` + `get_queryset` filtrado por `organization`
IDOR-002	Mass assignment ProgressLog	P1 — Esta sprint	Bajo	Reemplazar `serializer.save(**request.data)` por campos explícitos del servidor
IDOR-003	Permission class decorativa	P2 — Próxima sprint	Muy bajo	Implementar `has_object_permission()` real o eliminar la clase
VERIF-1/2	Middleware + admin bulk ops	P3 — Revisión siguiente	Medio	Revisión manual de codebase completo + pruebas de penetración básicas

Control de Acceso & IDOR Review

Riesgo CRÍTICO

Acceso irrestricto a planes de alimentación ajenos (GET/PUT/DELETE)

Mass Assignment en creación de ProgressLog + enumeración de pacientes

Permission class `IsOwnerOrNutritionist` sin `has_object_permission`

Middleware de tenant

Permisos de admin de clínica (`clinic_admin`)

Control de Acceso & IDOR Review

Riesgo CRÍTICO

Acceso irrestricto a planes de alimentación ajenos (GET/PUT/DELETE)

Mass Assignment en creación de ProgressLog + enumeración de pacientes

Permission class IsOwnerOrNutritionist sin has_object_permission

Middleware de tenant

Permisos de admin de clínica (clinic_admin)

Permission class `IsOwnerOrNutritionist` sin `has_object_permission`

Permisos de admin de clínica (`clinic_admin`)