simonec73 / threatsmanager

Threats Manager Platform Core libraries and SDK
MIT License
69 stars 13 forks source link

Removal All Threat Events - Double warning if mitigations have been associated to the threat event #34

Closed rsrinivasanhome closed 1 year ago

rsrinivasanhome commented 1 year ago

Is your feature request related to a problem? Please describe. I often have to click on the remove all threats button . We generally get a confirmation message "You are about to remove all Threat events assigned in the model" . Sometime I associate mitigations and mistakenly click on the "Remove all threat events" button . The relationship between threat event and mitigation is removed which is expected. If I could get a warning before that it would nice.

Describe the solution you'd like

No mitigations associated with threat events no change in flow

User Clicks on "Remove All Threats" A confirmation message "You are about to remove all Threat events assigned in the model" On click of oK threat events are removed

Mitigation associated with threat event User Clicks on "Remove All Threats" A confirmation message "You are about to remove all Threat events assigned in the model" On Click of ok another confirmation message " If you remove the threat events the mitigations will also be dissociated from the threat event " on click of ok the threat events should be removed.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

simonec73 commented 1 year ago

Hi, when you click the Remove all Threat Events button, the tool already asks you for confirmation. If you confirm, it removes the Threat Events. If I understand correctly, you are asking to have an additional request for confirmation. I do not think that this is necessary. Not only the tool already asks once, but the next version is going to have Undo/Redo. The implementation of the Undo/Redo capability is already in an advanced state, therefore it definitely is going to be included. Does it address your request?

rsrinivasanhome commented 1 year ago

The additional confirmation is only required if a threat event has been associated to a mitigation. We generally change the properties of the objects in the threat model and click on "Remove all threat button" and then click on "Generate Threat events and mitigation" to regenerate the threats based on the change in properties. This is because of issue https://github.com/simonec73/threatsmanager/issues/24

If a mitigation is assigned to the threat event its association is also removed. So if we have a second confirmation which only appears if mitigations assigned example - "If you remove the threat events the mitigations will also be dissociated from the threat event " it would make the user think twice before clicking on ok and dissociating the mitigations to the threat events.

Will the undo/redo be able to undo the regeneration of threats ?

simonec73 commented 1 year ago

No, this is not necessary. It is obvious that mitigations are going to be removed. What instead is necessary is to remove the automatically generated threats and related mitigations where no changes have been performed. This is something I am planning to do with a new version.