From 9256d2d4b40bc3f48c9bb2d54f868dbc5c4c2f89 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 21 Apr 2019 10:51:32 +0300 Subject: [PATCH] [#923] Refactored OAuthController#authorize definitions, added test. --- lib/pleroma/web/oauth/oauth_controller.ex | 25 ++++++++++++++--------- test/web/oauth/oauth_controller_test.exs | 21 +++++++++++++++++++ 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index 9874bac23..5ea04635d 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -23,6 +23,12 @@ defmodule Pleroma.Web.OAuth.OAuthController do action_fallback(Pleroma.Web.OAuth.FallbackController) + # Note: this definition is only called from error-handling methods with `conn.params` as 2nd arg + def authorize(conn, %{"authorization" => _} = params) do + {auth_attrs, params} = Map.pop(params, "authorization") + authorize(conn, Map.merge(params, auth_attrs)) + end + def authorize(%{assigns: %{token: %Token{} = token}} = conn, params) do if ControllerHelper.truthy_param?(params["force_login"]) do do_authorize(conn, params) @@ -44,21 +50,20 @@ 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: do_authorize(conn, auth_attrs) - - defp do_authorize(conn, auth_attrs) do - app = Repo.get_by(App, client_id: auth_attrs["client_id"]) + defp do_authorize(conn, params) do + app = Repo.get_by(App, client_id: params["client_id"]) available_scopes = (app && app.scopes) || [] - scopes = oauth_scopes(auth_attrs, nil) || available_scopes + scopes = oauth_scopes(params, nil) || available_scopes + # Note: `params` might differ from `conn.params`; use `@params` not `@conn.params` in template render(conn, Authenticator.auth_template(), %{ - response_type: auth_attrs["response_type"], - client_id: auth_attrs["client_id"], + response_type: params["response_type"], + client_id: params["client_id"], available_scopes: available_scopes, scopes: scopes, - redirect_uri: auth_attrs["redirect_uri"], - state: auth_attrs["state"], - params: auth_attrs + redirect_uri: params["redirect_uri"], + state: params["state"], + params: params }) end diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index fb505fab3..6e96537ec 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -365,6 +365,27 @@ test "renders authentication page", %{app: app, conn: conn} do assert html_response(conn, 200) =~ ~s(type="submit") end + test "properly handles internal calls with `authorization`-wrapped params", %{ + app: app, + conn: conn + } do + conn = + get( + conn, + "/oauth/authorize", + %{ + "authorization" => %{ + "response_type" => "code", + "client_id" => app.client_id, + "redirect_uri" => app.redirect_uris, + "scope" => "read" + } + } + ) + + assert html_response(conn, 200) =~ ~s(type="submit") + end + test "renders authentication page if user is already authenticated but `force_login` is tru-ish", %{app: app, conn: conn} do token = insert(:oauth_token, app_id: app.id)