Merge pull request #44 from papercups-io/revert-security-changes

Revert security changes

Author: reichert621

assets/src/api.ts

@@ -27,13 +27,13 @@ export type RegisterParams = LoginParams & {
   passwordConfirmation: string;
 };
 
-export const getAccessToken = (): string | null => {
+const getAccessToken = (): string | null => {
   const tokens = getAuthTokens();
 
   return (tokens && tokens.token) || null;
 };
 
-export const getRefreshToken = (): string | null => {
+const getRefreshToken = (): string | null => {
   const tokens = getAuthTokens();
 
   return (tokens && tokens.renew_token) || null;

assets/src/components/account/GettingStartedOverview.tsx

@@ -165,7 +165,7 @@ const ExamplePage = () => {
             subtitle={subtitle}
             primaryColor={color}
             accountId={accountId}
-            baseUrl="https://app.papercups.io"
+            baseUrl={'https://app.papercups.io'}
             defaultIsOpen
           />
         </Box>

assets/src/components/conversations/ConversationsProvider.tsx

@@ -97,7 +97,7 @@ export class ConversationsProvider extends React.Component<Props, State> {
   channel: Channel | null = null;
 
   async componentDidMount() {
-    socket.connect({token: API.getAccessToken()});
+    socket.connect();
 
     const [currentUser, account, numTotalMessages] = await Promise.all([
       API.me(),
@@ -279,13 +279,19 @@ export class ConversationsProvider extends React.Component<Props, State> {
     conversationId: string,
     cb?: () => void
   ) => {
+    const {account, currentUser} = this.state;
+    const {id: accountId} = account;
+    const {id: userId} = currentUser;
+
     if (!this.channel || !message || message.trim().length === 0) {
       return;
     }
 
     this.channel.push('shout', {
       body: message,
+      user_id: userId,
       conversation_id: conversationId,
+      account_id: accountId,
       sender: 'agent', // TODO: remove?
     });
 

lib/chat_api_web/channels/notification_channel.ex

@@ -5,19 +5,24 @@ defmodule ChatApiWeb.NotificationChannel do
   alias ChatApi.{Messages, Conversations}
 
   @impl true
-  def join("notification:" <> account_id, %{"ids" => ids}, socket) do
-    if authorized?(socket, account_id) do
-      topics = for conversation_id <- ids, do: "conversation:#{conversation_id}"
-
-      {:ok,
-       socket
-       |> assign(:topics, [])
-       |> put_new_topics(topics)}
+  def join("notification:lobby", payload, socket) do
+    if authorized?(payload) do
+      {:ok, socket}
     else
-      {:error, %{reason: "Unauthorized"}}
+      {:error, %{reason: "unauthorized"}}
     end
   end
 
+  # TODO: secure this better!
+  def join("notification:" <> _account_id, %{"ids" => ids}, socket) do
+    topics = for conversation_id <- ids, do: "conversation:#{conversation_id}"
+
+    {:ok,
+     socket
+     |> assign(:topics, [])
+     |> put_new_topics(topics)}
+  end
+
   # Channels can be used in a request/response fashion
   # by sending replies to requests from the client
   @impl true
@@ -46,22 +51,22 @@ defmodule ChatApiWeb.NotificationChannel do
   end
 
   def handle_in("shout", payload, socket) do
-    with %{current_user: current_user} <- socket.assigns,
-         %{id: user_id, account_id: account_id} <- current_user do
-      msg = Map.merge(payload, %{"user_id" => user_id, "account_id" => account_id})
-      {:ok, message} = Messages.create_message(msg)
-      message = Messages.get_message!(message.id)
-      result = ChatApiWeb.MessageView.render("expanded.json", message: message)
+    {:ok, message} = Messages.create_message(payload)
+    message = Messages.get_message!(message.id)
+    result = ChatApiWeb.MessageView.render("expanded.json", message: message)
+    # TODO: write doc explaining difference between push, broadcast, etc.
+    push(socket, "shout", result)
 
-      # TODO: write doc explaining difference between push, broadcast, etc.
-      push(socket, "shout", result)
+    case result do
+      %{conversation_id: conversation_id} ->
+        topic = "conversation:" <> conversation_id
 
-      %{conversation_id: conversation_id} = result
-      topic = "conversation:" <> conversation_id
+        ChatApiWeb.Endpoint.broadcast_from!(self(), topic, "shout", result)
 
-      ChatApiWeb.Endpoint.broadcast_from!(self(), topic, "shout", result)
+        ChatApi.Slack.send_conversation_message_alert(conversation_id, message.body, type: "agent")
 
-      ChatApi.Slack.send_conversation_message_alert(conversation_id, message.body, type: "agent")
+      _ ->
+        nil
     end
 
     {:noreply, socket}
@@ -93,12 +98,8 @@ defmodule ChatApiWeb.NotificationChannel do
     end)
   end
 
-  defp authorized?(socket, account_id) do
-    with %{current_user: current_user} <- socket.assigns,
-         %{account_id: acct} <- current_user do
-      acct == account_id
-    else
-      _ -> false
-    end
+  # Add authorization logic here as required.
+  defp authorized?(_payload) do
+    true
   end
 end

lib/chat_api_web/channels/user_socket.ex

@@ -18,13 +18,6 @@ defmodule ChatApiWeb.UserSocket do
   # See `Phoenix.Token` documentation for examples in
   # performing token verification on connect.
   @impl true
-  def connect(%{"token" => token}, socket, _connect_info) do
-    case get_credentials(socket, token, otp_app: :chat_api) do
-      nil -> :error
-      user -> {:ok, assign(socket, :current_user, user)}
-    end
-  end
-
   def connect(_params, socket, _connect_info) do
     {:ok, socket}
   end
@@ -41,17 +34,4 @@ defmodule ChatApiWeb.UserSocket do
   # Returning `nil` makes this socket anonymous.
   @impl true
   def id(_socket), do: nil
-
-  defp get_credentials(socket, signed_token, config) do
-    conn = %Plug.Conn{secret_key_base: socket.endpoint.config(:secret_key_base)}
-    store_config = [backend: Pow.Store.Backend.EtsCache]
-    salt = Atom.to_string(ChatApiWeb.APIAuthPlug)
-
-    with {:ok, token} <- Pow.Plug.verify_token(conn, salt, signed_token, config),
-         {user, _metadata} <- Pow.Store.CredentialsCache.get(store_config, token) do
-      user
-    else
-      _any -> nil
-    end
-  end
 end

test/chat_api_web/channels/notification_channel_test.exs

@@ -1,32 +1,18 @@
 defmodule ChatApiWeb.NotificationChannelTest do
   use ChatApiWeb.ChannelCase
 
-  alias ChatApi.{Repo, Accounts, Conversations, Users.User}
-
-  @password "secret1234"
+  alias ChatApi.{Accounts, Conversations}
 
   setup do
     {:ok, account} = Accounts.create_account(%{company_name: "Taro"})
 
-    user =
-      %User{}
-      |> User.changeset(%{
-        email: "test@example.com",
-        password: @password,
-        password_confirmation: @password,
-        account_id: account.id
-      })
-      |> Repo.insert!()
-
     {:ok, conversation} =
       Conversations.create_conversation(%{account_id: account.id, status: "open"})
 
     {:ok, _, socket} =
       ChatApiWeb.UserSocket
-      |> socket("user_id", %{current_user: user})
-      |> subscribe_and_join(ChatApiWeb.NotificationChannel, "notification:" <> account.id, %{
-        "ids" => [conversation.id]
-      })
+      |> socket("user_id", %{some: :assign})
+      |> subscribe_and_join(ChatApiWeb.NotificationChannel, "notification:lobby")
 
     %{socket: socket, account: account, conversation: conversation}
   end