From 30b012b099769e29fc00d2408a2e92840764ac9f Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Tue, 2 May 2017 23:22:46 -0700 Subject: [PATCH 01/10] Rename migration files to ensure being run first --- .../{1_create_user.exs => 00000000000001_create_user.exs} | 0 .../{2_create_group.exs => 00000000000002_create_group.exs} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename priv/repo/migrations/{1_create_user.exs => 00000000000001_create_user.exs} (100%) rename priv/repo/migrations/{2_create_group.exs => 00000000000002_create_group.exs} (100%) diff --git a/priv/repo/migrations/1_create_user.exs b/priv/repo/migrations/00000000000001_create_user.exs similarity index 100% rename from priv/repo/migrations/1_create_user.exs rename to priv/repo/migrations/00000000000001_create_user.exs diff --git a/priv/repo/migrations/2_create_group.exs b/priv/repo/migrations/00000000000002_create_group.exs similarity index 100% rename from priv/repo/migrations/2_create_group.exs rename to priv/repo/migrations/00000000000002_create_group.exs From a6ab00942a71f185881f1c1ad58be97e794e746b Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Tue, 2 May 2017 23:28:32 -0700 Subject: [PATCH 02/10] Remove `created_at` in favor of default `inserted_at` --- priv/repo/migrations/00000000000001_create_user.exs | 1 - web/models/user.ex | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/priv/repo/migrations/00000000000001_create_user.exs b/priv/repo/migrations/00000000000001_create_user.exs index feb3979..af64de8 100644 --- a/priv/repo/migrations/00000000000001_create_user.exs +++ b/priv/repo/migrations/00000000000001_create_user.exs @@ -9,7 +9,6 @@ defmodule Org.Repo.Migrations.CreateUser do add :login, :string add :company, :string add :location, :string - add :created_at, :string add :has_applied, :boolean, default: false, null: false add :role, :string add :comments, :string diff --git a/web/models/user.ex b/web/models/user.ex index 454e383..8899705 100644 --- a/web/models/user.ex +++ b/web/models/user.ex @@ -6,7 +6,6 @@ defmodule Org.User do field :bio, :string field :blog, :string field :company, :string - field :created_at, :string field :email, :string field :followers, :integer field :following, :integer @@ -20,7 +19,7 @@ defmodule Org.User do field :public_repos, :integer field :role, :string, default: "user" field :type, :string - field :has_applied, :boolean, default: false + field :has_applied, :boolean, default: false, null: false field :comments, :string has_many :groups, Org.Group @@ -34,12 +33,12 @@ defmodule Org.User do """ def changeset(struct, params \\ %{}) do struct - |> cast(params, [:avatar, :bio, :blog, :company, :created_at, :email, + |> cast(params, [:avatar, :bio, :blog, :company, :email, :followers, :following, :hireable, :html_url, :github_id, :location, :login, :name, :public_gists, :public_repos, :role, :type, :has_applied, :comments ]) - |> validate_required([:avatar, :blog, :company, :created_at, :email, + |> validate_required([:avatar, :blog, :company, :email, :followers, :following, :html_url, :github_id, :location, :login, :name, :public_gists, :public_repos, :role, :type, :has_applied ]) From c4b3374106ddd1e59e9886d2a75dfceec2112105 Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Tue, 2 May 2017 23:29:04 -0700 Subject: [PATCH 03/10] WIP: Attempt to set up simple test for #index action --- test/controllers/user_controller_test.exs | 5 ++-- test/support/conn_case.ex | 2 ++ test/support/test_helpers.ex | 28 +++++++++++++++++++++++ web/controllers/user_controller.ex | 4 +--- web/router.ex | 7 +++++- web/views/user_view.ex | 12 ++++++++++ 6 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 test/support/test_helpers.ex diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index 67c0e43..ce05d4c 100644 --- a/test/controllers/user_controller_test.exs +++ b/test/controllers/user_controller_test.exs @@ -1,13 +1,14 @@ defmodule Org.UserControllerTest do use Org.ConnCase - alias Org.User + @valid_attrs %{email: "some content", name: "some content"} @invalid_attrs %{} test "lists all entries on index", %{conn: conn} do + user = insert_user(name: "Test User") conn = get conn, user_path(conn, :index) - assert html_response(conn, 200) =~ "Listing users" + assert json_response(conn, 200) =~ %{name: user.name} end test "renders form for new resources", %{conn: conn} do diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 09b1ede..508e8bc 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -25,6 +25,8 @@ defmodule Org.ConnCase do import Ecto.Changeset import Ecto.Query + import Org.TestHelpers + import Org.Router.Helpers # The default endpoint for testing diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex new file mode 100644 index 0000000..0a7d1aa --- /dev/null +++ b/test/support/test_helpers.ex @@ -0,0 +1,28 @@ +defmodule Org.TestHelpers do + require IEx + alias Org.Repo + alias Org.User + + def insert_user(attrs \\ %{}) do + changes = Map.merge(%{ + avatar: "String", + blog: "My Blog", + company: "My Company", + email: "email@email.com", + followers: 1, + following: 1, + github_id: 1, + html_url: "https://github.com", + location: "Tempe", + login: "String", + public_gists: 1, + public_repos: 1, + type: "user" + }, Enum.into(attrs, %{})) + IEx.pry + + %User{} + |> User.changeset(changes) + |> Repo.insert!() + end +end diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index ccca9ae..110e47d 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -3,11 +3,9 @@ defmodule Org.UserController do alias Org.User - plug :scrub_params, "user" when action in [:create, :update, :apply] - def index(conn, _params) do users = Repo.all(from u in User, where: u.role != "user") - render(conn, "index.html", users: users) + render(conn, "index.json", users: users) end def create(conn, %{"user" => user_params}) do diff --git a/web/router.ex b/web/router.ex index 78d604e..ecf9175 100644 --- a/web/router.ex +++ b/web/router.ex @@ -41,7 +41,7 @@ defmodule Org.Router do scope "/", Org.Admin do pipe_through [:browser, :auth_admin] - resources "/users", UserController, except: [:show, :new, :update] + # resources "/users", UserController, except: [:show, :new, :update] resources "/groups", GroupController, except: [:index, :show] end @@ -59,6 +59,11 @@ defmodule Org.Router do get "/apply", PageController, :apply put "/apply/:id", UserController, :apply get "/thanks", PageController, :thanks + end + + scope "/api", Org.Api do + pipe_through :api + resources "/users", UserController, only: [:index, :show, :update] end diff --git a/web/views/user_view.ex b/web/views/user_view.ex index 70fa6e8..d6d6cf0 100644 --- a/web/views/user_view.ex +++ b/web/views/user_view.ex @@ -1,3 +1,15 @@ defmodule Org.UserView do use Org.Web, :view + + def render("index.json", %{users: users}) do + %{ + users: Enum.map(users, &(user_json(&1))) + } + end + + def user_json(user) do + %{ + name: user.name + } + end end From c50d04ba01b0b0349f6f96ed50d0ad076ff04781 Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Sun, 14 May 2017 21:51:52 -0700 Subject: [PATCH 04/10] Revert `created_at` change in user model --- priv/repo/migrations/00000000000001_create_user.exs | 1 + test/support/test_helpers.ex | 3 +-- web/models/user.ex | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/priv/repo/migrations/00000000000001_create_user.exs b/priv/repo/migrations/00000000000001_create_user.exs index af64de8..1b72c42 100644 --- a/priv/repo/migrations/00000000000001_create_user.exs +++ b/priv/repo/migrations/00000000000001_create_user.exs @@ -8,6 +8,7 @@ defmodule Org.Repo.Migrations.CreateUser do add :name, :string add :login, :string add :company, :string + add :created_at, :string add :location, :string add :has_applied, :boolean, default: false, null: false add :role, :string diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 0a7d1aa..65eeecd 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -1,5 +1,4 @@ defmodule Org.TestHelpers do - require IEx alias Org.Repo alias Org.User @@ -8,6 +7,7 @@ defmodule Org.TestHelpers do avatar: "String", blog: "My Blog", company: "My Company", + created_at: "100", email: "email@email.com", followers: 1, following: 1, @@ -19,7 +19,6 @@ defmodule Org.TestHelpers do public_repos: 1, type: "user" }, Enum.into(attrs, %{})) - IEx.pry %User{} |> User.changeset(changes) diff --git a/web/models/user.ex b/web/models/user.ex index 8899705..08100d1 100644 --- a/web/models/user.ex +++ b/web/models/user.ex @@ -6,6 +6,8 @@ defmodule Org.User do field :bio, :string field :blog, :string field :company, :string + # This is the GitHub created_at date + field :created_at, :string field :email, :string field :followers, :integer field :following, :integer @@ -33,12 +35,12 @@ defmodule Org.User do """ def changeset(struct, params \\ %{}) do struct - |> cast(params, [:avatar, :bio, :blog, :company, :email, + |> cast(params, [:avatar, :bio, :blog, :company, :created_at, :email, :followers, :following, :hireable, :html_url, :github_id, :location, :login, :name, :public_gists, :public_repos, :role, :type, :has_applied, :comments ]) - |> validate_required([:avatar, :blog, :company, :email, + |> validate_required([:avatar, :blog, :company, :created_at, :email, :followers, :following, :html_url, :github_id, :location, :login, :name, :public_gists, :public_repos, :role, :type, :has_applied ]) From b45f32c9db1c6685f8a064a7cfa9ab2337703fc9 Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Sun, 21 May 2017 11:49:50 -0700 Subject: [PATCH 05/10] Write test coverage for JSON #index action on UserController --- config/config.exs | 5 ---- mix.exs | 1 + mix.lock | 1 + test/controllers/user_controller_test.exs | 7 +++-- test/support/conn_case.ex | 3 +- test/support/factory.ex | 10 +++++++ test/support/model_case.ex | 1 + test/support/test_helpers.ex | 29 ++++--------------- test/test_helper.exs | 1 + test/views/api/user_view_test.exs | 35 +++++++++++++++++++++++ web/controllers/user_controller.ex | 2 +- web/models/user.ex | 4 +-- web/router.ex | 2 +- web/views/{ => api}/user_view.ex | 6 ++-- web/web.ex | 3 -- 15 files changed, 68 insertions(+), 42 deletions(-) create mode 100644 test/support/factory.ex create mode 100644 test/views/api/user_view_test.exs rename web/views/{ => api}/user_view.ex (64%) diff --git a/config/config.exs b/config/config.exs index 1c8e46b..7136ec0 100644 --- a/config/config.exs +++ b/config/config.exs @@ -27,11 +27,6 @@ config :logger, :console, # of this file so it overrides the configuration defined above. import_config "#{Mix.env}.exs" -# Configure phoenix generators -config :phoenix, :generators, - migration: false, - binary_id: true - config :org, GitHub, client_id: System.get_env("GITHUB_CLIENT_ID"), client_secret: System.get_env("GITHUB_CLIENT_SECRET"), diff --git a/mix.exs b/mix.exs index 1c20316..1c336c5 100644 --- a/mix.exs +++ b/mix.exs @@ -39,6 +39,7 @@ defmodule Org.Mixfile do {:gettext, "~> 0.11"}, {:oauth2, "~> 0.5"}, {:cowboy, "~> 1.0"}, + {:ex_machina, "~> 2.0", only: [:test]}, {:credo, "~> 0.7", only: [:dev, :test]}] end diff --git a/mix.lock b/mix.lock index 145ab57..3fea981 100644 --- a/mix.lock +++ b/mix.lock @@ -7,6 +7,7 @@ "db_connection": {:hex, :db_connection, "1.1.2", "2865c2a4bae0714e2213a0ce60a1b12d76a6efba0c51fbda59c9ab8d1accc7a8", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, "decimal": {:hex, :decimal, "1.3.1", "157b3cedb2bfcb5359372a7766dd7a41091ad34578296e951f58a946fcab49c6", [:mix], []}, "ecto": {:hex, :ecto, "2.1.4", "d1ba932813ec0e0d9db481ef2c17777f1cefb11fc90fa7c142ff354972dfba7e", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]}, + "ex_machina": {:hex, :ex_machina, "2.0.0", "ec284c6f57233729cea9319e083f66e613e82549f78eccdb2059aeba5d0df9f3", [:mix], [{:ecto, "~> 2.1", [hex: :ecto, optional: true]}]}, "fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], []}, "gettext": {:hex, :gettext, "0.13.1", "5e0daf4e7636d771c4c71ad5f3f53ba09a9ae5c250e1ab9c42ba9edccc476263", [:mix], []}, "hackney": {:hex, :hackney, "1.5.7", "f3809c0a17a3e523a865c65f6552b526f6b799acd0389803a05f88573ea26162", [:rebar3], [{:certifi, "0.4.0", [hex: :certifi, optional: false]}, {:idna, "1.2.0", [hex: :idna, optional: false]}, {:metrics, "1.0.1", [hex: :metrics, optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, optional: false]}, {:ssl_verify_fun, "1.1.0", [hex: :ssl_verify_fun, optional: false]}]}, diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index ce05d4c..99e8233 100644 --- a/test/controllers/user_controller_test.exs +++ b/test/controllers/user_controller_test.exs @@ -1,14 +1,15 @@ defmodule Org.UserControllerTest do use Org.ConnCase + alias Org.User @valid_attrs %{email: "some content", name: "some content"} @invalid_attrs %{} - test "lists all entries on index", %{conn: conn} do - user = insert_user(name: "Test User") + test "lists all non-user role users", %{conn: conn} do + [member, user] = [insert(:user), insert(:user, role: "user")] conn = get conn, user_path(conn, :index) - assert json_response(conn, 200) =~ %{name: user.name} + assert json_response(conn, 200) == render_json(Org.Api.UserView, "index.json", users: [member]) end test "renders form for new resources", %{conn: conn} do diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 508e8bc..eb5b5a5 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -25,6 +25,7 @@ defmodule Org.ConnCase do import Ecto.Changeset import Ecto.Query + import Org.Factory import Org.TestHelpers import Org.Router.Helpers @@ -38,7 +39,7 @@ defmodule Org.ConnCase do :ok = Ecto.Adapters.SQL.Sandbox.checkout(Org.Repo) unless tags[:async] do - Ecto.Adapters.SQL.Sandbox.mode(Org.Repo, {:shared, self()}) + Ecto.Adapters.SQL.Sandbox.mode(Org.Repo, {:shared, self()}) end {:ok, conn: Phoenix.ConnTest.build_conn()} diff --git a/test/support/factory.ex b/test/support/factory.ex new file mode 100644 index 0000000..ca0b93c --- /dev/null +++ b/test/support/factory.ex @@ -0,0 +1,10 @@ +defmodule Org.Factory do + use ExMachina.Ecto, repo: Org.Repo + + def user_factory do + %Org.User{ + name: "John Doe", + role: "member" + } + end +end diff --git a/test/support/model_case.ex b/test/support/model_case.ex index 7a6de43..2c1f4b1 100644 --- a/test/support/model_case.ex +++ b/test/support/model_case.ex @@ -22,6 +22,7 @@ defmodule Org.ModelCase do import Ecto.Changeset import Ecto.Query import Org.ModelCase + import Org.Factory end end diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 65eeecd..2855ca9 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -1,27 +1,8 @@ defmodule Org.TestHelpers do - alias Org.Repo - alias Org.User - - def insert_user(attrs \\ %{}) do - changes = Map.merge(%{ - avatar: "String", - blog: "My Blog", - company: "My Company", - created_at: "100", - email: "email@email.com", - followers: 1, - following: 1, - github_id: 1, - html_url: "https://github.com", - location: "Tempe", - login: "String", - public_gists: 1, - public_repos: 1, - type: "user" - }, Enum.into(attrs, %{})) - - %User{} - |> User.changeset(changes) - |> Repo.insert!() + def render_json(view, template, assigns) do + template + |> view.render(assigns) + |> Poison.encode! + |> Poison.decode! end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 7841bed..02691be 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -2,3 +2,4 @@ ExUnit.start Ecto.Adapters.SQL.Sandbox.mode(Org.Repo, :manual) +{:ok, _} = Application.ensure_all_started(:ex_machina) diff --git a/test/views/api/user_view_test.exs b/test/views/api/user_view_test.exs new file mode 100644 index 0000000..eccccca --- /dev/null +++ b/test/views/api/user_view_test.exs @@ -0,0 +1,35 @@ +defmodule Org.Api.UserViewTest do + use Org.ModelCase + alias Org.Api.UserView + + test "user_json" do + user = insert(:user) + rendered_user = UserView.user_json(user) + + assert rendered_user == %{ + name: user.name, + email: user.email, + role: user.role + } + end + + test "index.json" do + user = insert(:user) + + rendered_users = UserView.render("index.json", %{users: [user]}) + + assert rendered_users == %{ + users: [UserView.user_json(user)] + } + end + + # test "show.json" do + # user = insert(:user) + + # rendered_user = UserView.render("show.json", %{user: user}) + + # assert rendered_user == %{ + # user: userView.user_json(user) + # } + # end +end diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index 110e47d..d26eb76 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -1,4 +1,4 @@ -defmodule Org.UserController do +defmodule Org.Api.UserController do use Org.Web, :controller alias Org.User diff --git a/web/models/user.ex b/web/models/user.ex index 08100d1..0f06e3b 100644 --- a/web/models/user.ex +++ b/web/models/user.ex @@ -24,8 +24,8 @@ defmodule Org.User do field :has_applied, :boolean, default: false, null: false field :comments, :string - has_many :groups, Org.Group - embeds_one :languages, Org.Language + # has_many :groups, Org.Group + # embeds_one :languages, Org.Language timestamps() end diff --git a/web/router.ex b/web/router.ex index ecf9175..8309db3 100644 --- a/web/router.ex +++ b/web/router.ex @@ -49,7 +49,7 @@ defmodule Org.Router do scope "/", Org do pipe_through [:browser, :auth_member] - resources "/users", UserController, only: [:index, :show] + # resources "/users", UserController, only: [:index, :show] end # Scope for authenticated-only routes (user is logged in) diff --git a/web/views/user_view.ex b/web/views/api/user_view.ex similarity index 64% rename from web/views/user_view.ex rename to web/views/api/user_view.ex index d6d6cf0..a8989c8 100644 --- a/web/views/user_view.ex +++ b/web/views/api/user_view.ex @@ -1,4 +1,4 @@ -defmodule Org.UserView do +defmodule Org.Api.UserView do use Org.Web, :view def render("index.json", %{users: users}) do @@ -9,7 +9,9 @@ defmodule Org.UserView do def user_json(user) do %{ - name: user.name + name: user.name, + email: user.email, + role: user.role } end end diff --git a/web/web.ex b/web/web.ex index 1c5cb18..be62034 100644 --- a/web/web.ex +++ b/web/web.ex @@ -23,9 +23,6 @@ defmodule Org.Web do import Ecto import Ecto.Changeset import Ecto.Query - - @primary_key {:id, :binary_id, autogenerate: true} - @foreign_key_type :binary_id end end From 6aae0d1023ffa788bf6d67ebed8a657ea336ddce Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Sun, 21 May 2017 12:19:24 -0700 Subject: [PATCH 06/10] Setup UserController #show to return JSON --- test/controllers/user_controller_test.exs | 58 ++++++++--------------- test/views/api/user_view_test.exs | 18 +++---- web/controllers/user_controller.ex | 6 +-- web/views/api/user_view.ex | 8 ++-- 4 files changed, 34 insertions(+), 56 deletions(-) diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index 99e8233..33941fb 100644 --- a/test/controllers/user_controller_test.exs +++ b/test/controllers/user_controller_test.exs @@ -2,14 +2,30 @@ defmodule Org.UserControllerTest do use Org.ConnCase alias Org.User + alias Org.Api.UserView - @valid_attrs %{email: "some content", name: "some content"} @invalid_attrs %{} - test "lists all non-user role users", %{conn: conn} do - [member, user] = [insert(:user), insert(:user, role: "user")] - conn = get conn, user_path(conn, :index) - assert json_response(conn, 200) == render_json(Org.Api.UserView, "index.json", users: [member]) + describe "#index" do + test "lists all non-user role users", %{conn: conn} do + [member, _] = [insert(:user), insert(:user, role: "user")] + conn = get conn, user_path(conn, :index) + assert json_response(conn, 200) == render_json(UserView, "index.json", users: [member]) + end + end + + describe "#show" do + test "renders a user", %{conn: conn} do + user = insert(:user) + conn = get conn, user_path(conn, :show, user) + assert json_response(conn, 200) == render_json(UserView, "show.json", user: user) + end + + test "renders user not found when id is nonexistent", %{conn: conn} do + assert_error_sent 404, fn -> + get conn, user_path(conn, :show, 1234) + end + end end test "renders form for new resources", %{conn: conn} do @@ -17,52 +33,20 @@ defmodule Org.UserControllerTest do assert html_response(conn, 200) =~ "New user" end - test "creates resource and redirects when data is valid", %{conn: conn} do - conn = post conn, user_path(conn, :create), user: @valid_attrs - assert redirected_to(conn) == user_path(conn, :index) - assert Repo.get_by(User, @valid_attrs) - end - test "does not create resource and renders errors when data is invalid", %{conn: conn} do conn = post conn, user_path(conn, :create), user: @invalid_attrs assert html_response(conn, 200) =~ "New user" end - test "shows chosen resource", %{conn: conn} do - user = Repo.insert! %User{} - conn = get conn, user_path(conn, :show, user) - assert html_response(conn, 200) =~ "Show user" - end - - test "renders page not found when id is nonexistent", %{conn: conn} do - assert_error_sent 404, fn -> - get conn, user_path(conn, :show, "11111111-1111-1111-1111-111111111111") - end - end - test "renders form for editing chosen resource", %{conn: conn} do user = Repo.insert! %User{} conn = get conn, user_path(conn, :edit, user) assert html_response(conn, 200) =~ "Edit user" end - test "updates chosen resource and redirects when data is valid", %{conn: conn} do - user = Repo.insert! %User{} - conn = put conn, user_path(conn, :update, user), user: @valid_attrs - assert redirected_to(conn) == user_path(conn, :show, user) - assert Repo.get_by(User, @valid_attrs) - end - test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do user = Repo.insert! %User{} conn = put conn, user_path(conn, :update, user), user: @invalid_attrs assert html_response(conn, 200) =~ "Edit user" end - - test "deletes chosen resource", %{conn: conn} do - user = Repo.insert! %User{} - conn = delete conn, user_path(conn, :delete, user) - assert redirected_to(conn) == user_path(conn, :index) - refute Repo.get(User, user.id) - end end diff --git a/test/views/api/user_view_test.exs b/test/views/api/user_view_test.exs index eccccca..de34c64 100644 --- a/test/views/api/user_view_test.exs +++ b/test/views/api/user_view_test.exs @@ -15,21 +15,15 @@ defmodule Org.Api.UserViewTest do test "index.json" do user = insert(:user) - rendered_users = UserView.render("index.json", %{users: [user]}) - assert rendered_users == %{ - users: [UserView.user_json(user)] - } + assert rendered_users == %{users: [UserView.user_json(user)]} end - # test "show.json" do - # user = insert(:user) - - # rendered_user = UserView.render("show.json", %{user: user}) + test "show.json" do + user = insert(:user) + rendered_user = UserView.render("show.json", %{user: user}) - # assert rendered_user == %{ - # user: userView.user_json(user) - # } - # end + assert rendered_user == %{user: UserView.user_json(user)} + end end diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index d26eb76..4cd6cff 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -22,10 +22,8 @@ defmodule Org.Api.UserController do end def show(conn, %{"id" => id}) do - user = User - |> Repo.get!(id) - |> Repo.preload(:groups) - render(conn, "show.html", user: user) + user = Repo.get!(User, id) + render(conn, "show.json", user: user) end def edit(conn, %{"id" => id}) do diff --git a/web/views/api/user_view.ex b/web/views/api/user_view.ex index a8989c8..0f812b6 100644 --- a/web/views/api/user_view.ex +++ b/web/views/api/user_view.ex @@ -2,9 +2,11 @@ defmodule Org.Api.UserView do use Org.Web, :view def render("index.json", %{users: users}) do - %{ - users: Enum.map(users, &(user_json(&1))) - } + %{users: Enum.map(users, &(user_json(&1)))} + end + + def render("show.json", %{user: user}) do + %{user: user_json(user)} end def user_json(user) do From 04e540795004cbe8827a20a724e71919feb227d3 Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Sun, 21 May 2017 16:27:35 -0700 Subject: [PATCH 07/10] Update Auth plug to simply use function plugs --- .../admin/group_controller_test.exs | 110 +++++++++--------- .../admin/user_controller_test.exs | 110 +++++++++--------- test/controllers/group_controller_test.exs | 110 +++++++++--------- test/controllers/page_controller_test.exs | 14 +-- test/controllers/user_controller_test.exs | 59 +++++----- test/models/admin/group_test.exs | 28 ++--- test/models/admin/user_test.exs | 28 ++--- test/plugs/auth_test.exs | 92 +++++++++++++++ test/support/conn_case.ex | 1 - test/support/test_helpers.ex | 2 +- web/controllers/user_controller.ex | 35 +----- web/models/user.ex | 6 + web/plugs/auth.ex | 57 +++++++++ web/plugs/authenticated.ex | 34 ------ web/router.ex | 47 ++------ web/views/error_view.ex | 8 ++ web/web.ex | 2 + 17 files changed, 410 insertions(+), 333 deletions(-) create mode 100644 test/plugs/auth_test.exs create mode 100644 web/plugs/auth.ex delete mode 100644 web/plugs/authenticated.ex diff --git a/test/controllers/admin/group_controller_test.exs b/test/controllers/admin/group_controller_test.exs index ab9db47..0e5b43b 100644 --- a/test/controllers/admin/group_controller_test.exs +++ b/test/controllers/admin/group_controller_test.exs @@ -1,66 +1,66 @@ -defmodule Org.Admin.GroupControllerTest do - use Org.ConnCase +# defmodule Org.Admin.GroupControllerTest do +# use Org.ConnCase - alias Org.Group - @valid_attrs %{description: "some content", title: "some content", url: "some content"} - @invalid_attrs %{} +# alias Org.Group +# @valid_attrs %{description: "some content", title: "some content", url: "some content"} +# @invalid_attrs %{} - test "lists all entries on index", %{conn: conn} do - conn = get conn, group_path(conn, :index) - assert html_response(conn, 200) =~ "Listing groups" - end +# test "lists all entries on index", %{conn: conn} do +# conn = get conn, group_path(conn, :index) +# assert html_response(conn, 200) =~ "Listing groups" +# end - test "renders form for new resources", %{conn: conn} do - conn = get conn, group_path(conn, :new) - assert html_response(conn, 200) =~ "New group" - end +# test "renders form for new resources", %{conn: conn} do +# conn = get conn, group_path(conn, :new) +# assert html_response(conn, 200) =~ "New group" +# end - test "creates resource and redirects when data is valid", %{conn: conn} do - conn = post conn, group_path(conn, :create), group: @valid_attrs - assert redirected_to(conn) == group_path(conn, :index) - assert Repo.get_by(Group, @valid_attrs) - end +# test "creates resource and redirects when data is valid", %{conn: conn} do +# conn = post conn, group_path(conn, :create), group: @valid_attrs +# assert redirected_to(conn) == group_path(conn, :index) +# assert Repo.get_by(Group, @valid_attrs) +# end - test "does not create resource and renders errors when data is invalid", %{conn: conn} do - conn = post conn, group_path(conn, :create), group: @invalid_attrs - assert html_response(conn, 200) =~ "New group" - end +# test "does not create resource and renders errors when data is invalid", %{conn: conn} do +# conn = post conn, group_path(conn, :create), group: @invalid_attrs +# assert html_response(conn, 200) =~ "New group" +# end - test "shows chosen resource", %{conn: conn} do - group = Repo.insert! %Group{} - conn = get conn, group_path(conn, :show, group) - assert html_response(conn, 200) =~ "Show group" - end +# test "shows chosen resource", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = get conn, group_path(conn, :show, group) +# assert html_response(conn, 200) =~ "Show group" +# end - test "renders page not found when id is nonexistent", %{conn: conn} do - assert_error_sent 404, fn -> - get conn, group_path(conn, :show, "11111111-1111-1111-1111-111111111111") - end - end +# test "renders page not found when id is nonexistent", %{conn: conn} do +# assert_error_sent 404, fn -> +# get conn, group_path(conn, :show, "11111111-1111-1111-1111-111111111111") +# end +# end - test "renders form for editing chosen resource", %{conn: conn} do - group = Repo.insert! %Group{} - conn = get conn, group_path(conn, :edit, group) - assert html_response(conn, 200) =~ "Edit group" - end +# test "renders form for editing chosen resource", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = get conn, group_path(conn, :edit, group) +# assert html_response(conn, 200) =~ "Edit group" +# end - test "updates chosen resource and redirects when data is valid", %{conn: conn} do - group = Repo.insert! %Group{} - conn = put conn, group_path(conn, :update, group), group: @valid_attrs - assert redirected_to(conn) == group_path(conn, :show, group) - assert Repo.get_by(Group, @valid_attrs) - end +# test "updates chosen resource and redirects when data is valid", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = put conn, group_path(conn, :update, group), group: @valid_attrs +# assert redirected_to(conn) == group_path(conn, :show, group) +# assert Repo.get_by(Group, @valid_attrs) +# end - test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do - group = Repo.insert! %Group{} - conn = put conn, group_path(conn, :update, group), group: @invalid_attrs - assert html_response(conn, 200) =~ "Edit group" - end +# test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = put conn, group_path(conn, :update, group), group: @invalid_attrs +# assert html_response(conn, 200) =~ "Edit group" +# end - test "deletes chosen resource", %{conn: conn} do - group = Repo.insert! %Group{} - conn = delete conn, group_path(conn, :delete, group) - assert redirected_to(conn) == group_path(conn, :index) - refute Repo.get(Group, group.id) - end -end +# test "deletes chosen resource", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = delete conn, group_path(conn, :delete, group) +# assert redirected_to(conn) == group_path(conn, :index) +# refute Repo.get(Group, group.id) +# end +# end diff --git a/test/controllers/admin/user_controller_test.exs b/test/controllers/admin/user_controller_test.exs index 350dea4..4313b92 100644 --- a/test/controllers/admin/user_controller_test.exs +++ b/test/controllers/admin/user_controller_test.exs @@ -1,66 +1,66 @@ -defmodule Org.Admin.UserControllerTest do - use Org.ConnCase +# defmodule Org.Admin.UserControllerTest do +# use Org.ConnCase - alias Org.User - @valid_attrs %{email: "some content", name: "some content"} - @invalid_attrs %{} +# alias Org.User +# @valid_attrs %{email: "some content", name: "some content"} +# @invalid_attrs %{} - test "lists all entries on index", %{conn: conn} do - conn = get conn, user_path(conn, :index) - assert html_response(conn, 200) =~ "Listing users" - end +# test "lists all entries on index", %{conn: conn} do +# conn = get conn, user_path(conn, :index) +# assert html_response(conn, 200) =~ "Listing users" +# end - test "renders form for new resources", %{conn: conn} do - conn = get conn, user_path(conn, :new) - assert html_response(conn, 200) =~ "New user" - end +# test "renders form for new resources", %{conn: conn} do +# conn = get conn, user_path(conn, :new) +# assert html_response(conn, 200) =~ "New user" +# end - test "creates resource and redirects when data is valid", %{conn: conn} do - conn = post conn, user_path(conn, :create), user: @valid_attrs - assert redirected_to(conn) == user_path(conn, :index) - assert Repo.get_by(User, @valid_attrs) - end +# test "creates resource and redirects when data is valid", %{conn: conn} do +# conn = post conn, user_path(conn, :create), user: @valid_attrs +# assert redirected_to(conn) == user_path(conn, :index) +# assert Repo.get_by(User, @valid_attrs) +# end - test "does not create resource and renders errors when data is invalid", %{conn: conn} do - conn = post conn, user_path(conn, :create), user: @invalid_attrs - assert html_response(conn, 200) =~ "New user" - end +# test "does not create resource and renders errors when data is invalid", %{conn: conn} do +# conn = post conn, user_path(conn, :create), user: @invalid_attrs +# assert html_response(conn, 200) =~ "New user" +# end - test "shows chosen resource", %{conn: conn} do - user = Repo.insert! %User{} - conn = get conn, user_path(conn, :show, user) - assert html_response(conn, 200) =~ "Show user" - end +# test "shows chosen resource", %{conn: conn} do +# user = Repo.insert! %User{} +# conn = get conn, user_path(conn, :show, user) +# assert html_response(conn, 200) =~ "Show user" +# end - test "renders page not found when id is nonexistent", %{conn: conn} do - assert_error_sent 404, fn -> - get conn, user_path(conn, :show, "11111111-1111-1111-1111-111111111111") - end - end +# test "renders page not found when id is nonexistent", %{conn: conn} do +# assert_error_sent 404, fn -> +# get conn, user_path(conn, :show, "11111111-1111-1111-1111-111111111111") +# end +# end - test "renders form for editing chosen resource", %{conn: conn} do - user = Repo.insert! %User{} - conn = get conn, user_path(conn, :edit, user) - assert html_response(conn, 200) =~ "Edit user" - end +# test "renders form for editing chosen resource", %{conn: conn} do +# user = Repo.insert! %User{} +# conn = get conn, user_path(conn, :edit, user) +# assert html_response(conn, 200) =~ "Edit user" +# end - test "updates chosen resource and redirects when data is valid", %{conn: conn} do - user = Repo.insert! %User{} - conn = put conn, user_path(conn, :update, user), user: @valid_attrs - assert redirected_to(conn) == user_path(conn, :show, user) - assert Repo.get_by(User, @valid_attrs) - end +# test "updates chosen resource and redirects when data is valid", %{conn: conn} do +# user = Repo.insert! %User{} +# conn = put conn, user_path(conn, :update, user), user: @valid_attrs +# assert redirected_to(conn) == user_path(conn, :show, user) +# assert Repo.get_by(User, @valid_attrs) +# end - test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do - user = Repo.insert! %User{} - conn = put conn, user_path(conn, :update, user), user: @invalid_attrs - assert html_response(conn, 200) =~ "Edit user" - end +# test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do +# user = Repo.insert! %User{} +# conn = put conn, user_path(conn, :update, user), user: @invalid_attrs +# assert html_response(conn, 200) =~ "Edit user" +# end - test "deletes chosen resource", %{conn: conn} do - user = Repo.insert! %User{} - conn = delete conn, user_path(conn, :delete, user) - assert redirected_to(conn) == user_path(conn, :index) - refute Repo.get(User, user.id) - end -end +# test "deletes chosen resource", %{conn: conn} do +# user = Repo.insert! %User{} +# conn = delete conn, user_path(conn, :delete, user) +# assert redirected_to(conn) == user_path(conn, :index) +# refute Repo.get(User, user.id) +# end +# end diff --git a/test/controllers/group_controller_test.exs b/test/controllers/group_controller_test.exs index 43bf747..7b5e4cc 100644 --- a/test/controllers/group_controller_test.exs +++ b/test/controllers/group_controller_test.exs @@ -1,66 +1,66 @@ -defmodule Org.GroupControllerTest do - use Org.ConnCase +# defmodule Org.GroupControllerTest do +# use Org.ConnCase - alias Org.Group - @valid_attrs %{description: "some content", title: "some content", url: "some content"} - @invalid_attrs %{} +# alias Org.Group +# @valid_attrs %{description: "some content", title: "some content", url: "some content"} +# @invalid_attrs %{} - test "lists all entries on index", %{conn: conn} do - conn = get conn, group_path(conn, :index) - assert html_response(conn, 200) =~ "Listing groups" - end +# test "lists all entries on index", %{conn: conn} do +# conn = get conn, group_path(conn, :index) +# assert html_response(conn, 200) =~ "Listing groups" +# end - test "renders form for new resources", %{conn: conn} do - conn = get conn, group_path(conn, :new) - assert html_response(conn, 200) =~ "New group" - end +# test "renders form for new resources", %{conn: conn} do +# conn = get conn, group_path(conn, :new) +# assert html_response(conn, 200) =~ "New group" +# end - test "creates resource and redirects when data is valid", %{conn: conn} do - conn = post conn, group_path(conn, :create), group: @valid_attrs - assert redirected_to(conn) == group_path(conn, :index) - assert Repo.get_by(Group, @valid_attrs) - end +# test "creates resource and redirects when data is valid", %{conn: conn} do +# conn = post conn, group_path(conn, :create), group: @valid_attrs +# assert redirected_to(conn) == group_path(conn, :index) +# assert Repo.get_by(Group, @valid_attrs) +# end - test "does not create resource and renders errors when data is invalid", %{conn: conn} do - conn = post conn, group_path(conn, :create), group: @invalid_attrs - assert html_response(conn, 200) =~ "New group" - end +# test "does not create resource and renders errors when data is invalid", %{conn: conn} do +# conn = post conn, group_path(conn, :create), group: @invalid_attrs +# assert html_response(conn, 200) =~ "New group" +# end - test "shows chosen resource", %{conn: conn} do - group = Repo.insert! %Group{} - conn = get conn, group_path(conn, :show, group) - assert html_response(conn, 200) =~ "Show group" - end +# test "shows chosen resource", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = get conn, group_path(conn, :show, group) +# assert html_response(conn, 200) =~ "Show group" +# end - test "renders page not found when id is nonexistent", %{conn: conn} do - assert_error_sent 404, fn -> - get conn, group_path(conn, :show, "11111111-1111-1111-1111-111111111111") - end - end +# test "renders page not found when id is nonexistent", %{conn: conn} do +# assert_error_sent 404, fn -> +# get conn, group_path(conn, :show, "11111111-1111-1111-1111-111111111111") +# end +# end - test "renders form for editing chosen resource", %{conn: conn} do - group = Repo.insert! %Group{} - conn = get conn, group_path(conn, :edit, group) - assert html_response(conn, 200) =~ "Edit group" - end +# test "renders form for editing chosen resource", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = get conn, group_path(conn, :edit, group) +# assert html_response(conn, 200) =~ "Edit group" +# end - test "updates chosen resource and redirects when data is valid", %{conn: conn} do - group = Repo.insert! %Group{} - conn = put conn, group_path(conn, :update, group), group: @valid_attrs - assert redirected_to(conn) == group_path(conn, :show, group) - assert Repo.get_by(Group, @valid_attrs) - end +# test "updates chosen resource and redirects when data is valid", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = put conn, group_path(conn, :update, group), group: @valid_attrs +# assert redirected_to(conn) == group_path(conn, :show, group) +# assert Repo.get_by(Group, @valid_attrs) +# end - test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do - group = Repo.insert! %Group{} - conn = put conn, group_path(conn, :update, group), group: @invalid_attrs - assert html_response(conn, 200) =~ "Edit group" - end +# test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = put conn, group_path(conn, :update, group), group: @invalid_attrs +# assert html_response(conn, 200) =~ "Edit group" +# end - test "deletes chosen resource", %{conn: conn} do - group = Repo.insert! %Group{} - conn = delete conn, group_path(conn, :delete, group) - assert redirected_to(conn) == group_path(conn, :index) - refute Repo.get(Group, group.id) - end -end +# test "deletes chosen resource", %{conn: conn} do +# group = Repo.insert! %Group{} +# conn = delete conn, group_path(conn, :delete, group) +# assert redirected_to(conn) == group_path(conn, :index) +# refute Repo.get(Group, group.id) +# end +# end diff --git a/test/controllers/page_controller_test.exs b/test/controllers/page_controller_test.exs index 38bf09c..fc7fa68 100644 --- a/test/controllers/page_controller_test.exs +++ b/test/controllers/page_controller_test.exs @@ -1,8 +1,8 @@ -defmodule Org.PageControllerTest do - use Org.ConnCase +# defmodule Org.PageControllerTest do +# use Org.ConnCase - test "GET /", %{conn: conn} do - conn = get conn, "/" - assert html_response(conn, 200) =~ "Welcome to Phoenix!" - end -end +# test "GET /", %{conn: conn} do +# conn = get conn, "/" +# assert html_response(conn, 200) =~ "Welcome to Phoenix!" +# end +# end diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index 33941fb..a8d70cb 100644 --- a/test/controllers/user_controller_test.exs +++ b/test/controllers/user_controller_test.exs @@ -1,52 +1,49 @@ defmodule Org.UserControllerTest do use Org.ConnCase - alias Org.User alias Org.Api.UserView - @invalid_attrs %{} - - describe "#index" do - test "lists all non-user role users", %{conn: conn} do + describe "with an authenticated user" do + test "#index lists all non-user role users", %{conn: conn} do [member, _] = [insert(:user), insert(:user, role: "user")] - conn = get conn, user_path(conn, :index) + conn = + conn + |> assign(:current_user, member) + |> get(user_path(conn, :index)) + assert json_response(conn, 200) == render_json(UserView, "index.json", users: [member]) end - end - describe "#show" do - test "renders a user", %{conn: conn} do + test "#show renders a user", %{conn: conn} do user = insert(:user) - conn = get conn, user_path(conn, :show, user) + conn = + conn + |> assign(:current_user, user) + |> get(user_path(conn, :show, user)) + assert json_response(conn, 200) == render_json(UserView, "show.json", user: user) end - test "renders user not found when id is nonexistent", %{conn: conn} do + test "#show renders user not found when id is nonexistent", %{conn: conn} do + user = insert(:user) assert_error_sent 404, fn -> - get conn, user_path(conn, :show, 1234) + conn + |> assign(:current_user, user) + |> get(user_path(conn, :show, 1234)) end end end - test "renders form for new resources", %{conn: conn} do - conn = get conn, user_path(conn, :new) - assert html_response(conn, 200) =~ "New user" - end - - test "does not create resource and renders errors when data is invalid", %{conn: conn} do - conn = post conn, user_path(conn, :create), user: @invalid_attrs - assert html_response(conn, 200) =~ "New user" - end - - test "renders form for editing chosen resource", %{conn: conn} do - user = Repo.insert! %User{} - conn = get conn, user_path(conn, :edit, user) - assert html_response(conn, 200) =~ "Edit user" + describe "with a non-authenticated user" do + test "returns a 401", %{conn: conn} do + conn = get conn, user_path(conn, :index) + assert json_response(conn, 401) == render_json(Org.ErrorView, "401.json") + end end - test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do - user = Repo.insert! %User{} - conn = put conn, user_path(conn, :update, user), user: @invalid_attrs - assert html_response(conn, 200) =~ "Edit user" - end +# test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do +# user = Repo.insert! %User{} +# conn = put conn, user_path(conn, :update, user), user: @invalid_attrs +# assert html_response(conn, 200) =~ "Edit user" +# end end diff --git a/test/models/admin/group_test.exs b/test/models/admin/group_test.exs index e8e944c..ef85cc6 100644 --- a/test/models/admin/group_test.exs +++ b/test/models/admin/group_test.exs @@ -1,18 +1,18 @@ -defmodule Org.Admin.GroupTest do - use Org.ModelCase +# defmodule Org.Admin.GroupTest do +# use Org.ModelCase - alias Org.Group +# alias Org.Group - @valid_attrs %{description: "some content", title: "some content", url: "some content"} - @invalid_attrs %{} +# @valid_attrs %{description: "some content", title: "some content", url: "some content"} +# @invalid_attrs %{} - test "changeset with valid attributes" do - changeset = Group.changeset(%Group{}, @valid_attrs) - assert changeset.valid? - end +# test "changeset with valid attributes" do +# changeset = Group.changeset(%Group{}, @valid_attrs) +# assert changeset.valid? +# end - test "changeset with invalid attributes" do - changeset = Group.changeset(%Group{}, @invalid_attrs) - refute changeset.valid? - end -end +# test "changeset with invalid attributes" do +# changeset = Group.changeset(%Group{}, @invalid_attrs) +# refute changeset.valid? +# end +# end diff --git a/test/models/admin/user_test.exs b/test/models/admin/user_test.exs index 8349dc0..4a905c1 100644 --- a/test/models/admin/user_test.exs +++ b/test/models/admin/user_test.exs @@ -1,18 +1,18 @@ -defmodule Org.Admin.UserTest do - use Org.ModelCase +# defmodule Org.Admin.UserTest do +# use Org.ModelCase - alias Org.User +# alias Org.User - @valid_attrs %{email: "some content", name: "some content"} - @invalid_attrs %{} +# @valid_attrs %{email: "some content", name: "some content"} +# @invalid_attrs %{} - test "changeset with valid attributes" do - changeset = User.changeset(%User{}, @valid_attrs) - assert changeset.valid? - end +# test "changeset with valid attributes" do +# changeset = User.changeset(%User{}, @valid_attrs) +# assert changeset.valid? +# end - test "changeset with invalid attributes" do - changeset = User.changeset(%User{}, @invalid_attrs) - refute changeset.valid? - end -end +# test "changeset with invalid attributes" do +# changeset = User.changeset(%User{}, @invalid_attrs) +# refute changeset.valid? +# end +# end diff --git a/test/plugs/auth_test.exs b/test/plugs/auth_test.exs new file mode 100644 index 0000000..c75c7d8 --- /dev/null +++ b/test/plugs/auth_test.exs @@ -0,0 +1,92 @@ +defmodule Org.Plugs.AuthTest do + use Org.ConnCase + alias Org.Plugs.Auth + alias Org.User + + setup %{conn: conn} do + conn = + conn + |> bypass_through(Org.Router, :api) + + {:ok, %{conn: conn}} + end + + test "call places user from session into assigns", %{conn: conn} do + user = insert(:user) + conn = + conn + |> get("/") + |> put_session(:current_user, user.id) + |> Auth.call([]) + + assert conn.assigns.current_user.id == user.id + end + + test "call with no session sets current_user assign to nil", %{conn: conn} do + conn = + conn + |> get("/") + |> Auth.call([]) + + assert conn.assigns.current_user == nil + end + + test "authenticated continues when current_user exists", %{conn: conn} do + conn = + conn + |> get("/") + |> assign(:current_user, %User{}) + |> Auth.authenticated([]) + + refute conn.halted + end + + test "authenticated halts when no current_user exists", %{conn: conn} do + conn = + conn + |> get("/") + |> Auth.authenticated([]) + + assert conn.halted + end + + test "authenticate_for_roles continues when the current_user matches the specified roles", %{conn: conn} do + conn = + conn + |> get("/") + |> assign(:current_user, %User{role: "member"}) + |> Auth.authenticate_for_roles(["member"]) + + refute conn.halted + end + + test "authenticate_for_roles halts when the current_user does not match the specified roles", %{conn: conn} do + conn = + conn + |> get("/") + |> assign(:current_user, %User{role: "member"}) + |> Auth.authenticate_for_roles(["admin"]) + + assert conn.halted + end + + test "authenticate_self continues when the current_user is the same as the route id", %{conn: conn} do + conn = + conn + |> get("/api/users", %{id: 1}) + |> assign(:current_user, %User{id: 1}) + |> Auth.authenticate_self([]) + + refute conn.halted + end + + test "authenticate_self halts when the current_user is not the same as the route id", %{conn: conn} do + conn = + conn + |> get("/api/users", %{id: 1234}) + |> assign(:current_user, %User{id: 1}) + |> Auth.authenticate_self([]) + + assert conn.halted + end +end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index eb5b5a5..a091a81 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -27,7 +27,6 @@ defmodule Org.ConnCase do import Org.Factory import Org.TestHelpers - import Org.Router.Helpers # The default endpoint for testing diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 2855ca9..805b557 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -1,5 +1,5 @@ defmodule Org.TestHelpers do - def render_json(view, template, assigns) do + def render_json(view, template, assigns \\ %{}) do template |> view.render(assigns) |> Poison.encode! diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index 4cd6cff..a43929b 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -3,35 +3,18 @@ defmodule Org.Api.UserController do alias Org.User + plug :authenticate_self when action in [:update, :apply] + def index(conn, _params) do users = Repo.all(from u in User, where: u.role != "user") render(conn, "index.json", users: users) end - def create(conn, %{"user" => user_params}) do - changeset = User.changeset(%User{}, user_params) - - case Repo.insert(changeset) do - {:ok, _user} -> - conn - |> put_flash(:info, "User created successfully.") - |> redirect(to: user_path(conn, :index)) - {:error, changeset} -> - render(conn, "new.html", changeset: changeset) - end - end - def show(conn, %{"id" => id}) do user = Repo.get!(User, id) render(conn, "show.json", user: user) end - def edit(conn, %{"id" => id}) do - user = Repo.get!(User, id) - changeset = User.changeset(user) - render(conn, "edit.html", user: user, changeset: changeset) - end - def update(conn, %{"id" => id, "user" => user_params}) do user = Repo.get!(User, id) changeset = User.changeset(user, user_params) @@ -41,8 +24,8 @@ defmodule Org.Api.UserController do conn |> put_flash(:info, "User updated successfully.") |> redirect(to: user_path(conn, :show, user)) - {:error, changeset} -> - render(conn, "edit.html", user: user, changeset: changeset) + {:error, _changeset} -> + put_flash(conn, :error, "Failed to update user.") end end @@ -61,14 +44,4 @@ defmodule Org.Api.UserController do render(conn, "apply.html", user: user, changeset: changeset) end end - - def delete(conn, %{"id" => id}) do - user = Repo.get!(User, id) - - Repo.delete!(user) - - conn - |> put_flash(:info, "User deleted successfully.") - |> redirect(to: user_path(conn, :index)) - end end diff --git a/web/models/user.ex b/web/models/user.ex index 0f06e3b..6b7d0c7 100644 --- a/web/models/user.ex +++ b/web/models/user.ex @@ -46,4 +46,10 @@ defmodule Org.User do ]) |> unique_constraint(:github_id) end + + def constants do + %{ + roles: ["user", "member", "admin"] + } + end end diff --git a/web/plugs/auth.ex b/web/plugs/auth.ex new file mode 100644 index 0000000..d1dc2c6 --- /dev/null +++ b/web/plugs/auth.ex @@ -0,0 +1,57 @@ +defmodule Org.Plugs.Auth do + @behaviour Plug + + import Plug.Conn + import Phoenix.Controller + + alias Org.Repo + alias Org.User + + def init(default), do: default + + def call(conn, _opts) do + user_id = get_session(conn, :current_user) + + cond do + current_user = conn.assigns[:current_user] -> + conn + current_user = user_id && Repo.get(User, user_id) -> + assign(conn, :current_user, current_user) + true -> + assign(conn, :current_user, nil) + end + end + + def authenticated(conn, _opts) do + if conn.assigns.current_user do + conn + else + conn + |> put_status(401) + |> render(Org.ErrorView, "401.json", %{}) + |> halt() + end + end + + def authenticate_for_roles(conn, roles) do + if Enum.member?(roles, conn.assigns.current_user.role) do + conn + else + conn + |> put_status(403) + |> render(Org.ErrorView, "403.json", %{}) + |> halt() + end + end + + def authenticate_self(conn, _opts) do + if conn.assigns.current_user.id == conn.params["id"] do + conn + else + conn + |> put_status(403) + |> render(Org.ErrorView, "403.json", %{}) + |> halt() + end + end +end diff --git a/web/plugs/authenticated.ex b/web/plugs/authenticated.ex deleted file mode 100644 index 7fb2b93..0000000 --- a/web/plugs/authenticated.ex +++ /dev/null @@ -1,34 +0,0 @@ -defmodule Org.Plugs.Authenticated do - @behaviour Plug - - import Plug.Conn - import Phoenix.Controller - - alias Org.Repo - alias Org.User - - def init(default), do: default - - def call(conn, _) do - current_user = get_session(conn, :current_user) - - if current_user do - case Repo.get(User, current_user) do - nil -> conn - |> configure_session(drop: true) # Drop invalid user session - |> flash_and_redirect - user -> assign(conn, :current_user, user) - end - else - conn - |> flash_and_redirect - end - end - - defp flash_and_redirect(conn) do - conn - |> put_flash(:error, "Please sign in") - |> redirect(to: "/") - |> halt - end -end diff --git a/web/router.ex b/web/router.ex index 8309db3..c6b8dc9 100644 --- a/web/router.ex +++ b/web/router.ex @@ -7,25 +7,13 @@ defmodule Org.Router do plug :fetch_flash plug :protect_from_forgery plug :put_secure_browser_headers - plug Org.Plugs.AssignCurrentUser - end - - pipeline :authenticated do - plug Org.Plugs.Authenticated - end - - pipeline :auth_admin do - plug Org.Plugs.Authenticated - plug Org.Plugs.Authorized, ["admin"] - end - - pipeline :auth_member do - plug Org.Plugs.Authenticated - plug Org.Plugs.Authorized, ["member", "admin"] + plug Org.Plugs.Auth end pipeline :api do plug :accepts, ["json"] + plug :fetch_session + plug Org.Plugs.Auth end # Scope for OAuth2 routes @@ -38,18 +26,18 @@ defmodule Org.Router do end # Scope for admin-only routes - scope "/", Org.Admin do - pipe_through [:browser, :auth_admin] + # scope "/api", Org.Api.Admin do + # pipe_through [:api] - # resources "/users", UserController, except: [:show, :new, :update] - resources "/groups", GroupController, except: [:index, :show] - end + # resources "/users", UserController, except: [:show, :new, :update] + # resources "/groups", GroupController, except: [:index, :show] + # end - # Scope for member-only routes - scope "/", Org do - pipe_through [:browser, :auth_member] + scope "/api", Org.Api do + pipe_through [:api, :authenticated] - # resources "/users", UserController, only: [:index, :show] + resources "/users", UserController, only: [:index, :show, :update] + put "/apply/:id", UserController, :apply end # Scope for authenticated-only routes (user is logged in) @@ -61,12 +49,6 @@ defmodule Org.Router do get "/thanks", PageController, :thanks end - scope "/api", Org.Api do - pipe_through :api - - resources "/users", UserController, only: [:index, :show, :update] - end - # Scope for all other routes scope "/", Org do pipe_through :browser @@ -75,9 +57,4 @@ defmodule Org.Router do get "/signin", PageController, :signin resources "/groups", GroupController, only: [:index, :show] end - - # Other scopes may use custom stacks. - # scope "/api", Org do - # pipe_through :api - # end end diff --git a/web/views/error_view.ex b/web/views/error_view.ex index 6492430..2a0e887 100644 --- a/web/views/error_view.ex +++ b/web/views/error_view.ex @@ -9,6 +9,14 @@ defmodule Org.ErrorView do "Internal server error" end + def render("401.json", _assigns) do + %{error: "Please sign in"} + end + + def render("403.json", _assigns) do + %{error: "You do not have the proper permissions to do that"} + end + # In case no render clause matches or no # template is found, let's render it as 500 def template_not_found(_template, assigns) do diff --git a/web/web.ex b/web/web.ex index be62034..c737970 100644 --- a/web/web.ex +++ b/web/web.ex @@ -36,6 +36,7 @@ defmodule Org.Web do import Org.Router.Helpers import Org.Gettext + import Org.Plugs.Auth, only: [authenticated: 2, authenticate_self: 2] end end @@ -58,6 +59,7 @@ defmodule Org.Web do def router do quote do use Phoenix.Router + import Org.Plugs.Auth, only: [authenticated: 2] end end From 083ce55dd8ed89da343359b0a8850875ab789d8b Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Sun, 21 May 2017 16:33:24 -0700 Subject: [PATCH 08/10] Add new ErrorView tests for 403.json and 401.json --- test/views/error_view_test.exs | 17 ++++++++++++++--- web/plugs/auth.ex | 6 +++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/test/views/error_view_test.exs b/test/views/error_view_test.exs index 45d928e..8979656 100644 --- a/test/views/error_view_test.exs +++ b/test/views/error_view_test.exs @@ -1,21 +1,32 @@ defmodule Org.ErrorViewTest do use Org.ConnCase, async: true + alias Org.ErrorView + # Bring render/3 and render_to_string/3 for testing custom views import Phoenix.View test "renders 404.html" do - assert render_to_string(Org.ErrorView, "404.html", []) == + assert render_to_string(ErrorView, "404.html", []) == "Page not found" end test "render 500.html" do - assert render_to_string(Org.ErrorView, "500.html", []) == + assert render_to_string(ErrorView, "500.html", []) == "Internal server error" end + test "render 401.json" do + assert ErrorView.render("401.json", []) == %{error: "Please sign in"} + end + + test "render 403.json" do + assert ErrorView.render("403.json", []) == + %{error: "You do not have the proper permissions to do that"} + end + test "render any other" do - assert render_to_string(Org.ErrorView, "505.html", []) == + assert render_to_string(ErrorView, "505.html", []) == "Internal server error" end end diff --git a/web/plugs/auth.ex b/web/plugs/auth.ex index d1dc2c6..568b996 100644 --- a/web/plugs/auth.ex +++ b/web/plugs/auth.ex @@ -28,7 +28,7 @@ defmodule Org.Plugs.Auth do else conn |> put_status(401) - |> render(Org.ErrorView, "401.json", %{}) + |> render(Org.ErrorView, "401.json", []) |> halt() end end @@ -39,7 +39,7 @@ defmodule Org.Plugs.Auth do else conn |> put_status(403) - |> render(Org.ErrorView, "403.json", %{}) + |> render(Org.ErrorView, "403.json", []) |> halt() end end @@ -50,7 +50,7 @@ defmodule Org.Plugs.Auth do else conn |> put_status(403) - |> render(Org.ErrorView, "403.json", %{}) + |> render(Org.ErrorView, "403.json", []) |> halt() end end From 5d42c4c7c2901007602c765c07ffcce1a82b6b17 Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Mon, 29 May 2017 14:37:54 -0700 Subject: [PATCH 09/10] Update user_controller to json except apply action --- config/config.exs | 2 +- config/dev.exs | 2 +- priv/repo/seeds.exs | 18 ++++ test/controllers/user_controller_test.exs | 108 +++++++++++++++------- test/models/user_test.exs | 8 +- test/plugs/auth_test.exs | 20 ---- test/views/api/user_view_test.exs | 11 ++- test/views/error_view_test.exs | 5 +- web/controllers/auth_controller.ex | 3 +- web/controllers/helpers.ex | 3 + web/controllers/user_controller.ex | 45 ++++++--- web/exceptions/forbidden.ex | 3 + web/models/user.ex | 10 +- web/permissions/user_permissions.ex | 16 ++++ web/plugs/auth.ex | 11 --- web/router.ex | 8 -- web/views/api/user_view.ex | 9 +- web/views/error_view.ex | 16 +++- web/web.ex | 2 +- 19 files changed, 190 insertions(+), 110 deletions(-) create mode 100644 web/controllers/helpers.ex create mode 100644 web/exceptions/forbidden.ex create mode 100644 web/permissions/user_permissions.ex diff --git a/config/config.exs b/config/config.exs index 7136ec0..5a4ef8f 100644 --- a/config/config.exs +++ b/config/config.exs @@ -14,7 +14,7 @@ config :org, Org.Endpoint, url: [host: "localhost"], root: Path.dirname(__DIR__), secret_key_base: "dbaUFzsX9uXxHHCvaccSgGphSF+a+z9WYnY/J9qjYToo0UUgXgYkTqBpPsBllbJv", - render_errors: [accepts: ~w(html json)], + render_errors: [accepts: ~w(json)], pubsub: [name: Org.PubSub, adapter: Phoenix.PubSub.PG2] diff --git a/config/dev.exs b/config/dev.exs index 33e331d..9d992db 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -8,7 +8,7 @@ use Mix.Config # with brunch.io to recompile .js and .css sources. config :org, Org.Endpoint, http: [port: 4000], - debug_errors: true, + debug_errors: false, code_reloader: true, check_origin: false, watchers: [node: ["node_modules/brunch/bin/brunch", "watch", "--stdin", diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 8adb3f0..c29fe94 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -9,3 +9,21 @@ # # We recommend using the bang functions (`insert!`, `update!` # and so on) as they will fail if something goes wrong. + +alias Org.Repo +alias Org.User + +for admin <- ["Troy Mullaney", "Kevin Lanni"] do + Repo.get_by(User, name: admin) || + Repo.insert!(%User{name: admin, role: "admin"}) +end + +for member <- ["Dwyane Wade", "LeBron James"] do + Repo.get_by(User, name: member) || + Repo.insert!(%User{name: member, role: "member"}) +end + +for user <- ["John Doe", "Jane Doe"] do + Repo.get_by(User, name: user) || + Repo.insert!(%User{name: user, role: "user"}) +end diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index a8d70cb..4b9c71d 100644 --- a/test/controllers/user_controller_test.exs +++ b/test/controllers/user_controller_test.exs @@ -3,47 +3,91 @@ defmodule Org.UserControllerTest do alias Org.Api.UserView - describe "with an authenticated user" do - test "#index lists all non-user role users", %{conn: conn} do - [member, _] = [insert(:user), insert(:user, role: "user")] - conn = - conn - |> assign(:current_user, member) - |> get(user_path(conn, :index)) - - assert json_response(conn, 200) == render_json(UserView, "index.json", users: [member]) + setup %{conn: conn} = config do + if role = config[:login_as] do + user = insert(:user, role: role) + conn = assign(build_conn(), :current_user, user) + {:ok, conn: conn, user: user} + else + :ok end + end - test "#show renders a user", %{conn: conn} do - user = insert(:user) - conn = - conn - |> assign(:current_user, user) - |> get(user_path(conn, :show, user)) + # Tests for index/2 action + @tag login_as: "member" + test "index/2 lists all non-user role users", %{conn: conn, user: member} do + [admin, _] = [insert(:user, role: "admin"), insert(:user, role: "user")] + conn = get conn, user_path(conn, :index) + assert json_response(conn, 200) == render_json(UserView, "index.json", users: [member, admin]) + end - assert json_response(conn, 200) == render_json(UserView, "show.json", user: user) + @tag login_as: "admin" + test "index/2 lists all users", %{conn: conn, user: admin} do + [user, member] = [insert(:user, role: "user"), insert(:user, role: "member")] + conn = get conn, user_path(conn, :index) + assert json_response(conn, 200) == render_json(UserView, "index.json", users: [admin, user, member]) + end + + test "returns a 401 when a user is not authorized", %{conn: conn} do + conn = get conn, user_path(conn, :index) + assert json_response(conn, 401) == render_json(Org.ErrorView, "401.json") + end + + @tag login_as: "user" + test "index/2 returns a 403 when a \"user\" attempts to access it", %{conn: conn} do + conn = get conn, user_path(conn, :index) + assert json_response(conn, 403) == render_json(Org.ErrorView, "403.json") + end + + # Tests for show/2 action + @tag login_as: "member" + test "show/2 renders a user", %{conn: conn, user: member} do + conn = get conn, user_path(conn, :show, member) + assert json_response(conn, 200) == render_json(UserView, "show.json", user: member) + end + + @tag login_as: "member" + test "show/2 returns a 404 when id is nonexistent", %{conn: conn} do + assert_error_sent 404, fn -> + get conn, user_path(conn, :show, 1234) end + end - test "#show renders user not found when id is nonexistent", %{conn: conn} do - user = insert(:user) - assert_error_sent 404, fn -> - conn - |> assign(:current_user, user) - |> get(user_path(conn, :show, 1234)) - end + @tag login_as: "member" + test "show/2 returns a 404 when the id is of a \"user\" and the current_user is a member", %{conn: conn} do + assert_error_sent 404, fn -> + user = insert(:user, role: "user") + get conn, user_path(conn, :show, user) end end - describe "with a non-authenticated user" do - test "returns a 401", %{conn: conn} do - conn = get conn, user_path(conn, :index) - assert json_response(conn, 401) == render_json(Org.ErrorView, "401.json") + # Tests for update/2 action + @tag login_as: "member" + test "update/2 returns an updated member", %{conn: conn, user: member} do + params = %{bio: "My new bio"} + conn = put conn, user_path(conn, :update, member), user: params + assert json_response(conn, 200)["data"]["bio"] == params.bio + end + + @tag login_as: "member" + test "update/2 returns a 403 when a member tries to update another member", %{conn: conn} do + assert_error_sent 403, fn -> + member = insert(:user, role: "member") + put conn, user_path(conn, :update, member), user: %{bio: "bio"} end end -# test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do -# user = Repo.insert! %User{} -# conn = put conn, user_path(conn, :update, user), user: @invalid_attrs -# assert html_response(conn, 200) =~ "Edit user" -# end + @tag login_as: "user" + test "update/2 returns a 403 when a user tries to access the action", %{conn: conn, user: user} do + conn = put conn, user_path(conn, :update, user) + assert json_response(conn, 403) == render_json(Org.ErrorView, "403.json") + end + + @tag login_as: "member" + test "update/2 returns a 409 when provided an invalid changeset", %{conn: conn, user: member} do + conn = put conn, user_path(conn, :update, member), user: %{} + assert json_response(conn, 409) == render_json(Org.ErrorView, "409.json") + end + + # Tests for #apply action end diff --git a/test/models/user_test.exs b/test/models/user_test.exs index 6f84a2b..7336989 100644 --- a/test/models/user_test.exs +++ b/test/models/user_test.exs @@ -6,13 +6,13 @@ defmodule Org.UserTest do @valid_attrs %{avatar: "some content", bio: "some content", blog: "some content", company: "some content", created_at: "some content", email: "some content", followers: 42, following: 42, github_id: 42, hireable: true, html_url: "some content", location: "some content", login: "some content", name: "some content", public_gists: 42, public_repos: 42, role: "some content", type: "some content"} @invalid_attrs %{} - test "changeset with valid attributes" do - changeset = User.changeset(%User{}, @valid_attrs) + test "registration_changeset with valid attributes" do + changeset = User.registration_changeset(%User{}, @valid_attrs) assert changeset.valid? end - test "changeset with invalid attributes" do - changeset = User.changeset(%User{}, @invalid_attrs) + test "registration_changeset with invalid attributes" do + changeset = User.registration_changeset(%User{}, @invalid_attrs) refute changeset.valid? end end diff --git a/test/plugs/auth_test.exs b/test/plugs/auth_test.exs index c75c7d8..2d0431a 100644 --- a/test/plugs/auth_test.exs +++ b/test/plugs/auth_test.exs @@ -69,24 +69,4 @@ defmodule Org.Plugs.AuthTest do assert conn.halted end - - test "authenticate_self continues when the current_user is the same as the route id", %{conn: conn} do - conn = - conn - |> get("/api/users", %{id: 1}) - |> assign(:current_user, %User{id: 1}) - |> Auth.authenticate_self([]) - - refute conn.halted - end - - test "authenticate_self halts when the current_user is not the same as the route id", %{conn: conn} do - conn = - conn - |> get("/api/users", %{id: 1234}) - |> assign(:current_user, %User{id: 1}) - |> Auth.authenticate_self([]) - - assert conn.halted - end end diff --git a/test/views/api/user_view_test.exs b/test/views/api/user_view_test.exs index de34c64..6ea1ac5 100644 --- a/test/views/api/user_view_test.exs +++ b/test/views/api/user_view_test.exs @@ -2,14 +2,15 @@ defmodule Org.Api.UserViewTest do use Org.ModelCase alias Org.Api.UserView - test "user_json" do + test "user.json" do user = insert(:user) - rendered_user = UserView.user_json(user) + rendered_user = UserView.render("user.json", %{user: user}) assert rendered_user == %{ name: user.name, email: user.email, - role: user.role + role: user.role, + bio: user.bio } end @@ -17,13 +18,13 @@ defmodule Org.Api.UserViewTest do user = insert(:user) rendered_users = UserView.render("index.json", %{users: [user]}) - assert rendered_users == %{users: [UserView.user_json(user)]} + assert rendered_users == %{data: [UserView.render("user.json", %{user: user})]} end test "show.json" do user = insert(:user) rendered_user = UserView.render("show.json", %{user: user}) - assert rendered_user == %{user: UserView.user_json(user)} + assert rendered_user == %{data: UserView.render("user.json", %{user: user})} end end diff --git a/test/views/error_view_test.exs b/test/views/error_view_test.exs index 8979656..e27768d 100644 --- a/test/views/error_view_test.exs +++ b/test/views/error_view_test.exs @@ -17,12 +17,11 @@ defmodule Org.ErrorViewTest do end test "render 401.json" do - assert ErrorView.render("401.json", []) == %{error: "Please sign in"} + assert ErrorView.render("401.json", []) == %{message: "Unauthorized"} end test "render 403.json" do - assert ErrorView.render("403.json", []) == - %{error: "You do not have the proper permissions to do that"} + assert ErrorView.render("403.json", []) == %{message: "Forbidden"} end test "render any other" do diff --git a/web/controllers/auth_controller.ex b/web/controllers/auth_controller.ex index c5a4189..24881eb 100644 --- a/web/controllers/auth_controller.ex +++ b/web/controllers/auth_controller.ex @@ -73,7 +73,8 @@ defmodule Org.AuthController do nil -> %User{} # User not found, we build one user -> user # User exists, let's use it end - |> User.changeset(user) + user + |> User.changeset |> Repo.insert_or_update! end end diff --git a/web/controllers/helpers.ex b/web/controllers/helpers.ex new file mode 100644 index 0000000..b0922dd --- /dev/null +++ b/web/controllers/helpers.ex @@ -0,0 +1,3 @@ +defmodule Org.Controller.Helpers do + def current_user(conn), do: conn.assigns.current_user +end diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index a43929b..c2058a1 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -1,32 +1,38 @@ defmodule Org.Api.UserController do use Org.Web, :controller + import Org.User.Permissions + import Org.Controller.Helpers + alias Org.User - plug :authenticate_self when action in [:update, :apply] + plug :authenticate_for_roles, ~w(admin member) when action in [:index, :show, :update] def index(conn, _params) do - users = Repo.all(from u in User, where: u.role != "user") + users = + User + |> limit_for(current_user(conn).role) + |> Repo.all + render(conn, "index.json", users: users) end def show(conn, %{"id" => id}) do - user = Repo.get!(User, id) + user = + User + |> limit_for(current_user(conn).role) + |> Repo.get!(id) + render(conn, "show.json", user: user) end def update(conn, %{"id" => id, "user" => user_params}) do - user = Repo.get!(User, id) - changeset = User.changeset(user, user_params) - - case Repo.update(changeset) do - {:ok, user} -> - conn - |> put_flash(:info, "User updated successfully.") - |> redirect(to: user_path(conn, :show, user)) - {:error, _changeset} -> - put_flash(conn, :error, "Failed to update user.") - end + User + |> limit_for(current_user(conn).role) + |> Repo.get!(id) + |> authorize!(:update, current_user(conn)) + |> User.changeset(user_params) + |> update_user(conn, &(render(conn, "show.json", user: &1))) end def apply(conn, %{"id" => id, "user" => user_params}) do @@ -44,4 +50,15 @@ defmodule Org.Api.UserController do render(conn, "apply.html", user: user, changeset: changeset) end end + + defp update_user(changeset, conn, func) do + case Repo.update(changeset) do + {:ok, user} -> + func.(user) + {:error, %{errors: errors}} -> + conn + |> put_status(409) + |> render(Org.ErrorView, "409.json", errors: errors) + end + end end diff --git a/web/exceptions/forbidden.ex b/web/exceptions/forbidden.ex new file mode 100644 index 0000000..c603edd --- /dev/null +++ b/web/exceptions/forbidden.ex @@ -0,0 +1,3 @@ +defmodule Org.Forbidden do + defexception [message: "Forbidden", plug_status: 403] +end diff --git a/web/models/user.ex b/web/models/user.ex index 6b7d0c7..a1e73a1 100644 --- a/web/models/user.ex +++ b/web/models/user.ex @@ -34,6 +34,12 @@ defmodule Org.User do Builds a changeset based on the `struct` and `params`. """ def changeset(struct, params \\ %{}) do + struct + |> cast(params, [:bio]) + |> validate_required([:bio]) + end + + def registration_changeset(struct, params \\ %{}) do struct |> cast(params, [:avatar, :bio, :blog, :company, :created_at, :email, :followers, :following, :hireable, :html_url, :github_id, :location, @@ -48,8 +54,6 @@ defmodule Org.User do end def constants do - %{ - roles: ["user", "member", "admin"] - } + %{roles: ~w(user member admin)} end end diff --git a/web/permissions/user_permissions.ex b/web/permissions/user_permissions.ex new file mode 100644 index 0000000..09b19fa --- /dev/null +++ b/web/permissions/user_permissions.ex @@ -0,0 +1,16 @@ +defmodule Org.User.Permissions do + import Ecto.Query + + alias Org.User + + def limit_for(User, "admin"), do: User + def limit_for(User, "member"), do: where(User, [u], u.role != "user") + def limit_for(User, _role), do: where(User, false) + + def authorize!(user, :update, %{role: "admin"}), + do: user + def authorize!(user, :update, %{role: "member"} = current_user), + do: if current_user.id == user.id, do: user, else: raise Org.Forbidden + def authorize!(_user, :update, _current_user), + do: raise Org.Forbidden +end diff --git a/web/plugs/auth.ex b/web/plugs/auth.ex index 568b996..c8d9cf3 100644 --- a/web/plugs/auth.ex +++ b/web/plugs/auth.ex @@ -43,15 +43,4 @@ defmodule Org.Plugs.Auth do |> halt() end end - - def authenticate_self(conn, _opts) do - if conn.assigns.current_user.id == conn.params["id"] do - conn - else - conn - |> put_status(403) - |> render(Org.ErrorView, "403.json", []) - |> halt() - end - end end diff --git a/web/router.ex b/web/router.ex index c6b8dc9..2f35f73 100644 --- a/web/router.ex +++ b/web/router.ex @@ -25,14 +25,6 @@ defmodule Org.Router do delete "/logout", AuthController, :delete end - # Scope for admin-only routes - # scope "/api", Org.Api.Admin do - # pipe_through [:api] - - # resources "/users", UserController, except: [:show, :new, :update] - # resources "/groups", GroupController, except: [:index, :show] - # end - scope "/api", Org.Api do pipe_through [:api, :authenticated] diff --git a/web/views/api/user_view.ex b/web/views/api/user_view.ex index 0f812b6..86efd62 100644 --- a/web/views/api/user_view.ex +++ b/web/views/api/user_view.ex @@ -2,18 +2,19 @@ defmodule Org.Api.UserView do use Org.Web, :view def render("index.json", %{users: users}) do - %{users: Enum.map(users, &(user_json(&1)))} + %{data: render_many(users, Org.Api.UserView, "user.json")} end def render("show.json", %{user: user}) do - %{user: user_json(user)} + %{data: render_one(user, Org.Api.UserView, "user.json")} end - def user_json(user) do + def render("user.json", %{user: user}) do %{ name: user.name, email: user.email, - role: user.role + role: user.role, + bio: user.bio } end end diff --git a/web/views/error_view.ex b/web/views/error_view.ex index 2a0e887..2e77c39 100644 --- a/web/views/error_view.ex +++ b/web/views/error_view.ex @@ -10,11 +10,23 @@ defmodule Org.ErrorView do end def render("401.json", _assigns) do - %{error: "Please sign in"} + %{message: "Unauthorized"} end def render("403.json", _assigns) do - %{error: "You do not have the proper permissions to do that"} + %{message: "Forbidden"} + end + + def render("404.json", _assigns) do + %{message: "Not Found"} + end + + def render("409.json", _assigns) do + %{message: "Conflict"} + end + + def render("500.json", _assigns) do + %{message: "Internal Server Error"} end # In case no render clause matches or no diff --git a/web/web.ex b/web/web.ex index c737970..d396576 100644 --- a/web/web.ex +++ b/web/web.ex @@ -36,7 +36,7 @@ defmodule Org.Web do import Org.Router.Helpers import Org.Gettext - import Org.Plugs.Auth, only: [authenticated: 2, authenticate_self: 2] + import Org.Plugs.Auth, only: [authenticated: 2, authenticate_for_roles: 2] end end From df19449d04d081b04b4cc09f52a8d3f54eca43db Mon Sep 17 00:00:00 2001 From: Troy Mullaney Date: Tue, 30 May 2017 18:06:54 -0700 Subject: [PATCH 10/10] Remove admin user controller --- web/controllers/admin/user_controller.ex | 67 ------------------------ web/controllers/auth_controller.ex | 8 +-- web/controllers/user_controller.ex | 6 +-- 3 files changed, 7 insertions(+), 74 deletions(-) delete mode 100644 web/controllers/admin/user_controller.ex diff --git a/web/controllers/admin/user_controller.ex b/web/controllers/admin/user_controller.ex deleted file mode 100644 index f1e66c3..0000000 --- a/web/controllers/admin/user_controller.ex +++ /dev/null @@ -1,67 +0,0 @@ -defmodule Org.Admin.UserController do - use Org.Web, :controller - - alias Org.User - - plug :scrub_params, "user" when action in [:create, :update] - - def index(conn, _params) do - users = Repo.all(User) - render(conn, "index.html", users: users) - end - - def new(conn, _params) do - changeset = User.changeset(%User{}) - render(conn, "new.html", changeset: changeset) - end - - def create(conn, %{"user" => user_params}) do - changeset = User.changeset(%User{}, user_params) - - case Repo.insert(changeset) do - {:ok, _user} -> - conn - |> put_flash(:info, "User created successfully.") - |> redirect(to: user_path(conn, :index)) - {:error, changeset} -> - render(conn, "new.html", changeset: changeset) - end - end - - def show(conn, %{"id" => id}) do - user = Repo.get!(User, id) - render(conn, "show.html", user: user) - end - - def edit(conn, %{"id" => id}) do - user = Repo.get!(User, id) - changeset = User.changeset(user) - render(conn, "edit.html", user: user, changeset: changeset) - end - - def update(conn, %{"id" => id, "user" => user_params}) do - user = Repo.get!(User, id) - changeset = User.changeset(user, user_params) - - case Repo.update(changeset) do - {:ok, user} -> - conn - |> put_flash(:info, "User updated successfully.") - |> redirect(to: user_path(conn, :show, user)) - {:error, changeset} -> - render(conn, "edit.html", user: user, changeset: changeset) - end - end - - def delete(conn, %{"id" => id}) do - user = Repo.get!(User, id) - - # Here we use delete! (with a bang) because we expect - # it to always work (and if it does not, it will raise). - Repo.delete!(user) - - conn - |> put_flash(:info, "User deleted successfully.") - |> redirect(to: user_path(conn, :index)) - end -end diff --git a/web/controllers/auth_controller.ex b/web/controllers/auth_controller.ex index 24881eb..e7fbe32 100644 --- a/web/controllers/auth_controller.ex +++ b/web/controllers/auth_controller.ex @@ -13,7 +13,6 @@ defmodule Org.AuthController do def delete(conn, _params) do conn - |> put_flash(:info, "You have been logged out!") |> configure_session(drop: true) |> redirect(to: "/") end @@ -38,10 +37,10 @@ defmodule Org.AuthController do |> redirect(to: "/") end - defp authorize_url!("github"), do: GitHub.authorize_url! + defp authorize_url!("github"), do: GitHub.authorize_url! defp authorize_url!(_), do: raise "No matching provider available" - defp get_token!("github", code), do: GitHub.get_token!(code: code) + defp get_token!("github", code), do: GitHub.get_token!(code: code) defp get_token!(_, _), do: raise "No matching provider available" defp get_user!("github", token) do @@ -69,10 +68,11 @@ defmodule Org.AuthController do end defp find_or_create_user(user) do - case Repo.get_by(User, github_id: user[:github_id]) do + user = case Repo.get_by(User, github_id: user[:github_id]) do nil -> %User{} # User not found, we build one user -> user # User exists, let's use it end + user |> User.changeset |> Repo.insert_or_update! diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index c2058a1..2610b78 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -32,7 +32,7 @@ defmodule Org.Api.UserController do |> Repo.get!(id) |> authorize!(:update, current_user(conn)) |> User.changeset(user_params) - |> update_user(conn, &(render(conn, "show.json", user: &1))) + |> update_user(conn) end def apply(conn, %{"id" => id, "user" => user_params}) do @@ -51,10 +51,10 @@ defmodule Org.Api.UserController do end end - defp update_user(changeset, conn, func) do + defp update_user(changeset, conn) do case Repo.update(changeset) do {:ok, user} -> - func.(user) + render(conn, "show.json", user: user) {:error, %{errors: errors}} -> conn |> put_status(409)