Skip to content

Commit

Permalink
Prepare for v2.0.1
Browse files Browse the repository at this point in the history
- Fixes #441 causing the `refresh_token` endpoint for OpenID Connect
  integrations to not work when authentication is required by default.
- Fixes #427, handling WebSocket errors according to ASGI specification.
- Fixes #443, raising a detailed exception when more than one application is
  sharing the same instance of `Router`
- Fixes #438 and #436, restoring support for `uvicorn` used programmatically
  and reloading the application object more than once in the same process.
  • Loading branch information
RobertoPrevato authored Dec 9, 2023
1 parent 73b279c commit 5f9bc45
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 13 deletions.
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ on:
- "*"
paths-ignore:
- README.md
- CHANGELOG.md

env:
PROJECT_NAME: blacksheep
Expand Down
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [2.0.1] - 2023-12-09 :mount_fuji:

- Fixes #441 causing the `refresh_token` endpoint for OpenID Connect
integrations to not work when authentication is required by default.
- Fixes #427, handling WebSocket errors according to ASGI specification.
- Fixes #443, raising a detailed exception when more than one application is
sharing the same instance of `Router`
- Fixes #438 and #436, restoring support for `uvicorn` used programmatically
and reloading the application object more than once in the same process.

## [2.0.0] - 2023-11-18 :mage_man:

- Releases v2 as stable.
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
MIT License

Copyright (c) 2021 Roberto Prevato
Copyright (c) 2021-present Roberto Prevato [email protected]

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down
2 changes: 1 addition & 1 deletion blacksheep/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
used types to reduce the verbosity of the imports statements.
"""
__author__ = "Roberto Prevato <[email protected]>"
__version__ = "2.0.0"
__version__ = "2.0.1"

from .contents import Content as Content
from .contents import FormContent as FormContent
Expand Down
36 changes: 31 additions & 5 deletions blacksheep/server/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
handle_unauthorized,
)
from blacksheep.server.bindings import ControllerParameter
from blacksheep.server.controllers import router as controllers_router
from blacksheep.server.cors import CORSPolicy, CORSStrategy, get_cors_middleware
from blacksheep.server.env import EnvironmentSettings
from blacksheep.server.errors import ServerErrorDetailsHandler
Expand All @@ -65,6 +64,7 @@
RoutesRegistry,
)
from blacksheep.server.routing import router as default_router
from blacksheep.server.routing import validate_router
from blacksheep.server.websocket import WebSocket
from blacksheep.sessions import SessionMiddleware, SessionSerializer
from blacksheep.settings.di import di_settings
Expand Down Expand Up @@ -188,6 +188,7 @@ def __init__(
services = di_settings.get_default_container()
if mount is None:
mount = MountRegistry(env_settings.mount_auto_events)

super().__init__(show_error_details or env_settings.show_error_details, router)

assert services is not None
Expand All @@ -203,18 +204,27 @@ def __init__(
self.on_stop = ApplicationEvent(self)
self.on_middlewares_configuration = ApplicationSyncEvent(self)
self.started = False
self.controllers_router: RoutesRegistry = controllers_router
self.files_handler = FilesHandler()
self.server_error_details_handler = ServerErrorDetailsHandler()
self._session_middleware: Optional[SessionMiddleware] = None
self.base_path: str = ""
self._mount_registry = mount

validate_router(self)
parent_file = get_parent_file()

if parent_file:
_auto_import_controllers(parent_file)
_auto_import_routes(parent_file)

@property
def controllers_router(self) -> RoutesRegistry:
return self.router.controllers_routes

@controllers_router.setter
def controllers_router(self, value) -> None:
self.router.controllers_routes = value

@property
def services(self) -> ContainerProtocol:
"""
Expand Down Expand Up @@ -733,10 +743,26 @@ async def _handle_websocket(self, scope, receive, send):
ws.route_values = route.values
try:
return await route.handler(ws)
except UnauthorizedError:
await ws.close(401, "Unauthorized")
except UnauthorizedError as unauthorized_error:
# If the WebSocket connection was not accepted yet, we close the
# connection with an HTTP Status Code, otherwise we close the connection
# with a WebSocket status code
if ws.accepted:
# Use a WebSocket error code, not an HTTP error code
await ws.close(1005, "Unauthorized")
else:
# Still in handshake phase, we close with an HTTP Status Code
# https://asgi.readthedocs.io/en/latest/specs/www.html#close-send-event
await ws.close(403, str(unauthorized_error))
except HTTPException as http_exception:
await ws.close(http_exception.status, str(http_exception))
# Same like above
if ws.accepted:
# Use a WebSocket error code, not an HTTP error code
await ws.close(1005, str(http_exception))
else:
# Still in handshake phase, we close with an HTTP Status Code
# https://asgi.readthedocs.io/en/latest/specs/www.html#close-send-event
await ws.close(http_exception.status, str(http_exception))
await ws.close()

async def _handle_http(self, scope, receive, send):
Expand Down
2 changes: 2 additions & 0 deletions blacksheep/server/authentication/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,7 @@ def use_openid_connect(
cookie_name=scheme_name.lower(), auth_scheme=scheme_name
),
)

app.use_authentication().add(auth_handler)

handler = OpenIDConnectHandler(
Expand All @@ -1087,6 +1088,7 @@ async def handle_auth_redirect(request: Request):
async def redirect_to_logout(request: Request):
return await handler.handle_logout_redirect(request)

@allow_anonymous()
@app.router.post(settings.refresh_token_path)
@cache_control(no_cache=True, no_store=True)
async def refresh_token(request: Request):
Expand Down
6 changes: 4 additions & 2 deletions blacksheep/server/controllers.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@
view,
view_async,
)
from blacksheep.server.routing import RouteFilter, RoutesRegistry, normalize_filters
from blacksheep.server.routing import RouteFilter
from blacksheep.server.routing import RoutesRegistry as RoutesRegistry # noqa
from blacksheep.server.routing import controllers_routes, normalize_filters
from blacksheep.utils import AnyStr, join_fragments

# singleton router used to store initial configuration,
# before the application starts
# this is used as *default* router for controllers, but it can be overridden
# - see for example tests in test_controllers
router = RoutesRegistry()
router = controllers_routes


head = router.head
Expand Down
90 changes: 87 additions & 3 deletions blacksheep/server/routing.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import inspect
import logging
import re
from abc import ABC, abstractmethod
Expand Down Expand Up @@ -43,7 +44,7 @@ class RouteMethod:


class RouteException(Exception):
...
"""Base class for routing exceptions."""


class RouteDuplicate(RouteException):
Expand All @@ -61,6 +62,26 @@ def __init__(self, method, pattern, current_handler):
self.current_handler = current_handler


class InvalidRouterConfigurationError(RouteException):
"""Base class for router configuration errors"""


class SharedRouterError(InvalidRouterConfigurationError):
"""
Error raised when the more than one application is using the same router.
Each application object should use a different router.
"""

def __init__(self) -> None:
super().__init__(
"Invalid routers configuration: the same router is used in more "
"than one Application object. When working with multiple applications, "
"ensure that each application is configured to use different routers. "
"For more information, refer to: "
"https://www.neoteroi.dev/blacksheep/routing/"
)


class InvalidValuePatternName(RouteException):
def __init__(self, parameter_pattern_name: str, matched_parameter: str) -> None:
super().__init__(
Expand Down Expand Up @@ -593,7 +614,14 @@ def get_match(self, request: Request) -> Optional[RouteMatch]:


class Router(RouterBase):
__slots__ = ("routes", "_map", "_fallback", "_sub_routers", "_filters")
__slots__ = (
"routes",
"controllers_routes",
"_map",
"_fallback",
"_sub_routers",
"_filters",
)

def __init__(
self,
Expand All @@ -614,6 +642,7 @@ def __init__(
self._map = {}
self._fallback = None
self.routes: Dict[bytes, List[Route]] = defaultdict(list)
self.controllers_routes = RoutesRegistry()
self._sub_routers = sub_routers

if self._filters:
Expand All @@ -622,6 +651,16 @@ def __init__(
if self._sub_routers:
extend(self, MultiRouterMixin)

def reset(self):
"""Resets this router to its initial state."""
self._map = {}
self._fallback = None
self.routes = defaultdict(list)
self.controllers_routes.reset()
if self._sub_routers:
for sub_router in self._sub_routers:
sub_router.reset()

@property
def fallback(self):
return self._fallback
Expand Down Expand Up @@ -790,6 +829,10 @@ def __init__(
super().__init__(host=host, headers=headers, params=params, filters=filters)
self.routes: List[RegisteredRoute] = []

def reset(self):
"""Resets this routes registry to its initial state."""
self.routes = []

def __iter__(self):
yield from self.routes

Expand Down Expand Up @@ -843,6 +886,47 @@ def mount(self, path: str, app: Callable) -> None:
Mount = MountRegistry


_apps_by_router_id = {}


def validate_router(app):
"""
Ensures that the same router is not bound to more than one application object.
If the same application is being reloaded dynamically, like when using uvicorn
programmatically, the router is reset.
"""
app_router: Router = app.router
router_id = id(app_router)

# Get information about where the application was instantiated (which filename,
# which line_number)
_, filename, line_number, *_ = inspect.stack()[2]

try:
existing_app = _apps_by_router_id[router_id]
except KeyError:
# Good
_apps_by_router_id[router_id] = {
"app": app,
"filename": filename,
"line_number": line_number,
}
else:
if (
existing_app["filename"] == filename
and existing_app["line_number"] == line_number
):
# This is the same application! This can happen when imported dynamically
# by uvicorn reload feature, when uvicorn is started programmatically.
# See https://github.com/Neoteroi/BlackSheep/issues/438
logging.getLogger("blacksheep.server").warning(
"[BlackSheep] The application was reloaded, resetting its router."
)
app_router.reset()
return
raise SharedRouterError()


# Singleton router used to store initial configuration,
# before the application starts
# this is used as *default* router, but it can be overridden;
Expand All @@ -851,7 +935,7 @@ def mount(self, path: str, app: Callable) -> None:
# Application and Router (the same approach can be easily used for more complex use
# cases where more than one router is used).
router = Router()

controllers_routes = router.controllers_routes

head = router.head
get = router.get
Expand Down
12 changes: 11 additions & 1 deletion blacksheep/server/websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,20 @@ def __init__(
self.scope = scope # type: ignore
self._receive = self._wrap_receive(receive)
self._send = send
self._accepted = False
self.route_values = {}

self.client_state = WebSocketState.CONNECTING
self.application_state = WebSocketState.CONNECTING

@property
def accepted(self) -> bool:
"""
Returns a value indicating whether this WebSocket request was already accepted,
or if it is still in the HTTP handshake phase.
"""
return self._accepted

def __repr__(self):
return f"<WebSocket {self.url.value.decode()}>"

Expand All @@ -83,11 +92,12 @@ async def _connect(self) -> None:
self.client_state = WebSocketState.CONNECTED

async def accept(
self, headers: Optional[List] = None, subprotocol: str = None
self, headers: Optional[List] = None, subprotocol: Optional[str] = None
) -> None:
headers = headers or []

await self._connect()
self._accepted = True
self.application_state = WebSocketState.CONNECTED

message = {
Expand Down
12 changes: 12 additions & 0 deletions tests/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from blacksheep.server.openapi.v3 import OpenAPIHandler
from blacksheep.server.resources import get_resource_file_path
from blacksheep.server.responses import status_code, text
from blacksheep.server.routing import Router, SharedRouterError
from blacksheep.server.security.hsts import HSTSMiddleware
from blacksheep.testing.helpers import get_example_scope
from blacksheep.testing.messages import MockReceive, MockSend
Expand Down Expand Up @@ -4131,3 +4132,14 @@ async def some_async_gen():

assert initialized is True
assert disposed is True


def test_mounting_apps_using_the_same_router_raises_error():
# Recreates the scenario happening when the default singleton router is used for
# both parent app and child app
# https://github.com/Neoteroi/BlackSheep/issues/443
single_router = Router()
Application(router=single_router)

with pytest.raises(SharedRouterError):
Application(router=single_router)

0 comments on commit 5f9bc45

Please sign in to comment.