search

sparckles / Robyn

Robyn is a Super Fast Async Python Web Framework with a Rust runtime.
https://robyn.tech/
BSD 2-Clause "Simplified" License
4.44k stars 229 forks source link

Authentication in subroutes #1026

Open dave42w opened 1 week ago

dave42w commented 1 week ago

Bug Description

In this PR https://github.com/sparckles/Robyn/pull/1023 I first corrected the arguments for the SubRouter HTTP methods (Get, Post etc) as they were out of sync with the Robyn class - they did not have the auth_required argument.

I'm now pretty sure that there is a problem in Robyn.configure_authentication When a SubRouter is created the authentication_handler is none. However, Robyn.configure_authentication does not seem to set the authentication_handler for the SubRoutes, I'm also not sure if it is doing so for websockets or openapi endpoints (or even if it should)

That highlighted multiple testing issues

  1. There are no authentication tests for the subrouter (obviously because they should have been failing). I have committed some tests but they are failing with an AuthenticationNotConfiguredError()
  2. The only HTTP method with authentication is Get (Post, put etc are not tested in test_authentication.py) we should add tests for these as that is potentially a huge security hole.
  3. While looking into this I also noticed that we do have some unit tests (that should be obvious to me). However, our documentation never asks us to run these. Can we change the instructions from pytest integration_tests to pytest it still runs all the integration tests but also the unit tests.
  4. There don't seem to be Authentication tests for either Websockets or OpenApi. Should there be?
sansyrox commented 1 week ago

Hey @dave42w 👋

I agree with all of them!

Yes

While looking into this I also noticed that we do have some unit tests (that should be obvious to me). However, our documentation never asks us to run these. Can we change the instructions from pytest integration_tests to pytest it still runs all the integration tests but also the unit tests.

Yes!

dave42w commented 5 days ago

See this https://github.com/sparckles/Robyn/pull/889 and the links it contains.