You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/22 23:54:37 UTC

[PR] fix(mcp): require MCP_JWT_AUDIENCE when MCP JWT auth is enabled [superset]

rusackas opened a new pull request, #41292:
URL: https://github.com/apache/superset/pull/41292

   ### SUMMARY
   
   When the MCP service has JWT auth enabled (`MCP_AUTH_ENABLED=True`), the audience claim was only validated if `MCP_JWT_AUDIENCE` happened to be set. With it unset, the verifier was built with audience validation skipped, so any otherwise-valid token from the same issuer was accepted regardless of which service it was minted for.
   
   This makes audience configuration a required precondition for MCP JWT auth so tokens are correctly bound to this service:
   
   - The default auth factory (`create_default_mcp_auth_factory`) now raises a dedicated `MCPAuthConfigError` when `MCP_AUTH_ENABLED` is true but `MCP_JWT_AUDIENCE` is unset.
   - The server bootstrap (`_create_auth_provider`) re-raises that error rather than swallowing it. This matters because a swallowed build error returns a `None` provider, and the start path treats `auth_provider is None` as "auth disabled" — i.e. the service would otherwise come up unauthenticated. Failing closed makes the misconfiguration a fast, explicit startup error instead.
   - API-key-only deployments (JWT auth disabled) are unaffected and do not require an audience.
   
   The check is placed at the config/bootstrap layer so it covers both verifier variants (`MCPJWTVerifier` and `DetailedJWTVerifier`) uniformly.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — backend configuration behavior.
   
   ### TESTING INSTRUCTIONS
   
   Unit tests added in `tests/unit_tests/mcp_service/test_mcp_config.py` and `tests/unit_tests/mcp_service/test_mcp_server.py`:
   
   - `MCP_AUTH_ENABLED=True` without `MCP_JWT_AUDIENCE` raises `MCPAuthConfigError`.
   - API-key-only auth does not require an audience.
   - JWT auth with an audience set builds the verifier as before.
   - `_create_auth_provider` propagates `MCPAuthConfigError` instead of returning `None`.
   
   Manual: start the MCP service with `MCP_AUTH_ENABLED=True` and `MCP_JWT_AUDIENCE` unset → startup fails with a clear message. Set `MCP_JWT_AUDIENCE` → starts normally.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   > Note: this is a backwards-incompatible change for deployments that enable MCP JWT auth without an audience; documented in `UPDATING.md`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): require MCP_JWT_AUDIENCE when MCP JWT auth is enabled [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41292:
URL: https://github.com/apache/superset/pull/41292#issuecomment-4774204322

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): require MCP_JWT_AUDIENCE when MCP JWT auth is enabled [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41292:
URL: https://github.com/apache/superset/pull/41292#issuecomment-4795273767

   **Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): require MCP_JWT_AUDIENCE when MCP JWT auth is enabled [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41292:
URL: https://github.com/apache/superset/pull/41292


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): require MCP_JWT_AUDIENCE when MCP JWT auth is enabled [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41292:
URL: https://github.com/apache/superset/pull/41292#issuecomment-4774267511

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41292?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `20.00000%` with `4 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 63.57%. Comparing base ([`3b46a5f`](https://app.codecov.io/gh/apache/superset/commit/3b46a5f121365978cb5c9619ac791c7d71eae8c0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0475416`](https://app.codecov.io/gh/apache/superset/commit/0475416584b98c42ad5f11d3190036789722c294?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/41292?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/mcp\_service/mcp\_config.py](https://app.codecov.io/gh/apache/superset/pull/41292?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fmcp_config.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvbWNwX2NvbmZpZy5weQ==) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/41292?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/mcp\_service/server.py](https://app.codecov.io/gh/apache/superset/pull/41292?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fserver.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2Uvc2VydmVyLnB5) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/41292?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #41292      +/-   ##
   ==========================================
   - Coverage   64.34%   63.57%   -0.77%     
   ==========================================
     Files        2653     2653              
     Lines      144952   144957       +5     
     Branches    33433    33434       +1     
   ==========================================
   - Hits        93272    92160    -1112     
   - Misses      49996    51107    +1111     
   - Partials     1684     1690       +6     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.27% <20.00%> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.86% <20.00%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.88% <20.00%> (-1.64%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.71% <20.00%> (-0.01%)` | :arrow_down: |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/41292/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41292?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org