Respond with 404 (not 401) for requests to '/'

Ideally, a request to an unknown route would receive a 404. However,
for security reasons, we've chosen to require authentication for all
requests _except_ explicitly whitelisted routes that don't need
authentication (i.e., public routes). Because of that, any request that
doesn't match a whitelisted route currently gets a 401 response if the
request lacks valid credentials. This works well for routes that need
authentication (e.g., `GET /portals`), but it results in less-than-ideal
behavior for requests to nonexistent routes (e.g., `GET /`,
`POST /foo`). Requests to nonexistent routes _should_ receive a 404
response, but they currently receive a 401 response.

We get quite a few requests to '/', despite the fact that it's not a
valid route. Today, those requests get a 401 response if the request
lacks valid credentials. With the change in this commit, those requests
will instead receive a 404 response, which is more semantic and is more
likely to help the requester understand that '/' is not a valid route.

Admittedly, this change only helps requests to `/`, and does not help
requests to any other unknown route (`GET /foo`). Since requests to `/`
represent the overwhelming majority of requests to unknown routes, this
change will help in _most_ of the cases that we see in production.

We could implement a solution that works for all unknown routes, but the
solutions that I know of all have problematic tradeoffs:

- Apply the authentication middleware to each secure route instead of
  applying it to all routes by default. Consequence => This is less
  future-proof from a security standpoint. When we add a new endpoint,
  we need to remember to secure it, as opposed to having it be secure by
  default.
- Teach the authentication middleware to determine all known routes and
  only respond with a 401 if the request lacks valid authentication
  *and* is a known route. Consequence => Determining all known routes
  requires additional computation, and it's more code to maintain.

Author: jasonrudolph

Diff for: lib/controller-layer.js

@@ -13,7 +13,7 @@ module.exports = ({modelLayer, pubSubGateway, identityProvider, fetchICEServers,
   app.use(cors())
   app.use(bodyParser.json({limit: '1mb'}))
   app.use(enforceProtocol)
-  app.use(authenticate({identityProvider, ignoredPaths: ['/protocol-version', '/boomtown', '/_ping']}))
+  app.use(authenticate({identityProvider, ignoredPaths: ['/', '/protocol-version', '/boomtown', '/_ping']}))
   app.use(bugsnag.requestHandler)
 
   app.get('/protocol-version', function (req, res) {