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

feat: Adds Open Threat Modeling (OTM) support #787

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

mmcdonald4tw
Copy link
Collaborator

@mmcdonald4tw mmcdonald4tw commented Nov 17, 2023

Summary:
This change resolves issue #440 by adding import and export capabilities for threat models in the Open Threat Model format. The feature specifically targets the web applications Open and Save functions and allows for the conversion between the Threat Dragon format and the OTM 2.0 format.

Description for the changelog:
Added Open and Save feature to the web application to support threat models in the Open Threat Model (OTM) format.

Other info:
Change required updates to existing unit test cases and a new test case was created to cover the OTM conversion methods.

Thanks for submitting a pull request!
Please make sure you follow our code_of_conduct.md and our contributing guidelines contributing.md

- Enhances the web application interface to allow for the import and export of threat models in the OTM format.
- resolves OWASP#440
@jgadsden jgadsden self-requested a review November 21, 2023 07:18
@jgadsden
Copy link
Collaborator

many thanks for this important feature, and looking at this now @mmcdonald4tw

@mmcdonald4tw
Copy link
Collaborator Author

many thanks for this important feature, and looking at this now @mmcdonald4tw

Thanks @jgadsden , please let me know if you see anything that you would like for me to adjust.

@jgadsden
Copy link
Collaborator

Looking very good indeed, many thanks again @mmcdonald4tw
One thing got me thinking: the save button in the diagram edit requires a selection between OTM and TD each time. This may become error prone - the user thinks they saved in OTM but actually saved in TD for example.

So I thought I would collect my thoughts on the use cases

The user can open a model in 3 ways:

  • via paste or drop file which is handled by onImportClick in td.vue/src/views/ImportModel.vue
  • via open file dialog which again is handled by onImportClick in td.vue/src/views/ImportModel.vue
  • via a desktop pull down menu, which is handled by window.electronAPI.onOpenModel in td.vue/src/main.desktop.js but only for the desktop builds

The file is then converted if necessary to TD version 2.0 format from either OTM or TD version 1.x. There are also a couple of ways of opening a new model, but that is not relevant for our purposes. So the user has deliberately selected either a TD or an OTM file, and does not actually care (too much) what the format is within Threat Dragon.

The user does care (probably a lot) about what format the file is saved in. They may be disgruntled if they end up overwriting their OTM file in TD format and do not realise it until later, for example. It would be nice to use OTM exclusively for TD from now on, but that would probably upset our existing users :)

Anyhow, the model is saved from 3 places:

  • Save button in the diagram edit mode handled by THREATMODEL_SAVE veux action
  • Save button in the threat model edit mode again handled by THREATMODEL_SAVE veux action
  • opening a new empty model from desktop pull down or main window, handled by THREATMODEL_CREATE veux action

@jgadsden
Copy link
Collaborator

jgadsden commented Nov 21, 2023

So maybe the user should make the decision to store the model in TD or OTM format from the threat model edit window; where all the meta information is viewed and edited? And this decision is reused until the user changes their mind?
To be discussed :)

@mmcdonald4tw I will use the example file from the IriusRisk Open Threat Model repo

@mmcdonald4tw
Copy link
Collaborator Author

Thank you @jgadsden , a lot of great thoughts here.

I would need to take a deeper look at the code again but it sounds like the design would be to add an additional field to the model meta data that stored the current file format. This file format field would be populated at the time a model is opened:

  1. Open TD model -> File format field = TD.
  2. Open OTM model -> File format field = OTM.

The current file format data could be displayed in the UI next to the other meta data but would maybe be a drop-down allowing it to be changed. Changing the file format field in an open model would just change the format in which the model is saved in. The save function would look for this meta data field to determine if it should be saved out as TD or OTM. This would allow for only one, standard save button.

Have i understand your idea correctly? Is this something that you would like me to change within the current PR?

@jgadsden
Copy link
Collaborator

jgadsden commented Nov 22, 2023

Hello @mmcdonald4tw , I agree with what you are suggesting - thanks for working out my convoluted thoughts :)

The OTM models would always have something like "otmVersion": "0.2.0" in the JSON file? Similar to threat dragon's "version": "2.0.0", but different enough so that the file format can be determined when opening the file, and then quietly saving the file in the same format. So the user need never have to make a conscious decision?

But at some point they will think 'Hey, I would like to save as OTM now, rather than TD', and I like your suggestion of what buttons / drop down menu to put in place to allow them to do that

@jgadsden
Copy link
Collaborator

I can make pull requests to your branch mmcdonald4tw:otm-support if that helps

@jgadsden
Copy link
Collaborator

jgadsden commented Nov 24, 2023

thanks @mmcdonald4tw for access to the repo, I will try out some suggestions this weekend

@mmcdonald4tw
Copy link
Collaborator Author

Sounds good @jgadsden , I am also available and may do some investigation/PoC coding around these ideas.

@jgadsden
Copy link
Collaborator

@mmcdonald4tw version 2.1.2 was released yesterday because of a problematic bug that needed to be fixed, but version 2.1.3 can be released next month with the OTM feature when it is ready

@mmcdonald4tw
Copy link
Collaborator Author

Hi @jgadsden , I took some time to walk through a few use cases in TD related to our discussion and wanted to share my thoughts.

Opening an existing model:
Since both the TD and OTM formats are json, a seamless opening experience as developed in this PR seems to be most fitting. The transformation takes place under the hood to allow all of the existing TD functionality work by converting the OTM to TD at opening. The only improvement I could note for the future would maybe to add some type of visual indicator within the UI to remind the user what type of model they have opened.

Creating a new model:
With this PR, TD still functions completely with the model object in the standard TD format and therefore the thought of OTM should only come into play once the users needs to persist the model.

Saving a model:
Several thoughts here, some of which of personal opinion/preference...

  1. You mentioned a concern that the user may become confused and accidentally save one format over another. I tested this in the current PR and the Vue save method always produces a brand new copy json file within the downloads folder. The user would never be at risk of overwriting an existing model or the model that was originally opened.
  2. I believe that the most commonly utilized Save button would be the one that is located at the top right corner of the diagram edit page. The user would typically be saving changes that they have just made within the diagram. This button is the one that this PR has altered to provide the option of TD or OTM formats.
  3. We discussed an altered design that would have a generic save button on the diagram edit page and have the format field located in the model meta data page. After walking through a few scenarios as a user, I see two concerns with this. Firstly i believe that users may feel that having the format field in the meta data page as just added steps to changing the save format. This would require the user to close the diagram edit page, then open the meta data edit page, change the save format and then save. Secondly, this flow deviates from the common application save flows that I have experienced. The most common being the standard File -> Save / Save As function.

I look forward to hearing your thoughts and continuing the discussion.

@jgadsden
Copy link
Collaborator

Hello @mmcdonald4tw you make 3 very good points and I agree
I think we should go ahead and merge this pull request, and then if the community disagrees then it can always suggest differently :)

@jgadsden jgadsden merged commit 73142a8 into OWASP:main Nov 29, 2023
1 check passed
Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

all good, merged, many thanks @mmcdonald4tw

@rorybray
Copy link

I have tried every which way I can think of and the new version (2.1.3) simply will not open, save, import or export an OTM model. What am I missing? I've tried a local build, docker container and the desktop app. The app comes closest in that it actually has an export to OTM in the file menu but it is disabled. If I "open existing" and pass in the EXAMPLE.json from IriusRisk it still seems to try to parse it as a TD model.

@jgadsden
Copy link
Collaborator

The support of Open Threat Model is not functional with the release of Treat Dragon version 2.1.3
Apologies @rorybray , this loss of functionality was not planned: it was an unintended side-effect of getting the Save functionality working for desktop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request version-2.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for consuming and generating Open Threat Model (OTM)
3 participants