Merge branch '1478-oauth-admin-scopes-tweaks' into 'develop'

[#1478] OAuth `admin` scopes tweaks

Closes #1478

See merge request pleroma/pleroma!2081

CHANGELOG.md

@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - **Breaking:** attachments are removed along with statuses when there are no other references to it
 - **Breaking:** Elixir >=1.8 is now required (was >= 1.7)
 - **Breaking:** attachment links (`config :pleroma, :instance, no_attachment_links` and `config :pleroma, Pleroma.Upload, link_name`) disabled by default
+- **Breaking:** OAuth: defaulted `[:auth, :enforce_oauth_admin_scope_usage]` setting to `true` which demands `admin` OAuth scope to perform admin actions (in addition to `is_admin` flag on User); make sure to use bundled or newer versions of AdminFE & PleromaFE to access admin / moderator features.
 - Replaced [pleroma_job_queue](https://git.pleroma.social/pleroma/pleroma_job_queue) and `Pleroma.Web.Federator.RetryQueue` with [Oban](https://github.com/sorentwo/oban) (see [`docs/config.md`](docs/config.md) on migrating customized worker / retry settings)
 - Introduced [quantum](https://github.com/quantum-elixir/quantum-core) job scheduler
 - Enabled `:instance, extended_nickname_format` in the default config

config/config.exs

@@ -561,7 +561,7 @@
 
 config :pleroma,
        :auth,
-       enforce_oauth_admin_scope_usage: false,
+       enforce_oauth_admin_scope_usage: true,
        oauth_consumer_strategies: oauth_consumer_strategies
 
 config :pleroma, Pleroma.Emails.Mailer, adapter: Swoosh.Adapters.Sendmail, enabled: false

lib/pleroma/plugs/user_is_admin_plug.ex

@@ -23,6 +23,7 @@ def call(%{assigns: %{user: %User{is_admin: true}} = assigns} = conn, _) do
       token && OAuth.Scopes.contains_admin_scopes?(token.scopes) ->
         # Note: checking for _any_ admin scope presence, not necessarily fitting requested action.
         #   Thus, controller must explicitly invoke OAuthScopesPlug to verify scope requirements.
+        #   Admin might opt out of admin scope for some apps to block any admin actions from them.
         conn
 
       true ->

lib/pleroma/user.ex

@@ -1874,22 +1874,13 @@ defp truncate_field(%{"name" => name, "value" => value}) do
   end
 
   def admin_api_update(user, params) do
-    changeset =
-      cast(user, params, [
+    user
+    |> cast(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
-        # Admin status change results in change of accessible OAuth scopes, and instead of changing
-        #   already issued tokens we revoke them, requiring user to sign in again
-        global_sign_out(user)
-      end
-
-      {:ok, updated_user}
-    end
+    |> update_and_set_cache()
   end
 
   @doc "Signs user out of all applications"

lib/pleroma/web/admin_api/admin_api_controller.ex

@@ -32,19 +32,14 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
   plug(
     OAuthScopesPlug,
     %{scopes: ["read:accounts"], admin: true}
-    when action in [:list_users, :user_show, :right_get, :invites]
+    when action in [:list_users, :user_show, :right_get]
   )
 
   plug(
     OAuthScopesPlug,
     %{scopes: ["write:accounts"], admin: true}
     when action in [
-           :get_invite_token,
-           :revoke_invite,
-           :email_invite,
            :get_password_reset,
-           :user_follow,
-           :user_unfollow,
            :user_delete,
            :users_create,
            :user_toggle_activation,
@@ -57,6 +52,20 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
          ]
   )
 
+  plug(OAuthScopesPlug, %{scopes: ["read:invites"], admin: true} when action == :invites)
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["write:invites"], admin: true}
+    when action in [:create_invite_token, :revoke_invite, :email_invite]
+  )
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["write:follows"], admin: true}
+    when action in [:user_follow, :user_unfollow, :relay_follow, :relay_unfollow]
+  )
+
   plug(
     OAuthScopesPlug,
     %{scopes: ["read:reports"], admin: true}
@@ -90,7 +99,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
   plug(
     OAuthScopesPlug,
     %{scopes: ["write"], admin: true}
-    when action in [:relay_follow, :relay_unfollow, :config_update]
+    when action == :config_update
   )
 
   @users_page_size 50

lib/pleroma/web/oauth/oauth_controller.ex

@@ -222,7 +222,7 @@ def token_exchange(
          {:user_active, true} <- {:user_active, !user.deactivated},
          {:password_reset_pending, false} <-
            {:password_reset_pending, user.password_reset_pending},
-         {:ok, scopes} <- validate_scopes(app, params, user),
+         {:ok, scopes} <- validate_scopes(app, params),
          {:ok, auth} <- Authorization.create_authorization(app, user, scopes),
          {:ok, token} <- Token.exchange_token(app, auth) do
       json(conn, Token.Response.build(user, token))
@@ -471,7 +471,7 @@ defp do_create_authorization(
            {:get_user, (user && {:ok, user}) || Authenticator.get_user(conn)},
          %App{} = app <- Repo.get_by(App, client_id: client_id),
          true <- redirect_uri in String.split(app.redirect_uris),
-         {:ok, scopes} <- validate_scopes(app, auth_attrs, user),
+         {:ok, scopes} <- validate_scopes(app, auth_attrs),
          {:auth_active, true} <- {:auth_active, User.auth_active?(user)} do
       Authorization.create_authorization(app, user, scopes)
     end
@@ -487,12 +487,12 @@ defp get_session_registration_id(%Plug.Conn{} = conn), do: get_session(conn, :re
   defp put_session_registration_id(%Plug.Conn{} = conn, registration_id),
     do: put_session(conn, :registration_id, registration_id)
 
-  @spec validate_scopes(App.t(), map(), User.t()) ::
+  @spec validate_scopes(App.t(), map()) ::
           {:ok, list()} | {:error, :missing_scopes | :unsupported_scopes}
-  defp validate_scopes(%App{} = app, params, %User{} = user) do
+  defp validate_scopes(%App{} = app, params) do
     params
     |> Scopes.fetch_scopes(app.scopes)
-    |> Scopes.validate(app.scopes, user)
+    |> Scopes.validate(app.scopes)
   end
 
   def default_redirect_uri(%App{} = app) do

lib/pleroma/web/oauth/scopes.ex

@@ -8,7 +8,6 @@ defmodule Pleroma.Web.OAuth.Scopes do
   """
 
   alias Pleroma.Plugs.OAuthScopesPlug
-  alias Pleroma.User
 
   @doc """
   Fetch scopes from request params.
@@ -56,35 +55,18 @@ def to_string(scopes), do: Enum.join(scopes, " ")
   @doc """
   Validates scopes.
   """
-  @spec validate(list() | nil, list(), User.t()) ::
+  @spec validate(list() | nil, list()) ::
           {:ok, list()} | {:error, :missing_scopes | :unsupported_scopes}
-  def validate(blank_scopes, _app_scopes, _user) when blank_scopes in [nil, []],
+  def validate(blank_scopes, _app_scopes) when blank_scopes in [nil, []],
     do: {:error, :missing_scopes}
 
-  def validate(scopes, app_scopes, %User{} = user) do
-    with {:ok, _} <- ensure_scopes_support(scopes, app_scopes),
-         {:ok, scopes} <- authorize_admin_scopes(scopes, app_scopes, user) do
-      {:ok, scopes}
-    end
-  end
-
-  defp ensure_scopes_support(scopes, app_scopes) do
+  def validate(scopes, app_scopes) do
     case OAuthScopesPlug.filter_descendants(scopes, app_scopes) do
       ^scopes -> {:ok, scopes}
       _ -> {:error, :unsupported_scopes}
     end
   end
 
-  defp authorize_admin_scopes(scopes, app_scopes, %User{} = user) do
-    if user.is_admin || !contains_admin_scopes?(scopes) || !contains_admin_scopes?(app_scopes) do
-      {:ok, scopes}
-    else
-      # Gracefully dropping admin scopes from requested scopes if user isn't an admin (not raising)
-      scopes = scopes -- OAuthScopesPlug.filter_descendants(scopes, ["admin"])
-      validate(scopes, app_scopes, user)
-    end
-  end
-
   def contains_admin_scopes?(scopes) do
     scopes
     |> OAuthScopesPlug.filter_descendants(["admin"])

priv/repo/migrations/20191220174645_add_scopes_to_pleroma_feo_auth_records.exs

@@ -0,0 +1,17 @@
+defmodule Pleroma.Repo.Migrations.AddScopesToPleromaFEOAuthRecords do
+  use Ecto.Migration
+
+  def up do
+    update_scopes_clause = "SET scopes = '{read,write,follow,push,admin}'"
+    apps_where = "WHERE apps.client_name like 'PleromaFE_%' or apps.client_name like 'AdminFE_%'"
+    app_id_subquery_where = "WHERE app_id IN (SELECT apps.id FROM apps #{apps_where})"
+
+    execute("UPDATE apps #{update_scopes_clause} #{apps_where}")
+
+    for table <- ["oauth_authorizations", "oauth_tokens"] do
+      execute("UPDATE #{table} #{update_scopes_clause} #{app_id_subquery_where}")
+    end
+  end
+
+  def down, do: :noop
+end

priv/static/index.html

@@ -1 +1 @@
-<!DOCTYPE html><html lang=en><head><meta charset=utf-8><meta name=viewport content="width=device-width,initial-scale=1,user-scalable=no"><title>Pleroma</title><!--server-generated-meta--><link rel=icon type=image/png href=/favicon.png><link href=/static/css/vendors~app.b2603a50868c68a1c192.css rel=stylesheet><link href=/static/css/app.ae04505b31bb0ee2765e.css rel=stylesheet><link href=/static/fontello.1576166651574.css rel=stylesheet></head><body class=hidden><noscript>To use Pleroma, please enable JavaScript.</noscript><div id=app></div><script type=text/javascript src=/static/js/vendors~app.3f1ed7a4fdfc37ee27a7.js></script><script type=text/javascript src=/static/js/app.a9b3f4c3e79baf3fa8b7.js></script></body></html>
+<!DOCTYPE html><html lang=en><head><meta charset=utf-8><meta name=viewport content="width=device-width,initial-scale=1,user-scalable=no"><title>Pleroma</title><!--server-generated-meta--><link rel=icon type=image/png href=/favicon.png><link href=/static/css/vendors~app.b2603a50868c68a1c192.css rel=stylesheet><link href=/static/css/app.ae04505b31bb0ee2765e.css rel=stylesheet><link href=/static/fontello.1579102213354.css rel=stylesheet></head><body class=hidden><noscript>To use Pleroma, please enable JavaScript.</noscript><div id=app></div><script type=text/javascript src=/static/js/vendors~app.86bc6d5e06d2e17976c5.js></script><script type=text/javascript src=/static/js/app.a43640742dacfb13b6b0.js></script></body></html>

priv/static/static/font/fontello.1579102213354.svg

@@ -1,7 +1,7 @@
 <?xml version="1.0" standalone="no"?>
 <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
 <svg xmlns="http://www.w3.org/2000/svg">
-<metadata>Copyright (C) 2019 by original authors @ fontello.com</metadata>
+<metadata>Copyright (C) 2020 by original authors @ fontello.com</metadata>
 <defs>
 <font id="fontello" horiz-adv-x="1000" >
 <font-face font-family="fontello" font-weight="400" font-stretch="normal" units-per-em="1000" ascent="857" descent="-143" />
@@ -64,6 +64,18 @@
 
 <glyph glyph-name="zoom-in" unicode="&#xe81c;" d="M571 411v-36q0-7-5-13t-12-5h-125v-125q0-7-6-13t-12-5h-36q-7 0-13 5t-5 13v125h-125q-7 0-12 5t-6 13v36q0 7 6 12t12 5h125v125q0 8 5 13t13 5h36q7 0 12-5t6-13v-125h125q7 0 12-5t5-12z m72-18q0 103-73 176t-177 74-177-74-73-176 73-177 177-73 177 73 73 177z m286-465q0-29-21-50t-51-21q-30 0-50 21l-191 191q-100-69-223-69-80 0-153 31t-125 84-84 125-31 153 31 152 84 126 125 84 153 31 153-31 125-84 84-126 31-152q0-123-69-223l191-191q21-21 21-51z" horiz-adv-x="928.6" />
 
+<glyph glyph-name="users" unicode="&#xe81d;" d="M331 357q-90-3-148-71h-75q-45 0-77 22t-31 66q0 197 69 197 4 0 25-11t54-24 66-12q38 0 75 13-3-21-3-37 0-78 45-143z m598-355q0-67-41-106t-108-39h-488q-68 0-108 39t-41 106q0 29 2 57t8 61 14 61 24 54 35 45 48 30 62 11q6 0 24-12t41-26 59-27 76-12 75 12 60 27 41 26 24 12q34 0 62-11t47-30 35-45 24-54 15-61 8-61 2-57z m-572 712q0-59-42-101t-101-42-101 42-42 101 42 101 101 42 101-42 42-101z m393-214q0-89-63-152t-151-62-152 62-63 152 63 151 152 63 151-63 63-151z m321-126q0-43-31-66t-77-22h-75q-57 68-147 71 45 65 45 143 0 16-3 37 37-13 74-13 33 0 67 12t54 24 24 11q69 0 69-197z m-71 340q0-59-42-101t-101-42-101 42-42 101 42 101 101 42 101-42 42-101z" horiz-adv-x="1071.4" />
+
+<glyph glyph-name="chat" unicode="&#xe81e;" d="M786 428q0-77-53-143t-143-104-197-38q-48 0-98 9-70-49-155-72-21-5-48-9h-2q-6 0-12 5t-6 12q-1 1-1 3t1 4 1 3l1 3t2 3 2 3 3 3 2 2q3 3 13 14t15 16 12 17 14 21 11 25q-69 40-108 98t-40 125q0 78 53 144t143 104 197 38 197-38 143-104 53-144z m214-142q0-67-40-126t-108-98q5-14 11-25t14-21 13-16 14-17 13-14q0 0 2-2t3-3 2-3 2-3l1-3t1-3 1-4-1-3q-2-8-7-13t-12-4q-28 4-48 9-86 23-156 72-50-9-98-9-151 0-263 74 32-3 49-3 90 0 172 25t148 72q69 52 107 119t37 141q0 43-13 85 72-39 114-99t42-128z" horiz-adv-x="1000" />
+
+<glyph glyph-name="info-circled" unicode="&#xe81f;" d="M571 89v89q0 8-5 13t-12 5h-54v286q0 8-5 13t-13 5h-178q-8 0-13-5t-5-13v-89q0-8 5-13t13-5h53v-179h-53q-8 0-13-5t-5-13v-89q0-8 5-13t13-5h250q7 0 12 5t5 13z m-71 500v89q0 8-5 13t-13 5h-107q-8 0-13-5t-5-13v-89q0-8 5-13t13-5h107q8 0 13 5t5 13z m357-232q0-117-57-215t-156-156-215-58-216 58-155 156-58 215 58 215 155 156 216 58 215-58 156-156 57-215z" horiz-adv-x="857.1" />
+
+<glyph glyph-name="login" unicode="&#xe820;" d="M661 357q0-14-11-25l-303-304q-11-10-26-10t-25 10-10 25v161h-250q-15 0-25 11t-11 25v214q0 15 11 25t25 11h250v161q0 14 10 25t25 10 26-10l303-304q11-10 11-25z m196 196v-392q0-67-47-114t-114-47h-178q-7 0-13 5t-5 13q0 2-1 11t0 15 2 13 5 11 12 3h178q37 0 64 27t26 63v392q0 37-26 64t-64 26h-174t-6 0-6 2-5 3-4 5-1 8q0 2-1 11t0 15 2 13 5 11 12 3h178q67 0 114-47t47-114z" horiz-adv-x="857.1" />
+
+<glyph glyph-name="home-2" unicode="&#xe821;" d="M521 826q322-279 500-429 20-16 20-40 0-21-15-37t-36-15l-105 0 0-364q0-21-15-37t-36-16l-156 0q-22 0-37 16t-16 37l0 208-209 0 0-208q0-21-15-37t-36-16l-156 0q-21 0-37 16t-16 37l0 364-103 0q-22 0-37 15t-16 37 19 40z" horiz-adv-x="1041" />
+
+<glyph glyph-name="arrow-curved" unicode="&#xe822;" d="M799 302l0-56 112 0-223-223-224 223 112 0 0 56q0 116-81 197t-197 82-198-82-82-197q0 162 115 276t276 114 276-114 114-276z" horiz-adv-x="928" />
+
 <glyph glyph-name="spin3" unicode="&#xe832;" d="M494 857c-266 0-483-210-494-472-1-19 13-20 13-20l84 0c16 0 19 10 19 18 10 199 176 358 378 358 107 0 205-45 273-118l-58-57c-11-12-11-27 5-31l247-50c21-5 46 11 37 44l-58 227c-2 9-16 22-29 13l-65-60c-89 91-214 148-352 148z m409-508c-16 0-19-10-19-18-10-199-176-358-377-358-108 0-205 45-274 118l59 57c10 12 10 27-5 31l-248 50c-21 5-46-11-37-44l58-227c2-9 16-22 30-13l64 60c89-91 214-148 353-148 265 0 482 210 493 473 1 18-13 19-13 19l-84 0z" horiz-adv-x="1000" />
 
 <glyph glyph-name="spin4" unicode="&#xe834;" d="M498 857c-114 0-228-39-320-116l0 0c173 140 428 130 588-31 134-134 164-332 89-495-10-29-5-50 12-68 21-20 61-23 84 0 3 3 12 15 15 24 71 180 33 393-112 539-99 98-228 147-356 147z m-409-274c-14 0-29-5-39-16-3-3-13-15-15-24-71-180-34-393 112-539 185-185 479-195 676-31l0 0c-173-140-428-130-589 31-134 134-163 333-89 495 11 29 6 50-12 68-11 11-27 17-44 16z" horiz-adv-x="1001" />

priv/static/static/fontello.json

@@ -303,6 +303,42 @@
       "css": "gauge",
       "code": 61668,
       "src": "fontawesome"
+    },
+    {
+      "uid": "31972e4e9d080eaa796290349ae6c1fd",
+      "css": "users",
+      "code": 59421,
+      "src": "fontawesome"
+    },
+    {
+      "uid": "e82cedfa1d5f15b00c5a81c9bd731ea2",
+      "css": "info-circled",
+      "code": 59423,
+      "src": "fontawesome"
+    },
+    {
+      "uid": "w3nzesrlbezu6f30q7ytyq919p6gdlb6",
+      "css": "home-2",
+      "code": 59425,
+      "src": "typicons"
+    },
+    {
+      "uid": "dcedf50ab1ede3283d7a6c70e2fe32f3",
+      "css": "chat",
+      "code": 59422,
+      "src": "fontawesome"
+    },
+    {
+      "uid": "3a00327e61b997b58518bd43ed83c3df",
+      "css": "login",
+      "code": 59424,
+      "src": "fontawesome"
+    },
+    {
+      "uid": "f3ebd6751c15a280af5cc5f4a764187d",
+      "css": "arrow-curved",
+      "code": 59426,
+      "src": "iconic"
     }
   ]
 }

priv/static/static/styles.json

@@ -1,6 +1,7 @@
 {
   "pleroma-dark": [ "Pleroma Dark", "#121a24", "#182230", "#b9b9ba", "#d8a070", "#d31014", "#0fa00f", "#0095ff", "#ffa500" ],
   "pleroma-light": [ "Pleroma Light", "#f2f4f6", "#dbe0e8", "#304055", "#f86f0f", "#d31014", "#0fa00f", "#0095ff", "#ffa500" ],
+  "pleroma-amoled": [ "Pleroma Dark AMOLED", "#000000", "#111111", "#b0b0b1", "#d8a070", "#aa0000", "#0fa00f", "#0095ff", "#d59500"],
   "classic-dark": [ "Classic Dark", "#161c20", "#282e32", "#b9b9b9", "#baaa9c", "#d31014", "#0fa00f", "#0095ff", "#ffa500" ],
   "bird": [ "Bird", "#f8fafd", "#e6ecf0", "#14171a", "#0084b8", "#e0245e", "#17bf63", "#1b95e0", "#fab81e"],
   "ir-black": [ "Ir Black", "#000000", "#242422", "#b5b3aa", "#ff6c60", "#FF6C60", "#A8FF60", "#96CBFE", "#FFFFB6" ],

test/web/oauth/oauth_controller_test.exs

@@ -568,29 +568,34 @@ test "with existing authentication and OOB `redirect_uri`, redirects to app with
 
   describe "POST /oauth/authorize" do
     test "redirects with oauth authorization, " <>
-           "keeping only non-admin scopes for non-admin user" do
-      app = insert(:oauth_app, scopes: ["read", "write", "admin"])
+           "granting requested app-supported scopes to both admin- and non-admin users" do
+      app_scopes = ["read", "write", "admin", "secret_scope"]
+      app = insert(:oauth_app, scopes: app_scopes)
       redirect_uri = OAuthController.default_redirect_uri(app)
 
       non_admin = insert(:user, is_admin: false)
       admin = insert(:user, is_admin: true)
+      scopes_subset = ["read:subscope", "write", "admin"]
 
-      for {user, expected_scopes} <- %{
-            non_admin => ["read:subscope", "write"],
-            admin => ["read:subscope", "write", "admin"]
-          } do
+      # In case scope param is missing, expecting _all_ app-supported scopes to be granted
+      for user <- [non_admin, admin],
+          {requested_scopes, expected_scopes} <-
+            %{scopes_subset => scopes_subset, nil => app_scopes} do
         conn =
-          build_conn()
-          |> post("/oauth/authorize", %{
+          post(
+            build_conn(),
+            "/oauth/authorize",
+            %{
               "authorization" => %{
                 "name" => user.nickname,
                 "password" => "test",
                 "client_id" => app.client_id,
                 "redirect_uri" => redirect_uri,
-              "scope" => "read:subscope write admin",
+                "scope" => requested_scopes,
                 "state" => "statepassed"
               }
-          })
+            }
+          )
 
         target = redirected_to(conn)
         assert target =~ redirect_uri
@@ -631,13 +636,11 @@ test "returns 401 for wrong credentials", %{conn: conn} do
       assert result =~ "Invalid Username/Password"
     end
 
-    test "returns 401 for missing scopes " <>
-           "(including all admin-only scopes for non-admin user)" do
+    test "returns 401 for missing scopes" do
       user = insert(:user, is_admin: false)
       app = insert(:oauth_app, scopes: ["read", "write", "admin"])
       redirect_uri = OAuthController.default_redirect_uri(app)
 
-      for scope_param <- ["", "admin:read admin:write"] do
       result =
         build_conn()
         |> post("/oauth/authorize", %{
@@ -647,7 +650,7 @@ test "returns 401 for missing scopes " <>
             "client_id" => app.client_id,
             "redirect_uri" => redirect_uri,
             "state" => "statepassed",
-              "scope" => scope_param
+            "scope" => ""
           }
         })
         |> html_response(:unauthorized)
@@ -659,7 +662,6 @@ test "returns 401 for missing scopes " <>
       # Error message
       assert result =~ "This action is outside the authorized scopes"
     end
-    end
 
     test "returns 401 for scopes beyond app scopes hierarchy", %{conn: conn} do
       user = insert(:user)

test/web/pleroma_api/controllers/emoji_api_controller_test.exs

@@ -14,6 +14,10 @@ defmodule Pleroma.Web.PleromaAPI.EmojiAPIControllerTest do
                     "emoji"
                   )
 
+  clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
+    Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], false)
+  end
+
   test "shared & non-shared pack information in list_packs is ok" do
     conn = build_conn()
     resp = conn |> get(emoji_api_path(conn, :list_packs)) |> json_response(200)