From f1b07a2b2b6439579f0a35694f693712fb5ec5f4 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 28 Nov 2020 21:51:06 +0300 Subject: [PATCH] OAuth form user remembering feature. Local MastoFE login / logout fixes. --- .gitattributes | 7 +- CHANGELOG.md | 1 + docs/configuration/static_dir.md | 5 + lib/pleroma/user.ex | 4 + lib/pleroma/web/masto_fe_controller.ex | 34 +-- .../controllers/auth_controller.ex | 64 +++-- lib/pleroma/web/o_auth/o_auth_controller.ex | 21 +- lib/pleroma/web/templates/layout/app.html.eex | 236 +----------------- .../web/templates/o_auth/o_auth/show.html.eex | 66 +++-- priv/static/instance/static.css | Bin 0 -> 5021 bytes test/pleroma/user_test.exs | 5 + .../controllers/auth_controller_test.exs | 4 +- .../mastodon_api/masto_fe_controller_test.exs | 3 +- .../web/o_auth/o_auth_controller_test.exs | 39 ++- 14 files changed, 192 insertions(+), 297 deletions(-) create mode 100644 priv/static/instance/static.css diff --git a/.gitattributes b/.gitattributes index 68895bf88..355e17f3c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,8 +1,9 @@ *.ex diff=elixir *.exs diff=elixir -# At the time of writing all js/css files included -# in the repo are minified bundles, and we don't want -# to search/diff those as text files. + +# Most os js/css files included in the repo are minified bundles, +# and we don't want to search/diff those as text files. Exceptions are listed below. *.js binary *.js.map binary *.css binary +priv/static/instance/static.css diff=css diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ef66408..4b3ae2193 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Ability to view remote timelines, with ex. `/api/v1/timelines/public?instance=lain.com` and streams `public:remote` and `public:remote:media`. - The site title is now injected as a `title` tag like preloads or metadata. - Password reset tokens now are not accepted after a certain age. +- OAuth form improvements: users are remembered by their cookie, the CSS is overridable by the admin, and the style has been improved.
API Changes diff --git a/docs/configuration/static_dir.md b/docs/configuration/static_dir.md index 8ac07b725..a294bb604 100644 --- a/docs/configuration/static_dir.md +++ b/docs/configuration/static_dir.md @@ -88,3 +88,8 @@ config :pleroma, :frontend_configurations, Note the extra `static` folder for the terms-of-service.html Terms of Service will be shown to all users on the registration page. It's the best place where to write down the rules for your instance. You can modify the rules by adding and changing `$static_dir/static/terms-of-service.html`. + + +## Styling rendered pages + +To overwrite the CSS stylesheet of the OAuth form and other static pages, you can upload your own CSS file to `instance/static/static.css`. This will completely replace the CSS used by those pages, so it might be a good idea to copy the one from `priv/static/instance/static.css` and make your changes. diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index bcd5256c8..6a5a43a25 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -2406,4 +2406,8 @@ def sanitize_html(%User{} = user, filter) do |> Map.put(:bio, HTML.filter_tags(user.bio, filter)) |> Map.put(:fields, fields) end + + def get_host(%User{ap_id: ap_id} = _user) do + URI.parse(ap_id).host + end end diff --git a/lib/pleroma/web/masto_fe_controller.ex b/lib/pleroma/web/masto_fe_controller.ex index 08f92d55f..7011ae214 100644 --- a/lib/pleroma/web/masto_fe_controller.ex +++ b/lib/pleroma/web/masto_fe_controller.ex @@ -6,6 +6,8 @@ defmodule Pleroma.Web.MastoFEController do use Pleroma.Web, :controller alias Pleroma.User + alias Pleroma.Web.OAuth.Token + alias Pleroma.Web.MastodonAPI.AuthController alias Pleroma.Web.Plugs.EnsurePublicOrAuthenticatedPlug alias Pleroma.Web.Plugs.OAuthScopesPlug @@ -26,27 +28,27 @@ defmodule Pleroma.Web.MastoFEController do ) @doc "GET /web/*path" - def index(%{assigns: %{user: user, token: token}} = conn, _params) - when not is_nil(user) and not is_nil(token) do - conn - |> put_layout(false) - |> render("index.html", - token: token.token, - user: user, - custom_emojis: Pleroma.Emoji.get_all() - ) - end - def index(conn, _params) do - conn - |> put_session(:return_to, conn.request_path) - |> redirect(to: "/web/login") + with %{assigns: %{user: %User{} = user, token: %Token{app_id: token_app_id} = token}} <- conn, + {:ok, %{id: ^token_app_id}} <- AuthController.local_mastofe_app() do + conn + |> put_layout(false) + |> render("index.html", + token: token.token, + user: user, + custom_emojis: Pleroma.Emoji.get_all() + ) + else + _ -> + conn + |> put_session(:return_to, conn.request_path) + |> redirect(to: "/web/login") + end end @doc "GET /web/manifest.json" def manifest(conn, _params) do - conn - |> render("manifest.json") + render(conn, "manifest.json") end @doc "PUT /api/web/settings: Backend-obscure settings blob for MastoFE, don't parse/reuse elsewhere" diff --git a/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex b/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex index fa582dcfc..93d057a79 100644 --- a/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/auth_controller.ex @@ -8,10 +8,12 @@ defmodule Pleroma.Web.MastodonAPI.AuthController do import Pleroma.Web.ControllerHelper, only: [json_response: 3] alias Pleroma.Helpers.AuthHelper + alias Pleroma.Helpers.UriHelper alias Pleroma.User alias Pleroma.Web.OAuth.App alias Pleroma.Web.OAuth.Authorization alias Pleroma.Web.OAuth.Token + alias Pleroma.Web.OAuth.Token.Strategy.Revoke, as: RevokeToken alias Pleroma.Web.TwitterAPI.TwitterAPI action_fallback(Pleroma.Web.MastodonAPI.FallbackController) @@ -21,24 +23,35 @@ defmodule Pleroma.Web.MastodonAPI.AuthController do @local_mastodon_name "Mastodon-Local" @doc "GET /web/login" - def login(%{assigns: %{user: %User{}}} = conn, _params) do - redirect(conn, to: local_mastodon_root_path(conn)) - end - - # Local Mastodon FE login init action - def login(conn, %{"code" => auth_token}) do - with {:ok, app} <- get_or_make_app(), + # Local Mastodon FE login callback action + def login(conn, %{"code" => auth_token} = params) do + with {:ok, app} <- local_mastofe_app(), {:ok, auth} <- Authorization.get_by_token(app, auth_token), - {:ok, token} <- Token.exchange_token(app, auth) do + {:ok, oauth_token} <- Token.exchange_token(app, auth) do + redirect_to = + conn + |> local_mastodon_post_login_path() + |> UriHelper.modify_uri_params(%{"access_token" => oauth_token.token}) + conn - |> AuthHelper.put_session_token(token.token) - |> redirect(to: local_mastodon_root_path(conn)) + |> AuthHelper.put_session_token(oauth_token.token) + |> redirect(to: redirect_to) + else + _ -> redirect_to_oauth_form(conn, params) end end - # Local Mastodon FE callback action - def login(conn, _) do - with {:ok, app} <- get_or_make_app() do + def login(conn, params) do + with %{assigns: %{user: %User{}, token: %Token{app_id: app_id}}} <- conn, + {:ok, %{id: ^app_id}} <- local_mastofe_app() do + redirect(conn, to: local_mastodon_post_login_path(conn)) + else + _ -> redirect_to_oauth_form(conn, params) + end + end + + defp redirect_to_oauth_form(conn, _params) do + with {:ok, app} <- local_mastofe_app() do path = o_auth_path(conn, :authorize, response_type: "code", @@ -53,9 +66,16 @@ def login(conn, _) do @doc "DELETE /auth/sign_out" def logout(conn, _) do - conn - |> clear_session() - |> redirect(to: "/") + conn = + with %{assigns: %{token: %Token{} = oauth_token}} <- conn, + session_token = AuthHelper.get_session_token(conn), + {:ok, %Token{token: ^session_token}} <- RevokeToken.revoke(oauth_token) do + AuthHelper.delete_session_token(conn) + else + _ -> conn + end + + redirect(conn, to: "/") end @doc "POST /auth/password" @@ -67,7 +87,7 @@ def password_reset(conn, params) do json_response(conn, :no_content, "") end - defp local_mastodon_root_path(conn) do + defp local_mastodon_post_login_path(conn) do case get_session(conn, :return_to) do nil -> masto_fe_path(conn, :index, ["getting-started"]) @@ -78,9 +98,11 @@ defp local_mastodon_root_path(conn) do end end - @spec get_or_make_app() :: {:ok, App.t()} | {:error, Ecto.Changeset.t()} - defp get_or_make_app do - %{client_name: @local_mastodon_name, redirect_uris: "."} - |> App.get_or_make(["read", "write", "follow", "push", "admin"]) + @spec local_mastofe_app() :: {:ok, App.t()} | {:error, Ecto.Changeset.t()} + def local_mastofe_app do + App.get_or_make( + %{client_name: @local_mastodon_name, redirect_uris: "."}, + ["read", "write", "follow", "push", "admin"] + ) 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 8103395b3..965c0f879 100644 --- a/lib/pleroma/web/o_auth/o_auth_controller.ex +++ b/lib/pleroma/web/o_auth/o_auth_controller.ex @@ -80,6 +80,13 @@ 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 + scopes = if scopes == [] do available_scopes @@ -89,6 +96,8 @@ defp do_authorize(%Plug.Conn{} = conn, params) do # 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, @@ -132,11 +141,13 @@ defp handle_existing_authorization( end end - def create_authorization( - %Plug.Conn{} = conn, - %{"authorization" => _} = params, - opts \\ [] - ) do + def create_authorization(_, _, opts \\ []) + + def create_authorization(%Plug.Conn{assigns: %{user: %User{} = user}} = conn, params, []) do + create_authorization(conn, params, user: user) + end + + 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) diff --git a/lib/pleroma/web/templates/layout/app.html.eex b/lib/pleroma/web/templates/layout/app.html.eex index 3f28f1920..1ede59fd8 100644 --- a/lib/pleroma/web/templates/layout/app.html.eex +++ b/lib/pleroma/web/templates/layout/app.html.eex @@ -1,233 +1,19 @@ - + - - - - <%= Pleroma.Config.get([:instance, :name]) %> - - + + + <%= Pleroma.Config.get([:instance, :name]) %> + +
+ + +

<%= Pleroma.Config.get([:instance, :name]) %>

+ +
-

<%= Pleroma.Config.get([:instance, :name]) %>

<%= @inner_content %>
diff --git a/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex index b17142ff8..1a85818ec 100644 --- a/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex +++ b/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex @@ -5,32 +5,55 @@ <% end %> -

OAuth Authorization

<%= form_for @conn, o_auth_path(@conn, :authorize), [as: "authorization"], fn f -> %> -<%= if @params["registration"] in ["true", true] do %> -

This is the first time you visit! Please enter your Pleroma handle.

-

Choose carefully! You won't be able to change this later. You will be able to change your display name, though.

-
- <%= label f, :nickname, "Pleroma Handle" %> - <%= text_input f, :nickname, placeholder: "lain" %> +<%= if @user do %> + - <%= hidden_input f, :name, value: @params["name"] %> - <%= hidden_input f, :password, value: @params["password"] %> -
-<% else %> -
- <%= label f, :name, "Username" %> - <%= text_input f, :name %> -
-
- <%= label f, :password, "Password" %> - <%= password_input f, :password %> -
- <%= submit "Log In" %> - <%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %> <% end %> +
+ <%= if @app do %> +

Application <%= @app.client_name %> is requesting access to your account.

+ <%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %> + <% end %> + + <%= if @user do %> +
+ Cancel + <%= submit "Approve", class: "button--approve" %> +
+ <% else %> + <%= if @params["registration"] in ["true", true] do %> +

This is the first time you visit! Please enter your Pleroma handle.

+

Choose carefully! You won't be able to change this later. You will be able to change your display name, though.

+
+ <%= label f, :nickname, "Pleroma Handle" %> + <%= text_input f, :nickname, placeholder: "lain" %> +
+ <%= hidden_input f, :name, value: @params["name"] %> + <%= hidden_input f, :password, value: @params["password"] %> +
+ <% else %> +
+ <%= label f, :name, "Username" %> + <%= text_input f, :name %> +
+
+ <%= label f, :password, "Password" %> + <%= password_input f, :password %> +
+ <%= submit "Log In" %> + <% end %> + <% end %> +
+ <%= hidden_input f, :client_id, value: @client_id %> <%= hidden_input f, :response_type, value: @response_type %> <%= hidden_input f, :redirect_uri, value: @redirect_uri %> @@ -40,4 +63,3 @@ <%= if Pleroma.Config.oauth_consumer_enabled?() do %> <%= render @view_module, Pleroma.Web.Auth.Authenticator.oauth_consumer_template(), assigns %> <% end %> - diff --git a/priv/static/instance/static.css b/priv/static/instance/static.css new file mode 100644 index 0000000000000000000000000000000000000000..487e1ec27d6a1f3f1403c811fd3d896353b37bfc GIT binary patch literal 5021 zcmb_gYi{E<5dQB|5Dp4t+t84lIEl6BqW5TvfRZSijU);bl}8tQi~j8CdXf%@q(n+e zkVOmc1~DJ={N|fsek5Nvgyiyt_To>`o-+7dm0VF+`n>tJ*Cdc$Xcq5 zT$J}Lxl2C7b=YCW<4MUO*iQE;+uzvjN-93zzTfY!-R(|^hN)Mo`HLRK=STQ3d%qbp zFXQd|BYa>ROw48ZYTZ~^@x;{S(z;JZY9(7uE7pkmH6uN1d)jj)DzoK0w3nC0;q{J{ zCB_SV0P|0x%?Nw}gLb0rHERn_&zwOp(YP}gr?bw;ZPGzx2^j^XZefaHMy%?2*ibR% z>dZ>{4C+YQy^tID4>E{47;nmZI-!1|g_wj&`wHoLVY0 zZ~BZBaEvQsZo^*avncgBBR7e&c=VdELCZIk>GRO!EfY0HArfN);Q*&tDF#pp-dKGm zqEy+SrF*lw>Wmh;<|rF83NYpwNpGzbTzYnq&t+YHEWJjN2@(^n%U)#nB4Hl1@1_mQ zYUZVU;;O1mD$k-Uc{_QYk?RBG)CSg7=yzp(v_H=G2)l&r(GT$ToVwhMAa>5)v6j$SQx2VzZo^u7c7(f(HlHjBNk;`>)?Bp75=4#38ii6?8O+awX1=HV6uTR`)Feg3F zXgh@F1d7xzxQzMi>=K)_DZiN4pq7J*pN&<<^Wfl0(MQNy4Tdwv^BGONosKVjpY(ly zk;)7a#Y+#TFr{;iV_K&96*2-ff#i}^vKV@Vqa3tJxNjNnu$N2ST;Psj53tfk82Qoa zkD=G1dId_x`OD5+l(#UIm;&{8^8u)}y_y!-&Mu*iF4KN6h%3=|bbsS&v%KkfV;QB3 zv;a~@N{%wP%2Z=;e5%}(;9>47dK@@R&l(3aXEwGtJBN&<;xXZuv7aN}O#jZv&_O3E zSdSGLwhlM*t(A^xJ4St81D3AY+g~o%UQd};=@DETL0{rh1s|IG(}#F;%;a4!a!}-nmTERxM*qzwu!Z} zC#IQwsyeMlSJ7Up{~i#>_bByA%m}@?11KOL1Q>v)@d%KQKctSwqkkICJ!-hN&xJ8v zMuDLfl7a!AbmK5M(xxqI7OzUrND=VttJ3m!EUFd7V%#WmvC^`Z2ahL?iDOji^5 z(+jx_pP|N@PYCGQTXhO_GQJ-dY>j~js|m4_HVT5Ln0d>%0nqg$6@*tRQY}_F||unn**5tb#vUIEN-Huh7i!T+RG+pRHeU yH@ get("/web/login", %{code: auth.token}) assert conn.status == 302 - assert redirected_to(conn) == path + assert redirected_to(conn) =~ path end test "redirects to the getting-started page when referer is not present", %{conn: conn} do @@ -49,7 +49,7 @@ test "redirects to the getting-started page when referer is not present", %{conn conn = get(conn, "/web/login", %{code: auth.token}) assert conn.status == 302 - assert redirected_to(conn) == "/web/getting-started" + assert redirected_to(conn) =~ "/web/getting-started" end end diff --git a/test/pleroma/web/mastodon_api/masto_fe_controller_test.exs b/test/pleroma/web/mastodon_api/masto_fe_controller_test.exs index ed8add8d2..b9cd050df 100644 --- a/test/pleroma/web/mastodon_api/masto_fe_controller_test.exs +++ b/test/pleroma/web/mastodon_api/masto_fe_controller_test.exs @@ -64,7 +64,8 @@ test "redirects not logged-in users to the login page on private instances", %{ end test "does not redirect logged in users to the login page", %{conn: conn, path: path} do - token = insert(:oauth_token, scopes: ["read"]) + {:ok, app} = Pleroma.Web.MastodonAPI.AuthController.local_mastofe_app() + token = insert(:oauth_token, app: app, scopes: ["read"]) conn = conn 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 9c1debd06..b7fe5785f 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -611,6 +611,41 @@ test "redirects with oauth authorization, " <> end end + test "authorize from cookie" do + user = insert(:user) + app = insert(:oauth_app) + oauth_token = insert(:oauth_token, user: user, app: app) + redirect_uri = OAuthController.default_redirect_uri(app) + + conn = + build_conn() + |> Plug.Session.call(Plug.Session.init(@session_opts)) + |> fetch_session() + |> AuthHelper.put_session_token(oauth_token.token) + |> post( + "/oauth/authorize", + %{ + "authorization" => %{ + "name" => user.nickname, + "client_id" => app.client_id, + "redirect_uri" => redirect_uri, + "scope" => app.scopes, + "state" => "statepassed" + } + } + ) + + 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 + end + test "redirect to on two-factor auth page" do otp_secret = TOTP.generate_secret() @@ -1221,8 +1256,8 @@ test "returns 500" do end end - describe "POST /oauth/revoke - bad request" do - test "returns 500" do + describe "POST /oauth/revoke" do + test "returns 500 on bad request" do response = build_conn() |> post("/oauth/revoke", %{})