[#468] Defined OAuth restrictions for all applicable routes.

Improved missing "scopes" param handling.
Allowed "any of" / "all of" mode specification in OAuthScopesPlug.
Fixed auth UI / behavior when user selects no permissions at /oauth/authorize.

diff --git a/lib/pleroma/plugs/oauth_scopes_plug.ex b/lib/pleroma/plugs/oauth_scopes_plug.ex
index a81e298307a7963a9785d292e2cfcef8c1da957d..f2bfa2b1a7c9e3699fb6c97d841783ffce5b0d81 100644 (file)
--- a/lib/pleroma/plugs/oauth_scopes_plug.ex
+++ b/lib/pleroma/plugs/oauth_scopes_plug.ex
@@ -7,22 +7,35 @@ defmodule Pleroma.Plugs.OAuthScopesPlug do
 
   @behaviour Plug
 
-  def init(%{required_scopes: _} = options), do: options
+  def init(%{scopes: _} = options), do: options
 
-  def call(%Plug.Conn{assigns: assigns} = conn, %{required_scopes: required_scopes}) do
+  def call(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
+    op = options[:op] || :|
     token = assigns[:token]
-    granted_scopes = token && token.scopes
-
-    if is_nil(token) || required_scopes -- granted_scopes == [] do
-      conn
-    else
-      missing_scopes = required_scopes -- granted_scopes
-      error_message = "Insufficient permissions: #{Enum.join(missing_scopes, ", ")}."
-
-      conn
-      |> put_resp_content_type("application/json")
-      |> send_resp(403, Jason.encode!(%{error: error_message}))
-      |> halt()
+
+    cond do
+      is_nil(token) ->
+        conn
+
+      op == :| && scopes -- token.scopes != scopes ->
+        conn
+
+      op == :& && scopes -- token.scopes == [] ->
+        conn
+
+      options[:fallback] == :proceed_unauthenticated ->
+        conn
+        |> assign(:user, nil)
+        |> assign(:token, nil)
+
+      true ->
+        missing_scopes = scopes -- token.scopes
+        error_message = "Insufficient permissions: #{Enum.join(missing_scopes, " #{op} ")}."
+
+        conn
+        |> put_resp_content_type("application/json")
+        |> send_resp(403, Jason.encode!(%{error: error_message}))
+        |> halt()
     end
   end
 end

diff --git a/lib/pleroma/web/controller_helper.ex b/lib/pleroma/web/controller_helper.ex
index a32195b49dab1c32ef3f900825afd0a0fd4f49e1..8f36329ee989d25ea66c2fdd7f3a9476125c2a99 100644 (file)
--- a/lib/pleroma/web/controller_helper.ex
+++ b/lib/pleroma/web/controller_helper.ex
@@ -6,7 +6,7 @@ defmodule Pleroma.Web.ControllerHelper do
   use Pleroma.Web, :controller
 
   def oauth_scopes(params, default) do
-    Pleroma.Web.OAuth.parse_scopes(params["scopes"] || params["scope"], default)
+    Pleroma.Web.OAuth.parse_scopes(params["scope"] || params["scopes"], default)
   end
 
   def json_response(conn, status, json) do

diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex
index a1e9472b2b39ad408eb97767c855b2f55c3e57bf..0e77ec9070ef121287161c90f397dbfc48f30e36 100644 (file)
--- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex
+++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex
@@ -33,7 +33,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
   action_fallback(:errors)
 
   def create_app(conn, params) do
-    scopes = oauth_scopes(params, [])
+    scopes = oauth_scopes(params, ["read"])
 
     app_attrs =
       params

diff --git a/lib/pleroma/web/oauth.ex b/lib/pleroma/web/oauth.ex
index 8c78d110031f21d3b9907a55092dc3116fa2c8e1..d2835a0ba579c8a5c94b9a79736f07f9b48dd58a 100644 (file)
--- a/lib/pleroma/web/oauth.ex
+++ b/lib/pleroma/web/oauth.ex
@@ -3,16 +3,13 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.OAuth do
-  def parse_scopes(scopes, default) when is_list(scopes) do
-    scopes = Enum.filter(scopes, &(&1 not in [nil, ""]))
-
-    if Enum.any?(scopes),
-      do: scopes,
-      else: default
+  def parse_scopes(scopes, _default) when is_list(scopes) do
+    Enum.filter(scopes, &(&1 not in [nil, ""]))
   end
 
   def parse_scopes(scopes, default) when is_binary(scopes) do
     scopes
+    |> String.trim()
     |> String.split(~r/[\s,]+/)
     |> parse_scopes(default)
   end

diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex
index 3e905c7c70528510435f3a59d9f4dc1097f5213b..d8d3ea5b478618031f8750439e293af217891689 100644 (file)
--- a/lib/pleroma/web/oauth/oauth_controller.ex
+++ b/lib/pleroma/web/oauth/oauth_controller.ex
@@ -17,10 +17,20 @@ defmodule Pleroma.Web.OAuth.OAuthController do
   action_fallback(Pleroma.Web.OAuth.FallbackController)
 
   def authorize(conn, params) do
+    params_scopes = oauth_scopes(params, nil)
+
+    scopes =
+      if params_scopes do
+        params_scopes
+      else
+        app = Repo.get_by(App, client_id: params["client_id"])
+        app && app.scopes
+      end
+
     render(conn, "show.html", %{
       response_type: params["response_type"],
       client_id: params["client_id"],
-      scopes: oauth_scopes(params, []),
+      scopes: scopes || [],
       redirect_uri: params["redirect_uri"],
       state: params["state"]
     })
@@ -33,14 +43,14 @@ defmodule Pleroma.Web.OAuth.OAuthController do
             "password" => password,
             "client_id" => client_id,
             "redirect_uri" => redirect_uri
-          } = params
+          } = auth_params
       }) do
     with %User{} = user <- User.get_by_nickname_or_email(name),
          true <- Pbkdf2.checkpw(password, user.password_hash),
          {:auth_active, true} <- {:auth_active, User.auth_active?(user)},
          %App{} = app <- Repo.get_by(App, client_id: client_id),
          true <- redirect_uri in String.split(app.redirect_uris),
-         scopes <- oauth_scopes(params, app.scopes),
+         scopes <- oauth_scopes(auth_params, []),
          [] <- scopes -- app.scopes,
          true <- Enum.any?(scopes),
          {:ok, auth} <- Authorization.create_authorization(app, user, scopes) do
@@ -64,8 +74,8 @@ defmodule Pleroma.Web.OAuth.OAuthController do
           url_params = %{:code => auth.token}
 
           url_params =
-            if params["state"] do
-              Map.put(url_params, :state, params["state"])
+            if auth_params["state"] do
+              Map.put(url_params, :state, auth_params["state"])
             else
               url_params
             end
@@ -75,14 +85,20 @@ defmodule Pleroma.Web.OAuth.OAuthController do
           redirect(conn, external: url)
       end
     else
-      {:auth_active, false} ->
-        conn
-        |> put_flash(:error, "Account confirmation pending")
-        |> put_status(:forbidden)
-        |> authorize(params)
+      res ->
+        msg =
+          if res == {:auth_active, false},
+            do: "Account confirmation pending",
+            else: "Invalid Username/Password/Permissions"
+
+        app = Repo.get_by(App, client_id: client_id)
+        available_scopes = (app && app.scopes) || oauth_scopes(auth_params, [])
+        scope_param = Enum.join(available_scopes, " ")
 
-      error ->
-        error
+        conn
+        |> put_flash(:error, msg)
+        |> put_status(:unauthorized)
+        |> authorize(Map.merge(auth_params, %{"scope" => scope_param}))
     end
   end
 
@@ -119,6 +135,8 @@ defmodule Pleroma.Web.OAuth.OAuthController do
          true <- Pbkdf2.checkpw(password, user.password_hash),
          {:auth_active, true} <- {:auth_active, User.auth_active?(user)},
          scopes <- oauth_scopes(params, app.scopes),
+         [] <- scopes -- app.scopes,
+         true <- Enum.any?(scopes),
          {:ok, auth} <- Authorization.create_authorization(app, user, scopes),
          {:ok, token} <- Token.exchange_token(app, auth) do
       response = %{

diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
index 4ece311d3ef12d24f90d30a2a6ba7497dc08c1f9..6f17de1cafb7d64d96335c0dffc712c45ad8820b 100644 (file)
--- a/lib/pleroma/web/router.ex
+++ b/lib/pleroma/web/router.ex
@@ -74,16 +74,23 @@ defmodule Pleroma.Web.Router do
     plug(Pleroma.Plugs.EnsureUserKeyPlug)
   end
 
+  pipeline :oauth_read_or_unauthenticated do
+    plug(Pleroma.Plugs.OAuthScopesPlug, %{
+      scopes: ["read"],
+      fallback: :proceed_unauthenticated
+    })
+  end
+
   pipeline :oauth_read do
-    plug(Pleroma.Plugs.OAuthScopesPlug, %{required_scopes: ["read"]})
+    plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: ["read"]})
   end
 
   pipeline :oauth_write do
-    plug(Pleroma.Plugs.OAuthScopesPlug, %{required_scopes: ["write"]})
+    plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: ["write"]})
   end
 
   pipeline :oauth_follow do
-    plug(Pleroma.Plugs.OAuthScopesPlug, %{required_scopes: ["follow"]})
+    plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: ["follow"]})
   end
 
   pipeline :well_known do
@@ -113,6 +120,7 @@ defmodule Pleroma.Web.Router do
 
   scope "/api/pleroma", Pleroma.Web.TwitterAPI do
     pipe_through(:pleroma_api)
+
     get("/password_reset/:token", UtilController, :show_password_reset)
     post("/password_reset", UtilController, :password_reset)
     get("/emoji", UtilController, :emoji)
@@ -125,7 +133,8 @@ defmodule Pleroma.Web.Router do
   end
 
   scope "/api/pleroma/admin", Pleroma.Web.AdminAPI do
-    pipe_through(:admin_api)
+    pipe_through([:admin_api, :oauth_write])
+
     delete("/user", AdminAPIController, :user_delete)
     post("/user", AdminAPIController, :user_create)
     put("/users/tag", AdminAPIController, :tag_users)
@@ -147,9 +156,14 @@ defmodule Pleroma.Web.Router do
 
   scope "/", Pleroma.Web.TwitterAPI do
     pipe_through(:pleroma_html)
-    get("/ostatus_subscribe", UtilController, :remote_follow)
-    post("/ostatus_subscribe", UtilController, :do_remote_follow)
+
     post("/main/ostatus", UtilController, :remote_subscribe)
+    get("/ostatus_subscribe", UtilController, :remote_follow)
+
+    scope [] do
+      pipe_through(:oauth_follow)
+      post("/ostatus_subscribe", UtilController, :do_remote_follow)
+    end
   end
 
   scope "/api/pleroma", Pleroma.Web.TwitterAPI do
@@ -180,11 +194,11 @@ defmodule Pleroma.Web.Router do
   scope "/api/v1", Pleroma.Web.MastodonAPI do
     pipe_through(:authenticated_api)
 
-    get("/accounts/verify_credentials", MastodonAPIController, :verify_credentials)
-
     scope [] do
       pipe_through(:oauth_read)
 
+      get("/accounts/verify_credentials", MastodonAPIController, :verify_credentials)
+
       get("/accounts/relationships", MastodonAPIController, :relationships)
       get("/accounts/search", MastodonAPIController, :account_search)
 
@@ -284,33 +298,40 @@ defmodule Pleroma.Web.Router do
 
   scope "/api/v1", Pleroma.Web.MastodonAPI do
     pipe_through(:api)
+
     get("/instance", MastodonAPIController, :masto_instance)
     get("/instance/peers", MastodonAPIController, :peers)
     post("/apps", MastodonAPIController, :create_app)
     get("/custom_emojis", MastodonAPIController, :custom_emojis)
 
-    get("/timelines/public", MastodonAPIController, :public_timeline)
-    get("/timelines/tag/:tag", MastodonAPIController, :hashtag_timeline)
-    get("/timelines/list/:list_id", MastodonAPIController, :list_timeline)
-
-    get("/statuses/:id", MastodonAPIController, :get_status)
-    get("/statuses/:id/context", MastodonAPIController, :get_context)
     get("/statuses/:id/card", MastodonAPIController, :status_card)
+
     get("/statuses/:id/favourited_by", MastodonAPIController, :favourited_by)
     get("/statuses/:id/reblogged_by", MastodonAPIController, :reblogged_by)
 
-    get("/accounts/:id/statuses", MastodonAPIController, :user_statuses)
-    get("/accounts/:id/followers", MastodonAPIController, :followers)
-    get("/accounts/:id/following", MastodonAPIController, :following)
-    get("/accounts/:id", MastodonAPIController, :user)
-
     get("/trends", MastodonAPIController, :empty_array)
 
-    get("/search", MastodonAPIController, :search)
+    scope [] do
+      pipe_through(:oauth_read_or_unauthenticated)
+
+      get("/timelines/public", MastodonAPIController, :public_timeline)
+      get("/timelines/tag/:tag", MastodonAPIController, :hashtag_timeline)
+      get("/timelines/list/:list_id", MastodonAPIController, :list_timeline)
+
+      get("/statuses/:id", MastodonAPIController, :get_status)
+      get("/statuses/:id/context", MastodonAPIController, :get_context)
+
+      get("/accounts/:id/statuses", MastodonAPIController, :user_statuses)
+      get("/accounts/:id/followers", MastodonAPIController, :followers)
+      get("/accounts/:id/following", MastodonAPIController, :following)
+      get("/accounts/:id", MastodonAPIController, :user)
+
+      get("/search", MastodonAPIController, :search)
+    end
   end
 
   scope "/api/v2", Pleroma.Web.MastodonAPI do
-    pipe_through(:api)
+    pipe_through([:api, :oauth_read_or_unauthenticated])
     get("/search", MastodonAPIController, :search2)
   end
 
@@ -327,19 +348,11 @@ defmodule Pleroma.Web.Router do
   scope "/api", Pleroma.Web do
     pipe_through(:api)
 
-    get("/statuses/user_timeline", TwitterAPI.Controller, :user_timeline)
-    get("/qvitter/statuses/user_timeline", TwitterAPI.Controller, :user_timeline)
-    get("/users/show", TwitterAPI.Controller, :show_user)
-
-    get("/statuses/followers", TwitterAPI.Controller, :followers)
-    get("/statuses/friends", TwitterAPI.Controller, :friends)
-    get("/statuses/blocks", TwitterAPI.Controller, :blocks)
-    get("/statuses/show/:id", TwitterAPI.Controller, :fetch_status)
-    get("/statusnet/conversation/:id", TwitterAPI.Controller, :fetch_conversation)
-
     post("/account/register", TwitterAPI.Controller, :register)
     post("/account/password_reset", TwitterAPI.Controller, :password_reset)
 
+    post("/account/resend_confirmation_email", TwitterAPI.Controller, :resend_confirmation_email)
+
     get(
       "/account/confirm_email/:user_id/:token",
       TwitterAPI.Controller,
@@ -347,14 +360,26 @@ defmodule Pleroma.Web.Router do
       as: :confirm_email
     )
 
-    post("/account/resend_confirmation_email", TwitterAPI.Controller, :resend_confirmation_email)
+    scope [] do
+      pipe_through(:oauth_read_or_unauthenticated)
+
+      get("/statuses/user_timeline", TwitterAPI.Controller, :user_timeline)
+      get("/qvitter/statuses/user_timeline", TwitterAPI.Controller, :user_timeline)
+      get("/users/show", TwitterAPI.Controller, :show_user)
+
+      get("/statuses/followers", TwitterAPI.Controller, :followers)
+      get("/statuses/friends", TwitterAPI.Controller, :friends)
+      get("/statuses/blocks", TwitterAPI.Controller, :blocks)
+      get("/statuses/show/:id", TwitterAPI.Controller, :fetch_status)
+      get("/statusnet/conversation/:id", TwitterAPI.Controller, :fetch_conversation)
 
-    get("/search", TwitterAPI.Controller, :search)
-    get("/statusnet/tags/timeline/:tag", TwitterAPI.Controller, :public_and_external_timeline)
+      get("/search", TwitterAPI.Controller, :search)
+      get("/statusnet/tags/timeline/:tag", TwitterAPI.Controller, :public_and_external_timeline)
+    end
   end
 
   scope "/api", Pleroma.Web do
-    pipe_through(:api)
+    pipe_through([:api, :oauth_read_or_unauthenticated])
 
     get("/statuses/public_timeline", TwitterAPI.Controller, :public_timeline)
 
@@ -368,19 +393,19 @@ defmodule Pleroma.Web.Router do
   end
 
   scope "/api", Pleroma.Web, as: :twitter_api_search do
-    pipe_through(:api)
+    pipe_through([:api, :oauth_read_or_unauthenticated])
     get("/pleroma/search_user", TwitterAPI.Controller, :search_user)
   end
 
   scope "/api", Pleroma.Web, as: :authenticated_twitter_api do
     pipe_through(:authenticated_api)
 
-    get("/account/verify_credentials", TwitterAPI.Controller, :verify_credentials)
-    post("/account/verify_credentials", TwitterAPI.Controller, :verify_credentials)
-
     scope [] do
       pipe_through(:oauth_read)
 
+      get("/account/verify_credentials", TwitterAPI.Controller, :verify_credentials)
+      post("/account/verify_credentials", TwitterAPI.Controller, :verify_credentials)
+
       get("/statuses/home_timeline", TwitterAPI.Controller, :friends_timeline)
       get("/statuses/friends_timeline", TwitterAPI.Controller, :friends_timeline)
       get("/statuses/mentions", TwitterAPI.Controller, :mentions_timeline)
@@ -506,9 +531,16 @@ defmodule Pleroma.Web.Router do
   scope "/", Pleroma.Web.ActivityPub do
     pipe_through([:activitypub_client])
 
-    get("/api/ap/whoami", ActivityPubController, :whoami)
-    get("/users/:nickname/inbox", ActivityPubController, :read_inbox)
-    post("/users/:nickname/outbox", ActivityPubController, :update_outbox)
+    scope [] do
+      pipe_through(:oauth_read)
+      get("/api/ap/whoami", ActivityPubController, :whoami)
+      get("/users/:nickname/inbox", ActivityPubController, :read_inbox)
+    end
+
+    scope [] do
+      pipe_through(:oauth_write)
+      post("/users/:nickname/outbox", ActivityPubController, :update_outbox)
+    end
   end
 
   scope "/relay", Pleroma.Web.ActivityPub do
@@ -518,6 +550,7 @@ defmodule Pleroma.Web.Router do
 
   scope "/", Pleroma.Web.ActivityPub do
     pipe_through(:activitypub)
+
     post("/users/:nickname/inbox", ActivityPubController, :inbox)
     post("/inbox", ActivityPubController, :inbox)
   end
@@ -538,8 +571,12 @@ defmodule Pleroma.Web.Router do
     pipe_through(:mastodon_html)
 
     get("/web/login", MastodonAPIController, :login)
-    get("/web/*path", MastodonAPIController, :index)
     delete("/auth/sign_out", MastodonAPIController, :logout)
+
+    scope [] do
+      pipe_through(:oauth_read)
+      get("/web/*path", MastodonAPIController, :index)
+    end
   end
 
   pipeline :remote_media do
@@ -547,6 +584,7 @@ defmodule Pleroma.Web.Router do
 
   scope "/proxy/", Pleroma.Web.MediaProxy do
     pipe_through(:remote_media)
+
     get("/:sig/:url", MediaProxyController, :remote)
     get("/:sig/:url/:filename", MediaProxyController, :remote)
   end

diff --git a/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex
index 1ecb5b4444d931b0b0c18ddf07da79e69680472d..41b58aca0ea6597028aefef3913017b79c1d5fe1 100644 (file)
--- a/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex
+++ b/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex
@@ -14,7 +14,7 @@
 <%= label f, :scope, "Permissions" %>
 <br>
 <%= for scope <- @scopes do %>
-  <%= checkbox f, :"scopes_#{scope}", hidden_input: false, value: scope, checked_value: scope, name: "authorization[scopes][]" %>
+  <%= checkbox f, :"scopes_#{scope}", value: scope, checked_value: scope, unchecked_value: "", name: "authorization[scopes][]" %>
   <%= label f, :"scopes_#{scope}", String.capitalize(scope) %>
   <br>
 <% end %>