Skip to content

feat(catalog): add OAuth server setup flow - #113

Open
vishu-bh wants to merge 2 commits into
mainfrom
feature/catalog-oauth-add-flow
Open

feat(catalog): add OAuth server setup flow#113
vishu-bh wants to merge 2 commits into
mainfrom
feature/catalog-oauth-add-flow

Conversation

@vishu-bh

@vishu-bh vishu-bh commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add an OAuth-specific catalog setup dialog built from existing shadcn primitives.
  • Register credentials, launch authorization, enable the server, then discover tools.
  • Surface caller-scoped OAuth status and re-authorization actions on catalog cards.

Backend dependency

Requires backend PRs #6588 and #6620. Handwritten frontend contract types are temporary until OpenAPI generation includes the merged API.

Validation

  • npm run lint -- --max-warnings=0
  • npx tsc --noEmit
  • npm run test -- --run src/pages/ServerCatalog.test.tsx src/components/server-catalog/CatalogResults.test.tsx src/api/catalog.test.ts

NOTE

Please do not merge this till backend dependency is resolved. For the purpose of development handwritten contracts is added but it needs to be cleaned up once backend dependency is resolved

Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh marked this pull request as ready for review September 9, 2026 11:48
@vishu-bh
vishu-bh force-pushed the feature/catalog-oauth-add-flow branch from 4390417 to 700cf15 Compare September 9, 2026 11:51
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh requested review from gcgoncalves and marekdano and removed request for marekdano September 9, 2026 12:55
@marekdano
marekdano removed their request for review September 9, 2026 13:27

@marekdano marekdano left a comment

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.

1. 🔴 High — OAuth popup blocked on first-time setup

File: src/pages/ServerCatalog.tsx:638

handleOAuthSubmit does await registerServer(...) (an HTTP POST) before calling serversApi.triggerOAuthAuthorization(gatewayId). By the time window.open() runs inside that call, the browser no longer treats it as a direct response to the click, so popup blockers will block it (authWindow is null) — this will trigger on essentially every first-time OAuth registration.

Failure scenario: User clicks "Configure and authorize" on a new OAuth catalog server → popup blocked → "Failed to open OAuth authorization window" error. Retrying then hits the pendingOAuthGatewayId fast path, which calls triggerOAuthAuthorization synchronously and succeeds — so it looks like a transient glitch but is actually deterministic.


2. 🟠 Medium-High — Removed auth-type filter exposes unsupported servers

File: src/pages/ServerCatalog.tsx:454

The catalog's auth-type allow-list filter (previously SUPPORTED_AUTH_TYPE_SET) was removed. supportedServers = data?.servers ?? [] no longer filters by auth type, so servers with types outside Open/API Key/OAuth (e.g. Basic, mTLS) now render and are "Add"-able.

Failure scenario: Clicking "Add" on such a server falls through handleAdd's if/else chain straight to void registerServer(server) with no credentials — likely a generic server-side failure, or a silent no-auth registration. No test covers this path.


3. 🟡 Medium — Disconnect/Test not blocked during in-flight authorization

File: src/components/server-catalog/CatalogResults.tsx:202

The new Authorize action shares the isAdding pending flag with Test and Disconnect, but Test's/Disconnect's own disabled conditions were never updated to also check isAdding.

Failure scenario: User clicks Authorize (popup opens, isAdding true) → reopens the dropdown and clicks Disconnect (only checks isTesting || isDisconnecting) → gateway is deleted mid-authorization → when the popup resolves, toggleEnabled/fetchToolsAfterOAuth run against a deleted gateway id, surfacing a confusing error instead of being prevented up front.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants