fix/errores-despliegue-pedidos-unidad-prioridad#266
Conversation
|
Warning Review limit reached
More reviews will be available in 56 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughEl PR realiza tres cambios independientes: relaja la regla de autorización de ChangesCambio de regla de seguridad en backend
Cambios de frontend: onboarding y traducciones
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mjnizac
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/views/auth/OnboardingView.vue (1)
32-39: 🏗️ Heavy liftExtrae la creación/validación de unidad a un composable compartido.
En Line 33 y Line 38 estás metiendo validación y construcción del payload dentro del componente. Ya existe
frontend/src/composables/useUnitForm.js(Line 41-77) con esta responsabilidad para/units; mantener ambos caminos en paralelo facilita la deriva de reglas.As per coding guidelines, "Lógica reutilizable en composables (useApi.js, useValidation.js)" y "No lógica de negocio directamente en componentes".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/views/auth/OnboardingView.vue` around lines 32 - 39, The createUnit() function in OnboardingView.vue duplicates validation and payload construction logic that already exists in the shared composable useUnitForm.js. Instead of manually validating unitName and unitAddress fields (lines 33-34) and constructing the API payload (line 38), import the useUnitForm composable and leverage its existing validation and API methods. Remove the inline validation checks and api.post call from createUnit(), replacing them with calls to the appropriate composable functions that handle unit validation and creation according to the shared business rules.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/main/java/com/delivera/config/SecurityConfig.java`:
- Line 75: The requestMatchers configuration for the GET endpoint
`/loyal-users/me/orders` uses `authenticated()` instead of
`hasRole(LOYAL_USER)`, allowing any authenticated user regardless of role to
attempt access. While the underlying service may return an empty list for
non-loyal users, this violates the security principle of explicit role-based
access control. Either restore the `hasRole(LOYAL_USER)` requirement in the
requestMatchers call to enforce role-based authorization at the security layer,
or modify the loyalUserService.getMyOrders() method to explicitly throw an
exception (such as LoyalUserNotFoundException or AccessDeniedException) when the
authenticated user is not found in the loyal_user table, ensuring the
authorization is properly validated rather than silently failing with an empty
list.
---
Nitpick comments:
In `@frontend/src/views/auth/OnboardingView.vue`:
- Around line 32-39: The createUnit() function in OnboardingView.vue duplicates
validation and payload construction logic that already exists in the shared
composable useUnitForm.js. Instead of manually validating unitName and
unitAddress fields (lines 33-34) and constructing the API payload (line 38),
import the useUnitForm composable and leverage its existing validation and API
methods. Remove the inline validation checks and api.post call from
createUnit(), replacing them with calls to the appropriate composable functions
that handle unit validation and creation according to the shared business rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05d672dc-9c45-4899-977b-cc801b5b841d
📒 Files selected for processing (4)
backend/src/main/java/com/delivera/config/SecurityConfig.javafrontend/src/locales/en.ymlfrontend/src/locales/es.ymlfrontend/src/views/auth/OnboardingView.vue
| auth.requestMatchers(HttpMethod.POST, api + "/loyal-users").hasRole(ADMIN); | ||
| auth.requestMatchers(HttpMethod.PUT, api + "/loyal-users/**").hasRole(ADMIN); | ||
| auth.requestMatchers(HttpMethod.GET, api + "/loyal-users/me/orders").hasRole(LOYAL_USER); | ||
| auth.requestMatchers(HttpMethod.GET, api + "/loyal-users/me/orders").authenticated(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar la implementación de getMyOrders() para confirmar validación interna
# Buscar la implementación del método getMyOrders en el servicio
ast-grep --pattern $'public $_ getMyOrders() {
$$$
}'
# Buscar referencias a LOYAL_USER o validaciones de rol en LoyalUserService
rg -n -C5 'getMyOrders|LOYAL_USER|loyal.*user' --type=java -g '**/LoyalUserService*.java'
# Buscar si hay validación del usuario actual como loyal user
rg -n -C3 'getCurrentUser|getAuthenticated|SecurityContext' --type=java -g '**/LoyalUserService*.java'Repository: Delivera-Project/Delivera
Length of output: 5305
El endpoint relaja la autorización sin validación suficiente en el servicio.
El cambio de hasRole(LOYAL_USER) a authenticated() permite que cualquier usuario autenticado (ADMIN, ANALYST, OPERATOR, etc.) acceda al endpoint /loyal-users/me/orders.
Aunque loyalUserService.getMyOrders() busca internamente el usuario en la tabla loyal_user, no valida explícitamente que el usuario autenticado tenga el rol LOYAL_USER ni lanza una excepción cuando el usuario no es fidelizado. En su lugar, retorna una lista vacía (List.of()).
Esto viola la guía de codificación: "Seguridad: verifica que los endpoints estén correctamente protegidos según el rol". El endpoint debe:
- Mantener
hasRole(LOYAL_USER)en la configuración de seguridad, O - Lanzar una excepción en el servicio (ej.
LoyalUserNotFoundException) cuando el usuario autenticado no sea un loyal user, O - Justificar formalmente por qué usuarios no fidelizados deben tener acceso al path
/loyal-users/**
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/java/com/delivera/config/SecurityConfig.java` at line 75,
The requestMatchers configuration for the GET endpoint `/loyal-users/me/orders`
uses `authenticated()` instead of `hasRole(LOYAL_USER)`, allowing any
authenticated user regardless of role to attempt access. While the underlying
service may return an empty list for non-loyal users, this violates the security
principle of explicit role-based access control. Either restore the
`hasRole(LOYAL_USER)` requirement in the requestMatchers call to enforce
role-based authorization at the security layer, or modify the
loyalUserService.getMyOrders() method to explicitly throw an exception (such as
LoyalUserNotFoundException or AccessDeniedException) when the authenticated user
is not found in the loyal_user table, ensuring the authorization is properly
validated rather than silently failing with an empty list.
Source: Coding guidelines
mjnizac
force-pushed
the
fix/errores-despliegue-pedidos-unidad-prioridad
branch
from
79d69de to
01736b4
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
* fix/leaflet-navigation-and-ux-bugs (#243) * fix: Leaflet navigation crash and minor UX bugs * fix: corregir bugs de mapas, navegación, ranking y asignación de trabajadores * feat: ampliar datos de demo con global admin, nueva empresa y asignación de trabajadores a unidades * fix: aplicar sugerencias CodeRabbit en mapas, layout y tests * chore: actualizaciones de dependencias Dependabot (#253) * fix/my-orders-sidebar-unregistered-user (#254) * fix: mostrar mis pedidos en sidebar para usuarios LOYAL_USER * fix: añadir campo username al formulario de reclamación de pedido * fix: reutilizar fields.username en lugar de clave duplicada en tracking * feat: pedidos B2B cross-org, registro desde tracking con address, validaciones y mejoras UI * test: corregir tests tras cambios en OrderService, UnitService y WorkerService * fix: añadir LoyalUserCompany y actualizar test useOrderForm a b2bOrganizations * test: añadir address en test useUnitForm para pasar validación de ubicación * test: añadir campo address en E2E de registro individual * chore: skip commitlint en PR develop a main * feat: panel admin global con mapa de rutas, métricas y gestión de entidades (#256) * chore/actualizar-dependencias-dependabot (#264) * chore: actualizar dependencias de linting frontend (oxlint, eslint-plugin-vue, vitest-eslint-plugin) * fix: parchear vulnerabilidades vite (8.0.16) y esbuild (override 0.28.1) * fix: errores de mis pedidos, unidad en onboarding y prioridad (#266)
1 participant
Summary by CodeRabbit
Notas de la versión
Nuevas Características
Estilo