Skip to content

Commit

Permalink
Merge pull request Neztore#19 from Neztore/1.2.0
Browse files Browse the repository at this point in the history
v1.2.0
- Adds a log out button (and the server side code, to boot)
- Makes the authorization cookie sameSite = lax
- Updates dependencies (Important for security!)
- Hides the x-powered-by header properly
- Enables eslint under `npm test` and makes all of the code conform to it.
- Fixes URL shortening from the ShareX client. Old config files should work: The web API url has changed.
  • Loading branch information
Neztore authored Dec 28, 2020
2 parents b3228f9 + 0a8dffd commit 82be66f
Show file tree
Hide file tree
Showing 16 changed files with 980 additions and 53 deletions.
853 changes: 852 additions & 1 deletion package-lock.json

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"name": "save-server",
"version": "1.1.0",
"description": "A ShareX server built on Express, Bulma and SQLite with User support.",
"version": "1.2.0",
"description": "A powerful ShareX image and URL server, with support for multiple users.",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1",
"test": "eslint index.js server/",
"start": "node ."
},
"bugs": "https://github.com/neztore/save-server/issues",
Expand All @@ -31,6 +31,7 @@
},
"devDependencies": {
"@types/express": "^4.17.3",
"@types/multer": "^1.4.2"
"@types/multer": "^1.4.2",
"eslint": "^7.16.0"
}
}
2 changes: 1 addition & 1 deletion server/api/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ files.post("/", auth.header, upload.array("files", 10), errorCatch(async functio
if (!req.user) {
return console.log("what??");
}
if (req.files.length !== 0) {
if (req.files && req.files.length !== 0) {
for (const file of req.files) {
db.addFile(file._tok, file._ext || undefined, req.user.username);
}
Expand Down
38 changes: 34 additions & 4 deletions server/api/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { errorCatch, errorGenerator, generateFileName, prettyError, getBase } = r
const { isURL, trim, isEmpty, isAlphanumeric } = require("validator");
const auth = require("../middleware/auth");
const db = require("../util/db");
const csrf = require("../middleware/csrf");

const validTag = (tag) => typeof tag === "string" && !isEmpty(tag) && isAlphanumeric(tag);

Expand All @@ -22,9 +23,9 @@ url.get("/:tag", errorCatch(async function (req, res, next) {
res.status(400).send(prettyError(400, "Invalid short URL tag."));
}
}));
url.use(auth);
// Add new URL Shortening
url.post("/", errorCatch(async function (req, res) {


async function processAddLink (req, res) {
if (!req.user) {
throw new Error("no user...?");
}
Expand All @@ -44,12 +45,41 @@ url.post("/", errorCatch(async function (req, res) {
} else {
return res.status(400).send(errorGenerator(400, "Bad request: Header \"shorten-url\" must be provided and be a url."));
}
}

// For the ShareX client
url.post("/", auth.header, errorCatch(processAddLink));

// Web routes (Auth + CSRF protection)
url.use(auth);
url.use(csrf);

// Add new URL Shortening
url.post("/web", errorCatch(async function (req, res) {
if (!req.user) {
throw new Error("no user...?");
}

const url = req.headers["shorten-url"];
if (url && typeof url === "string" && isURL(url)) {
// Validate tag
const providedTag = req.body.tag;

const isValid = providedTag && validTag(providedTag) && providedTag.length < 20 && providedTag.length > 2;
// Check for in use
const inUse = await db.getLink(providedTag);

const tag = (isValid && !inUse) ? providedTag : await generateFileName(6);
await db.addLink(tag, trim(url), req.user.username);
res.send({ success: true, url: `${getBase(req)}/u/${tag}` });
} else {
return res.status(400).send(errorGenerator(400, "Bad request: Header \"shorten-url\" must be provided and be a url."));
}
}));


// This is an API request, so it returns JSON.
url.delete("/:tag", errorCatch(async function (req, res) {
url.delete("/:tag", auth, errorCatch(async function (req, res) {
const { tag } = req.params;
if (validTag(tag)) {
const link = await db.getLink(tag);
Expand Down
27 changes: 23 additions & 4 deletions server/api/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const express = require("express");
const users = express.Router();
const auth = require("../middleware/auth");
const db = require("../util/db");
const { errorGenerator, errorCatch, generateToken, errors, dest, hashRounds, adminUser, getBase} = require("../util");
const { errorGenerator, errorCatch, generateToken, errors, dest, hashRounds, adminUser, getBase } = require("../util");
const { isLength, isAlphanumeric, isEmpty } = require("validator");
const { compare, hash } = require("bcrypt");
const fs = require("fs");
Expand All @@ -31,7 +31,8 @@ users.post("/login", errorCatch(async function (req, res) {
httpOnly: true,
/* A week */
expires: new Date(Date.now() + 604800000),
secure: req.secure || false
secure: req.secure || false,
sameSite: "Lax"
});
return res.send({ success: true, message: "Logged in." });
} else {
Expand All @@ -42,7 +43,8 @@ users.post("/login", errorCatch(async function (req, res) {
httpOnly: true,
/* A week */
expires: new Date(Date.now() + 604800000),
secure: req.secure || false
secure: req.secure || false,
sameSite: "Lax"
});
return res.send({ success: true, message: "Logged in." });
}
Expand Down Expand Up @@ -163,6 +165,7 @@ users.delete("/:id", errorCatch(async function (req, res, next) {
res.send({ success: true, message: "User deleted." });
}));


users.get("/:id/config", errorCatch(async function (req, res) {
// Generate config
const isLink = req.query.link && req.query.link === "true";
Expand Down Expand Up @@ -239,12 +242,28 @@ users.patch("/:id/token", errorCatch(async function (req, res) {
httpOnly: true,
/* A week */
expires: new Date(Date.now() + 604800000),
secure: req.secure || false
secure: req.secure || false,
sameSite: "Lax"
});
}
return res.send({ success: true, token: newToken });
}));


users.post("/:id/logout", errorCatch(async function (req, res) {
await db.expireToken(req.target.username);
if (req.user.username === req.target.username) {
res.cookie("authorization", "", {
httpOnly: true,
/* Expired */
expires: new Date(Date.now() - 604800000),
secure: req.secure || false,
sameSite: "Lax"
});
}
return res.send({ success: true });
}));

users.get("/:id/files", errorCatch(async function (req, res) {
const files = await db.getUserFiles(req.target.username);
res.send({ success: true, files });
Expand Down
4 changes: 2 additions & 2 deletions server/client/js/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Api.delete = function (url, options) {
console.error(e);
}
};
const protectedMethods = ["post", "patch", "delete", "put"]
const protectedMethods = ["post", "patch", "delete", "put"];
Api._makeRequest = async function (url, options) {
const startChar = url.substr(0, 1);
options.credentials = "include";
Expand All @@ -53,7 +53,7 @@ Api._makeRequest = async function (url, options) {
}

if (protectedMethods.includes(options.method.toLowerCase())) {
options.headers["CSRF-Token"] = getCookie("CSRF-Token");
options.headers["CSRF-Token"] = window.getCookie("CSRF-Token");
}
url = (startChar === "/") ? `${ApiUrl}${url}` : `/${url}`;
const req = await fetch(url, options);
Expand Down
10 changes: 5 additions & 5 deletions server/client/js/document.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
document.addEventListener("DOMContentLoaded", (event) => {
document.addEventListener("DOMContentLoaded", () => {
const showRender = document.getElementById("show-render");
const showCode = document.getElementById("show-code");
const markRender = document.getElementById("markdown-render");
Expand All @@ -8,7 +8,7 @@ document.addEventListener("DOMContentLoaded", (event) => {
if (deleteButton) {
deleteButton.onclick = function () {
deleteButton.classList.add("is-loading");
deleteFile({ id: fileName }, function (deleted) {
window.deleteFile({ id: window.fileName }, function (deleted) {
deleteButton.classList.remove("is-loading");
if (deleted) {
document.location = "/dashboard";
Expand All @@ -33,13 +33,13 @@ document.addEventListener("DOMContentLoaded", (event) => {
code.classList.remove("hidden");
};

markRender.innerHTML = marked(markRender.innerHTML);
markRender.innerHTML = window.marked(markRender.innerHTML);
}
hljs.configure({
window.hljs.configure({
tabReplace: " ", // 4 spaces
});

document.querySelectorAll("code").forEach((block) => {
hljs.highlightBlock(block);
window.hljs.highlightBlock(block);
});
});
2 changes: 1 addition & 1 deletion server/client/js/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ document.addEventListener("DOMContentLoaded", function () {
const username = fields.username.value;
const password = fields.password.value;
if (username && password && username !== "" && password !== "") {
const res = await Api.post("/users/login", {
const res = await window.Api.post("/users/login", {
body: {
username,
password
Expand Down
19 changes: 17 additions & 2 deletions server/client/js/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function deleteFile (fileInfo, cb) {
const sure = confirm(`Are you sure you want to delete ${str}?`);
if (sure) {
// Delete the file
Api.delete(`/files/${fileInfo.id}`)
window.Api.delete(`/files/${fileInfo.id}`)
.then((res) => {
if (res.success) {
if (cb) cb(true);
Expand All @@ -55,6 +55,20 @@ function deleteFile (fileInfo, cb) {
if (cb) cb();
}
}
window.deleteFile = deleteFile;

function logout () {
window.Api.post("/users/@me/logout")
.then((res) => {
if (res.success) {
document.location.href = "/";
} else {
showError(res.error);
}
})
.catch(showError);
}
window.logout = logout;

// We create dynamically because it's easier than ensuring the HTML code exists on every page.
function showMessage (headerContent, content, colour, closeAfter, closeCb) {
Expand Down Expand Up @@ -121,8 +135,9 @@ function showError (error) {
function getCookie(name) {
const value = `; ${document.cookie}`;
const parts = value.split(`; ${name}=`);
if (parts.length === 2) return parts.pop().split(';').shift();
if (parts.length === 2) return parts.pop().split(";").shift();
}
window.getCookie = getCookie;

document.addEventListener("DOMContentLoaded", () => {
// Get all "navbar-burger" elements
Expand Down
2 changes: 1 addition & 1 deletion server/client/js/short.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ document.addEventListener("DOMContentLoaded", function () {
e.preventDefault();
const targetUrl = fields.target.value;
if (targetUrl && targetUrl !== "") {
const res = await Api.post("/links/", {
const res = await window.Api.post("/links/web", {
headers: {
"shorten-url": targetUrl
},
Expand Down
6 changes: 5 additions & 1 deletion server/client/pages/dashboard.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,11 @@
</div>
<div class="level-right">
<div class="level-item">
<button class="button is-success" id="create-user">Create user</button>
<div class="buttons">
<button class="button" onclick="logout()">Log out</button>
<button class="button is-success" id="create-user">Create user</button>
</div>

</div>
</div>
</nav>
Expand Down
7 changes: 4 additions & 3 deletions server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ app.set("view engine", "ejs");
app.enable("trust proxy");
app.use(bodyParser.json());
app.use(cookie());
app.set("x-powered-by", "false");
app.set("x-powered-by", false);


// Client
Expand All @@ -33,9 +33,10 @@ app.use("/favicon.ico", express.static(path.join(client, "favicon.ico")));

// Routes
app.use("/api/files", files.router);
app.use(csrf)
app.use("/api/users", users);
app.use("/api/links", links);

app.use(csrf);
app.use("/api/users", users);
app.use("/api/links", links);
app.use("/u", links);

Expand Down
5 changes: 3 additions & 2 deletions server/middleware/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { isLength, isAscii } = require("validator");

async function checkToken (req, useCookie) {
let authorization = useCookie ? req.cookies.authorization : req.headers.authorization;
if (!authorization || !isAscii(authorization)) {
if (!authorization || typeof authorization !== "string" || !isAscii(authorization)) {
return false;
} else {
authorization = decodeURIComponent(authorization);
Expand Down Expand Up @@ -52,6 +52,7 @@ module.exports.header = async function checkAuth (req, res, next) {
if (await checkToken(req)) {
next();
} else {
res.redirect("/login");
res.status(errors.unauthorized.error.status);
return res.send(errors.unauthorized);
}
};
44 changes: 22 additions & 22 deletions server/middleware/csrf.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@ const { generateToken, errorGenerator } = require("../util");

const protectedMethods = ["post", "patch", "put", "delete"];
module.exports = async function (req, res, next) {
function fail () {
return res.status(400).send(errorGenerator(400, "Failed CSRF Token validation"));
}
if (protectedMethods.includes(req.method.toLowerCase())) {
// Validate CSRF presence
if (req.cookies["CSRF-Token"] && req.get("CSRF-Token")) {
if (req.cookies["CSRF-Token"] === decodeURIComponent(req.get("CSRF-Token"))) {
return next();
}
}
return fail();
} else {
// It's a get
if (!req.cookies["CSRF-Token"]) {
res.cookie("CSRF-Token", await generateToken(), {
maxAge: 172800000,
sameSite: "strict",
httpOnly: false
});
}
return next();
}
function fail () {
return res.status(400).send(errorGenerator(400, "Failed CSRF Token validation"));
}
if (protectedMethods.includes(req.method.toLowerCase())) {
// Validate CSRF presence
if (req.cookies["CSRF-Token"] && req.get("CSRF-Token")) {
if (req.cookies["CSRF-Token"] === decodeURIComponent(req.get("CSRF-Token"))) {
return next();
}
}
return fail();
} else {
// It's a get
if (!req.cookies["CSRF-Token"]) {
res.cookie("CSRF-Token", await generateToken(), {
maxAge: 172800000,
sameSite: "strict",
httpOnly: false
});
}
return next();
}
};
4 changes: 4 additions & 0 deletions server/util/db.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ class Database extends sqlite.Database {
return this.getLots(SQL, [username]);
}
getUserByToken (token) {
// Just to double check, ensure token is defined.
if (!token || typeof token !== "string") {
throw new Error("Illegal authorisation token!");
}
const SQL = "SELECT username, token FROM users WHERE token = $1 LIMIT 1;";
return this.getOne(SQL, [token]);
}
Expand Down
1 change: 1 addition & 0 deletions server/util/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const ejs = require("ejs");
const path = require("path");

// "Unfriendly" error messages.
// eslint-disable-next-line no-unused-vars
function errorHandler (error, req, res, _next) {
if (error instanceof SyntaxError) {
res.status(400).send(errorGenerator(400, "Bad JSON."));
Expand Down

0 comments on commit 82be66f

Please sign in to comment.