From d1bbec0441e3fbace1444fe86838b25342f11624 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Sun, 21 Aug 2022 16:52:47 +0100 Subject: [PATCH] ensure token matches --- lib/pleroma/web/o_auth/o_auth_controller.ex | 15 ++- lib/pleroma/web/o_auth/token.ex | 4 +- lib/pleroma/web/o_auth/token/query.ex | 10 +- .../web/o_auth/o_auth_controller_test.exs | 114 +++++++++++++++++- 4 files changed, 131 insertions(+), 12 deletions(-) diff --git a/lib/pleroma/web/o_auth/o_auth_controller.ex b/lib/pleroma/web/o_auth/o_auth_controller.ex index 43536f95d..6a8006d31 100644 --- a/lib/pleroma/web/o_auth/o_auth_controller.ex +++ b/lib/pleroma/web/o_auth/o_auth_controller.ex @@ -71,16 +71,27 @@ defmodule Pleroma.Web.OAuth.OAuthController do def authorize(%Plug.Conn{} = conn, params), do: do_authorize(conn, params) + defp maybe_remove_token(%Plug.Conn{assigns: %{token: %{app: id}}} = conn, %App{id: id}) do + conn + end + + defp maybe_remove_token(conn, _app) do + conn + |> assign(:token, nil) + end + defp do_authorize(%Plug.Conn{} = conn, params) do app = Repo.get_by(App, client_id: params["client_id"]) + conn = maybe_remove_token(conn, app) available_scopes = (app && app.scopes) || [] scopes = Scopes.fetch_scopes(params, available_scopes) # if we already have a token for this specific setup, we can use that with false <- Params.truthy_param?(params["force_login"]), %App{} <- app, - {:ok, _} <- Scopes.validate(scopes, app.scopes), - {:ok, %Token{} = token} <- Token.get_by_app(app) do + %{assigns: %{user: %Pleroma.User{} = user}} <- conn, + {:ok, %Token{} = token} <- Token.get_preexisting_by_app_and_user(app, user), + true <- scopes == token.scopes do token = Repo.preload(token, :app) conn diff --git a/lib/pleroma/web/o_auth/token.ex b/lib/pleroma/web/o_auth/token.ex index 08c8cd298..6e91b6216 100644 --- a/lib/pleroma/web/o_auth/token.ex +++ b/lib/pleroma/web/o_auth/token.ex @@ -39,9 +39,9 @@ defmodule Pleroma.Web.OAuth.Token do |> Repo.find_resource() end - def get_by_app(%App{} = app) do + def get_preexisting_by_app_and_user(%App{} = app, %User{} = user) do app.id - |> Query.get_unexpired_by_app() + |> Query.get_unexpired_by_app_and_user(user) |> Repo.find_resource() end diff --git a/lib/pleroma/web/o_auth/token/query.ex b/lib/pleroma/web/o_auth/token/query.ex index 8edfcf5d7..acddf0533 100644 --- a/lib/pleroma/web/o_auth/token/query.ex +++ b/lib/pleroma/web/o_auth/token/query.ex @@ -23,10 +23,14 @@ defmodule Pleroma.Web.OAuth.Token.Query do from(q in query, where: q.token == ^token) end - @spec get_unexpired_by_app(query, String.t()) :: query - def get_unexpired_by_app(query \\ Token, app_id) do + @spec get_unexpired_by_app_and_user(query, String.t()) :: query + def get_unexpired_by_app_and_user(query \\ Token, app_id, %Pleroma.User{id: user_id}) do time = NaiveDateTime.utc_now() - from(q in query, where: q.app_id == ^app_id and q.valid_until > ^time, limit: 1) + + from(q in query, + where: q.app_id == ^app_id and q.valid_until > ^time and q.user_id == ^user_id, + limit: 1 + ) end @spec get_by_app(query, String.t()) :: query diff --git a/test/pleroma/web/o_auth/o_auth_controller_test.exs b/test/pleroma/web/o_auth/o_auth_controller_test.exs index 4e197a485..9f984b26f 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -470,12 +470,17 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do assert html_response(conn, 200) =~ ~s(type="submit") end - test "renders authentication page if user is already authenticated but user request with another client", + test "reuses authentication if the user is authenticated with another client", %{ - app: app, conn: conn } do - token = insert(:oauth_token, app: app) + user = insert(:user) + + app = insert(:oauth_app, redirect_uris: "https://redirect.url") + other_app = insert(:oauth_app, redirect_uris: "https://redirect.url") + + token = insert(:oauth_token, user: user, app: app) + reusable_token = insert(:oauth_token, app: other_app, user: user) conn = conn @@ -484,12 +489,111 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do "/oauth/authorize", %{ "response_type" => "code", - "client_id" => "another_client_id", - "redirect_uri" => OAuthController.default_redirect_uri(app), + "client_id" => other_app.client_id, + "redirect_uri" => OAuthController.default_redirect_uri(other_app), "scope" => "read" } ) + assert URI.decode(redirected_to(conn)) == + "https://redirect.url?access_token=#{reusable_token.token}" + end + + test "does not reuse other people's tokens", + %{ + conn: conn + } do + user = insert(:user) + other_user = insert(:user) + + app = insert(:oauth_app, redirect_uris: "https://redirect.url") + other_app = insert(:oauth_app, redirect_uris: "https://redirect.url") + + token = insert(:oauth_token, user: user, app: app) + _not_reusable_token = insert(:oauth_token, app: other_app, user: other_user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{token.token}") + |> get( + "/oauth/authorize", + %{ + "response_type" => "code", + "client_id" => other_app.client_id, + "redirect_uri" => OAuthController.default_redirect_uri(other_app), + "scope" => "read" + } + ) + + assert html_response(conn, 200) =~ ~s(type="submit") + end + + test "does not reuse expired tokens", + %{ + conn: conn + } do + user = insert(:user) + + app = insert(:oauth_app, redirect_uris: "https://redirect.url") + + other_app = insert(:oauth_app, redirect_uris: "https://redirect.url") + + token = insert(:oauth_token, user: user, app: app) + + _not_reusable_token = + insert(:oauth_token, + app: other_app, + user: user, + valid_until: NaiveDateTime.add(NaiveDateTime.utc_now(), -60 * 100) + ) + + conn = + conn + |> put_req_header("authorization", "Bearer #{token.token}") + |> get( + "/oauth/authorize", + %{ + "response_type" => "code", + "client_id" => other_app.client_id, + "redirect_uri" => OAuthController.default_redirect_uri(other_app), + "scope" => "read" + } + ) + + assert html_response(conn, 200) =~ ~s(type="submit") + end + + test "does not reuse tokens with the wrong scopes", + %{ + conn: conn + } do + user = insert(:user) + + app = insert(:oauth_app, redirect_uris: "https://redirect.url") + + other_app = insert(:oauth_app, redirect_uris: "https://redirect.url") + + token = insert(:oauth_token, user: user, app: app, scopes: ["read"]) + + _not_reusable_token = + insert(:oauth_token, + app: other_app, + user: user + ) + + conn = + conn + |> put_req_header("authorization", "Bearer #{token.token}") + |> get( + "/oauth/authorize", + %{ + "response_type" => "code", + "client_id" => other_app.client_id, + "redirect_uri" => OAuthController.default_redirect_uri(other_app), + "scope" => "read write" + } + ) + assert html_response(conn, 200) =~ ~s(type="submit") end