OAuth form user remembering feature. Local MastoFE login / logout fixes.

.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
+# Most os js/css files included in the repo are minified bundles,
-# to search/diff those as text files.
+#   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

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.
 
 <details>
   <summary>API Changes</summary>

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.

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

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,8 +28,9 @@ defmodule Pleroma.Web.MastoFEController do
   )
 
   @doc "GET /web/*path"
-  def index(%{assigns: %{user: user, token: token}} = conn, _params)
+  def index(conn, _params) do
-      when not is_nil(user) and not is_nil(token) do
+    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",
@@ -35,18 +38,17 @@ def index(%{assigns: %{user: user, token: token}} = conn, _params)
         user: user,
         custom_emojis: Pleroma.Emoji.get_all()
       )
-  end
+    else
-
+      _ ->
-  def index(conn, _params) do
         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(conn, "manifest.json")
-    |> render("manifest.json")
   end
 
   @doc "PUT /api/web/settings: Backend-obscure settings blob for MastoFE, don't parse/reuse elsewhere"

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
+  # Local Mastodon FE login callback action
-    redirect(conn, to: local_mastodon_root_path(conn))
+  def login(conn, %{"code" => auth_token} = params) do
-  end
+    with {:ok, app} <- local_mastofe_app(),
-
-  # Local Mastodon FE login init action
-  def login(conn, %{"code" => auth_token}) do
-    with {:ok, app} <- get_or_make_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
-      |> AuthHelper.put_session_token(token.token)
+        |> local_mastodon_post_login_path()
-      |> redirect(to: local_mastodon_root_path(conn))
+        |> 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)
     end
   end
 
-  # Local Mastodon FE callback action
+  def login(conn, params) do
-  def login(conn, _) do
+    with %{assigns: %{user: %User{}, token: %Token{app_id: app_id}}} <- conn,
-    with {:ok, app} <- get_or_make_app() do
+         {: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
+    conn =
-    |> clear_session()
+      with %{assigns: %{token: %Token{} = oauth_token}} <- conn,
-    |> redirect(to: "/")
+           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()}
+  @spec local_mastofe_app() :: {:ok, App.t()} | {:error, Ecto.Changeset.t()}
-  defp get_or_make_app do
+  def local_mastofe_app do
-    %{client_name: @local_mastodon_name, redirect_uris: "."}
+    App.get_or_make(
-    |> App.get_or_make(["read", "write", "follow", "push", "admin"])
+      %{client_name: @local_mastodon_name, redirect_uris: "."},
+      ["read", "write", "follow", "push", "admin"]
+    )
   end
 end

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(
+  def create_authorization(_, _, opts \\ [])
-        %Plug.Conn{} = conn,
+
-        %{"authorization" => _} = params,
+  def create_authorization(%Plug.Conn{assigns: %{user: %User{} = user}} = conn, params, []) do
-        opts \\ []
+    create_authorization(conn, params, user: user)
-      ) do
+  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)

lib/pleroma/web/templates/layout/app.html.eex

@@ -1,233 +1,19 @@
 <!DOCTYPE html>
-<html>
+<html lang="en">
   <head>
-    <meta charset="utf-8" />
+    <meta charset="utf-8">
-    <meta name="viewport" content="width=device-width,initial-scale=1,minimal-ui" />
+    <meta name="viewport" content="width=device-width,initial-scale=1,minimal-ui">
-    <title>
+    <title><%= Pleroma.Config.get([:instance, :name]) %></title>
-    <%= Pleroma.Config.get([:instance, :name]) %>
+    <link rel="stylesheet" href="/instance/static.css">
-    </title>
-    <style>
-      body {
-        background-color: #121a24;
-        font-family: sans-serif;
-        color: #b9b9ba;
-        text-align: center;
-      }
-
-      .container {
-        max-width: 420px;
-        padding: 20px;
-        background-color: #182230;
-        border-radius: 4px;
-        margin: auto;
-        margin-top: 10vh;
-        box-shadow: 0 1px 4px 0px rgba(0, 0, 0, 0.5);
-      }
-
-      h1 {
-        margin: 0;
-        font-size: 24px;
-      }
-
-      h2 {
-        color: #b9b9ba;
-        font-weight: normal;
-        font-size: 18px;
-        margin-bottom: 20px;
-      }
-
-      a {
-        color: #d8a070;
-        text-decoration: none;
-      }
-
-      form {
-        width: 100%;
-      }
-
-      .input {
-        text-align: left;
-        color: #89898a;
-        display: flex;
-        flex-direction: column;
-      }
-
-      input {
-        box-sizing: content-box;
-        padding: 10px;
-        margin-top: 5px;
-        margin-bottom: 10px;
-        background-color: #121a24;
-        color: #b9b9ba;
-        border: 0;
-        transition-property: border-bottom;
-        transition-duration: 0.35s;
-        border-bottom: 2px solid #2a384a;
-        font-size: 14px;
-      }
-
-      .scopes-input {
-        display: flex;
-        flex-direction: column;
-        margin-top: 1em;
-        text-align: left;
-        color: #89898a;
-      }
-
-      .scopes-input label:first-child {
-        height: 2em;
-      }
-
-      .scopes {
-        display: flex;
-        flex-wrap: wrap;
-        text-align: left;
-        color: #b9b9ba;
-      }
-
-      .scope {
-        display: flex;
-        flex-basis: 100%;
-        height: 2em;
-        align-items: center;
-      }
-
-      .scope:before {
-        color: #b9b9ba;
-        content: "✔\fe0e";
-        margin-left: 1em;
-        margin-right: 1em;
-      }
-
-      [type="checkbox"] + label {
-        display: none;
-        cursor: pointer;
-        margin: 0.5em;
-      }
-
-      [type="checkbox"] {
-        display: none;
-      }
-
-      [type="checkbox"] + label:before {
-        cursor: pointer;
-        display: inline-block;
-        color: white;
-        background-color: #121a24;
-        border: 4px solid #121a24;
-        box-shadow: 0px 0px 1px 0 #d8a070;
-        box-sizing: border-box;
-        width: 1.2em;
-        height: 1.2em;
-        margin-right: 1.0em;
-        content: "";
-        transition-property: background-color;
-        transition-duration: 0.35s;
-        color: #121a24;
-        margin-bottom: -0.2em;
-        border-radius: 2px;
-      }
-
-      [type="checkbox"]:checked + label:before {
-        background-color: #d8a070;
-      }
-
-      input:focus {
-        outline: none;
-        border-bottom: 2px solid #d8a070;
-      }
-
-      button {
-        box-sizing: border-box;
-        width: 100%;
-        background-color: #1c2a3a;
-        color: #b9b9ba;
-        border-radius: 4px;
-        border: none;
-        padding: 10px;
-        margin-top: 20px;
-        margin-bottom: 20px;
-        text-transform: uppercase;
-        font-size: 16px;
-        box-shadow: 0px 0px 2px 0px black,
-          0px 1px 0px 0px rgba(255, 255, 255, 0.2) inset,
-          0px -1px 0px 0px rgba(0, 0, 0, 0.2) inset;
-      }
-
-      button:hover {
-        cursor: pointer;
-        box-shadow: 0px 0px 0px 1px #d8a070,
-          0px 1px 0px 0px rgba(255, 255, 255, 0.2) inset,
-          0px -1px 0px 0px rgba(0, 0, 0, 0.2) inset;
-      }
-
-      .alert-danger {
-        box-sizing: border-box;
-        width: 100%;
-        background-color: #931014;
-        border: 1px solid #a06060;
-        border-radius: 4px;
-        padding: 10px;
-        margin-top: 20px;
-        font-weight: 500;
-        font-size: 16px;
-      }
-
-      .alert-info {
-        box-sizing: border-box;
-        width: 100%;
-        border-radius: 4px;
-        border: 1px solid #7d796a;
-        padding: 10px;
-        margin-top: 20px;
-        font-weight: 500;
-        font-size: 16px;
-      }
-
-      @media all and (max-width: 440px) {
-        .container {
-          margin-top: 0
-        }
-
-        .scope {
-          flex-basis: 0%;
-        }
-
-        .scope:before {
-          content: "";
-          margin-left: 0em;
-          margin-right: 1em;
-        }
-
-        .scope:first-child:before {
-          margin-left: 1em;
-          content: "✔\fe0e";
-        }
-
-        .scope:after {
-          content: ",";
-        }
-
-        .scope:last-child:after {
-          content: "";
-        }
-      }
-      .form-row {
-        display: flex;
-      }
-      .form-row > label {
-        text-align: left;
-        line-height: 47px;
-        flex: 1;
-      }
-      .form-row > input {
-        flex: 2;
-      }
-    </style>
   </head>
   <body>
+    <div class="instance-header">
+      <a class="instance-header__content" href="/">
+        <img class="instance-header__thumbnail" src="<%= Pleroma.Config.get([:instance, :instance_thumbnail]) %>">
+        <h1 class="instance-header__title"><%= Pleroma.Config.get([:instance, :name]) %></h1>
+      </a>
+    </div>
     <div class="container">
-      <h1><%= Pleroma.Config.get([:instance, :name]) %></h1>
       <%= @inner_content %>
     </div>
   </body>

lib/pleroma/web/templates/o_auth/o_auth/show.html.eex

@@ -5,10 +5,32 @@
 <p class="alert alert-danger" role="alert"><%= get_flash(@conn, :error) %></p>
 <% end %>
 
-<h2>OAuth Authorization</h2>
 <%= form_for @conn, o_auth_path(@conn, :authorize), [as: "authorization"], fn f -> %>
 
-<%= if @params["registration"] in ["true", true] do %>
+<%= if @user do %>
+  <div class="account-header">
+    <div class="account-header__banner" style="background-image: url('<%= Pleroma.User.banner_url(@user) %>')"></div>
+    <div class="account-header__avatar" style="background-image: url('<%= Pleroma.User.avatar_url(@user) %>')"></div>
+    <div class="account-header__meta">
+      <div class="account-header__display-name"><%= @user.name %></div>
+      <div class="account-header__nickname">@<%= @user.nickname %>@<%= Pleroma.User.get_host(@user) %></div>
+    </div>
+  </div>
+<% end %>
+
+<div class="container__content">
+  <%= if @app do %>
+    <p>Application <strong><%= @app.client_name %></strong> is requesting access to your account.</p>
+    <%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %>
+  <% end %>
+
+  <%= if @user do %>
+    <div class="actions">
+      <a class="button button--cancel" href="/">Cancel</a>
+      <%= submit "Approve", class: "button--approve" %>
+    </div>
+  <% else %>
+    <%= if @params["registration"] in ["true", true] do %>
       <h3>This is the first time you visit! Please enter your Pleroma handle.</h3>
       <p>Choose carefully! You won't be able to change this later. You will be able to change your display name, though.</p>
       <div class="input">
@@ -18,7 +40,7 @@
       <%= hidden_input f, :name, value: @params["name"] %>
       <%= hidden_input f, :password, value: @params["password"] %>
       <br>
-<% else %>
+    <% else %>
       <div class="input">
         <%= label f, :name, "Username" %>
         <%= text_input f, :name %>
@@ -28,8 +50,9 @@
         <%= password_input f, :password %>
       </div>
       <%= submit "Log In" %>
-  <%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %>
+    <% end %>
-<% end %>
+  <% end %>
+</div>
 
 <%= hidden_input f, :client_id, value: @client_id %>
 <%= hidden_input f, :response_type, value: @response_type %>
@@ -40,4 +63,3 @@
 <%= if Pleroma.Config.oauth_consumer_enabled?() do %>
   <%= render @view_module, Pleroma.Web.Auth.Authenticator.oauth_consumer_template(), assigns %>
 <% end %>
-

test/pleroma/user_test.exs

@@ -2171,4 +2171,9 @@ test "avatar fallback" do
 
     assert User.avatar_url(user, no_default: true) == nil
   end
+
+  test "get_host/1" do
+    user = insert(:user, ap_id: "https://lain.com/users/lain", nickname: "lain")
+    assert User.get_host(user) == "lain.com"
+  end
 end

test/pleroma/web/mastodon_api/controllers/auth_controller_test.exs

@@ -39,7 +39,7 @@ test "redirects to the saved path after log in", %{conn: conn, path: path} do
         |> 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
 

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

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
+  describe "POST /oauth/revoke" do
-    test "returns 500" do
+    test "returns 500 on bad request" do
       response =
         build_conn()
         |> post("/oauth/revoke", %{})