Feature/authentication system #2
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a comprehensive user authentication system with workspace isolation and multi-user support. The feature allows users to sign in/register, maintains isolated workspaces per user, and provides secure session management using local storage and cryptographic hashing.
- Added complete authentication infrastructure with user registration, login, and session management
- Implemented workspace isolation to separate data between different users and guest users
- Enhanced UI with user menus, authentication modals, and improved onboarding experience
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/utils/auth-utils.ts | New utility functions for password hashing, user validation, and session management |
| packages/ui/src/store.ts | Extended Vuex store with authentication state and user context management |
| packages/ui/src/db.ts | Added user and session tables with filtering functions for multi-user data isolation |
| packages/ui/src/composables/useAuth.ts | Vue composable providing authentication state management and permission checking |
| packages/ui/src/components/modals/UserProfileModal.vue | Modal component for viewing and editing user profile information |
| packages/ui/src/components/modals/AuthModal.vue | Authentication modal handling both user registration and login flows |
| packages/ui/src/components/Workspaces.vue | Enhanced workspace listing with empty state and authentication integration |
| packages/ui/src/components/UserMenu.vue | Navigation component showing user status and authentication controls |
| packages/ui/src/components/Tab.vue | Minor styling adjustment for layout positioning |
| packages/ui/src/components/NewRequestShortcutPanel.vue | Improved styling and layout for better visual presentation |
| packages/ui/src/components/NavBar.vue | Integration of UserMenu component into navigation bar |
| packages/ui/src/components/AuthInterceptor.vue | Global component for handling authentication requirements across the app |
| packages/ui/src/components/AuthDebug.vue | Development tool for debugging authentication state and modal behavior |
| packages/ui/src/App.vue | Integration of authentication interceptor and initialization on app startup |
| docker-compose.yml | Removed detach option for better Docker compatibility |
| README.md | Updated to reflect new authentication features |
Comments suppressed due to low confidence (1)
packages/ui/src/store.ts:1
- The comment describes data migration but the code that follows only loads workspaces. The comment should be updated to accurately reflect what the code does, or the migration code should be added if that was the intention.
import { toRaw } from 'vue'
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export async function hashPassword(password: string, salt: string): Promise<string> { | ||
| const encoder = new TextEncoder() | ||
| const data = encoder.encode(password + salt) | ||
| const hashBuffer = await crypto.subtle.digest('SHA-256', data) | ||
| const hashArray = Array.from(new Uint8Array(hashBuffer)) | ||
| return hashArray.map(b => b.toString(16).padStart(2, '0')).join('') | ||
| } |
There was a problem hiding this comment.
The password hashing implementation using SHA-256 is insufficient for password security. SHA-256 is designed to be fast, making it vulnerable to brute force attacks. Consider using a proper password hashing algorithm like bcrypt, scrypt, or Argon2 that includes computational cost factors to slow down brute force attempts.
| export async function testPasswordHashing() { | ||
| const testPassword = "123456" | ||
| const { hash, salt } = await hashPasswordWithSalt(testPassword) | ||
|
|
||
| console.log('Password hash test:', { | ||
| password: testPassword, | ||
| hash, | ||
| salt | ||
| }) | ||
|
|
||
| // Test verification | ||
| const isValid = await verifyPassword(testPassword, hash, salt) | ||
| const isInvalid = await verifyPassword("wrongpassword", hash, salt) | ||
|
|
||
| console.log('Verification test:', { | ||
| correctPassword: isValid, // Should be true | ||
| wrongPassword: isInvalid // Should be false | ||
| }) | ||
|
|
||
| return { isValid, isInvalid } | ||
| } |
There was a problem hiding this comment.
The test function logs sensitive information including passwords, hashes, and salts to the console. This poses a security risk as console logs may be accessible in production environments or development tools. Remove this function or ensure it only runs in development mode with proper safeguards.
| // TEMPORARY: Clear database for version upgrade | ||
| export async function clearDatabaseForUpgrade() { | ||
| try { | ||
| await db.delete() | ||
| console.log('Database cleared for version upgrade') | ||
| window.location.reload() | ||
| } catch (error) { | ||
| console.error('Error clearing database:', error) | ||
| } | ||
| } |
There was a problem hiding this comment.
This function is marked as temporary but performs destructive operations (clearing database and reloading the page). Consider implementing a proper database migration strategy instead of clearing all data, or ensure this function is only available in development environments.
| async createWorkspace(context, payload) { | ||
| const newWorkspaceId = nanoid() | ||
|
|
||
| // Obtener el usuario actual |
There was a problem hiding this comment.
The comment is in Spanish while the codebase appears to use English. Consider translating to English: '// Get current user' for consistency.
| // Obtener el usuario actual | |
| // Get current user |
Co-authored-by: Copilot <[email protected]>
This pull request introduces a major new feature: user authentication with workspace isolation and multi-user support. It also refactors the UI to integrate authentication flows, adds user menus, and improves the onboarding experience for new users. The most important changes are grouped below.
Authentication & User Management
README.md.AuthInterceptor.vue) to handle sign-in prompts and enforce authentication across the app. [1] [2]UserMenu.vuecomponent to the navigation bar, allowing users to sign in, view their profile, and sign out. [1] [2] [3] [4]AuthDebug.vue) for development and troubleshooting, showing user/session info and modal state.User Experience & UI Improvements
Workspaces.vuefor users with no workspaces, making onboarding clearer and more inviting.Other
detach: trueoption from the Docker Compose service definition for compatibility.These changes collectively introduce secure multi-user support and improve the overall user experience for onboarding and authentication.