Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLA config: allow to use it on all levels #11300

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Nov 20, 2024

Original PR: #10025

@github-actions github-actions bot added New Migration Adding a new migration file. Take care when merging. apiv2 ui labels Nov 20, 2024
Copy link

dryrunsecurity bot commented Nov 20, 2024

DryRun Security Summary

The pull request enhances the Dojo application's Service Level Agreement (SLA) management functionality by adding new models, serializers, and views to support granular SLA configuration, asynchronous updates, authorization controls, and improved information display across products, engagements, and tests.

Expand for full summary

Summary:

The code changes in this pull request are focused on enhancing the Service Level Agreement (SLA) management functionality in the Dojo application. The key changes include:

  1. SLA Configuration Management: New models and serializers have been added to support configuring SLA settings at the product, engagement, and test levels. This allows for more granular control over SLA requirements.

  2. Asynchronous SLA Expiration Date Updates: When SLA configurations are updated, the code ensures that the SLA expiration dates for associated findings are recalculated asynchronously. This helps maintain data integrity and prevent potential issues with inconsistent SLA data.

  3. Authorization and Access Control: The code includes various checks to ensure that users can only perform actions they are authorized to do, such as editing SLA configurations or managing tests and findings. This is an important security consideration.

  4. SLA Information Display: The changes include updates to the user interface to display SLA information, such as the time to remediate for different severity levels. This provides more visibility into the organization's security posture, but it's important to ensure that sensitive information is not exposed.

Overall, the changes appear to be focused on improving the SLA management functionality of the Dojo application, which can have a positive impact on the organization's ability to effectively track and remediate security vulnerabilities. From an application security perspective, the changes introduce some new security considerations, such as ensuring proper input validation, access control, and secure handling of sensitive data. Thorough review and testing of the implemented changes are recommended to identify and address any potential security risks.

Files Changed:

  1. dojo/db_migrations/0219_engagement_sla_configuration_test_sla_configuration.py: This database migration adds new fields to the Engagement and Test models to support SLA configurations.

  2. dojo/api_v2/serializers.py: The changes in this file focus on validating SLA configuration updates to prevent issues when the product's async_updating flag is set.

  3. dojo/engagement/views.py: The changes handle the new SLA_Configuration model and update the SLA expiration dates when the configuration is changed.

  4. dojo/forms.py: The changes disable the ability to modify the SLA configuration for tests associated with products or engagements that have findings being asynchronously updated.

  5. dojo/sla_config/views.py: The changes prevent the deletion of SLA configurations that are currently in use by products, engagements, or tests.

  6. dojo/sla_config/helpers.py: The changes introduce an asynchronous task to update SLA expiration dates and reset the async_updating flags.

  7. dojo/models.py: The changes introduce the SLA_Configuration model and update the Engagement, Test, and Finding models to support SLA functionality.

  8. dojo/product/views.py: The changes handle the update of a product's SLA configuration and the asynchronous recalculation of SLA expiration dates.

  9. dojo/templates/dojo/view_eng.html: The changes add a new section to display SLA information for the engagement.

  10. dojo/test/views.py: The changes add logic to handle the SLA configuration associated with a test and the recalculation of SLA expiration dates.

  11. dojo/templates/dojo/view_test.html: The changes add a new column to display the SLA information for each test.

Code Analysis

We ran 9 analyzers against 11 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

@kiblik kiblik force-pushed the sla_config_other_layers branch 2 times, most recently from 9644307 to 3fb7bb0 Compare November 20, 2024 17:47
@Maffooch
Copy link
Contributor

@kiblik Something I did not realize until taking another look at this new PR, there could be some tricky accommodations here when considering the possibility to update the SLA config on an engagement, while a product is running it's process of updating the SLA config.

Without doing a deep dive on any other tripwires, the amount of complexity that would be needed to accommodate these extra SLA config levels make me question the value that they will provide.

@kiblik
Copy link
Contributor Author

kiblik commented Nov 25, 2024

@kiblik Something I did not realize until taking another look at this new PR, there could be some tricky accommodations here when considering the possibility to update the SLA config on an engagement, while a product is running it's process of updating the SLA config.

Without doing a deep dive on any other tripwires, the amount of complexity that would be needed to accommodate these extra SLA config levels make me question the value that they will provide.

I planned to run the absolutely same validation + restriction as it is already implemented on product level:

def validate(self, data):
async_updating = getattr(self.instance, "async_updating", None)
if async_updating:
new_sla_config = data.get("sla_configuration", None)
old_sla_config = getattr(self.instance, "sla_configuration", None)
if new_sla_config and old_sla_config and new_sla_config != old_sla_config:
msg = "Finding SLA expiration dates are currently being recalculated. The SLA configuration for this product cannot be changed until the calculation is complete."
raise serializers.ValidationError(msg)

With reference to product.sla_configuration. So updates will be possible to perform only on the "whole product" level. I didn't plan to implement this kind of lock for Engs or Tests. Yes, it might look like a waste of resources (change of sla_config on the test level will trigger recalculation for the whole product) but I wasn't able to find a more elegant solution for now.

@kiblik kiblik force-pushed the sla_config_other_layers branch 5 times, most recently from a33f0a3 to f8b37d6 Compare December 2, 2024 15:25
@kiblik kiblik force-pushed the sla_config_other_layers branch from f8b37d6 to e6e03c7 Compare December 11, 2024 14:23
@mtesauro
Copy link
Contributor

Sorry for the delayed response.

I'm a bit concerned about the complexity this adds generally beyond what Cody raised ☝️

If we're documenting or explaining to people how DefectDojo works, we're going to have to say we have SLAs at Product, unless, they're at the Engagement, or unless it's at the test level. We'd also have to add SLA UI to both the Engagement and Test level (as well as API) to fully make this discoverable by DefectDojo users. We already have a rather cluttered UI with many UI elements that are used by a small portion of the community and it appears this would only add to that UI deficit. I'd rather spend cycles on updating the UI to a more modern toolkit/library TBH.

I'm also concerned about how a change of SLA at the test level will cause recalculation for the entire product. We're seeing people with 9700+ products doing 26K tests per day so for a group of our users the extra calculation could be debilitating.

I guess what I'm getting at is that the benefits of having SLA at Engagement & Test aren't worth the extra complexity and computation.

Personally, I've only ever used SLAs at the Product level and the more I think about it, I begin to wonder if needing SLAs at something other then Product is more of a data modeling problem then a DefectDojo feature problem. I'd rather have more products then have the cognitive load of remember what Engagement or Test had what SLA - that or having to look to the UI to tell me that.

Maybe I'm not describing it all that well here but in my gut this feels like a poor choice / trade-off between more features vs complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apiv2 New Migration Adding a new migration file. Take care when merging. ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants