tornadoweb / tornado

Tornado is a Python web framework and asynchronous networking library, originally developed at FriendFeed.
http://www.tornadoweb.org/
Apache License 2.0
21.74k stars 5.5k forks source link

async get_current_user #2434

Open minrk opened 6 years ago

minrk commented 6 years ago

Is it in-scope for @web.authenticated to support an async .get_current_user()? Or does this belong more appropriately in custom auth decorators in applications? It seems generic enough to me to warrant a default implementation, but making sure handler.current_user access works in a backward-compatible way seems tricky.

garenchan commented 6 years ago

@minrk , maybe you can set current_user asynchronously in prepare,prepare is called just before the real request handler and it's can be asynchronous!

bdarnell commented 6 years ago

Yeah, a standardized async implementation of authenticated/get_current_user is a good idea (although a prepare()-based solution is a decent workaround for now). The biggest stumbling block is the old current_user property - we can't make get_current_user lazy any more (that laziness is also imperfect and broken by the use of tornado.template: #820).

I think it would be possible to extend the current implementation in a mostly backwards-compatible way (e.g. "if get_current_user_async is defined, @authenticated will call it and store its result in handler.current_user. current_user may only be used from handlers with @authenticated (and maybe a new optional-authenticated decorator), instead of being usable from any handler as it is now). However, it might be cleaner to redesign the whole setup.

Other issues with the current system:

So as a strawman, what about something like this?

class RequestHandler:
    ...

    async def get_current_user_async(self):
        """Override instead of get_current_user to use a coroutine."""
        raise NotImplementedError()

    async def check_authentication(self, optional=False):
        if not hasattr(self, '_current_user'):
            try:
                self._current_user = await self.get_current_user_async()
            except NotImplementedError:
                self._current_user = self.get_current_user()
        if self._current_user is None and not optional:
            if login_url_is_configured():
                redirect_to_login()
           else:
               raise HTTPError(403)