Devin/oauth developer settings 1764693294#4
Open
DhirenMhatre wants to merge 104 commits into
Open
Conversation
- Add new developer OAuth page at /settings/developer/oAuth for users to submit OAuth client requests - Transform admin OAuth page into management dashboard for reviewing/approving submissions - Add OAuthClientApprovalStatus enum (PENDING, APPROVED, REJECTED) to track submission status - Add userId and createdAt fields to OAuthClient model for tracking submissions - Create email notifications for admin (new submission) and user (approval) - Add sidebar navigation link in developer section below API keys - Add comprehensive translations for new UI strings - Create OAuthClientRepository for data access following repository pattern Co-Authored-By: peer@cal.com <peer@cal.com>
Co-Authored-By: peer@cal.com <peer@cal.com>
Co-Authored-By: peer@cal.com <peer@cal.com>
…ls dialog Co-Authored-By: peer@cal.com <peer@cal.com>
- Remove duplicate 'there' JSON key in common.json - Add select clause to findByUserId to avoid exposing clientSecret - Add @@index([userId]) to OAuthClient model for query performance - Update migration to include the index Co-Authored-By: peer@cal.com <peer@cal.com>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…ility" This reverts commit c3e0b70.
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…/server issue Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…eact-dom/server issue" This reverts commit fc9d47c.
… styling - Replace 'Loading...' text with proper skeleton loaders in both developer and admin OAuth client views - Make client_id and copy button smaller in dialogs using size='sm' and text-sm styling - Add 'client_id' translation key to common.json for proper i18n Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
- Remove divide-y from container and use conditional border-b on rows - Match the exact structure from oauth-clients-view.tsx L126-160 - Use proper spacing for text elements (mt-1 instead of space-y-2) Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
- Add defense-in-depth admin authorization check in updateClientStatus handler - Fix broken dropdown menu by using DropdownItem with StartIcon prop - Fix sidebar menu label from 'oAuth' to 'oauth_clients' to match developer view Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…ents - Add regenerateSecret method to OAuthClientRepository - Regenerate secret when admin approves a PENDING confidential client - Include client secret in approval notification email - Add one-time warning message about storing the secret securely - Only regenerate on first approval (not re-approvals) Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…er approval - Add Website URL field to OAuth client forms (admin and developer views) - Fix Upload Logo section styling by wrapping in Label div with proper gap - Display client secret in dialog after admin approves a confidential OAuth client - Add websiteUrl field to Prisma schema with migration - Update tRPC handlers and repository to support websiteUrl - Add translation keys for new UI elements Co-Authored-By: peer@cal.com <peer@cal.com>
…er scoping Co-Authored-By: peer@cal.com <peer@cal.com>
PR SummaryWhat Changed
Key Changes by AreaAuthentication/Authorization
Admin/Developer Settings
Email Notifications
Database
Testing
Files Changed
Review Focus Areas
ArchitectureDesign Decisions
Scalability & Extensibility
Risks
|
![codity-dm[bot]](https://avatars.githubusercontent.com/u/234452856?s=60&v=4){kind=small}
Comment on lines
+40
to
+46
| } = trpc.viewer.oAuth.getClientForAuthorization.useQuery( | ||
| { | ||
| clientId: client_id as string, | ||
| redirectUri: redirect_uri, | ||
| }, | ||
| { | ||
| enabled: status !== "loading", | ||
| enabled: status === "authenticated" && !!redirect_uri, |
There was a problem hiding this comment.
The client lookup now requires redirect_uri and bails out unless it is present (enabled: status === "authenticated" && !!redirect_uri), but the later success/error redirects still assume client.redirectUri exists and build a URL from it. If redirect_uri is missing or malformed in the incoming authorize request, the page no longer loads the client and the user is left on a blank screen instead of being redirected or shown a clear error. For example, an authorize link without redirect_uri now never reaches the OAuth client fetch that used to run.
Suggested fix
{
clientId: client_id as string,
redirectUri: redirect_uri,
},
{
enabled: status === "authenticated",
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: apps/web/modules/auth/oauth2/authorize-view.tsx
Lines: 40-46
Issue Type: robustness-medium
Severity: medium
Issue Description:
The client lookup now requires `redirect_uri` and bails out unless it is present (`enabled: status === "authenticated" && !!redirect_uri`), but the later success/error redirects still assume `client.redirectUri` exists and build a URL from it. If `redirect_uri` is missing or malformed in the incoming authorize request, the page no longer loads the client and the user is left on a blank screen instead of being redirected or shown a clear error. For example, an authorize link without `redirect_uri` now never reaches the OAuth client fetch that used to run.
Current Code:
{
clientId: client_id as string,
redirectUri: redirect_uri,
},
{
enabled: status === "authenticated" && !!redirect_uri,
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 59.7s View vulnerability details (1 items)1. SEC-001 (CWE-601) MEDIUM File:
Fix: Review the security issue and implement appropriate mitigations. Security scan powered by Codity.ai |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist