lepture / authlib

The ultimate Python library in building OAuth, OpenID Connect clients and servers. JWS,JWE,JWK,JWA,JWT included.
https://authlib.org/
BSD 3-Clause "New" or "Revised" License
4.49k stars 448 forks source link

Starlette v0.26.0 support and authorize_redirect fix #533

Closed vicchi closed 1 year ago

vicchi commented 1 year ago

Starlette 0.26.0 changed the return value of url_for() from str to URL; see https://github.com/encode/starlette/pull/1385 for context.

Passing a redirect_uri to authorize_redirect causes a TypeError: cannot convert 'URL' object to bytes exception to be raised if the URL has been generated via url_for(). This fix detects an instance of URL and casts it to a string, thus allowing authentication to proceed and generally making people happy.

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)


lepture commented 1 year ago

How about just redirect_uri = str(redirect_uri) without the isinstance? For <0.26.0, str(redirect_uri) would still work.

vicchi commented 1 year ago

@lepture That is indeed true, but my thinking was that just casting to a string by default could introduce sutble gotchas if Starlette's url_for() wasn't being used to build the redirect URL. We can be sure that Starlette's URL class does have an __str__ dunder (see https://github.com/encode/starlette/blob/62b5b6042a39289ed561580c251c233250c3c088/starlette/datastructures.py#L166 for context) but we can't be sure there's nothing else at play here, hence me being cautious and checking that this is an instance of URL before casting.

More than happy to be told I'm being overly cautious here; it wouldn't be the first time.

vicchi commented 1 year ago

@lepture Thanks! 😄