Repository: OWASP/threat-dragon (js)

Issue:
Unable to create a new threat model while using GitHub as provider
**Describe the bug**
When using GitHub as the provider, an error occurs when trying to create a new threat model.

**Expected behaviour**
When pressing Save button during the creation of a new threat model while using GitHub as provider, a new model.json file should be created on the GitHub repo.

**Environment**
 - Version: 2.0.1
 - Platform: Web App
 - OS: MacOS
 - Browser: Chrome

**To Reproduce**
- Clone the main branch of the repository
- Configure .env file for using GitHub as the provider
- Start the application
- Enter localhost:8080
- Login using GitHub
- Select the option to start a new model
- Select the repository and branch
- Try to save the new model, with or without changing some values of the new model, like name or description.
- A 500 error occurs during the put method to the update api

**Any additional context, screenshots, etc**
![Screenshot 2023-03-24 at 11 44 56](https://user-images.githubusercontent.com/33671308/227557447-ce889ecf-95b7-4a02-8c23-33687d241b9b.png)

<img width="1032" alt="Screenshot 2023-03-24 at 11 47 25" src="https://user-images.githubusercontent.com/33671308/227558097-1eb850cf-6288-45c7-9c2d-5e793061dc9b.png">


Pull request:
Fix #631 error while creation of threat model when using GitHub
**Summary**
This PR fixes #631

**Description for the changelog**
Allow the creation of a threat model while using GitHub as a provider by making the ThreatModelEdit call the create method of the server when on /create url.

**Other info**
- Creation of ThreatModelCreate (/crete) url for ThreatModelEdit component to differentiate when on creation from editing a threat model.
- Change THREATMODEL_CREATE action of the threatmodel store so the new threat model gets created.
- Creation of createAsync method on threatmodelApi to call /create api endpoint.
- Change from PUT to POST of /create api endpoint of server routes.


Patch:
diff --git a/td.server/src/config/routes.config.js b/td.server/src/config/routes.config.js
index 121ad271..16b70b18 100644
--- a/td.server/src/config/routes.config.js
+++ b/td.server/src/config/routes.config.js
@@ -42,7 +42,7 @@ const routes = (router) => {
     // removed because of security denial of service concerns (denial of models)
     //router.delete('/api/threatmodel/:organisation/:repo/:branch/:model', threatmodelController.deleteModel);
 
-    router.put('/api/threatmodel/:organisation/:repo/:branch/:model/create', threatmodelController.create);
+    router.post('/api/threatmodel/:organisation/:repo/:branch/:model/create', threatmodelController.create);
     router.put('/api/threatmodel/:organisation/:repo/:branch/:model/update', threatmodelController.update);
 };
 
diff --git a/td.vue/src/router/git.js b/td.vue/src/router/git.js
index d31ee828..54d32013 100644
--- a/td.vue/src/router/git.js
+++ b/td.vue/src/router/git.js
@@ -1,4 +1,4 @@
-import { providerTypes } from '../service/provider/providerTypes.js';
+import { providerTypes } from '@/service/provider/providerTypes';
 
 const providerType = providerTypes.git;
 
@@ -29,6 +29,11 @@ export const gitRoutes = [
         name: `${providerType}NewThreatModel`,
         component: () => import(/* webpackChunkName: "new-threatmodel" */ '../views/NewThreatModel.vue')
     },
+    {
+        path: `/${providerType}/:provider/:repository/:branch/:threatmodel/create`,
+        name: `${providerType}ThreatModelCreate`,
+        component: () => import(/* webpackChunkName: "threatmodel-edit" */ '../views/ThreatModelEdit.vue')
+    },
     {
         path: `/${providerType}/:provider/:repository/:branch/:threatmodel/edit`,
         name: `${providerType}ThreatModelEdit`,
diff --git a/td.vue/src/service/api/threatmodelApi.js b/td.vue/src/service/api/threatmodelApi.js
index 0201f13f..555778c9 100644
--- a/td.vue/src/service/api/threatmodelApi.js
+++ b/td.vue/src/service/api/threatmodelApi.js
@@ -54,6 +54,11 @@ const modelAsync = (fullRepoName, branch, model) => {
     return api.getAsync(`${resource}/${org}/${repo}/${branch}/${model}/data`);
 };
 
+const createAsync = (fullRepoName, branch, modelName, threatModel) => {
+    const { org, repo } = extractRepoParts(fullRepoName);
+    return api.postAsync(`${resource}/${org}/${repo}/${branch}/${modelName}/create`, threatModel);
+};
+
 const updateAsync = (fullRepoName, branch, modelName, threatModel) => {
     const { org, repo } = extractRepoParts(fullRepoName);
     return api.putAsync(`${resource}/${org}/${repo}/${branch}/${modelName}/update`, threatModel);
@@ -65,5 +70,6 @@ export default {
     modelsAsync,
     organisationAsync,
     reposAsync,
+    createAsync,
     updateAsync
 };
diff --git a/td.vue/src/store/modules/threatmodel.js b/td.vue/src/store/modules/threatmodel.js
index 2642e14f..04d6d020 100644
--- a/td.vue/src/store/modules/threatmodel.js
+++ b/td.vue/src/store/modules/threatmodel.js
@@ -2,9 +2,9 @@ import Vue from 'vue';
 
 import demo from '@/service/demo/index.js';
 import isElectron from 'is-electron';
-import { getProviderType } from '../../service/provider/providers.js';
+import { getProviderType } from '@/service/provider/providers';
 import i18n from '../../i18n/index.js';
-import { providerTypes } from '../../service/provider/providerTypes.js';
+import { providerTypes } from '@/service/provider/providerTypes';
 import {
     THREATMODEL_CLEAR,
     THREATMODEL_CONTRIBUTORS_UPDATED,
@@ -29,7 +29,7 @@ export const clearState = (state) => {
     }
     state.all.length = 0;
     state.data = {};
-    state.fileName = '',
+    state.fileName = '';
     state.immutableCopy = '';
     state.selectedDiagram = {};
 };
@@ -49,7 +49,30 @@ const setThreatModel = (theState, threatModel) => {
 
 const actions = {
     [THREATMODEL_CLEAR]: ({ commit }) => commit(THREATMODEL_CLEAR),
-    [THREATMODEL_CREATE]: ({ commit }, threatModel) => commit(THREATMODEL_CREATE, threatModel),
+    [THREATMODEL_CREATE]: async ({ dispatch, rootState, state }) => {
+        try {
+            if (getProviderType(rootState.provider.selected) !== providerTypes.local) {
+                await threatmodelApi.createAsync(
+                    rootState.repo.selected,
+                    rootState.branch.selected,
+                    state.data.summary.title,
+                    state.data
+                );
+                Vue.$toast.success(i18n.get().t('threatmodel.saved') + ' : ' + state.fileName);
+            } else if (isElectron()) {
+                // desktop version always saves locally
+                await window.electronAPI.modelSaved(state.data, state.fileName);
+            } else {
+                // save locally for web app when local login
+                save.local(state.data, `${state.data.summary.title}.json`);
+            }
+            dispatch(THREATMODEL_SET_IMMUTABLE_COPY);
+        } catch (ex) {
+            console.error('Failed to update threat model!');
+            console.error(ex);
+            Vue.$toast.error(i18n.get().t('threatmodel.errors.save'));
+        }
+    },
     [THREATMODEL_DIAGRAM_SELECTED]: ({ commit }, diagram) => commit(THREATMODEL_DIAGRAM_SELECTED, diagram),
     [THREATMODEL_DIAGRAM_UPDATED]: ({ commit }, diagram) => commit(THREATMODEL_DIAGRAM_UPDATED, diagram),
     [THREATMODEL_FETCH]: async ({ commit, dispatch, rootState }, threatModel) => {
@@ -117,7 +140,6 @@ const actions = {
 
 const mutations = {
     [THREATMODEL_CLEAR]: (state) => clearState(state),
-    [THREATMODEL_CREATE]: (state, threatModel) => setThreatModel(state, threatModel),
     [THREATMODEL_CONTRIBUTORS_UPDATED]: (state, contributors) => {
         state.data.detail.contributors.length = 0;
         contributors.forEach((name, idx) => Vue.set(state.data.detail.contributors, idx, { name }));
diff --git a/td.vue/src/views/NewThreatModel.vue b/td.vue/src/views/NewThreatModel.vue
index 1b345fef..de5d3bb3 100644
--- a/td.vue/src/views/NewThreatModel.vue
+++ b/td.vue/src/views/NewThreatModel.vue
@@ -42,7 +42,11 @@ export default {
             // tell the desktop server that the model has changed
             window.electronAPI.modelOpened(newTm.summary.title);
         }
-        this.$router.push({ name: `${this.providerType}ThreatModelEdit`, params });
+        if (this.providerType === 'local') {
+            this.$router.push({ name: `${this.providerType}ThreatModelEdit`, params });
+        } else {
+            this.$router.push({ name: `${this.providerType}ThreatModelCreate`, params });
+        }
     }
 };
 </script>
diff --git a/td.vue/src/views/ThreatModelEdit.vue b/td.vue/src/views/ThreatModelEdit.vue
index 758f3617..0b01f269 100644
--- a/td.vue/src/views/ThreatModelEdit.vue
+++ b/td.vue/src/views/ThreatModelEdit.vue
@@ -173,7 +173,7 @@
     font-size: 12px;
 }
 
-select-diagram-type {
+.select-diagram-type {
     font-size: 12px;
 }
 </style>
@@ -183,7 +183,7 @@ import { mapState } from 'vuex';
 
 import { getProviderType } from '@/service/provider/providers.js';
 import TdFormButton from '@/components/FormButton.vue';
-import { THREATMODEL_CONTRIBUTORS_UPDATED, THREATMODEL_RESTORE, THREATMODEL_SAVE, THREATMODEL_UPDATE } from '@/store/actions/threatmodel.js';
+import { THREATMODEL_CONTRIBUTORS_UPDATED, THREATMODEL_CREATE, THREATMODEL_RESTORE, THREATMODEL_SAVE, THREATMODEL_UPDATE } from '@/store/actions/threatmodel.js';
 
 export default {
     name: 'ThreatModelEdit',
@@ -214,7 +214,11 @@ export default {
         },
         async onSaveClick(evt) {
             evt.preventDefault();
-            await this.$store.dispatch(THREATMODEL_SAVE);
+            if (this.$route.name === 'gitThreatModelCreate') {
+                await this.$store.dispatch(THREATMODEL_CREATE);
+            } else {
+                await this.$store.dispatch(THREATMODEL_SAVE);
+            }
             this.$router.push({ name: `${this.providerType}ThreatModel`, params: this.$route.params });
         },
         async onReloadClick(evt) {


Test patch:
diff --git a/td.server/test/config/routes.config.spec.js b/td.server/test/config/routes.config.spec.js
index e0c9d839..bc214962 100644
--- a/td.server/test/config/routes.config.spec.js
+++ b/td.server/test/config/routes.config.spec.js
@@ -87,35 +87,35 @@ describe('config/routes.config.js routes', () => {
                 threatmodelController.repos
             );
         });
-        
+
         it('routes GET /api/threatmodel/:organisation/:repo/branches', () => {
             expect(mockRouter.get).to.have.been.calledWith(
                 '/api/threatmodel/:organisation/:repo/branches',
                 threatmodelController.branches
             );
         });
-        
+
         it('routes GET /api/threatmodel/organisation', () => {
             expect(mockRouter.get).to.have.been.calledWith(
                 '/api/threatmodel/organisation',
                 threatmodelController.organisation
             );
         });
-        
+
         it('routes GET /api/threatmodel/:organisation/:repo/:branch/models', () => {
             expect(mockRouter.get).to.have.been.calledWith(
                 '/api/threatmodel/:organisation/:repo/:branch/models',
                 threatmodelController.models
             );
         });
-        
+
         it('routes GET /api/threatmodel/:organisation/:repo/:branch/:model/data', () => {
             expect(mockRouter.get).to.have.been.calledWith(
                 '/api/threatmodel/:organisation/:repo/:branch/:model/data',
                 threatmodelController.model
             );
         });
-        
+
         // removed because of security denial of service concerns (denial of models)
         /* it('routes DELETE /api/threatmodel/:organisation/:repo/:branch/:model', () => {
             expect(mockRouter.delete).to.have.been.calledWith(
@@ -123,14 +123,14 @@ describe('config/routes.config.js routes', () => {
                 threatmodelController.deleteModel
             );
         });*/
-        
-        it('routes PUT /api/threatmodel/:organisation/:repo/:branch/:model/create', () => {
-            expect(mockRouter.put).to.have.been.calledWith(
+
+        it('routes POST /api/threatmodel/:organisation/:repo/:branch/:model/create', () => {
+            expect(mockRouter.post).to.have.been.calledWith(
                 '/api/threatmodel/:organisation/:repo/:branch/:model/create',
                 threatmodelController.create
             );
         });
-        
+
         it('routes PUT /api/threatmodel/:organisation/:repo/:branch/:model/update', () => {
             expect(mockRouter.put).to.have.been.calledWith(
                 '/api/threatmodel/:organisation/:repo/:branch/:model/update',
diff --git a/td.vue/tests/unit/store/modules/threatmodel.spec.js b/td.vue/tests/unit/store/modules/threatmodel.spec.js
index ef15ccd4..9e0dbf77 100644
--- a/td.vue/tests/unit/store/modules/threatmodel.spec.js
+++ b/td.vue/tests/unit/store/modules/threatmodel.spec.js
@@ -15,7 +15,7 @@ import {
 } from '@/store/actions/threatmodel.js';
 import threatmodelModule, { clearState } from '@/store/modules/threatmodel.js';
 import threatmodelApi from '@/service/api/threatmodelApi.js';
-import { THREATMODEL_CONTRIBUTORS_UPDATED, THREATMODEL_RESTORE } from '../../../../src/store/actions/threatmodel';
+import { THREATMODEL_CONTRIBUTORS_UPDATED, THREATMODEL_RESTORE } from '@/store/actions/threatmodel';
 
 describe('store/modules/threatmodel.js', () => {
     const getRootState = () => ({
@@ -79,10 +79,57 @@ describe('store/modules/threatmodel.js', () => {
             expect(mocks.commit).toHaveBeenCalledWith(THREATMODEL_DIAGRAM_SELECTED, diagram);
         });
 
-        it('commits the create action with the data', () => {
-            const diagram = { foo: 'bar' };
-            threatmodelModule.actions[THREATMODEL_CREATE](mocks, diagram);
-            expect(mocks.commit).toHaveBeenCalledWith(THREATMODEL_CREATE, diagram);
+        describe('create action with the data', () => {
+            const data = 'foobar';
+
+            beforeEach(() => {
+                Vue.$toast = {
+                    success: jest.fn(),
+                    error: jest.fn()
+                };
+                mocks.state.data = {
+                    summary: {
+                        title: 'New Threat Model'
+                    }
+                };
+            });
+
+            describe('git provider', () => {
+                describe('without error', () => {
+                    beforeEach(async () => {
+                        jest.spyOn(threatmodelApi, 'createAsync').mockResolvedValue({ data });
+                        await threatmodelModule.actions[THREATMODEL_CREATE](mocks, 'tm');
+                    });
+
+                    it('dispatches the set immutable copy event', () => {
+                        expect(mocks.dispatch).toHaveBeenCalledWith(THREATMODEL_SET_IMMUTABLE_COPY);
+                    });
+
+                    it('calls the createAsync api', () => {
+                        expect(threatmodelApi.createAsync).toHaveBeenCalledTimes(1);
+                    });
+
+                    it('shows a toast success message', () => {
+                        expect(Vue.$toast.success).toHaveBeenCalledTimes(1);
+                    });
+                });
+
+                describe('with API error', () => {
+                    beforeEach(async () => {
+                        jest.spyOn(threatmodelApi, 'createAsync').mockRejectedValue({ data });
+                        console.error = jest.fn();
+                        await threatmodelModule.actions[THREATMODEL_CREATE](mocks, 'tm');
+                    });
+
+                    it('logs the error', () => {
+                        expect(console.error).toHaveBeenCalledTimes(2);
+                    });
+
+                    it('shows a toast error message', () => {
+                        expect(Vue.$toast.error).toHaveBeenCalledTimes(1);
+                    });
+                });
+            });
         });
 
         it('commits the diagram updated action', () => {
@@ -102,7 +149,7 @@ describe('store/modules/threatmodel.js', () => {
             it('dispatches the clear event', () => {
                 expect(mocks.dispatch).toHaveBeenCalledWith(THREATMODEL_CLEAR);
             });
-        
+
             it('commits the fetch action', () => {
                 expect(mocks.commit).toHaveBeenCalledWith(
                     THREATMODEL_FETCH,
@@ -110,7 +157,7 @@ describe('store/modules/threatmodel.js', () => {
                 );
             });
         });
-        
+
         it('commits the fetch all action', async () => {
             const data = 'foobar';
             jest.spyOn(threatmodelApi, 'modelsAsync').mockResolvedValue({ data });
@@ -225,31 +272,31 @@ describe('store/modules/threatmodel.js', () => {
                         jest.spyOn(threatmodelApi, 'updateAsync').mockResolvedValue({ data });
                         await threatmodelModule.actions[THREATMODEL_SAVE](mocks, 'tm');
                     });
-    
+
                     it('dispatches the set immutable copy event', () => {
                         expect(mocks.dispatch).toHaveBeenCalledWith(THREATMODEL_SET_IMMUTABLE_COPY);
                     });
-    
+
                     it('calls the updateAsync api', () => {
                         expect(threatmodelApi.updateAsync).toHaveBeenCalledTimes(1);
                     });
-    
+
                     it('shows a toast success message', () => {
                         expect(Vue.$toast.success).toHaveBeenCalledTimes(1);
                     });
                 });
-    
+
                 describe('with API error', () => {
                     beforeEach(async () => {
                         jest.spyOn(threatmodelApi, 'updateAsync').mockRejectedValue({ data });
                         console.error = jest.fn();
                         await threatmodelModule.actions[THREATMODEL_SAVE](mocks, 'tm');
                     });
-    
+
                     it('logs the error', () => {
                         expect(console.error).toHaveBeenCalledTimes(2);
                     });
-    
+
                     it('shows a toast error message', () => {
                         expect(Vue.$toast.error).toHaveBeenCalledTimes(1);
                     });
@@ -287,14 +334,6 @@ describe('store/modules/threatmodel.js', () => {
             });
         });
 
-        describe('create', () => {
-            it('sets the threat model', () => {
-                const tm = { foo: 'bar' };
-                threatmodelModule.mutations[THREATMODEL_CREATE](threatmodelModule.state, tm);
-                expect(threatmodelModule.state.data).toEqual(tm);
-            });
-        });
-
         describe.skip('diagramUpdated', () => {
             let diagram;
             beforeEach(() => {
diff --git a/td.vue/tests/unit/views/newThreatModel.spec.js b/td.vue/tests/unit/views/newThreatModel.spec.js
index d3f5b8ec..2d701d00 100644
--- a/td.vue/tests/unit/views/newThreatModel.spec.js
+++ b/td.vue/tests/unit/views/newThreatModel.spec.js
@@ -16,7 +16,6 @@ describe('NewThreatModel.vue', () => {
                 },
                 actions: {
                     'THREATMODEL_CLEAR': () => {},
-                    'THREATMODEL_CREATE': () => {},
                     'THREATMODEL_SELECTED': () => {}
                 }
             });
@@ -52,4 +51,50 @@ describe('NewThreatModel.vue', () => {
             });
         });
     });
+
+    describe('git provider', () => {
+        beforeEach(() => {
+            localVue = createLocalVue();
+            localVue.use(Vuex);
+            mockStore = new Vuex.Store({
+                state: {
+                    provider: { selected: 'github' }
+                },
+                actions: {
+                    'THREATMODEL_CLEAR': () => {},
+                    'THREATMODEL_SELECTED': () => {}
+                }
+            });
+            jest.spyOn(mockStore, 'dispatch');
+            router = { push: jest.fn() };
+            shallowMount(NewThreatModel, {
+                localVue,
+                store: mockStore,
+                mocks: {
+                    $router: router,
+                    $route: {
+                        params: { foo: 'bar' }
+                    }
+                }
+            });
+        });
+
+        it('clears the current threat model', () => {
+            expect(mockStore.dispatch).toHaveBeenCalledWith('THREATMODEL_CLEAR');
+        });
+
+        it('selects the new threatModel', () => {
+            expect(mockStore.dispatch).toHaveBeenCalledWith('THREATMODEL_SELECTED', expect.anything());
+        });
+
+        it('navigates to the edit page for creation', () => {
+            expect(router.push).toHaveBeenCalledWith({
+                name: 'gitThreatModelCreate',
+                params: {
+                    foo: 'bar',
+                    threatmodel: 'New Threat Model'
+                }
+            });
+        });
+    });
 });