Revisión técnica: autorización del panel, validación y datos de arranque - #2
Merged
Conversation
Las rutas estaban tras auth+verified, pero cualquier usuario autenticado podia listar, editar o borrar a cualquier otro. - UserPolicy aplicada con authorizeResource; un administrador no puede borrarse a si mismo y dejar el panel sin acceso. - store()/update() usan validated() en vez de $request->all(), que dejaba pasar cualquier campo del formulario a Eloquent, incluido password en texto plano. - celular se valida con digits:10: 'max:10|min:10' sobre un valor numerico comprueba el VALOR, no la longitud. - El listado pagina de 15 en 15 en lugar de traer la tabla entera. - catch (Exception $e) dentro de un namespace resuelve a App\...\Exception y no capturaba nada: la transaccion nunca se revertia y el usuario recibia una redireccion de exito aunque la operacion hubiese fallado. - Se elimina la ruta show, que apuntaba a un metodo inexistente.
- Los listeners implementan ShouldQueue: el alta de usuarios ya no espera al servidor SMTP ni se cae con el. - El resumen cargaba en memoria todos los usuarios de todos los paises y la vista lanzaba ademas un COUNT por fila (N+1). Ahora es una sola agregacion. - Si no existe administrador se registra un aviso en vez de reventar al leer una propiedad sobre null.
…word_resets - 'profile' faltaba en $fillable, asi que UserSeeder lo descartaba en silencio: el administrador quedaba con perfil Cliente y getUserAdmin() no encontraba a nadie. - PaisSeeder consultaba api.first.org con la verificacion TLS desactivada, de modo que migrate --seed fallaba sin conexion o en CI, y los pais_id fijos de UserSeeder dependian del orden de esa respuesta. - Faltaba la migracion de password_resets: el enlace de recuperar clave fallaba. - pais_id y categoria_id pasan a ser nullable, con lo que el registro publico de Breeze vuelve a funcionar. - phpunit.xml tenia comentada la configuracion de SQLite y la suite exigia un MySQL levantado.
23 pruebas, 38 aserciones, en verde.
Workflow con matriz PHP 8.0/8.1 sobre SQLite en memoria; docker compose con MySQL 8 y Mailhog. El README explica ahora que problema resuelve el proyecto y por que esta construido asi.
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
1 participant
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.
Revisión del proyecto antes de compartirlo.
Cualquier usuario autenticado administraba a todos
Las rutas estaban tras
auth+verified, pero eso solo comprueba que haysesión. Cualquier usuario podía listar, editar o borrar a cualquier otro. Añado
UserPolicyaplicada conauthorizeResource, que además impide que unadministrador se borre a sí mismo y deje el panel sin acceso.
Solo entra lo validado
User::create($request->all())dejaba pasar cualquier campo del formulario aEloquent, incluido
passworden texto plano. Ahora se usavalidated().Las reglas tampoco comprobaban lo que parecía:
celularusabamin:10|max:10,que sobre un valor numérico valida el VALOR y no la longitud, así que
"999"pasaba. Ahora es
digits:10.El administrador no existía
profileno estaba en$fillable, así queUserSeederlo descartaba ensilencio: el usuario se creaba con perfil
ClienteyUser::getUserAdmin()noencontraba a nadie, con lo que el correo de resumen fallaba al leer una propiedad
sobre
null.Datos de arranque deterministas
PaisSeederconsultabaapi.first.orgen tiempo de seeding y con la verificaciónTLS desactivada:
migrate --seedfallaba sin conexión o en CI, y lospais_idfijos del seeder de usuarios dependían del orden de esa respuesta. Ahora la lista
es local y las relaciones se resuelven por nombre.
Además
catch (Exception $e)que no captura nada dentro de un namespace: latransacción nunca se revertía y el usuario recibía una redirección de éxito
aunque la operación hubiera fallado.
ShouldQueue: el alta ya no espera alservidor SMTP ni se cae con él.
withy luego unCOUNTpor fila desde la vista(N+1) sobre una consulta que ya había cargado todos los usuarios en memoria.
Ahora es una sola agregación con
withCount.password_resets: el enlace de recuperar contraseñafallaba.
phpunit.xmltenía comentada la configuración de SQLite, así que la suiteexigía un MySQL levantado.
showque apuntaba a un métodoinexistente.
Verificación
php artisan test→ 23 pruebas, 38 aserciones, en verde, sobre SQLite enmemoria y sin conexión a internet.