feat(pymthouse): add billing provider, plan-builder APIs, and client-credentials auth#268
feat(pymthouse): add billing provider, plan-builder APIs, and client-credentials auth#268
Conversation
…r APIs - Added PymtHouse page to the dashboard with marketplace link and integration notes. - Implemented OAuth flow for PymtHouse, including token exchange and session management. - Created new API routes for PymtHouse capabilities catalog, SLA summary, and network pricing. - Introduced utility functions for handling PymtHouse OAuth and plan builder interactions. - Updated sidebar navigation to include PymtHouse link. - Documented PymtHouse integration details and API usage in the documentation. This commit establishes the foundation for integrating PymtHouse as a billing provider within the NaaP ecosystem.
…entication flow - Deleted the PymtHouse page from the dashboard, including all related UI components and links. - Updated the authentication flow for PymtHouse to utilize a server-to-server client_credentials method, eliminating the need for browser redirects. - Revised API routes to reflect the new authentication process, ensuring seamless integration with the NaaP ecosystem. - Updated documentation to clarify the new PymtHouse integration and environment variable requirements.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
🗃️ Database Migration Detected This PR includes changes to Prisma schema files. Please ensure:
Requesting review from the core team: @livepeer/core |
📝 WalkthroughWalkthroughThis PR introduces PymtHouse as a new billing provider using server-to-server OAuth client-credentials flow. It adds three data endpoints for platform capabilities, pricing, and SLA metrics, modifies authentication routes to handle PymtHouse separately from browser-based OAuth, and extends the database schema to support PKCE storage. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Start as /start Endpoint
participant Issuer as PymtHouse OIDC Issuer
participant LinkApi as PymtHouse Link API
participant DB as Database
Dev->>Start: POST /api/v1/auth/providers/pymthouse/start
Start->>Start: Extract user context
Start->>Issuer: POST /token (client_credentials)
Issuer-->>Start: access_token
Start->>LinkApi: POST /api/v1/naap/link-user<br/>(Bearer token, naapUserId, email)
LinkApi-->>Start: { api_key: "pmth_..." }
Start->>DB: Create BillingProviderOAuthSession<br/>(complete, api_key encrypted)
Start-->>Dev: { success, auth_url: null, api_key, poll_after_ms: 0 }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/web-next/src/lib/billing-oauth-origin.ts (2)
48-51: Verify edge case: non-localhost forwarded host without primary host header.When
hostis absent butforwardedHostis present and is NOT a localhost address, this branch doesn't execute and the function falls through to thelocalhost:3000fallback. This seems unlikely in practice but could lead to unexpected behavior if a proxy strips theHostheader.Consider whether the condition should be
if (forwardedHost)instead ofif (forwardedHost && isLocalHost(forwardedHost))to handle proxied non-localhost requests when theHostheader is missing.Potential fix to handle non-localhost forwarded hosts
- if (forwardedHost && isLocalHost(forwardedHost)) { - const protocol = forwardedProto || 'http'; - return `${protocol}://${forwardedHost}`; - } + if (forwardedHost) { + const protocol = isLocalHost(forwardedHost) + ? (forwardedProto || 'http') + : 'https'; + return `${protocol}://${forwardedHost}`; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/billing-oauth-origin.ts` around lines 48 - 51, The current branch only returns forwardedHost when isLocalHost(forwardedHost) is true, causing a fallthrough to the localhost:3000 fallback if Host is missing but forwardedHost is a non-local address; update the condition to check for presence of forwardedHost (i.e., if (forwardedHost)) so the function will return `${forwardedProto || 'http'}://${forwardedHost}` whenever forwardedHost exists, preserving the existing protocol default (forwardedProto || 'http') and avoiding the incorrect localhost:3000 fallback.
31-35: Consider adding IPv6 localhost shorthand detection.The
isLocalHostfunction checks for[::1]but users might also use the unbracketed form::1in some contexts. This is a minor edge case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/lib/billing-oauth-origin.ts` around lines 31 - 35, The isLocalHost function misses the unbracketed IPv6 localhost (::1); update the isLocalHost predicate to also detect the unbracketed form (e.g., include a check for '::1' using includes or startsWith) alongside the existing '[::1]' check so both bracketed and unbracketed IPv6 localhost addresses are recognized.plugins/developer-api/frontend/src/pages/DeveloperView.tsx (1)
579-645: Consider adding user feedback for direct API key flow.When
directApiKeyis present, the user sees the "Authenticating..." modal briefly before jumping to success. Consider providing more specific feedback (e.g., "Linking account...") since no browser redirect actually occurs for server-to-server flows. This is a UX polish suggestion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx` around lines 579 - 645, The UX should show a different status when the flow uses a direct (server-to-server) API key instead of the browser redirect path: detect when directApiKey (or a flag indicating direct API key flow) is present and set a clearer createStep/message (e.g., call setCreateStep('linking') or update the modal text to "Linking account..." via the same state used for the "Authenticating..." modal) before beginning the server-side token exchange, and ensure you call setCreating(false) and setCreateStep('form' or 'success') on error/success; update references in the create logic where providerApiKey, directApiKey, setCreateStep, setCreateError, and setCreating are used so the UI shows the appropriate message for direct API key flows.apps/web-next/src/app/api/v1/auth/providers/[providerSlug]/start/route.ts (1)
172-183: Consider extracting state frombuildDaydreamAuthUrlreturn value instead of re-parsing.The current approach builds the URL, then re-parses it to extract the state. A cleaner approach would be to return the state directly from
buildDaydreamAuthUrl.♻️ Suggested refactor
async function buildDaydreamAuthUrl( callbackUrl: string -): Promise<{ authUrl: string } | null> { +): Promise<{ authUrl: string; state: string } | null> { const base = resolveProviderAuthUrl('daydream'); if (!base) return null; const state = crypto.randomBytes(16).toString('hex'); return { authUrl: `${base}?redirect_url=${encodeURIComponent(callbackUrl)}&state=${encodeURIComponent(state)}`, + state, }; }Then use
built.statedirectly instead of re-parsing the URL.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/app/api/v1/auth/providers/`[providerSlug]/start/route.ts around lines 172 - 183, The code re-parses built.authUrl to extract state; modify buildDaydreamAuthUrl to return an object containing both authUrl and state (e.g., { authUrl, state }) and update the handler that references providerSlug to use built.state directly instead of new URL(built.authUrl).searchParams.get('state'), removing the extra URL parse and keeping the existing error checks (e.g., ensure built is truthy and built.state exists before proceeding).docs/pymthouse-integration.md (1)
23-35: Add language specifier to fenced code block.The static analysis tool flagged this code block for missing a language identifier. Since this is a text diagram, specifying
textsatisfies the linter and improves accessibility for screen readers.📝 Suggested fix
-``` +```text NaaP server PymtHouse OIDC │ │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pymthouse-integration.md` around lines 23 - 35, The fenced code block in docs/pymthouse-integration.md is missing a language specifier; update the opening triple-backticks for the ASCII diagram to include "text" (i.e. change ``` to ```text) so the linter and screen readers correctly recognize the block containing the NaaP server / PymtHouse OIDC diagram.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 78-81: The in-memory billing provider entries in the
inMemoryBillingProviders array use inconsistent icon casing versus the DB seed;
update the icon values to match the database seed casing (e.g., change the
'wallet' icon to 'Wallet' and ensure 'cloud' vs 'Cloud' matches the database) by
modifying the objects inside the inMemoryBillingProviders constant (look for the
entries with id 'bp-daydream' and 'bp-pymthouse') so icon string casing is
consistent with packages/database/src/billing-providers.ts.
---
Nitpick comments:
In `@apps/web-next/src/app/api/v1/auth/providers/`[providerSlug]/start/route.ts:
- Around line 172-183: The code re-parses built.authUrl to extract state; modify
buildDaydreamAuthUrl to return an object containing both authUrl and state
(e.g., { authUrl, state }) and update the handler that references providerSlug
to use built.state directly instead of new
URL(built.authUrl).searchParams.get('state'), removing the extra URL parse and
keeping the existing error checks (e.g., ensure built is truthy and built.state
exists before proceeding).
In `@apps/web-next/src/lib/billing-oauth-origin.ts`:
- Around line 48-51: The current branch only returns forwardedHost when
isLocalHost(forwardedHost) is true, causing a fallthrough to the localhost:3000
fallback if Host is missing but forwardedHost is a non-local address; update the
condition to check for presence of forwardedHost (i.e., if (forwardedHost)) so
the function will return `${forwardedProto || 'http'}://${forwardedHost}`
whenever forwardedHost exists, preserving the existing protocol default
(forwardedProto || 'http') and avoiding the incorrect localhost:3000 fallback.
- Around line 31-35: The isLocalHost function misses the unbracketed IPv6
localhost (::1); update the isLocalHost predicate to also detect the unbracketed
form (e.g., include a check for '::1' using includes or startsWith) alongside
the existing '[::1]' check so both bracketed and unbracketed IPv6 localhost
addresses are recognized.
In `@docs/pymthouse-integration.md`:
- Around line 23-35: The fenced code block in docs/pymthouse-integration.md is
missing a language specifier; update the opening triple-backticks for the ASCII
diagram to include "text" (i.e. change ``` to ```text) so the linter and screen
readers correctly recognize the block containing the NaaP server / PymtHouse
OIDC diagram.
In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx`:
- Around line 579-645: The UX should show a different status when the flow uses
a direct (server-to-server) API key instead of the browser redirect path: detect
when directApiKey (or a flag indicating direct API key flow) is present and set
a clearer createStep/message (e.g., call setCreateStep('linking') or update the
modal text to "Linking account..." via the same state used for the
"Authenticating..." modal) before beginning the server-side token exchange, and
ensure you call setCreating(false) and setCreateStep('form' or 'success') on
error/success; update references in the create logic where providerApiKey,
directApiKey, setCreateStep, setCreateError, and setCreating are used so the UI
shows the appropriate message for direct API key flows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e5ee130-4479-4aba-8873-1fac4fba1201
📒 Files selected for processing (14)
apps/web-next/.env.local.exampleapps/web-next/src/app/api/v1/auth/providers/[providerSlug]/callback/route.tsapps/web-next/src/app/api/v1/auth/providers/[providerSlug]/start/route.tsapps/web-next/src/app/api/v1/pymthouse/capabilities/catalog/route.tsapps/web-next/src/app/api/v1/pymthouse/network/price/route.tsapps/web-next/src/app/api/v1/pymthouse/sla/summary/route.tsapps/web-next/src/lib/billing-oauth-origin.tsapps/web-next/src/lib/pymthouse-oidc.tsapps/web-next/src/lib/pymthouse-plan-builder.tsdocs/pymthouse-integration.mdpackages/database/prisma/schema.prismapackages/database/src/billing-providers.tsplugins/developer-api/backend/src/server.tsplugins/developer-api/frontend/src/pages/DeveloperView.tsx
| const inMemoryBillingProviders = [ | ||
| { id: 'bp-daydream', slug: 'daydream', displayName: 'Daydream', description: 'AI-powered billing via Daydream', icon: 'cloud', authType: 'oauth' }, | ||
| { id: 'bp-pymthouse', slug: 'pymthouse', displayName: 'PymtHouse', description: 'Billing via PymtHouse OIDC', icon: 'wallet', authType: 'oauth' }, | ||
| ]; |
There was a problem hiding this comment.
Icon casing inconsistency with database seed data.
The in-memory fallback uses icon: 'wallet' (lowercase), but packages/database/src/billing-providers.ts defines icon: 'Wallet' (capitalized). This inconsistency could cause issues if icon rendering logic is case-sensitive.
Fix to match database seed casing
- { id: 'bp-pymthouse', slug: 'pymthouse', displayName: 'PymtHouse', description: 'Billing via PymtHouse OIDC', icon: 'wallet', authType: 'oauth' },
+ { id: 'bp-pymthouse', slug: 'pymthouse', displayName: 'PymtHouse', description: 'Billing via PymtHouse OIDC', icon: 'Wallet', authType: 'oauth' },Also update the existing daydream entry for consistency:
- { id: 'bp-daydream', slug: 'daydream', displayName: 'Daydream', description: 'AI-powered billing via Daydream', icon: 'cloud', authType: 'oauth' },
+ { id: 'bp-daydream', slug: 'daydream', displayName: 'Daydream', description: 'AI-powered billing via Daydream', icon: 'Cloud', authType: 'oauth' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/developer-api/backend/src/server.ts` around lines 78 - 81, The
in-memory billing provider entries in the inMemoryBillingProviders array use
inconsistent icon casing versus the DB seed; update the icon values to match the
database seed casing (e.g., change the 'wallet' icon to 'Wallet' and ensure
'cloud' vs 'Cloud' matches the database) by modifying the objects inside the
inMemoryBillingProviders constant (look for the entries with id 'bp-daydream'
and 'bp-pymthouse') so icon string casing is consistent with
packages/database/src/billing-providers.ts.
| │ │ | ||
| ├─ POST /api/v1/naap/link-user ──►│ Bearer {access_token} | ||
| │ { naapUserId } │ provisions / upserts user | ||
| │◄─ { api_key: "pmth_*" } ────────┤ returns 90-day gateway session |
There was a problem hiding this comment.
This should be configurable. We could also provide routes to return only the shorter lived JWT
| | GET | `/api/v1/pymthouse/capabilities/catalog` | Pipeline catalog + network models (`?limit=` optional) | | ||
| | GET | `/api/v1/pymthouse/sla/summary` | KPI, GPU capacity, perf-by-model (`timeframe`, `perfDays`) | | ||
| | GET | `/api/v1/pymthouse/network/price` | Pipeline pricing (`experimental: true`) | |
There was a problem hiding this comment.
These will be removed/replaced with NaaP routes. Not sure how they got in here
|
|
||
| ### PymtHouse setup (one-time) | ||
|
|
||
| 1. Set `NAAP_SERVICE_CLIENT_SECRET` in the PymtHouse environment. |
There was a problem hiding this comment.
this is incorrect/outdated info. PYMTHOUSE_ISSUER_URL, PMTHOUSE_CLIENT_ID and PMTHOUSE_CLIENT_SECRET are the only needed env vars for the integration. They can be any from PymtHouse app that is configured with client credentials grant type
There was a problem hiding this comment.
Will remove seed on pymthouse side. Worthwhile keeping one on NaaP side for billing provider, we just need these env vars to be used for it
| 3. Copy the printed secret into NaaP's **`PMTHOUSE_CLIENT_SECRET`** env var. | ||
| 4. Restart NaaP. | ||
|
|
||
| The deprecated endpoints `GET /api/v1/naap/auth` and `POST /api/v1/naap/exchange` are no longer called by NaaP. Set `LEGACY_NAAP_LINK_ENABLED=false` on PymtHouse when no other client depends on them. |
There was a problem hiding this comment.
We can remove these endpoints
| ### Verification checklist | ||
|
|
||
| - `PYMTHOUSE_ISSUER_URL` ends with `/api/v1/oidc`. | ||
| - `PMTHOUSE_CLIENT_SECRET` matches the PymtHouse `naap-service` secret. |
There was a problem hiding this comment.
Does not need to match specific service name, only the CLIENT_ID with it
Summary
Integrates PymtHouse as a NaaP billing provider and exposes stable JSON for PymtHouse’s plan-builder and admin tooling. End-user linking uses OIDC client credentials (server-to-server): no browser redirect or OAuth callback for “Create API Key,” replacing the earlier dashboard/OAuth-oriented approach.
What’s included
packages/databasebilling providers; developer API plugin metadata).POST /api/v1/auth/providers/pymthouse/start, NaaP obtains a service token viaclient_credentials, calls PymtHouse to link/provision the NaaP user, and returns the gateway API key directly. The OAuth callback route explicitly does not apply to PymtHouse.1.0):GET /api/v1/pymthouse/capabilities/catalog— pipelines/models catalogGET /api/v1/pymthouse/sla/summary— SLA-style aggregatesGET /api/v1/pymthouse/network/price— pricing hints (marked experimental where applicable)docs/pymthouse-integration.mdcovers env vars, PymtHouseoidc:seedsetup, marketplace URL behavior, and verification. Optional design/requirements doc:docs/pymthouse-minimal-design.md.Configuration (operators)
NaaP expects at minimum
PYMTHOUSE_ISSUER_URL(issuer ending in/api/v1/oidc) andPMTHOUSE_CLIENT_SECRET(confidential client, typicallynaap-servicewithgatewayscope).BILLING_PROVIDER_OAUTH_CALLBACK_ORIGINis not required for PymtHouse. See the integration doc for the full table and PymtHouse-side steps (NAAP_SERVICE_CLIENT_SECRET,npm run oidc:seed).Database
Pull schema/migration changes for
BillingProviderOAuthSessionas documented (e.g.pkceCodeVerifiernull for this flow).Testing / verification
[billing-auth:pymthouse] Linked user …and apmth_-prefixed key.GET /api/v1/pymthouse/...routes and optional caching behavior.Related
Summary by CodeRabbit
Release Notes
New Features
Configuration
Documentation