Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#183 Map OWASP API Top 10 to NodeGoat functionalities. #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sher04lock
Copy link

@sher04lock sher04lock commented Mar 30, 2020

This document describes how each vulnerability from OWASP API Top 10 (https://owasp.org/www-project-api-security/) can be exploited in NodeGoat application. For each exploit there's also a suggestion how it could be fixed.

Some vulnerabilities (API2, API4) are demonstrated with more than one example to show different exploits that falls into one vulnerability listed in API Top 10 document.

This PR refers to task #183.

This document describes how each vulnerability from OWASP API Top 10 can be exploited in NodeGoat application.

Some vulnerabilities (API2, API4) are demonstrated with more than one example to show different exploits that falls into one vulnerability listed in API Top 10 document.
@sher04lock sher04lock changed the title Map OWASP API Top 10 to NodeGoat functionalities. #183 Map OWASP API Top 10 to NodeGoat functionalities. Mar 30, 2020
@ckarande
Copy link
Member

@sher04lock Great work! Thank you very much for the very thoughtful mapping of API Top 10 vulnerabilities with NodeGoat features to implement those.

As a next step I think we should create stories/ issues for each item and start working on each. @UlisesGascon @KoolTheba Would this be inline with work you have been doing?

@ckarande
Copy link
Member

ckarande commented Apr 3, 2020

@sher04lock In case you have bandwidth, feel free to create a branch for suggested changes.

@sher04lock
Copy link
Author

@ckarande, do you mean branch in OWASP/NodeGoat repository for this file, or new branch in forked repo for implemented examples of API Top 10 vulnerabilties? If you meant the former then I'm not sure if I can create branches in this repository.

@ckarande
Copy link
Member

ckarande commented Apr 5, 2020

@sher04lock, a new branch in forked repo for implemented examples of API Top 10 vulnerabilities. Thanks.

@sher04lock
Copy link
Author

Hi @ckarande, @KoolTheba,
I was implementing these features on the branch forked from NodeGoat/master, but I realised that @KoolTheba is working on separating API and frontend in #192, where he already boostraped new Express appliation. As these examples of API vulnerabilities should ultimately be in the new version, maybe I would first help with finishing setting up React + Express app and then add implementened vulnerabilities to avoid any conflicts and mismatches?

If that's ok, I could contact with @KoolTheba on Gitter/Slack to confirm what's the status and what still needs to be done.

@ckarande
Copy link
Member

Thanks for the suggestion @sher04lock. That makes sense to me. In general, beyond initial setup tasks, having APIs implemented is essential for frontend work. So it is timely to coordinate between you and @KoolTheba.

@KoolTheba @UlisesGascon would that work for you? If so can you share status and tasks that @sher04lock can pick up? Also, do you have any preferred way to collaborate on this (e.g, woking off of a common branch, instead of @sher04lock working off of @KoolTheba's fork, or working off of the monorepo setup done as part of #187, etc )?

@ckarande
Copy link
Member

@sher04lock, Can you start working based on the React + Express app setup done by @KoolTheba? Please let me what you think is required in that setup before you can add vulnerabilities in the APIs.

I have created a branch https://github.com/OWASP/NodeGoat/tree/feature/192. You can make PR to this branch with your updates going forward.

@sher04lock
Copy link
Author

@ckarande I think there's nothing that could block implementing new features for the time being. Based on the checklist in @KoolTheba PRs (#193) unfinished tasks are:

  • Connect React to Express - This is not really needed for API development. If you'd like to have it done, I can try to add some very basic way of calling API from React app, although I'm not very familiar with React yet.
  • Add basic tests - Some example tests are already in place and can be be executed, so this is done.
  • Add Docker/Docker-componse - No files for React-Express app, so I'll add these.
  • Add CI support - Not sure about that, it was said in Feature/192 #193 that Lerna Implementation #187 needs to be finished first before CI starts working again. I think we can move on with implementing endpoints and vulnerabilities without it as long as we keep tests and docker builds passing.

I haven't seen any information about switching from MongoDB to some other database, so I assume we are sticking with Mongo?

@ckarande
Copy link
Member

Great! Thanks for the analysis. It is good that you don't have any major dependencies on Theba.
We will let her to continue on React frontend part.

Let's go ahead implementing APIs and test it using a REST client such as postman.
We will continue with MingoDB.

@UlisesGascon
Copy link
Collaborator

Hi @sher04lock!!

Sorry for the delay in my response, I was quite disconnected the last few weeks.
Thanks a lot for bring this idea. I love it! ❤️

Right now @KoolTheba and myself we are working on branch KoolTheba:feature/192. In that fork we have the most updated version for React + Express API. The Express server was setup already in folder apps/react-apirest/api/*. Also some basic tests were added already. Please feel free add routes, logic.. on that folder. As soon as @KoolTheba will finished the first component in the react part apps/react-apirest/client/* we will review and merge the PR #193.

Idea: Maybe we can merge the current PR #193 into a branch like react-api-version in order to let you fork it and work independently? What do you think @ckarande @KoolTheba ? So @sher04lock can work in the express part in the branch react-api-version and @KoolTheba too for the React part. We shouldn't have conflicts or weird merges. When the react app and express is ready we can easily merge them into master.

BTW: I will keep working on #189 in order to fix the CI issue.

@ckarande
Copy link
Member

Great! Thanks for sharing your thoughts @UlisesGascon! I like the idea to merge the current PR#192 into a branch like react-api-version and both @KoolTheba and @sher04lock to fork it and work independently. I had created feature/192 branch for it. Feel free to rename it as appropriate. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants