From d11c0ede3a2f8ffbca23e7382296cd9125f1365d Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Sun, 19 Jul 2020 19:37:54 -0500 Subject: [PATCH] Let the OAuth form remember you, fixes #1909 --- lib/pleroma/web/oauth/oauth_controller.ex | 21 ++++-- lib/pleroma/web/router.ex | 2 +- .../web/templates/o_auth/o_auth/show.html.eex | 60 ++++++++++++------ priv/static/instance/static.css | Bin 3485 -> 3718 bytes test/web/oauth/oauth_controller_test.exs | 37 +++++++++++ 5 files changed, 93 insertions(+), 27 deletions(-) diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index 7683589cf..5c93f96f1 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -76,8 +76,17 @@ defp do_authorize(%Plug.Conn{} = conn, params) do available_scopes = (app && app.scopes) || [] scopes = Scopes.fetch_scopes(params, available_scopes) + user = + with %{assigns: %{user: %User{} = user}} <- conn do + user + else + _ -> nil + end + # Note: `params` might differ from `conn.params`; use `@params` not `@conn.params` in template render(conn, Authenticator.auth_template(), %{ + app: app && Map.delete(app, :client_secret), + user: user, response_type: params["response_type"], client_id: params["client_id"], available_scopes: available_scopes, @@ -121,11 +130,13 @@ defp handle_existing_authorization( end end - def create_authorization( - %Plug.Conn{} = conn, - %{"authorization" => _} = params, - opts \\ [] - ) do + def create_authorization(_, _, opts \\ []) + + def create_authorization(%Plug.Conn{assigns: %{user: %User{} = user}} = conn, params, []) do + create_authorization(conn, params, user: user) + end + + def create_authorization(%Plug.Conn{} = conn, %{"authorization" => _} = params, opts) do with {:ok, auth, user} <- do_create_authorization(conn, params, opts[:user]), {:mfa_required, _, _, false} <- {:mfa_required, user, auth, MFA.require?(user)} do after_create_authorization(conn, auth, params) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 05841a8c4..bb74affa3 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -278,9 +278,9 @@ defmodule Pleroma.Web.Router do scope [] do pipe_through([:oauth, :after_auth]) get("/authorize", OAuthController, :authorize) + post("/authorize", OAuthController, :create_authorization) end - post("/authorize", OAuthController, :create_authorization) post("/token", OAuthController, :token_exchange) post("/revoke", OAuthController, :token_revoke) get("/registration_details", OAuthController, :registration_details) 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 b17142ff8..d7efbf184 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 @@ -5,32 +5,51 @@ <% end %> -

OAuth Authorization

<%= form_for @conn, o_auth_path(@conn, :authorize), [as: "authorization"], fn f -> %> -<%= if @params["registration"] in ["true", true] do %> -

This is the first time you visit! Please enter your Pleroma handle.

-

Choose carefully! You won't be able to change this later. You will be able to change your display name, though.

-
- <%= label f, :nickname, "Pleroma Handle" %> - <%= text_input f, :nickname, placeholder: "lain" %> +<%= if @user do %> + - <%= hidden_input f, :name, value: @params["name"] %> - <%= hidden_input f, :password, value: @params["password"] %> -
-<% else %> -
- <%= label f, :name, "Username" %> - <%= text_input f, :name %> -
-
- <%= label f, :password, "Password" %> - <%= password_input f, :password %> -
- <%= submit "Log In" %> +<% end %> + +<%= if @app do %> +

Application <%= @app.client_name %> is requesting access to your account.

<%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %> <% end %> +<%= if @user do %> + <%= submit "Authorize" %> +<% else %> + <%= if @params["registration"] in ["true", true] do %> +

This is the first time you visit! Please enter your Pleroma handle.

+

Choose carefully! You won't be able to change this later. You will be able to change your display name, though.

+
+ <%= label f, :nickname, "Pleroma Handle" %> + <%= text_input f, :nickname, placeholder: "lain" %> +
+ <%= hidden_input f, :name, value: @params["name"] %> + <%= hidden_input f, :password, value: @params["password"] %> +
+ <% else %> +
+ <%= label f, :name, "Username" %> + <%= text_input f, :name %> +
+
+ <%= label f, :password, "Password" %> + <%= password_input f, :password %> +
+ <%= submit "Log In" %> + <% end %> +<% end %> + <%= hidden_input f, :client_id, value: @client_id %> <%= hidden_input f, :response_type, value: @response_type %> <%= hidden_input f, :redirect_uri, value: @redirect_uri %> @@ -40,4 +59,3 @@ <%= if Pleroma.Config.oauth_consumer_enabled?() do %> <%= render @view_module, Pleroma.Web.Auth.Authenticator.oauth_consumer_template(), assigns %> <% end %> - diff --git a/priv/static/instance/static.css b/priv/static/instance/static.css index 8e2e071efad6873e169b69c51d2c84adffb660be..decd3edb859d6091a7a7ee210ea6f7c7638d9308 100644 GIT binary patch delta 196 zcmbO$-6p$XF?YRQVsdhRXEp&OE0l3u_O_vnH9R_nVcyl8CD8r eKyxQAWD`~e(s)hE%uQFQo-D|%ws`^fUq%4fxj<9^ delta 12 TcmZpZoh!XzG56*ko?narAg2WY diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index d389e4ce0..9e6eb9805 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -606,6 +606,43 @@ test "redirects with oauth authorization, " <> end end + test "authorize from cookie" do + app_scopes = ["read", "write"] + app = insert(:oauth_app) + redirect_uri = OAuthController.default_redirect_uri(app) + user = insert(:user) + + conn = + build_conn() + |> Plug.Session.call(Plug.Session.init(@session_opts)) + |> fetch_session() + |> put_session(:user_id, user.id) + |> post( + "/oauth/authorize", + %{ + "authorization" => %{ + "name" => user.nickname, + "client_id" => app.client_id, + "redirect_uri" => redirect_uri, + "scope" => app_scopes, + "state" => "statepassed" + } + } + ) + + assert Enum.count(Repo.all(Pleroma.Web.OAuth.Authorization)) == 1 + + target = redirected_to(conn) + assert target =~ redirect_uri + + query = URI.parse(target).query |> URI.query_decoder() |> Map.new() + + assert %{"state" => "statepassed", "code" => code} = query + auth = Repo.get_by(Authorization, token: code) + assert auth + assert auth.scopes == app_scopes + end + test "redirect to on two-factor auth page" do otp_secret = TOTP.generate_secret()