From 4c05a93ef8f135cc7e2676c12350ff08a2a1c21a Mon Sep 17 00:00:00 2001 From: Luiz Carlos Date: Sat, 10 Oct 2020 10:40:39 -0300 Subject: [PATCH 1/4] chore: general refactor on resource_manager --- .github/workflows/main.yml | 4 +- .tool-versions | 4 +- .../{cache.ex => blocklist_password_cache.ex} | 2 +- ...nager.ex => blocklist_password_manager.ex} | 12 +++--- .../commands/password_is_allowed.ex | 41 +++++++++++++++++++ .../lib/credentials/passwords.ex | 23 ----------- apps/resource_manager/lib/resource_manager.ex | 4 ++ .../blockist_password_manager_test.exs | 14 +++++++ .../commands/password_is_allowed_test.exs | 28 +++++++++++++ .../credentials/manager_test.exs | 14 ------- .../credentials/passwords_test.exs | 26 +----------- .../credentials/public_key_test.exs | 2 +- .../identity/client_applications_test.exs | 3 +- .../resource_manager/identity/users_test.exs | 2 +- .../permissions/scopes_test.exs | 2 +- config/config.exs | 4 +- config/test.exs | 2 +- 17 files changed, 108 insertions(+), 79 deletions(-) rename apps/resource_manager/lib/credentials/{cache.ex => blocklist_password_cache.ex} (79%) rename apps/resource_manager/lib/credentials/{manager.ex => blocklist_password_manager.ex} (90%) create mode 100644 apps/resource_manager/lib/credentials/commands/password_is_allowed.ex create mode 100644 apps/resource_manager/test/resource_manager/credentials/blockist_password_manager_test.exs create mode 100644 apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs delete mode 100644 apps/resource_manager/test/resource_manager/credentials/manager_test.exs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 71b80f5..2069099 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,8 +18,8 @@ jobs: strategy: matrix: - elixir: [1.10.3] - otp: [22.3] + elixir: [1.11] + otp: [23.1] steps: - uses: actions/checkout@v2 diff --git a/.tool-versions b/.tool-versions index 56ba8f7..21855b7 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ -elixir 1.10.4-otp-23 -erlang 23.0 +elixir 1.11.0-otp-23 +erlang 23.1 diff --git a/apps/resource_manager/lib/credentials/cache.ex b/apps/resource_manager/lib/credentials/blocklist_password_cache.ex similarity index 79% rename from apps/resource_manager/lib/credentials/cache.ex rename to apps/resource_manager/lib/credentials/blocklist_password_cache.ex index 1b0f9a2..bec6a8d 100644 --- a/apps/resource_manager/lib/credentials/cache.ex +++ b/apps/resource_manager/lib/credentials/blocklist_password_cache.ex @@ -1,4 +1,4 @@ -defmodule ResourceManager.Credentials.Cache do +defmodule ResourceManager.Credentials.BlocklistPasswordCache do @moduledoc """ Passwords credentials generic cache. diff --git a/apps/resource_manager/lib/credentials/manager.ex b/apps/resource_manager/lib/credentials/blocklist_password_manager.ex similarity index 90% rename from apps/resource_manager/lib/credentials/manager.ex rename to apps/resource_manager/lib/credentials/blocklist_password_manager.ex index 7be35bb..7852ba9 100644 --- a/apps/resource_manager/lib/credentials/manager.ex +++ b/apps/resource_manager/lib/credentials/blocklist_password_manager.ex @@ -1,13 +1,15 @@ -defmodule ResourceManager.Credentials.Manager do +defmodule ResourceManager.Credentials.BlocklistPasswordManager do @moduledoc """ - GenServer for dealing with session expirations. + GenServer for managing password blocklist. + + All passwords in this list should not be acceptable as user credentials. """ use GenServer require Logger - alias ResourceManager.Credentials.Cache + alias ResourceManager.Credentials.BlocklistPasswordCache @typedoc "Credentials manager supervisor state" @type state :: %{ @@ -81,7 +83,7 @@ defmodule ResourceManager.Credentials.Manager do ########## defp manage_passwords do - if Cache.size() == 0 do + if BlocklistPasswordCache.size() == 0 do Logger.debug("Credential manager Loading cache from dump") file_path() @@ -89,7 +91,7 @@ defmodule ResourceManager.Credentials.Manager do |> String.trim() |> String.split("\n") |> Enum.map(fn pwd -> %Nebulex.Object{key: pwd, value: pwd, version: 1} end) - |> Cache.set_many() + |> BlocklistPasswordCache.set_many() |> case do :ok -> Logger.debug("Credential manager cache loaded with success") diff --git a/apps/resource_manager/lib/credentials/commands/password_is_allowed.ex b/apps/resource_manager/lib/credentials/commands/password_is_allowed.ex new file mode 100644 index 0000000..96b01ab --- /dev/null +++ b/apps/resource_manager/lib/credentials/commands/password_is_allowed.ex @@ -0,0 +1,41 @@ +defmodule ResourceManager.Credentials.Commands.PasswordIsAllowed do + @moduledoc """ + Comand for checking if a password is allowed or not. + """ + + require Logger + + alias ResourceManager.Credentials.BlocklistPasswordCache + + @doc "Checks if the given password is strong enough to be used" + @spec execute(password :: String.t()) :: boolean() + def execute(password) when is_binary(password) do + Logger.info("Checking if password is allowed") + + with {:strong?, true} <- {:strong?, is_strong?(password)}, + {:blocklisted?, false} <- {:blocklisted?, is_blocklisted?(password)} do + Logger.info("Password allowed!") + true + else + {:strong?, false} -> + Logger.info("Password not allowed because it's not strong enough") + false + + {:blocklisted?, true} -> + Logger.info("Password not allowed because it's on blocklist") + false + end + end + + defp is_strong?(password) when byte_size(password) >= 6, do: true + defp is_strong?(password) when byte_size(password) < 6, do: false + + defp is_blocklisted?(password) do + password + |> BlocklistPasswordCache.get() + |> case do + nil -> false + _any -> true + end + end +end diff --git a/apps/resource_manager/lib/credentials/passwords.ex b/apps/resource_manager/lib/credentials/passwords.ex index 416f9c9..295a753 100644 --- a/apps/resource_manager/lib/credentials/passwords.ex +++ b/apps/resource_manager/lib/credentials/passwords.ex @@ -7,27 +7,4 @@ defmodule ResourceManager.Credentials.Passwords do """ use ResourceManager.Domain, schema_model: ResourceManager.Credentials.Schemas.Password - - alias ResourceManager.Credentials.Cache - - @doc "Checks if the given password is strong enough to be used" - @spec is_strong?(password :: String.t()) :: boolean() - def is_strong?(password) when is_binary(password) do - cond do - String.length(password) < 6 -> false - is_allowed?(password) == false -> false - true -> true - end - end - - @doc "Checks if the given password is one of the most common passwords" - @spec is_allowed?(password :: String.t()) :: boolean() - def is_allowed?(password) when is_binary(password) do - password - |> Cache.get() - |> case do - nil -> true - _any -> false - end - end end diff --git a/apps/resource_manager/lib/resource_manager.ex b/apps/resource_manager/lib/resource_manager.ex index 88f10ff..991fc01 100644 --- a/apps/resource_manager/lib/resource_manager.ex +++ b/apps/resource_manager/lib/resource_manager.ex @@ -3,6 +3,7 @@ defmodule ResourceManager do Application to deal with request's to the resource server. """ + alias ResourceManager.Credentials.Commands.PasswordIsAllowed alias ResourceManager.Identity.Commands.{CreateIdentity, GetIdentity} alias ResourceManager.Permissions.Commands.{ConsentScope, RemoveScope} @@ -17,4 +18,7 @@ defmodule ResourceManager do @doc "Delegates to #{RemoveScope}.execute/2" defdelegate remove_scope(identity, scopes), to: RemoveScope, as: :execute + + @doc "Delegates to #{PasswordIsAllowed}.execute/1" + defdelegate password_allowed?(password), to: PasswordIsAllowed, as: :execute end diff --git a/apps/resource_manager/test/resource_manager/credentials/blockist_password_manager_test.exs b/apps/resource_manager/test/resource_manager/credentials/blockist_password_manager_test.exs new file mode 100644 index 0000000..5bf39ce --- /dev/null +++ b/apps/resource_manager/test/resource_manager/credentials/blockist_password_manager_test.exs @@ -0,0 +1,14 @@ +defmodule ResourceManager.Credentials.BlocklistPasswordManagerTest do + use ResourceManager.DataCase, async: true + + alias ResourceManager.Credentials.{BlocklistPasswordCache, BlocklistPasswordManager} + + describe "#{BlocklistPasswordManager}.execute/o" do + test "populates the cache with the passwords" do + assert [] == BlocklistPasswordCache.all() + assert {:ok, :managed} = BlocklistPasswordManager.execute() + assert [password | _] = BlocklistPasswordCache.all() + assert is_binary(password) + end + end +end diff --git a/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs b/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs new file mode 100644 index 0000000..275dded --- /dev/null +++ b/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs @@ -0,0 +1,28 @@ +defmodule ResourceManager.Credentials.Commands.PasswordIsAllowedTest do + use ResourceManager.DataCase, async: true + + alias ResourceManager.Credentials.Commands.PasswordIsAllowed + alias ResourceManager.Credentials.BlocklistPasswordCache + + describe "#{PasswordIsAllowed}.execute/1" do + test "returnt true if password is strong enough" do + assert true == PasswordIsAllowed.execute("TheBiggestPasswordAll@Wed") + end + + test "returnt false if password is not strong enough" do + assert false == PasswordIsAllowed.execute("1234") + end + end + + describe "#{PasswordIsAllowed}.is_allowed?/1" do + test "returnt true if password is allowed" do + assert BlocklistPasswordCache.set("TheBiggestPasswordAll", "TheBiggestPasswordAll") + assert true == PasswordIsAllowed.execute("TheBiggestPasswordAll@Wed") + end + + test "returnt false if password is not strong enough" do + assert BlocklistPasswordCache.set("TheBiggestPasswordAll", "TheBiggestPasswordAll") + assert false == PasswordIsAllowed.execute("TheBiggestPasswordAll") + end + end +end diff --git a/apps/resource_manager/test/resource_manager/credentials/manager_test.exs b/apps/resource_manager/test/resource_manager/credentials/manager_test.exs deleted file mode 100644 index 44e083b..0000000 --- a/apps/resource_manager/test/resource_manager/credentials/manager_test.exs +++ /dev/null @@ -1,14 +0,0 @@ -defmodule ResourceManager.Credentials.ManagerTest do - use ResourceManager.DataCase, async: true - - alias ResourceManager.Credentials.{Cache, Manager} - - describe "Manager.execute/o" do - test "populates the cache with the passwords" do - assert [] == Cache.all() - assert {:ok, :managed} = Manager.execute() - assert [password | _] = Cache.all() - assert is_binary(password) - end - end -end diff --git a/apps/resource_manager/test/resource_manager/credentials/passwords_test.exs b/apps/resource_manager/test/resource_manager/credentials/passwords_test.exs index eaf44e0..20d5c9f 100644 --- a/apps/resource_manager/test/resource_manager/credentials/passwords_test.exs +++ b/apps/resource_manager/test/resource_manager/credentials/passwords_test.exs @@ -1,7 +1,7 @@ defmodule ResourceManager.Credentials.PasswordsTest do use ResourceManager.DataCase, async: true - alias ResourceManager.Credentials.{Cache, Passwords} + alias ResourceManager.Credentials.Passwords alias ResourceManager.Credentials.Schemas.Password setup do @@ -70,7 +70,7 @@ defmodule ResourceManager.Credentials.PasswordsTest do describe "#{Passwords}.delete/1" do test "succeed if params are valid", ctx do - assert {:ok, %Password{id: id} = password} = Passwords.delete(ctx.password) + assert {:ok, %Password{id: id}} = Passwords.delete(ctx.password) assert nil == Repo.get(Password, id) end @@ -80,26 +80,4 @@ defmodule ResourceManager.Credentials.PasswordsTest do end end end - - describe "#{Passwords}.is_strong?/1" do - test "returnt true if password is strong enough" do - assert true == Passwords.is_strong?("TheBiggestPasswordAll@Wed") - end - - test "returnt false if password is not strong enough" do - assert false == Passwords.is_strong?("1234") - end - end - - describe "#{Passwords}.is_allowed?/1" do - test "returnt true if password is allowed" do - assert Cache.set("TheBiggestPasswordAll", "TheBiggestPasswordAll") - assert true == Passwords.is_strong?("TheBiggestPasswordAll@Wed") - end - - test "returnt false if password is not strong enough" do - assert Cache.set("TheBiggestPasswordAll", "TheBiggestPasswordAll") - assert false == Passwords.is_strong?("TheBiggestPasswordAll") - end - end end diff --git a/apps/resource_manager/test/resource_manager/credentials/public_key_test.exs b/apps/resource_manager/test/resource_manager/credentials/public_key_test.exs index b415443..48a3a3c 100644 --- a/apps/resource_manager/test/resource_manager/credentials/public_key_test.exs +++ b/apps/resource_manager/test/resource_manager/credentials/public_key_test.exs @@ -74,7 +74,7 @@ defmodule ResourceManager.Credentials.PublicKeysTest do describe "#{PublicKeys}.delete/1" do test "succeed if params are valid", ctx do - assert {:ok, %PublicKey{id: id} = public_key} = PublicKeys.delete(ctx.public_key) + assert {:ok, %PublicKey{id: id}} = PublicKeys.delete(ctx.public_key) assert nil == Repo.get(PublicKey, id) end diff --git a/apps/resource_manager/test/resource_manager/identity/client_applications_test.exs b/apps/resource_manager/test/resource_manager/identity/client_applications_test.exs index ec8f5ea..879c4ac 100644 --- a/apps/resource_manager/test/resource_manager/identity/client_applications_test.exs +++ b/apps/resource_manager/test/resource_manager/identity/client_applications_test.exs @@ -73,8 +73,7 @@ defmodule ResourceManager.Identity.ClientApplicationsTest do describe "#{ClientApplications}.delete/1" do test "succeed if params are valid", ctx do - assert {:ok, %ClientApplication{id: id} = client_application} = - ClientApplications.delete(ctx.client_application) + assert {:ok, %ClientApplication{id: id}} = ClientApplications.delete(ctx.client_application) assert nil == Repo.get(ClientApplication, id) end diff --git a/apps/resource_manager/test/resource_manager/identity/users_test.exs b/apps/resource_manager/test/resource_manager/identity/users_test.exs index 97abe2a..709ca20 100644 --- a/apps/resource_manager/test/resource_manager/identity/users_test.exs +++ b/apps/resource_manager/test/resource_manager/identity/users_test.exs @@ -62,7 +62,7 @@ defmodule ResourceManager.Identity.UsersTest do describe "#{Users}.delete/1" do test "succeed if params are valid", ctx do - assert {:ok, %User{id: id} = user} = Users.delete(ctx.user) + assert {:ok, %User{id: id}} = Users.delete(ctx.user) assert nil == Repo.get(User, id) end diff --git a/apps/resource_manager/test/resource_manager/permissions/scopes_test.exs b/apps/resource_manager/test/resource_manager/permissions/scopes_test.exs index c84d827..f8dcab3 100644 --- a/apps/resource_manager/test/resource_manager/permissions/scopes_test.exs +++ b/apps/resource_manager/test/resource_manager/permissions/scopes_test.exs @@ -62,7 +62,7 @@ defmodule ResourceManager.Permissions.ScopesTest do describe "#{Scopes}.delete/1" do test "succeed if params are valid", ctx do - assert {:ok, %Scope{id: id} = scope} = Scopes.delete(ctx.scope) + assert {:ok, %Scope{id: id}} = Scopes.delete(ctx.scope) assert nil == Repo.get(Scope, id) end diff --git a/config/config.exs b/config/config.exs index f8f0451..cdfd3d0 100644 --- a/config/config.exs +++ b/config/config.exs @@ -15,8 +15,8 @@ config :resource_manager, ecto_repos: [ResourceManager.Repo] config :resource_manager, ResourceManager.Application, children: [ ResourceManager.Repo, - ResourceManager.Credentials.Cache, - ResourceManager.Credentials.Manager + ResourceManager.Credentials.BlocklistPasswordCache, + ResourceManager.Credentials.BlocklistPasswordManager ] config :resource_manager, ResourceManager.Repo, diff --git a/config/test.exs b/config/test.exs index 8c54f67..34bbde0 100644 --- a/config/test.exs +++ b/config/test.exs @@ -15,7 +15,7 @@ config :resource_manager, ResourceManager.Repo, show_sensitive_data_on_connection_error: true config :resource_manager, ResourceManager.Application, - children: [ResourceManager.Repo, ResourceManager.Credentials.Cache] + children: [ResourceManager.Repo, ResourceManager.Credentials.BlocklistPasswordCache] config :resource_manager, ResourceManager.Credentials.Ports.GenerateHash, command: ResourceManager.Credentials.Ports.GenerateHashMock From 9f16b9a0cc4b7135501f56fc164671445d397181 Mon Sep 17 00:00:00 2001 From: Luiz Carlos Date: Sat, 10 Oct 2020 10:42:56 -0300 Subject: [PATCH 2/4] chore: update elixir --- apps/authenticator/mix.exs | 2 +- apps/resource_manager/mix.exs | 2 +- apps/rest_api/mix.exs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/authenticator/mix.exs b/apps/authenticator/mix.exs index 86f2960..a620b3f 100644 --- a/apps/authenticator/mix.exs +++ b/apps/authenticator/mix.exs @@ -10,7 +10,7 @@ defmodule Authenticator.MixProject do elixirc_paths: elixirc_paths(Mix.env()), deps_path: "../../deps", lockfile: "../../mix.lock", - elixir: "~> 1.10", + elixir: "~> 1.11", start_permanent: Mix.env() == :prod, deps: deps(), aliases: aliases(), diff --git a/apps/resource_manager/mix.exs b/apps/resource_manager/mix.exs index e0594b0..6b89f95 100644 --- a/apps/resource_manager/mix.exs +++ b/apps/resource_manager/mix.exs @@ -10,7 +10,7 @@ defmodule ResourceManager.MixProject do elixirc_paths: elixirc_paths(Mix.env()), deps_path: "../../deps", lockfile: "../../mix.lock", - elixir: "~> 1.10", + elixir: "~> 1.11", start_permanent: Mix.env() == :prod, aliases: aliases(), deps: deps(), diff --git a/apps/rest_api/mix.exs b/apps/rest_api/mix.exs index dfdbaac..ed2e444 100644 --- a/apps/rest_api/mix.exs +++ b/apps/rest_api/mix.exs @@ -9,7 +9,7 @@ defmodule RestAPI.MixProject do config_path: "../../config/config.exs", deps_path: "../../deps", lockfile: "../../mix.lock", - elixir: "~> 1.10", + elixir: "~> 1.11", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix] ++ Mix.compilers(), start_permanent: Mix.env() == :prod, From 09ac6a8b932435b78eeefe12c081c6693b688cc3 Mon Sep 17 00:00:00 2001 From: Luiz Carlos Date: Sat, 10 Oct 2020 10:44:33 -0300 Subject: [PATCH 3/4] chore: update ci flow --- .github/workflows/main.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2069099..b3b6156 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -54,8 +54,8 @@ jobs: strategy: matrix: - elixir: [1.10.3] - otp: [22.3] + elixir: [1.11] + otp: [23.1] steps: - uses: actions/checkout@v2 @@ -86,8 +86,8 @@ jobs: strategy: matrix: - elixir: [1.10.3] - otp: [22.3] + elixir: [1.11] + otp: [23.1] steps: - uses: actions/checkout@v2 @@ -128,8 +128,8 @@ jobs: strategy: matrix: - elixir: [1.10.3] - otp: [22.3] + elixir: [1.11] + otp: [23.1] services: postgres: From 72381558e7f1abcc631abfb6006b309660a90952 Mon Sep 17 00:00:00 2001 From: Luiz Carlos Date: Sat, 10 Oct 2020 10:52:41 -0300 Subject: [PATCH 4/4] chore: make credo happy aggain --- .../credentials/commands/password_is_allowed_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs b/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs index 275dded..2db0a9c 100644 --- a/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs +++ b/apps/resource_manager/test/resource_manager/credentials/commands/password_is_allowed_test.exs @@ -1,8 +1,8 @@ defmodule ResourceManager.Credentials.Commands.PasswordIsAllowedTest do use ResourceManager.DataCase, async: true - alias ResourceManager.Credentials.Commands.PasswordIsAllowed alias ResourceManager.Credentials.BlocklistPasswordCache + alias ResourceManager.Credentials.Commands.PasswordIsAllowed describe "#{PasswordIsAllowed}.execute/1" do test "returnt true if password is strong enough" do