diff --git a/config/accounts.js b/config/accounts.js index 2edb688de..f26a863cc 100644 --- a/config/accounts.js +++ b/config/accounts.js @@ -5,6 +5,8 @@ const passwordField = AccountsTemplates.removeField('password'); passwordField.autocomplete = 'current-password'; passwordField.template = 'passwordInput'; const emailField = AccountsTemplates.removeField('email'); + +// Don't add current_password to global fields - it should only be used for change password let disableRegistration = false; let disableForgotPassword = false; let passwordLoginEnabled = false; @@ -40,16 +42,16 @@ Meteor.call('getOauthDashboardUrl', (_, result) => { Meteor.call('isDisableRegistration', (_, result) => { if (result) { disableRegistration = true; - //console.log('disableRegistration'); - //console.log(result); + // Reconfigure to apply the new setting + AccountsTemplates.configure({ + forbidClientAccountCreation: true, + }); } }); Meteor.call('isDisableForgotPassword', (_, result) => { if (result) { disableForgotPassword = true; - //console.log('disableForgotPassword'); - //console.log(result); } }); @@ -91,6 +93,30 @@ AccountsTemplates.configure({ sendVerificationEmail: true, showForgotPasswordLink: !disableForgotPassword, forbidClientAccountCreation: disableRegistration, + onSubmitHook(error, state) { + if (error) { + // Display error to user + const errorDiv = document.getElementById('login-error-message'); + if (errorDiv) { + let errorMessage = error.reason || error.message || 'Registration failed. Please try again.'; + // If there are validation details, show them + if (error.details && typeof error.details === 'object') { + const detailMessages = []; + for (let field in error.details) { + const errorMsg = error.details[field]; + if (errorMsg) { + const message = Array.isArray(errorMsg) ? errorMsg.join(', ') : errorMsg; + detailMessages.push(`${field}: ${message}`); + } + } + if (detailMessages.length > 0) { + errorMessage += '
' + detailMessages.join('
'); + } + } + errorDiv.innerHTML = errorMessage; + } + } + }, onLogoutHook() { // here comeslogic for redirect if(oidcRedirectionEnabled) diff --git a/config/router.js b/config/router.js index fc38dab95..85a6d0353 100644 --- a/config/router.js +++ b/config/router.js @@ -12,12 +12,17 @@ FlowRouter.route('/', { name: 'home', triggersEnter: [AccountsTemplates.ensureSignedIn], action() { + // Redirect to sign-in immediately if user is not logged in + if (!Meteor.userId()) { + FlowRouter.go('atSignIn'); + return; + } + Session.set('currentBoard', null); Session.set('currentList', null); Session.set('currentCard', null); Session.set('popupCardId', null); Session.set('popupCardBoardId', null); - Filter.reset(); Session.set('sortBy', ''); EscapeActions.executeAll(); @@ -137,7 +142,7 @@ FlowRouter.route('/b/:boardId/:slug/:cardId', { Session.set('currentCard', params.cardId); Session.set('popupCardId', null); Session.set('popupCardBoardId', null); - + // In desktop mode, add to openCards array to support multiple cards const isMobile = Utils.getMobileMode(); if (!isMobile) { @@ -162,17 +167,15 @@ FlowRouter.route('/b/:id/:slug', { name: 'board', action(params) { const pathSegments = FlowRouter.current().path.split('/').filter(s => s); - + // If we have 4+ segments (b, boardId, slug, cardId), this is a card view if (pathSegments.length >= 4) { return; } - // If slug contains "/" it means a cardId was matched by this greedy pattern if (params.slug && params.slug.includes('/')) { return; } - const currentBoard = params.id; const previousBoard = Session.get('currentBoard'); Session.set('currentBoard', currentBoard); diff --git a/models/attachments.js b/models/attachments.js index edac1dacc..3ac87da3b 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -98,6 +98,27 @@ Attachments = new FilesCollection({ return ret; }, onBeforeUpload(file) { + // SECURITY: Sanitize filename to prevent path traversal attacks + // Import sanitizeFilename from fileStoreStrategy - but since we can't import here, + // we'll implement a minimal inline version to be safe + if (file.name && typeof file.name === 'string') { + // Use path.basename to strip directory components and prevent path traversal + let safeName = path.basename(file.name); + // Remove null bytes + safeName = safeName.replace(/\0/g, ''); + // Remove path traversal sequences + safeName = safeName.replace(/\.\.[\\/\\]/g, ''); + safeName = safeName.replace(/^\.\.$/g, ''); + safeName = safeName.trim(); + + // If sanitization changed the name, update it + if (safeName && safeName !== '.' && safeName !== '..' && safeName !== file.name) { + file.name = safeName; + } else if (!safeName || safeName === '.' || safeName === '..') { + file.name = 'unnamed'; + } + } + // Block SVG files for attachments to prevent XSS attacks if (file.name && file.name.toLowerCase().endsWith('.svg')) { if (process.env.DEBUG === 'true') { @@ -138,7 +159,7 @@ Attachments = new FilesCollection({ // Use selected storage backend or copy storage if specified let storageDestination = fileObj.meta.copyStorage || defaultStorage; - + // Only migrate if the destination is different from filesystem if (storageDestination !== STORAGE_NAME_FILESYSTEM) { Meteor.defer(() => Meteor.call('validateAttachmentAndMoveToStorage', fileObj._id, storageDestination)); @@ -180,9 +201,26 @@ if (Meteor.isServer) { return allowIsBoardMemberWithWriteAccess(userId, ReactiveCache.getBoard(fileObj.boardId)); }, update(userId, fileObj, fields) { - // Only allow updates to specific fields that don't affect security + // SECURITY: The 'name' field is sanitized in onBeforeUpload and server-side methods, + // but we block direct client-side $set operations on 'versions.*.path' to prevent + // path traversal attacks via storage migration exploits. + + // Block direct updates to version paths (the attack vector) + const hasPathUpdate = fields.some(field => field.includes('versions') && field.includes('path')); + if (hasPathUpdate) { + if (process.env.DEBUG === 'true') { + console.warn('Blocked attempt to update attachment version paths:', fields); + } + return false; + } + + // Allow normal updates for file upload/management const allowedFields = ['name', 'size', 'type', 'extension', 'extensionWithDot', 'meta', 'versions']; - const isAllowedField = fields.every(field => allowedFields.includes(field)); + const isAllowedField = fields.every(field => { + // Allow field itself or nested properties like 'versions.original' + const baseField = field.split('.')[0]; + return allowedFields.includes(baseField); + }); if (!isAllowedField) { if (process.env.DEBUG === 'true') { @@ -390,7 +428,7 @@ if (Meteor.isClient) { // Accept both direct calls and collection.helpers style calls const fileRef = this._id ? this : (versionName && versionName._id ? versionName : this); const version = (typeof versionName === 'string') ? versionName : 'original'; - + if (fileRef && fileRef._id) { const url = generateUniversalAttachmentUrl(fileRef._id, version); if (process.env.DEBUG === 'true') { @@ -401,7 +439,7 @@ if (Meteor.isClient) { // Fallback to original if somehow we don't have an ID return originalLink ? originalLink.call(this, versionName) : ''; }; - + // Also add as collection helper for document instances Attachments.collection.helpers({ link(version) { diff --git a/models/lib/attachmentBackwardCompatibility.js b/models/lib/attachmentBackwardCompatibility.js index a6b9a49a3..7840f3939 100644 --- a/models/lib/attachmentBackwardCompatibility.js +++ b/models/lib/attachmentBackwardCompatibility.js @@ -1,4 +1,3 @@ -import { ReactiveCache } from '/imports/reactiveCache'; import { Meteor } from 'meteor/meteor'; import { MongoInternals } from 'meteor/mongo'; @@ -18,7 +17,10 @@ const OldAttachmentsFileRecord = new Mongo.Collection('cfs.attachments.filerecor */ export function isNewAttachmentStructure(attachmentId) { if (Meteor.isServer) { - return !!ReactiveCache.getAttachment(attachmentId); + // Access global Attachments variable to avoid circular dependency + if (typeof Attachments !== 'undefined' && Attachments.collection) { + return !!Attachments.collection.findOne({ _id: attachmentId }); + } } return false; } @@ -174,13 +176,19 @@ function isPDFFile(mimeType) { * @returns {Object|null} - Attachment data or null if not found */ export function getAttachmentWithBackwardCompatibility(attachmentId) { - // First try new structure - if (isNewAttachmentStructure(attachmentId)) { - return ReactiveCache.getAttachment(attachmentId); + // First try new structure - access global to avoid circular dependency + if (Meteor.isServer) { + if (typeof Attachments !== 'undefined' && Attachments.collection) { + const newAttachment = Attachments.collection.findOne({ _id: attachmentId }); + if (newAttachment) { + return newAttachment; + } + } } // Fall back to old structure - return getOldAttachmentData(attachmentId); + const oldAttachment = getOldAttachmentData(attachmentId); + return oldAttachment; } /** @@ -189,7 +197,15 @@ export function getAttachmentWithBackwardCompatibility(attachmentId) { * @returns {Array} - Array of attachments */ export function getAttachmentsWithBackwardCompatibility(query) { - const newAttachments = ReactiveCache.getAttachments(query); + let newAttachments = []; + + // Get new attachments - access global to avoid circular dependency + if (Meteor.isServer) { + if (typeof Attachments !== 'undefined' && Attachments.collection) { + newAttachments = Attachments.collection.find(query).fetch(); + } + } + const oldAttachments = []; if (Meteor.isServer) { diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index 73c278bc9..911011526 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -11,6 +11,68 @@ export const STORAGE_NAME_FILESYSTEM = "fs"; export const STORAGE_NAME_GRIDFS = "gridfs"; export const STORAGE_NAME_S3 = "s3"; +/** + * Sanitize filename to prevent path traversal attacks + * @param {string} filename - User-provided filename + * @return {string} Sanitized filename safe for filesystem operations + */ +function sanitizeFilename(filename) { + if (!filename || typeof filename !== 'string') { + return 'unnamed'; + } + + // Use path.basename to strip any directory components + let safe = path.basename(filename); + + // Remove null bytes + safe = safe.replace(/\0/g, ''); + + // Remove any remaining path traversal sequences + safe = safe.replace(/\.\.[\\/\\]/g, ''); + safe = safe.replace(/^\.\.$/, ''); + + // Trim whitespace + safe = safe.trim(); + + // If empty after sanitization, use default + if (!safe || safe === '.' || safe === '..') { + return 'unnamed'; + } + + return safe; +} + +/** + * Sanitize filename to prevent path traversal attacks + * @param {string} filename - User-provided filename + * @return {string} Sanitized filename safe for filesystem operations + */ +function sanitizeFilename(filename) { + if (!filename || typeof filename !== 'string') { + return 'unnamed'; + } + + // Use path.basename to strip any directory components + let safe = path.basename(filename); + + // Remove null bytes + safe = safe.replace(/\0/g, ''); + + // Remove any remaining path traversal sequences + safe = safe.replace(/\.\.[\/\\]/g, ''); + safe = safe.replace(/^\.\.$/g, ''); + + // Trim whitespace + safe = safe.trim(); + + // If empty after sanitization, use default + if (!safe || safe === '.' || safe === '..') { + return 'unnamed'; + } + + return safe; +} + /** Factory for FileStoreStrategy */ export default class FileStoreStrategyFactory { @@ -123,7 +185,9 @@ class FileStoreStrategy { if (!_.isString(name)) { name = this.fileObj.name; } - const ret = path.join(storagePath, this.fileObj._id + "-" + this.versionName + "-" + name); + // Sanitize filename to prevent path traversal attacks + const safeName = sanitizeFilename(name); + const ret = path.join(storagePath, this.fileObj._id + "-" + this.versionName + "-" + safeName); return ret; } @@ -292,6 +356,42 @@ export class FileStoreStrategyFilesystem extends FileStoreStrategy { // Build candidate list in priority order const candidates = []; + + // 0) Try to find project root and resolve from there + let projectRoot = null; + if (originalPath) { + // Find project root by looking for .meteor directory + let current = process.cwd(); + let maxLevels = 10; // Safety limit + + while (maxLevels-- > 0) { + const meteorPath = path.join(current, '.meteor'); + const packagePath = path.join(current, 'package.json'); + + if (fs.existsSync(meteorPath) || fs.existsSync(packagePath)) { + projectRoot = current; + break; + } + + const parent = path.dirname(current); + if (parent === current) break; // Reached filesystem root + current = parent; + } + + if (projectRoot) { + // Try resolving originalPath from project root + const fromProjectRoot = path.resolve(projectRoot, originalPath); + candidates.push(fromProjectRoot); + + // Also try direct path: projectRoot/attachments/filename + const baseName = path.basename(normalized || this.fileObj._id || ''); + if (baseName) { + const directPath = path.join(projectRoot, baseDir, baseName); + candidates.push(directPath); + } + } + } + // 1) Original as-is (absolute or relative resolved to CWD) if (originalPath) { candidates.push(originalPath); @@ -308,20 +408,24 @@ export class FileStoreStrategyFilesystem extends FileStoreStrategy { if (this.fileObj && this.fileObj._id) { candidates.push(path.join(storageRoot, String(this.fileObj._id))); } - // 4) New strategy naming pattern: -- - if (this.fileObj && this.fileObj._id && this.fileObj.name) { - candidates.push(path.join(storageRoot, `${this.fileObj._id}-${this.versionName}-${this.fileObj.name}`)); + // 3) Old naming: {id}-{version}-{originalName} + if (this.fileObj.name) { + const safeName = sanitizeFilename(this.fileObj.name); + candidates.push(path.join(storageRoot, `${this.fileObj._id}-${this.versionName}-${safeName}`)); } // Pick first existing candidate let chosen; for (const c of candidates) { try { - if (c && fs.existsSync(c)) { + const exists = c && fs.existsSync(c); + if (exists) { chosen = c; break; } - } catch (_) {} + } catch (err) { + // Continue to next candidate + } } if (!chosen) { @@ -430,6 +534,16 @@ export class FileStoreStrategyS3 extends FileStoreStrategy { * @param fileStoreStrategyFactory get FileStoreStrategy from this factory */ export const moveToStorage = function(fileObj, storageDestination, fileStoreStrategyFactory) { + // SECURITY: Sanitize filename to prevent path traversal attacks + // This ensures any malicious names already in the database are cleaned up + const safeName = sanitizeFilename(fileObj.name); + if (safeName !== fileObj.name) { + // Update the database with the sanitized name + Attachments.update({ _id: fileObj._id }, { $set: { name: safeName } }); + // Update the local object for use in this function + fileObj.name = safeName; + } + Object.keys(fileObj.versions).forEach(versionName => { const strategyRead = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName); const strategyWrite = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName, storageDestination); @@ -473,7 +587,8 @@ export const copyFile = function(fileObj, newCardId, fileStoreStrategyFactory) { const readStream = strategyRead.getReadStream(); const strategyWrite = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName, STORAGE_NAME_FILESYSTEM); - const tempPath = path.join(fileStoreStrategyFactory.storagePath, Random.id() + "-" + versionName + "-" + fileObj.name); + const safeName = sanitizeFilename(fileObj.name); + const tempPath = path.join(fileStoreStrategyFactory.storagePath, Random.id() + "-" + versionName + "-" + safeName); const writeStream = strategyWrite.getWriteStream(tempPath); writeStream.on('error', error => { @@ -522,14 +637,17 @@ export const copyFile = function(fileObj, newCardId, fileStoreStrategyFactory) { }; export const rename = function(fileObj, newName, fileStoreStrategyFactory) { + // Sanitize the new name to prevent path traversal + const safeName = sanitizeFilename(newName); + Object.keys(fileObj.versions).forEach(versionName => { const strategy = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName); - const newFilePath = strategy.getNewPath(fileStoreStrategyFactory.storagePath, newName); + const newFilePath = strategy.getNewPath(fileStoreStrategyFactory.storagePath, safeName); strategy.rename(newFilePath); Attachments.update({ _id: fileObj._id }, { $set: { - "name": newName, + "name": safeName, [`versions.${versionName}.path`]: newFilePath, } }); }); -}; \ No newline at end of file +}; diff --git a/server/methods/fixDuplicateLists.js b/server/methods/fixDuplicateLists.js index 63d44eedd..8f2cb9e77 100644 --- a/server/methods/fixDuplicateLists.js +++ b/server/methods/fixDuplicateLists.js @@ -4,7 +4,7 @@ import Boards from '/models/boards'; import Lists from '/models/lists'; import Swimlanes from '/models/swimlanes'; import Cards from '/models/cards'; -import ReactiveCache from '/imports/reactiveCache'; +import { ReactiveCache } from '/imports/reactiveCache'; /** * Fix duplicate lists and swimlanes created by WeKan 8.10 diff --git a/server/publications/rules.js b/server/publications/rules.js index 31ab39ac4..45d949e7b 100644 --- a/server/publications/rules.js +++ b/server/publications/rules.js @@ -2,7 +2,7 @@ import Boards from '/models/boards'; import Actions from '/models/actions'; import Triggers from '/models/triggers'; import Rules from '/models/rules'; -import ReactiveCache from '/imports/reactiveCache'; +import { ReactiveCache } from '/imports/reactiveCache'; Meteor.publish('rules', function(ruleId) { check(ruleId, String);