From f1726e3d528af8de8bec5561d7e460c3547085d9 Mon Sep 17 00:00:00 2001 From: "byte[]" Date: Tue, 12 Nov 2019 22:12:46 -0500 Subject: [PATCH] add bulk of totp logic --- lib/philomena/images/query.ex | 2 +- lib/philomena/users/password.ex | 10 +- lib/philomena/users/user.ex | 101 +++++++++++++++--- .../controllers/comment_controller.ex | 3 +- .../controllers/image_controller.ex | 2 +- .../controllers/profile_controller.ex | 3 +- .../registration/totp_controller.ex | 43 ++++++++ .../controllers/session/totp_controller.ex | 31 ++++++ lib/philomena_web/plugs/totp_plug.ex | 58 ++++++++++ lib/philomena_web/router.ex | 23 +++- .../phoenix/controller_callbacks.ex | 69 ------------ lib/pow_multi_factor/plug.ex | 37 ------- lib/search/parser.ex | 5 +- mix.exs | 1 - 14 files changed, 250 insertions(+), 138 deletions(-) create mode 100644 lib/philomena_web/controllers/registration/totp_controller.ex create mode 100644 lib/philomena_web/controllers/session/totp_controller.ex create mode 100644 lib/philomena_web/plugs/totp_plug.ex delete mode 100644 lib/pow_multi_factor/phoenix/controller_callbacks.ex delete mode 100644 lib/pow_multi_factor/plug.ex diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 71749b58..0233874b 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -51,7 +51,7 @@ defmodule Philomena.Images.Query do must_not end - %{bool: %{should: should, must_not: must_not}} + {:ok, %{bool: %{should: should, must_not: must_not}}} end def user_my_transform(_ctx, _value), diff --git a/lib/philomena/users/password.ex b/lib/philomena/users/password.ex index af31ed17..7a14739c 100644 --- a/lib/philomena/users/password.ex +++ b/lib/philomena/users/password.ex @@ -1,13 +1,13 @@ defmodule Philomena.Users.Password do def hash_pwd_salt(password, opts \\ []) do - pepper = Application.get_env(:philomena, :password_pepper) - - Bcrypt.hash_pwd_salt(<>, opts) + Bcrypt.hash_pwd_salt(<>, opts) end def verify_pass(password, stored_hash) do - pepper = Application.get_env(:philomena, :password_pepper) + Bcrypt.verify_pass(<>, stored_hash) + end - Bcrypt.verify_pass(<>, stored_hash) + defp password_pepper do + Application.get_env(:philomena, :password_pepper) end end diff --git a/lib/philomena/users/user.ex b/lib/philomena/users/user.ex index 2227d0ec..8b91316d 100644 --- a/lib/philomena/users/user.ex +++ b/lib/philomena/users/user.ex @@ -7,7 +7,7 @@ defmodule Philomena.Users.User do password_hash_methods: {&Password.hash_pwd_salt/1, &Password.verify_pass/2} use Pow.Extension.Ecto.Schema, - extensions: [PowResetPassword] + extensions: [PowResetPassword, PowPersistentSession] import Ecto.Changeset @@ -17,7 +17,7 @@ defmodule Philomena.Users.User do has_many :links, Philomena.Users.Link has_many :verified_links, Philomena.Users.Link, where: [aasm_state: "verified"] has_many :public_links, Philomena.Users.Link, where: [public: true, aasm_state: "verified"] - has_many :galleries, Philomena.Galleries.Gallery + has_many :galleries, Philomena.Galleries.Gallery, foreign_key: :creator_id has_many :awards, Philomena.Badges.Award belongs_to :current_filter, Philomena.Filters.Filter @@ -116,24 +116,97 @@ defmodule Philomena.Users.User do |> validate_required([]) end - def otp_secret(%{encrypted_otp_secret: x} = user) when x not in [nil, ""] do - Philomena.Users.Encryptor.decrypt_model( - user.encrypted_otp_secret, - user.encrypted_otp_secret_iv, - user.encrypted_otp_secret_salt - ) - end - - def otp_secret(_user), do: nil - - def put_otp_secret(user_or_changeset, secret) do + def create_totp_secret_changeset(user) do + secret = :crypto.strong_rand_bytes(15) |> Base.encode32() data = Philomena.Users.Encryptor.encrypt_model(secret) - user_or_changeset + user |> change(%{ encrypted_otp_secret: data.secret, encrypted_otp_secret_iv: data.iv, encrypted_otp_secret_salt: data.salt }) end + + def consume_totp_token_changeset(user, token) do + cond do + totp_valid?(user, token) -> + user + |> change(%{consumed_timestep: token}) + + backup_code_valid?(user, token) -> + user + |> change(%{otp_backup_codes: remove_backup_code(user, token)}) + + true -> + user + |> add_error(:consumed_timestep, "invalid token") + end + end + + def totp_changeset(user, params, backup_codes) do + token = to_string(params["twofactor_token"]) + + case user.otp_required_for_login do + true -> + # User wants to disable TOTP + user + |> pow_current_password_changeset(params) + |> consume_totp_token_changeset(token) + |> disable_totp_changeset() + + false -> + # User wants to enable TOTP + user + |> pow_current_password_changeset(params) + |> consume_totp_token_changeset(token) + |> enable_totp_changeset(backup_codes) + end + end + + def random_backup_codes do + (1..10) + |> Enum.map(fn _i -> + :crypto.strong_rand_bytes(6) |> Base.encode16(case: :lower) + end) + end + + + defp enable_totp_changeset(user, backup_codes) do + hashed_codes = + backup_codes + |> Enum.map(&Password.hash_pwd_salt/1) + + user + |> change(%{ + otp_required_for_login: true, + otp_backup_codes: hashed_codes + }) + end + + defp disable_totp_changeset(user) do + user + |> change(%{ + otp_required_for_login: false, + otp_backup_codes: [] + }) + end + + defp totp_valid?(user, token), + do: :pot.valid_totp(token, otp_secret(user), window: 60) + + defp backup_code_valid?(user, token), + do: Enum.any?(user.otp_backup_codes, &Password.verify_pass(token, &1)) + + defp remove_backup_code(user, token), + do: user.otp_backup_codes |> Enum.reject(&Password.verify_pass(token, &1)) + + + defp otp_secret(user) do + Philomena.Users.Encryptor.decrypt_model( + user.encrypted_otp_secret, + user.encrypted_otp_secret_iv, + user.encrypted_otp_secret_salt + ) + end end diff --git a/lib/philomena_web/controllers/comment_controller.ex b/lib/philomena_web/controllers/comment_controller.ex index 147ee825..5a7a666e 100644 --- a/lib/philomena_web/controllers/comment_controller.ex +++ b/lib/philomena_web/controllers/comment_controller.ex @@ -1,8 +1,7 @@ defmodule PhilomenaWeb.CommentController do use PhilomenaWeb, :controller - alias Philomena.{Images.Image, Comments.Comment, Textile.Renderer} - alias Philomena.Repo + alias Philomena.{Comments.Comment, Textile.Renderer} import Ecto.Query def index(conn, _params) do diff --git a/lib/philomena_web/controllers/image_controller.ex b/lib/philomena_web/controllers/image_controller.ex index 86f95a01..c63fd859 100644 --- a/lib/philomena_web/controllers/image_controller.ex +++ b/lib/philomena_web/controllers/image_controller.ex @@ -1,7 +1,7 @@ defmodule PhilomenaWeb.ImageController do use PhilomenaWeb, :controller - alias Philomena.{Images.Image, Comments.Comment, Tags.Tag, Textile.Renderer} + alias Philomena.{Images.Image, Comments.Comment, Textile.Renderer} alias Philomena.Repo import Ecto.Query diff --git a/lib/philomena_web/controllers/profile_controller.ex b/lib/philomena_web/controllers/profile_controller.ex index 180227dc..e663e629 100644 --- a/lib/philomena_web/controllers/profile_controller.ex +++ b/lib/philomena_web/controllers/profile_controller.ex @@ -1,8 +1,7 @@ defmodule PhilomenaWeb.ProfileController do use PhilomenaWeb, :controller - alias Philomena.{Images, Images.Image, Comments.Comment, Posts.Post, Users.User, Users.Link} - alias Philomena.Repo + alias Philomena.{Images, Images.Image, Users.User} import Ecto.Query plug :load_and_authorize_resource, model: User, only: :show, id_field: "slug", preload: [awards: :badge, public_links: :tag] diff --git a/lib/philomena_web/controllers/registration/totp_controller.ex b/lib/philomena_web/controllers/registration/totp_controller.ex new file mode 100644 index 00000000..34d5035a --- /dev/null +++ b/lib/philomena_web/controllers/registration/totp_controller.ex @@ -0,0 +1,43 @@ +defmodule PhilomenaWeb.Registration.TotpController do + use PhilomenaWeb, :controller + + alias Philomena.Users.User + alias Philomena.Repo + + def edit(conn, _params) do + user = conn.assigns.current_user + + case user.encrypted_otp_secret do + nil -> + user + |> User.create_totp_secret_changeset() + |> Repo.update() + + # Redirect to have Pow pick up the changes + redirect(conn, to: Routes.registration_totp_path(conn, :edit)) + + _ -> + changeset = Pow.Plug.change_user(conn) + render(conn, "edit.html", changeset: changeset) + end + end + + def update(conn, params) do + backup_codes = User.random_backup_codes() + + conn + |> Pow.Plug.current_user() + |> User.totp_changeset(params, backup_codes) + |> Repo.update() + |> case do + {:error, changeset} -> + render(conn, "edit.html", changeset: changeset) + + {:ok, user} -> + conn + |> PhilomenaWeb.Plugs.TotpPlug.update_valid_totp_at_for_session(user) + |> put_flash(:totp_backup_codes, backup_codes) + |> redirect(to: Routes.registration_totp_path(conn, :edit)) + end + end +end \ No newline at end of file diff --git a/lib/philomena_web/controllers/session/totp_controller.ex b/lib/philomena_web/controllers/session/totp_controller.ex new file mode 100644 index 00000000..ce304146 --- /dev/null +++ b/lib/philomena_web/controllers/session/totp_controller.ex @@ -0,0 +1,31 @@ +defmodule PhilomenaWeb.Session.TotpController do + use PhilomenaWeb, :controller + + alias Philomena.Users.User + alias Philomena.Repo + + def new(conn, _params) do + changeset = Pow.Plug.change_user(conn) + + render(conn, "new.html", changeset: changeset) + end + + def create(conn, params) do + conn + |> Pow.Plug.current_user() + |> User.consume_totp_token_changeset(params) + |> Repo.update() + |> case do + {:error, _changeset} -> + conn + |> Pow.Plug.clear_authenticated_user() + |> put_flash(:error, "Sorry, invalid TOTP token entered. Please sign in again.") + |> redirect(to: Routes.pow_session_path(conn, :new)) + + {:ok, user} -> + conn + |> PhilomenaWeb.Plugs.TotpPlug.update_valid_totp_at_for_session(user) + |> redirect(to: "/") + end + end +end \ No newline at end of file diff --git a/lib/philomena_web/plugs/totp_plug.ex b/lib/philomena_web/plugs/totp_plug.ex new file mode 100644 index 00000000..ae3a8fe8 --- /dev/null +++ b/lib/philomena_web/plugs/totp_plug.ex @@ -0,0 +1,58 @@ +defmodule PhilomenaWeb.Plugs.TotpPlug do + @moduledoc """ + This plug ensures that a user session has a valid TOTP. + + ## Example + + plug PhilomenaWeb.TotpPlug + """ + + alias PhilomenaWeb.Router.Helpers, as: Routes + + @doc false + @spec init(any()) :: any() + def init(opts), do: opts + + @doc false + @spec call(Plug.Conn.t(), any()) :: Plug.Conn.t() + def call(conn, _opts) do + conn + |> Pow.Plug.current_user() + |> case do + nil -> conn + user -> maybe_require_totp_phase(user, conn) + end + end + + defp maybe_require_totp_phase(%{otp_required_for_login: nil}, conn), do: conn + 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 -> + 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) + + plug.do_create(conn, user, config) + end +end \ No newline at end of file diff --git a/lib/philomena_web/router.ex b/lib/philomena_web/router.ex index e5ff64ac..048a765b 100644 --- a/lib/philomena_web/router.ex +++ b/lib/philomena_web/router.ex @@ -1,6 +1,7 @@ defmodule PhilomenaWeb.Router do use PhilomenaWeb, :router use Pow.Phoenix.Router + use Pow.Extension.Phoenix.Router, otp_app: :philomena pipeline :browser do plug :accepts, ["html"] @@ -16,14 +17,28 @@ defmodule PhilomenaWeb.Router do plug :accepts, ["json"] end + pipeline :ensure_totp do + plug PhilomenaWeb.Plugs.TotpPlug + end + scope "/" do - pipe_through :browser + pipe_through [:browser, :ensure_totp] - #pow_routes() + pow_routes() + pow_extension_routes() end scope "/", PhilomenaWeb do - pipe_through :browser + pipe_through [:browser, :ensure_totp] + + # Additional routes for TOTP + scope "/registration", Registration, as: :registration do + resources "/totp", TotpController, only: [:edit, :update], singleton: true + end + + scope "/session", Session, as: :session do + resources "/totp", TotpController, only: [:new, :create], singleton: true + end get "/", ActivityController, :index @@ -39,7 +54,7 @@ defmodule PhilomenaWeb.Router do resources "/comments", CommentController, only: [:index] scope "/filters", Filter, as: :filter do - resources "/current", CurrentController, only: [:update], singular: true + resources "/current", CurrentController, only: [:update], singleton: true end resources "/filters", FilterController resources "/profiles", ProfileController, only: [:show] diff --git a/lib/pow_multi_factor/phoenix/controller_callbacks.ex b/lib/pow_multi_factor/phoenix/controller_callbacks.ex deleted file mode 100644 index 1307d17d..00000000 --- a/lib/pow_multi_factor/phoenix/controller_callbacks.ex +++ /dev/null @@ -1,69 +0,0 @@ -defmodule PowMultiFactor.Phoenix.ControllerCallbacks do - @moduledoc """ - Controller callback logic for multi-factor authentication. - - ### 2FA code not submitted - - Triggers on `Pow.Phoenix.SessionController.create/2`. - - When a user with 2FA enabled attempts to sign in without submitting their - TOTP token, the session will be cleared, and the user redirected back to - `Pow.Phoenix.Routes.session_path/1`. - - ### User updates account - - Triggers on `Pow.Phoenix.RegistrationController.update/2` - - When a user changes their account settings, they are required to confirm a - current 2FA token. - - See `PowMultiFactor.Ecto.Schema` for more. - """ - - use Pow.Extension.Phoenix.ControllerCallbacks.Base - - alias Pow.Plug - alias PowMultiFactor.Plug, as: PowMultiFactorPlug - - def before_respond(Pow.Phoenix.SessionController, :create, {:ok, conn}, config) do - return_path = routes(conn).session_path(conn, :new) - - clear_unauthorized(conn, config, {:ok, conn}, return_path) - end - - def before_respond(Pow.Phoenix.RegistrationController, :update, {:ok, user, conn}, config) do - return_path = routes(conn).registration_path(conn, :edit) - - halt_unauthorized(conn, config, {:ok, user, conn}, return_path) - end - - defp clear_unauthorized(conn, config, success_response, return_path) do - case PowMultiFactorPlug.mfa_authorized?(conn, config) do - false -> clear_auth(conn) |> go_back(return_path) - true -> success_response - end - end - - defp halt_unauthorized(conn, config, success_response, return_path) do - case PowMultiFactorPlug.mfa_authorized?(conn, config) do - false -> go_back(conn, return_path) - true -> success_response - end - end - - def clear_auth(conn) do - {:ok, conn} = Plug.clear_authenticated_user(conn) - - conn - end - - defp go_back(conn, return_path) do - error = extension_messages(conn).invalid_multi_factor(conn) - conn = - conn - |> Phoenix.Controller.put_flash(:error, error) - |> Phoenix.Controller.redirect(to: return_path) - - {:halt, conn} - end -end diff --git a/lib/pow_multi_factor/plug.ex b/lib/pow_multi_factor/plug.ex deleted file mode 100644 index 33526153..00000000 --- a/lib/pow_multi_factor/plug.ex +++ /dev/null @@ -1,37 +0,0 @@ -defmodule PowMultiFactor.Plug do - @moduledoc """ - Plug helper methods. - """ - - alias Plug.Crypto - alias Pow.Plug - alias Pow.Config - - def mfa_authorized?(conn, config) do - user = Plug.current_user(conn) - - if user.otp_required_for_login do - secret = user.__struct__.otp_secret(user) - totp = Elixir2fa.generate_totp(secret) - - Crypto.secure_compare(totp, conn.params) - else - true - end - end - - def assign_mfa(conn, config) do - user = Plug.current_user(conn) - repo = Config.repo!(config) - - if user.encrypted_otp_secret in [nil, ""] do - {:ok, user} = - user.__struct__.put_otp_secret(Elixir2fa.random_secret()) - |> repo.update() - - user - else - user - end - end -end diff --git a/lib/search/parser.ex b/lib/search/parser.ex index e80466a1..5997a429 100644 --- a/lib/search/parser.ex +++ b/lib/search/parser.ex @@ -66,8 +66,9 @@ defmodule Search.Parser do {:error, msg} -> {:error, msg} - _ -> - {:error, "unknown parsing error"} + err -> + err + #{:error, "unknown parsing error"} end end diff --git a/mix.exs b/mix.exs index 090c0c67..5ce58eda 100644 --- a/mix.exs +++ b/mix.exs @@ -53,7 +53,6 @@ defmodule Philomena.MixProject do {:nimble_parsec, "~> 0.5.1"}, {:canary, "~> 1.1.1"}, {:scrivener_ecto, "~> 2.0"}, - {:elixir2fa, "~> 0.1.0"}, {:pbkdf2, "~> 2.0"} ] end