Fix hub invites. Add hub invite tests

config/test.exs

@@ -66,6 +66,8 @@ config :ret, Ret.Repo.Migrations.AdminSchemaInit, postgrest_password: "password"
 config :ret, Ret.Locking, lock_timeout_ms: 1000 * 60 * 15
 config :ret, Ret.Account, admin_email: "[email protected]"
 
+config :ret, RetWeb.HubChannel, enable_terminate_actions: false
+
 config :ret, Ret.PermsToken,
   # This config value is for local development only.
   perms_key:

lib/ret/hub_invite.ex

@@ -28,15 +28,15 @@ defmodule Ret.HubInvite do
     end
   end
 
-  def revoke_invite(hub_invite_sid) when is_binary(hub_invite_sid) do
-    hub_invite = HubInvite |> Repo.get_by(hub_invite_sid: hub_invite_sid)
+  def revoke_invite(%Hub{} = hub, hub_invite_sid) when is_binary(hub_invite_sid) do
+    hub_invite = HubInvite |> Repo.get_by(hub_id: hub.hub_id, hub_invite_sid: hub_invite_sid)
     change(hub_invite, state: :revoked) |> Repo.update!()
   end
 
-  def active?(nil = _hub_invite_sid), do: false
+  def active?(_hub, nil = _hub_invite_sid), do: false
 
-  def active?(hub_invite_sid) do
-    case Ret.HubInvite |> Ret.Repo.get_by(hub_invite_sid: hub_invite_sid) do
+  def active?(hub, hub_invite_sid) do
+    case Ret.HubInvite |> Ret.Repo.get_by(hub_id: hub.hub_id, hub_invite_sid: hub_invite_sid) do
       nil -> false
       %Ret.HubInvite{state: :revoked} -> false
       %Ret.HubInvite{state: :active} -> true

lib/ret_web/channels/hub_channel.ex

@@ -102,7 +102,7 @@ defmodule RetWeb.HubChannel do
           {nil, nil}
       end
 
-    has_active_invite = Ret.HubInvite.active?(params["hub_invite_id"])
+    has_active_invite = Ret.HubInvite.active?(hub, params["hub_invite_id"])
 
     params =
       params
@@ -458,8 +458,8 @@ defmodule RetWeb.HubChannel do
     hub = hub_for_socket(socket)
     account = Guardian.Phoenix.Socket.current_resource(socket)
 
-    if account |> can?(update_hub(hub)) do
-      HubInvite.revoke_invite(payload["hub_invite_id"])
+    if account |> can?(update_hub(hub)) and HubInvite.active?(hub, payload["hub_invite_id"]) do
+      HubInvite.revoke_invite(hub, payload["hub_invite_id"])
       # Hubs can only have one invite for now, so we create a new one when the old one was revoked.
       hub_invite = hub |> HubInvite.find_or_create_invite_for_hub()
       {:reply, {:ok, %{hub_invite_id: hub_invite.hub_invite_sid}}, socket}
@@ -771,9 +771,14 @@ defmodule RetWeb.HubChannel do
   end
 
   def terminate(_reason, socket) do
-    socket
-    |> SessionStat.stat_query_for_socket()
-    |> Repo.update_all(set: [ended_at: NaiveDateTime.utc_now()])
+    # enable_terminate_actions is set to false during tests. Since the GenServer is forcefully
+    # terminated when a test ends, we want to avoid running into an error that would happen if we
+    # invoked a DB mutation during termination.
+    if Application.get_env(:ret, __MODULE__)[:enable_terminate_actions] !== false do
+      socket
+      |> SessionStat.stat_query_for_socket()
+      |> Repo.update_all(set: [ended_at: NaiveDateTime.utc_now()])
+    end
 
     :ok
   end

test/ret_web/channels/hub_channel_test.exs

@@ -3,7 +3,9 @@ defmodule RetWeb.HubChannelTest do
   import Ret.TestHelpers
 
   alias RetWeb.{Presence, SessionSocket}
-  alias Ret.{AppConfig, Account, Repo, Hub}
+  alias Ret.{AppConfig, Account, Repo, Hub, HubInvite}
+
+  @default_join_params %{"profile" => %{}, "context" => %{}}
 
   setup [:create_account, :create_owned_file, :create_scene, :create_hub, :create_account]
 
@@ -12,52 +14,180 @@ defmodule RetWeb.HubChannelTest do
     {:ok, socket: socket}
   end
 
-  test "joining hub works", %{socket: socket, hub: hub} do
-    {:ok, %{session_id: _session_id}, _socket} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params())
-  end
+  describe "authorization" do
+    test "joining hub works", %{socket: socket, hub: hub} do
+      {:ok, %{session_id: _session_id}, _socket} =
+        subscribe_and_join(socket, "hub:#{hub.hub_sid}", @default_join_params)
+    end
 
-  test "joining hub does not work if sign in required", %{socket: socket, scene: scene} do
-    AppConfig.set_config_value("features|require_account_for_join", true)
-    {:ok, hub} = %Hub{} |> Hub.changeset(scene, %{name: "Test Hub"}) |> Repo.insert()
-    {:error, _reason} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params())
-    AppConfig.set_config_value("features|require_account_for_join", false)
-  end
+    test "joining hub does not work if sign in required", %{socket: socket, scene: scene} do
+      AppConfig.set_config_value("features|require_account_for_join", true)
+      {:ok, hub} = %Hub{} |> Hub.changeset(scene, %{name: "Test Hub"}) |> Repo.insert()
+      {:error, _reason} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", @default_join_params)
+      AppConfig.set_config_value("features|require_account_for_join", false)
+    end
 
-  test "joining hub does not work if account is disabled", %{socket: socket, hub: hub} do
-    disabled_account = create_account("disabled_account")
-    disabled_account |> Ecto.Changeset.change(state: :disabled) |> Ret.Repo.update!()
+    test "joining hub does not work if account is disabled", %{socket: socket, hub: hub} do
+      disabled_account = create_account("disabled_account")
+      disabled_account |> Ecto.Changeset.change(state: :disabled) |> Ret.Repo.update!()
 
-    {:error, %{reason: "join_denied"}} =
-      subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(disabled_account))
+      {:error, %{reason: "join_denied"}} =
+        subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(disabled_account))
+    end
   end
 
-  test "joining hub registers in presence", %{socket: socket, hub: hub} do
-    {:ok, %{session_id: session_id}, socket} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params())
-    :timer.sleep(100)
-    presence = socket |> Presence.list()
-    assert presence[session_id]
+  describe "presence" do
+    test "joining hub registers in presence", %{socket: socket, hub: hub} do
+      {:ok, %{session_id: session_id}, socket} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", @default_join_params)
+      :timer.sleep(100)
+      presence = socket |> Presence.list()
+      assert presence[session_id]
+    end
+
+    test "joining hub with an account with an identity registers identity in presence", %{
+      socket: socket,
+      hub: hub,
+      account: account
+    } do
+      account |> Account.set_identity!("Test User")
+
+      {:ok, %{session_id: session_id}, socket} =
+        subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))
+
+      :timer.sleep(100)
+      presence = socket |> Presence.list()
+      meta = presence[session_id][:metas] |> Enum.at(0)
+      assert meta[:profile]["identityName"] === "Test User"
+    end
   end
 
-  test "joining hub with an account with an identity registers identity in presence", %{
-    socket: socket,
-    hub: hub,
-    account: account
-  } do
-    account |> Account.set_identity!("Test User")
+  describe "hub invites" do
+    test "join is denied when joining without an invite", %{socket: socket} do
+      %{hub: hub} = create_invite_only_hub()
+
+      {:error, %{reason: "join_denied"}} = join_hub(socket, hub, @default_join_params)
+    end
+
+    test "join is denied when joining with an invalid invite", %{socket: socket} do
+      %{hub: hub} = create_invite_only_hub()
+
+      {:error, %{reason: "join_denied"}} = join_hub(socket, hub, join_params_for_hub_invite_id("invalid_invite_id"))
+    end
+
+    test "join is denied when joining with a revoked invite", %{socket: socket} do
+      %{hub: hub, hub_invite: hub_invite} = create_invite_only_hub()
+
+      Ret.HubInvite.revoke_invite(hub, hub_invite.hub_invite_sid)
+
+      {:error, %{reason: "join_denied"}} = join_hub(socket, hub, join_params_for_hub_invite(hub_invite))
+    end
+
+    test "join is denied when joining with a mismatched invite", %{socket: socket} do
+      %{hub: hub_one} = create_invite_only_hub()
+      %{hub: _hub_two, hub_invite: hub_two_invite} = create_invite_only_hub()
+
+      # Attempt to join hub_one, using invite associated with hub_two
+      {:error, %{reason: "join_denied"}} =
+        join_hub(
+          socket,
+          hub_one,
+          join_params_for_hub_invite(hub_two_invite)
+        )
+    end
+
+    test "revoke cannot be performed by an anonymous user", %{socket: socket} do
+      %{hub: hub, hub_invite: hub_invite} = create_invite_only_hub()
+
+      # Join hub as anonymous user
+      {:ok, _context, socket} = join_hub(socket, hub, join_params_for_hub_invite(hub_invite))
+
+      # Attempt to revoke invite
+      push(socket, "revoke_invite", %{hub_invite_id: hub_invite.hub_invite_sid})
+      |> assert_reply(:ok, %{})
+
+      # Join is still denied with invalid invite
+      {:error, %{reason: "join_denied"}} = join_hub(socket, hub, join_params_for_hub_invite_id("invalid_invite_id"))
+
+      # Join is still allowed with original invite
+      {:ok, _context, _socket} = join_hub(socket, hub, join_params_for_hub_invite(hub_invite))
+    end
+
+    test "revoke is keyed to hub", %{socket: socket} do
+      %{hub: hub_one} = create_invite_only_hub()
+      %{hub: hub_two, hub_invite: hub_two_invite} = create_invite_only_hub()
+
+      account_one = create_account("account_one")
+      assign_creator(hub_one, account_one)
+
+      # Join hub_one
+      {:ok, _context, socket} = join_hub(socket, hub_one, join_params_for_account(account_one))
 
-    {:ok, %{session_id: session_id}, socket} =
-      subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))
+      # Attempt to revoke invite associated with hub_two, using channel associated with hub_one
+      %{payload: response_payload} =
+        push(socket, "revoke_invite", %{hub_invite_id: hub_two_invite.hub_invite_sid})
+        |> assert_reply(:ok, %{})
 
-    :timer.sleep(100)
-    presence = socket |> Presence.list()
-    meta = presence[session_id][:metas] |> Enum.at(0)
-    assert meta[:profile]["identityName"] === "Test User"
+      # Response payload should be empty when a revoke is ignored
+      assert response_payload |> Map.keys() |> length === 0
+
+      # Joining hub_two is still denied with invalid invite
+      {:error, %{reason: "join_denied"}} = join_hub(socket, hub_two, join_params_for_hub_invite_id("invalid_invite_id"))
+
+      # Joining hub_two still allowed with original invite
+      {:ok, _context, _socket} = join_hub(socket, hub_two, join_params_for_hub_invite(hub_two_invite))
+    end
+
+    test "revoke can be performed by hub creator", %{socket: socket} do
+      %{hub: hub, hub_invite: hub_invite} = create_invite_only_hub()
+
+      account = create_account("test_account")
+      assign_creator(hub, account)
+
+      {:ok, _context, socket} = join_hub(socket, hub, join_params_for_account(account))
+
+      push(socket, "revoke_invite", %{hub_invite_id: hub_invite.hub_invite_sid})
+      |> assert_reply(:ok, %{hub_invite_id: new_hub_invite_id})
+
+      assert new_hub_invite_id !== hub_invite.hub_invite_sid
+    end
+
+    test "join is allowed on an invite-only hub with a correct invite id", %{socket: socket} do
+      %{hub: hub, hub_invite: hub_invite} = create_invite_only_hub()
+
+      {:ok, _context, _socket} = join_hub(socket, hub, join_params_for_hub_invite(hub_invite))
+    end
   end
 
-  defp join_params, do: %{"profile" => %{}, "context" => %{}}
+  defp join_params_for_hub_invite(%HubInvite{} = hub_invite) do
+    join_params_for_hub_invite_id(hub_invite.hub_invite_sid)
+  end
+
+  defp join_params_for_hub_invite_id(hub_invite_id) do
+    join_params(%{"hub_invite_id" => hub_invite_id})
+  end
 
   defp join_params_for_account(account) do
     {:ok, token, _params} = account |> Ret.Guardian.encode_and_sign()
-    %{"profile" => %{}, "context" => %{}, "auth_token" => token}
+    join_params(%{"auth_token" => token})
+  end
+
+  defp join_params(%{} = params) do
+    Map.merge(@default_join_params, params)
+  end
+
+  defp join_hub(socket, %Hub{} = hub, params) do
+    subscribe_and_join(socket, "hub:#{hub.hub_sid}", params)
+  end
+
+  defp create_invite_only_hub() do
+    {:ok, hub: hub} = create_hub(%{scene: nil})
+
+    hub
+    |> Ret.Hub.changeset_for_entry_mode(:invite)
+    |> Ret.Repo.update!()
+
+    hub_invite = Ret.HubInvite.find_or_create_invite_for_hub(hub)
+
+    %{hub: hub, hub_invite: hub_invite}
   end
 end