encode / starlette

The little ASGI framework that shines. 🌟
https://www.starlette.io/
BSD 3-Clause "New" or "Revised" License
9.82k stars 877 forks source link

Cannot access POST data from AuthenticationBackend #771

Closed gnat closed 6 months ago

gnat commented 4 years ago

Common use case: Retrieve POST data sent by an HTML login <form> to validate within AuthenticationBackend middleware.

Any common techniques or help appreciated.

[!IMPORTANT]

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.

Fund with Polar

gnat commented 4 years ago

One way I've worked around this temporarily is retrieving the form data upfront via middleware.

I feel this is not well documented (or intuitive?) for such a common use case and AuthenticationBackend should get the ability to access the form() method on its own. Or some other solution perhaps, since multiple calls to form() will hang. See https://github.com/encode/starlette/issues/495

from starlette.middleware.base import BaseHTTPMiddleware

class PostToState(BaseHTTPMiddleware):
    """
    Pulls post / form data, loads into current request state.
    """
    async def dispatch(self, request, call_next):
        post = await request.form()
        request.state.post = post
        response = await call_next(request)
        return response
gnat commented 4 years ago

Note to anyone using the middleware workaround: There's a sizeable performance hit for using await request.form() on pages that do not need it. You can restrict this call to only pages which need to check POST data within the AuthenticationBackend by including a check like this in your dispatch() method of your PostToState middleware:

if request['path'] in ('/login', '/signup', '/password_reset'):

Again, would love a more permanent, performant solution from a core dev, but I'm not sure what the best approach would be.

Would love to hear of anyone else tackling the issue of retrieving POST data in your AuthenticationBackend.

Kludex commented 1 year ago

Is this still an issue?

gnat commented 1 year ago

Yes, it's entirely related to #944 #1519

An Authentication check requires retrieving POST data.

Retrieving POST data requires consuming the stream.

adriangb commented 1 year ago

You should be able to achieve the same thing with a pure ASGI middleware: https://github.com/encode/starlette/blob/master/docs/middleware.md#pure-asgi-middleware (these docs are brand new). Let me know if you need any further guidance.

gnat commented 1 year ago

Converted it over to pure ASGI awhile ago. The core issue is unfortunately unrelated.

I am beginning to feel like everyone uses Starlette for API's and not websites. :laughing: Being able to accept POST data from an HTML form for logins should be very typical.

gnat commented 1 year ago

To be fair, this is not a showstopping bug, just a poor developer experience.

Caching the stream using middleware works, its just not obvious for newcomers to do that. Other ASGI frameworks cache the stream and users will never encounter this.

adriangb commented 1 year ago

You caught me red handed: I'm a backend/data engineer, I don't do much frontend. Apologies for misunderstanding your issue if I did.

The core issue is unfortunately unrelated.

Would you mind helping me understand what the core issue is? Does using the ASGI middleware not work, or is the issue now that this functionality (receiving auth info via a form) should be built in?

erewok commented 1 year ago

Being able to accept POST data from an HTML form for logins should be very typical.

Can you help me understand this a bit more? Typically, I have done this in a single handler (not middleware) which works like this:

The middleware then checks session information but doesn't need to validate the form on every request? This is similar to how Django works by default, but it sounds like you want to validate this form data on every request? I'm not questioning what you're doing, just trying to understand it.

gnat commented 1 year ago

The desired UX here is:

:arrow_right: User visits page, clicks "login" modal in the page header. :arrow_right: User POSTS using that modal <form> page refresh (or AJAX).
:arrow_right: User is now logged in on that page.

The middleware then checks session information but doesn't need to validate the form on every request? This is similar to how Django works by default, but it sounds like you want to validate this form data on every request? I'm not questioning what you're doing, just trying to understand it

I mean yes, there could be a separate authentication page, but in an ideal world, we avoid the extra steps/redirects, and enable any page to perform Auth on POST.

Maybe this shouldn't be built in, but many other ASGI frameworks handle #944 and #1519 automatically - FastAPI and Sanic to name a few.

It's unfortunate that it's related to a longstanding issue that's been discussed to death without resolution. This probably will never be resolved until the related issues are.

gnat commented 1 year ago

Gonna go ahead and close this, but to people searching Github Issues for this DX problem, feel free to leave comments, ideas, discussion, code.

Kludex commented 1 year ago

Do you mind if I keep this open?

gnat commented 1 year ago

No problem, I was just under the impression that Starlette wanted to get to 0 issues before 1.0, and this is only a DX issue that may never be resolved.

Kludex commented 1 year ago

I don't like closing issues without resolution. Thanks for reopening it. 🙏

Kludex commented 1 year ago

@gnat is this still an issue?

Kludex commented 6 months ago

What's the proposal here? For AuthenticationMiddleware to create a Request|WebSocket object instead of conn?

Kludex commented 6 months ago

After some thought, I understand the issue here, and I think it makes sense to change the AuthenticationMiddleware in a backward compatible way to allow reading the body.

That said, it doesn't look like people are interested in this. So I'll be closing this, but if people get interested in this in the future, we can reopen it.

I'd also be happy to merge a PR to the documentation that shows how to do what is proposed here using a pure ASGI middleware.