diff --git a/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex b/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex index 4920d65da..dccc39699 100644 --- a/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex @@ -34,7 +34,6 @@ def login(conn, %{"code" => auth_token} = params) do |> UriHelper.modify_uri_params(%{"access_token" => oauth_token.token}) conn - |> AuthHelper.put_session_token(oauth_token.token) |> redirect(to: redirect_to) else _ -> redirect_to_oauth_form(conn, params) @@ -42,9 +41,9 @@ def login(conn, %{"code" => auth_token} = params) do end def login(conn, params) do - with %{assigns: %{user: %User{}, token: %Token{app_id: app_id}}} <- conn, + with %{assigns: %{user: %User{}, token: %Token{app_id: app_id, token: token}}} <- conn, {:ok, %{id: ^app_id}} <- local_mastofe_app() do - redirect(conn, to: local_mastodon_post_login_path(conn)) + redirect(conn, to: local_mastodon_post_login_path(conn) <> "?access_token=#{token}") else _ -> redirect_to_oauth_form(conn, params) 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..63f82420c 100644 --- a/lib/pleroma/web/o_auth/o_auth_controller.ex +++ b/lib/pleroma/web/o_auth/o_auth_controller.ex @@ -77,33 +77,45 @@ defp do_authorize(%Plug.Conn{} = conn, params) do available_scopes = (app && app.scopes) || [] scopes = Scopes.fetch_scopes(params, available_scopes) - user = - with %{assigns: %{user: %User{} = user}} <- conn do - user - else - _ -> nil - end + # if we already have a token for this specific setup, we can use that + with %App{} <- app, + {:ok, _} <- Scopes.validate(scopes, app.scopes), + {:ok, %Token{} = token} <- Token.get_by_app(app) do + token = Repo.preload(token, :app) - scopes = - if scopes == [] do - available_scopes - else - scopes - end + conn + |> assign(:token, token) + |> handle_existing_authorization(params) + else + _ -> + user = + with %{assigns: %{user: %User{} = user}} <- conn do + user + else + _ -> nil + end - # Note: `params` might differ from `conn.params`; use `@params` not `@conn.params` in template - render(conn, Authenticator.auth_template(), %{ - user: user, - app: app && Map.delete(app, :client_secret), - response_type: params["response_type"], - client_id: params["client_id"], - available_scopes: available_scopes, - scopes: scopes, - redirect_uri: params["redirect_uri"], - state: params["state"], - params: params, - view_module: OAuthView - }) + scopes = + if scopes == [] do + available_scopes + else + scopes + end + + # Note: `params` might differ from `conn.params`; use `@params` not `@conn.params` in template + render(conn, Authenticator.auth_template(), %{ + user: user, + app: app && Map.delete(app, :client_secret), + response_type: params["response_type"], + client_id: params["client_id"], + available_scopes: available_scopes, + scopes: scopes, + redirect_uri: params["redirect_uri"], + state: params["state"], + params: params, + view_module: OAuthView + }) + end end defp handle_existing_authorization( diff --git a/lib/pleroma/web/o_auth/token.ex b/lib/pleroma/web/o_auth/token.ex index 9d69e9db4..612707e1f 100644 --- a/lib/pleroma/web/o_auth/token.ex +++ b/lib/pleroma/web/o_auth/token.ex @@ -39,6 +39,12 @@ def get_by_token(token) do |> Repo.find_resource() end + def get_by_app(%App{} = app) do + app.id + |> Query.get_by_app() + |> Repo.find_resource() + end + @doc "Gets token for app by access token" @spec get_by_token(App.t(), String.t()) :: {:ok, t()} | {:error, :not_found} def get_by_token(%App{id: app_id} = _app, token) do diff --git a/lib/pleroma/web/o_auth/token/query.ex b/lib/pleroma/web/o_auth/token/query.ex index d16a759d8..fb0f74086 100644 --- a/lib/pleroma/web/o_auth/token/query.ex +++ b/lib/pleroma/web/o_auth/token/query.ex @@ -25,7 +25,8 @@ def get_by_token(query \\ Token, token) do @spec get_by_app(query, String.t()) :: query def get_by_app(query \\ Token, app_id) do - from(q in query, where: q.app_id == ^app_id) + time = NaiveDateTime.utc_now() + from(q in query, where: q.app_id == ^app_id and q.valid_until > ^time, limit: 1) end @spec get_by_id(query, String.t()) :: query diff --git a/lib/pleroma/web/plugs/o_auth_plug.ex b/lib/pleroma/web/plugs/o_auth_plug.ex index 5e06ac3f6..29b3316b3 100644 --- a/lib/pleroma/web/plugs/o_auth_plug.ex +++ b/lib/pleroma/web/plugs/o_auth_plug.ex @@ -8,7 +8,6 @@ defmodule Pleroma.Web.Plugs.OAuthPlug do import Plug.Conn import Ecto.Query - alias Pleroma.Helpers.AuthHelper alias Pleroma.Repo alias Pleroma.User alias Pleroma.Web.OAuth.App @@ -18,8 +17,6 @@ defmodule Pleroma.Web.Plugs.OAuthPlug do def init(options), do: options - def call(%{assigns: %{user: %User{}}} = conn, _), do: conn - def call(conn, _) do with {:ok, token_str} <- fetch_token_str(conn) do with {:ok, user, user_token} <- fetch_user_and_token(token_str), @@ -82,7 +79,7 @@ defp fetch_token_str(%Plug.Conn{} = conn) do with {:ok, token} <- fetch_token_str(headers) do {:ok, token} else - _ -> fetch_token_from_session(conn) + _ -> :no_token_found end end @@ -96,12 +93,4 @@ defp fetch_token_str([token | tail]) do end defp fetch_token_str([]), do: :no_token_found - - @spec fetch_token_from_session(Plug.Conn.t()) :: :no_token_found | {:ok, String.t()} - defp fetch_token_from_session(conn) do - case AuthHelper.get_session_token(conn) do - nil -> :no_token_found - token -> {:ok, token} - end - end end diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 647d99278..1232bc8cb 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -793,10 +793,8 @@ defmodule Pleroma.Web.Router do get("/web/login", MastodonAPI.AuthController, :login) delete("/auth/sign_out", MastodonAPI.AuthController, :logout) - - post("/auth/password", MastodonAPI.AuthController, :password_reset) - get("/web/*path", MastoFEController, :index) + post("/auth/password", MastodonAPI.AuthController, :password_reset) get("/embed/:id", EmbedController, :show) end diff --git a/mix.exs b/mix.exs index e7f491997..170276b0a 100644 --- a/mix.exs +++ b/mix.exs @@ -131,7 +131,7 @@ defp deps do {:trailing_format_plug, "~> 0.0.7"}, {:fast_sanitize, "~> 0.2.3"}, {:html_entities, "~> 0.5", override: true}, - {:phoenix_html, "~> 3.1", override: true}, + {:phoenix_html, "~> 3.0", override: true}, {:calendar, "~> 1.0"}, {:cachex, "~> 3.4"}, {:poison, "~> 3.0", override: true}, @@ -152,7 +152,7 @@ defp deps do ref: "f75cd55325e33cbea198fb41fe41871392f8fb76"}, {:cors_plug, "~> 2.0"}, {:web_push_encryption, "~> 0.3.1"}, - {:swoosh, "~> 1.0"}, + {:swoosh, "~> 1.3"}, {:phoenix_swoosh, "~> 0.3"}, {:gen_smtp, "~> 0.13"}, {:ex_syslogger, "~> 1.4"}, 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..00ce0c95f 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -611,7 +611,7 @@ test "redirects with oauth authorization, " <> end end - test "authorize from cookie" do + test "should not authorize from cookie" do user = insert(:user) app = insert(:oauth_app) oauth_token = insert(:oauth_token, user: user, app: app) @@ -636,14 +636,7 @@ test "authorize from cookie" do ) target = redirected_to(conn) - assert target =~ redirect_uri - - query = URI.parse(target).query |> URI.query_decoder() |> Map.new() - - assert %{"state" => "statepassed", "code" => code} = query - auth = Repo.get_by(Authorization, token: code) - assert auth - assert auth.scopes == app.scopes + refute target =~ redirect_uri end test "redirect to on two-factor auth page" do diff --git a/test/pleroma/web/plugs/o_auth_plug_test.exs b/test/pleroma/web/plugs/o_auth_plug_test.exs index 9e4be5559..8fb7d1dfd 100644 --- a/test/pleroma/web/plugs/o_auth_plug_test.exs +++ b/test/pleroma/web/plugs/o_auth_plug_test.exs @@ -90,15 +90,15 @@ test "with invalid token, it does not assign the user", %{conn: conn} do %{conn: conn} end - test "if session-stored token matches a valid OAuth token, assigns :user and :token", %{ + test "if session-stored token matches a valid OAuth token, should not assign :user and :token", %{ conn: conn, user: user, token: oauth_token } do conn = OAuthPlug.call(conn, %{}) - assert conn.assigns.user && conn.assigns.user.id == user.id - assert conn.assigns.token && conn.assigns.token.id == oauth_token.id + refute conn.assigns[:user] + refute conn.assigns[:token] end test "if session-stored token matches an expired OAuth token, does nothing", %{