Skip to content

Conversation

@antirotor
Copy link
Member

Description of changes

This is experimental change adding some frontend tools for managing OAuth clients from ynput/ayon-backend#649

It allows adding and deleting client records. It is probably on wrong place and I didn't handle access permission and stuff - if ever we merge this, we will need to change it. I also need to add "Consent Page" for full OAuth functionality.

Technical details

So far, this can be tested by simple scripts from the backend PR or perhaps some other service using OAuth - my goal is to use it to autenticate RabbitMQ running as a service and client side code connecting directly there.

Additional context

it is still PoC and as such it is probably on a wrong place using wrong permissions, etc.
@antirotor antirotor self-assigned this Nov 6, 2025
@antirotor antirotor added the type: enhancement Improvement of existing functionality or minor addition label Nov 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds OAuth client management functionality to the settings page. It introduces a new OAuth clients API service and a user interface for creating, viewing, and deleting OAuth clients.

  • New OAuth clients API service with endpoints for get, create, and delete operations
  • Settings page integration with a new OAuth module accessible to managers
  • Complete UI implementation for managing OAuth clients with dialog-based creation flow

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/services/oauthClients.ts Defines OAuth client API endpoints and TypeScript interfaces
src/pages/SettingsPage/SettingsPage.jsx Adds lazy-loaded OAuth module and navigation entry
src/pages/SettingsPage/OAuth/index.ts TypeScript exports for OAuth components
src/pages/SettingsPage/OAuth/index.js JavaScript default export for OAuth manager
src/pages/SettingsPage/OAuth/OAuthManager.tsx Wrapper component that renders OAuth clients manager
src/pages/SettingsPage/OAuth/OAuthClientsManager.tsx Main component for OAuth clients table and management
src/pages/SettingsPage/OAuth/NewOAuthClientDialog.tsx Dialog form for creating new OAuth clients
shared/src/api/base/client.ts Adds 'oauthClients' cache tag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +4
export { default } from './OAuthManager'
export { default as OAuthManager } from './OAuthManager'
export { default as OAuthClientsManager } from './OAuthClientsManager'
export { default as NewOAuthClientDialog } from './NewOAuthClientDialog'
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

Both index.ts and index.js files exist in the same directory with overlapping exports. Having both files can cause module resolution issues and confusion. The project should use only one index file format. Since SettingsPage.jsx uses lazy(() => import('./OAuth')), only index.js is actually being used, making index.ts redundant.

Suggested change
export { default } from './OAuthManager'
export { default as OAuthManager } from './OAuthManager'
export { default as OAuthClientsManager } from './OAuthClientsManager'
export { default as NewOAuthClientDialog } from './NewOAuthClientDialog'
<DELETE FILE>

Copilot uses AI. Check for mistakes.
onChange={(values) => handleFieldChange('grantTypes', values)}
widthExpand
dataKey="value"
labelKey="label"
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The Dropdown for Grant Types appears to allow multiple selections (array value), but lacks a multiSelect or similar prop to indicate this to users. Consider adding a prop or documentation to clarify that multiple grant types can be selected, or ensuring the Dropdown component is configured for multi-selection mode.

Suggested change
labelKey="label"
labelKey="label"
multiSelect

Copilot uses AI. Check for mistakes.
Comment on lines +240 to +247
<Dropdown
value={formData.responseTypes}
options={RESPONSE_TYPE_OPTIONS}
onChange={(values) => handleFieldChange('responseTypes', values)}
widthExpand
dataKey="value"
labelKey="label"
/>
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The Dropdown for Response Types appears to allow multiple selections (array value), but lacks a multiSelect or similar prop to indicate this to users. Consider adding a prop or documentation to clarify that multiple response types can be selected, or ensuring the Dropdown component is configured for multi-selection mode.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement Improvement of existing functionality or minor addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants