commit: ccc2cf0e87f47618163da588ead76846c64cba7a
parent 73e66fd31fdfe8cc483fb77df235ddef31708aeb
Author: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Sat, 21 Nov 2020 19:47:25 +0300
Session-based OAuth auth fixes (token expiration check), refactoring, tweaks.
Diffstat:
11 files changed, 165 insertions(+), 197 deletions(-)
diff --git a/lib/pleroma/helpers/auth_helper.ex b/lib/pleroma/helpers/auth_helper.ex
@@ -5,13 +5,21 @@
defmodule Pleroma.Helpers.AuthHelper do
alias Pleroma.Web.Plugs.OAuthScopesPlug
+ import Plug.Conn
+
@doc """
Skips OAuth permissions (scopes) checks, assigns nil `:token`.
Intended to be used with explicit authentication and only when OAuth token cannot be determined.
"""
def skip_oauth(conn) do
conn
- |> Plug.Conn.assign(:token, nil)
+ |> assign(:token, nil)
|> OAuthScopesPlug.skip_plug()
end
+
+ def drop_auth_info(conn) do
+ conn
+ |> assign(:user, nil)
+ |> assign(:token, nil)
+ end
end
diff --git a/lib/pleroma/web/o_auth/o_auth_controller.ex b/lib/pleroma/web/o_auth/o_auth_controller.ex
@@ -363,7 +363,15 @@ defmodule Pleroma.Web.OAuth.OAuthController do
def token_revoke(%Plug.Conn{} = conn, %{"token" => _token} = params) do
with {:ok, app} <- Token.Utils.fetch_app(conn),
- {:ok, _token} <- RevokeToken.revoke(app, params) do
+ {:ok, %Token{} = oauth_token} <- RevokeToken.revoke(app, params) do
+ conn =
+ with session_token = get_session(conn, :oauth_token),
+ %Token{token: ^session_token} <- oauth_token do
+ delete_session(conn, :oauth_token)
+ else
+ _ -> conn
+ end
+
json(conn, %{})
else
_error ->
diff --git a/lib/pleroma/web/o_auth/token.ex b/lib/pleroma/web/o_auth/token.ex
@@ -27,6 +27,14 @@ defmodule Pleroma.Web.OAuth.Token do
timestamps()
end
+ @doc "Gets token by unique access token"
+ @spec get_by_token(String.t()) :: {:ok, t()} | {:error, :not_found}
+ def get_by_token(token) do
+ token
+ |> Query.get_by_token()
+ |> 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/plugs/o_auth_plug.ex b/lib/pleroma/web/plugs/o_auth_plug.ex
@@ -3,6 +3,8 @@
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.Plugs.OAuthPlug do
+ @moduledoc "Performs OAuth authentication by token from params / headers / cookies."
+
import Plug.Conn
import Ecto.Query
@@ -17,45 +19,26 @@ defmodule Pleroma.Web.Plugs.OAuthPlug do
def call(%{assigns: %{user: %User{}}} = conn, _), do: conn
- def call(%{params: %{"access_token" => access_token}} = conn, _) do
- with {:ok, user, token_record} <- fetch_user_and_token(access_token) do
- conn
- |> assign(:token, token_record)
- |> assign(:user, user)
- else
- _ ->
- # token found, but maybe only with app
- with {:ok, app, token_record} <- fetch_app_and_token(access_token) do
- conn
- |> assign(:token, token_record)
- |> assign(:app, app)
- else
- _ -> conn
- end
- end
- end
-
def call(conn, _) do
- case fetch_token_str(conn) do
- {:ok, token} ->
- with {:ok, user, token_record} <- fetch_user_and_token(token) do
- conn
- |> assign(:token, token_record)
- |> assign(:user, user)
- else
- _ ->
- # token found, but maybe only with app
- with {:ok, app, token_record} <- fetch_app_and_token(token) do
- conn
- |> assign(:token, token_record)
- |> assign(:app, app)
- else
- _ -> conn
- end
- end
-
- _ ->
+ with {:ok, token_str} <- fetch_token_str(conn) do
+ with {:ok, user, user_token} <- fetch_user_and_token(token_str),
+ false <- Token.is_expired?(user_token) do
conn
+ |> assign(:token, user_token)
+ |> assign(:user, user)
+ else
+ _ ->
+ with {:ok, app, app_token} <- fetch_app_and_token(token_str),
+ false <- Token.is_expired?(app_token) do
+ conn
+ |> assign(:token, app_token)
+ |> assign(:app, app)
+ else
+ _ -> conn
+ end
+ end
+ else
+ _ -> conn
end
end
@@ -70,7 +53,6 @@ defmodule Pleroma.Web.Plugs.OAuthPlug do
preload: [user: user]
)
- # credo:disable-for-next-line Credo.Check.Readability.MaxLineLength
with %Token{user: user} = token_record <- Repo.one(query) do
{:ok, user, token_record}
end
@@ -86,29 +68,23 @@ defmodule Pleroma.Web.Plugs.OAuthPlug do
end
end
- # Gets token from session by :oauth_token key
+ # Gets token string from conn (in params / headers / session)
#
- @spec fetch_token_from_session(Plug.Conn.t()) :: :no_token_found | {:ok, String.t()}
- defp fetch_token_from_session(conn) do
- case get_session(conn, :oauth_token) do
- nil -> :no_token_found
- token -> {:ok, token}
- end
+ @spec fetch_token_str(Plug.Conn.t() | list(String.t())) :: :no_token_found | {:ok, String.t()}
+ defp fetch_token_str(%Plug.Conn{params: %{"access_token" => access_token}} = _conn) do
+ {:ok, access_token}
end
- # Gets token from headers
- #
- @spec fetch_token_str(Plug.Conn.t()) :: :no_token_found | {:ok, String.t()}
defp fetch_token_str(%Plug.Conn{} = conn) do
headers = get_req_header(conn, "authorization")
- with :no_token_found <- fetch_token_str(headers),
- do: fetch_token_from_session(conn)
+ with {:ok, token} <- fetch_token_str(headers) do
+ {:ok, token}
+ else
+ _ -> fetch_token_from_session(conn)
+ end
end
- @spec fetch_token_str(Keyword.t()) :: :no_token_found | {:ok, String.t()}
- defp fetch_token_str([]), do: :no_token_found
-
defp fetch_token_str([token | tail]) do
trimmed_token = String.trim(token)
@@ -117,4 +93,14 @@ defmodule Pleroma.Web.Plugs.OAuthPlug do
_ -> fetch_token_str(tail)
end
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 get_session(conn, :oauth_token) do
+ nil -> :no_token_found
+ token -> {:ok, token}
+ end
+ end
end
diff --git a/lib/pleroma/web/plugs/session_authentication_plug.ex b/lib/pleroma/web/plugs/session_authentication_plug.ex
@@ -1,31 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Web.Plugs.SessionAuthenticationPlug do
- @moduledoc """
- Authenticates user by session-stored `:user_id` and request-contained username.
- Username can be provided via HTTP Basic Auth (the password is not checked and can be anything).
- """
-
- import Plug.Conn
-
- alias Pleroma.Helpers.AuthHelper
-
- def init(options) do
- options
- end
-
- def call(%{assigns: %{user: %Pleroma.User{}}} = conn, _), do: conn
-
- def call(conn, _) do
- with saved_user_id <- get_session(conn, :user_id),
- %{auth_user: %{id: ^saved_user_id}} <- conn.assigns do
- conn
- |> assign(:user, conn.assigns.auth_user)
- |> AuthHelper.skip_oauth()
- else
- _ -> conn
- end
- end
-end
diff --git a/lib/pleroma/web/plugs/set_user_session_id_plug.ex b/lib/pleroma/web/plugs/set_user_session_id_plug.ex
@@ -4,14 +4,15 @@
defmodule Pleroma.Web.Plugs.SetUserSessionIdPlug do
import Plug.Conn
- alias Pleroma.User
+
+ alias Pleroma.Web.OAuth.Token
def init(opts) do
opts
end
- def call(%{assigns: %{user: %User{id: id}}} = conn, _) do
- put_session(conn, :user_id, id)
+ def call(%{assigns: %{token: %Token{} = oauth_token}} = conn, _) do
+ put_session(conn, :oauth_token, oauth_token.token)
end
def call(conn, _), do: conn
diff --git a/lib/pleroma/web/plugs/user_enabled_plug.ex b/lib/pleroma/web/plugs/user_enabled_plug.ex
@@ -3,7 +3,7 @@
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.Plugs.UserEnabledPlug do
- import Plug.Conn
+ alias Pleroma.Helpers.AuthHelper
alias Pleroma.User
def init(options) do
@@ -12,8 +12,11 @@ defmodule Pleroma.Web.Plugs.UserEnabledPlug do
def call(%{assigns: %{user: %User{} = user}} = conn, _) do
case User.account_status(user) do
- :active -> conn
- _ -> assign(conn, :user, nil)
+ :active ->
+ conn
+
+ _ ->
+ AuthHelper.drop_auth_info(conn)
end
end
diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
@@ -34,6 +34,7 @@ defmodule Pleroma.Web.Router do
plug(:fetch_session)
plug(Pleroma.Web.Plugs.OAuthPlug)
plug(Pleroma.Web.Plugs.UserEnabledPlug)
+ plug(Pleroma.Web.Plugs.EnsureUserKeyPlug)
end
pipeline :expect_authentication do
@@ -48,7 +49,6 @@ defmodule Pleroma.Web.Router do
plug(Pleroma.Web.Plugs.OAuthPlug)
plug(Pleroma.Web.Plugs.BasicAuthDecoderPlug)
plug(Pleroma.Web.Plugs.UserFetcherPlug)
- plug(Pleroma.Web.Plugs.SessionAuthenticationPlug)
plug(Pleroma.Web.Plugs.AuthenticationPlug)
end
@@ -319,12 +319,12 @@ defmodule Pleroma.Web.Router do
scope "/oauth", Pleroma.Web.OAuth do
scope [] do
pipe_through(:oauth)
+
get("/authorize", OAuthController, :authorize)
+ post("/authorize", OAuthController, :create_authorization)
end
- post("/authorize", OAuthController, :create_authorization)
post("/token", OAuthController, :token_exchange)
- post("/revoke", OAuthController, :token_revoke)
get("/registration_details", OAuthController, :registration_details)
post("/mfa/challenge", MFAController, :challenge)
@@ -332,6 +332,12 @@ defmodule Pleroma.Web.Router do
get("/mfa", MFAController, :show)
scope [] do
+ pipe_through(:fetch_session)
+
+ post("/revoke", OAuthController, :token_revoke)
+ end
+
+ scope [] do
pipe_through(:browser)
get("/prepare_request", OAuthController, :prepare_request)
diff --git a/test/pleroma/web/plugs/o_auth_plug_test.exs b/test/pleroma/web/plugs/o_auth_plug_test.exs
@@ -5,43 +5,48 @@
defmodule Pleroma.Web.Plugs.OAuthPlugTest do
use Pleroma.Web.ConnCase, async: true
+ alias Pleroma.Web.OAuth.Token
+ alias Pleroma.Web.OAuth.Token.Strategy.Revoke
alias Pleroma.Web.Plugs.OAuthPlug
- import Pleroma.Factory
+ alias Plug.Session
- @session_opts [
- store: :cookie,
- key: "_test",
- signing_salt: "cooldude"
- ]
+ import Pleroma.Factory
setup %{conn: conn} do
user = insert(:user)
- {:ok, %{token: token}} = Pleroma.Web.OAuth.Token.create(insert(:oauth_app), user)
- %{user: user, token: token, conn: conn}
+ {:ok, oauth_token} = Token.create(insert(:oauth_app), user)
+ %{user: user, token: oauth_token, conn: conn}
+ end
+
+ test "it does nothing if a user is assigned", %{conn: conn} do
+ conn = assign(conn, :user, %Pleroma.User{})
+ ret_conn = OAuthPlug.call(conn, %{})
+
+ assert ret_conn == conn
end
- test "with valid token(uppercase), it assigns the user", %{conn: conn} = opts do
+ test "with valid token (uppercase) in auth header, it assigns the user", %{conn: conn} = opts do
conn =
conn
- |> put_req_header("authorization", "BEARER #{opts[:token]}")
+ |> put_req_header("authorization", "BEARER #{opts[:token].token}")
|> OAuthPlug.call(%{})
assert conn.assigns[:user] == opts[:user]
end
- test "with valid token(downcase), it assigns the user", %{conn: conn} = opts do
+ test "with valid token (downcase) in auth header, it assigns the user", %{conn: conn} = opts do
conn =
conn
- |> put_req_header("authorization", "bearer #{opts[:token]}")
+ |> put_req_header("authorization", "bearer #{opts[:token].token}")
|> OAuthPlug.call(%{})
assert conn.assigns[:user] == opts[:user]
end
- test "with valid token(downcase) in url parameters, it assigns the user", opts do
+ test "with valid token (downcase) in url parameters, it assigns the user", opts do
conn =
:get
- |> build_conn("/?access_token=#{opts[:token]}")
+ |> build_conn("/?access_token=#{opts[:token].token}")
|> put_req_header("content-type", "application/json")
|> fetch_query_params()
|> OAuthPlug.call(%{})
@@ -49,16 +54,16 @@ defmodule Pleroma.Web.Plugs.OAuthPlugTest do
assert conn.assigns[:user] == opts[:user]
end
- test "with valid token(downcase) in body parameters, it assigns the user", opts do
+ test "with valid token (downcase) in body parameters, it assigns the user", opts do
conn =
:post
- |> build_conn("/api/v1/statuses", access_token: opts[:token], status: "test")
+ |> build_conn("/api/v1/statuses", access_token: opts[:token].token, status: "test")
|> OAuthPlug.call(%{})
assert conn.assigns[:user] == opts[:user]
end
- test "with invalid token, it not assigns the user", %{conn: conn} do
+ test "with invalid token, it does not assign the user", %{conn: conn} do
conn =
conn
|> put_req_header("authorization", "bearer TTTTT")
@@ -67,14 +72,56 @@ defmodule Pleroma.Web.Plugs.OAuthPlugTest do
refute conn.assigns[:user]
end
- test "when token is missed but token in session, it assigns the user", %{conn: conn} = opts do
- conn =
- conn
- |> Plug.Session.call(Plug.Session.init(@session_opts))
- |> fetch_session()
- |> put_session(:oauth_token, opts[:token])
- |> OAuthPlug.call(%{})
-
- assert conn.assigns[:user] == opts[:user]
+ describe "with :oauth_token in session, " do
+ setup %{token: oauth_token, conn: conn} do
+ session_opts = [
+ store: :cookie,
+ key: "_test",
+ signing_salt: "cooldude"
+ ]
+
+ conn =
+ conn
+ |> Session.call(Session.init(session_opts))
+ |> fetch_session()
+ |> put_session(:oauth_token, oauth_token.token)
+
+ %{conn: conn}
+ end
+
+ test "if session-stored token matches a valid OAuth token, assigns :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
+ end
+
+ test "if session-stored token matches an expired OAuth token, does nothing", %{
+ conn: conn,
+ token: oauth_token
+ } do
+ expired_valid_until = NaiveDateTime.add(NaiveDateTime.utc_now(), -3600 * 24, :second)
+
+ oauth_token
+ |> Ecto.Changeset.change(valid_until: expired_valid_until)
+ |> Pleroma.Repo.update()
+
+ ret_conn = OAuthPlug.call(conn, %{})
+ assert ret_conn == conn
+ end
+
+ test "if session-stored token matches a revoked OAuth token, does nothing", %{
+ conn: conn,
+ token: oauth_token
+ } do
+ Revoke.revoke(oauth_token)
+
+ ret_conn = OAuthPlug.call(conn, %{})
+ assert ret_conn == conn
+ end
end
end
diff --git a/test/pleroma/web/plugs/session_authentication_plug_test.exs b/test/pleroma/web/plugs/session_authentication_plug_test.exs
@@ -1,65 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do
- use Pleroma.Web.ConnCase, async: true
-
- alias Pleroma.User
- alias Pleroma.Web.Plugs.OAuthScopesPlug
- alias Pleroma.Web.Plugs.PlugHelper
- alias Pleroma.Web.Plugs.SessionAuthenticationPlug
-
- setup %{conn: conn} do
- session_opts = [
- store: :cookie,
- key: "_test",
- signing_salt: "cooldude"
- ]
-
- conn =
- conn
- |> Plug.Session.call(Plug.Session.init(session_opts))
- |> fetch_session()
- |> assign(:auth_user, %User{id: 1})
-
- %{conn: conn}
- end
-
- test "it does nothing if a user is assigned", %{conn: conn} do
- conn = assign(conn, :user, %User{})
- ret_conn = SessionAuthenticationPlug.call(conn, %{})
-
- assert ret_conn == conn
- end
-
- # Scenario: requester has the cookie and knows the username (not necessarily knows the password)
- test "if the auth_user has the same id as the user_id in the session, it assigns the user", %{
- conn: conn
- } do
- conn =
- conn
- |> put_session(:user_id, conn.assigns.auth_user.id)
- |> SessionAuthenticationPlug.call(%{})
-
- assert conn.assigns.user == conn.assigns.auth_user
- assert conn.assigns.token == nil
- assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
- end
-
- # Scenario: requester has the cookie but doesn't know the username
- test "if the auth_user has a different id as the user_id in the session, it does nothing", %{
- conn: conn
- } do
- conn = put_session(conn, :user_id, -1)
- ret_conn = SessionAuthenticationPlug.call(conn, %{})
-
- assert ret_conn == conn
- end
-
- test "if the session does not contain user_id, it does nothing", %{
- conn: conn
- } do
- assert conn == SessionAuthenticationPlug.call(conn, %{})
- end
-end
diff --git a/test/pleroma/web/plugs/set_user_session_id_plug_test.exs b/test/pleroma/web/plugs/set_user_session_id_plug_test.exs
@@ -5,7 +5,6 @@
defmodule Pleroma.Web.Plugs.SetUserSessionIdPlugTest do
use Pleroma.Web.ConnCase, async: true
- alias Pleroma.User
alias Pleroma.Web.Plugs.SetUserSessionIdPlug
setup %{conn: conn} do
@@ -18,28 +17,26 @@ defmodule Pleroma.Web.Plugs.SetUserSessionIdPlugTest do
conn =
conn
|> Plug.Session.call(Plug.Session.init(session_opts))
- |> fetch_session
+ |> fetch_session()
%{conn: conn}
end
test "doesn't do anything if the user isn't set", %{conn: conn} do
- ret_conn =
- conn
- |> SetUserSessionIdPlug.call(%{})
+ ret_conn = SetUserSessionIdPlug.call(conn, %{})
assert ret_conn == conn
end
- test "sets the user_id in the session to the user id of the user assign", %{conn: conn} do
- Code.ensure_compiled(Pleroma.User)
+ test "sets :oauth_token in session to :token assign", %{conn: conn} do
+ %{user: user, token: oauth_token} = oauth_access(["read"])
- conn =
+ ret_conn =
conn
- |> assign(:user, %User{id: 1})
+ |> assign(:user, user)
+ |> assign(:token, oauth_token)
|> SetUserSessionIdPlug.call(%{})
- id = get_session(conn, :user_id)
- assert id == 1
+ assert get_session(ret_conn, :oauth_token) == oauth_token.token
end
end
