Skip to content

fix(semantic-layers): apply datasource access checks on layer endpoints#41430

Open
sha174n wants to merge 3 commits into
apache:masterfrom
sha174n:fix/semantic-layer-access-checks
Open

fix(semantic-layers): apply datasource access checks on layer endpoints#41430
sha174n wants to merge 3 commits into
apache:masterfrom
sha174n:fix/semantic-layer-access-checks

Conversation

@sha174n

@sha174n sha174n commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

Brings the per-object semantic layer endpoints in line with the list endpoint and with the semantic view endpoints for datasource_access handling.

SemanticLayerDAO.find_all() (used by the list endpoint) already scopes layers to the caller's datasource_access grants. This change introduces SemanticLayer.raise_for_access(), mirroring the existing SemanticView.raise_for_access(), and applies it consistently on:

  • GET /api/v1/semantic_layer/<uuid> — returns 404 when the layer is not accessible to the caller
  • PUT and DELETE /api/v1/semantic_layer/<uuid> — return 403 when not accessible
  • semantic view creation — now requires access to the parent layer

This keeps single-object access behavior consistent with the list endpoint and with the semantic view resource. The feature is behind the SEMANTIC_LAYERS flag (off by default).

TESTING INSTRUCTIONS

Unit tests added for SemanticLayer.raise_for_access() and for the get / update / delete / view-create command paths. Run:

pytest tests/unit_tests/commands/semantic_layer tests/unit_tests/semantic_layers/models_test.py

ADDITIONAL INFORMATION

  • Has associated issue: feature is behind the default-off SEMANTIC_LAYERS flag
  • Required feature flags: SEMANTIC_LAYERS
  • Changes UI
  • Includes DB Migration

sha174n and others added 2 commits June 25, 2026 16:58
…pdate/delete

Add SemanticLayer.raise_for_access(), mirroring the existing
SemanticView.raise_for_access(), and apply it on the single-layer GET
(returning 404 when the layer is not accessible), update, and delete paths.
This makes the per-object endpoints consistent with the list endpoint, which
already filters via SemanticLayerDAO.find_all(). Adds unit coverage for the
new model method and the command paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CreateSemanticViewCommand previously only checked that the parent layer
existed. Call the parent layer's raise_for_access() during validation so a
user without access to a layer cannot create a view under it, consistent with
the access checks on the layer get/update/delete paths. The batch view-create
endpoint reports this as a per-item error. Adds unit coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the api Related to the REST API label Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 2.77778% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.88%. Comparing base (a90c8e0) to head (757030f).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
superset/semantic_layers/api.py 0.00% 12 Missing ⚠️
superset/semantic_layers/models.py 11.11% 8 Missing ⚠️
superset/commands/semantic_layer/create.py 0.00% 7 Missing ⚠️
superset/commands/semantic_layer/delete.py 0.00% 4 Missing ⚠️
superset/commands/semantic_layer/update.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #41430      +/-   ##
==========================================
- Coverage   64.48%   63.88%   -0.60%     
==========================================
  Files        2661     2661              
  Lines      145642   145697      +55     
  Branches    33613    33618       +5     
==========================================
- Hits        93912    93082     -830     
- Misses      50029    50911     +882     
- Partials     1701     1704       +3     
Flag Coverage Δ
hive 39.22% <2.77%> (-0.03%) ⬇️
mysql 57.96% <2.77%> (-0.04%) ⬇️
postgres 58.03% <2.77%> (-0.04%) ⬇️
presto 40.80% <2.77%> (-0.03%) ⬇️
python 58.24% <2.77%> (-1.27%) ⬇️
sqlite 57.68% <2.77%> (-0.04%) ⬇️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sha174n sha174n marked this pull request as ready for review June 25, 2026 19:51
@dosubot dosubot Bot added the authentication:access-control Rlated to access control label Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the REST API authentication:access-control Rlated to access control size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants