diff --git a/README.md b/README.md index 244cbdcc..980d3271 100644 --- a/README.md +++ b/README.md @@ -688,20 +688,20 @@ IpAddress authentication is a good solution for server to server rest APIs. ```elixir creds = Coherence.Authentication.Basic.encode_credentials("Admin", "SecretPass") -Coherence.CredentialStore.Agent.put_credentials(creds, %{role: :admin}) +Coherence.CredentialStore.Server.put_credentials(creds, %{role: :admin}) ``` ### Add Token Credentials Example ```elixir token = Coherence.Authentication.Token.generate_token -Coherence.CredentialStore.Agent.put_credentials(token, %{role: :admin}) +Coherence.CredentialStore.Server.put_credentials(token, %{role: :admin}) ``` ### Add IP Credentials Example ```elixir -Coherence.CredentialStore.Agent.put_credentials({127.0.0.1}, %{role: :admin}) +Coherence.CredentialStore.Server.put_credentials({127.0.0.1}, %{role: :admin}) ``` IpAddress authentication does not require this step. Its optional. If the user_data diff --git a/lib/coherence.ex b/lib/coherence.ex index 7a89b1cc..a7111eb7 100644 --- a/lib/coherence.ex +++ b/lib/coherence.ex @@ -232,6 +232,31 @@ Run `$ mix help coherence.install` or `$ mix help coherence.install` for more in """ def current_user(conn), do: conn.assigns[Config.assigns_key] + @doc """ + Updates the user login data in the current sessions. + + Other sessions belonging to the same user won't be updated. + Requires access to the `conn`, which means it can't be called outside of the context of a conn. + To update all session belonging to the user see `t:update_user_login/1`. + """ + def update_user_login(conn, user) do + apply(Config.auth_module, + Config.update_login, + [conn, user, [id_key: Config.schema_key]]) + end + + @doc """ + Updates the user login data in the all sessions belonging to the user. + + All sessions belonging to the same user will be updated. + Doesn't need access to the `conn`, which means it can be called anywhere. + To update only the current session see `t:update_user_login/2` + """ + def update_user_logins(user) do + # Handle a user's DBStore + Coherence.CredentialStore.Server.update_user_logins(user) + end + @doc """ Get the currently logged in user name. """ diff --git a/lib/coherence/plugs/authorization/basic.ex b/lib/coherence/plugs/authorization/basic.ex index 8747c100..e155952b 100644 --- a/lib/coherence/plugs/authorization/basic.ex +++ b/lib/coherence/plugs/authorization/basic.ex @@ -25,11 +25,12 @@ defmodule Coherence.Authentication.Basic do import Coherence.Authentication.Utils alias Coherence.Messages + alias Coherence.CredentialStore.Types, as: T @doc """ Returns the encoded form for the given `user` and `password` combination. """ - @spec encode_credentials(atom | String.t, String.t | nil) :: String.t + @spec encode_credentials(atom | String.t, String.t | nil) :: T.credentials def encode_credentials(user, password), do: Base.encode64("#{user}:#{password}") @spec create_login(String.t, String.t, t, Keyword.t) :: t @@ -52,7 +53,7 @@ defmodule Coherence.Authentication.Basic do %{ realm: Keyword.get(opts, :realm, Messages.backend().restricted_area()), error: Keyword.get(opts, :error, Messages.backend().http_authentication_required()), - store: Keyword.get(opts, :store, Coherence.CredentialStore.Agent), + store: Keyword.get(opts, :store, Coherence.CredentialStore.Server), assigns_key: Keyword.get(opts, :assigns_key, :current_user), } end diff --git a/lib/coherence/plugs/authorization/credential_store/agent.ex b/lib/coherence/plugs/authorization/credential_store/agent.ex deleted file mode 100644 index c0902782..00000000 --- a/lib/coherence/plugs/authorization/credential_store/agent.ex +++ /dev/null @@ -1,44 +0,0 @@ -defmodule Coherence.CredentialStore.Agent do - @moduledoc """ - An Agent to save credential information. - """ - - @behaviour Coherence.CredentialStore - - @type t :: Ecto.Schema.t | Map.t - - @doc """ - Starts a new credentials store. - """ - @spec start_link() :: {:ok, pid} - def start_link do - Agent.start_link(fn -> %{} end, name: __MODULE__) - end - - @doc """ - Gets the user data for the given credentials - """ - @spec get_user_data(Map.t) :: String.t | nil - def get_user_data(credentials) do - Agent.get(__MODULE__, &Map.get(&1, credentials)) - end - - @doc """ - Puts the `user_data` for the given `credentials`. - """ - @spec put_credentials(Map.t, t) :: t - def put_credentials(credentials, user_data) do - Agent.update(__MODULE__, &Map.put(&1, credentials, user_data)) - end - - @doc """ - Deletes `credentials` from the store. - - Returns the current value of `credentials`, if `credentials` exists. - """ - @spec delete_credentials(Map.t) :: t - def delete_credentials(credentials) do - Agent.get_and_update(__MODULE__, &Map.pop(&1, credentials)) - end - -end diff --git a/lib/coherence/plugs/authorization/credential_store/server.ex b/lib/coherence/plugs/authorization/credential_store/server.ex new file mode 100644 index 00000000..55f3d7b5 --- /dev/null +++ b/lib/coherence/plugs/authorization/credential_store/server.ex @@ -0,0 +1,149 @@ +defmodule Coherence.CredentialStore.Server do + # ExActor will help us simplify the definition of our GenServer + use ExActor.GenServer, export: Coherence.CredentialStore.Server + + alias Coherence.CredentialStore.Types, as: T + + @behaviour Coherence.CredentialStore + + # Server State + # ------------ + # The state of the server is a record containing a store and an index. + require Record + # The state is not part of the server's API, so its type and constructor will be private. + # We want to have the flexibility of changing this later for efficiency reasons. + Record.defrecordp :state, [store: %{}, index: %{}] + # The store is a map mapping credentials to user_data + @typep store :: %{T.credentials => T.user_data} + # The index is a map mapping the user's id to that user's active sessions. + @typep index :: %{T.user_id => MapSet.t(T.credentials)} + # The index makes it much easier to find out which session credentials belong to a certain user. + # Without this index, to get or update all sessions belonging to a user, one has to iterate over all the items in the map + # This is linear on the number of active sessions (>= the number of users because there can be >= 1 session per user) + # With the index, accessing all the sessions belonging to a user costs a map lookup + # and iterating over all sessions it is linear on the number of sessions belonging to that user. + # This is a clear improvement if there are many concurrent users. + # + # The costs of this index are increased memory consumption (we need to keep what's basically a duplicated state) and slower update operations. + # For applications with many concurrent sessions, the benefits probably outweight the costs. + # For applications without many concurrent sessions these costs aren't great anyway. + + # Public API for our server + # ------------------------- + + defstart start_link, do: + # initially, both the store and the index are empty + initial_state(state(store: %{}, index: %{})) + + @spec update_user_logins(T.user_data) :: [T.credentials] + defcall update_user_logins(%{id: user_id} = user_data), state: state(store: store, index: index) do + # TODO: + # Maybe support updating ths user's ID. + # Currently it's not obvious what's the best API for this. + # ----------------------------------------- + # Get credentials for all sessions belonging to user + # This operations is read-only for the index. + sessions_credentials = Map.get(index, user_id, []) + # Build the changes to apply to the store. + # This is linear on the number of sessions belonging to the user. + # It is much better than the naive approach without the index, + # which is linear on the total number of active sessions. + delta = for credentials <- sessions_credentials, into: %{}, do: {credentials, user_data} + set_and_reply( + # Update the store with the new user_data model + # The index data is not touched, so the index is returned unchanged. + state(store: Map.merge(store, delta), index: index), + # Return the updated credentials for all sessions belonging to the user + Map.keys(delta) + ) + end + defcall update_user_logins(_), do: + # If the user_data doesn't contain an ID, there are no sessions belonging to the user + # There is no need to update anything and we just return an empty list + reply([]) + + @spec update_user_logins(T.credentials) :: T.user_data + defcall get_user_data(credentials), state: state(store: store) do + Map.get(store, credentials) |> reply + end + + @spec put_credentials(T.credentials, T.user_data) :: T.user_data + defcall put_credentials(credentials, user_data), state: state(store: store, index: index) do + # The data has been changed; We must update both the index and the store + # Update the index only if it makes sense + possibly_updated_index = maybe_add_credentials_to_index(index, user_data, credentials) + # Always update the store + updated_store = add_credentials_to_store(store, user_data, credentials) + # Update the state and reply + set_and_reply( + state(store: updated_store, index: possibly_updated_index), + # Return the updated store + user_data + ) + end + + @spec delete_credentials(T.credentials) :: store + defcall delete_credentials(credentials), state: state(store: store, index: index) do + user_data = Map.get(store, credentials) + # The data has been changed; We must update both the index and the store + # Update the index only if it makes sense + possibly_updated_index = maybe_delete_credentials_from_index(index, user_data, credentials) + # Always update the store + updated_store = delete_credentials_from_store(store, credentials) + set_and_reply( + state(store: updated_store, index: possibly_updated_index), + updated_store + ) + end + + defcast stop, do: stop_server(:normal) + + # Helper functions: + # ----------------- + @spec maybe_add_credentials_to_index(index, T.user_data, T.credentials) :: index + defp maybe_add_credentials_to_index(index, %{id: user_id}, credentials), do: + # If there isn't an entry for user_id in the index, create it. + # If there is already an entry for user_id, append the new credentials. + Map.update(index, + user_id, + # If the user has no credentials, create a new entry in the MapSet + # with the new credentials + MapSet.put(MapSet.new(), credentials), + # If the user already has some credentials, put the new credential + &(MapSet.put(&1, credentials))) + defp maybe_add_credentials_to_index(index, nil, _), do: index + defp maybe_add_credentials_to_index(index, _, _), do: index + + @spec maybe_delete_credentials_from_index(index, T.user_data, T.credentials) :: index + defp maybe_delete_credentials_from_index(index, %{id: user_id}, credentials) do + # We must handle 3 cases: + case index do + %{^user_id => sessions_credentials} -> + case MapSet.size(sessions_credentials) do + # 1. The user has a single session + # ----------------------------------- + # If there is only a single session belonging to that user, + # the user is no longer active after deleting the session. + # We can delete it from the index. + 1 -> Map.delete(index, user_id) + # 2. The user has more than one session + # ------------------------------------- + # We must delete the credentials from the set of session credentials + _ -> %{index | user_id => MapSet.delete(sessions_credentials, credentials)} + end + # 3. The user has no sessions (return the index unchanged) + _ -> index + end + end + defp maybe_delete_credentials_from_index(index, nil, _), do: index + defp maybe_delete_credentials_from_index(index, _, _), do: index + + + @spec add_credentials_to_store(store, T.user_data, T.credentials) :: store + defp add_credentials_to_store(store, user_data, credentials), do: + Map.put(store, credentials, user_data) + + @spec delete_credentials_from_store(store, T.credentials) :: store + defp delete_credentials_from_store(store, credentials), do: + Map.delete(store, credentials) +end \ No newline at end of file diff --git a/lib/coherence/plugs/authorization/credential_store/session.ex b/lib/coherence/plugs/authorization/credential_store/session.ex index 7f62f3f8..0b4b6279 100644 --- a/lib/coherence/plugs/authorization/credential_store/session.ex +++ b/lib/coherence/plugs/authorization/credential_store/session.ex @@ -2,7 +2,7 @@ defmodule Coherence.CredentialStore.Session do @moduledoc """ Stores current credential information. - Uses an Agent to save logged in credentials. + Uses an Server to save logged in credentials. Note: If you restart the phoenix server, this information is lost, requiring the user to log in again. @@ -21,17 +21,19 @@ defmodule Coherence.CredentialStore.Session do require Logger alias Coherence.DbStore - alias Coherence.CredentialStore.Agent + alias Coherence.CredentialStore.Server + alias Coherence.CredentialStore.Types, as: T @spec start_link() :: {:ok, pid} | {:error, atom} def start_link do - Agent.start_link + Server.start_link() end @doc """ Gets the user data for the given credentials """ - @spec get_user_data({HashDict.t, nil | struct, integer | nil}) :: any + + @spec get_user_data({T.credentials, nil | struct, integer | nil}) :: any def get_user_data({credentials, nil, _}) do get_data credentials end @@ -41,7 +43,7 @@ defmodule Coherence.CredentialStore.Session do case DbStore.get_user_data(db_model.__struct__, credentials, id_key) do nil -> nil user_data -> - Agent.put_credentials(credentials, user_data) + Server.put_credentials(credentials, user_data) user_data end other -> @@ -49,14 +51,15 @@ defmodule Coherence.CredentialStore.Session do end end - defp get_data(credentials), do: Agent.get_user_data(credentials) + @spec get_data(T.credentials) :: any + defp get_data(credentials), do: Server.get_user_data(credentials) @doc """ Puts the `user_data` for the given `credentials`. """ - @spec put_credentials({HashDict.t, any, atom}) :: any + @spec put_credentials({T.credentials, any, atom}) :: any def put_credentials({credentials, user_data, id_key}) do - Agent.put_credentials(credentials, user_data) + Server.put_credentials(credentials, user_data) DbStore.put_credentials(user_data, credentials, id_key) end @@ -65,13 +68,13 @@ defmodule Coherence.CredentialStore.Session do Returns the current value of `credentials`, if `credentials` exists. """ - @spec delete_credentials(HashDict.t) :: any + @spec delete_credentials(T.credentials) :: any def delete_credentials(credentials) do case get_data credentials do nil -> nil user_data -> DbStore.delete_credentials user_data, credentials - Agent.delete_credentials(credentials) + Server.delete_credentials(credentials) end end end diff --git a/lib/coherence/plugs/authorization/credential_store/types.ex b/lib/coherence/plugs/authorization/credential_store/types.ex new file mode 100644 index 00000000..f4d810ae --- /dev/null +++ b/lib/coherence/plugs/authorization/credential_store/types.ex @@ -0,0 +1,5 @@ +defmodule Coherence.CredentialStore.Types do + @type credentials :: String.t + @type user_data :: Ecto.Schema.t | Map.t + @type user_id :: any +end \ No newline at end of file diff --git a/lib/coherence/plugs/authorization/ip_address.ex b/lib/coherence/plugs/authorization/ip_address.ex index 80f8ceca..aa293245 100644 --- a/lib/coherence/plugs/authorization/ip_address.ex +++ b/lib/coherence/plugs/authorization/ip_address.ex @@ -11,7 +11,7 @@ defmodule Coherence.Authentication.IpAddress do If you would like access to the current user you must set each authorized IP address like: - Coherence.CredentialStore.Agent.put_credentials({127.0.0.1}, %{role: :admin}) + Coherence.CredentialStore.Server.put_credentials({127.0.0.1}, %{role: :admin}) or use a custom store like: @@ -65,7 +65,7 @@ defmodule Coherence.Authentication.IpAddress do Add the credentials for a `token`. `user_data` can be any term but must not be `nil`. """ @spec add_credentials(String.t, t, module) :: t - def add_credentials(ip, user_data, store \\ Coherence.CredentialStore.Agent) do + def add_credentials(ip, user_data, store \\ Coherence.CredentialStore.Server) do store.put_credentials(ip, user_data) end @@ -73,7 +73,7 @@ defmodule Coherence.Authentication.IpAddress do Remove the credentials for a `token`. """ @spec remove_credentials(String.t, module) :: t - def remove_credentials(ip, store \\ Coherence.CredentialStore.Agent) do + def remove_credentials(ip, store \\ Coherence.CredentialStore.Server) do store.delete_credentials(ip) end @@ -83,7 +83,7 @@ defmodule Coherence.Authentication.IpAddress do allow: Keyword.get(opts, :allow, []), deny: Keyword.get(opts, :deny, []), error: Keyword.get(opts, :error, Messages.backend().unauthorized_ip_address()), - store: Keyword.get(opts, :store, Coherence.CredentialStore.Agent), + store: Keyword.get(opts, :store, Coherence.CredentialStore.Server), assign_key: Keyword.get(opts, :assign_key, :current_user), } end diff --git a/lib/coherence/plugs/authorization/token.ex b/lib/coherence/plugs/authorization/token.ex index 532f93fe..1018bed8 100644 --- a/lib/coherence/plugs/authorization/token.ex +++ b/lib/coherence/plugs/authorization/token.ex @@ -53,7 +53,7 @@ defmodule Coherence.Authentication.Token do Add the credentials for a `token`. `user_data` can be any term but must not be `nil`. """ @spec add_credentials(String.t, t, module) :: t - def add_credentials(token, user_data, store \\ Coherence.CredentialStore.Agent) do + def add_credentials(token, user_data, store \\ Coherence.CredentialStore.Server) do store.put_credentials(token, user_data) end @@ -61,7 +61,7 @@ defmodule Coherence.Authentication.Token do Remove the credentials for a `token`. """ @spec remove_credentials(String.t, module) :: t - def remove_credentials(token, store \\ Coherence.CredentialStore.Agent) do + def remove_credentials(token, store \\ Coherence.CredentialStore.Server) do store.delete_credentials(token) end @@ -82,7 +82,7 @@ defmodule Coherence.Authentication.Token do source: opts |> Keyword.fetch!(:source) |> convert_source(param), error: Keyword.get(opts, :error, "HTTP Authentication Required"), assigns_key: Keyword.get(opts, :assigns_key, :current_user), - store: Keyword.get(opts, :store, Coherence.CredentialStore.Agent), + store: Keyword.get(opts, :store, Coherence.CredentialStore.Server), } end diff --git a/lib/coherence/plugs/authorization/utils.ex b/lib/coherence/plugs/authorization/utils.ex index 7c123d32..94e41257 100644 --- a/lib/coherence/plugs/authorization/utils.ex +++ b/lib/coherence/plugs/authorization/utils.ex @@ -52,7 +52,7 @@ defmodule Coherence.Authentication.Utils do Coherence.Authentication.Session -> Coherence.CredentialStore.Session Coherence.Authentication.Basic -> - Coherence.CredentialStore.Agent + Coherence.CredentialStore.Server end end diff --git a/lib/coherence/supervisor.ex b/lib/coherence/supervisor.ex index 8239b133..c5f29b9c 100644 --- a/lib/coherence/supervisor.ex +++ b/lib/coherence/supervisor.ex @@ -2,7 +2,7 @@ defmodule Coherence.Supervisor do @moduledoc """ Supervisor to start Coherence services. - Starts the configured credential store agent. Also starts + Starts the configured credential store server. Also starts the RememberableServer if this option is configured. """ diff --git a/mix.exs b/mix.exs index 510d8faa..547ee40f 100644 --- a/mix.exs +++ b/mix.exs @@ -49,7 +49,8 @@ defmodule Coherence.Mixfile do {:earmark, "~> 1.2", only: :dev, override: true}, {:postgrex, ">= 0.0.0", only: :test}, {:dialyxir, "~> 0.4", only: [:dev], runtime: false}, - {:credo, "~> 0.8", only: [:dev, :test]} + {:credo, "~> 0.8", only: [:dev, :test]}, + {:exactor, "~> 2.2.3", warn_missing: false} ] end diff --git a/mix.lock b/mix.lock index 13106f83..7a56f586 100644 --- a/mix.lock +++ b/mix.lock @@ -11,6 +11,7 @@ "ecto": {:hex, :ecto, "2.1.4", "d1ba932813ec0e0d9db481ef2c17777f1cefb11fc90fa7c142ff354972dfba7e", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, "elixir_make": {:hex, :elixir_make, "0.4.0", "992f38fabe705bb45821a728f20914c554b276838433349d4f2341f7a687cddf", [:mix], []}, "ex_doc": {:hex, :ex_doc, "0.16.2", "3b3e210ebcd85a7c76b4e73f85c5640c011d2a0b2f06dcdf5acdb2ae904e5084", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, optional: false]}]}, + "exactor": {:hex, :exactor, "2.2.3", "a6972f43bb6160afeb73e1d8ab45ba604cd0ac8b5244c557093f6e92ce582786", [:mix], []}, "floki": {:hex, :floki, "0.14.0", "91a6be57349e10a63cf52d7890479a19012cef9185fa93c305d4fe42e6a50dee", [:mix], [{:mochiweb, "~> 2.15", [hex: :mochiweb, optional: false]}]}, "gen_smtp": {:hex, :gen_smtp, "0.11.0", "d90ff2f021fc86cb2a4259b1f2b177ab6e506676265e26454bf5755855adc956", [:rebar3], []}, "gettext": {:hex, :gettext, "0.13.1", "5e0daf4e7636d771c4c71ad5f3f53ba09a9ae5c250e1ab9c42ba9edccc476263", [:mix], []}, diff --git a/test/plugs/authentication/basic_test.exs b/test/plugs/authentication/basic_test.exs index 212367ea..6bce4d8b 100644 --- a/test/plugs/authentication/basic_test.exs +++ b/test/plugs/authentication/basic_test.exs @@ -35,7 +35,7 @@ defmodule Coherence.Authentication.Basic.Test do end setup do - Coherence.Authentication.Basic.encode_credentials("Admin", "SecretPass") |> Coherence.CredentialStore.Agent.put_credentials(%{role: :admin}) + Coherence.Authentication.Basic.encode_credentials("Admin", "SecretPass") |> Coherence.CredentialStore.Server.put_credentials(%{role: :admin}) end test "request without credentials" do diff --git a/test/plugs/authentication/ip_address_test.exs b/test/plugs/authentication/ip_address_test.exs index 59287b0d..7c52db13 100644 --- a/test/plugs/authentication/ip_address_test.exs +++ b/test/plugs/authentication/ip_address_test.exs @@ -58,7 +58,7 @@ defmodule CoherenceTest.Authentication.IpAddress do end setup do - # Coherence.CredentialStore.Agent.put_credentials("secret_token", %{role: :admin}) + # Coherence.CredentialStore.Server.put_credentials("secret_token", %{role: :admin}) :ok end diff --git a/test/plugs/authentication/token_test.exs b/test/plugs/authentication/token_test.exs index bbe2b7d6..ea557050 100644 --- a/test/plugs/authentication/token_test.exs +++ b/test/plugs/authentication/token_test.exs @@ -70,7 +70,7 @@ defmodule CoherenceTest.Authentication.Token do defp auth_param(creds), do: {"auth_token", creds} setup do - Coherence.CredentialStore.Agent.put_credentials("secret_token", %{role: :admin}) + Coherence.CredentialStore.Server.put_credentials("secret_token", %{role: :admin}) end test "request without credentials using header-based auth" do diff --git a/test/test_helper.exs b/test/test_helper.exs index 2a182ee2..e29d0941 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,5 +1,5 @@ ExUnit.start() - +Application.ensure_all_started(:coherence) Code.require_file "./support/gettext.exs", __DIR__ Code.require_file "./support/messages.exs", __DIR__