Check and hit if Flask endpoints are reachable by WAF

[jimleroyer]

Summary | Résumé

Added Python script that checks if all Flask endpoints are accessible through AWS WAF. If it is not, it should get a 204 response. The script is meant to be executed on any Flask project, given that proper command line parameters are provided and the Flask app can list its endpoints with the default configuration.

Examples

> python .\scripts\waffles.py iron --app-loc /Projects/cds/notification-document-download-api --app-lib doc-api-env/Lib/site-packages --flask-mod application --flask-prop application --base-url=https://api.document.staging.notification.cdssandbox.xyz
Hitting endpoint 'https://api.document.staging.notification.cdssandbox.xyz//_status'... OK.
Hitting endpoint 'https://api.document.staging.notification.cdssandbox.xyz//services/<uuid:service_id>/documents'... WAF failure!
Hitting endpoint 'https://api.document.staging.notification.cdssandbox.xyz//services/<uuid:service_id>/documents/<uuid:document_id>'... WAF failure!
Hitting endpoint 'https://api.document.staging.notification.cdssandbox.xyz//static/<path:filename>'... WAF failure!

A few endpoints could not be hit!

Test instructions | Instructions pour tester la modification

Via the CLI

python .\waffles.py --help
Script to run reachability tests on AWS WAF using the project's endpoints.

The endpoints are determined with Flask's app configuration.

Usage:
    scripts/waffles.py list [options]
    scripts/waffles.py iron [options] [iron-option]

    Options:
    --app-libs=<libs_location>:  Project's libs directory location.
    --app-loc=<location>:        Project's directory location.
    --flask-mod=<mod>:           Flask app module to execute.
    --flask-prop=<prop>:         Flask app property in the module.

    iron-option:
    --base-url=<url>:       Hits the Flask endpoints and verify reachability.

Example:
        waffles.py list --app-loc /Projects/cds/notification-document-download-api --app-lib doc-api-env/Lib/site-packages --flask-mod application --flask-prop application
        waffles.py iron --base-url=https://api.document.notification.canada.ca --app-loc /Projects/cds/notification-document-download-api --app-lib doc-api-env/Lib/site-packages --flask-mod application --flask-prop application

Via VSCode

Example of VS code launcher configuration:

        {
            "name": "Python: Waffles: list",
            "type": "python",
            "request": "launch",
            "program": "scripts/waffles.py",
            "console": "integratedTerminal",
            "args": [
                "list",
                "--app-libs",
                "doc-api-env/Lib/site-packages",
                "--app-loc",
                "/Projects/cds/notification-document-download-api",
                "--flask-mod",
                "application",
                "--flask-prop",
                "application"
            ]
        },
        {
            "name": "Python: Waffles: iron",
            "type": "python",
            "request": "launch",
            "program": "scripts/waffles.py",
            "console": "integratedTerminal",
            "args": [
                "iron",
                "--app-libs",
                "doc-api-env/Lib/site-packages",
                "--app-loc",
                "/Projects/cds/notification-document-download-api",
                "--flask-mod",
                "application",
                "--flask-prop",
                "application",
                "--base-url",
                "https://api.document.staging.notification.cdssandbox.xyz"
            ]
        }

Help wanted | Aide requise

I ran successfully this command for the notification-document-api-download but it offers some challenges for other projects such as notification-api and notification-admin. Namely, the blinker dependency required by both Sentry and flask-signal triggers a load problem when the waffles logic tries to dynamically load the dependencies and evaluate the Flask application object. It seems the blinker dependency can't be found even though it is present in the site-packages folder.

It's easy to fix within the notification-api component as we can disable sentry if not SENTRY_URL environment variable was defined. At the moment, it always loads even though no such required URL exists. Hence we can avoid code paths that goes down the blinker rabbit hole by avoiding an unnecessary Sentry configuration.

The notification-admin component is more of a challenge though. It uses flask-signal in an integral manner to the web application to signal events. These signals are auto-registered on application start up. There are two actions I see to resolve this at the moment of this writing:

1- Introduce a basic configuration mode within notification-admin that will avoid extra initializations such as the signal libraries that depend on blinker.
2- Find the root cause and fix properly: debug the loading mechanism and trace back the origin on why it cannot find a dependency that should be on the Python system path.

Reviewer checklist | Liste de vérification du réviseur

This is a suggested checklist of questions reviewers might ask during their
review | Voici une suggestion de liste de vérification comprenant des questions
que les réviseurs pourraient poser pendant leur examen :

• Does this meet a user need? | Est-ce que ça répond à un besoin utilisateur?
• Is it accessible? | Est-ce que c’est accessible?
• Is it translated between both offical languages? | Est-ce dans les deux
  langues officielles?
• Is the code maintainable? | Est-ce que le code peut être maintenu?
• Have you tested it? | L’avez-vous testé?
• Are there automated tests? | Y a-t-il des tests automatisés?
• Does this cause automated test coverage to drop? | Est-ce que ça entraîne
  une baisse de la quantité de code couvert par les tests automatisés?
• Does this break existing functionality? | Est-ce que ça brise une
  fonctionnalité existante?
• Should this be split into smaller PRs to decrease change risk? | Est-ce
  que ça devrait être divisé en de plus petites demandes de tirage (« pull
  requests ») afin de réduire le risque lié aux modifications?
• Does this change the privacy policy? | Est-ce que ça entraîne une
  modification de la politique de confidentialité?
• Does this introduce any security concerns? | Est-ce que ça introduit des
  préoccupations liées à la sécurité?
• Does this significantly alter performance? | Est-ce que ça modifie de
  façon importante la performance?
• What is the risk level of using added dependencies? | Quel est le degré de
  risque d’utiliser des dépendances ajoutées?
• Should any documentation be updated as a result of this? (i.e. README
  setup, etc.) | Faudra-t-il mettre à jour la documentation à la suite de ce
  changement (fichier README, etc.)?

[lgtm-com]

This pull request introduces 4 alerts when merging 28b6f1f into 1998883 - view on LGTM.com

new alerts:

• 4 for Unused import

[lgtm-com]

This pull request introduces 4 alerts when merging 0e46fa7 into 1998883 - view on LGTM.com

new alerts:

• 4 for Unused import

[jimleroyer]

🖖

[jzbahrai]

from my rudimentary understanding this seems good to me. In regards to your q:

1- Introduce a basic configuration mode within notification-admin that will avoid extra initializations such as the signal libraries that depend on blinker.
2- Find the root cause and fix properly: debug the loading mechanism and trace back the origin on why it cannot find a dependency that should be on the Python system path.

Lets do 2? But I am assuming you suggested 1 as you couldn't debug 2. Have you tested with 1 at all?

[jimleroyer]

@jzbahrai I agree that we'd prefer to do #2 as well by identifying the root cause before going the #1 way.

We don't have to undertake any of these actions for now, until we have to support notification-admin (and probably we'll cover notification-api before undertaking this).

[sastels]

Let’s test this out!