OAuth credential sync and app integration enhancements #4
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
…11059) * Add credential sync .env variables * Add webhook to send app credentials * Upsert credentials when webhook called * Refresh oauth token from a specific endpoint * Pass appSlug * Add credential encryption * Move oauth helps into a folder * Create parse token response wrapper * Add OAuth helpers to apps * Clean up * Refactor `appDirName` to `appSlug` * Address feedback * Change to safe parse * Remove console.log --------- Co-authored-by: Syed Ali Shahbaz <[email protected]> Co-authored-by: Omar López <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Author
Greptile OverviewConfidence Score: 0/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant CalApp as Cal.com App
participant OAuthProvider as OAuth Provider
participant WebhookAPI as Webhook API
participant Database
participant SyncService as External Sync Service
Note over User,SyncService: OAuth Integration Flow
User->>CalApp: Install app integration
CalApp->>OAuthProvider: Initiate OAuth flow
OAuthProvider->>User: Request authorization
User->>OAuthProvider: Approve
OAuthProvider->>CalApp: Redirect with auth
CalApp->>OAuthProvider: Exchange for tokens
OAuthProvider-->>CalApp: Return tokens
CalApp->>Database: Store credentials
Note over User,SyncService: Credential Sync (New Feature)
SyncService->>WebhookAPI: POST encrypted credentials
WebhookAPI->>WebhookAPI: Verify authentication
WebhookAPI->>WebhookAPI: Decrypt payload
WebhookAPI->>Database: Validate user exists
WebhookAPI->>Database: Create or update credential
WebhookAPI-->>SyncService: Return status
Note over User,SyncService: Token Refresh (Modified)
CalApp->>CalApp: Check token expiration
alt Sync Enabled
CalApp->>SyncService: Request refresh
SyncService-->>CalApp: Return tokens
else Standard
CalApp->>OAuthProvider: Request refresh
OAuthProvider-->>CalApp: Return tokens
end
CalApp->>Database: Update credentials
|
There was a problem hiding this comment.
Additional Comments (2)
-
packages/app-store/_utils/oauth/decodeOAuthState.ts, line 9 (link)logic:
JSON.parsewithout try-catch allows malicious/malformed state parameter to crash server -
packages/app-store/_utils/oauth/createOAuthAppCredential.ts, line 59 (link)logic: Unreachable code - if
state?.teamIdexists, function returns on line 56, so this check will never execute for team credentials
40 files reviewed, 7 comments
| keys: z.string(), | ||
| }); | ||
| /** */ | ||
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { |
There was a problem hiding this comment.
logic: Missing HTTP method validation allows GET/PUT/DELETE requests
Suggested change
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { | |
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { | |
| if (req.method !== 'POST') { | |
| return res.status(405).json({ message: "Method not allowed" }); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/pages/api/webhook/app-credential.ts
Line: 17:17
Comment:
**logic:** Missing HTTP method validation allows GET/PUT/DELETE requests
```suggestion
export default async function handler(req: NextApiRequest, res: NextApiResponse) {
if (req.method !== 'POST') {
return res.status(405).json({ message: "Method not allowed" });
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+25
to
+27
| if (!refreshTokenResponse.data.refresh_token) { | ||
| refreshTokenResponse.data.refresh_token = "refresh_token"; | ||
| } |
There was a problem hiding this comment.
logic: Silently sets placeholder value on missing refresh_token - this will break OAuth flows that require valid refresh tokens
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts
Line: 25:27
Comment:
**logic:** Silently sets placeholder value on missing refresh_token - this will break OAuth flows that require valid refresh tokens
How can I resolve this? If you propose a fix, please make it concise.| const minimumTokenResponseSchema = z.object({ | ||
| access_token: z.string(), | ||
| // Assume that any property with a number is the expiry | ||
| [z.string().toString()]: z.number(), |
There was a problem hiding this comment.
syntax: Invalid Zod schema - bracket notation with z.string().toString() is not valid Zod syntax and will fail at runtime
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/_utils/oauth/parseRefreshTokenResponse.ts
Line: 8:8
Comment:
**syntax:** Invalid Zod schema - bracket notation with `z.string().toString()` is not valid Zod syntax and will fail at runtime
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+57
to
+59
| const keys = JSON.parse( | ||
| symmetricDecrypt(reqBody.keys, process.env.CALCOM_APP_CREDENTIAL_ENCRYPTION_KEY || "") | ||
| ); |
There was a problem hiding this comment.
logic: Empty string fallback for encryption key will cause decryption to silently fail or produce incorrect results
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/pages/api/webhook/app-credential.ts
Line: 57:59
Comment:
**logic:** Empty string fallback for encryption key will cause decryption to silently fail or produce incorrect results
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+8
to
+14
| const response = await fetch(process.env.CALCOM_CREDENTIAL_SYNC_ENDPOINT, { | ||
| method: "POST", | ||
| body: new URLSearchParams({ | ||
| calcomUserId: userId.toString(), | ||
| appSlug, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
style: Response from external sync endpoint is returned without validation or error handling
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/app-store/_utils/oauth/refreshOAuthTokens.ts
Line: 8:14
Comment:
**style:** Response from external sync endpoint is returned without validation or error handling
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.
Test 8
Replicated from ai-code-review-evaluation/cal.com-greptile#8