Revert "fix(salesforce): updated to more flexible oauth that allows production, developer, and custom domain salesforce orgs (#2441) (#2444)" #2453
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile Summary
Important Files Changed
Confidence score: 1/5
Sequence DiagramsequenceDiagram
participant User
participant SimStudio as "SimStudio App"
participant DB as "Database"
participant SalesforceAuth as "Salesforce OAuth"
participant SalesforceAPI as "Salesforce API"
User->>SimStudio: "Initiate OAuth connection"
SimStudio->>SalesforceAuth: "Redirect to authorization endpoint"
User->>SalesforceAuth: "Authorize application"
SalesforceAuth->>SimStudio: "Return authorization code"
SimStudio->>SalesforceAuth: "Exchange code for tokens"
SalesforceAuth->>SimStudio: "Return access token, refresh token, ID token"
SimStudio->>DB: "Store OAuth credentials"
User->>SimStudio: "Execute Salesforce tool"
SimStudio->>DB: "Retrieve stored credentials"
alt Token expired
SimStudio->>SalesforceAuth: "Refresh token request"
SalesforceAuth->>SimStudio: "Return new access token"
SimStudio->>DB: "Update stored tokens"
end
SimStudio->>SimStudio: "Extract instance URL from ID token"
SimStudio->>SalesforceAPI: "API request with access token"
SalesforceAPI->>SimStudio: "Return API response"
SimStudio->>User: "Return processed results"
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Additional Comments (2)
-
apps/sim/app/api/auth/oauth2/callback/salesforce/route.tslogic: Complete removal of Salesforce OAuth callback handler will break all Salesforce authentication flows. The
/api/auth/oauth2/callback/salesforceendpoint will return 404, causing OAuth failures. Is there a replacement callback handler being implemented, or should this revert to a previous working version instead of complete deletion? -
apps/sim/tools/salesforce/utils.ts, line 46 (link)style: Using
anytype instead of proper TypeScript typingContext Used: Context from
dashboard- TypeScript conventions and type safety (source)
12 files reviewed, 5 comments
Comment on lines
+157
to
+183
| if (!instanceUrl && params.idToken) { | ||
| try { | ||
| const base64Url = params.idToken.split('.')[1] | ||
| const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/') | ||
| const jsonPayload = decodeURIComponent( | ||
| atob(base64) | ||
| .split('') | ||
| .map((c) => `%${(`00${c.charCodeAt(0).toString(16)}`).slice(-2)}`) | ||
| .join('') | ||
| ) | ||
| const decoded = JSON.parse(jsonPayload) | ||
|
|
||
| if (decoded.profile) { | ||
| const match = decoded.profile.match(/^(https:\/\/[^/]+)/) | ||
| if (match) { | ||
| instanceUrl = match[1] | ||
| } | ||
| } else if (decoded.sub) { | ||
| const match = decoded.sub.match(/^(https:\/\/[^/]+)/) | ||
| if (match && match[1] !== 'https://login.salesforce.com') { | ||
| instanceUrl = match[1] | ||
| } | ||
| } | ||
| } catch (error) { | ||
| logger.error('Failed to decode Salesforce idToken', { error }) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
style: JWT token parsing logic now duplicated across multiple Salesforce tools. Should this logic be extracted to a shared utility to avoid duplication across other Salesforce tools?
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/tools/salesforce/update_account.ts
Line: 157:183
Comment:
**style:** JWT token parsing logic now duplicated across multiple Salesforce tools. Should this logic be extracted to a shared utility to avoid duplication across other Salesforce tools?
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+153
to
+160
| const base64Url = params.idToken.split('.')[1] | ||
| const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/') | ||
| const jsonPayload = decodeURIComponent( | ||
| atob(base64) | ||
| .split('') | ||
| .map((c) => `%${(`00${c.charCodeAt(0).toString(16)}`).slice(-2)}`) | ||
| .join('') | ||
| ) |
Contributor
There was a problem hiding this comment.
style: Complex JWT decoding logic should be extracted to a utility function for reuse across Salesforce tools
Context Used: Context from dashboard - Core architecture principles for the Sim app (source). Was there a specific issue with the centralized getInstanceUrl utility that required this reversion?
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/tools/salesforce/create_account.ts
Line: 153:160
Comment:
**style:** Complex JWT decoding logic should be extracted to a utility function for reuse across Salesforce tools
**Context Used:** Context from `dashboard` - Core architecture principles for the Sim app ([source](https://app.greptile.com/review/custom-context?memory=2d6e479d-8e35-4102-a11d-13cf026616dd)). Was there a specific issue with the centralized `getInstanceUrl` utility that required this reversion?
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+67
to
+93
| if (!instanceUrl && params.idToken) { | ||
| try { | ||
| const base64Url = params.idToken.split('.')[1] | ||
| const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/') | ||
| const jsonPayload = decodeURIComponent( | ||
| atob(base64) | ||
| .split('') | ||
| .map((c) => `%${(`00${c.charCodeAt(0).toString(16)}`).slice(-2)}`) | ||
| .join('') | ||
| ) | ||
| const decoded = JSON.parse(jsonPayload) | ||
|
|
||
| if (decoded.profile) { | ||
| const match = decoded.profile.match(/^(https:\/\/[^/]+)/) | ||
| if (match) { | ||
| instanceUrl = match[1] | ||
| } | ||
| } else if (decoded.sub) { | ||
| const match = decoded.sub.match(/^(https:\/\/[^/]+)/) | ||
| if (match && match[1] !== 'https://login.salesforce.com') { | ||
| instanceUrl = match[1] | ||
| } | ||
| } | ||
| } catch (error) { | ||
| logger.error('Failed to decode Salesforce idToken', { error }) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
style: This complex JWT parsing logic appears to be duplicated across multiple Salesforce tools. Consider extracting to a shared utility function to improve maintainability.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/tools/salesforce/get_accounts.ts
Line: 67:93
Comment:
**style:** This complex JWT parsing logic appears to be duplicated across multiple Salesforce tools. Consider extracting to a shared utility function to improve maintainability.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 9da19e8.