From af42c00cfffb2cd8e93857cd1cf2901113c45bd2 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Fri, 6 Dec 2019 00:25:44 +0300 Subject: [PATCH] [#1427] Reworked admin scopes support. Requalified users.is_admin flag as legacy accessor to admin actions in case token lacks admin scope(s). --- CHANGELOG.md | 1 + config/config.exs | 5 +++- config/description.exs | 9 ++++++ lib/mix/tasks/pleroma/user.ex | 9 ++---- lib/pleroma/config.ex | 7 +++++ lib/pleroma/plugs/user_is_admin_plug.ex | 28 ++++++++++++++++--- lib/pleroma/user.ex | 28 ++++++++++++++----- .../web/admin_api/admin_api_controller.ex | 18 ++++++------ lib/pleroma/web/oauth/scopes.ex | 2 +- .../controllers/emoji_api_controller.ex | 2 +- .../admin_api/admin_api_controller_test.exs | 3 +- 11 files changed, 82 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bd835a3d..dac61c174 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mix task to re-count statuses for all users (`mix pleroma.count_statuses`) - Support for `X-Forwarded-For` and similar HTTP headers which used by reverse proxies to pass a real user IP address to the backend. Must not be enabled unless your instance is behind at least one reverse proxy (such as Nginx, Apache HTTPD or Varnish Cache). - MRF: New module which handles incoming posts based on their age. By default, all incoming posts that are older than 2 days will be unlisted and not shown to their followers. +- OAuth: admin scopes support (relevant setting: `[:auth, :enforce_oauth_admin_scope_usage]`).
API Changes diff --git a/config/config.exs b/config/config.exs index bf2b3f6e2..64397484e 100644 --- a/config/config.exs +++ b/config/config.exs @@ -560,7 +560,10 @@ config :ueberauth, base_path: "/oauth", providers: ueberauth_providers -config :pleroma, :auth, oauth_consumer_strategies: oauth_consumer_strategies +config :pleroma, + :auth, + enforce_oauth_admin_scope_usage: false, + oauth_consumer_strategies: oauth_consumer_strategies config :pleroma, Pleroma.Emails.Mailer, adapter: Swoosh.Adapters.Sendmail, enabled: false diff --git a/config/description.exs b/config/description.exs index 70e963399..45e4b43f1 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2094,6 +2094,15 @@ config :pleroma, :config_description, [ type: :group, description: "Authentication / authorization settings", children: [ + %{ + key: :enforce_oauth_admin_scope_usage, + type: :boolean, + description: + "OAuth admin scope requirement toggle. " <> + "If `true`, admin actions explicitly demand admin OAuth scope(s) presence in OAuth token " <> + "(client app must support admin scopes). If `false` and token doesn't have admin scope(s)," <> + "`is_admin` user flag grants access to admin-specific actions." + }, %{ key: :auth_template, type: :string, diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex index 4e3b80db3..8c4739221 100644 --- a/lib/mix/tasks/pleroma/user.ex +++ b/lib/mix/tasks/pleroma/user.ex @@ -8,7 +8,6 @@ defmodule Mix.Tasks.Pleroma.User do alias Ecto.Changeset alias Pleroma.User alias Pleroma.UserInviteToken - alias Pleroma.Web.OAuth @shortdoc "Manages Pleroma users" @moduledoc File.read!("docs/administration/CLI_tasks/user.md") @@ -354,8 +353,7 @@ defmodule Mix.Tasks.Pleroma.User do start_pleroma() with %User{local: true} = user <- User.get_cached_by_nickname(nickname) do - OAuth.Token.delete_user_tokens(user) - OAuth.Authorization.delete_user_authorizations(user) + User.global_sign_out(user) shell_info("#{nickname} signed out from all apps.") else @@ -375,10 +373,7 @@ defmodule Mix.Tasks.Pleroma.User do end defp set_admin(user, value) do - {:ok, user} = - user - |> Changeset.change(%{is_admin: value}) - |> User.update_and_set_cache() + {:ok, user} = User.admin_api_update(user, %{is_admin: value}) shell_info("Admin status of #{user.nickname}: #{user.is_admin}") user diff --git a/lib/pleroma/config.ex b/lib/pleroma/config.ex index fcc039710..cadab2f15 100644 --- a/lib/pleroma/config.ex +++ b/lib/pleroma/config.ex @@ -65,4 +65,11 @@ defmodule Pleroma.Config do def oauth_consumer_strategies, do: get([:auth, :oauth_consumer_strategies], []) def oauth_consumer_enabled?, do: oauth_consumer_strategies() != [] + + def enforce_oauth_admin_scope_usage?, do: !!get([:auth, :enforce_oauth_admin_scope_usage]) + + def oauth_admin_scopes(scope) do + ["admin:#{scope}"] ++ + if enforce_oauth_admin_scope_usage?(), do: [], else: [scope] + end end diff --git a/lib/pleroma/plugs/user_is_admin_plug.ex b/lib/pleroma/plugs/user_is_admin_plug.ex index ee808f31f..8814556f1 100644 --- a/lib/pleroma/plugs/user_is_admin_plug.ex +++ b/lib/pleroma/plugs/user_is_admin_plug.ex @@ -5,19 +5,39 @@ defmodule Pleroma.Plugs.UserIsAdminPlug do import Pleroma.Web.TranslationHelpers import Plug.Conn + alias Pleroma.User + alias Pleroma.Web.OAuth def init(options) do options end - def call(%{assigns: %{user: %User{is_admin: true}}} = conn, _) do - conn + def call(%Plug.Conn{assigns: %{token: %OAuth.Token{scopes: oauth_scopes} = _token}} = conn, _) do + if OAuth.Scopes.contains_admin_scopes?(oauth_scopes) do + # Note: checking for _any_ admin scope presence, not necessarily fitting requested action. + # Thus, controller must explicitly invoke OAuthScopesPlug to verify scope requirements. + conn + else + fail(conn) + end + end + + unless Pleroma.Config.enforce_oauth_admin_scope_usage?() do + # To do: once AdminFE makes use of "admin" scope, disable the following func definition + # (fail on no admin scope(s) in token even if `is_admin` is true) + def call(%Plug.Conn{assigns: %{user: %User{is_admin: true}}} = conn, _) do + conn + end end def call(conn, _) do + fail(conn) + end + + defp fail(conn) do conn - |> render_error(:forbidden, "User is not admin.") - |> halt + |> render_error(:forbidden, "User is not an admin or OAuth admin scope is not granted.") + |> halt() end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index fcb1d5143..d05dccd3d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1732,13 +1732,27 @@ defmodule Pleroma.User do end def admin_api_update(user, params) do - user - |> cast(params, [ - :is_moderator, - :is_admin, - :show_role - ]) - |> update_and_set_cache() + changeset = + cast(user, params, [ + :is_moderator, + :is_admin, + :show_role + ]) + + with {:ok, updated_user} <- update_and_set_cache(changeset) do + if user.is_admin && !updated_user.is_admin do + # Tokens & authorizations containing any admin scopes must be revoked (revoking all) + global_sign_out(user) + end + + {:ok, updated_user} + end + end + + @doc "Signs user out of all applications" + def global_sign_out(user) do + OAuth.Authorization.delete_user_authorizations(user) + OAuth.Token.delete_user_tokens(user) end def mascot_update(user, url) do diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 01cd12c96..f9ace00d7 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -30,13 +30,13 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do plug( OAuthScopesPlug, - %{scopes: ["admin:read:accounts", "read:accounts"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("read:accounts")} when action in [:list_users, :user_show, :right_get, :invites] ) plug( OAuthScopesPlug, - %{scopes: ["admin:write:accounts", "write:accounts"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("write:accounts")} when action in [ :get_invite_token, :revoke_invite, @@ -58,35 +58,37 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do plug( OAuthScopesPlug, - %{scopes: ["admin:read:reports", "read:reports"]} when action in [:list_reports, :report_show] + %{scopes: Pleroma.Config.oauth_admin_scopes("read:reports")} + when action in [:list_reports, :report_show] ) plug( OAuthScopesPlug, - %{scopes: ["admin:reports", "write:reports"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("write:reports")} when action in [:report_update_state, :report_respond] ) plug( OAuthScopesPlug, - %{scopes: ["admin:read:statuses", "read:statuses"]} when action == :list_user_statuses + %{scopes: Pleroma.Config.oauth_admin_scopes("read:statuses")} + when action == :list_user_statuses ) plug( OAuthScopesPlug, - %{scopes: ["admin:write:statuses", "write:statuses"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("write:statuses")} when action in [:status_update, :status_delete] ) plug( OAuthScopesPlug, - %{scopes: ["admin:read", "read"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("read")} when action in [:config_show, :migrate_to_db, :migrate_from_db, :list_log] ) plug( OAuthScopesPlug, - %{scopes: ["admin:write", "write"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("write")} when action in [:relay_follow, :relay_unfollow, :config_update] ) diff --git a/lib/pleroma/web/oauth/scopes.ex b/lib/pleroma/web/oauth/scopes.ex index 0c8796ecb..5e04652c2 100644 --- a/lib/pleroma/web/oauth/scopes.ex +++ b/lib/pleroma/web/oauth/scopes.ex @@ -83,7 +83,7 @@ defmodule Pleroma.Web.OAuth.Scopes do end end - defp contains_admin_scopes?(scopes) do + def contains_admin_scopes?(scopes) do scopes |> OAuthScopesPlug.filter_descendants(["admin"]) |> Enum.any?() diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex index 1e8c62e3a..6f286032e 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex @@ -7,7 +7,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiAPIController do plug( OAuthScopesPlug, - %{scopes: ["admin:write", "write"]} + %{scopes: Pleroma.Config.oauth_admin_scopes("write")} when action in [ :create, :delete, diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 3a4c4d65c..fd179e8c2 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -1537,7 +1537,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do |> assign(:user, user) |> get("/api/pleroma/admin/reports") - assert json_response(conn, :forbidden) == %{"error" => "User is not admin."} + assert json_response(conn, :forbidden) == + %{"error" => "User is not an admin or OAuth admin scope is not granted."} end test "returns 403 when requested by anonymous" do