Repository: Neoteroi/BlackSheep
Pull request #379 (2023-06-18 09:23:18)

v1.2.17

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) Report
Patch coverage: **`100.00`**% and project coverage change: **`-0.07`** :warning:
> Comparison is base [(`779fcb5`)](https://app.codecov.io/gh/Neoteroi/BlackSheep/commit/779fcb507c1600179a25043c11658c5f772f5d06?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) 97.68% compared to head [(`d5ba7bc`)](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) 97.62%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please [install the Github App Integration](https://github.com/apps/codecov) for your organization. [Read more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##               v1     #379      +/-   ##
==========================================
- Coverage   97.68%   97.62%   -0.07%     
==========================================
  Files          61       61              
  Lines        5885     5893       +8     
==========================================
+ Hits         5749     5753       +4     
- Misses        136      140       +4     
```


| [Impacted Files](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) | Coverage Δ | |
|---|---|---|
| [blacksheep/server/application.py](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi#diff-YmxhY2tzaGVlcC9zZXJ2ZXIvYXBwbGljYXRpb24ucHk=) | `93.51% <ø> (ø)` | |
| [blacksheep/server/authorization/\_\_init\_\_.py](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi#diff-YmxhY2tzaGVlcC9zZXJ2ZXIvYXV0aG9yaXphdGlvbi9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |

... and [2 files with indirect coverage changes](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Neoteroi/BlackSheep/pull/379?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4e0d522..cd0cde2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,12 +5,23 @@ 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).
 
+## [1.2.17] - 2023-06-18 :ambulance:
+
+- Fixes `TypeError` when writing a request without host header.
+- Fixes `TypeError` when writing a small request with streamed content of known
+  length.
+- Fixes [#372](https://github.com/Neoteroi/BlackSheep/issues/372). Use the ASGI
+  scope `root_path` when possible, as `base_path`.
+- Fixes [#371](https://github.com/Neoteroi/BlackSheep/issues/371). Returns status
+  403 Forbidden when the user is authenticated but not authorized to perform an
+  action.
+
 ## [1.2.16] - 2023-06-14 :cow:
 
-- Add support for `StreamedContent` with specific content length; fixing
+- Adds support for `StreamedContent` with specific content length; fixing
   [#374](https://github.com/Neoteroi/BlackSheep/issues/374) both on the client
   and the server side.
-- Fix [#373](https://github.com/Neoteroi/BlackSheep/issues/373), about missing
+- Fixes [#373](https://github.com/Neoteroi/BlackSheep/issues/373), about missing
   closing ASGI message when an async generator does not yield a closing empty
   bytes sequence (`b""`).
 
diff --git a/blacksheep/messages.pyx b/blacksheep/messages.pyx
index ea3ee07..a9f450f 100644
--- a/blacksheep/messages.pyx
+++ b/blacksheep/messages.pyx
@@ -316,7 +316,16 @@ cdef class Request(Message):
 
     @property
     def base_path(self) -> str:
-        return self.__dict__.get("base_path", "")
+        # 1. if a base path was explicitly set, use it
+        # 2. if a root_path is set in the ASGI scope, use it
+        # 3. default to empty string otherwise
+        try:
+            return self.__dict__["base_path"]
+        except KeyError:
+            try:
+                return self.scope.get("root_path", "")
+            except AttributeError:
+                return ""
 
     @base_path.setter
     def base_path(self, value: str):
diff --git a/blacksheep/scribe.pyx b/blacksheep/scribe.pyx
index b66907e..8906bbe 100644
--- a/blacksheep/scribe.pyx
+++ b/blacksheep/scribe.pyx
@@ -67,7 +67,8 @@ cdef bytes write_request_uri(Request request):
 
 cdef void ensure_host_header(Request request):
     # TODO: if the request port is not default; add b':' + port to the Host value (?)
-    request._add_header_if_missing(b'host', request.url.host)
+    if request.url.host:
+        request._add_header_if_missing(b'host', request.url.host)
 
 
 cdef bint should_use_chunked_encoding(Content content):
@@ -122,7 +123,11 @@ cdef bint is_small_response(Response response):
     cdef Content content = response.content
     if not content:
         return True
-    if content.length > 0 and content.length < MAX_RESPONSE_CHUNK_SIZE:
+    if (
+        content.length > 0
+        and content.length < MAX_RESPONSE_CHUNK_SIZE
+        and content.body is not None
+    ):
         return True
     return False
 
@@ -131,7 +136,11 @@ cpdef bint is_small_request(Request request):
     cdef Content content = request.content
     if not content:
         return True
-    if content.length > 0 and content.length < MAX_RESPONSE_CHUNK_SIZE:
+    if (
+        content.length > 0
+        and content.length < MAX_RESPONSE_CHUNK_SIZE
+        and content.body is not None
+    ):
         return True
     return False
 
diff --git a/blacksheep/server/application.py b/blacksheep/server/application.py
index 2e12c16..2d897b0 100644
--- a/blacksheep/server/application.py
+++ b/blacksheep/server/application.py
@@ -36,7 +36,9 @@ from blacksheep.server.authentication import (
 )
 from blacksheep.server.authorization import (
     AuthorizationWithoutAuthenticationError,
+    ForbiddenError,
     get_authorization_middleware,
+    handle_forbidden,
     handle_unauthorized,
 )
 from blacksheep.server.bindings import ControllerParameter
@@ -425,6 +427,7 @@ class Application(BaseApplication):
             {  # type: ignore
                 AuthenticateChallenge: handle_authentication_challenge,
                 UnauthorizedError: handle_unauthorized,
+                ForbiddenError: handle_forbidden,
             }
         )
         return strategy
diff --git a/blacksheep/server/authorization/__init__.py b/blacksheep/server/authorization/__init__.py
index 63c5322..2e77fde 100644
--- a/blacksheep/server/authorization/__init__.py
+++ b/blacksheep/server/authorization/__init__.py
@@ -59,6 +59,14 @@ def allow_anonymous(value: bool = True) -> Callable[..., Any]:
     return decorator
 
 
+class ForbiddenError(UnauthorizedError):
+    """
+    Specific kind of authorization error, used to indicate that the application
+    understands a request but refuses to authorize it. In other words, the user context
+    is valid but the user is not authorized to perform a certain operation.
+    """
+
+
 def get_authorization_middleware(
     strategy: AuthorizationStrategy,
 ) -> Callable[[Request, Callable[..., Any]], Awaitable[Response]]:
@@ -68,14 +76,25 @@ def get_authorization_middleware(
         if getattr(handler, "allow_anonymous", False) is True:
             return await handler(request)
 
-        if hasattr(handler, "auth"):
-            # function decorated by auth;
-            await strategy.authorize(handler.auth_policy, identity)
-        else:
-            # function not decorated by auth; use the default policy.
-            # this is necessary to support configuring authorization rules globally
-            # without configuring every single request handler
-            await strategy.authorize(None, identity)
+        try:
+            if hasattr(handler, "auth"):
+                # function decorated by auth;
+                await strategy.authorize(handler.auth_policy, identity)
+            else:
+                # function not decorated by auth; use the default policy.
+                # this is necessary to support configuring authorization rules globally
+                # without configuring every single request handler
+                await strategy.authorize(None, identity)
+        except UnauthorizedError as unauthorized_error:
+            if identity and identity.is_authenticated():
+                raise ForbiddenError(
+                    unauthorized_error.failed,
+                    unauthorized_error.failed_requirements,
+                    unauthorized_error.scheme,
+                    unauthorized_error.error,
+                    unauthorized_error.error_description,
+                )
+            raise
 
         return await handler(request)
 
@@ -110,3 +129,13 @@ async def handle_unauthorized(
         [www_authenticate] if www_authenticate else None,
         content=TextContent("Unauthorized"),
     )
+
+
+async def handle_forbidden(
+    app: Any, request: Request, http_exception: UnauthorizedError
+):
+    return Response(
+        403,
+        None,
+        content=TextContent("Forbidden"),
+    )
diff --git a/setup.py b/setup.py
index 80a1332..0518808 100644
--- a/setup.py
+++ b/setup.py
@@ -11,7 +11,7 @@ COMPILE_ARGS = ["-O2"]
 
 setup(
     name="blacksheep",
-    version="1.2.16",
+    version="1.2.17",
     description="Fast web framework for Python asyncio",
     long_description=readme(),
     long_description_content_type="text/markdown",


Test patch:
diff --git a/itests/test_server.py b/itests/test_server.py
index 6faeb10..905d94a 100644
--- a/itests/test_server.py
+++ b/itests/test_server.py
@@ -328,7 +328,7 @@ def test_requires_authenticated_user(session_2, claims, expected_status):
     "claims,expected_status",
     [
         (None, 401),
-        ({"id": "001", "name": "Charlie Brown", "role": "user"}, 401),
+        ({"id": "001", "name": "Charlie Brown", "role": "user"}, 403),
         (
             {
                 "id": "002",
diff --git a/tests/test_auth.py b/tests/test_auth.py
index 7a5db47..dc72c0b 100644
--- a/tests/test_auth.py
+++ b/tests/test_auth.py
@@ -144,7 +144,7 @@ async def test_authentication_sets_identity_in_request(app):
 
 
 @pytest.mark.asyncio
-async def test_authorization_unauthorized_error(app):
+async def test_authorization_forbidden_error_1(app):
     app.use_authentication().add(MockAuthHandler())
 
     app.use_authorization().add(AdminsPolicy())
@@ -157,7 +157,26 @@ async def test_authorization_unauthorized_error(app):
     app.prepare()
     await app(get_example_scope("GET", "/"), MockReceive(), MockSend())
 
-    assert app.response.status == 401
+    assert app.response.status == 403
+
+
+@pytest.mark.asyncio
+async def test_authorization_forbidden_error_2(app):
+    admin = Identity({"id": "001", "name": "Charlie Brown", "role": "user"}, "JWT")
+
+    app.use_authentication().add(MockAuthHandler(admin))
+
+    app.use_authorization().add(AdminsPolicy())
+
+    @auth("admin")
+    @app.router.get("/")
+    async def home():
+        return None
+
+    app.prepare()
+    await app(get_example_scope("GET", "/"), MockReceive(), MockSend())
+
+    assert app.response.status == 403
 
 
 @pytest.mark.asyncio
diff --git a/tests/test_requests.py b/tests/test_requests.py
index 42762ea..0337721 100644
--- a/tests/test_requests.py
+++ b/tests/test_requests.py
@@ -1,7 +1,7 @@
 import pytest
 
 from blacksheep import Content, Request, scribe
-from blacksheep.contents import FormPart, MultiPartFormData
+from blacksheep.contents import FormPart, MultiPartFormData, StreamedContent
 from blacksheep.exceptions import BadRequestFormat
 from blacksheep.messages import get_absolute_url_to_path, get_request_absolute_url
 from blacksheep.scribe import write_request, write_small_request
@@ -548,3 +548,57 @@ content-length: 0
         value.extend(chunk)
 
     assert bytes(value) == expected_bytes
+
+
+@pytest.mark.asyncio
+async def test_write_small_request_streamed():
+    async def content_gen():
+        yield b"Hello"
+        yield b"World"
+
+    request = Request("POST", b"/", []).with_content(
+        StreamedContent(b"text/plain", content_gen)
+    )
+
+    data = bytearray()
+    async for chunk in write_request(request):
+        data.extend(chunk)
+    assert (
+        bytes(data)
+        == b"""POST / HTTP/1.1\r\ncontent-type: text/plain\r\ntransfer-encoding: chunked\r\n\r\n5\r\nHello\r\n5\r\nWorld\r\n0\r\n\r\n"""
+    )
+
+
+@pytest.mark.asyncio
+async def test_write_small_request_streamed_fixed_length():
+    async def content_gen():
+        yield b"Hello"
+        yield b"World"
+
+    request = Request("POST", b"/", []).with_content(
+        StreamedContent(b"text/plain", content_gen, len("HelloWorld"))
+    )
+
+    data = bytearray()
+    async for chunk in write_request(request):
+        data.extend(chunk)
+    assert (
+        bytes(data)
+        == b"POST / HTTP/1.1\r\ncontent-type: text/plain\r\ncontent-length: 10\r\n\r\nHelloWorld"
+    )
+
+
+def test_scope_root_path():
+    request = Request("GET", b"/", [])
+    assert request.base_path == ""
+
+    request.base_path = "/app"
+    assert request.base_path == "/app"
+
+    request = Request("GET", b"/", [])
+    request.scope = {"root_path": "/app"}  # type: ignore
+
+    assert request.base_path == "/app"
+
+    request.base_path = "/app2"
+    assert request.base_path == "/app2"
diff --git a/tests/test_responses.py b/tests/test_responses.py
index 3a128b8..ccdec0c 100644
--- a/tests/test_responses.py
+++ b/tests/test_responses.py
@@ -8,6 +8,7 @@ from uuid import UUID, uuid4
 import pytest
 
 from blacksheep import Content, Cookie, Response, scribe
+from blacksheep.contents import StreamedContent
 from blacksheep.server.controllers import (
     CannotDetermineDefaultViewNameError,
     Controller,
@@ -1129,3 +1130,51 @@ async def test_pretty_json_response_in_controller(
             assert f'    "{name}": "{value}"' in raw
         else:
             assert f'    "{name}": ' in raw
+
+
+@pytest.mark.asyncio
+async def test_response_streaming(app):
+    @app.router.get("/")
+    async def home():
+        return Response(200, [], StreamedContent(b"text/css", get_example_css))
+
+    app.normalize_handlers()
+
+    await app(
+        get_example_scope("GET", "/", []),
+        MockReceive(),
+        MockSend(),
+    )
+
+    response = app.response
+    assert response.status == 200
+    assert response.headers.get_single(b"content-type") == b"text/css"
+
+    text = await response.text()
+    assert text == await read_from_asynciterable(get_example_css)
+
+
+@pytest.mark.asyncio
+async def test_response_streaming_known_length(app):
+    expected_result = await read_from_asynciterable(get_example_css)
+
+    @app.router.get("/")
+    async def home():
+        return Response(
+            200, [], StreamedContent(b"text/css", get_example_css, len(expected_result))
+        )
+
+    app.normalize_handlers()
+
+    await app(
+        get_example_scope("GET", "/", []),
+        MockReceive(),
+        MockSend(),
+    )
+
+    response = app.response
+    assert response.status == 200
+    assert response.headers.get_single(b"content-type") == b"text/css"
+
+    text = await response.text()
+    assert text == await read_from_asynciterable(get_example_css)