diff --git a/NEWS.md b/NEWS.md index 31d6b32c..6535f187 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,19 @@ +2017-09-19 +========== + +This commit removes an old kludge that redirected users to HTTPS (when enabled) +specifically for the account authorization pages (e.g., `/login`). The code for +doing this was to work around limitations that no longer exist, and does not +represent current security best practices. + +The recommended solution to ensure that users are logged in securely (assuming +you've configured support for HTTPS) is to use +[Strict-Transport-Security](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) +to direct browsers to access the HTTPS version of the website at all times. You +can enable this by configuring a reverse proxy (e.g. nginx) in front of CyTube +to intercept HTTP traffic and redirect it to HTTPS, and add the +`Strict-Transport-Security` header when returning the response from CyTube. + 2017-07-22 ========== diff --git a/config.template.yaml b/config.template.yaml index ab0504a2..75c6010f 100644 --- a/config.template.yaml +++ b/config.template.yaml @@ -87,10 +87,6 @@ https: certfile: 'localhost.cert' cafile: '' ciphers: 'HIGH:!DSS:!aNULL@STRENGTH' - # Allow certain account pages to redirect to HTTPS if HTTPS is enabled. - # You may want to set this to false if you are reverse proxying HTTPS to a - # non-HTTPS address. - redirect: true # Page template values # title goes in the upper left corner, description goes in a tag diff --git a/package.json b/package.json index 537cd199..1bb6c863 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "author": "Calvin Montgomery", "name": "CyTube", "description": "Online media synchronizer and chat", - "version": "3.47.3", + "version": "3.48.0", "repository": { "url": "http://github.com/calzoneman/sync" }, diff --git a/src/config.js b/src/config.js index 46328e4e..29cbdf0d 100644 --- a/src/config.js +++ b/src/config.js @@ -56,8 +56,7 @@ var defaults = { passphrase: "", certfile: "localhost.cert", cafile: "", - ciphers: "HIGH:!DSS:!aNULL@STRENGTH", - redirect: true + ciphers: "HIGH:!DSS:!aNULL@STRENGTH" }, io: { domain: "http://localhost", diff --git a/src/web/account.js b/src/web/account.js index 7d12bc6a..6b18f375 100644 --- a/src/web/account.js +++ b/src/web/account.js @@ -23,10 +23,6 @@ let globalMessageBus; * Handles a GET request for /account/edit */ function handleAccountEditPage(req, res) { - if (webserver.redirectHttps(req, res)) { - return; - } - sendPug(res, "account-edit", {}); } @@ -178,10 +174,6 @@ function handleChangeEmail(req, res) { * Handles a GET request for /account/channels */ async function handleAccountChannelPage(req, res) { - if (webserver.redirectHttps(req, res)) { - return; - } - const user = await webserver.authorize(req); // TODO: error message if (!user) { @@ -349,10 +341,6 @@ async function handleDeleteChannel(req, res) { * Handles a GET request for /account/profile */ async function handleAccountProfilePage(req, res) { - if (webserver.redirectHttps(req, res)) { - return; - } - const user = await webserver.authorize(req); // TODO: error message if (!user) { @@ -462,10 +450,6 @@ async function handleAccountProfile(req, res) { * Handles a GET request for /account/passwordreset */ function handlePasswordResetPage(req, res) { - if (webserver.redirectHttps(req, res)) { - return; - } - sendPug(res, "account-passwordreset", { reset: false, resetEmail: "", diff --git a/src/web/auth.js b/src/web/auth.js index 1132d6b0..a4448898 100644 --- a/src/web/auth.js +++ b/src/web/auth.js @@ -111,10 +111,6 @@ function handleLogin(req, res) { * Handles a GET request for /login */ function handleLoginPage(req, res) { - if (webserver.redirectHttps(req, res)) { - return; - } - if (res.locals.loggedIn) { return sendPug(res, "login", { wasAlreadyLoggedIn: true @@ -158,10 +154,6 @@ function handleLogout(req, res) { * Handles a GET request for /register */ function handleRegisterPage(req, res) { - if (webserver.redirectHttps(req, res)) { - return; - } - if (res.locals.loggedIn) { sendPug(res, "register", {}); return; diff --git a/src/web/pug.js b/src/web/pug.js index 9c2dc452..639562e8 100644 --- a/src/web/pug.js +++ b/src/web/pug.js @@ -13,8 +13,6 @@ function merge(locals, res) { siteTitle: Config.get("html-template.title"), siteDescription: Config.get("html-template.description"), siteAuthor: "Calvin 'calzoneman' 'cyzon' Montgomery", - loginDomain: Config.get("https.enabled") ? Config.get("https.full-address") - : Config.get("http.full-address"), csrfToken: typeof res.req.csrfToken === 'function' ? res.req.csrfToken() : '', baseUrl: getBaseUrl(res), channelPath: Config.get("channel-path"), diff --git a/src/web/webserver.js b/src/web/webserver.js index 591fadce..c9b89817 100644 --- a/src/web/webserver.js +++ b/src/web/webserver.js @@ -60,23 +60,6 @@ function initPrometheus(app) { }); } -/** - * Redirects a request to HTTPS if the server supports it - */ -function redirectHttps(req, res) { - if (req.realProtocol !== 'https' && Config.get('https.enabled') && - Config.get('https.redirect')) { - var ssldomain = Config.get('https.full-address'); - if (ssldomain.indexOf(req.hostname) < 0) { - return false; - } - - res.redirect(ssldomain + req.path); - return true; - } - return false; -} - /** * Legacy socket.io configuration endpoint. This is being migrated to * /socketconfig/.json (see ./routes/socketconfig.js) @@ -280,8 +263,6 @@ module.exports = { initializeErrorHandlers(app); }, - redirectHttps: redirectHttps, - authorize: async function authorize(req) { if (!req.signedCookies || !req.signedCookies.auth) { return false; diff --git a/templates/nav.pug b/templates/nav.pug index 23a51fa2..03500e8c 100644 --- a/templates/nav.pug +++ b/templates/nav.pug @@ -16,12 +16,12 @@ mixin navdefaultlinks() if loggedIn li: a(href="javascript:$('#logoutform').submit();") Log out li.divider - li: a(href=loginDomain+"/account/channels") Channels - li: a(href=loginDomain+"/account/profile") Profile - li: a(href=loginDomain+"/account/edit") Change Password/Email + li: a(href="/account/channels") Channels + li: a(href="/account/profile") Profile + li: a(href="/account/edit") Change Password/Email else - li: a(href=loginDomain+"/login") Login - li: a(href=loginDomain+"/register") Register + li: a(href="/login") Login + li: a(href="/register") Register mixin navsuperadmin(newTab) if superadmin @@ -37,10 +37,8 @@ mixin navloginlogout() +navloginform() mixin navloginform() - if loginDomain == null - - loginDomain = "" .visible-lg - form#loginform.navbar-form.navbar-right(action=loginDomain+"/login", method="post") + form#loginform.navbar-form.navbar-right(action="/login", method="post") input(type="hidden", name="_csrf", value=csrfToken) .form-group input#username.form-control(type="text", name="name", placeholder="Username") @@ -54,7 +52,7 @@ mixin navloginform() button#login.btn.btn-default(type="submit") Login .visible-md p#loginform.navbar-text.pull-right - a#login.navbar-link(href=loginDomain+"/login") Log in + a#login.navbar-link(href="/login") Log in span  ·  a#register.navbar-link(href="/register") Register