Skip to content

Add Introduction to Security Testing#170

Merged
damithc merged 16 commits into
se-edu:masterfrom
madanalogy:sec-test
Apr 7, 2020
Merged

Add Introduction to Security Testing#170
damithc merged 16 commits into
se-edu:masterfrom
madanalogy:sec-test

Conversation

@madanalogy

Copy link
Copy Markdown
Contributor

Resolves #131

@jamessspanggg

Copy link
Copy Markdown
Contributor

It seems like the netlify preview doesn't show the intro to security chapter in the table of contents. Can you add it in so that we can view the chapter on netlify?

@jamessspanggg jamessspanggg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some general comments:

  1. Add the chapter to the initial table of contents.
  2. Perhaps the diagrams can be slightly smaller? Currently takes a lot of space.
  3. Label the diagrams.
  4. Perhaps can give examples for the types of security testing in the What is Security Testing section?

Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
@madanalogy

This comment has been minimized.

@jamessspanggg jamessspanggg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some more comments:

Comment thread contents/testing/security-testing.md Outdated
Comment thread _markbind/navigation/mainNav.md
Comment thread contents/testing/security-testing.md
Comment thread contents/testing/security-testing.md Outdated

@jamessspanggg jamessspanggg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM now :) do request a review from me again if there's any major changes

@madanalogy

Copy link
Copy Markdown
Contributor Author

LGTM now :) do request a review from me again if there's any major changes

Gotcha, thanks! :)

@damithc damithc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good start. Added some comments. Need to make the contents stronger. Try to base your content on a authoritative source (e.g., a popular textbook on the topic) so that the reader can be more confident about what is being read. In other words, don't let the reader worry if you might be inventing your own theory. Give a topic X, things such as 'types of X' 'objectives of X' have well-established answers. Just need to find those from a good source, adapt it to your context, and cite the source.

Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
@madanalogy

Copy link
Copy Markdown
Contributor Author

Good start. Added some comments. Need to make the contents stronger. Try to base your content on a authoritative source (e.g., a popular textbook on the topic) so that the reader can be more confident about what is being read. In other words, don't let the reader worry if you might be inventing your own theory. Give a topic X, things such as 'types of X' 'objectives of X' have well-established answers. Just need to find those from a good source, adapt it to your context, and cite the source.

Most of the content was lifted from an authoritative source. I should've included proper citations and references apologies for that oversight. I will go through your comments and make the necessary changes, thank you!

@madanalogy

madanalogy commented Mar 16, 2020

Copy link
Copy Markdown
Contributor Author

Hi @jamessspanggg will need your review again as there's been an overhaul of the chapter following comments from @damithc. Main changes:

  • Rephrased most content to be more accurate to the source material
  • Included proper citations and references to source material
  • Changed the images to be more meaningful
  • Reworded headings to be more consistent as previously suggested

The chapter structure however is largely the same. Additionally, would appreciate if you have any comments on the appropriateness of the At A Glace sub-section under the Security Testing in Action section. Thank you! :)

@jamessspanggg jamessspanggg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good improvements from the previous revision. Some comments;

Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md
Comment thread contents/testing/security-testing.md Outdated

@j-lum j-lum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good. The 'At a glance' section can benefit from having a bit more references.

Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
@madanalogy

Copy link
Copy Markdown
Contributor Author

Overall looks good. The 'At a glance' section can benefit from having a bit more references

I adapted them from the OWASP Testing Guide so it's really mostly referenced from there haha

@madanalogy madanalogy requested a review from j-lum March 24, 2020 10:53
@madanalogy madanalogy requested review from j-lum and removed request for j-lum March 27, 2020 09:49
@madanalogy

Copy link
Copy Markdown
Contributor Author

@tiuweehan hope you have time to provide the final review for this book chapter, thanks!

@madanalogy madanalogy requested a review from tiuweehan April 5, 2020 04:57
@madanalogy madanalogy removed their assignment Apr 5, 2020

@tiuweehan tiuweehan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me! The article is well-structured and easy to read. Not much to add since most of the issues have been pointed out by other reviewers, just a few suggestions.

Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
Comment thread contents/testing/security-testing.md Outdated
@madanalogy

Copy link
Copy Markdown
Contributor Author

@damithc chapter ready for merging! :)

@damithc

damithc commented Apr 7, 2020

Copy link
Copy Markdown
Contributor

@openorclose any idea what's going on in the footnotes here? known bug to be fixed in future?

image

@madanalogy

Copy link
Copy Markdown
Contributor Author

@openorclose any idea what's going on in the footnotes here? known bug to be fixed in future?

That was not there when I reviewed it locally with markbind serve. I am currently on markbind version 2.12.0 if that helps.

@openorclose

Copy link
Copy Markdown
Contributor

@openorclose any idea what's going on in the footnotes here? known bug to be fixed in future?

image

It's a bug, fixed in MarkBind/markbind#1155

@damithc

damithc commented Apr 7, 2020

Copy link
Copy Markdown
Contributor

It's a bug, fixed in MarkBind/markbind#1155

Thanks for checking @openorclose
I'll merge this then.

@damithc damithc merged commit cdc2083 into se-edu:master Apr 7, 2020
@madanalogy madanalogy deleted the sec-test branch April 7, 2020 04:51
@madanalogy madanalogy removed the request for review from j-lum June 10, 2020 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduction to Security Testing

7 participants