[feature] SC-166737/improve app proxy security by restricting where token replacements can go#54
Conversation
…oken replacements can go
Reviewer's GuideImplements a controlled settingsInjection mechanism in manifest.json for dynamic credential header injection on OAuth2 endpoints, and strengthens type safety for the placeholders constant by marking it immutable. Entity relationship diagram for updated manifest.json endpoint configurationerDiagram
ENDPOINT {
string url
string[] methods
int timeout
SettingsInjection settingsInjection
}
SettingsInjection {
string[] header
}
ENDPOINT ||--|{ SettingsInjection : contains
SettingsInjection }|--|{ Credential : injects
Credential {
string client_id
string client_secret
string integration_key
string secret_key
}
Class diagram for updated placeholders constantclassDiagram
class placeholders {
+string OAUTH2_ACCESS_TOKEN_PATH
+string OAUTH2_REFRESH_TOKEN_PATH
+string IS_USING_SANDBOX
}
placeholders : <<immutable>>
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Build for commit b99b12f deployed to: https://quickbooks-pr-54.ci.next.deskprodemo.com URLs: |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances security by restricting credential injection locations through the settingsInjection configuration in the proxy whitelist, and improves type safety in constants. The changes ensure that sensitive credentials are only injected into specific headers for OAuth endpoints.
Key Changes:
- Added
settingsInjectionconfiguration to OAuth and token revocation endpoints to explicitly define where credentials can be injected - Updated
placeholdersobject withas constfor improved type inference and immutability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/constants.ts | Added as const assertion to the placeholders object for better type safety |
| manifest.json | Added settingsInjection configuration to OAuth endpoints to restrict credential injection to Authorization headers only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "settingsInjection": { | ||
| "client_id": { | ||
| "header": ["Authorization"] | ||
| }, | ||
| "client_secret": { | ||
| "header": ["Authorization"] | ||
| }, | ||
| "integration_key": { | ||
| "header": ["Authorization"] | ||
| }, | ||
| "secret_key": { | ||
| "header": ["Authorization"] | ||
| } | ||
| } |
There was a problem hiding this comment.
All four credentials (client_id, client_secret, integration_key, secret_key) are configured to inject into the same Authorization header. This appears inconsistent with the actual usage patterns in the codebase: refreshAccessToken.ts uses __integration_key+':'+secret_key.base64__ in the Authorization header, while getQuickBooksAccessToken.ts sends client_id and client_secret in the request body. Consider reviewing whether all these credentials should actually be injected into the Authorization header, or if some should be restricted to body parameters instead.
1 participant
This pull request introduces improvements to the way sensitive credentials are injected into API requests and refines type safety in the constants file. The most significant updates are in the API manifest configuration, enabling dynamic injection of credentials into request headers for specific endpoints, and a minor enhancement to the constants definition for better type inference.
API Credential Injection Enhancements:
settingsInjectionconfiguration to relevant endpoints inmanifest.json, allowingclient_id,client_secret,integration_key, andsecret_keyto be dynamically injected into theAuthorizationheader for authentication requests.settingsInjectionlogic to the token revocation endpoint, supporting dynamic header injection forclient_idandclient_secret.Type Safety and Constants Improvements:
placeholdersobject insrc/constants.tsto useas const, improving type safety and ensuring the object’s values are treated as immutable literals.Summary by Sourcery
Limit where token-related settings can be injected by adding a
settingsInjectionconfiguration to OAuth token endpoints and strengthen constant definitions for safer type inferenceNew Features:
settingsInjectionAuthorizationheader for access token and token revocation URLsEnhancements:
placeholdersobject as immutable withas const