Fixed OAuth restrictions for :api routes. Made auth info dropped for :api routes...
authorIvan Tashkinov <ivantashkinov@gmail.com>
Wed, 22 Apr 2020 15:50:25 +0000 (18:50 +0300)
committerIvan Tashkinov <ivantashkinov@gmail.com>
Wed, 22 Apr 2020 15:50:25 +0000 (18:50 +0300)
14 files changed:
docs/dev.md
lib/pleroma/plugs/oauth_scopes_plug.ex
lib/pleroma/web/mastodon_api/controllers/account_controller.ex
lib/pleroma/web/mastodon_api/controllers/app_controller.ex
lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex
lib/pleroma/web/mastodon_api/controllers/instance_controller.ex
lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
lib/pleroma/web/mastodon_api/controllers/search_controller.ex
lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex
lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
lib/pleroma/web/pleroma_api/controllers/scrobble_controller.ex
lib/pleroma/web/router.ex
lib/pleroma/web/twitter_api/twitter_api_controller.ex
lib/pleroma/web/web.ex

index 0ecf43a9e4e6870cc2fd6f1460f2dfd8266687e9..f1b4cbf8bff8b47bd7a5f7e6bd540fe33aa9558d 100644 (file)
@@ -4,29 +4,19 @@ This document contains notes and guidelines for Pleroma developers.
 
 ## OAuth token-based authentication & authorization
 
-* Pleroma supports hierarchical OAuth scopes, just like Mastodon but with added granularity of admin scopes.
-  For a reference, see [Mastodon OAuth scopes](https://docs.joinmastodon.org/api/oauth-scopes/).
-
-* It is important to either define OAuth scope restrictions or explicitly mark OAuth scope check as skipped, for every 
-    controller action. To define scopes, call `plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: [...]})`. To explicitly set 
-    OAuth scopes check skipped, call `plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug <when ...>)`.
-
-* In controllers, `use Pleroma.Web, :controller` will result in `action/2` (see `Pleroma.Web.controller/0` for definition)
-    be called prior to actual controller action, and it'll perform security / privacy checks before passing control to
-    actual controller action. For routes with `:authenticated_api` pipeline, authentication & authorization are expected,
-    thus `OAuthScopesPlug` will be run unless explicitly skipped (also `EnsureAuthenticatedPlug` will be executed
-    immediately before action even if there was an early run to give an early error, since `OAuthScopesPlug` supports
-    `:proceed_unauthenticated` option, and other plugs may support similar options as well). For `:api` pipeline routes,
-    `EnsurePublicOrAuthenticatedPlug` will be called to ensure that the instance is not private or user is authenticated
-    (unless explicitly skipped). Such automated checks help to prevent human errors and result in higher security / privacy
-    for users.
+* Pleroma supports hierarchical OAuth scopes, just like Mastodon but with added granularity of admin scopes. For a reference, see [Mastodon OAuth scopes](https://docs.joinmastodon.org/api/oauth-scopes/).
+
+* It is important to either define OAuth scope restrictions or explicitly mark OAuth scope check as skipped, for every controller action. To define scopes, call `plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: [...]})`. To explicitly set OAuth scopes check skipped, call `plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug <when ...>)`.
+
+* In controllers, `use Pleroma.Web, :controller` will result in `action/2` (see `Pleroma.Web.controller/0` for definition) be called prior to actual controller action, and it'll perform security / privacy checks before passing control to actual controller action.
+
+  For routes with `:authenticated_api` pipeline, authentication & authorization are expected, thus `OAuthScopesPlug` will be run unless explicitly skipped (also `EnsureAuthenticatedPlug` will be executed immediately before action even if there was an early run to give an early error, since `OAuthScopesPlug` supports `:proceed_unauthenticated` option, and other plugs may support similar options as well).
+
+  For `:api` pipeline routes, it'll be verified whether `OAuthScopesPlug` was called or explicitly skipped, and if it was not then auth information will be dropped for request. Then `EnsurePublicOrAuthenticatedPlug` will be called to ensure that either the instance is not private or user is authenticated (unless explicitly skipped). Such automated checks help to prevent human errors and result in higher security / privacy for users.
 
 ## [HTTP Basic Authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization)
 
-* With HTTP Basic Auth, OAuth scopes check is _not_ performed for any action (since password is provided during the auth,
-    requester is able to obtain a token with full permissions anyways). `Pleroma.Plugs.AuthenticationPlug` and
-    `Pleroma.Plugs.LegacyAuthenticationPlug` both call `Pleroma.Plugs.OAuthScopesPlug.skip_plug(conn)` when password
-    is provided.
+* With HTTP Basic Auth, OAuth scopes check is _not_ performed for any action (since password is provided during the auth, requester is able to obtain a token with full permissions anyways). `Pleroma.Plugs.AuthenticationPlug` and `Pleroma.Plugs.LegacyAuthenticationPlug` both call `Pleroma.Plugs.OAuthScopesPlug.skip_plug(conn)` when password is provided.
 
 ## Auth-related configuration, OAuth consumer mode etc.
 
index a61582566ce6dfa6e7ae34ecc5a41f0ba4d02a78..efc25b79ff7e6c8afaa6175e79eb079d1f8a9a88 100644 (file)
@@ -28,9 +28,7 @@ defmodule Pleroma.Plugs.OAuthScopesPlug do
         conn
 
       options[:fallback] == :proceed_unauthenticated ->
-        conn
-        |> assign(:user, nil)
-        |> assign(:token, nil)
+        drop_auth_info(conn)
 
       true ->
         missing_scopes = scopes -- matched_scopes
@@ -46,6 +44,15 @@ defmodule Pleroma.Plugs.OAuthScopesPlug do
     end
   end
 
+  @doc "Drops authentication info from connection"
+  def drop_auth_info(conn) do
+    # To simplify debugging, setting a private variable on `conn` if auth info is dropped
+    conn
+    |> put_private(:authentication_ignored, true)
+    |> assign(:user, nil)
+    |> assign(:token, nil)
+  end
+
   @doc "Filters descendants of supported scopes"
   def filter_descendants(scopes, supported_scopes) do
     Enum.filter(
index 9b8cc0d0dc18a0474e5c879a10905b08541b8274..e501b35559e642affffebc95bd95d471f2e6648a 100644 (file)
@@ -37,7 +37,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   plug(
     OAuthScopesPlug,
     %{fallback: :proceed_unauthenticated, scopes: ["read:accounts"]}
-    when action in [:show, :endorsements]
+    when action in [:show, :followers, :following, :endorsements]
   )
 
   plug(
@@ -49,7 +49,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   plug(
     OAuthScopesPlug,
     %{scopes: ["read:accounts"]}
-    when action in [:endorsements, :verify_credentials, :followers, :following]
+    when action in [:endorsements, :verify_credentials]
   )
 
   plug(OAuthScopesPlug, %{scopes: ["write:accounts"]} when action == :update_credentials)
index 005c604447e3999cf756ec2dd1b842930d46df5e..408e1147494eaa383c04d994e46f8740edea8ae2 100644 (file)
@@ -5,6 +5,7 @@
 defmodule Pleroma.Web.MastodonAPI.AppController do
   use Pleroma.Web, :controller
 
+  alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
   alias Pleroma.Plugs.OAuthScopesPlug
   alias Pleroma.Repo
   alias Pleroma.Web.OAuth.App
@@ -13,7 +14,14 @@ defmodule Pleroma.Web.MastodonAPI.AppController do
 
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
 
+  plug(
+    :skip_plug,
+    [OAuthScopesPlug, EnsurePublicOrAuthenticatedPlug]
+    when action == :create
+  )
+
   plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :verify_credentials)
+
   plug(OpenApiSpex.Plug.CastAndValidate)
 
   @local_mastodon_name "Mastodon-Local"
index 3bfebef8b3c94f6d2464cde5da4630001c183fc1..000ad743f93205afd2ae0fc0ff3f31185a1773de 100644 (file)
@@ -7,6 +7,12 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiController do
 
   plug(OpenApiSpex.Plug.CastAndValidate)
 
+  plug(
+    :skip_plug,
+    [Pleroma.Plugs.OAuthScopesPlug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug]
+    when action == :index
+  )
+
   defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.CustomEmojiOperation
 
   def index(conn, _params) do
index 27b5b1a524c99c25c42e89df9bcc67b384445569..237f8567758e1c19de872b84ac94d38135f6e084 100644 (file)
@@ -5,6 +5,12 @@
 defmodule Pleroma.Web.MastodonAPI.InstanceController do
   use Pleroma.Web, :controller
 
+  plug(
+    :skip_plug,
+    [Pleroma.Plugs.OAuthScopesPlug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug]
+    when action in [:show, :peers]
+  )
+
   @doc "GET /api/v1/instance"
   def show(conn, _params) do
     render(conn, "show.json")
index f0492b1893b78302ba90c336d72ff48630667c95..e7767de4eca50536d26f7f54a310ce54771cfc86 100644 (file)
@@ -15,7 +15,11 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
 
   require Logger
 
-  plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug when action in [:empty_array, :empty_object])
+  plug(
+    :skip_plug,
+    [Pleroma.Plugs.OAuthScopesPlug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug]
+    when action in [:empty_array, :empty_object]
+  )
 
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
 
index b54c569675d9e7fe5937c4cbc80ca6d281c7a4b1..cd49da6ad5e236ddfb9ce072e55328e2f6423d83 100644 (file)
@@ -21,6 +21,8 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
   # Note: Mastodon doesn't allow unauthenticated access (requires read:accounts / read:search)
   plug(OAuthScopesPlug, %{scopes: ["read:search"], fallback: :proceed_unauthenticated})
 
+  # Note: on private instances auth is required (EnsurePublicOrAuthenticatedPlug is not skipped)
+
   plug(RateLimiter, [name: :search] when action in [:search, :search2, :account_search])
 
   def account_search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do
index 891f924bc929d3de8852849165f15b4cfe5ec809..040a0b9dd247d7c8eb33f1442e2c85841f993657 100644 (file)
@@ -9,6 +9,7 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
     only: [add_link_headers: 2, add_link_headers: 3, truthy_param?: 1, skip_relationships?: 1]
 
   alias Pleroma.Pagination
+  alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
   alias Pleroma.Plugs.OAuthScopesPlug
   alias Pleroma.Plugs.RateLimiter
   alias Pleroma.User
@@ -26,7 +27,13 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
   plug(OAuthScopesPlug, %{scopes: ["read:statuses"]} when action in [:home, :direct])
   plug(OAuthScopesPlug, %{scopes: ["read:lists"]} when action == :list)
 
-  plug(:skip_plug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug when action == :public)
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["read:statuses"], fallback: :proceed_unauthenticated}
+    when action in [:public, :hashtag]
+  )
+
+  plug(:skip_plug, EnsurePublicOrAuthenticatedPlug when action in [:public, :hashtag])
 
   plug(:put_view, Pleroma.Web.MastodonAPI.StatusView)
 
@@ -93,7 +100,9 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
 
     restrict? = Pleroma.Config.get([:restrict_unauthenticated, :timelines, cfg_key])
 
-    if not (restrict? and is_nil(user)) do
+    if restrict? and is_nil(user) do
+      render_error(conn, :unauthorized, "authorization required for timeline view")
+    else
       activities =
         params
         |> Map.put("type", ["Create", "Announce"])
@@ -110,12 +119,10 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
         as: :activity,
         skip_relationships: skip_relationships?(params)
       )
-    else
-      render_error(conn, :unauthorized, "authorization required for timeline view")
     end
   end
 
-  def hashtag_fetching(params, user, local_only) do
+  defp hashtag_fetching(params, user, local_only) do
     tags =
       [params["tag"], params["any"]]
       |> List.flatten()
index 7a65697e83ea0ebfa173195861b32a505b4c9ded..2c1874051ed833aac8336be4a58351ae30b5f41b 100644 (file)
@@ -26,6 +26,12 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIController do
     when action in [:conversation, :conversation_statuses]
   )
 
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["read:statuses"], fallback: :proceed_unauthenticated}
+    when action == :emoji_reactions_by
+  )
+
   plug(
     OAuthScopesPlug,
     %{scopes: ["write:statuses"]}
index c81e8535ec3ea9e69996d1e5acfadfb9023460dd..22da6c0ad6cf820a691e052635ac1a74059d95bb 100644 (file)
@@ -13,7 +13,11 @@ defmodule Pleroma.Web.PleromaAPI.ScrobbleController do
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.MastodonAPI.StatusView
 
-  plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :user_scrobbles)
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["read"], fallback: :proceed_unauthenticated} when action == :user_scrobbles
+  )
+
   plug(OAuthScopesPlug, %{scopes: ["write"]} when action != :user_scrobbles)
 
   def new_scrobble(%{assigns: %{user: user}} = conn, %{"title" => _} = params) do
index 04c1c594178218a83bfbf1f9339903ec5b287812..db158d366aad964aad3dfc4769933f4fdf2532b3 100644 (file)
@@ -312,14 +312,10 @@ defmodule Pleroma.Web.Router do
       post("/scrobble", ScrobbleController, :new_scrobble)
     end
 
-    scope [] do
-      pipe_through(:api)
-      get("/accounts/:id/favourites", AccountController, :favourites)
-    end
-
     scope [] do
       pipe_through(:authenticated_api)
 
+      get("/accounts/:id/favourites", AccountController, :favourites)
       post("/accounts/:id/subscribe", AccountController, :subscribe)
       post("/accounts/:id/unsubscribe", AccountController, :unsubscribe)
     end
@@ -353,6 +349,8 @@ defmodule Pleroma.Web.Router do
     post("/accounts/:id/mute", AccountController, :mute)
     post("/accounts/:id/unmute", AccountController, :unmute)
 
+    get("/apps/verify_credentials", AppController, :verify_credentials)
+
     get("/conversations", ConversationController, :index)
     post("/conversations/:id/read", ConversationController, :mark_as_read)
 
@@ -431,6 +429,7 @@ defmodule Pleroma.Web.Router do
 
     get("/timelines/home", TimelineController, :home)
     get("/timelines/direct", TimelineController, :direct)
+    get("/timelines/list/:list_id", TimelineController, :list)
   end
 
   scope "/api/web", Pleroma.Web do
@@ -442,15 +441,24 @@ defmodule Pleroma.Web.Router do
   scope "/api/v1", Pleroma.Web.MastodonAPI do
     pipe_through(:api)
 
-    post("/accounts", AccountController, :create)
     get("/accounts/search", SearchController, :account_search)
+    get("/search", SearchController, :search)
+
+    get("/accounts/:id/statuses", AccountController, :statuses)
+    get("/accounts/:id/followers", AccountController, :followers)
+    get("/accounts/:id/following", AccountController, :following)
+    get("/accounts/:id", AccountController, :show)
+
+    post("/accounts", AccountController, :create)
 
     get("/instance", InstanceController, :show)
     get("/instance/peers", InstanceController, :peers)
 
     post("/apps", AppController, :create)
-    get("/apps/verify_credentials", AppController, :verify_credentials)
 
+    get("/statuses", StatusController, :index)
+    get("/statuses/:id", StatusController, :show)
+    get("/statuses/:id/context", StatusController, :context)
     get("/statuses/:id/card", StatusController, :card)
     get("/statuses/:id/favourited_by", StatusController, :favourited_by)
     get("/statuses/:id/reblogged_by", StatusController, :reblogged_by)
@@ -461,20 +469,8 @@ defmodule Pleroma.Web.Router do
 
     get("/timelines/public", TimelineController, :public)
     get("/timelines/tag/:tag", TimelineController, :hashtag)
-    get("/timelines/list/:list_id", TimelineController, :list)
-
-    get("/statuses", StatusController, :index)
-    get("/statuses/:id", StatusController, :show)
-    get("/statuses/:id/context", StatusController, :context)
 
     get("/polls/:id", PollController, :show)
-
-    get("/accounts/:id/statuses", AccountController, :statuses)
-    get("/accounts/:id/followers", AccountController, :followers)
-    get("/accounts/:id/following", AccountController, :following)
-    get("/accounts/:id", AccountController, :show)
-
-    get("/search", SearchController, :search)
   end
 
   scope "/api/v2", Pleroma.Web.MastodonAPI do
index 55228616ab7d5bcfdecf63ac68f7b3ac66d1806f..e4f182b0202b90f7c674c78bee402f8dc9b4044c 100644 (file)
@@ -18,7 +18,7 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
     %{scopes: ["write:notifications"]} when action == :mark_notifications_as_read
   )
 
-  plug(:skip_plug, OAuthScopesPlug when action in [:oauth_tokens, :revoke_token])
+  plug(:skip_plug, OAuthScopesPlug when action in [:confirm_email, :oauth_tokens, :revoke_token])
 
   action_fallback(:errors)
 
@@ -47,13 +47,13 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
     json_reply(conn, 201, "")
   end
 
-  def errors(conn, {:param_cast, _}) do
+  defp errors(conn, {:param_cast, _}) do
     conn
     |> put_status(400)
     |> json("Invalid parameters")
   end
 
-  def errors(conn, _) do
+  defp errors(conn, _) do
     conn
     |> put_status(500)
     |> json("Something went wrong")
index ec04c05f088853cefc5a8b58561a3c1be58409e4..e2416fb2ea9dfbcbcc27c01602fd9702a5f5d565 100644 (file)
@@ -67,13 +67,25 @@ defmodule Pleroma.Web do
 
       # Executed just before actual controller action, invokes before-action hooks (callbacks)
       defp action(conn, params) do
-        with %Plug.Conn{halted: false} <- maybe_perform_public_or_authenticated_check(conn),
+        with %Plug.Conn{halted: false} <- maybe_drop_authentication_if_oauth_check_ignored(conn),
+             %Plug.Conn{halted: false} <- maybe_perform_public_or_authenticated_check(conn),
              %Plug.Conn{halted: false} <- maybe_perform_authenticated_check(conn),
              %Plug.Conn{halted: false} <- maybe_halt_on_missing_oauth_scopes_check(conn) do
           super(conn, params)
         end
       end
 
+      # For non-authenticated API actions, drops auth info if OAuth scopes check was ignored
+      #   (neither performed nor explicitly skipped)
+      defp maybe_drop_authentication_if_oauth_check_ignored(conn) do
+        if PlugHelper.plug_called?(conn, ExpectPublicOrAuthenticatedCheckPlug) and
+             not PlugHelper.plug_called_or_skipped?(conn, OAuthScopesPlug) do
+          OAuthScopesPlug.drop_auth_info(conn)
+        else
+          conn
+        end
+      end
+
       # Ensures instance is public -or- user is authenticated if such check was scheduled
       defp maybe_perform_public_or_authenticated_check(conn) do
         if PlugHelper.plug_called?(conn, ExpectPublicOrAuthenticatedCheckPlug) do