[Draft] Implement onboarding wizard and allow google SSO login configuration #183
Conversation
* Skip .next dir on publish * Refactor to array.includes instead of individual checks
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Introduced a comprehensive onboarding API with multiple endpoints for managing onboarding steps, including authentication, data sources, and user configurations. - Added middleware to check onboarding status and cache results to optimize API calls. - Implemented Google OAuth integration with validation and configuration management. - Enhanced user experience by redirecting users based on onboarding status. - Created new components for onboarding UI and improved data handling in the onboarding process. - Updated database schema to support onboarding progress tracking. This commit lays the groundwork for a streamlined onboarding experience, ensuring users can efficiently set up their applications.
Summary of ChangesHello @ATechAdventurer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a foundational onboarding wizard designed to simplify the initial setup experience for new users. It provides a guided, step-by-step process for configuring essential application settings, with a particular focus on authentication. The wizard now supports Google SSO integration, allowing for dynamic credential management and streamlined admin user creation. Supporting this, a new set of API endpoints and database schema have been introduced to manage onboarding state, alongside middleware to ensure a seamless flow between setup and application usage. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive onboarding wizard and dynamic Google SSO configuration. The changes are extensive, adding numerous API endpoints, UI components, middleware for routing, and database schema updates. My review has identified a critical security vulnerability concerning the exposure of a client secret, which must be addressed immediately. Additionally, I've noted several opportunities for significant refactoring to improve maintainability, particularly in a large UI component. Other comments focus on improving code quality by reducing duplication, ensuring type safety, and adopting more efficient database practices. Overall, this is a substantial feature addition that will greatly enhance the initial user experience once these issues are resolved.
| if (isAvailable && config.clientId && config.clientSecret) { | ||
| response.credentials = { | ||
| clientId: config.clientId, | ||
| clientSecret: config.clientSecret, | ||
| }; |
There was a problem hiding this comment.
Exposing the clientSecret to the frontend is a critical security vulnerability. The client secret must be kept confidential on the server and should never be sent to the client-side. The frontend only requires the clientId to initiate the OAuth flow. All operations requiring the clientSecret, such as exchanging the authorization code for a token, must be handled exclusively by the backend.
response.credentials = {
clientId: config.clientId,
};| export async function POST(request: NextRequest) { | ||
| try { | ||
| const body = await request.json(); | ||
| const { code, clientId, clientSecret } = body as { code?: string; clientId?: string; clientSecret?: string }; |
There was a problem hiding this comment.
Accepting the clientSecret from the request body is a critical security vulnerability. The client secret should never be exposed on the client-side or transmitted from it. The backend should securely retrieve the clientId and clientSecret from a secure source, such as environment variables or the database (via GoogleAuthService), after the user has configured them during the onboarding process.
| let progress = await prisma.onboardingProgress.findFirst({ | ||
| where: { userId: null }, // For initial setup, userId is null | ||
| }); |
There was a problem hiding this comment.
The logic here assumes there will only ever be one onboarding progress record with userId: null. If it's possible for multiple incomplete onboarding sessions to exist (e.g., multiple users attempting to set up the app simultaneously), findFirst without a more specific ordering or filter could lead to updating the wrong record, creating a race condition. Consider adding a unique token for each onboarding session to reliably identify and update the correct progress record.
| }, | ||
| ]; | ||
|
|
||
| export function AdminOnboarding({ onComplete, envVars }: AdminOnboardingProps) { |
There was a problem hiding this comment.
This component has grown to over 1400 lines, managing the state, logic, and rendering for the entire multi-step onboarding flow. Such large "god components" are difficult to understand, maintain, and test. This significantly increases the risk of introducing bugs in the future.
It is highly recommended to refactor this into smaller, more focused components. For example, you could have:
- A main
AdminOnboardingcomponent that manages the current step and state transitions. - Separate components for each step, such as
WelcomeStep,AuthStep, andAuthConfigStep.
This will improve code organization, readability, and reusability.
| const handleCompleteSetup = async () => { | ||
| console.log('handleCompleteSetup called with state:', { | ||
| selectedAuthMethod, | ||
| configSource, | ||
| googleUser, | ||
| credentialsValid, | ||
| session: session?.user, | ||
| }); | ||
|
|
||
| setIsLoading(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| // If application is already set up, just redirect to main page | ||
| if (isApplicationSetUp) { | ||
| onComplete(); | ||
| return; | ||
| } | ||
|
|
||
| if (!selectedAuthMethod) { | ||
| throw new Error('No authentication method selected'); | ||
| } | ||
|
|
||
| // Additional validation (not needed for Google OAuth as user info comes from OAuth) | ||
| if (selectedAuthMethod !== 'google') { | ||
| if (!adminData.name.trim()) { | ||
| throw new Error('Name is required'); | ||
| } | ||
|
|
||
| if (!adminData.email.trim()) { | ||
| throw new Error('Email is required'); | ||
| } | ||
| } | ||
|
|
||
| // Validate Google OAuth | ||
| if (selectedAuthMethod === 'google') { | ||
| console.log('Validating Google OAuth:', { configSource, googleUser, credentialsValid }); | ||
|
|
||
| // If not configured via environment variables, require manual configuration | ||
| if (configSource === 'none') { | ||
| if (!googleConfig.clientId.trim() || !googleConfig.clientSecret.trim()) { | ||
| throw new Error('Google Client ID and Client Secret are required'); | ||
| } | ||
|
|
||
| if (credentialsValid !== true) { | ||
| throw new Error('Please validate your Google OAuth credentials before proceeding'); | ||
| } | ||
| } | ||
|
|
||
| if (!googleUser) { | ||
| console.log('Google user validation failed:', { googleUser, session: session?.user }); | ||
| throw new Error('Please sign in with Google to select your admin account'); | ||
| } | ||
| } | ||
|
|
||
| // Validate SSO config if using SSO | ||
| if (selectedAuthMethod === 'sso') { | ||
| if (!ssoConfig.hostUrl.trim() || !ssoConfig.clientId.trim() || !ssoConfig.clientSecret.trim()) { | ||
| throw new Error('SSO configuration is incomplete. Please fill in all required fields.'); | ||
| } | ||
| } | ||
|
|
||
| // Validate password if using password auth | ||
| if (selectedAuthMethod === 'password') { | ||
| if (!adminData.password || !adminData.confirmPassword) { | ||
| throw new Error('Password and confirmation are required for password authentication.'); | ||
| } | ||
|
|
||
| if (adminData.password !== adminData.confirmPassword) { | ||
| throw new Error('Passwords do not match.'); | ||
| } | ||
|
|
||
| if (adminData.password.length < 8) { | ||
| throw new Error('Password must be at least 8 characters long.'); | ||
| } | ||
| } | ||
|
|
||
| const request: OnboardingRequest = { | ||
| authMethod: selectedAuthMethod, | ||
| adminData: | ||
| selectedAuthMethod === 'google' && googleUser | ||
| ? { | ||
| name: googleUser.name, | ||
| email: googleUser.email, | ||
| } | ||
| : { | ||
| name: adminData.name.trim(), | ||
| email: adminData.email.trim(), | ||
| password: adminData.password || undefined, | ||
| confirmPassword: adminData.confirmPassword || undefined, | ||
| }, | ||
| ssoConfig: selectedAuthMethod === 'sso' ? ssoConfig : undefined, | ||
| googleOAuthConfig: selectedAuthMethod === 'google' && configSource !== 'environment' ? googleConfig : undefined, | ||
| telemetryConsent, | ||
| }; | ||
|
|
||
| console.log('Sending onboarding request:', request); | ||
|
|
||
| const response = await onboardingApi.completeOnboarding(request); | ||
|
|
||
| if (!response.success) { | ||
| throw new Error(response.error); | ||
| } | ||
|
|
||
| setCurrentStep('complete'); | ||
|
|
||
| // Show countdown and automatically redirect to main page after successful completion | ||
| setRedirectCountdown(3); | ||
|
|
||
| const countdownInterval = setInterval(() => { | ||
| setRedirectCountdown((prev) => { | ||
| if (prev <= 1) { | ||
| clearInterval(countdownInterval); | ||
| onComplete(); | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| return prev - 1; | ||
| }); | ||
| }, 1000); | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : 'An error occurred'); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The validation logic within handleCompleteSetup is largely a duplication of the logic found in the handleNext function (specifically for the auth-config step). This duplication can easily lead to inconsistencies and bugs if one is updated and the other is not. This logic should be extracted into a single, reusable validation function that can be called from both places to ensure consistency and improve maintainability.
| if (authMethod === 'password') { | ||
| // For password auth, we need to create the user through the auth system | ||
| // This is a simplified approach - in a real implementation, you'd want to | ||
| // use the proper auth flow | ||
| adminUser = await prisma.user.create({ | ||
| data: { | ||
| name: adminData.name, | ||
| email: adminData.email, | ||
| emailVerified: true, | ||
| isAnonymous: false, | ||
| telemetryEnabled: telemetryConsent, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }, | ||
| }); | ||
| } else { | ||
| // For OAuth/SSO, create user without password | ||
| adminUser = await prisma.user.create({ | ||
| data: { | ||
| name: adminData.name, | ||
| email: adminData.email, | ||
| emailVerified: true, | ||
| isAnonymous: false, | ||
| telemetryEnabled: telemetryConsent, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The logic for creating a user is duplicated in the if (authMethod === 'password') block and the else block. Since the prisma.user.create call is identical in both branches, you can remove the conditional logic and have a single user creation step. This will make the code more concise and easier to maintain.
adminUser = await prisma.user.create({
data: {
name: adminData.name,
email: adminData.email,
emailVerified: true,
isAnonymous: false,
telemetryEnabled: telemetryConsent,
createdAt: new Date(),
updatedAt: new Date(),
},
});|
|
||
| async function storeGoogleUserData(userData: any) { | ||
| try { | ||
| const { prisma } = await import('~/lib/prisma'); |
There was a problem hiding this comment.
This file is a server-side route handler. The dynamic import await import('~/lib/prisma') is unnecessary here. It can be replaced with a static import at the top of the file for better performance, readability, and to allow static analysis tools to work correctly.
| adminData: { | ||
| ...((progress.adminData as any) || {}), | ||
| googleUser: googleUserData, | ||
| } as any, |
There was a problem hiding this comment.
The as any cast for adminData and progress.adminData bypasses TypeScript's type safety. This can lead to runtime errors if the shape of the data is not what you expect. It's better to ensure the googleUserData object is correctly typed and merged safely with the existing adminData. Using a Zod schema to parse progress.adminData before merging would be a much safer approach.
| // Create auth configuration dynamically | ||
| async function createAuthConfig() { | ||
| // Get Google OAuth configuration dynamically | ||
| const googleConfig = await GoogleAuthService.getGoogleOAuthConfig(); | ||
|
|
||
| return betterAuth({ | ||
| database: prismaAdapter(prisma, { | ||
| provider: 'postgresql', | ||
| }), | ||
| plugins: [ | ||
| // Add SSO plugin for OIDC support | ||
| ...(enableOIDCSSO | ||
| ? [ | ||
| sso({ | ||
| provisionUser: async ({ user }) => { | ||
| // Provision user when they sign in with SSO | ||
|
|
||
| const { email } = user; | ||
|
|
||
| if (!email) { | ||
| return; | ||
| } | ||
|
|
||
| // Skip anonymous users — we only want to grant system admin to the first | ||
| // non-anonymous user. | ||
| if (user.isAnonymous) { | ||
| return; | ||
| } | ||
| } catch { | ||
| // Don't fail the auth flow if invite acceptance fails | ||
| } | ||
| }, | ||
| }), | ||
| ] | ||
| : []), | ||
| ], | ||
| emailAndPassword: { | ||
| enabled: enableEmailPassword, // Enable email/password auth for anonymous user | ||
| }, | ||
| socialProviders: { | ||
| google: { | ||
| clientId: GOOGLE_CLIENT_ID!, | ||
| clientSecret: GOOGLE_CLIENT_SECRET!, | ||
| enabled: enableGoogleAuth, | ||
|
|
||
| // Only grant admin access if the application is not set up yet | ||
| const isApplicationSetUp = await userService.isApplicationSetUp(); | ||
|
|
||
| if (!isApplicationSetUp) { | ||
| await grantSystemAdminAccess(user.id).catch(); | ||
| } | ||
|
|
||
| // Check for pending invites and auto-accept them | ||
| try { | ||
| const pendingInvite = await inviteService.getInviteByEmail(email); | ||
|
|
||
| if (pendingInvite) { | ||
| await inviteService.acceptInvite(pendingInvite.id, user.id); | ||
| } | ||
| } catch { | ||
| // Don't fail the auth flow if invite acceptance fails | ||
| } | ||
| }, | ||
| }), | ||
| ] | ||
| : []), | ||
| ], | ||
| emailAndPassword: { | ||
| enabled: enableEmailPassword, // Enable email/password auth for anonymous user | ||
| }, | ||
| }, | ||
| baseURL: BASE_URL, | ||
| trustedOrigins: [BASE_URL], | ||
| advanced: { | ||
| database: { | ||
| generateId: false, // Assumes a database handles ID generation | ||
| socialProviders: { | ||
| google: { | ||
| clientId: googleConfig.clientId, | ||
| clientSecret: googleConfig.clientSecret, | ||
| enabled: googleConfig.enabled, | ||
| }, | ||
| }, | ||
| }, | ||
| hooks: { | ||
| after: createAuthMiddleware(async (ctx) => { | ||
| if (ctx.path.endsWith('callback/liblab')) { | ||
| return; | ||
| } | ||
|
|
||
| // Handle OAuth/SSO callbacks (Google OAuth or OIDC SSO) | ||
| if (ctx.path.startsWith('/callback/')) { | ||
| const newSession = ctx.context.newSession; | ||
| baseURL: BASE_URL, | ||
| trustedOrigins: [BASE_URL], | ||
| advanced: { | ||
| database: { | ||
| generateId: false, // Assumes a database handles ID generation | ||
| }, | ||
| }, | ||
| hooks: { | ||
| after: createAuthMiddleware(async (ctx) => { | ||
| if (ctx.path.endsWith('callback/liblab')) { | ||
| return; | ||
| } | ||
|
|
||
| // Handle OAuth/SSO callbacks (Google OAuth or OIDC SSO) | ||
| if (ctx.path.startsWith('/callback/')) { | ||
| const newSession = ctx.context.newSession; | ||
|
|
||
| if (!newSession?.user?.email) { | ||
| throw new Error('Unable to complete OAuth/SSO signup: Missing user email'); | ||
| if (!newSession?.user?.email) { | ||
| throw new Error('Unable to complete OAuth/SSO signup: Missing user email'); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Handle SSO callbacks | ||
| if (ctx.path.startsWith('/sso/callback/')) { | ||
| const newSession = ctx.context.newSession; | ||
| // Handle SSO callbacks | ||
| if (ctx.path.startsWith('/sso/callback/')) { | ||
| const newSession = ctx.context.newSession; | ||
|
|
||
| if (!newSession?.user?.email) { | ||
| throw new Error('Unable to complete SSO signup: Missing user email'); | ||
| if (!newSession?.user?.email) { | ||
| throw new Error('Unable to complete SSO signup: Missing user email'); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Email/password authentication is handled automatically by Better Auth | ||
| // Email/password authentication is handled automatically by Better Auth | ||
|
|
||
| const newSession = ctx.context.newSession; | ||
| const email = newSession?.user?.email as string | undefined; | ||
|
|
||
| const newSession = ctx.context.newSession; | ||
| const email = newSession?.user?.email as string | undefined; | ||
| if (!email) { | ||
| return; | ||
| } | ||
|
|
||
| if (!email) { | ||
| return; | ||
| } | ||
| const createdUser = await prisma.user.findUnique({ where: { email } }); | ||
|
|
||
| const createdUser = await prisma.user.findUnique({ where: { email } }); | ||
| if (!createdUser) { | ||
| return; | ||
| } | ||
|
|
||
| if (!createdUser) { | ||
| return; | ||
| } | ||
| // Skip anonymous users — we only want to grant system admin to the first | ||
| // non-anonymous user. | ||
| if (createdUser.isAnonymous) { | ||
| return; | ||
| } | ||
|
|
||
| // Skip anonymous users — we only want to grant system admin to the first | ||
| // non-anonymous user. | ||
| if (createdUser.isAnonymous) { | ||
| return; | ||
| } | ||
| // Only grant admin access if the application is not set up yet | ||
| const isApplicationSetUp = await userService.isApplicationSetUp(); | ||
|
|
||
| await grantSystemAdminAccess(createdUser.id); | ||
| if (!isApplicationSetUp) { | ||
| await grantSystemAdminAccess(createdUser.id); | ||
| } | ||
|
|
||
| // Check for pending invites and auto-accept them | ||
| try { | ||
| const pendingInvite = await inviteService.getInviteByEmail(email); | ||
| // Check for pending invites and auto-accept them | ||
| try { | ||
| const pendingInvite = await inviteService.getInviteByEmail(email); | ||
|
|
||
| if (pendingInvite) { | ||
| await inviteService.acceptInvite(pendingInvite.id, createdUser.id); | ||
| if (pendingInvite) { | ||
| await inviteService.acceptInvite(pendingInvite.id, createdUser.id); | ||
| } | ||
| } catch { | ||
| // Don't fail the auth flow if invite acceptance fails | ||
| } | ||
| } catch { | ||
| // Don't fail the auth flow if invite acceptance fails | ||
| } | ||
| }), | ||
| }, | ||
| }); | ||
| }), | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The introduction of an async function createAuthConfig to dynamically configure betterAuth is a great improvement. However, there's some duplicated logic between the provisionUser callback for SSO and the global hooks.after callback. Both contain logic for granting admin access and accepting invites. This could be consolidated into a single, reusable function to handle post-authentication actions for new users, regardless of the authentication method.
| let onboardingStatusCache: { isSetUp: boolean; timestamp: number } | null = null; | ||
| const CACHE_DURATION = 5 * 60 * 1000; // 5 minutes |
There was a problem hiding this comment.
Using a module-level variable onboardingStatusCache for caching in the middleware can lead to issues in serverless environments, where the middleware function might be instantiated multiple times. While this works in a single-process server, it's not a reliable caching strategy for edge or serverless functions. Consider using a more robust caching solution like @upstash/redis or Next.js's own data caching mechanisms if this is intended for a scalable environment.
This commit lays the groundwork for a streamlined onboarding experience, ensuring users can efficiently set up their applications.
📋 Pull Request Summary
🔗 Related Issues
📝 Changes Made
🧪 Testing
Testing Details:
📚 Documentation
🔄 Type of Change
🚨 Breaking Changes
Breaking Change Details:
📸 Screenshots/Videos
📋 Additional Notes