From ce17cc2396f79ea94a9dfca0a72fd468b770b0a4 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Wed, 3 Feb 2016 00:03:49 -0500 Subject: [PATCH 01/24] initial pass at schemas for game/character. --- apps/yggdrasil/lib/yggdrasil/user.ex | 3 +++ .../migrations/20160203035901_create_game.exs | 13 +++++++++ .../20160203035956_create_character.exs | 16 +++++++++++ test/models/character_test.exs | 18 +++++++++++++ test/models/game_test.exs | 18 +++++++++++++ web/models/character.ex | 27 +++++++++++++++++++ web/models/game.ex | 26 ++++++++++++++++++ 7 files changed, 121 insertions(+) create mode 100644 priv/repo/migrations/20160203035901_create_game.exs create mode 100644 priv/repo/migrations/20160203035956_create_character.exs create mode 100644 test/models/character_test.exs create mode 100644 test/models/game_test.exs create mode 100644 web/models/character.ex create mode 100644 web/models/game.ex diff --git a/apps/yggdrasil/lib/yggdrasil/user.ex b/apps/yggdrasil/lib/yggdrasil/user.ex index 794d738..5bd8588 100644 --- a/apps/yggdrasil/lib/yggdrasil/user.ex +++ b/apps/yggdrasil/lib/yggdrasil/user.ex @@ -9,6 +9,9 @@ defmodule Yggdrasil.User do field :hash, :string field :password, :string, virtual: true # not part of table field :password_confirmation, :string, virtual: true # not part of table + + has_many :characters, Character + timestamps end diff --git a/priv/repo/migrations/20160203035901_create_game.exs b/priv/repo/migrations/20160203035901_create_game.exs new file mode 100644 index 0000000..f1dfe76 --- /dev/null +++ b/priv/repo/migrations/20160203035901_create_game.exs @@ -0,0 +1,13 @@ +defmodule Yggdrasil.Repo.Migrations.CreateGame do + use Ecto.Migration + + def change do + create table(:games) do + add :name, :string + add :description, :string + + timestamps + end + + end +end diff --git a/priv/repo/migrations/20160203035956_create_character.exs b/priv/repo/migrations/20160203035956_create_character.exs new file mode 100644 index 0000000..05b773d --- /dev/null +++ b/priv/repo/migrations/20160203035956_create_character.exs @@ -0,0 +1,16 @@ +defmodule Yggdrasil.Repo.Migrations.CreateCharacter do + use Ecto.Migration + + def change do + create table(:characters) do + add :name, :string + add :user_id, references(:users, on_delete: :nothing) + add :game_id, references(:games, on_delete: :nothing) + + timestamps + end + create index(:characters, [:user_id]) + create index(:characters, [:game_id]) + + end +end diff --git a/test/models/character_test.exs b/test/models/character_test.exs new file mode 100644 index 0000000..ecc982f --- /dev/null +++ b/test/models/character_test.exs @@ -0,0 +1,18 @@ +defmodule Yggdrasil.CharacterTest do + use Yggdrasil.ModelCase + + alias Yggdrasil.Character + + @valid_attrs %{name: "some content"} + @invalid_attrs %{} + + test "changeset with valid attributes" do + changeset = Character.changeset(%Character{}, @valid_attrs) + assert changeset.valid? + end + + test "changeset with invalid attributes" do + changeset = Character.changeset(%Character{}, @invalid_attrs) + refute changeset.valid? + end +end diff --git a/test/models/game_test.exs b/test/models/game_test.exs new file mode 100644 index 0000000..9bca0e8 --- /dev/null +++ b/test/models/game_test.exs @@ -0,0 +1,18 @@ +defmodule Yggdrasil.GameTest do + use Yggdrasil.ModelCase + + alias Yggdrasil.Game + + @valid_attrs %{description: "some content", name: "some content"} + @invalid_attrs %{} + + test "changeset with valid attributes" do + changeset = Game.changeset(%Game{}, @valid_attrs) + assert changeset.valid? + end + + test "changeset with invalid attributes" do + changeset = Game.changeset(%Game{}, @invalid_attrs) + refute changeset.valid? + end +end diff --git a/web/models/character.ex b/web/models/character.ex new file mode 100644 index 0000000..6aecb50 --- /dev/null +++ b/web/models/character.ex @@ -0,0 +1,27 @@ +defmodule Yggdrasil.Character do + use Yggdrasil.Web, :model + + schema "characters" do + field :name, :string + belongs_to :user, Yggdrasil.User + belongs_to :game, Yggdrasil.Game + + timestamps + end + + @required_fields ~w(name user_id game_id) + @optional_fields ~w() + + @doc """ + Creates a changeset based on the `model` and `params`. + + If no params are provided, an invalid changeset is returned + with no validation performed. + """ + def changeset(model, params \\ :empty) do + model + |> cast(params, @required_fields, @optional_fields) + |> assoc_constraint(:user) + |> assoc_constraint(:game) + end +end diff --git a/web/models/game.ex b/web/models/game.ex new file mode 100644 index 0000000..642f4fb --- /dev/null +++ b/web/models/game.ex @@ -0,0 +1,26 @@ +defmodule Yggdrasil.Game do + use Yggdrasil.Web, :model + + schema "games" do + field :name, :string + field :description, :string + + has_many :characters, Character + + timestamps + end + + @required_fields ~w(name description) + @optional_fields ~w() + + @doc """ + Creates a changeset based on the `model` and `params`. + + If no params are provided, an invalid changeset is returned + with no validation performed. + """ + def changeset(model, params \\ :empty) do + model + |> cast(params, @required_fields, @optional_fields) + end +end From 20942042f4145a9b698ca306f9454106e566a026 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Wed, 3 Feb 2016 13:19:30 -0500 Subject: [PATCH 02/24] remove has many relationship out of game and for now out of user --- apps/yggdrasil/lib/yggdrasil/user.ex | 2 -- web/models/game.ex | 2 -- 2 files changed, 4 deletions(-) diff --git a/apps/yggdrasil/lib/yggdrasil/user.ex b/apps/yggdrasil/lib/yggdrasil/user.ex index 5bd8588..bde12a0 100644 --- a/apps/yggdrasil/lib/yggdrasil/user.ex +++ b/apps/yggdrasil/lib/yggdrasil/user.ex @@ -10,8 +10,6 @@ defmodule Yggdrasil.User do field :password, :string, virtual: true # not part of table field :password_confirmation, :string, virtual: true # not part of table - has_many :characters, Character - timestamps end diff --git a/web/models/game.ex b/web/models/game.ex index 642f4fb..7a34299 100644 --- a/web/models/game.ex +++ b/web/models/game.ex @@ -5,8 +5,6 @@ defmodule Yggdrasil.Game do field :name, :string field :description, :string - has_many :characters, Character - timestamps end From 178c296ee8d38241e38acf707b799ce7779e80de Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Wed, 3 Feb 2016 13:31:59 -0500 Subject: [PATCH 03/24] remove tests for game and character for the moment. --- test/models/character_test.exs | 18 ------------------ test/models/game_test.exs | 18 ------------------ 2 files changed, 36 deletions(-) delete mode 100644 test/models/character_test.exs delete mode 100644 test/models/game_test.exs diff --git a/test/models/character_test.exs b/test/models/character_test.exs deleted file mode 100644 index ecc982f..0000000 --- a/test/models/character_test.exs +++ /dev/null @@ -1,18 +0,0 @@ -defmodule Yggdrasil.CharacterTest do - use Yggdrasil.ModelCase - - alias Yggdrasil.Character - - @valid_attrs %{name: "some content"} - @invalid_attrs %{} - - test "changeset with valid attributes" do - changeset = Character.changeset(%Character{}, @valid_attrs) - assert changeset.valid? - end - - test "changeset with invalid attributes" do - changeset = Character.changeset(%Character{}, @invalid_attrs) - refute changeset.valid? - end -end diff --git a/test/models/game_test.exs b/test/models/game_test.exs deleted file mode 100644 index 9bca0e8..0000000 --- a/test/models/game_test.exs +++ /dev/null @@ -1,18 +0,0 @@ -defmodule Yggdrasil.GameTest do - use Yggdrasil.ModelCase - - alias Yggdrasil.Game - - @valid_attrs %{description: "some content", name: "some content"} - @invalid_attrs %{} - - test "changeset with valid attributes" do - changeset = Game.changeset(%Game{}, @valid_attrs) - assert changeset.valid? - end - - test "changeset with invalid attributes" do - changeset = Game.changeset(%Game{}, @invalid_attrs) - refute changeset.valid? - end -end From 202b05a4233314df4277598ba248effc355119f8 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Wed, 3 Feb 2016 16:33:10 -0500 Subject: [PATCH 04/24] added unique constraints to character for so that only one character name can exist per game and added one for game name as well. --- priv/repo/migrations/20160203035901_create_game.exs | 1 + priv/repo/migrations/20160203035956_create_character.exs | 1 + web/models/character.ex | 1 + web/models/game.ex | 1 + 4 files changed, 4 insertions(+) diff --git a/priv/repo/migrations/20160203035901_create_game.exs b/priv/repo/migrations/20160203035901_create_game.exs index f1dfe76..89d0b91 100644 --- a/priv/repo/migrations/20160203035901_create_game.exs +++ b/priv/repo/migrations/20160203035901_create_game.exs @@ -8,6 +8,7 @@ defmodule Yggdrasil.Repo.Migrations.CreateGame do timestamps end + create unique_index(:games, [:name]) end end diff --git a/priv/repo/migrations/20160203035956_create_character.exs b/priv/repo/migrations/20160203035956_create_character.exs index 05b773d..d7abdb8 100644 --- a/priv/repo/migrations/20160203035956_create_character.exs +++ b/priv/repo/migrations/20160203035956_create_character.exs @@ -11,6 +11,7 @@ defmodule Yggdrasil.Repo.Migrations.CreateCharacter do end create index(:characters, [:user_id]) create index(:characters, [:game_id]) + create unique_index(:characters, [:name, :game_id]) end end diff --git a/web/models/character.ex b/web/models/character.ex index 6aecb50..6e6a55c 100644 --- a/web/models/character.ex +++ b/web/models/character.ex @@ -21,6 +21,7 @@ defmodule Yggdrasil.Character do def changeset(model, params \\ :empty) do model |> cast(params, @required_fields, @optional_fields) + |> unique_constraint(name: :characters_name_game_id_index) |> assoc_constraint(:user) |> assoc_constraint(:game) end diff --git a/web/models/game.ex b/web/models/game.ex index 7a34299..048839c 100644 --- a/web/models/game.ex +++ b/web/models/game.ex @@ -20,5 +20,6 @@ defmodule Yggdrasil.Game do def changeset(model, params \\ :empty) do model |> cast(params, @required_fields, @optional_fields) + |> unique_constraint(:name) end end From d1b131086875af11c2be4829877dc8169ea238d8 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sat, 6 Feb 2016 11:38:15 -0500 Subject: [PATCH 05/24] adding tests for the new models --- test/models/character_test.exs | 149 +++++++++++++++++++++++++++++++++ test/models/game_test.exs | 48 +++++++++++ web/models/character.ex | 3 +- web/models/game.ex | 2 + 4 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 test/models/character_test.exs create mode 100644 test/models/game_test.exs diff --git a/test/models/character_test.exs b/test/models/character_test.exs new file mode 100644 index 0000000..364bf61 --- /dev/null +++ b/test/models/character_test.exs @@ -0,0 +1,149 @@ +defmodule Yggdrasil.CharacterTest do + use Yggdrasil.ModelCase + + alias Yggdrasil.User + alias Yggdrasil.Game + alias Yggdrasil.Character + + @min_len 3 + + @user1 %{username: "tester", + password: "test123", + password_confirmation: "test123" } + + @user2 %{username: "tester2", + password: "test123", + password_confirmation: "test123" } + + @game1 %{name: "abcdef", description: "abcdefghi"} + @game2 %{name: "ghijkl", description: "abcdefghi"} + + @character %{name: "foobar", user_id: nil, game_id: nil} + @invalid_character %{name: "ab", user_id: nil, game_id: nil} + + @invalid_length_msg "should be at least %{count} character(s)" + @missing_assoc_msg "does not exist" + @unique_msg "has already been taken" + + defp do_setup do + {:ok, user1} = %User{} + |> User.create_changeset(@user1) + |> Repo.insert + + {:ok, user2} = %User{} + |> User.create_changeset(@user2) + |> Repo.insert + + {:ok, game1} = %Game{} + |> Game.changeset(@game1) + |> Repo.insert + + {:ok, game2} = %Game{} + |> Game.changeset(@game2) + |> Repo.insert + + %{user1: user1, user2: user2, game1: game1, game2: game2} + end + + test "valid character produces a valid changeset" do + fixture = do_setup + + user = fixture.user1 + game = fixture.game1 + + valid_character = %{@character | user_id: user.id, game_id: game.id} + + character = Character.changeset(%Character{}, valid_character) + assert character.valid? == true + end + + test "valid character changset with valid references inserts" do + fixture = do_setup + + user = fixture.user1 + game = fixture.game1 + + valid_character = %{@character | user_id: user.id, game_id: game.id} + + character = Character.changeset(%Character{}, valid_character) + assert character.valid? == true + + {:ok, _character} = Repo.insert character + end + + test "invalid character name length < #{@min_len}" do + character = Character.changeset(%Character{}, @invalid_character) + assert character.valid? == false + end + + test "valid character changset assoc_constraint fires on invalid game" do + fixture = do_setup + + user = fixture.user1 + + valid_character = %{@character | user_id: user.id, game_id: 0} + + character = Character.changeset(%Character{}, valid_character) + assert character.valid? == true + + {:error, changeset} = Repo.insert character + assert {:game, @missing_assoc_msg} in changeset.errors + end + + test "valid character changset assoc_constraint fires on invalid user" do + fixture = do_setup + + game = fixture.game1 + + valid_character = %{@character | user_id: 0, game_id: game.id} + + character = Character.changeset(%Character{}, valid_character) + assert character.valid? == true + + {:error, changeset} = Repo.insert character + assert {:user, @missing_assoc_msg} in changeset.errors + end + + test "Same character name can be used if the game_id is different with the same user_id" do + fixture = do_setup + + valid_character1 = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} + valid_character2 = %{@character | user_id: fixture.user1.id, game_id: fixture.game2.id} + + assert {:ok, _char} = %Character{} |> Character.changeset(valid_character1) |> Repo.insert + assert {:ok, _char} = %Character{} |> Character.changeset(valid_character2) |> Repo.insert + end + + test "Same character name can be used if the game_id is different and the user_id" do + fixture = do_setup + + valid_character1 = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} + valid_character2 = %{@character | user_id: fixture.user2.id, game_id: fixture.game2.id} + + assert {:ok, _char} = %Character{} |> Character.changeset(valid_character1) |> Repo.insert + assert {:ok, _char} = %Character{} |> Character.changeset(valid_character2) |> Repo.insert + end + + test "Same character name fails if game_id is the same with same user_id" do + fixture = do_setup + + valid_character = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} + + assert {:ok, _char} = %Character{} |> Character.changeset(valid_character) |> Repo.insert + assert {:error, changeset} = %Character{} |> Character.changeset(valid_character) |> Repo.insert + + assert {:name, @unique_msg} in changeset.errors + end + + test "Same character name fails if game_id is the same with different user_ids" do + fixture = do_setup + + valid_character1 = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} + valid_character2 = %{@character | user_id: fixture.user2.id, game_id: fixture.game1.id} + + assert {:ok, _char} = %Character{} |> Character.changeset(valid_character1) |> Repo.insert + assert {:error, changeset} = %Character{} |> Character.changeset(valid_character2) |> Repo.insert + + assert {:name, @unique_msg} in changeset.errors + end +end diff --git a/test/models/game_test.exs b/test/models/game_test.exs new file mode 100644 index 0000000..8f5e69c --- /dev/null +++ b/test/models/game_test.exs @@ -0,0 +1,48 @@ +defmodule Yggdrasil.GameTest do + use Yggdrasil.ModelCase + + alias Yggdrasil.Game + + @min_len 3 + @valid_game %{name: "abcdef", description: "abcdefghi"} + @invalid_name_game %{@valid_game | name: "ab"} + @invalid_description_game %{@valid_game | description: "ab"} + @invalid_game %{name: "ab", description: "bs"} + + @invalid_length_msg "should be at least %{count} character(s)" + @unique_msg "has already been taken" + + test "valid game produces valid changeset" do + game = Game.changeset %Game{}, @valid_game + + assert game.valid? == true + end + + test "invalid game produces invalid changeset" do + game = Game.changeset %Game{}, @invalid_game + + assert game.valid? == false + end + + test "name less than {@min_len} produces min length error" do + game = Game.changeset %Game{}, @invalid_game + + assert game.valid? == false + assert {:name, {@invalid_length_msg, [count: @min_len]}} in game.errors + end + + test "description less than #{@min_len} produces min length error" do + game = Game.changeset %Game{}, @invalid_game + + assert game.valid? == false + assert {:description, {@invalid_length_msg, [count: @min_len]}} in game.errors + end + + test "unique constraint on game name is obeyed" do + game = Game.changeset %Game{}, @valid_game + {:ok, _game} = Repo.insert game + + {:error, changeset} = Repo.insert game + assert {:name, @unique_msg} in changeset.errors + end +end diff --git a/web/models/character.ex b/web/models/character.ex index 6e6a55c..73b13e0 100644 --- a/web/models/character.ex +++ b/web/models/character.ex @@ -21,8 +21,9 @@ defmodule Yggdrasil.Character do def changeset(model, params \\ :empty) do model |> cast(params, @required_fields, @optional_fields) - |> unique_constraint(name: :characters_name_game_id_index) + |> unique_constraint(:name, name: :characters_name_game_id_index) |> assoc_constraint(:user) |> assoc_constraint(:game) + |> validate_length(:name, min: 3) end end diff --git a/web/models/game.ex b/web/models/game.ex index 048839c..78d4acf 100644 --- a/web/models/game.ex +++ b/web/models/game.ex @@ -21,5 +21,7 @@ defmodule Yggdrasil.Game do model |> cast(params, @required_fields, @optional_fields) |> unique_constraint(:name) + |> validate_length(:name, min: 3) + |> validate_length(:description, min: 3) end end From 6a12c6e548adcefb1053ef58039d0489429cb98e Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sat, 6 Feb 2016 12:11:43 -0500 Subject: [PATCH 06/24] removed model helper, drags coverage down and not being used. --- apps/yggdrasil_web/test/support/model_case.ex | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/apps/yggdrasil_web/test/support/model_case.ex b/apps/yggdrasil_web/test/support/model_case.ex index 829d613..cc22e36 100644 --- a/apps/yggdrasil_web/test/support/model_case.ex +++ b/apps/yggdrasil_web/test/support/model_case.ex @@ -30,30 +30,4 @@ defmodule YggdrasilWeb.ModelCase do :ok end - - @doc """ - Helper for returning list of errors in model when passed certain data. - - ## Examples - - Given a User model that lists `:name` as a required field and validates - `:password` to be safe, it would return: - - iex> errors_on(%User{}, %{password: "password"}) - [password: "is unsafe", name: "is blank"] - - You could then write your assertion like: - - assert {:password, "is unsafe"} in errors_on(%User{}, %{password: "password"}) - - You can also create the changeset manually and retrieve the errors - field directly: - - iex> changeset = User.changeset(%User{}, password: "password") - iex> {:password, "is unsafe"} in changeset.errors - true - """ - def errors_on(model, data) do - model.__struct__.changeset(model, data).errors - end end From d33a8bf26dbbe6a5ee3da73b901eb7e0e15aa8dc Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Thu, 11 Feb 2016 20:37:02 -0500 Subject: [PATCH 07/24] added two more cases for required fields and now have a propper setup function. --- test/models/character_test.exs | 77 +++++++++++++++++----------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/test/models/character_test.exs b/test/models/character_test.exs index 364bf61..a3a40e5 100644 --- a/test/models/character_test.exs +++ b/test/models/character_test.exs @@ -24,8 +24,9 @@ defmodule Yggdrasil.CharacterTest do @invalid_length_msg "should be at least %{count} character(s)" @missing_assoc_msg "does not exist" @unique_msg "has already been taken" + @required_msg "can't be blank" - defp do_setup do + setup _context do {:ok, user1} = %User{} |> User.create_changeset(@user1) |> Repo.insert @@ -42,14 +43,12 @@ defmodule Yggdrasil.CharacterTest do |> Game.changeset(@game2) |> Repo.insert - %{user1: user1, user2: user2, game1: game1, game2: game2} + {:ok, %{user1: user1, user2: user2, game1: game1, game2: game2}} end - test "valid character produces a valid changeset" do - fixture = do_setup - - user = fixture.user1 - game = fixture.game1 + test "valid character produces a valid changeset", ctx do + user = ctx.user1 + game = ctx.game1 valid_character = %{@character | user_id: user.id, game_id: game.id} @@ -57,11 +56,9 @@ defmodule Yggdrasil.CharacterTest do assert character.valid? == true end - test "valid character changset with valid references inserts" do - fixture = do_setup - - user = fixture.user1 - game = fixture.game1 + test "valid character changset with valid references inserts", ctx do + user = ctx.user1 + game = ctx.game1 valid_character = %{@character | user_id: user.id, game_id: game.id} @@ -76,10 +73,24 @@ defmodule Yggdrasil.CharacterTest do assert character.valid? == false end - test "valid character changset assoc_constraint fires on invalid game" do - fixture = do_setup + test "character changset requires game_id" do + valid_character = %{@character | user_id: 0} + + character = Character.changeset(%Character{}, valid_character) + assert character.valid? == false + assert {:game_id, @required_msg} in character.errors + end - user = fixture.user1 + test "character changset requires user_id" do + valid_character = %{@character | game_id: 0} + + character = Character.changeset(%Character{}, valid_character) + assert character.valid? == false + assert {:user_id, @required_msg} in character.errors + end + + test "valid character changset assoc_constraint fires on invalid game", ctx do + user = ctx.user1 valid_character = %{@character | user_id: user.id, game_id: 0} @@ -90,10 +101,8 @@ defmodule Yggdrasil.CharacterTest do assert {:game, @missing_assoc_msg} in changeset.errors end - test "valid character changset assoc_constraint fires on invalid user" do - fixture = do_setup - - game = fixture.game1 + test "valid character changset assoc_constraint fires on invalid user", ctx do + game = ctx.game1 valid_character = %{@character | user_id: 0, game_id: game.id} @@ -104,30 +113,24 @@ defmodule Yggdrasil.CharacterTest do assert {:user, @missing_assoc_msg} in changeset.errors end - test "Same character name can be used if the game_id is different with the same user_id" do - fixture = do_setup - - valid_character1 = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} - valid_character2 = %{@character | user_id: fixture.user1.id, game_id: fixture.game2.id} + test "same character name can be used if the game_id is different with the same user_id", ctx do + valid_character1 = %{@character | user_id: ctx.user1.id, game_id: ctx.game1.id} + valid_character2 = %{@character | user_id: ctx.user1.id, game_id: ctx.game2.id} assert {:ok, _char} = %Character{} |> Character.changeset(valid_character1) |> Repo.insert assert {:ok, _char} = %Character{} |> Character.changeset(valid_character2) |> Repo.insert end - test "Same character name can be used if the game_id is different and the user_id" do - fixture = do_setup - - valid_character1 = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} - valid_character2 = %{@character | user_id: fixture.user2.id, game_id: fixture.game2.id} + test "same character name can be used if the game_id is different and the user_id", ctx do + valid_character1 = %{@character | user_id: ctx.user1.id, game_id: ctx.game1.id} + valid_character2 = %{@character | user_id: ctx.user2.id, game_id: ctx.game2.id} assert {:ok, _char} = %Character{} |> Character.changeset(valid_character1) |> Repo.insert assert {:ok, _char} = %Character{} |> Character.changeset(valid_character2) |> Repo.insert end - test "Same character name fails if game_id is the same with same user_id" do - fixture = do_setup - - valid_character = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} + test "same character name fails if game_id is the same with same user_id", ctx do + valid_character = %{@character | user_id: ctx.user1.id, game_id: ctx.game1.id} assert {:ok, _char} = %Character{} |> Character.changeset(valid_character) |> Repo.insert assert {:error, changeset} = %Character{} |> Character.changeset(valid_character) |> Repo.insert @@ -135,11 +138,9 @@ defmodule Yggdrasil.CharacterTest do assert {:name, @unique_msg} in changeset.errors end - test "Same character name fails if game_id is the same with different user_ids" do - fixture = do_setup - - valid_character1 = %{@character | user_id: fixture.user1.id, game_id: fixture.game1.id} - valid_character2 = %{@character | user_id: fixture.user2.id, game_id: fixture.game1.id} + test "same character name fails if game_id is the same with different user_ids", ctx do + valid_character1 = %{@character | user_id: ctx.user1.id, game_id: ctx.game1.id} + valid_character2 = %{@character | user_id: ctx.user2.id, game_id: ctx.game1.id} assert {:ok, _char} = %Character{} |> Character.changeset(valid_character1) |> Repo.insert assert {:error, changeset} = %Character{} |> Character.changeset(valid_character2) |> Repo.insert From 141f7e3420c5d7da3ce00e0af0e304957f786de2 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 11:14:23 -0500 Subject: [PATCH 08/24] update dir structure. --- .../yggdrasil/lib/yggdrasil}/character.ex | 3 +- .../yggdrasil/lib/yggdrasil}/game.ex | 3 +- .../migrations/20160203035901_create_game.exs | 0 .../20160203035956_create_character.exs | 0 .../test/yggdrasil}/character_test.exs | 14 ++-- .../yggdrasil/test/yggdrasil}/game_test.exs | 12 +++- .../test/channels/player_channel_test.exs | 72 +++++++++++-------- .../web/channels/player_channel.ex | 26 ++++--- 8 files changed, 84 insertions(+), 46 deletions(-) rename {web/models => apps/yggdrasil/lib/yggdrasil}/character.ex (94%) rename {web/models => apps/yggdrasil/lib/yggdrasil}/game.ex (93%) rename {priv => apps/yggdrasil/priv}/repo/migrations/20160203035901_create_game.exs (100%) rename {priv => apps/yggdrasil/priv}/repo/migrations/20160203035956_create_character.exs (100%) rename {test/models => apps/yggdrasil/test/yggdrasil}/character_test.exs (95%) rename {test/models => apps/yggdrasil/test/yggdrasil}/game_test.exs (87%) diff --git a/web/models/character.ex b/apps/yggdrasil/lib/yggdrasil/character.ex similarity index 94% rename from web/models/character.ex rename to apps/yggdrasil/lib/yggdrasil/character.ex index 73b13e0..d9b0924 100644 --- a/web/models/character.ex +++ b/apps/yggdrasil/lib/yggdrasil/character.ex @@ -1,5 +1,6 @@ defmodule Yggdrasil.Character do - use Yggdrasil.Web, :model + use Ecto.Schema + import Ecto.Changeset schema "characters" do field :name, :string diff --git a/web/models/game.ex b/apps/yggdrasil/lib/yggdrasil/game.ex similarity index 93% rename from web/models/game.ex rename to apps/yggdrasil/lib/yggdrasil/game.ex index 78d4acf..d9e1eab 100644 --- a/web/models/game.ex +++ b/apps/yggdrasil/lib/yggdrasil/game.ex @@ -1,5 +1,6 @@ defmodule Yggdrasil.Game do - use Yggdrasil.Web, :model + use Ecto.Schema + import Ecto.Changeset schema "games" do field :name, :string diff --git a/priv/repo/migrations/20160203035901_create_game.exs b/apps/yggdrasil/priv/repo/migrations/20160203035901_create_game.exs similarity index 100% rename from priv/repo/migrations/20160203035901_create_game.exs rename to apps/yggdrasil/priv/repo/migrations/20160203035901_create_game.exs diff --git a/priv/repo/migrations/20160203035956_create_character.exs b/apps/yggdrasil/priv/repo/migrations/20160203035956_create_character.exs similarity index 100% rename from priv/repo/migrations/20160203035956_create_character.exs rename to apps/yggdrasil/priv/repo/migrations/20160203035956_create_character.exs diff --git a/test/models/character_test.exs b/apps/yggdrasil/test/yggdrasil/character_test.exs similarity index 95% rename from test/models/character_test.exs rename to apps/yggdrasil/test/yggdrasil/character_test.exs index a3a40e5..bc61ee4 100644 --- a/test/models/character_test.exs +++ b/apps/yggdrasil/test/yggdrasil/character_test.exs @@ -1,9 +1,7 @@ defmodule Yggdrasil.CharacterTest do - use Yggdrasil.ModelCase + use ExUnit.Case, async: false - alias Yggdrasil.User - alias Yggdrasil.Game - alias Yggdrasil.Character + alias Yggdrasil.{Repo, Game, User, Character} @min_len 3 @@ -26,6 +24,14 @@ defmodule Yggdrasil.CharacterTest do @unique_msg "has already been taken" @required_msg "can't be blank" + setup tags do + unless tags[:async] do + Ecto.Adapters.SQL.restart_test_transaction(Yggdrasil.Repo, []) + end + + :ok + end + setup _context do {:ok, user1} = %User{} |> User.create_changeset(@user1) diff --git a/test/models/game_test.exs b/apps/yggdrasil/test/yggdrasil/game_test.exs similarity index 87% rename from test/models/game_test.exs rename to apps/yggdrasil/test/yggdrasil/game_test.exs index 8f5e69c..7eab24f 100644 --- a/test/models/game_test.exs +++ b/apps/yggdrasil/test/yggdrasil/game_test.exs @@ -1,7 +1,7 @@ defmodule Yggdrasil.GameTest do - use Yggdrasil.ModelCase + use ExUnit.Case, async: false - alias Yggdrasil.Game + alias Yggdrasil.{Repo, Game} @min_len 3 @valid_game %{name: "abcdef", description: "abcdefghi"} @@ -12,6 +12,14 @@ defmodule Yggdrasil.GameTest do @invalid_length_msg "should be at least %{count} character(s)" @unique_msg "has already been taken" + setup tags do + unless tags[:async] do + Ecto.Adapters.SQL.restart_test_transaction(Yggdrasil.Repo, []) + end + + :ok + end + test "valid game produces valid changeset" do game = Game.changeset %Game{}, @valid_game diff --git a/apps/yggdrasil_web/test/channels/player_channel_test.exs b/apps/yggdrasil_web/test/channels/player_channel_test.exs index 4c12b11..cf62c48 100644 --- a/apps/yggdrasil_web/test/channels/player_channel_test.exs +++ b/apps/yggdrasil_web/test/channels/player_channel_test.exs @@ -1,62 +1,76 @@ defmodule PlayerChannelTest do use YggdrasilWeb.ChannelCase alias YggdrasilWeb.PlayerChannel - alias Yggdrasil.Message + alias Yggdrasil.{Message, User, Game, Character} - test "it returns an auth error if the wrong user tries to connect" do - assert {:error, %{ error: "auth failure" }} = join_channel "1", "player:2" - end + @user %{username: "tester", + password: "test123", + password_confirmation: "test123" } + + @game %{name: "abcdef", description: "abcdefghi"} + + setup _context do + {:ok, user} = %User{} + |> User.create_changeset(@user) + |> Repo.insert - test "it will join the player to the game when recieving a 'join_game'" do - user_id = "20" + {:ok, game} = %Game{} + |> Game.changeset(@game) + |> Repo.insert - join_game user_id + character = %{name: "testchar", user_id: user.id, game_id: game.id} - assert Yggdrasil.Player.Registry.is_online(user_id, sync: true) + {:ok, char} = %Character{} + |> Character.changeset(character) + |> Repo.insert + + {:ok, %{char: char, user: user}} end - test "it notifies the player if another client issues a 'join_game'" do - user_id = "20" + test "it returns an auth error if the wrong user tries to connect" do + ctx = %{user: %User{id: 0}, char: %Character{id: 0}} + assert {:error, %{ error: "auth failure" }} = join_channel ctx + end - join_game user_id + test "it will join the player to the game when recieving a 'join_game'", ctx do + join_game ctx + + assert Yggdrasil.Player.Registry.is_online(ctx.char.id, sync: true) + end + + test "it notifies the player if another client issues a 'join_game'", ctx do + join_game ctx # simulates a second client trying to join Task.async fn -> - {:ok, _reply, socket2} = join_channel user_id - push socket2, "join_game" + {:ok, _reply, socket2} = join_channel ctx + push socket2, "join_game" end assert_push "event", %Message{message: "Another client has tried to connect"} end - test "it accepts commands as a 'player_cmd' event" do - user_id = "20" - - socket = join_game user_id + test "it accepts commands as a 'player_cmd' event", ctx do + socket = join_game ctx push socket, "player_cmd", %{ "text" => "move east" } assert_push "event", %Message{} end - test "it sends an 'error' event if the command fails to parse" do - user_id = "20" - - socket = join_game user_id + test "it sends an 'error' event if the command fails to parse", ctx do + socket = join_game ctx push socket, "player_cmd", %{ "text" => "I hope this doesn't work" } assert_push "error", %Message{} end - defp join_channel(user_id, topic \\ nil) - defp join_channel(user_id, topic) do - if topic == nil, do: topic = "player:#{user_id}" - - socket(:nil, %{ user: user_id}) - |> subscribe_and_join(PlayerChannel, topic) + defp join_channel(%{user: user, char: char}) do + socket(:nil, %{ user: user.id}) + |> subscribe_and_join(PlayerChannel, "player:#{char.id}") end - defp join_game(user_id) do - {:ok, _reply, socket} = join_channel user_id + defp join_game(ctx) do + {:ok, _reply, socket} = join_channel ctx ref = push socket, "join_game" assert_reply ref, :ok diff --git a/apps/yggdrasil_web/web/channels/player_channel.ex b/apps/yggdrasil_web/web/channels/player_channel.ex index 1a4cc5d..dbb1962 100644 --- a/apps/yggdrasil_web/web/channels/player_channel.ex +++ b/apps/yggdrasil_web/web/channels/player_channel.ex @@ -1,13 +1,21 @@ defmodule YggdrasilWeb.PlayerChannel do use YggdrasilWeb.Web, :channel - alias Yggdrasil.Player + + import Ecto.Query + alias YggdrasilWeb.Endpoint - alias Yggdrasil.Message - alias Yggdrasil.Command.Parser + alias Yggdrasil.{Player, Message, Command.Parser, Character} require Logger - def join("player:" <> user_id = _topic, _message, socket) do - if socket.assigns.user == user_id do + def join("player:" <> char_id = _topic, _message, socket) do + user_id = socket.assigns.user + + char = Repo.one from c in Character, + where: c.id == ^char_id and c.user_id == ^user_id, + select: c + + if char do + socket = assign socket, :character, char {:ok, socket} else {:error, %{ error: "auth failure"} } @@ -15,17 +23,17 @@ defmodule YggdrasilWeb.PlayerChannel do end def handle_in("join_game", _message, socket) do - user_id = socket.assigns.user + char = socket.assigns.character player_topic = socket.topic push_msg = fn (payload) -> Endpoint.broadcast! player_topic, "event", payload end - case Player.join_game user_id, self, push_msg do + case Player.join_game char.id, self, push_msg do {:ok, _pid} -> {:reply, :ok, socket} {:error, :already_registered} -> - Player.notify(user_id, + Player.notify(char.id, Message.info("Another client has tried to connect")) push socket, "event", Message.error(:already_registered) @@ -36,7 +44,7 @@ defmodule YggdrasilWeb.PlayerChannel do def handle_in("player_cmd", %{ "text" => text }, socket) do case Parser.parse(text) do {:ok, cmd} -> - Player.run_cmd(socket.assigns.user, cmd) + Player.run_cmd(socket.assigns.character.id, cmd) {:noreply, socket} {:error, reason} -> broadcast! socket, "error", Message.error(reason) From 3773bfcf2dd658a238dde7ccd32a2f231e06b74f Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 11:16:49 -0500 Subject: [PATCH 09/24] removed ecto.query --- apps/yggdrasil_web/web/channels/player_channel.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/yggdrasil_web/web/channels/player_channel.ex b/apps/yggdrasil_web/web/channels/player_channel.ex index dbb1962..288f02c 100644 --- a/apps/yggdrasil_web/web/channels/player_channel.ex +++ b/apps/yggdrasil_web/web/channels/player_channel.ex @@ -1,10 +1,9 @@ defmodule YggdrasilWeb.PlayerChannel do use YggdrasilWeb.Web, :channel - import Ecto.Query - alias YggdrasilWeb.Endpoint alias Yggdrasil.{Player, Message, Command.Parser, Character} + require Logger def join("player:" <> char_id = _topic, _message, socket) do From 45fedea236e86f1ad21e4faaf198d49f4583e1f5 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Fri, 12 Feb 2016 10:29:47 -0500 Subject: [PATCH 10/24] rename user_id to char_id in player modules --- apps/yggdrasil/lib/yggdrasil/player.ex | 22 ++++---- .../lib/yggdrasil/player/registry.ex | 50 +++++++++---------- .../lib/yggdrasil/player/supervisor.ex | 4 +- .../test/yggdrasil/player/registry_test.exs | 34 ++++++------- apps/yggdrasil/test/yggdrasil/player_test.exs | 10 ++-- 5 files changed, 60 insertions(+), 60 deletions(-) diff --git a/apps/yggdrasil/lib/yggdrasil/player.ex b/apps/yggdrasil/lib/yggdrasil/player.ex index 8f9fc0a..0e7f24e 100644 --- a/apps/yggdrasil/lib/yggdrasil/player.ex +++ b/apps/yggdrasil/lib/yggdrasil/player.ex @@ -5,18 +5,18 @@ defmodule Yggdrasil.Player do alias Yggdrasil.Player.Registry alias Yggdrasil.Command - def start_link(user_id, owner_pid, push_msg) do - GenServer.start_link __MODULE__, [user_id, owner_pid, push_msg] + def start_link(char_id, owner_pid, push_msg) do + GenServer.start_link __MODULE__, [char_id, owner_pid, push_msg] end - def join_game(user_id, owner_pid, push_msg) do - Supervisor.add_player user_id, [owner_pid, push_msg] + def join_game(char_id, owner_pid, push_msg) do + Supervisor.add_player char_id, [owner_pid, push_msg] end - def run_cmd(user_id, cmd) do + def run_cmd(char_id, cmd) do # horrible terrible temporary implementation # room would eventually do all of this - ctxt = %Yggdrasil.Room.Context{ player: user_id } + ctxt = %Yggdrasil.Room.Context{ player: char_id } ctxt = Command.execute cmd, ctxt Enum.each ctxt.actions, @@ -25,19 +25,19 @@ defmodule Yggdrasil.Player do end end - def notify(user_id, msg = %Message{}) do - player_pid = Registry.get_player user_id + def notify(char_id, msg = %Message{}) do + player_pid = Registry.get_player char_id GenServer.cast(player_pid, {:notify, msg}) end - def init([user_id, channel_pid, push_msg]) do - case Registry.register_player(user_id, self) do + def init([char_id, channel_pid, push_msg]) do + case Registry.register_player(char_id, self) do :ok -> monitor_ref = Process.monitor channel_pid push_msg.(Message.info("Welcome to the game")) {:ok, %{ - user: user_id, + user: char_id, channel: channel_pid, monitor: monitor_ref, push_msg: push_msg diff --git a/apps/yggdrasil/lib/yggdrasil/player/registry.ex b/apps/yggdrasil/lib/yggdrasil/player/registry.ex index a51ce21..7669cff 100644 --- a/apps/yggdrasil/lib/yggdrasil/player/registry.ex +++ b/apps/yggdrasil/lib/yggdrasil/player/registry.ex @@ -7,29 +7,29 @@ defmodule Yggdrasil.Player.Registry do GenServer.start_link __MODULE__, [], name: __MODULE__ end - def register_player(user_id, player_pid) do - GenServer.call __MODULE__, {:register, user_id, player_pid} + def register_player(char_id, player_pid) do + GenServer.call __MODULE__, {:register, char_id, player_pid} end - def is_online(user_id, options \\ []) - def is_online(user_id, options) do + def is_online(char_id, options \\ []) + def is_online(char_id, options) do sync = Keyword.get(options, :sync, false) if sync do - GenServer.call __MODULE__, {:is_online, user_id} + GenServer.call __MODULE__, {:is_online, char_id} else - do_is_online user_id + do_is_online char_id end end - def get_player(user_id, options \\ []) - def get_player(user_id, options) do + def get_player(char_id, options \\ []) + def get_player(char_id, options) do sync = Keyword.get(options, :sync, false) if sync do - GenServer.call __MODULE__, {:get_player, user_id} + GenServer.call __MODULE__, {:get_player, char_id} else - do_get_player user_id + do_get_player char_id end end @@ -42,23 +42,23 @@ defmodule Yggdrasil.Player.Registry do }} end - def handle_call({:register, user_id, player_pid}, _from, state) do - case :ets.lookup @registry_ets, user_id do + def handle_call({:register, char_id, player_pid}, _from, state) do + case :ets.lookup @registry_ets, char_id do [] -> - :ets.insert @registry_ets, {user_id, player_pid} + :ets.insert @registry_ets, {char_id, player_pid} monitor = Process.monitor player_pid - {:reply, :ok, add_mon2user(state, monitor, user_id)} + {:reply, :ok, add_mon2user(state, monitor, char_id)} [_] -> {:reply, {:error, :already_registered}, state} end end - def handle_call({:is_online, user_id}, _from, state) do - {:reply, do_is_online(user_id), state} + def handle_call({:is_online, char_id}, _from, state) do + {:reply, do_is_online(char_id), state} end - def handle_call({:get_player, user_id}, _from, state) do - {:reply, do_get_player(user_id), state} + def handle_call({:get_player, char_id}, _from, state) do + {:reply, do_get_player(char_id), state} end def handle_info({:DOWN, monitor, :process, _pid, _reason}, state) do @@ -67,25 +67,25 @@ defmodule Yggdrasil.Player.Registry do end - defp add_mon2user(state, monitor, user_id) do - %{ state | :mon2user => Map.put(state.mon2user, monitor, user_id)} + defp add_mon2user(state, monitor, char_id) do + %{ state | :mon2user => Map.put(state.mon2user, monitor, char_id)} end defp remove_mon2user(state, monitor) do %{ state | :mon2user => Map.delete(state.mon2user, monitor)} end - defp do_is_online(user_id) do - case do_get_player(user_id) do + defp do_is_online(char_id) do + case do_get_player(char_id) do nil -> false _pid -> true end end - defp do_get_player(user_id) do - case :ets.lookup @registry_ets, user_id do + defp do_get_player(char_id) do + case :ets.lookup @registry_ets, char_id do [] -> nil - [{^user_id, player_pid}] -> player_pid + [{^char_id, player_pid}] -> player_pid end end end diff --git a/apps/yggdrasil/lib/yggdrasil/player/supervisor.ex b/apps/yggdrasil/lib/yggdrasil/player/supervisor.ex index c16ede9..3e42229 100644 --- a/apps/yggdrasil/lib/yggdrasil/player/supervisor.ex +++ b/apps/yggdrasil/lib/yggdrasil/player/supervisor.ex @@ -13,7 +13,7 @@ defmodule Yggdrasil.Player.Supervisor do supervise(children, strategy: :simple_one_for_one) end - def add_player(user_id, args) do - Supervisor.start_child(__MODULE__, [user_id | args]) + def add_player(char_id, args) do + Supervisor.start_child(__MODULE__, [char_id | args]) end end diff --git a/apps/yggdrasil/test/yggdrasil/player/registry_test.exs b/apps/yggdrasil/test/yggdrasil/player/registry_test.exs index f49b1fd..e3bd642 100644 --- a/apps/yggdrasil/test/yggdrasil/player/registry_test.exs +++ b/apps/yggdrasil/test/yggdrasil/player/registry_test.exs @@ -3,34 +3,34 @@ defmodule PlayerRegistryTests do alias Yggdrasil.Player.Registry test "it allows a player to registry as online" do - user_id = 10 - assert :ok = Registry.register_player user_id, self + char_id = 10 + assert :ok = Registry.register_player char_id, self end test "it knows if a player is online" do - user_id = 11 - refute Registry.is_online user_id - assert :ok = Registry.register_player user_id, self - assert Registry.is_online user_id + char_id = 11 + refute Registry.is_online char_id + assert :ok = Registry.register_player char_id, self + assert Registry.is_online char_id end test "it will not allow the same player to register twice" do - user_id = 12 + char_id = 12 - assert :ok = Registry.register_player user_id, self + assert :ok = Registry.register_player char_id, self register_task = Task.async(fn -> - Registry.register_player user_id, self + Registry.register_player char_id, self end) assert {:error, :already_registered} = Task.await(register_task) end test "it knows when a player exits" do - user_id = 13 + char_id = 13 register_task = Task.async(fn -> - Registry.register_player user_id, self + Registry.register_player char_id, self end) # verify player was registered successfully @@ -39,20 +39,20 @@ defmodule PlayerRegistryTests do # we'll do this sync' so that the registry has a chance # to handle the exit message from the task - refute Registry.is_online user_id, sync: true + refute Registry.is_online char_id, sync: true end test "it can return the pid for the user" do - user_id = 14 - assert :ok = Registry.register_player user_id, self + char_id = 14 + assert :ok = Registry.register_player char_id, self - pid = Registry.get_player user_id + pid = Registry.get_player char_id assert is_pid(pid) end test "it returns the nil if the user is offline" do - user_id = 15 - assert Registry.get_player(user_id) == nil + char_id = 15 + assert Registry.get_player(char_id) == nil end end diff --git a/apps/yggdrasil/test/yggdrasil/player_test.exs b/apps/yggdrasil/test/yggdrasil/player_test.exs index 596b494..6bb5d33 100644 --- a/apps/yggdrasil/test/yggdrasil/player_test.exs +++ b/apps/yggdrasil/test/yggdrasil/player_test.exs @@ -4,21 +4,21 @@ defmodule PlayerTest do alias Yggdrasil.Player.Registry test "it registers the user as online when it starts" do - user_id = 1 - {:ok, _pid} = Player.start_link user_id, self, fn (msg) -> + char_id = 1 + {:ok, _pid} = Player.start_link char_id, self, fn (msg) -> msg end - assert Registry.is_online user_id + assert Registry.is_online char_id end test "it uses push_msg/1 to push a welcome message" do - user_id = 2 + char_id = 2 # using an agent to store messages {:ok, agent} = Agent.start_link(fn -> [] end) - {:ok, _pid} = Player.start_link user_id, self, fn (msg) -> + {:ok, _pid} = Player.start_link char_id, self, fn (msg) -> Agent.update agent, fn (msgs) -> [msg | msgs] end end From 28f58503ad2e983c29b401f01768df8cf1cecd54 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Fri, 12 Feb 2016 10:34:23 -0500 Subject: [PATCH 11/24] changer user key to character. --- apps/yggdrasil/lib/yggdrasil/player.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/yggdrasil/lib/yggdrasil/player.ex b/apps/yggdrasil/lib/yggdrasil/player.ex index 0e7f24e..74266da 100644 --- a/apps/yggdrasil/lib/yggdrasil/player.ex +++ b/apps/yggdrasil/lib/yggdrasil/player.ex @@ -37,7 +37,7 @@ defmodule Yggdrasil.Player do monitor_ref = Process.monitor channel_pid push_msg.(Message.info("Welcome to the game")) {:ok, %{ - user: char_id, + character: char_id, channel: channel_pid, monitor: monitor_ref, push_msg: push_msg From 6e42e4d856a4b5a537119e51fddcc6ca00f9e6f2 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Fri, 12 Feb 2016 10:39:06 -0500 Subject: [PATCH 12/24] renamed mon2user functions to mon2character. --- apps/yggdrasil/lib/yggdrasil/player/registry.ex | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/yggdrasil/lib/yggdrasil/player/registry.ex b/apps/yggdrasil/lib/yggdrasil/player/registry.ex index 7669cff..7c80c59 100644 --- a/apps/yggdrasil/lib/yggdrasil/player/registry.ex +++ b/apps/yggdrasil/lib/yggdrasil/player/registry.ex @@ -38,7 +38,7 @@ defmodule Yggdrasil.Player.Registry do table_id = :ets.new @registry_ets, [:named_table, :set] {:ok, %{ :table_id => table_id, - :mon2user => %{ } + :mon2character => %{ } }} end @@ -47,7 +47,7 @@ defmodule Yggdrasil.Player.Registry do [] -> :ets.insert @registry_ets, {char_id, player_pid} monitor = Process.monitor player_pid - {:reply, :ok, add_mon2user(state, monitor, char_id)} + {:reply, :ok, add_mon2character(state, monitor, char_id)} [_] -> {:reply, {:error, :already_registered}, state} end @@ -62,17 +62,17 @@ defmodule Yggdrasil.Player.Registry do end def handle_info({:DOWN, monitor, :process, _pid, _reason}, state) do - :ets.delete @registry_ets, state.mon2user[monitor] - {:noreply, remove_mon2user(state, monitor)} + :ets.delete @registry_ets, state.mon2character[monitor] + {:noreply, remove_mon2character(state, monitor)} end - defp add_mon2user(state, monitor, char_id) do - %{ state | :mon2user => Map.put(state.mon2user, monitor, char_id)} + defp add_mon2character(state, monitor, char_id) do + %{ state | :mon2character => Map.put(state.mon2character, monitor, char_id)} end - defp remove_mon2user(state, monitor) do - %{ state | :mon2user => Map.delete(state.mon2user, monitor)} + defp remove_mon2character(state, monitor) do + %{ state | :mon2character => Map.delete(state.mon2character, monitor)} end defp do_is_online(char_id) do From deadd4d1d594d95bf03ea90349110fb7a89b080e Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Mon, 15 Feb 2016 21:06:40 -0500 Subject: [PATCH 13/24] added game controller and view --- web/controllers/game_controller.ex | 11 +++++++++++ web/views/game_view.ex | 8 ++++++++ 2 files changed, 19 insertions(+) create mode 100644 web/controllers/game_controller.ex create mode 100644 web/views/game_view.ex diff --git a/web/controllers/game_controller.ex b/web/controllers/game_controller.ex new file mode 100644 index 0000000..dac58b2 --- /dev/null +++ b/web/controllers/game_controller.ex @@ -0,0 +1,11 @@ +defmodule Yggdrasil.GameController do + use Yggdrasil.Web, :controller + + alias Yggdrasil.Game + + def index(conn, _params) do + games = Repo.all Game + + render conn, :show, data: games + end +end diff --git a/web/views/game_view.ex b/web/views/game_view.ex new file mode 100644 index 0000000..8181e6b --- /dev/null +++ b/web/views/game_view.ex @@ -0,0 +1,8 @@ +defmodule Yggdrasil.GameView do + use Yggdrasil.Web, :view + use JaSerializer.PhoenixView + + attributes [:name, :description] + + def type, do: "games" +end From 5447bcc1039af50ea31703a425640f264ef09b21 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Tue, 16 Feb 2016 22:25:55 -0500 Subject: [PATCH 14/24] super basic game controller tests to get started with --- apps/yggdrasil_web/web/router.ex | 2 +- test/controllers/game_controller_test.exs | 82 +++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 test/controllers/game_controller_test.exs diff --git a/apps/yggdrasil_web/web/router.ex b/apps/yggdrasil_web/web/router.ex index 30a650c..62773ca 100644 --- a/apps/yggdrasil_web/web/router.ex +++ b/apps/yggdrasil_web/web/router.ex @@ -37,6 +37,6 @@ defmodule YggdrasilWeb.Router do pipe_through :api pipe_through :guardian - # not used at the moment, but setup + get "/games", GameController, :index end end diff --git a/test/controllers/game_controller_test.exs b/test/controllers/game_controller_test.exs new file mode 100644 index 0000000..04d490a --- /dev/null +++ b/test/controllers/game_controller_test.exs @@ -0,0 +1,82 @@ +defmodule Yggdrasil.GameControllerTest do + use Yggdrasil.ConnCase + require IEx + + alias Yggdrasil.{Repo, User, Game} + + @user %{username: "tester", + password: "password", + password_confirmation: "password"} + + @json_api_content_type "application/vnd.api+json" + @json_api_content_type_utf8 @json_api_content_type <> "; charset=utf-8" + + setup %{conn: conn} do + # need a user to gen a token for api + {:ok, user} = %User{} + |> User.create_changeset(@user) + |> Repo.insert + + #api token + {:ok, token, _claims} = Guardian.encode_and_sign user, :token + + + # set of games to have something to list + games = Enum.map 1..10, fn n -> + game = Game.changeset(%Game{}, %{"name" => "game#{n}", + "description" => "game desc #{n}"}) + {:ok, game} = Repo.insert game + + game + end + + conn = conn + |> put_req_header("content-type", @json_api_content_type) + |> put_req_header("accept", @json_api_content_type) + + {:ok, %{conn: conn, games: games, user: user, token: token}} + end + + defp compare_game_lists(resp, games) do + games_resp = Poison.decode! resp + + api_games = Enum.map games_resp["data"], fn g -> + attrs = g["attributes"] + {id, _remainder} = Integer.parse g["id"] + + game = Map.put attrs, "id", id + game = for {key, val} <- game, into: %{}, do: {String.to_atom(key), val} + end + + + db_games = Enum.map games, fn g -> + g + |> Map.from_struct + |> Map.drop([:inserted_at, :updated_at, :__meta__]) + end + + IEx.pry + db_games == api_games + end + + test "access denied (401) if auth header isn't present", ctx do + path = api_game_path(conn, :index) + conn = ctx.conn + |> get(path) + + assert response_content_type(conn, :json) == @json_api_content_type + assert response(conn, 401) + end + + test "all games are listed", ctx do + path = api_game_path(conn, :index) + conn = ctx.conn + |> put_req_header("authorization", ctx.token) + |> get(path) + + assert response_content_type(conn, :json) == @json_api_content_type_utf8 + games_match = compare_game_lists(response(conn, :ok), ctx.games) + + assert games_match + end +end From 945463e3860afb3800250480d6698c324f9acd9f Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Tue, 16 Feb 2016 22:31:48 -0500 Subject: [PATCH 15/24] had one unused variable, and removed IEx.pry calls --- test/controllers/game_controller_test.exs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/controllers/game_controller_test.exs b/test/controllers/game_controller_test.exs index 04d490a..2e490a3 100644 --- a/test/controllers/game_controller_test.exs +++ b/test/controllers/game_controller_test.exs @@ -1,6 +1,5 @@ defmodule Yggdrasil.GameControllerTest do use Yggdrasil.ConnCase - require IEx alias Yggdrasil.{Repo, User, Game} @@ -45,17 +44,15 @@ defmodule Yggdrasil.GameControllerTest do {id, _remainder} = Integer.parse g["id"] game = Map.put attrs, "id", id - game = for {key, val} <- game, into: %{}, do: {String.to_atom(key), val} + for {key, val} <- game, into: %{}, do: {String.to_atom(key), val} end - db_games = Enum.map games, fn g -> g |> Map.from_struct |> Map.drop([:inserted_at, :updated_at, :__meta__]) end - IEx.pry db_games == api_games end From 64db24b1b292eeff350c9b3d9117912bf851c3c9 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Tue, 16 Feb 2016 23:40:09 -0500 Subject: [PATCH 16/24] character controller roughed out. --- apps/yggdrasil_web/web/router.ex | 5 +++ web/controllers/character_controller.ex | 60 +++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 web/controllers/character_controller.ex diff --git a/apps/yggdrasil_web/web/router.ex b/apps/yggdrasil_web/web/router.ex index 62773ca..bba92d8 100644 --- a/apps/yggdrasil_web/web/router.ex +++ b/apps/yggdrasil_web/web/router.ex @@ -38,5 +38,10 @@ defmodule YggdrasilWeb.Router do pipe_through :guardian get "/games", GameController, :index + + get "/characters", CharacterController, :index + get "/characters/:char_id", CharacterController, :show + post "/characters", CharacterController, :create + delete "/characters/:char_id", CharacterController, :delete end end diff --git a/web/controllers/character_controller.ex b/web/controllers/character_controller.ex new file mode 100644 index 0000000..d1dac93 --- /dev/null +++ b/web/controllers/character_controller.ex @@ -0,0 +1,60 @@ +defmodule Yggdrasil.CharacterController do + use Yggdrasil.Web, :controller + + alias Yggdrasil.Character + + def index(conn, %{"filter" => %{"user_id" => user_id}}) do + # this should not happen, but could easily + # probably don't need to crash but rather return an error + ^user_id = conn.assigns.user + + chars = Repo.all from c in Character, + where: c.user_id == ^user_id, + select: c + + render conn, :show, data: chars + end + + def show(conn, %{"char_id" => char_id}) do + user_id = conn.assigns.user + + # returns nil if not found + # need to sort out what we want here. + char = Repo.one from c in Character, + where: c.id == ^char_id and c.user_id == ^user_id, + select: c + + render conn, :show, data: char + end + + def create(conn, %{"data" => %{"attributes" => attributes}}) do + char = Character.changeset(%Character{}, attributes) + + case Repo.insert(char) do + {:ok, new_char} -> + render conn, :show, data: new_char + {:error, err_changeset} -> + render conn, :errors, data: err_changeset + end + end + + def delete(conn, %{"char_id" => char_id}) do + user_id = conn.assigns.user + + # returns nil if not found + # need to sort out what we want here. + char = Repo.one from c in Character, + where: c.id == ^char_id and c.user_id == ^user_id, + select: c + + case Repo.delete(char)do + {:ok, _char} -> + # http://jsonapi.org/format/#crud-deleting + conn + |> put_status(204) + |> send_resp + {:error, err_changeset} -> + render conn, :errors, data: err_changeset + end + end +end From 13e508d75eed5a7f678e29de337ce412dafeaa2a Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 11:25:34 -0500 Subject: [PATCH 17/24] controllers updated to match app structure. --- .../yggdrasil_web/test}/controllers/game_controller_test.exs | 4 ++-- .../yggdrasil_web/web}/controllers/character_controller.ex | 4 ++-- .../yggdrasil_web/web}/controllers/game_controller.ex | 4 ++-- {web => apps/yggdrasil_web/web}/views/game_view.ex | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename {test => apps/yggdrasil_web/test}/controllers/game_controller_test.exs (96%) rename {web => apps/yggdrasil_web/web}/controllers/character_controller.ex (95%) rename {web => apps/yggdrasil_web/web}/controllers/game_controller.ex (62%) rename {web => apps/yggdrasil_web/web}/views/game_view.ex (59%) diff --git a/test/controllers/game_controller_test.exs b/apps/yggdrasil_web/test/controllers/game_controller_test.exs similarity index 96% rename from test/controllers/game_controller_test.exs rename to apps/yggdrasil_web/test/controllers/game_controller_test.exs index 2e490a3..8bbf72a 100644 --- a/test/controllers/game_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/game_controller_test.exs @@ -1,5 +1,5 @@ -defmodule Yggdrasil.GameControllerTest do - use Yggdrasil.ConnCase +defmodule YggdrasilWeb.GameControllerTest do + use YggdrasilWeb.ConnCase alias Yggdrasil.{Repo, User, Game} diff --git a/web/controllers/character_controller.ex b/apps/yggdrasil_web/web/controllers/character_controller.ex similarity index 95% rename from web/controllers/character_controller.ex rename to apps/yggdrasil_web/web/controllers/character_controller.ex index d1dac93..fbd0423 100644 --- a/web/controllers/character_controller.ex +++ b/apps/yggdrasil_web/web/controllers/character_controller.ex @@ -1,5 +1,5 @@ -defmodule Yggdrasil.CharacterController do - use Yggdrasil.Web, :controller +defmodule YggdrasilWeb.CharacterController do + use YggdrasilWeb.Web, :controller alias Yggdrasil.Character diff --git a/web/controllers/game_controller.ex b/apps/yggdrasil_web/web/controllers/game_controller.ex similarity index 62% rename from web/controllers/game_controller.ex rename to apps/yggdrasil_web/web/controllers/game_controller.ex index dac58b2..4a7ec75 100644 --- a/web/controllers/game_controller.ex +++ b/apps/yggdrasil_web/web/controllers/game_controller.ex @@ -1,5 +1,5 @@ -defmodule Yggdrasil.GameController do - use Yggdrasil.Web, :controller +defmodule YggdrasilWeb.GameController do + use YggdrasilWeb.Web, :controller alias Yggdrasil.Game diff --git a/web/views/game_view.ex b/apps/yggdrasil_web/web/views/game_view.ex similarity index 59% rename from web/views/game_view.ex rename to apps/yggdrasil_web/web/views/game_view.ex index 8181e6b..d74a182 100644 --- a/web/views/game_view.ex +++ b/apps/yggdrasil_web/web/views/game_view.ex @@ -1,5 +1,5 @@ -defmodule Yggdrasil.GameView do - use Yggdrasil.Web, :view +defmodule YggdrasilWeb.GameView do + use YggdrasilWeb.Web, :view use JaSerializer.PhoenixView attributes [:name, :description] From 5c12d97213036dcc954d37a8c3a2f24700c36175 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 12:11:24 -0500 Subject: [PATCH 18/24] added the start of character controller tests. --- .../controllers/character_controller_test.exs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 apps/yggdrasil_web/test/controllers/character_controller_test.exs diff --git a/apps/yggdrasil_web/test/controllers/character_controller_test.exs b/apps/yggdrasil_web/test/controllers/character_controller_test.exs new file mode 100644 index 0000000..789f885 --- /dev/null +++ b/apps/yggdrasil_web/test/controllers/character_controller_test.exs @@ -0,0 +1,63 @@ +defmodule YggdrasilWeb.CharacterControllerTest do + use YggdrasilWeb.ConnCase + + alias Yggdrasil.{Repo, User, Game, Character} + + @user %{username: "tester", + password: "password", + password_confirmation: "password"} + + @json_api_content_type "application/vnd.api+json" + @json_api_content_type_utf8 @json_api_content_type <> "; charset=utf-8" + + setup %{conn: conn} do + # need a user to gen a token for api + {:ok, user} = %User{} + |> User.create_changeset(@user) + |> Repo.insert + + #api token + {:ok, token, _claims} = Guardian.encode_and_sign user, :token + + + # set of games to have something to list + games = Enum.map 1..2, fn n -> + game = Game.changeset(%Game{}, %{"name" => "game#{n}", + "description" => "game desc #{n}"}) + {:ok, game} = Repo.insert game + + game + end + + chars = Enum.map games, fn g -> + Enum.map 1..2, fn n -> + char = Character.changeset(%Character{}, %{:name => "char #{g.name} #{n}", + :game_id => g.id, + :user_id => user.id}) + + {:ok, char} = Repo.insert char + + char + end + end + + conn = conn + |> put_req_header("content-type", @json_api_content_type) + |> put_req_header("accept", @json_api_content_type) + + {:ok, %{conn: conn, chars: chars, games: games, user: user, token: token}} + end + +# test "get character by id", ctx do +# path = api_character_path(conn, :show, ctx.user.id) + +# conn = ctx.conn +# |> put_req_header("authorization", ctx.token) +# |> get(path) + +# assert response_content_type(conn, :json) == @json_api_content_type_utf8 +# resp = response(conn, :ok) + +# assert resp == :ok +# end +end From ad49fe6d9248ce71e273375dc615cf30535a2ee1 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 13:58:51 -0500 Subject: [PATCH 19/24] bump guardian version --- apps/yggdrasil_web/mix.exs | 2 +- mix.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/yggdrasil_web/mix.exs b/apps/yggdrasil_web/mix.exs index 4e74d59..de5e511 100644 --- a/apps/yggdrasil_web/mix.exs +++ b/apps/yggdrasil_web/mix.exs @@ -42,7 +42,7 @@ defmodule YggdrasilWeb.Mixfile do {:cowboy, "~> 1.0"}, {:comeonin, "~> 2.0"}, {:ja_serializer, "~> 0.6.1"}, - {:guardian, "~> 0.9.0"}, + {:guardian, "~> 0.10.1"}, {:postgrex, ">= 0.0.0"}, {:cors_plug, "~> 0.1.4"}, {:excoveralls, "~>0.5.1"} diff --git a/mix.lock b/mix.lock index 05c4899..fb0ce92 100644 --- a/mix.lock +++ b/mix.lock @@ -12,7 +12,7 @@ "exjsx": {:hex, :exjsx, "3.2.0"}, "fs": {:hex, :fs, "0.9.2"}, "gettext": {:hex, :gettext, "0.9.0"}, - "guardian": {:hex, :guardian, "0.9.1"}, + "guardian": {:hex, :guardian, "0.10.1"}, "hackney": {:hex, :hackney, "1.4.8"}, "idna": {:hex, :idna, "1.0.3"}, "inflex": {:hex, :inflex, "1.5.0"}, From 57734d6bc45579f26c66e97354fb5d74b7f8df31 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 14:02:29 -0500 Subject: [PATCH 20/24] Load user via Guardian.Plug.LoadResource on each request This found some issues with the way I defined the YggdrasilWeb.GuardianSerializer. I have made appropriate changes and modified tests to reflect them. --- apps/yggdrasil_web/lib/guardian_serializer.ex | 2 +- .../registration_controller_test.exs | 2 +- .../controllers/session_controller_test.exs | 2 +- .../test/guardian_serializer_test.exs | 4 ++-- .../web/controllers/character_controller.ex | 19 +++++++++---------- apps/yggdrasil_web/web/router.ex | 1 + 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/apps/yggdrasil_web/lib/guardian_serializer.ex b/apps/yggdrasil_web/lib/guardian_serializer.ex index 8410e65..f55bfd2 100644 --- a/apps/yggdrasil_web/lib/guardian_serializer.ex +++ b/apps/yggdrasil_web/lib/guardian_serializer.ex @@ -7,6 +7,6 @@ defmodule YggdrasilWeb.GuardianSerializer do def for_token(user = %User{}), do: {:ok, "user:#{user.id}"} def for_token(_), do: {:error, "Unknown resource type."} - def from_token("user:" <> user_id), do: Repo.get(User, user_id) + def from_token("user:" <> user_id), do: {:ok, Repo.get(User, user_id)} def from_token(_), do: {:error, "Unknown resource type."} end diff --git a/apps/yggdrasil_web/test/controllers/registration_controller_test.exs b/apps/yggdrasil_web/test/controllers/registration_controller_test.exs index 61534c3..001b8e0 100644 --- a/apps/yggdrasil_web/test/controllers/registration_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/registration_controller_test.exs @@ -85,7 +85,7 @@ defmodule YggdrasilWeb.RegistrationControllerTest do user_id = res_user["data"]["id"] - assert Guardian.serializer.from_token(claims["sub"]) == Repo.get(User, user_id) + assert Guardian.serializer.from_token(claims["sub"]) == {:ok, Repo.get(User, user_id)} end test "user registration with password mismatch fails", %{conn: conn} do diff --git a/apps/yggdrasil_web/test/controllers/session_controller_test.exs b/apps/yggdrasil_web/test/controllers/session_controller_test.exs index 5b480f8..9606b8f 100644 --- a/apps/yggdrasil_web/test/controllers/session_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/session_controller_test.exs @@ -80,7 +80,7 @@ defmodule YggdrasilWeb.SessionControllerTest do # user above contains some extra keys from the creation # so need to test against a fresh db record. - assert Guardian.serializer.from_token(claims["sub"]) == Repo.get(User, user.id) + assert Guardian.serializer.from_token(claims["sub"]) == {:ok, Repo.get(User, user.id)} end test "user login correct username but bad password returns error", %{conn: conn} do diff --git a/apps/yggdrasil_web/test/guardian_serializer_test.exs b/apps/yggdrasil_web/test/guardian_serializer_test.exs index 52a67f4..dc00376 100644 --- a/apps/yggdrasil_web/test/guardian_serializer_test.exs +++ b/apps/yggdrasil_web/test/guardian_serializer_test.exs @@ -26,8 +26,8 @@ defmodule YggdrasilWeb.GuardianSerializerTest do test "from_token with correct user_id returns user of that id" do user = insert_user - - assert user.id == GuardianSerializer.from_token("user:#{user.id}").id + {:ok, token_user} = GuardianSerializer.from_token("user:#{user.id}") + assert user.id == token_user.id end test "from_token with nil returns expected error" do diff --git a/apps/yggdrasil_web/web/controllers/character_controller.ex b/apps/yggdrasil_web/web/controllers/character_controller.ex index fbd0423..56fcf1a 100644 --- a/apps/yggdrasil_web/web/controllers/character_controller.ex +++ b/apps/yggdrasil_web/web/controllers/character_controller.ex @@ -1,13 +1,11 @@ defmodule YggdrasilWeb.CharacterController do use YggdrasilWeb.Web, :controller + use Guardian.Phoenix.Controller alias Yggdrasil.Character - def index(conn, %{"filter" => %{"user_id" => user_id}}) do - # this should not happen, but could easily - # probably don't need to crash but rather return an error - ^user_id = conn.assigns.user - + def index(conn, _params, user, _claims) do + user_id = user.id chars = Repo.all from c in Character, where: c.user_id == ^user_id, select: c @@ -15,8 +13,8 @@ defmodule YggdrasilWeb.CharacterController do render conn, :show, data: chars end - def show(conn, %{"char_id" => char_id}) do - user_id = conn.assigns.user + def show(conn, %{"char_id" => char_id}, user, _claims) do + user_id = user.id # returns nil if not found # need to sort out what we want here. @@ -27,7 +25,8 @@ defmodule YggdrasilWeb.CharacterController do render conn, :show, data: char end - def create(conn, %{"data" => %{"attributes" => attributes}}) do + def create(conn, %{"data" => %{"attributes" => attributes}}, user, _claims) do + attributes = Map.put attributes, :user_id, user.id char = Character.changeset(%Character{}, attributes) case Repo.insert(char) do @@ -38,8 +37,8 @@ defmodule YggdrasilWeb.CharacterController do end end - def delete(conn, %{"char_id" => char_id}) do - user_id = conn.assigns.user + def delete(conn, %{"char_id" => char_id}, user, _claims) do + user_id = user.id # returns nil if not found # need to sort out what we want here. diff --git a/apps/yggdrasil_web/web/router.ex b/apps/yggdrasil_web/web/router.ex index bba92d8..4374406 100644 --- a/apps/yggdrasil_web/web/router.ex +++ b/apps/yggdrasil_web/web/router.ex @@ -10,6 +10,7 @@ defmodule YggdrasilWeb.Router do pipeline :guardian do plug Guardian.Plug.VerifyHeader plug Guardian.Plug.EnsureAuthenticated, handler: YggdrasilWeb.GuardianErrorHandler + plug Guardian.Plug.LoadResource end @docs """ From c475275cf204e9fb05faeda557de4490bd155397 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sun, 21 Feb 2016 14:04:51 -0500 Subject: [PATCH 21/24] add controller tests, which also needed a character_view module --- .../controllers/character_controller_test.exs | 53 +++++++++++++++---- .../yggdrasil_web/web/views/character_view.ex | 8 +++ 2 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 apps/yggdrasil_web/web/views/character_view.ex diff --git a/apps/yggdrasil_web/test/controllers/character_controller_test.exs b/apps/yggdrasil_web/test/controllers/character_controller_test.exs index 789f885..eb15541 100644 --- a/apps/yggdrasil_web/test/controllers/character_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/character_controller_test.exs @@ -1,6 +1,7 @@ defmodule YggdrasilWeb.CharacterControllerTest do use YggdrasilWeb.ConnCase + alias YggdrasilWeb.CharacterView alias Yggdrasil.{Repo, User, Game, Character} @user %{username: "tester", @@ -29,7 +30,7 @@ defmodule YggdrasilWeb.CharacterControllerTest do game end - chars = Enum.map games, fn g -> + chars = Enum.flat_map games, fn g -> Enum.map 1..2, fn n -> char = Character.changeset(%Character{}, %{:name => "char #{g.name} #{n}", :game_id => g.id, @@ -48,16 +49,48 @@ defmodule YggdrasilWeb.CharacterControllerTest do {:ok, %{conn: conn, chars: chars, games: games, user: user, token: token}} end -# test "get character by id", ctx do -# path = api_character_path(conn, :show, ctx.user.id) + test "get /api/characters/char_id returns the correct character", ctx do + char = Enum.at ctx.chars, 0 -# conn = ctx.conn -# |> put_req_header("authorization", ctx.token) -# |> get(path) + char_json = CharacterView.render("show.json", conn: %{}, data: char) + |> Poison.encode! + |> Poison.decode! -# assert response_content_type(conn, :json) == @json_api_content_type_utf8 -# resp = response(conn, :ok) + path = api_character_path(conn, :show, char.id) -# assert resp == :ok -# end + conn = ctx.conn + |> put_req_header("authorization", ctx.token) + |> get(path) + + assert response_content_type(conn, :json) == @json_api_content_type_utf8 + resp = Poison.decode! response(conn, :ok) + + assert resp == char_json + end + + test "get /characters returns all characters for user", ctx do + # ensure some order to the list when comparing below. + chars = Enum.sort_by ctx.chars, fn c -> c.id end + + # this looks weird but the view returns a map of the data to be encoded + # however it still has atoms as keys, where as the resp below decoded + # has strings as keys so by encoding and decoding ew have the same map + # decoding the response will have + chars_decoded = CharacterView.render("index.json", conn: %{}, data: chars) + |> Poison.encode! + |> Poison.decode! + + path = api_character_path(conn, :index) + + conn = ctx.conn + |> put_req_header("authorization", ctx.token) + |> get(path) + + assert response_content_type(conn, :json) == @json_api_content_type_utf8 + resp = Poison.decode! response(conn, :ok) + # sort by id here as well. + resp = %{ resp | "data" => Enum.sort_by(resp["data"], fn d -> d["id"] end) } + + assert chars_decoded == resp + end end diff --git a/apps/yggdrasil_web/web/views/character_view.ex b/apps/yggdrasil_web/web/views/character_view.ex new file mode 100644 index 0000000..f111eba --- /dev/null +++ b/apps/yggdrasil_web/web/views/character_view.ex @@ -0,0 +1,8 @@ +defmodule YggdrasilWeb.CharacterView do + use YggdrasilWeb.Web, :view + use JaSerializer.PhoenixView + + attributes [:name, :user_id, :game_id] + + def type, do: "characters" +end From 53836017630a3f8d9f7f9a39b59ac978471e0a9d Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Mon, 22 Feb 2016 15:52:15 -0500 Subject: [PATCH 22/24] added coverage for create/delete and ensured the others tests fulfilled what was stated. --- .../controllers/character_controller_test.exs | 91 +++++++++++++++---- .../web/controllers/character_controller.ex | 5 +- 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/apps/yggdrasil_web/test/controllers/character_controller_test.exs b/apps/yggdrasil_web/test/controllers/character_controller_test.exs index eb15541..5a56040 100644 --- a/apps/yggdrasil_web/test/controllers/character_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/character_controller_test.exs @@ -17,9 +17,15 @@ defmodule YggdrasilWeb.CharacterControllerTest do |> User.create_changeset(@user) |> Repo.insert - #api token - {:ok, token, _claims} = Guardian.encode_and_sign user, :token + {:ok, user2} = %User{} + |> User.create_changeset(%{@user | username: "tester2"}) + |> Repo.insert + + users = Enum.map [user, user2], fn u -> + {:ok, token, _claims} = Guardian.encode_and_sign u, :token + %{ model: u, token: token} + end # set of games to have something to list games = Enum.map 1..2, fn n -> @@ -30,36 +36,43 @@ defmodule YggdrasilWeb.CharacterControllerTest do game end - chars = Enum.flat_map games, fn g -> - Enum.map 1..2, fn n -> - char = Character.changeset(%Character{}, %{:name => "char #{g.name} #{n}", - :game_id => g.id, - :user_id => user.id}) + users = Enum.map users, fn u -> + chars = Enum.flat_map games, fn g -> + Enum.map 1..2, fn n -> + char = Character.changeset(%Character{}, %{:name => "char #{g.name} #{u.model.id} #{n}", + :game_id => g.id, + :user_id => u.model.id}) - {:ok, char} = Repo.insert char + {:ok, char} = Repo.insert char - char + char + end end + + Map.put u, :chars, chars end conn = conn |> put_req_header("content-type", @json_api_content_type) |> put_req_header("accept", @json_api_content_type) - {:ok, %{conn: conn, chars: chars, games: games, user: user, token: token}} + {:ok, %{conn: conn, games: games, users: users}} end test "get /api/characters/char_id returns the correct character", ctx do - char = Enum.at ctx.chars, 0 + user = Enum.at ctx.users, 0 + char = Enum.at user.chars, 0 char_json = CharacterView.render("show.json", conn: %{}, data: char) |> Poison.encode! |> Poison.decode! + assert user.model.id == char_json["data"]["attributes"]["user-id"] + path = api_character_path(conn, :show, char.id) conn = ctx.conn - |> put_req_header("authorization", ctx.token) + |> put_req_header("authorization", user.token) |> get(path) assert response_content_type(conn, :json) == @json_api_content_type_utf8 @@ -68,22 +81,21 @@ defmodule YggdrasilWeb.CharacterControllerTest do assert resp == char_json end - test "get /characters returns all characters for user", ctx do - # ensure some order to the list when comparing below. - chars = Enum.sort_by ctx.chars, fn c -> c.id end + test "get /characters returns all characters for the user the token specifies", ctx do + user = Enum.at ctx.users, 0 # first user has the token + chars = Enum.sort_by user.chars, fn c -> c.id end - # this looks weird but the view returns a map of the data to be encoded - # however it still has atoms as keys, where as the resp below decoded - # has strings as keys so by encoding and decoding ew have the same map - # decoding the response will have chars_decoded = CharacterView.render("index.json", conn: %{}, data: chars) |> Poison.encode! |> Poison.decode! + # assert all chracters stored in our context have the correct user_id + assert Enum.all? chars_decoded["data"], fn cd -> user.model.id == cd["attributes"]["user-id"] end + path = api_character_path(conn, :index) conn = ctx.conn - |> put_req_header("authorization", ctx.token) + |> put_req_header("authorization", user.token) |> get(path) assert response_content_type(conn, :json) == @json_api_content_type_utf8 @@ -91,6 +103,45 @@ defmodule YggdrasilWeb.CharacterControllerTest do # sort by id here as well. resp = %{ resp | "data" => Enum.sort_by(resp["data"], fn d -> d["id"] end) } + # since all of our characters from context have the right user_id + # if this succeeds then all the ones from the response also have the right user_id assert chars_decoded == resp end + + test "post /characters creates a character for a given user", ctx do + user = Enum.at ctx.users, 0 + game = Enum.at ctx.games, 0 + char = %{"name" => "unique_tester", "game-id" => game.id} # server fills in user_id + + json_api = %{data: %{type: "characters", attributes: char}} + json = Poison.encode! json_api + + path = api_character_path(conn, :create) + + conn = ctx.conn + |> put_req_header("authorization", user.token) + |> post(path, json) + + assert response_content_type(conn, :json) == @json_api_content_type_utf8 + + resp = Poison.decode! response(conn, :ok) + assert resp["data"]["attributes"] == Map.put(char, "user-id", user.model.id) + # sort by id here as well. + end + + test "delete /characters/char_id removes a character for a given user", ctx do + user = Enum.at ctx.users, 0 + char = Enum.at user.chars, 0 + + + path = api_character_path(conn, :delete, char.id) + + conn = ctx.conn + |> put_req_header("authorization", user.token) + |> delete(path) + + assert response(conn, :no_content) + + assert nil == Repo.get Character, char.id + end end diff --git a/apps/yggdrasil_web/web/controllers/character_controller.ex b/apps/yggdrasil_web/web/controllers/character_controller.ex index 56fcf1a..8a9c86c 100644 --- a/apps/yggdrasil_web/web/controllers/character_controller.ex +++ b/apps/yggdrasil_web/web/controllers/character_controller.ex @@ -26,7 +26,7 @@ defmodule YggdrasilWeb.CharacterController do end def create(conn, %{"data" => %{"attributes" => attributes}}, user, _claims) do - attributes = Map.put attributes, :user_id, user.id + attributes = Map.put attributes, "user_id", user.id char = Character.changeset(%Character{}, attributes) case Repo.insert(char) do @@ -50,8 +50,7 @@ defmodule YggdrasilWeb.CharacterController do {:ok, _char} -> # http://jsonapi.org/format/#crud-deleting conn - |> put_status(204) - |> send_resp + |> send_resp(:no_content, "") {:error, err_changeset} -> render conn, :errors, data: err_changeset end From 7ecac4e43c0c49817cb27ac0e019e36738a21291 Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Mon, 22 Feb 2016 22:48:59 -0500 Subject: [PATCH 23/24] more tests to ensure a character can only be created for a given user and that a character can only be removed if it belongs to a given user. A given user being the one specified in the token. --- .../controllers/character_controller_test.exs | 47 +++++++++++++++++-- .../web/controllers/character_controller.ex | 11 ++++- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/apps/yggdrasil_web/test/controllers/character_controller_test.exs b/apps/yggdrasil_web/test/controllers/character_controller_test.exs index 5a56040..a73e24d 100644 --- a/apps/yggdrasil_web/test/controllers/character_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/character_controller_test.exs @@ -11,6 +11,8 @@ defmodule YggdrasilWeb.CharacterControllerTest do @json_api_content_type "application/vnd.api+json" @json_api_content_type_utf8 @json_api_content_type <> "; charset=utf-8" + @invalid_charater "Invalid Character" + setup %{conn: conn} do # need a user to gen a token for api {:ok, user} = %User{} @@ -81,7 +83,7 @@ defmodule YggdrasilWeb.CharacterControllerTest do assert resp == char_json end - test "get /characters returns all characters for the user the token specifies", ctx do + test "get /characters returns all characters for the user in the token provided", ctx do user = Enum.at ctx.users, 0 # first user has the token chars = Enum.sort_by user.chars, fn c -> c.id end @@ -108,7 +110,7 @@ defmodule YggdrasilWeb.CharacterControllerTest do assert chars_decoded == resp end - test "post /characters creates a character for a given user", ctx do + test "post /characters creates a character for the given user in the token provided", ctx do user = Enum.at ctx.users, 0 game = Enum.at ctx.games, 0 char = %{"name" => "unique_tester", "game-id" => game.id} # server fills in user_id @@ -129,7 +131,29 @@ defmodule YggdrasilWeb.CharacterControllerTest do # sort by id here as well. end - test "delete /characters/char_id removes a character for a given user", ctx do + test "post /characters creates a character for the given user in the token regardless of the user_id provided", ctx do + user = Enum.at ctx.users, 0 + user2 = Enum.at ctx.users, 1 + game = Enum.at ctx.games, 0 + char = %{"name" => "unique_tester", "game-id" => game.id, "user-id" => user2.model.id} # server fills in user_id + + json_api = %{data: %{type: "characters", attributes: char}} + json = Poison.encode! json_api + + path = api_character_path(conn, :create) + + conn = ctx.conn + |> put_req_header("authorization", user.token) + |> post(path, json) + + assert response_content_type(conn, :json) == @json_api_content_type_utf8 + + resp = Poison.decode! response(conn, :ok) + assert resp["data"]["attributes"] == Map.put(char, "user-id", user.model.id) + # sort by id here as well. + end + + test "delete /characters/char_id removes the character specified for the given user in the token provided", ctx do user = Enum.at ctx.users, 0 char = Enum.at user.chars, 0 @@ -144,4 +168,21 @@ defmodule YggdrasilWeb.CharacterControllerTest do assert nil == Repo.get Character, char.id end + + test "delete /characters/char_id doesn't removes the character specified for another user instead of user in the token provided", ctx do + user = Enum.at ctx.users, 0 + user2 = Enum.at ctx.users, 1 + char = Enum.at user2.chars, 0 + + + path = api_character_path(conn, :delete, char.id) + + conn = ctx.conn + |> put_req_header("authorization", user.token) + |> delete(path) + + assert response(conn, :ok) =~ @invalid_charater + + assert char == Repo.get Character, char.id + end end diff --git a/apps/yggdrasil_web/web/controllers/character_controller.ex b/apps/yggdrasil_web/web/controllers/character_controller.ex index 8a9c86c..03c00af 100644 --- a/apps/yggdrasil_web/web/controllers/character_controller.ex +++ b/apps/yggdrasil_web/web/controllers/character_controller.ex @@ -46,7 +46,16 @@ defmodule YggdrasilWeb.CharacterController do where: c.id == ^char_id and c.user_id == ^user_id, select: c - case Repo.delete(char)do + do_delete conn, char + end + + defp do_delete(conn, nil) do + render conn, :errors, data: %{title: "Invalid Character", + detail: "Character not found for the specified user"} + end + + defp do_delete(conn, char) do + case Repo.delete(char) do {:ok, _char} -> # http://jsonapi.org/format/#crud-deleting conn From f595d8095353ec5302229fed77f452b096f2ddbe Mon Sep 17 00:00:00 2001 From: Matthew Philyaw Date: Sat, 2 Apr 2016 09:32:57 -0400 Subject: [PATCH 24/24] Char.perms Adds roles and permissions. This includes facilities to check that permissions are granted and a plug to control access to controllers based on permissions. --- .gitignore | 1 + apps/yggdrasil/lib/yggdrasil/permission.ex | 8 + apps/yggdrasil/lib/yggdrasil/resource.ex | 8 + apps/yggdrasil/lib/yggdrasil/role.ex | 10 + .../lib/yggdrasil/role_permission.ex | 13 + apps/yggdrasil/lib/yggdrasil/user.ex | 109 ++++++- apps/yggdrasil/lib/yggdrasil/user_role.ex | 20 ++ apps/yggdrasil/mix.exs | 4 +- .../20160209030030_add_rooms_table.exs | 6 - .../migrations/20160229161938_create_role.exs | 13 + .../20160229162004_create_resource.exs | 12 + .../20160229162017_create_permission.exs | 12 + .../20160229163000_create_role_permission.exs | 15 + .../20160303191523_create_users_roles.exs | 14 + apps/yggdrasil/priv/repo/seeds.exs | 75 +++++ apps/yggdrasil/test/test_helper.exs | 9 +- .../test/yggdrasil/user_role_test.exs | 94 ++++++ apps/yggdrasil/test/yggdrasil/user_test.exs | 268 +++++++++++++++++- apps/yggdrasil_web/lib/guardian_serializer.ex | 14 +- .../lib/yggdrasil_web/ensure_permission.ex | 43 +++ .../controllers/character_controller_test.exs | 9 +- .../test/controllers/game_controller_test.exs | 4 +- .../registration_controller_test.exs | 6 +- .../controllers/session_controller_test.exs | 6 +- .../test/ensure_permission_test.exs | 104 +++++++ .../web/controllers/character_controller.ex | 52 +++- .../web/controllers/game_controller.ex | 3 + .../controllers/registration_controller.ex | 7 +- mix.lock | 7 +- 29 files changed, 897 insertions(+), 49 deletions(-) create mode 100644 apps/yggdrasil/lib/yggdrasil/permission.ex create mode 100644 apps/yggdrasil/lib/yggdrasil/resource.ex create mode 100644 apps/yggdrasil/lib/yggdrasil/role.ex create mode 100644 apps/yggdrasil/lib/yggdrasil/role_permission.ex create mode 100644 apps/yggdrasil/lib/yggdrasil/user_role.ex delete mode 100644 apps/yggdrasil/priv/repo/migrations/20160209030030_add_rooms_table.exs create mode 100644 apps/yggdrasil/priv/repo/migrations/20160229161938_create_role.exs create mode 100644 apps/yggdrasil/priv/repo/migrations/20160229162004_create_resource.exs create mode 100644 apps/yggdrasil/priv/repo/migrations/20160229162017_create_permission.exs create mode 100644 apps/yggdrasil/priv/repo/migrations/20160229163000_create_role_permission.exs create mode 100644 apps/yggdrasil/priv/repo/migrations/20160303191523_create_users_roles.exs create mode 100644 apps/yggdrasil/test/yggdrasil/user_role_test.exs create mode 100644 apps/yggdrasil_web/lib/yggdrasil_web/ensure_permission.ex create mode 100644 apps/yggdrasil_web/test/ensure_permission_test.exs diff --git a/.gitignore b/.gitignore index 9288916..11959ef 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +*.swp /_build /cover apps/*/cover diff --git a/apps/yggdrasil/lib/yggdrasil/permission.ex b/apps/yggdrasil/lib/yggdrasil/permission.ex new file mode 100644 index 0000000..51ea2c0 --- /dev/null +++ b/apps/yggdrasil/lib/yggdrasil/permission.ex @@ -0,0 +1,8 @@ +defmodule Yggdrasil.Permission do + use Ecto.Schema + + @primary_key {:name, :string, []} + schema "permissions" do + timestamps + end +end diff --git a/apps/yggdrasil/lib/yggdrasil/resource.ex b/apps/yggdrasil/lib/yggdrasil/resource.ex new file mode 100644 index 0000000..ff9dff0 --- /dev/null +++ b/apps/yggdrasil/lib/yggdrasil/resource.ex @@ -0,0 +1,8 @@ +defmodule Yggdrasil.Resource do + use Ecto.Schema + + @primary_key {:name, :string, []} + schema "resources" do + timestamps + end +end diff --git a/apps/yggdrasil/lib/yggdrasil/role.ex b/apps/yggdrasil/lib/yggdrasil/role.ex new file mode 100644 index 0000000..e64f553 --- /dev/null +++ b/apps/yggdrasil/lib/yggdrasil/role.ex @@ -0,0 +1,10 @@ +defmodule Yggdrasil.Role do + use Ecto.Schema + + schema "roles" do + field :name, :string + has_many :role_permissions, Yggdrasil.RolePermission + + timestamps + end +end diff --git a/apps/yggdrasil/lib/yggdrasil/role_permission.ex b/apps/yggdrasil/lib/yggdrasil/role_permission.ex new file mode 100644 index 0000000..a8a43a2 --- /dev/null +++ b/apps/yggdrasil/lib/yggdrasil/role_permission.ex @@ -0,0 +1,13 @@ +defmodule Yggdrasil.RolePermission do + use Ecto.Schema + + @primary_key false + schema "roles_permissions" do + field :resource, :string + field :permission, :string + + belongs_to :role, Yggdrasil.Role + + timestamps + end +end diff --git a/apps/yggdrasil/lib/yggdrasil/user.ex b/apps/yggdrasil/lib/yggdrasil/user.ex index bde12a0..d708a32 100644 --- a/apps/yggdrasil/lib/yggdrasil/user.ex +++ b/apps/yggdrasil/lib/yggdrasil/user.ex @@ -1,14 +1,21 @@ defmodule Yggdrasil.User do use Ecto.Schema import Ecto.Changeset + import Ecto.Query, only: [from: 1, from: 2] import Comeonin.Bcrypt, only: [hashpwsalt: 1] + require Logger + alias Yggdrasil.{Repo, User, Role, UserRole} + + @default_role "player" + schema "users" do field :username, :string field :hash, :string field :password, :string, virtual: true # not part of table field :password_confirmation, :string, virtual: true # not part of table + field :permissions, :any, default: [], virtual: true # :any skips type checking timestamps end @@ -29,14 +36,108 @@ defmodule Yggdrasil.User do end @doc """ - adds password at end of chain protects the hashpwsalt from seeing a nil value - in the case of missing password field. + Creates the user and assigns the default roles of "player" """ - def hash_password(changeset = %{:valid? => false}) do + def create_with_default_role(attributes) do + changeset = User.create_changeset(%User{}, attributes) + + with {:ok, user} <- Repo.insert(changeset), + :ok <- assign_role(user, "player"), + do: {:ok, user} + end + + def assign_role(user, role_name) do + role = Repo.get_by!(Role, name: role_name) + + user_role = UserRole.changeset(%UserRole{}, + %{user_id: user.id, role_id: role.id}) + case Repo.insert(user_role) do + {:ok, _role} -> :ok + error -> error + end + end + + def load_permissions(user) do + query = from ur in UserRole, + where: ur.user_id == ^user.id, + preload: [role: [:role_permissions]] + + permissions = query + |> Repo.all() + |> Enum.flat_map(fn ur -> ur.role.role_permissions end) + |> Enum.group_by(fn rp -> rp.resource end) + |> Enum.map(fn {r, rps} -> {String.to_atom(r), Enum.map(rps, fn rp -> String.to_atom(rp.permission) end)} end) + |> Enum.into(%{}) + + %{ user | permissions: permissions} + end + + @doc ~S""" + Checks the list of resource/permissions pair given against what the user + has been granted and returns true if and only if all permissions supplied + have been granted for the user. + + ## roles + A user can have one or more roles with each role having a set of resources + and a set of resources and those resources having a set of permissions. Roles + are not checked directly and only serve as a grouping mechanism for resources. + + Since the user can have multiple roles, the permission set checked for a given + resource is the unique set across all the roles for that resource. + + So if the user has `Role1` and `Role2` defined as follows: + + * Role1 + * foo + * :read + * :write + * Role2 + * foo + * :read + * :all + + The set of permissions checked for foo will be `[:read, :write, :all]` + + ## Examples + # checking a single resource + User.is_granted? user, foo: [:read] + + # checking multiple resources is allowed as well + User.is_granted? user, foo: [:read], bar: [:read, :write] + """ + + def is_granted?(_user, []), do: raise_error "resource keyword list is empty" + def is_granted?(_user, nil), do: raise_error "resource keyword list is nil" + def is_granted?(user, res_perms) do + res_perms + |> Enum.map(fn {res, perms} -> {res, user.permissions[res], perms} end) + |> Enum.map(&has_permissions/1) + |> Enum.all? + end + + defp has_permissions({_res, nil, _requried}), do: false + defp has_permissions({res, _given, nil}), do: raise_error "required permission list is nil for #{res}" + defp has_permissions({res, [], _required}), do: raise_error "given permission list is empty for #{res}" + defp has_permissions({res, _given, []}), do: raise_error "required permission list is empty for #{res}" + defp has_permissions({_res, given, required}) do + given = MapSet.new given + required = MapSet.new required + + MapSet.subset?(required, given) + end + + defp raise_error(msg) do + Logger.error msg + raise ArgumentError, message: msg + end + + # adds password at end of chain protects the hashpwsalt from seeing a nil value + # in the case of missing password field. + defp hash_password(changeset = %{:valid? => false}) do changeset end - def hash_password(changeset = %{:valid? => true}) do + defp hash_password(changeset = %{:valid? => true}) do changeset |> put_change(:hash, hashpwsalt(changeset.params["password"])) end diff --git a/apps/yggdrasil/lib/yggdrasil/user_role.ex b/apps/yggdrasil/lib/yggdrasil/user_role.ex new file mode 100644 index 0000000..aeda39e --- /dev/null +++ b/apps/yggdrasil/lib/yggdrasil/user_role.ex @@ -0,0 +1,20 @@ +defmodule Yggdrasil.UserRole do + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + schema "users_roles" do + belongs_to :user, Yggdrasil.User + belongs_to :role, Yggdrasil.Role + + timestamps + end + + def changeset(model, params \\ :empty) do + model + |> cast(params, [:user_id, :role_id], []) + |> unique_constraint(:user_id, name: :users_roles_user_id_role_id_index) + |> assoc_constraint(:user) + |> assoc_constraint(:role) + end +end diff --git a/apps/yggdrasil/mix.exs b/apps/yggdrasil/mix.exs index a28f399..01b519e 100644 --- a/apps/yggdrasil/mix.exs +++ b/apps/yggdrasil/mix.exs @@ -41,7 +41,9 @@ defmodule Yggdrasil.Mixfile do [ {:postgrex, ">= 0.0.0"}, {:ecto, "~> 1.1"}, - {:excoveralls, "~>0.5.1"} + {:comeonin, "~> 2.0"}, + {:excoveralls, "~>0.5.1"}, + {:combination, "~> 0.0.2", only: :test} ] end diff --git a/apps/yggdrasil/priv/repo/migrations/20160209030030_add_rooms_table.exs b/apps/yggdrasil/priv/repo/migrations/20160209030030_add_rooms_table.exs deleted file mode 100644 index 25c6fcd..0000000 --- a/apps/yggdrasil/priv/repo/migrations/20160209030030_add_rooms_table.exs +++ /dev/null @@ -1,6 +0,0 @@ -defmodule Yggdrasil.Repo.Migrations.AddRoomsTable do - use Ecto.Migration - - def change do - end -end diff --git a/apps/yggdrasil/priv/repo/migrations/20160229161938_create_role.exs b/apps/yggdrasil/priv/repo/migrations/20160229161938_create_role.exs new file mode 100644 index 0000000..08ad774 --- /dev/null +++ b/apps/yggdrasil/priv/repo/migrations/20160229161938_create_role.exs @@ -0,0 +1,13 @@ +defmodule Yggdrasil.Repo.Migrations.CreateRole do + use Ecto.Migration + + def change do + create table(:roles) do + add :name, :string + + timestamps + end + create unique_index(:roles, [:name]) + + end +end diff --git a/apps/yggdrasil/priv/repo/migrations/20160229162004_create_resource.exs b/apps/yggdrasil/priv/repo/migrations/20160229162004_create_resource.exs new file mode 100644 index 0000000..e916dc0 --- /dev/null +++ b/apps/yggdrasil/priv/repo/migrations/20160229162004_create_resource.exs @@ -0,0 +1,12 @@ +defmodule Yggdrasil.Repo.Migrations.CreateResource do + use Ecto.Migration + + def change do + create table(:resources, primary_key: false) do + add :name, :string, primary_key: true + + timestamps + end + + end +end diff --git a/apps/yggdrasil/priv/repo/migrations/20160229162017_create_permission.exs b/apps/yggdrasil/priv/repo/migrations/20160229162017_create_permission.exs new file mode 100644 index 0000000..2aea694 --- /dev/null +++ b/apps/yggdrasil/priv/repo/migrations/20160229162017_create_permission.exs @@ -0,0 +1,12 @@ +defmodule Yggdrasil.Repo.Migrations.CreatePermission do + use Ecto.Migration + + def change do + create table(:permissions, primary_key: false) do + add :name, :string, primary_key: true + + timestamps + end + + end +end diff --git a/apps/yggdrasil/priv/repo/migrations/20160229163000_create_role_permission.exs b/apps/yggdrasil/priv/repo/migrations/20160229163000_create_role_permission.exs new file mode 100644 index 0000000..8dfaadc --- /dev/null +++ b/apps/yggdrasil/priv/repo/migrations/20160229163000_create_role_permission.exs @@ -0,0 +1,15 @@ +defmodule Yggdrasil.Repo.Migrations.CreateRoleResource do + use Ecto.Migration + + def change do + create table(:roles_permissions, primary_key: false) do + add :role_id, references(:roles, on_delete: :nothing) + add :resource, references(:resources, on_delete: :nothing, column: :name, type: :string) + add :permission, references(:permissions, on_delete: :nothing, column: :name, type: :string) + + timestamps + end + create unique_index(:roles_permissions, [:role_id, :resource, :permission]) + + end +end diff --git a/apps/yggdrasil/priv/repo/migrations/20160303191523_create_users_roles.exs b/apps/yggdrasil/priv/repo/migrations/20160303191523_create_users_roles.exs new file mode 100644 index 0000000..15f56fb --- /dev/null +++ b/apps/yggdrasil/priv/repo/migrations/20160303191523_create_users_roles.exs @@ -0,0 +1,14 @@ +defmodule Yggdrasil.Repo.Migrations.CreateUsersRoles do + use Ecto.Migration + + def change do + create table(:users_roles, primary_key: false) do + add :user_id, references(:users, on_delete: :nothing) + add :role_id, references(:roles, on_delete: :nothing) + + timestamps + end + create unique_index(:users_roles, [:user_id, :role_id]) + + end +end diff --git a/apps/yggdrasil/priv/repo/seeds.exs b/apps/yggdrasil/priv/repo/seeds.exs index f5de750..6ad4615 100644 --- a/apps/yggdrasil/priv/repo/seeds.exs +++ b/apps/yggdrasil/priv/repo/seeds.exs @@ -9,3 +9,78 @@ # # We recommend using the bang functions (`insert!`, `update!` # and so on) as they will fail if something goes wrong. +require Logger + +alias Yggdrasil.{Repo, Resource, Permission, Role, RolePermission} + +perms = ["read", "write", "all"] +res = ["character", "game"] + +Repo.transaction fn -> + Enum.each perms, fn p -> + if Repo.get(Permission, p) == nil do + Repo.insert! %Permission{name: p} + else + Logger.warn fn -> "found permission #{p} skipping insert" end + end + end + + Enum.each res, fn r -> + if Repo.get(Resource, r) == nil do + Repo.insert! %Resource{name: r} + else + Logger.warn fn -> "found resource #{r} skipping insert" end + end + end + + # NOTE: only checks to see if the role exists + # if so skips it, it doesn't try to be fancy + # if a role is found also ensure permissions + # are indeed there. + + # player role --- + player_role = Repo.get_by(Role, name: "player") + + if player_role == nil do + player_role = Repo.insert! %Role{name: "player"} + + player_perms = Enum.filter perms, fn p -> p != "all" end + + Enum.each res, fn r -> + permissions = if r == "game" do + Enum.filter player_perms, fn p -> p != "write" end + else + player_perms + end + + Enum.each permissions, fn p -> + Repo.insert! %RolePermission{ + role_id: player_role.id, + resource: r, + permission: p + } + end + end + else + Logger.warn fn -> "found role #{player_role.name} skipping insert" end + end + + # admin role --- + admin_role = Repo.get_by(Role, name: "admin") + + if admin_role == nil do + admin_role = Repo.insert! %Role{name: "admin"} + + Enum.each res, fn r -> + Enum.each perms, fn p -> + Repo.insert! %RolePermission{ + role_id: admin_role.id, + resource: r, + permission: p + } + end + end + else + Logger.warn fn -> "found role #{admin_role.name} skipping insert" end + end +end diff --git a/apps/yggdrasil/test/test_helper.exs b/apps/yggdrasil/test/test_helper.exs index ff8beca..a82cdac 100644 --- a/apps/yggdrasil/test/test_helper.exs +++ b/apps/yggdrasil/test/test_helper.exs @@ -1,4 +1,9 @@ ExUnit.start() -Mix.Task.run "ecto.create", ~w(-r Yggdrasil.Repo --quiet) -Mix.Task.run "ecto.migrate", ~w(-r Yggdrasil.Repo --quiet) +repo = ~w(-r Yggdrasil.Repo --quiet) + +Mix.Task.run "ecto.create", repo +Mix.Task.run "ecto.migrate", repo + +run_path = Application.app_dir :yggdrasil, Path.join(["priv", "repo", "seeds.exs"]) +Mix.Task.run "run", [run_path] diff --git a/apps/yggdrasil/test/yggdrasil/user_role_test.exs b/apps/yggdrasil/test/yggdrasil/user_role_test.exs new file mode 100644 index 0000000..6f8ca91 --- /dev/null +++ b/apps/yggdrasil/test/yggdrasil/user_role_test.exs @@ -0,0 +1,94 @@ +defmodule Yggdrasil.UserRoleTest do + use ExUnit.Case, async: false + + alias Yggdrasil.{Repo, User, Role, UserRole, RolePermission, Resource, Permission} + + @user %{username: "tester", + password: "password", + password_confirmation: "password"} + + @test_perms [:foo_perm, :bar_perm, :baz_perm] + @test_resources [:foo_res, :bar_res, :baz_res] + @test_roles [ + %{ + name: :foo_role, + resources: [ + %{ + name: :foo_res, + perms: [:foo_perm, :bar_perm] + }, + %{ + name: :bar_res, + perms: [:bar_perm, :baz_perm] + } + ] + } + ] + + setup tags do + unless tags[:async] do + Ecto.Adapters.SQL.restart_test_transaction(Yggdrasil.Repo, []) + end + + :ok + end + + setup _ctx do + Enum.each @test_perms, fn p -> + Repo.insert! %Permission{name: Atom.to_string(p)} + end + + Enum.each @test_resources, fn r -> + Repo.insert! %Resource{name: Atom.to_string(r)} + end + + roles = Enum.map @test_roles, fn tr -> + role = Repo.insert! %Role{name: Atom.to_string(tr.name)} + + Enum.each tr.resources, fn trs -> + Enum.each trs.perms, fn trpm -> + Repo.insert! %RolePermission{ + role_id: role.id, + resource: Atom.to_string(trs.name), + permission: Atom.to_string(trpm) + } + end + end + + role + end + + user = %User{} + |> User.create_changeset(@user) + |> Repo.insert!() + + [role] = roles + {:ok, %{user: user, role: role}} + end + + test "changeset/2 with invalid user and valid role returns an error", ctx do + result = %UserRole{} + |> UserRole.changeset(%{user_id: -12, role_id: ctx.role.id}) + |> Repo.insert + + {:error, changeset} = result + assert {:user, "does not exist"} in changeset.errors + end + + test "changeset/2 with invalid role and valid user returns an error", ctx do + result = %UserRole{} + |> UserRole.changeset(%{user_id: ctx.user.id, role_id: -12}) + |> Repo.insert + + {:error, changeset} = result + assert {:role, "does not exist"} in changeset.errors + end + + test "changeset/2 with valid user and role", ctx do + result = %UserRole{} + |> UserRole.changeset(%{user_id: ctx.user.id, role_id: ctx.role.id}) + |> Repo.insert + + {:ok, _} = result + end +end diff --git a/apps/yggdrasil/test/yggdrasil/user_test.exs b/apps/yggdrasil/test/yggdrasil/user_test.exs index 55aee0a..3d34330 100644 --- a/apps/yggdrasil/test/yggdrasil/user_test.exs +++ b/apps/yggdrasil/test/yggdrasil/user_test.exs @@ -1,7 +1,8 @@ defmodule Yggdrasil.UserTest do use ExUnit.Case, async: false - alias Yggdrasil.Repo - alias Yggdrasil.User + import Ecto.Query, only: [from: 1, from: 2] + + alias Yggdrasil.{Repo, User, UserRole, Role, RolePermission, Resource, Permission} alias Comeonin.Bcrypt @min_len 4 @@ -25,6 +26,41 @@ defmodule Yggdrasil.UserTest do @unique_msg "has already been taken" @invalid_length_msg "should be at least %{count} character(s)" + @test_perms [:foo_perm, :bar_perm, :baz_perm] + @test_resources [:foo_res, :bar_res, :baz_res] + @test_roles [ + %{ + name: :foo_role, + resources: [ + %{ + name: :foo_res, + perms: [:foo_perm, :bar_perm] + }, + %{ + name: :bar_res, + perms: [:bar_perm, :baz_perm] + } + ] + }, + %{ + name: :bar_role, + resources: [ + %{ + name: :foo_res, + perms: [:baz_perm] + }, + %{ + name: :bar_res, + perms: [:foo_perm, :baz_perm] + }, + %{ + name: :baz_res, + perms: [:foo_perm, :bar_perm, :baz_perm] + } + ] + } + ] + setup tags do unless tags[:async] do Ecto.Adapters.SQL.restart_test_transaction(Yggdrasil.Repo, []) @@ -43,6 +79,83 @@ defmodule Yggdrasil.UserTest do !Map.has_key?(changeset.changes, :hash) end + defp insert_test_roles do + Enum.each @test_perms, fn p -> + Repo.insert! %Permission{name: Atom.to_string(p)} + end + + Enum.each @test_resources, fn r -> + Repo.insert! %Resource{name: Atom.to_string(r)} + end + + roles = Enum.map @test_roles, fn tr -> + role = Repo.insert! %Role{name: Atom.to_string(tr.name)} + + Enum.each tr.resources, fn trs -> + Enum.each trs.perms, fn trpm -> + Repo.insert! %RolePermission{ + role_id: role.id, + resource: Atom.to_string(trs.name), + permission: Atom.to_string(trpm) + } + end + end + + role + end + + roles + end + + defp user_perms_for_roles(roles) do + insert_test_roles() + + user = %User{} + |> User.create_changeset(@valid_attrs) + |> Repo.insert!() + + Enum.each roles, fn r -> + :ok = User.assign_role(user, Atom.to_string(r.name)) + end + + user = User.load_permissions(user) + + res_perm_list = roles + |> Enum.flat_map(fn r -> r.resources end) + |> Enum.group_by(fn res -> res.name end) + |> Enum.map(fn {res_name, res} -> + {res_name, Enum.flat_map(res, fn res -> res.perms end)} + end) + + res_uniq_perm_list = Enum.map res_perm_list, fn {res, perms} -> + {res, Enum.uniq(perms)} + end + + {user, res_uniq_perm_list, res_perm_list} + end + + defp produce_resoure_perms_combos(res_perms) do + # unique combinations per resource + # [[foo: [:foo_p, :bar_p], foo: [:bar_p], foo: [foo_p]], [bar: [:foo_p, :bar_p], bar: [:foo_p], etc...]] + res_perm_combo = Enum.map res_perms, fn {k, v} -> + Enum.flat_map(1..Enum.count(v), &(Combination.combine(v, &1))) + |> Enum.map(fn combos -> {k, combos} end) + end + + # taks the res_perm_combo and produces a list of resource/perm list combos + # so the size of list combos is the number of resources and why the count of + # res_perm_list is taken. In order to produce list of combos the combinations where + # a resoure appears twice or for the whole combo list needs to be filtered out. + # so this produces + # [[foo: [:foo_p, :bar_p], bar: [:foo_p]], [foo: [:foo_p, :bar_p], bar: [:foo_p, :bar_p]]] etc.. + uniq_res_perm_combos = res_perm_combo + |> List.flatten + |> Combination.combine(Enum.count(res_perms)) + |> Enum.filter(fn combo -> combo == Enum.uniq_by(combo, fn {k, _} -> k end) end) + + {res_perm_combo, uniq_res_perm_combos} + end + # -- tests test "create_changeset with valid attributes" do @@ -194,4 +307,155 @@ defmodule Yggdrasil.UserTest do refute changeset.valid? refute Map.has_key?(changeset.changes, :hash) end + + test "is_granted?/2 returns true with all granted resource/perms" do + {user, res_uniq_perm_list, _} = user_perms_for_roles(@test_roles) + + assert User.is_granted? user, res_uniq_perm_list + end + + test "is_granted?/2 returns true for any one resources's granted permission set" do + {user, res_uniq_perm_list, _} = user_perms_for_roles(@test_roles) + + Enum.each res_uniq_perm_list, fn res_perm -> + assert User.is_granted? user, [res_perm] + end + end + + test "is_granted?/2 returns true any combination of granted permissions" do + {user, res_uniq_perm_list, _} = user_perms_for_roles(@test_roles) + + {res_perm_combo, uniq_res_perm_combos} = produce_resoure_perms_combos(res_uniq_perm_list) + + # lets check each resource combo individually first + Enum.each res_perm_combo, fn res_list -> + Enum.each res_list, fn combo -> + assert User.is_granted? user, [combo] + end + end + + # now lets check all the combos + Enum.each uniq_res_perm_combos, fn combo -> + assert User.is_granted? user, combo + end + end + + test "is_granted?/2 returns false for all combinations of non granted permissions" do + {user, res_uniq_perm_list, _} = user_perms_for_roles(@test_roles) + + res_uniq_perm_list = Enum.map res_uniq_perm_list, fn {res, perms} -> + perms = Enum.map perms, fn p -> + p = Atom.to_string(p) + p = p <> "_false" + String.to_atom(p) + end + + {res, perms} + end + + {res_perm_combo, uniq_res_perm_combos} = produce_resoure_perms_combos(res_uniq_perm_list) + + # lets check each resource combo individually first + Enum.each res_perm_combo, fn res_list -> + Enum.each res_list, fn combo -> + refute User.is_granted? user, [combo] + end + end + + # now lets check all the combos + Enum.each uniq_res_perm_combos, fn combo -> + refute User.is_granted? user, combo + end + end + + test "is_granted?/2 returns false when resource is missing on users permissions map" do + user = %User{permissions: %{bar: [:read, :write]}} + perm_set = [foo: [:write], bar: [:read]] + + refute User.is_granted?(user, perm_set) + end + + test "is_granted?/2 raises an error with with empty permissions set passed in for a resource" do + {user, res_uniq_perm_list, _} = user_perms_for_roles(@test_roles) + + all_empty = Enum.map(res_uniq_perm_list, fn {res, _} -> {res, []} end) + [{res, _perms} | t] = res_uniq_perm_list + + assert_raise ArgumentError, fn -> User.is_granted?(user, [{res, []}]) end + assert_raise ArgumentError, fn -> User.is_granted?(user, [{res, []} | t]) end + assert_raise ArgumentError, fn -> User.is_granted?(user, all_empty) end + end + + test "is_granted?/2 raises an error with empty resource perimssion list" do + {user, _, _} = user_perms_for_roles(@test_roles) + + assert_raise ArgumentError, fn -> User.is_granted?(user, []) end + end + + test "is_granted?/2 raises an error with nil resource perimssion list" do + {user, _, _} = user_perms_for_roles(@test_roles) + + assert_raise ArgumentError, fn -> User.is_granted?(user, nil) end + end + + test "is_granted?/2 raises an error with permission map on user container a resource key with empty list" do + user = %User{permissions: %{foo: [], bar: [:read, :write]}} + perm_set = [foo: [:write], bar: [:read]] + + assert_raise ArgumentError, fn -> User.is_granted?(user, perm_set) end + end + + test "is_granted?/2 does not raise an error when resource is missing on users permissions map" do + user = %User{permissions: %{bar: [:read, :write]}} + perm_set = [foo: [:write], bar: [:read]] + + User.is_granted?(user, perm_set) + end + + test "create_with_default_role/1 creates user and assigns 'player' as the default role" do + role = Repo.one!(from r in Role, where: r.name == "player", select: r) + + {:ok, user} = User.create_with_default_role(@valid_attrs) + + q = from ur in UserRole, + where: ur.role_id == ^role.id and ur.user_id == ^ user.id, + select: ur + + Repo.one! q + end + + test "assign_role/2 associates supplied role to user" do + [role|_rest] = insert_test_roles + + user = %User{} + |> User.create_changeset(@valid_attrs) + |> Repo.insert! + + :ok = User.assign_role(user, role.name) + + q = from ur in UserRole, + where: ur.role_id == ^role.id and ur.user_id == ^ user.id, + select: ur + + Repo.one! q + end + + test "assign_role/2 with invalid user returns an error" do + [role|_rest] = insert_test_roles + + user = %User{id: -12} + + assert {:error, msg} = User.assign_role(user, role.name) + assert {:user, "does not exist"} in msg.errors + end + + test "assign_role/2 with invalid role raises an Ecto.NoResultsError" do + user = %User{} + |> User.create_changeset(@valid_attrs) + |> Repo.insert! + + assert_raise Ecto.NoResultsError, fn -> + User.assign_role(user, "_not_valid") + end + end end diff --git a/apps/yggdrasil_web/lib/guardian_serializer.ex b/apps/yggdrasil_web/lib/guardian_serializer.ex index f55bfd2..0b20849 100644 --- a/apps/yggdrasil_web/lib/guardian_serializer.ex +++ b/apps/yggdrasil_web/lib/guardian_serializer.ex @@ -1,12 +1,20 @@ defmodule YggdrasilWeb.GuardianSerializer do @behaviour Guardian.Serializer - alias Yggdrasil.Repo - alias Yggdrasil.User + require Ecto.Query + import Ecto.Query, only: [from: 1, from: 2] + + alias Yggdrasil.{Repo, User, Role} def for_token(user = %User{}), do: {:ok, "user:#{user.id}"} def for_token(_), do: {:error, "Unknown resource type."} - def from_token("user:" <> user_id), do: {:ok, Repo.get(User, user_id)} + def from_token("user:" <> user_id) do + user = User + |> Repo.get!(user_id) + |> User.load_permissions + + {:ok, user} + end def from_token(_), do: {:error, "Unknown resource type."} end diff --git a/apps/yggdrasil_web/lib/yggdrasil_web/ensure_permission.ex b/apps/yggdrasil_web/lib/yggdrasil_web/ensure_permission.ex new file mode 100644 index 0000000..65d97bb --- /dev/null +++ b/apps/yggdrasil_web/lib/yggdrasil_web/ensure_permission.ex @@ -0,0 +1,43 @@ +defmodule YggdrasilWeb.EnsurePermission do + @moduledoc ~S""" + ## Overview + This plug checks to ensure all permissions provided have been granted for + the user on the connection. If all permissions have been granted then the + plug will allow the request to proceed otherwise it will send a 401 to the + client and halt the connection. + + Please check the docs for `Yggdrasil.User.is_granted?/2`, which is the internal + call that checks the user's permissions. More details can be found there. + + ## Examples + # checking a single resource + plug EnsurePermission, [foo: [:read]] + + # checking multiple resources is allowed as well + plug EnsurePermission, [foo: [:read], bar: [:read, :write]] + + being a plug it can also be used with guard to only restrict certain + actions with permissions. In the following example the :index action + will only be allowed if the foo resource has the :read permission + plug EnsurePermission, [foo: [:read]] when action [:index] + """ + + import Plug.Conn + + alias Yggdrasil.User + + def init(opts) do + opts + end + + def call(conn, res_perms) do + user = Guardian.Plug.current_resource(conn) + if User.is_granted?(user, res_perms) do + conn + else + conn + |> send_resp(:unauthorized, "") + |> halt + end + end +end diff --git a/apps/yggdrasil_web/test/controllers/character_controller_test.exs b/apps/yggdrasil_web/test/controllers/character_controller_test.exs index a73e24d..e1af410 100644 --- a/apps/yggdrasil_web/test/controllers/character_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/character_controller_test.exs @@ -15,13 +15,10 @@ defmodule YggdrasilWeb.CharacterControllerTest do setup %{conn: conn} do # need a user to gen a token for api - {:ok, user} = %User{} - |> User.create_changeset(@user) - |> Repo.insert + user2_attrs = %{@user | username: "tester2"} - {:ok, user2} = %User{} - |> User.create_changeset(%{@user | username: "tester2"}) - |> Repo.insert + {:ok, user} = User.create_with_default_role(@user) + {:ok, user2} = User.create_with_default_role(user2_attrs) users = Enum.map [user, user2], fn u -> {:ok, token, _claims} = Guardian.encode_and_sign u, :token diff --git a/apps/yggdrasil_web/test/controllers/game_controller_test.exs b/apps/yggdrasil_web/test/controllers/game_controller_test.exs index 8bbf72a..e7e4e4f 100644 --- a/apps/yggdrasil_web/test/controllers/game_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/game_controller_test.exs @@ -12,9 +12,7 @@ defmodule YggdrasilWeb.GameControllerTest do setup %{conn: conn} do # need a user to gen a token for api - {:ok, user} = %User{} - |> User.create_changeset(@user) - |> Repo.insert + {:ok, user} = User.create_with_default_role(@user) #api token {:ok, token, _claims} = Guardian.encode_and_sign user, :token diff --git a/apps/yggdrasil_web/test/controllers/registration_controller_test.exs b/apps/yggdrasil_web/test/controllers/registration_controller_test.exs index 001b8e0..c6b408a 100644 --- a/apps/yggdrasil_web/test/controllers/registration_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/registration_controller_test.exs @@ -85,7 +85,11 @@ defmodule YggdrasilWeb.RegistrationControllerTest do user_id = res_user["data"]["id"] - assert Guardian.serializer.from_token(claims["sub"]) == {:ok, Repo.get(User, user_id)} + user = User + |> Repo.get!(user_id) + |> User.load_permissions + + assert Guardian.serializer.from_token(claims["sub"]) == {:ok, user} end test "user registration with password mismatch fails", %{conn: conn} do diff --git a/apps/yggdrasil_web/test/controllers/session_controller_test.exs b/apps/yggdrasil_web/test/controllers/session_controller_test.exs index 9606b8f..9ff5084 100644 --- a/apps/yggdrasil_web/test/controllers/session_controller_test.exs +++ b/apps/yggdrasil_web/test/controllers/session_controller_test.exs @@ -80,7 +80,11 @@ defmodule YggdrasilWeb.SessionControllerTest do # user above contains some extra keys from the creation # so need to test against a fresh db record. - assert Guardian.serializer.from_token(claims["sub"]) == {:ok, Repo.get(User, user.id)} + user = User + |> Repo.get!(user.id) + |> User.load_permissions + + assert Guardian.serializer.from_token(claims["sub"]) == {:ok, user} end test "user login correct username but bad password returns error", %{conn: conn} do diff --git a/apps/yggdrasil_web/test/ensure_permission_test.exs b/apps/yggdrasil_web/test/ensure_permission_test.exs new file mode 100644 index 0000000..b954c03 --- /dev/null +++ b/apps/yggdrasil_web/test/ensure_permission_test.exs @@ -0,0 +1,104 @@ +defmodule YggdrasilWeb.EnsurePermissionTest do + use YggdrasilWeb.ConnCase + + alias YggdrasilWeb.EnsurePermission + alias Yggdrasil.{Repo, User, Role, RolePermission, Resource, Permission} + + @user %{username: "tester", + password: "password", + password_confirmation: "password"} + + @test_perms [:foo_perm, :bar_perm, :baz_perm] + @test_resources [:foo_res, :bar_res, :baz_res] + @test_roles [ + %{ + name: :foo_role, + resources: [ + %{ + name: :foo_res, + perms: [:foo_perm, :bar_perm] + }, + %{ + name: :bar_res, + perms: [:bar_perm, :baz_perm] + } + ] + }, + %{ + name: :bar_role, + resources: [ + %{ + name: :foo_res, + perms: [:baz_perm] + }, + %{ + name: :bar_res, + perms: [:foo_perm, :baz_perm] + }, + %{ + name: :baz_res, + perms: [:foo_perm, :bar_perm, :baz_perm] + } + ] + } + ] + + setup %{conn: conn} do + Enum.each @test_perms, fn p -> + Repo.insert! %Permission{name: Atom.to_string(p)} + end + + Enum.each @test_resources, fn r -> + Repo.insert! %Resource{name: Atom.to_string(r)} + end + + Enum.each @test_roles, fn tr -> + role = Repo.insert! %Role{name: Atom.to_string(tr.name)} + + Enum.each tr.resources, fn trs -> + Enum.each trs.perms, fn trpm -> + Repo.insert! %RolePermission{ + role_id: role.id, + resource: Atom.to_string(trs.name), + permission: Atom.to_string(trpm) + } + end + end + end + + user = %User{} + |> User.create_changeset(@user) + |> Repo.insert!() + + Enum.each @test_roles, fn r -> + User.assign_role(user, Atom.to_string(r.name)) + end + + {:ok, token, _claims} = Guardian.encode_and_sign user, :token + + # setup connection, this is boiler plate + conn = conn + |> put_req_header("authorization", token) + |> bypass_through(YggdrasilWeb.Router, [:guardian]) + |> get("/") # bypass_through skips router matching, so this is fine + + {:ok, %{conn: conn, user: %{model: user, token: token}}} + end + + test "EnsurePermission.call/2 doesn't halt connection with correct permissions", ctx do + conn = ctx.conn + |> EnsurePermission.call(foo_res: [:foo_perm]) + |> send_resp(:no_content, "") + + refute conn.halted + assert response(conn, :no_content) + end + + test "EnsurePermission.call/2 halts connection without correct permissions and issues a 401", ctx do + conn = ctx.conn + |> EnsurePermission.call(foo_res: [:foo_perm, :bar_bar]) + + assert conn.halted + assert response(conn, :unauthorized) + end +end diff --git a/apps/yggdrasil_web/web/controllers/character_controller.ex b/apps/yggdrasil_web/web/controllers/character_controller.ex index 03c00af..f582c35 100644 --- a/apps/yggdrasil_web/web/controllers/character_controller.ex +++ b/apps/yggdrasil_web/web/controllers/character_controller.ex @@ -2,25 +2,41 @@ defmodule YggdrasilWeb.CharacterController do use YggdrasilWeb.Web, :controller use Guardian.Phoenix.Controller - alias Yggdrasil.Character + alias Yggdrasil.{User, Character} + alias YggdrasilWeb.EnsurePermission + + plug EnsurePermission, [character: [:read]] when action in [:index, :show] + plug EnsurePermission, [character: [:write]] when action in [:create, :delete] def index(conn, _params, user, _claims) do - user_id = user.id - chars = Repo.all from c in Character, - where: c.user_id == ^user_id, - select: c + query = if has_all?(user) do + Character + else + from c in Character, + where: c.user_id == ^user.id, + select: c + end + + chars = Repo.all query render conn, :show, data: chars end def show(conn, %{"char_id" => char_id}, user, _claims) do - user_id = user.id # returns nil if not found # need to sort out what we want here. - char = Repo.one from c in Character, - where: c.id == ^char_id and c.user_id == ^user_id, - select: c + query = if has_all?(user) do + from c in Character, + where: c.id == ^char_id, + select: c + else + from c in Character, + where: c.id == ^char_id and c.user_id == ^user.id, + select: c + end + + char = Repo.one query render conn, :show, data: char end @@ -38,13 +54,19 @@ defmodule YggdrasilWeb.CharacterController do end def delete(conn, %{"char_id" => char_id}, user, _claims) do - user_id = user.id + query = if has_all?(user) do + from c in Character, + where: c.id == ^char_id, + select: c + else + from c in Character, + where: c.id == ^char_id and c.user_id == ^user.id, + select: c + end # returns nil if not found # need to sort out what we want here. - char = Repo.one from c in Character, - where: c.id == ^char_id and c.user_id == ^user_id, - select: c + char = Repo.one query do_delete conn, char end @@ -64,4 +86,8 @@ defmodule YggdrasilWeb.CharacterController do render conn, :errors, data: err_changeset end end + + defp has_all?(user) do + User.is_granted?(user, character: [:all]) + end end diff --git a/apps/yggdrasil_web/web/controllers/game_controller.ex b/apps/yggdrasil_web/web/controllers/game_controller.ex index 4a7ec75..624e3ed 100644 --- a/apps/yggdrasil_web/web/controllers/game_controller.ex +++ b/apps/yggdrasil_web/web/controllers/game_controller.ex @@ -2,6 +2,9 @@ defmodule YggdrasilWeb.GameController do use YggdrasilWeb.Web, :controller alias Yggdrasil.Game + alias YggdrasilWeb.EnsurePermission + + plug EnsurePermission, [character: [:read]] when action in [:index] def index(conn, _params) do games = Repo.all Game diff --git a/apps/yggdrasil_web/web/controllers/registration_controller.ex b/apps/yggdrasil_web/web/controllers/registration_controller.ex index 26791b1..c4df11b 100644 --- a/apps/yggdrasil_web/web/controllers/registration_controller.ex +++ b/apps/yggdrasil_web/web/controllers/registration_controller.ex @@ -11,15 +11,14 @@ defmodule YggdrasilWeb.RegistrationController do end def create(conn, %{"data" => %{"attributes" => attributes}}) do - changeset = User.create_changeset(%User{}, attributes) - case Repo.insert(changeset) do + case User.create_with_default_role(attributes) do {:ok, new_user} -> conn |> Guardian.Plug.api_sign_in(new_user) |> render(:show, data: new_user) - {:error, err_changeset} -> - render conn, :errors, data: err_changeset + {:error, err} -> + render conn, :errors, data: err end end diff --git a/mix.lock b/mix.lock index fb0ce92..3196994 100644 --- a/mix.lock +++ b/mix.lock @@ -1,13 +1,14 @@ %{"base64url": {:hex, :base64url, "0.0.1"}, "certifi": {:hex, :certifi, "0.3.0"}, + "combination": {:hex, :combination, "0.0.2"}, "comeonin": {:hex, :comeonin, "2.1.1"}, "connection": {:hex, :connection, "1.0.2"}, "cors_plug": {:hex, :cors_plug, "0.1.4"}, "cowboy": {:hex, :cowboy, "1.0.4"}, "cowlib": {:hex, :cowlib, "1.0.2"}, - "db_connection": {:hex, :db_connection, "0.2.3"}, + "db_connection": {:hex, :db_connection, "0.2.4"}, "decimal": {:hex, :decimal, "1.1.1"}, - "ecto": {:hex, :ecto, "1.1.3"}, + "ecto": {:hex, :ecto, "1.1.4"}, "excoveralls": {:hex, :excoveralls, "0.5.1"}, "exjsx": {:hex, :exjsx, "3.2.0"}, "fs": {:hex, :fs, "0.9.2"}, @@ -27,7 +28,7 @@ "plug": {:hex, :plug, "1.1.1"}, "poison": {:hex, :poison, "1.5.2"}, "poolboy": {:hex, :poolboy, "1.5.1"}, - "postgrex": {:hex, :postgrex, "0.11.0"}, + "postgrex": {:hex, :postgrex, "0.11.1"}, "ranch": {:hex, :ranch, "1.2.1"}, "ssl_verify_hostname": {:hex, :ssl_verify_hostname, "1.0.5"}, "uuid": {:hex, :uuid, "1.1.3"}}