fix: the SSO login button 500s — the redirect was never awaited
Authlib's Starlette client is the async one, so `authorize_redirect` hands back a coroutine. `sso_login` was a sync `def` that returned it, and FastAPI tried to serialise a coroutine as a response body: ValueError: 'coroutine' object is not iterable. Every click of the SSO button was a 500. It has presumably always been broken. Nothing caught it because nothing had a provider configured to click the button with — /auth/sso/config answered sso_enabled:false, so the button was never drawn. It surfaced within a minute of an Authentik being pointed at the app. Verified against the live provider: /api/auth/sso/login now answers 302 to sso.pedshub.com/application/o/authorize/ with response_type, scope, state and nonce. A shape test guards it — the endpoint is a coroutine function and the redirect is awaited — because the alternative is standing up an OIDC provider inside a unit test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
7f36e07af0
commit
6137bf65b8
2 changed files with 31 additions and 3 deletions
|
|
@ -512,8 +512,16 @@ def sso_config():
|
|||
|
||||
|
||||
@router.get("/sso/login")
|
||||
def sso_login(request: Request):
|
||||
"""Redirect user to the OIDC provider for login."""
|
||||
async def sso_login(request: Request):
|
||||
"""Redirect user to the OIDC provider for login.
|
||||
|
||||
`async`, and the redirect awaited. Authlib's Starlette client is the async
|
||||
one — `authorize_redirect` hands back a coroutine — so a sync endpoint
|
||||
returned that coroutine to FastAPI, which tried to serialise it as a
|
||||
response body and answered 500: "'coroutine' object is not iterable". The
|
||||
button had never worked; nothing found it because nothing had SSO
|
||||
configured to click it with.
|
||||
"""
|
||||
from authlib.integrations.starlette_client import OAuth
|
||||
from starlette.responses import RedirectResponse
|
||||
from app.config import settings as cfg
|
||||
|
|
@ -531,7 +539,7 @@ def sso_login(request: Request):
|
|||
)
|
||||
|
||||
redirect_uri = f"{cfg.APP_URL}/api/auth/sso/callback"
|
||||
return oauth.oidc.authorize_redirect(request, redirect_uri)
|
||||
return await oauth.oidc.authorize_redirect(request, redirect_uri)
|
||||
|
||||
|
||||
@router.get("/sso/callback")
|
||||
|
|
|
|||
|
|
@ -94,3 +94,23 @@ class RoleSyncTests(unittest.TestCase):
|
|||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
|
||||
class SsoLoginShapeTests(unittest.TestCase):
|
||||
"""The login redirect has to be awaited, not returned.
|
||||
|
||||
Authlib's Starlette client is the async one: `authorize_redirect` hands
|
||||
back a coroutine. A sync endpoint returned that coroutine to FastAPI,
|
||||
which tried to serialise it and answered 500 — so the SSO button 500'd the
|
||||
first time anybody had a provider configured to click it with. A cheap
|
||||
shape check, because the alternative is standing up an OIDC provider in a
|
||||
unit test.
|
||||
"""
|
||||
|
||||
def test_the_redirect_endpoint_is_a_coroutine_function(self):
|
||||
import inspect
|
||||
from app.routers import auth
|
||||
self.assertTrue(inspect.iscoroutinefunction(auth.sso_login))
|
||||
self.assertTrue(inspect.iscoroutinefunction(auth.sso_callback))
|
||||
source = inspect.getsource(auth.sso_login)
|
||||
self.assertIn("await oauth.oidc.authorize_redirect", source)
|
||||
|
|
|
|||
Loading…
Reference in a new issue