From cb0808dbadf611753797aee9656c4e64b1cada97 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 6 Mar 2019 18:45:26 +0100 Subject: [PATCH] Introduce admin and moderator role, check role on list_users action Signed-off-by: Thomas Citharel Add test for guards --- lib/mobilizon/users/service/tools.ex | 10 +++++ lib/mobilizon/users/user.ex | 10 ++++- lib/mobilizon_web/context.ex | 9 ++-- lib/mobilizon_web/guardian.ex | 5 +++ lib/mobilizon_web/resolvers/user.ex | 11 ++++- mix.lock | 2 +- .../20190307125009_move_user_role_to_enum.exs | 41 +++++++++++++++++++ test/mobilizon/users/service/tools.exs | 28 +++++++++++++ .../resolvers/user_resolver_test.exs | 30 +++++++++++++- test/support/factory.ex | 2 +- 10 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 priv/repo/migrations/20190307125009_move_user_role_to_enum.exs create mode 100644 test/mobilizon/users/service/tools.exs diff --git a/lib/mobilizon/users/service/tools.ex b/lib/mobilizon/users/service/tools.ex index 691ee5eba..9348e3a59 100644 --- a/lib/mobilizon/users/service/tools.ex +++ b/lib/mobilizon/users/service/tools.ex @@ -31,3 +31,13 @@ defmodule Mobilizon.Users.Service.Tools do |> Base.url_encode64() end end + +defmodule Mobilizon.Users.Guards do + @moduledoc """ + Guards for users + """ + + defguard is_admin(role) when is_atom(role) and role == :administrator + + defguard is_moderator(role) when is_atom(role) and role in [:administrator, :moderator] +end diff --git a/lib/mobilizon/users/user.ex b/lib/mobilizon/users/user.ex index bc271cba9..4828e7ec9 100644 --- a/lib/mobilizon/users/user.ex +++ b/lib/mobilizon/users/user.ex @@ -1,3 +1,11 @@ +import EctoEnum + +defenum(Mobilizon.Users.UserRoleEnum, :user_role_type, [ + :administrator, + :moderator, + :user +]) + defmodule Mobilizon.Users.User do @moduledoc """ Represents a local user @@ -12,7 +20,7 @@ defmodule Mobilizon.Users.User do field(:email, :string) field(:password_hash, :string) field(:password, :string, virtual: true) - field(:role, :integer, default: 0) + field(:role, Mobilizon.Users.UserRoleEnum, default: :user) has_many(:actors, Actor) belongs_to(:default_actor, Actor) field(:confirmed_at, :utc_datetime) diff --git a/lib/mobilizon_web/context.ex b/lib/mobilizon_web/context.ex index 9f88807d9..c9abf01d0 100644 --- a/lib/mobilizon_web/context.ex +++ b/lib/mobilizon_web/context.ex @@ -5,19 +5,18 @@ defmodule MobilizonWeb.Context do @behaviour Plug import Plug.Conn - require Logger + alias Mobilizon.Users.User def init(opts) do opts end def call(conn, _) do - case Guardian.Plug.current_resource(conn) do + with %User{} = user <- Guardian.Plug.current_resource(conn) do + put_private(conn, :absinthe, %{context: %{current_user: user}}) + else nil -> conn - - user -> - put_private(conn, :absinthe, %{context: %{current_user: user}}) end end end diff --git a/lib/mobilizon_web/guardian.ex b/lib/mobilizon_web/guardian.ex index 1574b8d23..465178f1b 100644 --- a/lib/mobilizon_web/guardian.ex +++ b/lib/mobilizon_web/guardian.ex @@ -11,6 +11,7 @@ defmodule MobilizonWeb.Guardian do alias Mobilizon.Users alias Mobilizon.Users.User + require Logger def subject_for_token(%User{} = user, _claims) do {:ok, "User:" <> to_string(user.id)} @@ -21,6 +22,8 @@ defmodule MobilizonWeb.Guardian do end def resource_from_claims(%{"sub" => "User:" <> uid_str}) do + Logger.debug(fn -> "Receiving claim for user #{uid_str}" end) + try do case Integer.parse(uid_str) do {uid, ""} -> @@ -39,6 +42,8 @@ defmodule MobilizonWeb.Guardian do end def after_encode_and_sign(resource, claims, token, _options) do + Logger.debug(fn -> "after_encode_and_sign #{inspect(claims)}" end) + with {:ok, _} <- Guardian.DB.after_encode_and_sign(resource, claims["typ"], claims, token) do {:ok, token} end diff --git a/lib/mobilizon_web/resolvers/user.ex b/lib/mobilizon_web/resolvers/user.ex index afbfc8098..df9c8669e 100644 --- a/lib/mobilizon_web/resolvers/user.ex +++ b/lib/mobilizon_web/resolvers/user.ex @@ -6,6 +6,7 @@ defmodule MobilizonWeb.Resolvers.User do alias Mobilizon.Users.User alias Mobilizon.{Actors, Users} alias Mobilizon.Users.Service.{ResetPassword, Activation} + import Mobilizon.Users.Guards require Logger @doc """ @@ -32,14 +33,20 @@ defmodule MobilizonWeb.Resolvers.User do def list_and_count_users( _parent, %{page: page, limit: limit, sort: sort, direction: direction}, - _resolution - ) do + %{ + context: %{current_user: %User{role: role}} + } + ) + when is_moderator(role) do total = Task.async(&Users.count_users/0) elements = Task.async(fn -> Users.list_users(page, limit, sort, direction) end) {:ok, %{total: Task.await(total), elements: Task.await(elements)}} end + def list_and_count_users(_parent, _args, _resolution), + do: {:error, "You need to have admin access to list users"} + @doc """ Login an user. Returns a token and the user """ diff --git a/mix.lock b/mix.lock index 03e81a8a1..4cd4ce06a 100644 --- a/mix.lock +++ b/mix.lock @@ -55,7 +55,7 @@ "hackney": {:hex, :hackney, "1.15.1", "9f8f471c844b8ce395f7b6d8398139e26ddca9ebc171a8b91342ee15a19963f4", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.4", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"}, "http_sign": {:hex, :http_sign, "0.1.1", "b16edb83aa282892f3271f9a048c155e772bf36e15700ab93901484c55f8dd10", [:mix], [{:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"}, "httpoison": {:hex, :httpoison, "1.5.0", "71ae9f304bdf7f00e9cd1823f275c955bdfc68282bc5eb5c85c3a9ade865d68e", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"}, - "icalendar": {:git, "git@framagit.org:tcit/icalendar.git", "7090ac1f72093c6178a67e167ebaed248f60dd64", []}, + "icalendar": {:git, "https://framagit.org/tcit/icalendar", "7090ac1f72093c6178a67e167ebaed248f60dd64", []}, "idna": {:hex, :idna, "6.0.0", "689c46cbcdf3524c44d5f3dde8001f364cd7608a99556d8fbd8239a5798d4c10", [:rebar3], [{:unicode_util_compat, "0.4.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"}, "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"}, "jose": {:hex, :jose, "1.9.0", "4167c5f6d06ffaebffd15cdb8da61a108445ef5e85ab8f5a7ad926fdf3ada154", [:mix, :rebar3], [{:base64url, "~> 0.0.1", [hex: :base64url, repo: "hexpm", optional: false]}], "hexpm"}, diff --git a/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs b/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs new file mode 100644 index 000000000..1508391ce --- /dev/null +++ b/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs @@ -0,0 +1,41 @@ +defmodule Mobilizon.Repo.Migrations.MoveUserRoleToEnum do + use Ecto.Migration + + alias Mobilizon.Users.UserRoleEnum + + def up do + UserRoleEnum.create_type() + + alter table(:users) do + add(:role_tmp, UserRoleEnum.type(), default: "user") + end + + execute("UPDATE users set role_tmp = 'user' where role = 0") + execute("UPDATE users set role_tmp = 'moderator' where role = 1") + execute("UPDATE users set role_tmp = 'administrator' where role = 2") + + alter table(:users) do + remove(:role) + end + + rename(table(:users), :role_tmp, to: :role) + end + + def down do + alter table(:users) do + add(:role_tmp, :integer, default: 0) + end + + execute("UPDATE users set role_tmp = 0 where role = 'user'") + execute("UPDATE users set role_tmp = 1 where role = 'moderator'") + execute("UPDATE users set role_tmp = 2 where role = 'administrator'") + + alter table(:users) do + remove(:role) + end + + UserRoleEnum.drop_type() + + rename(table(:users), :role_tmp, to: :role) + end +end diff --git a/test/mobilizon/users/service/tools.exs b/test/mobilizon/users/service/tools.exs new file mode 100644 index 000000000..2de00d07a --- /dev/null +++ b/test/mobilizon/users/service/tools.exs @@ -0,0 +1,28 @@ +defmodule Mobilizon.Users.Service.ToolsTest do + use Mobilizon.DataCase + + import Mobilizon.Factory + + setup do + user = insert(:user) + moderator = insert(:user, role: :moderator) + administrator = insert(:user, role: :administrator) + {:ok, user: user, moderator: moderator, administrator: administrator} + end + + describe "test guards" do + import Mobilizon.Users.Guards + + test "is_moderator/1 guard", %{user: user, moderator: moderator, administrator: administrator} do + refute is_moderator(user.role) + assert is_moderator(moderator.role) + assert is_moderator(administrator.role) + end + + test "is_admin/1 guard", %{user: user, moderator: moderator, administrator: administrator} do + refute is_admin(user.role) + refute is_admin(moderator.role) + assert is_admin(administrator.role) + end + end +end diff --git a/test/mobilizon_web/resolvers/user_resolver_test.exs b/test/mobilizon_web/resolvers/user_resolver_test.exs index 186452734..18879596b 100644 --- a/test/mobilizon_web/resolvers/user_resolver_test.exs +++ b/test/mobilizon_web/resolvers/user_resolver_test.exs @@ -75,8 +75,33 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do end describe "Resolver: List users" do + test "list_users/3 doesn't return anything with a non moderator user", context do + insert(:user, email: "riri@example.com", role: :moderator) + user = insert(:user, email: "fifi@example.com") + insert(:user, email: "loulou@example.com", role: :administrator) + + query = """ + { + users { + total, + elements { + email + } + } + } + """ + + res = + context.conn + |> auth_conn(user) + |> get("/api", AbsintheHelpers.query_skeleton(query, "user")) + + assert hd(json_response(res, 200)["errors"])["message"] == + "You need to have admin access to list users" + end + test "list_users/3 returns a list of users", context do - insert(:user, email: "riri@example.com") + user = insert(:user, email: "riri@example.com", role: :moderator) insert(:user, email: "fifi@example.com") insert(:user, email: "loulou@example.com") @@ -93,6 +118,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do res = context.conn + |> auth_conn(user) |> get("/api", AbsintheHelpers.query_skeleton(query, "user")) assert json_response(res, 200)["errors"] == nil @@ -119,6 +145,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do res = context.conn + |> auth_conn(user) |> get("/api", AbsintheHelpers.query_skeleton(query, "user")) assert json_response(res, 200)["errors"] == nil @@ -142,6 +169,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do res = context.conn + |> auth_conn(user) |> get("/api", AbsintheHelpers.query_skeleton(query, "user")) assert json_response(res, 200)["errors"] == nil diff --git a/test/support/factory.ex b/test/support/factory.ex index f3a42b445..57f364677 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -9,7 +9,7 @@ defmodule Mobilizon.Factory do %Mobilizon.Users.User{ password_hash: "Jane Smith", email: sequence(:email, &"email-#{&1}@example.com"), - role: 0, + role: :user, confirmed_at: DateTime.utc_now() |> DateTime.truncate(:second), confirmation_sent_at: nil, confirmation_token: nil