This is an automated email from the ASF dual-hosted git repository. Vitor-Avila pushed a commit to branch feat/oauth2-token-management in repository https://gitbox.apache.org/repos/asf/superset.git
commit 055ef35d5b98960a17010d6fb6139f9e98bd8717 Author: Vitor Avila <[email protected]> AuthorDate: Tue Oct 20 09:46:07 2026 -0300 feat(DB OAuth2): Allow admins to manage DB OAuth2 tokens --- .../features/databases/OAuth2TokensModal.test.tsx | 223 +++++++++ .../src/features/databases/OAuth2TokensModal.tsx | 339 ++++++++++++++ superset-frontend/src/features/databases/types.ts | 1 + .../DatabaseList.oauth2Tokens.test.tsx | 135 ++++++ superset-frontend/src/pages/DatabaseList/index.tsx | 39 +- superset/commands/database/exceptions.py | 9 + superset/commands/database/oauth2.py | 52 ++- superset/daos/database.py | 21 + superset/databases/api.py | 209 ++++++++- superset/databases/filters.py | 34 +- superset/databases/schemas.py | 13 + superset/initialization/__init__.py | 3 +- ...6e9c104a_add_oauth2_token_management_indexes.py | 40 ++ superset/models/core.py | 2 + superset/security/manager.py | 1 + superset/semantic_layers/api.py | 2 + tests/unit_tests/databases/api_test.py | 2 + tests/unit_tests/databases/oauth2_tokens_test.py | 511 +++++++++++++++++++++ .../semantic_layers/connections_access_test.py | 17 + 19 files changed, 1644 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/features/databases/OAuth2TokensModal.test.tsx b/superset-frontend/src/features/databases/OAuth2TokensModal.test.tsx new file mode 100644 index 00000000000..93c5d54f5c4 --- /dev/null +++ b/superset-frontend/src/features/databases/OAuth2TokensModal.test.tsx @@ -0,0 +1,223 @@ +/** + * 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 fetchMock from 'fetch-mock'; +import rison from 'rison'; +import userEvent from '@testing-library/user-event'; +import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; +import OAuth2TokensModal from './OAuth2TokensModal'; +import { ConfigurationMethod } from './types'; + +const database = { + id: 1, + uuid: '846ddf91-9cc3-4fb7-a218-e9d26ca5fe39', + database_name: 'OAuth database', + name: 'OAuth database', + driver: 'trino', + configuration_method: ConfigurationMethod.SqlalchemyUri, + is_managed_externally: false, +}; +const endpoint = `/api/v1/database/${database.id}/oauth2_tokens/`; +type ListQuery = { page_size?: number; filters?: { value: string }[] }; +const decodeListQuery = (query: string) => rison.decode(query) as ListQuery; +const token = (index: number) => ({ + id: index, + user: { + username: `user${index}`, + first_name: `First${index}`, + last_name: 'Example', + email: `user${index}@example.com`, + }, + access_token_expiration: '2027-01-01T00:00:00', +}); +const addSuccessToast = jest.fn(); +const addDangerToast = jest.fn(); + +beforeEach(() => { + jest.clearAllMocks(); + fetchMock.clearHistory().removeRoutes(); + fetchMock.get(`glob:*${endpoint}?*`, { + count: 50, + result: [token(1), token(2)], + }); + fetchMock.post(`glob:*${endpoint}delete/`, { message: 'OK' }); + fetchMock.post(`glob:*${endpoint}all/`, { message: 'OK' }); +}); +afterEach(() => fetchMock.clearHistory().removeRoutes()); + +const renderModal = () => + render( + <OAuth2TokensModal + database={database} + onHide={jest.fn()} + addSuccessToast={addSuccessToast} + addDangerToast={addDangerToast} + />, + ); +const confirmDeletion = async () => { + const dialog = screen + .getByText('Delete OAuth2 tokens?') + .closest('[role=dialog]') as HTMLElement; + const button = within(dialog).getByRole('button', { + name: 'Delete', + }); + expect(button).toBeDisabled(); + await userEvent.type( + within(dialog).getByLabelText('Type "DELETE" to confirm'), + 'DELETE', + ); + await userEvent.click(button); +}; +const expectPayload = (payload: object) => { + expect( + fetchMock.callHistory.lastCall(`glob:*${endpoint}delete/`)?.options.body, + ).toBe(JSON.stringify(payload)); +}; + +test('individual deletion requires typed confirmation and sends only that credential ID', async () => { + renderModal(); + await screen.findByText('user1'); + await userEvent.click( + screen.getByRole('button', { name: 'Delete OAuth2 tokens for user1' }), + ); + const confirmation = screen + .getByText('Delete OAuth2 tokens?') + .closest('[role=dialog]') as HTMLElement; + expect(within(confirmation).queryByText('user1')).not.toBeInTheDocument(); + expect( + within(confirmation).getByText(/Delete 1 stored credential pair/), + ).toBeInTheDocument(); + await confirmDeletion(); + await waitFor(() => expect(addSuccessToast).toHaveBeenCalled()); + expectPayload({ token_ids: [token(1).id] }); +}); + +test('bulk deletion submits selected rows on the current page', async () => { + renderModal(); + await screen.findByText('user1'); + await userEvent.click(screen.getAllByRole('checkbox')[0]); + expect(screen.getByText('2 Selected')).toBeInTheDocument(); + await userEvent.click( + screen.getByRole('button', { name: 'Delete selected' }), + ); + await confirmDeletion(); + await waitFor(() => expect(addSuccessToast).toHaveBeenCalled()); + expectPayload({ token_ids: [token(1).id, token(2).id] }); +}); + +test('changing pages clears selection and fetches a server-side page', async () => { + renderModal(); + await screen.findByText('user1'); + await userEvent.click(screen.getAllByRole('checkbox')[1]); + expect(screen.getByText('1 Selected')).toBeInTheDocument(); + await userEvent.click(screen.getByTitle('2')); + expect(screen.getByText('0 Select')).toBeInTheDocument(); + await waitFor(() => + expect( + fetchMock.callHistory + .calls() + .some(call => + new URL(call.url).searchParams.get('q')?.includes('page:1'), + ), + ).toBe(true), + ); +}); + +test('searching clears selection and delete-all remains connection-wide', async () => { + renderModal(); + await screen.findByText('user1'); + await userEvent.click(screen.getAllByRole('checkbox')[1]); + await userEvent.type( + screen.getByPlaceholderText('Search by username, name, or email'), + 'First1', + ); + expect(screen.getByText('0 Select')).toBeInTheDocument(); + await waitFor(() => + expect( + fetchMock.callHistory.calls().some(call => { + const query = new URL(call.url).searchParams.get('q'); + return ( + query && + decodeListQuery(query).filters?.some( + filter => filter.value === 'First1', + ) + ); + }), + ).toBe(true), + ); + await waitFor(() => + expect( + screen.getByRole('button', { name: 'Delete all tokens' }), + ).toBeEnabled(), + ); + const totalRequests = fetchMock.callHistory.calls().filter(call => { + const query = new URL(call.url).searchParams.get('q'); + return query && decodeListQuery(query).page_size === 1; + }); + expect(totalRequests).toHaveLength(1); + await userEvent.click( + screen.getByRole('button', { name: 'Delete all tokens' }), + ); + expect( + screen.getByText(/Delete all 50 stored credential pairs/), + ).toHaveTextContent('including users outside this page and search'); + await confirmDeletion(); + await waitFor(() => expect(addSuccessToast).toHaveBeenCalled()); + expect( + fetchMock.callHistory.lastCall(`glob:*${endpoint}all/`)?.options.body, + ).toBeUndefined(); + expect(fetchMock.callHistory.calls(`glob:*${endpoint}delete/`)).toHaveLength( + 0, + ); +}); + +test('a failed delete retains selection and permits retry', async () => { + fetchMock.removeRoutes(); + fetchMock.get(`glob:*${endpoint}?*`, { + count: 1, + + result: [token(1)], + }); + fetchMock.post(`glob:*${endpoint}delete/`, 500); + renderModal(); + await screen.findByText('user1'); + await userEvent.click(screen.getAllByRole('checkbox')[1]); + await userEvent.click( + screen.getByRole('button', { name: 'Delete selected' }), + ); + await confirmDeletion(); + await waitFor(() => expect(addDangerToast).toHaveBeenCalled()); + expect(screen.getByText('1 Selected')).toBeInTheDocument(); + expect( + screen + .getByText('Delete OAuth2 tokens?') + .closest('[role=dialog]') as HTMLElement, + ).toBeInTheDocument(); + expect(addSuccessToast).not.toHaveBeenCalled(); +}); + +test('failed listing disables deletion and offers retry', async () => { + fetchMock.removeRoutes(); + fetchMock.get(`glob:*${endpoint}?*`, 500); + renderModal(); + await screen.findByText('Failed to load OAuth2 tokens.'); + expect( + screen.getByRole('button', { name: 'Delete all tokens' }), + ).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Retry' })).toBeEnabled(); +}); diff --git a/superset-frontend/src/features/databases/OAuth2TokensModal.tsx b/superset-frontend/src/features/databases/OAuth2TokensModal.tsx new file mode 100644 index 00000000000..22778148cc5 --- /dev/null +++ b/superset-frontend/src/features/databases/OAuth2TokensModal.tsx @@ -0,0 +1,339 @@ +/** + * 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 { useEffect, useState, type Key } from 'react'; +import rison from 'rison'; +import { t, tn } from '@apache-superset/core/translation'; +import { Alert } from '@apache-superset/core/components'; +import { SupersetClient } from '@superset-ui/core'; +import { + ActionButton, + Button, + DeleteModal, + Flex, + Input, + Modal, + Space, + Table, +} from '@superset-ui/core/components'; +import { Icons } from '@superset-ui/core/components/Icons'; +import type { DatabaseObject } from './types'; + +type TokenMetadata = { + id: number; + user: { + username: string; + first_name: string; + last_name: string; + email: string; + }; + access_token_expiration: string | null; +}; + +type TokenPage = { + count: number; + result: TokenMetadata[]; +}; + +type Deletion = { token_ids: number[] } | { all: true }; + +type Props = { + database: DatabaseObject; + onHide: () => void; + addSuccessToast: (message: string) => void; + addDangerToast: (message: string) => void; +}; + +const PAGE_SIZE = 25; + +/** Manage stored OAuth2 credentials without exposing access or refresh tokens. */ +export default function OAuth2TokensModal({ + database, + onHide, + addSuccessToast, + addDangerToast, +}: Props) { + const [search, setSearch] = useState(''); + const [page, setPage] = useState(0); + const [data, setData] = useState<TokenPage>({ + count: 0, + result: [], + }); + const [totalCount, setTotalCount] = useState<number | null>(null); + const [totalError, setTotalError] = useState(false); + const [selectedKeys, setSelectedKeys] = useState<Key[]>([]); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(false); + const [reload, setReload] = useState(0); + const [deletion, setDeletion] = useState<Deletion | null>(null); + const [deleting, setDeleting] = useState(false); + const endpoint = `/api/v1/database/${database.id}/oauth2_tokens/`; + + useEffect(() => { + let active = true; + setTotalCount(null); + setTotalError(false); + const query = rison.encode_uri({ page: 0, page_size: 1 }); + SupersetClient.get({ endpoint: `${endpoint}?q=${query}` }) + .then(({ json }) => { + if (active) setTotalCount(json.count); + }) + .catch(() => { + if (active) setTotalError(true); + }); + return () => { + active = false; + }; + }, [endpoint, reload]); + + useEffect(() => { + let active = true; + setLoading(true); + setError(false); + const timer = window.setTimeout( + () => { + const query = rison.encode_uri({ + page, + page_size: PAGE_SIZE, + filters: search + ? [{ col: 'id', opr: 'oauth2_tokens_user', value: search }] + : [], + }); + SupersetClient.get({ endpoint: `${endpoint}?q=${query}` }) + .then(({ json: response }) => { + const json = response as TokenPage; + if (!active) return; + const lastPage = Math.max(0, Math.ceil(json.count / PAGE_SIZE) - 1); + if (page > lastPage) { + setPage(lastPage); + return; + } + setData(json); + setLoading(false); + }) + .catch(() => { + if (!active) return; + setError(true); + setLoading(false); + }); + }, + search ? 300 : 0, + ); + return () => { + active = false; + window.clearTimeout(timer); + }; + }, [endpoint, page, search, reload]); + + const confirmDelete = async () => { + if (!deletion || deleting) return; + setDeleting(true); + try { + await SupersetClient.post( + 'all' in deletion + ? { endpoint: `${endpoint}all/` } + : { endpoint: `${endpoint}delete/`, jsonPayload: deletion }, + ); + addSuccessToast( + t( + 'OAuth2 tokens deleted. Affected users must authorize this connection again.', + ), + ); + setSelectedKeys([]); + setDeletion(null); + setLoading(true); + setReload(value => value + 1); + } catch { + addDangerToast(t('Failed to delete OAuth2 tokens. Please try again.')); + } finally { + setDeleting(false); + } + }; + + const controlsDisabled = loading || deleting || deletion !== null || error; + const deletionCount = + deletion && 'all' in deletion + ? (totalCount ?? 0) + : (deletion?.token_ids.length ?? 0); + + return ( + <> + <Modal + show + onHide={() => { + if (!deleting) onHide(); + }} + name="oauth2-tokens" + title={t('Manage OAuth2 tokens: %s', database.database_name)} + width={800} + footer={ + <Button onClick={onHide} disabled={deleting}> + {t('Close')} + </Button> + } + > + <Space direction="vertical" size="middle" style={{ width: '100%' }}> + <Alert + type="info" + showIcon + message={t( + 'Deleting credentials removes both access and refresh tokens from Superset. Affected users must authorize this connection again. Tokens are not revoked at the OAuth2 provider.', + )} + /> + <Input.Search + aria-label={t('Search OAuth2 tokens')} + placeholder={t('Search by username, name, or email')} + value={search} + maxLength={250} + disabled={deleting || deletion !== null} + allowClear + onChange={event => { + setSearch(event.target.value); + setPage(0); + setSelectedKeys([]); + setLoading(true); + }} + /> + <Flex justify="space-between" align="center" gap="small" wrap> + <Space wrap> + <span> + {selectedKeys.length === 0 + ? t('%s Select', 0) + : t('%s Selected', selectedKeys.length)} + </span> + <Button + buttonStyle="danger" + disabled={controlsDisabled || selectedKeys.length === 0} + onClick={() => + setDeletion({ token_ids: selectedKeys.map(Number) }) + } + > + {t('Delete selected')} + </Button> + </Space> + <Button + buttonStyle="danger" + disabled={ + controlsDisabled || totalCount === null || totalCount === 0 + } + onClick={() => setDeletion({ all: true })} + > + {t('Delete all tokens')} + </Button> + </Flex> + {(error || totalError) && ( + <Alert + type="error" + message={t('Failed to load OAuth2 tokens.')} + action={ + <Button onClick={() => setReload(value => value + 1)}> + {t('Retry')} + </Button> + } + /> + )} + <Table<TokenMetadata> + rowKey="id" + data={error ? [] : data.result} + loading={loading} + rowSelection={{ + selectedRowKeys: selectedKeys, + onChange: setSelectedKeys, + getCheckboxProps: () => ({ disabled: controlsDisabled }), + }} + pagination={{ + current: page + 1, + pageSize: PAGE_SIZE, + total: data.count, + showSizeChanger: false, + disabled: deleting || deletion !== null, + onChange: nextPage => { + setPage(nextPage - 1); + setSelectedKeys([]); + setLoading(true); + }, + }} + columns={[ + { title: t('Username'), dataIndex: ['user', 'username'] }, + { + title: t('Name'), + key: 'name', + render: (_value: unknown, row: TokenMetadata) => + [row.user.first_name, row.user.last_name] + .filter(Boolean) + .join(' '), + }, + { title: t('Email'), dataIndex: ['user', 'email'] }, + { + title: t('Expires on'), + dataIndex: 'access_token_expiration', + render: (value: string | null) => + value ? new Date(value).toLocaleString() : t('Unknown'), + }, + { + title: t('Actions'), + key: 'actions', + render: (_value: unknown, row: TokenMetadata) => ( + <ActionButton + label={t('Delete OAuth2 tokens for %s', row.user.username)} + tooltip={t( + 'Delete OAuth2 tokens for %s', + row.user.username, + )} + icon={<Icons.DeleteOutlined />} + disabled={controlsDisabled} + onClick={() => setDeletion({ token_ids: [row.id] })} + /> + ), + }, + ]} + /> + </Space> + </Modal> + {deletion && ( + <DeleteModal + open + onHide={() => { + if (!deleting) setDeletion(null); + }} + onConfirm={confirmDelete} + loading={deleting} + title={t('Delete OAuth2 tokens?')} + name="delete-oauth2-tokens" + description={ + 'all' in deletion + ? tn( + 'Delete all %s stored credential pair for "%s", including users outside this page and search? Affected users must authorize this connection again.', + 'Delete all %s stored credential pairs for "%s", including users outside this page and search? Affected users must authorize this connection again.', + deletionCount, + deletionCount, + database.database_name, + ) + : tn( + 'Delete %s stored credential pair for "%s"? Affected users must authorize this connection again.', + 'Delete %s stored credential pairs for "%s"? Affected users must authorize this connection again.', + deletionCount, + deletionCount, + database.database_name, + ) + } + /> + )} + </> + ); +} diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts index 162980f7f81..e07e8e03e97 100644 --- a/superset-frontend/src/features/databases/types.ts +++ b/superset-frontend/src/features/databases/types.ts @@ -117,6 +117,7 @@ export type DatabaseObject = { disable_ssh_tunneling?: boolean; supports_dynamic_catalog?: boolean; supports_oauth2?: boolean; + oauth2_enabled?: boolean; }; // SSH Tunnel information diff --git a/superset-frontend/src/pages/DatabaseList/DatabaseList.oauth2Tokens.test.tsx b/superset-frontend/src/pages/DatabaseList/DatabaseList.oauth2Tokens.test.tsx new file mode 100644 index 00000000000..3a6bfb9fe3d --- /dev/null +++ b/superset-frontend/src/pages/DatabaseList/DatabaseList.oauth2Tokens.test.tsx @@ -0,0 +1,135 @@ +/** + * 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 fetchMock from 'fetch-mock'; +import { FeatureFlag } from '@superset-ui/core'; +import { configureStore } from '@reduxjs/toolkit'; +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import DatabaseList from 'src/pages/DatabaseList'; + +const uuid = '846ddf91-9cc3-4fb7-a218-e9d26ca5fe39'; + +afterEach(() => { + window.featureFlags = {}; + fetchMock.clearHistory().removeRoutes(); +}); + +test.each([ + [true, true, false, 'database', true], + [false, true, false, 'database', false], + [true, false, false, 'database', false], + [true, true, true, 'database', true], + [false, true, true, 'database', false], + [true, false, true, 'database', false], + [true, true, true, 'semantic_layer', false], +] as const)( + 'OAuth2=%s, permission=%s, semantic layers=%s, source=%s controls visibility', + async (enabled, permitted, semanticLayers, sourceType, visible) => { + window.featureFlags = { + ['SEMANTIC_LAYERS' as FeatureFlag]: semanticLayers, + }; + fetchMock.clearHistory().removeRoutes(); + fetchMock.get('glob:*/api/v1/database/_info*', { + permissions: ['can_read', 'can_write'], + }); + fetchMock.get('glob:*/api/v1/database/related/*', { result: [], count: 0 }); + const rowResponse = { + count: 1, + result: [ + { + id: 1, + uuid, + source_type: sourceType, + sl_type: 'demo', + database_name: 'OAuth database', + backend: 'trino', + changed_by: null, + engine_information: { + supports_oauth2: true, + oauth2_enabled: enabled, + }, + }, + ], + }; + fetchMock.get('glob:*/api/v1/database/?q=*', rowResponse); + fetchMock.get('glob:*/api/v1/semantic_layer/connections/?q=*', rowResponse); + fetchMock.get('glob:*/api/v1/database/1/oauth2_tokens/?*', { + count: 0, + total_count: 0, + result: [], + }); + const user = { + userId: 1, + firstName: 'Custom', + lastName: 'User', + roles: { + Custom: [ + ['can_read', 'Database'], + ['can_write', 'Database'], + ['can_read', 'SemanticLayer'], + ['can_write', 'SemanticLayer'], + ...(permitted ? [['can_manage_oauth2_tokens', 'Database']] : []), + ], + }, + permissions: {}, + isActive: true, + email: '[email protected]', + createdOn: '2026-01-01T00:00:00', + }; + const store = configureStore({ + reducer: { + user: (state = user) => state, + common: ( + state = { + conf: { + CSV_EXTENSIONS: ['csv'], + EXCEL_EXTENSIONS: ['xls'], + COLUMNAR_EXTENSIONS: ['parquet'], + ALLOWED_EXTENSIONS: ['csv', 'xls', 'parquet'], + SYNC_DB_PERMISSIONS_IN_ASYNC_MODE: false, + }, + }, + ) => state, + }, + middleware: getDefaultMiddleware => + getDefaultMiddleware({ + serializableCheck: false, + immutableCheck: false, + }), + }); + render(<DatabaseList user={user} />, { + store, + useQueryParams: true, + useRouter: true, + }); + await screen.findByText('OAuth database'); + if (visible) { + await userEvent.click( + screen.getByRole('button', { name: 'Manage OAuth2 tokens' }), + ); + expect( + await screen.findByText('Manage OAuth2 tokens: OAuth database'), + ).toBeInTheDocument(); + } else { + expect( + screen.queryByRole('button', { name: 'Manage OAuth2 tokens' }), + ).not.toBeInTheDocument(); + } + }, +); diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx index c0fdb53dcdd..8b2fa2edb00 100644 --- a/superset-frontend/src/pages/DatabaseList/index.tsx +++ b/superset-frontend/src/pages/DatabaseList/index.tsx @@ -66,6 +66,7 @@ import { ExtensionConfigs } from 'src/features/home/types'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import type { MenuObjectProps } from 'src/types/bootstrapTypes'; import DatabaseModal from 'src/features/databases/DatabaseModal'; +import OAuth2TokensModal from 'src/features/databases/OAuth2TokensModal'; import UploadDataModal from 'src/features/databases/UploadDataModel'; import SemanticLayerModal from 'src/features/semanticLayers/SemanticLayerModal'; import { DatabaseObject } from 'src/features/databases/types'; @@ -270,6 +271,9 @@ function DatabaseList({ user, }: DatabaseListProps) { const theme = useTheme(); + const [tokensDatabase, setTokensDatabase] = useState<DatabaseObject | null>( + null, + ); const showSemanticLayers = isFeatureEnabled(SEMANTIC_LAYERS_FLAG); const fullUser = useSelector< @@ -281,6 +285,11 @@ function DatabaseList({ 'Database', fullUser.roles, ); + const canManageOAuth2Tokens = findPermission( + 'can_manage_oauth2_tokens', + 'Database', + fullUser.roles, + ); const canReadLayer = findPermission( 'can_read', 'SemanticLayer', @@ -939,7 +948,7 @@ function DatabaseList({ const handleDelete = () => openDatabaseDeleteModal(original); const handleExport = () => handleDatabaseExport(original); const handleSync = () => handleDatabasePermSync(original); - if (!canEdit && !canDelete && !canExport) { + if (!canEdit && !canDelete && !canExport && !canManageOAuth2Tokens) { return null; } return ( @@ -976,6 +985,17 @@ function DatabaseList({ onClick={handleSync} /> )} + {canManageOAuth2Tokens && + original.engine_information?.oauth2_enabled && ( + <ActionButton + label={t('Manage OAuth2 tokens')} + tooltip={t('Manage OAuth2 tokens')} + placement="bottom" + icon={<Icons.KeyOutlined iconSize="l" />} + dataTest="database-oauth2-tokens" + onClick={() => setTokensDatabase(original)} + /> + )} {canDelete && ( <ActionButton label={t('Delete %s', databaseLabelLower())} @@ -991,7 +1011,12 @@ function DatabaseList({ }, Header: t('Actions'), id: 'actions', - hidden: !canEdit && !canDelete && !canExport && !canWriteLayer, + hidden: + !canEdit && + !canDelete && + !canExport && + !canWriteLayer && + !canManageOAuth2Tokens, disableSortBy: true, }, { @@ -1010,6 +1035,7 @@ function DatabaseList({ canDelete, canEdit, canExport, + canManageOAuth2Tokens, handleDatabaseEditModal, handleDatabaseExport, handleDatabasePermSync, @@ -1115,6 +1141,15 @@ function DatabaseList({ return ( <> <SubMenu {...menuData} /> + {tokensDatabase && ( + <OAuth2TokensModal + key={tokensDatabase.id} + database={tokensDatabase} + onHide={() => setTokensDatabase(null)} + addDangerToast={addDangerToast} + addSuccessToast={addSuccessToast} + /> + )} <DatabaseModal databaseId={currentDatabase?.id} show={databaseModalOpen} diff --git a/superset/commands/database/exceptions.py b/superset/commands/database/exceptions.py index bb36f3962fc..1aa1bca72f9 100644 --- a/superset/commands/database/exceptions.py +++ b/superset/commands/database/exceptions.py @@ -83,6 +83,15 @@ class DatabaseNotFoundError(CommandException): message = _("Database not found.") +class OAuth2TokenNotFoundError(CommandException): + status = 404 + message = _("One or more OAuth2 tokens were not found for this database.") + + +class OAuth2TokensDeleteFailedError(DeleteFailedError): + message = _("OAuth2 tokens could not be deleted.") + + class DatabaseSchemaNotFoundError(CommandException): status = 404 message = _("Schema not found.") diff --git a/superset/commands/database/oauth2.py b/superset/commands/database/oauth2.py index dc019226269..8513f3fd7ee 100644 --- a/superset/commands/database/oauth2.py +++ b/superset/commands/database/oauth2.py @@ -26,8 +26,12 @@ from marshmallow import ValidationError from superset import db, security_manager from superset.commands.base import BaseCommand -from superset.commands.database.exceptions import DatabaseNotFoundError -from superset.daos.database import DatabaseUserOAuth2TokensDAO +from superset.commands.database.exceptions import ( + DatabaseNotFoundError, + OAuth2TokenNotFoundError, + OAuth2TokensDeleteFailedError, +) +from superset.daos.database import DatabaseDAO, DatabaseUserOAuth2TokensDAO from superset.daos.key_value import KeyValueDAO from superset.databases.schemas import OAuth2ProviderResponseSchema from superset.exceptions import OAuth2Error, OAuth2RejectedError @@ -151,3 +155,47 @@ class OAuth2StoreTokenCommand(BaseCommand): self._database = database else: raise DatabaseNotFoundError("Database not found") + + +class OAuth2DeleteTokensCommand(BaseCommand): + """ + Command to delete OAuth2 tokens for a DB connection. + + Supports deleting specific tokens by ID(s) or all existing tokens. + """ + + def __init__( + self, + database_id: int, + token_ids: list[int] | None = None, + *, + delete_all: bool = False, + ) -> None: + self._database_id = database_id + self._token_ids = token_ids + self._delete_all = delete_all + self._database: Database | None = None + self._tokens: list[DatabaseUserOAuth2Tokens] = [] + + @transaction(on_error=partial(on_error, reraise=OAuth2TokensDeleteFailedError)) + def run(self) -> None: + self.validate() + if self._delete_all: + assert self._database is not None + DatabaseUserOAuth2TokensDAO.delete_all_tokens(self._database.id) + else: + DatabaseUserOAuth2TokensDAO.delete(self._tokens) + + def validate(self) -> None: + if self._delete_all == (self._token_ids is not None): + raise ValueError("Specify token_ids or delete_all=True, exclusively.") + self._database = DatabaseDAO.find_by_id(self._database_id) + if self._database is None: + raise DatabaseNotFoundError() + if self._token_ids is not None: + token_ids = list(dict.fromkeys(self._token_ids)) + self._tokens = DatabaseUserOAuth2TokensDAO.find_by_database_and_ids( + self._database.id, token_ids + ) + if not self._tokens or len(self._tokens) != len(token_ids): + raise OAuth2TokenNotFoundError() diff --git a/superset/daos/database.py b/superset/daos/database.py index 0c6f215c7ae..20577e7ee3b 100644 --- a/superset/daos/database.py +++ b/superset/daos/database.py @@ -377,6 +377,27 @@ class DatabaseUserOAuth2TokensDAO(BaseDAO[DatabaseUserOAuth2Tokens]): DAO for OAuth2 tokens. """ + @classmethod + def find_by_database_and_ids( + cls, database_id: int, token_ids: list[int] + ) -> list[DatabaseUserOAuth2Tokens]: + """Get selected OAuth2 tokens belonging to a DB connection in a single query.""" + if not token_ids: + return [] + return cls.query( + db.session.query(DatabaseUserOAuth2Tokens).filter( + DatabaseUserOAuth2Tokens.database_id == database_id, + DatabaseUserOAuth2Tokens.id.in_(token_ids), + ) + ) + + @classmethod + def delete_all_tokens(cls, database_id: int) -> None: + """Delete credentials in bulk without loading every user's token secrets.""" + db.session.query(DatabaseUserOAuth2Tokens).filter_by( + database_id=database_id + ).delete(synchronize_session=False) + @classmethod def get_database(cls, database_id: int) -> Database | None: """ diff --git a/superset/databases/api.py b/superset/databases/api.py index d38f44c497b..1eb0fc08572 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -33,6 +33,7 @@ from flask import ( Response, ) from flask_appbuilder.api import expose, protect, rison as parse_rison, safe +from flask_appbuilder.api.schemas import get_list_schema from flask_appbuilder.models.sqla.interface import SQLAInterface from marshmallow import ValidationError from sqlalchemy.exc import NoSuchTableError, OperationalError, SQLAlchemyError @@ -49,10 +50,15 @@ from superset.commands.database.exceptions import ( DatabaseNotFoundError, DatabaseUpdateFailedError, InvalidParametersError, + OAuth2TokenNotFoundError, + OAuth2TokensDeleteFailedError, ) from superset.commands.database.export import ExportDatabasesCommand from superset.commands.database.importers.dispatcher import ImportDatabasesCommand -from superset.commands.database.oauth2 import OAuth2StoreTokenCommand +from superset.commands.database.oauth2 import ( + OAuth2DeleteTokensCommand, + OAuth2StoreTokenCommand, +) from superset.commands.database.ssh_tunnel.exceptions import ( SSHTunnelDatabasePortError, SSHTunnelHostKeyVerificationError, @@ -83,7 +89,11 @@ from superset.commands.importers.v1.utils import ( from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.daos.database import DatabaseDAO from superset.databases.decorators import check_table_access -from superset.databases.filters import DatabaseFilter, DatabaseUploadEnabledFilter +from superset.databases.filters import ( + DatabaseFilter, + DatabaseOAuth2TokensUserFilter, + DatabaseUploadEnabledFilter, +) from superset.databases.schemas import ( CatalogsResponseSchema, database_catalogs_query_schema, @@ -91,6 +101,7 @@ from superset.databases.schemas import ( database_tables_query_schema, DatabaseConnectionSchema, DatabaseFunctionNamesResponse, + DatabaseOAuth2TokensDeleteSchema, DatabasePostSchema, DatabasePutSchema, DatabaseRelatedObjectsResponse, @@ -125,7 +136,7 @@ from superset.exceptions import ( TableNotFoundException, ) from superset.extensions import security_manager -from superset.models.core import Database +from superset.models.core import Database, DatabaseUserOAuth2Tokens from superset.sql.parse import Partition, Table from superset.superset_typing import FlaskResponse from superset.utils.core import ( @@ -2137,3 +2148,195 @@ class DatabaseRestApi(BaseSupersetModelRestApi): database, database.get_default_catalog(), schemas_allowed, True ) return self.response(200, schemas=schemas_allowed_processed) + + +class DatabaseOAuth2TokensRestApi(BaseSupersetModelRestApi): + datamodel = SQLAInterface(DatabaseUserOAuth2Tokens) + resource_name = "database" + class_permission_name = "Database" + method_permission_name = { + "get_list": "manage_oauth2_tokens", + "delete_oauth2_tokens": "manage_oauth2_tokens", + "delete_all_oauth2_tokens": "manage_oauth2_tokens", + } + include_route_methods = { + RouteMethod.GET_LIST, + "delete_oauth2_tokens", + "delete_all_oauth2_tokens", + } + openapi_spec_component_schemas = (DatabaseOAuth2TokensDeleteSchema,) + allow_browser_login = True + openapi_spec_tag = "Database" + openapi_spec_methods = { + "get_list": { + "get": { + "summary": "List OAuth2 credential metadata for a database", + "parameters": [ + { + "in": "path", + "name": "pk", + "required": True, + "schema": {"type": "integer"}, + }, + { + "in": "query", + "name": "q", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_list_schema" + } + } + }, + }, + ], + }, + }, + } + list_columns = [ + "id", + "user.username", + "user.first_name", + "user.last_name", + "user.email", + "access_token_expiration", + ] + list_select_columns = list_columns + order_columns = ["id", "user.username", "access_token_expiration"] + base_order = ("id", "asc") + search_columns = ["id", "database"] + search_filters = {"id": [DatabaseOAuth2TokensUserFilter]} + max_page_size = 100 + + @expose("/<int:pk>/oauth2_tokens/", methods=("GET",)) + @protect() + @safe + @statsd_metrics + @parse_rison(get_list_schema) + def get_list( # pylint: disable=arguments-differ + self, pk: int, **kwargs: Any + ) -> FlaskResponse: + """List OAuth2 token metadata scoped to the accessible database.""" + database = DatabaseDAO.find_by_id(pk) + if database is None: + return self.response_404() + kwargs["rison"].setdefault("filters", []).append( + {"col": "database", "opr": "rel_o_m", "value": database.id} + ) + return self.get_list_headless(**kwargs) + + def pre_get_list(self, data: dict[str, Any]) -> None: + """ + Include timezone offsets when serializing stored local expiration times. + + Tokens are stored with a naive `datetime.now()`, so this method tries to + fix the timezone difference between the user and the serve. + """ + for row in data["result"]: + if expiration := row.get("access_token_expiration"): + row["access_token_expiration"] = ( + datetime.fromisoformat(expiration).astimezone().isoformat() + ) + + @expose("/<int:pk>/oauth2_tokens/delete/", methods=["POST"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, + *args, + **kwargs: f"{self.__class__.__name__}.delete_oauth2_tokens", + log_to_statsd=False, + ) + @requires_json + def delete_oauth2_tokens(self, pk: int) -> FlaskResponse: + """Delete specific OAuth2 tokens for a DB connection. + --- + post: + summary: Delete OAuth2 access and refresh tokens for a DB connection. + parameters: + - in: path + name: pk + required: true + schema: + type: integer + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/DatabaseOAuth2TokensDeleteSchema' + responses: + 200: + description: OAuth2 credentials deleted successfully + 400: + $ref: '#/components/responses/400' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + """ + try: + parameters = DatabaseOAuth2TokensDeleteSchema().load(request.get_json()) + except ValidationError as error: + return self.response_400(message=error.messages) + try: + OAuth2DeleteTokensCommand(pk, token_ids=parameters["token_ids"]).run() + except (DatabaseNotFoundError, OAuth2TokenNotFoundError): + return self.response_404() + except OAuth2TokensDeleteFailedError as ex: + logger.error( + "Error deleting OAuth2 tokens for database %s", + pk, + exc_info=True, + ) + return self.response_422(message=str(ex)) + return self.response(200, message="OK") + + @expose("/<int:pk>/oauth2_tokens/all/", methods=["POST"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, + *args, + **kwargs: f"{self.__class__.__name__}.delete_all_oauth2_tokens", + log_to_statsd=False, + ) + def delete_all_oauth2_tokens(self, pk: int) -> FlaskResponse: + """Delete all OAuth2 tokens for a DB connection. + --- + post: + summary: Delete all OAuth2 access and refresh tokens for a DB connection. + parameters: + - in: path + name: pk + required: true + schema: + type: integer + responses: + 200: + description: All OAuth2 credentials deleted successfully + 400: + $ref: '#/components/responses/400' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + """ + try: + OAuth2DeleteTokensCommand(pk, delete_all=True).run() + except DatabaseNotFoundError: + return self.response_404() + except OAuth2TokensDeleteFailedError as ex: + logger.error( + "Error deleting OAuth2 tokens for database %s", + pk, + exc_info=True, + ) + return self.response_422(message=str(ex)) + return self.response(200, message="OK") diff --git a/superset/databases/filters.py b/superset/databases/filters.py index 8afc3acbe28..cff0086f7a9 100644 --- a/superset/databases/filters.py +++ b/superset/databases/filters.py @@ -18,7 +18,7 @@ from typing import Any from flask import current_app, g from flask_babel import lazy_gettext as _ -from sqlalchemy import or_ +from sqlalchemy import func, or_ from sqlalchemy.orm import Query from sqlalchemy.sql.expression import cast from sqlalchemy.sql.sqltypes import JSON @@ -105,3 +105,35 @@ class DatabaseUploadEnabledFilter(BaseFilter): # pylint: disable=too-few-public cast(Database.extra, JSON)["schemas_allowed_for_file_upload"] != [], ) ) + + +class DatabaseOAuth2TokensUserFilter(BaseFilter): + """Search credential owners using the standard FAB filtering protocol.""" + + name = _("User") + arg_name = "oauth2_tokens_user" + + def apply(self, query: Query, value: Any) -> Query: + """Match username, full name or email without selecting token secrets.""" + if not value: + return query + user = security_manager.user_model + full_name = func.trim( + func.coalesce(user.first_name, "") + " " + func.coalesce(user.last_name, "") + ) + return query.filter( + self.model.user.has( + or_( + *( + column.ilike(f"%{value}%") + for column in ( + user.username, + user.first_name, + user.last_name, + user.email, + full_name, + ) + ) + ) + ) + ) diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index 1e64b31d6ee..13f4f3fcbf3 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -1221,6 +1221,9 @@ class IdentifierQuoteSchema(Schema): class EngineInformationSchema(Schema): + oauth2_enabled = fields.Boolean( + metadata={"description": "OAuth2 authentication is enabled on this connection"} + ) supports_file_upload = fields.Boolean( metadata={"description": "Users can upload files to the database"} ) @@ -1634,3 +1637,13 @@ class QualifiedTableSchema(Schema): load_default=None, metadata={"description": "The table catalog"}, ) + + +class DatabaseOAuth2TokensDeleteSchema(Schema): + """Require a nonempty list of credential IDs for selected deletion.""" + + token_ids = fields.List( + fields.Integer(strict=True, validate=Range(min=1)), + required=True, + validate=Length(min=1, max=100), + ) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index d11ae366634..f9353872e76 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -209,7 +209,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods from superset.dashboards.api import DashboardRestApi from superset.dashboards.filter_state.api import DashboardFilterStateRestApi from superset.dashboards.permalink.api import DashboardPermalinkRestApi - from superset.databases.api import DatabaseRestApi + from superset.databases.api import DatabaseOAuth2TokensRestApi, DatabaseRestApi from superset.datasets.api import DatasetRestApi from superset.datasets.columns.api import DatasetColumnsRestApi from superset.datasets.metrics.api import DatasetMetricRestApi @@ -304,6 +304,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods appbuilder.add_api(DashboardPermalinkRestApi) appbuilder.add_api(DashboardRestApi) appbuilder.add_api(DatabaseRestApi) + appbuilder.add_api(DatabaseOAuth2TokensRestApi) appbuilder.add_api(DatasetRestApi) appbuilder.add_api(DatasetColumnsRestApi) appbuilder.add_api(DatasetMetricRestApi) diff --git a/superset/migrations/versions/2026-10-22_12-00_f72b6e9c104a_add_oauth2_token_management_indexes.py b/superset/migrations/versions/2026-10-22_12-00_f72b6e9c104a_add_oauth2_token_management_indexes.py new file mode 100644 index 00000000000..04a290e6ffe --- /dev/null +++ b/superset/migrations/versions/2026-10-22_12-00_f72b6e9c104a_add_oauth2_token_management_indexes.py @@ -0,0 +1,40 @@ +# 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. + +"""Add a database lookup index for OAuth2 token management. + +Revision ID: f72b6e9c104a +Revises: d623a0cb6bb0 +""" + +from superset.migrations.shared.utils import create_index, drop_index + +revision: str = "f72b6e9c104a" +down_revision: str = "d623a0cb6bb0" + +TABLE_NAME = "database_user_oauth2_tokens" +INDEX_NAME = "idx_oauth2_tokens_database_id" + + +def upgrade() -> None: + """Index connection-scoped token listings, counts, and deletions.""" + create_index(TABLE_NAME, INDEX_NAME, ["database_id"]) + + +def downgrade() -> None: + """Remove the lookup index while preserving credentials and uniqueness.""" + drop_index(TABLE_NAME, INDEX_NAME) diff --git a/superset/models/core.py b/superset/models/core.py index e35d887d753..6e6a8c7f483 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -498,6 +498,7 @@ class Database(CoreDatabase, AuditMixinNullable, ImportExportMixin): # pylint: def engine_information(self) -> dict[str, Any]: try: engine_information = self.db_engine_spec.get_public_information() + engine_information["oauth2_enabled"] = self.is_oauth2_enabled() except Exception: # pylint: disable=broad-except engine_information = {} return engine_information @@ -1759,6 +1760,7 @@ class DatabaseUserOAuth2Tokens(Model, AuditMixinNullable): __tablename__ = "database_user_oauth2_tokens" __table_args__ = ( sqla.Index("idx_user_id_database_id", "user_id", "database_id", unique=True), + sqla.Index("idx_oauth2_tokens_database_id", "database_id"), ) id = Column(Integer, primary_key=True) diff --git a/superset/security/manager.py b/superset/security/manager.py index 1e30fc85de0..3bd7cba04de 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -2156,6 +2156,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods } ADMIN_ONLY_PERMISSIONS = { + "can_manage_oauth2_tokens", "update_roles_users", "list_roles", "can_update_role", diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 165bbf4126d..570b2dd3c30 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -1331,6 +1331,7 @@ class SemanticLayerRestApi(_RuntimeGuardedApiMixin, BaseSupersetApi): Database.uuid, Database.database_name, Database.sqlalchemy_uri, + Database.encrypted_extra, Database.allow_run_async, Database.allow_dml, Database.allow_file_upload, @@ -1414,6 +1415,7 @@ class SemanticLayerRestApi(_RuntimeGuardedApiMixin, BaseSupersetApi): "uuid": str(obj.uuid), "database_name": obj.database_name, "backend": obj.backend, + "engine_information": obj.engine_information, "allow_run_async": obj.allow_run_async, "allow_dml": obj.allow_dml, "allow_file_upload": obj.allow_file_upload, diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py index 119474e69e6..b5e7b488cd6 100644 --- a/tests/unit_tests/databases/api_test.py +++ b/tests/unit_tests/databases/api_test.py @@ -333,6 +333,7 @@ def test_database_connection( "database_name": "my_database", "driver": "gsheets", "engine_information": { + "oauth2_enabled": False, "disable_ssh_tunneling": True, "supports_dynamic_catalog": False, "supports_file_upload": True, @@ -429,6 +430,7 @@ def test_database_connection( "database_name": "my_database", "driver": "gsheets", "engine_information": { + "oauth2_enabled": False, "disable_ssh_tunneling": True, "supports_dynamic_catalog": False, "supports_file_upload": True, diff --git a/tests/unit_tests/databases/oauth2_tokens_test.py b/tests/unit_tests/databases/oauth2_tokens_test.py new file mode 100644 index 00000000000..c1e9193e2bc --- /dev/null +++ b/tests/unit_tests/databases/oauth2_tokens_test.py @@ -0,0 +1,511 @@ +# 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 datetime import datetime +from typing import Any +from uuid import uuid4 + +import prison +import pytest +from marshmallow import ValidationError +from pytest_mock import MockerFixture +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session + +from superset import security_manager +from superset.commands.database.exceptions import ( + DatabaseNotFoundError, + OAuth2TokenNotFoundError, + OAuth2TokensDeleteFailedError, +) +from superset.commands.database.oauth2 import ( + OAuth2DeleteTokensCommand, +) +from superset.daos.database import DatabaseDAO, DatabaseUserOAuth2TokensDAO +from superset.databases.api import DatabaseOAuth2TokensRestApi, DatabaseRestApi +from superset.databases.schemas import DatabaseOAuth2TokensDeleteSchema +from superset.models.core import Database, DatabaseUserOAuth2Tokens + +DATABASE_ID = 1 +ENDPOINT = f"/api/v1/database/{DATABASE_ID}/oauth2_tokens/" + + [email protected] +def token_rows( + session: Session, mocker: MockerFixture +) -> list[DatabaseUserOAuth2Tokens]: + """Create credentials on the unmodified metadata schema for two connections.""" + Database.metadata.create_all(session.get_bind()) + mocker.patch.object(DatabaseRestApi.datamodel, "_session", session) + mocker.patch.object(DatabaseOAuth2TokensRestApi.datamodel, "_session", session) + user_model = security_manager.user_model + for user_id in range(1, 5): + session.execute( + user_model.__table__.insert().values( + id=user_id, + username=f"user{user_id}", + first_name=f"First{user_id}", + last_name="Example", + email=f"user{user_id}@example.com", + active=True, + ) + ) + for database_id in (1, 2): + session.execute( + Database.__table__.insert().values( + id=database_id, + database_name=f"db{database_id}", + sqlalchemy_uri="sqlite://", + uuid=uuid4(), + ) + ) + tokens = [ + DatabaseUserOAuth2Tokens( + user_id=user_id, + database_id=1 if user_id < 4 else 2, + access_token=f"access-secret-{user_id}", + refresh_token=f"refresh-secret-{user_id}", + access_token_expiration=datetime(2027, 1, 1), + ) + for user_id in range(1, 5) + ] + session.add_all(tokens) + session.commit() + return tokens + + [email protected]( + "payload", + [ + {}, + {"all": False}, + {"token_ids": []}, + {"token_ids": ["invalid"]}, + {"token_ids": [0]}, + {"token_ids": [-1]}, + {"token_ids": [True]}, + {"token_ids": [1.5]}, + {"token_ids": ["1"]}, + {"all": True, "token_ids": [1]}, + {"all": "true"}, + {"all": None}, + {"token_ids": [1] * 101}, + ], +) +def test_delete_scope_rejects_invalid_payload(payload: Any) -> None: + """An invalid deletion scope must never become delete-all.""" + with pytest.raises(ValidationError): + DatabaseOAuth2TokensDeleteSchema().load(payload) + + [email protected]( + ("method", "suffix", "payload"), + [ + ("GET", "", None), + ("POST", "delete/", {"token_ids": [1]}), + ("POST", "all/", None), + ], +) +def test_management_routes_require_dedicated_permission( + client: Any, + mocker: MockerFixture, + full_api_access: None, + method: str, + suffix: str, + payload: dict[str, Any] | None, +) -> None: + """Database read/write permission alone does not permit token management.""" + mocker.patch.object(security_manager, "is_item_public", return_value=False) + access = mocker.patch.object(security_manager, "has_access", return_value=False) + response = client.open( + ENDPOINT + suffix, + method=method, + json=payload, + ) + assert response.status_code == 403 + access.assert_called_with("can_manage_oauth2_tokens", "Database") + + +def test_delete_rejects_missing_database(mocker: MockerFixture) -> None: + """Do not query credentials when the connection is missing or filtered out.""" + lookup = mocker.patch.object(DatabaseDAO, "find_by_id", return_value=None) + tokens = mocker.patch.object( + DatabaseUserOAuth2TokensDAO, "find_by_database_and_ids" + ) + with pytest.raises(DatabaseNotFoundError): + OAuth2DeleteTokensCommand(DATABASE_ID, [1]).run() + lookup.assert_called_once_with(DATABASE_ID) + tokens.assert_not_called() + + [email protected]("operation", ["list", "delete", "all"]) +def test_management_respects_database_dao_filter( + session: Session, + client: Any, + mocker: MockerFixture, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], + operation: str, +) -> None: + """A real DAO lookup must hide connections outside the caller's grants.""" + mocker.patch.object( + security_manager, "can_access_all_databases", return_value=False + ) + mocker.patch.object(security_manager, "user_view_menu_names", return_value=set()) + if operation == "list": + response = client.get(ENDPOINT) + elif operation == "all": + response = client.post(ENDPOINT + "all/") + else: + response = client.post(ENDPOINT + "delete/", json={"token_ids": [1]}) + assert response.status_code == 404 + assert session.query(DatabaseUserOAuth2Tokens).count() == 4 + + +def test_fab_listing_search_pagination_and_safe_columns( + client: Any, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], +) -> None: + """FAB supports owner search and pagination without returning token secrets.""" + response = client.get( + ENDPOINT, query_string={"q": prison.dumps({"page": 0, "page_size": 2})} + ) + assert response.status_code == 200 + assert response.json["count"] == 3 + assert [row["user"]["username"] for row in response.json["result"]] == [ + "user1", + "user2", + ] + assert "secret" not in str(response.json) + assert "access_token" not in response.json["result"][0] + assert "refresh_token" not in response.json["result"][0] + response = client.get( + ENDPOINT, query_string={"q": prison.dumps({"page": 1, "page_size": 2})} + ) + assert response.json["result"][0]["user"]["username"] == "user3" + for search in ("USER2", "First2", "First2 Example", "[email protected]"): + response = client.get( + ENDPOINT, + query_string={ + "q": prison.dumps( + { + "filters": [ + { + "col": "id", + "opr": "oauth2_tokens_user", + "value": search, + } + ], + } + ) + }, + ) + assert response.status_code == 200 + assert response.json["count"] == 1 + assert response.json["result"][0]["user"]["username"] == "user2" + for argument in ("columns", "select_columns"): + response = client.get( + ENDPOINT, + query_string={ + "q": prison.dumps({argument: ["access_token", "refresh_token"]}) + }, + ) + assert response.status_code == 200 + assert all( + "access_token" not in row and "refresh_token" not in row + for row in response.json["result"] + ) + + +def test_listing_cannot_override_database_scope( + client: Any, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], +) -> None: + """Client filters cannot replace the database constraint from the route.""" + response = client.get( + ENDPOINT, + query_string={ + "q": prison.dumps( + {"filters": [{"col": "database", "opr": "rel_o_m", "value": 2}]} + ) + }, + ) + assert response.status_code == 200 + assert response.json["count"] == 0 + assert response.json["result"] == [] + + +def test_fab_listing_caps_page_size( + client: Any, + mocker: MockerFixture, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], +) -> None: + """Client pagination arguments cannot bypass the bounded FAB page size.""" + query = mocker.spy(DatabaseOAuth2TokensRestApi.datamodel, "query") + response = client.get( + ENDPOINT, query_string={"q": prison.dumps({"page_size": 1000})} + ) + assert response.status_code == 200 + assert query.call_args.kwargs["page_size"] == 100 + assert ( + response.json["result"][0]["access_token_expiration"] + == datetime(2027, 1, 1).astimezone().isoformat() + ) + + [email protected]( + ("ids", "expected"), + [([1, 4], {1}), ([4], set()), ([1, 1, 2], {1, 2}), ([999], set()), ([], set())], +) +def test_scoped_token_lookup( + token_rows: list[DatabaseUserOAuth2Tokens], ids: list[int], expected: set[int] +) -> None: + """Exclude foreign and missing credentials before they reach the command.""" + tokens = DatabaseUserOAuth2TokensDAO.find_by_database_and_ids(1, ids) + assert {token.id for token in tokens} == expected + assert all(token.database_id == 1 for token in tokens) + + [email protected]("ids", [[1], [1, 2], [1, 1], [1, 1, 2]]) +def test_selected_deletion_uses_base_dao( + session: Session, + mocker: MockerFixture, + token_rows: list[DatabaseUserOAuth2Tokens], + ids: list[int], +) -> None: + """Validate selected IDs and delete complete credential rows through BaseDAO.""" + mocker.patch.object(security_manager, "can_access_all_databases", return_value=True) + delete = mocker.spy(DatabaseUserOAuth2TokensDAO, "delete") + assert OAuth2DeleteTokensCommand(DATABASE_ID, ids).run() is None + delete.assert_called_once() + assert {token.id for token in delete.call_args.args[0]} == set(ids) + assert session.query(DatabaseUserOAuth2Tokens).count() == 4 - len(set(ids)) + assert session.query(DatabaseUserOAuth2Tokens).filter_by(database_id=2).count() == 1 + + [email protected]("ids", [[999], [1, 999], [4], [1, 4], [], [1, 1, 4]]) +def test_selected_deletion_rejects_invalid_ids_atomically( + session: Session, + mocker: MockerFixture, + token_rows: list[DatabaseUserOAuth2Tokens], + ids: list[int], +) -> None: + """Missing or foreign IDs prevent every deletion, including mixed duplicates.""" + mocker.patch.object(security_manager, "can_access_all_databases", return_value=True) + with pytest.raises(OAuth2TokenNotFoundError): + OAuth2DeleteTokensCommand(DATABASE_ID, ids).run() + assert session.query(DatabaseUserOAuth2Tokens).count() == 4 + + +def test_delete_all_is_scoped_without_loading_tokens( + session: Session, + mocker: MockerFixture, + token_rows: list[DatabaseUserOAuth2Tokens], +) -> None: + """Delete-all uses one bulk operation and does not instantiate token models.""" + mocker.patch.object(security_manager, "can_access_all_databases", return_value=True) + lookup = mocker.spy(DatabaseUserOAuth2TokensDAO, "find_by_database_and_ids") + delete = mocker.spy(DatabaseUserOAuth2TokensDAO, "delete") + assert OAuth2DeleteTokensCommand(DATABASE_ID, delete_all=True).run() is None + lookup.assert_not_called() + delete.assert_not_called() + assert session.query(DatabaseUserOAuth2Tokens).filter_by(database_id=2).count() == 1 + assert OAuth2DeleteTokensCommand(DATABASE_ID, delete_all=True).run() is None + + [email protected]("ids", [[999], [1, 4], [1, 1, 4]]) +def test_delete_api_reports_invalid_selected_ids( + session: Session, + client: Any, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], + ids: list[int], +) -> None: + """Return not-found for stale or foreign IDs without partially deleting rows.""" + response = client.post(ENDPOINT + "delete/", json={"token_ids": ids}) + assert response.status_code == 404 + assert session.query(DatabaseUserOAuth2Tokens).count() == 4 + + [email protected]( + "payload", + [ + {}, + {"all": True}, + {"all": False}, + {"token_ids": []}, + {"all": True, "token_ids": [1]}, + ], +) +def test_delete_api_rejects_unsafe_scope_before_command( + client: Any, + mocker: MockerFixture, + full_api_access: None, + payload: dict[str, Any], +) -> None: + """An invalid deletion request must never reach the mutation command.""" + command = mocker.patch("superset.databases.api.OAuth2DeleteTokensCommand") + assert client.post(ENDPOINT + "delete/", json=payload).status_code == 400 + command.assert_not_called() + + [email protected]("payload", [{"token_ids": [1, 2]}, {"token_ids": [1, 1, 2]}]) +def test_delete_api_returns_standard_success_message( + session: Session, + client: Any, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], + payload: dict[str, Any], +) -> None: + """Selected deletion returns a success message without a count.""" + response = client.post(ENDPOINT + "delete/", json=payload) + assert response.status_code == 200 + assert response.json == {"message": "OK"} + assert session.query(DatabaseUserOAuth2Tokens).count() == 2 + + +def test_delete_all_api_has_an_explicit_route( + session: Session, + client: Any, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], +) -> None: + """Only the explicit all route deletes every credential for the database.""" + response = client.post(ENDPOINT + "all/") + assert response.status_code == 200 + assert response.json == {"message": "OK"} + assert session.query(DatabaseUserOAuth2Tokens).count() == 1 + assert session.query(DatabaseUserOAuth2Tokens).one().database_id == 2 + + [email protected]( + "body", ["{}", '{"token_ids": [1]}', '{"all": true}', "null", "invalid"] +) +def test_delete_all_api_ignores_request_bodies( + client: Any, + mocker: MockerFixture, + full_api_access: None, + body: str, +) -> None: + """The explicit all route determines deletion scope regardless of the body.""" + command = mocker.patch("superset.databases.api.OAuth2DeleteTokensCommand") + response = client.post( + ENDPOINT + "all/", data=body, content_type="application/json" + ) + assert response.status_code == 200 + command.assert_called_once_with(DATABASE_ID, delete_all=True) + command.return_value.run.assert_called_once_with() + + +def test_selected_deletion_rejects_an_absent_body( + client: Any, + mocker: MockerFixture, + full_api_access: None, +) -> None: + """A request without JSON cannot fall through to deletion of all tokens.""" + command = mocker.patch("superset.databases.api.OAuth2DeleteTokensCommand") + assert client.post(ENDPOINT + "delete/").status_code == 400 + command.assert_not_called() + + [email protected]( + ("token_ids", "delete_all"), + [(None, False), ([1], True), ([], True)], +) +def test_delete_command_requires_exactly_one_explicit_mode( + mocker: MockerFixture, + token_ids: list[int] | None, + delete_all: bool, +) -> None: + """Neither omitted IDs nor conflicting modes can trigger a deletion.""" + lookup = mocker.patch.object(DatabaseDAO, "find_by_id") + selected = mocker.patch.object(DatabaseUserOAuth2TokensDAO, "delete") + all_tokens = mocker.patch.object(DatabaseUserOAuth2TokensDAO, "delete_all_tokens") + with pytest.raises(ValueError, match="Specify token_ids or delete_all=True"): + OAuth2DeleteTokensCommand( + DATABASE_ID, token_ids=token_ids, delete_all=delete_all + ).run() + lookup.assert_not_called() + selected.assert_not_called() + all_tokens.assert_not_called() + + [email protected]("delete_all", [False, True]) [email protected]("failure_phase", ["mutation", "commit"]) +def test_deletion_failures_roll_back_and_raise_domain_error( + session: Session, + mocker: MockerFixture, + token_rows: list[DatabaseUserOAuth2Tokens], + delete_all: bool, + failure_phase: str, +) -> None: + """Translate database failures after rolling back both selected and all deletion.""" + mocker.patch.object(security_manager, "can_access_all_databases", return_value=True) + failure = SQLAlchemyError("database-failure-sentinel") + rollback = mocker.spy(session, "rollback") + commit = mocker.spy(session, "commit") + if failure_phase == "commit": + commit = mocker.patch.object(session, "commit", side_effect=failure) + else: + mocker.patch.object( + DatabaseUserOAuth2TokensDAO, + "delete_all_tokens" if delete_all else "delete", + side_effect=failure, + ) + command = ( + OAuth2DeleteTokensCommand(DATABASE_ID, delete_all=True) + if delete_all + else OAuth2DeleteTokensCommand(DATABASE_ID, token_ids=[1, 2]) + ) + with pytest.raises(OAuth2TokensDeleteFailedError) as raised: + command.run() + assert raised.value.__cause__ is failure + rollback.assert_called_once() + if failure_phase == "commit": + commit.assert_called_once() + else: + commit.assert_not_called() + assert session.query(DatabaseUserOAuth2Tokens).count() == 4 + + [email protected]("delete_all", [False, True]) +def test_deletion_routes_report_domain_failure_consistently( + session: Session, + client: Any, + mocker: MockerFixture, + full_api_access: None, + token_rows: list[DatabaseUserOAuth2Tokens], + delete_all: bool, +) -> None: + """Both routes return a deletion error without exposing the database exception.""" + mocker.patch.object( + DatabaseUserOAuth2TokensDAO, + "delete_all_tokens" if delete_all else "delete", + side_effect=SQLAlchemyError("database-failure-sentinel"), + ) + response = ( + client.post(ENDPOINT + "all/") + if delete_all + else client.post(ENDPOINT + "delete/", json={"token_ids": [1, 2]}) + ) + assert response.status_code == 422 + assert response.json == {"message": "OAuth2 tokens could not be deleted."} + assert "database-failure-sentinel" not in response.get_data(as_text=True) + assert session.query(DatabaseUserOAuth2Tokens).count() == 4 diff --git a/tests/unit_tests/semantic_layers/connections_access_test.py b/tests/unit_tests/semantic_layers/connections_access_test.py index 6746638e1f9..849a93b693a 100644 --- a/tests/unit_tests/semantic_layers/connections_access_test.py +++ b/tests/unit_tests/semantic_layers/connections_access_test.py @@ -400,3 +400,20 @@ def test_connections_combines_read_grants_from_separate_roles( assert {call.args[0] for call in query.call_args_list} == {Database, SemanticLayer} assert query.call_count == 2 session.rollback() + + [email protected]("enabled", [True, False]) +def test_database_connection_serialization_includes_oauth2_state( + session: Session, mocker: MockerFixture, enabled: bool +) -> None: + """Combined database rows expose the OAuth2 action flag without credentials.""" + Database.metadata.create_all(session.get_bind()) + database = Database(database_name="OAuth database", sqlalchemy_uri="sqlite://") + session.add(database) + session.flush() + mocker.patch.object(Database, "is_oauth2_enabled", return_value=enabled) + result = SemanticLayerRestApi._serialize_database(database) + assert result["source_type"] == "database" + assert result["engine_information"]["oauth2_enabled"] is enabled + assert "encrypted_extra" not in result + session.rollback()
