From 3a48398dc38c054f4675fc9629b1da7fc248f3e1 Mon Sep 17 00:00:00 2001 From: Leandro Pereira Date: Thu, 25 Jul 2024 18:20:43 -0400 Subject: [PATCH 1/3] Remove Authorization System is not fully baked yet, although it's functional, so it's better to not promote it at this moment while we can resolve the final public API to avoid breaking changes. --- lib/beacon/authorization.ex | 26 +++++++++------ lib/beacon/authorization/default_policy.ex | 13 +++++--- lib/beacon/authorization/policy.ex | 37 ++++++++++++---------- lib/beacon/beacon.ex | 1 - lib/beacon/config.ex | 22 +++++++------ lib/errors.ex | 15 +++++---- 6 files changed, 67 insertions(+), 47 deletions(-) diff --git a/lib/beacon/authorization.ex b/lib/beacon/authorization.ex index c998a316c..f4dfbce41 100644 --- a/lib/beacon/authorization.ex +++ b/lib/beacon/authorization.ex @@ -1,11 +1,19 @@ defmodule Beacon.Authorization do - @moduledoc """ - Executes the authorization rules defined by `Beacon.Authorization.Policy`. + @moduledoc false - Most of those calls are done in Beacon LiveAdmin and you don't need to call them directly, - unless you're building a custom admin page or custom logic then you can rely on this module - to get the agent (who is performing the operation) and check if the agent is authorized to perform such operation. - """ + # TODO: review authz + # """ + # Protects resources by authorizing agents to perform operations in Beacon. + + # An agent might be a user, a system, or any entity that is performing an operation. + + # Executes the authorization rules defined by `Beacon.Authorization.Policy`. + + # Most of those calls are done in Beacon LiveAdmin or internally in Beacon contexts so you don't need to call them directly, + # unless you're building a custom admin page or custom logic then you can rely on this module + # to get the agent (who is performing the operation) with `get_agent/2` and check if the agent + # is authorized to perform such operation with `authorized?/4`. + # """ alias Beacon.Behaviors.Helpers @@ -16,11 +24,11 @@ defmodule Beacon.Authorization do |> do_get_agent(payload) end - defp do_get_agent(nil = _authorization_source, _data), do: nil + defp do_get_agent(nil = _authorization_source, _payload), do: nil - defp do_get_agent(authorization_source, data) do + defp do_get_agent(authorization_source, payload) do if Beacon.exported?(authorization_source, :get_agent, 1) do - authorization_source.get_agent(data) + authorization_source.get_agent(payload) else nil end diff --git a/lib/beacon/authorization/default_policy.ex b/lib/beacon/authorization/default_policy.ex index 07fc6462a..75c98d706 100644 --- a/lib/beacon/authorization/default_policy.ex +++ b/lib/beacon/authorization/default_policy.ex @@ -1,10 +1,13 @@ defmodule Beacon.Authorization.DefaultPolicy do - @moduledoc """ - Provides a simple authorization policy that allows every operation for any agent. + @moduledoc false - That's the default policy used when no other is defined, - see `Beacon.Authorization.Policy` for more information. - """ + # TODO: review authz + # """ + # Provides a simple authorization policy that allows every operation for any agent. + + # That's the default policy used when no other is defined, + # see `Beacon.Authorization.Policy` for more information. + # """ @behaviour Beacon.Authorization.Policy diff --git a/lib/beacon/authorization/policy.ex b/lib/beacon/authorization/policy.ex index 3b6a29e97..d3c22d4a0 100644 --- a/lib/beacon/authorization/policy.ex +++ b/lib/beacon/authorization/policy.ex @@ -1,27 +1,32 @@ defmodule Beacon.Authorization.Policy do - @moduledoc """ - Provides hooks into Beacon Authorization. + @moduledoc false - ## Example + # TODO: review authz + # """ + # Rules to authorize agents to perform operations in Beacon. - defmodule MyApp.Beacon.AuthzPolicy do - @behaviour Beacon.Authorization.Policy + # Below is an example of a policy that finds the person performing the operation through a session_id + # returned in the login and checks if the person is authorized to perform some operations. - def get_agent(%{"session_id" => session_id}) do - MyApp.Identity.find_by_session_id!(session_id) # returns %{user_id: 1, role: :admin} - end + # ## Example - # admin has access to all operations - def authorized?(%{role: :admin}, _, _), do: true + # defmodule MyApp.Beacon.AuthzPolicy do + # @behaviour Beacon.Authorization.Policy - # everyone can access page editor index - def authorized?(_, :index, %{mod: :page_editor}), do: true + # def get_agent(%{"session_id" => session_id}) do + # MyApp.Identity.find_by_session_id!(session_id) # returns %{user_id: 1, role: :admin} + # end - # specific role can't delete a resource in page editor - def authorized?(%{role: :fact_checker}, :delete, %{mod: :page_editor}), do: false - end + # # admin has access to all operations + # def authorized?(%{role: :admin}, _, _), do: true - """ + # # everyone can access page editor index (list pages) + # def authorized?(_, :index, %{mod: :page_editor}), do: true + + # # role external_contributor can't delete pages + # def authorized?(%{role: :external_contributor}, :delete, %{mod: :page_editor}), do: false + # end + # """ # TODO: doc payload, possible operations, and context diff --git a/lib/beacon/beacon.ex b/lib/beacon/beacon.ex index 8d655ccf6..844cd6982 100644 --- a/lib/beacon/beacon.ex +++ b/lib/beacon/beacon.ex @@ -16,7 +16,6 @@ defmodule Beacon do * `Beacon.Lifecycle` - inject custom logic into Beacon lifecycle to change how pages are loaded an rendred, and more. * `Beacon.Content` - manage content as layouts, pages, page variants, snippets, and more. * `Beacon.MediaLibrary` - upload images, videos, and documents that can be used in your content. - * `Beacon.Authorization` - define permissions to limit access to content and features, also used on Beacon LiveAdmin. Get started with [your first site](https://hexdocs.pm/beacon/your-first-site.html) and check out the guides for more information. diff --git a/lib/beacon/config.ex b/lib/beacon/config.ex index b36c40be4..2b1f4a3df 100644 --- a/lib/beacon/config.ex +++ b/lib/beacon/config.ex @@ -55,10 +55,10 @@ defmodule Beacon.Config do """ @type skip_boot? :: boolean() - @typedoc """ - A module that implements `Beacon.Authorization.Policy`, used to provide authorization rules. - """ - @type authorization_source :: module() + # @typedoc """ + # A module that implements `Beacon.Authorization.Policy`, used to provide authorization rules. + # """ + # @type authorization_source :: module() @typedoc """ A module that implements `Beacon.RuntimeCSS`. @@ -184,7 +184,7 @@ defmodule Beacon.Config do router: router(), repo: repo(), skip_boot?: skip_boot?(), - authorization_source: authorization_source(), + # authorization_source: authorization_source(), css_compiler: css_compiler(), tailwind_config: tailwind_config(), live_socket_path: live_socket_path(), @@ -218,6 +218,7 @@ defmodule Beacon.Config do router: nil, repo: nil, skip_boot?: false, + # TODO: rename to `authorization_policy` authorization_source: Beacon.Authorization.DefaultPolicy, css_compiler: Beacon.RuntimeCSS.TailwindCompiler, tailwind_config: nil, @@ -247,7 +248,7 @@ defmodule Beacon.Config do | {:router, router()} | {:repo, repo()} | {:skip_boot?, skip_boot?()} - | {:authorization_source, authorization_source()} + # | {:authorization_source, authorization_source()} | {:css_compiler, css_compiler()} | {:tailwind_config, tailwind_config()} | {:live_socket_path, live_socket_path()} @@ -260,6 +261,11 @@ defmodule Beacon.Config do | {:extra_asset_fields, extra_asset_fields()} | {:default_meta_tags, default_meta_tags()} + # TODO: review authz + # * `:authorization_source` - `t:authorization_source/0` (optional). Defaults to `Beacon.Authorization.DefaultPolicy`. + # authorization_source: MyApp.MySiteAuthzPolicy, + # authorization_source: MyApp.SiteAuthnPolicy, + @doc """ Build a new `%Beacon.Config{}` instance to hold the entire configuration for each site. @@ -275,8 +281,6 @@ defmodule Beacon.Config do * `:skip_boot?` - `t:skip_boot?/0` (optional). Defaults to `false`. - * `:authorization_source` - `t:authorization_source/0` (optional). Defaults to `Beacon.Authorization.DefaultPolicy`. - * `css_compiler` - `t:css_compiler/0` (optional). Defaults to `Beacon.RuntimeCSS.TailwindCompiler`. * `:tailwind_config` - `t:tailwind_config/0` (optional). Defaults to `Path.join(Application.app_dir(:beacon, "priv"), "tailwind.config.js")`. @@ -313,7 +317,6 @@ defmodule Beacon.Config do endpoint: MyAppWeb.Endpoint, router: MyAppWeb.Router, repo: MyApp.Repo, - authorization_source: MyApp.MySiteAuthzPolicy, tailwind_config: Path.join(Application.app_dir(:my_app, "priv"), "tailwind.config.js"), template_formats: [ {:custom_format, "My Custom Format"} @@ -342,7 +345,6 @@ defmodule Beacon.Config do router: MyAppWeb.Router, repo: MyApp.Repo, skip_boot?: false, - authorization_source: MyApp.SiteAuthnPolicy, css_compiler: Beacon.RuntimeCSS.TailwindCompiler, tailwind_config: "/my_app/priv/tailwind.config.js", live_socket_path: "/live", diff --git a/lib/errors.ex b/lib/errors.ex index 19090cc5d..d3a8caf60 100644 --- a/lib/errors.ex +++ b/lib/errors.ex @@ -35,13 +35,16 @@ defmodule Beacon.RuntimeError do end defmodule Beacon.AuthorizationError do - @moduledoc """ - Raised when there is an error in the `get_agent/1` or `authorized?/3` callbacks of - a `Beacon.Authorization.Policy`. + @moduledoc false - If you're seeing this error, and have implemented a custom `Beacon.Authorization.Policy`, - check the logic in your policy module. - """ + # TODO: review authz + # """ + # Raised when there is an error in the `get_agent/1` or `authorized?/3` callbacks of + # a `Beacon.Authorization.Policy`. + + # If you're seeing this error, and have implemented a custom `Beacon.Authorization.Policy`, + # check the logic in your policy module. + # """ defexception message: "error in Beacon.Authorization" end From 4d429b759fd28669fd8caefe61db7843d6bc7c36 Mon Sep 17 00:00:00 2001 From: Leandro Pereira Date: Thu, 25 Jul 2024 18:33:09 -0400 Subject: [PATCH 2/3] remove files --- lib/beacon/authorization.ex | 71 ------------------- lib/beacon/authorization/default_policy.ex | 19 ----- lib/beacon/authorization/policy.ex | 46 ------------ lib/beacon/beacon.ex | 17 +---- lib/beacon/config.ex | 16 +---- lib/errors.ex | 15 ---- .../authorization/default_policy_test.exs | 18 ----- test/beacon/authorization_test.exs | 18 ----- test/beacon/config_test.exs | 1 - test/test_helper.exs | 3 +- 10 files changed, 5 insertions(+), 219 deletions(-) delete mode 100644 lib/beacon/authorization.ex delete mode 100644 lib/beacon/authorization/default_policy.ex delete mode 100644 lib/beacon/authorization/policy.ex delete mode 100644 test/beacon/authorization/default_policy_test.exs delete mode 100644 test/beacon/authorization_test.exs diff --git a/lib/beacon/authorization.ex b/lib/beacon/authorization.ex deleted file mode 100644 index f4dfbce41..000000000 --- a/lib/beacon/authorization.ex +++ /dev/null @@ -1,71 +0,0 @@ -defmodule Beacon.Authorization do - @moduledoc false - - # TODO: review authz - # """ - # Protects resources by authorizing agents to perform operations in Beacon. - - # An agent might be a user, a system, or any entity that is performing an operation. - - # Executes the authorization rules defined by `Beacon.Authorization.Policy`. - - # Most of those calls are done in Beacon LiveAdmin or internally in Beacon contexts so you don't need to call them directly, - # unless you're building a custom admin page or custom logic then you can rely on this module - # to get the agent (who is performing the operation) with `get_agent/2` and check if the agent - # is authorized to perform such operation with `authorized?/4`. - # """ - - alias Beacon.Behaviors.Helpers - - @spec get_agent(Beacon.Types.site(), any()) :: any() | nil - def get_agent(site, payload) when is_atom(site) do - site - |> get_authorization_source!() - |> do_get_agent(payload) - end - - defp do_get_agent(nil = _authorization_source, _payload), do: nil - - defp do_get_agent(authorization_source, payload) do - if Beacon.exported?(authorization_source, :get_agent, 1) do - authorization_source.get_agent(payload) - else - nil - end - rescue - error in FunctionClauseError -> - Helpers.reraise_function_clause_error(error.function, error.arity, __STACKTRACE__, Beacon.AuthorizationError) - - error -> - reraise error, __STACKTRACE__ - end - - @spec authorized?(Beacon.Types.site(), any(), atom(), map()) :: boolean() - def authorized?(site, agent, operation, context) when is_atom(site) do - site - |> get_authorization_source!() - |> do_authorized?(agent, operation, context) - end - - defp do_authorized?(authorization_source, agent, operation, context) do - if Beacon.exported?(authorization_source, :authorized?, 3) do - authorization_source.authorized?(agent, operation, context) - else - raise """ - authorization source is misconfigured. - - Expected a module implementing Beacon.Authorization.Policy callbacks. - - Got: #{inspect(authorization_source)} - """ - end - rescue - error in FunctionClauseError -> - Helpers.reraise_function_clause_error(error.function, error.arity, __STACKTRACE__, Beacon.AuthorizationError) - - error -> - reraise error, __STACKTRACE__ - end - - defp get_authorization_source!(site), do: Beacon.Config.fetch!(site).authorization_source -end diff --git a/lib/beacon/authorization/default_policy.ex b/lib/beacon/authorization/default_policy.ex deleted file mode 100644 index 75c98d706..000000000 --- a/lib/beacon/authorization/default_policy.ex +++ /dev/null @@ -1,19 +0,0 @@ -defmodule Beacon.Authorization.DefaultPolicy do - @moduledoc false - - # TODO: review authz - # """ - # Provides a simple authorization policy that allows every operation for any agent. - - # That's the default policy used when no other is defined, - # see `Beacon.Authorization.Policy` for more information. - # """ - - @behaviour Beacon.Authorization.Policy - - @impl true - def get_agent(data), do: data - - @impl true - def authorized?(_agent, _operation, _context), do: true -end diff --git a/lib/beacon/authorization/policy.ex b/lib/beacon/authorization/policy.ex deleted file mode 100644 index d3c22d4a0..000000000 --- a/lib/beacon/authorization/policy.ex +++ /dev/null @@ -1,46 +0,0 @@ -defmodule Beacon.Authorization.Policy do - @moduledoc false - - # TODO: review authz - # """ - # Rules to authorize agents to perform operations in Beacon. - - # Below is an example of a policy that finds the person performing the operation through a session_id - # returned in the login and checks if the person is authorized to perform some operations. - - # ## Example - - # defmodule MyApp.Beacon.AuthzPolicy do - # @behaviour Beacon.Authorization.Policy - - # def get_agent(%{"session_id" => session_id}) do - # MyApp.Identity.find_by_session_id!(session_id) # returns %{user_id: 1, role: :admin} - # end - - # # admin has access to all operations - # def authorized?(%{role: :admin}, _, _), do: true - - # # everyone can access page editor index (list pages) - # def authorized?(_, :index, %{mod: :page_editor}), do: true - - # # role external_contributor can't delete pages - # def authorized?(%{role: :external_contributor}, :delete, %{mod: :page_editor}), do: false - # end - # """ - - # TODO: doc payload, possible operations, and context - - @doc """ - Return the agent assigned by `Beacon.LiveAdmin.Hooks.AssignAgent` - """ - @callback get_agent(payload :: any()) :: any() - - @doc """ - Return `true` to authorize `agent` to perform `operation` in the given `context`, - otherwise return `false` to block such operation. - - Note that Beacon LiveAdmin will either redirect or display a flash message - if the operation is not authorized. - """ - @callback authorized?(agent :: any(), operation :: atom(), context :: map()) :: boolean() -end diff --git a/lib/beacon/beacon.ex b/lib/beacon/beacon.ex index 844cd6982..213052fae 100644 --- a/lib/beacon/beacon.ex +++ b/lib/beacon/beacon.ex @@ -52,8 +52,7 @@ defmodule Beacon do config :my_app, Beacon, sites: [ [site: :my_site, endpoint: MyAppWeb.Endpoint] - ], - authorization_source: MyApp.AuthorizationPolicy + ] # lib/my_app/application.ex def start(_type, _args) do @@ -86,22 +85,10 @@ defmodule Beacon do :pg.start_link(:beacon_cluster) - authorization_source = Keyword.get(opts, :authorization_source) - - children = - sites - |> Enum.map(fn site_config -> assign_authorization_source(site_config, authorization_source) end) - |> Enum.map(&site_child_spec/1) - + children = Enum.map(sites, &site_child_spec/1) Supervisor.init(children, strategy: :one_for_one) end - defp assign_authorization_source(site_config, nil), do: site_config - - defp assign_authorization_source(site_config, authorization_source) do - Keyword.put_new(site_config, :authorization_source, authorization_source) - end - defp site_child_spec(opts) do config = Config.new(opts) Supervisor.child_spec({Beacon.SiteSupervisor, config}, id: config.site) diff --git a/lib/beacon/config.ex b/lib/beacon/config.ex index 2b1f4a3df..b1a63b565 100644 --- a/lib/beacon/config.ex +++ b/lib/beacon/config.ex @@ -55,11 +55,6 @@ defmodule Beacon.Config do """ @type skip_boot? :: boolean() - # @typedoc """ - # A module that implements `Beacon.Authorization.Policy`, used to provide authorization rules. - # """ - # @type authorization_source :: module() - @typedoc """ A module that implements `Beacon.RuntimeCSS`. """ @@ -184,7 +179,6 @@ defmodule Beacon.Config do router: router(), repo: repo(), skip_boot?: skip_boot?(), - # authorization_source: authorization_source(), css_compiler: css_compiler(), tailwind_config: tailwind_config(), live_socket_path: live_socket_path(), @@ -218,8 +212,8 @@ defmodule Beacon.Config do router: nil, repo: nil, skip_boot?: false, - # TODO: rename to `authorization_policy` - authorization_source: Beacon.Authorization.DefaultPolicy, + # TODO: rename to `authorization_policy`, see https://github.com/BeaconCMS/beacon/pull/563 + # authorization_source: Beacon.Authorization.DefaultPolicy, css_compiler: Beacon.RuntimeCSS.TailwindCompiler, tailwind_config: nil, live_socket_path: "/live", @@ -248,7 +242,6 @@ defmodule Beacon.Config do | {:router, router()} | {:repo, repo()} | {:skip_boot?, skip_boot?()} - # | {:authorization_source, authorization_source()} | {:css_compiler, css_compiler()} | {:tailwind_config, tailwind_config()} | {:live_socket_path, live_socket_path()} @@ -261,11 +254,6 @@ defmodule Beacon.Config do | {:extra_asset_fields, extra_asset_fields()} | {:default_meta_tags, default_meta_tags()} - # TODO: review authz - # * `:authorization_source` - `t:authorization_source/0` (optional). Defaults to `Beacon.Authorization.DefaultPolicy`. - # authorization_source: MyApp.MySiteAuthzPolicy, - # authorization_source: MyApp.SiteAuthnPolicy, - @doc """ Build a new `%Beacon.Config{}` instance to hold the entire configuration for each site. diff --git a/lib/errors.ex b/lib/errors.ex index d3a8caf60..5ba00cde6 100644 --- a/lib/errors.ex +++ b/lib/errors.ex @@ -34,21 +34,6 @@ defmodule Beacon.RuntimeError do defexception message: "runtime error in Beacon", plug_status: 404 end -defmodule Beacon.AuthorizationError do - @moduledoc false - - # TODO: review authz - # """ - # Raised when there is an error in the `get_agent/1` or `authorized?/3` callbacks of - # a `Beacon.Authorization.Policy`. - - # If you're seeing this error, and have implemented a custom `Beacon.Authorization.Policy`, - # check the logic in your policy module. - # """ - - defexception message: "error in Beacon.Authorization" -end - defmodule Beacon.ParserError do @moduledoc """ Raised when Beacon's Markdown engine attempts to convert Markdown to HTML unsuccessfully. diff --git a/test/beacon/authorization/default_policy_test.exs b/test/beacon/authorization/default_policy_test.exs deleted file mode 100644 index 0a32536fd..000000000 --- a/test/beacon/authorization/default_policy_test.exs +++ /dev/null @@ -1,18 +0,0 @@ -defmodule Beacon.Authorization.DefaultPolicyTest do - use ExUnit.Case, async: true - - alias Beacon.Authorization.DefaultPolicy - - describe "get_agent/1" do - test "passes data through" do - assert %{session_id: "admin_session_123"} = DefaultPolicy.get_agent(%{session_id: "admin_session_123"}) - end - end - - describe "authorized?/3" do - test "returns true" do - assert DefaultPolicy.authorized?(%{role: :editor}, :upload, %{}) - assert DefaultPolicy.authorized?(%{role: :admin}, :upload, %{}) - end - end -end diff --git a/test/beacon/authorization_test.exs b/test/beacon/authorization_test.exs deleted file mode 100644 index 160c91b71..000000000 --- a/test/beacon/authorization_test.exs +++ /dev/null @@ -1,18 +0,0 @@ -defmodule Beacon.AuthorizationTest do - use ExUnit.Case, async: true - - alias Beacon.Authorization - - describe "get_agent/1" do - test "returns agent" do - assert %{role: :admin} = Authorization.get_agent(:my_site, %{"session_id" => "admin_session_123"}) - end - end - - describe "authorized?/3" do - test "returns boolean" do - refute Authorization.authorized?(:my_site, %{role: :editor}, :upload, %{}) - assert Authorization.authorized?(:my_site, %{role: :admin}, :upload, %{}) - end - end -end diff --git a/test/beacon/config_test.exs b/test/beacon/config_test.exs index c7be6bdfc..ab79b7c2a 100644 --- a/test/beacon/config_test.exs +++ b/test/beacon/config_test.exs @@ -10,7 +10,6 @@ defmodule Beacon.ConfigTest do test "returns the site config" do assert %Beacon.Config{ css_compiler: Beacon.RuntimeCSS.TailwindCompiler, - authorization_source: Beacon.BeaconTest.BeaconAuthorizationSource, live_socket_path: "/custom_live", safe_code_check: false, site: :my_site, diff --git a/test/test_helper.exs b/test/test_helper.exs index 78f9f0a9e..95cc72b86 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -152,8 +152,7 @@ Supervisor.start_link( ] ] ] - ], - authorization_source: Beacon.BeaconTest.BeaconAuthorizationSource}, + ]}, Beacon.BeaconTest.Endpoint ], strategy: :one_for_one From 71780d877279c47f0f4ed69d48592928d2a95907 Mon Sep 17 00:00:00 2001 From: Leandro Pereira Date: Thu, 25 Jul 2024 18:36:52 -0400 Subject: [PATCH 3/3] remove doc mods --- mix.exs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/mix.exs b/mix.exs index 924a02835..51a7ec983 100644 --- a/mix.exs +++ b/mix.exs @@ -174,11 +174,6 @@ defmodule Beacon.MixProject do Beacon.RuntimeCSS.TailwindCompiler, Beacon.Web.BeaconAssigns ], - "Authn and Authz": [ - Beacon.Authorization, - Beacon.Authorization.Policy, - Beacon.Authorization.DefaultPolicy - ], Extensibility: [ Beacon.Config, Beacon.Lifecycle,