Skip to content

✨ feat: Add functionality to add new findings in audits#128

Merged
jllanosg merged 4 commits intodevelopmentfrom
feature/add-vulns-audit
Oct 15, 2024
Merged

✨ feat: Add functionality to add new findings in audits#128
jllanosg merged 4 commits intodevelopmentfrom
feature/add-vulns-audit

Conversation

@caverav
Copy link
Copy Markdown
Owner

@caverav caverav commented Oct 8, 2024

Descripción

Este pull request introduce nuevas funcionalidades y mejoras en la gestión de hallazgos en auditorías. Se ha mejorado la visualización de los hallazgos en el componente AuditSidebar, se ha añadido la capacidad de agregar vulnerabilidades y hallazgos, y se ha actualizado el botón NewVulnButton para aceptar una función de clic personalizada.

Motivación y Contexto

Este cambio es necesario para facilitar la gestión de hallazgos durante una auditoría. Se ha añadido la capacidad de agregar nuevas vulnerabilidades y hallazgos, lo cual mejora la funcionalidad y usabilidad de la aplicación para los usuarios que realizan auditorías de seguridad.

¿Cómo ha sido probado?

Haciendo click en el símbolo + en la tabla de vulnerabilidades y añadir una vulnerabilidad nueva

Tipos de cambios

  • Bugfix (cambio que no interrumpe el funcionamiento y que soluciona un problema)
  • New feature (cambio que no interrumpe el funcionamiento y que añade funcionalidad)
  • Breaking change (corrección o funcionalidad que podría causar que la funcionalidad existente cambie)

Lista de verificación:

  • Mi código sigue el estilo de código de este proyecto.
  • Mi cambio requiere una modificación en la documentación.
  • He actualizado la documentación en consecuencia.
  • Requiere nuevos tests.

Summary by CodeRabbit

Resumen por CodeRabbit

  • Nuevas Características

    • Se ha mejorado la visualización de los hallazgos en el componente AuditSidebar con un nuevo identificador y colores de fondo según el nivel de severidad.
    • Se ha añadido la capacidad de agregar vulnerabilidades y hallazgos con notificaciones para el usuario en el componente Add.
    • Se ha actualizado el botón NewVulnButton para aceptar una función de clic personalizada.
    • Se ha estandarizado la estructura de los hallazgos asociados a auditorías, mejorando la gestión de vulnerabilidades.
    • Se ha mejorado la generación de PDF con manejo de errores y encriptación de contraseñas.
    • Se ha añadido una nueva propiedad opcional category para categorizar vulnerabilidades.
  • Correcciones de Errores

    • Mejoras en la gestión del estado y la lógica de ordenación de los hallazgos.
    • Se ha añadido un nuevo elemento de localización que podría causar conflictos.

@caverav caverav added the enhancement New feature or request label Oct 8, 2024
@caverav caverav self-assigned this Oct 8, 2024
@jllanosg
Copy link
Copy Markdown
Collaborator

jllanosg commented Oct 8, 2024

imagen

que pasa con esto?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 8, 2024

📝 Walkthrough

Walkthrough

Se han realizado cambios significativos en varios componentes del frontend, incluyendo la adición de una nueva propiedad identifier al tipo Finding, la introducción de una función getSeverityColor para asignar colores de fondo según el nivel de severidad, y modificaciones en la lógica de ordenamiento de hallazgos. Además, se han implementado nuevos métodos en el componente Add para gestionar la adición de vulnerabilidades y hallazgos, junto con la creación de nuevos tipos y funciones en el archivo de servicios.

Changes

Archivo Resumen de cambios
frontend/src/components/navbar/AuditSidebar.tsx Se añadió la propiedad identifier al tipo Finding, se introdujo getSeverityColor, y se modificó la lógica de ordenamiento de hallazgos.
frontend/src/routes/audits/edit/AuditRoot.tsx Se actualizó el estado findings para incluir identifier, se agregó la función cvssStringToSeverity, y se simplificaron las opciones de ordenamiento.
frontend/src/routes/audits/edit/findings/add/add.tsx Se añadieron nuevos métodos para manejar la adición de vulnerabilidades y hallazgos, junto con un nuevo estado newVulnTitle y la recuperación de auditId de los parámetros de URL.
frontend/src/routes/audits/edit/findings/add/NewVulnButton.tsx Se modificó el componente para aceptar un prop onClick, reemplazando la función vacía anterior.
frontend/src/services/audits.ts Se definió un nuevo tipo AuditFinding y se añadieron funciones asíncronas addVuln y addFinding para gestionar la adición de vulnerabilidades y hallazgos.
frontend/src/i18n/en-US/index.ts Se añadió una cadena duplicada copiedToClipboard en la sección msg, lo que podría causar conflictos en la gestión de localización.
frontend/src/services/vulnerabilities.ts Se añadió la propiedad opcional category al tipo Details y se modificaron los tipos NewVulnerability y UpdateVulnerabilityData para incluir esta propiedad.

Possibly related PRs

  • feat: Add remediationComplexity and priority props to audits services #138: Los cambios en este PR implican la adición de las propiedades remediationComplexity y priority al tipo Finding en el archivo audits.ts, lo que se relaciona directamente con las actualizaciones realizadas en el PR principal donde se modificó el tipo Finding para incluir una nueva propiedad identifier.
  • Hotfix/i18 fixes #103: Este PR modifica el componente AuditRoot, que también incluye actualizaciones al tipo Finding, asegurando la consistencia en la estructura de datos utilizada en los componentes.
  • Feature/CWE recommendations related #137: Este PR mejora la funcionalidad relacionada con las recomendaciones de CWE, que pueden interactuar con los hallazgos mostrados en el componente AuditSidebar, especialmente en cómo se categorizan y muestran los hallazgos según la severidad.
  • feat: add severity none to cvss calculator box #142: Este PR introduce un nuevo nivel de severidad al calculador CVSS, que podría ser relevante para la función getSeverityColor introducida en el PR principal, ya que ambos tratan sobre la representación de la severidad en los hallazgos.

Suggested reviewers

  • Icksir
  • jllanosg
  • massi-ponce
  • Sealra
  • iTzGooDLife

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 352428a and b38e103.

📒 Files selected for processing (5)
  • frontend/src/components/navbar/AuditSidebar.tsx (4 hunks)
  • frontend/src/routes/audits/edit/AuditRoot.tsx (4 hunks)
  • frontend/src/routes/audits/edit/findings/add/NewVulnButton.tsx (1 hunks)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (5 hunks)
  • frontend/src/services/audits.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (9)
frontend/src/routes/audits/edit/findings/add/NewVulnButton.tsx (1)

8-8: ¡Excelente implementación del prop onClick!

Has aplicado correctamente el prop onClick al componente PrimaryButton. Esta modificación permite que el componente padre controle el comportamiento del botón, lo cual es exactamente lo que se esperaba con el cambio en la firma del componente.

frontend/src/routes/audits/edit/AuditRoot.tsx (5)

71-72: Verifica que finding.cvssv3 siempre tenga un valor válido.

Al usar cvssStringToSeverity(finding.cvssv3), asegúrate de que finding.cvssv3 no sea nulo ni inválido para evitar excepciones no controladas.


80-80: Confirma que la eliminación de otras opciones de ordenamiento es intencional.

Al restringir sortOptions solo a 'CVSS Score', verifica que esto no afecte otras partes de la aplicación que dependían de más opciones de ordenamiento.


84-84: Buena adición de la opción 'Ascending' en sortOrderOptions.

La inclusión de la opción ascendente mejora la usabilidad al permitir ordenar en ambos sentidos.


24-30: Actualización correcta de la estructura de datos findings.

La inclusión del campo identifier permite una identificación única de cada hallazgo, lo cual es esencial para su gestión.


87-87: Asegura el manejo correcto de connectedUsers cuando está vacío.

Al inicializar connectedUsers como un arreglo vacío, verifica que los componentes que lo utilizan funcionen correctamente sin datos y manejen adecuadamente este caso.

frontend/src/routes/audits/edit/findings/add/add.tsx (1)

168-168: Verificar el valor pasado al manejador onChange.

En la línea 168, el manejador onChange está definido como e => setNewVulnTitle(e). Por favor, verifica que e es el valor del input y no el evento. Si e es el evento, deberías utilizar e.target.value para obtener el valor introducido por el usuario.

frontend/src/components/navbar/AuditSidebar.tsx (1)

22-22: La adición de la propiedad identifier es acertada.

Incluir identifier en el tipo Finding mejora la identificación y manejo de los hallazgos.

frontend/src/services/audits.ts (1)

240-248: Definición correcta del tipo AuditFinding.

La nueva definición del tipo AuditFinding está bien estructurada y cumple con las convenciones del proyecto.

Comment thread frontend/src/routes/audits/edit/findings/add/NewVulnButton.tsx
Comment thread frontend/src/routes/audits/edit/AuditRoot.tsx
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx Outdated
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx Outdated
Comment thread frontend/src/components/navbar/AuditSidebar.tsx
Comment thread frontend/src/components/navbar/AuditSidebar.tsx Outdated
Comment thread frontend/src/services/audits.ts
Comment thread frontend/src/services/audits.ts Outdated
Comment thread frontend/src/services/audits.ts
Comment thread frontend/src/services/audits.ts Outdated
massi-ponce
massi-ponce previously approved these changes Oct 8, 2024
Copy link
Copy Markdown
Collaborator

@massi-ponce massi-ponce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between b38e103 and 66a6442.

📒 Files selected for processing (3)
  • frontend/src/components/navbar/AuditSidebar.tsx (2 hunks)
  • frontend/src/routes/audits/edit/AuditRoot.tsx (4 hunks)
  • frontend/src/services/audits.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (8)
frontend/src/routes/audits/edit/AuditRoot.tsx (2)

24-30: ⚠️ Potential issue

Inconsistencia de tipos en la asignación de 'id'

En la definición del estado findings (líneas 24-30~), el campo id se declara como number. Sin embargo, en el mapeo de los datos (línea 75~), se asigna finding.identifier, que es de tipo string, al campo id. Esto puede causar errores de tipo. Por favor, asegúrese de que los tipos sean consistentes y realice las correcciones necesarias.

Para solucionar este problema, puede cambiar el tipo de id a string en la definición del estado:

24       {
25-        id: number;
26+        id: string;
27         name: string;
28         category: string;
29         severity: string;
30         identifier: string;
31       }[]

Also applies to: 75-76


44-64: 🧹 Nitpick (assertive)

⚠️ Potential issue

Revisar el manejo de puntajes CVSS de 0.0

Actualmente, si el puntaje CVSS es exactamente 0.0, la función cvssStringToSeverity retornará 'I' (Informational). Sin embargo, según las especificaciones de CVSS, un puntaje de 0.0 corresponde a 'None'. Por favor, confirme que este comportamiento es el deseado y ajuste la función si es necesario.

Además, para optimizar las evaluaciones condicionales, considere utilizar else if en lugar de múltiples if, ya que las condiciones son mutuamente excluyentes.

Puede aplicar el siguiente cambio:

48       if (score >= 9.0) {
49         return 'C';
50       }
51-      if (score >= 7.0) {
+      else if (score >= 7.0) {
52         return 'H';
53       }
54-      if (score >= 4.0) {
+      else if (score >= 4.0) {
55         return 'M';
56       }
57-      if (score >= 0.1) {
+      else if (score >= 0.1) {
58         return 'L';
59       }
+      } else if (score === 0.0) {
+        return 'N'; // 'None' severity
60       }
frontend/src/components/navbar/AuditSidebar.tsx (4)

20-20: Uso adecuado de 'identifier' en el tipo 'Finding'.

La adición de la propiedad identifier: string; al tipo Finding es correcta y permite una identificación más precisa de los hallazgos.


56-62: Excelente implementación de 'severityColorMap'.

Has simplificado correctamente el código reemplazando el switch por un objeto de mapeo para asociar las severidades con sus colores correspondientes. Esto mejora la legibilidad y el mantenimiento del código.


64-66: Uso eficiente de 'getSeverityColor' con mapeo.

La función getSeverityColor ahora utiliza el objeto de mapeo eficientemente, lo cual es una buena práctica que optimiza el rendimiento del código.


79-87: Definición apropiada de 'severityOrder' para la ordenación.

La creación del objeto severityOrder es correcta y facilita la ordenación de los hallazgos según su severidad.

frontend/src/services/audits.ts (2)

240-248: Definición correcta del tipo 'AuditFinding'

La nueva declaración del tipo AuditFinding está bien estructurada y mejora la consistencia de los datos relacionados con los hallazgos en auditorías.


590-615: Implementación correcta de la función addFinding

La función addFinding está bien implementada y sigue las buenas prácticas en el manejo de operaciones asíncronas y gestión de errores.

Comment thread frontend/src/routes/audits/edit/AuditRoot.tsx
Comment thread frontend/src/services/audits.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 66a6442 and 8f526c0.

📒 Files selected for processing (2)
  • frontend/src/components/navbar/AuditSidebar.tsx (2 hunks)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (5 hunks)
🧰 Additional context used
🔇 Additional comments (5)
frontend/src/routes/audits/edit/findings/add/add.tsx (2)

117-131: El manejo de errores sigue siendo inadecuado en el bloque .catch

El comentario de revisión previo sobre mejorar el manejo de errores aún es aplicable aquí. Deberías notificar al usuario cuando ocurra un error, no solo registrar el error en la consola.


140-152: El manejo de errores sigue siendo inadecuado en el bloque .catch

El comentario de revisión previo sobre mejorar el manejo de errores aún es aplicable aquí. Deberías notificar al usuario cuando ocurra un error, no solo registrar el error en la consola.

frontend/src/components/navbar/AuditSidebar.tsx (3)

20-20: Buena adición de la propiedad identifier al tipo Finding

La inclusión de identifier ayudará a identificar de forma única los hallazgos, mejorando su manejo y referencia en el código.


56-66: Implementación efectiva de severityColorMap y getSeverityColor

La refactorización utilizando un mapa para asociar severidades con colores mejora la legibilidad y facilita el mantenimiento futuro del código.


88-93: ⚠️ Potential issue

Revisar la lógica de ordenamiento de los hallazgos

La asignación de orderMultiplier podría estar invertida. Actualmente, cuando sortOrder es 'Ascending', orderMultiplier se establece en -1. Esto puede provocar que el ordenamiento de los hallazgos no refleje el orden ascendente esperado según su severidad.

Para confirmar si los hallazgos se están ordenando correctamente por severidad, puedes ejecutar el siguiente script:

Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
@caverav caverav requested a review from massi-ponce October 9, 2024 03:25
@iTzGooDLife
Copy link
Copy Markdown
Collaborator

Consulta, al agregar un finding, este deberia aparecer a la izquierda cierto? o no está contemplado?

image

Copy link
Copy Markdown
Collaborator

@iTzGooDLife iTzGooDLife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Al agregar una vulnerabilidad el sidebar no se actualiza hasta que sales y vuelves a entrar a al auditoria, lo cual no parece el funcionamiento correcto, ya que se debería refrescar sin la necesidad de salir.

Agregando findings:
image

Luego de salir y entrar del audit:
image

@caverav
Copy link
Copy Markdown
Owner Author

caverav commented Oct 9, 2024

Al agregar una vulnerabilidad el sidebar no se actualiza hasta que sales y vuelves a entrar a al auditoria, lo cual no parece el funcionamiento correcto, ya que se debería refrescar sin la necesidad de salir.

Agregando findings:
image

Luego de salir y entrar del audit:
image

Sí, eso es un pendiente pero no creo conveniente incluirlo en el PR, ya que al no ser hijo un componente del otro, no se pueden pasar estados fácilmente, así que se debe dejar pendiente como issue

Comment thread frontend/src/services/audits.ts Outdated
@caverav caverav requested a review from iTzGooDLife October 10, 2024 15:10
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 28d88e8 and 73c64a2.

📒 Files selected for processing (2)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (5 hunks)
  • frontend/src/services/audits.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (4)
frontend/src/routes/audits/edit/findings/add/add.tsx (1)

194-194: ⚠️ Potential issue

Corregir el identificador en keyExtractor para evitar errores.

En el componente UITable, estás utilizando item._id en keyExtractor, pero en tus datos, los objetos tienen la propiedad id. Esto puede causar problemas al renderizar la tabla.

Realiza el siguiente cambio:

- keyExtractor={item => item._id}
+ keyExtractor={item => item.id}

Likely invalid or redundant comment.

frontend/src/services/audits.ts (3)

240-248: Definición del tipo AuditFinding

El tipo AuditFinding está bien definido, incluyendo los campos necesarios para representar un hallazgo en una auditoría.


536-597: Actualización adecuada para soportar múltiples locales en los detalles de vulnerabilidades

La función addVuln ahora recibe locale como parámetro y busca el detalle correspondiente en data.details, mejorando la funcionalidad para soportar múltiples idiomas.


599-624: Función addFinding implementada correctamente

La función addFinding permite añadir un nuevo hallazgo al informe, enviando el título al servidor como se esperaba.

Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx Outdated
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/services/audits.ts Outdated
Copy link
Copy Markdown
Collaborator

@jllanosg jllanosg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La funcionalidad está OK; solamente cambiar el font weight para mejorar el contrase.

Comment thread frontend/src/components/navbar/AuditSidebar.tsx
@caverav caverav requested a review from jllanosg October 11, 2024 00:54
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 73c64a2 and 9f65de5.

⛔ Files ignored due to path filters (1)
  • backend/report-templates/liccc.docx is excluded by !**/*.docx
📒 Files selected for processing (3)
  • frontend/src/components/navbar/AuditSidebar.tsx (2 hunks)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (5 hunks)
  • frontend/src/services/audits.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (4)
frontend/src/routes/audits/edit/findings/add/add.tsx (1)

194-194: ⚠️ Potential issue

Corregir keyExtractor para usar item.id en lugar de item._id

La función keyExtractor está accediendo a item._id, pero según la definición de TableData, el identificador correcto es id. Esto puede provocar errores al renderizar la tabla.

Aplica este cambio para corregir el identificador:

- keyExtractor={item => item._id}
+ keyExtractor={item => item.id}

Likely invalid or redundant comment.

frontend/src/components/navbar/AuditSidebar.tsx (2)

176-176: Utiliza finding.identifier como clave única en la lista

Si identifier es único para cada finding, usarlo como key puede evitar posibles conflictos en el renderizado de listas en React.


88-93: Evita mutar directamente las props findings al ordenar

Mutar directamente las props puede causar comportamientos inesperados en React. Es recomendable crear una copia de findings antes de ordenarlos.

frontend/src/services/audits.ts (1)

240-248: Definición correcta del tipo 'AuditFinding'

La nueva definición del tipo AuditFinding está bien estructurada y cumple con las convenciones establecidas.

Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/components/navbar/AuditSidebar.tsx
Comment thread frontend/src/services/audits.ts
@sealra
Copy link
Copy Markdown
Collaborator

sealra commented Oct 11, 2024

image

Permite agregar indefinidamente el mismo audit, ¿esto es un comportamiento esperado?

@caverav
Copy link
Copy Markdown
Owner Author

caverav commented Oct 11, 2024

image

Permite agregar indefinidamente el mismo audit, ¿esto es un comportamiento esperado?

@sealra
Copy link
Copy Markdown
Collaborator

sealra commented Oct 11, 2024

image

Si title esta vacío no muestra ningún toast al apretar el botón 'New vulnerability' de por qué no pasa nada (porque retorna error).
image

Al ingresar a la vista, este dropdown inicia vacío, queda algo raro como UI y creo que debería venir ya con una opción.
image

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 9f65de5 and ba50f17.

📒 Files selected for processing (2)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (6 hunks)
  • frontend/src/services/audits.ts (3 hunks)
🧰 Additional context used

Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/services/audits.ts
Comment thread frontend/src/services/audits.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between ba50f17 and 4027e6b.

📒 Files selected for processing (2)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (7 hunks)
  • frontend/src/services/audits.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (4)
frontend/src/routes/audits/edit/findings/add/add.tsx (3)

154-154: El comentario anterior sobre localizar el texto del botón en rowActions sigue siendo válido. Deberías utilizar t('add') en lugar de la cadena hardcodeada 'Add' para garantizar la localización.


159-170: El comentario anterior sobre validar el campo newVulnTitle antes de agregar una nueva vulnerabilidad aún es aplicable. Asegúrate de verificar que newVulnTitle no esté vacío antes de llamar a addFinding para evitar crear hallazgos sin título.


132-150: El comentario anterior sobre la necesidad de resetear newVulnTitle en handleAddVuln sigue siendo relevante. Dado que newVulnTitle no se utiliza en este contexto, es posible que no sea necesario resetearlo aquí.

frontend/src/services/audits.ts (1)

240-248: Definición correcta del tipo 'AuditFinding'

El nuevo tipo AuditFinding está bien definido y mejora la estructura de los hallazgos asociados a las auditorías.

Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx Outdated
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
@iTzGooDLife
Copy link
Copy Markdown
Collaborator

LGTM, aprobaré luego de que @massi-ponce haga implementación de los campos faltantes al agregar findings.

Comment thread frontend/src/services/audits.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 4027e6b and 17172fb.

📒 Files selected for processing (1)
  • frontend/src/routes/audits/edit/findings/add/add.tsx (7 hunks)
🧰 Additional context used

Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Comment thread frontend/src/routes/audits/edit/findings/add/add.tsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 17172fb and b49b75c.

📒 Files selected for processing (1)
  • frontend/src/services/audits.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (2)
frontend/src/services/audits.ts (2)

12-13: Los nuevos campos añadidos al tipo Finding son adecuados

La incorporación de remediationComplexity y priority al tipo Finding mejora la especificación de los hallazgos.


242-250: Definición correcta del nuevo tipo AuditFinding

La creación del tipo AuditFinding aporta claridad y estructura al manejo de hallazgos en auditorías.

Comment thread frontend/src/services/audits.ts
Comment thread frontend/src/services/audits.ts
Comment thread frontend/src/services/audits.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between b49b75c and 43a6bd9.

📒 Files selected for processing (2)
  • frontend/src/i18n/en-US/index.ts (1 hunks)
  • frontend/src/routes/audits/edit/AuditRoot.tsx (5 hunks)
🧰 Additional context used
🔇 Additional comments (1)
frontend/src/i18n/en-US/index.ts (1)

Line range hint 1-148: El resto del archivo parece estar en orden.

He revisado minuciosamente el resto del archivo y no he encontrado más problemas o redundancias. La estructura y organización de las cadenas de localización parecen adecuadas.

Also applies to: 150-605

Comment thread frontend/src/i18n/en-US/index.ts Outdated
Comment thread frontend/src/routes/audits/edit/AuditRoot.tsx
Comment thread frontend/src/routes/audits/edit/AuditRoot.tsx
Comment thread frontend/src/routes/audits/edit/AuditRoot.tsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 504cd57 and 0f09a79.

📒 Files selected for processing (1)
  • frontend/src/routes/audits/edit/AuditRoot.tsx (6 hunks)
🧰 Additional context used
🔇 Additional comments (2)
frontend/src/routes/audits/edit/AuditRoot.tsx (2)

1-6: Importaciones añadidas correctamente

Las importaciones de Cvss3P1 desde 'ae-cvss-calculator' y toast desde 'sonner' están integradas adecuadamente para su uso en el componente.


88-92: Las opciones de ordenamiento se han actualizado correctamente

La inclusión de la opción 'Ascending' en sortOrderOptions mejora la experiencia del usuario al permitir ordenar los resultados en orden ascendente.

Comment thread frontend/src/routes/audits/edit/AuditRoot.tsx
@Icksir
Copy link
Copy Markdown
Collaborator

Icksir commented Oct 13, 2024

Al agregar una vulnerabilidad el sidebar no se actualiza hasta que sales y vuelves a entrar a al auditoria, lo cual no parece el funcionamiento correcto, ya que se debería refrescar sin la necesidad de salir.

Agregando findings: image

Luego de salir y entrar del audit: image

Pienso esto mismo, los cambios siento que deberían visualizarse sin refrescar la página

@jllanosg
Copy link
Copy Markdown
Collaborator

Al agregar una vulnerabilidad el sidebar no se actualiza hasta que sales y vuelves a entrar a al auditoria, lo cual no parece el funcionamiento correcto, ya que se debería refrescar sin la necesidad de salir.
Agregando findings: image
Luego de salir y entrar del audit: image

Pienso esto mismo, los cambios siento que deberían visualizarse sin refrescar la página

esto ya fue respondido si no me equivoco, y es que se escapa del scope del pr; son componentes separados y es más dificil hacer la actualización

Added a 'category' field to the vulnerability details object in audits.ts and vulnerabilities.ts files to categorize vulnerabilities, defaulting to 'No Category' if not specified.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (10)
frontend/src/services/vulnerabilities.ts (4)

12-12: ¡Bien hecho! Pero considera agregar una descripción.

La adición del campo category es una buena mejora para la gestión de vulnerabilidades. Sin embargo, sería beneficioso agregar un comentario que describa el propósito y el uso previsto de este campo para mejorar la claridad del código.

Considera agregar un comentario descriptivo sobre el campo category:

/** Categoría de la vulnerabilidad, útil para clasificación y filtrado */
category?: string;

Line range hint 22-22: Inconsistencia en la definición del campo category.

El campo category en NewVulnerability está definido como string | null, lo cual difiere de su definición en Details. Esto podría llevar a inconsistencias en el manejo de datos.

Te sugiero cambiar la definición para que sea consistente con Details:

category?: string;

Esto permitirá una mayor flexibilidad y coherencia en el manejo de la categoría en toda la aplicación.


Line range hint 32-32: Inconsistencia en la definición del campo category en UpdateVulnerabilityData.

La definición actual del campo category como string | null en UpdateVulnerabilityData no es consistente con su definición en Details. Esto podría causar problemas de tipado y manejo de datos inconsistente.

Te recomiendo cambiar la definición para que sea idéntica a la de Details:

category?: string;

Esta modificación asegurará una mayor consistencia en el manejo de la categoría a lo largo de toda la aplicación y evitará posibles errores de tipado.


Line range hint 1-338: Revisión general de los cambios en vulnerabilities.ts

Los cambios realizados para agregar el campo category son un buen paso hacia la mejora de la gestión de vulnerabilidades. Sin embargo, hay algunas inconsistencias en la implementación que deben ser abordadas:

  1. La definición de category varía entre los tipos Details, NewVulnerability, y UpdateVulnerabilityData.
  2. No se han realizado cambios en las funciones de servicio API para manejar el nuevo campo category.

Para completar esta funcionalidad, considera:

  1. Unificar la definición de category en todos los tipos relacionados.
  2. Actualizar las funciones de servicio API (como postVulnerability, updateVulnerability) para manejar el nuevo campo category.
  3. Asegurarte de que el backend esté preparado para recibir y procesar el campo category.
  4. Agregar pruebas unitarias para verificar el correcto manejo del nuevo campo en todas las operaciones CRUD.

¿Necesitas ayuda para implementar estos cambios adicionales?

frontend/src/routes/audits/edit/AuditRoot.tsx (5)

Line range hint 1-1: Importación de Cvss3P1 y actualización del estado de findings: Aprobado con sugerencia

La adición de Cvss3P1 y la actualización del estado findings son cambios positivos que mejoran la funcionalidad y la estructura de datos. Sin embargo, hay una oportunidad de mejora.

Considera usar un tipo personalizado para la estructura de findings para mejorar la legibilidad y la reutilización. Podrías definirlo así:

type Finding = {
  id: number;
  name: string;
  category: string;
  severity: string;
  identifier: string;
};

const [findings, setFindings] = useState<Finding[]>([]);

Esto hará que el código sea más mantenible a largo plazo.

Also applies to: 28-36


Line range hint 38-55: Función cvssStringToSeverity: Necesita mejoras

La función cvssStringToSeverity es una adición importante, pero tiene algunas deficiencias que deben abordarse:

  1. No manejas el caso de un puntaje CVSS de 0.0, que según el estándar CVSS corresponde a una severidad 'None'.
  2. El manejo de errores actual no es suficiente para un entorno de producción.

Implementa estos cambios para mejorar la función:

const cvssStringToSeverity = (cvssScore: string): string => {
  try {
    const cvssVector = new Cvss3P1(cvssScore);
    const score = cvssVector.calculateExactOverallScore();
    if (score >= 9.0) return 'C';
    if (score >= 7.0) return 'H';
    if (score >= 4.0) return 'M';
    if (score > 0.0) return 'L';
    if (score === 0.0) return 'N';
    return 'I';
  } catch (error) {
    console.error('Vector CVSS inválido:', error);
    // Considera lanzar un error o devolver un valor por defecto
    return 'I';
  }
};

Además, deberías considerar implementar un sistema de logging más robusto para errores en producción.


Line range hint 57-74: Manejo de errores en useEffect: Necesita mejora

El useEffect para obtener y mapear los datos del audit tiene un problema crítico en el manejo de errores:

El uso de console.error para manejar errores no es suficiente en un entorno de producción. Esto podría resultar en errores silenciosos que afecten la experiencia del usuario.

Implementa un manejo de errores más robusto:

useEffect(() => {
  getAuditById(auditId)
    .then(audit => {
      setFindings(
        audit.datas.findings.map((finding: Finding) => ({
          id: finding.identifier,
          name: finding.title,
          category: 'No Category',
          severity: cvssStringToSeverity(finding.cvssv3),
          identifier: finding._id,
        }))
      );
      setAuditName(audit.datas.name);
    })
    .catch(error => {
      console.error('Error al obtener datos del audit:', error);
      // Notificar al usuario sobre el error
      toast.error(t('err.failedToFetchAuditData'));
      // Considerar establecer un estado de error
    });
}, [auditId]);

Esto proporcionará una mejor experiencia de usuario y facilitará la depuración de problemas.


Line range hint 89-131: Funcionalidad de exportación de PDF: Aprobado con sugerencias de mejora

La implementación de la exportación de PDF encriptado es una adición valiosa, pero hay algunas áreas que pueden mejorarse:

  1. Considera extraer la lógica de generación de PDF a un servicio separado para mejorar la organización del código.
  2. El mensaje de error 'Error generating PDF' está en inglés, mientras que otros mensajes usan la función de traducción t(). Mantén la consistencia usando:
throw new Error(t('err.errorGeneratingPdf'));
  1. Los comentarios en inglés (líneas 151-153) deberían traducirse al español para mantener la consistencia con el resto del código.

Implementa estos cambios para mejorar la calidad y mantenibilidad del código.

Also applies to: 151-153


Line range hint 205-237: Estructura del componente y gestión de props: Sugerencias de mejora

La estructura del componente es sólida, pero hay oportunidades de mejora:

  1. Considera usar PropTypes o TypeScript para tipar las props de los componentes hijos, especialmente AuditSidebar. Esto mejorará la robustez y la documentación del código.

  2. Hay muchas props pasadas a AuditSidebar. Podrías reducir el prop drilling usando el patrón de composición o Context API de React. Por ejemplo:

const AuditContext = React.createContext();

// En el componente padre
<AuditContext.Provider value={{
  activeItem,
  connectedUsers,
  fileTypes,
  findings,
  // ... otras props
}}>
  <AuditSidebar />
</AuditContext.Provider>

// En AuditSidebar
const { activeItem, connectedUsers, /* ... */ } = useContext(AuditContext);

Esto hará que tu código sea más mantenible y escalable a largo plazo.

frontend/src/services/audits.ts (1)

587-587: ¡Bien hecho! Adición de categoría a los hallazgos.

La inclusión del campo category en la solicitud POST es una mejora valiosa. Permite una mejor organización y filtrado de las vulnerabilidades en los informes de auditoría. El uso del operador de coalescencia nula ?? es apropiado para manejar casos donde la categoría no está definida.

Sin embargo, para mejorar la claridad del código, considera extraer esta lógica a una constante con un nombre descriptivo. Esto haría que la intención del código sea aún más evidente.

Podrías refactorizar el código de la siguiente manera:

+ const findingCategory = data.details[detailIndex].category ?? 'No Category';
  body: JSON.stringify({
    // ... otros campos ...
-   category: data.details[detailIndex].category ?? 'No Category',
+   category: findingCategory,
    // ... resto del cuerpo ...
  }),

Esta pequeña refactorización mejora la legibilidad del código sin cambiar su funcionalidad.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 0f09a79 and 5830582.

📒 Files selected for processing (4)
  • frontend/src/i18n/en-US/index.ts (0 hunks)
  • frontend/src/routes/audits/edit/AuditRoot.tsx (1 hunks)
  • frontend/src/services/audits.ts (1 hunks)
  • frontend/src/services/vulnerabilities.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • frontend/src/i18n/en-US/index.ts
🧰 Additional context used

Comment thread frontend/src/services/audits.ts
Copy link
Copy Markdown
Collaborator

@massi-ponce massi-ponce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iTzGooDLife iTzGooDLife self-requested a review October 15, 2024 01:36
Copy link
Copy Markdown
Collaborator

@iTzGooDLife iTzGooDLife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Por favor al mergear hacer el issue para el refresco de los findings al momento de ser agregadas.

Copy link
Copy Markdown
Collaborator

@jllanosg jllanosg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gut

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants