diff --git a/README.md b/README.md index d6aeeb20..309cac5e 100644 --- a/README.md +++ b/README.md @@ -1 +1,19 @@ -# yh-message-app-fullstack \ No newline at end of file +# Säker systemutveckling + +Det här projektet är en del av kursen Säker systemutveckling och bygger på en redan existerande fullstack-applikation för meddelanden som finns på GitHub. +Applikationen låter användaren: + +- Skapa ett konto +- Logga in +- Skapa meddelanden +- Redigera meddelanden +- Ta bort meddelanden + +Syftet med projektet är inte att bygga en helt ny applikation från grunden, utan istället att analysera och förstå en existerande applikation ur ett säkerhetsperspektiv. +Uppgiften är uppdelad i tre faser: + +1.Planering + +2.Kodförståelse + +3.Säkerhetsgranskning diff --git a/backend/server.js b/backend/server.js index c8d0c218..286aa77a 100644 --- a/backend/server.js +++ b/backend/server.js @@ -1,59 +1,82 @@ -import "dotenv/config" -import helmet from "helmet" -import cors from "cors" -import express from "express" -import mongoose from "mongoose" -import bcrypt from "bcrypt" -import jwt from "jsonwebtoken" -import { Message } from "./models/Message.js" -import { User } from "./models/User.js" -import { authenticateUser } from "./middleware/auth.js" -import "./config/db.js" -import listEndpoints from "express-list-endpoints" - -if (!process.env.JWT_SECRET) throw new Error("JWT_SECRET is not set in .env") - -const PORT = process.env.PORT || "3000" -const app = express() -app.use(helmet()) -app.use(cors({ - origin: "*", -})) -app.use(express.json()) +import "dotenv/config"; +import helmet from "helmet"; +import cors from "cors"; +import express from "express"; +import mongoose from "mongoose"; +import bcrypt from "bcrypt"; +import jwt from "jsonwebtoken"; +import { Message } from "./models/Message.js"; +import { User } from "./models/User.js"; +import { authenticateUser } from "./middleware/auth.js"; +import "./config/db.js"; +import listEndpoints from "express-list-endpoints"; + +if (!process.env.JWT_SECRET) throw new Error("JWT_SECRET is not set in .env"); + +const PORT = process.env.PORT || "3000"; +const app = express(); +app.use(helmet()); +app.use( + cors({ + origin: "*", + }), +); +app.use(express.json()); app.get("/", (req, res) => { - res.send(listEndpoints(app)) -}) + res.send(listEndpoints(app)); +}); app.post("/register", async (req, res) => { try { - const { email, password, username } = req.body + const { email, password, username } = req.body; + + // SECURITY FIX: + // Passwords should not be too weak. + // This checks minimum length and requires letters and numbers. + const passwordRegex = /^(?=.*[A-Za-z])(?=.*\d).{8,}$/; + + if (!password || !passwordRegex.test(password)) { + return res.status(400).json({ + success: false, + message: + "Password must be at least 8 characters and include both letters and numbers", + }); + } if (!username || username.trim().length < 2) { - return res.status(400).json({ success: false, message: "Username must be at least 2 characters" }) + return res.status(400).json({ + success: false, + message: "Username must be at least 2 characters", + }); } const existingUser = await User.findOne({ - $or: [{ email: email.toLowerCase() }, { username: username.trim() }] - }) + $or: [{ email: email.toLowerCase() }, { username: username.trim() }], + }); if (existingUser) { - const field = existingUser.email === email.toLowerCase() ? "email" : "username" + const field = + existingUser.email === email.toLowerCase() ? "email" : "username"; return res.status(400).json({ success: false, - message: `A user with this ${field} already exists` - }) + message: `A user with this ${field} already exists`, + }); } - const hashedPassword = await bcrypt.hash(password, 10) - const user = new User({ username: username.trim(), email, password: hashedPassword }) - await user.save() + const hashedPassword = await bcrypt.hash(password, 10); + const user = new User({ + username: username.trim(), + email, + password: hashedPassword, + }); + await user.save(); const accessToken = jwt.sign( { userId: user._id, username: user.username }, process.env.JWT_SECRET, - { expiresIn: "2h" } - ) + { expiresIn: "2h" }, + ); res.status(201).json({ success: true, @@ -63,45 +86,45 @@ app.post("/register", async (req, res) => { id: user._id, accessToken, }, - }) + }); } catch (error) { res.status(400).json({ success: false, message: "Could not create user", error: error, - }) + }); } -}) +}); app.post("/login", async (req, res) => { try { - const { login, password } = req.body + const { login, password } = req.body; const user = await User.findOne({ - $or: [{ username: login }, { email: login }] - }) + $or: [{ username: login }, { email: login }], + }); if (!user) { return res.status(401).json({ success: false, message: "No account found with that username or email", response: null, - }) + }); } - const passwordMatch = await bcrypt.compare(password, user.password) + const passwordMatch = await bcrypt.compare(password, user.password); if (!passwordMatch) { return res.status(401).json({ success: false, message: "Password is incorrect", response: null, - }) + }); } const accessToken = jwt.sign( { userId: user._id, username: user.username }, process.env.JWT_SECRET, - { expiresIn: "2h" } - ) + { expiresIn: "2h" }, + ); res.json({ success: true, @@ -111,17 +134,17 @@ app.post("/login", async (req, res) => { id: user._id, accessToken, }, - }) + }); } catch (error) { res.status(500).json({ success: false, message: "Something went wrong", error: error, - }) + }); } -}) +}); -const isValidId = (id) => mongoose.Types.ObjectId.isValid(id) +const isValidId = (id) => mongoose.Types.ObjectId.isValid(id); app.get("/messages", async (req, res) => { try { @@ -129,54 +152,86 @@ app.get("/messages", async (req, res) => { .sort({ createdAt: "desc" }) .limit(20) .populate("user", "username") - .exec() - res.json(messages) + .exec(); + res.json(messages); } catch (error) { - res.status(500).json({ message: "Could not fetch messages" }) + res.status(500).json({ message: "Could not fetch messages" }); } -}) +}); app.post("/messages", authenticateUser, async (req, res) => { - const message = new Message({ message: req.body.message, user: req.user._id }) + const { message } = req.body; + + // SECURITY FIX: + // Validate user input before saving it to the database. + // This prevents empty messages and limits very long input. + if (typeof message !== "string") { + return res.status(400).json({ error: "Message must be text" }); + } + + const trimmedMessage = message.trim(); + + if (trimmedMessage.length < 1 || trimmedMessage.length > 200) { + return res.status(400).json({ + error: "Message must be between 1 and 200 characters", + }); + } + + const newMessage = new Message({ + message: trimmedMessage, + user: req.user._id, + }); + try { - const saved = await message.save() - res.status(201).json(saved) + const saved = await newMessage.save(); + res.status(201).json(saved); } catch (err) { - res.status(400).json({ message: "Could not save message", errors: err.errors }) + res + .status(400) + .json({ message: "Could not save message", errors: err.errors }); } -}) +}); app.patch("/messages/:id", authenticateUser, async (req, res) => { - if (!isValidId(req.params.id)) return res.status(400).json({ error: "Invalid message ID" }) + if (!isValidId(req.params.id)) + return res.status(400).json({ error: "Invalid message ID" }); try { - const message = await Message.findById(req.params.id) - if (!message) return res.status(404).json({ error: "Message not found" }) + const message = await Message.findById(req.params.id); + if (!message) return res.status(404).json({ error: "Message not found" }); if (message.user.toString() !== req.user._id.toString()) { - return res.status(403).json({ error: "You can only edit your own messages" }) + return res + .status(403) + .json({ error: "You can only edit your own messages" }); } - message.message = req.body.editedMessage - await message.save() - const updated = await message.populate("user", "username") - res.json(updated) + message.message = req.body.editedMessage; + await message.save(); + const updated = await message.populate("user", "username"); + res.json(updated); } catch (error) { - res.status(400).json({ error: "Could not update message" }) + res.status(400).json({ error: "Could not update message" }); } -}) +}); -app.delete("/messages/:id", async (req, res) => { - if (!isValidId(req.params.id)) return res.status(400).json({ error: "Invalid message ID" }) +app.delete("/messages/:id", authenticateUser, async (req, res) => { + if (!isValidId(req.params.id)) + return res.status(400).json({ error: "Invalid message ID" }); try { - const message = await Message.findById(req.params.id) - if (!message) return res.status(404).json({ error: "Message not found" }) - await message.deleteOne() - res.status(204).send() + const message = await Message.findById(req.params.id); + if (!message) return res.status(404).json({ error: "Message not found" }); + if (message.user.toString() !== req.user._id.toString()) { + return res + .status(403) + .json({ error: "You can only delete your own messages" }); + } + await message.deleteOne(); + res.status(204).send(); } catch (error) { - res.status(400).json({ error: "Could not delete message" }) + res.status(400).json({ error: "Could not delete message" }); } -}) +}); app.listen(PORT, () => { - console.log(`Listening on port ${PORT}`) -}) + console.log(`Listening on port ${PORT}`); +}); diff --git a/frontend/src/api.js b/frontend/src/api.js index ee6f3531..a3a60b4a 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -1 +1,2 @@ -export const BASE_URL = "https://yh-message-app-fullstack.onrender.com" +//export const BASE_URL = "https://yh-message-app-fullstack.onrender.com" +export const BASE_URL = "http://localhost:3000"; diff --git a/frontend/src/components/PostMessage.jsx b/frontend/src/components/PostMessage.jsx index ba71b001..678511e2 100644 --- a/frontend/src/components/PostMessage.jsx +++ b/frontend/src/components/PostMessage.jsx @@ -1,14 +1,19 @@ -import { useState } from "react" -import { BASE_URL } from "../api" +import { useState } from "react"; +import { BASE_URL } from "../api"; -export const PostMessage = ({ newMessage, fetchPosts, user, onUnauthorized }) => { - const [newPost, setNewPost] = useState("") - const [errorMessage, setErrorMessage] = useState("") - const [submitting, setSubmitting] = useState(false) +export const PostMessage = ({ + newMessage, + fetchPosts, + user, + onUnauthorized, +}) => { + const [newPost, setNewPost] = useState(""); + const [errorMessage, setErrorMessage] = useState(""); + const [submitting, setSubmitting] = useState(false); const handleFormSubmit = async (event) => { - event.preventDefault() - setSubmitting(true) + event.preventDefault(); + setSubmitting(true); try { const res = await fetch(`${BASE_URL}/messages`, { @@ -18,38 +23,38 @@ export const PostMessage = ({ newMessage, fetchPosts, user, onUnauthorized }) => Authorization: `Bearer ${user?.response?.accessToken}`, }, body: JSON.stringify({ message: newPost }), - }) + }); - console.log("Token being sent:", user?.response?.accessToken) + console.log("Token being sent:", user?.response?.accessToken); if (res.status === 401) { - onUnauthorized() - setSubmitting(false) - return + onUnauthorized(); + setSubmitting(false); + return; } - const data = await res.json() + const data = await res.json(); - if (data.message && !data._id) { - console.log(data) - setErrorMessage(data.message) - setSubmitting(false) - return + if (!res.ok) { + console.log(data); + setErrorMessage(data.error || data.message || "Something went wrong"); + setSubmitting(false); + return; } - newMessage(data) - setNewPost("") - setErrorMessage("") - setSubmitting(false) - await fetchPosts() + newMessage(data); + setNewPost(""); + setErrorMessage(""); + setSubmitting(false); + await fetchPosts(); } catch (error) { - console.error(error) - setSubmitting(false) + console.error(error); + setSubmitting(false); } - } + }; if (!user) { - return

Log in to write a message

+ return

Log in to write a message

; } return ( @@ -62,11 +67,13 @@ export const PostMessage = ({ newMessage, fetchPosts, user, onUnauthorized }) => placeholder="Write your message here..." value={newPost} onChange={(e) => { - setNewPost(e.target.value) - setErrorMessage("") + setNewPost(e.target.value); + setErrorMessage(""); }} /> -

{errorMessage}

+

+ {errorMessage} +

- ) -} + ); +}; diff --git a/frontend/src/components/SingleMessage.jsx b/frontend/src/components/SingleMessage.jsx index 50d2274b..7df73a5c 100644 --- a/frontend/src/components/SingleMessage.jsx +++ b/frontend/src/components/SingleMessage.jsx @@ -1,12 +1,17 @@ -import { useState } from "react" -import { BASE_URL } from "../api" +import { useState } from "react"; +import { BASE_URL } from "../api"; -export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) => { - const [isEditing, setIsEditing] = useState(false) - const [editedText, setEditedText] = useState(message.message) - const [editError, setEditError] = useState("") +export const SingleMessage = ({ + message, + user, + onUnauthorized, + fetchPosts, +}) => { + const [isEditing, setIsEditing] = useState(false); + const [editedText, setEditedText] = useState(message.message); + const [editError, setEditError] = useState(""); - const isOwner = user && user.response.id === message.user?._id + const isOwner = user && user.response.id === message.user?._id; const onDelete = async () => { try { @@ -15,18 +20,28 @@ export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) => headers: { Authorization: `Bearer ${user?.response?.accessToken}`, }, - }) + }); if (res.status === 401) { - onUnauthorized() - return + onUnauthorized(); + return; } - await fetchPosts() + if (res.status === 403) { + alert("🤠 Whoa there cowboy! You can only delete your own messages."); + return; + } + + if (!res.ok) { + alert("Something went wrong when deleting the message."); + return; + } + + await fetchPosts(); } catch (error) { - console.error(error) + console.error(error); } - } + }; const onSave = async () => { try { @@ -37,28 +52,28 @@ export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) => Authorization: `Bearer ${user?.response?.accessToken}`, }, body: JSON.stringify({ editedMessage: editedText }), - }) + }); if (res.status === 401) { - onUnauthorized() - return + onUnauthorized(); + return; } - const data = await res.json() + const data = await res.json(); if (data.error) { - console.log(data) - setEditError(data.error) - return + console.log(data); + setEditError(data.error); + return; } - setIsEditing(false) - setEditError("") - await fetchPosts() + setIsEditing(false); + setEditError(""); + await fetchPosts(); } catch (error) { - console.error(error) + console.error(error); } - } + }; return (
@@ -73,8 +88,8 @@ export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) => rows="3" value={editedText} onChange={(event) => { - setEditedText(event.target.value) - setEditError("") + setEditedText(event.target.value); + setEditError(""); }} />

{editError}

@@ -83,14 +98,24 @@ export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) => )}
- + {isOwner && !isEditing && ( - + )} {isOwner && isEditing && ( - + )} {isOwner && isEditing && ( @@ -98,8 +123,8 @@ export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) => type="button" className="cancel-btn" onClick={() => { - setIsEditing(false) - setEditError("") + setIsEditing(false); + setEditError(""); }} > ❌ @@ -112,5 +137,5 @@ export const SingleMessage = ({ message, user, onUnauthorized, fetchPosts }) =>
{message.user?.username || ""}
- ) -} + ); +}; diff --git a/frontend/src/main.jsx b/frontend/src/main.jsx index 78b5d6f4..9827b37d 100644 --- a/frontend/src/main.jsx +++ b/frontend/src/main.jsx @@ -1,5 +1,5 @@ -import { createRoot } from "react-dom/client" -import "./index.css" -import { App } from "./App.jsx" +import { createRoot } from "react-dom/client"; +import "./index.css"; +import { App } from "./App.jsx"; -createRoot(document.getElementById("root")).render() +createRoot(document.getElementById("root")).render(); diff --git a/granskningsfasen.md b/granskningsfasen.md index 088f9718..80a2ba89 100644 --- a/granskningsfasen.md +++ b/granskningsfasen.md @@ -1 +1,45 @@ -# Inlämning 3 - Granskningsfasen \ No newline at end of file +# Phase 3 – Review + +## Tools Used + +- npm audit for dependency analysis, directly in terminal of the VS code, phase 2. of SDLC (code) +- Dependabot for continous dependency monitoring in GitHub, phase 6. of SDLC (operate) +- CodeQL for static code analysis + +## Findings + +### Vulnerable Dependencies + +Dependabot identified 16 dependency vulnerabilities, including issues in jsonwebtoken, tar, qs, vite and esbuild. + +OWASP: A06 – Vulnerable and Outdated Components + +### Missing Rate Limiting + +CodeQL identified endpoints performing database operations without rate limiting. + +OWASP: A04 – Insecure Design + +### Permissive CORS Configuration + +CodeQL identified a permissive CORS configuration using origin: "\*". + +OWASP: A05 – Security Misconfiguration + +## Recommendations + +- Upgrade vulnerable dependencies +- Implement rate limiting +- Restrict CORS to trusted domains +- Continue dependency monitoring using Dependabot + +## Conclusion + +The review identified several security findings, including vulnerable dependencies, missing rate limiting and permissive CORS configuration. However, no findings were identified that immediately compromise the application's confidentiality, integrity or availability. + +The review highlighted areas where the security of the application can be improved, particularly through dependency updates, rate limiting and configuration hardening. +Different security review methods found different problems. No single tool found everything. That is why its important to use multiple approaches in secure development. + +## Presentation + +[Phase 3 Presentation (Canva)](https://canva.link/a0tqjpfpleicqdw) diff --git a/images/slide1.PNG b/images/slide1.PNG new file mode 100644 index 00000000..6706512a Binary files /dev/null and b/images/slide1.PNG differ diff --git a/images/slide2.PNG b/images/slide2.PNG new file mode 100644 index 00000000..2b21a116 Binary files /dev/null and b/images/slide2.PNG differ diff --git a/images/slide3.png b/images/slide3.png new file mode 100644 index 00000000..6fd90dd9 Binary files /dev/null and b/images/slide3.png differ diff --git a/planeringsfasen.md b/planeringsfasen.md index 661cae0e..e2071a67 100644 --- a/planeringsfasen.md +++ b/planeringsfasen.md @@ -1 +1,20 @@ -# Inlämning 1 - Planeringsfasen \ No newline at end of file +# Inlämning 1 - Planeringsfasen + +## App Flow Overview + +![App Flow](images/slide1.PNG) + +Den här sliden visar hur applikationen är uppbyggd och hur data rör sig genom systemet. Användaren interagerar med applikationen genom browsern, frontend skickar requests till backend/API och backend kommunicerar med databasen. Att förstå app flow hjälper till att identifiera var säkerhetshot och sårbarheter kan uppstå. + +## STRIDE Threat Modeling + +![STRIDE](images/slide2.PNG) + +Den här sliden visar möjliga hot i olika delar av applikationen med hjälp av STRIDE-modellen. Syftet är att identifiera vanliga säkerhetsrisker som spoofing, tampering, information disclosure och elevation of privilege. Threat modeling hjälper till att koppla säkerhetsrisker till specifika delar av systemarkitekturen. +Jag valde att applicera STRIDE på applikationens komponenter istället för direkt på dataflödena, för att göra hotmodellen tydligare och enklare att följa. + +## Threat Modeling and Security Requirements + +![Threat Modeling Table](images/slide3.png) + +Den här sliden kopplar identifierade hot till konkreta säkerhetskrav. Målet är att minska risker genom att definiera hur applikationen ska hantera authentication, authorization, input validation, password security och dependency management. Säkerhetskraven är baserade på de hot som identifierades under planeringsfasen.