owaspsamm / sammwise

NextJS-based single-page application for completing and reviewing SAMM assessments
Apache License 2.0
68 stars 51 forks source link

Secure Build Matury By Practice wrong values #2

Closed claudioandreantonio closed 2 years ago

claudioandreantonio commented 2 years ago

Hi there.

It seems that the calculation for the Secure Build answers are not being correctly calculated.

Even If I respond all questions to "Yes, for most or all of the applications", in the results I keep seeing 1.5 of total value. I can see that the overall result is affect (probably in a correct way)

claudioandreantonio commented 2 years ago

image

claudioandreantonio commented 2 years ago

Ok, I've discovered an additional thing. If I affect the question"44. Are deployment processes automated and employing security checks?" it actually influences the value of Secure Build and not Secure Deployment. Seems that there are questions associated with the incorrect category.

amithmurthy commented 2 years ago

Hi Claudio, thanks for raising the issue. Will check it out and get back to you.

amithmurthy commented 2 years ago

Hi Claudio, please pull the latest changes this bug has been addressed. Let us know if you still face the same issue however.

claudioandreantonio commented 2 years ago

Hi @amithmurthy . Thank so much for the rapid response. I confirm it is solved. Thank you.

claudioandreantonio commented 2 years ago

Hi @amithmurthy . It seems that now there are issues between Incident Management and Environment Management.

image

Environment has plenty of things incomplete but indicate a value of 3. Based on the answers it should be Incident that should have 3.

claudioandreantonio commented 2 years ago

https://github.com/owaspsamm/sammwise/blob/main/comps/surveyDisplay/graphs/testCalculator.js

 "Incident Management":{
                    "answers": this.getAnswerMap(73),
                    "score":0
                },
                "Environment Management":{
                        "answers":this.getAnswerMap(79),
                        "score":0
                    },
                "Operations Management":{
                    "answers":this.getAnswerMap(85),
                    "score":0

This seems to be the origin.

The answers are switched. The Environment Management should start at 73 and Incident Management start at 79 correct?

amithmurthy commented 2 years ago

Hi Claudio, so the practices in operations didn't reflect the structure from the SAMM Model. The latest updates have addressed it :) ( no need to alter testCalculator.js)

claudioandreantonio commented 2 years ago

Hi @amithmurthy . The pull request for testCalculator was to guarantee that the correct questions are taken into account. The getAnswerMap() are starting in the incorrect numbers for Environment Management and Incident Management. 73 vs 79. If you have everything with value 1 on the Incident Management answers, this is what happens:

image

amithmurthy commented 2 years ago

Hi @claudioandreantonio , is the image from the latest commit? Because I'm getting this: image

amithmurthy commented 2 years ago

The reason not to update the getAnswerMap() starting values is that in that version 'Incident Management' and 'Environment Management' were not in the correct order in the DOM according to the SAMM model. Our latest commit addressed this issue. So 'Incident Management' now begins at question 73 - as it's meant to. Thanks for all the feedback so far, appreciate it. image

claudioandreantonio commented 2 years ago

My bad @amithmurthy . I discovered what happened. I had the previously saved answers with the old order. When I loaded them obviously associated the answers of Incident Management to the Environment Management. Sorry for the confusion.