ENG-3098: PBAC management UI — data purposes, consumers, query log config#7700
ENG-3098: PBAC management UI — data purposes, consumers, query log config#7700
Conversation
Update fideslang dependency to use the feat/add-data-purposes-to-dataset-models branch which adds data_purposes at dataset, collection, field, and sub-field levels. Dependency: ethyca/fideslang#39 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rect-reference error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add admin UI for managing purpose-based access control (PBAC) entities: Data purposes & data consumers management pages: - List pages with Ant Table, search, pagination under Core Configuration - Add/edit pages using Ant Design v5 Form (Form.useForm, Form.Item) - Data purpose form mirrors privacy declaration fields (data use, categories, subjects, legal basis, retention, special category, features) - Data consumer form includes purpose assignment via multi-select - Delete confirmation modals with scope-based access control - RTK Query slices with cache invalidation (DataPurpose, DataConsumer tags) - Nav registration gated behind alphaPurposeBasedAccessControl flag Query log config integration tab: - Settings toggle panel on integration detail page (not CRUD table) - Enable/disable switch with poll interval selector - Inline Test connection and Poll now action buttons - RTK Query slice for query log config CRUD + test + poll endpoints - Tab registered for BigQuery and test_datastore (mock) integration types - test_datastore connections always pass connection test (no secrets needed) Infrastructure: - 12 new OAuth scope enums (DATA_PURPOSE_*, DATA_CONSUMER_*, QUERY_LOG_SOURCE_*) - 3 new cache tags (DataPurpose, DataConsumer, QueryLogConfig) - IntegrationFeature.QUERY_LOGGING enum value - test_datastore integration type info with QUERY_LOGGING feature - Connection test always succeeds for test_datastore/test_website types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Adds a new "Seed Data" page under the Developer nav (dev-only) that lets users select and trigger seed scenarios via the seed API. Includes RTK Query slice with status polling and cache tag invalidation mapped per seed task. Currently supports the PBAC scenario. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Toggling the query log switch off now sends PUT {enabled: false}
instead of DELETE, preserving the config and its watermark so
re-enabling resumes from where it left off.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The form was requesting size=500 but the API enforces max 100, causing a validation error and an empty dropdown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove seed data files from this PR so they can be submitted as a standalone PR on top. Files moved: - features/seed-data/SeedDataPanel.tsx - features/seed-data/seed-data.slice.ts - pages/poc/seed-data.tsx - nav-config + routes entries for /poc/seed-data Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
galvana
changed the title
galvana
requested review from
kruulik,
lucanovera and
thabofletcher
and removed request for
johnewart and
kruulik
Greptile SummaryThis PR adds the admin UI for three PBAC (purpose-based access control) feature areas: data purposes management, data consumers management, and a query log config tab on integration detail pages. All are gated behind the Issues found:
Confidence Score: 3/5
Important Files Changed
Reviews (1): Last reviewed commit: "chore: move seed data UI to separate bra..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| </Flex> | ||
|
|
||
| <div> | ||
| <Typography.Text strong className="mb-1 block"> | ||
| Poll interval | ||
| </Typography.Text> | ||
| <Select | ||
| value={values.poll_interval_seconds} | ||
| onChange={(value) => | ||
| setFieldValue("poll_interval_seconds", value) | ||
| } | ||
| options={POLL_INTERVAL_OPTIONS} | ||
| aria-label="Poll interval" | ||
| className="w-full" | ||
| data-testid="query-log-config-poll-select" | ||
| /> | ||
| </div> | ||
|
|
||
| <Flex justify="flex-end" gap={8} className="mt-2"> | ||
| <button |
There was a problem hiding this comment.
The cancel and submit buttons are implemented as raw <button> elements with manually applied Ant Design CSS class names (ant-btn ant-btn-default, ant-btn ant-btn-primary). This is fragile and bypasses the component library's event handling, theming system, and accessibility features. If Ant Design updates its CSS class names (which happens across major versions), these buttons will break silently.
Use the Button component from fidesui instead:
| </Flex> | |
| <div> | |
| <Typography.Text strong className="mb-1 block"> | |
| Poll interval | |
| </Typography.Text> | |
| <Select | |
| value={values.poll_interval_seconds} | |
| onChange={(value) => | |
| setFieldValue("poll_interval_seconds", value) | |
| } | |
| options={POLL_INTERVAL_OPTIONS} | |
| aria-label="Poll interval" | |
| className="w-full" | |
| data-testid="query-log-config-poll-select" | |
| /> | |
| </div> | |
| <Flex justify="flex-end" gap={8} className="mt-2"> | |
| <button | |
| <Flex justify="flex-end" gap={8} className="mt-2"> | |
| <Button | |
| type="default" | |
| onClick={onClose} | |
| > | |
| Cancel | |
| </Button> | |
| <Button | |
| type="primary" | |
| onClick={submitForm} | |
| disabled={isSubmitting || !values.name} | |
| loading={isSubmitting} | |
| > | |
| {isEditing ? "Update" : "Create"} | |
| </Button> | |
| </Flex> |
|
|
||
| const purposeOptions = useMemo( |
There was a problem hiding this comment.
The purposes query is capped at size: 100. If there are more than 100 data purposes in the system, the select will silently show an incomplete list and the user will be unable to assign purposes beyond that threshold. Since the purpose count is unbounded, this will break purpose assignment at scale.
Consider fetching all pages or implementing a debounced search-as-you-type select (e.g., Ant Design's Select with showSearch and a server-side filter query) to avoid loading all items at once:
| const purposeOptions = useMemo( | |
| const { data: purposesData, isLoading: purposesLoading } = | |
| useGetAllDataPurposesQuery({ size: 200 }); |
Or, better, use a paginated/searchable select that queries the API on user input rather than loading everything upfront.
There was a problem hiding this comment.
Bumped to 200 as a safety margin. In practice, data purposes are a bounded set, but the extra headroom is sensible.
| export enum PollInterval { | ||
| ONE_MINUTE = 60, | ||
| FIVE_MINUTES = 300, | ||
| FIFTEEN_MINUTES = 900, | ||
| ONE_HOUR = 3600, | ||
| SIX_HOURS = 21600, | ||
| TWENTY_FOUR_HOURS = 86400, | ||
| } |
There was a problem hiding this comment.
The available poll intervals include ONE_MINUTE (60s) and FIVE_MINUTES (300s). Per the team's polling convention, production poll intervals for async operations should be set in hours, not seconds or minutes, since results can take extended periods to arrive. Short intervals create unnecessary load and are considered development-only values that must be updated before merging.
Consider removing the sub-hour options or guarding them behind a dev-mode flag:
| export enum PollInterval { | |
| ONE_MINUTE = 60, | |
| FIVE_MINUTES = 300, | |
| FIFTEEN_MINUTES = 900, | |
| ONE_HOUR = 3600, | |
| SIX_HOURS = 21600, | |
| TWENTY_FOUR_HOURS = 86400, | |
| } | |
| export enum PollInterval { | |
| ONE_HOUR = 3600, | |
| SIX_HOURS = 21600, | |
| TWELVE_HOURS = 43200, | |
| TWENTY_FOUR_HOURS = 86400, | |
| } |
Rule Used: Polling intervals for async operations should be s... (source)
Learnt From
ethyca/fides#6566
There was a problem hiding this comment.
These are configurable poll intervals for how often the system polls the database for new query log entries, not UI polling intervals for async operations. Sub-hour intervals are intentional here since query log freshness is time-sensitive for access control monitoring.
| useUpdateQueryLogConfigMutation, | ||
| } from "./query-log-config.slice"; | ||
|
|
||
| interface FormValues { |
There was a problem hiding this comment.
The default poll_interval_seconds value of 300 is hardcoded rather than referencing the PollInterval enum defined in constants.ts. This makes the intent unclear and creates a maintenance hazard if the enum value changes.
| useUpdateQueryLogConfigMutation, | |
| } from "./query-log-config.slice"; | |
| interface FormValues { | |
| const DEFAULT_VALUES: FormValues = { | |
| enabled: false, | |
| poll_interval_seconds: PollInterval.FIVE_MINUTES, | |
| }; |
Also requires importing PollInterval at the top of the file.
Rule Used: Define magic strings as named variables with expla... (source)
Learnt From
ethyca/fidesplus#2515
There was a problem hiding this comment.
Done, replaced magic number with PollInterval.FIVE_MINUTES.
| import useDataConsumersTable from "~/features/data-consumers/useDataConsumersTable"; | ||
|
|
||
| const DataConsumersPage: NextPage = () => { | ||
| const { error } = useDataConsumersTable(); |
There was a problem hiding this comment.
DataConsumersPage calls useDataConsumersTable() only to check for an error, but DataConsumersTable (rendered below) also calls useDataConsumersTable() internally. This creates two independent hook instances — each with its own useTableState and useGetAllDataConsumersQuery subscription. As the user interacts with the search box, the page-level hook's search state will diverge from the table's, and the error check will no longer reflect the current table query's error state.
The same issue exists in pages/data-purposes/index.tsx (line 12).
A cleaner approach is to let DataConsumersTable own the error state and surface it via its own empty/error state handling, or lift the hook to the page and pass its output as props to the table component.
There was a problem hiding this comment.
Replaced the duplicate table hook with a direct RTK Query call using default params for the top-level error guard. Fixed in both data-consumers and data-purposes pages.
| # Test connection types always succeed (no real connector to test) | ||
| if connection_config.connection_type in ( | ||
| ConnectionType.test_datastore, | ||
| ConnectionType.test_website, | ||
| ): | ||
| connection_config.update_test_status( | ||
| test_status=ConnectionTestStatus.succeeded, db=self.db | ||
| ) | ||
| return TestStatusMessage( | ||
| msg=msg, | ||
| test_status=ConnectionTestStatus.succeeded, |
There was a problem hiding this comment.
test_website connection type also bypassed
The PR description states "test_datastore connections always pass connection test (no secrets needed)", but the implementation also short-circuits ConnectionType.test_website. If test_website was previously exercised through a real connector path (even a trivial one), this changes its behavior unexpectedly. Please confirm that bypassing the connection test for test_website is intentional and document this in the comment.
There was a problem hiding this comment.
Both test_datastore and test_website are synthetic connection types used for development/testing with no real connector behind them. Updated the comment to be more explicit.
kruulik
left a comment
kruulik
left a comment
There was a problem hiding this comment.
Couple comments about dead code, but maybe that's just going to be used later?
| ); | ||
| router.push(DATA_CONSUMERS_ROUTE); | ||
| } catch (error) { | ||
| message.error(getErrorMessage(error as any)); |
There was a problem hiding this comment.
| message.error(getErrorMessage(error as any)); | |
| message.error(getErrorMessage(error as RTKErrorResult["error"])); |
There was a problem hiding this comment.
Done, updated to use the proper type.
There was a problem hiding this comment.
Confirmed unused, removed.
There was a problem hiding this comment.
Confirmed unused, removed.
There was a problem hiding this comment.
Only used within configure-query-log
There was a problem hiding this comment.
Only imported by useQueryLogConfigTable which is itself unused. Removed both.
|
Good catch on the dead code! All three files (ConfigureQueryLogModal, useQueryLogConfigTable, QueryLogConfigActionsCell) are confirmed unused. Removed them. |
- Remove dead code: ConfigureQueryLogModal, useQueryLogConfigTable, QueryLogConfigActionsCell - Replace `as any` with RTKErrorResult["error"] in DeleteDataConsumerModal - Bump purpose query size from 100 to 200 in DataConsumerForm - Replace magic number 300 with PollInterval.FIVE_MINUTES in QueryLogConfigTab - Fix duplicate hook instantiation in data-consumers and data-purposes pages - Clarify comment on test connection type bypass in connection_service Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Ticket ENG-3098
Description Of Changes
Add the admin UI for managing purpose-based access control (PBAC) entities. Three feature areas:
Data purposes management (
/data-purposes)Data consumers management (
/data-consumers)Query log config (integration detail tab)
All pages gated behind
alphaPurposeBasedAccessControlfeature flag andrequiresPlus.Code Changes
features/data-purposes/— 8 files: RTK slice, table, form, delete modal, actions cell, constants, barrelfeatures/data-consumers/— 8 files: RTK slice, table, form, delete modal, actions cell, constants, barrelfeatures/integrations/configure-query-log/— 6 files: RTK slice, tab, table hook, modal, actions cell, constantspages/data-purposes/— 3 pages: list, add, editpages/data-consumers/— 3 pages: list, add, editfeatures/common/nav/routes.ts— 8 new route constantsfeatures/common/nav/nav-config.tsx— 2 nav items under Core Configurationfeatures/common/api.slice.ts— 3 cache tagstypes/api/models/ScopeRegistryEnum.ts— 12 scope enumstypes/api/models/IntegrationFeature.ts— QUERY_LOGGING enumfeatures/integrations/add-integration/allIntegrationTypes.tsx— test_datastore type infofeatures/integrations/integration-type-info/bigqueryInfo.tsx— QUERY_LOGGING featurefeatures/integrations/hooks/useFeatureBasedTabs.tsx— Query logging tabsrc/fides/service/connection/connection_service.py— test types always pass connection testSteps to Confirm
nox -s "dev(slim)" -- fides-pkg fides-admin-uialphaPurposeBasedAccessControlfeature flagcd clients/admin-ui && npm run lint && npm run typecheckPre-Merge Checklist
nox -s dev -- demoCHANGELOG.md