Add worker to clean obsolete application data, token revokation and spec conformance

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

.sobelow-skips

@@ -38,4 +38,5 @@ FE1EEB91EA633570F703B251AE2D4D4E
 49DE9560D506F9E7EF3AFD8DA6E5564B
 759F752FA0768CCC7871895DC2A5CD51
 7EEC79571F3F7CEEB04A8B86D908382A
-E7967805C1EA5301F2722C7BDB2F25F3
+E7967805C1EA5301F2722C7BDB2F25F3
+BDFB0FB1AAF69C18212CBCFD42F8B717

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}
   ]

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

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

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

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
-          conn,
+        |> Plug.Conn.put_status(400)
-          400,
+        |> json(%{
-          dgettext(
+          "error" => "invalid_scope",
-            "errors",
+          "error_description" =>
-            "The scope parameter is not a space separated list of valid scopes"
+            dgettext(
-          )
+              "errors",
-        )
+              "The scope parameter is not a space separated list of valid scopes"
+            )
+        })
 
       {:error, error} ->
         Logger.error(inspect(error))
 
-        send_resp(
+        conn
-          conn,
+        |> Plug.Conn.put_status(500)
-          500,
+        |> json(%{
-          dgettext(
+          "error" => "server_error",
-            "errors",
+          "error_description" =>
-            "Impossible to create application."
+            dgettext(
-          )
+              "errors",
-        )
+              "Impossible to create application."
+            )
+        })
     end
   end
 
   def create_application(conn, _args) do
-    send_resp(
+    conn
-      conn,
+    |> Plug.Conn.put_status(400)
-      400,
+    |> json(%{
-      dgettext(
+      "error" => "invalid_request",
-        "errors",
+      "error_description" =>
-        "All of name, scope and redirect_uri parameters are required to create an application"
+        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(
+      if is_binary(redirect_uri) and valid_uri?(redirect_uri) do
-        conn,
+        redirect(conn,
-        400,
+          external:
-        dgettext(
+            append_parameters(redirect_uri,
-          "errors",
+              error: "invalid_request",
-          "You need to specify client_id, redirect_uri, scope and state to autorize an application"
+              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
+        conn
-          "json" ->
+        |> Plug.Conn.put_resp_header("cache-control", "no-store")
-            json(conn, res)
+        |> json(res)
-
-          _ ->
-            send_resp(conn, 200, URI.encode_query(res))
-        end
 
       {: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
-      conn,
+    |> Plug.Conn.put_status(400)
-      400,
+    |> json(%{
-      "You need to pass both client_id and scope as parameters to obtain a device code"
+      "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
+        conn
-          "json" ->
+        |> Plug.Conn.put_resp_header("cache-control", "no-store")
-            json(conn, res)
+        |> json(res)
-
-          _ ->
-            send_resp(
-              conn,
-              200,
-              URI.encode_query(res)
-            )
-        end
 
       {: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
-      conn,
+    |> Plug.Conn.put_status(400)
-      400,
+    |> json(%{
-      "Incorrect parameters sent. You need to provide at least the grant_type and client_id parameters, depending on the grant type being used."
+      "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

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

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

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
+    test "without a valid URI", %{conn: conn} do
-      conn = get(conn, "/oauth/authorize?client_id=hello")
+      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",
+               "expires_in" => 900,
-               "interval" => "5",
+               "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