Do not use session for oauth tokens

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

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 =
+    # if we already have a token for this specific setup, we can use that
-      with %{assigns: %{user: %User{} = user}} <- conn do
+    with %App{} <- app,
-        user
+         {:ok, _} <- Scopes.validate(scopes, app.scopes),
-      else
+         {:ok, %Token{} = token} <- Token.get_by_app(app) do
-        _ -> nil
+      token = Repo.preload(token, :app)
-      end
 
-    scopes =
+      conn
-      if scopes == [] do
+      |> assign(:token, token)
-        available_scopes
+      |> handle_existing_authorization(params)
-      else
+    else
-        scopes
+      _ ->
-      end
+        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
+        scopes =
-    render(conn, Authenticator.auth_template(), %{
+          if scopes == [] do
-      user: user,
+            available_scopes
-      app: app && Map.delete(app, :client_secret),
+          else
-      response_type: params["response_type"],
+            scopes
-      client_id: params["client_id"],
+          end
-      available_scopes: available_scopes,
+
-      scopes: scopes,
+        # Note: `params` might differ from `conn.params`; use `@params` not `@conn.params` in template
-      redirect_uri: params["redirect_uri"],
+        render(conn, Authenticator.auth_template(), %{
-      state: params["state"],
+          user: user,
-      params: params,
+          app: app && Map.delete(app, :client_secret),
-      view_module: OAuthView
+          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(

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

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

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

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

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"},

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
+      refute 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
     end
 
     test "redirect to on two-factor auth page" do

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
+      refute conn.assigns[:user]
-      assert conn.assigns.token && conn.assigns.token.id == oauth_token.id
+      refute conn.assigns[:token]
     end
 
     test "if session-stored token matches an expired OAuth token, does nothing", %{