Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit discovery session authorization using Request context variable #621

Merged
merged 6 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions asab/api/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
jwcrypto = None

from .. import Service
from ..contextvars import Tenant
from ..contextvars import Tenant, Request


L = logging.getLogger(__name__)
Expand Down Expand Up @@ -401,19 +401,25 @@ def session(
...
"""
_headers = {}
if isinstance(auth, aiohttp.web.Request):
# TODO: This should be the default option. Use contextvar to access the request.

if auth is None:
# By default, use the authorization from the incoming request
request = Request.get(None)
if request is not None:
_headers["Authorization"] = request.headers.get("Authorization")

elif isinstance(auth, aiohttp.web.Request):
assert "Authorization" in auth.headers
_headers["Authorization"] = auth.headers.get("Authorization")

elif auth == "internal":
if jwcrypto is None:
raise ModuleNotFoundError(
"You are trying to use internal auth without 'jwcrypto' installed. "
"Please run 'pip install jwcrypto' or install asab with 'authz' optional dependency."
)
_headers["Authorization"] = "Bearer {}".format(self.InternalAuthToken.serialize())
elif auth is None:
pass

else:
raise ValueError(
"Invalid 'auth' value. "
Expand Down
3 changes: 3 additions & 0 deletions asab/contextvars.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import contextvars

Tenant = contextvars.ContextVar("Tenant")

# Contains aiohttp.web.Request
Request = contextvars.ContextVar("Request")
14 changes: 14 additions & 0 deletions asab/web/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from ..tls import SSLContextBuilder
from .service import WebService
from ..application import Application
from ..contextvars import Request

#

Expand Down Expand Up @@ -136,6 +137,19 @@ def __init__(self, websvc: WebService, config_section_name: str, config: typing.
preflight_paths = re.split(r"[,\s]+", preflight_str, re.MULTILINE)
self.add_preflight_handlers(preflight_paths)

@aiohttp.web.middleware
async def set_request_context(request: aiohttp.web.Request, handler):
"""
Make sure that the incoming aiohttp.web.Request is available via Request context variable
"""
request_ctx = Request.set(request)
try:
return await handler(request)
finally:
Request.reset(request_ctx)

self.WebApp.middlewares.append(set_request_context)
Comment on lines +140 to +151
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure if asab.web.container is the right place for this piece of code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that needed?

Copy link
Collaborator Author

@byewokko byewokko Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at the moment mostly because of DiscoveryService.session(). most discovery session calls are (or soon will need to be) authorized which means you have to pass the authorization from the incoming request to the outgoing request.

instead of requiring devs to always pass on the request object (like DiscoveryService.session(auth=request)), it can be taken automatically from the context variable and used as the default auth value.

also, HTTP request objects should not be passed from the handler layer to the service layer. it makes sense to treat the request like a context variable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am already using this :)



async def _start(self, app: Application):
await self.WebAppRunner.setup()
Expand Down
Loading