diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex
index 4320d3562..21ee6eaec 100644
--- a/lib/mobilizon/actors/actor.ex
+++ b/lib/mobilizon/actors/actor.ex
@@ -19,6 +19,7 @@ defmodule Mobilizon.Actors.Actor do
alias Mobilizon.Web.Endpoint
alias Mobilizon.Web.Router.Helpers, as: Routes
import Mobilizon.Web.Gettext, only: [dgettext: 2]
+ import Mobilizon.Service.Guards, only: [is_valid_string: 1]
require Logger
@@ -313,6 +314,7 @@ defmodule Mobilizon.Actors.Actor do
|> build_urls()
|> common_changeset(attrs)
|> unique_username_validator()
+ |> username_validator()
|> validate_required(@registration_required_attrs)
end
@@ -356,6 +358,7 @@ defmodule Mobilizon.Actors.Actor do
|> put_change(:keys, Crypto.generate_rsa_2048_private_key())
|> put_change(:type, :Group)
|> unique_username_validator()
+ |> username_validator()
|> validate_required(@group_creation_required_attrs)
|> validate_length(:summary, max: 5000)
|> validate_length(:preferred_username, max: 100)
@@ -381,6 +384,23 @@ defmodule Mobilizon.Actors.Actor do
# When we don't even have any preferred_username, don't even try validating preferred_username
defp unique_username_validator(changeset), do: changeset
+ defp username_validator(%Ecto.Changeset{} = changeset) do
+ username = Ecto.Changeset.fetch_field!(changeset, :preferred_username)
+
+ if is_valid_string(username) and Regex.match?(~r/^[a-z0-9_]+$/, username) do
+ changeset
+ else
+ add_error(
+ changeset,
+ :preferred_username,
+ dgettext(
+ "errors",
+ "Username must only contain alphanumeric lowercased characters and underscores."
+ )
+ )
+ end
+ end
+
@spec build_urls(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t()
defp build_urls(changeset, type \\ :Person)
diff --git a/test/graphql/resolvers/group_test.exs b/test/graphql/resolvers/group_test.exs
index 6fa9944ba..225b8f842 100644
--- a/test/graphql/resolvers/group_test.exs
+++ b/test/graphql/resolvers/group_test.exs
@@ -8,7 +8,7 @@ defmodule Mobilizon.Web.Resolvers.GroupTest do
alias Mobilizon.GraphQL.AbsintheHelpers
@non_existent_username "nonexistent"
- @new_group_params %{groupname: "new group"}
+ @new_group_params %{name: "new group", preferredUsername: "new_group"}
setup %{conn: conn} do
user = insert(:user)
@@ -17,49 +17,75 @@ defmodule Mobilizon.Web.Resolvers.GroupTest do
{:ok, conn: conn, actor: actor, user: user}
end
- describe "create a group" do
- test "create_group/3 creates a group and check a group with this name does not already exist",
+ describe "create_group/3" do
+ @create_group_mutation """
+ mutation CreateGroup(
+ $preferredUsername: String!
+ $name: String!
+ $summary: String
+ $avatar: MediaInput
+ $banner: MediaInput
+ ) {
+ createGroup(
+ preferredUsername: $preferredUsername
+ name: $name
+ summary: $summary
+ banner: $banner
+ avatar: $avatar
+ ) {
+ preferredUsername
+ type
+ banner {
+ id
+ url
+ }
+ }
+ }
+ """
+
+ test "creates a group and check a group with this name does not already exist",
%{conn: conn, user: user} do
- mutation = """
- mutation {
- createGroup(
- preferred_username: "#{@new_group_params.groupname}"
- ) {
- preferred_username,
- type
- }
- }
- """
+ res =
+ conn
+ |> auth_conn(user)
+ |> AbsintheHelpers.graphql_query(
+ query: @create_group_mutation,
+ variables: @new_group_params
+ )
+
+ assert res["errors"] == nil
+
+ assert res["data"]["createGroup"]["preferredUsername"] ==
+ @new_group_params.preferredUsername
+
+ assert res["data"]["createGroup"]["type"] == "GROUP"
res =
conn
|> auth_conn(user)
- |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+ |> AbsintheHelpers.graphql_query(
+ query: @create_group_mutation,
+ variables: @new_group_params
+ )
- assert json_response(res, 200)["data"]["createGroup"]["preferred_username"] ==
- @new_group_params.groupname
-
- assert json_response(res, 200)["data"]["createGroup"]["type"] == "GROUP"
-
- mutation = """
- mutation {
- createGroup(
- preferred_username: "#{@new_group_params.groupname}"
- ) {
- preferred_username,
- type
- }
- }
- """
-
- res =
- conn
- |> auth_conn(user)
- |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
-
- assert hd(json_response(res, 200)["errors"])["message"] ==
+ assert hd(res["errors"])["message"] ==
"A profile or group with that name already exists"
end
+
+ test "doesn't creates a group if the username doesn't match the requirements",
+ %{conn: conn, user: user} do
+ res =
+ conn
+ |> auth_conn(user)
+ |> AbsintheHelpers.graphql_query(
+ query: @create_group_mutation,
+ variables: Map.put(@new_group_params, :preferredUsername, "no@way")
+ )
+
+ assert hd(res["errors"])["message"] == [
+ "Username must only contain alphanumeric lowercased characters and underscores."
+ ]
+ end
end
describe "list groups" do
diff --git a/test/graphql/resolvers/person_test.exs b/test/graphql/resolvers/person_test.exs
index 173e45b1a..dabdf3dfb 100644
--- a/test/graphql/resolvers/person_test.exs
+++ b/test/graphql/resolvers/person_test.exs
@@ -13,23 +13,34 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
@non_existent_username "nonexistent"
- describe "Person Resolver" do
- @get_person_query """
- query Person($id: ID!) {
- person(id: $id) {
- preferredUsername,
- }
+ @get_person_query """
+ query Person($id: ID!) {
+ person(id: $id) {
+ preferredUsername,
}
- """
+ }
+ """
- @fetch_person_query """
- query FetchPerson($preferredUsername: String!) {
- fetchPerson(preferredUsername: $preferredUsername) {
- preferredUsername,
- }
+ @fetch_person_query """
+ query FetchPerson($preferredUsername: String!) {
+ fetchPerson(preferredUsername: $preferredUsername) {
+ preferredUsername,
}
- """
+ }
+ """
+ @fetch_identities_query """
+ {
+ identities {
+ avatar {
+ url
+ },
+ preferredUsername,
+ }
+ }
+ """
+
+ describe "Getting a person" do
test "get_person/3 returns a person by its username", %{conn: conn} do
user = insert(:user)
actor = insert(:actor, user: user)
@@ -128,107 +139,114 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
assert json_response(res, 200)["data"]["loggedPerson"]["avatar"]["url"] =~ Endpoint.url()
end
+ end
- test "create_person/3 creates a new identity", context do
+ describe "create_person/3" do
+ @create_person_mutation """
+ mutation CreatePerson(
+ $preferredUsername: String!
+ $name: String!
+ $summary: String
+ $avatar: MediaInput
+ $banner: MediaInput
+ ) {
+ createPerson(
+ preferredUsername: $preferredUsername
+ name: $name
+ summary: $summary
+ avatar: $avatar
+ banner: $banner
+ ) {
+ id
+ preferredUsername
+ avatar {
+ id,
+ url
+ },
+ banner {
+ id,
+ name,
+ url
+ }
+ }
+ }
+ """
+
+ test "creates a new identity", %{conn: conn} do
user = insert(:user)
actor = insert(:actor, user: user)
- mutation = """
- mutation {
- createPerson(
- preferredUsername: "new_identity",
- name: "secret person",
- summary: "no-one will know who I am"
- ) {
- id,
- preferredUsername
- }
- }
- """
-
res =
- context.conn
- |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+ conn
+ |> AbsintheHelpers.graphql_query(
+ query: @create_person_mutation,
+ variables: %{
+ preferredUsername: "new_identity",
+ name: "secret person",
+ summary: "no-one will know who I am"
+ }
+ )
- assert json_response(res, 200)["data"]["createPerson"] == nil
+ assert res["data"]["createPerson"] == nil
- assert hd(json_response(res, 200)["errors"])["message"] ==
+ assert hd(res["errors"])["message"] ==
"You need to be logged in"
res =
- context.conn
+ conn
|> auth_conn(user)
- |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+ |> AbsintheHelpers.graphql_query(
+ query: @create_person_mutation,
+ variables: %{
+ preferredUsername: "new_identity",
+ name: "secret person",
+ summary: "no-one will know who I am"
+ }
+ )
- assert json_response(res, 200)["data"]["createPerson"]["preferredUsername"] ==
+ assert res["data"]["createPerson"]["preferredUsername"] ==
"new_identity"
- query = """
- {
- identities {
- avatar {
- url
- },
- preferredUsername,
- }
- }
- """
-
res =
- context.conn
- |> get("/api", AbsintheHelpers.query_skeleton(query, "identities"))
+ conn
+ |> AbsintheHelpers.graphql_query(query: @fetch_identities_query)
- assert json_response(res, 200)["data"]["identities"] == nil
+ assert res["data"]["identities"] == nil
- assert hd(json_response(res, 200)["errors"])["message"] ==
+ assert hd(res["errors"])["message"] ==
"You need to be logged in"
res =
- context.conn
+ conn
|> auth_conn(user)
- |> get("/api", AbsintheHelpers.query_skeleton(query, "identities"))
+ |> AbsintheHelpers.graphql_query(query: @fetch_identities_query)
- assert json_response(res, 200)["data"]["identities"]
+ assert res["data"]["identities"]
|> Enum.map(fn identity -> Map.get(identity, "preferredUsername") end)
|> MapSet.new() ==
MapSet.new([actor.preferred_username, "new_identity"])
end
- test "create_person/3 with an avatar and an banner creates a new identity", context do
+ test "with an avatar and an banner creates a new identity", %{conn: conn} do
user = insert(:user)
insert(:actor, user: user)
- mutation = """
- mutation {
- createPerson(
- preferredUsername: "new_identity",
- name: "secret person",
- summary: "no-one will know who I am",
- banner: {
- media: {
- file: "landscape.jpg",
- name: "irish landscape",
- alt: "The beautiful atlantic way"
- }
- }
- ) {
- id,
- preferredUsername
- avatar {
- id,
- url
- },
- banner {
- id,
- name,
- url
- }
- }
+ variables = %{
+ preferredUsername: "new_identity",
+ name: "secret person",
+ summary: "no-one will know who I am",
+ banner: %{
+ media: %{
+ file: "landscape.jpg",
+ name: "irish landscape",
+ alt: "The beautiful atlantic way"
}
- """
+ }
+ }
map = %{
- "query" => mutation,
+ "query" => @create_person_mutation,
+ "variables" => variables,
"landscape.jpg" => %Plug.Upload{
path: "test/fixtures/picture.png",
filename: "landscape.jpg"
@@ -236,34 +254,63 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
}
res =
- context.conn
+ conn
|> put_req_header("content-type", "multipart/form-data")
- |> post("/api", map)
+ |> post(
+ "/api",
+ map
+ )
+ |> json_response(200)
- assert json_response(res, 200)["data"]["createPerson"] == nil
+ assert res["data"]["createPerson"] == nil
- assert hd(json_response(res, 200)["errors"])["message"] ==
+ assert hd(res["errors"])["message"] ==
"You need to be logged in"
res =
- context.conn
+ conn
|> auth_conn(user)
|> put_req_header("content-type", "multipart/form-data")
- |> post("/api", map)
+ |> post(
+ "/api",
+ map
+ )
+ |> json_response(200)
- assert json_response(res, 200)["data"]["createPerson"]["preferredUsername"] ==
+ assert res["data"]["createPerson"]["preferredUsername"] ==
"new_identity"
- assert json_response(res, 200)["data"]["createPerson"]["banner"]["id"]
+ assert res["data"]["createPerson"]["banner"]["id"]
- assert json_response(res, 200)["data"]["createPerson"]["banner"]["name"] ==
+ assert res["data"]["createPerson"]["banner"]["name"] ==
"The beautiful atlantic way"
- assert json_response(res, 200)["data"]["createPerson"]["banner"]["url"] =~
+ assert res["data"]["createPerson"]["banner"]["url"] =~
Endpoint.url() <> "/media/"
end
- test "update_person/3 updates an existing identity", context do
+ test "with an username that is not acceptable", %{conn: conn} do
+ user = insert(:user)
+ _actor = insert(:actor, user: user)
+
+ res =
+ conn
+ |> auth_conn(user)
+ |> AbsintheHelpers.graphql_query(
+ query: @create_person_mutation,
+ variables: %{
+ preferredUsername: "me@no",
+ name: "wrong person"
+ }
+ )
+
+ assert hd(res["errors"])["message"] ==
+ ["Username must only contain alphanumeric lowercased characters and underscores."]
+ end
+ end
+
+ describe "update_person/3" do
+ test "updates an existing identity", context do
user = insert(:user)
%Actor{id: person_id} = insert(:actor, user: user, preferred_username: "riri")
@@ -333,7 +380,7 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
assert res_person["banner"]["url"] =~ Endpoint.url() <> "/media/"
end
- test "update_person/3 should fail to update a not owned identity", context do
+ test "should fail to update a not owned identity", context do
user1 = insert(:user)
user2 = insert(:user)
%Actor{id: person_id} = insert(:actor, user: user2, preferred_username: "riri")
@@ -360,7 +407,7 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
"Profile is not owned by authenticated user"
end
- test "update_person/3 should fail to update a not existing identity", context do
+ test "should fail to update a not existing identity", context do
user = insert(:user)
insert(:actor, user: user, preferred_username: "riri")
@@ -385,8 +432,10 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
assert hd(json_response(res, 200)["errors"])["message"] ==
"Profile not found"
end
+ end
- test "delete_person/3 should fail to update a not owned identity", context do
+ describe "delete_person/3" do
+ test "should fail to update a not owned identity", context do
user1 = insert(:user)
user2 = insert(:user)
%Actor{id: person_id} = insert(:actor, user: user2, preferred_username: "riri")
@@ -410,7 +459,7 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
"Profile is not owned by authenticated user"
end
- test "delete_person/3 should fail to delete a not existing identity", context do
+ test "should fail to delete a not existing identity", context do
user = insert(:user)
insert(:actor, user: user, preferred_username: "riri")
@@ -433,7 +482,7 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
"Profile not found"
end
- test "delete_person/3 should fail to delete the last user identity", context do
+ test "should fail to delete the last user identity", context do
user = insert(:user)
%Actor{id: person_id} = insert(:actor, user: user, preferred_username: "riri")
@@ -456,7 +505,7 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
"Cannot remove the last identity of a user"
end
- test "delete_person/3 should fail to delete an identity that is the last admin of a group",
+ test "should fail to delete an identity that is the last admin of a group",
context do
group = insert(:group)
classic_user = insert(:user)
@@ -488,7 +537,7 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
"Cannot remove the last administrator of a group"
end
- test "delete_person/3 should delete an actor identity", context do
+ test "should delete an actor identity", context do
user = insert(:user)
%Actor{id: person_id} = insert(:actor, user: user, preferred_username: "riri")
insert(:actor, user: user, preferred_username: "fifi")
diff --git a/test/mobilizon/actors/actors_test.exs b/test/mobilizon/actors/actors_test.exs
index 423b5c758..4ddfc333a 100644
--- a/test/mobilizon/actors/actors_test.exs
+++ b/test/mobilizon/actors/actors_test.exs
@@ -25,7 +25,7 @@ defmodule Mobilizon.ActorsTest do
suspended: true,
uri: "some uri",
url: "some url",
- preferred_username: "some username"
+ preferred_username: "some_username"
}
@update_attrs %{
summary: "some updated description",
@@ -35,7 +35,7 @@ defmodule Mobilizon.ActorsTest do
suspended: false,
uri: "some updated uri",
url: "some updated url",
- preferred_username: "some updated username"
+ preferred_username: "some_updated_username"
}
@invalid_attrs %{
summary: nil,
@@ -234,7 +234,7 @@ defmodule Mobilizon.ActorsTest do
assert actor.domain == "some domain"
assert actor.keys == "some keypair"
assert actor.suspended
- assert actor.preferred_username == "some username"
+ assert actor.preferred_username == "some_username"
end
test "create_actor/1 with empty data returns error changeset" do
@@ -381,13 +381,13 @@ defmodule Mobilizon.ActorsTest do
@valid_attrs %{
summary: "some description",
suspended: true,
- preferred_username: "some-title",
+ preferred_username: "some_title",
name: "Some Title"
}
@update_attrs %{
summary: "some updated description",
suspended: false,
- preferred_username: "some-updated-title",
+ preferred_username: "some_updated_title",
name: "Some Updated Title"
}
@invalid_attrs %{summary: nil, suspended: nil, preferred_username: nil, name: nil}
@@ -400,7 +400,7 @@ defmodule Mobilizon.ActorsTest do
assert group.summary == "some description"
refute group.suspended
- assert group.preferred_username == "some-title"
+ assert group.preferred_username == "some_title"
end
test "create_group/1 with an existing profile username fails" do