diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a3c7f7a2..61e40300 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -15,6 +15,7 @@ on: - "*" paths-ignore: - README.md + - CHANGELOG.md env: PROJECT_NAME: blacksheep diff --git a/CHANGELOG.md b/CHANGELOG.md index d1f44498..2715e487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/LICENSE b/LICENSE index 050e1073..17fee425 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2021 Roberto Prevato +Copyright (c) 2021-present Roberto Prevato roberto.prevato@gmail.com Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/blacksheep/__init__.py b/blacksheep/__init__.py index 2ee59794..11d29bcd 100644 --- a/blacksheep/__init__.py +++ b/blacksheep/__init__.py @@ -3,7 +3,7 @@ used types to reduce the verbosity of the imports statements. """ __author__ = "Roberto Prevato " -__version__ = "2.0.0" +__version__ = "2.0.1" from .contents import Content as Content from .contents import FormContent as FormContent diff --git a/blacksheep/server/application.py b/blacksheep/server/application.py index 96966845..de2e945a 100644 --- a/blacksheep/server/application.py +++ b/blacksheep/server/application.py @@ -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 @@ -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 @@ -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 @@ -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: """ @@ -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): diff --git a/blacksheep/server/authentication/oidc.py b/blacksheep/server/authentication/oidc.py index 80b19bf2..805084d1 100644 --- a/blacksheep/server/authentication/oidc.py +++ b/blacksheep/server/authentication/oidc.py @@ -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( @@ -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): diff --git a/blacksheep/server/controllers.py b/blacksheep/server/controllers.py index 635444e8..2e3e5cca 100644 --- a/blacksheep/server/controllers.py +++ b/blacksheep/server/controllers.py @@ -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 diff --git a/blacksheep/server/routing.py b/blacksheep/server/routing.py index b4e22625..009511fc 100644 --- a/blacksheep/server/routing.py +++ b/blacksheep/server/routing.py @@ -1,3 +1,4 @@ +import inspect import logging import re from abc import ABC, abstractmethod @@ -43,7 +44,7 @@ class RouteMethod: class RouteException(Exception): - ... + """Base class for routing exceptions.""" class RouteDuplicate(RouteException): @@ -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__( @@ -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, @@ -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: @@ -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 @@ -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 @@ -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; @@ -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 diff --git a/blacksheep/server/websocket.py b/blacksheep/server/websocket.py index 50590076..7fa12a0d 100644 --- a/blacksheep/server/websocket.py +++ b/blacksheep/server/websocket.py @@ -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"" @@ -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 = { diff --git a/tests/test_application.py b/tests/test_application.py index 638abe6d..733c87fe 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -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 @@ -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)