Skip to content

Add Payments API base URL configuration - #38

Open
jSylvestre wants to merge 3 commits into
mainfrom
JCS/UseKeyVaultForPaymentsAPI
Open

jSylvestre wants to merge 3 commits into
mainfrom
JCS/UseKeyVaultForPaymentsAPI

Conversation

@jSylvestre

@jSylvestre jSylvestre commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features
    • Added Payments connection settings to team overviews, showing connection status and allowing team or site administrators to add or replace an API key and recheck the connection. Editors and viewers can see payment status but cannot manage the key.
    • Added validation and feedback for saved keys, connection failures, and access restrictions.
  • Configuration
    • Added an optional Payments API base URL setting for Azure deployments and local server configuration. Empty values are omitted.
  • Documentation
    • Documented Payments setup for test and production deployments, including Key Vault access.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b544c5de-718a-4994-b4fb-79316db27273

📥 Commits

Reviewing files that changed from the base of the PR and between 6717cd7 and ad8066e.


📒 Files selected for processing (22)
  • README.md
  • client/src/features/teams/EditTeamPaymentsDialog.tsx
  • client/src/features/teams/TeamPaymentsSettings.tsx
  • client/src/features/teams/models/TeamPaymentsSettings.ts
  • client/src/queries/teams.ts
  • client/src/routes/(authenticated)/teams.$teamSlug.index.tsx
  • client/src/test/mswUtils.ts
  • client/src/test/routes/(authenticated)/admin.teams.test.tsx
  • client/src/test/routes/(authenticated)/teams.test.tsx
  • infrastructure/azure/README.md
  • server/.env.example
  • server/Controllers/TeamPaymentsController.cs
  • server/Models/Payments/PaymentsOptions.cs
  • server/Models/Payments/PaymentsTeam.cs
  • server/Models/Teams/SaveTeamPaymentsRequest.cs
  • server/Models/Teams/TeamPaymentsResponse.cs
  • server/Program.cs
  • server/Services/PaymentsService.cs
  • server/Services/SecretsService.cs
  • tests/server.tests/Controllers/TeamPaymentsControllerTests.cs
  • tests/server.tests/Services/PaymentsServiceTests.cs
  • tests/server.tests/Services/SecretsServiceTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The change adds team-level Payments connections. Administrators can submit an API key for validation and storage, while team members can view connection status and request a recheck. Azure deployment paths can configure the Payments base URL.

Changes

Team Payments connections

Layer / File(s) Summary
Payments service and configuration
server/Models/Payments/*, server/Services/PaymentsService.cs, server/Services/SecretsService.cs, server/Program.cs, server/appsettings.json, server/.env.example, tests/server.tests/Services/*
Adds Payments configuration and team data contracts, service lookups by API key or secret name, and lazy Key Vault client resolution. Service tests cover requests, validation, error handling, and client configuration.
Team connection API and persistence
server/Models/Teams/*, server/Controllers/TeamPaymentsController.cs, tests/server.tests/Controllers/TeamPaymentsControllerTests.cs
Adds endpoints to retrieve and save team Payments settings. The save endpoint validates the candidate key before storing it under a generated secret name and updating the team connection. Controller tests cover access rules, failure handling, and saved-state preservation.
Team overview settings and editing
client/src/features/teams/*, client/src/features/teams/models/TeamPaymentsSettings.ts, client/src/queries/teams.ts, client/src/routes/(authenticated)/teams.$teamSlug.index.tsx, client/src/test/*
Adds the team Payments settings panel, API-key editor, queries, and team overview integration. Client tests cover saves, status checks, access changes, team switches, and effective-user changes.
Azure Payments URL deployment configuration
.github/workflows/*, infrastructure/azure/*, README.md
Adds PAYMENTS_BASE_URL configuration and maps non-empty values to the App Service setting Payments__BaseUrl. Documentation describes Payments connection setup and deployment configuration.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature · Unblocks: 1 PR

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant TeamPaymentsDialog
  participant TeamPaymentsController
  participant PaymentsService
  participant SecretsService
  participant Database
  Admin->>TeamPaymentsDialog: Submit API key
  TeamPaymentsDialog->>TeamPaymentsController: PUT team payment settings
  TeamPaymentsController->>PaymentsService: Validate candidate key
  PaymentsService-->>TeamPaymentsController: Return team details
  TeamPaymentsController->>SecretsService: Store key under generated secret name
  TeamPaymentsController->>Database: Save secret reference and team slug
  TeamPaymentsController-->>TeamPaymentsDialog: Return connection settings
Loading

Merge Risk

Merge Risk: ⚪ Minimal · up to ad806

This change adds team-level Payments connections: administrators can verify and store an API key, and team members can see the connection status. Keys are checked before they are stored and are sent only to an HTTPS endpoint. No concrete defects were identified, so the change appears ready to merge.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to ad806

Team permissions and secure transport protect the new credential flow. However, replacements and interrupted saves can leave credentials stored without a current team reference, and their reconciliation and retirement process is not established.

Retained concerns

  • Medium · security · observed: The new save lifecycle leaves superseded and failed-save credential copies in Key Vault without an application reconciliation or retirement path. Each save creates a unique secret, while the team retains only its current reference. Repetition, concurrent replacements, and interruption after the vault write can therefore leave credentials outside current connection ownership. External revocation or an operational cleanup process was not established.

Security review details

Security Blast Radius

  • inferred — Ordinary request authority is bounded to the user's authorized teams; existing site administrators retain all-team authority. At the storage boundary, the configured application identity has vault-wide authority, so its compromise could expose credentials for multiple teams in that environment, including retained historical copies. Actual deployed grants, environment separation, and each key's downstream Payments privileges were not verified.

Security Findings and Attack Paths

  • inferred — The lifecycle concern does not establish an unauthenticated disclosure path. It extends potential exposure for an actor who obtains vault secret-read authority: superseded or unattached keys remain stored after the current team reference changes. Continued upstream usability depends on external revocation, which was not established.

Trust Boundaries and Controls

  • observed — Authentication and authorization middleware precede controller dispatch. GET requires active team access and independently rechecks access before credential retrieval. PUT requires the team-admin policy and an explicit current-state administration check before upstream validation or vault mutation. Antiforgery protection applies to the controller, and responses prohibit storage.

Resilience and Maintainability Implications

  • observed — The Payments client bounds verification with a 15-second timeout and 64 KiB response buffer. Rejected credentials, upstream failures, and saved-team identity mismatches produce distinct connection states without mutating the saved reference during GET. These controls contain verification failures but do not resolve cross-store credential ownership after interrupted saves.

Hardening Proposals

  • proposed — Introduce a recoverable credential lifecycle that records pending writes and reconciles them against authoritative committed team references before retirement. Use a grace period for uncertain commits and concurrent readers; do not immediately delete a secret merely because a save returned an error. Define retry identity, retention ownership, and external key-revocation expectations.



🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 20 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes a real and significant change: adding the Payments API base URL configuration. The pull request also adds broader Payments integration features, but the title remains conci…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 20 files. (3 skipped: 3 unsupported.)



  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

1 participant