mike-goodwin / owasp-threat-dragon-desktop

An installable desktop variant of OWASP Threat Dragon
Apache License 2.0
594 stars 90 forks source link

Can't save on Mac after adding threat #73

Closed coillarach closed 4 years ago

coillarach commented 4 years ago

Saving seems to work fine when the diagram only consists of dataflow objects. As soon as a threat is added, none of the save options work. The menu option displays a save file dialog even though the json file already exists and overwriting it removes the contents leaving just a JSON null. Using the button in the toolbar displays an on-screen error message which says "Cannot read property 'detail' of null". Removing the threat again does not get rid of the problem. I am using the Desktop version 0.1.26 on MacOS 10.14.6

jgadsden commented 4 years ago

I do not have this problem when running TD v0.1.26 on MacOS 10.15. @coillarach could you try opening the sample model and seeing if you get the same problem? Just trying to rule out any problem with filesystem write permissions. BTW I agree that v01.26 is the latest MacOS release, but it is very old at May 2017. I run the later versions of TD by cloning/ downloading the repo and then 'npm install' followed by 'npm run start'

lmcthbe commented 4 years ago

I have similar issues on 10.15 after cloning the repo, the selected element are not showed and when it does, none of the save options works.

I get around it by installing xcode cli and fixing npm error installation related to gyp and when save option is gray out from the diagram even after having new element, by clicking on Cancel Edit and Cancel it to be back to diagram view, the save button is not gray out anymore and the diagram can be saved properly and re-opened.

jgadsden commented 4 years ago

@lmcthbe could you detail the steps that you take to reproduce this problem on MacOS 10.15 and cloning the latest TD ? I suspect that I do not see it because I am doing something different than you each time. Or is it because there is an error when running npm install?

lmcthbe commented 4 years ago

@jgadsden After fixing the gyp error when doing 'npm install' it helps TD to work better and element can be added to the diagram. About the save button, I have done extra testing and I found that I can get the save button not gray out just by clicking outside of the element that I have moved, it looks that the save button does not appears (after moving an element) when a another element or the background is not selected/clicked. After leaving the saved diagram and closing, everything is in place when re-opening.

So no issue and great tool BTW.

I will introduce it in my organization to perform Risk Assessment when designing new feature.

jgadsden commented 4 years ago

Thanks for the info @lmcthbe - I can reproduce this issue from your instructions. It seems the Electron pull-down 'Save' always works even if the diagram save button is still greyed out. I think this can be corrected in the owasp-threat-dragon-core source code, and have added an enhancement request in that repo: https://github.com/mike-goodwin/owasp-threat-dragon-core/issues/44#issue-519324302

lmcthbe commented 4 years ago

Great.

jgadsden commented 4 years ago

Hello @coillarach - the new version 1.0 of TD desktop is released: https://github.com/mike-goodwin/owasp-threat-dragon-desktop/releases/tag/v1.0 and you should not have this issue any more. Thanks for your help @lmcthbe