Board controller authorizations tests

CONTRIBUTIONS.md

@@ -13,7 +13,7 @@ conventions.  Please refer to these guidelines!
 * Write [code](https://en.wikipedia.org/wiki/Computer_programming)
   in [Elixir](https://elixir-lang.org/) to implement your feature
 
-  (Instructions for your computer to do things)
+* Use Logger instead of IO.puts/inspect
 
 * Commit code using [Angular Commit Message](https://gist.github.com/brianclements/841ea7bffdb01346392c)
   conventions.

config/test.exs

@@ -24,7 +24,11 @@ config :epochtalk_server, EpochtalkServerWeb.Endpoint,
 config :epochtalk_server, EpochtalkServer.Mailer, adapter: Swoosh.Adapters.Test
 
 # Print only warnings and errors during test
-config :logger, level: :warning
+# Accept module and parameters
+config :logger, :console,
+  level: :warning,
+  format: "[$level] $message $metadata\n",
+  metadata: [:module, :parameters]
 
 # Initialize plugs at runtime for faster test compilation
 config :phoenix, :plug_init_mode, :runtime

lib/epochtalk_server/models/banned_address.ex

@@ -6,6 +6,13 @@ defmodule EpochtalkServer.Models.BannedAddress do
   alias EpochtalkServer.Repo
   alias EpochtalkServer.Models.BannedAddress
 
+  @one_week_in_ms 1000 * 60 * 60 * 24 * 7
+  @inital_amount 0.8897
+  @rate_of_decay 0.9644
+  @ip32_weight 1
+  @ip24_weight 0.04
+  @ip16_weight 0.0016
+
   @moduledoc """
   `BannedAddress` model, for performing actions relating to banning by ip/hostname
   """
@@ -222,11 +229,13 @@ defmodule EpochtalkServer.Models.BannedAddress do
               0
           end
 
-        ip32_score = calculate_ip32_score(ip)
-        ip24_score = calculate_ip24_score(ip)
-        ip16_score = calculate_ip16_score(ip)
+        # calculate ip scores with weight constants
+        ip32_score = calculate_ip32_score(ip) * @ip32_weight
+        ip24_score = calculate_ip24_score(ip) * @ip24_weight
+        ip16_score = calculate_ip16_score(ip) * @ip16_weight
         # calculate malicious score using all scores
-        malicious_score = hostname_score + ip32_score + 0.04 + ip24_score + 0.0016 + ip16_score
+        malicious_score = hostname_score + ip32_score + ip24_score + ip16_score
+
         if malicious_score < 1, do: nil, else: malicious_score
 
       # invalid ip address, return nil for malicious score
@@ -310,11 +319,10 @@ defmodule EpochtalkServer.Models.BannedAddress do
   # in ms since the weight was last updated
   defp decay_for_time(time, weight) do
     weight = Decimal.to_float(weight)
-    one_week = 1000 * 60 * 60 * 24 * 7
-    weeks = time / one_week
-    a = 0.8897
-    r = 0.9644
-    a ** ((r ** weeks - 1) / (r - 1)) * weight ** (r ** weeks)
+    weeks = time / @one_week_in_ms
+
+    @inital_amount ** ((@rate_of_decay ** weeks - 1) / (@rate_of_decay - 1)) *
+      weight ** (@rate_of_decay ** weeks)
   end
 
   # returns the decayed score given a banned address

lib/epochtalk_server/models/board_moderator.ex

@@ -60,7 +60,7 @@ defmodule EpochtalkServer.Models.BoardModerator do
     do: Repo.all(from(b in BoardModerator, select: b.board_id, where: b.user_id == ^user_id))
 
   @doc """
-  Check if a specific `User` is moderater of a `Board` using a `Board` ID
+  Check if a specific `User` is moderator of a `Board` using a `Board` ID
   """
   @spec user_is_moderator(
           board_id :: non_neg_integer,
@@ -77,7 +77,7 @@ defmodule EpochtalkServer.Models.BoardModerator do
   end
 
   @doc """
-  Check if a specific `User` is moderater of a `Board` using a `Thread` ID
+  Check if a specific `User` is moderator of a `Board` using a `Thread` ID
   """
   @spec user_is_moderator_with_thread_id(thread_id :: non_neg_integer, user_id :: non_neg_integer) ::
           boolean
@@ -95,7 +95,7 @@ defmodule EpochtalkServer.Models.BoardModerator do
   end
 
   @doc """
-  Check if a specific `User` is moderater of a `Board` using a `Post` ID
+  Check if a specific `User` is moderator of a `Board` using a `Post` ID
   """
   @spec user_is_moderator_with_post_id(
           post_id :: non_neg_integer,

lib/epochtalk_server/session.ex

@@ -1,10 +1,14 @@
 defmodule EpochtalkServer.Session do
   @one_day_in_seconds 1 * 24 * 60 * 60
   @four_weeks_in_seconds 4 * 7 * @one_day_in_seconds
+  # in redis, if a key exists and has no expiration, its TTL value is -1
+  # if the key does not exist, its TTL value is -2
+  @redis_ttl_no_expire_with_key -1
 
   @moduledoc """
   Manages `User` sessions in Redis. Used by Auth related `User` actions.
   """
+  require Logger
   alias EpochtalkServer.Auth.Guardian
   alias EpochtalkServer.Models.User
   alias EpochtalkServer.Models.Role
@@ -52,30 +56,38 @@ defmodule EpochtalkServer.Session do
   @doc """
   Update session performs the following actions for active session:
   * Saves `User` session info to redis (avatar, roles, moderating, ban info, etc)
-  * returns {:ok, user}
+  * returns {:ok, user} on success
+  * returns {:error, reason} on failure
 
   DOES NOT change:
   * User's session id, timestamp, ttl
   * Guardian token
   """
-  @spec update(user_id :: non_neg_integer) :: {:ok, user :: User.t()}
+  @spec update(user_id :: non_neg_integer) ::
+          {:ok, user :: User.t()}
+          | {:error, reason :: String.t() | Postgrex.Error.t()}
   def update(user_id) do
-    {:ok, user} = User.by_id(user_id)
-    avatar = if is_nil(user.profile), do: nil, else: user.profile.avatar
-    update_user_info(user.id, user.username, avatar: avatar)
-    update_roles(user.id, user.roles)
-
-    ban_info =
-      if is_nil(user.ban_info), do: %{}, else: %{ban_expiration: user.ban_info.expiration}
-
-    ban_info =
-      if !is_nil(user.malicious_score) && user.malicious_score >= 1,
-        do: Map.put(ban_info, :malicious_score, user.malicious_score),
-        else: ban_info
-
-    update_ban_info(user.id, ban_info)
-    update_moderating(user.id, user.moderating)
-    {:ok, user}
+    with {:ok, user} <- User.by_id(user_id),
+         true <- has_sessions?(user_id) do
+      avatar = if is_nil(user.profile), do: nil, else: user.profile.avatar
+      update_user_info(user.id, user.username, avatar: avatar)
+      update_roles(user.id, user.roles)
+
+      ban_info =
+        if is_nil(user.ban_info), do: %{}, else: %{ban_expiration: user.ban_info.expiration}
+
+      ban_info =
+        if !is_nil(user.malicious_score) && user.malicious_score >= 1,
+          do: Map.put(ban_info, :malicious_score, user.malicious_score),
+          else: ban_info
+
+      update_ban_info(user.id, ban_info)
+      update_moderating(user.id, user.moderating)
+      {:ok, user}
+    else
+      {:error, error} -> {:error, error}
+      false -> {:error, :no_sessions}
+    end
   end
 
   @doc """
@@ -87,7 +99,7 @@ defmodule EpochtalkServer.Session do
           | {:error, reason :: String.t() | Redix.Error.t() | Redix.ConnectionError.t()}
   def get_resource(user_id, session_id) do
     # check if session is active in redis
-    is_active_for_user_id(session_id, user_id)
+    is_active_for_user_id?(session_id, user_id)
     |> case do
       {:error, error} ->
         {:error, "Error finding resource from claims #{inspect(error)}"}
@@ -122,17 +134,19 @@ defmodule EpochtalkServer.Session do
             else: resource
 
         ban_key = generate_key(user_id, "baninfo")
-        ban_expiration = Redix.command!(:redix, ["HEXISTS", ban_key, "ban_expiration"])
+        ban_expiration_exists = Redix.command!(:redix, ["HEXISTS", ban_key, "ban_expiration"])
+        ban_expiration = Redix.command!(:redix, ["HGET", ban_key, "ban_expiration"])
 
         resource =
-          if ban_expiration != 0,
+          if ban_expiration_exists != 0,
             do: Map.put(resource, :ban_expiration, ban_expiration),
             else: resource
 
-        malicious_score = Redix.command!(:redix, ["HEXISTS", ban_key, "malicious_score"])
+        malicious_score_exists = Redix.command!(:redix, ["HEXISTS", ban_key, "malicious_score"])
+        malicious_score = Redix.command!(:redix, ["HGET", ban_key, "malicious_score"])
 
         resource =
-          if malicious_score != 0,
+          if malicious_score_exists != 0,
             do: Map.put(resource, :malicious_score, malicious_score),
             else: resource
 
@@ -153,7 +167,19 @@ defmodule EpochtalkServer.Session do
     Redix.command(:redix, ["ZRANGE", session_key, 0, -1])
   end
 
-  defp is_active_for_user_id(session_id, user_id) do
+  defp has_sessions?(user_id) do
+    session_key = generate_key(user_id, "sessions")
+    # check ZCARD for key
+    # returns number of sorted set elements at key
+    # returns 0 if key does not exist (set is empty)
+    case Redix.command(:redix, ["ZCARD", session_key]) do
+      {:error, error} -> {:error, error}
+      {:ok, 0} -> false
+      {:ok, _score} -> true
+    end
+  end
+
+  defp is_active_for_user_id?(session_id, user_id) do
     session_key = generate_key(user_id, "sessions")
     # check ZSCORE for user_id:session_id pair
     # returns score if it is a member of the set
@@ -247,8 +273,10 @@ defmodule EpochtalkServer.Session do
     moderating = moderating |> Enum.map(& &1.board_id)
     # save/replace moderating boards to redis under "user:{user_id}:moderating"
     moderating_key = generate_key(user_id, "moderating")
-    # get current TTL
-    {:ok, old_ttl} = Redix.command(:redix, ["TTL", moderating_key])
+    # use user_key for base ttl, since moderating may not be populated (TTL == -2)
+    user_key = generate_key(user_id, "user")
+    # get current TTL of session
+    {:ok, session_ttl} = Redix.command(:redix, ["TTL", user_key])
     Redix.command(:redix, ["DEL", moderating_key])
 
     unless moderating == [],
@@ -257,10 +285,10 @@ defmodule EpochtalkServer.Session do
     # if ttl is provided
     if ttl do
       # maybe extend ttl
-      maybe_extend_ttl(moderating_key, ttl, old_ttl)
+      maybe_extend_ttl(moderating_key, ttl, session_ttl)
     else
-      # otherwise, re-set old_ttl
-      maybe_extend_ttl(moderating_key, old_ttl)
+      # otherwise, re-set session_ttl from user_key
+      maybe_extend_ttl(moderating_key, session_ttl)
     end
   end
 
@@ -292,23 +320,26 @@ defmodule EpochtalkServer.Session do
   defp update_ban_info(user_id, ban_info, ttl \\ nil) do
     # save/replace ban_expiration to redis under "user:{user_id}:baninfo"
     ban_key = generate_key(user_id, "baninfo")
-    # get current TTL
-    {:ok, old_ttl} = Redix.command(:redix, ["TTL", ban_key])
+    # use user_key for base ttl, since baninfo may not be populated (TTL == -2)
+    user_key = generate_key(user_id, "user")
+    # get current TTL of session
+    {:ok, session_ttl} = Redix.command(:redix, ["TTL", user_key])
     Redix.command(:redix, ["HDEL", ban_key, "ban_expiration", "malicious_score"])
 
-    if ban_exp = Map.get(ban_info, :ban_expiration),
-      do: Redix.command(:redix, ["HSET", ban_key, "ban_expiration", ban_exp])
+    if ban_exp = Map.get(ban_info, :ban_expiration) do
+      Redix.command(:redix, ["HSET", ban_key, "ban_expiration", ban_exp])
+    end
 
     if malicious_score = Map.get(ban_info, :malicious_score),
       do: Redix.command(:redix, ["HSET", ban_key, "malicious_score", malicious_score])
 
     # if ttl is provided
     if ttl do
       # maybe extend ttl
-      maybe_extend_ttl(ban_key, ttl, old_ttl)
+      maybe_extend_ttl(ban_key, ttl, session_ttl)
     else
-      # otherwise, re-set old_ttl
-      maybe_extend_ttl(ban_key, old_ttl)
+      # otherwise, re-set session_ttl from user_key
+      maybe_extend_ttl(ban_key, session_ttl)
     end
   end
 
@@ -353,13 +384,27 @@ defmodule EpochtalkServer.Session do
     maybe_extend_ttl(key, ttl, ttl)
   end
 
+  # set ttl to max of old_ttl, new_ttl, and existing ttl
+  # - or @four_weeks_in_seconds if old and new ttl's are invalid
   defp maybe_extend_ttl(key, new_ttl, old_ttl) do
-    if old_ttl > -1 do
-      # re-set old expiry only if old expiry was valid and key has no expiry
-      Redix.command(:redix, ["EXPIRE", key, old_ttl, "NX"])
-    else
-      # if old expiry was invalid, set new expiry only if key has no expiry
-      Redix.command(:redix, ["EXPIRE", key, new_ttl, "NX"])
+    cond do
+      old_ttl > @redis_ttl_no_expire_with_key ->
+        # re-set old expiry only if old expiry was valid and key has no expiry
+        Redix.command(:redix, ["EXPIRE", key, old_ttl, "NX"])
+
+      new_ttl > @redis_ttl_no_expire_with_key ->
+        # if old expiry was invalid, set new expiry only if new expiry is valid and key has no expiry
+        Redix.command(:redix, ["EXPIRE", key, new_ttl, "NX"])
+
+      true ->
+        # catch-all, in case both given expiries are invalid
+        Logger.warning("Invalid TTL's, setting max expiry", %{
+          module: __MODULE__,
+          parameters: "[old_ttl: #{old_ttl}, new_ttl: #{new_ttl}, key: #{key}]"
+        })
+
+        # if neither expiry is valid, set ttl to max
+        Redix.command(:redix, ["EXPIRE", key, @four_weeks_in_seconds])
     end
 
     # set expiry only if new expiry is greater than current (GT)

lib/epochtalk_server_web/controllers/post.ex

@@ -151,14 +151,14 @@ defmodule EpochtalkServerWeb.Controllers.Post do
   """
   # TODO(akinsey): Implement for completion
   # - parser
-  # - Implement guard in Validate that prevents passing in page and start at the same time
   def by_thread(conn, attrs) do
     # Parameter Validation
     with thread_id <- Validate.cast(attrs, "thread_id", :integer, required: true),
          page <- Validate.cast(attrs, "page", :integer, default: 1, min: 1),
          start <- Validate.cast(attrs, "start", :integer, min: 1),
          limit <- Validate.cast(attrs, "limit", :integer, default: 25, min: 1, max: 100),
          desc <- Validate.cast(attrs, "desc", :boolean, default: true),
+         :ok <- Validate.mutually_exclusive!(attrs, ["page", "start"]),
          user <- Guardian.Plug.current_resource(conn),
          user_priority <- ACL.get_user_priority(conn),
 

lib/epochtalk_server_web/controllers/thread.ex

@@ -43,9 +43,7 @@ defmodule EpochtalkServerWeb.Controllers.Thread do
   # TODO(akinsey): Post pre processing, authorizations, image processing, hooks. Things to consider:
   # - does html sanitizer need to run on the front end too
   # - how do we run the same parser/sanitizer on the elixir back end as the node frontend
-  # - processing mentions
   # - does createImageReferences need to be called here? was this missing from the old code?
-  # - does updateUserActivity need to be called here? was this missing from the old code?
   def create(conn, attrs) do
     # authorization checks
     with :ok <- ACL.allow!(conn, "threads.create"),

lib/epochtalk_server_web/errors/custom_errors.ex

@@ -49,6 +49,7 @@ defmodule EpochtalkServerWeb.CustomErrors do
     def exception(value) do
       case value do
         [] -> %InvalidPayload{}
+        [message: message] -> %InvalidPayload{message: message}
         _ -> %InvalidPayload{message: gen_message(Enum.into(value, %{required: false}))}
       end
     end