mike-goodwin / owasp-threat-dragon-core Goto Github PK

The diagram Paper is automatically extended when dragging diagram elements right or downwards. The Delete button removes all elements but leaves the extended Paper.
Ideally the Paper would return to the default original size when all elements are deleted using the Delete button.

@andk123 writes
add a name to the boundary like this:

It can be useful to identify the origin of a said boundary when doing threat modelling

Some files are not used and could be removed:

• src/content/threatdragon.css
• src/content/threatdragon.min.css

The travis build is failing with

sh: 0: Can't open /etc/init.d/xvfb
The command "sh -e /etc/init.d/xvfb start" failed and exited with 127 during 

so the file .travis.yml may need to be updated for this?

If grid lines are added to the apps, an additional helpful feature would be "snap-to-grid". This would make it easier to align elements for neater models.

Depends on #14

There is a bug in src/diagrams/diagramdirectives.js where (technically) both cellX and cellY are out of scope ... although js handles this gracefully

🚨 You need to enable Continuous Integration on Greenkeeper branches of this repository. 🚨

To enable Greenkeeper, you need to make sure that a commit status is reported on all branches. This is required by Greenkeeper because it uses your CI build statuses to figure out when to notify you about breaking changes.

Since we didn’t receive a CI status on the greenkeeper/initial branch, it’s possible that you don’t have CI set up yet. We recommend using Travis CI, but Greenkeeper will work with every other CI service as well.

If you have already set up a CI for this repository, you might need to check how it’s configured. Make sure it is set to run on all new branches. If you don’t want it to run on absolutely every branch, you can whitelist branches starting with greenkeeper/.

Once you have installed and configured CI on this repository correctly, you’ll need to re-trigger Greenkeeper’s initial pull request. To do this, please click the 'fix repo' button on account.greenkeeper.io.

The save button is only enabled after changes are made in the diagram, and the element is de-selected. This has confused some users of Threat Dragon, so to avoid concern it is proposed to keep the save button always enabled.

It is a good thing to disable the save button when no changes have been made, so issue #97 has been raised so this feature should be reinstated.

It would be useful to be able to duplicate existing diagram with threats within the same model as it is simpler to create multiple diagrams for each user story instead of having a single diagram (TD already supports multiple diagram creation in one model)

In the same spirit, it should also be useful to be able to duplicate existing element (process, store,..) with their threats from a diagram (via context menu ?) to speed up the modeling.

@thomaskonrad reports:
The fact that I have to edit the model to add a diagram: That seems counter-intuitive to me. I'd put the diagrams into the edit view of the model, and only put metadata into the edit dialog.

when running npm audit --production there are 18 vulnerabilities (4 moderate, 14 high) reported in 789 scanned packages. Suggest update for

• jointjs
• lodash
• jquery
• snyk

An enhancement, would be useful if there was markdown formatting for text boxes, such as High level system description .

This is probably the wrong place to ask questions like this but I didn't find another place.

Severity can be rated in 3 levels. It looks very similar to TMT priority.
What exactly is the practical use of severity?
From a classical risk oriented approach a priority is similar to a resulting risk with different levels before and after mitigation. Before mitigation it helps to prioritize activities applying countermeasures.
Risk is defined as a product of impact and likelihood if you want to simplify. Impact in my opinion is simply characterized by sensitivity of the data processed in the data flow and broken SLA. Both are different (business) impacts to be referenced to the character of the STRIDE attack vector. The other dimension likelihood is just a guess how easy it is to materialize the threat.
If possible I try to setup likelihood and impact based on the companies risk definition and throw out the risk level as a result.
Using priority TMT had the "problem" that it was setup by guessing not having a basis with risk assessment. Therefore it produced no real value for me. I changed it to calculate risks in a spreadsheet instead of prioritizing within TMT.
It would be helpful to know what the reason is for implementing severity this way.

It would be good to be able to specify bidirectional data flows, as well as the existing single direction data flow, as it would make some diagrams look less busy and would not detract from the information in the threat model

At present if the 'Save' button is greyed out, ie no changes have been made to the diagram since the last save, then if:

1. an element is moved
2. the save button is still inactive
3. the diagram can only be saved using the Electron pull down menu 'Save'
4. or if another element/background is selected then save button becomes active

It would be good for the 'Save' button to become active as soon as any change to the diagram is made, such as moving an element.

@thomaskonrad reports:
The protocol isn't shown in the Data Flow: There is no indicator which protocol is in use, or whether it's encrypted, although I can specify these properties.

Since version 0.7.1 and the present version 0.8.0: the behaviour of the duplicate feature has changed and the duplicated element has an empty 'name' label.
This works in version 0.7.1, just not now, and is not picked up in the tests.

• copy must fill in the new element name with 'Copy of ...'
• tests to be extended to so that this bug is picked up

I'm unable to get the Cut and Past feature to work on OSX desktop version 1.1.0 from the "Edit" menu or when using command x (⌘X) and command v ( ⌘V).

The sample diagram has some elements (background worker, worker config, etc) which show in red. It's not clear what red means. (dashed lines seem to be used for both trust boundaries and Out of scope).

I suggest adding a key, but possibly alternately reducing use of color to address the threat of black & white printing.

The text areas for :

• High level system description
• Element Description
• Element Reason for out of scope
• Threat Description
• Threat Mitigation
  do not preserve the text formatting such as line breaks in the report
  In addition it would be good if the High level system description preserved line breaks after editing

It is redundant work to type threat's description and mitigations. It'll be easy to copy threats over from place to other. And it'll be convenient to have copy and paste options to copy/paste text all around

A new version of angular is available:
"angular": "1.7.9"
update from existing 1.7.8

It would be good to have the ability to move several elements at once in the diagram editor pane. I have drawn the diagram to far over to the right, and wished to centre it, but could not move more than one element at once.

@thomaskonrad reports:
I cannot drag whole trust boundaries: When I point at a trust boundary, the cursor indicates that I can drag and drop the whole boundary, but instead, a new point is added to the curve, which I then drag. I could not find out a way to move a whole trust boundary at once.

STRIDE isn't used to categorise threats, but as an aid to discovering them, so some threats may feel naturally associated with more than one STRIDE type.

This is a severe bug and should be fixed as soon as possible.
Say there are 3 diagrams, ids 0, 1, 2
delete middle diagram to get ids 0, 2
then clicking on right hand diagram does not find id 1, and 'loading ...' indefinitely
duplicate new diagram will result in ids 0, 2, 1
then clicking on middle diagram will edit id 1, not 2

According to the width of the main window, a vertical dead space is present on the right hand side of the diagram edit area:

This now fails, and this is associated with an error when duplicating a boundary in the diagram edit view.
This is a corner case, in practice it is very rare that a boundary is duplicated, but the test is quite rightly picking this up.

Tested on MacOS Catalina version 10.15. Steps to reproduce:

1. Create a new diagram
2. Save it
3. Make some changes
4. Discard changes - but changes are not reverted
   This does not happen when opening an existing TM

A nice feature to add to the web and Electron apps would be grid lines that could be toggled. This would make it easier to align elements for neater models.

@thomaskonrad reports:
Data Flow arrows are misaligned: The arrows seem to point towards the direct line between two objects, instead of the direction of the very last part of the curve. That makes it misaligned when it's curved.

The zoom feature was removed temporarily, issue #94 .
This feature needs to be reinstated so that:

• when zooming in it is possible to scroll across the paper
• when zoomed out, the paper size is adjusted when dragging elements across the paper
• it is possible to pan across the whole diagram when zoomed in, see issue #72
• anchors on the elements are scaled correctly when zoomed in, see issue #73
• dead space can not be created by zooming in and adding elements, see issue #60

When running npm install there are warnings. These packages could be updated so that the warnings are reduced.

 npm install
npm WARN deprecated [email protected]: This module is no longer maintained, try this instead:
npm WARN deprecated   npm i nyc
npm WARN deprecated Visit https://istanbul.js.org/integrations for other alternatives.
npm WARN deprecated [email protected]: this package is now deprecated
npm WARN deprecated [email protected]: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated [email protected]: CoffeeScript on NPM has moved to "coffeescript" (no hyphen)
npm WARN deprecated [email protected]: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)
npm WARN deprecated [email protected]: This module is no longer maintained, try this instead:
npm WARN deprecated   npm i nyc
npm WARN deprecated Visit https://istanbul.js.org/integrations for other alternatives.
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated [email protected]: Jade has been renamed to pug, please install the latest version of pug instead of jade
npm WARN deprecated [email protected]: This module moved to @hapi/hawk. Please make sure to switch over as this distribution is no longer supported and may contain bugs and critical security issues.
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated [email protected]: Please update to at least constantinople 3.1.1
npm WARN deprecated [email protected]: Deprecated, use jstransformer
npm WARN deprecated [email protected]: This version has been deprecated in accordance with the hapi support policy (hapi.im/support). Please upgrade to the latest version to get the best features, bug fixes, and security patches. If you are unable to upgrade at this time, paid support is available for older versions (hapi.im/commercial).
npm WARN deprecated [email protected]: This version has been deprecated in accordance with the hapi support policy (hapi.im/support). Please upgrade to the latest version to get the best features, bug fixes, and security patches. If you are unable to upgrade at this time, paid support is available for older versions (hapi.im/commercial).
npm WARN deprecated [email protected]: This version has been deprecated in accordance with the hapi support policy (hapi.im/support). Please upgrade to the latest version to get the best features, bug fixes, and security patches. If you are unable to upgrade at this time, paid support is available for older versions (hapi.im/commercial).
npm WARN deprecated [email protected]: This module moved to @hapi/sntp. Please make sure to switch over as this distribution is no longer supported and may contain bugs and critical security issues.
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated [email protected]: Please use the native JSON object instead of JSON 3
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
...
npm WARN notsup Unsupported engine for [email protected]: wanted: {"node":"0.10 || 0.12 || 4 || 5 || 6 || 7 || 8"} (current: {"node":"13.2.0","npm":"6.14.2"})
npm WARN notsup Not compatible with your version of node/npm: [email protected]

We are assessing threat dragon for our threat modeling workshops and lately we started to extend it to privacy threats. We came accros the great LINDDUN framework that is very similar to STRIDE and also uses DFDs.

Therefor it would be great to have either LINDDUN categories in the threat engine or even a "custom" threat option where users can create a list of custom threat categories that are displayed in the drop-down.

This is a bug introduced by PR #102
If the line of text is long, then it can go beyond the text area in the projects description and reports. Although there is no loss of information, this looks wrong.

The zoom "+" and "-" buttons scale the diagram with the top left corner as an anchor point, so zooming in pushes items on right and bottom out of the viewport. For bigger diagrams, this may become a usability issue.

Proposals for improvement:

Preferred : Add a "hand" icon that allows you to drag the view to add pan functionality (most intuitive)

Alternatively : When an element is selected, use a point on the selected element to serve as anchor point for the zoom operation instead of the top-left corner.

If a diagram is deleted, and then another one added, then the diagram ID may be duplicated.

At present correct operation depends on the deleted diagram being the last in the diagram list, so that a new diagram takes it place. However, if the deleted diagram is not the last on the list then it is given the same id as a diagram further down the list

After cloning and running script npm install, when invoking npm run-script build the following error is obtained:

> rework-npm ./src/content/app.css -o ./src/content/threatdragon-core.css
sh: 1: rework-npm: not found

the package rework-npm-cli is needed in the package JSON

When selecting an item in the threat model, then "suggest threats for the selected element", a dialog pops up with a suggest threat. Accepting or Ignoring the dialog may cause further threats to show up. It would be useful if the title of the dialog could show a count. E.g. "Add this threat? (1 / 4)"

The save button was only enabled after changes are made in the diagram, but this feature has been temporarily removed in issue #96 .
The problem was that the save button was only enabled after an element is de-selected. This confused some users of Threat Dragon, so to avoid concern the save button is always enabled.

It is a good thing to disable the save button when no changes have been made, so this feature should be reinstated.

When grid is turned on, Store and Process components do not have grid points within them.
However Actor components have grid points shown within, ideally Actor components should behave the same as Store and Process components.

Tests are now failing for src/diagrams/diagramdirectives.js, probably due to the change in use of boundary box. Tests failures are:
resize and scroll

• should scroll the diagram to the right
• should scroll down
• should expand the diagram to the right
• should expand the diagram down

When zooming in for the attached threat model, there is a loss of vertical scrolling ability, so that elements of the diagram are not accessible.
MacOSX 10.15
TD version 0.6.3
test.json.txt
Note that once the canvas is increased downwards then zooming+scrolling works as expected.

Arc trust boundaries are ambiguous. For example, in the sample diagram, one arc can be extended to split "background worker process", another can be extended to bisect "database."

Closed boundaries (such as boxes) are more clear, and should be the default shape.

See sccreenshot:

The zoom feature and buttons make it possible to lose parts of the diagram. Because the jointjs 'scale' only scales the elements and not the paper, if elements are dragged or placed when zoomed out then these can be inaccessible when zoomed back in. This results in these elements only being accessible when zoomed out which is very difficult to use.
Until the zooming, scaling, panning and paper size can be fixed it is best to remove the zoom feature. Issue #95 has been raised to reinstate the zoom feature

It would be good to increase the number of browsers we test against, at present it is:

• Chrome
• Firefox
• IE
• PhantomJS
  Consider using Karma-detect-browsers: https://www.npmjs.com/package/karma-detect-browsers
  to increase coverage to:
• Opera
• Safari
• Edge

Like the title says: it would be cool if selection and move of multiple elements would be supported.
(If it is supported: I'm sorry I couldn't figure out how 😉 )

Undo command is missing in Edit - it makes life very difficult when you accidentally delete something by mistake (Redo is needed as well)

It would be useful to add a description field to each diagram element - except for boundary. This has come up through use of Threat Dragon in dev teams within ForgeRock, and there has been a need to add descriptive text to the elements.
The dev teams had a work around which was to put the element out of scope, add a description into the 'Reason for out of scope' box, and then put the element back in scope. This is not ideal, and so adding a Description to the element would help.