Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/cypress/cypress/support/commands/odh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,10 @@ declare global {
type: 'POST /maas/api/v1/api-keys',
response: { data: OdhResponse<CreateAPIKeyResponse> },
) => Cypress.Chainable<null>) &
((
type: 'GET /maas/api/v1/is-maas-admin',
response: OdhResponse<{ data: { allowed: boolean } }>,
) => Cypress.Chainable<null>) &
((
type: 'GET /maas/api/v1/user',
response: OdhResponse<{ data: { userId: string; clusterAdmin: boolean } }>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { mockDashboardConfig, mockDscStatus } from '@odh-dashboard/internal/__mocks__';
import { DataScienceStackComponent } from '@odh-dashboard/internal/concepts/areas/types';
import type { APIKey } from '@odh-dashboard/maas/types/api-key';
import { asProductAdminUser } from '../../../utils/mockUsers';
import { asClusterAdminUser, asProjectAdminUser } from '../../../utils/mockUsers';
import {
apiKeysPage,
bulkRevokeAPIKeyModal,
Expand All @@ -22,7 +22,7 @@ const mockSearchResponse = (keys: APIKey[]) => ({

describe('API Keys Page', () => {
beforeEach(() => {
asProductAdminUser();
asClusterAdminUser();
cy.interceptOdh(
'GET /api/config',
mockDashboardConfig({
Expand All @@ -33,6 +33,7 @@ describe('API Keys Page', () => {
cy.interceptOdh('GET /maas/api/v1/user', {
data: { userId: 'test-user', clusterAdmin: false },
});
cy.interceptOdh('GET /maas/api/v1/is-maas-admin', { data: { allowed: true } });
cy.interceptOdh('GET /maas/api/v1/namespaces', { data: [] });

cy.interceptOdh(
Expand Down Expand Up @@ -92,6 +93,13 @@ describe('API Keys Page', () => {
.should('contain.text', 'production-backend');
});

it('should not display the username filter for non-MaaS admins', () => {
asProjectAdminUser();
apiKeysPage.visit();
apiKeysPage.findFilterInput().should('not.exist');
apiKeysPage.findUsernameFilterTooltip().should('not.exist');
});

it('should filter api keys by status', () => {
const nonActiveKeys = mockAPIKeys().filter((k) => k.status !== 'active');

Expand Down
8 changes: 4 additions & 4 deletions packages/maas/frontend/src/app/api/subscriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@ const isMaaSSubscriptionRef = (v: unknown): v is ModelSubscriptionRef =>
isRecord(v) &&
typeof v.name === 'string' &&
typeof v.namespace === 'string' &&
Array.isArray(v.tokenRateLimits) &&
v.tokenRateLimits.every(isTokenRateLimit) &&
(v.tokenRateLimits === undefined ||
(Array.isArray(v.tokenRateLimits) && v.tokenRateLimits.every(isTokenRateLimit))) &&
(v.tokenRateLimitRef === undefined || typeof v.tokenRateLimitRef === 'string') &&
(v.billingRate === undefined || typeof v.billingRate === 'object');

const isMaaSSubscription = (v: unknown): v is MaaSSubscription =>
isRecord(v) &&
typeof v.name === 'string' &&
typeof v.namespace === 'string' &&
typeof v.phase === 'string' &&
(v.phase === undefined || typeof v.phase === 'string') &&
(v.priority === undefined || typeof v.priority === 'number') &&
typeof v.owner === 'object' &&
Array.isArray(v.modelRefs) &&
v.modelRefs.every(isMaaSSubscriptionRef) &&
(v.tokenMetadata === undefined || typeof v.tokenMetadata === 'object') &&
typeof v.creationTimestamp === 'string';
(v.creationTimestamp === undefined || typeof v.creationTimestamp === 'string');
const isTokenRateLimit = (v: unknown): v is TokenRateLimit =>
isRecord(v) && typeof v.limit === 'number' && typeof v.window === 'string';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ const AllApiKeysPage: React.FC = () => {
const [isModalOpen, setIsModalOpen] = React.useState(false);
const [revokeApiKey, setRevokeApiKey] = React.useState<APIKey | undefined>(undefined);

// TODO: use this for hiding the username search for non-admins and for allowing admins to see all keys
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [isMaasAdmin] = useIsMaasAdmin();

const [filterData, setFilterData] = React.useState<ApiKeyFilterDataType>(initialApiKeyFilterData);
Expand Down Expand Up @@ -157,6 +155,7 @@ const AllApiKeysPage: React.FC = () => {
isFetching={isFetching}
toolbarContent={
<ApiKeysToolbar
isMaasAdmin={isMaasAdmin}
setIsModalOpen={setIsModalOpen}
filterData={filterData}
localUsername={localUsername}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type ApiKeysToolbarProps = {
activeApiKeys: APIKey[];
refresh: () => void;
onClearFilters: () => void;
isMaasAdmin: boolean;
};

const ApiKeysToolbar: React.FC<ApiKeysToolbarProps> = ({
Expand All @@ -42,6 +43,7 @@ const ApiKeysToolbar: React.FC<ApiKeysToolbarProps> = ({
activeApiKeys,
refresh,
onClearFilters,
isMaasAdmin,
}) => {
const [isStatusSelectOpen, setIsStatusSelectOpen] = React.useState(false);

Expand Down Expand Up @@ -109,34 +111,36 @@ const ApiKeysToolbar: React.FC<ApiKeysToolbarProps> = ({
</SelectList>
</Select>
</ToolbarFilter>
<ToolbarFilter
labels={filterData.username ? [filterData.username] : []}
deleteLabel={() => {
setLocalUsername('');
onUsernameChange('');
}}
categoryName="Username"
>
<Tooltip
content="Please enter the full username"
data-testid="username-filter-tooltip"
{isMaasAdmin && (
<ToolbarFilter
labels={filterData.username ? [filterData.username] : []}
deleteLabel={() => {
setLocalUsername('');
onUsernameChange('');
}}
categoryName="Username"
>
<SearchInput
aria-label="Filter by username"
placeholder="Filter by username"
data-testid="username-filter-input"
value={localUsername}
onChange={(_event, value) => {
setLocalUsername(value);
}}
onSearch={(_event, value) => onUsernameChange(value)}
onClear={() => {
setLocalUsername('');
onUsernameChange('');
}}
/>
</Tooltip>
</ToolbarFilter>
<Tooltip
content="Please enter the full username"
data-testid="username-filter-tooltip"
>
<SearchInput
aria-label="Filter by username"
placeholder="Filter by username"
data-testid="username-filter-input"
value={localUsername}
onChange={(_event, value) => {
setLocalUsername(value);
}}
onSearch={(_event, value) => onUsernameChange(value)}
onClear={() => {
setLocalUsername('');
onUsernameChange('');
}}
/>
</Tooltip>
</ToolbarFilter>
)}
Comment on lines +114 to +143
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hidden username filter state can persist after role downgrade.

When this block unmounts, a previously set username filter can still affect requests (see packages/maas/frontend/src/app/pages/api-keys/AllApiKeysPage.tsx, Line 41), leaving users with invisible filtering and no direct clear control.

Proposed fix
 const ApiKeysToolbar: React.FC<ApiKeysToolbarProps> = ({
   setIsModalOpen,
   filterData,
   localUsername,
   setLocalUsername,
   onUsernameChange,
   onStatusToggle,
   onStatusClear,
   activeApiKeys,
   refresh,
   onClearFilters,
   isMaasAdmin,
 }) => {
   const [isStatusSelectOpen, setIsStatusSelectOpen] = React.useState(false);
+
+  React.useEffect(() => {
+    if (!isMaasAdmin && localUsername) {
+      setLocalUsername('');
+      onUsernameChange('');
+    }
+  }, [isMaasAdmin, localUsername, onUsernameChange, setLocalUsername]);

   return (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/maas/frontend/src/app/pages/api-keys/allKeys/ApiKeysToolbar.tsx`
around lines 114 - 143, When the username filter UI unmounts the hidden state
can persist; add a useEffect inside ApiKeysToolbar that watches isMaasAdmin and
on unmount or when isMaasAdmin becomes false calls setLocalUsername('') and
onUsernameChange('') to clear any lingering filter state (use a cleanup function
or effect branch). This ensures the username filter (localUsername,
setLocalUsername, onUsernameChange) is reset whenever the ToolbarFilter block is
removed so invisible filtering cannot persist.

</ToolbarGroup>
</ToolbarToggleGroup>
<ToolbarGroup>
Expand Down
6 changes: 3 additions & 3 deletions packages/maas/frontend/src/app/types/subscriptions.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
export type MaaSSubscription = {
name: string;
namespace: string;
phase: string;
phase?: string;
priority?: number;
owner: OwnerSpec;
modelRefs: ModelSubscriptionRef[];
tokenMetadata?: TokenMetadata;
creationTimestamp: string;
creationTimestamp?: string;
};

export type ModelSubscriptionRef = {
name: string;
namespace: string;
tokenRateLimits: TokenRateLimit[];
tokenRateLimits?: TokenRateLimit[];
tokenRateLimitRef?: string;
billingRate?: BillingRate;
};
Expand Down
Loading