feat(Plugs.SecureHeaders): add secure headers to errors and static fi…

…les (#2216)

* chore(ControllerHelpers): consolidate rendering 404s and 500s

* feat(Plugs.SecureHeaders): add even more headers

* feat(DotcomWeb.Router): handle errors with secure headers

* reorg

* typo

* avoid compile vs runtime check

config/config.exs

@@ -4,6 +4,7 @@ config :elixir, ansi_enabled: true
 
 config :dotcom, :aws_client, AwsClient.Behaviour
 
+config :dotcom, :content_security_policy_definition, ""
 config :dotcom, :cms_api_module, CMS.Api
 
 config :dotcom, :httpoison, HTTPoison

config/runtime.exs

@@ -144,6 +144,33 @@ end
 if config_env() == :prod do
   config :dotcom, alerts_bus_stop_change_bucket: System.get_env("S3_PREFIX_BUSCHANGE")
 
+  # Extract the host fron the sentry dsn
+  sentry_dsn_host =
+    case Regex.run(~r/@(.*)\//, System.get_env("SENTRY_DSN", ""), capture: :all_but_first) do
+      nil -> ""
+      [match | _] -> match
+    end
+
+  static_host = System.get_env("STATIC_HOST", "")
+
+  config :dotcom,
+         :content_security_policy_definition,
+         DotcomWeb.Plugs.SecureHeaders.base_csp_directives()
+         |> Enum.map(fn
+           {:connect, directive} ->
+             directive ++ ["wss://#{host}", sentry_dsn_host]
+
+           {:img, directive} ->
+             directive ++ [static_host, System.get_env("CMS_API_BASE_URL", "")]
+
+           {key, directive} when key in [:font, :script, :style] ->
+             directive ++ [static_host]
+
+           {_, directive} ->
+             directive
+         end)
+         |> Enum.map_join("; ", &Enum.join(&1, " "))
+
   config :dotcom, DotcomWeb.Endpoint,
     http: [
       # Enable IPv6 and bind on all interfaces.
@@ -205,42 +232,3 @@ if System.get_env("LOGGER_LEVEL") in ~w(emergency alert critical error warning n
   config :logger, level: String.to_atom(System.get_env("LOGGER_LEVEL"))
   config :logger, :console, level: String.to_atom(System.get_env("LOGGER_LEVEL"))
 end
-
-# Extract the host fron the sentry dsn
-sentry_dsn_host =
-  case Regex.run(~r/@(.*)\//, System.get_env("SENTRY_DSN", ""), capture: :all_but_first) do
-    nil -> ""
-    [match | _] -> match
-  end
-
-# Set the content security policy
-case config_env() do
-  :prod ->
-    config :dotcom,
-           :content_security_policy_definition,
-           Enum.join(
-             [
-               "default-src 'none'",
-               "img-src 'self' cdn.mbta.com #{System.get_env("STATIC_HOST", "")} #{System.get_env("CMS_API_BASE_URL", "")} px.ads.linkedin.com www.linkedin.com www.facebook.com *.google.com *.googleapis.com *.gstatic.com *.s3.amazonaws.com data: i.ytimg.com www.googletagmanager.com *.arcgis.com",
-               "style-src 'self' 'unsafe-inline' www.gstatic.com #{System.get_env("STATIC_HOST", "")} cdn.jsdelivr.net",
-               "script-src 'self' 'unsafe-eval' 'unsafe-inline' #{System.get_env("STATIC_HOST", "")} insitez.blob.core.windows.net snap.licdn.com connect.facebook.net www.instagram.com www.google-analytics.com *.google.com www.gstatic.com www.googletagmanager.com *.googleapis.com data.mbta.com *.arcgis.com",
-               "font-src 'self' #{System.get_env("STATIC_HOST", "")}",
-               "connect-src 'self' wss://#{host} #{sentry_dsn_host || ""} *.googleapis.com analytics.google.com www.google-analytics.com www.google.com px.ads.linkedin.com stats.g.doubleclick.net *.arcgis.com *.s3.amazonaws.com",
-               "frame-src 'self' data.mbta.com www.youtube.com www.google.com cdn.knightlab.com livestream.com www.instagram.com *.arcgis.com",
-               "worker-src blob: ;"
-             ],
-             "; "
-           )
-
-  # Dev is only used for local development, so we don't need, and in
-  # fact actively do not want, a restrictive CSP
-  :dev ->
-    config :dotcom, :content_security_policy_definition, ""
-
-  :test ->
-    config :dotcom, :content_security_policy_definition, ""
-
-  # Unknown env, reject all
-  _ ->
-    config :dotcom, :content_security_policy_definition, "default-src 'none'"
-end

lib/dotcom_web/controllers/cms_controller.ex

@@ -92,10 +92,7 @@ defmodule DotcomWeb.CMSController do
   end
 
   defp handle_page_response({:error, _}, conn) do
-    conn
-    |> put_status(500)
-    |> put_view(DotcomWeb.ErrorView)
-    |> render("500.html", [])
+    render_500(conn)
   end
 
   @spec render_page(Conn.t(), Page.t()) :: Conn.t()

lib/dotcom_web/controllers/helpers.ex

@@ -2,7 +2,7 @@ defmodule DotcomWeb.ControllerHelpers do
   @moduledoc false
 
   import Plug.Conn, only: [halt: 1, put_resp_content_type: 2, put_status: 2]
-  import Phoenix.Controller, only: [render: 3, put_view: 2, put_layout: 2]
+  import Phoenix.Controller, only: [render: 3, put_view: 2]
 
   alias Alerts.{Alert, InformedEntity, Match, Repo}
   alias DotcomWeb.CMSController
@@ -39,13 +39,11 @@ defmodule DotcomWeb.ControllerHelpers do
     |> halt()
   end
 
-  def render_not_found(conn) do
+  def render_500(conn) do
     conn
-    |> put_status(:not_found)
-    |> put_layout({DotcomWeb.LayoutView, "root.html"})
+    |> put_status(:internal_server_error)
     |> put_view(DotcomWeb.ErrorView)
-    |> render("404.html", [])
-    |> halt()
+    |> render("500.html", [])
   end
 
   def return_internal_error(conn),

lib/dotcom_web/controllers/transit_near_me_controller.ex

@@ -16,10 +16,7 @@ defmodule DotcomWeb.TransitNearMeController do
     |> flash_if_error()
     |> case do
       :error ->
-        conn
-        |> put_status(:internal_server_error)
-        |> put_view(DotcomWeb.ErrorView)
-        |> render("500.html", [])
+        render_500(conn)
 
       %Conn{} = loaded_conn ->
         render(loaded_conn, "index.html", breadcrumbs: [Breadcrumb.build("Transit Near Me")])

lib/dotcom_web/plugs/secure_headers.ex

@@ -0,0 +1,97 @@
+defmodule DotcomWeb.Plugs.SecureHeaders do
+  @moduledoc """
+  Configures secure browser headers for Dotcom requests, including specifying
+  content security policy directives at funtime.
+  """
+
+  @base_csp_directives %{
+    connect: ~w[
+      connect-src
+      'self'
+      *.arcgis.com
+      *.googleapis.com
+      *.s3.amazonaws.com
+      analytics.google.com
+      px.ads.linkedin.com
+      stats.g.doubleclick.net
+      www.google-analytics.com
+      www.google.com
+    ],
+    default: ~w[default-src 'none'],
+    font: ~w[font-src 'self'],
+    frame: ~w[
+      frame-src
+      'self'
+      *.arcgis.com
+      cdn.knightlab.com
+      data.mbta.com
+      livestream.com
+      www.youtube.com
+      www.google.com
+      www.instagram.com
+    ],
+    img: ~w[
+      img-src
+      'self'
+      *.arcgis.com
+      *.google.com
+      *.googleapis.com
+      *.gstatic.com
+      *.s3.amazonaws.com
+      cdn.mbta.com
+      data:
+      i.ytimg.com
+      px.ads.linkedin.com
+      www.linkedin.com
+      www.facebook.com
+      www.googletagmanager.com
+    ],
+    script: ~w[
+      script-src
+      'self'
+      'unsafe-eval'
+      'unsafe-inline'
+      *.arcgis.com
+      *.google.com
+      *.googleapis.com
+      connect.facebook.net
+      data.mbta.com
+      insitez.blob.core.windows.net
+      snap.licdn.com
+      www.instagram.com
+      www.google-analytics.com
+      www.gstatic.com
+      www.googletagmanager.com
+    ],
+    style: ~w[style-src 'self' 'unsafe-inline' www.gstatic.com],
+    worker: ~w[worker-src blob: ;]
+  }
+
+  @default_secure_headers %{
+    "strict-transport-security" => "max-age=31536000",
+    "x-content-type-options" => "nosniff",
+    "x-frame-options" => "DENY",
+    "x-xss-protection" => "1; mode=block"
+  }
+
+  @behaviour Plug
+
+  @impl Plug
+  def init(opts), do: opts
+
+  @impl Plug
+  @spec call(Plug.Conn.t(), any()) :: Plug.Conn.t()
+  def call(conn, _) do
+    Phoenix.Controller.put_secure_browser_headers(conn, default_secure_headers())
+  end
+
+  def base_csp_directives, do: @base_csp_directives
+
+  def default_secure_headers,
+    do:
+      @default_secure_headers
+      |> Map.put(
+        "content-security-policy",
+        Application.get_env(:dotcom, :content_security_policy_definition, "")
+      )
+end

lib/dotcom_web/plugs/static.ex

@@ -1,5 +1,11 @@
 defmodule DotcomWeb.Plugs.Static do
+  @moduledoc """
+  Configure Plug.Static for Dotcom
+  """
+
   use Plug.Builder
+
+  import DotcomWeb.Plugs.SecureHeaders, only: [default_secure_headers: 0]
   import Phoenix.Controller, only: [redirect: 2]
 
   plug(:check_if_apple_touch_icon_request)
@@ -9,8 +15,8 @@ defmodule DotcomWeb.Plugs.Static do
     at: "/",
     from: :dotcom,
     gzip: Mix.env() == :prod,
-    headers: %{"access-control-allow-origin" => "*"},
-    cache_control_for_etags: "public, max-age=86400",
+    headers: default_secure_headers() |> Map.put("access-control-allow-origin", "*"),
+    cache_control_for_etags: "max-age=86400",
     only: DotcomWeb.static_paths(),
     only_matching: ~w(favicon apple-touch-icon)
   )

lib/dotcom_web/redirector.ex

@@ -16,7 +16,7 @@ defmodule DotcomWeb.Redirector do
   @spec call(Plug.Conn.t(), Keyword.t()) :: Plug.Conn.t()
   def call(%Plug.Conn{params: %{"id" => id}} = conn, to: to) when to not in ["/projects"] do
     case find_record(id, to) do
-      :not_found -> DotcomWeb.ControllerHelpers.render_not_found(conn)
+      :not_found -> DotcomWeb.ControllerHelpers.render_404(conn)
       record -> redirect_to_show(conn, to, record)
     end
   end

lib/dotcom_web/router.ex

@@ -2,6 +2,24 @@ defmodule DotcomWeb.Router do
   @moduledoc false
 
   use DotcomWeb, :router
+  use Plug.ErrorHandler
+
+  alias DotcomWeb.ControllerHelpers
+
+  @impl Plug.ErrorHandler
+  def handle_errors(conn, %{reason: reason}) do
+    conn
+    |> DotcomWeb.Plugs.SecureHeaders.call([])
+    |> then(fn conn ->
+      case reason do
+        %Phoenix.Router.NoRouteError{plug_status: 404} ->
+          ControllerHelpers.render_404(conn)
+
+        _ ->
+          ControllerHelpers.render_500(conn)
+      end
+    end)
+  end
 
   alias DotcomWeb.StaticPage
 
@@ -16,16 +34,16 @@ defmodule DotcomWeb.Router do
     plug(:fetch_session)
     plug(:fetch_flash)
     plug(:fetch_cookies)
-    plug(:put_secure_browser_headers_runtime, %{})
     plug(:put_root_layout, {DotcomWeb.LayoutView, :root})
-    plug(DotcomWeb.Plugs.CanonicalHostname)
     plug(DotcomWeb.Plugs.Banner)
+    plug(DotcomWeb.Plugs.CanonicalHostname)
+    plug(DotcomWeb.Plugs.ClearCookies)
+    plug(DotcomWeb.Plugs.Cookies)
     plug(DotcomWeb.Plugs.CommonFares)
     plug(DotcomWeb.Plugs.Date)
     plug(DotcomWeb.Plugs.DateTime)
     plug(DotcomWeb.Plugs.RewriteUrls)
-    plug(DotcomWeb.Plugs.ClearCookies)
-    plug(DotcomWeb.Plugs.Cookies)
+    plug(DotcomWeb.Plugs.SecureHeaders)
     plug(:optional_disable_indexing)
   end
 
@@ -321,13 +339,4 @@ defmodule DotcomWeb.Router do
       Plug.Conn.put_resp_header(conn, "x-robots-tag", "noindex")
     end
   end
-
-  defp put_secure_browser_headers_runtime(conn, default_headers) do
-    runtime_headers = %{
-      "content-security-policy" =>
-        Application.get_env(:dotcom, :content_security_policy_definition)
-    }
-
-    put_secure_browser_headers(conn, Map.merge(default_headers, runtime_headers))
-  end
 end

lib/dotcom_web/views/error_view.ex

@@ -29,35 +29,6 @@ defmodule DotcomWeb.ErrorView do
     )
   end
 
-  # Stubbed out, but untested
-  # def render("403.html", assigns) do
-  #   render(__MODULE__, "error_layout.html", Map.merge(assigns, %{
-  #     error_code: "403",
-  #     error_type: "Access denied",
-  #     error_title: "This stop is closed.",
-  #     error_description: "Unfortunately, you do not have access to this page.",
-  #     error_instructions: "Try searching for what you're looking for below."
-  #   }))
-  # end
-  # def render("404_missing_file.html", assigns) do
-  #   render(__MODULE__, "error_layout.html", Map.merge(assigns, %{
-  #     error_code: "404",
-  #     error_type: "File not found",
-  #     error_title: "Uh oh! We can't find the file.",
-  #     error_description: "The file you were looking for cannot be found.",
-  #     error_instructions: "We may have deleted it. Try searching for what you're looking for below."
-  #   }))
-  # end
-  # def render("503.html", assigns) do
-  #   render(__MODULE__, "error_layout.html", Map.merge(assigns, %{
-  #     error_code: "503",
-  #     error_type: "Maintenance",
-  #     error_title: "Service change in effect.",
-  #     error_description: "This page is down for maintenance.",
-  #     error_instructions: "Please check back after at least 24 hours..."
-  #   }))
-  # end
-
   # In case no render clause matches or no
   # template is found, let's render it as 500
   def template_not_found(_template, assigns) do

test/dotcom_web/controllers/helpers_test.exs

@@ -24,18 +24,6 @@ defmodule DotcomWeb.ControllerHelpersTest do
     end
   end
 
-  describe "render_not_found/1" do
-    test "renders the 404 bus page", %{conn: conn} do
-      rendered =
-        conn
-        |> put_private(:phoenix_endpoint, DotcomWeb.Endpoint)
-        |> render_not_found()
-        |> html_response(404)
-
-      assert rendered =~ "This page is no longer in service."
-    end
-  end
-
   describe "return_internal_error/1" do
     test "renders an 'Internal error' 500 json response" do
       response =