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

Docs: Update usage/security/guards chapter #3356

Open
aranvir opened this issue Apr 9, 2024 · 4 comments · May be fixed by #3385
Open

Docs: Update usage/security/guards chapter #3356

aranvir opened this issue Apr 9, 2024 · 4 comments · May be fixed by #3385
Labels
Documentation 📚 This is related to documentation

Comments

@aranvir
Copy link
Contributor

aranvir commented Apr 9, 2024

Summary

Follow-up from discussion: https://github.com/orgs/litestar-org/discussions/3355

Suggesting changes to chapter: https://docs.litestar.dev/latest/usage/security/guards.html

  • Guards take a Request object as first argument, not an ASGIConnection. Needs correction throughout the chapter.
  • Add examples for how to access path parameters, query parameters, and query body from within a guard.

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@aranvir aranvir added the Documentation 📚 This is related to documentation label Apr 9, 2024
@peterschutt
Copy link
Contributor

Guards take a Request object as first argument, not an ASGIConnection. Needs correction throughout the chapter.

Just to clarify this point, guards of HTTP routes receive a Request object. However, guards are also called on ASGI and websocket routes, and receive the relevant connection type in those cases.

@aranvir
Copy link
Contributor Author

aranvir commented Apr 13, 2024

@peterschutt prepared a draft PR to update typing and docs.

While working on this I looke a bit into the Request methods, e.g., how json() works. You mentioned in the discussion on this topic that the asgi messages can only be consumed once. So, I'm wondering now how helpful this type hin adjustment actually is - or if "hiding" the type from the user is beneficial for preventing wrong usage.

Alternatively, I could extend the documentation to elaborate that it is fine to access query and path parameters but that the request body may not be accessed by the guard. I'm learning about all of this on the fly, so how I handled it now for my project is, that I put "body guards" (pun intended) at the top of route handler function instead of in the route handler guard list. Maybe it's worth mentioning that in the docs just so newbies don't have to go through the same trial&error process as me :D what do you think?

@Alc-Alc
Copy link
Contributor

Alc-Alc commented Apr 13, 2024

asgi messages can only be consumed once.

When you do .json or .body on the request instance, it does consume the message body, but it is then cached, further calls to these methods use the cached value. I do not think Peter meant this to be a blanket statement saying "read it only once". It usually is not required to read body in guards, but there is nothing "wrong" with doing this, IMO ofc.

@aranvir
Copy link
Contributor Author

aranvir commented Apr 13, 2024

Thansk @Alc-Alc that's already good to hear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation 📚 This is related to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants