[#923] Nickname & email selection for external registrations, option to connect to existing account.

lib/pleroma/registration.ex

@@ -11,6 +11,8 @@ defmodule Pleroma.Registration do
   alias Pleroma.Repo
   alias Pleroma.User
 
+  @primary_key {:id, Pleroma.FlakeId, autogenerate: true}
+
   schema "registrations" do
     belongs_to(:user, User, type: Pleroma.FlakeId)
     field(:provider, :string)
@@ -20,6 +22,18 @@ defmodule Pleroma.Registration do
     timestamps()
   end
 
+  def nickname(registration, default \\ nil),
+    do: Map.get(registration.info, "nickname", default)
+
+  def email(registration, default \\ nil),
+    do: Map.get(registration.info, "email", default)
+
+  def name(registration, default \\ nil),
+    do: Map.get(registration.info, "name", default)
+
+  def description(registration, default \\ nil),
+    do: Map.get(registration.info, "description", default)
+
   def changeset(registration, params \\ %{}) do
     registration
     |> cast(params, [:user_id, :provider, :uid, :info])
@@ -28,6 +42,12 @@ def changeset(registration, params \\ %{}) do
     |> unique_constraint(:uid, name: :registrations_provider_uid_index)
   end
 
+  def bind_to_user(registration, user) do
+    registration
+    |> changeset(%{user_id: (user && user.id) || nil})
+    |> Repo.update()
+  end
+
   def get_by_provider_uid(provider, uid) do
     Repo.get_by(Registration,
       provider: to_string(provider),

lib/pleroma/web/auth/authenticator.ex

@@ -4,6 +4,7 @@
 
 defmodule Pleroma.Web.Auth.Authenticator do
   alias Pleroma.User
+  alias Pleroma.Registration
 
   def implementation do
     Pleroma.Config.get(
@@ -15,10 +16,15 @@ def implementation do
   @callback get_user(Plug.Conn.t(), Map.t()) :: {:ok, User.t()} | {:error, any()}
   def get_user(plug, params), do: implementation().get_user(plug, params)
 
-  @callback get_by_external_registration(Plug.Conn.t(), Map.t()) ::
+  @callback create_from_registration(Plug.Conn.t(), Map.t(), Registration.t()) ::
               {:ok, User.t()} | {:error, any()}
-  def get_by_external_registration(plug, params),
+  def create_from_registration(plug, params, registration),
-    do: implementation().get_by_external_registration(plug, params)
+    do: implementation().create_from_registration(plug, params, registration)
+
+  @callback get_registration(Plug.Conn.t(), Map.t()) ::
+              {:ok, Registration.t()} | {:error, any()}
+  def get_registration(plug, params),
+    do: implementation().get_registration(plug, params)
 
   @callback handle_error(Plug.Conn.t(), any()) :: any()
   def handle_error(plug, error), do: implementation().handle_error(plug, error)

lib/pleroma/web/auth/ldap_authenticator.ex

@@ -8,10 +8,15 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do
   require Logger
 
   @behaviour Pleroma.Web.Auth.Authenticator
+  @base Pleroma.Web.Auth.PleromaAuthenticator
 
   @connection_timeout 10_000
   @search_timeout 10_000
 
+  defdelegate get_registration(conn, params), to: @base
+
+  defdelegate create_from_registration(conn, params, registration), to: @base
+
   def get_user(%Plug.Conn{} = conn, params) do
     if Pleroma.Config.get([:ldap, :enabled]) do
       {name, password} =
@@ -29,19 +34,17 @@ def get_user(%Plug.Conn{} = conn, params) do
 
         {:error, {:ldap_connection_error, _}} ->
           # When LDAP is unavailable, try default authenticator
-          Pleroma.Web.Auth.PleromaAuthenticator.get_user(conn, params)
+          @base.get_user(conn, params)
 
         error ->
           error
       end
     else
       # Fall back to default authenticator
-      Pleroma.Web.Auth.PleromaAuthenticator.get_user(conn, params)
+      @base.get_user(conn, params)
     end
   end
 
-  def get_by_external_registration(conn, params), do: get_user(conn, params)
-
   def handle_error(%Plug.Conn{} = _conn, error) do
     error
   end

lib/pleroma/web/auth/pleroma_authenticator.ex

@@ -29,68 +29,63 @@ def get_user(%Plug.Conn{} = _conn, params) do
     end
   end
 
-  def get_by_external_registration(
+  def get_registration(
         %Plug.Conn{assigns: %{ueberauth_auth: %{provider: provider, uid: uid} = auth}},
         _params
       ) do
     registration = Registration.get_by_provider_uid(provider, uid)
 
     if registration do
-      user = Repo.preload(registration, :user).user
+      {:ok, registration}
-      {:ok, user}
     else
       info = auth.info
-      email = info.email
-      nickname = info.nickname
 
-      # Note: nullifying email in case this email is already taken
+      Registration.changeset(%Registration{}, %{
-      email =
+        provider: to_string(provider),
-        if email && User.get_by_email(email) do
+        uid: to_string(uid),
-          nil
+        info: %{
-        else
+          "nickname" => info.nickname,
-          email
+          "email" => info.email,
-        end
+          "name" => info.name,
-
+          "description" => info.description
-      # Note: generating a random numeric suffix to nickname in case this nickname is already taken
+        }
-      nickname =
+      })
-        if nickname && User.get_by_nickname(nickname) do
+      |> Repo.insert()
-          "#{nickname}#{:os.system_time()}"
-        else
-          nickname
-        end
-
-      random_password = :crypto.strong_rand_bytes(64) |> Base.encode64()
-
-      with {:ok, new_user} <-
-             User.register_changeset(
-               %User{},
-               %{
-                 name: info.name,
-                 bio: info.description,
-                 email: email,
-                 nickname: nickname,
-                 password: random_password,
-                 password_confirmation: random_password
-               },
-               external: true,
-               confirmed: true
-             )
-             |> Repo.insert(),
-           {:ok, _} <-
-             Registration.changeset(%Registration{}, %{
-               user_id: new_user.id,
-               provider: to_string(provider),
-               uid: to_string(uid),
-               info: %{nickname: info.nickname, email: info.email}
-             })
-             |> Repo.insert() do
-        {:ok, new_user}
-      end
     end
   end
 
-  def get_by_external_registration(%Plug.Conn{} = _conn, _params),
+  def get_registration(%Plug.Conn{} = _conn, _params), do: {:error, :missing_credentials}
-    do: {:error, :missing_credentials}
+
+  def create_from_registration(_conn, params, registration) do
+    nickname = value([params["nickname"], Registration.nickname(registration)])
+    email = value([params["email"], Registration.email(registration)])
+    name = value([params["name"], Registration.name(registration)]) || nickname
+    bio = value([params["bio"], Registration.description(registration)])
+
+    random_password = :crypto.strong_rand_bytes(64) |> Base.encode64()
+
+    with {:ok, new_user} <-
+           User.register_changeset(
+             %User{},
+             %{
+               email: email,
+               nickname: nickname,
+               name: name,
+               bio: bio,
+               password: random_password,
+               password_confirmation: random_password
+             },
+             external: true,
+             confirmed: true
+           )
+           |> Repo.insert(),
+         {:ok, _} <-
+           Registration.changeset(registration, %{user_id: new_user.id}) |> Repo.update() do
+      {:ok, new_user}
+    end
+  end
+
+  defp value(list), do: Enum.find(list, &(to_string(&1) != ""))
 
   def handle_error(%Plug.Conn{} = _conn, error) do
     error

lib/pleroma/web/oauth/oauth_controller.ex

@@ -7,6 +7,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do
 
   alias Pleroma.Repo
   alias Pleroma.User
+  alias Pleroma.Registration
   alias Pleroma.Web.Auth.Authenticator
   alias Pleroma.Web.OAuth.App
   alias Pleroma.Web.OAuth.Authorization
@@ -21,52 +22,6 @@ defmodule Pleroma.Web.OAuth.OAuthController do
 
   action_fallback(Pleroma.Web.OAuth.FallbackController)
 
-  def request(conn, params) do
-    message =
-      if params["provider"] do
-        "Unsupported OAuth provider: #{params["provider"]}."
-      else
-        "Bad OAuth request."
-      end
-
-    conn
-    |> put_flash(:error, message)
-    |> redirect(to: "/")
-  end
-
-  def callback(%{assigns: %{ueberauth_failure: failure}} = conn, %{"redirect_uri" => redirect_uri}) do
-    messages = for e <- Map.get(failure, :errors, []), do: e.message
-    message = Enum.join(messages, "; ")
-
-    conn
-    |> put_flash(:error, "Failed to authenticate: #{message}.")
-    |> redirect(external: redirect_uri(conn, redirect_uri))
-  end
-
-  def callback(
-        conn,
-        %{"client_id" => client_id, "redirect_uri" => redirect_uri} = params
-      ) do
-    with {:ok, user} <- Authenticator.get_by_external_registration(conn, params) do
-      do_create_authorization(
-        conn,
-        %{
-          "authorization" => %{
-            "client_id" => client_id,
-            "redirect_uri" => redirect_uri,
-            "scope" => oauth_scopes(params, nil)
-          }
-        },
-        user
-      )
-    else
-      _ ->
-        conn
-        |> put_flash(:error, "Failed to set up user account.")
-        |> redirect(external: redirect_uri(conn, redirect_uri))
-    end
-  end
-
   def authorize(conn, params) do
     app = Repo.get_by(App, client_id: params["client_id"])
     available_scopes = (app && app.scopes) || []
@@ -83,29 +38,16 @@ def authorize(conn, params) do
     })
   end
 
-  def create_authorization(conn, params), do: do_create_authorization(conn, params, nil)
+  def create_authorization(
-
+        conn,
-  defp do_create_authorization(
+        %{
-         conn,
+          "authorization" => %{"redirect_uri" => redirect_uri} = auth_params
-         %{
+        } = params,
-           "authorization" =>
+        opts \\ []
-             %{
+      ) do
-               "client_id" => client_id,
+    with {:ok, auth} <-
-               "redirect_uri" => redirect_uri
+           (opts[:auth] && {:ok, opts[:auth]}) ||
-             } = auth_params
+             do_create_authorization(conn, params, opts[:user]) do
-         } = params,
-         user
-       ) do
-    with {_, {:ok, %User{} = user}} <-
-           {:get_user, (user && {:ok, user}) || Authenticator.get_user(conn, params)},
-         %App{} = app <- Repo.get_by(App, client_id: client_id),
-         true <- redirect_uri in String.split(app.redirect_uris),
-         scopes <- oauth_scopes(auth_params, []),
-         {:unsupported_scopes, []} <- {:unsupported_scopes, scopes -- app.scopes},
-         # Note: `scope` param is intentionally not optional in this context
-         {:missing_scopes, false} <- {:missing_scopes, scopes == []},
-         {:auth_active, true} <- {:auth_active, User.auth_active?(user)},
-         {:ok, auth} <- Authorization.create_authorization(app, user, scopes) do
       redirect_uri = redirect_uri(conn, redirect_uri)
 
       cond do
@@ -232,6 +174,166 @@ def token_revoke(conn, %{"token" => token} = params) do
     end
   end
 
+  def request(conn, params) do
+    message =
+      if params["provider"] do
+        "Unsupported OAuth provider: #{params["provider"]}."
+      else
+        "Bad OAuth request."
+      end
+
+    conn
+    |> put_flash(:error, message)
+    |> redirect(to: "/")
+  end
+
+  def callback(%{assigns: %{ueberauth_failure: failure}} = conn, %{"redirect_uri" => redirect_uri}) do
+    messages = for e <- Map.get(failure, :errors, []), do: e.message
+    message = Enum.join(messages, "; ")
+
+    conn
+    |> put_flash(:error, "Failed to authenticate: #{message}.")
+    |> redirect(external: redirect_uri(conn, redirect_uri))
+  end
+
+  def callback(
+        conn,
+        %{"client_id" => client_id, "redirect_uri" => redirect_uri} = params
+      ) do
+    with {:ok, registration} <- Authenticator.get_registration(conn, params) do
+      user = Repo.preload(registration, :user).user
+
+      auth_params = %{
+        "client_id" => client_id,
+        "redirect_uri" => redirect_uri,
+        "scopes" => oauth_scopes(params, nil)
+      }
+
+      if user do
+        create_authorization(
+          conn,
+          %{"authorization" => auth_params},
+          user: user
+        )
+      else
+        registration_params =
+          Map.merge(auth_params, %{
+            "nickname" => Registration.nickname(registration),
+            "email" => Registration.email(registration)
+          })
+
+        conn
+        |> put_session(:registration_id, registration.id)
+        |> redirect(to: o_auth_path(conn, :registration_details, registration_params))
+      end
+    else
+      _ ->
+        conn
+        |> put_flash(:error, "Failed to set up user account.")
+        |> redirect(external: redirect_uri(conn, redirect_uri))
+    end
+  end
+
+  def registration_details(conn, params) do
+    render(conn, "register.html", %{
+      client_id: params["client_id"],
+      redirect_uri: params["redirect_uri"],
+      scopes: oauth_scopes(params, []),
+      nickname: params["nickname"],
+      email: params["email"]
+    })
+  end
+
+  def register(conn, %{"op" => "connect"} = params) do
+    create_authorization_params = %{
+      "authorization" => Map.merge(params, %{"name" => params["auth_name"]})
+    }
+
+    with registration_id when not is_nil(registration_id) <- get_session_registration_id(conn),
+         %Registration{} = registration <- Repo.get(Registration, registration_id),
+         {:ok, auth} <- do_create_authorization(conn, create_authorization_params),
+         %User{} = user <- Repo.preload(auth, :user).user,
+         {:ok, _updated_registration} <- Registration.bind_to_user(registration, user) do
+      conn
+      |> put_session_registration_id(nil)
+      |> create_authorization(
+        create_authorization_params,
+        auth: auth
+      )
+    else
+      _ ->
+        conn
+        |> put_flash(:error, "Unknown error, please try again.")
+        |> redirect(to: o_auth_path(conn, :registration_details, params))
+    end
+  end
+
+  def register(conn, params) do
+    with registration_id when not is_nil(registration_id) <- get_session_registration_id(conn),
+         %Registration{} = registration <- Repo.get(Registration, registration_id),
+         {:ok, user} <- Authenticator.create_from_registration(conn, params, registration) do
+      conn
+      |> put_session_registration_id(nil)
+      |> create_authorization(
+        %{
+          "authorization" => %{
+            "client_id" => params["client_id"],
+            "redirect_uri" => params["redirect_uri"],
+            "scopes" => oauth_scopes(params, nil)
+          }
+        },
+        user: user
+      )
+    else
+      {:error, changeset} ->
+        message =
+          Enum.map(changeset.errors, fn {field, {error, _}} ->
+            "#{field} #{error}"
+          end)
+          |> Enum.join("; ")
+
+        message =
+          String.replace(
+            message,
+            "ap_id has already been taken",
+            "nickname has already been taken"
+          )
+
+        conn
+        |> put_flash(:error, "Error: #{message}.")
+        |> redirect(to: o_auth_path(conn, :registration_details, params))
+
+      _ ->
+        conn
+        |> put_flash(:error, "Unknown error, please try again.")
+        |> redirect(to: o_auth_path(conn, :registration_details, params))
+    end
+  end
+
+  defp do_create_authorization(
+         conn,
+         %{
+           "authorization" =>
+             %{
+               "client_id" => client_id,
+               "redirect_uri" => redirect_uri
+             } = auth_params
+         } = params,
+         user \\ nil
+       ) do
+    with {_, {:ok, %User{} = user}} <-
+           {:get_user, (user && {:ok, user}) || Authenticator.get_user(conn, params)},
+         %App{} = app <- Repo.get_by(App, client_id: client_id),
+         true <- redirect_uri in String.split(app.redirect_uris),
+         scopes <- oauth_scopes(auth_params, []),
+         {:unsupported_scopes, []} <- {:unsupported_scopes, scopes -- app.scopes},
+         # Note: `scope` param is intentionally not optional in this context
+         {:missing_scopes, false} <- {:missing_scopes, scopes == []},
+         {:auth_active, true} <- {:auth_active, User.auth_active?(user)} do
+      Authorization.create_authorization(app, user, scopes)
+    end
+  end
+
   # XXX - for whatever reason our token arrives urlencoded, but Plug.Conn should be
   # decoding it.  Investigate sometime.
   defp fix_padding(token) do
@@ -269,4 +371,9 @@ defp get_app_from_request(conn, params) do
   defp redirect_uri(conn, "."), do: mastodon_api_url(conn, :login)
 
   defp redirect_uri(_conn, redirect_uri), do: redirect_uri
+
+  defp get_session_registration_id(conn), do: get_session(conn, :registration_id)
+
+  defp put_session_registration_id(conn, registration_id),
+    do: put_session(conn, :registration_id, registration_id)
 end

lib/pleroma/web/router.ex

@@ -208,12 +208,14 @@ defmodule Pleroma.Web.Router do
     post("/authorize", OAuthController, :create_authorization)
     post("/token", OAuthController, :token_exchange)
     post("/revoke", OAuthController, :token_revoke)
+    get("/registration_details", OAuthController, :registration_details)
 
     scope [] do
       pipe_through(:browser)
 
       get("/:provider", OAuthController, :request)
       get("/:provider/callback", OAuthController, :callback)
+      post("/register", OAuthController, :register)
     end
   end
 

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

@@ -0,0 +1,48 @@
+<%= if get_flash(@conn, :info) do %>
+  <p class="alert alert-info" role="alert"><%= get_flash(@conn, :info) %></p>
+<% end %>
+<%= if get_flash(@conn, :error) do %>
+  <p class="alert alert-danger" role="alert"><%= get_flash(@conn, :error) %></p>
+<% end %>
+
+<h2>Registration Details</h2>
+
+<p>If you'd like to register a new account,
+<br>
+please provide the details below.</p>
+<br>
+
+<%= form_for @conn, o_auth_path(@conn, :register), [], fn f -> %>
+
+<div class="input">
+  <%= label f, :nickname, "Nickname" %>
+  <%= text_input f, :nickname, value: @nickname %>
+</div>
+<div class="input">
+  <%= label f, :email, "Email" %>
+  <%= text_input f, :email, value: @email %>
+</div>
+
+<%= submit "Proceed as new user", name: "op", value: "register" %>
+
+<br>
+<br>
+<br>
+<p>Alternatively, sign in to connect to existing account.</p>
+
+<div class="input">
+  <%= label f, :auth_name, "Name or email" %>
+  <%= text_input f, :auth_name %>
+</div>
+<div class="input">
+  <%= label f, :password, "Password" %>
+  <%= password_input f, :password %>
+</div>
+
+<%= submit "Proceed as existing user", name: "op", value: "connect" %>
+
+<%= hidden_input f, :client_id, value: @client_id %>
+<%= hidden_input f, :redirect_uri, value: @redirect_uri %>
+<%= hidden_input f, :scope, value: Enum.join(@scopes, " ") %>
+
+<% end %>

priv/repo/migrations/20190315101315_create_registrations.exs

@@ -2,7 +2,8 @@ defmodule Pleroma.Repo.Migrations.CreateRegistrations do
   use Ecto.Migration
 
   def change do
-    create table(:registrations) do
+    create table(:registrations, primary_key: false) do
+      add :id, :uuid, primary_key: true
       add :user_id, references(:users, type: :uuid, on_delete: :delete_all)
       add :provider, :string
       add :uid, :string