From 681653e0351763eae7397ab172103c65dd48e204 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 29 Jan 2019 11:02:32 +0100 Subject: [PATCH] Introduce registerPerson mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To register a profile from an unactivated user Signed-off-by: Thomas Citharel 👤 Fix Person interface use Signed-off-by: Thomas Citharel Change host function for data property Signed-off-by: Thomas Citharel --- js/src/graphql/actor.ts | 28 ++- js/src/graphql/user.ts | 4 +- js/src/router/index.ts | 50 +---- js/src/router/user.ts | 59 ++++++ js/src/types/actor.model.ts | 2 +- js/src/views/Account/Register.vue | 147 ++++++--------- js/src/views/User/Register.vue | 177 ++++++++++++++++++ lib/mobilizon/actors/actor.ex | 12 ++ lib/mobilizon/actors/user.ex | 17 +- lib/mobilizon_web/resolvers/person.ex | 23 ++- lib/mobilizon_web/schema/actors/person.ex | 20 +- lib/mobilizon_web/schema/utils.ex | 4 +- .../resolvers/user_resolver_test.exs | 148 ++++++++++++++- 13 files changed, 519 insertions(+), 172 deletions(-) create mode 100644 js/src/router/user.ts create mode 100644 js/src/views/User/Register.vue diff --git a/js/src/graphql/actor.ts b/js/src/graphql/actor.ts index 6fa5a65a2..5167e7ff6 100644 --- a/js/src/graphql/actor.ts +++ b/js/src/graphql/actor.ts @@ -39,10 +39,34 @@ query { export const CREATE_PERSON = gql` mutation CreatePerson($preferredUsername: String!) { - createPerson(preferredUsername: $preferredUsername) { + createPerson( + preferredUsername: $preferredUsername, + name: $name, + summary: $summary + ) { preferredUsername, name, + summary, avatarUrl } } -` \ No newline at end of file +`; + +/** + * This one is used only to register the first account. Prefer CREATE_PERSON when creating another identity + */ +export const REGISTER_PERSON = gql` +mutation ($preferredUsername: String!, $name: String!, $summary: String!, $email: String!) { + registerPerson( + preferredUsername: $preferredUsername, + name: $name, + summary: $summary, + email: $email + ) { + preferredUsername, + name, + summary, + avatarUrl, + } +} +`; diff --git a/js/src/graphql/user.ts b/js/src/graphql/user.ts index 9031370e5..0db4b5486 100644 --- a/js/src/graphql/user.ts +++ b/js/src/graphql/user.ts @@ -1,8 +1,8 @@ import gql from 'graphql-tag'; export const CREATE_USER = gql` -mutation CreateUser($email: String!, $username: String!, $password: String!) { - createUser(email: $email, username: $username, password: $password) { +mutation CreateUser($email: String!, $password: String!) { + createUser(email: $email, password: $password) { email, confirmationSentAt } diff --git a/js/src/router/index.ts b/js/src/router/index.ts index 389cffbf5..817b50dd2 100644 --- a/js/src/router/index.ts +++ b/js/src/router/index.ts @@ -8,17 +8,12 @@ import Location from '@/views/Location.vue'; import CreateEvent from '@/views/Event/Create.vue'; import CategoryList from '@/views/Category/List.vue'; import CreateCategory from '@/views/Category/Create.vue'; -import Register from '@/views/Account/Register.vue'; -import Login from '@/views/User/Login.vue'; -import Validate from '@/views/User/Validate.vue'; -import ResendConfirmation from '@/views/User/ResendConfirmation.vue'; -import SendPasswordReset from '@/views/User/SendPasswordReset.vue'; -import PasswordReset from '@/views/User/PasswordReset.vue'; import Profile from '@/views/Account/Profile.vue'; import CreateGroup from '@/views/Group/Create.vue'; import Group from '@/views/Group/Group.vue'; import GroupList from '@/views/Group/GroupList.vue'; import Identities from '@/views/Account/Identities.vue'; +import userRoutes from './user'; Vue.use(Router); @@ -26,6 +21,7 @@ const router = new Router({ mode: 'history', base: '/', routes: [ + ...userRoutes, { path: '/', name: 'Home', @@ -69,48 +65,6 @@ const router = new Router({ component: CreateCategory, meta: { requiredAuth: true }, }, - { - path: '/register', - name: 'Register', - component: Register, - props: true, - meta: { requiredAuth: false }, - }, - { - path: '/resend-instructions', - name: 'ResendConfirmation', - component: ResendConfirmation, - props: true, - meta: { requiresAuth: false }, - }, - { - path: '/password-reset/send', - name: 'SendPasswordReset', - component: SendPasswordReset, - props: true, - meta: { requiresAuth: false }, - }, - { - path: '/password-reset/:token', - name: 'PasswordReset', - component: PasswordReset, - meta: { requiresAuth: false }, - props: true, - }, - { - path: '/validate/:token', - name: 'Validate', - component: Validate, - props: true, - meta: { requiresAuth: false }, - }, - { - path: '/login', - name: 'Login', - component: Login, - props: true, - meta: { requiredAuth: false }, - }, { path: '/identities', name: 'Identities', diff --git a/js/src/router/user.ts b/js/src/router/user.ts new file mode 100644 index 000000000..bdec67529 --- /dev/null +++ b/js/src/router/user.ts @@ -0,0 +1,59 @@ +import RegisterUser from '@/views/User/Register.vue'; +import RegisterProfile from '@/views/Account/Register.vue'; +import Login from '@/views/User/Login.vue'; +import Validate from '@/views/User/Validate.vue'; +import ResendConfirmation from '@/views/User/ResendConfirmation.vue'; +import SendPasswordReset from '@/views/User/SendPasswordReset.vue'; +import PasswordReset from '@/views/User/PasswordReset.vue'; + +export default [ + { + path: '/register/user', + name: 'Register', + component: RegisterUser, + props: true, + meta: { requiredAuth: false }, + }, + { + path: '/register/profile', + name: 'RegisterProfile', + component: RegisterProfile, + props: true, + meta: { requiredAuth: false }, + }, + { + path: '/resend-instructions', + name: 'ResendConfirmation', + component: ResendConfirmation, + props: true, + meta: { requiresAuth: false }, + }, + { + path: '/password-reset/send', + name: 'SendPasswordReset', + component: SendPasswordReset, + props: true, + meta: { requiresAuth: false }, + }, + { + path: '/password-reset/:token', + name: 'PasswordReset', + component: PasswordReset, + meta: { requiresAuth: false }, + props: true, + }, + { + path: '/validate/:token', + name: 'Validate', + component: Validate, + props: true, + meta: { requiresAuth: false }, + }, + { + path: '/login', + name: 'Login', + component: Login, + props: true, + meta: { requiredAuth: false }, + }, +]; \ No newline at end of file diff --git a/js/src/types/actor.model.ts b/js/src/types/actor.model.ts index 78891e493..a5803c0b2 100644 --- a/js/src/types/actor.model.ts +++ b/js/src/types/actor.model.ts @@ -2,7 +2,7 @@ export interface IActor { id: string; url: string; name: string; - domain: string; + domain: string|null; summary: string; preferredUsername: string; suspended: boolean; diff --git a/js/src/views/Account/Register.vue b/js/src/views/Account/Register.vue index 543c23ff6..a644049f0 100644 --- a/js/src/views/Account/Register.vue +++ b/js/src/views/Account/Register.vue @@ -10,102 +10,63 @@
-
-
-

Features

-
    -
  • Create your communities and your events
    • -
  • Other stuff…
    • -
-
-

- Learn more on - joinmobilizon.org -

-
-
-

About this instance

-

- Your local administrator resumed it's policy: -

-
    -
  • Please be nice to each other
    • -
  • meditate a bit
    • -
-

- Please read the full rules -

-
-
- +
- - + + + +

+ @{{ host }} +

+ - - + + - - + +
-
- - Didn't receive the instructions ? - -
-
- - Login - -
-

- A validation email was sent to %{email} +

+ Your account is nearly ready, %{username}

+

+ A validation email was sent to %{email} +

Before you can login, you need to click on the link inside it to validate your account

@@ -120,8 +81,9 @@ + + diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex index 8dacdfa5c..b264eee4a 100644 --- a/lib/mobilizon/actors/actor.ex +++ b/lib/mobilizon/actors/actor.ex @@ -103,6 +103,8 @@ defmodule Mobilizon.Actors.Actor do :user_id ]) |> build_urls() + # Needed because following constraint can't work for domain null values (local) + |> unique_username_validator() |> unique_constraint(:preferred_username, name: :actors_preferred_username_domain_type_index) |> unique_constraint(:url, name: :actors_url_index) |> validate_required([:preferred_username, :keys, :suspended, :url, :type]) @@ -177,6 +179,16 @@ defmodule Mobilizon.Actors.Actor do |> put_change(:local, true) end + def unique_username_validator( + %Ecto.Changeset{changes: %{preferred_username: username}} = changeset + ) do + if Actors.get_local_actor_by_name(username) do + changeset |> add_error(:preferred_username, "Username is already taken") + else + changeset + end + end + @spec build_urls(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t() defp build_urls(changeset, type \\ :Person) diff --git a/lib/mobilizon/actors/user.ex b/lib/mobilizon/actors/user.ex index 9fc1c6792..d69be537b 100644 --- a/lib/mobilizon/actors/user.ex +++ b/lib/mobilizon/actors/user.ex @@ -30,6 +30,7 @@ defmodule Mobilizon.Actors.User do |> cast(attrs, [ :email, :role, + :password, :password_hash, :confirmed_at, :confirmation_sent_at, @@ -38,13 +39,13 @@ defmodule Mobilizon.Actors.User do :reset_password_token ]) |> validate_required([:email]) - |> unique_constraint(:email, message: "registration.error.email_already_used") - |> validate_format(:email, ~r/@/) + |> unique_constraint(:email, message: "This email is already used.") + |> validate_email() |> validate_length( :password, min: 6, max: 100, - message: "registration.error.password_too_short" + message: "The choosen password is too short." ) if Map.has_key?(attrs, :default_actor) do @@ -57,21 +58,13 @@ defmodule Mobilizon.Actors.User do def registration_changeset(struct, params) do struct |> changeset(params) - |> cast(params, ~w(password)a, []) |> cast_assoc(:default_actor) |> validate_required([:email, :password]) - |> validate_email() - |> validate_length( - :password, - min: 6, - max: 100, - message: "registration.error.password_too_short" - ) |> hash_password() |> save_confirmation_token() |> unique_constraint( :confirmation_token, - message: "regisration.error.confirmation_token_already_in_use" + message: "The registration is already in use, this looks like an issue on our side." ) end diff --git a/lib/mobilizon_web/resolvers/person.ex b/lib/mobilizon_web/resolvers/person.ex index 347a70a7e..2258d49d7 100644 --- a/lib/mobilizon_web/resolvers/person.ex +++ b/lib/mobilizon_web/resolvers/person.ex @@ -3,7 +3,7 @@ defmodule MobilizonWeb.Resolvers.Person do Handles the person-related GraphQL calls """ alias Mobilizon.Actors - alias Mobilizon.Actors.Actor + alias Mobilizon.Actors.{Actor, User} alias Mobilizon.Service.ActivityPub @deprecated "Use find_person/3 or find_group/3 instead" @@ -52,6 +52,9 @@ defmodule MobilizonWeb.Resolvers.Person do {:error, "You need to be logged-in to view your list of identities"} end + @doc """ + This function is used to create more identities from an existing user + """ def create_person(_parent, %{preferred_username: _preferred_username} = args, %{ context: %{current_user: user} }) do @@ -59,9 +62,23 @@ defmodule MobilizonWeb.Resolvers.Person do with {:ok, %Actor{} = new_person} <- Actors.new_person(args) do {:ok, new_person} + end + end + + @doc """ + This function is used to register a person afterwards the user has been created (but not activated) + """ + def register_person(_parent, args, _resolution) do + with {:ok, %User{} = user} <- Actors.get_user_by_email(args.email), + {:no_actor, nil} <- {:no_actor, Actors.get_actor_for_user(user)}, + args <- Map.put(args, :user_id, user.id), + {:ok, %Actor{} = new_person} <- Actors.new_person(args) do + {:ok, new_person} else - {:error, %Ecto.Changeset{} = _e} -> - {:error, "Unable to create a profile with this username"} + {:error, :user_not_found} -> + {:error, "User with email not found"} + {:no_actor, _} -> + {:error, "You already have a profile for this user"} end end end diff --git a/lib/mobilizon_web/schema/actors/person.ex b/lib/mobilizon_web/schema/actors/person.ex index 70610a324..4397caf6e 100644 --- a/lib/mobilizon_web/schema/actors/person.ex +++ b/lib/mobilizon_web/schema/actors/person.ex @@ -6,6 +6,7 @@ defmodule MobilizonWeb.Schema.Actors.PersonType do import Absinthe.Resolution.Helpers, only: [dataloader: 1] alias Mobilizon.Events alias MobilizonWeb.Resolvers + import MobilizonWeb.Schema.Utils @desc """ Represents a person identity @@ -69,11 +70,24 @@ defmodule MobilizonWeb.Schema.Actors.PersonType do @desc "Create a new person for user" field :create_person, :person do arg(:preferred_username, non_null(:string)) - arg(:name, :string, description: "The displayed name for the new profile") - arg(:description, :string, description: "The summary for the new profile", default_value: "") + arg(:name, :string, description: "The displayed name for the new profile", default_value: "") - resolve(&Resolvers.Person.create_person/3) + arg(:summary, :string, description: "The summary for the new profile", default_value: "") + + resolve(handle_errors(&Resolvers.Person.create_person/3)) + end + + @desc "Register a first profile on registration" + field :register_person, :person do + arg(:preferred_username, non_null(:string)) + + arg(:name, :string, description: "The displayed name for the new profile", default_value: "") + + arg(:summary, :string, description: "The summary for the new profile", default_value: "") + arg(:email, non_null(:string), description: "The email from the user previously created") + + resolve(handle_errors(&Resolvers.Person.register_person/3)) end end end diff --git a/lib/mobilizon_web/schema/utils.ex b/lib/mobilizon_web/schema/utils.ex index a2bf553df..f7371ff8f 100644 --- a/lib/mobilizon_web/schema/utils.ex +++ b/lib/mobilizon_web/schema/utils.ex @@ -12,8 +12,8 @@ defmodule MobilizonWeb.Schema.Utils do # {:error, [email: {"has already been taken", []}]} errors = changeset.errors - |> Enum.map(fn {_key, {value, context}} -> - [message: "#{value}", details: context] + |> Enum.map(fn {key, {value, _context}} -> + [message: "#{value}", details: key] end) {:error, errors} diff --git a/test/mobilizon_web/resolvers/user_resolver_test.exs b/test/mobilizon_web/resolvers/user_resolver_test.exs index de969f41e..9381296bb 100644 --- a/test/mobilizon_web/resolvers/user_resolver_test.exs +++ b/test/mobilizon_web/resolvers/user_resolver_test.exs @@ -73,21 +73,25 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do end describe "Resolver: Create an user & actor" do - @account_creation %{ + @user_creation %{ email: "test@demo.tld", - password: "long password" + password: "long password", + username: "toto", + name: "Sir Toto", + summary: "Sir Toto, prince of the functional tests" } - @account_creation_bad_email %{ + @user_creation_bad_email %{ email: "y@l@", password: "long password" } - test "test create_user/3 creates an user", context do + test "test create_user/3 creates an user and register_person/3 registers a profile", + context do mutation = """ mutation { createUser( - email: "#{@account_creation.email}", - password: "#{@account_creation.password}", + email: "#{@user_creation.email}", + password: "#{@user_creation.password}", ) { id, email @@ -99,15 +103,141 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do context.conn |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) - assert json_response(res, 200)["data"]["createUser"]["email"] == @account_creation.email + assert json_response(res, 200)["data"]["createUser"]["email"] == @user_creation.email + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "#{@user_creation.email}", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["data"]["registerPerson"]["preferredUsername"] == + @user_creation.username + end + + test "register_person/3 doesn't register a profile from an unknown email", context do + mutation = """ + mutation { + createUser( + email: "#{@user_creation.email}", + password: "#{@user_creation.password}", + ) { + id, + email + } + } + """ + + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "random", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == + "User with email not found" + end + + test "register_person/3 can't be called with an existing profile", context do + mutation = """ + mutation { + createUser( + email: "#{@user_creation.email}", + password: "#{@user_creation.password}", + ) { + id, + email + } + } + """ + + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "#{@user_creation.email}", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["data"]["registerPerson"]["preferredUsername"] == + @user_creation.username + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "#{@user_creation.email}", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == + "You already have a profile for this user" end test "test create_user/3 doesn't create an user with bad email", context do mutation = """ mutation { createUser( - email: "#{@account_creation_bad_email.email}", - password: "#{@account_creation.password}", + email: "#{@user_creation_bad_email.email}", + password: "#{@user_creation_bad_email.password}", ) { id, email