Compare commits

..
Author SHA1 Message Date
sadpandajoe 4e322f0d96 fix(explore): require a value for simple adhoc filters before allowing save
The Save button in the adhoc filter popover stayed enabled when a
comparator-taking operator had no value, because `AdhocFilter.isValid()`
only rejected a `null` comparator. An unset comparator is `undefined`, not
`null`: selecting a subject resets it (and falls back to the `IN` operator),
and the value Select's clear affordance emits `undefined` as well.

This was most visible on boolean columns, whose operator list is restricted
to unary operators, so a freshly picked boolean column lands on `IN` with no
value and the popover looks complete. Saving sent a filter with no `val` to
the query API, which tripped a bare `assert isinstance(eq, (tuple, list))`
in the query builder and surfaced as a generic error instead of inline
client-side validation.

Extend the existing check to treat `undefined` like `null`, matching the
empty-array guard already applied to `IN`/`NOT IN` comparators. Unary
operators are unaffected: they short-circuit earlier via
DISABLE_INPUT_OPERATORS.
2026-08-18 23:55:54 +00:00
DanielSwift1992 097c99b19c fix: remove a labeler glob that matches no files (#43270) 2026-08-18 16:21:01 -07:00
David Dallakyan 5ce52e531d fix(clickhouse): add PT1S time grain (#43217) 2026-08-18 15:49:18 -07:00
34cd50cc48 test(chart): mock the event log endpoint in the drill-to-detail menu test (#43183)
Co-authored-by: bikashJMV <bikash@jmv.co.in>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Joe Li <joe@preset.io>
2026-08-18 13:51:54 -07:00
c0ab5f3385 fix(dashboard): preserve native filter keys for dataset-less filters on save (#42898)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Mehmet Salih Yavuz <salih.yavuz@proton.me>
2026-08-18 21:30:50 +03:00
ʈᵃᵢ 7d4f30574f feat(tooltip): add Truncate labels control to timeseries charts (#43272) 2026-08-18 11:05:07 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> e4ea6e23d8 chore(deps): bump supercluster from 8.0.1 to 9.0.0 in /superset-frontend (#43290)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-18 10:35:17 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> a13a5f1af4 chore(deps-dev): bump @typescript-eslint/eslint-plugin from 8.65.0 to 8.67.0 in /superset-websocket (#43284)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-18 10:34:39 -07:00
Endi Monan f994602096 fix(dashboard): align list OpenAPI schema (#43256) 2026-08-18 10:22:50 -07:00
Amin GhadersohiandClaude Fable 5 086b4af65d feat(mcp): per-resource token scopes with user-permission intersection (#42297)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-18 13:11:27 -04:00
fd063d17bf fix(security): harden account password-change and session-invalidation handling (#42934)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Joe Li <joe@preset.io>
2026-08-18 17:40:08 +01:00
60e1802c52 fix(dashboard): mute the Group By display control loading spinner (#42879)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
2026-08-18 09:37:43 -07:00
Joe LiandClaude Opus 4.8 2d1daac11a fix(explore): samples endpoint now honors requested row limit (#43148)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-18 09:28:04 -07:00
70 changed files with 3006 additions and 479 deletions
+1 -1
View File
@@ -29,7 +29,7 @@
"dependencies:python":
- changed-files:
- any-glob-to-any-file:
- 'superset/requirements/**'
- 'requirements/**'
- 'superset/translations/requirements.txt'
- 'RELEASING/requirements.txt'
+2
View File
@@ -24,6 +24,8 @@ assists people when migrating to a new version.
## Next
- `SAMPLES_ROW_LIMIT` is now the default for `/datasource/samples` requests without a valid explicit `per_page`, rather than a hard per-request ceiling; explicit limits are honored up to the existing global row-limit ceiling, matching `/chart/data` SAMPLES requests.
### OAuth2 database callback metrics include their outcome
The unqualified `DatabaseRestApi.oauth2` StatsD counter has been replaced with
+13 -1
View File
@@ -400,7 +400,7 @@ Once enabled, each user manages their own keys from their profile page:
1. Open the user menu (top-right) and click **Info** to navigate to the User Info page
2. Expand the **API Keys** section
3. Click **+ API Key**
4. Enter a name and (optionally) an expiration date
4. Enter a name and optionally select resource scopes
5. Copy the generated token — it is shown only once
Only users with the `can_read` and `can_write` permissions on `ApiKey` (granted by default to Admins) can manage API keys.
@@ -415,6 +415,18 @@ Authorization: Bearer <your-api-key>
This works for all REST API endpoints and the MCP server. The request is executed with the permissions of the user who created the key.
#### API Key Scopes
The creation dialog can restrict an API key to MCP resource actions such as
`superset:dashboard:read` or `superset:chart:write`. A scope is an additional
restriction: it never grants a permission that the creating user does not
already have through Superset RBAC. Write scopes also cover update and delete
operations for that resource; `superset:sqllab:write` covers SQL execution.
Keys created without scopes retain legacy RBAC-only behavior. The scoped-key
restrictions described here are enforced by the MCP server; regular REST API
routes continue to apply their existing Superset RBAC checks.
#### Use Cases
- **CI/CD pipelines** — automated chart/dashboard exports and imports
+44 -32
View File
@@ -3407,22 +3407,26 @@
"nullable": true,
"type": "string"
},
"description": {
"nullable": true,
"type": "string"
},
"editors": {
"items": {
"$ref": "#/components/schemas/DashboardRestApi.get_list.Subject"
},
"type": "array"
},
"id": {
"type": "integer"
},
"is_managed_externally": {
"type": "boolean"
},
"owners": {
"$ref": "#/components/schemas/DashboardRestApi.get_list.User2"
},
"published": {
"nullable": true,
"type": "boolean"
},
"roles": {
"$ref": "#/components/schemas/DashboardRestApi.get_list.Role"
},
"slug": {
"maxLength": 255,
"nullable": true,
@@ -3432,10 +3436,10 @@
"readOnly": true
},
"tags": {
"$ref": "#/components/schemas/DashboardRestApi.get_list.Tag"
},
"thumbnail_url": {
"readOnly": true
"items": {
"$ref": "#/components/schemas/DashboardRestApi.get_list.Tag"
},
"type": "array"
},
"url": {
"readOnly": true
@@ -3444,21 +3448,46 @@
"format": "uuid",
"nullable": true,
"type": "string"
},
"viewers": {
"items": {
"$ref": "#/components/schemas/DashboardRestApi.get_list.Subject1"
},
"type": "array"
}
},
"type": "object"
},
"DashboardRestApi.get_list.Role": {
"DashboardRestApi.get_list.Subject": {
"properties": {
"id": {
"type": "integer"
},
"name": {
"maxLength": 64,
"label": {
"maxLength": 255,
"type": "string"
},
"type": {
"type": "integer"
}
},
"required": ["name"],
"required": ["label", "type"],
"type": "object"
},
"DashboardRestApi.get_list.Subject1": {
"properties": {
"id": {
"type": "integer"
},
"label": {
"maxLength": 255,
"type": "string"
},
"type": {
"type": "integer"
}
},
"required": ["label", "type"],
"type": "object"
},
"DashboardRestApi.get_list.Tag": {
@@ -3511,23 +3540,6 @@
"required": ["first_name", "last_name"],
"type": "object"
},
"DashboardRestApi.get_list.User2": {
"properties": {
"first_name": {
"maxLength": 64,
"type": "string"
},
"id": {
"type": "integer"
},
"last_name": {
"maxLength": 64,
"type": "string"
}
},
"required": ["first_name", "last_name"],
"type": "object"
},
"DashboardRestApi.post": {
"properties": {
"certification_details": {
@@ -16506,7 +16518,7 @@
},
"result": {
"items": {
"type": "object"
"$ref": "#/components/schemas/DashboardRestApi.get_list"
},
"type": "array"
}
+13 -4
View File
@@ -27185,9 +27185,9 @@
}
},
"node_modules/kdbush": {
"version": "4.0.2",
"resolved": "https://registry.npmjs.org/kdbush/-/kdbush-4.0.2.tgz",
"integrity": "sha512-WbCVYJ27Sz8zi9Q7Q0xHC+05iwkm3Znipc2XTlrnJbsHMYktW4hPhXUE8Ys1engBrvffoSCqbil1JQAa7clRpA==",
"version": "4.1.0",
"resolved": "https://registry.npmjs.org/kdbush/-/kdbush-4.1.0.tgz",
"integrity": "sha512-e9vurzrXJQrFX6ckpHP3bvj5l+9CnYzkxDNnNQ1h2QTqdWsUAJgXiKdGNcOa1EY85dU8KbQ+z/FdQdB7P+9yfQ==",
"license": "ISC"
},
"node_modules/keyv": {
@@ -43659,7 +43659,7 @@
"mapbox-gl": "^3.28.1",
"maplibre-gl": "^5.24.0",
"react-map-gl": "^8.1.2",
"supercluster": "^8.0.1"
"supercluster": "^9.0.0"
},
"peerDependencies": {
"@apache-superset/core": "*",
@@ -43669,6 +43669,15 @@
"react-dom": "^18.3.0"
}
},
"plugins/plugin-chart-point-cluster-map/node_modules/supercluster": {
"version": "9.0.0",
"resolved": "https://registry.npmjs.org/supercluster/-/supercluster-9.0.0.tgz",
"integrity": "sha512-SaU8dQaxagTXr8+a1f67Pxg5CiRcZsq+LPQsZoV2r+FD8EF3Wpg5v7Zl8STPhml3oWHF1CQ8YaJGf8Uo8zS5qg==",
"license": "ISC",
"dependencies": {
"kdbush": "^4.1.0"
}
},
"plugins/plugin-chart-table": {
"name": "@superset-ui/plugin-chart-table",
"version": "0.20.3",
@@ -65,7 +65,6 @@ export type AntdExposedProps = Pick<
| 'onOpenChange'
| 'optionRender'
| 'placeholder'
| 'prefix'
| 'showArrow'
| 'showSearch'
| 'tokenSeparators'
@@ -19,19 +19,71 @@
import { t } from '@apache-superset/core/translation';
import { sanitizeHtml } from './html';
export type TooltipTruncationMode = 'off' | 'end' | 'start' | 'middle';
export const TRUNCATION_MAX_CHARS = 40;
const TRUNCATION_STYLE = `
max-width: 300px;
overflow: hidden;
text-overflow: ellipsis;
`;
const NOWRAP_STYLE = `
white-space: nowrap;
`;
/**
* Shortens plain text so a tooltip label stays readable, placing the ellipsis
* where the caller asked for it.
*
* Only 'start' and 'middle' slice. 'end' is handled by CSS in tooltipHtml, and
* 'off' means no truncation at all, so both return the input untouched.
*
* The input must be plain text. Callers are responsible for truncating before
* any markup (such as the ECharts series marker) is prepended, and before
* sanitization — slicing a string that already contains markup would cut into
* a tag.
*/
export function truncateLabel(
text: string,
mode: TooltipTruncationMode = 'end',
): string {
if (
(mode !== 'start' && mode !== 'middle') ||
text.length <= TRUNCATION_MAX_CHARS
) {
return text;
}
const budget = TRUNCATION_MAX_CHARS - 1;
if (mode === 'start') {
return `${text.slice(-budget)}`;
}
const head = Math.ceil(budget / 2);
const tail = Math.floor(budget / 2);
return `${text.slice(0, head)}${text.slice(-tail)}`;
}
function getTruncationStyle(mode: TooltipTruncationMode): string {
if (mode === 'end') {
return TRUNCATION_STYLE;
}
if (mode === 'off') {
return '';
}
// 'start' and 'middle' are already sliced upstream; keep them on one line.
return NOWRAP_STYLE;
}
export function tooltipHtml(
data: string[][],
title?: string,
focusedRow?: number,
truncation: TooltipTruncationMode = 'end',
) {
const truncationStyle = getTruncationStyle(truncation);
const titleRow = title
? `<span style="font-weight: 700;${TRUNCATION_STYLE}">${title}</span>`
? `<span style="font-weight: 700;${truncationStyle}">${title}</span>`
: '';
return sanitizeHtml(`
<div>
@@ -46,7 +98,7 @@ export function tooltipHtml(
const cellStyle = `
text-align: ${j > 0 ? 'right' : 'left'};
padding-left: ${j === 0 ? 0 : 16}px;
${TRUNCATION_STYLE}
${truncationStyle}
`;
return `<td style="${cellStyle}">${cell}</td>`;
});
@@ -16,7 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
import { sanitizeHtml, tooltipHtml } from '@superset-ui/core';
import {
sanitizeHtml,
tooltipHtml,
truncateLabel,
TRUNCATION_MAX_CHARS,
} from '@superset-ui/core';
const TITLE_STYLE =
'style="font-weight: 700;max-width:300px;overflow:hidden;text-overflow:ellipsis;"';
@@ -182,3 +187,88 @@ test('should preserve table styling after sanitization (fixes ECharts tooltip fo
expect(html).toContain('padding-left:16px');
expect(html).toContain('max-width:300px');
});
describe('truncateLabel', () => {
const long = 'prod-us-east-1-service-checkout-latency-p99'; // 43 chars
test('returns text unchanged for off and end', () => {
expect(truncateLabel(long, 'off')).toBe(long);
expect(truncateLabel(long, 'end')).toBe(long);
});
test('defaults to end, which does not slice', () => {
expect(truncateLabel(long)).toBe(long);
});
test('truncates the start, keeping the distinguishing suffix', () => {
expect(truncateLabel(long, 'start')).toBe(
'…-us-east-1-service-checkout-latency-p99',
);
expect(truncateLabel(long, 'start')).toHaveLength(TRUNCATION_MAX_CHARS);
});
test('truncates the middle, keeping both ends', () => {
expect(truncateLabel(long, 'middle')).toBe(
'prod-us-east-1-servi…heckout-latency-p99',
);
expect(truncateLabel(long, 'middle')).toHaveLength(TRUNCATION_MAX_CHARS);
});
test('leaves text at or under the limit untouched', () => {
const atLimit = 'x'.repeat(TRUNCATION_MAX_CHARS);
expect(truncateLabel(atLimit, 'start')).toBe(atLimit);
expect(truncateLabel(atLimit, 'middle')).toBe(atLimit);
expect(truncateLabel('short', 'start')).toBe('short');
expect(truncateLabel('', 'middle')).toBe('');
});
test('truncates text one character over the limit', () => {
const overLimit = 'x'.repeat(TRUNCATION_MAX_CHARS + 1);
expect(truncateLabel(overLimit, 'start')).toBe(
`${'x'.repeat(TRUNCATION_MAX_CHARS - 1)}`,
);
});
});
describe('tooltipHtml truncation modes', () => {
const rows = [['label', 'value']];
// sanitizeHtml normalizes spacing inside style attributes, and it does so
// differently across versions, so compare with whitespace stripped.
const styles = (
title: string | undefined,
truncation?: 'off' | 'end' | 'start' | 'middle',
) => removeWhitespaces(tooltipHtml(rows, title, undefined, truncation));
test('emits the 300px cap for end and for the default', () => {
expect(styles('Title', 'end')).toContain('max-width:300px');
expect(tooltipHtml(rows, 'Title')).toBe(
tooltipHtml(rows, 'Title', undefined, 'end'),
);
});
test('emits no truncation style for off', () => {
const html = styles('Title', 'off');
expect(html).not.toContain('max-width');
expect(html).not.toContain('text-overflow');
expect(html).not.toContain('white-space');
});
test.each(['start', 'middle'] as const)(
'emits nowrap instead of a cap for %s',
mode => {
const html = styles('Title', mode);
expect(html).toContain('white-space:nowrap');
expect(html).not.toContain('max-width');
},
);
test('never slices cell text itself, whatever the mode', () => {
const longCell = 'y'.repeat(TRUNCATION_MAX_CHARS + 20);
(['off', 'end', 'start', 'middle'] as const).forEach(mode => {
expect(tooltipHtml([[longCell]], undefined, undefined, mode)).toContain(
longCell,
);
});
});
});
@@ -40,6 +40,7 @@ import {
TimeseriesChartDataResponseResult,
TimeseriesDataRecord,
tooltipHtml,
truncateLabel,
ValueFormatter,
} from '@superset-ui/core';
import { GenericDataType } from '@apache-superset/core/common';
@@ -207,6 +208,7 @@ export default function transformProps(
zoomable,
richTooltip,
tooltipSortByMetric,
tooltipTruncation,
xAxisBounds,
xAxisLabelRotation,
xAxisLabelInterval,
@@ -907,13 +909,19 @@ export default function transformProps(
formatter: primarySeries.has(key)
? tooltipFormatter
: tooltipFormatterSecondary,
truncation: tooltipTruncation,
});
rows.push(row);
if (key === focusedSeries) {
focusedRow = rows.length - 1;
}
});
return tooltipHtml(rows, tooltipFormatter(xValue), focusedRow);
return tooltipHtml(
rows,
truncateLabel(tooltipFormatter(xValue), tooltipTruncation),
focusedRow,
tooltipTruncation,
);
},
},
legend: {
@@ -24,6 +24,7 @@ import {
ContributionType,
TimeFormatter,
AxisType,
TooltipTruncationMode,
} from '@superset-ui/core';
import {
BaseChartProps,
@@ -59,6 +60,7 @@ export type EchartsMixedTimeseriesFormData = QueryFormData & {
timeGrainSqla?: TimeGranularity;
forceMaxInterval?: boolean;
tooltipTimeFormat?: string;
tooltipTruncation?: TooltipTruncationMode;
zoomable: boolean;
richTooltip: boolean;
showQueryIdentifiers?: boolean;
@@ -108,6 +110,7 @@ export const DEFAULT_FORM_DATA: EchartsMixedTimeseriesFormData = {
yAxisFormatSecondary: TIMESERIES_DEFAULTS.yAxisFormat,
yAxisTitleSecondary: DEFAULT_TITLE_FORM_DATA.yAxisTitle,
tooltipTimeFormat: TIMESERIES_DEFAULTS.tooltipTimeFormat,
tooltipTruncation: TIMESERIES_DEFAULTS.tooltipTruncation,
xAxisBounds: TIMESERIES_DEFAULTS.xAxisBounds,
xAxisForceCategorical: TIMESERIES_DEFAULTS.xAxisForceCategorical,
xAxisTimeFormat: TIMESERIES_DEFAULTS.xAxisTimeFormat,
@@ -73,6 +73,7 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
seriesType: EchartsTimeseriesSeriesType.Line,
stack: false,
tooltipTimeFormat: 'smart_date',
tooltipTruncation: 'end',
xAxisTimeFormat: 'smart_date',
xAxisNumberFormat: 'SMART_NUMBER',
truncateXAxis: true,
@@ -30,6 +30,7 @@ import {
DTTM_ALIAS,
ensureIsArray,
tooltipHtml,
truncateLabel,
getCustomFormatter,
getMetricLabel,
getNumberFormatter,
@@ -303,6 +304,7 @@ export default function transformProps(
tooltipSortByMetric,
showTooltipTotal,
showTooltipPercentage,
tooltipTruncation,
truncateXAxis,
truncateYAxis,
xAxis: xAxisOrig,
@@ -1449,6 +1451,7 @@ export default function transformProps(
seriesName: key,
formatter,
marker,
truncation: tooltipTruncation,
});
const annotationRow = annotationLayers.some(
@@ -1482,7 +1485,12 @@ export default function transformProps(
}
rows.push(totalRow);
}
return tooltipHtml(rows, tooltipFormatter(xValue), focusedRow);
return tooltipHtml(
rows,
truncateLabel(tooltipFormatter(xValue), tooltipTruncation),
focusedRow,
tooltipTruncation,
);
},
},
legend: {
@@ -25,6 +25,7 @@ import {
QueryFormMetric,
TimeFormatter,
TimeGranularity,
TooltipTruncationMode,
} from '@superset-ui/core';
import {
BaseChartProps,
@@ -82,6 +83,7 @@ export type EchartsTimeseriesFormData = QueryFormData & {
tooltipTimeFormat?: string;
showTooltipTotal?: boolean;
showTooltipPercentage?: boolean;
tooltipTruncation?: TooltipTruncationMode;
truncateXAxis: boolean;
truncateYAxis: boolean;
yAxisFormat?: string;
@@ -315,6 +315,27 @@ const tooltipPercentageControl: ControlSetItem = {
},
};
const tooltipTruncationControl: ControlSetItem = {
name: 'tooltipTruncation',
config: {
type: 'SelectControl',
freeForm: false,
label: t('Truncate labels'),
renderTrigger: true,
default: 'end',
clearable: false,
choices: [
['off', t('Off')],
['end', t('End')],
['start', t('Start')],
['middle', t('Middle')],
],
description: t(
'Where to place the ellipsis when a tooltip label is too long. Choose Off to always show the full label, or Start when labels share a common prefix.',
),
},
};
export const richTooltipSection: ControlSetRow[] = [
[<ControlSubSectionHeader>{t('Tooltip')}</ControlSubSectionHeader>],
[richTooltipControl],
@@ -322,6 +343,7 @@ export const richTooltipSection: ControlSetRow[] = [
[tooltipPercentageControl],
[tooltipSortByMetricControl],
[tooltipTimeFormatControl],
[tooltipTruncationControl],
];
const sortSeriesType: ControlSetItem = {
@@ -16,7 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import { DataRecord, DTTM_ALIAS, ValueFormatter } from '@superset-ui/core';
import {
DataRecord,
DTTM_ALIAS,
truncateLabel,
TooltipTruncationMode,
ValueFormatter,
} from '@superset-ui/core';
import type { OptionName, SeriesOption } from 'echarts/types/src/util/types';
import type { TooltipMarker } from 'echarts/types/src/util/format';
import {
@@ -91,12 +97,16 @@ export const formatForecastTooltipSeries = ({
forecastUpper,
marker,
formatter,
truncation = 'end',
}: ForecastValue & {
seriesName: string;
marker: TooltipMarker;
formatter: ValueFormatter;
truncation?: TooltipTruncationMode;
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
// Truncate before sanitizing and before the marker is prepended: slicing a
// string that already contains markup would cut into the marker's tag.
const name = `${marker}${sanitizeHtml(truncateLabel(seriesName, truncation))}`;
let value = typeof observation === 'number' ? formatter(observation) : '';
// Use finite-number checks rather than truthiness so that legitimate
// zero values (e.g. a forecast that crosses zero, or a confidence bound of
@@ -27,6 +27,7 @@ import {
VizType,
ChartDataResponseResult,
TimeGranularity,
TooltipTruncationMode,
} from '@superset-ui/core';
import { GenericDataType } from '@apache-superset/core/common';
import {
@@ -1295,3 +1296,59 @@ test('y-axis title position: non-Left sets nameLocation to end', () => {
expect(yAxis[1].nameGap).toEqual(30);
expect(yAxis[1].nameLocation).toEqual('end');
});
describe('EchartsMixedTimeseries tooltip truncation', () => {
const longSeriesName = 'prod-us-east-1-service-checkout-latency-p99';
const marker = '<span style="background-color:#1f77b4;"></span>';
const buildTooltip = (tooltipTruncation?: TooltipTruncationMode) => {
const chartProps = createEchartsTimeseriesTestChartProps<
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesProps
>({
...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
defaultQueriesData: queriesData,
formData: {
...formData,
...(tooltipTruncation ? { tooltipTruncation } : {}),
},
queriesData,
});
const { echartOptions } = transformProps(chartProps);
const { formatter } = echartOptions.tooltip as {
formatter: (params: unknown) => string;
};
// richTooltip is false in this fixture, so the trigger is 'item' and the
// formatter receives a single param object rather than an array.
return formatter({
seriesId: longSeriesName,
seriesName: longSeriesName,
value: [599616000000, 1],
marker,
});
};
test('keeps full text with the CSS cap by default', () => {
const html = buildTooltip();
expect(html.replace(/\s/g, '')).toContain('max-width:300px');
expect(html).toContain(longSeriesName);
});
test('removes the cap and keeps full text when off', () => {
const html = buildTooltip('off');
expect(html).not.toContain('max-width');
expect(html).toContain(longSeriesName);
});
test('drops the shared prefix when truncating from the start', () => {
const html = buildTooltip('start');
expect(html).not.toContain('prod-us-east');
expect(html).toContain('latency-p99');
expect(html).toContain('background-color:#1f77b4');
});
test('keeps both ends when truncating the middle', () => {
const html = buildTooltip('middle');
expect(html).toContain('prod-us-east-1-servi…heckout-latency-p99');
expect(html).not.toContain(longSeriesName);
});
});
@@ -32,6 +32,7 @@ import {
TimeseriesAnnotationLayer,
ChartDataResponseResult,
TimeGranularity,
TooltipTruncationMode,
} from '@superset-ui/core';
import { GenericDataType } from '@apache-superset/core/common';
import { supersetTheme } from '@apache-superset/core/theme';
@@ -2437,3 +2438,94 @@ test('honors the snake_case flag the compare-chart migration stores in params',
[BASE_TIMESTAMP + 300000000, 2],
]);
});
describe('EchartsTimeseries tooltip truncation', () => {
const longSeriesName = 'prod-us-east-1-service-checkout-latency-p99';
const marker = '<span style="background-color:#1f77b4;"></span>';
const buildTooltip = (
tooltipTruncation?: TooltipTruncationMode,
xValue: string | number = 599616000000,
) => {
const chartProps = new ChartProps({
formData: {
colorScheme: 'bnbColors',
datasource: '3__table',
granularity_sqla: 'ds',
metric: 'sum__num',
groupby: ['foo'],
viz_type: 'my_viz',
...(tooltipTruncation ? { tooltipTruncation } : {}),
} as SqlaFormData,
width: 800,
height: 600,
queriesData: [
{
data: [
{ [longSeriesName]: 1, __timestamp: 599616000000 },
{ [longSeriesName]: 3, __timestamp: 599916000000 },
],
},
],
theme: supersetTheme,
});
const { echartOptions } = transformProps(
chartProps as EchartsTimeseriesChartProps,
);
const { formatter } = echartOptions.tooltip as {
formatter: (params: unknown) => string;
};
return formatter([
{
seriesId: longSeriesName,
seriesName: longSeriesName,
value: [xValue, 1],
marker,
},
]);
};
test('applies the CSS cap and keeps full text by default', () => {
const html = buildTooltip();
expect(html).toContain(longSeriesName);
// sanitizeHtml normalizes spacing inside style attributes, so compare with
// whitespace stripped rather than hard-coding one version's formatting.
expect(html.replace(/\s/g, '')).toContain('max-width:300px');
});
test('removes the cap and keeps full text when off', () => {
const html = buildTooltip('off');
expect(html).not.toContain('max-width');
expect(html).toContain(longSeriesName);
});
test('drops the shared prefix when truncating from the start', () => {
const html = buildTooltip('start');
expect(html).not.toContain('prod-us-east');
expect(html).toContain('latency-p99');
expect(html.replace(/\s/g, '')).toContain('white-space:nowrap');
});
test('keeps both ends when truncating the middle', () => {
const html = buildTooltip('middle');
expect(html).toContain('prod-us-east-1-servi…heckout-latency-p99');
expect(html).not.toContain(longSeriesName);
});
test('preserves the echarts marker in every mode', () => {
(['off', 'end', 'start', 'middle'] as const).forEach(mode => {
expect(buildTooltip(mode)).toContain('background-color:#1f77b4');
});
});
test('truncates a long non-temporal x-axis title', () => {
const longCategory = 'prod-us-east-1-service-checkout-cohort-2026';
const html = buildTooltip('start', longCategory);
expect(html).not.toContain(longCategory);
expect(html).toContain('cohort-2026');
});
test('leaves a long title alone in the default mode', () => {
const longCategory = 'prod-us-east-1-service-checkout-cohort-2026';
expect(buildTooltip(undefined, longCategory)).toContain(longCategory);
});
});
@@ -16,7 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
import { getNumberFormatter, NumberFormats } from '@superset-ui/core';
import {
getNumberFormatter,
NumberFormats,
TRUNCATION_MAX_CHARS,
} from '@superset-ui/core';
import { SeriesOption } from 'echarts';
import {
extractForecastSeriesContext,
@@ -411,3 +415,52 @@ test('formatForecastTooltipSeries should skip non-finite forecast values', () =>
}),
).toEqual(['<img>qwerty', '10']);
});
describe('formatForecastTooltipSeries truncation', () => {
const marker =
'<span style="display:inline-block;width:10px;height:10px;background-color:#1f77b4;"></span>';
const longName = 'prod-us-east-1-service-checkout-latency-p99'; // 43 chars
const intFormatter = getNumberFormatter(NumberFormats.INTEGER);
const format = (truncation?: 'off' | 'end' | 'start' | 'middle') =>
formatForecastTooltipSeries({
seriesName: longName,
observation: 1,
marker,
formatter: intFormatter,
...(truncation ? { truncation } : {}),
})[0];
test('leaves the name intact by default and for off/end', () => {
expect(format()).toContain(longName);
expect(format('off')).toContain(longName);
expect(format('end')).toContain(longName);
});
test('slices the start of the name without harming the marker', () => {
const cell = format('start');
expect(cell).toContain(marker);
expect(cell).toContain('…-us-east-1-service-checkout-latency-p99');
expect(cell).not.toContain('prod-us-east');
});
test('slices the middle of the name without harming the marker', () => {
const cell = format('middle');
expect(cell).toContain(marker);
expect(cell).toContain('prod-us-east-1-servi…heckout-latency-p99');
});
test('measures the budget against the name, not the marker markup', () => {
// The marker alone is far longer than the budget. If truncation were
// applied to the concatenated cell, a short name would be mangled.
expect(marker.length).toBeGreaterThan(TRUNCATION_MAX_CHARS);
const [cell] = formatForecastTooltipSeries({
seriesName: 'cpu',
observation: 1,
marker,
formatter: intFormatter,
truncation: 'start',
});
expect(cell).toBe(`${marker}cpu`);
});
});
@@ -33,7 +33,7 @@
"mapbox-gl": "^3.28.1",
"maplibre-gl": "^5.24.0",
"react-map-gl": "^8.1.2",
"supercluster": "^8.0.1"
"supercluster": "^9.0.0"
},
"peerDependencies": {
"@apache-superset/core": "*",
@@ -17,6 +17,7 @@
* under the License.
*/
import { useState } from 'react';
import fetchMock from 'fetch-mock';
import {
cleanup,
render,
@@ -35,6 +36,10 @@ import { useDrillDetailMenuItems, DrillDetailMenuItemsProps } from './index';
/* eslint jest/expect-expect: ["warn", { "assertFunctionNames": ["expect*"] }] */
// Opening the context menu logs an event, and an unmatched request makes
// fetch-mock throw inside the component.
fetchMock.post('glob:*/log/?*', {});
jest.mock(
'../DrillDetail/DrillDetailPane',
() =>
@@ -16,8 +16,20 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
ChartCustomizationType,
type ChartCustomization,
} from '@superset-ui/core';
import { LabeledValue } from '@superset-ui/core/components';
import { createLabelSortComparator } from './GroupByFilterCard';
import { render, screen } from 'spec/helpers/testing-library';
import GroupByFilterCard, {
createLabelSortComparator,
} from './GroupByFilterCard';
jest.mock('src/utils/cachedSupersetGet', () => ({
// Never resolves, pinning the card in its column-loading state.
cachedSupersetGet: jest.fn(() => new Promise(() => {})),
}));
const apple: LabeledValue = { value: 'a', label: 'Apple' };
const banana: LabeledValue = { value: 'b', label: 'Banana' };
@@ -39,3 +51,27 @@ test('preserves source order when sortAscending is unset', () => {
expect(compare(apple, banana)).toBe(0);
expect(compare(banana, apple)).toBe(0);
});
const groupByCustomization: ChartCustomization = {
id: 'groupby-1',
name: 'Group By',
filterType: 'filter_groupby',
type: ChartCustomizationType.ChartCustomization,
targets: [{ datasetId: 1 }],
scope: { rootPath: [], excluded: [] },
controlValues: {},
defaultDataMask: {},
};
test('renders the column-loading spinner small and muted', async () => {
render(<GroupByFilterCard customizationItem={groupByCustomization} />, {
useRedux: true,
initialState: {
dataMask: {},
nativeFilters: { filters: {} },
},
});
const spinner = await screen.findByTestId('loading-indicator');
expect(spinner).toHaveClass('inline');
expect(spinner).toHaveStyle({ opacity: 0.25, width: '40px' });
});
@@ -645,7 +645,7 @@ const GroupByFilterCard: FC<GroupByFilterCardProps> = ({
{loading && (
<div style={{ textAlign: 'center', marginTop: 8 }}>
<Loading position="inline" />
<Loading position="inline" size="s" muted />
</div>
)}
</div>
@@ -72,3 +72,13 @@ test('omits datasourceType when undefined', () => {
});
expect(target).not.toHaveProperty('datasourceType');
});
test('omits datasourceType when there is no dataset', () => {
// The modal stamps a hidden ``datasourceType`` field on every filter form,
// including dataset-less types. Without a dataset there is nothing for it to
// describe, and emitting it would diverge from the ``{}`` target the import
// and seed paths write.
expect(
buildNativeFilterTarget({ datasourceType: DatasourceType.Table }),
).toEqual({});
});
@@ -33,9 +33,9 @@ export interface TargetFormInputs {
* Build the ``NativeFilterTarget`` carried by a native filter or chart
* customization from its form inputs.
*
* Consolidates what used to live in three places ``filterTransformer``,
* ``customizationTransformer``, and ``createHandleSave`` so changes to the
* target shape only need to happen here.
* Consolidates what used to live in ``filterTransformer`` and
* ``customizationTransformer`` so changes to the target shape only need to
* happen here.
*/
export function buildNativeFilterTarget(
formInputs: TargetFormInputs,
@@ -49,7 +49,11 @@ export function buildNativeFilterTarget(
: formInputs.dataset;
}
if (formInputs.datasourceType) {
// ``datasourceType`` describes the selected dataset, so it only belongs on a
// target that has one. Emitting it for a dataset-less filter (e.g.
// ``filter_time``) would make a UI save serialize a target the import and
// seed paths write as ``{}``.
if (formInputs.dataset != null && formInputs.datasourceType) {
target.datasourceType = formInputs.datasourceType;
}
@@ -0,0 +1,102 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { ChartCustomization, ChartCustomizationType } from '@superset-ui/core';
import { ChartCustomizationsFormItem } from '../types';
import { transformCustomizationForSave } from './customizationTransformer';
const baseFormItem = {
type: ChartCustomizationType.ChartCustomization,
scope: { rootPath: ['ROOT_ID'], excluded: [] },
controlValues: {},
requiredFirst: {},
defaultValue: null,
defaultDataMask: { filterState: {}, extraFormData: {} },
sortMetric: null,
description: '',
// form-only field that must never leak into the saved customization
defaultValueQueriesData: null,
} as unknown as ChartCustomizationsFormItem;
test('serializes a dataset-less customization into a full ChartCustomization', () => {
// Customization plugins declaring ``datasourceCount: 0`` render no dataset
// control, so their form item carries neither ``dataset`` nor ``targets``.
const formItem = {
...baseFormItem,
name: 'Layer visibility',
filterType: 'customization_deckgl_layer_visibility',
} as unknown as ChartCustomizationsFormItem;
const result = transformCustomizationForSave(
'CHART_CUSTOMIZATION-abc',
formItem,
) as ChartCustomization;
expect(result.targets).toEqual([{}]);
expect(result.defaultDataMask).toBeDefined();
expect(result.removed).toBe(false);
expect(result).not.toHaveProperty('defaultValueQueriesData');
});
test('serializes a dataset-backed customization into a full ChartCustomization', () => {
const formItem = {
...baseFormItem,
name: 'Group by',
filterType: 'customization_dynamic_group_by',
dataset: { value: 42, label: 'sales' },
column: 'region',
} as unknown as ChartCustomizationsFormItem;
const result = transformCustomizationForSave(
'CHART_CUSTOMIZATION-def',
formItem,
) as ChartCustomization;
expect(result.targets).toEqual([
{ datasetId: 42, column: { name: 'region' } },
]);
expect(result).not.toHaveProperty('defaultValueQueriesData');
});
test('passes an already-saved ChartCustomization through untouched', () => {
const saved: ChartCustomization = {
id: 'CHART_CUSTOMIZATION-ghi',
name: 'Group by',
filterType: 'customization_dynamic_group_by',
type: ChartCustomizationType.ChartCustomization,
targets: [{ datasetId: 42, column: { name: 'region' } }],
defaultDataMask: { filterState: {}, extraFormData: {} },
controlValues: {},
scope: { rootPath: ['ROOT_ID'], excluded: [] },
description: ' needs trim ',
chartsInScope: [1, 2],
tabsInScope: ['TAB-1'],
};
const result = transformCustomizationForSave(
'CHART_CUSTOMIZATION-ghi',
saved,
) as ChartCustomization;
expect(result.targets).toEqual([
{ datasetId: 42, column: { name: 'region' } },
]);
expect(result.chartsInScope).toEqual([1, 2]);
expect(result.tabsInScope).toEqual(['TAB-1']);
expect(result.description).toBe('needs trim');
});
@@ -69,7 +69,10 @@ function isDividerType(
function isFormInput(
formInputs: ChartCustomizationFormOrSaved,
): formInputs is ChartCustomizationsFormItem {
return 'dataset' in formInputs && typeof formInputs.dataset === 'object';
// Mirrors `filterTransformer`: a saved customization always carries a
// serialized `targets` array, and dataset-less types (e.g. the deck.gl layer
// visibility customization) have no `dataset` to discriminate on.
return !('targets' in formInputs);
}
function transformCustomizationDivider(
@@ -0,0 +1,156 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { DatasourceType, Filter, NativeFilterType } from '@superset-ui/core';
import { NativeFiltersFormItem } from '../types';
import { transformFilterForSave } from './filterTransformer';
const baseFormItem = {
type: NativeFilterType.NativeFilter,
scope: { rootPath: ['ROOT_ID'], excluded: [] },
controlValues: {},
requiredFirst: {},
defaultValue: null,
defaultDataMask: { filterState: {}, extraFormData: {} },
description: '',
// form-only fields that must never leak into the saved filter
defaultValueQueriesData: null,
} as unknown as NativeFiltersFormItem;
test('serializes a dataset-less filter (filter_time) into a full Filter', () => {
// A ``filter_time`` filter has no dataset/column controls, so its form item
// carries neither a ``dataset`` nor a ``targets`` key. It must still be
// transformed like any other native filter rather than persisted verbatim.
const formItem: NativeFiltersFormItem = {
...baseFormItem,
name: 'Time Range',
filterType: 'filter_time',
dependencies: ['NATIVE_FILTER-parent'],
// the modal stamps this on every filter form, dataset or not
datasourceType: DatasourceType.Table,
};
const result = transformFilterForSave(
'NATIVE_FILTER-abc',
formItem,
) as Filter;
// Keys the bug used to strip are present and well-formed. The target matches
// the ``{}`` the import and seed paths write, so one logical filter has one
// serialization regardless of provenance.
expect(result.targets).toEqual([{}]);
expect(result.defaultDataMask).toBeDefined();
expect(result.cascadeParentIds).toEqual(['NATIVE_FILTER-parent']);
// Form-only keys must not leak into the persisted config.
expect(result).not.toHaveProperty('defaultValueQueriesData');
expect(result).not.toHaveProperty('dependencies');
// Empty requiredFirst collapses to undefined instead of the raw form object.
expect(result.requiredFirst).toBeUndefined();
// A dataset-less filter has no sort metric control, so the persisted document
// must not gain a ``sortMetric`` key it never had. Asserted on the serialized
// form because ``undefined`` values survive in the object but not in JSON.
expect(JSON.parse(JSON.stringify(result))).not.toHaveProperty('sortMetric');
expect(result.name).toBe('Time Range');
expect(result.filterType).toBe('filter_time');
});
test('serializes a dataset-backed filter (filter_select) into a full Filter', () => {
const formItem: NativeFiltersFormItem = {
...baseFormItem,
name: 'Region',
filterType: 'filter_select',
dataset: { value: 42, label: 'sales' },
column: 'region',
dependencies: [],
};
const result = transformFilterForSave(
'NATIVE_FILTER-def',
formItem,
) as Filter;
expect(result.targets).toEqual([
{ datasetId: 42, column: { name: 'region' } },
]);
expect(result.defaultDataMask).toBeDefined();
expect(result.cascadeParentIds).toEqual([]);
expect(result).not.toHaveProperty('defaultValueQueriesData');
});
test('passes an already-saved Filter through untouched (aside from trimming)', () => {
// Values coming from the stored filter config map (e.g. cascade-parent
// cleanup) already carry a ``targets`` array and must be preserved as-is.
const savedFilter: Filter = {
id: 'NATIVE_FILTER-ghi',
name: 'Time Range',
filterType: 'filter_time',
type: NativeFilterType.NativeFilter,
targets: [{}],
defaultDataMask: { filterState: {}, extraFormData: {} },
cascadeParentIds: ['NATIVE_FILTER-parent'],
controlValues: {},
scope: { rootPath: ['ROOT_ID'], excluded: [] },
description: ' needs trim ',
chartsInScope: [1, 2],
tabsInScope: ['TAB-1'],
};
const result = transformFilterForSave(
'NATIVE_FILTER-ghi',
savedFilter,
) as Filter;
expect(result.targets).toEqual([{}]);
expect(result.cascadeParentIds).toEqual(['NATIVE_FILTER-parent']);
expect(result.chartsInScope).toEqual([1, 2]);
expect(result.tabsInScope).toEqual(['TAB-1']);
expect(result.description).toBe('needs trim');
});
test('rebuilds a saved filter whose targets were already stripped', () => {
// Dashboards affected by this bug hold ``filter_time`` entries with no
// ``targets``. They no longer match the saved-filter branch, so they take the
// form-item path and are repaired on the next save. ``cascadeParentIds`` is
// read from the form's ``dependencies``, which such an entry does not carry —
// the same write that stripped ``targets`` stripped ``cascadeParentIds`` too.
const strippedFilter = {
id: 'NATIVE_FILTER-jkl',
name: 'Time Range',
filterType: 'filter_time',
type: NativeFilterType.NativeFilter,
scope: { rootPath: ['ROOT_ID'], excluded: [] },
controlValues: { timeShift: false },
description: '',
requiredFirst: { 'NATIVE_FILTER-jkl': true },
defaultValueQueriesData: null,
} as unknown as NativeFiltersFormItem;
const result = transformFilterForSave(
'NATIVE_FILTER-jkl',
strippedFilter,
) as Filter;
expect(result.targets).toEqual([{}]);
expect(result.defaultDataMask).toBeDefined();
expect(result.requiredFirst).toBe(true);
expect(result.cascadeParentIds).toEqual([]);
expect(result).not.toHaveProperty('defaultValueQueriesData');
});
@@ -67,7 +67,10 @@ function isDividerType(
function isFormInput(
formInputs: NativeFilterFormOrSaved,
): formInputs is NativeFiltersFormItem {
return 'dataset' in formInputs;
// A saved filter always carries a serialized `targets` array; a form item
// never does. Keying this off `dataset` misclassified filter types with no
// dataset control (e.g. `filter_time`) as already saved.
return !('targets' in formInputs);
}
function transformDivider(
@@ -115,7 +118,7 @@ function transformFormInput(
adhoc_filters: formInputs.adhoc_filters,
time_range: formInputs.time_range,
granularity_sqla: formInputs.granularity_sqla,
sortMetric: formInputs.sortMetric ?? null,
sortMetric: formInputs.sortMetric,
requiredFirst: formInputs.requiredFirst
? Object.values(formInputs.requiredFirst).find(rf => rf)
: undefined,
@@ -18,21 +18,15 @@
*/
import type { FormInstance } from '@superset-ui/core/components';
import { nanoid } from 'nanoid';
import { getInitialDataMask } from 'src/dataMask/reducer';
import {
FilterConfiguration,
NativeFilterType,
NativeFilterTarget,
Filter,
Divider,
ChartCustomizationType,
ChartCustomizationConfiguration,
ChartCustomization,
ChartCustomizationDivider,
} from '@superset-ui/core';
import { logging } from '@apache-superset/core/utils';
import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
import { buildNativeFilterTarget } from './transformers/buildTarget';
import {
ChartCustomizationsForm,
FilterChangesType,
@@ -101,70 +95,6 @@ export const validateForm = async (
}
};
export const createHandleSave =
(
saveForm: Function,
filterChanges: FilterChangesType,
values: NativeFiltersForm,
filterConfigMap: Record<string, Filter | Divider>,
) =>
async () => {
const transformFilter = (id: string) => {
const formInputs = values.filters?.[id] || filterConfigMap[id];
if (!formInputs) {
return undefined;
}
if (formInputs.type === NativeFilterType.Divider) {
return {
id,
type: NativeFilterType.Divider,
scope: {
rootPath: [DASHBOARD_ROOT_ID],
excluded: [],
},
title: formInputs.title,
description: formInputs.description,
};
}
const target: Partial<NativeFilterTarget> =
buildNativeFilterTarget(formInputs);
return {
id,
adhoc_filters: formInputs.adhoc_filters,
time_range: formInputs.time_range,
controlValues: formInputs.controlValues ?? {},
granularity_sqla: formInputs.granularity_sqla,
...(formInputs.time_grains?.length
? { time_grains: formInputs.time_grains }
: {}),
requiredFirst: Object.values(formInputs.requiredFirst ?? {}).find(
rf => rf,
),
name: formInputs.name,
filterType: formInputs.filterType,
targets: [target],
defaultDataMask: formInputs.defaultDataMask ?? getInitialDataMask(),
cascadeParentIds: formInputs.dependencies || [],
scope: formInputs.scope,
sortMetric: formInputs.sortMetric,
type: formInputs.type,
description: (formInputs.description || '').trim(),
};
};
const transformedModified = filterChanges.modified
.map(transformFilter)
.filter(Boolean);
const newFilterChanges = {
...filterChanges,
modified: transformedModified,
};
await saveForm(newFilterChanges);
};
export const createHandleRemoveItem =
(
setRemovedFilters: (
@@ -68,7 +68,6 @@ export const TableControls = ({
canDownload,
rowLimit,
rowLimitOptions,
effectiveRowLimit,
onRowLimitChange,
onDownloadCSV,
onDownloadXLSX,
@@ -112,18 +111,14 @@ export const TableControls = ({
value={rowLimit}
onChange={onRowLimitChange}
options={rowLimitOptions ?? []}
// Labelled as the applied limit to avoid a second row count next to RowCountLabel.
prefix={t('Limit')}
css={css`
min-width: 160px;
min-width: 110px;
`}
/>
)}
<RowCountLabel
rowcount={rowcount}
limit={effectiveRowLimit ?? rowLimit}
loading={isLoading}
/>
{(!onRowLimitChange || rowcount < (rowLimit ?? Infinity)) && (
<RowCountLabel rowcount={rowcount} loading={isLoading} />
)}
{canDownload && onDownloadCSV && onDownloadXLSX && (
<DownloadDropdown
onDownloadCSV={onDownloadCSV}
@@ -56,7 +56,6 @@ export const SingleQueryResultPane = ({
columnDisplayNames,
rowLimit,
rowLimitOptions,
effectiveRowLimit,
onRowLimitChange,
onDownloadCSV,
onDownloadXLSX,
@@ -87,7 +86,6 @@ export const SingleQueryResultPane = ({
canDownload={canDownload}
rowLimit={rowLimit}
rowLimitOptions={rowLimitOptions}
effectiveRowLimit={effectiveRowLimit}
onRowLimitChange={onRowLimitChange}
onDownloadCSV={onDownloadCSV}
onDownloadXLSX={onDownloadXLSX}
@@ -236,7 +236,6 @@ export const useResultsPane = ({
columnDisplayNames={columnDisplayNames}
rowLimit={rowLimit}
rowLimitOptions={ROW_LIMIT_OPTIONS}
effectiveRowLimit={effectiveRowLimit}
onRowLimitChange={handleRowLimitChange}
/>
</StyledDiv>
@@ -1,83 +0,0 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import { GenericDataType } from '@apache-superset/core/common';
import {
TableControls,
ROW_LIMIT_OPTIONS,
} from '../components/DataTableControls';
import { TableControlsProps } from '../types';
const setup = (overrides: Partial<TableControlsProps> = {}) =>
render(
<TableControls
data={[]}
columnNames={['name']}
columnTypes={[GenericDataType.String]}
rowcount={0}
onInputChange={jest.fn()}
isLoading={false}
canDownload
rowLimit={100}
rowLimitOptions={ROW_LIMIT_OPTIONS}
onRowLimitChange={jest.fn()}
{...overrides}
/>,
{ useRedux: true },
);
test('shows the row count when the result fills the selected row limit', () => {
setup({ rowcount: 100, rowLimit: 100 });
expect(screen.getByTestId('row-count-label')).toHaveTextContent('100 rows');
});
test('warns that the row limit was reached when the result fills it', async () => {
setup({ rowcount: 100, rowLimit: 100 });
userEvent.hover(screen.getByTestId('row-count-label'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(
'The row limit set for the chart was reached',
);
});
test('does not warn when the result is smaller than the selected row limit', () => {
setup({ rowcount: 42, rowLimit: 100 });
expect(screen.getByTestId('row-count-label')).toHaveTextContent('42 rows');
userEvent.hover(screen.getByTestId('row-count-label'));
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
});
test("warns when the chart's own row limit truncates below the selected row limit", async () => {
setup({ rowcount: 250, rowLimit: 1000, effectiveRowLimit: 250 });
userEvent.hover(screen.getByTestId('row-count-label'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(
'The row limit set for the chart was reached',
);
});
test('labels the row limit selector so it is not read as a second row count', () => {
setup({ rowcount: 100, rowLimit: 100 });
expect(screen.getByText('Limit')).toBeInTheDocument();
});
@@ -84,9 +84,6 @@ export interface TableControlsProps extends DrillControlsProps {
canDownload: boolean;
rowLimit?: number;
rowLimitOptions?: { value: number; label: string }[];
// Effective result limit, capped by the chart's row limit.
// Defaults to `rowLimit` and controls the "row limit reached" warning.
effectiveRowLimit?: number;
onRowLimitChange?: (limit: number) => void;
}
@@ -107,6 +104,5 @@ export interface SingleQueryResultPaneProp
columnDisplayNames?: Record<string, string>;
rowLimit?: number;
rowLimitOptions?: { value: number; label: string }[];
effectiveRowLimit?: number;
onRowLimitChange?: (limit: number) => void;
}
@@ -207,6 +207,38 @@ describe('AdhocFilter', () => {
expect(adhocFilter10.isValid()).toBe(true);
});
test('is invalid when a comparator-taking operator has no comparator', () => {
// A comparator that was never set, or that was cleared through the value
// Select's clear affordance, is `undefined` rather than `null` or `[]`.
const adhocFilter1 = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'is_intro',
operator: 'IN',
comparator: undefined,
clause: Clauses.Where,
});
expect(adhocFilter1.isValid()).toBe(false);
const adhocFilter2 = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'is_intro',
operator: '==',
comparator: undefined,
clause: Clauses.Where,
});
expect(adhocFilter2.isValid()).toBe(false);
// `false` is a legitimate boolean comparator, not a missing value
const adhocFilter3 = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'is_intro',
operator: '==',
comparator: false,
clause: Clauses.Where,
});
expect(adhocFilter3.isValid()).toBe(true);
});
test('can translate from simple expressions to sql expressions', () => {
const adhocFilter1 = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
@@ -163,8 +163,10 @@ export default class AdhocFilter {
// A non-empty array of values ('IN' or 'NOT IN' clauses)
return this.comparator.length > 0;
}
// A value has been selected or typed
return this.comparator !== null;
// A value has been selected or typed. An unset comparator is
// `undefined` rather than `null`: picking a new subject resets it, and
// the value Select's clear affordance emits `undefined` too.
return this.comparator != null;
}
}
@@ -181,6 +181,29 @@ describe('AdhocFilterEditPopover', () => {
expect(saveButton).toBeDisabled();
});
test('disables save button when a boolean column has no value selected', async () => {
const booleanColumn = { type: 'BOOL', column_name: 'is_intro' };
renderPopover({
adhocFilter: new AdhocFilter({
expressionType: ExpressionTypes.Simple,
clause: Clauses.Where,
}),
options: [booleanColumn],
datasource: { columns: [booleanColumn], filter_select: false },
});
// Picking the subject resets the comparator to `undefined`; the value
// control is then left untouched, mirroring the reported repro.
await userEvent.click(screen.getByTestId('select-element'));
await userEvent.click(
await screen.findByRole('option', { name: /is_intro/ }),
);
expect(
screen.getByTestId('adhoc-filter-edit-popover-save-button'),
).toBeDisabled();
});
test('initiates resize when resize handle is dragged', async () => {
const onResize = jest.fn();
renderPopover({ onResize });
@@ -27,8 +27,15 @@ import {
Input,
Button,
Modal,
Select,
} from '@superset-ui/core/components';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import copyTextToClipboard from 'src/utils/copy';
import {
API_KEY_SCOPE_OPTIONS,
getApiKeyScopesHelpText,
serializeApiKeyScopes,
} from './apiKeyScopes';
interface ApiKeyCreateModalProps {
show: boolean;
@@ -38,6 +45,7 @@ interface ApiKeyCreateModalProps {
interface FormValues {
name: string;
scopes?: string[];
}
export function ApiKeyCreateModal({
@@ -62,9 +70,13 @@ export function ApiKeyCreateModal({
const handleFormSubmit = async (values: FormValues) => {
try {
const scopes = serializeApiKeyScopes(values.scopes);
const response = await SupersetClient.post({
endpoint: '/api/v1/security/api_keys/',
jsonPayload: values,
jsonPayload: {
name: values.name,
...(scopes && { scopes }),
},
});
const key = response.json?.result?.key;
if (!key) {
@@ -83,7 +95,7 @@ export function ApiKeyCreateModal({
return;
}
try {
await navigator.clipboard.writeText(createdKey);
await copyTextToClipboard(() => Promise.resolve(createdKey));
setCopied(true);
if (copyTimerRef.current) {
clearTimeout(copyTimerRef.current);
@@ -170,6 +182,24 @@ export function ApiKeyCreateModal({
placeholder={t('e.g., CI/CD Pipeline, Analytics Script')}
/>
</FormItem>
<FormItem
name="scopes"
label={t('MCP scopes')}
help={getApiKeyScopesHelpText()}
>
<Select
name="scopes"
mode="multiple"
allowClear
showSearch
options={API_KEY_SCOPE_OPTIONS}
placeholder={t('Select MCP resource scopes (optional)')}
data-test="api-key-scopes-select"
getPopupContainer={(trigger: HTMLElement) =>
trigger.closest<HTMLElement>('.ant-modal-container') ?? trigger
}
/>
</FormItem>
</FormModal>
);
}
@@ -162,6 +162,19 @@ export function ApiKeyList() {
key: 'status',
render: (_: unknown, record: ApiKey) => getStatusBadge(record),
},
{
title: t('MCP scopes'),
dataIndex: 'scopes',
key: 'scopes',
render: (scopes: string | null) =>
scopes ? (
<Tooltip title={scopes}>
<Tag>{t('%s MCP scopes', scopes.split(',').length)}</Tag>
</Tooltip>
) : (
<Tag>{t('RBAC only')}</Tag>
),
},
{
title: t('Actions'),
key: 'actions',
@@ -0,0 +1,50 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import {
API_KEY_SCOPE_OPTIONS,
getApiKeyScopesHelpText,
serializeApiKeyScopes,
} from './apiKeyScopes';
test('offers read and write scopes for every supported resource', () => {
expect(API_KEY_SCOPE_OPTIONS).toHaveLength(32);
expect(API_KEY_SCOPE_OPTIONS).toContainEqual({
label: 'superset:dashboard:read',
value: 'superset:dashboard:read',
});
expect(API_KEY_SCOPE_OPTIONS).toContainEqual({
label: 'superset:sqllab:write',
value: 'superset:sqllab:write',
});
});
test('serializes selected scopes for the FAB API', () => {
expect(
serializeApiKeyScopes(['superset:dashboard:read', 'superset:chart:write']),
).toBe('superset:dashboard:read,superset:chart:write');
expect(serializeApiKeyScopes([])).toBeUndefined();
expect(serializeApiKeyScopes()).toBeUndefined();
});
test('explains that scopes apply to MCP rather than REST APIs', () => {
expect(getApiKeyScopesHelpText()).toContain('MCP resources');
expect(getApiKeyScopesHelpText()).toContain(
'do not restrict REST API requests',
);
});
@@ -0,0 +1,55 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { t } from '@apache-superset/core/translation';
const API_KEY_SCOPE_RESOURCES = [
'annotation',
'chart',
'dashboard',
'database',
'dataset',
'explore',
'query',
'report',
'role',
'rls',
'savedquery',
'sqllab',
'tag',
'task',
'theme',
'user',
] as const;
const API_KEY_SCOPE_ACTIONS = ['read', 'write'] as const;
export const API_KEY_SCOPE_OPTIONS = API_KEY_SCOPE_RESOURCES.flatMap(resource =>
API_KEY_SCOPE_ACTIONS.map(action => {
const value = `superset:${resource}:${action}`;
return { label: value, value };
}),
);
export const serializeApiKeyScopes = (scopes?: string[]) =>
scopes?.length ? scopes.join(',') : undefined;
export const getApiKeyScopesHelpText = () =>
t(
'Limit which MCP resources and actions this key can access. These scopes do not restrict REST API requests and never grant permissions the user does not already have. Leave empty for legacy RBAC-only behavior.',
);
+1 -156
View File
@@ -24,7 +24,7 @@
"@types/lodash-es": "^4.17.12",
"@types/node": "^26.2.0",
"@types/ws": "^8.18.1",
"@typescript-eslint/eslint-plugin": "^8.65.0",
"@typescript-eslint/eslint-plugin": "^8.67.0",
"@typescript-eslint/parser": "^8.67.0",
"eslint": "^10.8.1",
"eslint-config-prettier": "^10.1.8",
@@ -1123,136 +1123,6 @@
"typescript": ">=4.8.4 <6.1.0"
}
},
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/project-service": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.67.0.tgz",
"integrity": "sha512-cvE8c7ulYeXN9fYuszhCeCsbzyVEXuhrRCybnBre7TUmqb5nRmBfQAwCj0O3WJFDeyAZt4VYv51vMCC9LHSdYw==",
"dev": true,
"license": "MIT",
"dependencies": {
"@typescript-eslint/tsconfig-utils": "^8.67.0",
"@typescript-eslint/types": "^8.67.0",
"debug": "^4.4.3"
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
},
"peerDependencies": {
"typescript": ">=4.8.4 <6.1.0"
}
},
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/scope-manager": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.67.0.tgz",
"integrity": "sha512-EgvsleTwS4E+WzzSvem8fAUubLwatMNF1B5hHSLQxcvs7q2dtRhGyujHwLJSYlG41niJ7GP24Aha2+0mb1b2kg==",
"dev": true,
"license": "MIT",
"dependencies": {
"@typescript-eslint/types": "8.67.0",
"@typescript-eslint/visitor-keys": "8.67.0"
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
}
},
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/tsconfig-utils": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.67.0.tgz",
"integrity": "sha512-vV+LUSv5njUWsknE71fqKTlXUva+R76SaeORd6Zojcunk/6DvKFXONU3BrAs2H49mbygUXt6gbYunzwqNwlhdg==",
"dev": true,
"license": "MIT",
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
},
"peerDependencies": {
"typescript": ">=4.8.4 <6.1.0"
}
},
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/types": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.67.0.tgz",
"integrity": "sha512-sBtgslww8nsMYUjhdPBiSyUqSzT8uR6g93A2QXnQC8+cGdjz0CyaOdqHDRJb1AtORbZCNUJBBeFA/tNR2uQmww==",
"dev": true,
"license": "MIT",
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
}
},
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/typescript-estree": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.67.0.tgz",
"integrity": "sha512-EKQBCE9yNlRJYm7jdTW5AhDacDUmSwQb0FAJAmK2EKYrNXIsa2vxcSZx6PvJ/dEdI6lS+Y9W+EXckLj0iPFGcw==",
"dev": true,
"license": "MIT",
"dependencies": {
"@typescript-eslint/project-service": "8.67.0",
"@typescript-eslint/tsconfig-utils": "8.67.0",
"@typescript-eslint/types": "8.67.0",
"@typescript-eslint/visitor-keys": "8.67.0",
"debug": "^4.4.3",
"minimatch": "^10.2.2",
"semver": "^7.7.3",
"tinyglobby": "^0.2.15",
"ts-api-utils": "^2.5.0"
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
},
"peerDependencies": {
"typescript": ">=4.8.4 <6.1.0"
}
},
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/visitor-keys": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.67.0.tgz",
"integrity": "sha512-fkv8dHRDqfGtTHuJeebdrQ7cX6Ad4WAS00rgHh9UGvMycF1mjBfsxry1XsLIFhWZ6Judlh6UdzK+TYlbpCXgnA==",
"dev": true,
"license": "MIT",
"dependencies": {
"@typescript-eslint/types": "8.67.0",
"eslint-visitor-keys": "^5.0.0"
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
}
},
"node_modules/@typescript-eslint/parser/node_modules/eslint-visitor-keys": {
"version": "5.0.1",
"resolved": "https://registry.npmjs.org/eslint-visitor-keys/-/eslint-visitor-keys-5.0.1.tgz",
"integrity": "sha512-tD40eHxA35h0PEIZNeIjkHoDR4YjjJp34biM0mDvplBe//mB+IHCqHDGV7pxF+7MklTvighcCPPZC7ynWyjdTA==",
"dev": true,
"license": "Apache-2.0",
"engines": {
"node": "^20.19.0 || ^22.13.0 || >=24"
},
"funding": {
"url": "https://opencollective.com/eslint"
}
},
"node_modules/@typescript-eslint/project-service": {
"version": "8.67.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.67.0.tgz",
@@ -3364,31 +3234,6 @@
"typescript": ">=4.8.4 <6.1.0"
}
},
"node_modules/typescript-eslint/node_modules/@typescript-eslint/parser": {
"version": "8.66.0",
"resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.66.0.tgz",
"integrity": "sha512-X6ypGChaWYk6PBtUg2BwuTZEFFcHJAtGTVJ9/lCTOufhZ4i9fNolQNnktq+kkMCwMj7V8Svsq7+TxSDslmhE0g==",
"dev": true,
"license": "MIT",
"dependencies": {
"@typescript-eslint/scope-manager": "8.66.0",
"@typescript-eslint/types": "8.66.0",
"@typescript-eslint/typescript-estree": "8.66.0",
"@typescript-eslint/visitor-keys": "8.66.0",
"debug": "^4.4.3"
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
},
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/typescript-eslint"
},
"peerDependencies": {
"eslint": "^8.57.0 || ^9.0.0 || ^10.0.0",
"typescript": ">=4.8.4 <6.1.0"
}
},
"node_modules/undici-types": {
"version": "8.3.0",
"resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz",
+1 -1
View File
@@ -32,7 +32,7 @@
"@types/lodash-es": "^4.17.12",
"@types/node": "^26.2.0",
"@types/ws": "^8.18.1",
"@typescript-eslint/eslint-plugin": "^8.65.0",
"@typescript-eslint/eslint-plugin": "^8.67.0",
"@typescript-eslint/parser": "^8.67.0",
"eslint": "^10.8.1",
"eslint-config-prettier": "^10.1.8",
+2 -1
View File
@@ -418,7 +418,8 @@ class DashboardRestApi(
result:
type: array
items:
type: object
$ref: >-
#/components/schemas/{{self.__class__.__name__}}.get_list
400:
$ref: '#/components/responses/400'
401:
+4 -4
View File
@@ -83,7 +83,7 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
| Databricks (legacy) | 70 | Supported | Partial | Supported | Partial | Partial | Not supported |
| StarRocks | 69 | Supported | Partial | Supported | Partial | Partial | Partial |
| SingleStore | 68 | Supported | Partial | Supported | Not supported | Partial | Not supported |
| ClickHouse Connect (Superset) | 61 | Supported | Partial | Partial | Partial | Partial | Not supported |
| ClickHouse Connect (Superset) | 62 | Supported | Partial | Supported | Partial | Partial | Not supported |
| Google Sheets | 61 | Supported | Partial | Supported | Supported | Partial | Partial |
| Aurora MySQL (Data API) | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
| MariaDB | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
@@ -91,7 +91,7 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
| OceanBase | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
| MotherDuck | 58 | Supported | Partial | Supported | Not supported | Partial | Not supported |
| KustoSQL | 54 | Supported | Partial | Supported | Partial | Partial | Not supported |
| ClickHouse | 51 | Supported | Partial | Partial | Partial | Partial | Not supported |
| ClickHouse | 52 | Supported | Partial | Supported | Partial | Partial | Not supported |
| Databend | 51 | Supported | Partial | Supported | Partial | Partial | Not supported |
| Apache Drill | 50 | Supported | Partial | Supported | Partial | Partial | Partial |
| Apache Druid | 47 | Partial | Partial | Supported | Partial | Partial | Not supported |
@@ -293,8 +293,8 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
| Aurora MySQL (Data API) | True | True | True | True | True | True | True | True |
| Aurora PostgreSQL (Data API) | True | True | True | True | True | True | True | True |
| Azure Synapse | True | True | True | True | True | True | True | True |
| ClickHouse | False | True | True | True | True | True | True | True |
| ClickHouse Connect (Superset) | False | True | True | True | True | True | True | True |
| ClickHouse | True | True | True | True | True | True | True | True |
| ClickHouse Connect (Superset) | True | True | True | True | True | True | True | True |
| CockroachDB | True | True | True | True | True | True | True | True |
| Couchbase | True | True | True | True | False | True | True | True |
| CrateDB | True | True | True | True | True | True | True | True |
+1
View File
@@ -112,6 +112,7 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec):
_time_grain_expressions = {
None: "{col}",
"PT1S": "toStartOfSecond(toDateTime64({col}, 3))",
"PT1M": "toStartOfMinute(toDateTime({col}))",
"PT5M": "toDateTime(intDiv(toUInt32(toDateTime({col})), 300)*300)",
"PT10M": "toDateTime(intDiv(toUInt32(toDateTime({col})), 600)*600)",
+75 -25
View File
@@ -68,6 +68,11 @@ from superset.mcp_service.session_scope import _mcp_session_token
from superset.mcp_service.utils.error_sanitization import (
sanitize_for_log as _sanitize_for_log,
)
from superset.security.api_key_scopes import (
get_resource_scope,
METHOD_PERMISSION_SCOPE_ACTION,
RESOURCE_SCOPE_NAME as RESOURCE_SCOPE_NAME,
)
from superset.security.guest_token import GuestUser
if TYPE_CHECKING:
@@ -126,19 +131,24 @@ class MCPNoAuthSourceError(ValueError):
# is a privileged, write-class operation and therefore requires the write
# scope. When introducing a new method permission, add it here.
_METHOD_TO_REQUIRED_SCOPE = {
"read": "superset:read",
# "get" is the read-class permission FAB registers on its security API
# views (User/Role) — those views have no can_read, so tools targeting
# them declare method_permission_name="get".
"get": "superset:read",
"write": "superset:write",
"delete": "superset:write",
# SQL execution (execute_sql, get_chart_sql) runs arbitrary queries and is
# treated as a write-class privileged operation for scope purposes.
"execute_sql_query": "superset:write",
method: f"superset:{action}"
for method, action in METHOD_PERMISSION_SCOPE_ACTION.items()
}
def _required_resource_scope(
class_permission_name: str, method_permission_name: str
) -> str | None:
"""Compute the ``superset:<resource>:<action>`` scope string for a tool.
Returns None if either the resource or the action isn't mapped — callers
must treat that as "no per-resource scope available," not as a grant;
the flat ``_METHOD_TO_REQUIRED_SCOPE`` fallback still applies in that case
(see ``_token_scope_allows``).
"""
return get_resource_scope(class_permission_name, method_permission_name)
def _get_token_scopes() -> set[str] | None:
"""Return the set of scopes on the current JWT access token, or None.
@@ -154,8 +164,13 @@ def _get_token_scopes() -> set[str] | None:
try:
access_token = get_access_token()
except Exception: # noqa: BLE001 - no JWT context for this request
return None
except Exception: # noqa: BLE001 - fail closed on token-context errors
logger.exception("Unable to resolve MCP access-token scopes")
# ``None`` means that no scoped credential was presented and enables
# legacy RBAC-only behavior. An empty set instead makes every scope
# check fail, so an unexpected context error cannot erase restrictions
# carried by a credential.
return set()
if access_token is None:
return None
@@ -167,12 +182,21 @@ def _get_token_scopes() -> set[str] | None:
return {str(s) for s in scopes}
def _token_scope_allows(method_permission_name: str) -> bool:
def _token_scope_allows(
method_permission_name: str, class_permission_name: str | None = None
) -> bool:
"""Return whether the current token's scopes permit the given method.
Back-compat: returns True (allow) when the token carries no scopes or there
is no JWT context, so deployments not using scopes keep RBAC-only behavior.
Only when the token advertises scopes is the mapped required scope enforced.
The per-resource scope (``superset:<resource>:<action>``, derived via
``_required_resource_scope``) is an ALTERNATIVE grant path alongside the
flat method scope: a token carrying either the flat scope
(e.g. ``superset:read``) or the matching per-resource scope
(e.g. ``superset:dashboard:read``) is allowed, so already-issued
flat-scoped tokens keep working unchanged.
"""
token_scopes = _get_token_scopes()
if token_scopes is None:
@@ -190,7 +214,15 @@ def _token_scope_allows(method_permission_name: str) -> bool:
method_permission_name,
)
return False
return required_scope in token_scopes
if required_scope in token_scopes:
return True
if class_permission_name is not None:
resource_scope = _required_resource_scope(
class_permission_name, method_permission_name
)
if resource_scope is not None and resource_scope in token_scopes:
return True
return False
class MCPPermissionDeniedError(PermissionError):
@@ -234,12 +266,20 @@ def _log_scope_denial(
cyclomatic complexity in check.
"""
required_scope = _METHOD_TO_REQUIRED_SCOPE.get(method_permission_name)
resource_scope = _required_resource_scope(
class_permission_name, method_permission_name
)
scope_desc = (
resource_scope
or required_scope
or f"unmapped method permission '{method_permission_name}'"
)
if log_denial:
logger.warning(
"Scope denied for user %s: token lacks required scope "
"'%s' for %s on %s (tool: %s)",
_sanitize_for_log(g.user.username),
required_scope,
scope_desc,
permission_str,
class_permission_name,
func.__name__,
@@ -248,7 +288,7 @@ def _log_scope_denial(
logger.debug(
"Tool hidden for user %s: token lacks required scope '%s' (tool: %s)",
_sanitize_for_log(g.user.username),
required_scope,
scope_desc,
func.__name__,
)
@@ -354,8 +394,13 @@ def check_tool_permission( # noqa: C901
)
return False
method_permission_name = getattr(func, METHOD_PERMISSION_ATTR, "read")
class_permission_name = getattr(func, CLASS_PERMISSION_ATTR, None)
# Token capabilities and user RBAC are independent restrictions.
# Disabling RBAC must not discard scopes explicitly carried by a key.
if not current_app.config.get("MCP_RBAC_ENABLED", True):
return True
return _token_scope_allows(method_permission_name, class_permission_name)
if not hasattr(g, "user") or not g.user:
if log_denial:
@@ -368,7 +413,6 @@ def check_tool_permission( # noqa: C901
)
return False
class_permission_name = getattr(func, CLASS_PERMISSION_ATTR, None)
if not class_permission_name:
# No RBAC configured for this tool; allow by default. This is a
# supported configuration (a protected tool may intentionally
@@ -382,9 +426,17 @@ def check_tool_permission( # noqa: C901
"class_permission_name; allowing access without an RBAC check",
func.__name__,
)
if not _token_scope_allows(method_permission_name):
if log_denial:
logger.warning(
"Scope denied for permission-less tool %s: token lacks "
"flat scope for method %s",
func.__name__,
method_permission_name,
)
return False
return True
method_permission_name = getattr(func, METHOD_PERMISSION_ATTR, "read")
permission_str = f"{PERMISSION_PREFIX}{method_permission_name}"
has_permission = security_manager.can_access(
@@ -399,7 +451,9 @@ def check_tool_permission( # noqa: C901
# advertises scopes. Tokens/deployments that don't use scopes (API keys,
# scope-less JWTs, dev-mode) fall through to RBAC-only behavior — see
# ``_token_scope_allows``.
if has_permission and not _token_scope_allows(method_permission_name):
if has_permission and not _token_scope_allows(
method_permission_name, class_permission_name
):
_log_scope_denial(
func,
method_permission_name,
@@ -462,7 +516,7 @@ def is_tool_visible_to_current_user(tool: Any) -> bool:
return False
if not current_app.config.get("MCP_RBAC_ENABLED", True):
return True
return check_tool_permission(tool_func, log_denial=False)
from superset.mcp_service.privacy import (
tool_requires_data_model_metadata_access,
@@ -475,10 +529,6 @@ def is_tool_visible_to_current_user(tool: Any) -> bool:
):
return False
class_permission_name = getattr(tool_func, CLASS_PERMISSION_ATTR, None)
if not class_permission_name:
return True
return check_tool_permission(tool_func, log_denial=False)
except (AttributeError, RuntimeError, ValueError):
@@ -113,15 +113,19 @@ class CompositeTokenVerifier(TokenVerifier):
)
self._api_key_prefixes = tuple(valid)
def _validate_api_key_sync(self, token: str) -> str | None:
"""Validate an API key against FAB and return the user's username.
def _validate_api_key_sync(self, token: str) -> tuple[str, list[str]] | None:
"""Validate an API key against FAB and return (username, scopes).
Runs synchronously inside a thread executor. Pushes a fresh Flask
app context so that FAB's SecurityManager can access the database.
Returns the username on success, or ``None`` if the key is invalid,
FAB does not support ``validate_api_key``, or an unexpected error
occurs (fail closed).
``scopes`` is the key's own ``ApiKey.scopes`` column, parsed from
FAB's comma-separated string storage format into a list (empty list
if the key has no scopes set, matching the "no scopes advertised"
convention used elsewhere in this module and in ``auth.py``).
Returns ``None`` if the key is invalid, FAB does not support
``validate_api_key``, or an unexpected error occurs (fail closed).
"""
if self._app is None:
return None
@@ -135,12 +139,21 @@ class CompositeTokenVerifier(TokenVerifier):
)
return None
user = sm.validate_api_key(token)
username = user.username if user else None
# Unbind the local reference so this frame no longer points at
# the raw token (defense-in-depth). Python does not zero the
# underlying string memory on rebind.
token = "" # noqa: S105
return username
if user is None:
return None
username = user.username
scopes_str = (
sm.get_api_key_scopes(token)
if hasattr(sm, "get_api_key_scopes")
else None
)
scopes = (
[s.strip() for s in scopes_str.split(",") if s.strip()]
if scopes_str
else []
)
token = "" # noqa: S105 -- unbind raw token, defense-in-depth
return username, scopes
except Exception: # noqa: BLE001 — catch-all: DB errors, FAB internals, etc.
logger.warning(
"API key transport validation failed unexpectedly; rejecting token",
@@ -168,21 +181,25 @@ class CompositeTokenVerifier(TokenVerifier):
if any(token.startswith(prefix) for prefix in self._api_key_prefixes):
if self._app is not None:
loop = asyncio.get_running_loop()
username = await loop.run_in_executor(
result = await loop.run_in_executor(
None, self._validate_api_key_sync, token
)
if username is None:
if result is None:
logger.debug(
"API key rejected at transport layer (invalid or expired)"
)
return None
username, key_scopes = result
logger.debug(
"API key validated at transport layer for user=%s", username
)
return AccessToken(
token=token,
client_id="api_key",
scopes=list(self.required_scopes or []),
# Preserve the key's own scopes exactly. An empty list
# means "no scopes advertised" and therefore retains the
# RBAC-only behavior for existing unscoped API keys.
scopes=key_scopes,
claims={
API_KEY_PASSTHROUGH_CLAIM: True,
API_KEY_VALIDATED_USERNAME_CLAIM: username,
@@ -190,10 +207,11 @@ class CompositeTokenVerifier(TokenVerifier):
)
# No app configured: fall back to prefix-only pass-through so
# ``_resolve_user_from_api_key`` handles DB validation.
# NOTE: ``MCP_REQUIRED_SCOPES`` is intentionally not enforced for
# API-key auth — FAB API keys do not carry scopes. Authorization is
# enforced downstream via ``check_tool_permission`` (RBAC).
# ``_resolve_user_from_api_key`` handles DB validation. Without an
# app there is no DB access here, so the key's own ApiKey.scopes
# cannot be read — the verifier-global required_scopes are used
# instead. Authorization is still enforced downstream via
# ``check_tool_permission`` (RBAC).
logger.debug("API key token detected (prefix match), passing through")
return AccessToken(
token=token,
+3 -4
View File
@@ -653,10 +653,9 @@ def _build_composite_verifier(
if api_key_enabled:
if required_scopes := app.config.get("MCP_REQUIRED_SCOPES", []):
logger.warning(
"MCP_REQUIRED_SCOPES is configured but API key tokens bypass "
"scope enforcement. API key holders gain access regardless of "
"MCP_REQUIRED_SCOPES=%r. Enforce per-key authorization via FAB "
"roles/RBAC instead.",
"MCP_REQUIRED_SCOPES=%r is configured, but API key tokens use "
"the scopes stored on each key instead. Unscoped API keys "
"retain legacy RBAC-only behavior.",
required_scopes,
)
raw_prefixes: str | Sequence[str] = app.config.get(
@@ -30,7 +30,7 @@ from fastmcp import Context
from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.extensions import event_logger
from superset.mcp_service.auth import MCPPermissionDeniedError
from superset.mcp_service.auth import _token_scope_allows, MCPPermissionDeniedError
from superset.mcp_service.common.schema_discovery import (
CHART_DEFAULT_COLUMNS,
CHART_SEARCH_COLUMNS,
@@ -235,9 +235,10 @@ async def get_schema(
from superset import security_manager
if current_app.config.get("MCP_RBAC_ENABLED", True) and not (
security_manager.can_access("can_read", class_permission)
):
rbac_allows = not current_app.config.get(
"MCP_RBAC_ENABLED", True
) or security_manager.can_access("can_read", class_permission)
if not (rbac_allows and _token_scope_allows("read", class_permission)):
user_str = getattr(getattr(g, "user", None), "username", None)
logger.warning(
"get_schema RBAC denied: user=%s type=%s view=%s",
+77
View File
@@ -0,0 +1,77 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Canonical resource and action mappings for scoped API keys."""
# Map FAB method permissions used by MCP tools to the coarser actions supported
# by API-key scopes. Keep this explicit so an unknown permission fails closed.
METHOD_PERMISSION_SCOPE_ACTION: dict[str, str] = {
"read": "read",
"get": "read",
"write": "write",
"update": "write",
"delete": "write",
"execute_sql_query": "write",
}
# Map MCP/FAB class permission names to stable public resource slugs. These
# cannot be derived by lowercasing because several names contain spaces or use
# public spellings that differ from their internal class names.
RESOURCE_SCOPE_NAME: dict[str, str] = {
"Annotation": "annotation",
"Chart": "chart",
"Dashboard": "dashboard",
"Database": "database",
"Dataset": "dataset",
"Explore": "explore",
"Query": "query",
"ReportSchedule": "report",
"Role": "role",
"Row Level Security": "rls",
"SavedQuery": "savedquery",
"SQLLab": "sqllab",
"Tag": "tag",
"Task": "task",
"Theme": "theme",
"User": "user",
}
RESOURCE_SCOPE_CLASS: dict[str, str] = {
resource: class_name for class_name, resource in RESOURCE_SCOPE_NAME.items()
}
RESOURCE_SCOPE_ACTIONS: frozenset[str] = frozenset(
METHOD_PERMISSION_SCOPE_ACTION.values()
)
SCOPE_ACTION_METHOD_PERMISSIONS: dict[str, tuple[str, ...]] = {
action: tuple(
method
for method, mapped_action in METHOD_PERMISSION_SCOPE_ACTION.items()
if mapped_action == action
)
for action in RESOURCE_SCOPE_ACTIONS
}
def get_resource_scope(
class_permission_name: str, method_permission_name: str
) -> str | None:
"""Return the resource scope required by a FAB class/method permission."""
resource = RESOURCE_SCOPE_NAME.get(class_permission_name)
action = METHOD_PERMISSION_SCOPE_ACTION.get(method_permission_name)
if resource is None or action is None:
return None
return f"superset:{resource}:{action}"
+213 -22
View File
@@ -17,6 +17,7 @@
# pylint: disable=too-many-lines
"""A set of constants and methods to manage permissions and security"""
import datetime
import logging
import re
import time
@@ -36,7 +37,7 @@ from urllib.parse import quote
from flask import current_app, Flask, g, has_app_context, Request, Response
from flask_appbuilder import Model
from flask_appbuilder.api import expose, protect, safe
from flask_appbuilder.api import expose, permission_name, protect, safe
from flask_appbuilder.models.filters import BaseFilter
from flask_appbuilder.security.manager import AUTH_REMOTE_USER
from flask_appbuilder.security.sqla.apis import GroupApi, RoleApi, UserApi
@@ -394,8 +395,11 @@ class SupersetUserApi(UserApi):
"""
Overriding the UserApi to sync Subject rows, filter excluded users,
handle deletion constraints, and add audit logging.
UserApi has custom post/put that bypass hooks, so we override them
and sync after the parent method succeeds.
The Subject sync happens in ``pre_add``/``pre_update``, which FAB calls
*before* the commit that ``self.datamodel.add``/``edit`` issues -- so the
sync rides that same commit rather than needing one of its own after the
fact.
"""
base_filters = [["username", ExcludeUsersFilter, lambda: []]]
@@ -415,6 +419,45 @@ class SupersetUserApi(UserApi):
"changed_on",
]
def pre_add(self, item: Model) -> None:
"""Hash the password (FAB's own ``pre_add``), then sync the user's
``Subject`` row before FAB's own commit.
``UserApi.post`` calls ``pre_add`` *before* ``self.datamodel.add``,
which is what actually issues the commit -- so flushing the new user
here (to obtain its id) and syncing its ``Subject`` row alongside it
means both writes ride the same transaction and commit together,
instead of the subject sync needing a second, separate commit after
the fact.
"""
super().pre_add(item)
from superset.daos.user import UserDAO
self.datamodel.session.add(item)
self.datamodel.session.flush()
UserDAO._sync_subject(item)
def pre_update(self, item: Model, data: dict[str, Any]) -> None:
"""Same reasoning as ``pre_add``: ``UserApi.put`` calls ``pre_update``
before ``self.datamodel.edit`` commits, so the subject sync lands in
that same transaction.
"""
super().pre_update(item, data)
from superset.daos.user import UserDAO
UserDAO._sync_subject(item)
if data.get("password"):
# An admin-initiated password change via this endpoint must
# invalidate the target account's other outstanding sessions,
# the same as the self-service ``/me/`` path and the two
# password-reset views.
from superset.security.session_invalidation import (
invalidate_sessions_for_user,
)
invalidate_sessions_for_user(item.id)
@expose("/", methods=["POST"])
@protect()
@safe
@@ -430,17 +473,7 @@ class SupersetUserApi(UserApi):
500:
description: Server error
"""
response = super().post()
if response.status_code == 201:
from superset.daos.user import UserDAO
user_id = response.json.get("id")
if user_id:
user = self.datamodel.session.get(self.datamodel.obj, user_id)
if user:
UserDAO._sync_subject(user)
self.datamodel.session.commit() # pylint: disable=consider-using-transaction
return response
return super().post()
@expose("/<pk>", methods=["PUT"])
@protect()
@@ -464,15 +497,42 @@ class SupersetUserApi(UserApi):
500:
description: Server error
"""
response = super().put(pk)
if response.status_code == 200:
from superset.daos.user import UserDAO
return super().put(pk)
user = self.datamodel.get(pk, self._base_filters)
if user:
UserDAO._sync_subject(user)
self.datamodel.session.commit() # pylint: disable=consider-using-transaction
return response
@expose("/<int:pk>/sessions", methods=["DELETE"])
@protect()
@permission_name("put")
@safe
def terminate_sessions(self, pk: int) -> Response:
"""Terminate a user's outstanding sessions without disabling their account.
---
delete:
parameters:
- in: path
name: pk
schema:
type: integer
responses:
200:
description: Sessions terminated
404:
$ref: '#/components/responses/404'
500:
$ref: '#/components/responses/500'
"""
from superset.security.session_invalidation import invalidate_sessions_for_user
user = self.datamodel.get(pk, self._base_filters)
if not user:
return self.response_404()
invalidate_sessions_for_user(user.id)
self.datamodel.session.commit() # pylint: disable=consider-using-transaction
_log_audit_event(
"UserSessionsTerminated",
{"target_username": user.username, "target_user_id": user.id},
)
return self.response(200, message="User sessions terminated.")
def pre_delete(self, item: Model) -> None:
from superset.daos.user import UserDAO
@@ -1565,9 +1625,23 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
bypassed. We distinguish the two by comparing the acting user
(``g.user``) against the target ``userid``: they match for a
self-service reset and differ for an admin reset.
Also stamps the session-invalidation epoch for the target user, so
any session for the account that predates this reset stops working --
regardless of which of the two paths triggered it.
"""
super().reset_password(userid, password)
# pylint: disable=import-outside-toplevel
from superset import db
from superset.security.session_invalidation import invalidate_sessions_for_user
invalidate_sessions_for_user(int(userid))
# ``super().reset_password`` (FAB's ``update_user``) already committed
# its own change in a separate transaction, so the epoch stamp above
# needs its own commit too, rather than riding an existing one.
db.session.commit() # pylint: disable=consider-using-transaction
acting_user = getattr(g, "user", None)
acting_user_id = getattr(acting_user, "id", None)
# ``userid`` arrives as a string (the ``pk`` request arg) on the admin
@@ -4945,6 +5019,123 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
raw_token, secret, algorithms=[algo], audience=audience
)
def get_api_key_scopes(self, api_key_string: str) -> Optional[str]:
"""Return the ``scopes`` value for a validated API key.
FAB's ``validate_api_key`` resolves the matching ``ApiKey`` row
internally (by lookup hash) but only returns the associated
``User`` the row's ``scopes`` column is otherwise unreachable by
callers. This repeats the same cheap, indexed lookup so MCP's
``CompositeTokenVerifier`` can propagate per-key scopes instead of
silently falling back to verifier-global scopes. Call only after
``validate_api_key`` has already succeeded for this token this
method does not itself verify the key hash or active status.
"""
lookup = self._compute_lookup_hash(api_key_string) # type: ignore[attr-defined]
api_key = (
self.session.query(self.api_key_model) # type: ignore[attr-defined]
.filter(self.api_key_model.lookup_hash == lookup)
.one_or_none()
)
return api_key.scopes if api_key else None
def _validate_requested_api_key_scopes(
self, user: Any, scopes: Optional[str]
) -> None:
"""Raise if ``scopes`` would grant a user more than their own RBAC.
Enforces the "intersection, never broader" rule confirmed for this
feature: a user must never be able to mint a token scoped beyond
what their own role already permits, even if they hand-author the
scopes string themselves at issuance time.
Per-resource scopes (``superset:<resource>:<action>``) are checked
against the user's actual ``can_<method>`` RBAC grant for that
resource. Flat scopes (``superset:read``/``superset:write``, the
pre-per-resource form) can only be self-issued by Admins a flat
scope grants a method across every resource, and there's no single
RBAC check that soundly proves a non-Admin has that for "every
resource," so it's rejected for anyone else rather than guessed at.
Unrecognized scope strings are rejected outright (fail closed).
NOTE: this only prevents the request from being honored; it does
not (yet) produce a clean 400 response, since FAB's ``ApiKeyApi``
has no validation hook this can plug into without replacing the API
registration entirely. Raising here surfaces as a 500 via FAB's
``@safe`` decorator until that's addressed — tracked as a known
follow-up, not silently accepted.
"""
if not scopes:
return
# pylint: disable-next=import-outside-toplevel
from superset.security.api_key_scopes import (
RESOURCE_SCOPE_ACTIONS,
RESOURCE_SCOPE_CLASS,
SCOPE_ACTION_METHOD_PERMISSIONS,
)
admin_role_name = get_conf()["AUTH_ROLE_ADMIN"]
is_admin = any(
role.name == admin_role_name for role in getattr(user, "roles", [])
)
for raw_scope in scopes.split(","):
scope = raw_scope.strip()
if not scope:
continue
parts = scope.split(":")
if len(parts) == 3 and parts[0] == "superset":
_, resource_slug, action = parts
class_permission_name = RESOURCE_SCOPE_CLASS.get(resource_slug)
if class_permission_name is None:
raise ValueError(
f"Requested scope '{scope}' names an unrecognized "
f"resource '{resource_slug}'"
)
if action not in RESOURCE_SCOPE_ACTIONS:
raise ValueError(
f"Requested scope '{scope}' names an unrecognized "
f"action '{action}'"
)
if any(
self._has_view_access(user, f"can_{method}", class_permission_name)
for method in SCOPE_ACTION_METHOD_PERMISSIONS[action]
):
continue
raise ValueError(
f"Requested scope '{scope}' exceeds the issuing user's "
"own permissions"
)
if (
len(parts) == 2
and parts[0] == "superset"
and parts[1] in RESOURCE_SCOPE_ACTIONS
and is_admin
):
continue
raise ValueError(
f"Requested scope '{scope}' is not a recognized "
"superset:<resource>:<action> scope, or requires Admin to "
"self-issue as a flat scope"
)
def create_api_key(
self,
user: Any,
name: str,
scopes: Optional[str] = None,
expires_on: Optional[datetime.datetime] = None,
) -> Optional[dict[str, Any]]:
"""Create a new API key, enforcing the scope-intersection rule.
Thin wrapper around FAB's ``SecurityManager.create_api_key`` — see
``_validate_requested_api_key_scopes`` for the actual check. FAB's
base implementation is otherwise unchanged.
"""
self._validate_requested_api_key_scopes(user, scopes)
return super().create_api_key( # type: ignore[misc]
user=user, name=name, scopes=scopes, expires_on=expires_on
)
@staticmethod
def is_guest_user(user: Optional[Any] = None) -> bool:
# pylint: disable=import-outside-toplevel
+29 -1
View File
@@ -41,7 +41,7 @@ from typing import Any, Optional
from flask import flash, session
from flask_babel import gettext as __
from flask_login import current_user, logout_user
from sqlalchemy import event, inspect
from sqlalchemy import event, inspect, or_
from sqlalchemy.exc import IntegrityError
from werkzeug.wrappers import Response
@@ -163,9 +163,20 @@ def invalidate_user_sessions(connection: Any, user_id: int) -> None:
)
def _stamp_existing() -> int:
# Guard against two concurrent writers regressing the epoch: a
# transaction that computed an earlier ``now`` can reach this UPDATE
# after one with a later ``now`` has already committed. Only apply
# the write when it would advance (or initialize) the stored value,
# so the epoch is monotonic regardless of commit order.
return connection.execute(
table.update()
.where(table.c.user_id == user_id)
.where(
or_(
table.c.sessions_invalidated_at.is_(None),
table.c.sessions_invalidated_at < now,
)
)
.values(sessions_invalidated_at=now, changed_on=now)
).rowcount
@@ -187,6 +198,23 @@ def invalidate_user_sessions(connection: Any, user_id: int) -> None:
_stamp_existing()
def invalidate_sessions_for_user(user_id: int) -> None:
"""Stamp the invalidation epoch for ``user_id`` from ordinary application code.
Convenience wrapper around ``invalidate_user_sessions`` for callers that
don't have the raw ``Connection`` the ``after_update`` event listener
receives -- e.g. a password-change flow. The stamp is written through the
current session's own connection, so it participates in whatever
transaction the caller's other pending changes belong to; it is not
committed here, so the caller's own commit (or the next flush that
triggers one) is what makes it durable.
"""
# pylint: disable=import-outside-toplevel
from superset.extensions import db
invalidate_user_sessions(db.session.connection(), user_id)
def _stamp_epoch_on_disable(_mapper: Any, connection: Any, target: Any) -> None:
history = inspect(target).attrs.active.history
# Only act when ``active`` actually changed to False — ignore the
+15 -2
View File
@@ -27,8 +27,9 @@ class CustomTagsOptimizationMixin:
When enabled via config, this mixin:
1. Configures list_columns to use custom_tags (filtered relationship)
2. Rewrites frontend requests from 'tags.*' to 'custom_tags.*'
3. Transforms responses to rename 'custom_tags' back to 'tags'
2. Exposes custom_tags as tags in the response schema
3. Rewrites frontend requests from 'tags.*' to 'custom_tags.*'
4. Transforms responses to rename 'custom_tags' back to 'tags'
This provides SQL query optimization (97% reduction) while maintaining
frontend compatibility.
@@ -62,6 +63,18 @@ class CustomTagsOptimizationMixin:
self._custom_tags_only = current_app.config.get(config_key, False)
self.list_columns = custom_columns if self._custom_tags_only else full_columns
def _init_model_schemas(self) -> None:
"""Keep the optimized relationship's public schema name stable."""
super()._init_model_schemas() # type: ignore[misc]
list_model_schema = getattr(self, "list_model_schema", None)
if (
self._custom_tags_only
and list_model_schema
and "custom_tags" in list_model_schema.fields
):
list_model_schema.fields["custom_tags"].data_key = "tags"
def get_list(self, **kwargs: Any) -> Response:
"""Override to rewrite request parameters for custom_tags optimization.
+8 -2
View File
@@ -28,7 +28,11 @@ from superset.common.query_context_factory import QueryContextFactory
from superset.common.utils.query_cache_manager import QueryCacheManager
from superset.constants import CacheRegion
from superset.daos.datasource import DatasourceDAO
from superset.utils.core import extract_dataframe_dtypes, QueryStatus
from superset.utils.core import (
apply_max_row_limit,
extract_dataframe_dtypes,
QueryStatus,
)
from superset.views.datasource.schemas import SamplesPayloadSchema
if TYPE_CHECKING:
@@ -45,9 +49,11 @@ def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str,
if isinstance(page, int) and isinstance(per_page, int):
limit = int(per_page)
if limit < 0 or limit > samples_row_limit:
if limit < 0:
# reset limit value if input is invalid
limit = samples_row_limit
elif limit:
limit = apply_max_row_limit(limit)
offset = max((int(page) - 1) * limit, 0)
+36 -2
View File
@@ -23,11 +23,12 @@ from flask_appbuilder.security.decorators import protect
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from sqlalchemy.orm.exc import NoResultFound
from werkzeug.security import generate_password_hash
from werkzeug.security import check_password_hash, generate_password_hash
from superset import is_feature_enabled
from superset.daos.user import UserDAO
from superset.extensions import db, event_logger
from superset.security.session_invalidation import invalidate_sessions_for_user
from superset.utils.slack import get_user_avatar, SlackClientError
from superset.views.base_api import BaseSupersetApi, requires_json, statsd_metrics
from superset.views.users.schemas import CurrentUserPutSchema, UserResponseSchema
@@ -49,12 +50,45 @@ class CurrentUserRestApi(BaseSupersetApi):
def pre_update(self, item: User, data: Dict[str, Any]) -> None:
item.changed_on = datetime.now()
item.changed_by_fk = g.user.id
# Pop unconditionally: this key is only meaningful for verifying a
# password change below, and it isn't a real column on the user
# model -- it must never reach ``UserDAO.update``'s ``setattr`` loop.
current_password = data.pop("current_password", None)
if "password" in data and data["password"]:
# An account with no password set yet (e.g. provisioned via an
# external auth backend) has nothing to prove knowledge of; for
# every other account, the caller must confirm the existing
# password before it can be replaced.
proof_ok = (
item.password
and current_password
and check_password_hash(item.password, current_password)
)
if item.password and not proof_ok:
raise ValidationError(
{"current_password": ["Incorrect current password."]}
)
# Compute and assign the hash, then drop the plaintext from
# ``data`` -- it is passed to ``UserDAO.update`` as ``attributes``
# right after this, and ``BaseDAO.update`` sets every key in it
# via ``setattr``. Leaving the plaintext in would overwrite the
# hash just assigned below with the raw value.
new_password = data.pop("password")
item.password = generate_password_hash(
password=data["password"],
password=new_password,
method=app.config.get("FAB_PASSWORD_HASH_METHOD", "scrypt"),
salt_length=app.config.get("FAB_PASSWORD_HASH_SALT_LENGTH", 16),
)
# A changed password invalidates any other outstanding session
# for this account.
invalidate_sessions_for_user(item.id)
elif "password" in data:
# A falsy value (e.g. an empty string, which the complexity
# validator lets through when password complexity is disabled)
# skips the block above, but the key must still never reach
# ``UserDAO.update``'s ``setattr`` loop -- it would blank out
# the account's stored hash.
data.pop("password")
@expose("/", methods=("GET",))
@protect()
+27 -1
View File
@@ -14,17 +14,22 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from typing import Any
from flask_appbuilder.security.sqla.apis.user.schema import User
from flask_appbuilder.security.sqla.apis.user.validator import (
PasswordComplexityValidator,
)
from marshmallow import fields, Schema
from marshmallow import fields, Schema, validates_schema, ValidationError
from marshmallow.fields import Boolean, Integer, String
from marshmallow.validate import Length
first_name_description = "The current user's first name"
last_name_description = "The current user's last name"
password_description = "The current user's password for authentication" # noqa: S105
# Required, and verified against the account's existing password, whenever
# ``password`` is included in the payload.
current_password_description = "The current user's existing password" # noqa: S105
class UserGroupSchema(Schema):
@@ -64,3 +69,24 @@ class CurrentUserPutSchema(Schema):
validate=[PasswordComplexityValidator()],
metadata={"description": password_description},
)
current_password = fields.String(
required=False,
load_only=True,
metadata={"description": current_password_description},
)
@validates_schema
def validate_current_password_required_with_password(
self, data: dict[str, Any], **kwargs: object
) -> None:
"""Require ``current_password`` whenever ``password`` is being set.
This only checks that the field was supplied -- whether it actually
matches the account's existing password is verified against the
database in ``CurrentUserRestApi.pre_update``, which has access to
the user record this schema doesn't.
"""
if data.get("password") and not data.get("current_password"):
raise ValidationError(
{"current_password": ["This field is required to change the password."]}
)
+52
View File
@@ -16,6 +16,7 @@
# under the License.
"""Unit tests for Superset"""
from pathlib import Path
from unittest.mock import patch
import rison
@@ -42,6 +43,57 @@ class TestOpenApiSpec(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
validate(response)
def test_dashboard_list_uses_generated_response_schema(self):
"""Keep the generated dashboard list contract aligned with published docs.
If an intentional list schema change breaks this test, regenerate
``docs/static/resources/openapi.json`` with ``superset update-api-docs``
under the production-default configuration, then review the focused
dashboard diff.
"""
self.login(ADMIN_USERNAME)
rv = self.client.get("api/v1/_openapi")
assert rv.status_code == 200
generated_spec = json.loads(rv.data.decode("utf-8"))
published_spec_path = (
Path(__file__).parents[2] / "docs" / "static" / "resources" / "openapi.json"
)
published_spec = json.loads(published_spec_path.read_text(encoding="utf-8"))
generated_result_items = generated_spec["paths"]["/api/v1/dashboard/"]["get"][
"responses"
]["200"]["content"]["application/json"]["schema"]["properties"]["result"][
"items"
]
published_result_items = published_spec["paths"]["/api/v1/dashboard/"]["get"][
"responses"
]["200"]["content"]["application/json"]["schema"]["properties"]["result"][
"items"
]
assert (
generated_result_items
== published_result_items
== {"$ref": "#/components/schemas/DashboardRestApi.get_list"}
)
schema_prefix = "DashboardRestApi.get_list"
generated_schemas = {
name: schema
for name, schema in generated_spec["components"]["schemas"].items()
if name == schema_prefix or name.startswith(f"{schema_prefix}.")
}
published_schemas = {
name: schema
for name, schema in published_spec["components"]["schemas"].items()
if name == schema_prefix or name.startswith(f"{schema_prefix}.")
}
assert generated_schemas == published_schemas, (
"Dashboard list OpenAPI components changed; regenerate the published spec"
)
def test_info_endpoint(self):
"""
API: Test info endpoint
@@ -18,7 +18,6 @@
# isort:skip_file
"""Unit tests for Superset"""
from datetime import datetime
from io import BytesIO
from typing import Optional
from unittest.mock import Mock, patch
@@ -606,7 +605,10 @@ class TestSavedQueryApi(SupersetTestCase):
db.session.query(SavedQuery).filter(SavedQuery.label == "label1").all()[0]
)
self.login(ADMIN_USERNAME)
with freeze_time(datetime.now()):
# Freeze relative to the persisted timestamp so database-specific
# timestamp precision cannot make the humanized value age into the
# next bucket while the request is being handled.
with freeze_time(saved_query.changed_on):
uri = f"api/v1/saved_query/{saved_query.id}"
rv = self.get_assert_metric(uri, "get")
assert rv.status_code == 200
@@ -62,6 +62,20 @@ def test_convert_dttm(
assert_convert_dttm(spec, target_type, expected_result, dttm)
@pytest.mark.parametrize(
"time_grain,expected",
[
(None, "{col}"),
("PT1S", "toStartOfSecond(toDateTime64({col}, 3))"),
("PT1M", "toStartOfMinute(toDateTime({col}))"),
],
)
def test_time_grain_expressions(time_grain: Optional[str], expected: str) -> None:
from superset.db_engine_specs.clickhouse import ClickHouseBaseEngineSpec
assert ClickHouseBaseEngineSpec._time_grain_expressions[time_grain] == expected
def test_convert_dttm_normalizes_aware_datetime_to_utc() -> None:
from superset.db_engine_specs.clickhouse import (
ClickHouseEngineSpec as spec, # noqa: N813
@@ -66,7 +66,7 @@ def mock_auth():
@pytest.fixture(autouse=True)
def allow_data_model_metadata():
def allow_data_model_metadata(): # noqa: PT004
"""Keep the standalone get_schema suite in the unrestricted default path."""
with patch.object(
get_schema_module,
@@ -606,3 +606,40 @@ class TestGetSchemaPermissionMap:
factories = set(get_schema_module._SCHEMA_CORE_FACTORIES.keys())
perms = set(get_schema_module._MODEL_TYPE_CLASS_PERMISSION.keys())
assert factories == perms
@pytest.mark.asyncio
async def test_resource_scope_is_enforced(self, app, mcp_server):
"""RBAC access alone cannot bypass a scoped token's resource limit."""
with (
patch.dict(app.config, {"MCP_RBAC_ENABLED": True}),
patch("superset.security_manager.can_access", return_value=True),
patch.object(
get_schema_module, "_token_scope_allows", return_value=False
) as scope_allows,
):
async with Client(mcp_server) as client:
with pytest.raises(ToolError, match="Permission denied"):
await client.call_tool(
"get_schema", {"request": {"model_type": "chart"}}
)
scope_allows.assert_called_once_with("read", "Chart")
@pytest.mark.asyncio
async def test_resource_scope_is_enforced_when_rbac_disabled(self, app, mcp_server):
"""The RBAC feature flag does not disable credential scopes."""
with (
patch.dict(app.config, {"MCP_RBAC_ENABLED": False}),
patch("superset.security_manager.can_access") as can_access,
patch.object(
get_schema_module, "_token_scope_allows", return_value=False
) as scope_allows,
):
async with Client(mcp_server) as client:
with pytest.raises(ToolError, match="Permission denied"):
await client.call_tool(
"get_schema", {"request": {"model_type": "chart"}}
)
can_access.assert_not_called()
scope_allows.assert_called_once_with("read", "Chart")
+158 -1
View File
@@ -23,12 +23,14 @@ import pytest
from flask import g
from superset.mcp_service.auth import (
_required_resource_scope,
check_tool_permission,
CLASS_PERMISSION_ATTR,
is_tool_visible_to_current_user,
MCPPermissionDeniedError,
METHOD_PERMISSION_ATTR,
PERMISSION_PREFIX,
RESOURCE_SCOPE_NAME,
)
@@ -108,6 +110,17 @@ def test_check_tool_permission_no_class_permission_allows(app_context) -> None:
assert check_tool_permission(func) is True
def test_scoped_token_constrains_permissionless_tool(app_context) -> None:
"""Resource-only scopes do not grant permission-less tools."""
g.user = MagicMock(username="admin")
func = _make_tool_func()
with _patch_token_scopes(["superset:dashboard:read"]):
assert check_tool_permission(func) is False
with _patch_token_scopes(["superset:read"]):
assert check_tool_permission(func) is True
def test_check_tool_permission_no_user_denies(app_context) -> None:
"""If no g.user, permission check should deny."""
g.user = None
@@ -170,6 +183,19 @@ def test_check_tool_permission_disabled_via_config(app_context, app) -> None:
app.config["MCP_RBAC_ENABLED"] = True
def test_disabled_rbac_still_enforces_token_scopes(app_context, app) -> None:
"""Disabling user RBAC does not disable credential restrictions."""
func = _make_tool_func(class_perm="Chart", method_perm="write")
app.config["MCP_RBAC_ENABLED"] = False
try:
with _patch_token_scopes(["superset:dashboard:read"]):
assert check_tool_permission(func) is False
with _patch_token_scopes(["superset:chart:write"]):
assert check_tool_permission(func) is True
finally:
app.config["MCP_RBAC_ENABLED"] = True
# -- Permission constants --
@@ -289,6 +315,19 @@ def test_visibility_public_tool_no_class_permission(app_context) -> None:
assert is_tool_visible_to_current_user(tool) is True
def test_visibility_hides_permissionless_tool_from_resource_scoped_token(
app_context,
) -> None:
"""Permission-less tools require a flat scope in tools/list too."""
g.user = MagicMock(username="viewer")
tool = _make_mock_tool(fn=_make_tool_func())
with _patch_token_scopes(["superset:dashboard:read"]):
assert is_tool_visible_to_current_user(tool) is False
with _patch_token_scopes(["superset:read"]):
assert is_tool_visible_to_current_user(tool) is True
def test_visibility_allowed_tool(app_context) -> None:
"""Tools where security_manager grants access are visible."""
g.user = MagicMock(username="admin")
@@ -431,6 +470,23 @@ def test_scope_falls_back_to_rbac_when_no_jwt_context(app_context) -> None:
assert result is True
def test_scope_context_error_fails_closed(app_context) -> None:
"""An unexpected token lookup failure cannot erase token restrictions."""
g.user = MagicMock(username="editor")
func = _make_tool_func(class_perm="Chart", method_perm="read")
mock_sm = MagicMock()
mock_sm.can_access = MagicMock(return_value=True)
with (
patch("superset.mcp_service.auth.security_manager", mock_sm),
patch(
"fastmcp.server.dependencies.get_access_token",
side_effect=TypeError("invalid token context"),
),
):
assert check_tool_permission(func) is False
def test_scope_read_denied_when_token_lacks_read_scope(app_context) -> None:
"""A read tool is denied when the token only carries an unrelated scope."""
g.user = MagicMock(username="viewer")
@@ -447,7 +503,9 @@ def test_scope_read_denied_when_token_lacks_read_scope(app_context) -> None:
assert result is False
def test_scope_denies_unmapped_method_for_scoped_token(app_context) -> None:
def test_scope_denies_unmapped_method_for_scoped_token(
app_context, caplog: pytest.LogCaptureFixture
) -> None:
"""A scoped token presented for a method permission that is NOT in the
scope map fails closed (denied), even when RBAC grants, so an unmapped
custom permission cannot silently bypass scope enforcement."""
@@ -463,6 +521,8 @@ def test_scope_denies_unmapped_method_for_scoped_token(app_context) -> None:
result = check_tool_permission(func)
assert result is False
assert "unmapped method permission 'some_custom_perm'" in caplog.text
assert "required scope 'None'" not in caplog.text
def test_scope_execute_sql_query_requires_write_scope(app_context) -> None:
@@ -480,6 +540,103 @@ def test_scope_execute_sql_query_requires_write_scope(app_context) -> None:
assert check_tool_permission(func) is True
# -- Per-resource scopes (superset:<resource>:<action>) --
def test_required_resource_scope_special_names() -> None:
"""The explicit resource map handles names a naive lower() would break:
'Row Level Security' (spaces) and 'ReportSchedule'/'SQLLab' (misnames)."""
assert _required_resource_scope("Row Level Security", "read") == "superset:rls:read"
assert _required_resource_scope("ReportSchedule", "write") == (
"superset:report:write"
)
assert _required_resource_scope("SQLLab", "execute_sql_query") == (
"superset:sqllab:write"
)
assert _required_resource_scope("Chart", "update") == "superset:chart:write"
def test_required_resource_scope_unmapped_returns_none() -> None:
"""An unmapped resource or method yields None (no per-resource scope),
which callers must NOT treat as a grant."""
assert _required_resource_scope("NotAResource", "read") is None
assert _required_resource_scope("Chart", "not_a_method") is None
def test_resource_scope_name_covers_all_tool_resource_classes() -> None:
"""RESOURCE_SCOPE_NAME must cover every class_permission_name declared by
MCP tools. If a new resource class is added, add it to the map."""
assert set(RESOURCE_SCOPE_NAME.keys()) == {
"Annotation",
"Chart",
"Dashboard",
"Database",
"Dataset",
"Explore",
"Query",
"ReportSchedule",
"Role",
"Row Level Security",
"SavedQuery",
"SQLLab",
"Tag",
"Task",
"Theme",
"User",
}
def test_per_resource_scope_grants_matching_tool(app_context) -> None:
"""A token scoped ONLY to superset:chart:write (no flat superset:write)
still grants a Chart/write tool via the per-resource grant path."""
g.user = MagicMock(username="editor")
func = _make_tool_func(class_perm="Chart", method_perm="write")
mock_sm = MagicMock()
mock_sm.can_access = MagicMock(return_value=True)
with (
patch("superset.mcp_service.auth.security_manager", mock_sm),
_patch_token_scopes(["superset:chart:write"]),
):
result = check_tool_permission(func)
assert result is True
def test_per_resource_scope_does_not_leak_across_resources(app_context) -> None:
"""A token scoped to superset:chart:write does NOT grant a Dashboard/write
tool (resource isolation)."""
g.user = MagicMock(username="editor")
func = _make_tool_func(class_perm="Dashboard", method_perm="write")
mock_sm = MagicMock()
mock_sm.can_access = MagicMock(return_value=True)
with (
patch("superset.mcp_service.auth.security_manager", mock_sm),
_patch_token_scopes(["superset:chart:write"]),
):
result = check_tool_permission(func)
assert result is False
def test_per_resource_scope_enforces_action(app_context) -> None:
"""A token scoped to superset:chart:read does NOT grant a Chart/write tool
(action still enforced within the resource)."""
g.user = MagicMock(username="editor")
func = _make_tool_func(class_perm="Chart", method_perm="write")
mock_sm = MagicMock()
mock_sm.can_access = MagicMock(return_value=True)
with (
patch("superset.mcp_service.auth.security_manager", mock_sm),
_patch_token_scopes(["superset:chart:read"]),
):
result = check_tool_permission(func)
assert result is False
# ---------------------------------------------------------------------------
# User/Role tools must request a permission FAB actually registers.
#
@@ -233,13 +233,22 @@ async def test_api_key_passthrough_propagates_required_scopes() -> None:
# -- Transport-layer DB validation (app configured) --
def _make_app_with_api_key(username: str | None) -> MagicMock:
"""Return a mock Flask app whose SecurityManager validates to ``username``."""
def _make_app_with_api_key(
username: str | None, scopes: str | None = None
) -> MagicMock:
"""Return a mock Flask app whose SecurityManager validates to ``username``.
``scopes`` is what ``get_api_key_scopes`` returns (FAB stores scopes as a
comma-separated string, or None). It must be configured explicitly an
unconfigured MagicMock return value would raise on ``.split(",")`` inside
the verifier's broad except-block and silently read as a rejected key.
"""
mock_user = MagicMock()
mock_user.username = username
mock_sm = MagicMock()
mock_sm.validate_api_key = MagicMock(return_value=mock_user if username else None)
mock_sm.get_api_key_scopes = MagicMock(return_value=scopes)
mock_app = MagicMock()
mock_app.app_context.return_value.__enter__ = MagicMock(return_value=None)
@@ -264,6 +273,41 @@ async def test_transport_validation_valid_key_returns_access_token() -> None:
assert result.claims.get(API_KEY_VALIDATED_USERNAME_CLAIM) == "alice"
@pytest.mark.asyncio
async def test_transport_validation_uses_keys_own_scopes() -> None:
"""A key with its own ApiKey.scopes carries them on the AccessToken,
parsed from FAB's comma-separated storage format."""
mock_app = _make_app_with_api_key(
"alice", scopes="superset:dashboard:read, superset:chart:read"
)
verifier = CompositeTokenVerifier(
jwt_verifier=None, api_key_prefixes=["sst_"], app=mock_app
)
result = await verifier.verify_token("sst_valid_key")
assert result is not None
assert result.scopes == ["superset:dashboard:read", "superset:chart:read"]
@pytest.mark.asyncio
async def test_transport_validation_no_key_scopes_remains_unscoped() -> None:
"""A key without scopes remains unscoped despite global JWT requirements."""
mock_app = _make_app_with_api_key("alice", scopes=None)
jwt_verifier = MagicMock()
jwt_verifier.required_scopes = ["superset:read"]
jwt_verifier.verify_token = AsyncMock()
verifier = CompositeTokenVerifier(
jwt_verifier=jwt_verifier, api_key_prefixes=["sst_"], app=mock_app
)
result = await verifier.verify_token("sst_valid_key")
assert result is not None
assert result.scopes == []
@pytest.mark.asyncio
async def test_transport_validation_invalid_key_returns_none() -> None:
"""An invalid API key is rejected at transport (returns None → HTTP 401)."""
@@ -0,0 +1,54 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from pathlib import Path
from superset.utils import json
def test_documented_dashboard_list_schema_matches_public_contract() -> None:
"""The published dashboard list contract must use generated relationships."""
spec_path = (
Path(__file__).parents[3] / "docs" / "static" / "resources" / "openapi.json"
)
spec = json.loads(spec_path.read_text(encoding="utf-8"))
schemas = spec["components"]["schemas"]
schema_name = "DashboardRestApi.get_list"
properties = schemas[schema_name]["properties"]
result_items = spec["paths"]["/api/v1/dashboard/"]["get"]["responses"]["200"][
"content"
]["application/json"]["schema"]["properties"]["result"]["items"]
assert result_items == {"$ref": f"#/components/schemas/{schema_name}"}
assert "description" in properties
assert {"owners", "roles", "thumbnail_url"}.isdisjoint(properties)
relationship_refs = {
"editors": "DashboardRestApi.get_list.Subject",
"tags": "DashboardRestApi.get_list.Tag",
"viewers": "DashboardRestApi.get_list.Subject1",
}
for field_name, component_name in relationship_refs.items():
assert properties[field_name] == {
"items": {"$ref": f"#/components/schemas/{component_name}"},
"type": "array",
}
assert component_name in schemas
assert f"{schema_name}.Role" not in schemas
assert f"{schema_name}.User2" not in schemas
@@ -0,0 +1,251 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Tests for API key scope validation in SupersetSecurityManager.
Covers the "intersection, never broader" rule: a user must not be able to
mint an API key scoped beyond what their own RBAC already permits.
"""
import re
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
from superset.extensions import appbuilder
from superset.security.api_key_scopes import (
RESOURCE_SCOPE_ACTIONS,
RESOURCE_SCOPE_CLASS,
)
from superset.security.manager import SupersetSecurityManager
def _make_user(*role_names: str) -> MagicMock:
"""Build a mock user whose roles carry the given names."""
user = MagicMock()
roles = []
for role_name in role_names:
role = MagicMock()
role.name = role_name
roles.append(role)
user.roles = roles
return user
@pytest.fixture
def sm(app_context: None) -> SupersetSecurityManager:
return SupersetSecurityManager(appbuilder)
def test_frontend_scope_catalog_matches_backend_contract() -> None:
"""Keep the UI picker aligned with the canonical enforcement vocabulary."""
frontend_catalog = (
Path(__file__).parents[3]
/ "superset-frontend/src/features/apiKeys/apiKeyScopes.ts"
).read_text()
resources_source = re.search(
r"const API_KEY_SCOPE_RESOURCES = \[(.*?)\] as const;",
frontend_catalog,
re.DOTALL,
)
actions_source = re.search(
r"const API_KEY_SCOPE_ACTIONS = \[(.*?)\] as const;",
frontend_catalog,
re.DOTALL,
)
assert resources_source is not None
assert actions_source is not None
assert set(re.findall(r"'([^']+)'", resources_source.group(1))) == set(
RESOURCE_SCOPE_CLASS
)
assert set(re.findall(r"'([^']+)'", actions_source.group(1))) == set(
RESOURCE_SCOPE_ACTIONS
)
def test_no_scopes_is_a_noop(sm: SupersetSecurityManager) -> None:
"""No scopes requested: nothing to validate, no RBAC lookups."""
sm._has_view_access = MagicMock()
sm._validate_requested_api_key_scopes(_make_user("Gamma"), None)
sm._validate_requested_api_key_scopes(_make_user("Gamma"), "")
sm._has_view_access.assert_not_called()
def test_per_resource_scope_allowed_when_user_has_permission(
sm: SupersetSecurityManager,
) -> None:
"""A per-resource scope the user's RBAC covers is allowed, and is checked
against the matching can_<method> grant."""
sm._has_view_access = MagicMock(return_value=True)
user = _make_user("Gamma")
sm._validate_requested_api_key_scopes(user, "superset:dashboard:read")
sm._has_view_access.assert_called_once_with(user, "can_read", "Dashboard")
def test_per_resource_scope_rejected_when_user_lacks_permission(
sm: SupersetSecurityManager,
) -> None:
"""A per-resource scope beyond the user's RBAC is rejected."""
sm._has_view_access = MagicMock(return_value=False)
with pytest.raises(ValueError, match="exceeds the issuing user's own"):
sm._validate_requested_api_key_scopes(
_make_user("Gamma"), "superset:dashboard:write"
)
@pytest.mark.parametrize(
("scope", "registered_permission"),
[
("superset:user:read", "can_get"),
("superset:role:read", "can_get"),
("superset:sqllab:write", "can_execute_sql_query"),
],
)
def test_scope_issuance_uses_runtime_method_mapping(
sm: SupersetSecurityManager, scope: str, registered_permission: str
) -> None:
"""Issuance accepts the FAB method permission used by runtime tools."""
user = _make_user("Gamma")
sm._has_view_access = MagicMock(
side_effect=lambda _user, permission, _view: permission == registered_permission
)
sm._validate_requested_api_key_scopes(user, scope)
assert any(
call.args[1] == registered_permission
for call in sm._has_view_access.call_args_list
)
def test_custom_admin_role_can_issue_flat_scope(
sm: SupersetSecurityManager,
) -> None:
"""Flat-scope issuance honors AUTH_ROLE_ADMIN rather than a fixed name."""
with patch("superset.security.manager.get_conf") as get_conf:
get_conf.return_value = {"AUTH_ROLE_ADMIN": "PlatformAdmin"}
sm._validate_requested_api_key_scopes(
_make_user("PlatformAdmin"), "superset:write"
)
@pytest.mark.parametrize("action", ["delete", "update", "garbage"])
def test_unrecognized_actions_are_rejected(
sm: SupersetSecurityManager, action: str
) -> None:
"""Actions that runtime enforcement cannot consume are rejected."""
sm._has_view_access = MagicMock()
with pytest.raises(ValueError, match="unrecognized action"):
sm._validate_requested_api_key_scopes(
_make_user("Gamma"), f"superset:chart:{action}"
)
sm._has_view_access.assert_not_called()
def test_unrecognized_resource_slug_rejected_without_rbac_lookup(
sm: SupersetSecurityManager,
) -> None:
"""An unknown resource slug is rejected outright (fail closed) and never
consults RBAC."""
sm._has_view_access = MagicMock()
with pytest.raises(ValueError, match="unrecognized resource"):
sm._validate_requested_api_key_scopes(
_make_user("Admin"), "superset:notathing:read"
)
sm._has_view_access.assert_not_called()
def test_flat_scope_allowed_for_admin(sm: SupersetSecurityManager) -> None:
"""A flat scope (superset:write) may be self-issued by an Admin, with no
per-resource RBAC lookups."""
sm._has_view_access = MagicMock()
sm._validate_requested_api_key_scopes(_make_user("Admin"), "superset:write")
sm._has_view_access.assert_not_called()
def test_unrecognized_flat_scope_rejected_for_admin(
sm: SupersetSecurityManager,
) -> None:
"""Admins cannot mint undefined flat scopes."""
sm._has_view_access = MagicMock()
with pytest.raises(ValueError, match="not a recognized"):
sm._validate_requested_api_key_scopes(_make_user("Admin"), "superset:garbage")
sm._has_view_access.assert_not_called()
def test_flat_scope_rejected_for_non_admin(sm: SupersetSecurityManager) -> None:
"""A flat scope grants a method across every resource; non-Admins cannot
self-issue it."""
sm._has_view_access = MagicMock()
with pytest.raises(ValueError, match="requires Admin"):
sm._validate_requested_api_key_scopes(_make_user("Gamma"), "superset:write")
def test_any_failing_scope_rejects_the_whole_request(
sm: SupersetSecurityManager,
) -> None:
"""With multiple comma-separated scopes, one failure rejects the request
even when other scopes are individually allowed."""
sm._has_view_access = MagicMock(
side_effect=lambda user, perm, view: view == "Chart"
)
with pytest.raises(ValueError, match="exceeds the issuing user's own"):
sm._validate_requested_api_key_scopes(
_make_user("Gamma"),
"superset:chart:read, superset:dashboard:write",
)
def test_create_api_key_rejects_before_delegating_to_fab(
sm: SupersetSecurityManager,
) -> None:
"""create_api_key validates scopes BEFORE calling FAB's implementation:
a rejected request never reaches FAB."""
sm._has_view_access = MagicMock(return_value=False)
with patch(
"flask_appbuilder.security.sqla.manager.SecurityManager.create_api_key"
) as fab_create:
with pytest.raises(ValueError, match="exceeds the issuing user's own"):
sm.create_api_key(
user=_make_user("Gamma"),
name="my key",
scopes="superset:dashboard:write",
)
fab_create.assert_not_called()
def test_create_api_key_delegates_to_fab_on_success(
sm: SupersetSecurityManager,
) -> None:
"""A validated request is delegated to FAB's create_api_key unchanged."""
sm._has_view_access = MagicMock(return_value=True)
user = _make_user("Gamma")
with patch(
"flask_appbuilder.security.sqla.manager.SecurityManager.create_api_key",
return_value={"key": "sst_secret"},
) as fab_create:
result = sm.create_api_key(
user=user,
name="my key",
scopes="superset:dashboard:read",
)
fab_create.assert_called_once_with(
user=user, name="my key", scopes="superset:dashboard:read", expires_on=None
)
assert result == {"key": "sst_secret"}
@@ -0,0 +1,276 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Password-change paths and the session-invalidation epoch.
``UserAttribute.sessions_invalidated_at`` (see
``superset.security.session_invalidation``) is the mechanism that forces
outstanding sessions to log out. Originally it was stamped exclusively by the
``after_update`` listener that fires when an account's ``active`` flag flips
to ``False``; these tests now cover the additional password-change paths --
self-service reset, admin-initiated reset, and the ``PUT /api/v1/me/``
self-service update -- which also stamp that epoch, so a session authenticated
before a password change stops working after it.
"""
from __future__ import annotations
from collections.abc import Iterator
from types import SimpleNamespace
from unittest.mock import patch
import pytest
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_appbuilder.security.sqla.models import User
from superset import db, security_manager
from superset.daos.user import UserDAO
from superset.models.user_attributes import UserAttribute
from superset.security.manager import SupersetUserApi
from superset.views.users.api import CurrentUserRestApi
from tests.unit_tests.fixtures.common import admin_user, after_each # noqa: F401
def _invalidated_at(user_id: int):
attr = db.session.query(UserAttribute).filter_by(user_id=user_id).one_or_none()
return attr.sessions_invalidated_at if attr else None
@pytest.fixture
def two_admins() -> Iterator[tuple[User, User]]:
"""Two admin-role users for the reset_password tests below.
``SupersetSecurityManager.reset_password`` -> FAB's ``update_user``
hard-commits the session (``commit=True`` by default), so the rollback
the shared ``after_each``/``admin_user`` fixtures rely on can't undo it.
This fixture creates its own users and deletes them again on teardown so
a committed reset doesn't leak rows into later tests.
"""
role = db.session.query(security_manager.role_model).filter_by(name="Admin").one()
target = User(
first_name="Target",
last_name="User",
email="session_invalidation_target@example.org",
username="session_invalidation_target",
roles=[role],
)
actor = User(
first_name="Acting",
last_name="Admin",
email="session_invalidation_actor@example.org",
username="session_invalidation_actor",
roles=[role],
)
db.session.add_all([target, actor])
db.session.commit()
yield target, actor
db.session.query(UserAttribute).filter(
UserAttribute.user_id.in_([target.id, actor.id])
).delete(synchronize_session=False)
db.session.query(User).filter(User.id.in_([target.id, actor.id])).delete(
synchronize_session=False
)
db.session.commit()
def test_self_service_password_reset_invalidates_other_sessions(
two_admins: tuple[User, User],
) -> None:
"""``SupersetSecurityManager.reset_password`` used for a self-service
reset (acting user resets their own password) stamps the session epoch,
so any other outstanding session for the account stops working after the
password changes.
"""
target, _actor = two_admins
with patch("superset.security.manager.g") as mock_g:
mock_g.user = target
security_manager.reset_password(target.id, "BrandNewPassw0rd!")
assert _invalidated_at(target.id) is not None
def test_admin_password_reset_invalidates_target_sessions(
two_admins: tuple[User, User],
) -> None:
"""An admin-initiated reset of *another* user's password also stamps the
epoch, so the target's outstanding sessions stop working -- this is the
closest existing action to an explicit "terminate that user's sessions",
short of disabling the account.
"""
target, actor = two_admins
with patch("superset.security.manager.g") as mock_g:
mock_g.user = actor # differs from target: an admin-initiated reset
security_manager.reset_password(target.id, "TemporaryPassw0rd!")
assert _invalidated_at(target.id) is not None
def test_update_me_password_change_invalidates_other_sessions(
admin_user: User, # noqa: F811
after_each: None, # noqa: F811
) -> None:
"""The ``PUT /api/v1/me/`` self-service password change (``pre_update`` +
``UserDAO.update`` in ``CurrentUserRestApi.update_me``) also stamps the
session-invalidation epoch. ``admin_user`` starts with no password set, so
no ``current_password`` proof is required for this change to go through.
"""
api = CurrentUserRestApi()
data = {"password": "BrandNewPassw0rd!"}
with patch("superset.views.users.api.g") as mock_g:
mock_g.user = admin_user
api.pre_update(admin_user, data)
UserDAO.update(item=admin_user, attributes=data)
db.session.flush()
assert _invalidated_at(admin_user.id) is not None
def test_admin_edit_user_password_via_put_invalidates_target_sessions(
after_each: None, # noqa: F811
) -> None:
"""An admin editing another user's password via ``PUT
/api/v1/security/users/<pk>`` (``SupersetUserApi.pre_update``, which FAB's
``UserApi.put`` calls before its own commit) must also stamp the target's
session-invalidation epoch, the same as the self-service ``/me/`` path and
the two password-reset views -- otherwise this admin path is the one way
to change a user's password that leaves their other sessions alive.
"""
role = db.session.query(security_manager.role_model).filter_by(name="Admin").one()
user = User(
first_name="Target",
last_name="User",
email="admin_edit_password_target@example.org",
username="admin_edit_password_target",
roles=[role],
)
db.session.add(user)
db.session.commit()
api = SupersetUserApi()
api.datamodel = SQLAInterface(User, db.session)
api.appbuilder = SimpleNamespace(
sm=SimpleNamespace(current_user=SimpleNamespace(id=1))
)
api.pre_update(user, {"password": "AdminSetPassw0rd!"})
assert _invalidated_at(user.id) is not None
db.session.query(UserAttribute).filter_by(user_id=user.id).delete(
synchronize_session=False
)
db.session.query(User).filter_by(id=user.id).delete(synchronize_session=False)
db.session.commit()
def test_admin_edit_user_without_password_change_does_not_invalidate_sessions(
after_each: None, # noqa: F811
) -> None:
"""Editing a user through the same endpoint *without* touching the
password (e.g. renaming them) must not stamp the epoch -- only an actual
password change should force other sessions to log out.
"""
role = db.session.query(security_manager.role_model).filter_by(name="Admin").one()
user = User(
first_name="Target",
last_name="User",
email="admin_edit_no_password_target@example.org",
username="admin_edit_no_password_target",
roles=[role],
)
db.session.add(user)
db.session.commit()
api = SupersetUserApi()
api.datamodel = SQLAInterface(User, db.session)
api.appbuilder = SimpleNamespace(
sm=SimpleNamespace(current_user=SimpleNamespace(id=1))
)
api.pre_update(user, {"first_name": "Renamed"})
assert _invalidated_at(user.id) is None
db.session.query(User).filter_by(id=user.id).delete(synchronize_session=False)
db.session.commit()
def _make_api_for_target(user: User) -> SupersetUserApi:
"""A ``SupersetUserApi`` instance wired to a fake ``datamodel`` that
resolves any pk lookup to ``user`` -- enough to exercise
``terminate_sessions`` without going through HTTP/auth plumbing, mirroring
the pattern used in ``test_superset_user_api_subject_sync.py``.
"""
api = SupersetUserApi()
api.datamodel = SimpleNamespace(
session=db.session,
obj=User,
get=lambda pk, base_filters=None: user,
)
api._base_filters = None
return api
def test_terminate_sessions_action_stamps_target_epoch_without_disabling_account(
after_each: None, # noqa: F811
) -> None:
"""``SupersetUserApi.terminate_sessions`` -- the direct, explicit
"terminate this user's sessions" admin action -- stamps the epoch for the
target user without flipping ``active`` or otherwise touching the account,
unlike the only other action that has this effect (disabling the user).
"""
role = db.session.query(security_manager.role_model).filter_by(name="Admin").one()
user = User(
first_name="Target",
last_name="User",
email="terminate_sessions_target@example.org",
username="terminate_sessions_target",
roles=[role],
)
db.session.add(user)
db.session.flush()
with patch.object(security_manager, "has_access", return_value=True):
response = _make_api_for_target(user).terminate_sessions(user.id)
assert response.status_code == 200
assert _invalidated_at(user.id) is not None
assert user.active
def test_terminate_sessions_action_404s_for_unknown_user(
after_each: None, # noqa: F811
) -> None:
"""A pk that doesn't resolve to a user (or is filtered out by
``base_filters``) 404s rather than stamping anything.
"""
api = SupersetUserApi()
api.datamodel = SimpleNamespace(
session=db.session,
obj=User,
get=lambda pk, base_filters=None: None,
)
api._base_filters = None
with patch.object(security_manager, "has_access", return_value=True):
response = api.terminate_sessions(999999)
assert response.status_code == 404
@@ -0,0 +1,229 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""``SupersetUserApi`` syncs the ``Subject`` row that mirrors each ``User``.
``UserApi.post``/``put`` call ``self.pre_add``/``self.pre_update`` *before*
the write that actually commits (``self.datamodel.add``/``edit``). Overriding
those hooks -- instead of syncing after the fact, in a second commit issued
once ``post``/``put`` have already returned -- means the subject sync rides
the same transaction as the user write: one commit persists both, and a
failure of that commit rolls both back together instead of leaving an
orphaned user with no matching ``Subject`` row.
These tests exercise ``pre_add``/``pre_update`` directly, plus the exact call
pairs FAB's ``UserApi.post``/``put`` make (``pre_add``/``pre_update`` followed
by the real ``SQLAInterface.add``/``edit``), to confirm that pairing shares a
single commit and a single rollback.
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_appbuilder.security.sqla.models import User
from superset import db, security_manager
from superset.security.manager import SupersetUserApi
from superset.subjects.models import Subject
from superset.subjects.types import SubjectType
from tests.unit_tests.fixtures.common import after_each # noqa: F401
def _admin_role() -> object:
return db.session.query(security_manager.role_model).filter_by(name="Admin").one()
def _make_pending_user(username: str) -> User:
"""A transient ``User``, not yet added to the session.
Mirrors what FAB's ``UserApi.post()`` builds (``model = User()``, with
attributes set from the request payload) just before it calls
``self.pre_add(model)`` and then ``self.datamodel.add(model)``.
"""
return User(
first_name="New",
last_name="Guy",
email=f"{username}@example.org",
username=username,
roles=[_admin_role()],
password="irrelevant-pre-hash-value", # noqa: S106
)
def _make_persisted_user(username: str) -> User:
"""A ``User`` already committed, standing in for one an earlier request
created -- the starting point for a ``put``/``pre_update`` flow.
"""
user = User(
first_name="New",
last_name="Guy",
email=f"{username}@example.org",
username=username,
roles=[_admin_role()],
)
db.session.add(user)
db.session.commit()
return user
def _api_for(session=db.session) -> SupersetUserApi: # noqa: ANN001
api = SupersetUserApi()
api.datamodel = SQLAInterface(User, session)
# FAB's own ``pre_update`` (which ``SupersetUserApi.pre_update`` calls via
# ``super()``) reads ``self.appbuilder.sm.current_user.id`` to stamp
# ``changed_by_fk`` -- stand in for the acting admin so that lookup
# succeeds outside of a real request/login context.
api.appbuilder = SimpleNamespace(
sm=SimpleNamespace(current_user=SimpleNamespace(id=1))
)
return api
def _subject_for(user_id: int) -> Subject | None:
return (
db.session.query(Subject)
.filter_by(user_id=user_id, type=SubjectType.USER)
.one_or_none()
)
def test_pre_add_syncs_subject_without_committing(
after_each: None, # noqa: F811
) -> None:
"""``pre_add`` flushes the new user (to obtain its id) and syncs its
``Subject`` row, but does not commit -- that's still FAB's job, in
``self.datamodel.add``, which runs right after.
"""
user = _make_pending_user("new_guy_pre_add")
api = _api_for()
real_commit = db.session.commit
commit_calls = MagicMock(wraps=real_commit)
with patch.object(db.session, "commit", commit_calls):
api.pre_add(user)
assert commit_calls.call_count == 0
assert user.id is not None
assert _subject_for(user.id) is not None
def test_pre_add_and_datamodel_add_share_a_single_commit(
after_each: None, # noqa: F811
) -> None:
"""The exact pair of calls FAB's ``UserApi.post()`` makes --
``self.pre_add(model)`` then ``self.datamodel.add(model)`` -- persist the
user and its ``Subject`` row together, via exactly one commit.
"""
user = _make_pending_user("new_guy_shared_commit")
api = _api_for()
real_commit = db.session.commit
commit_calls = MagicMock(wraps=real_commit)
with patch.object(db.session, "commit", commit_calls):
api.pre_add(user)
api.datamodel.add(user)
assert commit_calls.call_count == 1
subject = _subject_for(user.id)
assert subject is not None
def test_pre_add_failure_rolls_back_user_and_subject_together(
after_each: None, # noqa: F811
) -> None:
"""If the commit that follows ``pre_add`` fails (standing in: FAB's own
``self.datamodel.add`` raising), the new user and the ``Subject`` row
flushed alongside it roll back together -- there is no window where the
user persists without a matching ``Subject``.
"""
user = _make_pending_user("new_guy_pre_add_fail")
api = _api_for()
api.pre_add(user)
user_id = user.id
assert user_id is not None
assert _subject_for(user_id) is not None # flushed, visible pre-rollback
db.session.rollback() # stands in for the follow-up commit failing
assert db.session.query(User).filter_by(id=user_id).one_or_none() is None
assert _subject_for(user_id) is None
def test_pre_update_syncs_subject_without_committing(
after_each: None, # noqa: F811
) -> None:
"""Same reasoning as ``pre_add``, for an edit: ``pre_update`` syncs the
``Subject`` row without committing, ahead of FAB's own
``self.datamodel.edit``.
"""
user = _make_persisted_user("new_guy_pre_update")
api = _api_for()
real_commit = db.session.commit
commit_calls = MagicMock(wraps=real_commit)
with patch.object(db.session, "commit", commit_calls):
api.pre_update(user, {})
assert commit_calls.call_count == 0
assert _subject_for(user.id) is not None
def test_pre_update_and_datamodel_edit_share_a_single_commit(
after_each: None, # noqa: F811
) -> None:
"""The exact pair of calls FAB's ``UserApi.put()`` makes --
``self.pre_update(model, item)`` then ``self.datamodel.edit(model)`` --
persist the edit and the ``Subject`` sync together, via exactly one
commit.
"""
user = _make_persisted_user("new_guy_shared_commit_put")
api = _api_for()
real_commit = db.session.commit
commit_calls = MagicMock(wraps=real_commit)
with patch.object(db.session, "commit", commit_calls):
api.pre_update(user, {})
api.datamodel.edit(user)
assert commit_calls.call_count == 1
assert _subject_for(user.id) is not None
def test_pre_update_failure_rolls_back_subject_sync_without_orphaning(
after_each: None, # noqa: F811
) -> None:
"""If the commit that follows ``pre_update`` fails, the ``Subject`` row it
flushed rolls back too -- the previously-persisted user row (created by
an earlier, already-successful request) is left exactly as it was, with
no half-applied sync attached to it.
"""
user = _make_persisted_user("new_guy_pre_update_fail")
user_id = user.id
api = _api_for()
api.pre_update(user, {})
assert _subject_for(user_id) is not None # flushed, visible pre-rollback
db.session.rollback() # stands in for the follow-up commit failing
# The user itself predates this (failed) request and survives.
assert db.session.query(User).filter_by(id=user_id).one_or_none() is not None
# But the subject sync this request attempted never landed.
assert _subject_for(user_id) is None
@@ -19,9 +19,11 @@
from unittest.mock import MagicMock, patch
import pytest
from flask import current_app
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetSecurityException
from superset.views.datasource.utils import get_limit_clause
@patch("superset.views.datasource.utils.get_limit_clause")
@@ -215,3 +217,62 @@ def test_get_samples_count_star_access_denied(mock_get_limit_clause: MagicMock):
mock_samples_context.raise_for_access.assert_called_once()
# Verify count context was also checked
mock_count_context.raise_for_access.assert_called_once()
@pytest.mark.parametrize("per_page", [5000, 10000])
def test_get_limit_clause_honors_per_page_above_samples_row_limit(
per_page: int,
) -> None:
"""Regression guard: the Explore Data panel "Samples" tab silently caps at
``SAMPLES_ROW_LIMIT`` (config default 1000).
The samples row-limit dropdown offers 5k/10k options and the samples
endpoint's ``SamplesRequestSchema`` accepts ``per_page`` up to 10000, yet
``get_limit_clause`` resets any ``per_page`` above ``SAMPLES_ROW_LIMIT``
back down to it. A user who selects 5k/10k therefore silently receives at
most 1000 rows, with no signal that the requested limit was overridden.
The rows a user is allowed to request and the rows actually returned must
stay consistent: a ``per_page`` the endpoint accepts must not be silently
reduced below the request.
"""
assert get_limit_clause(page=1, per_page=per_page) == {
"row_offset": 0,
"row_limit": per_page,
}
@pytest.mark.parametrize(
"per_page,expected_row_limit",
[
(0, 0),
(-1, 1000),
],
)
def test_get_limit_clause_preserves_zero_and_negative_per_page(
per_page: int,
expected_row_limit: int,
) -> None:
assert get_limit_clause(page=1, per_page=per_page) == {
"row_offset": 0,
"row_limit": expected_row_limit,
}
def test_get_limit_clause_caps_per_page_at_sql_max_row(
app_context: None,
) -> None:
"""When an operator configures ``SQL_MAX_ROW`` below the schema's
``per_page`` maximum, ``apply_max_row_limit`` still reduces the
requested limit, and the offset for subsequent pages must be computed
from that reduced (effective) limit, not the raw request.
"""
with patch.dict(current_app.config, {"SQL_MAX_ROW": 2000}):
assert get_limit_clause(page=1, per_page=10000) == {
"row_offset": 0,
"row_limit": 2000,
}
assert get_limit_clause(page=2, per_page=10000) == {
"row_offset": 2000,
"row_limit": 2000,
}
@@ -0,0 +1,160 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Unit tests for the ``CurrentUserRestApi`` self-service update flow.
Covers the ``password`` handling in ``PUT /api/v1/me/``: whether the caller
must prove knowledge of the existing password, and whether the value that
ends up persisted is the hash computed in ``pre_update`` or the raw value
from the request payload.
"""
from __future__ import annotations
from typing import Any
from unittest.mock import patch
import pytest
from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from werkzeug.security import check_password_hash, generate_password_hash
from superset import db
from superset.daos.user import UserDAO
from superset.views.users.api import CurrentUserRestApi
from superset.views.users.schemas import CurrentUserPutSchema
from tests.unit_tests.fixtures.common import admin_user, after_each # noqa: F401
def _run_update_me(user: User, data: dict[str, Any]) -> None:
"""Reproduce the body of ``CurrentUserRestApi.update_me`` for ``data``.
Exercises the same two calls the endpoint makes -- ``pre_update`` followed
by ``UserDAO.update`` with the schema-loaded payload as ``attributes`` --
without going through HTTP/auth plumbing, since neither call depends on it.
"""
api = CurrentUserRestApi()
with patch("superset.views.users.api.g") as mock_g:
mock_g.user = user
api.pre_update(user, data)
UserDAO.update(item=user, attributes=data)
def test_current_user_put_schema_has_current_password_field() -> None:
"""The payload schema for ``PUT /api/v1/me/`` carries a field for proving
knowledge of the existing password, required whenever ``password`` is
supplied.
"""
schema = CurrentUserPutSchema()
assert "password" in schema.fields
assert "current_password" in schema.fields
with pytest.raises(ValidationError):
schema.load({"password": "BrandNewPassw0rd!"})
# Present alongside "password", it loads fine (the schema only checks
# that it was *supplied*; whether it's actually correct is verified
# against the database in ``CurrentUserRestApi.pre_update``).
loaded = schema.load(
{"password": "BrandNewPassw0rd!", "current_password": "OldPassw0rd!"}
)
assert loaded["current_password"] == "OldPassw0rd!" # noqa: S105
def test_update_me_rejects_password_change_without_correct_current_password(
admin_user: User, # noqa: F811
after_each: None, # noqa: F811
) -> None:
"""A caller can no longer change the password by supplying only the new
value: an account with an existing password must prove knowledge of it
via ``current_password`` -- omitting the field, or getting it wrong, both
reject the change and leave the stored password untouched. Supplying the
correct current password lets the change through.
"""
original_hash = generate_password_hash("OldPassw0rd!")
admin_user.password = original_hash
with pytest.raises(ValidationError):
_run_update_me(admin_user, {"password": "BrandNewPassw0rd!"})
assert admin_user.password == original_hash
with pytest.raises(ValidationError):
_run_update_me(
admin_user,
{
"password": "BrandNewPassw0rd!",
"current_password": "WrongPassw0rd!",
},
)
assert admin_user.password == original_hash
_run_update_me(
admin_user,
{"password": "BrandNewPassw0rd!", "current_password": "OldPassw0rd!"},
)
db.session.flush()
assert admin_user.password != original_hash
assert check_password_hash(admin_user.password, "BrandNewPassw0rd!")
def test_update_me_password_change_persists_a_hash_not_plaintext(
admin_user: User, # noqa: F811
after_each: None, # noqa: F811
) -> None:
"""``pre_update`` computes ``generate_password_hash(data["password"])`` and
assigns it to the user, and ``UserDAO.update`` is then called with the
*same* ``data`` dict as ``attributes``. ``BaseDAO.update`` blindly
``setattr``s every key in ``attributes``, so ``pre_update`` must remove
the plaintext ``password`` key (and any ``current_password``) from that
dict once it's done with them, or the plaintext would overwrite the hash
that was just computed and reach the database instead of it.
``admin_user`` starts with no password set, so this exercises the
first-password-set path, which requires no proof of a prior password.
"""
new_password = "BrandNewPassw0rd!" # noqa: S105
_run_update_me(admin_user, {"password": new_password})
db.session.flush()
stored_password = admin_user.password
# The stored value should be a password hash that verifies against the
# new password -- not the plaintext value itself.
assert stored_password != new_password
assert check_password_hash(stored_password, new_password)
def test_update_me_falsy_password_does_not_blank_stored_hash(
admin_user: User, # noqa: F811
after_each: None, # noqa: F811
) -> None:
"""A falsy ``password`` (e.g. an empty string, which the schema's
complexity validator lets through when password complexity validation is
disabled) skips the hashing branch entirely -- ``pre_update`` must still
drop the key from ``data`` so it never reaches ``UserDAO.update``'s
``setattr`` loop and blanks the account's stored hash.
"""
original_hash = generate_password_hash("OldPassw0rd!")
admin_user.password = original_hash
_run_update_me(admin_user, {"password": "", "first_name": "Foo"})
db.session.flush()
assert admin_user.password == original_hash
assert admin_user.first_name == "Foo"
@@ -0,0 +1,109 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Tests for both renaming mechanisms in ``CustomTagsOptimizationMixin``.
The schema-level ``data_key`` rename only applies to the API's default list
schema; requests that pass ``select_columns`` make FAB build a fresh schema
on the fly, so for those the ``pre_get_list`` response rewrite is what keeps
the public ``tags`` name. Both paths need coverage.
"""
from typing import Any
from marshmallow import fields, Schema
from superset.views.custom_tags_api_mixin import CustomTagsOptimizationMixin
class BaseApi:
"""Stub for the FAB ``ModelRestApi`` base. The mixin chains via
``super()``, so the stub must define the hooks the mixin overrides."""
list_model_schema: Schema
pre_get_list_calls: int = 0
def _init_model_schemas(self) -> None:
self.list_model_schema = Schema.from_dict(
{"custom_tags": fields.List(fields.String())}
)()
def pre_get_list(self, _data: dict[str, Any]) -> None:
self.pre_get_list_calls += 1
class CustomTagsApi(CustomTagsOptimizationMixin, BaseApi):
_custom_tags_only = True
class UnoptimizedTagsApi(CustomTagsOptimizationMixin, BaseApi):
_custom_tags_only = False
def test_custom_tags_schema_uses_public_tags_name() -> None:
api = CustomTagsApi()
api._init_model_schemas()
assert api.list_model_schema.dump({"custom_tags": ["critical"]}) == {
"tags": ["critical"]
}
def test_custom_tags_schema_keeps_name_when_optimization_disabled() -> None:
api = UnoptimizedTagsApi()
api._init_model_schemas()
assert api.list_model_schema.dump({"custom_tags": ["critical"]}) == {
"custom_tags": ["critical"]
}
def test_pre_get_list_renames_custom_tags_when_enabled() -> None:
api = CustomTagsApi()
data: dict[str, Any] = {
"result": [
{"id": 1, "custom_tags": [{"name": "critical"}]},
{"id": 2},
]
}
api.pre_get_list(data)
assert data["result"][0] == {"id": 1, "tags": [{"name": "critical"}]}
assert data["result"][1] == {"id": 2}
assert api.pre_get_list_calls == 1
def test_pre_get_list_keeps_custom_tags_when_disabled() -> None:
api = UnoptimizedTagsApi()
data: dict[str, Any] = {"result": [{"id": 1, "custom_tags": []}]}
api.pre_get_list(data)
assert data["result"][0] == {"id": 1, "custom_tags": []}
assert api.pre_get_list_calls == 1
def test_pre_get_list_tolerates_missing_result_key() -> None:
api = CustomTagsApi()
data: dict[str, Any] = {"count": 0}
api.pre_get_list(data)
assert data == {"count": 0}
assert api.pre_get_list_calls == 1