Basic auth middleware

[awdem]

See Asana Card

This PR:

• adds a BasicAuthMiddleware class adapted from DemocracyClub/dc_django_utils@a9b4037 and the starlette authentication middleware
• adds unit and integration tests for said class
• renames SAM_LAMBDA_CONFIG_ENV to DC_ENVIRONMENT and adds DC_ENVIRONMENT to the Lambda function environment in the SAM config
• updates the post-deploy CI tests to pass basic auth on dev and stage

This project is a pure starlette app rather than django, so it was necessary to create a new middleware rather than use the one from dc_django_utils.

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1208397645646736

[symroe]

This seems reasonable so far, but I think testing is needed, and that might change the way you e.g detect the environment. See https://docs.pytest.org/en/7.1.x/how-to/monkeypatch.html#monkeypatching-environment-variables for more on this, but it might be that setting the env var in the tests is a bit of pain.

Either way, the more idea seems sound, let's just try to test it :)

[chris48s]

Any particular reason for passing enable_auth() in as a function here?

Seems like we could either

Make enable_auth a boolean param and call it when we add the middleware i.e: Middleware(BasicAuthMiddleware, enable_auth=enable_auth())

or

Remove the enable_auth param and just inspect the env vars in BasicAuthMiddleware.__init__ i.e:

import os

class BasicAuthMiddleware:
    def __init__(self, app: ASGIApp) -> None:
        self.enable_auth = os.environ.get("DC_ENVIRONMENT") in ["development", "staging"]
        self.app = app

which is basically what we're doing in the equivalent middleware in dc_django_utils
https://github.com/DemocracyClub/dc_django_utils/blob/78b20cf5e955c2994f49fd3d5db3fdad8ee5beba/dc_utils/middleware.py#L8-L15

Personally I'd find one or other of those two options clearer. The callable seems like a layer of indirection we don't really need.

The first one (boolean param) is probably easier as it would require fewer changes to the tests in tests/test_middleware.py as you can just replace mock_auth_enabled() and mock_auth_disabled() with True and False.

[awdem]

I had originally written in the first way you've proposed as a boolean param, but after discussion with Sym I changed it to callable. I'm not sure if I'll be able to articulate the reasoning clearly, but I'll try.

It had to do with when enable_auth() is resolved. When it's passed as a boolean, the function is resolving when settings.py is imported and passing its return value then. I think the issue with that is, when that import happens, the DC_ENVIRONMENT variable is not set yet so the function doesn't detect the environment properly.

Passing it as a callable means that it will be called each time the middleware is checking for auth which means it should have access to the env variable. I think I'm making a mistake, maybe @symroe can clarify?

I do think that 2nd proposal addresses the same issue and is maybe a better approach, though.