alex-oleshkevich / starsessions

Advanced sessions for Starlette and FastAPI frameworks
MIT License
92 stars 10 forks source link

Find automatically correct type of request.session #10

Closed euri10 closed 2 years ago

euri10 commented 2 years ago

Currently any if request.session.is_empty I'm using lead mypy to complain error: "Dict[Any, Any]" has no attribute "is_empty" [attr-defined]

so currently I got around it with

session : Session = request.session
if session.is_empty:

but I wondered if there was a way to make it less verbose, I'm not a mypy pro so cant be sure about it, but isn't adding the py.typed (https://www.python.org/dev/peps/pep-0561/) the way to solve it from here ?

alex-oleshkevich commented 2 years ago

Well, the issue comes not from starsessions but from Starlette's Request.session typing. IIRC, it is declared as dict so it is not fully compatible with this library which has more options (because some backends need to be loaded first before they can be accessed).

You have several workarounds here:

  1. use typing.cast function: if cast(Session, request.session).is_empty: ...
  2. create your own Request class with properly typed attributes and use it in your type hints.

I like the last point more as you may also want to type your request.state variable. If you have an alternate idea I would be glad to hear.

BTW, what is your usecase? You don't need to access "is_empty" usually.

euri10 commented 2 years ago

ok seems number 2 is indeed better !

BTW, what is your usecase? You don't need to access "is_empty" usually.

I'm on a task where I can only do only server-side rendering and I'm using it in a jinja2 template layout this way, not sure this is appropriate though :

            {% if not request.session.is_empty %}
                <li><a href="{{ url_for('get_me') }}">{{ request.session.data.username }}</a></li>
                {% if 'admin' in request.session.data.roles  %}
                <li><a href="{{ url_for('admin')}}">Admin</a> </li>
                {% endif %}
                <li><a href="{{ url_for('logout') }}">Logout</a></li>
            {% else %}
                <li><a href="{{ url_for('register_get') }}">Register</a></li>
                <li><a href="{{ url_for('login_get') }}">Login</a></li>
            {% endif %}
alex-oleshkevich commented 2 years ago

Well, Session.is_empty checks that the session has no data. The SessionMiddleware will remove the session cookie if is_empty returns True.

Your use case is authentication and authorization. So you should use request.auth or request.user for this purpose.

https://www.starlette.io/authentication/ - load user https://www.starlette.io/authentication/#authcredentials - for permissions

Your code can look like this:

{% if request.user %}
  {% if 'admin' in request.user.roles %}... {% endif %}
  OR
  {% if 'admin' in request.auth.scopes %}... {% endif %}
{% else %}
  <!-- render login links -->
{% endif %}
euri10 commented 2 years ago

I'm not using starlette directly but fastapi, and this is early exploration so I'm not too sure yet on the design, but one contraint is that endpoints can be either restricted by roles or permissions in a rbac system

so my solution is to have a /login path that sets the username/role/permission in the session and a class dependency that checks for the proper roles or perms and returns a 403 if that doesn't match what the given endpoint wants/needs

alex-oleshkevich commented 2 years ago

Got you. Okay, the Session class is dict-like. You can go like this:

if 'data' in request.session: ...

if 'admin' in request.session.get('data', []): ...

This will make mypy happier.

euri10 commented 2 years ago

thanks, solved for me !