From 6d2310aca8eefe11bd4730ee3277dfcf71a31ca8 Mon Sep 17 00:00:00 2001
From: Mohamed Nur <85885638+mohamed-cds@users.noreply.github.com>
Date: Thu, 2 Sep 2021 15:50:49 -0400
Subject: [PATCH] chore: refactored to organize routes and client (#106)

* chore: refactored to organize routes and client

* chore: bandit scan and linting fixes

* fix: script permissions
---
 .devcontainer/Dockerfile                     |  1 +
 .github/workflows/bandith_security_scan.yml  | 17 +++++
 .github/workflows/scripts/run_bandit_scan.sh |  9 +++
 api/api_gateway/api.py                       | 20 ++++++
 api/api_gateway/{v1 => routers}/__init__.py  |  0
 api/api_gateway/routers/ops.py               | 41 +++++++++++
 api/api_gateway/routers/organisations.py     | 40 +++++++++++
 api/api_gateway/routers/scans.py             | 17 +++++
 api/api_gateway/v1/api.py                    | 76 --------------------
 api/front_end/view.py                        | 10 +--
 api/main.py                                  | 13 +---
 api/tests/api_gateway/test_api.py            | 16 ++---
 api/tests/front_end/test_view.py             |  2 +-
 bin/api_create_organisation.sh               |  2 +-
 bin/api_healthcheck.sh                       |  2 +-
 bin/api_version.sh                           |  2 +-
 16 files changed, 164 insertions(+), 104 deletions(-)
 create mode 100644 .github/workflows/bandith_security_scan.yml
 create mode 100755 .github/workflows/scripts/run_bandit_scan.sh
 create mode 100644 api/api_gateway/api.py
 rename api/api_gateway/{v1 => routers}/__init__.py (100%)
 create mode 100644 api/api_gateway/routers/ops.py
 create mode 100644 api/api_gateway/routers/organisations.py
 create mode 100644 api/api_gateway/routers/scans.py
 delete mode 100644 api/api_gateway/v1/api.py

diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile
index 101613be..78bd28c7 100644
--- a/.devcontainer/Dockerfile
+++ b/.devcontainer/Dockerfile
@@ -92,3 +92,4 @@ output=json\n\
 
 
 RUN su vscode -c "source /usr/local/share/nvm/nvm.sh && npm install -g typescript" 2>&1
+ENV DEV_ENVIRONMENT=true
\ No newline at end of file
diff --git a/.github/workflows/bandith_security_scan.yml b/.github/workflows/bandith_security_scan.yml
new file mode 100644
index 00000000..46f1710d
--- /dev/null
+++ b/.github/workflows/bandith_security_scan.yml
@@ -0,0 +1,17 @@
+name: Source code security scan using Bandit
+on:
+  pull_request:
+    paths:
+      - "**/*.py"
+
+jobs:
+  bandit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Bandit
+      run: |
+        .github/workflows/scripts/run_bandit_scan.sh
+
+
+
diff --git a/.github/workflows/scripts/run_bandit_scan.sh b/.github/workflows/scripts/run_bandit_scan.sh
new file mode 100755
index 00000000..e51a0d6c
--- /dev/null
+++ b/.github/workflows/scripts/run_bandit_scan.sh
@@ -0,0 +1,9 @@
+#!/bin/bash
+set -euo pipefail
+IFS=$'\n\t'
+
+# Make sure we are using the latest version
+docker pull cytopia/bandit:latest
+
+# Scan source code and only report on high severity issues
+docker run --rm -v "$(pwd)":/data cytopia/bandit -r /data -ll
\ No newline at end of file
diff --git a/api/api_gateway/api.py b/api/api_gateway/api.py
new file mode 100644
index 00000000..3011edc7
--- /dev/null
+++ b/api/api_gateway/api.py
@@ -0,0 +1,20 @@
+from os import environ
+from fastapi import FastAPI
+
+from front_end import view
+from .routers import ops, organisations, scans
+
+if "DEV_ENVIRONMENT" in environ:
+    app = FastAPI()
+else:
+    # This is only needed until a dedicated domain name is live
+    app = FastAPI(root_path="/v1")
+
+app.include_router(ops.router, prefix="/ops", tags=["ops"])
+app.include_router(
+    organisations.router,
+    prefix="/organisations",
+    tags=["organisations"],
+)
+app.include_router(scans.router, prefix="/scans", tags=["scans"])
+app.include_router(view.router)
diff --git a/api/api_gateway/v1/__init__.py b/api/api_gateway/routers/__init__.py
similarity index 100%
rename from api/api_gateway/v1/__init__.py
rename to api/api_gateway/routers/__init__.py
diff --git a/api/api_gateway/routers/ops.py b/api/api_gateway/routers/ops.py
new file mode 100644
index 00000000..b0d5e8ba
--- /dev/null
+++ b/api/api_gateway/routers/ops.py
@@ -0,0 +1,41 @@
+from os import environ
+from fastapi import APIRouter, Depends
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm import Session
+from database.db import db_session
+from logger import log
+
+router = APIRouter()
+
+
+# Dependency
+def get_db():
+    db = db_session()
+    try:
+        yield db
+    finally:
+        db.close()
+
+
+@router.get("/version")
+def version():
+    return {"version": environ.get("GIT_SHA", "unknown")}
+
+
+def get_db_version(session):
+
+    query = "SELECT version_num FROM alembic_version"
+    full_name = session.execute(query).fetchone()[0]
+    return full_name
+
+
+@router.get("/healthcheck")
+def healthcheck(session: Session = Depends(get_db)):
+    try:
+        full_name = get_db_version(session)
+        db_status = {"able_to_connect": True, "db_version": full_name}
+    except SQLAlchemyError as err:
+        log.error(err)
+        db_status = {"able_to_connect": False}
+
+    return {"database": db_status}
diff --git a/api/api_gateway/routers/organisations.py b/api/api_gateway/routers/organisations.py
new file mode 100644
index 00000000..2aa5d8e1
--- /dev/null
+++ b/api/api_gateway/routers/organisations.py
@@ -0,0 +1,40 @@
+from fastapi import APIRouter, Depends, Response, status
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm import Session
+from fastapi.responses import RedirectResponse
+from database.db import db_session
+from logger import log
+
+from models.Organisation import Organisation
+from schemas.Organization import OrganizationCreate
+
+router = APIRouter()
+
+
+# Dependency
+def get_db():
+    db = db_session()
+    try:
+        yield db
+    finally:
+        db.close()
+
+
+# TODO Require auth and redirect to home
+# TODO Push errors to cloudwatch metric and response when debug enabled
+@router.post("/create", response_class=RedirectResponse)
+def create_organisation(
+    organisation: OrganizationCreate,
+    response: Response,
+    session: Session = Depends(get_db),
+):
+
+    try:
+        new_organisation = Organisation(name=organisation.name)
+        session.add(new_organisation)
+        session.commit()
+        return RedirectResponse("/dashboard")
+    except SQLAlchemyError as err:
+        log.error(err)
+        response.status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
+        return {"error": f"error creating organisation: {err}"}
diff --git a/api/api_gateway/routers/scans.py b/api/api_gateway/routers/scans.py
new file mode 100644
index 00000000..9f8181d5
--- /dev/null
+++ b/api/api_gateway/routers/scans.py
@@ -0,0 +1,17 @@
+from fastapi import APIRouter
+from logger import log
+from pydantic import BaseModel
+from crawler.crawler import crawl
+import uuid
+
+router = APIRouter()
+
+
+class CrawlUrl(BaseModel):
+    url: str
+
+
+@router.post("/crawl")
+def crawl_endpoint(crawl_url: CrawlUrl):
+    log.info(f"Crawling {crawl_url}")
+    crawl(uuid.uuid4(), crawl_url.url)
diff --git a/api/api_gateway/v1/api.py b/api/api_gateway/v1/api.py
deleted file mode 100644
index 0b490306..00000000
--- a/api/api_gateway/v1/api.py
+++ /dev/null
@@ -1,76 +0,0 @@
-from os import environ
-from fastapi import Depends, FastAPI, HTTPException
-from sqlalchemy.exc import SQLAlchemyError
-from sqlalchemy.orm import Session
-from fastapi.responses import RedirectResponse
-from database.db import db_session
-from logger import log
-
-from models.Organisation import Organisation
-from schemas.Organization import OrganizationCreate
-
-# from crawler.crawler import crawl
-# import uuid
-from pydantic import BaseModel
-
-app = FastAPI()
-
-
-# Dependency
-def get_db():
-    db = db_session()
-    try:
-        yield db
-    finally:
-        db.close()
-
-
-@app.get("/api/v1/version")
-def version():
-    return {"version": environ.get("GIT_SHA", "unknown")}
-
-
-def get_db_version(session):
-
-    query = "SELECT version_num FROM alembic_version"
-    full_name = session.execute(query).fetchone()[0]
-    return full_name
-
-
-@app.get("/api/v1/healthcheck")
-def healthcheck(session: Session = Depends(get_db)):
-    try:
-        full_name = get_db_version(session)
-        db_status = {"able_to_connect": True, "db_version": full_name}
-    except SQLAlchemyError as err:
-        log.error(err)
-        db_status = {"able_to_connect": False}
-
-    return {"database": db_status}
-
-
-# TODO Require auth and redirect to home
-# TODO Push errors to cloudwatch metric and response when debug enabled
-@app.post("/api/v1/organisation", response_class=RedirectResponse)
-def create_organisation(
-    organisation: OrganizationCreate, session: Session = Depends(get_db)
-):
-
-    try:
-        new_organisation = Organisation(name=organisation.name)
-        session.add(new_organisation)
-        session.commit()
-        return RedirectResponse("/dashboard")
-    except Exception as e:
-        log.error(e)
-        raise HTTPException(status_code=500, detail=str(e))
-
-
-class CrawlUrl(BaseModel):
-    url: str
-
-
-# @app.post("/crawl")
-# def crawl_endpoint(crawl_url: CrawlUrl):
-#   log.info(f"Crawling {crawl_url}")
-#    crawl(uuid.uuid4(), crawl_url.url)
diff --git a/api/front_end/view.py b/api/front_end/view.py
index ce306df1..cb5f9081 100644
--- a/api/front_end/view.py
+++ b/api/front_end/view.py
@@ -1,4 +1,4 @@
-from fastapi import Depends, FastAPI, Request, HTTPException
+from fastapi import APIRouter, Depends, Request, HTTPException
 from fastapi.responses import HTMLResponse, RedirectResponse
 from fastapi.templating import Jinja2Templates
 from babel.plural import PluralRule
@@ -12,7 +12,7 @@
 import json
 import os
 
-app = FastAPI()
+router = APIRouter()
 
 
 # Dependency
@@ -66,12 +66,12 @@ def plural_formatting(key_value, input, locale):
 templates.env.filters["plural_formatting"] = plural_formatting
 
 
-@app.get("/", response_class=HTMLResponse)
+@router.get("/", response_class=HTMLResponse)
 async def force_lang():
     return RedirectResponse("/en")
 
 
-@app.get("/{locale}", response_class=HTMLResponse)
+@router.get("/{locale}", response_class=HTMLResponse)
 async def home(request: Request, locale: str):
     try:
         if locale not in languages:
@@ -88,7 +88,7 @@ async def home(request: Request, locale: str):
 # TODO Require auth & limit to users current organisation
 # TODO Push errors to cloudwatch metric and response when debug enabled
 # TODO Enable detailed error messages via debug flag
-@app.get("/{locale}/dashboard", response_class=HTMLResponse)
+@router.get("/{locale}/dashboard", response_class=HTMLResponse)
 async def dashboard(request: Request, locale: str, session: Session = Depends(get_db)):
     try:
         if locale not in languages:
diff --git a/api/main.py b/api/main.py
index 2392a3e3..c5bfa8cf 100644
--- a/api/main.py
+++ b/api/main.py
@@ -1,6 +1,5 @@
 from mangum import Mangum
-from api_gateway.v1 import api
-from front_end import view
+from api_gateway import api
 from logger import log
 from database.migrate import migrate_head
 from storage import storage
@@ -18,9 +17,7 @@
 from models.TemplateScanTrigger import TemplateScanTrigger  # noqa: F401
 from models.User import User  # noqa: F401
 
-
-api_v1 = api.app
-app = view.app
+app = api.app
 
 
 def print_env_variables():
@@ -35,13 +32,7 @@ def handler(event, context):
     if "httpMethod" in event:
         # Assume it is an API Gateway event
         asgi_handler = Mangum(app)
-
-        if "path" in event:
-            if event["path"].lower().startswith("/api/v1"):
-                asgi_handler = Mangum(api_v1)
-
         response = asgi_handler(event, context)
-
         return response
 
     elif "Records" in event:
diff --git a/api/tests/api_gateway/test_api.py b/api/tests/api_gateway/test_api.py
index 871d473b..2124d4d5 100644
--- a/api/tests/api_gateway/test_api.py
+++ b/api/tests/api_gateway/test_api.py
@@ -3,39 +3,39 @@
 from fastapi.testclient import TestClient
 from unittest.mock import patch
 
-from api_gateway.v1 import api
+from api_gateway import api
 from sqlalchemy.exc import SQLAlchemyError
 
 client = TestClient(api.app)
 
 
 def test_version_with_no_GIT_SHA():
-    response = client.get("/api/v1/version")
+    response = client.get("/ops/version")
     assert response.status_code == 200
     assert response.json() == {"version": "unknown"}
 
 
 @patch.dict(os.environ, {"GIT_SHA": "foo"}, clear=True)
 def test_version_with_GIT_SHA():
-    response = client.get("/api/v1/version")
+    response = client.get("/ops/version")
     assert response.status_code == 200
     assert response.json() == {"version": "foo"}
 
 
-@patch("api_gateway.v1.api.get_db_version")
+@patch("api_gateway.routers.ops.get_db_version")
 def test_healthcheck_success(mock_get_db_version):
     mock_get_db_version.return_value = "foo"
-    response = client.get("/api/v1/healthcheck")
+    response = client.get("/ops/healthcheck")
     assert response.status_code == 200
     expected_val = {"database": {"able_to_connect": True, "db_version": "foo"}}
     assert response.json() == expected_val
 
 
-@patch("api_gateway.v1.api.get_db_version")
-@patch("api_gateway.v1.api.log")
+@patch("api_gateway.routers.ops.get_db_version")
+@patch("api_gateway.routers.ops.log")
 def test_healthcheck_failure(mock_log, mock_get_db_version):
     mock_get_db_version.side_effect = SQLAlchemyError()
-    response = client.get("/api/v1/healthcheck")
+    response = client.get("/ops/healthcheck")
     assert response.status_code == 200
     expected_val = {"database": {"able_to_connect": False}}
     assert response.json() == expected_val
diff --git a/api/tests/front_end/test_view.py b/api/tests/front_end/test_view.py
index c3b5381c..dd821f98 100644
--- a/api/tests/front_end/test_view.py
+++ b/api/tests/front_end/test_view.py
@@ -2,7 +2,7 @@
 
 from front_end import view
 
-client = TestClient(view.app)
+client = TestClient(view.router)
 
 
 def test_langing_page_redirect_to_en():
diff --git a/bin/api_create_organisation.sh b/bin/api_create_organisation.sh
index 37b8b916..81d1f559 100755
--- a/bin/api_create_organisation.sh
+++ b/bin/api_create_organisation.sh
@@ -3,7 +3,7 @@
 echo hitting create organisation endpoint
 curl "http://api:8080/2015-03-31/functions/function/invocations" -d '{
     "resource": "/",
-    "path": "/api/v1/organisation",
+    "path": "/organisation",
     "requestContext": {},
     "httpMethod": "POST",
     "headers": {},
diff --git a/bin/api_healthcheck.sh b/bin/api_healthcheck.sh
index 59f5e84b..2f359ae2 100755
--- a/bin/api_healthcheck.sh
+++ b/bin/api_healthcheck.sh
@@ -3,7 +3,7 @@
 echo hitting api healthcheck endpoint
 curl "http://api:8080/2015-03-31/functions/function/invocations" -d '{
     "resource": "/",
-    "path": "/api/v1/healthcheck",
+    "path": "/healthcheck",
     "requestContext": {},
     "httpMethod": "GET",
     "headers": {},
diff --git a/bin/api_version.sh b/bin/api_version.sh
index 92b25aa8..3892a648 100755
--- a/bin/api_version.sh
+++ b/bin/api_version.sh
@@ -3,7 +3,7 @@
 echo hitting api version endpoint
 curl "http://api:8080/2015-03-31/functions/function/invocations" -d '{
     "resource": "/",
-    "path": "/api/v1/version",
+    "path": "/version",
     "requestContext": {},
     "httpMethod": "GET",
     "headers": {},