From c3f12cf3c3597385481290b53a6bce31730a6a29 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 10 Apr 2019 21:40:38 +0300 Subject: [PATCH 1/6] [#923] OAuth consumer params handling refactoring. Registration and authorization-related params are wrapped in "authorization" in order to reduce edge cases number and simplify handling logic. --- lib/pleroma/web/auth/authenticator.ex | 18 +-- lib/pleroma/web/auth/ldap_authenticator.ex | 24 ++-- lib/pleroma/web/auth/pleroma_authenticator.ex | 29 +++-- lib/pleroma/web/oauth/fallback_controller.ex | 2 +- lib/pleroma/web/oauth/oauth_controller.ex | 115 +++++++++--------- .../templates/o_auth/o_auth/_scopes.html.eex | 2 +- .../templates/o_auth/o_auth/consumer.html.eex | 2 +- .../templates/o_auth/o_auth/register.html.eex | 7 +- .../web/templates/o_auth/o_auth/show.html.eex | 2 +- test/web/oauth/oauth_controller_test.exs | 90 ++++++++------ 10 files changed, 153 insertions(+), 138 deletions(-) diff --git a/lib/pleroma/web/auth/authenticator.ex b/lib/pleroma/web/auth/authenticator.ex index 89d88af32..b02f595dc 100644 --- a/lib/pleroma/web/auth/authenticator.ex +++ b/lib/pleroma/web/auth/authenticator.ex @@ -13,21 +13,21 @@ def implementation do ) end - @callback get_user(Plug.Conn.t(), Map.t()) :: {:ok, User.t()} | {:error, any()} - def get_user(plug, params), do: implementation().get_user(plug, params) + @callback get_user(Plug.Conn.t()) :: {:ok, User.t()} | {:error, any()} + def get_user(plug), do: implementation().get_user(plug) - @callback create_from_registration(Plug.Conn.t(), Map.t(), Registration.t()) :: + @callback create_from_registration(Plug.Conn.t(), Registration.t()) :: {:ok, User.t()} | {:error, any()} - def create_from_registration(plug, params, registration), - do: implementation().create_from_registration(plug, params, registration) + def create_from_registration(plug, registration), + do: implementation().create_from_registration(plug, registration) - @callback get_registration(Plug.Conn.t(), Map.t()) :: + @callback get_registration(Plug.Conn.t()) :: {:ok, Registration.t()} | {:error, any()} - def get_registration(plug, params), - do: implementation().get_registration(plug, params) + def get_registration(plug), do: implementation().get_registration(plug) @callback handle_error(Plug.Conn.t(), any()) :: any() - def handle_error(plug, error), do: implementation().handle_error(plug, error) + def handle_error(plug, error), + do: implementation().handle_error(plug, error) @callback auth_template() :: String.t() | nil def auth_template do diff --git a/lib/pleroma/web/auth/ldap_authenticator.ex b/lib/pleroma/web/auth/ldap_authenticator.ex index 8b6d5a77f..363c99597 100644 --- a/lib/pleroma/web/auth/ldap_authenticator.ex +++ b/lib/pleroma/web/auth/ldap_authenticator.ex @@ -13,14 +13,16 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do @connection_timeout 10_000 @search_timeout 10_000 - defdelegate get_registration(conn, params), to: @base + defdelegate get_registration(conn), to: @base + defdelegate create_from_registration(conn, registration), to: @base + defdelegate handle_error(conn, error), to: @base + defdelegate auth_template, to: @base + defdelegate oauth_consumer_template, to: @base - defdelegate create_from_registration(conn, params, registration), to: @base - - def get_user(%Plug.Conn{} = conn, params) do + def get_user(%Plug.Conn{} = conn) do if Pleroma.Config.get([:ldap, :enabled]) do {name, password} = - case params do + case conn.params do %{"authorization" => %{"name" => name, "password" => password}} -> {name, password} @@ -34,25 +36,17 @@ def get_user(%Plug.Conn{} = conn, params) do {:error, {:ldap_connection_error, _}} -> # When LDAP is unavailable, try default authenticator - @base.get_user(conn, params) + @base.get_user(conn) error -> error end else # Fall back to default authenticator - @base.get_user(conn, params) + @base.get_user(conn) end end - def handle_error(%Plug.Conn{} = _conn, error) do - error - end - - def auth_template, do: nil - - def oauth_consumer_template, do: nil - defp ldap_user(name, password) do ldap = Pleroma.Config.get(:ldap, []) host = Keyword.get(ldap, :host, "localhost") diff --git a/lib/pleroma/web/auth/pleroma_authenticator.ex b/lib/pleroma/web/auth/pleroma_authenticator.ex index c826adb4c..d647f1e05 100644 --- a/lib/pleroma/web/auth/pleroma_authenticator.ex +++ b/lib/pleroma/web/auth/pleroma_authenticator.ex @@ -10,9 +10,9 @@ defmodule Pleroma.Web.Auth.PleromaAuthenticator do @behaviour Pleroma.Web.Auth.Authenticator - def get_user(%Plug.Conn{} = _conn, params) do + def get_user(%Plug.Conn{} = conn) do {name, password} = - case params do + case conn.params do %{"authorization" => %{"name" => name, "password" => password}} -> {name, password} @@ -29,10 +29,9 @@ def get_user(%Plug.Conn{} = _conn, params) do end end - def get_registration( - %Plug.Conn{assigns: %{ueberauth_auth: %{provider: provider, uid: uid} = auth}}, - _params - ) do + def get_registration(%Plug.Conn{ + assigns: %{ueberauth_auth: %{provider: provider, uid: uid} = auth} + }) do registration = Registration.get_by_provider_uid(provider, uid) if registration do @@ -40,7 +39,8 @@ def get_registration( else info = auth.info - Registration.changeset(%Registration{}, %{ + %Registration{} + |> Registration.changeset(%{ provider: to_string(provider), uid: to_string(uid), info: %{ @@ -54,13 +54,16 @@ def get_registration( end end - def get_registration(%Plug.Conn{} = _conn, _params), do: {:error, :missing_credentials} + def get_registration(%Plug.Conn{} = _conn), do: {:error, :missing_credentials} - def create_from_registration(_conn, params, registration) do - nickname = value([params["nickname"], Registration.nickname(registration)]) - email = value([params["email"], Registration.email(registration)]) - name = value([params["name"], Registration.name(registration)]) || nickname - bio = value([params["bio"], Registration.description(registration)]) + def create_from_registration( + %Plug.Conn{params: %{"authorization" => registration_attrs}}, + registration + ) do + nickname = value([registration_attrs["nickname"], Registration.nickname(registration)]) + email = value([registration_attrs["email"], Registration.email(registration)]) + name = value([registration_attrs["name"], Registration.name(registration)]) || nickname + bio = value([registration_attrs["bio"], Registration.description(registration)]) random_password = :crypto.strong_rand_bytes(64) |> Base.encode64() diff --git a/lib/pleroma/web/oauth/fallback_controller.ex b/lib/pleroma/web/oauth/fallback_controller.ex index afaa00242..e3984f009 100644 --- a/lib/pleroma/web/oauth/fallback_controller.ex +++ b/lib/pleroma/web/oauth/fallback_controller.ex @@ -24,6 +24,6 @@ def call(conn, _error) do conn |> put_status(:unauthorized) |> put_flash(:error, "Invalid Username/Password") - |> OAuthController.authorize(conn.params["authorization"]) + |> OAuthController.authorize(conn.params) end end diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index bee7084ad..8e5a83466 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -44,36 +44,40 @@ def authorize(%{assigns: %{token: %Token{} = token}} = conn, params) do def authorize(conn, params), do: do_authorize(conn, params) - defp do_authorize(conn, params) do - app = Repo.get_by(App, client_id: params["client_id"]) + defp do_authorize(conn, %{"authorization" => auth_attrs}) do + app = Repo.get_by(App, client_id: auth_attrs["client_id"]) available_scopes = (app && app.scopes) || [] - scopes = oauth_scopes(params, nil) || available_scopes + scopes = oauth_scopes(auth_attrs, nil) || available_scopes render(conn, Authenticator.auth_template(), %{ - response_type: params["response_type"], - client_id: params["client_id"], + response_type: auth_attrs["response_type"], + client_id: auth_attrs["client_id"], available_scopes: available_scopes, scopes: scopes, - redirect_uri: params["redirect_uri"], - state: params["state"], - params: params + redirect_uri: auth_attrs["redirect_uri"], + state: auth_attrs["state"], + params: auth_attrs }) end + defp do_authorize(conn, auth_attrs), do: do_authorize(conn, %{"authorization" => auth_attrs}) + def create_authorization( conn, - %{"authorization" => auth_params} = params, + %{"authorization" => _} = params, opts \\ [] ) do with {:ok, auth} <- do_create_authorization(conn, params, opts[:user]) do - after_create_authorization(conn, auth, auth_params) + after_create_authorization(conn, auth, params) else error -> - handle_create_authorization_error(conn, error, auth_params) + handle_create_authorization_error(conn, error, params) end end - def after_create_authorization(conn, auth, %{"redirect_uri" => redirect_uri} = auth_params) do + def after_create_authorization(conn, auth, %{ + "authorization" => %{"redirect_uri" => redirect_uri} = auth_attrs + }) do redirect_uri = redirect_uri(conn, redirect_uri) if redirect_uri == "urn:ietf:wg:oauth:2.0:oob" do @@ -86,8 +90,8 @@ def after_create_authorization(conn, auth, %{"redirect_uri" => redirect_uri} = a url_params = %{:code => auth.token} url_params = - if auth_params["state"] do - Map.put(url_params, :state, auth_params["state"]) + if auth_attrs["state"] do + Map.put(url_params, :state, auth_attrs["state"]) else url_params end @@ -98,26 +102,34 @@ def after_create_authorization(conn, auth, %{"redirect_uri" => redirect_uri} = a end end - defp handle_create_authorization_error(conn, {scopes_issue, _}, auth_params) + defp handle_create_authorization_error( + conn, + {scopes_issue, _}, + %{"authorization" => _} = params + ) when scopes_issue in [:unsupported_scopes, :missing_scopes] do # Per https://github.com/tootsuite/mastodon/blob/ # 51e154f5e87968d6bb115e053689767ab33e80cd/app/controllers/api/base_controller.rb#L39 conn |> put_flash(:error, "This action is outside the authorized scopes") |> put_status(:unauthorized) - |> authorize(auth_params) + |> authorize(params) end - defp handle_create_authorization_error(conn, {:auth_active, false}, auth_params) do + defp handle_create_authorization_error( + conn, + {:auth_active, false}, + %{"authorization" => _} = params + ) do # Per https://github.com/tootsuite/mastodon/blob/ # 51e154f5e87968d6bb115e053689767ab33e80cd/app/controllers/api/base_controller.rb#L76 conn |> put_flash(:error, "Your login is missing a confirmed e-mail address") |> put_status(:forbidden) - |> authorize(auth_params) + |> authorize(params) end - defp handle_create_authorization_error(conn, error, _auth_params) do + defp handle_create_authorization_error(conn, error, %{"authorization" => _}) do Authenticator.handle_error(conn, error) end @@ -151,7 +163,7 @@ def token_exchange( conn, %{"grant_type" => "password"} = params ) do - with {_, {:ok, %User{} = user}} <- {:get_user, Authenticator.get_user(conn, params)}, + with {_, {:ok, %User{} = user}} <- {:get_user, Authenticator.get_user(conn)}, %App{} = app <- get_app_from_request(conn, params), {:auth_active, true} <- {:auth_active, User.auth_active?(user)}, {:user_active, true} <- {:user_active, !user.info.deactivated}, @@ -214,19 +226,19 @@ def token_revoke(conn, %{"token" => token} = params) do end @doc "Prepares OAuth request to provider for Ueberauth" - def prepare_request(conn, %{"provider" => provider} = params) do + def prepare_request(conn, %{"provider" => provider, "authorization" => auth_attrs}) do scope = - oauth_scopes(params, []) + oauth_scopes(auth_attrs, []) |> Enum.join(" ") state = - params + auth_attrs |> Map.delete("scopes") |> Map.put("scope", scope) |> Poison.encode!() params = - params + auth_attrs |> Map.drop(~w(scope scopes client_id redirect_uri)) |> Map.put("state", state) @@ -260,26 +272,26 @@ def callback(%{assigns: %{ueberauth_failure: failure}} = conn, params) do def callback(conn, params) do params = callback_params(params) - with {:ok, registration} <- Authenticator.get_registration(conn, params) do + with {:ok, registration} <- Authenticator.get_registration(conn) do user = Repo.preload(registration, :user).user - auth_params = Map.take(params, ~w(client_id redirect_uri scope scopes state)) + auth_attrs = Map.take(params, ~w(client_id redirect_uri scope scopes state)) if user do create_authorization( conn, - %{"authorization" => auth_params}, + %{"authorization" => auth_attrs}, user: user ) else registration_params = - Map.merge(auth_params, %{ + Map.merge(auth_attrs, %{ "nickname" => Registration.nickname(registration), "email" => Registration.email(registration) }) conn |> put_session(:registration_id, registration.id) - |> registration_details(registration_params) + |> registration_details(%{"authorization" => registration_params}) end else _ -> @@ -293,53 +305,44 @@ defp callback_params(%{"state" => state} = params) do Map.merge(params, Poison.decode!(state)) end - def registration_details(conn, params) do + def registration_details(conn, %{"authorization" => auth_attrs}) do render(conn, "register.html", %{ - client_id: params["client_id"], - redirect_uri: params["redirect_uri"], - state: params["state"], - scopes: oauth_scopes(params, []), - nickname: params["nickname"], - email: params["email"] + client_id: auth_attrs["client_id"], + redirect_uri: auth_attrs["redirect_uri"], + state: auth_attrs["state"], + scopes: oauth_scopes(auth_attrs, []), + nickname: auth_attrs["nickname"], + email: auth_attrs["email"] }) end - def register(conn, %{"op" => "connect"} = params) do - authorization_params = Map.put(params, "name", params["auth_name"]) - create_authorization_params = %{"authorization" => authorization_params} - + def register(conn, %{"authorization" => _, "op" => "connect"} = params) do with registration_id when not is_nil(registration_id) <- get_session_registration_id(conn), %Registration{} = registration <- Repo.get(Registration, registration_id), {_, {:ok, auth}} <- - {:create_authorization, do_create_authorization(conn, create_authorization_params)}, + {:create_authorization, do_create_authorization(conn, params)}, %User{} = user <- Repo.preload(auth, :user).user, {:ok, _updated_registration} <- Registration.bind_to_user(registration, user) do conn |> put_session_registration_id(nil) - |> after_create_authorization(auth, authorization_params) + |> after_create_authorization(auth, params) else {:create_authorization, error} -> - {:register, handle_create_authorization_error(conn, error, create_authorization_params)} + {:register, handle_create_authorization_error(conn, error, params)} _ -> {:register, :generic_error} end end - def register(conn, %{"op" => "register"} = params) do + def register(conn, %{"authorization" => _, "op" => "register"} = params) do with registration_id when not is_nil(registration_id) <- get_session_registration_id(conn), %Registration{} = registration <- Repo.get(Registration, registration_id), - {:ok, user} <- Authenticator.create_from_registration(conn, params, registration) do + {:ok, user} <- Authenticator.create_from_registration(conn, registration) do conn |> put_session_registration_id(nil) |> create_authorization( - %{ - "authorization" => %{ - "client_id" => params["client_id"], - "redirect_uri" => params["redirect_uri"], - "scopes" => oauth_scopes(params, nil) - } - }, + params, user: user ) else @@ -374,15 +377,15 @@ defp do_create_authorization( %{ "client_id" => client_id, "redirect_uri" => redirect_uri - } = auth_params - } = params, + } = auth_attrs + }, user \\ nil ) do with {_, {:ok, %User{} = user}} <- - {:get_user, (user && {:ok, user}) || Authenticator.get_user(conn, params)}, + {: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), - scopes <- oauth_scopes(auth_params, []), + scopes <- oauth_scopes(auth_attrs, []), {:unsupported_scopes, []} <- {:unsupported_scopes, scopes -- app.scopes}, # Note: `scope` param is intentionally not optional in this context {:missing_scopes, false} <- {:missing_scopes, scopes == []}, diff --git a/lib/pleroma/web/templates/o_auth/o_auth/_scopes.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/_scopes.html.eex index 4b8fb5dae..e6cfe108b 100644 --- a/lib/pleroma/web/templates/o_auth/o_auth/_scopes.html.eex +++ b/lib/pleroma/web/templates/o_auth/o_auth/_scopes.html.eex @@ -5,7 +5,7 @@ <%= for scope <- @available_scopes do %> <%# Note: using hidden input with `unchecked_value` in order to distinguish user's empty selection from `scope` param being omitted %>
- <%= checkbox @form, :"scope_#{scope}", value: scope in @scopes && scope, checked_value: scope, unchecked_value: "", name: assigns[:scope_param] || "scope[]" %> + <%= checkbox @form, :"scope_#{scope}", value: scope in @scopes && scope, checked_value: scope, unchecked_value: "", name: "authorization[scope][]" %> <%= label @form, :"scope_#{scope}", String.capitalize(scope) %>
<% end %> diff --git a/lib/pleroma/web/templates/o_auth/o_auth/consumer.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/consumer.html.eex index 85f62ca64..4bcda7300 100644 --- a/lib/pleroma/web/templates/o_auth/o_auth/consumer.html.eex +++ b/lib/pleroma/web/templates/o_auth/o_auth/consumer.html.eex @@ -1,6 +1,6 @@

Sign in with external provider

-<%= form_for @conn, o_auth_path(@conn, :prepare_request), [method: "get"], fn f -> %> +<%= form_for @conn, o_auth_path(@conn, :prepare_request), [as: "authorization", method: "get"], fn f -> %> <%= render @view_module, "_scopes.html", Map.put(assigns, :form, f) %> <%= hidden_input f, :client_id, value: @client_id %> diff --git a/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex index 126390391..facedc8db 100644 --- a/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex +++ b/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex @@ -8,8 +8,7 @@

Registration Details

If you'd like to register a new account, please provide the details below.

- -<%= form_for @conn, o_auth_path(@conn, :register), [], fn f -> %> +<%= form_for @conn, o_auth_path(@conn, :register), [as: "authorization"], fn f -> %>
<%= label f, :nickname, "Nickname" %> @@ -25,8 +24,8 @@

Alternatively, sign in to connect to existing account.

- <%= label f, :auth_name, "Name or email" %> - <%= text_input f, :auth_name %> + <%= label f, :name, "Name or email" %> + <%= text_input f, :name %>
<%= label f, :password, "Password" %> 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 87278e636..3e360a52c 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 @@ -17,7 +17,7 @@ <%= password_input f, :password %>
-<%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f, scope_param: "authorization[scope][]"}) %> +<%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %> <%= hidden_input f, :client_id, value: @client_id %> <%= hidden_input f, :response_type, value: @response_type %> diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index ac7843f9b..fb505fab3 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -68,10 +68,12 @@ test "GET /oauth/prepare_request encodes parameters as `state` and redirects", % "/oauth/prepare_request", %{ "provider" => "twitter", - "scope" => "read follow", - "client_id" => app.client_id, - "redirect_uri" => app.redirect_uris, - "state" => "a_state" + "authorization" => %{ + "scope" => "read follow", + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "state" => "a_state" + } } ) @@ -104,7 +106,7 @@ test "with user-bound registration, GET /oauth//callback redirects to } with_mock Pleroma.Web.Auth.Authenticator, - get_registration: fn _, _ -> {:ok, registration} end do + get_registration: fn _ -> {:ok, registration} end do conn = get( conn, @@ -134,7 +136,7 @@ test "with user-unbound registration, GET /oauth//callback renders reg } with_mock Pleroma.Web.Auth.Authenticator, - get_registration: fn _, _ -> {:ok, registration} end do + get_registration: fn _ -> {:ok, registration} end do conn = get( conn, @@ -193,12 +195,14 @@ test "GET /oauth/registration_details renders registration details form", %{ conn, "/oauth/registration_details", %{ - "scopes" => app.scopes, - "client_id" => app.client_id, - "redirect_uri" => app.redirect_uris, - "state" => "a_state", - "nickname" => nil, - "email" => "john@doe.com" + "authorization" => %{ + "scopes" => app.scopes, + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "state" => "a_state", + "nickname" => nil, + "email" => "john@doe.com" + } } ) @@ -221,12 +225,14 @@ test "with valid params, POST /oauth/register?op=register redirects to `redirect "/oauth/register", %{ "op" => "register", - "scopes" => app.scopes, - "client_id" => app.client_id, - "redirect_uri" => app.redirect_uris, - "state" => "a_state", - "nickname" => "availablenick", - "email" => "available@email.com" + "authorization" => %{ + "scopes" => app.scopes, + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "state" => "a_state", + "nickname" => "availablenick", + "email" => "available@email.com" + } } ) @@ -244,17 +250,23 @@ test "with invalid params, POST /oauth/register?op=register renders registration params = %{ "op" => "register", - "scopes" => app.scopes, - "client_id" => app.client_id, - "redirect_uri" => app.redirect_uris, - "state" => "a_state", - "nickname" => "availablenickname", - "email" => "available@email.com" + "authorization" => %{ + "scopes" => app.scopes, + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "state" => "a_state", + "nickname" => "availablenickname", + "email" => "available@email.com" + } } for {bad_param, bad_param_value} <- [{"nickname", another_user.nickname}, {"email", another_user.email}] do - bad_params = Map.put(params, bad_param, bad_param_value) + bad_registration_attrs = %{ + "authorization" => Map.put(params["authorization"], bad_param, bad_param_value) + } + + bad_params = Map.merge(params, bad_registration_attrs) conn = conn @@ -281,12 +293,14 @@ test "with valid params, POST /oauth/register?op=connect redirects to `redirect_ "/oauth/register", %{ "op" => "connect", - "scopes" => app.scopes, - "client_id" => app.client_id, - "redirect_uri" => app.redirect_uris, - "state" => "a_state", - "auth_name" => user.nickname, - "password" => "testpassword" + "authorization" => %{ + "scopes" => app.scopes, + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "state" => "a_state", + "name" => user.nickname, + "password" => "testpassword" + } } ) @@ -304,12 +318,14 @@ test "with invalid params, POST /oauth/register?op=connect renders registration_ params = %{ "op" => "connect", - "scopes" => app.scopes, - "client_id" => app.client_id, - "redirect_uri" => app.redirect_uris, - "state" => "a_state", - "auth_name" => user.nickname, - "password" => "wrong password" + "authorization" => %{ + "scopes" => app.scopes, + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "state" => "a_state", + "name" => user.nickname, + "password" => "wrong password" + } } conn = From 128aae05f374b7212e7676844520a4ddbbf8a94e Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 17 Apr 2019 11:33:21 +0300 Subject: [PATCH 2/6] [#923] Minor semantic adjustment. --- lib/pleroma/web/oauth/oauth_controller.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index 8e5a83466..9874bac23 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -44,7 +44,9 @@ def authorize(%{assigns: %{token: %Token{} = token}} = conn, params) do def authorize(conn, params), do: do_authorize(conn, params) - defp do_authorize(conn, %{"authorization" => auth_attrs}) do + defp do_authorize(conn, %{"authorization" => auth_attrs}), do: do_authorize(conn, auth_attrs) + + defp do_authorize(conn, auth_attrs) do app = Repo.get_by(App, client_id: auth_attrs["client_id"]) available_scopes = (app && app.scopes) || [] scopes = oauth_scopes(auth_attrs, nil) || available_scopes @@ -60,8 +62,6 @@ defp do_authorize(conn, %{"authorization" => auth_attrs}) do }) end - defp do_authorize(conn, auth_attrs), do: do_authorize(conn, %{"authorization" => auth_attrs}) - def create_authorization( conn, %{"authorization" => _} = params, From 2140e164d75e053a6b6c6131c939ae5ce9eebf03 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 17 Apr 2019 20:05:09 +0000 Subject: [PATCH 3/6] activitypub: properly filter out transitive activities concerning blocked users --- lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- test/web/activity_pub/activity_pub_test.exs | 23 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 54dd4097c..68317ee6a 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -712,7 +712,7 @@ defp restrict_blocked(query, %{"blocking_user" => %User{info: info}}) do from( activity in query, where: fragment("not (? = ANY(?))", activity.actor, ^blocks), - where: fragment("not (?->'to' \\?| ?)", activity.data, ^blocks), + where: fragment("not (? && ?)", activity.recipients, ^blocks), where: fragment("not (split_part(?, '/', 3) = ANY(?))", activity.actor, ^domain_blocks) ) end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 17fec05b1..5454bffde 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -341,6 +341,29 @@ test "doesn't return blocked activities" do assert Enum.member?(activities, activity_one) end + test "doesn't return transitive interactions concerning blocked users" do + blocker = insert(:user) + blockee = insert(:user) + friend = insert(:user) + + {:ok, blocker} = User.block(blocker, blockee) + + {:ok, activity_one} = CommonAPI.post(friend, %{"status" => "hey!"}) + + {:ok, activity_two} = CommonAPI.post(friend, %{"status" => "hey! @#{blockee.nickname}"}) + + {:ok, activity_three} = CommonAPI.post(blockee, %{"status" => "hey! @#{friend.nickname}"}) + + {:ok, activity_four} = CommonAPI.post(blockee, %{"status" => "hey! @#{blocker.nickname}"}) + + activities = ActivityPub.fetch_activities([], %{"blocking_user" => blocker}) + + assert Enum.member?(activities, activity_one) + refute Enum.member?(activities, activity_two) + refute Enum.member?(activities, activity_three) + refute Enum.member?(activities, activity_four) + end + test "doesn't return muted activities" do activity_one = insert(:note_activity) activity_two = insert(:note_activity) From 36f78c6dcdea48dfb0231a30561825832cdb4518 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Wed, 17 Apr 2019 22:27:59 +0000 Subject: [PATCH 4/6] activitypub: fix filtering of boosts from blocked users --- lib/pleroma/web/activity_pub/activity_pub.ex | 7 +++++++ test/web/activity_pub/activity_pub_test.exs | 22 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 68317ee6a..cb88ba308 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -713,6 +713,13 @@ defp restrict_blocked(query, %{"blocking_user" => %User{info: info}}) do activity in query, where: fragment("not (? = ANY(?))", activity.actor, ^blocks), where: fragment("not (? && ?)", activity.recipients, ^blocks), + where: + fragment( + "not (?->>'type' = 'Announce' and ?->'to' \\?| ?)", + activity.data, + activity.data, + ^blocks + ), where: fragment("not (split_part(?, '/', 3) = ANY(?))", activity.actor, ^domain_blocks) ) end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 5454bffde..79116824e 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -364,6 +364,28 @@ test "doesn't return transitive interactions concerning blocked users" do refute Enum.member?(activities, activity_four) end + test "doesn't return announce activities concerning blocked users" do + blocker = insert(:user) + blockee = insert(:user) + friend = insert(:user) + + {:ok, blocker} = User.block(blocker, blockee) + + {:ok, activity_one} = CommonAPI.post(friend, %{"status" => "hey!"}) + + {:ok, activity_two} = CommonAPI.post(blockee, %{"status" => "hey! @#{friend.nickname}"}) + + {:ok, activity_three, _} = CommonAPI.repeat(activity_two.id, friend) + + activities = + ActivityPub.fetch_activities([], %{"blocking_user" => blocker}) + |> Enum.map(fn act -> act.id end) + + assert Enum.member?(activities, activity_one.id) + refute Enum.member?(activities, activity_two.id) + refute Enum.member?(activities, activity_three.id) + end + test "doesn't return muted activities" do activity_one = insert(:note_activity) activity_two = insert(:note_activity) From 1aa4994f6d867e5c3e0d56dc26d7ebad7e4ecb56 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 18 Apr 2019 12:44:25 -0500 Subject: [PATCH 5/6] Do not require authentication for user search in MastoAPI --- docs/api/differences_in_mastoapi_responses.md | 6 ++++++ lib/pleroma/web/router.ex | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/api/differences_in_mastoapi_responses.md b/docs/api/differences_in_mastoapi_responses.md index 923d94db2..ed3fd9b67 100644 --- a/docs/api/differences_in_mastoapi_responses.md +++ b/docs/api/differences_in_mastoapi_responses.md @@ -41,6 +41,12 @@ Has these additional fields under the `pleroma` object: - `is_admin`: boolean, true if user is an admin - `confirmation_pending`: boolean, true if a new user account is waiting on email confirmation to be activated +## Account Search + +Behavior has changed: + +- `/api/v1/accounts/search`: Does not require authentication + ## Notifications Has these additional fields under the `pleroma` object: diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index a809347be..8b665d61b 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -242,7 +242,6 @@ defmodule Pleroma.Web.Router do get("/accounts/verify_credentials", MastodonAPIController, :verify_credentials) get("/accounts/relationships", MastodonAPIController, :relationships) - get("/accounts/search", MastodonAPIController, :account_search) get("/accounts/:id/lists", MastodonAPIController, :account_lists) get("/accounts/:id/identity_proofs", MastodonAPIController, :empty_array) @@ -377,6 +376,8 @@ defmodule Pleroma.Web.Router do get("/trends", MastodonAPIController, :empty_array) + get("/accounts/search", MastodonAPIController, :account_search) + scope [] do pipe_through(:oauth_read_or_unauthenticated) From ada384207b2b49ce410ea19b45c97868625d6d8d Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Fri, 19 Apr 2019 07:50:21 +0000 Subject: [PATCH 6/6] typo fix docs for RelMe provider --- CHANGELOG.md | 1 + README.md | 2 +- config/config.exs | 4 +++- docs/config.md | 3 ++- lib/pleroma/web/metadata/rel_me.ex | 13 +++++++++++++ test/web/metadata/rel_me_test.exs | 18 ++++++++++++++++++ 6 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 lib/pleroma/web/metadata/rel_me.ex create mode 100644 test/web/metadata/rel_me_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 21ad83a01..c90ff61cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: `/api/v1/notifications/destroy_multiple` (glitch-soc extension) - Mastodon API: [Reports](https://docs.joinmastodon.org/api/rest/reports/) - ActivityPub C2S: OAuth endpoints +- Metadata RelMe provider ### Changed - **Breaking:** Configuration: move from Pleroma.Mailer to Pleroma.Emails.Mailer diff --git a/README.md b/README.md index c45190fc3..987f973ea 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Pleroma -**Note**: This readme as well as complete documentation is also availible at +**Note**: This readme as well as complete documentation is also available at ## About Pleroma diff --git a/config/config.exs b/config/config.exs index 595e3505c..1114dc84d 100644 --- a/config/config.exs +++ b/config/config.exs @@ -337,7 +337,9 @@ ip: {0, 0, 0, 0}, port: 9999 -config :pleroma, Pleroma.Web.Metadata, providers: [], unfurl_nsfw: false +config :pleroma, Pleroma.Web.Metadata, + providers: [Pleroma.Web.Metadata.Providers.RelMe], + unfurl_nsfw: false config :pleroma, :suggestions, enabled: false, diff --git a/docs/config.md b/docs/config.md index d618c5dde..5a97033b2 100644 --- a/docs/config.md +++ b/docs/config.md @@ -343,9 +343,10 @@ This config contains two queues: `federator_incoming` and `federator_outgoing`. * `max_retries`: The maximum number of times a federation job is retried ## Pleroma.Web.Metadata -* `providers`: a list of metadata providers to enable. Providers availible: +* `providers`: a list of metadata providers to enable. Providers available: * Pleroma.Web.Metadata.Providers.OpenGraph * Pleroma.Web.Metadata.Providers.TwitterCard + * Pleroma.Web.Metadata.Providers.RelMe - add links from user bio with rel=me into the `
` as `` * `unfurl_nsfw`: If set to `true` nsfw attachments will be shown in previews ## :rich_media diff --git a/lib/pleroma/web/metadata/rel_me.ex b/lib/pleroma/web/metadata/rel_me.ex new file mode 100644 index 000000000..03af899c4 --- /dev/null +++ b/lib/pleroma/web/metadata/rel_me.ex @@ -0,0 +1,13 @@ +defmodule Pleroma.Web.Metadata.Providers.RelMe do + alias Pleroma.Web.Metadata.Providers.Provider + @behaviour Provider + + @impl Provider + def build_tags(%{user: user}) do + (Floki.attribute(user.bio, "link[rel~=me]", "href") ++ + Floki.attribute(user.bio, "a[rel~=me]", "href")) + |> Enum.map(fn link -> + {:link, [rel: "me", href: link], []} + end) + end +end diff --git a/test/web/metadata/rel_me_test.exs b/test/web/metadata/rel_me_test.exs new file mode 100644 index 000000000..f66bf7834 --- /dev/null +++ b/test/web/metadata/rel_me_test.exs @@ -0,0 +1,18 @@ +defmodule Pleroma.Web.Metadata.Providers.RelMeTest do + use Pleroma.DataCase + import Pleroma.Factory + alias Pleroma.Web.Metadata.Providers.RelMe + + test "it renders all links with rel='me' from user bio" do + bio = + ~s(https://some-link.com https://another-link.com + "], []}, + {:link, [rel: "me", href: "https://another-link.com"], []} + ] + end +end