Remove old HTTPS redirect kludges

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
 ==========
 

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 <meta> tag

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"
   },

src/config.js

@@ -56,8 +56,7 @@ var defaults = {
         passphrase: "",
         certfile: "localhost.cert",
         cafile: "",
-        ciphers: "HIGH:!DSS:!aNULL@STRENGTH",
+        ciphers: "HIGH:!DSS:!aNULL@STRENGTH"
-        redirect: true
     },
     io: {
         domain: "http://localhost",

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: "",

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;

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"),

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/<channel name>.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;

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="/account/channels") Channels
-        li: a(href=loginDomain+"/account/profile") Profile
+        li: a(href="/account/profile") Profile
-        li: a(href=loginDomain+"/account/edit") Change Password/Email
+        li: a(href="/account/edit") Change Password/Email
       else
-        li: a(href=loginDomain+"/login") Login
+        li: a(href="/login") Login
-        li: a(href=loginDomain+"/register") Register
+        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