Skip to content

Commit

Permalink
Merge pull request #108 from bento-platform/feat/use-flask-logger-if-…
Browse files Browse the repository at this point in the history
…avail

[WIP] feat: use flask built-in logger if no logger set in flask authz middleware
  • Loading branch information
davidlougheed authored Jul 24, 2023
2 parents 4838a6a + 2cca3d1 commit c0922b8
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 2 deletions.
8 changes: 6 additions & 2 deletions bento_lib/auth/middleware/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def __init__(
self._verify_ssl: bool = not debug_mode

self._enabled: bool = enabled
self._logger = logger or logging.getLogger(__name__)
self._logger: logging.Logger | None = logger

self._drs_compat: bool = drs_compat
self._sr_compat: bool = sr_compat
Expand Down Expand Up @@ -61,8 +61,12 @@ def _extract_token_and_build_headers(
self.check_require_token(require_token, tkn_header)
return {"Authorization": tkn_header} if tkn_header else {}

def _log_error(self, message: str):
if self._logger:
self._logger.error(message)

def _gen_exc_non_200_error_from_authz(self, code: int, content: bytes):
self._logger.error(f"Got non-200 response from authorization service: {code} {content}")
self._log_error(f"Got non-200 response from authorization service: {code} {content}")
# Generic error - don't leak errors from authz service!
raise BentoAuthException("Error from authz service", status_code=500)

Expand Down
9 changes: 9 additions & 0 deletions bento_lib/auth/middleware/django.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging

from asgiref.sync import iscoroutinefunction, markcoroutinefunction
from django.http import JsonResponse, HttpRequest, HttpResponse
from typing import Awaitable, Callable
Expand All @@ -12,6 +14,13 @@


class DjangoAuthMiddleware(BaseAuthMiddleware):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# If no logger was passed, create a new logger
if self._logger is None:
self._logger = logging.getLogger(__name__)

def get_authz_header_value(self, request: HttpRequest) -> str | None:
return request.headers.get("Authorization")

Expand Down
8 changes: 8 additions & 0 deletions bento_lib/auth/middleware/fastapi.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging

from fastapi import Depends, FastAPI, Request, Response, status
from fastapi.responses import JSONResponse
from typing import Awaitable, Callable
Expand All @@ -18,8 +20,14 @@ def attach(self, app: FastAPI):
Attach the middleware to an application. Must be called in order for requests to be checked.
:param app: A FastAPI application.
"""

# Attach our instance's dispatch method to the FastAPI instance as a middleware function
app.middleware("http")(self.dispatch)

# If no logger was passed, create a new logger
if self._logger is None:
self._logger = logging.getLogger(__name__)

async def dispatch(self, request: Request, call_next: Callable[[Request], Awaitable[Response]]) -> Response:
if not self.enabled or request.method == "OPTIONS":
# - Skip checks if the authorization middleware is disabled
Expand Down
4 changes: 4 additions & 0 deletions bento_lib/auth/middleware/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@ def attach(self, app: Flask):
Attach the middleware to an application. Must be called in order for requests to be checked.
:param app: A Flask application.
"""

app.before_request(self.middleware_pre)
app.after_request(self.middleware_post)

if self._logger is None:
self._logger = app.logger

def middleware_pre(self) -> None:
if self.enabled:
g.bento_determined_authz = False
Expand Down
6 changes: 6 additions & 0 deletions tests/test_platform_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
)


def test_django_authz_logger_init():
from bento_lib.auth.middleware.django import DjangoAuthMiddleware
mw = DjangoAuthMiddleware("https://bento-auth.local")
assert mw._logger is not None


@pytest.mark.parametrize(authz_test_case_params, authz_test_cases)
@responses.activate
def test_django_auth(
Expand Down
8 changes: 8 additions & 0 deletions tests/test_platform_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,14 @@ def _expect_error(r: HttpxResponse, code: int, msg: str):
assert data["errors"][0]["message"] == msg


def test_fastapi_middleware_init_logger():
inst = FastApiAuthMiddleware(bento_authz_service_url="https://bento-auth.local")
inst._log_error("doesn't appear")
inst.attach(FastAPI()) # should create a logger if not specified
inst._log_error("does appear")
assert inst._logger is not None


def test_fastapi_http_exception_404(test_client: TestClient):
_expect_error(test_client.get("/get-404"), 404, "Hello")

Expand Down
7 changes: 7 additions & 0 deletions tests/test_platform_flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ def auth_disabled_post_private():
# -----------------------------------------------------------------------------


def test_flask_middleware_init_logger():
inst = FlaskAuthMiddleware(bento_authz_service_url="https://bento-auth.local")
app = Flask(__name__)
inst.attach(app)
assert inst._logger == app.logger


def test_flask_errors(flask_client):
# non-existent endpoint

Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ skip_install = true
commands =
pip install -r requirements.txt
pytest -svv --cov=bento_lib --cov-branch {posargs}
coverage html
flake8 ./bento_lib ./tests

0 comments on commit c0922b8

Please sign in to comment.