diff --git a/SECURITY.md b/SECURITY.md index 2089aae0a..c13cd8818 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -182,6 +182,16 @@ Meteor.startup(() => { - This means attachments are downloaded instead of rendered inline by default. This mitigates HTML/JS/SVG based stored XSS vectors. - Avatars and inline images remain supported but SVG uploads are blocked and never rendered inline. +## Users: Client update restrictions + +- Client-side updates to user documents are limited to safe fields only: + - `username` + - `profile.*` +- Sensitive fields are blocked from any client updates and can only be modified by server methods with authorization: + - `orgs`, `teams`, `roles`, `isAdmin`, `createdThroughApi`, `loginDisabled`, `authenticationMethod`, `services.*`, `emails.*`, `sessionData.*` +- Attempts to update forbidden fields from the client are denied. +- Admin operations like managing org/team membership or toggling flags must use server methods that check permissions. + ## Brute force login protection - https://github.com/wekan/wekan/commit/23e5e1e3bd081699ce39ce5887db7e612616014d diff --git a/client/00-startup.js b/client/00-startup.js index a6f049322..52a1c536c 100644 --- a/client/00-startup.js +++ b/client/00-startup.js @@ -15,3 +15,50 @@ import '/client/components/migrationProgress'; // Import cron settings import '/client/components/settings/cronSettings'; + +// Mirror Meteor login token into a cookie for server-side file route auth +// This enables cookie-based auth for /cdn/storage/* without leaking ROOT_URL +// Token already lives in localStorage; cookie adds same-origin send-on-request semantics +Meteor.startup(() => { + const COOKIE_NAME = 'meteor_login_token'; + const cookieAttrs = () => { + const attrs = ['Path=/', 'SameSite=Lax']; + try { + if (window.location && window.location.protocol === 'https:') { + attrs.push('Secure'); + } + } catch (_) {} + return attrs.join('; '); + }; + + const setCookie = (name, value) => { + if (!value) return; + document.cookie = `${encodeURIComponent(name)}=${encodeURIComponent(value)}; ${cookieAttrs()}`; + }; + const clearCookie = (name) => { + document.cookie = `${encodeURIComponent(name)}=; Expires=Thu, 01 Jan 1970 00:00:00 GMT; ${cookieAttrs()}`; + }; + + const syncCookie = () => { + try { + const token = Accounts && typeof Accounts._storedLoginToken === 'function' ? Accounts._storedLoginToken() : null; + if (token) setCookie(COOKIE_NAME, token); else clearCookie(COOKIE_NAME); + } catch (e) { + // ignore + } + }; + + // Initial sync on startup + syncCookie(); + + // Keep cookie in sync on login/logout + if (Accounts && typeof Accounts.onLogin === 'function') Accounts.onLogin(syncCookie); + if (Accounts && typeof Accounts.onLogout === 'function') Accounts.onLogout(syncCookie); + + // Sync across tabs/windows when localStorage changes + window.addEventListener('storage', (ev) => { + if (ev && typeof ev.key === 'string' && ev.key.indexOf('Meteor.loginToken') !== -1) { + syncCookie(); + } + }); +}); diff --git a/client/components/users/userAvatar.jade b/client/components/users/userAvatar.jade index b61eb5033..e00fc188f 100644 --- a/client/components/users/userAvatar.jade +++ b/client/components/users/userAvatar.jade @@ -1,7 +1,7 @@ template(name="userAvatar") a.member(class="js-{{#if assignee}}assignee{{else}}member{{/if}}" title="{{userData.profile.fullname}} ({{userData.username}}) {{_ memberType}}") if userData.profile.avatarUrl - img.avatar.avatar-image(src="{{userData.profile.avatarUrl}}") + img.avatar.avatar-image(src="{{avatarUrl}}") else +userAvatarInitials(userId=userData._id) diff --git a/client/components/users/userAvatar.js b/client/components/users/userAvatar.js index 2869a9750..f2db90ee3 100644 --- a/client/components/users/userAvatar.js +++ b/client/components/users/userAvatar.js @@ -15,6 +15,21 @@ Template.userAvatar.helpers({ }); }, + avatarUrl() { + const user = ReactiveCache.getUser(this.userId, { fields: { profile: 1 } }); + const base = (user && user.profile && user.profile.avatarUrl) || ''; + if (!base) return ''; + // Append current boardId when available so public viewers can access avatars on public boards + try { + const boardId = Utils.getCurrentBoardId && Utils.getCurrentBoardId(); + if (boardId) { + const sep = base.includes('?') ? '&' : '?'; + return `${base}${sep}boardId=${encodeURIComponent(boardId)}`; + } + } catch (_) {} + return base; + }, + memberType() { const user = ReactiveCache.getUser(this.userId); return user && user.isBoardAdmin() ? 'admin' : 'normal'; diff --git a/models/users.js b/models/users.js index 712098e55..da2fa0c0a 100644 --- a/models/users.js +++ b/models/users.js @@ -569,15 +569,41 @@ Users.attachSchema( }), ); +// Security helpers for user updates +export const USER_UPDATE_ALLOWED_EXACT = ['username']; +export const USER_UPDATE_ALLOWED_PREFIXES = ['profile.']; +export const USER_UPDATE_FORBIDDEN_PREFIXES = [ + 'services', + 'emails', + 'roles', + 'isAdmin', + 'createdThroughApi', + 'orgs', + 'teams', + 'loginDisabled', + 'authenticationMethod', + 'sessionData', +]; + +export function isUserUpdateAllowed(fields) { + return fields.every((f) => + USER_UPDATE_ALLOWED_EXACT.includes(f) || USER_UPDATE_ALLOWED_PREFIXES.some((p) => f.startsWith(p)) + ); +} + +export function hasForbiddenUserUpdateField(fields) { + return fields.some((f) => USER_UPDATE_FORBIDDEN_PREFIXES.some((p) => f === p || f.startsWith(p + '.'))); +} + Users.allow({ - update(userId, doc) { - const user = ReactiveCache.getUser(userId) || ReactiveCache.getCurrentUser(); - if (user?.isAdmin) - return true; - if (!user) { - return false; - } - return doc._id === userId; + update(userId, doc, fields /*, modifier */) { + // Only the owner can update, and only for allowed fields + if (!userId || doc._id !== userId) return false; + if (!Array.isArray(fields) || fields.length === 0) return false; + // Disallow if any forbidden field present + if (hasForbiddenUserUpdateField(fields)) return false; + // Allow only username and profile.* + return isUserUpdateAllowed(fields); }, remove(userId, doc) { // Disable direct client-side user removal for security @@ -588,10 +614,10 @@ Users.allow({ fetch: [], }); -// Non-Admin users can not change to Admin +// Deny any attempts to touch forbidden fields from client updates Users.deny({ - update(userId, board, fieldNames) { - return _.contains(fieldNames, 'isAdmin') && !ReactiveCache.getCurrentUser().isAdmin; + update(userId, doc, fields /*, modifier */) { + return hasForbiddenUserUpdateField(fields); }, fetch: [], }); diff --git a/server/cors.js b/server/cors.js index f99258eae..42952a95b 100644 --- a/server/cors.js +++ b/server/cors.js @@ -1,18 +1,11 @@ Meteor.startup(() => { - // Set Permissions-Policy header to suppress browser warnings about experimental features - WebApp.rawConnectHandlers.use(function(req, res, next) { - // Disable experimental advertising and privacy features that cause browser warnings - res.setHeader('Permissions-Policy', - 'browsing-topics=(), ' + - 'run-ad-auction=(), ' + - 'join-ad-interest-group=(), ' + - 'private-state-token-redemption=(), ' + - 'private-state-token-issuance=(), ' + - 'private-aggregation=(), ' + - 'attribution-reporting=()' - ); - return next(); - }); + // Optional: Set Permissions-Policy only if explicitly provided to avoid browser warnings about unrecognized features + if (process.env.PERMISSIONS_POLICY && process.env.PERMISSIONS_POLICY.trim() !== '') { + WebApp.rawConnectHandlers.use(function(req, res, next) { + res.setHeader('Permissions-Policy', process.env.PERMISSIONS_POLICY); + return next(); + }); + } if (process.env.CORS) { // Listen to incoming HTTP requests, can only be used on the server diff --git a/server/lib/tests/index.js b/server/lib/tests/index.js index bcc3f567c..c46057bd6 100644 --- a/server/lib/tests/index.js +++ b/server/lib/tests/index.js @@ -1 +1,2 @@ import './utils.tests'; +import './users.security.tests'; diff --git a/server/lib/tests/users.security.tests.js b/server/lib/tests/users.security.tests.js new file mode 100644 index 000000000..395ee912b --- /dev/null +++ b/server/lib/tests/users.security.tests.js @@ -0,0 +1,43 @@ +/* eslint-env mocha */ +import { expect } from 'chai'; +import { isUserUpdateAllowed, hasForbiddenUserUpdateField } from '/models/users'; + +describe('users security', function() { + describe('isUserUpdateAllowed', function() { + it('allows username update', function() { + expect(isUserUpdateAllowed(['username'])).to.equal(true); + }); + it('allows profile updates', function() { + expect(isUserUpdateAllowed(['profile.fullname'])).to.equal(true); + expect(isUserUpdateAllowed(['profile.avatarUrl', 'profile.language'])).to.equal(true); + }); + it('denies other top-level fields', function() { + expect(isUserUpdateAllowed(['orgs'])).to.equal(false); + expect(isUserUpdateAllowed(['teams'])).to.equal(false); + expect(isUserUpdateAllowed(['loginDisabled'])).to.equal(false); + expect(isUserUpdateAllowed(['authenticationMethod'])).to.equal(false); + expect(isUserUpdateAllowed(['services'])).to.equal(false); + expect(isUserUpdateAllowed(['emails'])).to.equal(false); + expect(isUserUpdateAllowed(['isAdmin'])).to.equal(false); + }); + }); + + describe('hasForbiddenUserUpdateField', function() { + it('flags forbidden sensitive fields', function() { + expect(hasForbiddenUserUpdateField(['orgs'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['teams'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['loginDisabled'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['authenticationMethod'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['services.facebook'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['emails.0.verified'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['roles'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['isAdmin'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['createdThroughApi'])).to.equal(true); + expect(hasForbiddenUserUpdateField(['sessionData.totalHits'])).to.equal(true); + }); + it('does not flag allowed fields', function() { + expect(hasForbiddenUserUpdateField(['username'])).to.equal(false); + expect(hasForbiddenUserUpdateField(['profile.fullname'])).to.equal(false); + }); + }); +}); diff --git a/server/routes/universalFileServer.js b/server/routes/universalFileServer.js index 15423e43c..3e7159078 100644 --- a/server/routes/universalFileServer.js +++ b/server/routes/universalFileServer.js @@ -7,8 +7,10 @@ import { Meteor } from 'meteor/meteor'; import { WebApp } from 'meteor/webapp'; import { ReactiveCache } from '/imports/reactiveCache'; +import { Accounts } from 'meteor/accounts-base'; import Attachments, { fileStoreStrategyFactory as attachmentStoreFactory } from '/models/attachments'; import Avatars, { fileStoreStrategyFactory as avatarStoreFactory } from '/models/avatars'; +import '/models/boards'; import { getAttachmentWithBackwardCompatibility, getOldAttachmentStream } from '/models/lib/attachmentBackwardCompatibility'; import fs from 'fs'; import path from 'path'; @@ -162,6 +164,154 @@ if (Meteor.isServer) { return false; } + /** + * Determine if an avatar request is authorized + * Rules: + * - If a boardId query is provided and that board is public -> allow + * - Else if requester is authenticated (valid token) -> allow + * - Else if avatar's owner belongs to at least one public board -> allow + * - Otherwise -> deny + */ + function isAuthorizedForAvatar(req, avatar) { + try { + if (!avatar) return false; + + // 1) Check explicit board context via query + const q = parseQuery(req); + const boardId = q.boardId || q.board || q.b; + if (boardId) { + const board = ReactiveCache.getBoard(boardId); + if (board && board.isPublic && board.isPublic()) return true; + + // If private board is specified, require membership of requester + const token = extractLoginToken(req); + const user = token ? getUserFromToken(token) : null; + if (user && board && board.hasMember && board.hasMember(user._id)) return true; + return false; + } + + // 2) Authenticated request without explicit board context + const token = extractLoginToken(req); + const user = token ? getUserFromToken(token) : null; + if (user) return true; + + // 3) Allow if avatar owner is on any public board (so avatars are public only when on public boards) + // Use a lightweight query against Boards + const found = Boards && Boards.findOne({ permission: 'public', 'members.userId': avatar.userId }, { fields: { _id: 1 } }); + return !!found; + } catch (e) { + if (process.env.DEBUG === 'true') { + console.warn('Avatar authorization check failed:', e); + } + return false; + } + } + + /** + * Parse cookies from request headers into an object map + */ + function parseCookies(req) { + const header = req.headers && req.headers.cookie; + const out = {}; + if (!header) return out; + const parts = header.split(';'); + for (const part of parts) { + const idx = part.indexOf('='); + if (idx === -1) continue; + const k = decodeURIComponent(part.slice(0, idx).trim()); + const v = decodeURIComponent(part.slice(idx + 1).trim()); + out[k] = v; + } + return out; + } + + /** + * Get query parameters as a simple object + */ + function parseQuery(req) { + const out = {}; + const q = (req.url || '').split('?')[1] || ''; + if (!q) return out; + const pairs = q.split('&'); + for (const p of pairs) { + if (!p) continue; + const [rawK, rawV] = p.split('='); + const k = decodeURIComponent((rawK || '').trim()); + const v = decodeURIComponent((rawV || '').trim()); + if (k) out[k] = v; + } + return out; + } + + /** + * Extract a login token from Authorization header, query param, or cookie + * Supported sources (priority order): + * - Authorization: Bearer + * - X-Auth-Token header + * - authToken query parameter + * - meteor_login_token or wekan_login_token cookie + */ + function extractLoginToken(req) { + // Authorization: Bearer + const authz = req.headers && (req.headers.authorization || req.headers.Authorization); + if (authz && typeof authz === 'string') { + const m = authz.match(/^Bearer\s+(.+)$/i); + if (m && m[1]) return m[1].trim(); + } + + // X-Auth-Token + const xAuth = req.headers && (req.headers['x-auth-token'] || req.headers['X-Auth-Token']); + if (xAuth && typeof xAuth === 'string') return xAuth.trim(); + + // Query parameter + const q = parseQuery(req); + if (q.authToken && typeof q.authToken === 'string') return q.authToken.trim(); + + // Cookies + const cookies = parseCookies(req); + if (cookies.meteor_login_token) return cookies.meteor_login_token.trim(); + if (cookies.wekan_login_token) return cookies.wekan_login_token.trim(); + + return null; + } + + /** + * Resolve a user from a raw login token string + */ + function getUserFromToken(rawToken) { + try { + if (!rawToken || typeof rawToken !== 'string' || rawToken.length < 10) return null; + const hashed = Accounts._hashLoginToken(rawToken); + return Meteor.users.findOne({ 'services.resume.loginTokens.hashedToken': hashed }, { fields: { _id: 1 } }); + } catch (e) { + // In case accounts-base is not available or any error occurs + if (process.env.DEBUG === 'true') { + console.warn('Token resolution error:', e); + } + return null; + } + } + + /** + * Authorization helper for board-bound files + * - Public boards: allow + * - Private boards: require valid user who is a member + */ + function isAuthorizedForBoard(req, board) { + try { + if (!board) return false; + if (board.isPublic && board.isPublic()) return true; + const token = extractLoginToken(req); + const user = token ? getUserFromToken(token) : null; + return !!(user && board.hasMember && board.hasMember(user._id)); + } catch (e) { + if (process.env.DEBUG === 'true') { + console.warn('Authorization check failed:', e); + } + return false; + } + } + /** * Helper function to stream file with error handling */ @@ -205,8 +355,8 @@ if (Meteor.isServer) { return; } - // Get attachment from database - const attachment = ReactiveCache.getAttachment(fileId); + // Get attachment from database with backward compatibility + const attachment = getAttachmentWithBackwardCompatibility(fileId); if (!attachment) { res.writeHead(404); res.end('Attachment not found'); @@ -221,24 +371,28 @@ if (Meteor.isServer) { return; } - // TODO: Implement proper authentication via cookies/headers - // Meteor.userId() returns undefined in WebApp.connectHandlers middleware - // For now, allow access - ostrio:files protected() method provides fallback auth - // const userId = null; // Need to extract from req.headers.cookie - // if (!board.isPublic() && (!userId || !board.hasMember(userId))) { - // res.writeHead(403); - // res.end('Access denied'); - // return; - // } + // Enforce cookie/header/query-based auth for private boards + if (!isAuthorizedForBoard(req, board)) { + res.writeHead(403); + res.end('Access denied'); + return; + } // Handle conditional requests if (handleConditionalRequest(req, res, attachment)) { return; } - // Get file strategy and stream - const strategy = attachmentStoreFactory.getFileStrategy(attachment, 'original'); - const readStream = strategy.getReadStream(); + // Choose proper streaming based on source + let readStream; + if (attachment?.meta?.source === 'legacy') { + // Legacy CollectionFS GridFS stream + readStream = getOldAttachmentStream(fileId); + } else { + // New Meteor-Files storage + const strategy = attachmentStoreFactory.getFileStrategy(attachment, 'original'); + readStream = strategy.getReadStream(); + } if (!readStream) { res.writeHead(404); @@ -296,9 +450,12 @@ if (Meteor.isServer) { return; } - // TODO: Implement proper authentication for avatars - // Meteor.userId() returns undefined in WebApp.connectHandlers middleware - // For now, allow avatar viewing - they're typically public anyway + // Enforce visibility: avatars are public only in the context of public boards + if (!isAuthorizedForAvatar(req, avatar)) { + res.writeHead(403); + res.end('Access denied'); + return; + } // Handle conditional requests if (handleConditionalRequest(req, res, avatar)) { @@ -366,9 +523,12 @@ if (Meteor.isServer) { return; } - // TODO: Implement proper authentication via cookies/headers - // Meteor.userId() returns undefined in WebApp.connectHandlers middleware - // For now, allow access for compatibility + // Enforce cookie/header/query-based auth for private boards + if (!isAuthorizedForBoard(req, board)) { + res.writeHead(403); + res.end('Access denied'); + return; + } // Handle conditional requests if (handleConditionalRequest(req, res, attachment)) { @@ -435,9 +595,12 @@ if (Meteor.isServer) { return; } - // TODO: Implement proper authentication for legacy avatars - // Meteor.userId() returns undefined in WebApp.connectHandlers middleware - // For now, allow avatar viewing for compatibility + // Enforce visibility for legacy avatars as well + if (!isAuthorizedForAvatar(req, avatar)) { + res.writeHead(403); + res.end('Access denied'); + return; + } // Handle conditional requests if (handleConditionalRequest(req, res, avatar)) {