fix(mcp_service): reduce deprecated authlib.jose.errors imports#41248
Open
eschutho wants to merge 1 commit into
Open
fix(mcp_service): reduce deprecated authlib.jose.errors imports#41248eschutho wants to merge 1 commit into
eschutho wants to merge 1 commit into
Conversation
The authlib.jose module is deprecated since authlib 1.3+ in favour of
joserfc. Our MCP service imported four specific error subclasses
(BadSignatureError, DecodeError, ExpiredTokenError, JoseError) from the
deprecated path. This commit:
- jwt_verifier.py: collapse the four imports to a single `JoseError`
import. The three specific decode-path except clauses are merged into
one `except JoseError` block that discriminates by `e.error` attribute
value ("bad_signature", "expired_token", or generic). The header-parse
catch is tightened from `except (ValueError, DecodeError)` to
`except ValueError` since `_decode_token_header` only raises ValueError.
- mcp_config.py: remove the `JoseError` import entirely. The verifier-
construction fallback is broadened from `except (ValueError, JoseError)`
to `except Exception` — the handler already suppresses details to avoid
leaking key material, so no information is lost.
Both changes are no-behaviour-change for correct code paths. The
filterwarnings call in __init__.py continues to suppress the authlib.jose
deprecation warning emitted by fastmcp's internal import.
Contributor
Code Review Agent Run #a07c40Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41248 +/- ##
==========================================
- Coverage 64.35% 64.34% -0.01%
==========================================
Files 2651 2651
Lines 144804 144832 +28
Branches 33413 33421 +8
==========================================
+ Hits 93187 93195 +8
- Misses 49947 49965 +18
- Partials 1670 1672 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deprecation warning
authlib.josehas been deprecated since authlib 1.3+ in favour ofjoserfc. The library itself forcesAuthlibDeprecationWarningvisible at all warning levels by callingwarnings.simplefilter("always", AuthlibDeprecationWarning)before emitting it. Our MCP service previously imported four specific error subclasses directly from the deprecated path.What changed
superset/mcp_service/jwt_verifier.pyauthlib.jose.errorsimports (BadSignatureError,DecodeError,ExpiredTokenError,JoseError) to a singleJoseErrorimport.except (ValueError, DecodeError)toexcept ValueError—_decode_token_header()only raisesValueError(viabase64.urlsafe_b64decode→binascii.Error→ subclass, andjson.JSONDecodeError→ subclass).except JoseError as e:block that discriminates by thee.errorattribute value ("bad_signature"→ "Signature verification failed","expired_token"→ "Token has expired (detected during decode)", else → "Token decode failed"). Attribute strings verified against authlib source.superset/mcp_service/mcp_config.pyfrom authlib.jose.errors import JoseErrorimport entirely.except (ValueError, JoseError)toexcept Exception. The handler already deliberately suppresses exception details (they may contain key material); this also covers realistic throws the old handler missed (pydantic.ValidationError,cryptography.*Errorfrom malformed keys).The
warnings.filterwarningscall in__init__.pyis still needed (fastmcp importsauthlib.joseinternally) and is unchanged.No behavior change
BadSignatureError,DecodeError, andExpiredTokenErrorremain correct — the new handler routes each to the same failure reason string viae.errorattribute matching._decode_token_header()cannot raise aJoseError.Test plan
pytest tests/unit_tests/mcp_service/test_jwt_verifier.py -x— all jwt_verifier tests passpytest tests/unit_tests/mcp_service/test_mcp_config.py -x— mcp_config tests passAuthlibDeprecationWarningleaks appear when running MCP service tests (-W error::DeprecationWarningor checking test output)🤖 Generated with Claude Code