From 618cf7ff7f60f9a81a5d85160416107032ad7b34 Mon Sep 17 00:00:00 2001 From: floatingghost Date: Thu, 25 Aug 2022 14:37:51 +0000 Subject: [PATCH] reuse valid oauth tokens (#182) Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/182 --- lib/pleroma/helpers/auth_helper.ex | 13 ++ .../controllers/auth_controller.ex | 3 +- lib/pleroma/web/o_auth/authorization.ex | 5 + lib/pleroma/web/o_auth/o_auth_controller.ex | 32 ++++- lib/pleroma/web/o_auth/token.ex | 18 +++ lib/pleroma/web/o_auth/token/query.ex | 13 ++ .../web/o_auth/o_auth_controller_test.exs | 123 ++++++++++++++++++ 7 files changed, 202 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/helpers/auth_helper.ex b/lib/pleroma/helpers/auth_helper.ex index 13e4c8158..37765da4d 100644 --- a/lib/pleroma/helpers/auth_helper.ex +++ b/lib/pleroma/helpers/auth_helper.ex @@ -9,6 +9,7 @@ defmodule Pleroma.Helpers.AuthHelper do import Plug.Conn @oauth_token_session_key :oauth_token + @oauth_user_session_key :oauth_user @doc """ Skips OAuth permissions (scopes) checks, assigns nil `:token`. @@ -43,4 +44,16 @@ defmodule Pleroma.Helpers.AuthHelper do def delete_session_token(%Conn{} = conn) do delete_session(conn, @oauth_token_session_key) end + + def put_session_user(%Conn{} = conn, user) do + put_session(conn, @oauth_user_session_key, user) + end + + def delete_session_user(%Conn{} = conn) do + delete_session(conn, @oauth_user_session_key) + end + + def get_session_user(%Conn{} = conn) do + get_session(conn, @oauth_user_session_key) + end end diff --git a/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex b/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex index 4920d65da..a9ccaa982 100644 --- a/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex @@ -27,7 +27,8 @@ defmodule Pleroma.Web.MastodonAPI.AuthController do def login(conn, %{"code" => auth_token} = params) do with {:ok, app} <- local_mastofe_app(), {:ok, auth} <- Authorization.get_by_token(app, auth_token), - {:ok, oauth_token} <- Token.exchange_token(app, auth) do + %User{} = user <- User.get_cached_by_id(auth.user_id), + {:ok, oauth_token} <- Token.get_or_exchange_token(auth, app, user) do redirect_to = conn |> local_mastodon_post_login_path() diff --git a/lib/pleroma/web/o_auth/authorization.ex b/lib/pleroma/web/o_auth/authorization.ex index e0ecb0f4f..e56704164 100644 --- a/lib/pleroma/web/o_auth/authorization.ex +++ b/lib/pleroma/web/o_auth/authorization.ex @@ -94,4 +94,9 @@ defmodule Pleroma.Web.OAuth.Authorization do from(t in __MODULE__, where: t.app_id == ^app_id and t.token == ^token) |> Repo.find_resource() end + + def get_preeexisting_by_app_and_user(%App{id: app_id} = _app, %User{id: user_id} = _user) do + from(t in __MODULE__, where: t.app_id == ^app_id and t.user_id == ^user_id, limit: 1) + |> Repo.find_resource() + end end diff --git a/lib/pleroma/web/o_auth/o_auth_controller.ex b/lib/pleroma/web/o_auth/o_auth_controller.ex index 358120fe6..455af11d7 100644 --- a/lib/pleroma/web/o_auth/o_auth_controller.ex +++ b/lib/pleroma/web/o_auth/o_auth_controller.ex @@ -59,18 +59,39 @@ defmodule Pleroma.Web.OAuth.OAuthController do # after user already authorized to MastodonFE. # So we have to check client and token. def authorize( - %Plug.Conn{assigns: %{token: %Token{} = token}} = conn, + %Plug.Conn{assigns: %{token: %Token{} = token, user: %User{} = user}} = conn, %{"client_id" => client_id} = params ) do with %Token{} = t <- Repo.get_by(Token, token: token.token) |> Repo.preload(:app), ^client_id <- t.app.client_id do handle_existing_authorization(conn, params) + else + _ -> + maybe_reuse_token(conn, params, user.id) + end + end + + def authorize(%Plug.Conn{} = conn, params) do + # if we have a user in the session, attempt to authenticate as them + # otherwise show the login form + maybe_reuse_token(conn, params, AuthHelper.get_session_user(conn)) + end + + defp maybe_reuse_token(conn, params, user_id) when is_binary(user_id) do + with %User{} = user <- User.get_cached_by_id(user_id), + %App{} = app <- Repo.get_by(App, client_id: params["client_id"]), + {:ok, %Token{} = token} <- Token.get_preeexisting_by_app_and_user(app, user), + {:ok, %Authorization{} = auth} <- + Authorization.get_preeexisting_by_app_and_user(app, user) do + conn + |> assign(:token, token) + |> after_create_authorization(auth, %{"authorization" => params}) else _ -> do_authorize(conn, params) end end - def authorize(%Plug.Conn{} = conn, params), do: do_authorize(conn, params) + defp maybe_reuse_token(conn, params, _user), do: do_authorize(conn, params) defp do_authorize(%Plug.Conn{} = conn, params) do app = Repo.get_by(App, client_id: params["client_id"]) @@ -148,7 +169,9 @@ defmodule Pleroma.Web.OAuth.OAuthController do 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) + conn + |> AuthHelper.put_session_user(user.id) + |> after_create_authorization(auth, params) else error -> handle_create_authorization_error(conn, error, params) @@ -269,7 +292,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do fixed_token = Token.Utils.fix_padding(params["code"]), {:ok, auth} <- Authorization.get_by_token(app, fixed_token), %User{} = user <- User.get_cached_by_id(auth.user_id), - {:ok, token} <- Token.exchange_token(app, auth) do + {:ok, token} <- Token.get_or_exchange_token(auth, app, user) do after_token_exchange(conn, %{user: user, token: token}) else error -> @@ -321,6 +344,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do def after_token_exchange(%Plug.Conn{} = conn, %{token: token} = view_params) do conn |> AuthHelper.put_session_token(token.token) + |> AuthHelper.put_session_user(token.user_id) |> json(OAuthView.render("token.json", view_params)) end diff --git a/lib/pleroma/web/o_auth/token.ex b/lib/pleroma/web/o_auth/token.ex index 9d69e9db4..c9398aeaa 100644 --- a/lib/pleroma/web/o_auth/token.ex +++ b/lib/pleroma/web/o_auth/token.ex @@ -70,6 +70,16 @@ defmodule Pleroma.Web.OAuth.Token do end end + def get_preeexisting_by_app_and_user(app, user) do + Query.get_by_app(app.id) + |> Query.get_by_user(user.id) + |> Query.get_unexpired() + |> Query.preload([:user]) + |> Query.sort_by_inserted_at() + |> Query.limit(1) + |> Repo.find_resource() + end + defp put_token(changeset) do changeset |> change(%{token: Token.Utils.generate_token()}) @@ -86,6 +96,14 @@ defmodule Pleroma.Web.OAuth.Token do |> unique_constraint(:refresh_token) end + def get_or_exchange_token(%Authorization{} = auth, %App{} = app, %User{} = user) do + if auth.used do + get_preeexisting_by_app_and_user(app, user) + else + exchange_token(app, auth) + end + end + defp put_valid_until(changeset, attrs) do valid_until = Map.get(attrs, :valid_until, NaiveDateTime.add(NaiveDateTime.utc_now(), lifespan())) diff --git a/lib/pleroma/web/o_auth/token/query.ex b/lib/pleroma/web/o_auth/token/query.ex index d16a759d8..662e7856d 100644 --- a/lib/pleroma/web/o_auth/token/query.ex +++ b/lib/pleroma/web/o_auth/token/query.ex @@ -38,6 +38,19 @@ defmodule Pleroma.Web.OAuth.Token.Query do from(q in query, where: q.user_id == ^user_id) end + def get_unexpired(query) do + now = NaiveDateTime.utc_now() + from(q in query, where: q.valid_until > ^now) + end + + def limit(query, limit) do + from(q in query, limit: ^limit) + end + + def sort_by_inserted_at(query) do + from(q in query, order_by: [desc: :updated_at]) + end + @spec preload(query, any) :: query def preload(query \\ Token, assoc_preload \\ []) 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 0fdd5b8e9..5a1258ec3 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -494,6 +494,129 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do assert html_response(conn, 200) =~ ~s(type="submit") end + test "allows access if the user has a prior authorization but is authenticated with another client", + %{ + app: app, + conn: conn + } do + user = insert(:user) + token = insert(:oauth_token, app: app, user: user) + + other_app = insert(:oauth_app, redirect_uris: "https://other_redirect.url") + authorization = insert(:oauth_authorization, user: user, app: other_app) + _reusable_token = insert(:oauth_token, app: other_app, user: user) + + conn = + conn + |> AuthHelper.put_session_token(token.token) + |> AuthHelper.put_session_user(user.id) + |> get( + "/oauth/authorize", + %{ + "response_type" => "code", + "client_id" => other_app.client_id, + "redirect_uri" => OAuthController.default_redirect_uri(other_app), + "scope" => "read" + } + ) + + assert URI.decode(redirected_to(conn)) == + "https://other_redirect.url?code=#{authorization.token}" + end + + test "renders login page if the user has an authorization but no token", + %{ + app: app, + conn: conn + } do + user = insert(:user) + token = insert(:oauth_token, app: app, user: user) + + other_app = insert(:oauth_app, redirect_uris: "https://other_redirect.url") + _authorization = insert(:oauth_authorization, user: user, app: other_app) + + conn = + conn + |> AuthHelper.put_session_token(token.token) + |> AuthHelper.put_session_user(user.id) + |> 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 other people's tokens", + %{ + app: app, + conn: conn + } do + user = insert(:user) + other_user = insert(:user) + token = insert(:oauth_token, app: app, user: user) + + other_app = insert(:oauth_app, redirect_uris: "https://other_redirect.url") + _authorization = insert(:oauth_authorization, user: other_user, app: other_app) + _reusable_token = insert(:oauth_token, app: other_app, user: other_user) + + conn = + conn + |> AuthHelper.put_session_token(token.token) + |> AuthHelper.put_session_user(user.id) + |> 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", + %{ + app: app, + conn: conn + } do + user = insert(:user) + token = insert(:oauth_token, app: app, user: user) + + other_app = insert(:oauth_app, redirect_uris: "https://other_redirect.url") + _authorization = insert(:oauth_authorization, user: user, app: other_app) + + _reusable_token = + insert(:oauth_token, + app: other_app, + user: user, + valid_until: NaiveDateTime.add(NaiveDateTime.utc_now(), -100) + ) + + conn = + conn + |> AuthHelper.put_session_token(token.token) + |> AuthHelper.put_session_user(user.id) + |> 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 "with existing authentication and non-OOB `redirect_uri`, redirects to app with `token` and `state` params", %{ app: app,