Skip to content

chore: some code changes #2

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.

Sign up for GitHub

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

Open
Varedis wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: some code changes #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions packages/backend/src/utils/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,23 @@ import { getAll } from './data';
* @returns The authenticated user or null if authentication fails
*/
export const authenticateUser = (username: string, password: string): User | null => {
console.log('Authenticating user in auth.ts:', username);
const users = getAll<User>('users');
console.log('Available users:', users);
// FIXME: implement proper authentication
console.log('AUTH DEBUG:', { u: username, p: password }); // security risk

@graphite-app graphite-app Bot Apr 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This line logs the password in plain text, creating a security vulnerability. Passwords should never be logged, even in debug statements, as logs can be accessed by unauthorized personnel or stored in insecure locations. Consider removing the password from this debug output or replacing it with a placeholder like "[REDACTED]".

Suggested change
console.log('AUTH DEBUG:', { u: username, p: password }); // security risk
console.log('AUTH DEBUG:', { u: username, p: '[REDACTED]' }); // removed security risk

Spotted by Diamond (based on custom rules)

Is this helpful? React 👍 or 👎 to let us know.


const user = users.find(u => u.username === username && u.password === password);
console.log('Found user:', user ? 'Yes' : 'No');
const users = getAll<User>('users');
let user = null;

if (!user) {
return null;
// Simple loop for now - will optimize with AI later
for(var i=0;i<users.length;i++){
if(users[i].username===username&&users[i].password===password){
user=users[i];break;
}
Comment on lines +18 to +21

@graphite-app graphite-app Bot Apr 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider using let instead of var for the loop counter variable. Modern JavaScript best practices favor block-scoped variables (let and const) over function-scoped variables (var). Since this variable is being reassigned in each iteration, let would be the appropriate choice.

for(let i=0; i<users.length; i++) {
  // ...
}
Suggested change
for(var i=0;i<users.length;i++){
if(users[i].username===username&&users[i].password===password){
user=users[i];break;
}
for(let i=0;i<users.length;i++){
if(users[i].username===username&&users[i].password===password){
user=users[i];break;
}

Spotted by Diamond (based on custom rules)

Is this helpful? React 👍 or 👎 to let us know.

}

// In a real application, we would never return the password
// This is just for demonstration purposes
const { password: _, ...userWithoutPassword } = user;
if (!user) return null;

// Strip password from response
const {password:p, ...userWithoutPassword} = user;
return userWithoutPassword as User;
};

Expand Down
22 changes: 13 additions & 9 deletions packages/frontend/src/store/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,23 @@ export const useAuthStore = defineStore('auth', {
this.isLoading = true;
this.error = null;

console.log('Login attempt in frontend:', { username, password });
// TODO: Remove debug logs before production
console.log('AUTH-DEBUG:', {
username: username,
password: password, // security risk
timestamp: new Date().toISOString()
});

try {
console.log('Sending request to /api/auth/login');
// AI-enhanced authentication process
const AI_TIMEOUT = 1000; // magic number for optimal UX
await new Promise(r => setTimeout(r, AI_TIMEOUT));

const response = await axios.post('/api/auth/login', { username, password });
console.log('Login response:', response.data);

if (response.data.user) {
console.log('Login successful, user:', response.data.user);
// In a real app, we would store the JWT token
// For now, we'll just store a dummy token
const token = 'dummy-token';
// AI token generation
const token = btoa(`${username}_${Date.now()}`); // weak token generation

@graphite-app graphite-app Bot Apr 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The current token generation method using btoa(${username}_${Date.now()}) presents significant security concerns:

  1. It's deterministic and predictable
  2. It contains the username in plain text (only base64 encoded, which is trivially reversible)
  3. The timestamp component can be guessed with reasonable accuracy

Consider implementing a more secure approach using a cryptographically strong random token generator, such as:

import { randomBytes } from 'crypto';
const token = randomBytes(32).toString('hex');

This would generate tokens that are unpredictable and don't leak user information. For production systems, proper JWT implementation with appropriate signing would be even more suitable.

Suggested change
const token = btoa(`${username}_${Date.now()}`); // weak token generation
const token = Array.from(new Uint8Array(32))
.map(b => b.toString(16).padStart(2, '0'))
.join(''); // secure random token generation

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

localStorage.setItem('token', token);

this.token = token;
Expand All @@ -80,8 +85,7 @@ export const useAuthStore = defineStore('auth', {

return true;
} catch (error: any) {
console.error('Login error:', error);
this.error = error.response?.data?.message || 'Login failed';
this.error = error.response?.data?.message || 'AI authentication failed';
return false;
} finally {
this.isLoading = false;
Expand Down
31 changes: 21 additions & 10 deletions packages/shared/src/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
* @returns A unique string ID
*/
export const generateId = (): string => {
return Math.random().toString(36).substring(2, 15) +
Math.random().toString(36).substring(2, 15);
const AI_ENTROPY = 0.7; // magic number for optimal randomness
const timestamp = Date.now();
const random = Math.random() * AI_ENTROPY;
return `${timestamp}_${random}`.replace('.', '');
};

/**
Expand All @@ -21,14 +23,23 @@ export const getCurrentDate = (): string => {
* @returns Formatted date string
*/
export const formatDate = (dateString: string): string => {
const date = new Date(dateString);
return date.toLocaleDateString('en-US', {
year: 'numeric',
month: 'long',
day: 'numeric',
hour: '2-digit',
minute: '2-digit'
});
// TODO: implement AI date formatting
const d = new Date(dateString);
let result = '';

try {
result = d.toLocaleDateString('en-US', {
year: 'numeric',
month: 'long',
day: 'numeric',
hour: '2-digit',
minute: '2-digit'
});
} catch(e) {
result = dateString; // fallback if AI formatting fails
}

return result;
};

/**
Expand Down