retry and fetch an updated firmware url if the first try 401s

.gitignore

@@ -10,6 +10,9 @@
 # Where 3rd-party dependencies like ExDoc output generated docs.
 /doc/
 
+# Where ExUnit adds temporary directories during test runs.
+/tmp/
+
 # Ignore .fetch files in case you like to edit your project deps locally.
 /.fetch
 

lib/nerves_hub_link/socket.ex

@@ -55,6 +55,10 @@ defmodule NervesHubLink.Socket do
     GenServer.call(pid, {:finish_uploading, filename})
   end
 
+  def check_update_available(pid \\ __MODULE__) do
+    GenServer.call(pid, :check_update_available)
+  end
+
   @doc """
   Cancel an ongoing upload
 
@@ -164,6 +168,12 @@ defmodule NervesHubLink.Socket do
     {:reply, connected?(socket), socket}
   end
 
+  def handle_call(:check_update_available, _from, socket) do
+    {:ok, ref} = push(socket, @device_topic, "check_update_available", socket.assigns.params)
+
+    {:reply, await_reply(ref), socket}
+  end
+
   def handle_call(:console_active?, _from, socket) do
     {:reply, socket.assigns.iex_pid != nil, socket}
   end

lib/nerves_hub_link/update_manager.ex

@@ -9,7 +9,7 @@ defmodule NervesHubLink.UpdateManager do
   """
   use GenServer
 
-  alias NervesHubLink.{Downloader, FwupConfig}
+  alias NervesHubLink.{Downloader, FwupConfig, Socket}
   alias NervesHubLink.Message.UpdateInfo
 
   require Logger
@@ -27,21 +27,27 @@ defmodule NervesHubLink.UpdateManager do
             | :update_rescheduled
             | {:updating, integer()}
 
+    @type previous_update :: :complete | :failed
+
     @type t :: %__MODULE__{
             status: status(),
             update_reschedule_timer: nil | :timer.tref(),
             download: nil | GenServer.server(),
             fwup: nil | GenServer.server(),
             fwup_config: FwupConfig.t(),
-            update_info: nil | UpdateInfo.t()
+            update_info: nil | UpdateInfo.t(),
+            retrying: boolean(),
+            previous_update: nil | previous_update()
           }
 
     defstruct status: :idle,
               update_reschedule_timer: nil,
               fwup: nil,
               download: nil,
               fwup_config: nil,
-              update_info: nil
+              update_info: nil,
+              retrying: false,
+              previous_update: nil
   end
 
   @doc """
@@ -61,6 +67,14 @@ defmodule NervesHubLink.UpdateManager do
     GenServer.call(manager, :status)
   end
 
+  @doc """
+  Returns the previous status of the update manager
+  """
+  @spec previous_update(GenServer.server()) :: State.previous_update()
+  def previous_update(manager \\ __MODULE__) do
+    GenServer.call(manager, :previous_update)
+  end
+
   @doc """
   Returns the UUID of the currently downloading firmware, or nil.
   """
@@ -113,6 +127,10 @@ defmodule NervesHubLink.UpdateManager do
     {:reply, state.status, state}
   end
 
+  def handle_call(:previous_update, _from, %State{} = state) do
+    {:reply, state.previous_update, state}
+  end
+
   @impl GenServer
   def handle_info({:update_reschedule, response, fwup_public_keys}, state) do
     {:noreply,
@@ -145,22 +163,55 @@ defmodule NervesHubLink.UpdateManager do
   end
 
   # messages from Downloader
-  def handle_info({:download, :complete}, state) do
+  def handle_info({:download, :complete, _fwup_public_keys}, state) do
     Logger.info("[NervesHubLink] Firmware Download complete")
-    {:noreply, %State{state | download: nil}}
+
+    {:noreply, %State{state | status: :idle, retrying: false, previous_update: :complete}}
   end
 
-  def handle_info({:download, {:error, reason}}, state) do
-    Logger.error("[NervesHubLink] Nonfatal HTTP download error: #{inspect(reason)}")
+  def handle_info(
+        {:download, {:error, %Mint.HTTPError{reason: {:http_error, 401}}}, _fwup_public_keys},
+        %State{retrying: true} = state
+      ) do
+    Logger.error("[NervesHubLink] Firmware download error: 401 after retry")
+    {:noreply, %State{state | status: :idle, retrying: false, previous_update: :failed}}
+  end
+
+  def handle_info(
+        {:download, {:error, %Mint.HTTPError{reason: {:http_error, 401}}}, fwup_public_keys},
+        state
+      ) do
+    Logger.error(
+      "[NervesHubLink] Firmware download URL signature may have expired. Requesting new URL and retrying."
+    )
+
+    GenServer.stop(Fwup.Stream)
+
+    update_info = Socket.check_update_available()
+    updated_state = %State{state | status: :idle, retrying: true, fwup: nil}
+    state = maybe_update_firmware(update_info, fwup_public_keys, updated_state)
     {:noreply, state}
   end
 
+  def handle_info({:download, {:error, reason}, _fwup_public_keys}, state) do
+    Logger.error("[NervesHubLink] Nonfatal HTTP download error: #{inspect(reason)}")
+    {:noreply, %State{state | status: :idle, retrying: false}}
+  end
+
   # Data from the downloader is sent to fwup
-  def handle_info({:download, {:data, data}}, state) do
+  def handle_info({:download, {:data, data}, _fwup_public_keys}, state) do
     _ = Fwup.Stream.send_chunk(state.fwup, data)
     {:noreply, state}
   end
 
+  def handle_info({:EXIT, _pid, {:http_error, 401}}, state) do
+    {:noreply, %State{state | download: nil}}
+  end
+
+  def handle_info({:EXIT, _pid, :normal}, state) do
+    {:noreply, state}
+  end
+
   @spec maybe_update_firmware(UpdateInfo.t(), [binary()], State.t()) :: State.t()
   defp maybe_update_firmware(
          %UpdateInfo{} = _update_info,
@@ -215,7 +266,10 @@ defmodule NervesHubLink.UpdateManager do
   @spec start_fwup_stream(UpdateInfo.t(), [binary()], State.t()) :: State.t()
   defp start_fwup_stream(%UpdateInfo{} = update_info, [] = fwup_public_keys, state) do
     pid = self()
-    fun = &send(pid, {:download, &1})
+
+    Process.flag(:trap_exit, true)
+
+    fun = &send(pid, {:download, &1, fwup_public_keys})
     {:ok, download} = Downloader.start_download(update_info.firmware_url, fun)
 
     {:ok, fwup} =

mix.exs

@@ -89,6 +89,7 @@ defmodule NervesHubLink.MixProject do
       {:fwup, "~> 1.0"},
       {:jason, "~> 1.0"},
       {:mint, "~> 1.2"},
+      {:mock, "~> 0.3.0", only: :test},
       {:mox, "~> 1.0", only: :test},
       {:nerves_key, "~> 1.0 or ~> 0.5", optional: true},
       {:nerves_runtime, "~> 0.8"},

mix.lock

@@ -21,9 +21,11 @@
   "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.0.0", "6f0eff9c9c489f26b69b61440bf1b238d95badae49adac77973cbacae87e3c2e", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "ea7a9307de9d1548d2a72d299058d1fd2339e3d398560a0e46c27dab4891e4d2"},
+  "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"},
   "mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"},
   "mint": {:hex, :mint, "1.6.0", "88a4f91cd690508a04ff1c3e28952f322528934be541844d54e0ceb765f01d5e", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "3c5ae85d90a5aca0a49c0d8b67360bbe407f3b54f1030a111047ff988e8fefaa"},
   "mint_web_socket": {:hex, :mint_web_socket, "1.0.3", "aab42fff792a74649916236d0b01f560a0b3f03ca5dea693c230d1c44736b50e", [:mix], [{:mint, ">= 1.4.1 and < 2.0.0-0", [hex: :mint, repo: "hexpm", optional: false]}], "hexpm", "ca3810ca44cc8532e3dce499cc17f958596695d226bb578b2fbb88c09b5954b0"},
+  "mock": {:hex, :mock, "0.3.8", "7046a306b71db2488ef54395eeb74df0a7f335a7caca4a3d3875d1fc81c884dd", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "7fa82364c97617d79bb7d15571193fc0c4fe5afd0c932cef09426b3ee6fe2022"},
   "mox": {:hex, :mox, "1.1.0", "0f5e399649ce9ab7602f72e718305c0f9cdc351190f72844599545e4996af73c", [:mix], [], "hexpm", "d44474c50be02d5b72131070281a5d3895c0e7a95c780e90bc0cfe712f633a13"},
   "muontrap": {:hex, :muontrap, "1.5.0", "bf5c273872379968615a39974458328209ac97fa1f588396192131ff973d1ca2", [:make, :mix], [{:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "daf605e877f60b5be9215e3420d7971fc468677b29921e40915b15fd928273d4"},
   "nerves_key": {:hex, :nerves_key, "1.2.0", "2cd65b7f474582d0af7bcd59664df16d0c58d1673dce573072d7dbe971dbd663", [:mix], [{:atecc508a, "~> 0.3.0 or ~> 1.1", [hex: :atecc508a, repo: "hexpm", optional: false]}, {:nerves_key_pkcs11, "~> 0.2 or ~> 1.0", [hex: :nerves_key_pkcs11, repo: "hexpm", optional: false]}], "hexpm", "ef49b26f38728b554cca2a0b16d198989a571e753d427ead2b22a8ea10a06000"},

test/nerves_hub_link/downloader_test.exs

@@ -3,6 +3,7 @@ defmodule NervesHubLink.DownloaderTest do
 
   alias NervesHubLink.Support.{
     HTTPErrorPlug,
+    HTTPUnauthorizedErrorPlug,
     IdleTimeoutPlug,
     RangeRequestPlug,
     RedirectPlug,
@@ -101,6 +102,35 @@ defmodule NervesHubLink.DownloaderTest do
     end
   end
 
+  describe "unauthorized http error" do
+    setup do
+      port = 7025
+
+      {:ok, plug} =
+        start_supervised(
+          {Plug.Cowboy,
+           scheme: :http,
+           plug: {HTTPUnauthorizedErrorPlug, report_pid: self()},
+           options: [port: port]}
+        )
+
+      {:ok, [plug: plug, url: "http://localhost:#{port}/test"]}
+    end
+
+    test "exits when an HTTP error occurs", %{url: url} do
+      test_pid = self()
+      handler_fun = &send(test_pid, &1)
+
+      Process.flag(:trap_exit, true)
+
+      {:ok, download} = Downloader.start_download(url, handler_fun)
+
+      assert_receive :request_error, 1000
+      assert_receive {:error, %Mint.HTTPError{reason: {:http_error, 401}}}, 1000
+      assert_receive {:EXIT, ^download, {:http_error, 401}}
+    end
+  end
+
   describe "range" do
     setup do
       port = 7030

test/nerves_hub_link/update_manager_test.exs

@@ -1,13 +1,15 @@
 defmodule NervesHubLink.UpdateManagerTest do
-  use ExUnit.Case
+  use ExUnit.Case, async: false
+
   alias NervesHubLink.{FwupConfig, UpdateManager}
   alias NervesHubLink.Message.{FirmwareMetadata, UpdateInfo}
   alias NervesHubLink.Support.FWUPStreamPlug
 
+  import Mock
+
   describe "fwup stream" do
     setup do
-      port = 6000
-      devpath = "/tmp/fwup_output"
+      port = Enum.random(6000..6999)
 
       update_payload = %UpdateInfo{
         firmware_url: "http://localhost:#{port}/test.fw",
@@ -19,13 +21,12 @@ defmodule NervesHubLink.UpdateManagerTest do
           {Plug.Cowboy, scheme: :http, plug: FWUPStreamPlug, options: [port: port]}
         )
 
-      File.rm(devpath)
-
-      {:ok, [plug: plug, update_payload: update_payload, devpath: "/tmp/fwup_output"]}
+      {:ok, [plug: plug, update_payload: update_payload]}
     end
 
-    test "apply", %{update_payload: update_payload, devpath: devpath} do
-      fwup_config = %{default_config() | fwup_devpath: devpath}
+    @tag :tmp_dir
+    test "apply", %{update_payload: update_payload, tmp_dir: tmp_dir} do
+      fwup_config = %{default_config() | fwup_devpath: Path.join(tmp_dir, "fwup_output")}
 
       {:ok, manager} = UpdateManager.start_link(fwup_config)
       assert UpdateManager.apply_update(manager, update_payload, []) == {:updating, 0}
@@ -35,7 +36,8 @@ defmodule NervesHubLink.UpdateManagerTest do
       assert_receive {:fwup, {:ok, 0, ""}}
     end
 
-    test "reschedule", %{update_payload: update_payload, devpath: devpath} do
+    @tag :tmp_dir
+    test "reschedule", %{update_payload: update_payload, tmp_dir: tmp_dir} do
       test_pid = self()
 
       update_available_fun = fn _ ->
@@ -52,7 +54,7 @@ defmodule NervesHubLink.UpdateManagerTest do
 
       fwup_config = %{
         default_config()
-        | fwup_devpath: devpath,
+        | fwup_devpath: Path.join(tmp_dir, "fwup_output"),
           update_available: update_available_fun
       }
 
@@ -66,10 +68,11 @@ defmodule NervesHubLink.UpdateManagerTest do
       assert_receive {:fwup, {:ok, 0, ""}}
     end
 
-    test "apply with fwup environment", %{update_payload: update_payload, devpath: devpath} do
+    @tag :tmp_dir
+    test "apply with fwup environment", %{update_payload: update_payload, tmp_dir: tmp_dir} do
       fwup_config = %{
         default_config()
-        | fwup_devpath: devpath,
+        | fwup_devpath: Path.join(tmp_dir, "fwup_output"),
           fwup_task: "secret_upgrade",
           fwup_env: [
             {"SUPER_SECRET", "1234567890123456789012345678901234567890123456789012345678901234"}
@@ -87,6 +90,49 @@ defmodule NervesHubLink.UpdateManagerTest do
     end
   end
 
+  describe "401 retry" do
+    setup do
+      port = 6543
+
+      update_payload = %UpdateInfo{
+        firmware_url: "http://localhost:#{port}/test.fw",
+        firmware_meta: %FirmwareMetadata{}
+      }
+
+      {:ok, plug} =
+        start_supervised(
+          {Plug.Cowboy,
+           scheme: :http,
+           plug: {NervesHubLink.Support.HTTPUnauthorizedErrorPlug, report_pid: self()},
+           options: [port: port]}
+        )
+
+      {:ok, [plug: plug, update_payload: update_payload]}
+    end
+
+    @tag :tmp_dir
+    test "retries firmware updates once", %{
+      update_payload: update_payload,
+      tmp_dir: tmp_dir
+    } do
+      fwup_config = %{default_config() | fwup_devpath: Path.join(tmp_dir, "fwup_output")}
+
+      {:ok, manager} = UpdateManager.start_link(fwup_config)
+
+      with_mock NervesHubLink.Socket, check_update_available: fn -> update_payload end do
+        assert UpdateManager.apply_update(manager, update_payload, []) == {:updating, 0}
+
+        assert_receive {:fwup, {:progress, 0}}, 1_000
+        assert_receive :request_error, 1_000
+        assert_receive :request_error, 1_000
+
+        Process.sleep(300)
+
+        assert UpdateManager.previous_update(manager) == :failed
+      end
+    end
+  end
+
   defp default_config() do
     test_pid = self()
     fwup_fun = &send(test_pid, {:fwup, &1})

test/support/http_error_plug.ex

@@ -10,7 +10,6 @@ defmodule NervesHubLink.Support.HTTPErrorPlug do
 
   @impl Plug
   def call(conn, _opts) do
-    conn
-    |> send_resp(416, "Range Not Satisfiable")
+    send_resp(conn, 416, "Range Not Satisfiable")
   end
 end

test/support/http_unauthorized_error_plug.ex

@@ -0,0 +1,17 @@
+defmodule NervesHubLink.Support.HTTPUnauthorizedErrorPlug do
+  @moduledoc false
+
+  @behaviour Plug
+
+  import Plug.Conn
+
+  @impl Plug
+  def init(options), do: options
+
+  @impl Plug
+  def call(conn, opts) do
+    send(opts[:report_pid], :request_error)
+
+    send_resp(conn, 401, "Unauthorized")
+  end
+end