From 2a4a4f3342bb3d2bbbd2354e858278d2e17f8654 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Fri, 15 Feb 2019 19:54:37 +0300 Subject: [PATCH] [#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. --- lib/pleroma/plugs/oauth_scopes_plug.ex | 37 ++++-- lib/pleroma/web/controller_helper.ex | 2 +- .../mastodon_api/mastodon_api_controller.ex | 2 +- lib/pleroma/web/oauth.ex | 9 +- lib/pleroma/web/oauth/oauth_controller.ex | 42 ++++-- lib/pleroma/web/router.ex | 124 ++++++++++++------ .../web/templates/o_auth/o_auth/show.html.eex | 2 +- 7 files changed, 142 insertions(+), 76 deletions(-) diff --git a/lib/pleroma/plugs/oauth_scopes_plug.ex b/lib/pleroma/plugs/oauth_scopes_plug.ex index a81e29830..f2bfa2b1a 100644 --- 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, ", ")}." + cond do + is_nil(token) -> + conn - conn - |> put_resp_content_type("application/json") - |> send_resp(403, Jason.encode!(%{error: error_message})) - |> halt() + 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 a32195b49..8f36329ee 100644 --- 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 a1e9472b2..0e77ec907 100644 --- 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 8c78d1100..d2835a0ba 100644 --- 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 3e905c7c7..d8d3ea5b4 100644 --- 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" - error -> - error + app = Repo.get_by(App, client_id: client_id) + available_scopes = (app && app.scopes) || oauth_scopes(auth_params, []) + scope_param = Enum.join(available_scopes, " ") + + 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 4ece311d3..6f17de1ca 100644 --- 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("/search", TwitterAPI.Controller, :search) - get("/statusnet/tags/timeline/:tag", TwitterAPI.Controller, :public_and_external_timeline) + 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) + 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 1ecb5b444..41b58aca0 100644 --- 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" %>
<%= 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) %>
<% end %>