Skip to content
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

Usernames are now case insensitive #105

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions server/src/config/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Logger from '../logger.js';
import { up_v1_5_0_public } from './migrations/20241119-migration.js';
import { up_v1_5_0_oidc } from './migrations/20241120-migration.js';
import { fileURLToPath } from 'url';
import { up_v1_5_0_usernames } from './migrations/20241121-migration.js';

let db = null;
let checkpointInterval = null;
Expand Down Expand Up @@ -155,6 +156,7 @@ function initializeDatabase() {
up_v1_5_0(db);
up_v1_5_0_public(db);
up_v1_5_0_oidc(db);
up_v1_5_0_usernames(db);
}

startCheckpointInterval();
Expand Down
51 changes: 51 additions & 0 deletions server/src/config/migrations/20241121-migration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import Logger from '../../logger.js';

function needsMigration(db) {
try {
const hasNormalizedColumn = db.prepare(`
SELECT COUNT(*) as count
FROM pragma_table_info('users')
WHERE name = 'username_normalized'
`).get();

return hasNormalizedColumn.count === 0;
} catch (error) {
Logger.error('v1.5.0-usernames - Error checking migration status:', error);
throw error;
}
}

async function up_v1_5_0_usernames(db) {
if (!needsMigration(db)) {
Logger.debug('v1.5.0-usernames - Migration not needed');
return;
}

Logger.debug('v1.5.0-usernames - Starting migration...');

try {
db.transaction(() => {
db.exec(`
ALTER TABLE users ADD COLUMN username_normalized TEXT;
CREATE UNIQUE INDEX IF NOT EXISTS idx_users_username_normalized
ON users(username_normalized COLLATE NOCASE);
`);

const users = db.prepare('SELECT id, username FROM users').all();
const updateStmt = db.prepare(
'UPDATE users SET username_normalized = ? WHERE id = ?'
);

for (const user of users) {
updateStmt.run(user.username.toLowerCase(), user.id);
}
})();

Logger.debug('v1.5.0-usernames - Migration completed successfully');
} catch (error) {
Logger.error('v1.5.0-usernames - Migration failed:', error);
throw error;
}
}

export { up_v1_5_0_usernames };
25 changes: 17 additions & 8 deletions server/src/repositories/userRepository.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ class UserRepository {
const db = getDb();

this.createUserStmt = db.prepare(`
INSERT INTO users (username, password_hash)
VALUES (?, ?)
INSERT INTO users (username, username_normalized, password_hash)
VALUES (?, ?, ?)
`);

this.findByUsernameStmt = db.prepare(`
SELECT id, username, password_hash, created_at, email, name, oidc_id, oidc_provider
FROM users
WHERE username = ?
WHERE username_normalized = ? COLLATE NOCASE
`);

this.findByIdStmt = db.prepare(`
Expand All @@ -41,16 +41,19 @@ class UserRepository {
this.createUserWithOIDCStmt = db.prepare(`
INSERT INTO users (
username,
username_normalized,
password_hash,
oidc_id,
oidc_provider,
email,
name
) VALUES (?, '', ?, ?, ?, ?)
) VALUES (?, ?, '', ?, ?, ?, ?)
`);

this.findUsernameCountStmt = db.prepare(`
SELECT COUNT(*) as count FROM users WHERE username = ?
SELECT COUNT(*) as count
FROM users
WHERE username_normalized = ? COLLATE NOCASE
`);
}
}
Expand All @@ -61,8 +64,13 @@ class UserRepository {
try {
const saltRounds = 10;
const passwordHash = await bcrypt.hash(password, saltRounds);
const normalizedUsername = username.toLowerCase();

const result = this.createUserStmt.run(username, passwordHash);
const result = this.createUserStmt.run(
username,
normalizedUsername,
passwordHash
);

return this.findById(result.lastInsertRowid);
} catch (error) {
Expand All @@ -75,7 +83,7 @@ class UserRepository {

async findByUsername(username) {
this.#initializeStatements();
return this.findByUsernameStmt.get(username);
return this.findByUsernameStmt.get(username.toLowerCase());
}

async findById(id) {
Expand All @@ -95,7 +103,7 @@ class UserRepository {
let username = baseUsername;
let counter = 1;

while (this.findUsernameCountStmt.get(username).count > 0) {
while (this.findUsernameCountStmt.get(username.toLowerCase()).count > 0) {
username = `${baseUsername}${counter}`;
counter++;
}
Expand All @@ -118,6 +126,7 @@ class UserRepository {

const result = this.createUserWithOIDCStmt.run(
username,
username.toLowerCase(),
profile.sub,
provider,
profile.email,
Expand Down
5 changes: 5 additions & 0 deletions server/src/services/userService.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ class UserService {
throw new Error('Username can only contain letters, numbers, underscores, and hyphens');
}

const existing = await userRepository.findByUsername(username);
if (existing) {
throw new Error('Username already exists');
}

return await userRepository.create(username, password);
} catch (error) {
Logger.error('Service Error - createUser:', error);
Expand Down