diff --git a/index.js b/index.js index 0339181..e86a2b7 100644 --- a/index.js +++ b/index.js @@ -77,6 +77,11 @@ app.use("/plumbing", require("./plumbing")); // list, exactly like its user-facing routes do. app.param("structure_id", (req, res, next, id) => { if (!model.canAccessStructure(req.session.userId, id)) { + // Logged-out visitors get a chance to sign in and come back; logged-in + // non-members stay 404 so we never confirm the structure exists to them. + if (!req.session.userId && req.method === "GET") { + return res.redirect("/login?next=" + encodeURIComponent(req.originalUrl)); + } return res.status(404).send("Not found"); } next(); @@ -373,46 +378,61 @@ buildRoutes(); // | | _____| || |___ | | | | // |_______||_______||_______||___| |_| +// Only same-site absolute paths survive as post-login redirect targets, so a +// crafted ?next= can't bounce someone to another origin (open redirect). A +// leading `//` is protocol-relative and would escape our origin, so reject it. +function safeNext(next) { + if (typeof next !== "string" || !next.startsWith("/") || next.startsWith("//")) { + return null; + } + return next; +} + app.post("/register", async (req, res) => { + const next = safeNext(req.body.next); try { const { username, password } = req.body; const hashedPassword = await bcrypt.hash(password, 10); // 10 is the saltRounds const userId = model.createUser(username, hashedPassword); req.session.userId = userId; - res.redirect("/workshop"); + res.redirect(next || "/workshop"); } catch (e) { - return res.send(eta.render("auth/register", { error: e })); + return res.send(eta.render("auth/register", { error: e, next })); } }); app.post("/login", async (req, res) => { const { username, password } = req.body; + const next = safeNext(req.body.next); const user = model.getUser(username); if (user && (await bcrypt.compare(password, user.password))) { req.session.userId = user.id; - return res.redirect("/"); + return res.redirect(next || "/"); } return res.send( eta.render("auth/login", { error: "are you sure you entered that right?", + next, }), ); }); app.get("/register", async (req, res) => { + const next = safeNext(req.query.next); if (req.session.userId) { - return res.redirect("/"); + return res.redirect(next || "/"); } - return res.send(eta.render("auth/register", { error: null })); + return res.send(eta.render("auth/register", { error: null, next })); }); app.get("/login", async (req, res) => { + const next = safeNext(req.query.next); if (req.session.userId) { - return res.redirect("/"); + return res.redirect(next || "/"); } - return res.send(eta.render("auth/login", { error: null })); + return res.send(eta.render("auth/login", { error: null, next })); }); app.all("/logout", async (req, res) => { @@ -1068,8 +1088,13 @@ app.all("*", async (req, res) => { const route = routeMatch.route; // Private structures are invisible to uninvited users: 404, so a private - // route is indistinguishable from one that doesn't exist. + // route is indistinguishable from one that doesn't exist. Logged-out + // visitors on a GET get bounced to login and returned here afterward; + // logged-in non-members still 404. if (!model.canAccessStructure(req.session.userId, route.structure_id)) { + if (!req.session.userId && verb === "GET") { + return res.redirect("/login?next=" + encodeURIComponent(req.originalUrl)); + } return res.status(404).json({ success: false, message: "Path not found" }); } diff --git a/views/auth/login.eta b/views/auth/login.eta index 05bf6d2..2e19e76 100644 --- a/views/auth/login.eta +++ b/views/auth/login.eta @@ -1,12 +1,13 @@
+ <% if (it.next) { %><% } %> <% if (it.error) { %> - <%= error %> + <%= it.error %> <% } %>
-
or you can move in here. +
or you can move in here. diff --git a/views/auth/register.eta b/views/auth/register.eta index 0e33b8b..b26d4b8 100644 --- a/views/auth/register.eta +++ b/views/auth/register.eta @@ -1,4 +1,5 @@
+ <% if (it.next) { %><% } %>