diff --git a/eslint-rules/require-security-check.js b/eslint-rules/require-security-check.js new file mode 100644 index 0000000000..d6462eecf8 --- /dev/null +++ b/eslint-rules/require-security-check.js @@ -0,0 +1,208 @@ +import { ESLintUtils } from '@typescript-eslint/utils'; +// Walk the AST to find a node that matches the callback, up to maxDepth levels deep +// This is a depth-first search and could be improved +function walkFind(node, callback, maxDepth = 10) { + if (maxDepth <= 0) + return null; + if (node && typeof node !== 'object') + return null; + if (callback(node)) + return node; + for (const key in node) { + if (key === 'parent') + continue; + const val = node[key]; + if (Array.isArray(val)) { + for (const child of val) { + const result = walkFind(child, callback, maxDepth - 1); + if (result) + return result; + } + } + else { + const result = walkFind(val, callback, maxDepth - 1); + if (result) + return result; + } + } + return null; +} +function blockStatementIncludesSecurityCall(block) { + const nodeIsSecurityCall = (node) => { + // Valid check if the node is a CallExpression of the form: + // *.locals.security.require*() + // or locals.security.require*() + if (node?.type === 'CallExpression' && node.callee.type === 'MemberExpression') { + const obj = node.callee.object; + const prop = node.callee.property; + if (obj.type === 'MemberExpression' && + // object is either *.locals or just locals + ((obj.object.type === 'MemberExpression' && + obj.object.property.type === 'Identifier' && + obj.object.property.name === 'locals') || + (obj.object.type === 'Identifier' && obj.object.name === 'locals')) && + // locals.security + obj.property.type === 'Identifier' && + obj.property.name === 'security' && + prop.type === 'Identifier' && + prop.name.startsWith('require') // ex. requireAuthenticated, requireRole, etc. + ) { + return true; + } + } + return false; + }; + return !!block.body.find((n) => !!walkFind(n, nodeIsSecurityCall, 6)); +} +export default ESLintUtils.RuleCreator(() => '')({ + name: 'require-security-check', + meta: { + type: 'problem', + hasSuggestions: true, + docs: { + description: 'Require calling event.locals.security.* in every server load/action' + }, + schema: [], + messages: { + missingSecurityCheck: 'Missing call to requireAuthenticated() or similar security check. This must be of the form locals.security.require*()', + unexpectedFunction: 'Unexpected export function type {{type}}. Expected ArrowFunctionExpression, FunctionExpression, TSSatisfiesExpression or ObjectExpression.', + suggestAuthCall: 'Require authentication by adding "event.locals.security.requireAuthenticated();" to the start of the function body.', + suggestNoAuthCall: 'Mark this function as not requiring any auth by adding "event.locals.security.requireNothing();" to the start of the function body.' + } + }, + defaultOptions: [], + create(context) { + return { + ExportNamedDeclaration(node) { + // Application of this rule is limited to: + // export const load = async (event) => { ... } + // export const actions = { default: async (event) => { ... } } + // export function load(event) { ... } + // export POST/GET/PUT/etc. function for endpoints in +server.ts files + // Only check .server.ts and +server.ts files + if (!context.filename.endsWith('.server.ts') && !context.filename.endsWith('+server.ts')) + return; + // Find the function we are interested in + let functionExport; + // load or actions in .server.ts files + if (context.filename.endsWith('.server.ts') && node.declaration) { + if (node.declaration.type === 'FunctionDeclaration') { + if (node.declaration.id?.name === 'load') { + functionExport = node.declaration; + } + } + else { + functionExport = node.declaration.declarations?.find((decl) => { + return (decl.id.type === 'Identifier' && + (decl.id.name === 'load' || decl.id.name === 'actions')); + }); + } + // endpoint functions in +server.ts files + } + else if (context.filename.endsWith('+server.ts') && node.declaration) { + if (node.declaration.type === 'FunctionDeclaration') { + // export function POST/GET/PUT/etc. ... + if (node.declaration.id && + ['POST', 'GET', 'PUT', 'DELETE', 'PATCH'].includes(node.declaration.id.name)) { + functionExport = node.declaration; + } + } + else { + functionExport = node.declaration.declarations?.find((decl) => { + return (decl.id.type === 'Identifier' && + ['POST', 'GET', 'PUT', 'DELETE', 'PATCH'].includes(decl.id.name)); + }); + } + } + // This export is not a load, actions, or endpoint function + if (!functionExport) + return; + const blockStatements = []; + if (functionExport.id.name === 'actions') { + // This is the actions object, so find all the functions in its properties + const objectExpression = functionExport.type === 'VariableDeclarator' + ? walkFind(functionExport.init, (n) => n.type === 'ObjectExpression', 4) + : null; + if (functionExport.type !== 'VariableDeclarator' || !objectExpression) { + context.report({ + node: node.declaration, + data: { type: functionExport.type }, + messageId: 'unexpectedFunction' + }); + return; + } + const properties = objectExpression.properties.filter((p) => p.type === 'Property'); + for (const prop of properties) { + if (prop.value.type === 'ArrowFunctionExpression' || + prop.value.type === 'FunctionExpression') { + // There should be a block statement (function body) within a couple levels + const block = walkFind(prop.value, (n) => n.type === 'BlockStatement', 4); + if (block) + blockStatements.push(block); + } + else { + context.report({ + node: prop, + data: { type: prop.value.type }, + messageId: 'unexpectedFunction' + }); + } + } + } + else if ((!functionExport?.init || + !['ArrowFunctionExpression', 'FunctionExpression', 'TSSatisfiesExpression'].includes(functionExport.init.type)) && + node.declaration.type !== 'FunctionDeclaration') { + // Unexpected type of export + context.report({ + node: node.declaration, + data: { type: functionExport.init?.type }, + messageId: 'unexpectedFunction' + }); + return; + } + else { + // This is a single function (load or endpoint), so find its body + // There should be a block statement (function body) within a couple levels + const blockStatement = walkFind(functionExport.init || + functionExport, (n) => n.type === 'BlockStatement', 4); + if (blockStatement) + blockStatements.push(blockStatement); + else { + context.report({ + node: node.declaration, + data: { type: functionExport.type }, + messageId: 'unexpectedFunction' + }); + return; + } + } + blockStatements.forEach((bs) => { + if (!blockStatementIncludesSecurityCall(bs)) { + // No security check found in this function's block statement + // Flag an error on the parent node of the block statement (the whole function) + context.report({ + node: bs.parent, + messageId: 'missingSecurityCheck', + suggest: [ + { + messageId: 'suggestAuthCall', + fix: (fixer) => fixer.insertTextBefore(bs.body[0] || bs, 'event.locals.security.requireAuthenticated();\n' + + (functionExport.id.name === 'actions' + ? ' ' + : ' ')) + }, + { + messageId: 'suggestNoAuthCall', + fix: (fixer) => fixer.insertTextBefore(bs.body[0] || bs, 'event.locals.security.requireNothing();\n' + + (functionExport.id.name === 'actions' + ? ' ' + : ' ')) + } + ] + }); + } + }); + } + }; + } +}); diff --git a/eslint-rules/require-security-check.ts b/eslint-rules/require-security-check.ts new file mode 100644 index 0000000000..8b13033100 --- /dev/null +++ b/eslint-rules/require-security-check.ts @@ -0,0 +1,266 @@ +/** + * ESLint rule to require a security check in every server load/action or + * endpoint function. This is a sanity check to avoid forgetting to add a + * security check on endpoints that return data or perform actions. It + * looks for calls to locals.security.require*() in the function body of + * exported load and actions in +page.server.ts/+layout.server.ts files, + * or POST/GET/PUT/etc. functions in +server.ts files. + * + * IT IS NOT FOOLPROOF. + * IT IS POSSIBLE TO BYPASS THIS CHECK (but hopefully not accidentally). + * + * After updating this file, run `tsc` from the `eslint-rules` directory to + * compile it to JavaScript (which is checked in). + */ +import type { TSESTree } from '@typescript-eslint/utils'; +import { ESLintUtils } from '@typescript-eslint/utils'; + +// Walk the AST to find a node that matches the callback, up to maxDepth levels deep +// This is a depth-first search and could be improved +function walkFind( + node: TSESTree.Node | undefined | null, + callback: (node: TSESTree.Node) => boolean, + maxDepth = 10 +): TSESTree.Node | null { + if (maxDepth <= 0) return null; + if (node && typeof node !== 'object') return null; + if (callback(node)) return node; + for (const key in node) { + if (key === 'parent') continue; + const val = node[key]; + if (Array.isArray(val)) { + for (const child of val) { + const result = walkFind(child, callback, maxDepth - 1); + if (result) return result; + } + } else { + const result = walkFind(val, callback, maxDepth - 1); + if (result) return result; + } + } + return null; +} + +function blockStatementIncludesSecurityCall(block: TSESTree.BlockStatement): boolean { + const nodeIsSecurityCall = (node: TSESTree.Node) => { + // Valid check if the node is a CallExpression of the form: + // *.locals.security.require*() + // or locals.security.require*() + if (node?.type === 'CallExpression' && node.callee.type === 'MemberExpression') { + const obj = node.callee.object; + const prop = node.callee.property; + if ( + obj.type === 'MemberExpression' && + // object is either *.locals or just locals + ((obj.object.type === 'MemberExpression' && + obj.object.property.type === 'Identifier' && + obj.object.property.name === 'locals') || + (obj.object.type === 'Identifier' && obj.object.name === 'locals')) && + // locals.security + obj.property.type === 'Identifier' && + obj.property.name === 'security' && + prop.type === 'Identifier' && + prop.name.startsWith('require') // ex. requireAuthenticated, requireRole, etc. + ) { + return true; + } + } + return false; + }; + + return !!block.body.find((n) => !!walkFind(n, nodeIsSecurityCall, 6)); +} + +export default ESLintUtils.RuleCreator(() => '')({ + name: 'require-security-check', + meta: { + type: 'problem', + hasSuggestions: true, + docs: { + description: 'Require calling event.locals.security.* in every server load/action' + }, + schema: [], + messages: { + missingSecurityCheck: + 'Missing call to requireAuthenticated() or similar security check. This must be of the form locals.security.require*()', + unexpectedFunction: + 'Unexpected export function type {{type}}. Expected ArrowFunctionExpression, FunctionExpression, TSSatisfiesExpression or ObjectExpression.', + suggestAuthCall: + 'Require authentication by adding "event.locals.security.requireAuthenticated();" to the start of the function body.', + suggestNoAuthCall: + 'Mark this function as not requiring any auth by adding "event.locals.security.requireNothing();" to the start of the function body.' + } + }, + defaultOptions: [], + create(context) { + return { + ExportNamedDeclaration(node) { + // Application of this rule is limited to: + // export const load = async (event) => { ... } + // export const actions = { default: async (event) => { ... } } + // export function load(event) { ... } + // export POST/GET/PUT/etc. function for endpoints in +server.ts files + + // Only check .server.ts and +server.ts files + if (!context.filename.endsWith('.server.ts') && !context.filename.endsWith('+server.ts')) + return; + + // Find the function we are interested in + let functionExport: + | TSESTree.VariableDeclaratorMaybeInit + | TSESTree.VariableDeclaratorDefiniteAssignment + | TSESTree.FunctionDeclaration; + + // load or actions in .server.ts files + if (context.filename.endsWith('.server.ts') && node.declaration) { + if (node.declaration.type === 'FunctionDeclaration') { + if (node.declaration.id?.name === 'load') { + functionExport = node.declaration; + } + } else { + functionExport = (node.declaration as TSESTree.VariableDeclaration).declarations?.find( + (decl) => { + return ( + decl.id.type === 'Identifier' && + (decl.id.name === 'load' || decl.id.name === 'actions') + ); + } + ); + } + // endpoint functions in +server.ts files + } else if (context.filename.endsWith('+server.ts') && node.declaration) { + if (node.declaration.type === 'FunctionDeclaration') { + // export function POST/GET/PUT/etc. ... + if ( + node.declaration.id && + ['POST', 'GET', 'PUT', 'DELETE', 'PATCH'].includes(node.declaration.id.name) + ) { + functionExport = node.declaration; + } + } else { + functionExport = (node.declaration as TSESTree.VariableDeclaration).declarations?.find( + (decl) => { + return ( + decl.id.type === 'Identifier' && + ['POST', 'GET', 'PUT', 'DELETE', 'PATCH'].includes(decl.id.name) + ); + } + ); + } + } + // This export is not a load, actions, or endpoint function + if (!functionExport) return; + + const blockStatements: TSESTree.BlockStatement[] = []; + if ((functionExport.id as TSESTree.Identifier).name === 'actions') { + // This is the actions object, so find all the functions in its properties + const objectExpression: TSESTree.ObjectExpression = + functionExport.type === 'VariableDeclarator' + ? (walkFind( + functionExport.init, + (n) => n.type === 'ObjectExpression', + 4 + ) as TSESTree.ObjectExpression) + : null; + if (functionExport.type !== 'VariableDeclarator' || !objectExpression) { + context.report({ + node: node.declaration, + data: { type: functionExport.type }, + messageId: 'unexpectedFunction' + }); + return; + } + const properties = objectExpression.properties.filter( + (p): p is TSESTree.Property => p.type === 'Property' + ); + for (const prop of properties) { + if ( + prop.value.type === 'ArrowFunctionExpression' || + prop.value.type === 'FunctionExpression' + ) { + // There should be a block statement (function body) within a couple levels + const block = walkFind( + prop.value, + (n) => n.type === 'BlockStatement', + 4 + ) as TSESTree.BlockStatement | null; + if (block) blockStatements.push(block); + } else { + context.report({ + node: prop, + data: { type: prop.value.type }, + messageId: 'unexpectedFunction' + }); + } + } + } else if ( + (!(functionExport as TSESTree.VariableDeclaratorMaybeInit)?.init || + !['ArrowFunctionExpression', 'FunctionExpression', 'TSSatisfiesExpression'].includes( + (functionExport as TSESTree.VariableDeclaratorMaybeInit).init.type + )) && + node.declaration.type !== 'FunctionDeclaration' + ) { + // Unexpected type of export + context.report({ + node: node.declaration, + data: { type: (functionExport as TSESTree.VariableDeclaratorMaybeInit).init?.type }, + messageId: 'unexpectedFunction' + }); + return; + } else { + // This is a single function (load or endpoint), so find its body + // There should be a block statement (function body) within a couple levels + const blockStatement = walkFind( + (functionExport as TSESTree.VariableDeclaratorMaybeInit).init || + (functionExport as TSESTree.FunctionDeclaration), + (n) => n.type === 'BlockStatement', + 4 + ) as TSESTree.BlockStatement; + if (blockStatement) blockStatements.push(blockStatement); + else { + context.report({ + node: node.declaration, + data: { type: functionExport.type }, + messageId: 'unexpectedFunction' + }); + return; + } + } + blockStatements.forEach((bs) => { + if (!blockStatementIncludesSecurityCall(bs)) { + // No security check found in this function's block statement + // Flag an error on the parent node of the block statement (the whole function) + context.report({ + node: bs.parent, + messageId: 'missingSecurityCheck', + suggest: [ + { + messageId: 'suggestAuthCall', + fix: (fixer) => + fixer.insertTextBefore( + bs.body[0] || bs, + 'event.locals.security.requireAuthenticated();\n' + + ((functionExport.id as TSESTree.Identifier).name === 'actions' + ? ' ' + : ' ') + ) + }, + { + messageId: 'suggestNoAuthCall', + fix: (fixer) => + fixer.insertTextBefore( + bs.body[0] || bs, + 'event.locals.security.requireNothing();\n' + + ((functionExport.id as TSESTree.Identifier).name === 'actions' + ? ' ' + : ' ') + ) + } + ] + }); + } + }); + } + }; + } +}); diff --git a/eslint-rules/tsconfig.json b/eslint-rules/tsconfig.json new file mode 100644 index 0000000000..7789009940 --- /dev/null +++ b/eslint-rules/tsconfig.json @@ -0,0 +1,8 @@ +{ +"compilerOptions": { + "target": "esnext", + "module": "esnext", + "moduleResolution": "node", + "skipLibCheck": true +} +} \ No newline at end of file diff --git a/eslint.config.js b/eslint.config.js index 13b669f4f6..0e5a6692d5 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -7,6 +7,7 @@ import svelte from 'eslint-plugin-svelte'; import globals from 'globals'; import ts from 'typescript-eslint'; import svelteConfig from './svelte.config.js'; +import requireSecurityCheck from './eslint-rules/require-security-check.js'; export default ts.config( js.configs.recommended, @@ -22,6 +23,13 @@ export default ts.config( ...globals.browser, ...globals.node } + }, + plugins: { + 'sveltekit-sec': { + rules: { + "require-security-check": requireSecurityCheck + } + } } }, { @@ -60,6 +68,7 @@ export default ts.config( } ], 'import/no-unresolved': 'off', + 'sveltekit-sec/require-security-check': 'error' } }, globalIgnores([ diff --git a/package-lock.json b/package-lock.json index f8fcb55467..53de3e2d34 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43,8 +43,9 @@ "@types/express": "^5.0.0", "@types/node": "^22.10.10", "@types/nodemailer": "^7.0.1", - "@typescript-eslint/eslint-plugin": "^8.21.0", - "@typescript-eslint/parser": "^8.21.0", + "@typescript-eslint/eslint-plugin": "^8.44.0", + "@typescript-eslint/parser": "^8.44.0", + "@typescript-eslint/utils": "^8.44.0", "@vvo/tzdb": "^6.159.0", "@zerodevx/svelte-toast": "^0.9.6", "bullmq": "^5.52.0", @@ -78,7 +79,7 @@ "ts-node": "^10.9.2", "tslib": "^2.8.1", "typescript": "^5.7.3", - "typescript-eslint": "^8.37.0", + "typescript-eslint": "^8.44.0", "valibot": "^1.0.0-beta.14", "vite": "^6.0.11", "vitest": "^3.0.4", @@ -4663,17 +4664,17 @@ } }, "node_modules/@typescript-eslint/eslint-plugin": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.37.0.tgz", - "integrity": "sha512-jsuVWeIkb6ggzB+wPCsR4e6loj+rM72ohW6IBn2C+5NCvfUVY8s33iFPySSVXqtm5Hu29Ne/9bnA0JmyLmgenA==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.44.0.tgz", + "integrity": "sha512-EGDAOGX+uwwekcS0iyxVDmRV9HX6FLSM5kzrAToLTsr9OWCIKG/y3lQheCq18yZ5Xh78rRKJiEpP0ZaCs4ryOQ==", "dev": true, "license": "MIT", "dependencies": { "@eslint-community/regexpp": "^4.10.0", - "@typescript-eslint/scope-manager": "8.37.0", - "@typescript-eslint/type-utils": "8.37.0", - "@typescript-eslint/utils": "8.37.0", - "@typescript-eslint/visitor-keys": "8.37.0", + "@typescript-eslint/scope-manager": "8.44.0", + "@typescript-eslint/type-utils": "8.44.0", + "@typescript-eslint/utils": "8.44.0", + "@typescript-eslint/visitor-keys": "8.44.0", "graphemer": "^1.4.0", "ignore": "^7.0.0", "natural-compare": "^1.4.0", @@ -4687,9 +4688,9 @@ "url": "https://opencollective.com/typescript-eslint" }, "peerDependencies": { - "@typescript-eslint/parser": "^8.37.0", + "@typescript-eslint/parser": "^8.44.0", "eslint": "^8.57.0 || ^9.0.0", - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/eslint-plugin/node_modules/ignore": { @@ -4703,16 +4704,16 @@ } }, "node_modules/@typescript-eslint/parser": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.37.0.tgz", - "integrity": "sha512-kVIaQE9vrN9RLCQMQ3iyRlVJpTiDUY6woHGb30JDkfJErqrQEmtdWH3gV0PBAfGZgQXoqzXOO0T3K6ioApbbAA==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.44.0.tgz", + "integrity": "sha512-VGMpFQGUQWYT9LfnPcX8ouFojyrZ/2w3K5BucvxL/spdNehccKhB4jUyB1yBCXpr2XFm0jkECxgrpXBW2ipoAw==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/scope-manager": "8.37.0", - "@typescript-eslint/types": "8.37.0", - "@typescript-eslint/typescript-estree": "8.37.0", - "@typescript-eslint/visitor-keys": "8.37.0", + "@typescript-eslint/scope-manager": "8.44.0", + "@typescript-eslint/types": "8.44.0", + "@typescript-eslint/typescript-estree": "8.44.0", + "@typescript-eslint/visitor-keys": "8.44.0", "debug": "^4.3.4" }, "engines": { @@ -4724,18 +4725,18 @@ }, "peerDependencies": { "eslint": "^8.57.0 || ^9.0.0", - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/project-service": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.37.0.tgz", - "integrity": "sha512-BIUXYsbkl5A1aJDdYJCBAo8rCEbAvdquQ8AnLb6z5Lp1u3x5PNgSSx9A/zqYc++Xnr/0DVpls8iQ2cJs/izTXA==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.44.0.tgz", + "integrity": "sha512-ZeaGNraRsq10GuEohKTo4295Z/SuGcSq2LzfGlqiuEvfArzo/VRrT0ZaJsVPuKZ55lVbNk8U6FcL+ZMH8CoyVA==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/tsconfig-utils": "^8.37.0", - "@typescript-eslint/types": "^8.37.0", + "@typescript-eslint/tsconfig-utils": "^8.44.0", + "@typescript-eslint/types": "^8.44.0", "debug": "^4.3.4" }, "engines": { @@ -4746,18 +4747,18 @@ "url": "https://opencollective.com/typescript-eslint" }, "peerDependencies": { - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/scope-manager": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.37.0.tgz", - "integrity": "sha512-0vGq0yiU1gbjKob2q691ybTg9JX6ShiVXAAfm2jGf3q0hdP6/BruaFjL/ManAR/lj05AvYCH+5bbVo0VtzmjOA==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.44.0.tgz", + "integrity": "sha512-87Jv3E+al8wpD+rIdVJm/ItDBe/Im09zXIjFoipOjr5gHUhJmTzfFLuTJ/nPTMc2Srsroy4IBXwcTCHyRR7KzA==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.37.0", - "@typescript-eslint/visitor-keys": "8.37.0" + "@typescript-eslint/types": "8.44.0", + "@typescript-eslint/visitor-keys": "8.44.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -4768,9 +4769,9 @@ } }, "node_modules/@typescript-eslint/tsconfig-utils": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.37.0.tgz", - "integrity": "sha512-1/YHvAVTimMM9mmlPvTec9NP4bobA1RkDbMydxG8omqwJJLEW/Iy2C4adsAESIXU3WGLXFHSZUU+C9EoFWl4Zg==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.44.0.tgz", + "integrity": "sha512-x5Y0+AuEPqAInc6yd0n5DAcvtoQ/vyaGwuX5HE9n6qAefk1GaedqrLQF8kQGylLUb9pnZyLf+iEiL9fr8APDtQ==", "dev": true, "license": "MIT", "engines": { @@ -4781,19 +4782,19 @@ "url": "https://opencollective.com/typescript-eslint" }, "peerDependencies": { - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/type-utils": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-8.37.0.tgz", - "integrity": "sha512-SPkXWIkVZxhgwSwVq9rqj/4VFo7MnWwVaRNznfQDc/xPYHjXnPfLWn+4L6FF1cAz6e7dsqBeMawgl7QjUMj4Ow==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-8.44.0.tgz", + "integrity": "sha512-9cwsoSxJ8Sak67Be/hD2RNt/fsqmWnNE1iHohG8lxqLSNY8xNfyY7wloo5zpW3Nu9hxVgURevqfcH6vvKCt6yg==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.37.0", - "@typescript-eslint/typescript-estree": "8.37.0", - "@typescript-eslint/utils": "8.37.0", + "@typescript-eslint/types": "8.44.0", + "@typescript-eslint/typescript-estree": "8.44.0", + "@typescript-eslint/utils": "8.44.0", "debug": "^4.3.4", "ts-api-utils": "^2.1.0" }, @@ -4806,13 +4807,13 @@ }, "peerDependencies": { "eslint": "^8.57.0 || ^9.0.0", - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/types": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.37.0.tgz", - "integrity": "sha512-ax0nv7PUF9NOVPs+lmQ7yIE7IQmAf8LGcXbMvHX5Gm+YJUYNAl340XkGnrimxZ0elXyoQJuN5sbg6C4evKA4SQ==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.44.0.tgz", + "integrity": "sha512-ZSl2efn44VsYM0MfDQe68RKzBz75NPgLQXuGypmym6QVOWL5kegTZuZ02xRAT9T+onqvM6T8CdQk0OwYMB6ZvA==", "dev": true, "license": "MIT", "engines": { @@ -4824,16 +4825,16 @@ } }, "node_modules/@typescript-eslint/typescript-estree": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.37.0.tgz", - "integrity": "sha512-zuWDMDuzMRbQOM+bHyU4/slw27bAUEcKSKKs3hcv2aNnc/tvE/h7w60dwVw8vnal2Pub6RT1T7BI8tFZ1fE+yg==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.44.0.tgz", + "integrity": "sha512-lqNj6SgnGcQZwL4/SBJ3xdPEfcBuhCG8zdcwCPgYcmiPLgokiNDKlbPzCwEwu7m279J/lBYWtDYL+87OEfn8Jw==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/project-service": "8.37.0", - "@typescript-eslint/tsconfig-utils": "8.37.0", - "@typescript-eslint/types": "8.37.0", - "@typescript-eslint/visitor-keys": "8.37.0", + "@typescript-eslint/project-service": "8.44.0", + "@typescript-eslint/tsconfig-utils": "8.44.0", + "@typescript-eslint/types": "8.44.0", + "@typescript-eslint/visitor-keys": "8.44.0", "debug": "^4.3.4", "fast-glob": "^3.3.2", "is-glob": "^4.0.3", @@ -4849,20 +4850,20 @@ "url": "https://opencollective.com/typescript-eslint" }, "peerDependencies": { - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/utils": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.37.0.tgz", - "integrity": "sha512-TSFvkIW6gGjN2p6zbXo20FzCABbyUAuq6tBvNRGsKdsSQ6a7rnV6ADfZ7f4iI3lIiXc4F4WWvtUfDw9CJ9pO5A==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.44.0.tgz", + "integrity": "sha512-nktOlVcg3ALo0mYlV+L7sWUD58KG4CMj1rb2HUVOO4aL3K/6wcD+NERqd0rrA5Vg06b42YhF6cFxeixsp9Riqg==", "dev": true, "license": "MIT", "dependencies": { "@eslint-community/eslint-utils": "^4.7.0", - "@typescript-eslint/scope-manager": "8.37.0", - "@typescript-eslint/types": "8.37.0", - "@typescript-eslint/typescript-estree": "8.37.0" + "@typescript-eslint/scope-manager": "8.44.0", + "@typescript-eslint/types": "8.44.0", + "@typescript-eslint/typescript-estree": "8.44.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -4873,17 +4874,17 @@ }, "peerDependencies": { "eslint": "^8.57.0 || ^9.0.0", - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/@typescript-eslint/visitor-keys": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.37.0.tgz", - "integrity": "sha512-YzfhzcTnZVPiLfP/oeKtDp2evwvHLMe0LOy7oe+hb9KKIumLNohYS9Hgp1ifwpu42YWxhZE8yieggz6JpqO/1w==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.44.0.tgz", + "integrity": "sha512-zaz9u8EJ4GBmnehlrpoKvj/E3dNbuQ7q0ucyZImm3cLqJ8INTc970B1qEqDX/Rzq65r3TvVTN7kHWPBoyW7DWw==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.37.0", + "@typescript-eslint/types": "8.44.0", "eslint-visitor-keys": "^4.2.1" }, "engines": { @@ -12222,16 +12223,16 @@ } }, "node_modules/typescript-eslint": { - "version": "8.37.0", - "resolved": "https://registry.npmjs.org/typescript-eslint/-/typescript-eslint-8.37.0.tgz", - "integrity": "sha512-TnbEjzkE9EmcO0Q2zM+GE8NQLItNAJpMmED1BdgoBMYNdqMhzlbqfdSwiRlAzEK2pA9UzVW0gzaaIzXWg2BjfA==", + "version": "8.44.0", + "resolved": "https://registry.npmjs.org/typescript-eslint/-/typescript-eslint-8.44.0.tgz", + "integrity": "sha512-ib7mCkYuIzYonCq9XWF5XNw+fkj2zg629PSa9KNIQ47RXFF763S5BIX4wqz1+FLPogTZoiw8KmCiRPRa8bL3qw==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/eslint-plugin": "8.37.0", - "@typescript-eslint/parser": "8.37.0", - "@typescript-eslint/typescript-estree": "8.37.0", - "@typescript-eslint/utils": "8.37.0" + "@typescript-eslint/eslint-plugin": "8.44.0", + "@typescript-eslint/parser": "8.44.0", + "@typescript-eslint/typescript-estree": "8.44.0", + "@typescript-eslint/utils": "8.44.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -12242,7 +12243,7 @@ }, "peerDependencies": { "eslint": "^8.57.0 || ^9.0.0", - "typescript": ">=4.8.4 <5.9.0" + "typescript": ">=4.8.4 <6.0.0" } }, "node_modules/unbox-primitive": { diff --git a/package.json b/package.json index 3c9e1214cd..51a247d904 100644 --- a/package.json +++ b/package.json @@ -55,8 +55,9 @@ "@types/express": "^5.0.0", "@types/node": "^22.10.10", "@types/nodemailer": "^7.0.1", - "@typescript-eslint/eslint-plugin": "^8.21.0", - "@typescript-eslint/parser": "^8.21.0", + "@typescript-eslint/eslint-plugin": "^8.44.0", + "@typescript-eslint/parser": "^8.44.0", + "@typescript-eslint/utils": "^8.44.0", "@vvo/tzdb": "^6.159.0", "@zerodevx/svelte-toast": "^0.9.6", "bullmq": "^5.52.0", @@ -90,7 +91,7 @@ "ts-node": "^10.9.2", "tslib": "^2.8.1", "typescript": "^5.7.3", - "typescript-eslint": "^8.37.0", + "typescript-eslint": "^8.44.0", "valibot": "^1.0.0-beta.14", "vite": "^6.0.11", "vitest": "^3.0.4", diff --git a/src/app.d.ts b/src/app.d.ts index 4e6667e94d..0c934c5d33 100644 --- a/src/app.d.ts +++ b/src/app.d.ts @@ -1,13 +1,54 @@ -import type { ParaglideLocals } from '@inlang/paraglide-sveltekit'; -import type { AvailableLanguageTag } from '../../lib/paraglide/runtime'; // See https://kit.svelte.dev/docs/types#app // for information about these interfaces + +// Security class type because this file cannot import anything declare global { + interface SecurityLike { + userId: number; + roles: Map; + } + class Security implements SecurityLike { + public readonly isSuperAdmin: boolean; + public readonly userId: number; + public readonly organizationMemberships: number[]; + public readonly roles: Map; + public readonly sessionForm: Session['user']; + requireAuthenticated(): void | never; + requireSuperAdmin(): this | never; + requireAdminOfOrg(organizationId: number): this | never; + requireAdminOfOrgIn(organizationIds: number[]): this | never; + requireAdminOfAny(): this | never; + requireHasRole(organizationId: number, roleId: number, orOrgAdmin?: boolean): this | never; + requireMemberOfOrg(organizationId: number): this | never; + requireMemberOfOrgOrSuperAdmin(organizationId: number): this | never; + requireProjectWriteAccess( + project?: { OwnerId: number; OrganizationId: number } | null + ): this | never; + requireProjectReadAccess( + userGroups: { GroupId: number }[], + project?: { + OwnerId: number; + OrganizationId: number; + GroupId: number; + } | null + ): this | never; + requireProjectClaimable( + userGroups: { GroupId: number }[], + project?: { + OwnerId: number; + OrganizationId: number; + GroupId: number; + } + ): this | never; + requireMemberOfAnyOrg(): this | never; + requireNothing(): this | never; + } namespace App { // interface Error {} interface Locals { + // This typing doesn't work, but we never use it paraglide: ParaglideLocals; - error?: number; + security: Security; } // interface PageData {} // interface Platform {} diff --git a/src/auth.ts b/src/auth.ts index eb629ced80..f390b64fd9 100644 --- a/src/auth.ts +++ b/src/auth.ts @@ -6,27 +6,21 @@ import { } from '@auth/sveltekit'; import Auth0Provider from '@auth/sveltekit/providers/auth0'; import { trace } from '@opentelemetry/api'; -import { type Handle, redirect } from '@sveltejs/kit'; +import { type Handle, error, redirect } from '@sveltejs/kit'; import { env } from '$env/dynamic/private'; import { checkInviteErrors } from '$lib/organizationInvites'; import { localizeHref } from '$lib/paraglide/runtime'; -import type { RoleId } from '$lib/prisma'; -import { verifyCanEdit, verifyCanView } from '$lib/projects/server'; +import { RoleId } from '$lib/prisma'; import { DatabaseReads, DatabaseWrites } from '$lib/server/database'; -import { adminOrgs } from '$lib/users/server'; -import { ServerStatus } from '$lib/utils'; -import { isAdmin, isAdminForOrg, isSuperAdmin } from '$lib/utils/roles'; declare module '@auth/sveltekit' { interface Session { - user: { + user: DefaultSession['user'] & { userId: number; - /** [organizationId, RoleId][]*/ roles: [number, RoleId][]; - } & DefaultSession['user']; + }; } } - // Stupidly hacky way to get access to the request data from the auth callbacks // This is a global variable that is set in a separate hook handle let currentInviteToken: string | null = null; @@ -90,6 +84,8 @@ const config: SvelteKitAuthConfig = { } throw new Error('Invalid state'); } finally { + tokenStatus = null; + currentInviteToken = null; trace.getActiveSpan()?.addEvent('signIn callback completed', { 'auth.signIn.profile': profile ? profile.email + ' - ' + profile.sub : 'unknown', 'auth.signIn.tokenStatus': tokenStatus ?? 'null', @@ -129,7 +125,7 @@ const config: SvelteKitAuthConfig = { UserId: token.userId as number } }); - session.user.roles = userRoles.map((role) => [role.OrganizationId, role.RoleId]); + session.user.roles = userRoles.map(({ OrganizationId, RoleId }) => [OrganizationId, RoleId]); trace.getActiveSpan()?.addEvent('session callback completed', { 'auth.session.userId': session.user.userId, 'auth.session.roles': JSON.stringify(session.user.roles) @@ -138,47 +134,214 @@ const config: SvelteKitAuthConfig = { } } }; -// Handles the /auth route, which is used to handle external auth0 authentication -export const { handle: authRouteHandle, signIn, signOut } = SvelteKitAuth(config); -export const checkUserExistsHandle: Handle = async ({ event, resolve }) => { - // If the user does not exist in the database, invalidate the login and redirect to prevent unauthorized access - // This can happen when the user is deleted from the database but still has a valid session. - // This should only happen when a superadmin manually deletes a user but is particularly annoying in development - // The user should also be redirected if they are not a member of any organizations - // Finally, the user should be redirected if they are locked - const userId = (await event.locals.auth())?.user.userId; - if (!userId) { - // User has no session at all; allow normal events - return resolve(event); +export class Security { + public readonly isSuperAdmin; + public securityHandled = false; + public readonly sessionForm: Session['user']; + constructor( + public readonly event: Parameters[0]['event'], + // Note these three CAN be null if the user is not authenticated + public readonly userId: number, + public readonly organizationMemberships: number[], + public readonly roles: Map + ) { + this.isSuperAdmin = roles?.values().some((r) => r.includes(RoleId.SuperAdmin)) ?? false; + this.sessionForm = { + userId: this.userId, + roles: [ + ...(roles + ?.entries() + .map(([orgId, roleIds]) => roleIds.map((r) => [orgId, r]) as [number, RoleId][]) ?? []) + ].flat(1) + }; } - const user = await DatabaseReads.users.findUnique({ - where: { - Id: userId - }, - include: { - OrganizationMemberships: true + + requireAuthenticated() { + this.securityHandled = true; + if (!this.userId || !this.organizationMemberships || !this.roles) { + // Redirect to login + const originalUrl = this.event.url; + const returnTo = originalUrl.pathname + originalUrl.search; + throw redirect(302, localizeHref('/login?returnTo=' + encodeURIComponent(returnTo))); } - }); - trace.getActiveSpan()?.addEvent('checkUserExistsHandle completed', { - 'auth.checkUserExists.userId': userId, - 'auth.checkUserExists.user': user ? user.Email + ' - ' + user.Id : 'null', - 'auth.checkUserExists.user.OrganizationMemberships': user - ? JSON.stringify(user.OrganizationMemberships.map((m) => m.OrganizationId)) - : 'null', - 'auth.checkUserExists.user.IsLocked': user ? user.IsLocked.toString() : 'null' - }); - if (!user || (user.OrganizationMemberships.length === 0 && !event.cookies.get('inviteToken'))) { - event.cookies.set('authjs.session-token', '', { path: '/' }); - return redirect(302, localizeHref('/login/no-organization')); } - if (user.IsLocked) { - event.cookies.set('authjs.session-token', '', { path: '/' }); - return redirect(302, localizeHref('/login/locked')); + + requireSuperAdmin() { + this.requireAuthenticated(); + if (!this.isSuperAdmin) error(403, 'User is not a super admin'); + return this; + } + + requireAdminOfOrgIn(organizationIds: number[]) { + this.requireAuthenticated(); + if ( + !this.isSuperAdmin && + !organizationIds.some((id) => this.roles.get(id)?.includes(RoleId.OrgAdmin)) + ) { + error(403, 'User is not an admin for the organization'); + } + return this; + } + + requireAdminOfOrg(organizationId: number) { + this.requireAuthenticated(); + this.requireAdminOfOrgIn([organizationId]); + return this; + } + + requireAdminOfAny() { + this.requireAuthenticated(); + if (!this.isSuperAdmin && !this.roles.values().some((r) => r.includes(RoleId.OrgAdmin))) + error(403, 'User is not an admin of any organization'); + return this; + } + + requireHasRole(organizationId: number, roleId: RoleId, orOrgAdmin = true) { + this.requireAuthenticated(); + if ( + !this.isSuperAdmin && + !this.roles.get(organizationId)?.includes(roleId) && + !(orOrgAdmin && this.roles.get(organizationId)?.includes(RoleId.OrgAdmin)) + ) + error(403, 'User does not have the required role ' + roleId); + return this; + } + + requireMemberOfOrg(organizationId: number) { + this.requireAuthenticated(); + if (!this.organizationMemberships.includes(organizationId)) + error(403, 'User is not a member of the organization'); + return this; + } + + requireMemberOfOrgOrSuperAdmin(organizationId: number) { + this.requireAuthenticated(); + if (!this.isSuperAdmin) this.requireMemberOfOrg(organizationId); + return this; + } + + requireProjectWriteAccess(project?: { OwnerId: number; OrganizationId: number } | null) { + this.requireAuthenticated(); + if (!project) { + error(404, 'Project not found'); + } + if (!this.isSuperAdmin && project.OwnerId !== this.userId) { + this.requireAdminOfOrg(project.OrganizationId); + } + return this; + } + + requireProjectReadAccess( + userGroups: { GroupId: number }[], + project?: { OwnerId: number; OrganizationId: number; GroupId: number } | null + ) { + this.requireAuthenticated(); + if (!project) { + error(404, 'Project not found'); + } + if (!userGroups.find((ug) => ug.GroupId === project.GroupId)) { + this.requireProjectWriteAccess(project); + } + return this; + } + + requireProjectClaimable( + userGroups: { GroupId: number }[], + project?: { OwnerId: number; OrganizationId: number; GroupId: number } | null + ) { + this.requireAuthenticated(); + if (!project) { + error(404, 'Project not found'); + } + if (this.userId === project.OwnerId) { + error(400, 'Project owner cannot claim own project'); + } + if (!this.isSuperAdmin && !userGroups.some((ug) => ug.GroupId === project.GroupId)) { + error(400, 'User must share a group with the project in order to claim it!'); + } + return this.requireHasRole( + project.OrganizationId, + RoleId.AppBuilder, + true + ).requireProjectReadAccess(userGroups, project); + } + + requireMemberOfAnyOrg() { + this.requireAuthenticated(); + if (this.organizationMemberships.length === 0) + error(403, 'User is not a member of any organization'); + return this; + } + + requireNothing() { + this.securityHandled = true; + return this; + } +} + +export const populateSecurityInfo: Handle = async ({ event, resolve }) => { + const security = (await event.locals.auth())?.user; + try { + if (security) { + // If the user does not exist in the database, invalidate the login and redirect to prevent unauthorized access + // This can happen when the user is deleted from the database but still has a valid session. + // This should only happen when a superadmin manually deletes a user but is particularly annoying in development + // The user should also be redirected if they are not a member of any organizations + // Finally, the user should be redirected if they are locked + const user = await DatabaseReads.users.findUnique({ + where: { + Id: security.userId + }, + include: { + OrganizationMemberships: true + } + }); + + trace.getActiveSpan()?.addEvent('checkUserExistsHandle completed', { + 'auth.userId': security?.userId, + 'auth.user': user ? user.Email + ' - ' + user.Id : 'null', + 'auth.user.OrganizationMemberships': + user?.OrganizationMemberships.map((o) => o.OrganizationId).join(', ') ?? 'null', + 'auth.user.roles': user ? JSON.stringify([...security.roles.entries()]) : 'null', + 'auth.user.IsLocked': user ? user.IsLocked + '' : 'null' + }); + + if ( + !user || + (user.OrganizationMemberships.length === 0 && !event.cookies.get('inviteToken')) + ) { + event.cookies.set('authjs.session-token', '', { path: '/' }); + return redirect(302, localizeHref('/login/no-organization')); + } + if (user.IsLocked) { + event.cookies.set('authjs.session-token', '', { path: '/' }); + return redirect(302, localizeHref('/login/locked')); + } + const roleMap = new Map(); + for (const [orgId, roleId] of security.roles) { + if (!roleMap.has(orgId)) roleMap.set(orgId, []); + roleMap.get(orgId)!.push(roleId); + } + event.locals.security = new Security( + event, + security.userId, + user.OrganizationMemberships.map((o) => o.OrganizationId), + roleMap + ); + } + } finally { + if (!event.locals.security) { + // @ts-expect-error Not typed as null but null is allowed + event.locals.security = new Security(event, null, null, null); + } } - return resolve(event); + return await resolve(event); }; +// Handles the /auth route, which is used to handle external auth0 authentication +export const { handle: authRouteHandle, signIn, signOut } = SvelteKitAuth(config); + // Handle organization invites export const organizationInviteHandle: Handle = async ({ event, resolve }) => { // Hacky solution to get the request object in the auth callbacks @@ -204,124 +367,3 @@ export const organizationInviteHandle: Handle = async ({ event, resolve }) => { tokenStatus = null; return result; }; - -// Locks down the authenticated routes by redirecting to /login -// This guarantees a logged in user under (authenticated) but does not guarantee -// authorization to the route. Each page must manually check in +page.server.ts or here -export const localRouteHandle: Handle = async ({ event, resolve }) => { - if ( - !event.route.id?.startsWith('/(unauthenticated)') && - event.route.id !== '/' && - event.route.id !== null - ) { - const session = await event.locals.auth(); - if (!session) return redirect(302, localizeHref('/login')); - const status = await validateRouteForAuthenticatedUser( - session, - event.route.id, - event.params, - event.request.method - ); - trace.getActiveSpan()?.addEvent('localRouteHandle completed', { - 'auth.localRouteHandle.routeId': event.route.id ?? 'null', - 'auth.localRouteHandle.params': JSON.stringify(event.params), - 'auth.localRouteHandle.session.userId': session.user.userId, - 'auth.localRouteHandle.session.roles': JSON.stringify(session.user.roles), - 'auth.localRouteHandle.status': status - }); - if (status !== ServerStatus.Ok) { - // error.html is extremely ugly, so we use a manual error throw - event.locals.error = status; - } - } - return resolve(event); -}; - -async function validateRouteForAuthenticatedUser( - session: Session, - route: string, - params: Partial>, - method: string -): Promise { - const path = route.split('/').filter((r) => !!r); - // Only guarding authenticated routes - if (path[0] === '(authenticated)') { - if (path[1] === 'admin' || path[1] === 'workflow-instances') - return isSuperAdmin(session?.user?.roles) ? ServerStatus.Ok : ServerStatus.Forbidden; - else if (path[1] === 'directory' || path[1] === 'open-source') - // Always allowed. Open pages - return ServerStatus.Ok; - else if (path[1] === 'organizations') { - // Must be org admin for some organization (or a super admin) - if (!isAdmin(session?.user?.roles)) return ServerStatus.Forbidden; - if (params.id) { - // Must be org admin for specified organization (or a super admin) - const Id = parseInt(params.id); - if (isNaN(Id) || !(await DatabaseReads.organizations.findFirst({ where: { Id } }))) { - return ServerStatus.NotFound; - } - return isAdminForOrg(Id, session?.user?.roles) ? ServerStatus.Ok : ServerStatus.Forbidden; - } - return ServerStatus.Ok; - } else if (path[1] === 'products') { - try { - const product = await DatabaseReads.products.findFirst({ - where: { - Id: params.id - } - }); - if (!product) return ServerStatus.NotFound; - // Must be allowed to view associated project - // (this route was originally part of the project page but was moved elsewhere to improve load time) - return await verifyCanView(session, product.ProjectId); - } catch { - return ServerStatus.NotFound; - } - } else if (path[1] === 'projects') { - if (path[2] === '[filter=projectSelector]') return ServerStatus.Ok; - else if (path[2] === '[id=idNumber]') { - // prevent edits and actions without breaking SSE - if (path[3] === 'edit' || (method !== 'GET' && path[3] !== 'sse')) { - return await verifyCanEdit(session, parseInt(params.id!)); - } - // A project can be viewed if the user is an admin or in the project's group - return await verifyCanView(session, parseInt(params.id!)); - } - return ServerStatus.Ok; - } else if (path[1] === 'tasks') { - // Own task list always allowed, and specific products checked manually - return ServerStatus.Ok; - } else if (path[1] === 'users') { - // /(invite): admin - if (path.length === 2 || path[2] === 'invite') { - return isAdmin(session?.user?.roles) ? ServerStatus.Ok : ServerStatus.Forbidden; - } else if (path[2] === '[id=idNumber]') { - // /id: not implemented yet (ISSUE #1142) - const subjectId = parseInt(params.id!); - if (!(await DatabaseReads.users.findFirst({ where: { Id: subjectId } }))) - return ServerStatus.NotFound; - const admin = !!(await DatabaseReads.organizations.findFirst({ - where: adminOrgs(subjectId, session.user.userId, isSuperAdmin(session.user.roles)), - select: { - Id: true - } - })); - // /id/settings/(profile): self and admin - if (!path.at(4) || path[4] === 'profile') { - return subjectId === session.user.userId || admin - ? ServerStatus.Ok - : ServerStatus.Forbidden; - } else { - // /id/settings/*: admin - return admin ? ServerStatus.Ok : ServerStatus.Forbidden; - } - } - return ServerStatus.Ok; - } else { - // Unknown route. We'll assume it's a legal route - return ServerStatus.Ok; - } - } else { - return ServerStatus.Ok; - } -} diff --git a/src/hooks.server.ts b/src/hooks.server.ts index e23b377086..487befbb57 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -3,14 +3,9 @@ // when code is bundled, we cannot keep the file separate to import it first. import { SpanStatusCode, trace } from '@opentelemetry/api'; -import { type Handle, type HandleServerError } from '@sveltejs/kit'; +import { type Handle, type HandleServerError, error } from '@sveltejs/kit'; import { sequence } from '@sveltejs/kit/hooks'; -import { - authRouteHandle, - checkUserExistsHandle, - localRouteHandle, - organizationInviteHandle -} from './auth'; +import { authRouteHandle, organizationInviteHandle, populateSecurityInfo } from './auth'; import { building } from '$app/environment'; import OTEL from '$lib/otel'; @@ -62,8 +57,7 @@ const heartbeat: Handle = async ({ event, resolve }) => { 'Connected to Valkey:', QueueConnected() ); - // HTTP 503 *should* be the correct semantics here? - event.locals.error = 503; + throw error(503, 'Database connection error'); } } return resolve(event); @@ -71,7 +65,7 @@ const heartbeat: Handle = async ({ event, resolve }) => { const tracer = trace.getTracer('IncomingRequest'); -const authSequence = sequence(authRouteHandle, checkUserExistsHandle, localRouteHandle); +const authSequence = sequence(authRouteHandle, populateSecurityInfo); export const handle: Handle = async ({ event, resolve }) => { if (event.url.pathname.startsWith('/.well-known/appspecific/')) { @@ -96,31 +90,74 @@ export const handle: Handle = async ({ event, resolve }) => { 'http.x-forwarded-for': event.request.headers.get('x-forwarded-for') ?? '', 'svelte.route_id': event.route.id ?? '' }); - const response = await sequence( - paraglideHandle, - heartbeat, - organizationInviteHandle, - // Handle auth hooks in a separate OTEL span - async function hookTelementry({ event, resolve }) { - return tracer.startActiveSpan('Auth Hooks', async (span) => { - // Call the auth sequence - const ret = await authSequence({ - event, - resolve: (...args) => { - span.end(); - return resolve(...args); + try { + const response = await sequence( + paraglideHandle, + heartbeat, + organizationInviteHandle, + // Handle auth hooks in a separate OTEL span + async function hookTelementry({ event, resolve }) { + return tracer.startActiveSpan('Auth Hooks', async (span) => { + // Call the auth sequence + let spanEnded = false; + try { + const ret = await authSequence({ + event, + resolve: (...args) => { + if (!spanEnded) { + span.end(); + spanEnded = true; + } + return resolve(...args); + } + }); + return ret; + } finally { + if (!spanEnded) { + span.end(); + spanEnded = true; + } } }); - return ret; + }, + bullboardHandle + )({ event, resolve }); + // Note: if an error is thrown by the route the following is skipped + // handleError is called instead and logging occurs there + if ( + // Don't enforce security checks on auth routes (handled by authRouteHandle before populateSecurityInfo) + !event.url.pathname.startsWith('/auth/') && + // @ts-expect-error securityHandled is not in the type definition + !event.locals.security.securityHandled + ) { + OTEL.instance.logger.error( + `Security checks were not performed for route ${event.url.pathname}`, + { + route: event.route.id, + method: event.request.method, + url: event.url.href, + userId: event.locals.security.userId ?? 'unauthenticated', + http_status_for_response: response.status + } + ); + span.recordException(new Error('Security checks were not performed')); + span.setStatus({ + code: SpanStatusCode.ERROR, + message: 'Security checks were not performed' }); - }, - bullboardHandle - )({ event, resolve }); - span.setAttributes({ - 'http.status_code': response.status - }); - span.end(); - return response; + span.setAttributes({ + 'http.status_code': 500 + }); + // This is a server error because a developer misconfigured the route + return new Response('Authorization misconfigured', { status: 500 }); + } + span.setAttributes({ + 'http.status_code': response.status + }); + return response; + } finally { + span.end(); + } }); }; diff --git a/src/lib/projects/components/ProjectActionMenu.svelte b/src/lib/projects/components/ProjectActionMenu.svelte index 48e004fd41..f1c7ed3825 100644 --- a/src/lib/projects/components/ProjectActionMenu.svelte +++ b/src/lib/projects/components/ProjectActionMenu.svelte @@ -68,7 +68,7 @@