From a9d8786d11d099a444ff7eb4a46f649c0fd242db Mon Sep 17 00:00:00 2001 From: Nikos Papadakis Date: Sat, 29 Jul 2023 14:45:09 +0300 Subject: [PATCH] Add typespecs, remove unneeded documentation --- app/lib/prymn/accounts.ex | 31 ++++++++++++++++--- app/lib/prymn/accounts/user_notifier.ex | 8 +++-- .../controllers/user_session_controller.ex | 7 +++-- app/lib/prymn_web/user_auth.ex | 22 ++----------- 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/app/lib/prymn/accounts.ex b/app/lib/prymn/accounts.ex index edbc37c..c2b9da8 100644 --- a/app/lib/prymn/accounts.ex +++ b/app/lib/prymn/accounts.ex @@ -63,20 +63,21 @@ defmodule Prymn.Accounts do @spec get_user!(integer()) :: User.t() def get_user!(id), do: Repo.get!(User, id) - ## User registration + ## User registration, used by the registration flow @doc """ Registers a user. ## Examples - iex> register_user(%{field: value}) + iex> register_user(%{email: "email@example.com", password: "examplepassword"}) {:ok, %User{}} - iex> register_user(%{field: bad_value}) + iex> register_user(%{field: "value"}) {:error, %Ecto.Changeset{}} """ + @spec register_user(map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def register_user(attrs) do %User{} |> User.registration_changeset(attrs) @@ -92,11 +93,12 @@ defmodule Prymn.Accounts do %Ecto.Changeset{data: %User{}} """ + @spec change_user_registration(User.t(), map()) :: Ecto.Changeset.t() def change_user_registration(%User{} = user, attrs \\ %{}) do User.registration_changeset(user, attrs, hash_password: false, validate_email: false) end - ## Settings + ## Settings, used by the account settings flows @doc """ Returns an `%Ecto.Changeset{}` for changing the user email. @@ -107,6 +109,7 @@ defmodule Prymn.Accounts do %Ecto.Changeset{data: %User{}} """ + @spec change_user_email(User.t(), map()) :: Ecto.Changeset.t() def change_user_email(user, attrs \\ %{}) do User.email_changeset(user, attrs, validate_email: false) end @@ -124,6 +127,8 @@ defmodule Prymn.Accounts do {:error, %Ecto.Changeset{}} """ + @spec apply_user_email(User.t(), String.t(), map()) :: + {:ok, User.t()} | {:error, Ecto.Changeset.t()} def apply_user_email(user, password, attrs) do user |> User.email_changeset(attrs) @@ -135,8 +140,9 @@ defmodule Prymn.Accounts do Updates the user email using the given 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. + The `confirmed_at` date is also updated to the current time. """ + @spec update_user_email(User.t(), binary()) :: :ok | :error def update_user_email(user, token) do context = "change:#{user.email}" @@ -169,6 +175,8 @@ defmodule Prymn.Accounts do {:ok, %{to: ..., body: ...}} """ + @spec deliver_user_update_email_instructions(User.t(), String.t(), (... -> String.t())) :: + {:ok, Swoosh.Email.t()} def deliver_user_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}") @@ -186,6 +194,7 @@ defmodule Prymn.Accounts do %Ecto.Changeset{data: %User{}} """ + @spec change_user_password(User.t(), map()) :: Ecto.Changeset.t() def change_user_password(user, attrs \\ %{}) do User.password_changeset(user, attrs, hash_password: false) end @@ -202,6 +211,8 @@ defmodule Prymn.Accounts do {:error, %Ecto.Changeset{}} """ + @spec update_user_password(User.t(), String.t(), map()) :: + {:ok, User.t()} | {:error, Ecto.Changeset.t()} def update_user_password(user, password, attrs) do changeset = user @@ -223,6 +234,7 @@ defmodule Prymn.Accounts do @doc """ Generates a session token. """ + @spec generate_user_session_token(User.t()) :: binary() def generate_user_session_token(user) do {token, user_token} = UserToken.build_session_token(user) Repo.insert!(user_token) @@ -232,6 +244,7 @@ defmodule Prymn.Accounts do @doc """ Gets the user with the given signed token. """ + @spec get_user_by_session_token(binary()) :: User.t() def get_user_by_session_token(token) do {:ok, query} = UserToken.verify_session_token_query(token) Repo.one(query) @@ -240,6 +253,7 @@ defmodule Prymn.Accounts do @doc """ Deletes the signed token with the given context. """ + @spec delete_user_session_token(binary()) :: :ok def delete_user_session_token(token) do Repo.delete_all(UserToken.token_and_context_query(token, "session")) :ok @@ -259,6 +273,8 @@ defmodule Prymn.Accounts do {:error, :already_confirmed} """ + @spec deliver_user_confirmation_instructions(User.t(), (... -> String.t())) :: + {:ok, Swoosh.Email.t()} | {: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 @@ -276,6 +292,7 @@ defmodule Prymn.Accounts do If the token matches, the user account is marked as confirmed and the token is deleted. """ + @spec confirm_user(binary()) :: {:ok, User.t()} | :error def confirm_user(token) do with {:ok, query} <- UserToken.verify_email_token_query(token, "confirm"), %User{} = user <- Repo.one(query), @@ -303,6 +320,8 @@ defmodule Prymn.Accounts do {:ok, %{to: ..., body: ...}} """ + @spec deliver_user_reset_password_instructions(User.t(), (... -> String.t())) :: + {:ok, Swoosh.Email.t()} 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") @@ -322,6 +341,7 @@ defmodule Prymn.Accounts do nil """ + @spec get_user_by_reset_password_token(binary()) :: User.t() | 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 @@ -343,6 +363,7 @@ defmodule Prymn.Accounts do {:error, %Ecto.Changeset{}} """ + @spec reset_user_password(User.t(), map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def reset_user_password(user, attrs) do Ecto.Multi.new() |> Ecto.Multi.update(:user, User.password_changeset(user, attrs)) diff --git a/app/lib/prymn/accounts/user_notifier.ex b/app/lib/prymn/accounts/user_notifier.ex index 4651b0f..7753077 100644 --- a/app/lib/prymn/accounts/user_notifier.ex +++ b/app/lib/prymn/accounts/user_notifier.ex @@ -12,14 +12,14 @@ defmodule Prymn.Accounts.UserNotifier do |> subject(subject) |> text_body(body) - with {:ok, _metadata} <- Mailer.deliver(email) do - {:ok, email} - end + {:ok, _metadata} = Mailer.deliver(email) + {:ok, email} end @doc """ Deliver instructions to confirm account. """ + @spec deliver_confirmation_instructions(User.t(), String.t()) :: {:ok, Swoosh.Email.t()} def deliver_confirmation_instructions(user, url) do deliver(user.email, "Confirmation instructions", """ @@ -40,6 +40,7 @@ defmodule Prymn.Accounts.UserNotifier do @doc """ Deliver instructions to reset a user password. """ + @spec deliver_reset_password_instructions(User.t(), String.t()) :: {:ok, Swoosh.Email.t()} def deliver_reset_password_instructions(user, url) do deliver(user.email, "Reset password instructions", """ @@ -60,6 +61,7 @@ defmodule Prymn.Accounts.UserNotifier do @doc """ Deliver instructions to update a user email. """ + @spec deliver_update_email_instructions(User.t(), String.t()) :: {:ok, Swoosh.Email.t()} def deliver_update_email_instructions(user, url) do deliver(user.email, "Update email instructions", """ diff --git a/app/lib/prymn_web/controllers/user_session_controller.ex b/app/lib/prymn_web/controllers/user_session_controller.ex index 5e26c13..376d989 100644 --- a/app/lib/prymn_web/controllers/user_session_controller.ex +++ b/app/lib/prymn_web/controllers/user_session_controller.ex @@ -1,9 +1,12 @@ defmodule PrymnWeb.UserSessionController do - use PrymnWeb, :controller - alias Prymn.Accounts alias PrymnWeb.UserAuth + use PrymnWeb, :controller + + @spec create(Plug.Conn.t(), Phoenix.Param.t()) :: Plug.Conn.t() + @spec delete(Plug.Conn.t(), Phoenix.Param.t()) :: Plug.Conn.t() + def create(conn, %{"_action" => "registered"} = params) do create(conn, params, "Account created successfully!") end diff --git a/app/lib/prymn_web/user_auth.ex b/app/lib/prymn_web/user_auth.ex index a5075ea..154fe53 100644 --- a/app/lib/prymn_web/user_auth.ex +++ b/app/lib/prymn_web/user_auth.ex @@ -1,11 +1,11 @@ defmodule PrymnWeb.UserAuth do - use PrymnWeb, :verified_routes - import Plug.Conn import Phoenix.Controller alias Prymn.Accounts + use PrymnWeb, :verified_routes + # Make the remember me cookie valid for 60 days. # If you want bump or reduce this value, also change # the token expiry itself in UserToken. @@ -44,21 +44,6 @@ defmodule PrymnWeb.UserAuth 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) @@ -190,9 +175,6 @@ defmodule PrymnWeb.UserAuth do @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