diff --git a/apps/fg_http/coveralls.json b/apps/fg_http/coveralls.json new file mode 100644 index 000000000..3d771278d --- /dev/null +++ b/apps/fg_http/coveralls.json @@ -0,0 +1,5 @@ +{ + "skip_files": [ + "test" + ] +} diff --git a/apps/fg_http/lib/fg_http/devices.ex b/apps/fg_http/lib/fg_http/devices.ex index bc8fa3944..94286b018 100644 --- a/apps/fg_http/lib/fg_http/devices.ex +++ b/apps/fg_http/lib/fg_http/devices.ex @@ -8,50 +8,10 @@ defmodule FgHttp.Devices do alias FgHttp.Devices.Device - @doc """ - Returns the list of devices. - - ## Examples - - iex> list_devices() - [%Device{}, ...] - - """ - def list_devices, do: Repo.all(Device) - def list_devices(user_id) do Repo.all(from d in Device, where: d.user_id == ^user_id) end - @doc """ - Returns a new device with an initialized configuration. - - ## Examples - - iex> new_device() - %Device{ - "conf" => "new_conf" - } - """ - def new_device(attrs \\ %{}) do - device = %Device{} - Map.merge(device, attrs) - end - - @doc """ - Gets a single device. - - Raises `Ecto.NoResultsError` if the Device does not exist. - - ## Examples - - iex> get_device!(123) - %Device{} - - iex> get_device!(456) - ** (Ecto.NoResultsError) - - """ def get_device!(id), do: Repo.get!(Device, id) def get_device!(id, with_rules: true) do @@ -62,67 +22,22 @@ defmodule FgHttp.Devices do ) end - @doc """ - Creates a device. - - ## Examples - - iex> create_device(%{field: value}) - {:ok, %Device{}} - - iex> create_device(%{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ def create_device(attrs \\ %{}) do %Device{} |> Device.changeset(attrs) |> Repo.insert() end - @doc """ - Updates a device. - - ## Examples - - iex> update_device(device, %{field: new_value}) - {:ok, %Device{}} - - iex> update_device(device, %{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ def update_device(%Device{} = device, attrs) do device |> Device.changeset(attrs) |> Repo.update() end - @doc """ - Deletes a device. - - ## Examples - - iex> delete_device(device) - {:ok, %Device{}} - - iex> delete_device(device) - {:error, %Ecto.Changeset{}} - - """ def delete_device(%Device{} = device) do Repo.delete(device) end - @doc """ - Returns an `%Ecto.Changeset{}` for tracking device changes. - - ## Examples - - iex> change_device(device) - %Ecto.Changeset{source: %Device{}} - - """ def change_device(%Device{} = device) do Device.changeset(device, %{}) end diff --git a/apps/fg_http/lib/fg_http/devices/device.ex b/apps/fg_http/lib/fg_http/devices/device.ex index 1e93550ed..c93b60c2b 100644 --- a/apps/fg_http/lib/fg_http/devices/device.ex +++ b/apps/fg_http/lib/fg_http/devices/device.ex @@ -20,7 +20,6 @@ defmodule FgHttp.Devices.Device do timestamps(type: :utc_datetime_usec) end - @doc false def changeset(device, attrs) do device |> cast(attrs, [:last_ip, :ifname, :user_id, :name, :public_key]) diff --git a/apps/fg_http/lib/fg_http/rules.ex b/apps/fg_http/lib/fg_http/rules.ex index fd426adae..0a1b57d23 100644 --- a/apps/fg_http/lib/fg_http/rules.ex +++ b/apps/fg_http/lib/fg_http/rules.ex @@ -8,15 +8,6 @@ defmodule FgHttp.Rules do alias FgHttp.Rules.Rule - @doc """ - Returns the list of rules. - - ## Examples - - iex> list_rules() - [%Rule{}, ...] - - """ def list_rules(device_id) do Repo.all(from r in Rule, where: r.device_id == ^device_id) end @@ -25,83 +16,24 @@ defmodule FgHttp.Rules do Repo.all(Rule) end - @doc """ - Gets a single rule. - - Raises `Ecto.NoResultsError` if the Rule does not exist. - - ## Examples - - iex> get_rule!(123) - %Rule{} - - iex> get_rule!(456) - ** (Ecto.NoResultsError) - - """ def get_rule!(id), do: Repo.get!(Rule, id) - @doc """ - Creates a rule. - - ## Examples - - iex> create_rule(%{field: value}) - {:ok, %Rule{}} - - iex> create_rule(%{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ def create_rule(attrs \\ %{}) do %Rule{} |> Rule.changeset(attrs) |> Repo.insert() end - @doc """ - Updates a rule. - - ## Examples - - iex> update_rule(rule, %{field: new_value}) - {:ok, %Rule{}} - - iex> update_rule(rule, %{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ def update_rule(%Rule{} = rule, attrs) do rule |> Rule.changeset(attrs) |> Repo.update() end - @doc """ - Deletes a rule. - - ## Examples - - iex> delete_rule(rule) - {:ok, %Rule{}} - - iex> delete_rule(rule) - {:error, %Ecto.Changeset{}} - - """ def delete_rule(%Rule{} = rule) do Repo.delete(rule) end - @doc """ - Returns an `%Ecto.Changeset{}` for tracking rule changes. - - ## Examples - - iex> change_rule(rule) - %Ecto.Changeset{source: %Rule{}} - - """ def change_rule(%Rule{} = rule) do Rule.changeset(rule, %{}) end diff --git a/apps/fg_http/lib/fg_http/rules/rule.ex b/apps/fg_http/lib/fg_http/rules/rule.ex index 6be9a535e..13bb981c7 100644 --- a/apps/fg_http/lib/fg_http/rules/rule.ex +++ b/apps/fg_http/lib/fg_http/rules/rule.ex @@ -21,7 +21,6 @@ defmodule FgHttp.Rules.Rule do timestamps(type: :utc_datetime_usec) end - @doc false def changeset(rule, attrs) do rule |> cast(attrs, [:device_id, :priority, :action, :destination, :port, :protocol, :enabled]) diff --git a/apps/fg_http/lib/fg_http/sessions.ex b/apps/fg_http/lib/fg_http/sessions.ex index 0f34da247..98f623bf8 100644 --- a/apps/fg_http/lib/fg_http/sessions.ex +++ b/apps/fg_http/lib/fg_http/sessions.ex @@ -24,6 +24,8 @@ defmodule FgHttp.Sessions do """ def get_session!(email: email), do: Repo.get_by!(Session, email: email) def get_session!(id), do: Repo.get!(Session, id) + def get_session(email: email), do: Repo.get_by(Session, email: email) + def get_session(id), do: Repo.get(Session, id) @doc """ Creates a session. diff --git a/apps/fg_http/lib/fg_http/users.ex b/apps/fg_http/lib/fg_http/users.ex index 08f0a2b85..49ddb0b1a 100644 --- a/apps/fg_http/lib/fg_http/users.ex +++ b/apps/fg_http/lib/fg_http/users.ex @@ -8,100 +8,32 @@ defmodule FgHttp.Users do alias FgHttp.Users.User - @doc """ - Returns the list of users. - - ## Examples - - iex> list_users() - [%User{}, ...] - - """ def list_users do Repo.all(User) end - @doc """ - Gets a single user. - - Raises `Ecto.NoResultsError` if the User does not exist. - - ## Examples - - iex> get_user!(123) - %User{} - - iex> get_user!(456) - ** (Ecto.NoResultsError) - - """ def get_user!(email: email) do Repo.get_by!(User, email: email) end def get_user!(id), do: Repo.get!(User, id) - @doc """ - Creates a user. - - ## Examples - - iex> create_user(%{field: value}) - {:ok, %User{}} - - iex> create_user(%{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ def create_user(attrs \\ %{}) do %User{} |> User.create_changeset(attrs) |> Repo.insert() end - @doc """ - Updates a user. - - ## Examples - - iex> update_user(user, %{field: new_value}) - {:ok, %User{}} - - iex> update_user(user, %{field: bad_value}) - {:error, %Ecto.Changeset{}} - - """ def update_user(%User{} = user, attrs) do user |> User.update_changeset(attrs) |> Repo.update() end - @doc """ - Deletes a user. - - ## Examples - - iex> delete_user(user) - {:ok, %User{}} - - iex> delete_user(user) - {:error, %Ecto.Changeset{}} - - """ def delete_user(%User{} = user) do Repo.delete(user) end - @doc """ - Returns an `%Ecto.Changeset{}` for tracking user changes. - - ## Examples - - iex> change_user(user) - %Ecto.Changeset{source: %User{}} - - """ def change_user(%User{} = user) do User.changeset(user, %{}) end diff --git a/apps/fg_http/lib/fg_http/users/password_reset.ex b/apps/fg_http/lib/fg_http/users/password_reset.ex index 4444648a3..11cc51c78 100644 --- a/apps/fg_http/lib/fg_http/users/password_reset.ex +++ b/apps/fg_http/lib/fg_http/users/password_reset.ex @@ -20,19 +20,16 @@ defmodule FgHttp.Users.PasswordReset do field :email, :string end - @doc false def changeset do %__MODULE__{} |> cast(%{}, [:password, :password_confirmation, :reset_token]) end - @doc false def changeset(%__MODULE__{} = password_reset, attrs \\ %{}) do password_reset |> cast(attrs, [:password, :password_confirmation, :reset_token]) end - @doc false def create_changeset(%__MODULE__{} = password_reset, attrs) do password_reset |> cast(attrs, [:email, :reset_sent_at, :reset_token]) @@ -44,7 +41,6 @@ defmodule FgHttp.Users.PasswordReset do |> validate_required([:reset_sent_at]) end - @doc false def update_changeset(%__MODULE__{} = password_reset, attrs) do password_reset |> cast(attrs, [ diff --git a/apps/fg_http/lib/fg_http/users/session.ex b/apps/fg_http/lib/fg_http/users/session.ex index c44ef2021..ff701f423 100644 --- a/apps/fg_http/lib/fg_http/users/session.ex +++ b/apps/fg_http/lib/fg_http/users/session.ex @@ -16,12 +16,16 @@ defmodule FgHttp.Users.Session do def create_changeset(session, attrs \\ %{}) do session - |> cast(attrs, [:email, :password]) - |> validate_required([:email, :password]) + |> cast(attrs, [:email, :password, :last_signed_in_at]) + |> log_it() |> authenticate_user() |> set_last_signed_in_at() end + defp log_it(changeset) do + changeset + end + defp set_last_signed_in_at(%Ecto.Changeset{valid?: true} = changeset) do last_signed_in_at = DateTime.utc_now() change(changeset, last_signed_in_at: last_signed_in_at) @@ -32,22 +36,24 @@ defmodule FgHttp.Users.Session do defp authenticate_user( %Ecto.Changeset{ valid?: true, - changes: %{email: email, password: password} + changes: %{ + password: password + } } = changeset ) do - user = Users.get_user!(email: email) + session = changeset.data + user = Users.get_user!(email: session.email) case User.authenticate_user(user, password) do {:ok, _} -> - # Remove the user's password so it doesn't accidentally end up somewhere changeset - |> delete_change(:password) - |> change(%{id: user.id}) {:error, error_msg} -> - raise("There was an issue with your password: #{error_msg}") + add_error(changeset, :password, "invalid: #{error_msg}") end end - defp authenticate_user(changeset), do: delete_change(changeset, :password) + defp authenticate_user(changeset) do + changeset + end end diff --git a/apps/fg_http/lib/fg_http/users/user.ex b/apps/fg_http/lib/fg_http/users/user.ex index b8b3dcf25..e79599eb3 100644 --- a/apps/fg_http/lib/fg_http/users/user.ex +++ b/apps/fg_http/lib/fg_http/users/user.ex @@ -7,7 +7,7 @@ defmodule FgHttp.Users.User do import Ecto.Changeset import FgHttp.Users.PasswordHelpers - alias FgHttp.Devices.Device + alias FgHttp.{Devices.Device, Util.FgMap} schema "users" do field :email, :string @@ -25,11 +25,11 @@ defmodule FgHttp.Users.User do timestamps(type: :utc_datetime_usec) end - @doc false def create_changeset(user, attrs \\ %{}) do user |> cast(attrs, [:email, :password_hash, :password, :password_confirmation]) |> validate_required([:email, :password, :password_confirmation]) + |> validate_password_equality() |> unique_constraint(:email) |> put_password_hash() |> validate_required([:password_hash]) @@ -39,17 +39,26 @@ defmodule FgHttp.Users.User do def update_changeset( user, %{ - user: %{ - password: _password, - password_confirmation: _password_confirmation, - current_password: _current_password - } + "password" => nil, + "password_confirmation" => nil, + "current_password" => nil + } = attrs + ) do + update_changeset(user, FgMap.compact(attrs)) + end + + def update_changeset( + user, + %{ + "password" => _password, + "password_confirmation" => _password_confirmation, + "current_password" => _current_password } = attrs ) do user |> cast(attrs, [:email, :password, :password_confirmation, :current_password]) - |> verify_current_password(attrs[:current_password]) |> validate_required([:password, :password_confirmation, :current_password]) + |> verify_current_password(user) |> validate_password_equality() |> put_password_hash() |> validate_required([:password_hash]) @@ -59,10 +68,8 @@ defmodule FgHttp.Users.User do def update_changeset( user, %{ - user: %{ - password: _password, - password_confirmation: _password_confirmation - } + "password" => _password, + "password_confirmation" => _password_confirmation } = attrs ) do user @@ -74,22 +81,30 @@ defmodule FgHttp.Users.User do end # Only email being updated - def update_changeset(user, %{user: %{email: _email}} = attrs) do + def update_changeset(user, %{"email" => _email} = attrs) do user |> cast(attrs, [:email]) |> validate_required([:email]) end - def changeset(%__MODULE__{} = _user, _attrs \\ %{}) do - change(%__MODULE__{}) + def changeset(user, attrs) do + user + |> cast(attrs, [:email, :confirmed_at, :last_signed_in_at]) end def authenticate_user(user, password_candidate) do Argon2.check_pass(user, password_candidate) end - defp verify_current_password(user, current_password) do - {:ok, user} = authenticate_user(user, current_password) - user + defp verify_current_password(changeset, user) do + case authenticate_user(user, changeset.changes.current_password) do + {:ok, _user} -> + changeset + |> delete_change(:current_password) + + {:error, error_msg} -> + changeset + |> add_error(:current_password, "is invalid: #{error_msg}") + end end end diff --git a/apps/fg_http/lib/fg_http/util/fg_map.ex b/apps/fg_http/lib/fg_http/util/fg_map.ex new file mode 100644 index 000000000..1e39ebb64 --- /dev/null +++ b/apps/fg_http/lib/fg_http/util/fg_map.ex @@ -0,0 +1,12 @@ +defmodule FgHttp.Util.FgMap do + @moduledoc """ + Utilities for working with Maps + """ + + @doc """ + Removes key, value pairs from a Map if the value is nil + """ + def compact(%{} = map) do + for {k, v} <- map, v != nil, into: %{}, do: {k, v} + end +end diff --git a/apps/fg_http/lib/fg_http_web/controllers/device_controller.ex b/apps/fg_http/lib/fg_http_web/controllers/device_controller.ex index 0d26a9198..f6643d3cd 100644 --- a/apps/fg_http/lib/fg_http_web/controllers/device_controller.ex +++ b/apps/fg_http/lib/fg_http_web/controllers/device_controller.ex @@ -28,8 +28,8 @@ defmodule FgHttpWeb.DeviceController do all_params = Map.merge(device_params, our_params) case Devices.create_device(all_params) do - {:ok, device} -> - redirect(conn, to: Routes.device_path(conn, :show, device)) + {:ok, _device} -> + redirect(conn, to: Routes.device_path(conn, :index)) {:error, %Ecto.Changeset{} = changeset} -> render(conn, "new.html", changeset: changeset) diff --git a/apps/fg_http/lib/fg_http_web/controllers/rule_controller.ex b/apps/fg_http/lib/fg_http_web/controllers/rule_controller.ex index deb6df527..7f2d440fc 100644 --- a/apps/fg_http/lib/fg_http_web/controllers/rule_controller.ex +++ b/apps/fg_http/lib/fg_http_web/controllers/rule_controller.ex @@ -28,9 +28,9 @@ defmodule FgHttpWeb.RuleController do {:ok, rule} -> conn |> put_flash(:info, "Rule created successfully.") - |> redirect(to: Routes.rule_path(conn, :show, rule)) + |> redirect(to: Routes.device_rule_path(conn, :index, rule.device_id)) - {:error, %Ecto.Changeset{} = changeset} -> + {:error, changeset} -> device = Devices.get_device!(device_id) render(conn, "new.html", device: device, changeset: changeset) end @@ -55,19 +55,20 @@ defmodule FgHttpWeb.RuleController do {:ok, rule} -> conn |> put_flash(:info, "Rule updated successfully.") - |> redirect(to: Routes.rule_path(conn, :show, rule)) + |> redirect(to: Routes.device_rule_path(conn, :index, rule.device_id)) - {:error, %Ecto.Changeset{} = changeset} -> + {:error, changeset} -> render(conn, "edit.html", rule: rule, changeset: changeset) end end def delete(conn, %{"id" => id}) do rule = Rules.get_rule!(id) + device_id = rule.device_id {:ok, _rule} = Rules.delete_rule(rule) conn |> put_flash(:info, "Rule deleted successfully.") - |> redirect(to: Routes.rule_path(conn, :index, rule.device)) + |> redirect(to: Routes.device_rule_path(conn, :index, device_id)) end end diff --git a/apps/fg_http/lib/fg_http_web/controllers/session_controller.ex b/apps/fg_http/lib/fg_http_web/controllers/session_controller.ex index 5b42e7e65..6ff135490 100644 --- a/apps/fg_http/lib/fg_http_web/controllers/session_controller.ex +++ b/apps/fg_http/lib/fg_http_web/controllers/session_controller.ex @@ -16,7 +16,7 @@ defmodule FgHttpWeb.SessionController do # POST /sessions def create(conn, %{"session" => %{"email" => email} = session_params}) do - case Sessions.get_session!(email: email) do + case Sessions.get_session(email: email) do %Session{} = session -> case Sessions.create_session(session, session_params) do {:ok, session} -> diff --git a/apps/fg_http/lib/fg_http_web/controllers/user_controller.ex b/apps/fg_http/lib/fg_http_web/controllers/user_controller.ex index 1892dea81..de12f65fc 100644 --- a/apps/fg_http/lib/fg_http_web/controllers/user_controller.ex +++ b/apps/fg_http/lib/fg_http_web/controllers/user_controller.ex @@ -4,9 +4,10 @@ defmodule FgHttpWeb.UserController do """ use FgHttpWeb, :controller - alias FgHttp.{Users, Users.Session, Users.User} + alias FgHttp.{Sessions, Users, Users.User} plug FgHttpWeb.Plugs.SessionLoader when action in [:show, :edit, :update, :delete] + plug :scrub_params, "user" when action in [:update] # GET /users/new def new(conn, _params) do @@ -18,9 +19,12 @@ defmodule FgHttpWeb.UserController do def create(conn, %{"user" => user_params}) do case Users.create_user(user_params) do {:ok, user} -> + # XXX: Cast the user to a session struct to prevent this db call + session = Sessions.get_session!(user.id) + conn |> put_session(:user_id, user.id) - |> assign(:session, %Session{id: user.id, email: user.email}) + |> assign(:session, session) |> put_flash(:info, "User created successfully.") |> redirect(to: Routes.device_path(conn, :index)) @@ -33,7 +37,8 @@ defmodule FgHttpWeb.UserController do # GET /user/edit def edit(conn, _params) do - user = conn.current_user + # XXX: Cast the session to a user struct to prevent this db call + user = Users.get_user!(conn.assigns.session.id) changeset = Users.change_user(user) render(conn, "edit.html", changeset: changeset) @@ -41,18 +46,26 @@ defmodule FgHttpWeb.UserController do # GET /user def show(conn, _params) do + # XXX: Cast the session to a user struct to prevent this db call + user = Users.get_user!(conn.assigns.session.id) + conn - |> render("show.html", user: conn.current_user) + |> render("show.html", user: user, session: conn.assigns.session) end # PATCH /user - def update(conn, params) do - case Users.update_user(conn.current_user, params) do + def update(conn, %{"user" => user_params}) do + user = Users.get_user!(conn.assigns.session.id) + + case Users.update_user(user, user_params) do {:ok, user} -> + # XXX: Cast the user to a session struct to prevent this db call + session = Sessions.get_session!(user.id) + conn - |> assign(:current_user, user) + |> assign(:session, session) |> put_flash(:info, "User updated successfully.") - |> redirect(to: Routes.user_path(conn, :show, conn.current_user)) + |> redirect(to: Routes.user_path(conn, :show)) {:error, changeset} -> conn @@ -63,17 +76,20 @@ defmodule FgHttpWeb.UserController do # DELETE /user def delete(conn, _params) do - case Users.delete_user(conn.current_user) do + user = Users.get_user!(conn.assigns.session.id) + + case Users.delete_user(user) do {:ok, _user} -> conn - |> assign(:current_user, nil) + |> clear_session() + |> assign(:session, nil) |> put_flash(:info, "User deleted successfully.") |> redirect(to: "/") - {:error, _changeset} -> + {:error, changeset} -> conn |> put_flash(:error, "Error deleting User.") - |> redirect(to: Routes.user_path(:show, conn.current_user)) + |> render("edit.html", changeset: changeset) end end end diff --git a/apps/fg_http/lib/fg_http_web/plugs/redirect_authenticated.ex b/apps/fg_http/lib/fg_http_web/plugs/redirect_authenticated.ex index 433838ee7..ce5b7e41f 100644 --- a/apps/fg_http/lib/fg_http_web/plugs/redirect_authenticated.ex +++ b/apps/fg_http/lib/fg_http_web/plugs/redirect_authenticated.ex @@ -5,17 +5,17 @@ defmodule FgHttpWeb.Plugs.RedirectAuthenticated do import Plug.Conn import Phoenix.Controller, only: [redirect: 2] + alias FgHttpWeb.Router.Helpers, as: Routes def init(default), do: default def call(conn, _default) do - if get_session(conn, :email) do + if get_session(conn, :user_id) do conn - |> redirect(to: "/") + |> redirect(to: Routes.device_path(conn, :index)) |> halt() else conn - |> assign(:session, nil) end end end diff --git a/apps/fg_http/lib/fg_http_web/plugs/session_loader.ex b/apps/fg_http/lib/fg_http_web/plugs/session_loader.ex index 0c1a66857..37a5121ab 100644 --- a/apps/fg_http/lib/fg_http_web/plugs/session_loader.ex +++ b/apps/fg_http/lib/fg_http_web/plugs/session_loader.ex @@ -10,16 +10,13 @@ defmodule FgHttpWeb.Plugs.SessionLoader do def init(default), do: default - # Don't load an already loaded session - def call(%Plug.Conn{assigns: %{session: %Session{}}} = conn, _default), do: conn - def call(conn, _default) do case get_session(conn, :user_id) do nil -> unauthed(conn) user_id -> - case Sessions.get_session!(user_id) do + case Sessions.get_session(user_id) do %Session{} = session -> conn |> assign(:session, session) @@ -32,6 +29,7 @@ defmodule FgHttpWeb.Plugs.SessionLoader do defp unauthed(conn) do conn + |> clear_session() |> put_flash(:error, "Please sign in to access that page.") |> redirect(to: Routes.session_path(conn, :new)) |> halt() diff --git a/apps/fg_http/lib/fg_http_web/router.ex b/apps/fg_http/lib/fg_http_web/router.ex index ff9841bb3..7169ea690 100644 --- a/apps/fg_http/lib/fg_http_web/router.ex +++ b/apps/fg_http/lib/fg_http_web/router.ex @@ -37,7 +37,8 @@ defmodule FgHttpWeb.Router do resources "/rules", RuleController, only: [:show, :update, :delete, :edit] - resources "/sessions", SessionController, only: [:new, :create, :delete] + resources "/session", SessionController, singleton: true, only: [:delete] + resources "/sessions", SessionController, only: [:new, :create] get "/", SessionController, :new end diff --git a/apps/fg_http/lib/fg_http_web/templates/layout/app.html.eex b/apps/fg_http/lib/fg_http_web/templates/layout/app.html.eex index a2aa2d9cf..9a0bf8344 100644 --- a/apps/fg_http/lib/fg_http_web/templates/layout/app.html.eex +++ b/apps/fg_http/lib/fg_http_web/templates/layout/app.html.eex @@ -16,8 +16,9 @@