From: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Thu, 11 Feb 2021 12:02:50 +0000 (+0300)
Subject: [#2510] Improved support for app-bound OAuth tokens. Auth-related refactoring.
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=df89b5019b2c284d02361de509a2306db5115153;p=akkoma

[#2510] Improved support for app-bound OAuth tokens. Auth-related refactoring.
---

diff --git a/lib/pleroma/web/mastodon_api/controllers/app_controller.ex b/lib/pleroma/web/mastodon_api/controllers/app_controller.ex
index a7e4d93f5..dd3b39c77 100644
--- a/lib/pleroma/web/mastodon_api/controllers/app_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/app_controller.ex
@@ -3,6 +3,11 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.MastodonAPI.AppController do
+  @moduledoc """
+  Controller for supporting app-related actions.
+  If authentication is an option, app tokens (user-unbound) must be supported.
+  """
+
   use Pleroma.Web, :controller
 
   alias Pleroma.Repo
@@ -17,11 +22,9 @@ defmodule Pleroma.Web.MastodonAPI.AppController do
   plug(
     :skip_plug,
     [OAuthScopesPlug, EnsurePublicOrAuthenticatedPlug]
-    when action == :create
+    when action in [:create, :verify_credentials]
   )
 
-  plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :verify_credentials)
-
   plug(Pleroma.Web.ApiSpec.CastAndValidate)
 
   @local_mastodon_name "Mastodon-Local"
@@ -44,10 +47,13 @@ defmodule Pleroma.Web.MastodonAPI.AppController do
     end
   end
 
-  @doc "GET /api/v1/apps/verify_credentials"
-  def verify_credentials(%{assigns: %{user: _user, token: token}} = conn, _) do
-    with %Token{app: %App{} = app} <- Repo.preload(token, :app) do
-      render(conn, "short.json", app: app)
+  @doc """
+  GET /api/v1/apps/verify_credentials
+  Gets compact non-secret representation of the app. Supports app tokens and user tokens.
+  """
+  def verify_credentials(%{assigns: %{token: %Token{} = token}} = conn, _) do
+    with %{app: %App{} = app} <- Repo.preload(token, :app) do
+      render(conn, "compact_non_secret.json", app: app)
     end
   end
 end
diff --git a/lib/pleroma/web/mastodon_api/views/app_view.ex b/lib/pleroma/web/mastodon_api/views/app_view.ex
index 3d7131e09..c406b5a27 100644
--- a/lib/pleroma/web/mastodon_api/views/app_view.ex
+++ b/lib/pleroma/web/mastodon_api/views/app_view.ex
@@ -34,10 +34,10 @@ defmodule Pleroma.Web.MastodonAPI.AppView do
     |> with_vapid_key()
   end
 
-  def render("short.json", %{app: %App{website: webiste, client_name: name}}) do
+  def render("compact_non_secret.json", %{app: %App{website: website, client_name: name}}) do
     %{
       name: name,
-      website: webiste
+      website: website
     }
     |> with_vapid_key()
   end
diff --git a/lib/pleroma/web/plugs/ensure_authenticated_plug.ex b/lib/pleroma/web/plugs/ensure_authenticated_plug.ex
index a4b5dc257..31e7410d6 100644
--- a/lib/pleroma/web/plugs/ensure_authenticated_plug.ex
+++ b/lib/pleroma/web/plugs/ensure_authenticated_plug.ex
@@ -3,6 +3,10 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.Plugs.EnsureAuthenticatedPlug do
+  @moduledoc """
+  Ensures _user_ authentication (app-bound user-unbound tokens are not accepted).
+  """
+
   import Plug.Conn
   import Pleroma.Web.TranslationHelpers
 
diff --git a/lib/pleroma/web/plugs/ensure_public_or_authenticated_plug.ex b/lib/pleroma/web/plugs/ensure_public_or_authenticated_plug.ex
index b6dfc4f3c..8a8532f41 100644
--- a/lib/pleroma/web/plugs/ensure_public_or_authenticated_plug.ex
+++ b/lib/pleroma/web/plugs/ensure_public_or_authenticated_plug.ex
@@ -3,6 +3,11 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.Plugs.EnsurePublicOrAuthenticatedPlug do
+  @moduledoc """
+  Ensures instance publicity or _user_ authentication
+  (app-bound user-unbound tokens are accepted only if the instance is public).
+  """
+
   import Pleroma.Web.TranslationHelpers
   import Plug.Conn
 
diff --git a/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex b/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex
index 3a2b5dda8..534b0cff1 100644
--- a/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex
+++ b/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex
@@ -28,6 +28,11 @@ defmodule Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug do
     end
   end
 
+  # App-bound token case (obtained with client_id and client_secret)
+  def call(%{assigns: %{token: %Token{user_id: nil}}} = conn, _) do
+    assign(conn, :user, nil)
+  end
+
   def call(conn, _) do
     conn
     |> assign(:user, nil)
diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
index 2105d7e9e..297f03fbd 100644
--- a/lib/pleroma/web/router.ex
+++ b/lib/pleroma/web/router.ex
@@ -37,11 +37,13 @@ defmodule Pleroma.Web.Router do
     plug(Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug)
   end
 
-  pipeline :expect_authentication do
+  # Note: expects _user_ authentication (user-unbound app-bound tokens don't qualify)
+  pipeline :expect_user_authentication do
     plug(Pleroma.Web.Plugs.ExpectAuthenticatedCheckPlug)
   end
 
-  pipeline :expect_public_instance_or_authentication do
+  # Note: expects public instance or _user_ authentication (user-unbound tokens don't qualify)
+  pipeline :expect_public_instance_or_user_authentication do
     plug(Pleroma.Web.Plugs.ExpectPublicOrAuthenticatedCheckPlug)
   end
 
@@ -66,23 +68,30 @@ defmodule Pleroma.Web.Router do
     plug(OpenApiSpex.Plug.PutApiSpec, module: Pleroma.Web.ApiSpec)
   end
 
-  pipeline :api do
-    plug(:expect_public_instance_or_authentication)
+  pipeline :no_auth_or_privacy_expectations_api do
     plug(:base_api)
     plug(:after_auth)
     plug(Pleroma.Web.Plugs.IdempotencyPlug)
   end
 
+  # Pipeline for app-related endpoints (no user auth checks — app-bound tokens must be supported)
+  pipeline :app_api do
+    plug(:no_auth_or_privacy_expectations_api)
+  end
+
+  pipeline :api do
+    plug(:expect_public_instance_or_user_authentication)
+    plug(:no_auth_or_privacy_expectations_api)
+  end
+
   pipeline :authenticated_api do
-    plug(:expect_authentication)
-    plug(:base_api)
-    plug(:after_auth)
+    plug(:expect_user_authentication)
+    plug(:no_auth_or_privacy_expectations_api)
     plug(Pleroma.Web.Plugs.EnsureAuthenticatedPlug)
-    plug(Pleroma.Web.Plugs.IdempotencyPlug)
   end
 
   pipeline :admin_api do
-    plug(:expect_authentication)
+    plug(:expect_user_authentication)
     plug(:base_api)
     plug(Pleroma.Web.Plugs.AdminSecretAuthenticationPlug)
     plug(:after_auth)
@@ -432,8 +441,6 @@ 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)
 
@@ -524,6 +531,13 @@ defmodule Pleroma.Web.Router do
     put("/settings", MastoFEController, :put_settings)
   end
 
+  scope "/api/v1", Pleroma.Web.MastodonAPI do
+    pipe_through(:app_api)
+
+    post("/apps", AppController, :create)
+    get("/apps/verify_credentials", AppController, :verify_credentials)
+  end
+
   scope "/api/v1", Pleroma.Web.MastodonAPI do
     pipe_through(:api)
 
@@ -540,8 +554,6 @@ defmodule Pleroma.Web.Router do
     get("/instance", InstanceController, :show)
     get("/instance/peers", InstanceController, :peers)
 
-    post("/apps", AppController, :create)
-
     get("/statuses", StatusController, :index)
     get("/statuses/:id", StatusController, :show)
     get("/statuses/:id/context", StatusController, :context)
diff --git a/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
index 238fd265b..76d81b942 100644
--- a/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
+++ b/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
@@ -12,22 +12,26 @@ defmodule Pleroma.Web.MastodonAPI.AppControllerTest do
   import Pleroma.Factory
 
   test "apps/verify_credentials", %{conn: conn} do
-    token = insert(:oauth_token)
+    user_bound_token = insert(:oauth_token)
+    app_bound_token = insert(:oauth_token, user: nil)
+    refute app_bound_token.user
 
-    conn =
-      conn
-      |> put_req_header("authorization", "Bearer #{token.token}")
-      |> get("/api/v1/apps/verify_credentials")
+    for token <- [app_bound_token, user_bound_token] do
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{token.token}")
+        |> get("/api/v1/apps/verify_credentials")
 
-    app = Repo.preload(token, :app).app
+      app = Repo.preload(token, :app).app
 
-    expected = %{
-      "name" => app.client_name,
-      "website" => app.website,
-      "vapid_key" => Push.vapid_config() |> Keyword.get(:public_key)
-    }
+      expected = %{
+        "name" => app.client_name,
+        "website" => app.website,
+        "vapid_key" => Push.vapid_config() |> Keyword.get(:public_key)
+      }
 
-    assert expected == json_response_and_validate_schema(conn, 200)
+      assert expected == json_response_and_validate_schema(conn, 200)
+    end
   end
 
   test "creates an oauth app", %{conn: conn} do