From 98f4166ea81ee811442ef5ee817da55edadf93c9 Mon Sep 17 00:00:00 2001 From: liamwhite Date: Tue, 28 Jul 2020 16:56:26 -0400 Subject: [PATCH] Replace Pow with generated Phoenix auth (#10) --- .github/workflows/elixir.yml | 12 +- .gitignore | 3 + config/config.exs | 11 - config/dev.exs | 2 +- config/prod.secret.exs | 2 +- config/test.exs | 11 +- lib/philomena/application.ex | 6 - lib/{philomena_web => philomena}/mailer.ex | 2 +- lib/philomena/mnesia_cluster_supervisor.ex | 17 - lib/philomena/tags/tag.ex | 5 +- lib/philomena/users.ex | 475 +++++++++++-- lib/philomena/users/user.ex | 200 ++++-- lib/philomena/users/user_notifier.ex | 101 +++ lib/philomena/users/user_token.ex | 163 +++++ .../controllers/commission_controller.ex | 4 +- .../controllers/confirmation_controller.ex | 45 ++ .../controllers/password_controller.ex | 61 ++ .../registration/email_controller.ex | 44 ++ .../registration/password_controller.ex | 25 + .../registration/totp_controller.ex | 10 +- .../controllers/registration_controller.ex | 48 ++ .../controllers/session/totp_controller.ex | 16 +- .../controllers/session_controller.ex | 42 ++ .../controllers/unlock_controller.ex | 45 ++ lib/philomena_web/endpoint.ex | 16 - lib/philomena_web/plugs/api_token_plug.ex | 10 +- lib/philomena_web/plugs/captcha_plug.ex | 12 +- .../plugs/compromised_password_check_plug.ex | 9 +- lib/philomena_web/plugs/current_ban_plug.ex | 7 +- .../plugs/current_filter_plug.ex | 6 +- .../plugs/ensure_user_enabled_plug.ex | 18 +- lib/philomena_web/plugs/filter_select_plug.ex | 5 +- lib/philomena_web/plugs/image_filter_plug.ex | 3 +- .../plugs/notification_count_plug.ex | 8 +- lib/philomena_web/plugs/pagination_plug.ex | 5 +- .../plugs/pow_invalidated_session_plug.ex | 176 ----- lib/philomena_web/plugs/reload_user_plug.ex | 33 - lib/philomena_web/plugs/require_user_plug.ex | 3 +- lib/philomena_web/plugs/tor_plug.ex | 2 +- lib/philomena_web/plugs/totp_plug.ex | 39 +- .../plugs/user_attribution_plug.ex | 2 +- lib/philomena_web/pow_mailer.ex | 28 - lib/philomena_web/router.ex | 82 +-- .../templates/avatar/edit.html.slime | 4 +- .../subscription/_subscription.html.slime | 4 +- .../templates/confirmation/new.html.slime | 12 + .../subscription/_subscription.html.slime | 4 +- .../subscription/_subscription.html.slime | 4 +- .../image/_add_to_gallery_dropdown.html.slime | 4 +- .../templates/image/reporting/show.html.slime | 2 +- .../subscription/_subscription.html.slime | 2 +- .../templates/layout/_header.html.slime | 8 +- .../edit.html.slime | 7 +- .../templates/password/new.html.slime | 14 + .../templates/pow/session/new.html.slime | 28 - .../reset_password/new.html.slime | 18 - .../{pow => }/registration/edit.html.slime | 45 +- .../registration/name/edit.html.slime | 2 +- .../{pow => }/registration/new.html.slime | 6 +- .../registration/totp/edit.html.slime | 2 +- .../templates/session/new.html.slime | 28 + .../templates/session/totp/new.html.slime | 4 + .../templates/setting/edit.html.slime | 2 +- .../subscription/_subscription.html.slime | 4 +- .../templates/unlock/new.html.slime | 12 + lib/philomena_web/user_auth.ex | 196 ++++++ lib/philomena_web/views/confirmation_view.ex | 3 + lib/philomena_web/views/password_view.ex | 3 + .../views/pow/registration_view.ex | 3 - lib/philomena_web/views/pow/session_view.ex | 3 - .../pow_reset_password/reset_password_view.ex | 3 - lib/philomena_web/views/registration_view.ex | 3 + lib/philomena_web/views/session_view.ex | 3 + lib/philomena_web/views/unlock_view.ex | 3 + lib/pow_captcha/captcha.ex | 7 - .../controllers/controller_callbacks.ex | 45 -- lib/pow_lockout/ecto/context.ex | 63 -- lib/pow_lockout/ecto/schema.ex | 132 ---- lib/pow_lockout/lockout.ex | 16 - .../controllers/controller_callbacks.ex | 93 --- .../phoenix/controllers/unlock_controller.ex | 23 - lib/pow_lockout/phoenix/mailers/mailer.ex | 11 - .../phoenix/mailers/mailer_template.ex | 25 - .../phoenix/mailers/mailer_view.ex | 4 - lib/pow_lockout/phoenix/messages.ex | 13 - lib/pow_lockout/phoenix/router.ex | 12 - lib/pow_lockout/plug.ex | 86 --- mix.exs | 7 +- ...0200725234412_create_users_auth_tables.exs | 31 + priv/repo/seeds.exs | 11 +- priv/repo/seeds.json | 1 - priv/repo/seeds_development.exs | 12 +- priv/repo/seeds_development.json | 3 - priv/repo/structure.sql | 90 ++- test/philomena/users_test.exs | 627 ++++++++++++++++++ .../confirmation_controller_test.exs | 84 +++ .../controllers/password_controller_test.exs | 111 ++++ .../registration/email_controller_test.exs | 71 ++ .../registration/password_controller_test.exs | 41 ++ .../registration_controller_test.exs | 64 ++ .../controllers/session_controller_test.exs | 73 ++ .../controllers/unlock_controller_test.exs | 84 +++ .../pow_invalidated_session_plug_test.exs | 199 ------ test/philomena_web/user_auth_test.exs | 193 ++++++ test/support/conn_case.ex | 41 +- test/support/fixtures/users_fixtures.ex | 45 ++ 106 files changed, 3188 insertions(+), 1380 deletions(-) rename lib/{philomena_web => philomena}/mailer.ex (57%) delete mode 100644 lib/philomena/mnesia_cluster_supervisor.ex create mode 100644 lib/philomena/users/user_notifier.ex create mode 100644 lib/philomena/users/user_token.ex create mode 100644 lib/philomena_web/controllers/confirmation_controller.ex create mode 100644 lib/philomena_web/controllers/password_controller.ex create mode 100644 lib/philomena_web/controllers/registration/email_controller.ex create mode 100644 lib/philomena_web/controllers/registration/password_controller.ex create mode 100644 lib/philomena_web/controllers/registration_controller.ex create mode 100644 lib/philomena_web/controllers/session_controller.ex create mode 100644 lib/philomena_web/controllers/unlock_controller.ex delete mode 100644 lib/philomena_web/plugs/pow_invalidated_session_plug.ex delete mode 100644 lib/philomena_web/plugs/reload_user_plug.ex delete mode 100644 lib/philomena_web/pow_mailer.ex create mode 100644 lib/philomena_web/templates/confirmation/new.html.slime rename lib/philomena_web/templates/{pow_reset_password/reset_password => password}/edit.html.slime (70%) create mode 100644 lib/philomena_web/templates/password/new.html.slime delete mode 100644 lib/philomena_web/templates/pow/session/new.html.slime delete mode 100644 lib/philomena_web/templates/pow_reset_password/reset_password/new.html.slime rename lib/philomena_web/templates/{pow => }/registration/edit.html.slime (55%) rename lib/philomena_web/templates/{pow => }/registration/new.html.slime (88%) create mode 100644 lib/philomena_web/templates/session/new.html.slime create mode 100644 lib/philomena_web/templates/unlock/new.html.slime create mode 100644 lib/philomena_web/user_auth.ex create mode 100644 lib/philomena_web/views/confirmation_view.ex create mode 100644 lib/philomena_web/views/password_view.ex delete mode 100644 lib/philomena_web/views/pow/registration_view.ex delete mode 100644 lib/philomena_web/views/pow/session_view.ex delete mode 100644 lib/philomena_web/views/pow_reset_password/reset_password_view.ex create mode 100644 lib/philomena_web/views/registration_view.ex create mode 100644 lib/philomena_web/views/session_view.ex create mode 100644 lib/philomena_web/views/unlock_view.ex delete mode 100644 lib/pow_captcha/captcha.ex delete mode 100644 lib/pow_captcha/phoenix/controllers/controller_callbacks.ex delete mode 100644 lib/pow_lockout/ecto/context.ex delete mode 100644 lib/pow_lockout/ecto/schema.ex delete mode 100644 lib/pow_lockout/lockout.ex delete mode 100644 lib/pow_lockout/phoenix/controllers/controller_callbacks.ex delete mode 100644 lib/pow_lockout/phoenix/controllers/unlock_controller.ex delete mode 100644 lib/pow_lockout/phoenix/mailers/mailer.ex delete mode 100644 lib/pow_lockout/phoenix/mailers/mailer_template.ex delete mode 100644 lib/pow_lockout/phoenix/mailers/mailer_view.ex delete mode 100644 lib/pow_lockout/phoenix/messages.ex delete mode 100644 lib/pow_lockout/phoenix/router.ex delete mode 100644 lib/pow_lockout/plug.ex create mode 100644 priv/repo/migrations/20200725234412_create_users_auth_tables.exs create mode 100644 test/philomena/users_test.exs create mode 100644 test/philomena_web/controllers/confirmation_controller_test.exs create mode 100644 test/philomena_web/controllers/password_controller_test.exs create mode 100644 test/philomena_web/controllers/registration/email_controller_test.exs create mode 100644 test/philomena_web/controllers/registration/password_controller_test.exs create mode 100644 test/philomena_web/controllers/registration_controller_test.exs create mode 100644 test/philomena_web/controllers/session_controller_test.exs create mode 100644 test/philomena_web/controllers/unlock_controller_test.exs delete mode 100644 test/philomena_web/plug/pow_invalidated_session_plug_test.exs create mode 100644 test/philomena_web/user_auth_test.exs create mode 100644 test/support/fixtures/users_fixtures.ex diff --git a/.github/workflows/elixir.yml b/.github/workflows/elixir.yml index 508dcfdf..5fe84d75 100644 --- a/.github/workflows/elixir.yml +++ b/.github/workflows/elixir.yml @@ -20,6 +20,9 @@ jobs: --health-interval 10s --health-timeout 5s --health-retries 5 + elasticsearch: + image: elasticsearch:7.6.2 + ports: ['9200:9200'] redis: image: redis:5.0.9 ports: ['6379:6379'] @@ -30,15 +33,22 @@ jobs: apt-get update apt-get -yqq install libpq-dev postgresql-client - name: Install Dependencies + env: + MIX_ENV: 'test' run: | mix local.rebar --force mix local.hex --force mix deps.get - name: Run Tests env: + MIX_ENV: 'test' PGUSER: 'postgres' PGPASSWORD: 'postgres' PGDATABASE: 'philomena_test' PGPORT: 5432 PGHOST: db - run: mix test + run: | + mix ecto.create + mix ecto.load + mix reindex_all + mix test diff --git a/.gitignore b/.gitignore index 37f47d92..18245583 100644 --- a/.gitignore +++ b/.gitignore @@ -42,5 +42,8 @@ npm-debug.log # Intellij IDEA .idea +# ElixirLS +.elixir_ls + # Index dumps *.jsonl diff --git a/config/config.exs b/config/config.exs index b84e4493..cbe58a61 100644 --- a/config/config.exs +++ b/config/config.exs @@ -31,17 +31,6 @@ config :philomena, proxy_host: nil, app_dir: File.cwd!() -config :philomena, :pow, - user: Philomena.Users.User, - repo: Philomena.Repo, - web_module: PhilomenaWeb, - users_context: Philomena.Users, - extensions: [PowResetPassword, PowLockout, PowCaptcha, PowPersistentSession], - controller_callbacks: Pow.Extension.Phoenix.ControllerCallbacks, - mailer_backend: PhilomenaWeb.PowMailer, - captcha_verifier: Philomena.Captcha, - cache_store_backend: Pow.Store.Backend.MnesiaCache - config :exq, queues: [{"videos", 2}, {"images", 4}, {"indexing", 16}], scheduler_enable: true, diff --git a/config/dev.exs b/config/dev.exs index 7fbec076..443e4ac5 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -71,7 +71,7 @@ config :logger, :console, format: "[$level] $message\n" config :logger, compile_time_purge_matching: [[application: :remote_ip], [application: :mint]] # Set up mailer -config :philomena, PhilomenaWeb.Mailer, adapter: Bamboo.LocalAdapter +config :philomena, Philomena.Mailer, adapter: Bamboo.LocalAdapter config :philomena, :mailer_address, "noreply@philomena.lc" diff --git a/config/prod.secret.exs b/config/prod.secret.exs index 038f61d4..ef81571a 100644 --- a/config/prod.secret.exs +++ b/config/prod.secret.exs @@ -43,7 +43,7 @@ config :philomena, Philomena.Repo, url: database_url, pool_size: String.to_integer(System.get_env("POOL_SIZE") || "32") -config :philomena, PhilomenaWeb.Mailer, +config :philomena, Philomena.Mailer, adapter: Bamboo.SMTPAdapter, server: System.get_env("SMTP_RELAY"), hostname: System.get_env("SMTP_DOMAIN"), diff --git a/config/test.exs b/config/test.exs index ce412172..6291bab8 100644 --- a/config/test.exs +++ b/config/test.exs @@ -1,5 +1,8 @@ import Config +# Only in tests, remove the complexity from the password hashing algorithm +config :bcrypt_elixir, :log_rounds, 1 + # Configure your database config :philomena, Philomena.Repo, username: "postgres", @@ -8,11 +11,17 @@ config :philomena, Philomena.Repo, pool: Ecto.Adapters.SQL.Sandbox config :philomena, - redis_host: "redis" + elasticsearch_url: "http://elasticsearch:9200", + redis_host: "redis", + pwned_passwords: false, + captcha: false config :exq, host: "redis" +config :philomena, Philomena.Mailer, adapter: Bamboo.LocalAdapter +config :philomena, :mailer_address, "test@philomena.lc" + # We don't run a server during test. If one is required, # you can enable the server option below. config :philomena, PhilomenaWeb.Endpoint, diff --git a/lib/philomena/application.ex b/lib/philomena/application.ex index fc74de6b..4413bf01 100644 --- a/lib/philomena/application.ex +++ b/lib/philomena/application.ex @@ -8,12 +8,6 @@ defmodule Philomena.Application do def start(_type, _args) do # List all child processes to be supervised children = [ - # Connect to cluster nodes - {Cluster.Supervisor, [[philomena: [strategy: Cluster.Strategy.ErlangHosts]]]}, - - # Session storage - Philomena.MnesiaClusterSupervisor, - # Start the Ecto repository Philomena.Repo, diff --git a/lib/philomena_web/mailer.ex b/lib/philomena/mailer.ex similarity index 57% rename from lib/philomena_web/mailer.ex rename to lib/philomena/mailer.ex index 1bde668c..93bed3ba 100644 --- a/lib/philomena_web/mailer.ex +++ b/lib/philomena/mailer.ex @@ -1,3 +1,3 @@ -defmodule PhilomenaWeb.Mailer do +defmodule Philomena.Mailer do use Bamboo.Mailer, otp_app: :philomena end diff --git a/lib/philomena/mnesia_cluster_supervisor.ex b/lib/philomena/mnesia_cluster_supervisor.ex deleted file mode 100644 index 50328122..00000000 --- a/lib/philomena/mnesia_cluster_supervisor.ex +++ /dev/null @@ -1,17 +0,0 @@ -defmodule Philomena.MnesiaClusterSupervisor do - use Supervisor - - def start_link(init_arg) do - Supervisor.start_link(__MODULE__, init_arg, name: __MODULE__) - end - - @impl true - def init(_init_arg) do - children = [ - {Pow.Store.Backend.MnesiaCache, extra_db_nodes: Node.list()}, - Pow.Store.Backend.MnesiaCache.Unsplit - ] - - Supervisor.init(children, strategy: :one_for_one) - end -end diff --git a/lib/philomena/tags/tag.ex b/lib/philomena/tags/tag.ex index a3a0d475..8c2fba2c 100644 --- a/lib/philomena/tags/tag.ex +++ b/lib/philomena/tags/tag.ex @@ -169,7 +169,10 @@ defmodule Philomena.Tags.Tag do # with ascii quotes, trim space from end name |> String.downcase() - |> String.replace(~r/[[:space:]\x{00a0}\x{1680}\x{180e}\x{2000}-\x{200f}\x{202f}\x{205f}\x{3000}\x{feff}]+/u, " ") + |> String.replace( + ~r/[[:space:]\x{00a0}\x{1680}\x{180e}\x{2000}-\x{200f}\x{202f}\x{205f}\x{3000}\x{feff}]+/u, + " " + ) |> String.replace(~r/[\x{00b4}\x{2018}\x{2019}\x{201a}\x{201b}\x{2032}]/u, "'") |> String.replace(~r/[\x{201c}\x{201d}\x{201e}\x{201f}\x{2033}]/u, "\"") |> String.trim() diff --git a/lib/philomena/users.ex b/lib/philomena/users.ex index db285491..a4e1e7b5 100644 --- a/lib/philomena/users.ex +++ b/lib/philomena/users.ex @@ -7,8 +7,7 @@ defmodule Philomena.Users do alias Ecto.Multi alias Philomena.Repo - alias Philomena.Users.Uploader - alias Philomena.Users.User + alias Philomena.Users.{User, UserToken, UserNotifier, Uploader} alias Philomena.{Forums, Forums.Forum} alias Philomena.Topics alias Philomena.Roles.Role @@ -19,21 +18,87 @@ defmodule Philomena.Users do alias Philomena.Galleries alias Philomena.Reports - use Pow.Ecto.Context, - repo: Repo, - user: User + ## Database getters @doc """ - Returns the list of users. + Gets a user by API token. ## Examples - iex> list_users() - [%User{}, ...] + iex> get_user_by_authentication_token("5Ow89k7nW24E0K34d3zX") + %User{} + + iex> get_user_by_authentication_token("invalid") + nil """ - def list_users do - Repo.all(User) + def get_user_by_authentication_token(token) when is_binary(token) do + Repo.get_by(User, authentication_token: token) + end + + @doc """ + Gets a user by email. + + ## Examples + + iex> get_user_by_email("foo@example.com") + %User{} + + iex> get_user_by_email("unknown@example.com") + nil + + """ + def get_user_by_email(email) when is_binary(email) do + Repo.get_by(User, email: email) + end + + @doc """ + Gets a user by email and password. + + ## Examples + + iex> get_user_by_email_and_password("foo@example.com", "correct_password") + %User{} + + iex> get_user_by_email_and_password("foo@example.com", "invalid_password") + nil + + """ + def get_user_by_email_and_password(email, password, unlock_url_fun) + when is_binary(email) and is_binary(password) do + user = Repo.get_by(User, email: email) + + cond do + is_nil(user) or not is_nil(user.locked_at) -> + nil + + User.valid_password?(user, password) -> + user + |> User.successful_attempt_changeset() + |> Repo.update!() + + true -> + user + |> User.failed_attempt_changeset() + |> Repo.update!() + |> maybe_send_unlock_instructions(unlock_url_fun) + + nil + end + end + + defp maybe_send_unlock_instructions(%{failed_attempts: attempts}, _unlock_url_fun) + when attempts < 10 do + nil + end + + defp maybe_send_unlock_instructions(%User{} = user, unlock_url_fun) do + user + |> User.lock_changeset() + |> Repo.update!() + |> deliver_user_unlock_instructions(unlock_url_fun) + + nil end @doc """ @@ -52,24 +117,368 @@ defmodule Philomena.Users do """ def get_user!(id), do: Repo.get!(User, id) + ## User registration + @doc """ - Creates a user. + Registers a user. ## Examples - iex> create_user(%{field: value}) + iex> register_user(%{field: value}) {:ok, %User{}} - iex> create_user(%{field: bad_value}) + iex> register_user(%{field: bad_value}) {:error, %Ecto.Changeset{}} """ - def create_user(attrs \\ %{}) do + def register_user(attrs) do %User{} - |> User.changeset(attrs) + |> User.registration_changeset(attrs) |> Repo.insert() end + @doc """ + Returns an `%Ecto.Changeset{}` for tracking user changes. + + ## Examples + + iex> change_user_registration(user) + %Ecto.Changeset{data: %User{}} + + """ + def change_user_registration(%User{} = user, attrs \\ %{}) do + User.registration_changeset(user, attrs) + end + + ## Settings + + @doc """ + Returns an `%Ecto.Changeset{}` for changing the user email. + + ## Examples + + iex> change_user_email(user) + %Ecto.Changeset{data: %User{}} + + """ + def change_user_email(user, attrs \\ %{}) do + User.email_changeset(user, attrs) + end + + @doc """ + Emulates that the email will change without actually changing + it in the database. + + ## Examples + + iex> apply_user_email(user, "valid password", %{email: ...}) + {:ok, %User{}} + + iex> apply_user_email(user, "invalid password", %{email: ...}) + {:error, %Ecto.Changeset{}} + + """ + def apply_user_email(user, password, attrs) do + user + |> User.email_changeset(attrs) + |> User.validate_current_password(password) + |> Ecto.Changeset.apply_action(:update) + end + + @doc """ + Updates the user email in token. + + If the token matches, the user email is updated and the token is deleted. + The confirmed_at date is also updated to the current time. + """ + def update_user_email(user, token) do + context = "change:#{user.email}" + + with {:ok, query} <- UserToken.verify_change_email_token_query(token, context), + %UserToken{sent_to: email} <- Repo.one(query), + {:ok, _} <- Repo.transaction(user_email_multi(user, email, context)) do + :ok + else + _ -> :error + end + end + + defp user_email_multi(user, email, context) do + changeset = user |> User.email_changeset(%{email: email}) |> User.confirm_changeset() + + Ecto.Multi.new() + |> Ecto.Multi.update(:user, changeset) + |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, [context])) + end + + @doc """ + Delivers the update email instructions to the given user. + + ## Examples + + iex> deliver_update_email_instructions(user, current_email, &Routes.user_update_email_url(conn, :edit, &1)) + {:ok, %{to: ..., body: ...}} + + """ + def deliver_update_email_instructions(%User{} = user, current_email, update_email_url_fun) + when is_function(update_email_url_fun, 1) do + {encoded_token, user_token} = UserToken.build_email_token(user, "change:#{current_email}") + + Repo.insert!(user_token) + UserNotifier.deliver_update_email_instructions(user, update_email_url_fun.(encoded_token)) + end + + @doc """ + Unlocks the user by the given token. + + If the token matches, the user is marked as unlocked + and the token is deleted. + """ + def unlock_user(token) do + with {:ok, query} <- UserToken.verify_email_token_query(token, "unlock"), + %User{} = user <- Repo.one(query), + {:ok, %{user: user}} <- Repo.transaction(unlock_user_multi(user)) do + {:ok, user} + else + _ -> :error + end + end + + defp unlock_user_multi(user) do + changeset = User.unlock_changeset(user) + + Ecto.Multi.new() + |> Ecto.Multi.update(:user, changeset) + |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, ["unlock"])) + end + + @doc """ + Delivers the unlock instructions to the given user. + + ## Examples + + iex> deliver_user_unlock_instructions(user, &Routes.unlock_url(conn, :show, &1)) + {:ok, %{to: ..., body: ...}} + + """ + def deliver_user_unlock_instructions(%User{} = user, unlock_url_fun) + when is_function(unlock_url_fun, 1) do + if is_nil(user.locked_at) do + {:error, :not_locked} + else + {encoded_token, user_token} = UserToken.build_email_token(user, "unlock") + Repo.insert!(user_token) + UserNotifier.deliver_unlock_instructions(user, unlock_url_fun.(encoded_token)) + end + end + + @doc """ + Returns an `%Ecto.Changeset{}` for changing the user password. + + ## Examples + + iex> change_user_password(user) + %Ecto.Changeset{data: %User{}} + + """ + def change_user_password(user, attrs \\ %{}) do + User.password_changeset(user, attrs) + end + + @doc """ + Updates the user password. + + ## Examples + + iex> update_user_password(user, "valid password", %{password: ...}) + {:ok, %User{}} + + iex> update_user_password(user, "invalid password", %{password: ...}) + {:error, %Ecto.Changeset{}} + + """ + def update_user_password(user, password, attrs) do + changeset = + user + |> User.password_changeset(attrs) + |> User.validate_current_password(password) + + Ecto.Multi.new() + |> Ecto.Multi.update(:user, changeset) + |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) + |> Repo.transaction() + |> case do + {:ok, %{user: user}} -> {:ok, user} + {:error, :user, changeset, _} -> {:error, changeset} + end + end + + ## Session + + @doc """ + Generates a session token. + """ + def generate_user_session_token(user) do + {token, user_token} = UserToken.build_session_token(user) + Repo.insert!(user_token) + token + end + + @doc """ + Generates a TOTP token. + """ + def generate_user_totp_token(user) do + {token, user_token} = UserToken.build_totp_token(user) + Repo.insert!(user_token) + token + end + + @doc """ + Gets the user with the given signed token. + """ + def get_user_by_session_token(token) do + {:ok, query} = UserToken.verify_session_token_query(token) + load_with_roles(query) + end + + def user_totp_token_valid?(nil, _token) do + false + end + + def user_totp_token_valid?(user, token) do + {:ok, query} = UserToken.verify_totp_token_query(user, token) + Repo.exists?(query) + end + + @doc """ + Deletes the signed token with the given context. + """ + def delete_session_token(token) do + Repo.delete_all(UserToken.token_and_context_query(token, "session")) + :ok + end + + @doc """ + Deletes the signed token with the given context. + """ + def delete_totp_token(token) do + Repo.delete_all(UserToken.token_and_context_query(token, "totp")) + :ok + end + + ## Confirmation + + @doc """ + Delivers the confirmation email instructions to the given user. + + ## Examples + + iex> deliver_user_confirmation_instructions(user, &Routes.user_confirmation_url(conn, :confirm, &1)) + {:ok, %{to: ..., body: ...}} + + iex> deliver_user_confirmation_instructions(confirmed_user, &Routes.user_confirmation_url(conn, :confirm, &1)) + {:error, :already_confirmed} + + """ + def deliver_user_confirmation_instructions(%User{} = user, confirmation_url_fun) + when is_function(confirmation_url_fun, 1) do + if user.confirmed_at do + {:error, :already_confirmed} + else + {encoded_token, user_token} = UserToken.build_email_token(user, "confirm") + Repo.insert!(user_token) + UserNotifier.deliver_confirmation_instructions(user, confirmation_url_fun.(encoded_token)) + end + end + + @doc """ + Confirms a user by the given token. + + If the token matches, the user account is marked as confirmed + and the token is deleted. + """ + def confirm_user(token) do + with {:ok, query} <- UserToken.verify_email_token_query(token, "confirm"), + %User{} = user <- Repo.one(query), + {:ok, %{user: user}} <- Repo.transaction(confirm_user_multi(user)) do + {:ok, user} + else + _ -> :error + end + end + + defp confirm_user_multi(user) do + Ecto.Multi.new() + |> Ecto.Multi.update(:user, User.confirm_changeset(user)) + |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, ["confirm"])) + end + + ## Reset password + + @doc """ + Delivers the reset password email to the given user. + + ## Examples + + iex> deliver_user_reset_password_instructions(user, &Routes.user_reset_password_url(conn, :edit, &1)) + {:ok, %{to: ..., body: ...}} + + """ + def deliver_user_reset_password_instructions(%User{} = user, reset_password_url_fun) + when is_function(reset_password_url_fun, 1) do + {encoded_token, user_token} = UserToken.build_email_token(user, "reset_password") + Repo.insert!(user_token) + UserNotifier.deliver_reset_password_instructions(user, reset_password_url_fun.(encoded_token)) + end + + @doc """ + Gets the user by reset password token. + + ## Examples + + iex> get_user_by_reset_password_token("validtoken") + %User{} + + iex> get_user_by_reset_password_token("invalidtoken") + nil + + """ + def get_user_by_reset_password_token(token) do + with {:ok, query} <- UserToken.verify_email_token_query(token, "reset_password"), + %User{} = user <- Repo.one(query) do + user + else + _ -> nil + end + end + + @doc """ + Resets the user password. + + ## Examples + + iex> reset_user_password(user, %{password: "new long password", password_confirmation: "new long password"}) + {:ok, %User{}} + + iex> reset_user_password(user, %{password: "valid", password_confirmation: "not the same"}) + {:error, %Ecto.Changeset{}} + + """ + def reset_user_password(user, attrs) do + Ecto.Multi.new() + |> Ecto.Multi.update(:user, User.password_changeset(user, attrs)) + |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) + |> Repo.transaction() + |> case do + {:ok, %{user: user}} -> {:ok, user} + {:error, :user, changeset, _} -> {:error, changeset} + end + end + + def change_user(%User{} = user) do + User.changeset(user, %{}) + end + @doc """ Updates a user. @@ -227,38 +636,10 @@ defmodule Philomena.Users do |> Repo.update() 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 - - @impl Pow.Ecto.Context - def delete(user) do - {:error, User.changeset(user, %{})} - end - - @impl Pow.Ecto.Context - def create(params) do - %User{} - |> User.creation_changeset(params) - |> Repo.insert() - end - - @impl Pow.Ecto.Context - def get_by(clauses) do - User - |> join(:left, [u], _ in assoc(u, :roles)) - |> join(:left, [u, _], _ in assoc(u, :current_filter)) - |> preload([_, r, cf], current_filter: cf, roles: r) - |> Repo.get_by(clauses) + defp load_with_roles(query) do + query + |> Repo.one() + |> Repo.preload([:roles, :current_filter]) |> setup_roles() end diff --git a/lib/philomena/users/user.ex b/lib/philomena/users/user.ex index f71bcd2f..1430c0ed 100644 --- a/lib/philomena/users/user.ex +++ b/lib/philomena/users/user.ex @@ -3,14 +3,6 @@ defmodule Philomena.Users.User do alias Philomena.Slug use Ecto.Schema - - use Pow.Ecto.Schema, - password_hash_methods: {&Password.hash_pwd_salt/1, &Password.verify_pass/2}, - password_min_length: 6 - - use Pow.Extension.Ecto.Schema, - extensions: [PowResetPassword, PowLockout] - import Ecto.Changeset alias Philomena.Schema.TagList @@ -30,7 +22,7 @@ defmodule Philomena.Users.User do alias Philomena.Donations.Donation @derive {Phoenix.Param, key: :slug} - + @derive {Inspect, except: [:password]} schema "users" do has_many :links, UserLink has_many :verified_links, UserLink, where: [aasm_state: "verified"] @@ -53,27 +45,20 @@ defmodule Philomena.Users.User do # Authentication field :email, :string + field :password, :string, virtual: true field :encrypted_password, :string - field :password_hash, :string, source: :encrypted_password - field :reset_password_token, :string - field :reset_password_sent_at, :naive_datetime - field :remember_created_at, :naive_datetime - field :sign_in_count, :integer, default: 0 - field :current_sign_in_at, :naive_datetime - field :last_sign_in_at, :naive_datetime - field :current_sign_in_ip, EctoNetwork.INET - field :last_sign_in_ip, EctoNetwork.INET + field :hashed_password, :string, source: :encrypted_password + field :confirmed_at, :naive_datetime field :otp_required_for_login, :boolean field :authentication_token, :string - # field :failed_attempts, :integer + field :failed_attempts, :integer # field :unlock_token, :string - # field :locked_at, :naive_datetime + field :locked_at, :naive_datetime field :encrypted_otp_secret, :string field :encrypted_otp_secret_iv, :string field :encrypted_otp_secret_salt, :string field :consumed_timestep, :integer field :otp_backup_codes, {:array, :string} - pow_user_fields() # General attributes field :name, :string @@ -147,16 +132,140 @@ defmodule Philomena.Users.User do timestamps(inserted_at: :created_at) end - @doc false - def changeset(user, attrs) do + @doc """ + A user changeset for registration. + + It is important to validate the length of both email and password. + Otherwise databases may truncate the email without warnings, which + could lead to unpredictable or insecure behaviour. Long passwords may + also be very expensive to hash for certain algorithms. + """ + def registration_changeset(user, attrs) do user - |> pow_changeset(attrs) - |> pow_extension_changeset(attrs) - |> cast(attrs, []) - |> validate_required([]) + |> cast(attrs, [:name, :email, :password]) + |> validate_name() + |> validate_email() + |> validate_password() + |> put_api_key() + |> put_slug() |> unique_constraints() end + defp validate_name(changeset) do + changeset + |> validate_required([:name]) + |> validate_length(:name, max: 50) + end + + defp validate_email(changeset) do + changeset + |> validate_required([:email]) + |> validate_format(:email, ~r/^[^\s]+@[^\s]+$/, message: "must have the @ sign and no spaces") + |> validate_length(:email, max: 160) + |> unsafe_validate_unique(:email, Philomena.Repo) + end + + defp validate_password(changeset) do + changeset + |> validate_required([:password]) + |> validate_length(:password, min: 12, max: 80) + |> prepare_changes(&hash_password/1) + end + + defp hash_password(changeset) do + password = get_change(changeset, :password) + + changeset + |> put_change(:hashed_password, Password.hash_pwd_salt(password)) + |> delete_change(:password) + end + + @doc """ + A user changeset for changing the email. + + It requires the email to change otherwise an error is added. + """ + def email_changeset(user, attrs) do + user + |> cast(attrs, [:email]) + |> validate_email() + |> case do + %{changes: %{email: _}} = changeset -> changeset + %{} = changeset -> add_error(changeset, :email, "did not change") + end + end + + @doc """ + A user changeset for changing the password. + """ + def password_changeset(user, attrs) do + user + |> cast(attrs, [:password]) + |> validate_confirmation(:password, message: "does not match password") + |> validate_password() + end + + @doc """ + Confirms the account by setting `confirmed_at`. + """ + def confirm_changeset(user) do + now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + change(user, confirmed_at: now) + end + + @doc """ + Verifies the password. + + If there is no user or the user doesn't have a password, we call + `Bcrypt.no_user_verify/0` to avoid timing attacks. + """ + def valid_password?(%User{hashed_password: hashed_password}, password) + when is_binary(hashed_password) and byte_size(password) > 0 do + Password.verify_pass(password, hashed_password) + end + + def valid_password?(_, _) do + Bcrypt.no_user_verify() + false + end + + @doc """ + Validates the current password otherwise adds an error to the changeset. + """ + def validate_current_password(changeset, password) do + if valid_password?(changeset.data, password) do + changeset + else + add_error(changeset, :current_password, "is not valid") + end + end + + def successful_attempt_changeset(user) do + change(user, failed_attempts: 0) + end + + def failed_attempt_changeset(user) do + if not is_integer(user.failed_attempts) or user.failed_attempts < 0 do + change(user, failed_attempts: 1) + else + change(user, failed_attempts: user.failed_attempts + 1) + end + end + + def lock_changeset(user) do + locked_at = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + + change(user, locked_at: locked_at) + end + + def unlock_changeset(user) do + change(user, locked_at: nil, failed_attempts: 0) + end + + def changeset(user, attrs) do + cast(user, attrs, []) + end + def update_changeset(user, attrs, roles) do user |> cast(attrs, [:name, :email, :role, :secondary_role, :hide_default_role]) @@ -167,17 +276,6 @@ defmodule Philomena.Users.User do |> unique_constraints() end - def creation_changeset(user, attrs) do - user - |> pow_changeset(attrs) - |> pow_extension_changeset(attrs) - |> cast(attrs, [:name]) - |> validate_required([:name]) - |> put_api_key() - |> put_slug() - |> unique_constraints() - end - def filter_changeset(user, filter) do changeset = change(user) user = changeset.data @@ -352,23 +450,25 @@ defmodule Philomena.Users.User do end def totp_changeset(changeset, params, backup_codes) do + %{"user" => %{"current_password" => password}} = params changeset = change(changeset, %{}) user = changeset.data - case user.otp_required_for_login do - true -> + cond do + !!user.otp_required_for_login and valid_password?(user, password) -> # User wants to disable TOTP changeset - |> pow_password_changeset(params) |> consume_totp_token_changeset(params) |> disable_totp_changeset() - _falsy -> + !user.otp_required_for_login and valid_password?(user, password) -> # User wants to enable TOTP changeset - |> pow_password_changeset(params) |> consume_totp_token_changeset(params) |> enable_totp_changeset(backup_codes) + + true -> + add_error(changeset, :current_password, "is invalid") end end @@ -409,20 +509,16 @@ defmodule Philomena.Users.User do end defp enable_totp_changeset(user, backup_codes) do - hashed_codes = - backup_codes - |> Enum.map(&Password.hash_pwd_salt/1) + hashed_codes = Enum.map(backup_codes, &Password.hash_pwd_salt/1) - user - |> change(%{ + change(user, %{ otp_required_for_login: true, otp_backup_codes: hashed_codes }) end defp disable_totp_changeset(user) do - user - |> change(%{ + change(user, %{ otp_required_for_login: false, otp_backup_codes: [], encrypted_otp_secret: nil, @@ -437,7 +533,6 @@ defmodule Philomena.Users.User do |> unique_constraint(:slug, name: :index_users_on_slug) |> unique_constraint(:email, name: :index_users_on_email) |> unique_constraint(:authentication_token, name: :index_users_on_authentication_token) - |> unique_constraint(:name, name: :temp_unique_index_users_on_name) end defp extract_token(%{"user" => %{"twofactor_token" => t}}), @@ -455,8 +550,7 @@ defmodule Philomena.Users.User do defp put_slug(changeset) do name = get_field(changeset, :name) - changeset - |> put_change(:slug, Slug.slug(name)) + put_change(changeset, :slug, Slug.slug(name)) end defp totp_valid?(user, token) do diff --git a/lib/philomena/users/user_notifier.ex b/lib/philomena/users/user_notifier.ex new file mode 100644 index 00000000..8b5fb8bf --- /dev/null +++ b/lib/philomena/users/user_notifier.ex @@ -0,0 +1,101 @@ +defmodule Philomena.Users.UserNotifier do + alias Bamboo.Email + alias Philomena.Mailer + + defp deliver(to, subject, body) do + email = + Email.new_email( + to: to, + from: mailer_address(), + subject: subject, + text_body: body + ) + |> Mailer.deliver_later() + + {:ok, email} + end + + defp mailer_address do + Application.get_env(:philomena, :mailer_address) + end + + @doc """ + Deliver instructions to confirm account. + """ + def deliver_confirmation_instructions(user, url) do + deliver(user.email, "Confirmation instructions for your account", """ + + ============================== + + Hi #{user.name}, + + You can confirm your account by visiting the URL below: + + #{url} + + If you didn't create an account with us, please ignore this. + + ============================== + """) + end + + @doc """ + Deliver instructions to reset password for an account. + """ + def deliver_reset_password_instructions(user, url) do + deliver(user.email, "Password reset instructions for your account", """ + + ============================== + + Hi #{user.name}, + + You can reset your password by visiting the URL below: + + #{url} + + If you didn't request this change, please ignore this. + + ============================== + """) + end + + @doc """ + Deliver instructions to update an account email. + """ + def deliver_update_email_instructions(user, url) do + deliver(user.email, "Email update instructions for your account", """ + + ============================== + + Hi #{user.name}, + + You can change your email by visiting the URL below: + + #{url} + + If you didn't request this change, please ignore this. + + ============================== + """) + end + + @doc """ + Deliver instructions to unlock an account. + """ + def deliver_unlock_instructions(user, url) do + deliver(user.email, "Unlock instructions for your account", """ + + ============================== + + Hi #{user.name}, + + Your account has been automatically locked due to too many attempts to sign in. + + You can unlock your account by visting the URL below: + + #{url} + + ============================== + """) + end +end diff --git a/lib/philomena/users/user_token.ex b/lib/philomena/users/user_token.ex new file mode 100644 index 00000000..a30c6b42 --- /dev/null +++ b/lib/philomena/users/user_token.ex @@ -0,0 +1,163 @@ +defmodule Philomena.Users.UserToken do + use Ecto.Schema + import Ecto.Query + + @hash_algorithm :sha256 + @rand_size 32 + + # It is very important to keep the reset password token expiry short, + # since someone with access to the email may take over the account. + @reset_password_validity_in_days 1 + @confirm_validity_in_days 7 + @change_email_validity_in_days 7 + @unlock_email_validity_in_days 7 + @session_validity_in_days 365 + + schema "user_tokens" do + field :token, :binary + field :context, :string + field :sent_to, :string + belongs_to :user, Philomena.Users.User + + timestamps(inserted_at: :created_at, updated_at: false) + end + + @doc """ + Generates a token that will be stored in a signed place, + such as session or cookie. As they are signed, those + tokens do not need to be hashed. + """ + def build_session_token(user) do + token = :crypto.strong_rand_bytes(@rand_size) + {token, %Philomena.Users.UserToken{token: token, context: "session", user_id: user.id}} + end + + @doc """ + Checks if the token is valid and returns its underlying lookup query. + + The query returns the user found by the token. + """ + def verify_session_token_query(token) do + query = + from token in token_and_context_query(token, "session"), + join: user in assoc(token, :user), + where: token.created_at > ago(@session_validity_in_days, "day"), + select: user + + {:ok, query} + end + + @doc """ + Generates a token that will be stored in a signed place, + such as session or cookie. As they are signed, those + tokens do not need to be hashed. + """ + def build_totp_token(user) do + token = :crypto.strong_rand_bytes(@rand_size) + {token, %Philomena.Users.UserToken{token: token, context: "totp", user_id: user.id}} + end + + @doc """ + Checks if the TOTP token is valid and returns its underlying lookup query. + """ + def verify_totp_token_query(%{id: id}, token) do + query = + from token in token_and_context_query(token, "totp"), + where: token.user_id == ^id, + where: token.created_at > ago(@session_validity_in_days, "day") + + {:ok, query} + end + + @doc """ + Builds a token with a hashed counter part. + + The non-hashed token is sent to the user email while the + hashed part is stored in the database, to avoid reconstruction. + The token is valid for a week as long as users don't change + their email. + """ + def build_email_token(user, context) do + build_hashed_token(user, context, user.email) + end + + defp build_hashed_token(user, context, sent_to) do + token = :crypto.strong_rand_bytes(@rand_size) + hashed_token = :crypto.hash(@hash_algorithm, token) + + {Base.url_encode64(token, padding: false), + %Philomena.Users.UserToken{ + token: hashed_token, + context: context, + sent_to: sent_to, + user_id: user.id + }} + end + + @doc """ + Checks if the token is valid and returns its underlying lookup query. + + The query returns the user found by the token. + """ + def verify_email_token_query(token, context) do + case Base.url_decode64(token, padding: false) do + {:ok, decoded_token} -> + hashed_token = :crypto.hash(@hash_algorithm, decoded_token) + days = days_for_context(context) + + query = + from token in token_and_context_query(hashed_token, context), + join: user in assoc(token, :user), + where: token.created_at > ago(^days, "day") and token.sent_to == user.email, + select: user + + {:ok, query} + + :error -> + :error + end + end + + defp days_for_context("confirm"), do: @confirm_validity_in_days + defp days_for_context("reset_password"), do: @reset_password_validity_in_days + defp days_for_context("unlock"), do: @unlock_email_validity_in_days + + @doc """ + Checks if the token is valid and returns its underlying lookup query. + + The query returns the user token record. + """ + def verify_change_email_token_query(token, context) do + case Base.url_decode64(token, padding: false) do + {:ok, decoded_token} -> + hashed_token = :crypto.hash(@hash_algorithm, decoded_token) + + query = + from token in token_and_context_query(hashed_token, context), + where: token.created_at > ago(@change_email_validity_in_days, "day") + + {:ok, query} + + :error -> + :error + end + end + + @doc """ + Returns the given token with the given context. + """ + def token_and_context_query(token, context) do + from Philomena.Users.UserToken, where: [token: ^token, context: ^context] + end + + @doc """ + Gets all tokens for the given user for the given contexts. + """ + def user_and_contexts_query(user, :all) do + from t in Philomena.Users.UserToken, where: t.user_id == ^user.id + end + + def user_and_contexts_query(user, [_ | _] = contexts) do + from t in Philomena.Users.UserToken, where: t.user_id == ^user.id and t.context in ^contexts + end +end diff --git a/lib/philomena_web/controllers/commission_controller.ex b/lib/philomena_web/controllers/commission_controller.ex index 3a49ca10..5a220549 100644 --- a/lib/philomena_web/controllers/commission_controller.ex +++ b/lib/philomena_web/controllers/commission_controller.ex @@ -111,8 +111,8 @@ defmodule PhilomenaWeb.CommissionController do user -> user = Repo.preload(user, :commission) - config = Pow.Plug.fetch_config(conn) - Pow.Plug.assign_current_user(conn, user, config) + + assign(conn, :current_user, user) end end end diff --git a/lib/philomena_web/controllers/confirmation_controller.ex b/lib/philomena_web/controllers/confirmation_controller.ex new file mode 100644 index 00000000..3c0576a7 --- /dev/null +++ b/lib/philomena_web/controllers/confirmation_controller.ex @@ -0,0 +1,45 @@ +defmodule PhilomenaWeb.ConfirmationController do + use PhilomenaWeb, :controller + + alias Philomena.Users + + plug PhilomenaWeb.CaptchaPlug when action in [:create] + + def new(conn, _params) do + render(conn, "new.html") + end + + def create(conn, %{"user" => %{"email" => email}}) do + if user = Users.get_user_by_email(email) do + Users.deliver_user_confirmation_instructions( + user, + &Routes.confirmation_url(conn, :show, &1) + ) + end + + # Regardless of the outcome, show an impartial success/error message. + conn + |> put_flash( + :info, + "If your email is in our system and it has not been confirmed yet, " <> + "you will receive an email with instructions shortly." + ) + |> redirect(to: "/") + end + + # Do not log in the user after confirmation to avoid a + # leaked token giving the user access to the account. + def show(conn, %{"id" => token}) do + case Users.confirm_user(token) do + {:ok, _} -> + conn + |> put_flash(:info, "Account confirmed successfully.") + |> redirect(to: "/") + + :error -> + conn + |> put_flash(:error, "Confirmation link is invalid or it has expired.") + |> redirect(to: "/") + end + end +end diff --git a/lib/philomena_web/controllers/password_controller.ex b/lib/philomena_web/controllers/password_controller.ex new file mode 100644 index 00000000..feb4de52 --- /dev/null +++ b/lib/philomena_web/controllers/password_controller.ex @@ -0,0 +1,61 @@ +defmodule PhilomenaWeb.PasswordController do + use PhilomenaWeb, :controller + + alias Philomena.Users + + plug PhilomenaWeb.CaptchaPlug when action in [:create] + plug PhilomenaWeb.CompromisedPasswordCheckPlug when action in [:update] + plug :get_user_by_reset_password_token when action in [:edit, :update] + + def new(conn, _params) do + render(conn, "new.html") + end + + def create(conn, %{"user" => %{"email" => email}}) do + if user = Users.get_user_by_email(email) do + Users.deliver_user_reset_password_instructions( + user, + &Routes.password_url(conn, :edit, &1) + ) + end + + # Regardless of the outcome, show an impartial success/error message. + conn + |> put_flash( + :info, + "If your email is in our system, you will receive instructions to reset your password shortly." + ) + |> redirect(to: "/") + end + + def edit(conn, _params) do + render(conn, "edit.html", changeset: Users.change_user_password(conn.assigns.user)) + end + + # Do not log in the user after reset password to avoid a + # leaked token giving the user access to the account. + def update(conn, %{"user" => user_params}) do + case Users.reset_user_password(conn.assigns.user, user_params) do + {:ok, _} -> + conn + |> put_flash(:info, "Password reset successfully.") + |> redirect(to: Routes.session_path(conn, :new)) + + {:error, changeset} -> + render(conn, "edit.html", changeset: changeset) + end + end + + defp get_user_by_reset_password_token(conn, _opts) do + %{"id" => token} = conn.params + + if user = Users.get_user_by_reset_password_token(token) do + conn |> assign(:user, user) |> assign(:token, token) + else + conn + |> put_flash(:error, "Reset password link is invalid or it has expired.") + |> redirect(to: "/") + |> halt() + end + end +end diff --git a/lib/philomena_web/controllers/registration/email_controller.ex b/lib/philomena_web/controllers/registration/email_controller.ex new file mode 100644 index 00000000..e20a9bb0 --- /dev/null +++ b/lib/philomena_web/controllers/registration/email_controller.ex @@ -0,0 +1,44 @@ +defmodule PhilomenaWeb.Registration.EmailController do + use PhilomenaWeb, :controller + + alias Philomena.Users + + def create(conn, %{"current_password" => password, "user" => user_params}) do + user = conn.assigns.current_user + + case Users.apply_user_email(user, password, user_params) do + {:ok, applied_user} -> + Users.deliver_update_email_instructions( + applied_user, + user.email, + &Routes.registration_email_url(conn, :show, &1) + ) + + conn + |> put_flash( + :info, + "A link to confirm your email change has been sent to the new address." + ) + |> redirect(to: Routes.registration_path(conn, :edit)) + + {:error, _changeset} -> + conn + |> put_flash(:error, "Failed to update email.") + |> redirect(to: Routes.registration_path(conn, :edit)) + end + end + + def show(conn, %{"id" => token}) do + case Users.update_user_email(conn.assigns.current_user, token) do + :ok -> + conn + |> put_flash(:info, "Email changed successfully.") + |> redirect(to: Routes.registration_path(conn, :edit)) + + :error -> + conn + |> put_flash(:error, "Email change link is invalid or it has expired.") + |> redirect(to: Routes.registration_path(conn, :edit)) + end + end +end diff --git a/lib/philomena_web/controllers/registration/password_controller.ex b/lib/philomena_web/controllers/registration/password_controller.ex new file mode 100644 index 00000000..909df6eb --- /dev/null +++ b/lib/philomena_web/controllers/registration/password_controller.ex @@ -0,0 +1,25 @@ +defmodule PhilomenaWeb.Registration.PasswordController do + use PhilomenaWeb, :controller + + alias Philomena.Users + alias PhilomenaWeb.UserAuth + + plug PhilomenaWeb.CompromisedPasswordCheckPlug when action in [:update] + + def update(conn, %{"current_password" => password, "user" => user_params}) do + user = conn.assigns.current_user + + case Users.update_user_password(user, password, user_params) do + {:ok, user} -> + conn + |> put_flash(:info, "Password updated successfully.") + |> put_session(:user_return_to, Routes.registration_path(conn, :edit)) + |> UserAuth.log_in_user(user) + + {:error, _changeset} -> + conn + |> put_flash(:error, "Failed to update password.") + |> redirect(to: Routes.registration_path(conn, :edit)) + end + end +end diff --git a/lib/philomena_web/controllers/registration/totp_controller.ex b/lib/philomena_web/controllers/registration/totp_controller.ex index 6cf57377..2aa64a0f 100644 --- a/lib/philomena_web/controllers/registration/totp_controller.ex +++ b/lib/philomena_web/controllers/registration/totp_controller.ex @@ -1,7 +1,9 @@ defmodule PhilomenaWeb.Registration.TotpController do use PhilomenaWeb, :controller + alias PhilomenaWeb.UserAuth alias Philomena.Users.User + alias Philomena.Users alias Philomena.Repo def edit(conn, _params) do @@ -13,11 +15,11 @@ defmodule PhilomenaWeb.Registration.TotpController do |> User.create_totp_secret_changeset() |> Repo.update() - # Redirect to have Pow pick up the changes + # Redirect to have the conn pick up the changes redirect(conn, to: Routes.registration_totp_path(conn, :edit)) _ -> - changeset = Pow.Plug.change_user(conn) + changeset = Users.change_user(user) secret = User.totp_secret(user) qrcode = User.totp_qrcode(user) @@ -32,7 +34,7 @@ defmodule PhilomenaWeb.Registration.TotpController do def update(conn, params) do backup_codes = User.random_backup_codes() - user = Pow.Plug.current_user(conn) + user = conn.assigns.current_user user |> User.totp_changeset(params, backup_codes) @@ -45,7 +47,7 @@ defmodule PhilomenaWeb.Registration.TotpController do {:ok, user} -> conn - |> PhilomenaWeb.TotpPlug.update_valid_totp_at_for_session(user) + |> UserAuth.totp_auth_user(user, %{}) |> put_flash(:totp_backup_codes, backup_codes) |> redirect(to: Routes.registration_totp_path(conn, :edit)) end diff --git a/lib/philomena_web/controllers/registration_controller.ex b/lib/philomena_web/controllers/registration_controller.ex new file mode 100644 index 00000000..dd4cf107 --- /dev/null +++ b/lib/philomena_web/controllers/registration_controller.ex @@ -0,0 +1,48 @@ +defmodule PhilomenaWeb.RegistrationController do + use PhilomenaWeb, :controller + + alias Philomena.Users + alias Philomena.Users.User + + plug PhilomenaWeb.CaptchaPlug when action in [:create] + plug PhilomenaWeb.CompromisedPasswordCheckPlug when action in [:create] + plug :assign_email_and_password_changesets when action in [:edit] + + def new(conn, _params) do + changeset = Users.change_user_registration(%User{}) + render(conn, "new.html", changeset: changeset) + end + + def create(conn, %{"user" => user_params}) do + case Users.register_user(user_params) do + {:ok, user} -> + {:ok, _} = + Users.deliver_user_confirmation_instructions( + user, + &Routes.confirmation_url(conn, :show, &1) + ) + + conn + |> put_flash( + :info, + "Account created successfully. Check your email for confirmation instructions." + ) + |> redirect(to: "/") + + {:error, %Ecto.Changeset{} = changeset} -> + render(conn, "new.html", changeset: changeset) + end + end + + def edit(conn, _params) do + render(conn, "edit.html") + end + + defp assign_email_and_password_changesets(conn, _opts) do + user = conn.assigns.current_user + + conn + |> assign(:email_changeset, Users.change_user_email(user)) + |> assign(:password_changeset, Users.change_user_password(user)) + end +end diff --git a/lib/philomena_web/controllers/session/totp_controller.ex b/lib/philomena_web/controllers/session/totp_controller.ex index fb1588d6..2e429589 100644 --- a/lib/philomena_web/controllers/session/totp_controller.ex +++ b/lib/philomena_web/controllers/session/totp_controller.ex @@ -2,30 +2,32 @@ defmodule PhilomenaWeb.Session.TotpController do use PhilomenaWeb, :controller alias PhilomenaWeb.LayoutView + alias PhilomenaWeb.UserAuth alias Philomena.Users.User + alias Philomena.Users alias Philomena.Repo def new(conn, _params) do - changeset = Pow.Plug.change_user(conn) + changeset = Users.change_user(conn.assigns.current_user) render(conn, "new.html", layout: {LayoutView, "two_factor.html"}, changeset: changeset) end def create(conn, params) do - conn - |> Pow.Plug.current_user() + %{"user" => user_params} = params + + conn.assigns.current_user |> User.consume_totp_token_changeset(params) |> Repo.update() |> case do {:error, _changeset} -> conn - |> Pow.Plug.delete() - |> put_flash(:error, "Sorry, invalid TOTP token entered. Please sign in again.") - |> redirect(to: Routes.pow_session_path(conn, :new)) + |> put_flash(:error, "Invalid TOTP token entered. Please sign in again.") + |> UserAuth.log_out_user() {:ok, user} -> conn - |> PhilomenaWeb.TotpPlug.update_valid_totp_at_for_session(user) + |> UserAuth.totp_auth_user(user, user_params) |> redirect(to: "/") end end diff --git a/lib/philomena_web/controllers/session_controller.ex b/lib/philomena_web/controllers/session_controller.ex new file mode 100644 index 00000000..01e995aa --- /dev/null +++ b/lib/philomena_web/controllers/session_controller.ex @@ -0,0 +1,42 @@ +defmodule PhilomenaWeb.SessionController do + use PhilomenaWeb, :controller + + alias Philomena.Users + alias PhilomenaWeb.UserAuth + + def new(conn, _params) do + render(conn, "new.html", error_message: nil) + end + + def create(conn, %{"user" => user_params}) do + %{"email" => email, "password" => password} = user_params + + user = + Users.get_user_by_email_and_password( + email, + password, + &Routes.unlock_url(conn, :show, &1) + ) + + cond do + not is_nil(user) and is_nil(user.confirmed_at) -> + conn + |> put_flash(:error, "You must confirm your account before logging in.") + |> redirect(to: "/") + + not is_nil(user) -> + conn + |> put_flash(:info, "Successfully logged in.") + |> UserAuth.log_in_user(user, user_params) + + true -> + render(conn, "new.html", error_message: "Invalid email or password") + end + end + + def delete(conn, _params) do + conn + |> put_flash(:info, "Logged out successfully.") + |> UserAuth.log_out_user() + end +end diff --git a/lib/philomena_web/controllers/unlock_controller.ex b/lib/philomena_web/controllers/unlock_controller.ex new file mode 100644 index 00000000..c70530f5 --- /dev/null +++ b/lib/philomena_web/controllers/unlock_controller.ex @@ -0,0 +1,45 @@ +defmodule PhilomenaWeb.UnlockController do + use PhilomenaWeb, :controller + + alias Philomena.Users + + plug PhilomenaWeb.CaptchaPlug when action in [:create] + + def new(conn, _params) do + render(conn, "new.html") + end + + def create(conn, %{"user" => %{"email" => email}}) do + if user = Users.get_user_by_email(email) do + Users.deliver_user_unlock_instructions( + user, + &Routes.unlock_url(conn, :show, &1) + ) + end + + # Regardless of the outcome, show an impartial success/error message. + conn + |> put_flash( + :info, + "If your email is in our system and your account has been locked, " <> + "you will receive an email with instructions shortly." + ) + |> redirect(to: "/") + end + + # Do not log in the user after unlocking to avoid a + # leaked token giving the user access to the account. + def show(conn, %{"id" => token}) do + case Users.unlock_user(token) do + {:ok, _} -> + conn + |> put_flash(:info, "Account unlocked successfully. You may now log in.") + |> redirect(to: "/") + + :error -> + conn + |> put_flash(:error, "Unlock link is invalid or it has expired.") + |> redirect(to: "/") + end + end +end diff --git a/lib/philomena_web/endpoint.ex b/lib/philomena_web/endpoint.ex index c645480f..0a14d1d3 100644 --- a/lib/philomena_web/endpoint.ex +++ b/lib/philomena_web/endpoint.ex @@ -45,22 +45,6 @@ defmodule PhilomenaWeb.Endpoint do signing_salt: "signed cookie", encryption_salt: "authenticated encrypted cookie" - # This is used to capture tokens being invalidated to store for temporary - # reuse - plug PhilomenaWeb.PowInvalidatedSessionPlug, :pow_session - plug PhilomenaWeb.PowInvalidatedSessionPlug, :pow_persistent_session - - plug Pow.Plug.Session, otp_app: :philomena - - plug PowPersistentSession.Plug.Cookie, - otp_app: :philomena, - persistent_session_cookie_opts: [extra: "SameSite=Lax"] - - # This is used as fallback to load user if the Pow session could not be - # loaded - plug PhilomenaWeb.PowInvalidatedSessionPlug, :load - - plug PhilomenaWeb.ReloadUserPlug plug PhilomenaWeb.RenderTimePlug plug PhilomenaWeb.ReferrerPlug plug PhilomenaWeb.Router diff --git a/lib/philomena_web/plugs/api_token_plug.ex b/lib/philomena_web/plugs/api_token_plug.ex index 087f912b..b9746597 100644 --- a/lib/philomena_web/plugs/api_token_plug.ex +++ b/lib/philomena_web/plugs/api_token_plug.ex @@ -1,6 +1,6 @@ defmodule PhilomenaWeb.ApiTokenPlug do alias Philomena.Users - alias Pow.Plug + alias Plug.Conn def init([]), do: [] @@ -12,15 +12,13 @@ defmodule PhilomenaWeb.ApiTokenPlug do defp maybe_find_user(conn, nil), do: {conn, nil} - defp maybe_find_user(conn, key) do - user = Users.get_by(authentication_token: key) + defp maybe_find_user(conn, token) do + user = Users.get_user_by_authentication_token(token) {conn, user} end defp assign_user({conn, user}) do - config = Plug.fetch_config(conn) - - Plug.assign_current_user(conn, user, config) + Conn.assign(conn, :current_user, user) end end diff --git a/lib/philomena_web/plugs/captcha_plug.ex b/lib/philomena_web/plugs/captcha_plug.ex index a43d8dc7..75f1b85e 100644 --- a/lib/philomena_web/plugs/captcha_plug.ex +++ b/lib/philomena_web/plugs/captcha_plug.ex @@ -6,10 +6,10 @@ defmodule PhilomenaWeb.CaptchaPlug do def init([]), do: false def call(conn, _opts) do - user = conn |> Pow.Plug.current_user() - - conn - |> maybe_check_captcha(user) + case captcha_enabled?() do + true -> maybe_check_captcha(conn, conn.assigns.current_user) + false -> conn + end end defp maybe_check_captcha(conn, nil) do @@ -46,4 +46,8 @@ defmodule PhilomenaWeb.CaptchaPlug do _ -> false end end + + def captcha_enabled? do + Application.get_env(:philomena, :captcha) != false + end end diff --git a/lib/philomena_web/plugs/compromised_password_check_plug.ex b/lib/philomena_web/plugs/compromised_password_check_plug.ex index 5cfb4964..d14e3a83 100644 --- a/lib/philomena_web/plugs/compromised_password_check_plug.ex +++ b/lib/philomena_web/plugs/compromised_password_check_plug.ex @@ -5,7 +5,10 @@ defmodule PhilomenaWeb.CompromisedPasswordCheckPlug do def init(opts), do: opts def call(conn, _opts) do - error_if_password_compromised(conn, conn.params) + case pwned_passwords_enabled?() do + true -> error_if_password_compromised(conn, conn.params) + false -> conn + end end defp error_if_password_compromised(conn, %{"user" => %{"password" => password}}) do @@ -41,4 +44,8 @@ defmodule PhilomenaWeb.CompromisedPasswordCheckPlug do defp make_api_url(prefix) do "https://api.pwnedpasswords.com/range/#{prefix}" end + + defp pwned_passwords_enabled? do + Application.get_env(:philomena, :pwned_passwords) != false + end end diff --git a/lib/philomena_web/plugs/current_ban_plug.ex b/lib/philomena_web/plugs/current_ban_plug.ex index 2d2080e1..f82e74b1 100644 --- a/lib/philomena_web/plugs/current_ban_plug.ex +++ b/lib/philomena_web/plugs/current_ban_plug.ex @@ -8,7 +8,6 @@ defmodule PhilomenaWeb.CurrentBanPlug do """ alias Philomena.Bans alias Plug.Conn - alias Pow.Plug @doc false @spec init(any()) :: any() @@ -17,12 +16,10 @@ defmodule PhilomenaWeb.CurrentBanPlug do @doc false @spec call(Conn.t(), any()) :: Conn.t() def call(conn, _opts) do - conn = - conn - |> Conn.fetch_cookies() + conn = Conn.fetch_cookies(conn) fingerprint = conn.cookies["_ses"] - user = Plug.current_user(conn) + user = conn.assigns.current_user ip = conn.remote_ip ban = Bans.exists_for?(user, ip, fingerprint) diff --git a/lib/philomena_web/plugs/current_filter_plug.ex b/lib/philomena_web/plugs/current_filter_plug.ex index 1d1fef15..a9264910 100644 --- a/lib/philomena_web/plugs/current_filter_plug.ex +++ b/lib/philomena_web/plugs/current_filter_plug.ex @@ -3,14 +3,14 @@ defmodule PhilomenaWeb.CurrentFilterPlug do alias Philomena.{Filters, Filters.Filter, Users.User} alias Philomena.Repo - alias Pow.Plug + # No options def init([]), do: false # Assign current filter def call(conn, _opts) do - conn = conn |> fetch_session() - user = conn |> Plug.current_user() + conn = fetch_session(conn) + user = conn.assigns.current_user {filter, forced_filter} = if user do diff --git a/lib/philomena_web/plugs/ensure_user_enabled_plug.ex b/lib/philomena_web/plugs/ensure_user_enabled_plug.ex index ff2e9385..ad644ef6 100644 --- a/lib/philomena_web/plugs/ensure_user_enabled_plug.ex +++ b/lib/philomena_web/plugs/ensure_user_enabled_plug.ex @@ -6,10 +6,10 @@ defmodule PhilomenaWeb.EnsureUserEnabledPlug do plug PhilomenaWeb.EnsureUserEnabledPlug """ - alias PhilomenaWeb.Router.Helpers, as: Routes + alias Phoenix.Controller alias Plug.Conn - alias Pow.Plug + alias PhilomenaWeb.UserAuth @doc false @spec init(any()) :: any() @@ -18,19 +18,19 @@ defmodule PhilomenaWeb.EnsureUserEnabledPlug do @doc false @spec call(Conn.t(), any()) :: Conn.t() def call(conn, _opts) do - conn - |> Plug.current_user() - |> disabled?() + conn.assigns.current_user + |> disabled_or_unconfirmed?() |> maybe_halt(conn) end - defp disabled?(%{deleted_at: deleted_at}) when not is_nil(deleted_at), do: true - defp disabled?(_user), do: false + defp disabled_or_unconfirmed?(%{deleted_at: deleted_at}) when not is_nil(deleted_at), do: true + defp disabled_or_unconfirmed?(%{confirmed_at: nil}), do: true + defp disabled_or_unconfirmed?(_user), do: false defp maybe_halt(true, conn) do conn - |> Plug.delete() - |> Controller.redirect(to: Routes.pow_session_path(conn, :new)) + |> Controller.put_flash(:error, "Your account is not currently active.") + |> UserAuth.log_out_user() |> Conn.halt() end diff --git a/lib/philomena_web/plugs/filter_select_plug.ex b/lib/philomena_web/plugs/filter_select_plug.ex index 9a2318ee..e1c86cc5 100644 --- a/lib/philomena_web/plugs/filter_select_plug.ex +++ b/lib/philomena_web/plugs/filter_select_plug.ex @@ -28,10 +28,7 @@ defmodule PhilomenaWeb.FilterSelectPlug do @doc false @spec call(Conn.t(), any()) :: Conn.t() def call(conn, _opts) do - user = Pow.Plug.current_user(conn) - - conn - |> maybe_assign_filters(user) + maybe_assign_filters(conn, conn.assigns.current_user) end defp maybe_assign_filters(conn, nil), do: conn diff --git a/lib/philomena_web/plugs/image_filter_plug.ex b/lib/philomena_web/plugs/image_filter_plug.ex index 24351e12..3281d0e8 100644 --- a/lib/philomena_web/plugs/image_filter_plug.ex +++ b/lib/philomena_web/plugs/image_filter_plug.ex @@ -3,14 +3,13 @@ defmodule PhilomenaWeb.ImageFilterPlug do import Philomena.Search.String alias Philomena.Images.Query - alias Pow.Plug # No options def init([]), do: false # Assign current filter def call(conn, _opts) do - user = conn |> Plug.current_user() + user = conn.assigns.current_user filter = defaults(conn.assigns[:current_filter]) forced = defaults(conn.assigns[:forced_filter]) diff --git a/lib/philomena_web/plugs/notification_count_plug.ex b/lib/philomena_web/plugs/notification_count_plug.ex index e970fae4..d8afbef9 100644 --- a/lib/philomena_web/plugs/notification_count_plug.ex +++ b/lib/philomena_web/plugs/notification_count_plug.ex @@ -22,7 +22,7 @@ defmodule PhilomenaWeb.NotificationCountPlug do @doc false @spec call(Plug.Conn.t(), any()) :: Plug.Conn.t() def call(conn, _opts) do - user = Pow.Plug.current_user(conn) + user = conn.assigns.current_user conn |> maybe_assign_notifications(user) @@ -34,8 +34,7 @@ defmodule PhilomenaWeb.NotificationCountPlug do defp maybe_assign_notifications(conn, user) do notifications = Notifications.count_unread_notifications(user) - conn - |> Conn.assign(:notification_count, notifications) + Conn.assign(conn, :notification_count, notifications) end defp maybe_assign_conversations(conn, nil), do: conn @@ -43,7 +42,6 @@ defmodule PhilomenaWeb.NotificationCountPlug do defp maybe_assign_conversations(conn, user) do conversations = Conversations.count_unread_conversations(user) - conn - |> Conn.assign(:conversation_count, conversations) + Conn.assign(conn, :conversation_count, conversations) end end diff --git a/lib/philomena_web/plugs/pagination_plug.ex b/lib/philomena_web/plugs/pagination_plug.ex index 59b1f6b4..f1706952 100644 --- a/lib/philomena_web/plugs/pagination_plug.ex +++ b/lib/philomena_web/plugs/pagination_plug.ex @@ -1,14 +1,13 @@ defmodule PhilomenaWeb.PaginationPlug do import Plug.Conn - alias Pow.Plug # No options def init([]), do: [] # Assign pagination info def call(conn, _opts) do - conn = conn |> fetch_query_params() - user = conn |> Plug.current_user() + conn = fetch_query_params(conn) + user = conn.assigns.current_user params = conn.params page_size = get_page_size(params) diff --git a/lib/philomena_web/plugs/pow_invalidated_session_plug.ex b/lib/philomena_web/plugs/pow_invalidated_session_plug.ex deleted file mode 100644 index 7cb0260e..00000000 --- a/lib/philomena_web/plugs/pow_invalidated_session_plug.ex +++ /dev/null @@ -1,176 +0,0 @@ -defmodule PhilomenaWeb.PowInvalidatedSessionPlug do - @moduledoc """ - This plug ensures that invalidated sessions can still be used for a short - amount of time. - - This MAY introduce a slight timing attack vector, but in practice would be - unlikely as all tokens expires after 60 seconds. - - ## Example - - plug MyAppWeb.PowInvalidatedSessionPlug, :pow_session - plug MyAppWeb.PowInvalidatedSessionPlug, :pow_persistent_session - plug Pow.Plug.Session, otp_app: :my_app - plug PowPersistentSession.Plug.Cookie - plug MyAppWeb.PowInvalidatedSessionPlug, :load - - """ - alias Plug.Conn - alias Pow.{Config, Plug, Store.Backend.EtsCache} - - @store_ttl :timer.minutes(1) - @otp_app :philomena - @session_key "#{@otp_app}_auth" - @session_signing_salt Atom.to_string(Pow.Plug.Session) - @persistent_cookie_key "#{@otp_app}_persistent_session" - @persistent_cookie_signing_salt Atom.to_string(PowPersistentSession.Plug.Cookie) - - def init(:load), do: :load - - def init(:pow_session) do - [ - fetch_token: &__MODULE__.client_store_fetch_session/1, - namespace: :session - ] - end - - def init(:pow_persistent_session) do - [ - fetch_token: &__MODULE__.client_store_fetch_persistent_cookie/1, - namespace: :persistent_session - ] - end - - def init({type, opts}) do - type - |> init() - |> Keyword.merge(opts) - end - - def call(conn, :load) do - Enum.reduce(conn.private[:invalidated_session_opts], conn, fn opts, conn -> - maybe_load_from_cache(conn, Plug.current_user(conn), opts) - end) - end - - def call(conn, opts) do - fetch_fn = Keyword.fetch!(opts, :fetch_token) - token = fetch_fn.(conn) - - conn - |> put_opts_in_private(opts) - |> Conn.register_before_send(fn conn -> - maybe_put_cache(conn, Plug.current_user(conn), token, opts) - end) - end - - defp maybe_load_from_cache(conn, nil, opts) do - fetch_fn = Keyword.fetch!(opts, :fetch_token) - - case fetch_fn.(conn) do - nil -> conn - token -> load_from_cache(conn, token, opts) - end - end - - defp maybe_load_from_cache(conn, _any, _opts), do: conn - - defp put_opts_in_private(conn, opts) do - plug_opts = (conn.private[:invalidated_session_opts] || []) ++ [opts] - - Conn.put_private(conn, :invalidated_session_opts, plug_opts) - end - - defp maybe_put_cache(conn, nil, _old_token, _opts), do: conn - defp maybe_put_cache(conn, _user, nil, _opts), do: conn - - defp maybe_put_cache(conn, user, old_token, opts) do - fetch_fn = Keyword.fetch!(opts, :fetch_token) - - metadata = - conn.private - |> Map.get(:pow_session_metadata, []) - |> Keyword.take([:valid_totp_at]) - - case fetch_fn.(conn) do - ^old_token -> conn - _token -> put_cache(conn, {user, metadata}, old_token, opts) - end - end - - defp put_cache(conn, user, token, opts) do - {store, store_config} = invalidated_cache(conn, opts) - - store.put(store_config, token, user) - - conn - end - - defp load_from_cache(conn, token, opts) do - config = Plug.fetch_config(conn) - {store, store_config} = invalidated_cache(conn, opts) - - case store.get(store_config, token) do - :not_found -> - conn - - {user, metadata} -> - metadata = Keyword.merge(metadata, conn.private[:pow_session_metadata] || []) - - conn - |> Conn.put_private(:pow_session_metadata, metadata) - |> Plug.assign_current_user(user, config) - - user -> - Plug.assign_current_user(conn, user, config) - end - end - - @doc false - def client_store_fetch_session(conn) do - conn = - conn - |> Plug.put_config(otp_app: @otp_app) - |> Conn.fetch_session() - - with session_id when is_binary(session_id) <- Conn.get_session(conn, @session_key), - {:ok, session_id} <- Plug.verify_token(conn, @session_signing_salt, session_id) do - session_id - else - _any -> nil - end - end - - @doc false - def client_store_fetch_persistent_cookie(conn) do - conn = - conn - |> Plug.put_config(otp_app: @otp_app) - |> Conn.fetch_cookies() - - with token when is_binary(token) <- conn.cookies[@persistent_cookie_key], - {:ok, token} <- Plug.verify_token(conn, @persistent_cookie_signing_salt, token) do - token - else - _any -> nil - end - end - - defp invalidated_cache(conn, opts) do - store_config = store_config(opts) - config = Plug.fetch_config(conn) - store = Config.get(config, :cache_store_backend, EtsCache) - - {store, store_config} - end - - defp store_config(opts) do - namespace = Keyword.fetch!(opts, :namespace) - ttl = Keyword.get(opts, :ttl, @store_ttl) - - [ - ttl: ttl, - namespace: "invalidated_#{namespace}" - ] - end -end diff --git a/lib/philomena_web/plugs/reload_user_plug.ex b/lib/philomena_web/plugs/reload_user_plug.ex deleted file mode 100644 index 3efb7d2d..00000000 --- a/lib/philomena_web/plugs/reload_user_plug.ex +++ /dev/null @@ -1,33 +0,0 @@ -defmodule PhilomenaWeb.ReloadUserPlug do - alias Plug.Conn - alias Pow.Plug - alias Philomena.Users - - alias PhilomenaWeb.UserIpUpdater - alias PhilomenaWeb.UserFingerprintUpdater - - def init(opts), do: opts - - def call(conn, _opts) do - config = Plug.fetch_config(conn) - - case Plug.current_user(conn, config) do - nil -> - conn - - user -> - update_usages(conn, user) - reloaded_user = Users.get_by(id: user.id) - - Plug.assign_current_user(conn, reloaded_user, config) - end - end - - defp update_usages(conn, user) do - now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) - conn = Conn.fetch_cookies(conn) - - UserIpUpdater.cast(user.id, conn.remote_ip, now) - UserFingerprintUpdater.cast(user.id, conn.cookies["_ses"], now) - end -end diff --git a/lib/philomena_web/plugs/require_user_plug.ex b/lib/philomena_web/plugs/require_user_plug.ex index 681ad99c..292ae91d 100644 --- a/lib/philomena_web/plugs/require_user_plug.ex +++ b/lib/philomena_web/plugs/require_user_plug.ex @@ -1,14 +1,13 @@ defmodule PhilomenaWeb.RequireUserPlug do import Phoenix.Controller import Plug.Conn - import Pow.Plug # No options def init([]), do: false # Redirect if not logged in def call(conn, _opts) do - user = conn |> current_user() + user = conn.assigns.current_user if user do conn diff --git a/lib/philomena_web/plugs/tor_plug.ex b/lib/philomena_web/plugs/tor_plug.ex index 9372467b..484e0d22 100644 --- a/lib/philomena_web/plugs/tor_plug.ex +++ b/lib/philomena_web/plugs/tor_plug.ex @@ -26,7 +26,7 @@ defmodule PhilomenaWeb.TorPlug do def maybe_redirect(conn, nil, {127, 0, 0, 1}, true) do conn - |> Controller.redirect(to: Routes.pow_session_path(conn, :new)) + |> Controller.redirect(to: Routes.session_path(conn, :new)) |> Conn.halt() end diff --git a/lib/philomena_web/plugs/totp_plug.ex b/lib/philomena_web/plugs/totp_plug.ex index c0511ed9..9d8665c9 100644 --- a/lib/philomena_web/plugs/totp_plug.ex +++ b/lib/philomena_web/plugs/totp_plug.ex @@ -16,9 +16,7 @@ defmodule PhilomenaWeb.TotpPlug do @doc false @spec call(Plug.Conn.t(), any()) :: Plug.Conn.t() def call(conn, _opts) do - conn - |> Pow.Plug.current_user() - |> case do + case conn.assigns.current_user do nil -> conn user -> maybe_require_totp_phase(user, conn) end @@ -28,39 +26,14 @@ defmodule PhilomenaWeb.TotpPlug do defp maybe_require_totp_phase(%{otp_required_for_login: false}, conn), do: conn defp maybe_require_totp_phase(_user, conn) do - conn.private - |> Map.get(:pow_session_metadata, []) - |> Keyword.get(:valid_totp_at) - |> case do - nil -> + case conn.assigns.totp_valid? do + true -> + conn + + _falsy -> conn |> Phoenix.Controller.redirect(to: Routes.session_totp_path(conn, :new)) |> Plug.Conn.halt() - - _valid_at -> - conn end end - - @doc false - @spec update_valid_totp_at_for_session(Plug.Conn.t(), map()) :: Plug.Conn.t() - def update_valid_totp_at_for_session(conn, user) do - metadata = - conn.private - |> Map.get(:pow_session_metadata, []) - |> Keyword.put(:valid_totp_at, DateTime.utc_now()) - - config = Pow.Plug.fetch_config(conn) - plug = Pow.Plug.get_plug(config) - conn = Plug.Conn.put_private(conn, :pow_session_metadata, metadata) - - conn = - conn - |> Plug.Conn.put_private(:pow_persistent_session_metadata, - session_metadata: Keyword.take(metadata, [:valid_totp_at]) - ) - |> PowPersistentSession.Plug.Cookie.create(user, config) - - plug.do_create(conn, user, config) - end end diff --git a/lib/philomena_web/plugs/user_attribution_plug.ex b/lib/philomena_web/plugs/user_attribution_plug.ex index 806650f8..f2fb2ed6 100644 --- a/lib/philomena_web/plugs/user_attribution_plug.ex +++ b/lib/philomena_web/plugs/user_attribution_plug.ex @@ -19,7 +19,7 @@ defmodule PhilomenaWeb.UserAttributionPlug do def call(conn, _opts) do {:ok, remote_ip} = EctoNetwork.INET.cast(conn.remote_ip) conn = Conn.fetch_cookies(conn) - user = Pow.Plug.current_user(conn) + user = conn.assigns.current_user attributes = [ ip: remote_ip, diff --git a/lib/philomena_web/pow_mailer.ex b/lib/philomena_web/pow_mailer.ex deleted file mode 100644 index 1fb8b078..00000000 --- a/lib/philomena_web/pow_mailer.ex +++ /dev/null @@ -1,28 +0,0 @@ -defmodule PhilomenaWeb.PowMailer do - use Pow.Phoenix.Mailer - alias PhilomenaWeb.Mailer - alias Philomena.Users.User - import Bamboo.Email - - def cast(%{ - user: %User{email: email}, - subject: subject, - text: text, - html: html, - assigns: _assigns - }) do - # Build email struct to be used in `process/1` - new_email( - to: email, - from: Application.get_env(:philomena, :mailer_address), - subject: subject, - text_body: text, - html_body: html - ) - end - - def process(email) do - email - |> Mailer.deliver_later() - end -end diff --git a/lib/philomena_web/router.ex b/lib/philomena_web/router.ex index 129f3ab6..1c0518f3 100644 --- a/lib/philomena_web/router.ex +++ b/lib/philomena_web/router.ex @@ -1,7 +1,7 @@ defmodule PhilomenaWeb.Router do use PhilomenaWeb, :router - use Pow.Phoenix.Router - use Pow.Extension.Phoenix.Router, otp_app: :philomena + + import PhilomenaWeb.UserAuth pipeline :browser do plug :accepts, ["html"] @@ -9,6 +9,7 @@ defmodule PhilomenaWeb.Router do plug :fetch_flash plug :protect_from_forgery plug :put_secure_browser_headers + plug :fetch_current_user plug PhilomenaWeb.ContentSecurityPolicyPlug plug PhilomenaWeb.CurrentFilterPlug plug PhilomenaWeb.ImageFilterPlug @@ -52,55 +53,56 @@ defmodule PhilomenaWeb.Router do plug PhilomenaWeb.FilterBannedUsersPlug end - pipeline :ensure_password_not_compromised do - plug PhilomenaWeb.CompromisedPasswordCheckPlug - end + scope "/", PhilomenaWeb do + pipe_through [:browser, :redirect_if_user_is_authenticated] - pipeline :protected do - plug Pow.Plug.RequireAuthenticated, - error_handler: Pow.Phoenix.PlugErrorHandler - end - - scope "/" do - pipe_through [ - :browser, - :ensure_totp, - :ensure_not_banned, - :ensure_tor_authorized, - :ensure_password_not_compromised - ] - - pow_registration_routes() - end - - scope "/" do - pipe_through [:browser, :ensure_totp] - - pow_session_routes() - end - - scope "/" do - pipe_through [:browser, :ensure_totp, :ensure_tor_authorized] - - pow_extension_routes() + resources "/sessions", SessionController, only: [:new, :create], singleton: true end scope "/", PhilomenaWeb do - pipe_through [:browser, :protected] + pipe_through [:browser, :require_authenticated_user] # Additional routes for TOTP - scope "/registrations", Registration, as: :registration do - resources "/totp", TotpController, only: [:edit, :update], singleton: true - resources "/name", NameController, only: [:edit, :update], singleton: true - end - scope "/sessions", Session, as: :session do resources "/totp", TotpController, only: [:new, :create], singleton: true end end + scope "/", PhilomenaWeb do + pipe_through [ + :browser, + :ensure_not_banned, + :ensure_tor_authorized, + :redirect_if_user_is_authenticated + ] + + resources "/registrations", RegistrationController, only: [:new, :create], singleton: true + resources "/passwords", PasswordController, only: [:new, :create, :edit, :update] + resources "/confirmations", ConfirmationController, only: [:new, :create, :show] + resources "/unlocks", UnlockController, only: [:new, :create, :show] + end + + scope "/", PhilomenaWeb do + pipe_through [ + :browser, + :ensure_totp, + :ensure_tor_authorized, + :require_authenticated_user + ] + + resources "/registrations", RegistrationController, only: [:edit, :update], singleton: true + resources "/sessions", SessionController, only: [:delete], singleton: true + + scope "/registrations", Registration, as: :registration do + resources "/totp", TotpController, only: [:edit, :update], singleton: true + resources "/name", NameController, only: [:edit, :update], singleton: true + resources "/password", PasswordController, only: [:update], singleton: true + resources "/email", EmailController, only: [:create, :show] + end + end + scope "/api/v1/rss", PhilomenaWeb.Api.Rss, as: :api_rss do - pipe_through [:accepts_rss, :api, :protected] + pipe_through [:accepts_rss, :api, :require_authenticated_user] resources "/watched", WatchedController, only: [:index] end @@ -155,7 +157,7 @@ defmodule PhilomenaWeb.Router do end scope "/", PhilomenaWeb do - pipe_through [:browser, :ensure_totp, :protected] + pipe_through [:browser, :ensure_totp, :require_authenticated_user] scope "/notifications", Notification, as: :notification do resources "/unread", UnreadController, only: [:index] diff --git a/lib/philomena_web/templates/avatar/edit.html.slime b/lib/philomena_web/templates/avatar/edit.html.slime index 53ac6e8d..59ce5afb 100644 --- a/lib/philomena_web/templates/avatar/edit.html.slime +++ b/lib/philomena_web/templates/avatar/edit.html.slime @@ -1,5 +1,3 @@ - - .profile-top .profile-top__avatar = render PhilomenaWeb.UserAttributionView, "_user_avatar.html", object: %{user: @current_user}, conn: @conn @@ -43,7 +41,7 @@ br - return_to = if blank?(@conn.params["profile"]) do - = Routes.pow_registration_path(@conn, :edit) + = Routes.registration_path(@conn, :edit) - else = Routes.profile_path(@conn, :show, @current_user) = link "Back", to: return_to diff --git a/lib/philomena_web/templates/channel/subscription/_subscription.html.slime b/lib/philomena_web/templates/channel/subscription/_subscription.html.slime index e38a97fd..c12b47fa 100644 --- a/lib/philomena_web/templates/channel/subscription/_subscription.html.slime +++ b/lib/philomena_web/templates/channel/subscription/_subscription.html.slime @@ -17,7 +17,7 @@ elixir: span.hide-mobile ' Unsubscribe - else - a.media-box__header.media-box__header--channel.media-box__header--link href=Routes.pow_session_path(@conn, :new) + a.media-box__header.media-box__header--channel.media-box__header--link href=Routes.session_path(@conn, :new) i.fa.fa-bell> span.hide-mobile - ' Subscribe \ No newline at end of file + ' Subscribe diff --git a/lib/philomena_web/templates/confirmation/new.html.slime b/lib/philomena_web/templates/confirmation/new.html.slime new file mode 100644 index 00000000..be1fe8cd --- /dev/null +++ b/lib/philomena_web/templates/confirmation/new.html.slime @@ -0,0 +1,12 @@ +h1 Resend confirmation instructions + += form_for :user, Routes.confirmation_path(@conn, :create), fn f -> + .field + = email_input f, :email, placeholder: "Email", class: "input", required: true + + .field + = checkbox f, :captcha, class: "js-captcha", value: 0 + = label f, :captcha, "I am not a robot!" + + div + = submit "Resend confirmation instructions", class: "button" diff --git a/lib/philomena_web/templates/forum/subscription/_subscription.html.slime b/lib/philomena_web/templates/forum/subscription/_subscription.html.slime index a54ffca5..1477c4e7 100644 --- a/lib/philomena_web/templates/forum/subscription/_subscription.html.slime +++ b/lib/philomena_web/templates/forum/subscription/_subscription.html.slime @@ -17,7 +17,7 @@ elixir: span.hide-mobile ' Unsubscribe - else - a href=Routes.pow_session_path(@conn, :new) + a href=Routes.session_path(@conn, :new) i.fa.fa-bell> span.hide-mobile - ' Subscribe \ No newline at end of file + ' Subscribe diff --git a/lib/philomena_web/templates/gallery/subscription/_subscription.html.slime b/lib/philomena_web/templates/gallery/subscription/_subscription.html.slime index 43614c6c..55cec270 100644 --- a/lib/philomena_web/templates/gallery/subscription/_subscription.html.slime +++ b/lib/philomena_web/templates/gallery/subscription/_subscription.html.slime @@ -17,7 +17,7 @@ elixir: span.hide-mobile ' Unsubscribe - else - a href=Routes.pow_session_path(@conn, :new) + a href=Routes.session_path(@conn, :new) i.fa.fa-bell> span.hide-mobile - ' Subscribe \ No newline at end of file + ' Subscribe diff --git a/lib/philomena_web/templates/image/_add_to_gallery_dropdown.html.slime b/lib/philomena_web/templates/image/_add_to_gallery_dropdown.html.slime index a7e1bdb6..3c7b7659 100644 --- a/lib/philomena_web/templates/image/_add_to_gallery_dropdown.html.slime +++ b/lib/philomena_web/templates/image/_add_to_gallery_dropdown.html.slime @@ -36,6 +36,6 @@ i.fa.fa-plus> span.hide-limited-desktop.hide-mobile Create a gallery - else - a.block__list__link.primary href=Routes.pow_registration_path(@conn, :new) + a.block__list__link.primary href=Routes.session_path(@conn, :new) i.fa.fa-user-plus> - span.hide-limited-desktop.hide-mobile Register to create a gallery + span.hide-limited-desktop.hide-mobile Sign in to create a gallery diff --git a/lib/philomena_web/templates/image/reporting/show.html.slime b/lib/philomena_web/templates/image/reporting/show.html.slime index cbd206dc..5a18b6c8 100644 --- a/lib/philomena_web/templates/image/reporting/show.html.slime +++ b/lib/philomena_web/templates/image/reporting/show.html.slime @@ -15,7 +15,7 @@ a href=Routes.image_report_path(@conn, :new, @image) - else p ' You must - a> href=Routes.pow_session_path(@conn, :new) log in + a> href=Routes.session_path(@conn, :new) log in ' to report duplicate images. - target_reports = Enum.filter(@dupe_reports, & &1.duplicate_of_image_id == @image.id) diff --git a/lib/philomena_web/templates/image/subscription/_subscription.html.slime b/lib/philomena_web/templates/image/subscription/_subscription.html.slime index 4f62481c..063fd896 100644 --- a/lib/philomena_web/templates/image/subscription/_subscription.html.slime +++ b/lib/philomena_web/templates/image/subscription/_subscription.html.slime @@ -15,6 +15,6 @@ elixir: i.fa.fa-bell-slash> ' Unsubscribe - else - a href=Routes.pow_session_path(@conn, :new) + a href=Routes.session_path(@conn, :new) i.fa.fa-bell> ' Subscribe diff --git a/lib/philomena_web/templates/layout/_header.html.slime b/lib/philomena_web/templates/layout/_header.html.slime index 455c122d..c675e320 100644 --- a/lib/philomena_web/templates/layout/_header.html.slime +++ b/lib/philomena_web/templates/layout/_header.html.slime @@ -92,10 +92,10 @@ header.header a.header__link href="/conversations" i.fa.fa-fw.fa-envelope> | Messages - a.header__link href=Routes.pow_registration_path(@conn, :edit) + a.header__link href=Routes.registration_path(@conn, :edit) i.fa.fa-fw.fa-user> | Account - a.header__link href="/session" data-method="delete" + a.header__link href=Routes.session_path(@conn, :delete) data-method="delete" i.fa.fa-fw.fa-sign-out-alt> | Logout - else @@ -107,9 +107,9 @@ header.header a.header__link href="/settings/edit" i.fa.fa-fw.fa-cogs.hide-desktop> | Settings - a.header__link href="/registration/new" + a.header__link href=Routes.registration_path(@conn, :new) | Register - a.header__link href="/session/new" + a.header__link href=Routes.session_path(@conn, :new) | Login nav.header.header--secondary diff --git a/lib/philomena_web/templates/pow_reset_password/reset_password/edit.html.slime b/lib/philomena_web/templates/password/edit.html.slime similarity index 70% rename from lib/philomena_web/templates/pow_reset_password/reset_password/edit.html.slime rename to lib/philomena_web/templates/password/edit.html.slime index 7e5798b2..02a420f6 100644 --- a/lib/philomena_web/templates/pow_reset_password/reset_password/edit.html.slime +++ b/lib/philomena_web/templates/password/edit.html.slime @@ -1,6 +1,6 @@ h1 Reset password -= form_for @changeset, @action, [as: :user], fn f -> += form_for @changeset, Routes.password_path(@conn, :update, @token), fn f -> = if @changeset.action do .alert.alert-danger p Oops, something went wrong! Please check the errors below. @@ -10,7 +10,8 @@ h1 Reset password = error_tag f, :password .field - = password_input f, :password_confirmation, class: "input", placeholder: "Confirm password" + = password_input f, :password_confirmation, class: "input", placeholder: "Confirm new password" = error_tag f, :password_confirmation - = submit "Submit", class: "button" + div + = submit "Submit", class: "button" diff --git a/lib/philomena_web/templates/password/new.html.slime b/lib/philomena_web/templates/password/new.html.slime new file mode 100644 index 00000000..3bdd5553 --- /dev/null +++ b/lib/philomena_web/templates/password/new.html.slime @@ -0,0 +1,14 @@ +h1 Forgot your password? +p + ' Provide the email address you signed up with and we will email you + ' password reset instructions. + += form_for :user, Routes.password_path(@conn, :create), fn f -> + .field + = email_input f, :email, class: "input", placeholder: "Email", required: true + + .field + = checkbox f, :captcha, class: "js-captcha", value: 0 + = label f, :captcha, "I am not a robot!" + + = submit "Send instructions to reset password", class: "button" diff --git a/lib/philomena_web/templates/pow/session/new.html.slime b/lib/philomena_web/templates/pow/session/new.html.slime deleted file mode 100644 index d9673999..00000000 --- a/lib/philomena_web/templates/pow/session/new.html.slime +++ /dev/null @@ -1,28 +0,0 @@ -h1 Sign in - -= form_for @changeset, @action, [as: :user], fn f -> - = if @changeset.action do - .alert.alert-danger - p Oops, something went wrong! Please check the errors below. - - p = link "Forgot your password?", to: Routes.pow_reset_password_reset_password_path(@conn, :new) - - .field - = text_input f, :email, class: "input", required: true, placeholder: "Email", autofocus: true, pattern: ".*@.*" - = error_tag f, :email - - .field - = password_input f, :password, class: "input", required: true, placeholder: "Password" - = error_tag f, :password - - .field - = checkbox f, :persistent_session - = label f, :persistent_session, "Remember me" - - = submit "Sign in", class: "button" - -p - strong - ' Haven't read the - a<> href="/pages/rules" site rules - ' lately? Make sure you read them before posting or editing metadata! diff --git a/lib/philomena_web/templates/pow_reset_password/reset_password/new.html.slime b/lib/philomena_web/templates/pow_reset_password/reset_password/new.html.slime deleted file mode 100644 index c97c99dc..00000000 --- a/lib/philomena_web/templates/pow_reset_password/reset_password/new.html.slime +++ /dev/null @@ -1,18 +0,0 @@ -h1 Forgot your password? -p - ' Provide the email address you signed up with and we will email you - ' password reset instructions. - -= form_for @changeset, @action, [as: :user], fn f -> - = if @changeset.action do - .alert.alert-danger - p Oops, something went wrong! Please check the errors below. - - .field - = text_input f, :email, class: "input", placeholder: "Email" - - .field - = checkbox f, :captcha, class: "js-captcha", value: 0 - = label f, :captcha, "I am not a robot!" - - = submit "Submit", class: "button" \ No newline at end of file diff --git a/lib/philomena_web/templates/pow/registration/edit.html.slime b/lib/philomena_web/templates/registration/edit.html.slime similarity index 55% rename from lib/philomena_web/templates/pow/registration/edit.html.slime rename to lib/philomena_web/templates/registration/edit.html.slime index 2d65f96d..c105ddd6 100644 --- a/lib/philomena_web/templates/pow/registration/edit.html.slime +++ b/lib/philomena_web/templates/registration/edit.html.slime @@ -27,23 +27,31 @@ p ' Avoid sharing this key with others, as it could be used to compromise ' your account. -h3 Update Settings -p - strong - ' Don't forget to confirm your changes by entering your current password - ' at the bottom of the page! +h3 Change email -= form_for @changeset, @action, [as: :user], fn f -> - = if @changeset.action do += form_for @email_changeset, Routes.registration_email_path(@conn, :create), [method: :post], fn f -> + = if @email_changeset.action do .alert.alert-danger p Oops, something went wrong! Please check the errors below. - h3 Email address .field - = text_input f, :email, class: "input", placeholder: "Email", required: true, pattern: ".*@.*" + = email_input f, :email, class: "input", placeholder: "Email", required: true, pattern: ".*@.*" = error_tag f, :email - h3 Change Password + .field + = password_input f, :current_password, class: "input", required: true, name: "current_password", placeholder: "Current password" + = error_tag f, :current_password + + div + = submit "Change email", class: "button" + +h3 Change password + += form_for @password_changeset, Routes.registration_password_path(@conn, :update), fn f -> + = if @password_changeset.action do + .alert.alert-danger + p Oops, something went wrong! Please check the errors below. + .field = password_input f, :password, class: "input", placeholder: "New password" = error_tag f, :password @@ -52,17 +60,8 @@ p = password_input f, :password_confirmation, class: "input", placeholder: "Confirm new password" = error_tag f, :password_confirmation - .fieldlabel - ' Leave these blank if you don't want to change your password. + .field + = password_input f, :current_password, name: "current_password", class: "input", placeholder: "Current password" + = error_tag f, :current_password - br - - .block.block--fixed.block--warning - h3 Confirm - .field - = password_input f, :current_password, class: "input", placeholder: "Current password" - = error_tag f, :current_password - .fieldlabel - ' We need your current password to confirm all of these changes - - = submit "Save Account", class: "button" + = submit "Change password", class: "button" diff --git a/lib/philomena_web/templates/registration/name/edit.html.slime b/lib/philomena_web/templates/registration/name/edit.html.slime index a7737e76..529b57a1 100644 --- a/lib/philomena_web/templates/registration/name/edit.html.slime +++ b/lib/philomena_web/templates/registration/name/edit.html.slime @@ -14,4 +14,4 @@ h1 Editing Name .action = submit "Save", class: "button" -p = link "Back", to: Routes.pow_registration_path(@conn, :edit) +p = link "Back", to: Routes.registration_path(@conn, :edit) diff --git a/lib/philomena_web/templates/pow/registration/new.html.slime b/lib/philomena_web/templates/registration/new.html.slime similarity index 88% rename from lib/philomena_web/templates/pow/registration/new.html.slime rename to lib/philomena_web/templates/registration/new.html.slime index 1dc6b2fd..b181e086 100644 --- a/lib/philomena_web/templates/pow/registration/new.html.slime +++ b/lib/philomena_web/templates/registration/new.html.slime @@ -1,6 +1,6 @@ h1 Register -= form_for @changeset, @action, [as: :user], fn f -> += form_for @changeset, Routes.registration_path(@conn, :create), fn f -> = if @changeset.action do .alert.alert-danger p Oops, something went wrong! Please check the errors below. @@ -16,11 +16,11 @@ h1 Register ' You'll use your email address to log in, and we'll use this to get in ' touch if we need to. Don't worry, we won't share this or spam you. .field - = text_input f, :email, class: "input", placeholder: "Email", required: true, pattern: ".*@.*" + = email_input f, :email, class: "input", placeholder: "Email", required: true, pattern: ".*@.*" = error_tag f, :email .fieldlabel - ' Pick a good strong password - longer is better! Minimum of 6 characters. + ' Pick a good strong password - longer is better! Minimum of 12 characters. .field = password_input f, :password, class: "input", placeholder: "Password", required: true = error_tag f, :password diff --git a/lib/philomena_web/templates/registration/totp/edit.html.slime b/lib/philomena_web/templates/registration/totp/edit.html.slime index ccf9912d..f4c49089 100644 --- a/lib/philomena_web/templates/registration/totp/edit.html.slime +++ b/lib/philomena_web/templates/registration/totp/edit.html.slime @@ -135,4 +135,4 @@ h1 Two Factor Authentication = submit "Save Account", class: "button" -p = link "Back", to: Routes.pow_registration_path(@conn, :edit) +p = link "Back", to: Routes.registration_path(@conn, :edit) diff --git a/lib/philomena_web/templates/session/new.html.slime b/lib/philomena_web/templates/session/new.html.slime new file mode 100644 index 00000000..24a86c8f --- /dev/null +++ b/lib/philomena_web/templates/session/new.html.slime @@ -0,0 +1,28 @@ +h1 Sign in + += form_for @conn, Routes.session_path(@conn, :create), [as: :user], fn f -> + = if @error_message do + .alert.alert-danger + p = @error_message + + p = link "Forgot your password?", to: Routes.password_path(@conn, :new) + + .field + = email_input f, :email, class: "input", required: true, placeholder: "Email", autofocus: true, pattern: ".*@.*" + = error_tag f, :email + + .field + = password_input f, :password, class: "input", required: true, placeholder: "Password" + = error_tag f, :password + + .field + = checkbox f, :remember_me + = label f, :remember_me, "Remember me" + + = submit "Sign in", class: "button" + +p + strong + ' Haven't read the + a<> href="/pages/rules" site rules + ' lately? Make sure you read them before posting or editing metadata! diff --git a/lib/philomena_web/templates/session/totp/new.html.slime b/lib/philomena_web/templates/session/totp/new.html.slime index 8c8439b3..4c93e914 100644 --- a/lib/philomena_web/templates/session/totp/new.html.slime +++ b/lib/philomena_web/templates/session/totp/new.html.slime @@ -5,4 +5,8 @@ h1 Two Factor Authentication h4 Please enter your 2FA code = text_input f, :twofactor_token, class: "input", placeholder: "6-digit code", required: true, autofocus: true, autocomplete: "off" + .field + => checkbox f, :remember_me, class: "input" + = label f, :remember_me, "Remember this device" + = submit "Sign in", class: "button" diff --git a/lib/philomena_web/templates/setting/edit.html.slime b/lib/philomena_web/templates/setting/edit.html.slime index a7fe7e6f..40d87ec1 100644 --- a/lib/philomena_web/templates/setting/edit.html.slime +++ b/lib/philomena_web/templates/setting/edit.html.slime @@ -132,7 +132,7 @@ h1 Content Settings .block__tab.hidden data-tab="join-the-herd" p ' Consider - => link "creating an account!", to: Routes.pow_registration_path(@conn, :new) + => link "creating an account!", to: Routes.registration_path(@conn, :new) br ' You will be able to customize the number of images and comments you get on a single page, as well as change the appearance of the site with custom themes. diff --git a/lib/philomena_web/templates/topic/subscription/_subscription.html.slime b/lib/philomena_web/templates/topic/subscription/_subscription.html.slime index 7c5d9f6d..022a1965 100644 --- a/lib/philomena_web/templates/topic/subscription/_subscription.html.slime +++ b/lib/philomena_web/templates/topic/subscription/_subscription.html.slime @@ -17,7 +17,7 @@ elixir: span.hide-mobile ' Unsubscribe - else - a href=Routes.pow_session_path(@conn, :new) + a href=Routes.session_path(@conn, :new) i.fa.fa-bell> span.hide-mobile - ' Subscribe \ No newline at end of file + ' Subscribe diff --git a/lib/philomena_web/templates/unlock/new.html.slime b/lib/philomena_web/templates/unlock/new.html.slime new file mode 100644 index 00000000..e9ffb608 --- /dev/null +++ b/lib/philomena_web/templates/unlock/new.html.slime @@ -0,0 +1,12 @@ +h1 Resend unlock instructions + += form_for :user, Routes.unlock_path(@conn, :create), fn f -> + .field + = email_input f, :email, placeholder: "Email", class: "input", required: true + + .field + = checkbox f, :captcha, class: "js-captcha", value: 0 + = label f, :captcha, "I am not a robot!" + + div + = submit "Resend unlock instructions", class: "button" diff --git a/lib/philomena_web/user_auth.ex b/lib/philomena_web/user_auth.ex new file mode 100644 index 00000000..374e5379 --- /dev/null +++ b/lib/philomena_web/user_auth.ex @@ -0,0 +1,196 @@ +defmodule PhilomenaWeb.UserAuth do + import Plug.Conn + import Phoenix.Controller + + alias Philomena.Users + alias PhilomenaWeb.Router.Helpers, as: Routes + + # Make the remember me cookie valid for 365 days. + # If you want bump or reduce this value, also change + # the token expiry itself in UserToken. + @max_age 60 * 60 * 24 * 365 + @remember_me_cookie "user_remember_me" + @remember_me_options [sign: true, max_age: @max_age, same_site: "Lax"] + @totp_auth_cookie "user_totp_auth" + @totp_auth_options [sign: true, max_age: @max_age, same_site: "Lax"] + + @doc """ + Logs the user in. + + It renews the session ID and clears the whole session + to avoid fixation attacks. See the renew_session + function to customize this behaviour. + + It also sets a `:live_socket_id` key in the session, + so LiveView sessions are identified and automatically + disconnected on log out. The line can be safely removed + if you are not using LiveView. + """ + def log_in_user(conn, user, params \\ %{}) do + token = Users.generate_user_session_token(user) + user_return_to = get_session(conn, :user_return_to) + + conn + |> renew_session() + |> put_session(:user_token, token) + |> put_session(:live_socket_id, "users_sessions:#{Base.url_encode64(token)}") + |> maybe_write_remember_me_cookie(token, params) + |> redirect(to: user_return_to || signed_in_path(conn)) + end + + defp maybe_write_remember_me_cookie(conn, token, %{"remember_me" => "true"}) do + put_resp_cookie(conn, @remember_me_cookie, token, @remember_me_options) + end + + defp maybe_write_remember_me_cookie(conn, _token, _params) do + conn + end + + @doc """ + Writes TOTP session metadata for an authenticated user. + """ + def totp_auth_user(conn, user, params \\ %{}) do + token = Users.generate_user_totp_token(user) + user_return_to = get_session(conn, :user_return_to) + + conn + |> put_session(:totp_token, token) + |> maybe_write_totp_auth_cookie(token, params) + |> redirect(to: user_return_to || signed_in_path(conn)) + end + + defp maybe_write_totp_auth_cookie(conn, token, %{"remember_me" => "true"}) do + put_resp_cookie(conn, @totp_auth_cookie, token, @totp_auth_options) + end + + defp maybe_write_totp_auth_cookie(conn, _token, _params) do + conn + end + + # This function renews the session ID and erases the whole + # session to avoid fixation attacks. If there is any data + # in the session you may want to preserve after log in/log out, + # you must explicitly fetch the session data before clearing + # and then immediately set it after clearing, for example: + # + # defp renew_session(conn) do + # preferred_locale = get_session(conn, :preferred_locale) + # + # conn + # |> configure_session(renew: true) + # |> clear_session() + # |> put_session(:preferred_locale, preferred_locale) + # end + # + defp renew_session(conn) do + conn + |> configure_session(renew: true) + |> clear_session() + end + + @doc """ + Logs the user out. + + It clears all session data for safety. See renew_session. + """ + def log_out_user(conn) do + user_token = get_session(conn, :user_token) + user_token && Users.delete_session_token(user_token) + + totp_token = get_session(conn, :totp_token) + totp_token && Users.delete_totp_token(totp_token) + + if live_socket_id = get_session(conn, :live_socket_id) do + PhilomenaWeb.Endpoint.broadcast(live_socket_id, "disconnect", %{}) + end + + conn + |> renew_session() + |> delete_resp_cookie(@remember_me_cookie) + |> delete_resp_cookie(@totp_auth_cookie) + |> redirect(to: "/") + end + + @doc """ + Authenticates the user by looking into the session + and remember me token. + """ + def fetch_current_user(conn, _opts) do + {user_token, conn} = ensure_user_token(conn) + {totp_token, conn} = ensure_totp_token(conn) + + user = user_token && Users.get_user_by_session_token(user_token) + totp = totp_token && Users.user_totp_token_valid?(user, totp_token) + + conn + |> assign(:current_user, user) + |> assign(:totp_valid?, totp) + end + + defp ensure_user_token(conn) do + if user_token = get_session(conn, :user_token) do + {user_token, conn} + else + conn = fetch_cookies(conn, signed: [@remember_me_cookie]) + + if user_token = conn.cookies[@remember_me_cookie] do + {user_token, put_session(conn, :user_token, user_token)} + else + {nil, conn} + end + end + end + + defp ensure_totp_token(conn) do + if totp_token = get_session(conn, :totp_token) do + {totp_token, conn} + else + conn = fetch_cookies(conn, signed: [@totp_auth_cookie]) + + if totp_token = conn.cookies[@totp_auth_cookie] do + {totp_token, put_session(conn, :totp_token, totp_token)} + else + {nil, conn} + end + end + end + + @doc """ + Used for routes that require the user to not be authenticated. + """ + def redirect_if_user_is_authenticated(conn, _opts) do + if conn.assigns[:current_user] do + conn + |> redirect(to: signed_in_path(conn)) + |> halt() + else + conn + end + end + + @doc """ + Used for routes that require the user to be authenticated. + + If you want to enforce the user email is confirmed before + they use the application at all, here would be a good place. + """ + def require_authenticated_user(conn, _opts) do + if conn.assigns[:current_user] do + conn + else + conn + |> put_flash(:error, "You must log in to access this page.") + |> maybe_store_return_to() + |> redirect(to: Routes.session_path(conn, :new)) + |> halt() + end + end + + defp maybe_store_return_to(%{method: "GET", request_path: request_path} = conn) do + put_session(conn, :user_return_to, request_path) + end + + defp maybe_store_return_to(conn), do: conn + + defp signed_in_path(_conn), do: "/" +end diff --git a/lib/philomena_web/views/confirmation_view.ex b/lib/philomena_web/views/confirmation_view.ex new file mode 100644 index 00000000..780d70f2 --- /dev/null +++ b/lib/philomena_web/views/confirmation_view.ex @@ -0,0 +1,3 @@ +defmodule PhilomenaWeb.ConfirmationView do + use PhilomenaWeb, :view +end diff --git a/lib/philomena_web/views/password_view.ex b/lib/philomena_web/views/password_view.ex new file mode 100644 index 00000000..ec8f9b4d --- /dev/null +++ b/lib/philomena_web/views/password_view.ex @@ -0,0 +1,3 @@ +defmodule PhilomenaWeb.PasswordView do + use PhilomenaWeb, :view +end diff --git a/lib/philomena_web/views/pow/registration_view.ex b/lib/philomena_web/views/pow/registration_view.ex deleted file mode 100644 index 940fa244..00000000 --- a/lib/philomena_web/views/pow/registration_view.ex +++ /dev/null @@ -1,3 +0,0 @@ -defmodule PhilomenaWeb.Pow.RegistrationView do - use PhilomenaWeb, :view -end diff --git a/lib/philomena_web/views/pow/session_view.ex b/lib/philomena_web/views/pow/session_view.ex deleted file mode 100644 index ddf846a6..00000000 --- a/lib/philomena_web/views/pow/session_view.ex +++ /dev/null @@ -1,3 +0,0 @@ -defmodule PhilomenaWeb.Pow.SessionView do - use PhilomenaWeb, :view -end diff --git a/lib/philomena_web/views/pow_reset_password/reset_password_view.ex b/lib/philomena_web/views/pow_reset_password/reset_password_view.ex deleted file mode 100644 index 56320fcd..00000000 --- a/lib/philomena_web/views/pow_reset_password/reset_password_view.ex +++ /dev/null @@ -1,3 +0,0 @@ -defmodule PhilomenaWeb.PowResetPassword.ResetPasswordView do - use PhilomenaWeb, :view -end diff --git a/lib/philomena_web/views/registration_view.ex b/lib/philomena_web/views/registration_view.ex new file mode 100644 index 00000000..d22aa4df --- /dev/null +++ b/lib/philomena_web/views/registration_view.ex @@ -0,0 +1,3 @@ +defmodule PhilomenaWeb.RegistrationView do + use PhilomenaWeb, :view +end diff --git a/lib/philomena_web/views/session_view.ex b/lib/philomena_web/views/session_view.ex new file mode 100644 index 00000000..a198f704 --- /dev/null +++ b/lib/philomena_web/views/session_view.ex @@ -0,0 +1,3 @@ +defmodule PhilomenaWeb.SessionView do + use PhilomenaWeb, :view +end diff --git a/lib/philomena_web/views/unlock_view.ex b/lib/philomena_web/views/unlock_view.ex new file mode 100644 index 00000000..740b0396 --- /dev/null +++ b/lib/philomena_web/views/unlock_view.ex @@ -0,0 +1,3 @@ +defmodule PhilomenaWeb.UnlockView do + use PhilomenaWeb, :view +end diff --git a/lib/pow_captcha/captcha.ex b/lib/pow_captcha/captcha.ex deleted file mode 100644 index eaf724f2..00000000 --- a/lib/pow_captcha/captcha.ex +++ /dev/null @@ -1,7 +0,0 @@ -defmodule PowCaptcha do - @moduledoc false - use Pow.Extension.Base - - @impl true - def phoenix_controller_callbacks?(), do: true -end diff --git a/lib/pow_captcha/phoenix/controllers/controller_callbacks.ex b/lib/pow_captcha/phoenix/controllers/controller_callbacks.ex deleted file mode 100644 index 96c14ab5..00000000 --- a/lib/pow_captcha/phoenix/controllers/controller_callbacks.ex +++ /dev/null @@ -1,45 +0,0 @@ -defmodule PowCaptcha.Phoenix.ControllerCallbacks do - @moduledoc """ - Controller callback logic for captcha verification. - """ - use Pow.Extension.Phoenix.ControllerCallbacks.Base - - alias Pow.Config - alias Plug.Conn - alias Phoenix.Controller - - alias Pow.Phoenix.RegistrationController - alias PowResetPassword.Phoenix.ResetPasswordController - - @doc false - @impl true - def before_process(RegistrationController, :create, conn, config) do - verifier = Config.get(config, :captcha_verifier) - return_path = routes(conn).registration_path(conn, :new) - - verifier.valid_solution?(conn.params) - |> maybe_halt(conn, return_path) - end - - def before_process(ResetPasswordController, :create, conn, config) do - verifier = Config.get(config, :captcha_verifier) - return_path = routes(conn).path_for(conn, ResetPasswordController, :new) - - verifier.valid_solution?(conn.params) - |> maybe_halt(conn, return_path) - end - - defp maybe_halt(false, conn, return_path) do - conn - |> Controller.put_flash( - :error, - "There was an error verifying you're not a robot. Please try again." - ) - |> Controller.redirect(to: return_path) - |> Conn.halt() - end - - defp maybe_halt(true, conn, _return_path) do - conn - end -end diff --git a/lib/pow_lockout/ecto/context.ex b/lib/pow_lockout/ecto/context.ex deleted file mode 100644 index 2f1fff7d..00000000 --- a/lib/pow_lockout/ecto/context.ex +++ /dev/null @@ -1,63 +0,0 @@ -defmodule PowLockout.Ecto.Context do - @moduledoc """ - Handles lockout context for user. - """ - alias Pow.{Config, Ecto.Context} - alias PowLockout.Ecto.Schema - - @doc """ - Finds a user by the `:unlock_token` column. - """ - @spec get_by_unlock_token(binary(), Config.t()) :: Context.user() | nil - def get_by_unlock_token(token, config), - do: Context.get_by([unlock_token: token], config) - - @doc """ - Checks if the user is current locked out. - """ - @spec locked_out?(Context.user(), Config.t()) :: boolean() - def locked_out?(%{locked_at: time}, _config) when not is_nil(time), - do: true - - def locked_out?(_user, _config), - do: false - - @doc """ - Unlocks the account. - - See `PowLockout.Ecto.Schema.unlock_changeset/1`. - """ - @spec unlock_account(Context.user(), Config.t()) :: - {:ok, Context.user()} | {:error, Context.changeset()} - def unlock_account(user, config) do - user - |> Schema.unlock_changeset() - |> Context.do_update(config) - end - - @doc """ - Increases the attempts counter and possibly locks the account. - - See `PowLockout.Ecto.Schema.attempt_changeset/1`. - """ - @spec fail_attempt(Context.user(), Config.t()) :: - {:ok, Context.user()} | {:error, Context.changeset()} - def fail_attempt(user, config) do - user - |> Schema.attempt_changeset() - |> Context.do_update(config) - end - - @doc """ - Sets the attempts counter to zero. - - See `PowLockout.Ecto.Schema.attempt_reset_changeset/1`. - """ - @spec succeed_attempt(Context.user(), Config.t()) :: - {:ok, Context.user()} | {:error, Context.changeset()} - def succeed_attempt(user, config) do - user - |> Schema.attempt_reset_changeset() - |> Context.do_update(config) - end -end diff --git a/lib/pow_lockout/ecto/schema.ex b/lib/pow_lockout/ecto/schema.ex deleted file mode 100644 index 20338536..00000000 --- a/lib/pow_lockout/ecto/schema.ex +++ /dev/null @@ -1,132 +0,0 @@ -defmodule PowLockout.Ecto.Schema do - @moduledoc """ - Handles the lockout schema for user. - - ## Customize PowLockout fields - - If you need to modify any of the fields that `PowLockout` adds to - the user schema, you can override them by defining them before - `pow_user_fields/0`: - - defmodule MyApp.Users.User do - use Ecto.Schema - use Pow.Ecto.Schema - use Pow.Extension.Ecto.Schema, - extensions: [PowLockout] - - schema "users" do - field :unlock_token, :string - field :locked_at, :utc_datetime - field :failed_attempts, :integer - - pow_user_fields() - - timestamps() - end - end - """ - - use Pow.Extension.Ecto.Schema.Base - alias Ecto.Changeset - alias Pow.UUID - - @doc false - @impl true - def attrs(_config) do - [ - {:unlock_token, :string}, - {:locked_at, :utc_datetime}, - {:failed_attempts, :integer} - ] - end - - @doc false - @impl true - def indexes(_config) do - [{:unlock_token, true}] - end - - @doc """ - Sets the account as unlocked. - - This sets `:locked_at` and `:unlock_token` to nil, and sets - `failed_attempts` to 0. - """ - @spec unlock_changeset(Ecto.Schema.t() | Changeset.t()) :: Changeset.t() - def unlock_changeset(user_or_changeset) do - changes = [ - locked_at: nil, - unlock_token: nil, - failed_attempts: 0 - ] - - user_or_changeset - |> Changeset.change(changes) - end - - @doc """ - Sets the account as locked. - - This sets `:locked_at` to now and sets `:unlock_token` to a random UUID. - """ - @spec lock_changeset(Ecto.Schema.t() | Changeset.t()) :: Changeset.t() - def lock_changeset(user_or_changeset) do - changeset = Changeset.change(user_or_changeset) - locked_at = Pow.Ecto.Schema.__timestamp_for__(changeset.data.__struct__, :locked_at) - - changes = [ - locked_at: locked_at, - unlock_token: UUID.generate() - ] - - changeset - |> Changeset.change(changes) - end - - @doc """ - Updates the failed attempt count. - - This increments `:failed_attempts` by 1, or sets it to 1 if it is nil. - The first time it becomes greater than 10, it also locks the user. - """ - @spec attempt_changeset(Ecto.Schema.t() | Changeset.t()) :: Changeset.t() - def attempt_changeset(%Changeset{data: %{failed_attempts: attempts}} = changeset) - when is_integer(attempts) and attempts < 10 do - Changeset.change(changeset, failed_attempts: attempts + 1) - end - - def attempt_changeset( - %Changeset{data: %{failed_attempts: attempts, locked_at: nil}} = changeset - ) - when is_integer(attempts) do - lock_changeset(changeset) - end - - def attempt_changeset( - %Changeset{data: %{failed_attempts: attempts, locked_at: _locked_at}} = changeset - ) - when is_integer(attempts) do - changeset - end - - def attempt_changeset(%Changeset{} = changeset) do - Changeset.change(changeset, failed_attempts: 1) - end - - def attempt_changeset(user) do - user - |> Changeset.change() - |> attempt_changeset() - end - - @doc """ - Resets the failed attempt count. - - This sets `:failed_attempts` to 0. - """ - @spec attempt_reset_changeset(Ecto.Schema.t() | Changeset.t()) :: Changeset.t() - def attempt_reset_changeset(user_or_changeset) do - user_or_changeset - |> Changeset.change(failed_attempts: 0) - end -end diff --git a/lib/pow_lockout/lockout.ex b/lib/pow_lockout/lockout.ex deleted file mode 100644 index 21e21269..00000000 --- a/lib/pow_lockout/lockout.ex +++ /dev/null @@ -1,16 +0,0 @@ -defmodule PowLockout do - @moduledoc false - use Pow.Extension.Base - - @impl true - def ecto_schema?(), do: true - - @impl true - def phoenix_controller_callbacks?(), do: true - - @impl true - def phoenix_router?(), do: true - - @impl true - def phoenix_messages?(), do: true -end diff --git a/lib/pow_lockout/phoenix/controllers/controller_callbacks.ex b/lib/pow_lockout/phoenix/controllers/controller_callbacks.ex deleted file mode 100644 index 166b8ab3..00000000 --- a/lib/pow_lockout/phoenix/controllers/controller_callbacks.ex +++ /dev/null @@ -1,93 +0,0 @@ -defmodule PowLockout.Phoenix.ControllerCallbacks do - @moduledoc """ - Controller callback logic for account lockout. - - ### User is locked out - - Triggers on `Pow.Phoenix.SessionController.create/2`. - - When a user is locked out, the credentials will be treated as if they were - invalid and the user will be redirected back to `Pow.Phoenix.Routes.`. - - ### User successfully authenticates - - Triggers on `Pow.Phoenix.SessionController.create/2`. - - When a user successfully signs in, the failed attempts counter will be - reset to zero. - - ### Users unsuccessfully authenticates - - Triggers on `Pow.Phoenix.SessionController.create/2`. - - When a user unsuccessfully signs in, the failed attempts counter will be - incremented, and the user may be locked out. - - See `PowLockout.Ecto.Schema` for more. - """ - use Pow.Extension.Phoenix.ControllerCallbacks.Base - - alias Plug.Conn - alias Pow.Plug - alias PowLockout.Phoenix.{UnlockController, Mailer} - alias PowLockout.Plug, as: PowLockoutPlug - - @doc false - @impl true - def before_respond(Pow.Phoenix.SessionController, :create, {result, conn}, _config) do - PowLockoutPlug.user_for_attempts(conn) - |> maybe_fail_attempt(conn, result) - end - - defp maybe_fail_attempt(nil, conn, result), - do: {result, conn} - - defp maybe_fail_attempt(%{locked_at: nil} = user, conn, :ok) do - case PowLockoutPlug.succeed_attempt(conn, user) do - {:error, _changeset, conn} -> - {:halt, conn} - - {:ok, _user, conn} -> - {:ok, conn} - end - end - - defp maybe_fail_attempt(_locked_user, conn, :ok) do - {:error, invalid_credentials(conn)} - end - - defp maybe_fail_attempt(user, conn, _error) do - PowLockoutPlug.fail_attempt(conn, user) - |> case do - {:error, _changeset, conn} -> - {:halt, conn} - - {:ok, %{locked_at: nil}, conn} -> - {:error, invalid_credentials(conn)} - - {:ok, user, conn} -> - send_unlock_email(user, conn) - - {:error, invalid_credentials(conn)} - end - end - - defp invalid_credentials(conn) do - Plug.delete(conn) - end - - @doc """ - Sends an unlock e-mail to the user. - """ - @spec send_unlock_email(map(), Conn.t()) :: any() - def send_unlock_email(user, conn) do - url = unlock_url(conn, user.unlock_token) - email = Mailer.email_unlock(conn, user, url) - - Pow.Phoenix.Mailer.deliver(conn, email) - end - - defp unlock_url(conn, token) do - routes(conn).url_for(conn, UnlockController, :show, [token]) - end -end diff --git a/lib/pow_lockout/phoenix/controllers/unlock_controller.ex b/lib/pow_lockout/phoenix/controllers/unlock_controller.ex deleted file mode 100644 index 824b0d34..00000000 --- a/lib/pow_lockout/phoenix/controllers/unlock_controller.ex +++ /dev/null @@ -1,23 +0,0 @@ -defmodule PowLockout.Phoenix.UnlockController do - @moduledoc false - use Pow.Extension.Phoenix.Controller.Base - - alias Plug.Conn - alias PowLockout.Plug - - @spec process_show(Conn.t(), map()) :: {:ok | :error, map(), Conn.t()} - def process_show(conn, %{"id" => token}), do: Plug.unlock_account(conn, token) - - @spec respond_show({:ok | :error, map(), Conn.t()}) :: Conn.t() - def respond_show({:ok, _user, conn}) do - conn - |> put_flash(:info, extension_messages(conn).account_has_been_unlocked(conn)) - |> redirect(to: routes(conn).session_path(conn, :new)) - end - - def respond_show({:error, _changeset, conn}) do - conn - |> put_flash(:error, extension_messages(conn).account_unlock_failed(conn)) - |> redirect(to: routes(conn).session_path(conn, :new)) - end -end diff --git a/lib/pow_lockout/phoenix/mailers/mailer.ex b/lib/pow_lockout/phoenix/mailers/mailer.ex deleted file mode 100644 index 68e04dbf..00000000 --- a/lib/pow_lockout/phoenix/mailers/mailer.ex +++ /dev/null @@ -1,11 +0,0 @@ -defmodule PowLockout.Phoenix.Mailer do - @moduledoc false - alias Plug.Conn - alias Pow.Phoenix.Mailer.Mail - alias PowLockout.Phoenix.MailerView - - @spec email_unlock(Conn.t(), map(), binary()) :: Mail.t() - def email_unlock(conn, user, url) do - Mail.new(conn, user, {MailerView, :email_unlock}, url: url) - end -end diff --git a/lib/pow_lockout/phoenix/mailers/mailer_template.ex b/lib/pow_lockout/phoenix/mailers/mailer_template.ex deleted file mode 100644 index 0f877976..00000000 --- a/lib/pow_lockout/phoenix/mailers/mailer_template.ex +++ /dev/null @@ -1,25 +0,0 @@ -defmodule PowLockout.Phoenix.MailerTemplate do - @moduledoc false - use Pow.Phoenix.Mailer.Template - - template( - :email_unlock, - "Unlock your account", - """ - Hi, - - Your account has been automatically disabled due to too many unsuccessful - attempts to sign in. - - Please use the following link to unlock your account: - - <%= @url %> - """, - """ - <%= content_tag(:h3, "Hi,") %> - <%= content_tag(:p, "Your account has been automatically disabled due to too many unsuccessful attempts to sign in.") %> - <%= content_tag(:p, "Please use the following link to unlock your account:") %> - <%= content_tag(:p, link(@url, to: @url)) %> - """ - ) -end diff --git a/lib/pow_lockout/phoenix/mailers/mailer_view.ex b/lib/pow_lockout/phoenix/mailers/mailer_view.ex deleted file mode 100644 index 08828ae6..00000000 --- a/lib/pow_lockout/phoenix/mailers/mailer_view.ex +++ /dev/null @@ -1,4 +0,0 @@ -defmodule PowLockout.Phoenix.MailerView do - @moduledoc false - use Pow.Phoenix.Mailer.View -end diff --git a/lib/pow_lockout/phoenix/messages.ex b/lib/pow_lockout/phoenix/messages.ex deleted file mode 100644 index 32d4c9c1..00000000 --- a/lib/pow_lockout/phoenix/messages.ex +++ /dev/null @@ -1,13 +0,0 @@ -defmodule PowLockout.Phoenix.Messages do - @moduledoc false - - @doc """ - Flash message to show when account has been unlocked. - """ - def account_has_been_unlocked(_conn), do: "Account successfully unlocked. You may now log in." - - @doc """ - Flash message to show when account couldn't be unlocked. - """ - def account_unlock_failed(_conn), do: "Account unlock failed." -end diff --git a/lib/pow_lockout/phoenix/router.ex b/lib/pow_lockout/phoenix/router.ex deleted file mode 100644 index e469cd26..00000000 --- a/lib/pow_lockout/phoenix/router.ex +++ /dev/null @@ -1,12 +0,0 @@ -defmodule PowLockout.Phoenix.Router do - @moduledoc false - use Pow.Extension.Phoenix.Router.Base - - alias Pow.Phoenix.Router - - defmacro routes(_config) do - quote location: :keep do - Router.pow_resources("/unlock", UnlockController, only: [:show]) - end - end -end diff --git a/lib/pow_lockout/plug.ex b/lib/pow_lockout/plug.ex deleted file mode 100644 index f4fa79d6..00000000 --- a/lib/pow_lockout/plug.ex +++ /dev/null @@ -1,86 +0,0 @@ -defmodule PowLockout.Plug do - @moduledoc """ - Plug helper methods. - """ - alias Plug.Conn - alias Pow.Plug - alias PowLockout.Ecto.Context - require IEx - - @doc """ - Check if the current user is locked out. - """ - @spec locked_out?(Conn.t()) :: boolean() - def locked_out?(conn) do - config = Plug.fetch_config(conn) - - conn - |> Plug.current_user() - |> Context.locked_out?(config) - end - - @doc """ - Get the user belonging to the id field. - """ - @spec user_for_attempts(Conn.t()) :: map() | nil - def user_for_attempts(conn) do - config = Plug.fetch_config(conn) - id_field = Pow.Ecto.Schema.user_id_field(config) - id_value = to_string(conn.params["user"][to_string(id_field)]) - - Pow.Ecto.Context.get_by([{id_field, id_value}], config) - end - - @doc """ - Unlocks the user found by the provided unlock token. - """ - @spec unlock_account(Conn.t(), binary()) :: {:ok, map(), Conn.t()} | {:error, map(), Conn.t()} - def unlock_account(conn, token) do - config = Plug.fetch_config(conn) - - token - |> Context.get_by_unlock_token(config) - |> maybe_unlock_account(conn, config) - end - - defp maybe_unlock_account(nil, conn, _config) do - {:error, nil, conn} - end - - defp maybe_unlock_account(user, conn, config) do - user - |> Context.unlock_account(config) - |> case do - {:error, changeset} -> {:error, changeset, conn} - {:ok, user} -> {:ok, user, conn} - end - end - - @doc """ - Increments the failed attempts counter and possibly locks the user out. - """ - @spec fail_attempt(Conn.t(), map()) :: {:ok, map(), Conn.t()} | {:error, map(), Conn.t()} - def fail_attempt(conn, user) do - config = Plug.fetch_config(conn) - - Context.fail_attempt(user, config) - |> case do - {:error, changeset} -> {:error, changeset, conn} - {:ok, user} -> {:ok, user, conn} - end - end - - @doc """ - Resets the failed attempts counter to 0. - """ - @spec succeed_attempt(Conn.t(), map()) :: {:ok, map(), Conn.t()} | {:error, map(), Conn.t()} - def succeed_attempt(conn, user) do - config = Plug.fetch_config(conn) - - Context.succeed_attempt(user, config) - |> case do - {:error, changeset} -> {:error, changeset, conn} - {:ok, user} -> {:ok, user, conn} - end - end -end diff --git a/mix.exs b/mix.exs index 8629f050..5398627d 100644 --- a/mix.exs +++ b/mix.exs @@ -45,8 +45,7 @@ defmodule Philomena.MixProject do {:plug_cowboy, "~> 2.3"}, {:phoenix_slime, "~> 0.13"}, {:ecto_network, "~> 1.3"}, - {:pow, "~> 1.0"}, - {:bcrypt_elixir, "~> 2.2"}, + {:bcrypt_elixir, "~> 2.0"}, {:pot, "~> 0.11"}, {:secure_compare, "~> 0.1.0"}, {:elastix, "~> 0.8.0"}, @@ -67,7 +66,6 @@ defmodule Philomena.MixProject do {:tesla, "~> 1.3"}, {:castore, "~> 0.1"}, {:mint, "~> 1.1"}, - {:libcluster, "~> 3.2"}, {:exq, "~> 0.13"}, {:dialyxir, "~> 1.0", only: :dev, runtime: false} ] @@ -90,8 +88,7 @@ defmodule Philomena.MixProject do ], "ecto.reset": ["ecto.drop", "ecto.setup"], "ecto.migrate": ["ecto.migrate", "ecto.dump"], - "ecto.rollback": ["ecto.rollback", "ecto.dump"], - test: ["ecto.create --quiet", "ecto.load", "test"] + "ecto.rollback": ["ecto.rollback", "ecto.dump"] ] end end diff --git a/priv/repo/migrations/20200725234412_create_users_auth_tables.exs b/priv/repo/migrations/20200725234412_create_users_auth_tables.exs new file mode 100644 index 00000000..8f526dd1 --- /dev/null +++ b/priv/repo/migrations/20200725234412_create_users_auth_tables.exs @@ -0,0 +1,31 @@ +defmodule Philomena.Repo.Migrations.CreateUsersAuthTables do + use Ecto.Migration + + def change do + alter table(:users) do + add :confirmed_at, :naive_datetime + end + + create table(:user_tokens) do + add :user_id, references(:users, on_delete: :delete_all), null: false + add :token, :binary, null: false + add :context, :string, null: false + add :sent_to, :string + timestamps(inserted_at: :created_at, updated_at: false) + end + + execute(&email_citext_up/0, &email_citext_down/0) + create index(:user_tokens, [:user_id]) + create unique_index(:user_tokens, [:context, :token]) + end + + defp email_citext_up() do + repo().query!("create extension citext") + repo().query!("alter table users alter column email type citext") + end + + defp email_citext_down() do + repo().query!("alter table users alter column email type character varying") + repo().query!("drop extension citext") + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 13ebd626..7c75aaeb 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -26,6 +26,7 @@ alias Philomena.{ } alias Philomena.Elasticsearch +alias Philomena.Users alias Philomena.Tags import Ecto.Query @@ -76,9 +77,13 @@ end IO.puts("---- Generating users") for user_def <- resources["users"] do - %User{role: user_def["role"]} - |> User.creation_changeset(user_def) - |> Repo.insert(on_conflict: :nothing) + {:ok, user} = Users.register_user(user_def) + + user + |> Repo.preload([:roles]) + |> User.confirm_changeset() + |> User.update_changeset(%{role: user_def["role"]}, []) + |> Repo.update!() end IO.puts("---- Generating roles") diff --git a/priv/repo/seeds.json b/priv/repo/seeds.json index ac40820d..aae34185 100644 --- a/priv/repo/seeds.json +++ b/priv/repo/seeds.json @@ -64,7 +64,6 @@ "name": "Administrator", "email": "admin@example.com", "password": "trixieisbestpony", - "password_confirmation": "trixieisbestpony", "role": "admin" }], "rating_tags": [ diff --git a/priv/repo/seeds_development.exs b/priv/repo/seeds_development.exs index 315f71c1..848f09d8 100644 --- a/priv/repo/seeds_development.exs +++ b/priv/repo/seeds_development.exs @@ -10,7 +10,7 @@ # We recommend using the bang functions (`insert!`, `update!` # and so on) as they will fail if something goes wrong. -alias Philomena.{Repo, Forums.Forum, Users.User} +alias Philomena.{Repo, Forums.Forum, Users, Users.User} alias Philomena.Comments alias Philomena.Images alias Philomena.Topics @@ -27,9 +27,13 @@ resources = IO.puts "---- Generating users" for user_def <- resources["users"] do - %User{role: user_def["role"]} - |> User.creation_changeset(user_def) - |> Repo.insert(on_conflict: :nothing) + {:ok, user} = Users.register_user(user_def) + + user + |> Repo.preload([:roles]) + |> User.confirm_changeset() + |> User.update_changeset(%{role: user_def["role"]}, []) + |> Repo.update!() end pleb = Repo.get_by!(User, name: "Pleb") diff --git a/priv/repo/seeds_development.json b/priv/repo/seeds_development.json index f6371b65..bebde1b1 100644 --- a/priv/repo/seeds_development.json +++ b/priv/repo/seeds_development.json @@ -3,21 +3,18 @@ "name": "Hot Pocket Consumer", "email": "moderator@example.com", "password": "willdeleteglimmerposts4hotpockets", - "password_confirmation": "willdeleteglimmerposts4hotpockets", "role": "moderator" }, { "name": "Hoping For a Promotion", "email": "assistant@example.com", "password": "hotpocketfetchingass", - "password_confirmation": "hotpocketfetchingass", "role": "assistant" }, { "name": "Pleb", "email": "user@example.com", "password": "glimmerpostingplebeian", - "password_confirmation": "glimmerpostingplebeian", "role": "user" } ], diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index 04ce14c9..b90b65dc 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -16,6 +16,20 @@ SET xmloption = content; SET client_min_messages = warning; SET row_security = off; +-- +-- Name: citext; Type: EXTENSION; Schema: -; Owner: - +-- + +CREATE EXTENSION IF NOT EXISTS citext WITH SCHEMA public; + + +-- +-- Name: EXTENSION citext; Type: COMMENT; Schema: -; Owner: - +-- + +COMMENT ON EXTENSION citext IS 'data type for case-insensitive character strings'; + + SET default_tablespace = ''; SET default_table_access_method = heap; @@ -1816,6 +1830,39 @@ CREATE SEQUENCE public.user_statistics_id_seq ALTER SEQUENCE public.user_statistics_id_seq OWNED BY public.user_statistics.id; +-- +-- Name: user_tokens; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.user_tokens ( + id bigint NOT NULL, + user_id bigint NOT NULL, + token bytea NOT NULL, + context character varying(255) NOT NULL, + sent_to character varying(255), + created_at timestamp(0) without time zone NOT NULL +); + + +-- +-- Name: user_tokens_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.user_tokens_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: user_tokens_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.user_tokens_id_seq OWNED BY public.user_tokens.id; + + -- -- Name: user_whitelists; Type: TABLE; Schema: public; Owner: - -- @@ -1854,7 +1901,7 @@ ALTER SEQUENCE public.user_whitelists_id_seq OWNED BY public.user_whitelists.id; CREATE TABLE public.users ( id integer NOT NULL, - email character varying DEFAULT ''::character varying NOT NULL, + email public.citext DEFAULT ''::character varying NOT NULL, encrypted_password character varying DEFAULT ''::character varying NOT NULL, reset_password_token character varying, reset_password_sent_at timestamp without time zone, @@ -1926,7 +1973,8 @@ CREATE TABLE public.users ( otp_required_for_login boolean, otp_backup_codes character varying[], last_renamed_at timestamp without time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, - forced_filter_id bigint + forced_filter_id bigint, + confirmed_at timestamp(0) without time zone ); @@ -2310,6 +2358,13 @@ ALTER TABLE ONLY public.user_name_changes ALTER COLUMN id SET DEFAULT nextval('p ALTER TABLE ONLY public.user_statistics ALTER COLUMN id SET DEFAULT nextval('public.user_statistics_id_seq'::regclass); +-- +-- Name: user_tokens id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_tokens ALTER COLUMN id SET DEFAULT nextval('public.user_tokens_id_seq'::regclass); + + -- -- Name: user_whitelists id; Type: DEFAULT; Schema: public; Owner: - -- @@ -2691,6 +2746,14 @@ ALTER TABLE ONLY public.user_statistics ADD CONSTRAINT user_statistics_pkey PRIMARY KEY (id); +-- +-- Name: user_tokens user_tokens_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_tokens + ADD CONSTRAINT user_tokens_pkey PRIMARY KEY (id); + + -- -- Name: user_whitelists user_whitelists_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -3891,6 +3954,20 @@ CREATE INDEX index_vpns_on_ip ON public.vpns USING gist (ip inet_ops); CREATE INDEX intensities_index ON public.images USING btree (se_intensity, sw_intensity, ne_intensity, nw_intensity, average_intensity); +-- +-- Name: user_tokens_context_token_index; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX user_tokens_context_token_index ON public.user_tokens USING btree (context, token); + + +-- +-- Name: user_tokens_user_id_index; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX user_tokens_user_id_index ON public.user_tokens USING btree (user_id); + + -- -- Name: channels fk_rails_021c624081; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -4691,6 +4768,14 @@ ALTER TABLE ONLY public.image_sources ADD CONSTRAINT image_sources_image_id_fkey FOREIGN KEY (image_id) REFERENCES public.images(id); +-- +-- Name: user_tokens user_tokens_user_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_tokens + ADD CONSTRAINT user_tokens_user_id_fkey FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE; + + -- -- Name: users users_forced_filter_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -4708,3 +4793,4 @@ INSERT INTO public."schema_migrations" (version) VALUES (20200607000511); INSERT INTO public."schema_migrations" (version) VALUES (20200617111116); INSERT INTO public."schema_migrations" (version) VALUES (20200617113333); INSERT INTO public."schema_migrations" (version) VALUES (20200706171350); +INSERT INTO public."schema_migrations" (version) VALUES (20200725234412); diff --git a/test/philomena/users_test.exs b/test/philomena/users_test.exs new file mode 100644 index 00000000..be34bbfc --- /dev/null +++ b/test/philomena/users_test.exs @@ -0,0 +1,627 @@ +defmodule Philomena.UsersTest do + use Philomena.DataCase + + alias Philomena.Users + import Philomena.UsersFixtures + alias Philomena.Users.{User, UserToken} + + describe "get_user_by_email/1" do + test "does not return the user if the email does not exist" do + refute Users.get_user_by_email("unknown@example.com") + end + + test "returns the user if the email exists" do + %{id: id} = user = user_fixture() + assert %User{id: ^id} = Users.get_user_by_email(user.email) + end + end + + describe "get_user_by_email_and_password/3" do + test "does not return the user if the email does not exist" do + refute Users.get_user_by_email_and_password("unknown@example.com", "hello world!", & &1) + end + + test "does not return the user if the password is not valid" do + user = user_fixture() + refute Users.get_user_by_email_and_password(user.email, "invalid", & &1) + + user = Users.get_user!(user.id) + assert user.failed_attempts == 1 + end + + test "sends lock email if too many attempts to sign in are made" do + user = user_fixture() + + Enum.map(1..10, fn _ -> + refute Users.get_user_by_email_and_password(user.email, "invalid", & &1) + end) + + user = Users.get_user!(user.id) + + token = + extract_user_token(fn url -> + Users.deliver_user_unlock_instructions(user, url) + end) + + {:ok, token} = Base.url_decode64(token, padding: false) + assert user_token = Repo.get_by(UserToken, token: :crypto.hash(:sha256, token)) + assert user_token.user_id == user.id + assert user_token.sent_to == user.email + assert user_token.context == "unlock" + assert user.failed_attempts == 10 + refute is_nil(user.locked_at) + end + + test "denies access to account if locked" do + user = user_fixture() + + Enum.map(1..10, fn _ -> + refute Users.get_user_by_email_and_password(user.email, "invalid", & &1) + end) + + refute Users.get_user_by_email_and_password(user.email, valid_user_password(), & &1) + end + + test "returns the user if the email and password are valid" do + %{id: id} = user = user_fixture() + + assert %User{id: ^id} = + Users.get_user_by_email_and_password(user.email, valid_user_password(), & &1) + end + end + + describe "get_user!/1" do + test "raises if id is invalid" do + assert_raise Ecto.NoResultsError, fn -> + Users.get_user!(-1) + end + end + + test "returns the user with the given id" do + %{id: id} = user = user_fixture() + assert %User{id: ^id} = Users.get_user!(user.id) + end + end + + describe "register_user/1" do + test "requires email and password to be set" do + {:error, changeset} = Users.register_user(%{}) + + assert %{ + password: ["can't be blank"], + email: ["can't be blank"] + } = errors_on(changeset) + end + + test "validates email and password when given" do + {:error, changeset} = Users.register_user(%{email: "not valid", password: "not valid"}) + + assert %{ + email: ["must have the @ sign and no spaces"], + password: ["should be at least 12 character(s)"] + } = errors_on(changeset) + end + + test "validates maximum values for email and password for security" do + too_long = String.duplicate("db", 100) + {:error, changeset} = Users.register_user(%{email: too_long, password: too_long}) + assert "should be at most 160 character(s)" in errors_on(changeset).email + assert "should be at most 80 character(s)" in errors_on(changeset).password + end + + test "validates email uniqueness" do + %{email: email} = user_fixture() + + {:error, changeset} = Users.register_user(%{name: email, email: email}) + assert "has already been taken" in errors_on(changeset).email + + # Now try with the upper cased email too, to check that email case is ignored. + {:error, changeset} = + Users.register_user(%{ + name: String.upcase(email), + email: String.upcase(email), + password: valid_user_password() + }) + + assert "has already been taken" in errors_on(changeset).email + end + + test "registers users with a hashed password" do + email = unique_user_email() + + {:ok, user} = + Users.register_user(%{name: email, email: email, password: valid_user_password()}) + + assert user.email == email + assert is_binary(user.hashed_password) + assert is_nil(user.confirmed_at) + assert is_nil(user.password) + end + end + + describe "change_user_registration/2" do + test "returns a changeset" do + assert %Ecto.Changeset{} = changeset = Users.change_user_registration(%User{}) + assert changeset.required == [:password, :email, :name] + end + end + + describe "change_user_email/2" do + test "returns a user changeset" do + assert %Ecto.Changeset{} = changeset = Users.change_user_email(%User{}) + assert changeset.required == [:email] + end + end + + describe "apply_user_email/3" do + setup do + %{user: user_fixture()} + end + + test "requires email to change", %{user: user} do + {:error, changeset} = Users.apply_user_email(user, valid_user_password(), %{}) + assert %{email: ["did not change"]} = errors_on(changeset) + end + + test "validates email", %{user: user} do + {:error, changeset} = + Users.apply_user_email(user, valid_user_password(), %{email: "not valid"}) + + assert %{email: ["must have the @ sign and no spaces"]} = errors_on(changeset) + end + + test "validates maximum value for email for security", %{user: user} do + too_long = String.duplicate("db", 100) + + {:error, changeset} = + Users.apply_user_email(user, valid_user_password(), %{email: too_long}) + + assert "should be at most 160 character(s)" in errors_on(changeset).email + end + + test "validates email uniqueness", %{user: user} do + %{email: email} = user_fixture() + + {:error, changeset} = Users.apply_user_email(user, valid_user_password(), %{email: email}) + + assert "has already been taken" in errors_on(changeset).email + end + + test "validates current password", %{user: user} do + {:error, changeset} = Users.apply_user_email(user, "invalid", %{email: unique_user_email()}) + + assert %{current_password: ["is not valid"]} = errors_on(changeset) + end + + test "applies the email without persisting it", %{user: user} do + email = unique_user_email() + {:ok, user} = Users.apply_user_email(user, valid_user_password(), %{email: email}) + assert user.email == email + assert Users.get_user!(user.id).email != email + end + end + + describe "deliver_update_email_instructions/3" do + setup do + %{user: user_fixture()} + end + + test "sends token through notification", %{user: user} do + token = + extract_user_token(fn url -> + Users.deliver_update_email_instructions(user, "current@example.com", url) + end) + + {:ok, token} = Base.url_decode64(token, padding: false) + assert user_token = Repo.get_by(UserToken, token: :crypto.hash(:sha256, token)) + assert user_token.user_id == user.id + assert user_token.sent_to == user.email + assert user_token.context == "change:current@example.com" + end + end + + describe "update_user_email/2" do + setup do + user = user_fixture() + email = unique_user_email() + + token = + extract_user_token(fn url -> + Users.deliver_update_email_instructions(%{user | email: email}, user.email, url) + end) + + %{user: user, token: token, email: email} + end + + test "updates the email with a valid token", %{user: user, token: token, email: email} do + assert Users.update_user_email(user, token) == :ok + changed_user = Repo.get!(User, user.id) + assert changed_user.email != user.email + assert changed_user.email == email + assert changed_user.confirmed_at + assert changed_user.confirmed_at != user.confirmed_at + refute Repo.get_by(UserToken, user_id: user.id) + end + + test "does not update email with invalid token", %{user: user} do + assert Users.update_user_email(user, "oops") == :error + assert Repo.get!(User, user.id).email == user.email + assert Repo.get_by(UserToken, user_id: user.id) + end + + test "does not update email if user email changed", %{user: user, token: token} do + assert Users.update_user_email(%{user | email: "current@example.com"}, token) == :error + assert Repo.get!(User, user.id).email == user.email + assert Repo.get_by(UserToken, user_id: user.id) + end + + test "does not update email if token expired", %{user: user, token: token} do + {1, nil} = Repo.update_all(UserToken, set: [created_at: ~N[2020-01-01 00:00:00]]) + assert Users.update_user_email(user, token) == :error + assert Repo.get!(User, user.id).email == user.email + assert Repo.get_by(UserToken, user_id: user.id) + end + end + + describe "change_user_password/2" do + test "returns a user changeset" do + assert %Ecto.Changeset{} = changeset = Users.change_user_password(%User{}) + assert changeset.required == [:password] + end + end + + describe "update_user_password/3" do + setup do + %{user: user_fixture()} + end + + test "validates password", %{user: user} do + {:error, changeset} = + Users.update_user_password(user, valid_user_password(), %{ + password: "not valid", + password_confirmation: "another" + }) + + assert %{ + password: ["should be at least 12 character(s)"], + password_confirmation: ["does not match password"] + } = errors_on(changeset) + end + + test "validates maximum values for password for security", %{user: user} do + too_long = String.duplicate("db", 200) + + {:error, changeset} = + Users.update_user_password(user, valid_user_password(), %{password: too_long}) + + assert "should be at most 80 character(s)" in errors_on(changeset).password + end + + test "validates current password", %{user: user} do + {:error, changeset} = + Users.update_user_password(user, "invalid", %{password: valid_user_password()}) + + assert %{current_password: ["is not valid"]} = errors_on(changeset) + end + + test "updates the password", %{user: user} do + {:ok, user} = + Users.update_user_password(user, valid_user_password(), %{ + password: "new valid password" + }) + + assert is_nil(user.password) + assert Users.get_user_by_email_and_password(user.email, "new valid password", & &1) + end + + test "deletes all tokens for the given user", %{user: user} do + _ = Users.generate_user_session_token(user) + + {:ok, _} = + Users.update_user_password(user, valid_user_password(), %{ + password: "new valid password" + }) + + refute Repo.get_by(UserToken, user_id: user.id) + end + end + + describe "generate_user_session_token/1" do + setup do + %{user: user_fixture()} + end + + test "generates a token", %{user: user} do + token = Users.generate_user_session_token(user) + assert user_token = Repo.get_by(UserToken, token: token) + assert user_token.context == "session" + + # Creating the same token for another user should fail + assert_raise Ecto.ConstraintError, fn -> + Repo.insert!(%UserToken{ + token: user_token.token, + user_id: user_fixture().id, + context: "session" + }) + end + end + end + + describe "get_user_by_session_token/1" do + setup do + user = user_fixture() + token = Users.generate_user_session_token(user) + %{user: user, token: token} + end + + test "returns user by token", %{user: user, token: token} do + assert session_user = Users.get_user_by_session_token(token) + assert session_user.id == user.id + end + + test "does not return user for invalid token" do + refute Users.get_user_by_session_token("oops") + end + + test "does not return user for expired token", %{token: token} do + {1, nil} = Repo.update_all(UserToken, set: [created_at: ~N[2019-01-01 00:00:00]]) + refute Users.get_user_by_session_token(token) + end + end + + describe "delete_session_token/1" do + test "deletes the token" do + user = user_fixture() + token = Users.generate_user_session_token(user) + assert Users.delete_session_token(token) == :ok + refute Users.get_user_by_session_token(token) + end + end + + describe "generate_user_totp_token/1" do + setup do + %{user: user_fixture()} + end + + test "generates a token", %{user: user} do + token = Users.generate_user_totp_token(user) + assert user_token = Repo.get_by(UserToken, token: token) + assert user_token.context == "totp" + + # Creating the same token for another user should fail + assert_raise Ecto.ConstraintError, fn -> + Repo.insert!(%UserToken{ + token: user_token.token, + user_id: user_fixture().id, + context: "totp" + }) + end + end + end + + describe "user_totp_token_valid?/1" do + setup do + user = user_fixture() + token = Users.generate_user_totp_token(user) + %{user: user, token: token} + end + + test "returns true for valid token", %{user: user, token: token} do + assert Users.user_totp_token_valid?(user, token) + end + + test "returns false for invalid token", %{user: user} do + refute Users.user_totp_token_valid?(user, "oops") + end + + test "returns false for expired token", %{user: user, token: token} do + {1, nil} = Repo.update_all(UserToken, set: [created_at: ~N[2019-01-01 00:00:00]]) + refute Users.user_totp_token_valid?(user, token) + end + end + + describe "delete_totp_token/1" do + test "deletes the token" do + user = user_fixture() + token = Users.generate_user_totp_token(user) + assert Users.delete_totp_token(token) == :ok + refute Users.user_totp_token_valid?(user, token) + end + end + + describe "deliver_user_confirmation_instructions/2" do + setup do + %{user: user_fixture()} + end + + test "sends token through notification", %{user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_confirmation_instructions(user, url) + end) + + {:ok, token} = Base.url_decode64(token, padding: false) + assert user_token = Repo.get_by(UserToken, token: :crypto.hash(:sha256, token)) + assert user_token.user_id == user.id + assert user_token.sent_to == user.email + assert user_token.context == "confirm" + end + end + + describe "confirm_user/2" do + setup do + user = user_fixture() + + token = + extract_user_token(fn url -> + Users.deliver_user_confirmation_instructions(user, url) + end) + + %{user: user, token: token} + end + + test "confirms the email with a valid token", %{user: user, token: token} do + assert {:ok, confirmed_user} = Users.confirm_user(token) + assert confirmed_user.confirmed_at + assert confirmed_user.confirmed_at != user.confirmed_at + assert Repo.get!(User, user.id).confirmed_at + refute Repo.get_by(UserToken, user_id: user.id) + end + + test "does not confirm with invalid token", %{user: user} do + assert Users.confirm_user("oops") == :error + refute Repo.get!(User, user.id).confirmed_at + assert Repo.get_by(UserToken, user_id: user.id) + end + + test "does not confirm email if token expired", %{user: user, token: token} do + {1, nil} = Repo.update_all(UserToken, set: [created_at: ~N[2020-01-01 00:00:00]]) + assert Users.confirm_user(token) == :error + refute Repo.get!(User, user.id).confirmed_at + assert Repo.get_by(UserToken, user_id: user.id) + end + end + + describe "deliver_user_unlock_instructions/2" do + setup do + %{user: locked_user_fixture()} + end + + test "sends token through notification", %{user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_unlock_instructions(user, url) + end) + + {:ok, token} = Base.url_decode64(token, padding: false) + assert user_token = Repo.get_by(UserToken, token: :crypto.hash(:sha256, token)) + assert user_token.user_id == user.id + assert user_token.sent_to == user.email + assert user_token.context == "unlock" + end + end + + describe "unlock_user/2" do + setup do + user = locked_user_fixture() + + token = + extract_user_token(fn url -> + Users.deliver_user_unlock_instructions(user, url) + end) + + %{user: user, token: token} + end + + test "unlocks the user with a valid token", %{user: user, token: token} do + assert {:ok, unlocked_user} = Users.unlock_user(token) + refute unlocked_user.locked_at + refute Repo.get!(User, user.id).locked_at + refute Repo.get_by(UserToken, user_id: user.id) + end + + test "does not confirm with invalid token", %{user: user} do + assert Users.unlock_user("oops") == :error + assert Repo.get!(User, user.id).locked_at + assert Repo.get_by(UserToken, user_id: user.id) + end + + test "does not unlocked if token expired", %{user: user, token: token} do + {1, nil} = Repo.update_all(UserToken, set: [created_at: ~N[2020-01-01 00:00:00]]) + assert Users.unlock_user(token) == :error + assert Repo.get!(User, user.id).locked_at + assert Repo.get_by(UserToken, user_id: user.id) + end + end + + describe "deliver_user_reset_password_instructions/2" do + setup do + %{user: user_fixture()} + end + + test "sends token through notification", %{user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_reset_password_instructions(user, url) + end) + + {:ok, token} = Base.url_decode64(token, padding: false) + assert user_token = Repo.get_by(UserToken, token: :crypto.hash(:sha256, token)) + assert user_token.user_id == user.id + assert user_token.sent_to == user.email + assert user_token.context == "reset_password" + end + end + + describe "get_user_by_reset_password_token/2" do + setup do + user = user_fixture() + + token = + extract_user_token(fn url -> + Users.deliver_user_reset_password_instructions(user, url) + end) + + %{user: user, token: token} + end + + test "returns the user with valid token", %{user: %{id: id}, token: token} do + assert %User{id: ^id} = Users.get_user_by_reset_password_token(token) + assert Repo.get_by(UserToken, user_id: id) + end + + test "does not return the user with invalid token", %{user: user} do + refute Users.get_user_by_reset_password_token("oops") + assert Repo.get_by(UserToken, user_id: user.id) + end + + test "does not return the user if token expired", %{user: user, token: token} do + {1, nil} = Repo.update_all(UserToken, set: [created_at: ~N[2020-01-01 00:00:00]]) + refute Users.get_user_by_reset_password_token(token) + assert Repo.get_by(UserToken, user_id: user.id) + end + end + + describe "reset_user_password/3" do + setup do + %{user: user_fixture()} + end + + test "validates password", %{user: user} do + {:error, changeset} = + Users.reset_user_password(user, %{ + password: "not valid", + password_confirmation: "another" + }) + + assert %{ + password: ["should be at least 12 character(s)"], + password_confirmation: ["does not match password"] + } = errors_on(changeset) + end + + test "validates maximum values for password for security", %{user: user} do + too_long = String.duplicate("db", 100) + {:error, changeset} = Users.reset_user_password(user, %{password: too_long}) + assert "should be at most 80 character(s)" in errors_on(changeset).password + end + + test "updates the password", %{user: user} do + {:ok, updated_user} = Users.reset_user_password(user, %{password: "new valid password"}) + assert is_nil(updated_user.password) + assert Users.get_user_by_email_and_password(user.email, "new valid password", & &1) + end + + test "deletes all tokens for the given user", %{user: user} do + _ = Users.generate_user_session_token(user) + {:ok, _} = Users.reset_user_password(user, %{password: "new valid password"}) + refute Repo.get_by(UserToken, user_id: user.id) + end + end + + describe "inspect/2" do + test "does not include password" do + refute inspect(%User{password: "123456"}) =~ "password: \"123456\"" + end + end +end diff --git a/test/philomena_web/controllers/confirmation_controller_test.exs b/test/philomena_web/controllers/confirmation_controller_test.exs new file mode 100644 index 00000000..f5c0a2f1 --- /dev/null +++ b/test/philomena_web/controllers/confirmation_controller_test.exs @@ -0,0 +1,84 @@ +defmodule PhilomenaWeb.ConfirmationControllerTest do + use PhilomenaWeb.ConnCase, async: true + + alias Philomena.Users + alias Philomena.Repo + import Philomena.UsersFixtures + + setup do + %{user: user_fixture()} + end + + describe "GET /confirmations/new" do + test "renders the confirmation page", %{conn: conn} do + conn = get(conn, Routes.confirmation_path(conn, :new)) + response = html_response(conn, 200) + assert response =~ "

Resend confirmation instructions

" + end + end + + describe "POST /confirmations" do + @tag :capture_log + test "sends a new confirmation token", %{conn: conn, user: user} do + conn = + post(conn, Routes.confirmation_path(conn, :create), %{ + "user" => %{"email" => user.email} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + assert Repo.get_by!(Users.UserToken, user_id: user.id).context == "confirm" + end + + test "does not send confirmation token if account is confirmed", %{conn: conn, user: user} do + Repo.update!(Users.User.confirm_changeset(user)) + + conn = + post(conn, Routes.confirmation_path(conn, :create), %{ + "user" => %{"email" => user.email} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + refute Repo.get_by(Users.UserToken, user_id: user.id) + end + + test "does not send confirmation token if email is invalid", %{conn: conn} do + conn = + post(conn, Routes.confirmation_path(conn, :create), %{ + "user" => %{"email" => "unknown@example.com"} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + assert Repo.all(Users.UserToken) == [] + end + end + + describe "GET /confirmations/:id" do + test "confirms the given token once", %{conn: conn, user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_confirmation_instructions(user, url) + end) + + conn = get(conn, Routes.confirmation_path(conn, :show, token)) + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "Account confirmed successfully" + assert Users.get_user!(user.id).confirmed_at + refute get_session(conn, :user_token) + assert Repo.all(Users.UserToken) == [] + + conn = get(conn, Routes.confirmation_path(conn, :show, token)) + assert redirected_to(conn) == "/" + assert get_flash(conn, :error) =~ "Confirmation link is invalid or it has expired" + end + + test "does not confirm email with invalid token", %{conn: conn, user: user} do + conn = get(conn, Routes.confirmation_path(conn, :show, "oops")) + assert redirected_to(conn) == "/" + assert get_flash(conn, :error) =~ "Confirmation link is invalid or it has expired" + refute Users.get_user!(user.id).confirmed_at + end + end +end diff --git a/test/philomena_web/controllers/password_controller_test.exs b/test/philomena_web/controllers/password_controller_test.exs new file mode 100644 index 00000000..78bd4701 --- /dev/null +++ b/test/philomena_web/controllers/password_controller_test.exs @@ -0,0 +1,111 @@ +defmodule PhilomenaWeb.PasswordControllerTest do + use PhilomenaWeb.ConnCase, async: true + + alias Philomena.Users + alias Philomena.Repo + import Philomena.UsersFixtures + + setup do + %{user: confirmed_user_fixture()} + end + + describe "GET /passwords/new" do + test "renders the reset password page", %{conn: conn} do + conn = get(conn, Routes.password_path(conn, :new)) + html_response(conn, 200) + end + end + + describe "POST /passwords" do + @tag :capture_log + test "sends a new reset password token", %{conn: conn, user: user} do + conn = + post(conn, Routes.password_path(conn, :create), %{ + "user" => %{"email" => user.email} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + assert Repo.get_by!(Users.UserToken, user_id: user.id).context == "reset_password" + end + + test "does not send reset password token if email is invalid", %{conn: conn} do + conn = + post(conn, Routes.password_path(conn, :create), %{ + "user" => %{"email" => "unknown@example.com"} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + assert Repo.all(Users.UserToken) == [] + end + end + + describe "GET /passwords/:token" do + setup %{user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_reset_password_instructions(user, url) + end) + + %{token: token} + end + + test "renders reset password", %{conn: conn, token: token} do + conn = get(conn, Routes.password_path(conn, :edit, token)) + html_response(conn, 200) + end + + test "does not render reset password with invalid token", %{conn: conn} do + conn = get(conn, Routes.password_path(conn, :edit, "oops")) + assert redirected_to(conn) == "/" + assert get_flash(conn, :error) =~ "Reset password link is invalid or it has expired" + end + end + + describe "PUT /passwords/:token" do + setup %{user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_reset_password_instructions(user, url) + end) + + %{token: token} + end + + test "resets password once", %{conn: conn, user: user, token: token} do + conn = + put(conn, Routes.password_path(conn, :update, token), %{ + "user" => %{ + "password" => "new valid password", + "password_confirmation" => "new valid password" + } + }) + + assert redirected_to(conn) == Routes.session_path(conn, :new) + refute get_session(conn, :user_token) + assert get_flash(conn, :info) =~ "Password reset successfully" + assert Users.get_user_by_email_and_password(user.email, "new valid password", & &1) + end + + test "does not reset password on invalid data", %{conn: conn, token: token} do + conn = + put(conn, Routes.password_path(conn, :update, token), %{ + "user" => %{ + "password" => "too short", + "password_confirmation" => "does not match" + } + }) + + response = html_response(conn, 200) + assert response =~ "should be at least 12 character" + assert response =~ "does not match password" + end + + test "does not reset password with invalid token", %{conn: conn} do + conn = put(conn, Routes.password_path(conn, :update, "oops")) + assert redirected_to(conn) == "/" + assert get_flash(conn, :error) =~ "Reset password link is invalid or it has expired" + end + end +end diff --git a/test/philomena_web/controllers/registration/email_controller_test.exs b/test/philomena_web/controllers/registration/email_controller_test.exs new file mode 100644 index 00000000..e2f2f5bd --- /dev/null +++ b/test/philomena_web/controllers/registration/email_controller_test.exs @@ -0,0 +1,71 @@ +defmodule PhilomenaWeb.Registration.EmailControllerTest do + use PhilomenaWeb.ConnCase, async: true + + alias Philomena.Users + import Philomena.UsersFixtures + + setup :register_and_log_in_user + + describe "POST /registrations/email" do + @tag :capture_log + test "updates the user email", %{conn: conn, user: user} do + conn = + post(conn, Routes.registration_email_path(conn, :create), %{ + "current_password" => valid_user_password(), + "user" => %{"email" => unique_user_email()} + }) + + assert redirected_to(conn) == Routes.registration_path(conn, :edit) + assert get_flash(conn, :info) =~ "A link to confirm your email" + assert Users.get_user_by_email(user.email) + end + + test "does not update email on invalid data", %{conn: conn} do + conn = + post(conn, Routes.registration_email_path(conn, :create), %{ + "current_password" => "invalid", + "user" => %{"email" => "with spaces"} + }) + + assert get_flash(conn, :error) =~ "Failed to update email" + end + end + + describe "GET /registrations/email/:token" do + setup %{user: user} do + email = unique_user_email() + + token = + extract_user_token(fn url -> + Users.deliver_update_email_instructions(%{user | email: email}, user.email, url) + end) + + %{token: token, email: email} + end + + test "updates the user email once", %{conn: conn, user: user, token: token, email: email} do + conn = get(conn, Routes.registration_email_path(conn, :show, token)) + assert redirected_to(conn) == Routes.registration_path(conn, :edit) + assert get_flash(conn, :info) =~ "Email changed successfully" + refute Users.get_user_by_email(user.email) + assert Users.get_user_by_email(email) + + conn = get(conn, Routes.registration_email_path(conn, :show, token)) + assert redirected_to(conn) == Routes.registration_path(conn, :edit) + assert get_flash(conn, :error) =~ "Email change link is invalid or it has expired" + end + + test "does not update email with invalid token", %{conn: conn, user: user} do + conn = get(conn, Routes.registration_email_path(conn, :show, "oops")) + assert redirected_to(conn) == Routes.registration_path(conn, :edit) + assert get_flash(conn, :error) =~ "Email change link is invalid or it has expired" + assert Users.get_user_by_email(user.email) + end + + test "redirects if user is not logged in", %{token: token} do + conn = build_conn() + conn = get(conn, Routes.registration_email_path(conn, :show, token)) + assert redirected_to(conn) == Routes.session_path(conn, :new) + end + end +end diff --git a/test/philomena_web/controllers/registration/password_controller_test.exs b/test/philomena_web/controllers/registration/password_controller_test.exs new file mode 100644 index 00000000..c8ad2581 --- /dev/null +++ b/test/philomena_web/controllers/registration/password_controller_test.exs @@ -0,0 +1,41 @@ +defmodule PhilomenaWeb.Registration.PasswordControllerTest do + use PhilomenaWeb.ConnCase, async: true + + alias Philomena.Users + import Philomena.UsersFixtures + + setup :register_and_log_in_user + + describe "PUT /registrations/password" do + test "updates the user password and resets tokens", %{conn: conn, user: user} do + new_password_conn = + put(conn, Routes.registration_password_path(conn, :update), %{ + "current_password" => valid_user_password(), + "user" => %{ + "password" => "new valid password", + "password_confirmation" => "new valid password" + } + }) + + assert redirected_to(new_password_conn) == Routes.registration_path(conn, :edit) + assert get_session(new_password_conn, :user_token) != get_session(conn, :user_token) + assert get_flash(new_password_conn, :info) =~ "Password updated successfully" + assert Users.get_user_by_email_and_password(user.email, "new valid password", & &1) + end + + test "does not update password on invalid data", %{conn: conn} do + old_password_conn = + put(conn, Routes.registration_password_path(conn, :update), %{ + "current_password" => "invalid", + "user" => %{ + "password" => "too short", + "password_confirmation" => "does not match" + } + }) + + assert redirected_to(old_password_conn) == Routes.registration_path(conn, :edit) + assert get_flash(old_password_conn, :error) =~ "Failed to update password" + assert get_session(old_password_conn, :user_token) == get_session(conn, :user_token) + end + end +end diff --git a/test/philomena_web/controllers/registration_controller_test.exs b/test/philomena_web/controllers/registration_controller_test.exs new file mode 100644 index 00000000..98a5811f --- /dev/null +++ b/test/philomena_web/controllers/registration_controller_test.exs @@ -0,0 +1,64 @@ +defmodule PhilomenaWeb.RegistrationControllerTest do + use PhilomenaWeb.ConnCase, async: true + + import Philomena.UsersFixtures + + describe "GET /registrations/new" do + test "renders registration page", %{conn: conn} do + conn = get(conn, Routes.registration_path(conn, :new)) + html_response(conn, 200) + end + + test "redirects if already logged in", %{conn: conn} do + conn = + conn |> log_in_user(confirmed_user_fixture()) |> get(Routes.registration_path(conn, :new)) + + assert redirected_to(conn) == "/" + end + end + + describe "POST /registrations" do + @tag :capture_log + test "creates account but doesn't log the user in", %{conn: conn} do + email = unique_user_email() + + conn = + post(conn, Routes.registration_path(conn, :create), %{ + "user" => %{"name" => email, "email" => email, "password" => valid_user_password()} + }) + + assert redirected_to(conn) =~ "/" + + conn = get(conn, "/sessions/new") + html_response(conn, 200) + assert get_flash(conn, :info) =~ "email for confirmation instructions" + end + + test "render errors for invalid data", %{conn: conn} do + conn = + post(conn, Routes.registration_path(conn, :create), %{ + "user" => %{"email" => "with spaces", "password" => "too short"} + }) + + response = html_response(conn, 200) + assert response =~ "must have the @ sign and no spaces" + assert response =~ "should be at least 12 character" + end + end + + describe "GET /registration/edit" do + setup :register_and_log_in_user + + test "renders settings page", %{conn: conn} do + conn = get(conn, Routes.registration_path(conn, :edit)) + response = html_response(conn, 200) + assert response =~ "Settings" + end + + test "redirects if user is not logged in" do + conn = build_conn() + conn = get(conn, Routes.registration_path(conn, :edit)) + assert redirected_to(conn) == Routes.session_path(conn, :new) + end + end +end diff --git a/test/philomena_web/controllers/session_controller_test.exs b/test/philomena_web/controllers/session_controller_test.exs new file mode 100644 index 00000000..1b9e95a6 --- /dev/null +++ b/test/philomena_web/controllers/session_controller_test.exs @@ -0,0 +1,73 @@ +defmodule PhilomenaWeb.SessionControllerTest do + use PhilomenaWeb.ConnCase, async: true + + import Philomena.UsersFixtures + + setup do + %{user: confirmed_user_fixture()} + end + + describe "GET /sessions/new" do + test "renders log in page", %{conn: conn} do + conn = get(conn, Routes.session_path(conn, :new)) + html_response(conn, 200) + end + + test "redirects if already logged in", %{conn: conn, user: user} do + conn = conn |> log_in_user(user) |> get(Routes.session_path(conn, :new)) + assert redirected_to(conn) == "/" + end + end + + describe "POST /sessions" do + test "logs the user in", %{conn: conn, user: user} do + conn = + post(conn, Routes.session_path(conn, :create), %{ + "user" => %{"email" => user.email, "password" => valid_user_password()} + }) + + assert get_session(conn, :user_token) + assert redirected_to(conn) =~ "/" + + # Now do a logged in request and assert on the menu + conn = get(conn, "/registrations/edit") + response = html_response(conn, 200) + assert response =~ user.email + assert response =~ "Settings" + assert response =~ "Logout" + end + + test "logs the user in with remember me", %{conn: conn, user: user} do + conn = + post(conn, Routes.session_path(conn, :create), %{ + "user" => %{ + "email" => user.email, + "password" => valid_user_password(), + "remember_me" => "true" + } + }) + + assert conn.resp_cookies["user_remember_me"] + assert redirected_to(conn) =~ "/" + end + + test "emits error message with invalid credentials", %{conn: conn, user: user} do + conn = + post(conn, Routes.session_path(conn, :create), %{ + "user" => %{"email" => user.email, "password" => "invalid_password"} + }) + + response = html_response(conn, 200) + assert response =~ "Invalid email or password" + end + end + + describe "DELETE /sessions" do + test "logs the user out", %{conn: conn, user: user} do + conn = conn |> log_in_user(user) |> delete(Routes.session_path(conn, :delete)) + assert redirected_to(conn) == "/" + refute get_session(conn, :user_token) + assert get_flash(conn, :info) =~ "Logged out successfully" + end + end +end diff --git a/test/philomena_web/controllers/unlock_controller_test.exs b/test/philomena_web/controllers/unlock_controller_test.exs new file mode 100644 index 00000000..c7945e62 --- /dev/null +++ b/test/philomena_web/controllers/unlock_controller_test.exs @@ -0,0 +1,84 @@ +defmodule PhilomenaWeb.UnlockControllerTest do + use PhilomenaWeb.ConnCase, async: true + + alias Philomena.Users + alias Philomena.Repo + import Philomena.UsersFixtures + + setup do + %{user: locked_user_fixture()} + end + + describe "GET /unlocks/new" do + test "renders the unlock page", %{conn: conn} do + conn = get(conn, Routes.unlock_path(conn, :new)) + response = html_response(conn, 200) + assert response =~ "

Resend unlock instructions

" + end + end + + describe "POST /unlocks" do + @tag :capture_log + test "sends a new unlock token", %{conn: conn, user: user} do + conn = + post(conn, Routes.unlock_path(conn, :create), %{ + "user" => %{"email" => user.email} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + assert Repo.get_by!(Users.UserToken, user_id: user.id).context == "unlock" + end + + test "does not send unlock token if account is not locked", %{conn: conn, user: user} do + Repo.update!(Users.User.unlock_changeset(user)) + + conn = + post(conn, Routes.unlock_path(conn, :create), %{ + "user" => %{"email" => user.email} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + refute Repo.get_by(Users.UserToken, user_id: user.id) + end + + test "does not send unlock token if email is invalid", %{conn: conn} do + conn = + post(conn, Routes.unlock_path(conn, :create), %{ + "user" => %{"email" => "unknown@example.com"} + }) + + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "If your email is in our system" + assert Repo.all(Users.UserToken) == [] + end + end + + describe "GET /unlocks/:id" do + test "confirms the given token once", %{conn: conn, user: user} do + token = + extract_user_token(fn url -> + Users.deliver_user_unlock_instructions(user, url) + end) + + conn = get(conn, Routes.unlock_path(conn, :show, token)) + assert redirected_to(conn) == "/" + assert get_flash(conn, :info) =~ "Account unlocked successfully" + refute Users.get_user!(user.id).locked_at + refute get_session(conn, :user_token) + assert Repo.all(Users.UserToken) == [] + + conn = get(conn, Routes.unlock_path(conn, :show, token)) + assert redirected_to(conn) == "/" + assert get_flash(conn, :error) =~ "Unlock link is invalid or it has expired" + end + + test "does not unlock with invalid token", %{conn: conn, user: user} do + conn = get(conn, Routes.unlock_path(conn, :show, "oops")) + assert redirected_to(conn) == "/" + assert get_flash(conn, :error) =~ "Unlock link is invalid or it has expired" + assert Users.get_user!(user.id).locked_at + end + end +end diff --git a/test/philomena_web/plug/pow_invalidated_session_plug_test.exs b/test/philomena_web/plug/pow_invalidated_session_plug_test.exs deleted file mode 100644 index 73b26192..00000000 --- a/test/philomena_web/plug/pow_invalidated_session_plug_test.exs +++ /dev/null @@ -1,199 +0,0 @@ -defmodule PhilomenaWeb.PowInvalidatedSessionPlugTest do - use PhilomenaWeb.ConnCase - doctest PhilomenaWeb.PowInvalidatedSessionPlug - - alias PhilomenaWeb.PowInvalidatedSessionPlug - alias Philomena.{Users.User, Repo} - - @otp_app :philomena - @config [otp_app: @otp_app, user: User, repo: Repo] - @session_key "#{@otp_app}_auth" - @cookie_key "#{@otp_app}_persistent_session" - @invalidated_ttl 250 - - alias Plug.{Conn, Test} - alias Plug.Session, as: PlugSession - alias Pow.Plug.Session - alias PowPersistentSession.Plug.Cookie - - setup do - user = - %User{authentication_token: "token", name: "John Doe", slug: "john-doe"} - |> User.changeset(%{ - "email" => "test@example.com", - "password" => "password", - "password_confirmation" => "password" - }) - |> Repo.insert!() - - {:ok, user: user} - end - - test "call/2 session id is reusable for short amount of time", %{conn: init_conn, user: user} do - config = Keyword.put(@config, :session_ttl_renewal, 0) - init_conn = prepare_session_conn(init_conn, user, config) - - assert session_id = - init_conn - |> init_session_plug() - |> Conn.fetch_session() - |> Conn.get_session(@session_key) - - conn = run_plug(init_conn, config) - - assert Pow.Plug.current_user(conn).id == user.id - assert Conn.get_session(conn, @session_key) != session_id - assert metadata = conn.private[:pow_session_metadata] - refute metadata[:valid_totp_at] - - :timer.sleep(100) - conn = run_plug(init_conn, config) - - assert Pow.Plug.current_user(conn).id == user.id - assert Conn.get_session(conn, @session_key) == session_id - assert metadata = conn.private[:pow_session_metadata] - refute metadata[:valid_totp_at] - - :timer.sleep(@invalidated_ttl - 100) - conn = run_plug(init_conn) - - refute Pow.Plug.current_user(conn) - end - - test "call/2 persistent session id is reusable", %{conn: init_conn, user: user} do - init_conn = prepare_persistent_session_conn(init_conn, user) - - assert persistent_session_id = init_conn.req_cookies[@cookie_key] - - conn = run_plug(init_conn) - - assert Pow.Plug.current_user(conn).id == user.id - assert conn.cookies[@cookie_key] != persistent_session_id - assert metadata = conn.private[:pow_session_metadata] - refute metadata[:valid_totp_at] - - :timer.sleep(100) - conn = run_plug(init_conn) - - assert Pow.Plug.current_user(conn).id == user.id - assert conn.cookies[@cookie_key] == persistent_session_id - assert metadata = conn.private[:pow_session_metadata] - refute metadata[:valid_totp_at] - - :timer.sleep(@invalidated_ttl - 100) - conn = run_plug(init_conn) - - refute Pow.Plug.current_user(conn) - assert conn.cookies[@cookie_key] == persistent_session_id - end - - test "call/2 with TOTP turned on", %{conn: init_conn, user: user} do - user = - user - |> Ecto.Changeset.change(%{otp_required_for_login: true}) - |> Repo.update!() - - config = Keyword.put(@config, :session_ttl_renewal, 0) - - no_otp_auth_conn = - init_conn - |> prepare_session_conn(user, config) - |> init_plug(config) - - assert no_otp_auth_conn.halted - - init_conn = - init_conn - |> Conn.put_private(:pow_session_metadata, valid_totp_at: DateTime.utc_now()) - |> prepare_session_conn(user, config) - - conn = run_plug(init_conn, config) - - assert Pow.Plug.current_user(conn).id == user.id - assert metadata = conn.private[:pow_session_metadata] - assert metadata[:valid_totp_at] - assert metadata[:inserted_at] - - :timer.sleep(100) - conn = run_plug(init_conn, config) - - assert Pow.Plug.current_user(conn).id == user.id - assert metadata = conn.private[:pow_session_metadata] - assert metadata[:valid_totp_at] - refute metadata[:inserted_at] - end - - defp init_session_plug(conn) do - conn - |> Map.put(:secret_key_base, String.duplicate("abcdefghijklmnopqrstuvxyz0123456789", 2)) - |> PlugSession.call(PlugSession.init(store: :cookie, key: "foobar", signing_salt: "salt")) - end - - defp init_plug(conn, config) do - conn - |> init_session_plug() - |> PowInvalidatedSessionPlug.call( - PowInvalidatedSessionPlug.init({:pow_session, ttl: @invalidated_ttl}) - ) - |> PowInvalidatedSessionPlug.call( - PowInvalidatedSessionPlug.init({:pow_persistent_session, ttl: @invalidated_ttl}) - ) - |> Session.call(Session.init(config)) - |> Cookie.call(Cookie.init([])) - |> PowInvalidatedSessionPlug.call(PowInvalidatedSessionPlug.init(:load)) - |> PhilomenaWeb.TotpPlug.call(PhilomenaWeb.TotpPlug.init([])) - end - - defp run_plug(conn, config \\ @config) do - conn - |> init_plug(config) - |> Conn.send_resp(200, "") - end - - defp create_persistent_session(conn, user, config) do - conn - |> init_plug(config) - |> Session.do_create(user, config) - |> Cookie.create(user, config) - |> Conn.send_resp(200, "") - end - - defp prepare_persistent_session_conn(conn, user, config \\ @config) do - session_conn = create_persistent_session(conn, user, config) - - :timer.sleep(100) - - no_session_conn = - conn - |> Test.recycle_cookies(session_conn) - |> delete_session_from_conn(config) - - :timer.sleep(100) - - conn - |> Test.recycle_cookies(no_session_conn) - |> Conn.fetch_cookies() - end - - defp delete_session_from_conn(conn, config) do - conn - |> init_plug(config) - |> Session.do_delete(config) - |> Conn.send_resp(200, "") - end - - defp create_session(conn, user, config) do - conn - |> init_plug(config) - |> Session.do_create(user, config) - |> Conn.send_resp(200, "") - end - - defp prepare_session_conn(conn, user, config) do - session_conn = create_session(conn, user, config) - - :timer.sleep(100) - - Test.recycle_cookies(conn, session_conn) - end -end diff --git a/test/philomena_web/user_auth_test.exs b/test/philomena_web/user_auth_test.exs new file mode 100644 index 00000000..bb04ee35 --- /dev/null +++ b/test/philomena_web/user_auth_test.exs @@ -0,0 +1,193 @@ +defmodule PhilomenaWeb.UserAuthTest do + use PhilomenaWeb.ConnCase, async: true + + alias Philomena.Users + alias PhilomenaWeb.UserAuth + import Philomena.UsersFixtures + + setup %{conn: conn} do + conn = + conn + |> Map.replace!(:secret_key_base, PhilomenaWeb.Endpoint.config(:secret_key_base)) + |> init_test_session(%{}) + + %{user: user_fixture(), conn: conn} + end + + describe "log_in_user/3" do + test "stores the user token in the session", %{conn: conn, user: user} do + conn = UserAuth.log_in_user(conn, user) + assert token = get_session(conn, :user_token) + assert get_session(conn, :live_socket_id) == "users_sessions:#{Base.url_encode64(token)}" + assert redirected_to(conn) == "/" + assert Users.get_user_by_session_token(token) + end + + test "clears everything previously stored in the session", %{conn: conn, user: user} do + conn = conn |> put_session(:to_be_removed, "value") |> UserAuth.log_in_user(user) + refute get_session(conn, :to_be_removed) + end + + test "redirects to the configured path", %{conn: conn, user: user} do + conn = conn |> put_session(:user_return_to, "/hello") |> UserAuth.log_in_user(user) + assert redirected_to(conn) == "/hello" + end + + test "writes a cookie if remember_me is configured", %{conn: conn, user: user} do + conn = conn |> fetch_cookies() |> UserAuth.log_in_user(user, %{"remember_me" => "true"}) + assert get_session(conn, :user_token) == conn.cookies["user_remember_me"] + + assert %{value: signed_token, max_age: max_age} = conn.resp_cookies["user_remember_me"] + assert signed_token != get_session(conn, :user_token) + assert max_age == 31_536_000 + end + end + + describe "totp_auth_user/3" do + test "stores the user token in the session", %{conn: conn, user: user} do + conn = UserAuth.totp_auth_user(conn, user) + assert token = get_session(conn, :totp_token) + assert redirected_to(conn) == "/" + assert Users.user_totp_token_valid?(user, token) + end + + test "redirects to the configured path", %{conn: conn, user: user} do + conn = conn |> put_session(:user_return_to, "/hello") |> UserAuth.totp_auth_user(user) + assert redirected_to(conn) == "/hello" + end + + test "writes a cookie if remember_me is configured", %{conn: conn, user: user} do + conn = conn |> fetch_cookies() |> UserAuth.totp_auth_user(user, %{"remember_me" => "true"}) + assert get_session(conn, :totp_token) == conn.cookies["user_totp_auth"] + + assert %{value: signed_token, max_age: max_age} = conn.resp_cookies["user_totp_auth"] + assert signed_token != get_session(conn, :totp_token) + assert max_age == 31_536_000 + end + end + + describe "logout_user/1" do + test "erases session and cookies", %{conn: conn, user: user} do + user_token = Users.generate_user_session_token(user) + totp_token = Users.generate_user_totp_token(user) + + conn = + conn + |> put_session(:user_token, user_token) + |> put_session(:totp_token, totp_token) + |> put_req_cookie("user_remember_me", user_token) + |> put_req_cookie("user_totp_auth", totp_token) + |> fetch_cookies() + |> UserAuth.log_out_user() + + refute get_session(conn, :user_token) + refute get_session(conn, :totp_token) + refute conn.cookies["user_remember_me"] + refute conn.cookies["user_totp_auth"] + assert %{max_age: 0} = conn.resp_cookies["user_remember_me"] + assert %{max_age: 0} = conn.resp_cookies["user_totp_auth"] + assert redirected_to(conn) == "/" + refute Users.get_user_by_session_token(user_token) + refute Users.user_totp_token_valid?(user, totp_token) + end + + test "broadcasts to the given live_socket_id", %{conn: conn} do + live_socket_id = "users_sessions:abcdef-token" + PhilomenaWeb.Endpoint.subscribe(live_socket_id) + + conn + |> put_session(:live_socket_id, live_socket_id) + |> UserAuth.log_out_user() + + assert_receive %Phoenix.Socket.Broadcast{ + event: "disconnect", + topic: "users_sessions:abcdef-token" + } + end + + test "works even if user is already logged out", %{conn: conn} do + conn = conn |> fetch_cookies() |> UserAuth.log_out_user() + refute get_session(conn, :user_token) + assert %{max_age: 0} = conn.resp_cookies["user_remember_me"] + assert redirected_to(conn) == "/" + end + end + + describe "fetch_current_user/2" do + test "authenticates user from session", %{conn: conn, user: user} do + user_token = Users.generate_user_session_token(user) + conn = conn |> put_session(:user_token, user_token) |> UserAuth.fetch_current_user([]) + assert conn.assigns.current_user.id == user.id + end + + test "authenticates user from cookies", %{conn: conn, user: user} do + logged_in_conn = + conn |> fetch_cookies() |> UserAuth.log_in_user(user, %{"remember_me" => "true"}) + + user_token = logged_in_conn.cookies["user_remember_me"] + %{value: signed_token} = logged_in_conn.resp_cookies["user_remember_me"] + + conn = + conn + |> put_req_cookie("user_remember_me", signed_token) + |> UserAuth.fetch_current_user([]) + + assert get_session(conn, :user_token) == user_token + assert conn.assigns.current_user.id == user.id + end + + test "does not authenticate if data is missing", %{conn: conn, user: user} do + _ = Users.generate_user_session_token(user) + conn = UserAuth.fetch_current_user(conn, []) + refute get_session(conn, :user_token) + refute conn.assigns.current_user + end + end + + describe "redirect_if_user_is_authenticated/2" do + test "redirects if user is authenticated", %{conn: conn, user: user} do + conn = conn |> assign(:current_user, user) |> UserAuth.redirect_if_user_is_authenticated([]) + assert conn.halted + assert redirected_to(conn) == "/" + end + + test "does not redirect if user is not authenticated", %{conn: conn} do + conn = UserAuth.redirect_if_user_is_authenticated(conn, []) + refute conn.halted + refute conn.status + end + end + + describe "require_authenticated_user/2" do + test "redirects if user is not authenticated", %{conn: conn} do + conn = conn |> fetch_flash() |> UserAuth.require_authenticated_user([]) + assert conn.halted + assert redirected_to(conn) == Routes.session_path(conn, :new) + assert get_flash(conn, :error) == "You must log in to access this page." + end + + test "stores the path to redirect to on GET", %{conn: conn} do + halted_conn = + %{conn | request_path: "/foo?bar"} + |> fetch_flash() + |> UserAuth.require_authenticated_user([]) + + assert halted_conn.halted + assert get_session(halted_conn, :user_return_to) == "/foo?bar" + + halted_conn = + %{conn | request_path: "/foo?bar", method: "POST"} + |> fetch_flash() + |> UserAuth.require_authenticated_user([]) + + assert halted_conn.halted + refute get_session(halted_conn, :user_return_to) + end + + test "does not redirect if user is authenticated", %{conn: conn, user: user} do + conn = conn |> assign(:current_user, user) |> UserAuth.require_authenticated_user([]) + refute conn.halted + refute conn.status + end + end +end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 55b3b0f4..aafac224 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -18,7 +18,9 @@ defmodule PhilomenaWeb.ConnCase do using do quote do # Import conveniences for testing with connections - use Phoenix.ConnTest + import Plug.Conn + import Phoenix.ConnTest + import PhilomenaWeb.ConnCase alias PhilomenaWeb.Router.Helpers, as: Routes # The default endpoint for testing @@ -33,6 +35,41 @@ defmodule PhilomenaWeb.ConnCase do Ecto.Adapters.SQL.Sandbox.mode(Philomena.Repo, {:shared, self()}) end - {:ok, conn: Phoenix.ConnTest.build_conn()} + # Insert default filter + %Philomena.Filters.Filter{name: "Default", system: true} + |> Philomena.Filters.change_filter() + |> Philomena.Repo.insert!() + + conn = + Phoenix.ConnTest.build_conn() + |> Phoenix.ConnTest.put_req_cookie("_ses", Integer.to_string(System.unique_integer())) + + {:ok, conn: conn} + end + + @doc """ + Setup helper that registers and logs in users. + + setup :register_and_log_in_user + + It stores an updated connection and a registered user in the + test context. + """ + def register_and_log_in_user(%{conn: conn}) do + user = Philomena.UsersFixtures.confirmed_user_fixture() + %{conn: log_in_user(conn, user), user: user} + end + + @doc """ + Logs the given `user` into the `conn`. + + It returns an updated `conn`. + """ + def log_in_user(conn, user) do + token = Philomena.Users.generate_user_session_token(user) + + conn + |> Phoenix.ConnTest.init_test_session(%{}) + |> Plug.Conn.put_session(:user_token, token) end end diff --git a/test/support/fixtures/users_fixtures.ex b/test/support/fixtures/users_fixtures.ex new file mode 100644 index 00000000..76a2ff91 --- /dev/null +++ b/test/support/fixtures/users_fixtures.ex @@ -0,0 +1,45 @@ +defmodule Philomena.UsersFixtures do + @moduledoc """ + This module defines test helpers for creating + entities via the `Philomena.Users` context. + """ + + alias Philomena.Users + alias Philomena.Repo + + def unique_user_email, do: "user#{System.unique_integer()}@example.com" + def valid_user_password, do: "hello world!" + + def user_fixture(attrs \\ %{}) do + email = unique_user_email() + + {:ok, user} = + attrs + |> Enum.into(%{ + name: email, + email: email, + password: valid_user_password() + }) + |> Users.register_user() + + user + end + + def confirmed_user_fixture(attrs \\ %{}) do + user_fixture(attrs) + |> Users.User.confirm_changeset() + |> Repo.update!() + end + + def locked_user_fixture(attrs \\ %{}) do + user_fixture(attrs) + |> Users.User.lock_changeset() + |> Repo.update!() + end + + def extract_user_token(fun) do + {:ok, captured} = fun.(&"[TOKEN]#{&1}[TOKEN]") + [_, token, _] = String.split(captured.text_body, "[TOKEN]") + token + end +end