Use Base64Url encoding and decoding in Session Cookie

[allezxandre]

• Initially raised as discussion SessionMiddleware may create malformed cookie #1259

This Pull Request makes the Session JWS use Base64Url encoding without padding, as described by RFC 7515, Appendix C.

Should close #1259.

[Kludex]

Thanks for the PR.

@allezxandre Would you mind rebasing it? I'm not allowed to make changes to this branch.

[Kludex]

If you can provide an MRE comparing how it works on Flask and DJango, it will speed up the process here.

What I want is to see that the behavior between the web frameworks are matching.

[allezxandre]

My experience with Flask is quite dated, but looking through Flask's code, it looks like they are encoding Cookies using a custom URL-safe serialiser from their itsdangerous package : https://itsdangerous.palletsprojects.com/en/2.1.x/url_safe/

I have never used Django so I wouldn't really know how to read their code.

[Kludex]

But we also use itsdangerous on the SessionMiddleware... 🤔

[allezxandre]

We could use their custom encoder.

The current code change simply proposes to replace Base64 encoding with Cookie/url-safe Base64 encoding as suggested by the JWS RFC.

I think using itsdangerous.url_safe.URLSafeSerializer encoding in place of url-safe Base64 would also be fine, but such a change would increase coupling with itsdangerous, and it would be a bigger code change as it looks like URLSafeSerializer makes some code (i.e. the signing) redundant.

[Kludex]

If you can provide an MRE comparing how it works on Flask and DJango, it will speed up the process here.

What I want is to see that the behavior between the web frameworks are matching.

I need this to get this PR reviewed. To speed up, if you can provide it, it will be cool. Otherwise, I'll create them over the weekend.

[allezxandre]

Sorry, but what’s an MRE? Wikipedia tells me it might be a Minimum Reproducible Example?

[Kludex]

Yep. A snippet with those other web frameworks that shows how they behave considering this PR.

[Kludex]

Why should we use this approach, and not the one that flask uses (with double quotes)?

What does Django do?

[allezxandre]

I don't think the Flask code only uses double quotes? From what I understand they first serialize the cookie using their custom URL-safe serializer.

The point of this PR was more about making the current serializing URL-safe, as it currently isn't and might create malformed cookies (see #1259), so I didn't really put much thought into what serializing method would be more appropriate, I just kept JSON Web Signature Base64 encoding and made it URL Safe.

Currently, it might sometimes serialize a Cookie that might look like this: abcdef1234= (with an = sign used for Base64 encoding), which is unsafe as it can be parsed as a key-value separator:

Set-Cookie: session=abcdef1234=; max-age=31536000; domain=example.com; path=/; secure; SameSite=Lax

For this reason, some browsers (Safari) will escape = before saving, but will not un-escape it reliably when sending it back, so the Session middleware interprets it as malformed and ignores it.

[hasansezertasan]

I have copied the commited starlette/middleware/sessions.pys content and tried the MRE below, mounted the Flask Application to a FastAPI application. Tried to get the same session data from FastAPI side that has set by Flask side and vice versa, didn't work for me.

Flask Application:

# app.py
from flask import Flask, jsonify, session
import datetime

app = Flask(__name__)

config = {
    "SECRET_KEY": "super-secret",
    "PERMANENT_SESSION_LIFETIME": datetime.timedelta(days=7),
}
app.config.update(config)


@app.get("/")
def index():
    return jsonify({"message": "Hello World"})

@app.get("/set-session")
def set_session():
    session["application"] = "flask"
    session.modified = True
    return jsonify({"message": "Session set"})

@app.get("/get-session")
def get_session():
    return jsonify({"message": session.get("application")})

@app.get("/delete-session")
def delete_session():
    session.pop("application")
    session.modified = True
    return jsonify({"message": "Session deleted"})

@app.get("/get-session-data")
def get_session_data():
    print(session.keys())
    print(session.items())
    print(session.values())
    return {"message": "session"}

FastAPI Application:

# main.py
from fastapi import FastAPI, Request, Response
from starlette.middleware.sessions import SessionMiddleware
from starlette.middleware.wsgi import WSGIMiddleware

from app import app as flask_app

app = FastAPI(
    title="FastAPI",
    version="0.0.1",
)
app.mount("/flask-application", WSGIMiddleware(flask_app))
app.add_middleware(
    SessionMiddleware,
    secret_key="super-secret",
)


@app.middleware("http")
async def add_process_time_header(request: Request, call_next):
    response = await call_next(request)
    response.headers["X-Process-Time"] = "100"
    print(response.headers)
    return response


@app.get("/")
async def root(req: Request, res: Response):
    return {"message": "Hello World"}


@app.get("/set-session")
async def set_session(req: Request, res: Response):
    req.session.update({"application": "fastapi"})
    return {"message": "Session set"}


@app.get("/get-session")
async def get_session(req: Request, res: Response):
    return {"message": req.session.get("application")}


@app.get("/delete-session")
async def delete_session(req: Request, res: Response):
    req.session.pop("application")
    return {"message": "Session deleted"}


@app.get("/get-session-data")
async def get_session_data(req: Request, res: Response):
    return {"message": req.session.items()}

I implemented a solution in this discussion tiangolo/fastapi#9318, just yesterday for this, it works quite well. I can send a PR if you want.

[Kludex]

Closing this.

Reason on #1259 (comment).