CookieAuthPlug code quality improvements

# Conflicts:
#	CHANGELOG.md
#	lib/pleroma/web/templates/layout/app.html.eex

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Experimental websocket-based federation between Pleroma instances.
 - App metrics: ability to restrict access to specified IP whitelist.
 - Configuration: Add `:instance, autofollowing_nicknames` setting to provide a way to make accounts automatically follow new users that register on the local Pleroma instance.
+- OAuth form improvements: users are remembered by their cookie, the CSS is overridable by the admin, and the style has been improved.
 
 ### Changed
 

docs/configuration/static_dir.md

@@ -88,3 +88,7 @@ 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

@@ -2408,4 +2408,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/mastodon_api/controllers/auth_controller.ex

@@ -20,7 +20,7 @@ defmodule Pleroma.Web.MastodonAPI.AuthController do
   @local_mastodon_name "Mastodon-Local"
 
   @doc "GET /web/login"
-  def login(%{assigns: %{user: %User{}}} = conn, _params) do
+  def login(%{assigns: %{user: %User{}, token: _}} = conn, _params) do
     redirect(conn, to: local_mastodon_root_path(conn))
   end
 

lib/pleroma/web/o_auth/o_auth_controller.ex

@@ -79,6 +79,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
@@ -88,6 +95,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(), %{
+      app: app && Map.delete(app, :client_secret),
+      user: user,
       response_type: params["response_type"],
       client_id: params["client_id"],
       available_scopes: available_scopes,
@@ -131,11 +140,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)
@@ -364,7 +375,9 @@ defp handle_token_exchange_error(%Plug.Conn{} = conn, _error) 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
-      json(conn, %{})
+      conn
+      |> Plug.Conn.delete_session(:user_id)
+      |> json(%{})
     else
       _error ->
         # RFC 7009: invalid tokens [in the request] do not cause an error response

lib/pleroma/web/plugs/cookie_auth_plug.ex

@@ -0,0 +1,27 @@
+# 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.CookieAuthPlug do
+  alias Pleroma.User
+  import Plug.Conn
+
+  def init(opts) do
+    opts
+  end
+
+  # If the user is already assigned (by a bearer token, probably), skip ahead.
+  def call(%{assigns: %{user: %User{}}} = conn, _), do: conn
+
+  # Authenticate with a session cookie, if available.
+  # For staticly-rendered pages (like the OAuth form)
+  # this is the only way it can authenticate.
+  def call(conn, _) do
+    with user_id when is_binary(user_id) <- get_session(conn, :user_id),
+         %User{} = user <- User.get_by_id(user_id) do
+      assign(conn, :user, user)
+    else
+      _ -> conn
+    end
+  end
+end

lib/pleroma/web/router.ex

@@ -33,7 +33,9 @@ defmodule Pleroma.Web.Router do
   pipeline :oauth do
     plug(:fetch_session)
     plug(Pleroma.Web.Plugs.OAuthPlug)
+    plug(Pleroma.Web.Plugs.CookieAuthPlug)
     plug(Pleroma.Web.Plugs.UserEnabledPlug)
+    plug(Pleroma.Web.Plugs.EnsureUserKeyPlug)
   end
 
   pipeline :expect_authentication do
@@ -319,9 +321,9 @@ defmodule Pleroma.Web.Router 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)

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

@@ -1,233 +1,19 @@
 <!DOCTYPE html>
-<html>
+<html lang="en">
   <head>
-    <meta charset="utf-8" />
-    <meta name="viewport" content="width=device-width,initial-scale=1,minimal-ui" />
-    <title>
-    <%= Pleroma.Config.get([:instance, :name]) %>
-    </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>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width,initial-scale=1,minimal-ui">
+    <title><%= Pleroma.Config.get([:instance, :name]) %></title>
+    <link rel="stylesheet" href="/instance/static.css">
   </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,32 +5,55 @@
 <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 %>
-  <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">
-    <%= label f, :nickname, "Pleroma Handle" %>
-    <%= text_input f, :nickname, placeholder: "lain" %>
+<%= 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>
-  <%= hidden_input f, :name, value: @params["name"] %>
-  <%= hidden_input f, :password, value: @params["password"] %>
-  <br>
-<% else %>
-  <div class="input">
-    <%= label f, :name, "Username" %>
-    <%= text_input f, :name %>
-  </div>
-  <div class="input">
-    <%= label f, :password, "Password" %>
-    <%= password_input f, :password %>
-  </div>
-  <%= submit "Log In" %>
-  <%= render @view_module, "_scopes.html", Map.merge(assigns, %{form: f}) %>
 <% 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">
+        <%= label f, :nickname, "Pleroma Handle" %>
+        <%= text_input f, :nickname, placeholder: "lain" %>
+      </div>
+      <%= hidden_input f, :name, value: @params["name"] %>
+      <%= hidden_input f, :password, value: @params["password"] %>
+      <br>
+    <% else %>
+      <div class="input">
+        <%= label f, :name, "Username" %>
+        <%= text_input f, :name %>
+      </div>
+      <div class="input">
+        <%= label f, :password, "Password" %>
+        <%= password_input f, :password %>
+      </div>
+      <%= submit "Log In" %>
+    <% end %>
+  <% end %>
+</div>
+
 <%= 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 %>
-

test/pleroma/user_test.exs

@@ -2139,4 +2139,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/o_auth/o_auth_controller_test.exs

@@ -609,6 +609,43 @@ test "redirects with oauth authorization, " <>
       end
     end
 
+    test "authorize from cookie" do
+      app_scopes = ["read", "write"]
+      app = insert(:oauth_app)
+      redirect_uri = OAuthController.default_redirect_uri(app)
+      user = insert(:user)
+
+      conn =
+        build_conn()
+        |> Plug.Session.call(Plug.Session.init(@session_opts))
+        |> fetch_session()
+        |> put_session(:user_id, user.id)
+        |> post(
+          "/oauth/authorize",
+          %{
+            "authorization" => %{
+              "name" => user.nickname,
+              "client_id" => app.client_id,
+              "redirect_uri" => redirect_uri,
+              "scope" => app_scopes,
+              "state" => "statepassed"
+            }
+          }
+        )
+
+      assert Enum.count(Repo.all(Pleroma.Web.OAuth.Authorization)) == 1
+
+      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()
 
@@ -1219,6 +1256,44 @@ test "returns 500" do
     end
   end
 
+  describe "POST /oauth/revoke" do
+    test "deletes a token" do
+      app = insert(:oauth_app, scopes: ["read"])
+      token = insert(:oauth_token, app: app)
+
+      result =
+        build_conn()
+        |> post("/oauth/revoke", %{
+          "client_id" => app.client_id,
+          "client_secret" => app.client_secret,
+          "token" => token.token
+        })
+        |> json_response(200)
+
+      assert result == %{}
+      assert {:error, :not_found} = Pleroma.Web.OAuth.Token.get_by_token(app, token.token)
+    end
+
+    test "clears the session_id from user cookies" do
+      user = insert(:user)
+      app = insert(:oauth_app, scopes: ["read"])
+      token = insert(:oauth_token, app: app, user: user)
+
+      conn =
+        build_conn()
+        |> Plug.Session.call(Plug.Session.init(@session_opts))
+        |> fetch_session()
+        |> put_session(:user_id, user.id)
+        |> post("/oauth/revoke", %{
+          "client_id" => app.client_id,
+          "client_secret" => app.client_secret,
+          "token" => token.token
+        })
+
+      refute get_session(conn, :user_id)
+    end
+  end
+
   describe "POST /oauth/revoke - bad request" do
     test "returns 500" do
       response =

test/pleroma/web/plugs/cookie_auth_plug_test.exs

@@ -0,0 +1,48 @@
+# 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.CookieAuthPlugTest do
+  use Pleroma.Web.ConnCase, async: true
+  alias Pleroma.Web.Plugs.CookieAuthPlug
+  import Pleroma.Factory
+
+  @session_opts [
+    store: :cookie,
+    key: "_test",
+    signing_salt: "cooldude"
+  ]
+
+  setup %{conn: conn} do
+    conn =
+      conn
+      |> Plug.Session.call(Plug.Session.init(@session_opts))
+      |> fetch_session()
+
+    %{conn: conn}
+  end
+
+  test "if the conn has a user key set, it does nothing", %{conn: conn} do
+    conn = assign(conn, :user, 1)
+    result = CookieAuthPlug.call(conn, %{})
+
+    assert result == conn
+  end
+
+  test "if the session has a user_id, it sets the user", %{conn: conn} do
+    user = insert(:user)
+
+    conn =
+      conn
+      |> put_session(:user_id, user.id)
+      |> CookieAuthPlug.call(%{})
+
+    assert conn.assigns[:user] == user
+  end
+
+  test "if the conn has no key set, it does nothing", %{conn: conn} do
+    result = CookieAuthPlug.call(conn, %{})
+
+    assert result == conn
+  end
+end