From 986ae45f522daf56678a23dab3959ef5aaae0392 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Thu, 23 Mar 2023 18:37:53 +0100 Subject: [PATCH] Add worker to clean obsolete application data, token revokation and spec conformance Signed-off-by: Thomas Citharel --- .sobelow-skips | 3 +- config/config.exs | 6 +- lib/mobilizon/applications.ex | 19 ++ lib/service/auth/applications.ex | 69 +++- lib/service/workers/clean_application_data.ex | 32 ++ lib/web/controllers/application_controller.ex | 314 +++++++++++++----- lib/web/router.ex | 1 + test/mobilizon/applications_test.exs | 4 +- .../application_controller_test.exs | 148 +++++---- 9 files changed, 436 insertions(+), 160 deletions(-) create mode 100644 lib/service/workers/clean_application_data.ex diff --git a/.sobelow-skips b/.sobelow-skips index 7bc6e895d..3e438cb32 100644 --- a/.sobelow-skips +++ b/.sobelow-skips @@ -38,4 +38,5 @@ FE1EEB91EA633570F703B251AE2D4D4E 49DE9560D506F9E7EF3AFD8DA6E5564B 759F752FA0768CCC7871895DC2A5CD51 7EEC79571F3F7CEEB04A8B86D908382A -E7967805C1EA5301F2722C7BDB2F25F3 \ No newline at end of file +E7967805C1EA5301F2722C7BDB2F25F3 +BDFB0FB1AAF69C18212CBCFD42F8B717 \ No newline at end of file diff --git a/config/config.exs b/config/config.exs index 77591db87..ede8c0179 100644 --- a/config/config.exs +++ b/config/config.exs @@ -312,7 +312,11 @@ config :mobilizon, Oban, {"@hourly", Mobilizon.Service.Workers.CleanUnconfirmedUsersWorker, queue: :background}, {"@hourly", Mobilizon.Service.Workers.ExportCleanerWorker, queue: :background}, {"@hourly", Mobilizon.Service.Workers.SendActivityRecapWorker, queue: :notifications}, - {"@daily", Mobilizon.Service.Workers.CleanOldActivityWorker, queue: :background} + {"@daily", Mobilizon.Service.Workers.CleanOldActivityWorker, queue: :background}, + {"@hourly", Mobilizon.Service.Workers.CleanApplicationData, + queue: :background, args: %{type: :application_token}}, + {"@hourly", Mobilizon.Service.Workers.CleanApplicationData, + queue: :background, args: %{type: :application_device_activation}} ]}, {Oban.Plugins.Pruner, max_age: 300} ] diff --git a/lib/mobilizon/applications.ex b/lib/mobilizon/applications.ex index 6080ffe34..787fc05f7 100644 --- a/lib/mobilizon/applications.ex +++ b/lib/mobilizon/applications.ex @@ -285,6 +285,16 @@ defmodule Mobilizon.Applications do ApplicationToken.changeset(application_token, attrs) end + @spec prune_old_application_tokens(pos_integer()) :: {non_neg_integer(), nil} + def prune_old_application_tokens(lifetime) do + exp = DateTime.add(NaiveDateTime.utc_now(), -lifetime) + + ApplicationToken + |> where([at], at.status != :success) + |> where([at], at.inserted_at < ^exp) + |> Repo.delete_all() + end + alias Mobilizon.Applications.ApplicationDeviceActivation @doc """ @@ -413,4 +423,13 @@ defmodule Mobilizon.Applications do ) do ApplicationDeviceActivation.changeset(application_device_activation, attrs) end + + @spec prune_old_application_device_activations(pos_integer()) :: {non_neg_integer(), nil} + def prune_old_application_device_activations(lifetime) do + exp = DateTime.add(NaiveDateTime.utc_now(), -lifetime) + + ApplicationDeviceActivation + |> where([at], at.expires_in < ^exp) + |> Repo.delete_all() + end end diff --git a/lib/service/auth/applications.ex b/lib/service/auth/applications.ex index 95f4d4393..c5031c1ef 100644 --- a/lib/service/auth/applications.ex +++ b/lib/service/auth/applications.ex @@ -14,6 +14,12 @@ defmodule Mobilizon.Service.Auth.Applications do @app_access_tokens_ttl {8, :hour} @app_refresh_tokens_ttl {26, :week} + @device_code_expires_in 900 + @device_code_interval 5 + + @authorization_code_lifetime 60 + @application_device_activation_lifetime @device_code_expires_in * 2 + @type access_token_details :: %{ required(:access_token) => String.t(), required(:expires_in) => pos_integer(), @@ -58,7 +64,8 @@ defmodule Mobilizon.Service.Auth.Applications do user_id: user_id, application_id: app_id, authorization_code: code, - scope: scope + scope: scope, + status: :pending }) else nil -> @@ -113,22 +120,28 @@ defmodule Mobilizon.Service.Auth.Applications do | :provided_code_does_not_match | :invalid_client_secret | :invalid_or_expired + | :scope_not_included | any()} def generate_access_token(client_id, client_secret, code, redirect_uri, scope) do with {:application, %Application{ id: application_id, client_secret: app_client_secret, - redirect_uris: redirect_uris + redirect_uris: redirect_uris, + scope: app_scope }} <- {:application, Applications.get_application_by_client_id(client_id)}, - # TODO: check that app token scope still are acceptable + {:scope_included, true} <- {:scope_included, request_scope_valid?(app_scope, scope)}, {:redirect_uri, true} <- {:redirect_uri, redirect_uri in redirect_uris}, {:app_token, %ApplicationToken{} = app_token} <- {:app_token, Applications.get_application_token_by_authorization_code(code)}, + {:expired, false} <- {:expired, authorization_code_expired?(app_token)}, {:ok, %ApplicationToken{application_id: application_id_from_token} = app_token} <- - Applications.update_application_token(app_token, %{authorization_code: nil}), + Applications.update_application_token(app_token, %{ + authorization_code: nil, + status: :success + }), {:same_app, true} <- {:same_app, application_id === application_id_from_token}, {:same_client_secret, true} <- {:same_client_secret, app_client_secret == client_secret}, {:ok, access_token} <- @@ -160,6 +173,12 @@ defmodule Mobilizon.Service.Auth.Applications do {:app_token, _} -> {:error, :invalid_or_expired} + {:expired, true} -> + {:error, :invalid_or_expired} + + {:scope_included, false} -> + {:error, :scope_not_included} + {:error, err} -> {:error, err} end @@ -184,7 +203,8 @@ defmodule Mobilizon.Service.Auth.Applications do user_id: user_id, application_id: app_id, authorization_code: nil, - scope: scope + scope: scope, + status: :success }) {:ok, access_token} = @@ -205,14 +225,12 @@ defmodule Mobilizon.Service.Auth.Applications do end %ApplicationDeviceActivation{status: :incorrect_device_code} -> - Logger.error("Incorrect device code set") {:error, :incorrect_device_code} %ApplicationDeviceActivation{status: :access_denied} -> {:error, :access_denied} nil -> - Logger.error("nil returned") {:error, :incorrect_device_code} err -> @@ -231,9 +249,6 @@ defmodule Mobilizon.Service.Auth.Applications do |> Enum.join("") end - @expires_in 900 - @interval 5 - @spec register_device_code(String.t(), String.t() | nil) :: {:ok, ApplicationDeviceActivation.t()} | {:error, :application_not_found} @@ -250,7 +265,7 @@ defmodule Mobilizon.Service.Auth.Applications do Applications.create_application_device_activation(%{ device_code: device_code, user_code: user_code, - expires_in: @expires_in, + expires_in: @device_code_expires_in, application_id: application.id, scope: scope }) do @@ -260,7 +275,7 @@ defmodule Mobilizon.Service.Auth.Applications do |> Map.take([:device_code, :user_code, :expires_in]) |> Map.update!(:user_code, &user_code_displayed/1) |> Map.merge(%{ - interval: @interval, + interval: @device_code_interval, verification_uri: verification_uri })} else @@ -359,9 +374,37 @@ defmodule Mobilizon.Service.Auth.Applications do :lt end + def prune_old_tokens do + Applications.prune_old_application_tokens(@authorization_code_lifetime) + end + + def prune_old_application_device_activations do + Applications.prune_old_application_device_activations(@application_device_activation_lifetime) + end + + @spec revoke_token(String.t()) :: + {:ok, map()} + | {:error, any(), any(), any()} + | {:error, :token_not_found} + def revoke_token(token) do + case Guardian.resource_from_token(token) do + {:ok, %ApplicationToken{} = app_token, _claims} -> + Guardian.revoke(token) + revoke_application_token(app_token) + + {:error, _err} -> + {:error, :token_not_found} + end + end + + defp authorization_code_expired?(%ApplicationToken{inserted_at: inserted_at}) do + NaiveDateTime.compare(NaiveDateTime.add(inserted_at, 60), NaiveDateTime.utc_now()) == + :lt + end + defp request_scope_valid?(app_scope, request_scope) do app_scopes = app_scope |> String.split(" ") |> MapSet.new() request_scopes = request_scope |> String.split(" ") |> MapSet.new() - MapSet.subset?(request_scopes, app_scopes) + MapSet.subset?(request_scopes, app_scopes) and AppScope.scopes_valid?(request_scope) end end diff --git a/lib/service/workers/clean_application_data.ex b/lib/service/workers/clean_application_data.ex new file mode 100644 index 000000000..16e265ac5 --- /dev/null +++ b/lib/service/workers/clean_application_data.ex @@ -0,0 +1,32 @@ +defmodule Mobilizon.Service.Workers.CleanApplicationData do + @moduledoc """ + Worker to send activity recaps + """ + + use Oban.Worker, queue: "background" + alias Mobilizon.Service.Auth.Applications + require Logger + + @impl Oban.Worker + def perform(%Job{args: %{type: :application}}) do + Logger.info("Cleaning expired applications data") + + # TODO: Clear unused applications after a while + end + + @impl Oban.Worker + def perform(%Job{args: %{type: :application_token}}) do + Logger.info("Cleaning expired application tokens data") + + Applications.prune_old_tokens() + :ok + end + + @impl Oban.Worker + def perform(%Job{args: %{type: :application_device_activation}}) do + Logger.info("Cleaning expired application device activation data") + + Applications.prune_old_application_device_activations() + :ok + end +end diff --git a/lib/web/controllers/application_controller.ex b/lib/web/controllers/application_controller.ex index c633b186b..cf695fa79 100644 --- a/lib/web/controllers/application_controller.ex +++ b/lib/web/controllers/application_controller.ex @@ -22,44 +22,51 @@ defmodule Mobilizon.Web.ApplicationController do Map.get(args, "website") ) do {:ok, %Application{} = app} -> - json( - conn, + conn + |> Plug.Conn.put_resp_header("cache-control", "no-store") + |> json( Map.take(app, [:name, :website, :redirect_uris, :client_id, :client_secret, :scope]) ) {:error, :invalid_scope} -> - send_resp( - conn, - 400, - dgettext( - "errors", - "The scope parameter is not a space separated list of valid scopes" - ) - ) + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_scope", + "error_description" => + dgettext( + "errors", + "The scope parameter is not a space separated list of valid scopes" + ) + }) {:error, error} -> Logger.error(inspect(error)) - send_resp( - conn, - 500, - dgettext( - "errors", - "Impossible to create application." - ) - ) + conn + |> Plug.Conn.put_status(500) + |> json(%{ + "error" => "server_error", + "error_description" => + dgettext( + "errors", + "Impossible to create application." + ) + }) end end def create_application(conn, _args) do - send_resp( - conn, - 400, - dgettext( - "errors", - "All of name, scope and redirect_uri parameters are required to create an application" - ) - ) + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_request", + "error_description" => + dgettext( + "errors", + "All of name, scope and redirect_uri parameters are required to create an application" + ) + }) end @doc """ @@ -77,7 +84,8 @@ defmodule Mobilizon.Web.ApplicationController do state = conn.query_params["state"] scope = conn.query_params["scope"] - if is_binary(client_id) and is_binary(redirect_uri) and is_binary(state) and is_binary(scope) do + if is_binary(client_id) and is_binary(redirect_uri) and valid_uri?(redirect_uri) and + is_binary(state) and is_binary(scope) do redirect(conn, to: Routes.page_path(conn, :authorize, @@ -88,46 +96,89 @@ defmodule Mobilizon.Web.ApplicationController do ) ) else - send_resp( - conn, - 400, - dgettext( - "errors", - "You need to specify client_id, redirect_uri, scope and state to autorize an application" + if is_binary(redirect_uri) and valid_uri?(redirect_uri) do + redirect(conn, + external: + append_parameters(redirect_uri, + error: "invalid_request", + error_description: + dgettext( + "errors", + "You need to specify client_id, redirect_uri, scope and state to autorize an application" + ) + ) ) - ) + else + send_resp( + conn, + 400, + dgettext( + "errors", + "You need to provide a valid redirect_uri to autorize an application" + ) + ) + end end end def device_code(conn, %{"client_id" => client_id, "scope" => scope}) do case Applications.register_device_code(client_id, scope) do {:ok, res} when is_map(res) -> - case get_format(conn) do - "json" -> - json(conn, res) - - _ -> - send_resp(conn, 200, URI.encode_query(res)) - end + conn + |> Plug.Conn.put_resp_header("cache-control", "no-store") + |> json(res) {:error, :scope_not_included} -> - send_resp(conn, 400, "The given scope is not in the list of the app declared scopes") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_scope", + "error_description" => + dgettext( + "errors", + "The given scope is not in the list of the app declared scopes" + ) + }) {:error, :application_not_found} -> - send_resp(conn, 400, "No application with this client_id was found") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_client", + "error_description" => + dgettext( + "errors", + "No application with this client_id was found" + ) + }) {:error, %Ecto.Changeset{} = err} -> Logger.error(inspect(err)) - send_resp(conn, 500, "Unable to produce device code") + + conn + |> Plug.Conn.put_status(500) + |> json(%{ + "error" => "server_error", + "error_description" => + dgettext( + "errors", + "Unable to produce device code" + ) + }) end end def device_code(conn, _args) do - send_resp( - conn, - 400, - "You need to pass both client_id and scope as parameters to obtain a device code" - ) + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_request", + "error_description" => + dgettext( + "errors", + "You need to pass both client_id and scope as parameters to obtain a device code" + ) + }) end @spec generate_access_token(Plug.Conn.t(), map()) :: Plug.Conn.t() @@ -141,11 +192,19 @@ defmodule Mobilizon.Web.ApplicationController do }) do case do_generate_access_token(client_id, client_secret, code, redirect_uri, scope) do {:ok, token} -> - json(conn, token) + conn + |> Plug.Conn.put_resp_header("cache-control", "no-store") + |> json(token) - {:error, msg} -> + {:error, code, msg} -> Logger.debug(msg) - json(conn, %{error: true, details: msg}) + + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => to_string(code), + "error_description" => msg + }) end end @@ -156,26 +215,45 @@ defmodule Mobilizon.Web.ApplicationController do }) do case Applications.generate_access_token_for_device_flow(client_id, device_code) do {:ok, res} -> - case get_format(conn) do - "json" -> - json(conn, res) - - _ -> - send_resp( - conn, - 200, - URI.encode_query(res) - ) - end + conn + |> Plug.Conn.put_resp_header("cache-control", "no-store") + |> json(res) {:error, :incorrect_device_code} -> - send_resp(conn, 400, "The client_id provided or the device_code associated is invalid") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_grant", + "error_description" => + dgettext( + "errors", + "The client_id provided or the device_code associated is invalid" + ) + }) {:error, :access_denied} -> - send_resp(conn, 401, "The user rejected the requested authorization") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "access_denied", + "error_description" => + dgettext( + "errors", + "The user rejected the requested authorization" + ) + }) {:error, :expired} -> - send_resp(conn, 400, "The given device_code has expired") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_grant", + "error_description" => + dgettext( + "errors", + "The given device_code has expired" + ) + }) end end @@ -187,29 +265,74 @@ defmodule Mobilizon.Web.ApplicationController do }) do case Applications.refresh_tokens(refresh_token, client_id, client_secret) do {:ok, res} -> - json(conn, res) + conn + |> Plug.Conn.put_resp_header("cache-control", "no-store") + |> json(res) {:error, :invalid_client_credentials} -> - send_resp(conn, 400, "Invalid client credentials provided") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_client", + "error_description" => dgettext("errors", "Invalid client credentials provided") + }) {:error, :invalid_refresh_token} -> - send_resp(conn, 400, "Invalid refresh token provided") + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_grant", + "error_description" => dgettext("errors", "Invalid refresh token provided") + }) {:error, err} when is_atom(err) -> - send_resp(conn, 500, to_string(err)) + conn + |> Plug.Conn.put_status(500) + |> json(%{ + "error" => "server_error", + "error_description" => to_string(err) + }) end end def generate_access_token(conn, _args) do - send_resp( - conn, - 400, - "Incorrect parameters sent. You need to provide at least the grant_type and client_id parameters, depending on the grant type being used." - ) + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_request", + "error_description" => + dgettext( + "errors", + "Incorrect parameters sent. You need to provide at least the grant_type and client_id parameters, depending on the grant type being used." + ) + }) + end + + def revoke_token(conn, %{"token" => token} = _args) do + case Applications.revoke_token(token) do + {:ok, _res} -> + send_resp(conn, 200, "") + + {:error, _, _, _} -> + conn + |> Plug.Conn.put_status(500) + |> json(%{ + "error" => "server_error", + "error_description" => dgettext("errors", "Unable to revoke token") + }) + + {:error, :token_not_found} -> + conn + |> Plug.Conn.put_status(:not_found) + |> json(%{ + "error" => "invalid_request", + "error_description" => dgettext("errors", "Token not found") + }) + end end @spec do_generate_access_token(String.t(), String.t(), String.t(), String.t(), String.t()) :: - {:ok, Applications.access_token_details()} | {:error, String.t()} + {:ok, Applications.access_token_details()} | {:error, atom(), String.t()} defp do_generate_access_token(client_id, client_secret, code, redirect_uri, scope) do case Applications.generate_access_token( client_id, @@ -222,19 +345,48 @@ defmodule Mobilizon.Web.ApplicationController do {:ok, token} {:error, :application_not_found} -> - {:error, dgettext("errors", "No application was found with this client_id")} + {:error, :invalid_request, + dgettext("errors", "No application was found with this client_id")} {:error, :redirect_uri_not_in_allowed} -> - {:error, dgettext("errors", "This redirect URI is not allowed")} + {:error, :invalid_request, dgettext("errors", "This redirect URI is not allowed")} {:error, :invalid_or_expired} -> - {:error, dgettext("errors", "The provided code is invalid or expired")} + {:error, :invalid_grant, dgettext("errors", "The provided code is invalid or expired")} {:error, :provided_code_does_not_match} -> - {:error, dgettext("errors", "The provided client_id does not match the provided code")} + {:error, :invalid_grant, + dgettext("errors", "The provided client_id does not match the provided code")} {:error, :invalid_client_secret} -> - {:error, dgettext("errors", "The provided client_secret is invalid")} + {:error, :invalid_client, dgettext("errors", "The provided client_secret is invalid")} + + {:error, :scope_not_included} -> + {:error, :invalid_scope, + dgettext( + "errors", + "The provided scope is invalid or not included in the app declared scopes" + )} + end + end + + defp valid_uri?(url) do + uri = URI.parse(url) + uri.scheme != nil and uri.host =~ "." + end + + @spec append_parameters(String.t(), Enum.t()) :: String.t() + defp append_parameters(uri_str, parameters) do + query_parameters = URI.encode_query(parameters) + + case URI.parse(uri_str) do + %URI{query: nil} = uri -> + uri + |> URI.merge(%URI{query: query_parameters}) + |> URI.to_string() + + uri -> + "#{URI.to_string(uri)}&#{query_parameters}" end end end diff --git a/lib/web/router.ex b/lib/web/router.ex index 06496b325..2657dea21 100644 --- a/lib/web/router.ex +++ b/lib/web/router.ex @@ -221,6 +221,7 @@ defmodule Mobilizon.Web.Router do post("/login/device/code", ApplicationController, :device_code) post("/oauth/token", ApplicationController, :generate_access_token) + post("/oauth/revoke", ApplicationController, :revoke_token) end scope "/proxy/", Mobilizon.Web do diff --git a/test/mobilizon/applications_test.exs b/test/mobilizon/applications_test.exs index 9e8fd57c6..2431d3159 100644 --- a/test/mobilizon/applications_test.exs +++ b/test/mobilizon/applications_test.exs @@ -183,7 +183,7 @@ defmodule Mobilizon.ApplicationsTest do assert {:ok, %ApplicationDeviceActivation{} = application_device_activation} = Applications.create_application_device_activation(valid_attrs) - assert application_device_activation == "read" + assert application_device_activation.scope == "read" end test "create_application_device_activation/1 with invalid data returns error changeset" do @@ -204,7 +204,7 @@ defmodule Mobilizon.ApplicationsTest do update_attrs ) - assert application_device_activation == "success" + assert application_device_activation.status == :success end test "update_application_device_activation/2 with invalid data returns error changeset" do diff --git a/test/web/controllers/application_controller_test.exs b/test/web/controllers/application_controller_test.exs index fd1f0d7f3..6e12c661d 100644 --- a/test/web/controllers/application_controller_test.exs +++ b/test/web/controllers/application_controller_test.exs @@ -11,7 +11,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do conn |> post("/apps", %{"name" => "hello"}) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_request" + + assert error["error_description"] == "All of name, scope and redirect_uri parameters are required to create an application" end @@ -25,7 +28,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do "scope" => "write nothing" }) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_scope" + + assert error["error_description"] == "The scope parameter is not a space separated list of valid scopes" end @@ -57,18 +63,29 @@ defmodule Mobilizon.Web.ApplicationControllerTest do end describe "authorize" do - test "without all required params", %{conn: conn} do - conn = get(conn, "/oauth/authorize?client_id=hello") + test "without a valid URI", %{conn: conn} do + conn = get(conn, "/oauth/authorize?client_id=hello&redirect_uri=toto") assert response(conn, 400) == - "You need to specify client_id, redirect_uri, scope and state to autorize an application" + "You need to provide a valid redirect_uri to autorize an application" + end + + test "without all valid params", %{conn: conn} do + conn = + get( + conn, + "/oauth/authorize?client_id=hello&redirect_uri=#{URI.encode("https://somewhere.org/callback")}" + ) + + assert redirected_to(conn) =~ + "error=invalid_request&error_description=#{URI.encode_www_form("You need to specify client_id, redirect_uri, scope and state to autorize an application")}" end test "with all required params redirects to authorization page", %{conn: conn} do conn = get( conn, - "/oauth/authorize?client_id=hello&redirect_uri=somewhere&state=something&scope=everything" + "/oauth/authorize?client_id=hello&redirect_uri=#{URI.encode("https://somewhere.org/callback&state=something&scope=everything")}" ) assert redirected_to(conn) =~ "/oauth/autorize_approve" @@ -79,14 +96,19 @@ defmodule Mobilizon.Web.ApplicationControllerTest do test "without all required params", %{conn: conn} do conn = post(conn, "/login/device/code", client_id: "hello") - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_request" + + assert error["error_description"] == "You need to pass both client_id and scope as parameters to obtain a device code" end test "with an invalid client_id", %{conn: conn} do conn = post(conn, "/login/device/code", client_id: "hello", scope: "write:event:create") - assert response(conn, 400) == "No application with this client_id was found" + assert error = json_response(conn, 400) + assert error["error"] == "invalid_client" + assert error["error_description"] == "No application with this client_id was found" end test "with a scope not matching app registered scopes", %{conn: conn} do @@ -96,7 +118,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do conn = post(conn, "/login/device/code", client_id: app.client_id, scope: "write:event:delete") - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_scope" + + assert error["error_description"] == "The given scope is not in the list of the app declared scopes" end @@ -107,14 +132,14 @@ defmodule Mobilizon.Web.ApplicationControllerTest do conn = post(conn, "/login/device/code", client_id: app.client_id, scope: "write:event:create") - res = conn |> response(200) |> URI.decode_query() + res = json_response(conn, 200) verification_uri = Routes.page_url(Mobilizon.Web.Endpoint, :auth_device) assert %{ "device_code" => _device_code, - "expires_in" => "900", - "interval" => "5", + "expires_in" => 900, + "interval" => 5, "user_code" => user_code, "verification_uri" => ^verification_uri } = res @@ -151,7 +176,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do test "without valid parameters", %{conn: conn} do conn = post(conn, "/oauth/token") - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_request" + + assert error["error_description"] == "Incorrect parameters sent. You need to provide at least the grant_type and client_id parameters, depending on the grant type being used." end @@ -163,7 +191,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do device_code: "hello" ) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_grant" + + assert error["error_description"] == "The client_id provided or the device_code associated is invalid" end @@ -188,7 +219,9 @@ defmodule Mobilizon.Web.ApplicationControllerTest do device_code: "hello" ) - assert response(conn, 401) == "The user rejected the requested authorization" + assert error = json_response(conn, 400) + assert error["error"] == "access_denied" + assert error["error_description"] == "The user rejected the requested authorization" end test "with incorrect device code", %{conn: conn} do @@ -212,7 +245,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do device_code: "hello" ) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_grant" + + assert error["error_description"] == "The client_id provided or the device_code associated is invalid" end @@ -240,46 +276,13 @@ defmodule Mobilizon.Web.ApplicationControllerTest do device_code: "hello" ) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_grant" + + assert error["error_description"] == "The given device_code has expired" end - test "with valid params", %{conn: conn} do - user = insert(:user) - - {:ok, app} = - Applications.create("My app", ["hello"], "write:event:create write:event:update") - - assert {:ok, _res} = - Mobilizon.Applications.create_application_device_activation(%{ - device_code: "hello", - user_code: "world", - expires_in: 600, - application_id: app.id, - scope: "write:event:create write:event:update", - status: "success", - user_id: user.id - }) - - conn = - post(conn, "/oauth/token", - grant_type: "urn:ietf:params:oauth:grant-type:device_code", - client_id: app.client_id, - device_code: "hello" - ) - - res = conn |> response(200) |> URI.decode_query() - - assert %{ - "access_token" => _access_token, - "expires_in" => "28800", - "refresh_token" => _refresh_token, - "refresh_token_expires_in" => "15724800", - "scope" => "write:event:create write:event:update", - "token_type" => "bearer" - } = res - end - test "with valid params as JSON", %{conn: conn} do user = insert(:user) @@ -331,7 +334,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do scope: "hello" ) - assert json_response(conn, 200)["details"] == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_request" + + assert json_response(conn, 400)["error_description"] == "No application was found with this client_id" end @@ -346,10 +352,13 @@ defmodule Mobilizon.Web.ApplicationControllerTest do client_secret: app.client_secret, code: "hello", redirect_uri: "nope", - scope: "hello" + scope: "write:event:create" ) - assert json_response(conn, 200)["details"] == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_request" + + assert error["error_description"] == "This redirect URI is not allowed" end @@ -364,10 +373,13 @@ defmodule Mobilizon.Web.ApplicationControllerTest do client_secret: app.client_secret, code: "hello", redirect_uri: "hello", - scope: "hello" + scope: "write:event:create" ) - assert json_response(conn, 200)["details"] == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_grant" + + assert error["error_description"] == "The provided code is invalid or expired" end @@ -394,7 +406,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do scope: "write:event:create write:event:update" ) - assert json_response(conn, 200)["details"] == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_client" + + assert error["error_description"] == "The provided client_secret is invalid" end @@ -424,7 +439,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do scope: "write:event:create write:event:update" ) - assert json_response(conn, 200)["details"] == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_grant" + + assert error["error_description"] == "The provided client_id does not match the provided code" end @@ -474,7 +492,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do refresh_token: "none" ) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_grant" + + assert error["error_description"] == "Invalid refresh token provided" end @@ -511,7 +532,10 @@ defmodule Mobilizon.Web.ApplicationControllerTest do refresh_token: res["refresh_token"] ) - assert response(conn, 400) == + assert error = json_response(conn, 400) + assert error["error"] == "invalid_client" + + assert error["error_description"] == "Invalid client credentials provided" end