Repository navigation
Fix: OAuth2 JWKS-fetch errors should be 401, not unhandled 500 - #337
Conversation
If ckanext.unaids.auth0_domain/oauth2_api_audience are unset, or the JWKS fetch fails for any reason (network issue, Auth0 outage), this previously raised an uncaught TypeError/URLError that fell through to a generic 500 Internal Server Error, since only jwt.decode() itself was wrapped in error handling. Bearer-token requests now get a clean 401 with a useful message in both cases instead.
The IAuthenticator.identify() hook only set g.userobj/g.user directly, but CKAN's API action dispatcher (ckan/views/api.py) builds its context from Flask-Login's current_user, not g.user/g.userobj -- and identify_user() unconditionally re-derives g.user/g.userobj from current_user right after every IAuthenticator.identify() call, silently overwriting what this plugin set. Net effect: Bearer-token requests authenticated successfully (no error) but ran as the anonymous user, so e.g. organization_list_for_user returned an empty list instead of the caller's actual orgs. This is a real regression from the CKAN 2.11 migration: the original pre-2.11 code had a g.current_user = payload line, which the migration's post-migration cleanup (#331) removed as 'confirmed write-only' -- true, but the actual fix needed was calling Flask-Login's login_user(), the same pattern ckanext-saml2auth already uses for CKAN 2.10+.
|
The IAuthenticator.identify() hook only set g.userobj/g.user directly, This is a real regression from the CKAN 2.11 migration: the original |
The bare mock User class only had .name, but login_user() (added in the previous commit) also needs is_active and get_id() -- both normally provided by flask_login's UserMixin, which this minimal test double doesn't inherit.
Summary
Root cause of a live Naomi <-> ADR integration bug:
SSO/Bearer-token API calls (e.g.
organization_list_for_user) were returning a generic 500 Internal Server Error.Root cause
ckanext.unaids.auth0_domainandckanext.unaids.oauth2_api_audiencewere not configured WithAUTH0_DOMAINunset:raises an immediate
TypeError(concatenatingstr+None), completely outside thetry/exceptthat only wrapped the laterjwt.decode()call. SinceTypeErrorisn'tOAuth2AuthenticationError/OAuth2AuthorizationError, the registered Flask error handlers didn't catch it either, so it fell through to CKAN's generic unhandled-exception 500.The missing config itself is fixed separately in
adx_develop(deploy ini files). This PR fixes the underlying code fragility so the same class of issue (Auth0 outage, network blip, or any other future config mistake) degrades to a proper 401 instead of crashing.Fix
AUTH0_DOMAIN/API_AUDIENCEaren't configured, raiseOAuth2AuthenticationError(→ clean 401) instead of letting aTypeErrorpropagate.urlopen+json.loads) intry/except, logging the real exception and raisingOAuth2AuthenticationErroron any failure.