From 2a95014b9d7142aa2549e70f428293af78fae8eb Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 27 Mar 2019 15:39:35 +0300 Subject: [PATCH] [#923] OAuth consumer improvements, fixes, refactoring. --- config/config.exs | 5 +--- lib/pleroma/web/auth/authenticator.ex | 6 ++++ lib/pleroma/web/auth/ldap_authenticator.ex | 2 ++ lib/pleroma/web/auth/pleroma_authenticator.ex | 2 ++ lib/pleroma/web/oauth/oauth_controller.ex | 28 +++++++++++++------ lib/pleroma/web/router.ex | 1 + .../templates/o_auth/o_auth/_scopes.html.eex | 13 +++++++++ .../templates/o_auth/o_auth/consumer.html.eex | 15 ++++++---- .../web/templates/o_auth/o_auth/show.html.eex | 16 ++--------- mix.lock | 7 +---- 10 files changed, 59 insertions(+), 36 deletions(-) create mode 100644 lib/pleroma/web/templates/o_auth/o_auth/_scopes.html.eex diff --git a/config/config.exs b/config/config.exs index 586844516..bdaf5205a 100644 --- a/config/config.exs +++ b/config/config.exs @@ -385,10 +385,7 @@ oauth_consumer_strategies = String.split(System.get_env("OAUTH_CONSUMER_STRATEGI ueberauth_providers = for strategy <- oauth_consumer_strategies do - strategy_module_name = - System.get_env("UEBERAUTH_#{String.upcase(strategy)}_STRATEGY_MODULE") || - "Elixir.Ueberauth.Strategy.#{String.capitalize(strategy)}" - + strategy_module_name = "Elixir.Ueberauth.Strategy.#{String.capitalize(strategy)}" strategy_module = String.to_atom(strategy_module_name) {String.to_atom(strategy), {strategy_module, [callback_params: ["state"]]}} end diff --git a/lib/pleroma/web/auth/authenticator.ex b/lib/pleroma/web/auth/authenticator.ex index 1f614668c..bb87b323c 100644 --- a/lib/pleroma/web/auth/authenticator.ex +++ b/lib/pleroma/web/auth/authenticator.ex @@ -33,4 +33,10 @@ defmodule Pleroma.Web.Auth.Authenticator do def auth_template do implementation().auth_template() || Pleroma.Config.get(:auth_template, "show.html") end + + @callback oauth_consumer_template() :: String.t() | nil + def oauth_consumer_template do + implementation().oauth_consumer_template() || + Pleroma.Config.get(:oauth_consumer_template, "consumer.html") + end end diff --git a/lib/pleroma/web/auth/ldap_authenticator.ex b/lib/pleroma/web/auth/ldap_authenticator.ex index 65abd7f38..8b6d5a77f 100644 --- a/lib/pleroma/web/auth/ldap_authenticator.ex +++ b/lib/pleroma/web/auth/ldap_authenticator.ex @@ -51,6 +51,8 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do 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 60847ce6a..8b190f97f 100644 --- a/lib/pleroma/web/auth/pleroma_authenticator.ex +++ b/lib/pleroma/web/auth/pleroma_authenticator.ex @@ -92,4 +92,6 @@ defmodule Pleroma.Web.Auth.PleromaAuthenticator do end def auth_template, do: nil + + def oauth_consumer_template, do: nil end diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index b300c96df..078839d5c 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -174,6 +174,25 @@ defmodule Pleroma.Web.OAuth.OAuthController do end end + def prepare_request(conn, %{"provider" => provider} = params) do + scope = + oauth_scopes(params, []) + |> Enum.join(" ") + + state = + params + |> Map.delete("scopes") + |> Map.put("scope", scope) + |> Poison.encode!() + + params = + params + |> Map.drop(~w(scope scopes client_id redirect_uri)) + |> Map.put("state", state) + + redirect(conn, to: o_auth_path(conn, :request, provider, params)) + end + def request(conn, params) do message = if params["provider"] do @@ -235,14 +254,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do end defp callback_params(%{"state" => state} = params) do - [client_id, redirect_uri, scope, state] = String.split(state, "|") - - Map.merge(params, %{ - "client_id" => client_id, - "redirect_uri" => redirect_uri, - "scope" => scope, - "state" => state - }) + Map.merge(params, Poison.decode!(state)) end def registration_details(conn, params) do diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index f2cec574b..4d0e04d9f 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -213,6 +213,7 @@ defmodule Pleroma.Web.Router do scope [] do pipe_through(:browser) + get("/prepare_request", OAuthController, :prepare_request) get("/:provider", OAuthController, :request) get("/:provider/callback", OAuthController, :callback) post("/register", OAuthController, :register) 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 new file mode 100644 index 000000000..4b8fb5dae --- /dev/null +++ b/lib/pleroma/web/templates/o_auth/o_auth/_scopes.html.eex @@ -0,0 +1,13 @@ +
+ <%= label @form, :scope, "Permissions" %> + +
+ <%= 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[]" %> + <%= 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 a64859a49..002f014e6 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 @@ -2,9 +2,14 @@

Sign in with external provider

-<%= for strategy <- Pleroma.Config.get([:auth, :oauth_consumer_strategies], []) do %> - <%= form_for @conn, o_auth_path(@conn, :request, strategy), [method: "get"], fn f -> %> - <%= hidden_input f, :state, value: Enum.join([@client_id, @redirect_uri, Enum.join(@available_scopes, " "), @state], "|") %> - <%= submit "Sign in with #{String.capitalize(strategy)}" %> - <% end %> +<%= form_for @conn, o_auth_path(@conn, :prepare_request), [method: "get"], fn f -> %> + <%= render @view_module, "_scopes.html", Map.put(assigns, :form, f) %> + + <%= hidden_input f, :client_id, value: @client_id %> + <%= hidden_input f, :redirect_uri, value: @redirect_uri %> + <%= hidden_input f, :state, value: @state %> + + <%= for strategy <- Pleroma.Config.get([:auth, :oauth_consumer_strategies], []) do %> + <%= submit "Sign in with #{String.capitalize(strategy)}", name: "provider", value: strategy %> + <% end %> <% 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 b2381869a..e6cf1db45 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 @@ -16,18 +16,8 @@ <%= label f, :password, "Password" %> <%= password_input f, :password %> -
-<%= label f, :scope, "Permissions" %> -
- <%= 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 f, :"scope_#{scope}", value: scope in @scopes && scope, checked_value: scope, unchecked_value: "", name: "authorization[scope][]" %> - <%= label f, :"scope_#{scope}", String.capitalize(scope) %> -
- <% end %> -
-
+ +<%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f, scope_param: "authorization[scope][]"}) %> <%= hidden_input f, :client_id, value: @client_id %> <%= hidden_input f, :response_type, value: @response_type %> @@ -37,5 +27,5 @@ <% end %> <%= if Pleroma.Config.get([:auth, :oauth_consumer_enabled]) do %> - <%= render @view_module, "consumer.html", assigns %> + <%= render @view_module, Pleroma.Web.Auth.Authenticator.oauth_consumer_template(), assigns %> <% end %> diff --git a/mix.lock b/mix.lock index 6a6cee1a9..ee8617124 100644 --- a/mix.lock +++ b/mix.lock @@ -43,9 +43,6 @@ "mock": {:hex, :mock, "0.3.1", "994f00150f79a0ea50dc9d86134cd9ebd0d177ad60bd04d1e46336cdfdb98ff9", [:mix], [{:meck, "~> 0.8.8", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm"}, "mogrify": {:hex, :mogrify, "0.6.1", "de1b527514f2d95a7bbe9642eb556061afb337e220cf97adbf3a4e6438ed70af", [:mix], [], "hexpm"}, "nimble_parsec": {:hex, :nimble_parsec, "0.4.0", "ee261bb53214943679422be70f1658fff573c5d0b0a1ecd0f18738944f818efe", [:mix], [], "hexpm"}, - "oauth": {:git, "https://github.com/tim/erlang-oauth.git", "bd19896e31125f99ff45bb5850b1c0e74b996743", []}, - "oauth2": {:hex, :oauth2, "0.9.4", "632e8e8826a45e33ac2ea5ac66dcc019ba6bb5a0d2ba77e342d33e3b7b252c6e", [:mix], [{:hackney, "~> 1.7", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, - "oauther": {:hex, :oauther, "1.1.1", "7d8b16167bb587ecbcddd3f8792beb9ec3e7b65c1f8ebd86b8dd25318d535752", [:mix], [], "hexpm"}, "parse_trans": {:hex, :parse_trans, "3.3.0", "09765507a3c7590a784615cfd421d101aec25098d50b89d7aa1d66646bc571c1", [:rebar3], [], "hexpm"}, "pbkdf2_elixir": {:hex, :pbkdf2_elixir, "0.12.3", "6706a148809a29c306062862c803406e88f048277f6e85b68faf73291e820b84", [:mix], [], "hexpm"}, "phoenix": {:hex, :phoenix, "1.4.1", "801f9d632808657f1f7c657c8bbe624caaf2ba91429123ebe3801598aea4c3d9", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 1.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 1.0 or ~> 2.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm"}, @@ -66,9 +63,7 @@ "timex": {:hex, :timex, "3.5.0", "b0a23167da02d0fe4f1a4e104d1f929a00d348502b52432c05de875d0b9cffa5", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, repo: "hexpm", optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm"}, "trailing_format_plug": {:hex, :trailing_format_plug, "0.0.7", "64b877f912cf7273bed03379936df39894149e35137ac9509117e59866e10e45", [:mix], [{:plug, "> 0.12.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, "tzdata": {:hex, :tzdata, "0.5.17", "50793e3d85af49736701da1a040c415c97dc1caf6464112fd9bd18f425d3053b", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, - "ueberauth": {:hex, :ueberauth, "0.5.0", "4570ec94d7f784dc4c4aa94c83391dbd9b9bd7b66baa30e95a666c5ec1b168b1", [:mix], [{:plug, "~> 1.2", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, - "ueberauth_facebook": {:hex, :ueberauth_facebook, "0.8.0", "9ec8571f804dd5c06f4e305d70606b39fc0ac8a8f43ed56ebb76012a97d14729", [:mix], [{:oauth2, "~> 0.9", [hex: :oauth2, repo: "hexpm", optional: false]}, {:ueberauth, "~> 0.4", [hex: :ueberauth, repo: "hexpm", optional: false]}], "hexpm"}, - "ueberauth_twitter": {:hex, :ueberauth_twitter, "0.2.4", "770ac273cc696cde986582e7a36df0923deb39fa3deff0152fbf150343809f81", [:mix], [{:httpoison, "~> 0.7", [hex: :httpoison, repo: "hexpm", optional: false]}, {:oauther, "~> 1.1", [hex: :oauther, repo: "hexpm", optional: false]}, {:poison, "~> 1.3 or ~> 2.0", [hex: :poison, repo: "hexpm", optional: false]}, {:ueberauth, "~> 0.2", [hex: :ueberauth, repo: "hexpm", optional: false]}], "hexpm"}, + "ueberauth": {:hex, :ueberauth, "0.6.1", "9e90d3337dddf38b1ca2753aca9b1e53d8a52b890191cdc55240247c89230412", [:mix], [{:plug, "~> 1.5", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, "unicode_util_compat": {:hex, :unicode_util_compat, "0.4.1", "d869e4c68901dd9531385bb0c8c40444ebf624e60b6962d95952775cac5e90cd", [:rebar3], [], "hexpm"}, "unsafe": {:hex, :unsafe, "1.0.0", "7c21742cd05380c7875546b023481d3a26f52df8e5dfedcb9f958f322baae305", [:mix], [], "hexpm"}, "web_push_encryption": {:hex, :web_push_encryption, "0.2.1", "d42cecf73420d9dc0053ba3299cc8c8d6ff2be2487d67ca2a57265868e4d9a98", [:mix], [{:httpoison, "~> 1.0", [hex: :httpoison, repo: "hexpm", optional: false]}, {:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: false]}, {:poison, "~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"}, -- 2.45.2