Merge pull request #712 from mozilla/remove-promotion-token

Remove token promotion from entity state

lib/ret/owned_file.ex

@@ -61,6 +61,17 @@ defmodule Ret.OwnedFile do
     end
   end
 
+  def set_inactive(owned_file_uuid) when is_binary(owned_file_uuid) do
+    case get_by_uuid(owned_file_uuid) do
+      nil ->
+        {:error, :file_not_found}
+
+      owned_file ->
+        set_state(owned_file, :inactive)
+    end
+  end
+
+  @spec set_inactive(Ret.OwnedFile.t()) :: any()
   def set_inactive(%OwnedFile{} = owned_file) do
     set_state(owned_file, :inactive)
   end
@@ -69,6 +80,10 @@ defmodule Ret.OwnedFile do
     Repo.one(from OwnedFile, where: [owned_file_uuid: ^owned_file_uuid, account_id: ^account_id])
   end
 
+  def get_by_uuid(owned_file_uuid) do
+    Repo.one(from OwnedFile, where: [owned_file_uuid: ^owned_file_uuid])
+  end
+
   defp set_state(nil, _state), do: nil
 
   defp set_state(%OwnedFile{} = owned_file, state) do

lib/ret/storage.ex

@@ -233,24 +233,26 @@ defmodule Ret.Storage do
     end
   end
 
-  def promote(_id, nil, nil, _account) do
+  def promote(id, key, promotion_token, account, require_token \\ true)
+
+  def promote(_id, nil, nil, _account, _require_token) do
     {:error, :not_allowed}
   end
 
-  def promote(nil, _key, _promotion_token, _account) do
+  def promote(nil, _key, _promotion_token, _account, _require_token) do
     {:error, :not_found}
   end
 
-  def promote(_id, nil, _promotion_token, _account) do
+  def promote(_id, nil, _promotion_token, _account, _require_token) do
     {:error, :not_allowed}
   end
 
   # Promotes an expiring stored file to a permanently stored file in the specified Account.
-  def promote(id, key, promotion_token, %Account{} = account) do
+  def promote(id, key, promotion_token, %Account{} = account, require_token) do
     # Check if this file has already been promoted
     OwnedFile
     |> Repo.get_by(owned_file_uuid: id)
-    |> promote_or_return_owned_file(id, key, promotion_token, account)
+    |> promote_or_return_owned_file(id, key, promotion_token, account, require_token)
   end
 
   # Promotes multiple files into the given account.
@@ -283,17 +285,18 @@ defmodule Ret.Storage do
          _id,
          _key,
          _promotion_token,
-         _account
+         _account,
+         _require_token
        ) do
-    {:ok, owned_file}
+        {:ok, owned_file}
   end
 
   # Promoting a stored file to being owned has two side effects: the file is moved
   # into the owned files directory (which prevents it from being vacuumed) and an
   # OwnedFile record is inserted into the database which includes the decryption key.
   # If the stored file has an associated promotion token, the given promotion token is verified against it.
   # If the given promotion token fails verification, the file is not promoted.
-  defp promote_or_return_owned_file(nil, id, key, promotion_token, account) do
+  defp promote_or_return_owned_file(nil, id, key, promotion_token, account, require_token) do
     with(
       storage_path when is_binary(storage_path) <- module_config(:storage_path),
       {:ok, uuid} <- Ecto.UUID.cast(id),
@@ -307,7 +310,7 @@ defmodule Ret.Storage do
         "promotion_token" => actual_promotion_token
       } <-
         File.read!(meta_file_path) |> Poison.decode!(),
-      {:ok} <- check_promotion_token(actual_promotion_token, promotion_token),
+      {:ok} <- (if require_token, do: check_promotion_token(actual_promotion_token, promotion_token), else: {:ok}),
       {:ok} <- check_blob_file_key(blob_file_path, key)
     ) do
       owned_file_params = %{

lib/ret_web/channels/hub_channel.ex

@@ -1472,8 +1472,8 @@ defmodule RetWeb.HubChannel do
     }
   end
 
-  defp maybe_set_owned_file_inactive(%{"file_id" => file_id}, %Account{account_id: account_id}) do
-    OwnedFile.set_inactive(file_id, account_id)
+  defp maybe_set_owned_file_inactive(%{"file_id" => file_id}, _account) do
+    OwnedFile.set_inactive(file_id)
   end
 
   defp maybe_set_owned_file_inactive(_payload, _account) do
@@ -1490,7 +1490,8 @@ defmodule RetWeb.HubChannel do
              params.file_id,
              params.file_access_token,
              params.promotion_token,
-             account
+             account,
+             false
            ) do
       OwnedFile.set_active(params.file_id, account.account_id)
       :ok

test/ret_web/channels/entity_test.exs

@@ -8,6 +8,7 @@ defmodule RetWeb.EntityTest do
 
   @payload_save_entity_state read_json("save_entity_state_payload.json")
   @payload_save_entity_state_2 read_json("save_entity_state_payload_2.json")
+  @payload_save_entity_state_promotable_no_token read_json("save_entity_state_payload_promotable_no_token.json")
   @payload_save_entity_state_promotable read_json("save_entity_state_payload_promotable.json")
   @payload_save_entity_state_unpromotable read_json("save_entity_state_payload_unpromotable.json")
   @payload_update_entity_state read_json("update_entity_state_payload.json")
@@ -73,6 +74,27 @@ defmodule RetWeb.EntityTest do
       }
     end
 
+    test "save_entity_state succeeds if no promotion keys are provided", %{
+      socket: socket,
+      hub: hub,
+      account: account
+    } do
+      %HubRoleMembership{hub: hub, account: account} |> Repo.insert!()
+      temp_file = generate_temp_file("test")
+
+      {:ok, _, socket} =
+        subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))
+
+      {:ok, uuid} = Storage.store(%Plug.Upload{path: temp_file}, "text/plain", "secret")
+
+      updated_map =
+        @payload_save_entity_state_promotable_no_token
+        |> Map.put("file_id", uuid)
+        |> Map.put("file_access_token", "secret")
+
+      assert_reply push(socket, "save_entity_state", updated_map), :ok
+    end
+
     test "save_entity_state succeeds if provided correct promotion keys", %{
       socket: socket,
       hub: hub,

test/support/utils/save_entity_state_payload_promotable_no_token.json

@@ -0,0 +1,53 @@
+{
+  "nid": "hvpg97x",
+  "create_message": {
+    "version": 1,
+    "networkId": "hvpg97x",
+    "prefabName": "media",
+    "initialData": {
+      "src": "https://hubs.local:4000/files/101ac987-61f4-4645-b512-def3f0336fdf.jpg?token=2bbfa5ff552a159cff50b8bc212a477c",
+      "recenter": true,
+      "resize": true,
+      "animateLoad": true,
+      "fileId": "101ac987-61f4-4645-b512-def3f0336fdf",
+      "isObjectMenuTarget": true
+    }
+  },
+  "updates": [
+    {
+      "root_nid": "hvpg97x",
+      "nid": "hvpg97x",
+      "update_message": {
+        "nid": "hvpg97x",
+        "lastOwnerTime": 1157850816,
+        "timestamp": 1157852116,
+        "owner": "9eac3551-20d7-4b04-80fc-2009b5e8fe10",
+        "data": {
+          "networked-transform": {
+            "version": 1,
+            "data": {
+              "position": [
+                44.335304260253906,
+                1.4967195987701416,
+                -6.183996677398682
+              ],
+              "rotation": [
+                -0.03446304053068161,
+                0.6161360144615173,
+                0.026999054476618767,
+                0.7864221334457397
+              ],
+              "scale": [
+                1,
+                1,
+                1
+              ]
+            }
+          }
+        }
+      }
+    }
+  ],
+  "file_id": "101ac987-61f4-4645-b512-def3f0336fdf",
+  "file_access_token": "2bbfa5ff552a159cff50b8bc212a477c"
+}