Skip to content

OAuth provider crud #759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 64 commits into from
Jul 19, 2025
Merged

OAuth provider crud #759

merged 64 commits into from
Jul 19, 2025

Conversation

fomalhautb
Copy link
Contributor

@fomalhautb fomalhautb commented Jul 11, 2025

Important

Add CRUD operations for OAuth providers, update schemas and error handling, and include tests for new functionality.

  • Behavior:
    • Adds CRUD operations for OAuth providers in client-interface.ts and server-interface.ts.
    • Introduces oauthProviderCrud in oauth-providers.ts for managing OAuth provider data.
    • Updates schema-fields.ts to include new schemas for OAuth provider attributes.
    • Adds error handling for OAuth provider operations in known-errors.tsx.
  • Schema:
    • Defines oauthProviderCrudClientUpdateSchema, oauthProviderCrudServerUpdateSchema, and oauthProviderCrudServerCreateSchema in oauth-providers.ts.
    • Updates projects.ts to include oauthProviderReadSchema and oauthProviderWriteSchema.
  • Tests:
    • Adds tests for OAuth provider CRUD operations in oauth-providers.test.ts.
  • Misc:
    • Renames oauth.ts to connected-accounts.ts in crud directory.
    • Updates projects.test.ts to include provider_config_id in OAuth provider configurations.

This description was created by Ellipsis for bca9bcf. You can customize this summary. It will automatically update as commits are pushed.


Important

Add CRUD operations for OAuth providers, update schemas, handle errors, and include tests.

  • Behavior:
    • Adds CRUD operations for OAuth providers in client-interface.ts and server-interface.ts.
    • Introduces oauthProviderCrud in oauth-providers.ts for managing OAuth provider data.
    • Updates schema-fields.ts to include new schemas for OAuth provider attributes.
    • Adds error handling for OAuth provider operations in known-errors.tsx.
  • Schema:
    • Defines oauthProviderCrudClientUpdateSchema, oauthProviderCrudServerUpdateSchema, and oauthProviderCrudServerCreateSchema in oauth-providers.ts.
    • Updates projects.ts to include oauthProviderReadSchema and oauthProviderWriteSchema.
  • Tests:
    • Adds tests for OAuth provider CRUD operations in oauth-providers.test.ts.
  • Misc:
    • Renames oauth.ts to connected-accounts.ts in crud directory.
    • Updates projects.test.ts to include provider_config_id in OAuth provider configurations.

This description was created by Ellipsis for aaa55f7. You can customize this summary. It will automatically update as commits are pushed.

- Removed the user_id and provider_id parameters from the onCreate method, replacing them with a single data object.
- Updated error messages to reflect changes in parameter names.
- Added a new schema for provider_config_id to validate incoming requests.
- Deleted unused route file for user-specific OAuth provider retrieval.
- Adjusted tests to align with the new API structure and ensure proper validation.
Copy link

vercel bot commented Jul 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2025 0:44am
stack-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2025 0:44am
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2025 0:44am
stack-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2025 0:44am

Copy link

recurseml bot commented Jul 11, 2025

✨ No issues found! Your code is sparkling clean! ✨

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
  • Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

  • Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
  • The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

  • Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
  • The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

  • 'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

  • When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

⚠️ Only 5 files were analyzed due to processing limits.

Need help? Join our Discord for support!
https://discord.gg/qEjHQk64Z9

@fomalhautb fomalhautb marked this pull request as ready for review July 16, 2025 20:30
@fomalhautb fomalhautb requested a review from N2D4 July 16, 2025 20:30
@fomalhautb fomalhautb assigned N2D4 and unassigned fomalhautb Jul 16, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR implements a significant refactoring of the OAuth provider system in stack-auth. The key changes include:

  1. Restructuring OAuth account relationships by merging ConnectedAccount functionality into ProjectUserOAuthAccount
  2. Adding explicit allowSignIn and allowConnectedAccounts flags to control OAuth account usage
  3. Introducing a new provider_config_id field for more flexible OAuth provider configuration management
  4. Adding comprehensive CRUD operations for OAuth providers with proper validation and access controls
  5. Improving token storage and management by tying tokens directly to OAuth accounts

The changes enable more flexible OAuth provider management while maintaining data integrity and security. The new structure allows OAuth accounts to exist independently of users (optional projectUserId) and provides better control over how OAuth accounts can be used for authentication or as connected accounts.

Confidence score: 3/5

  1. This PR introduces significant structural changes to OAuth handling that should be carefully tested
  2. While the implementation is thorough, the complexity of OAuth-related changes and the broad scope of the refactoring introduces risks
  3. The following files need particular attention:
    • apps/backend/prisma/migrations/20250711232750_oauth_method/migration.sql (complex data migration)
    • apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx (critical auth flow changes)
    • apps/backend/src/app/api/latest/oauth-providers/crud.tsx (new validation logic)

22 files reviewed, 5 comments
Edit PR Review Bot Settings | Greptile

@N2D4 N2D4 changed the title OAuth provider curd OAuth provider crud Jul 17, 2025
Copy link
Contributor

@N2D4 N2D4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! it's very satisfying to see how many of the edge cases are handled

@N2D4 N2D4 assigned fomalhautb and unassigned N2D4 Jul 17, 2025
@fomalhautb fomalhautb merged commit 018be1f into dev Jul 19, 2025
14 of 17 checks passed
@fomalhautb fomalhautb deleted the oauth-provider-curd branch July 19, 2025 00:50
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.

2 participants