From 6b5615f91caea4116151508c72e00d85e77982a0 Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Sat, 14 Apr 2018 14:36:23 +0800 Subject: [PATCH 1/7] Add styles from source-academy2 --- frontend/src/styles/_admin.scss | 35 +++++++++++++++++++++++++++++ frontend/src/styles/_variables.scss | 6 +++++ frontend/src/styles/index.scss | 2 ++ 3 files changed, 43 insertions(+) create mode 100644 frontend/src/styles/_admin.scss diff --git a/frontend/src/styles/_admin.scss b/frontend/src/styles/_admin.scss new file mode 100644 index 000000000..b52be0d14 --- /dev/null +++ b/frontend/src/styles/_admin.scss @@ -0,0 +1,35 @@ +.sa-admin-navbar { + position: fixed; + top: 50px; + left: 0px; + width: 100%; + z-index: 1; + background: $sa-gray3; + + a { + color: $sa-dark-blue; + font-weight: 500; + text-transform: uppercase; + padding: 5px; + } + + .first-row, .second-row { + padding: 0px 20px; + } + + .first-row a { + font-size: 1.1em; + } + + .second-row a { + font-size: 0.85em; + } + + .second-row { + background: $sa-gray4; + } + + a.pt-button::after { + display: none !important; + } +} diff --git a/frontend/src/styles/_variables.scss b/frontend/src/styles/_variables.scss index a84e0fe96..6f90a8899 100644 --- a/frontend/src/styles/_variables.scss +++ b/frontend/src/styles/_variables.scss @@ -10,6 +10,12 @@ $pt-font-size: 14px !default; $sa-dark-blue: #101e35 !default; $sa-darker-blue: darken($sa-dark-blue, 5%); +$sa-gray5: #F2F4F7; +$sa-gray4: darken($sa-gray5, 5%); +$sa-gray3: darken($sa-gray5, 10%); +$sa-gray2: darken($sa-gray5, 15%); +$sa-gray1: darken($sa-gray5, 20%); + // Layout Variables $sa-header-bg-color: black; diff --git a/frontend/src/styles/index.scss b/frontend/src/styles/index.scss index b2aeb7575..3765eed6d 100644 --- a/frontend/src/styles/index.scss +++ b/frontend/src/styles/index.scss @@ -2,3 +2,5 @@ @import 'layout'; @import 'session'; + +@import 'admin'; From 2c7f56bb323d1ea1e14787b7190a7b4d1a1eda36 Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Sat, 14 Apr 2018 14:37:07 +0800 Subject: [PATCH 2/7] Add admin check pipeline --- lib/cadet_web/plug/check_admin.ex | 22 ++++++++++++++++++++++ lib/cadet_web/router.ex | 4 ++++ lib/cadet_web/views/view_helpers.ex | 8 ++++++++ 3 files changed, 34 insertions(+) create mode 100644 lib/cadet_web/plug/check_admin.ex diff --git a/lib/cadet_web/plug/check_admin.ex b/lib/cadet_web/plug/check_admin.ex new file mode 100644 index 000000000..7245b2f8b --- /dev/null +++ b/lib/cadet_web/plug/check_admin.ex @@ -0,0 +1,22 @@ +defmodule CadetWeb.Plug.CheckAdmin do + @moduledoc """ + A plug that checks whether :current_user is an admin + """ + + import Plug.Conn + + import Phoenix.Controller + + def init(opts), do: opts + + def call(conn, _) do + if conn.assigns[:current_user] != nil && conn.assigns[:current_user].role == :admin do + conn + else + conn + |> put_status(:forbidden) + |> render(CadetWeb.ErrorView, "403.html") + |> halt() + end + end +end diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex index 5301a5b86..57561c4b1 100644 --- a/lib/cadet_web/router.ex +++ b/lib/cadet_web/router.ex @@ -18,6 +18,10 @@ defmodule CadetWeb.Router do plug(Guardian.Plug.EnsureAuthenticated) end + pipeline :ensure_admin do + plug(CadetWeb.Plug.CheckAdmin) + end + # Public Pages scope "/", CadetWeb do pipe_through([:browser, :auth]) diff --git a/lib/cadet_web/views/view_helpers.ex b/lib/cadet_web/views/view_helpers.ex index c829536cc..aa6c5b614 100644 --- a/lib/cadet_web/views/view_helpers.ex +++ b/lib/cadet_web/views/view_helpers.ex @@ -7,4 +7,12 @@ defmodule CadetWeb.ViewHelpers do def logged_in?(conn) do conn.assigns[:current_user] != nil end + + def is_admin?(conn) do + if logged_in?(conn) do + conn.assigns[:current_user].role == :admin + else + false + end + end end From 177ba1c773fa617f0a5626c5bc0594525a791827 Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Sat, 14 Apr 2018 14:38:27 +0800 Subject: [PATCH 3/7] Add admin interface --- lib/cadet_web/controllers/admin_controller.ex | 7 ++++++ lib/cadet_web/router.ex | 7 ++++++ lib/cadet_web/templates/admin/index.html.eex | 24 +++++++++++++++++++ .../templates/layout/app.navbar.html.eex | 6 ++++- lib/cadet_web/views/admin_view.ex | 3 +++ .../controllers/admin_controller_test.exs | 17 +++++++++++++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 lib/cadet_web/controllers/admin_controller.ex create mode 100644 lib/cadet_web/templates/admin/index.html.eex create mode 100644 lib/cadet_web/views/admin_view.ex create mode 100644 test/cadet_web/controllers/admin_controller_test.exs diff --git a/lib/cadet_web/controllers/admin_controller.ex b/lib/cadet_web/controllers/admin_controller.ex new file mode 100644 index 000000000..3520d940f --- /dev/null +++ b/lib/cadet_web/controllers/admin_controller.ex @@ -0,0 +1,7 @@ +defmodule CadetWeb.AdminController do + use CadetWeb, :controller + + def index(conn, _) do + render(conn, "index.html") + end +end diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex index 57561c4b1..c0e985dac 100644 --- a/lib/cadet_web/router.ex +++ b/lib/cadet_web/router.ex @@ -37,6 +37,13 @@ defmodule CadetWeb.Router do get("/", PageController, :index) end + # Admin Pages + scope "/", CadetWeb do + pipe_through([:browser, :auth, :ensure_auth, :ensure_admin]) + + get("/admin", AdminController, :index) + end + # Other scopes may use custom stacks. # scope "/api", CadetWeb do # pipe_through :api diff --git a/lib/cadet_web/templates/admin/index.html.eex b/lib/cadet_web/templates/admin/index.html.eex new file mode 100644 index 000000000..4a8566bde --- /dev/null +++ b/lib/cadet_web/templates/admin/index.html.eex @@ -0,0 +1,24 @@ +
+
+
+
+ <%= link "Announcements", to: "#", class: "pt-button" %> + <%= link "Assessments", to: "#", class: "pt-button" %> + <%= link "My Students", to: "#", class: "pt-button" %> + <%= link "Gradings", to: "#", class: "pt-button" %> + <%= link "Path Submissions", to: "#", class: "pt-button" %> +
+
+
+
+
+
+ <%= link "Achievements", to: "#", class: "pt-button" %> + <%= link "Materials", to: "#", class: "pt-button" %> + <%= link "Discussion Groups", to: "#", class: "pt-button" %> + <%= link "Students", to: "#", class: "pt-button" %> + <%= link "Libraries", to: "#", class: "pt-button" %> +
+
+
+
diff --git a/lib/cadet_web/templates/layout/app.navbar.html.eex b/lib/cadet_web/templates/layout/app.navbar.html.eex index 35f4f640f..51d24847b 100644 --- a/lib/cadet_web/templates/layout/app.navbar.html.eex +++ b/lib/cadet_web/templates/layout/app.navbar.html.eex @@ -11,7 +11,7 @@ - <% end %> @@ -20,6 +20,10 @@ Playground + <%= if is_admin?(@conn) do %> + <%= button "Admin", to: admin_path(@conn, :index), method: "get", class: "pt-button pt-minimal pt-icon-settings" %> + <% end %> + <%= if logged_in?(@conn) do %> <%= button "Logout", to: session_path(@conn, :logout), method: "get", class: "pt-button pt-minimal pt-intent-danger" %> <% else %> diff --git a/lib/cadet_web/views/admin_view.ex b/lib/cadet_web/views/admin_view.ex new file mode 100644 index 000000000..870665eaa --- /dev/null +++ b/lib/cadet_web/views/admin_view.ex @@ -0,0 +1,3 @@ +defmodule CadetWeb.AdminView do + use CadetWeb, :view +end diff --git a/test/cadet_web/controllers/admin_controller_test.exs b/test/cadet_web/controllers/admin_controller_test.exs new file mode 100644 index 000000000..80f13677c --- /dev/null +++ b/test/cadet_web/controllers/admin_controller_test.exs @@ -0,0 +1,17 @@ +defmodule CadetWeb.AdminControllerTest do + use CadetWeb.ConnCase + + describe "Unauthenticated User" do + test "GET /admin" + end + + @tag authenticate: :student + describe "Authenticated Student" do + test "GET /admin" + end + + @tag authenticate: :admin + describe "Authenticated Admin" do + test "GET /admin" + end +end From 3ddc4a7d2956ffe2f789de055644813b42c00782 Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Tue, 17 Apr 2018 19:56:26 +0800 Subject: [PATCH 4/7] Add tests --- lib/cadet_web/plug/check_admin.ex | 12 ++++++------ lib/cadet_web/router.ex | 4 ++-- .../controllers/admin_controller_test.exs | 15 ++++++++++++--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lib/cadet_web/plug/check_admin.ex b/lib/cadet_web/plug/check_admin.ex index 7245b2f8b..2a22471a6 100644 --- a/lib/cadet_web/plug/check_admin.ex +++ b/lib/cadet_web/plug/check_admin.ex @@ -1,21 +1,21 @@ defmodule CadetWeb.Plug.CheckAdmin do @moduledoc """ - A plug that checks whether :current_user is an admin + Checks whether :current_user is an admin. + If the user is not an admin, the default HTTP 403 Forbidden page will be + rendered. """ import Plug.Conn - import Phoenix.Controller - def init(opts), do: opts def call(conn, _) do - if conn.assigns[:current_user] != nil && conn.assigns[:current_user].role == :admin do + if conn.assigns[:current_user].role == :admin do conn else conn - |> put_status(:forbidden) - |> render(CadetWeb.ErrorView, "403.html") + |> put_resp_content_type("text/html") + |> send_resp(:forbidden, "Not admin") |> halt() end end diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex index c0e985dac..7abd42a22 100644 --- a/lib/cadet_web/router.ex +++ b/lib/cadet_web/router.ex @@ -38,10 +38,10 @@ defmodule CadetWeb.Router do end # Admin Pages - scope "/", CadetWeb do + scope "/admin", CadetWeb do pipe_through([:browser, :auth, :ensure_auth, :ensure_admin]) - get("/admin", AdminController, :index) + get("/", AdminController, :index) end # Other scopes may use custom stacks. diff --git a/test/cadet_web/controllers/admin_controller_test.exs b/test/cadet_web/controllers/admin_controller_test.exs index 80f13677c..b86770a36 100644 --- a/test/cadet_web/controllers/admin_controller_test.exs +++ b/test/cadet_web/controllers/admin_controller_test.exs @@ -2,16 +2,25 @@ defmodule CadetWeb.AdminControllerTest do use CadetWeb.ConnCase describe "Unauthenticated User" do - test "GET /admin" + test "GET /admin", %{conn: conn} do + conn = get(conn, "/admin") + assert html_response(conn, 401) =~ "unauthenticated" + end end @tag authenticate: :student describe "Authenticated Student" do - test "GET /admin" + test "GET /admin", %{conn: conn} do + conn = get(conn, "/admin") + assert html_response(conn, 403) =~ "Not admin" + end end @tag authenticate: :admin describe "Authenticated Admin" do - test "GET /admin" + test "GET /admin", %{conn: conn} do + conn = get(conn, "/admin") + assert html_response(conn, 200) =~ "Admin" + end end end From d9e6aaa0a05924c328c76cf315c3338ce8bf74a2 Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Tue, 17 Apr 2018 19:58:04 +0800 Subject: [PATCH 5/7] Fix moduledoc for CadetWeb.Plug.CheckAdmin --- lib/cadet_web/plug/check_admin.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cadet_web/plug/check_admin.ex b/lib/cadet_web/plug/check_admin.ex index 2a22471a6..de1b84271 100644 --- a/lib/cadet_web/plug/check_admin.ex +++ b/lib/cadet_web/plug/check_admin.ex @@ -1,8 +1,7 @@ defmodule CadetWeb.Plug.CheckAdmin do @moduledoc """ Checks whether :current_user is an admin. - If the user is not an admin, the default HTTP 403 Forbidden page will be - rendered. + If the user is not an admin, the HTTP 403 response will be sent. """ import Plug.Conn From 85a4517c1a463d018eb943de397f596853e4518e Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Tue, 17 Apr 2018 20:16:23 +0800 Subject: [PATCH 6/7] Add more tests --- test/cadet_web/plug/check_admin_test.exs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/cadet_web/plug/check_admin_test.exs diff --git a/test/cadet_web/plug/check_admin_test.exs b/test/cadet_web/plug/check_admin_test.exs new file mode 100644 index 000000000..f1f247979 --- /dev/null +++ b/test/cadet_web/plug/check_admin_test.exs @@ -0,0 +1,18 @@ +defmodule CadetWeb.Plug.CheckAdminTest do + use CadetWeb.ConnCase + + alias CadetWeb.Plug.AssignCurrentUser + alias CadetWeb.Plug.CheckAdmin + + test "init" do + CheckAdmin.init(%{}) + # nothing to test + end + + @tag authenticate: :student + test "logged in as student", %{conn: conn} do + conn = AssignCurrentUser.call(conn, %{}) + conn = CheckAdmin.call(conn, %{}) + assert html_response(conn, 403) =~ "Not admin" + end +end From f08205dcfe7aa49f425f91e17501b1ef1ad38b61 Mon Sep 17 00:00:00 2001 From: Julius Putra Tanu Setiaji Date: Wed, 18 Apr 2018 22:42:05 +0800 Subject: [PATCH 7/7] Refactor into EnsureRoles --- lib/cadet_web/plug/check_admin.ex | 21 ------------ lib/cadet_web/plug/ensure_roles.ex | 25 +++++++++++++++ lib/cadet_web/router.ex | 6 ++-- .../templates/layout/app.navbar.html.eex | 2 +- lib/cadet_web/views/view_helpers.ex | 4 +-- test/cadet_web/plug/check_admin_test.exs | 18 ----------- test/cadet_web/plug/ensure_roles_test.exs | 32 +++++++++++++++++++ 7 files changed, 63 insertions(+), 45 deletions(-) delete mode 100644 lib/cadet_web/plug/check_admin.ex create mode 100644 lib/cadet_web/plug/ensure_roles.ex delete mode 100644 test/cadet_web/plug/check_admin_test.exs create mode 100644 test/cadet_web/plug/ensure_roles_test.exs diff --git a/lib/cadet_web/plug/check_admin.ex b/lib/cadet_web/plug/check_admin.ex deleted file mode 100644 index de1b84271..000000000 --- a/lib/cadet_web/plug/check_admin.ex +++ /dev/null @@ -1,21 +0,0 @@ -defmodule CadetWeb.Plug.CheckAdmin do - @moduledoc """ - Checks whether :current_user is an admin. - If the user is not an admin, the HTTP 403 response will be sent. - """ - - import Plug.Conn - - def init(opts), do: opts - - def call(conn, _) do - if conn.assigns[:current_user].role == :admin do - conn - else - conn - |> put_resp_content_type("text/html") - |> send_resp(:forbidden, "Not admin") - |> halt() - end - end -end diff --git a/lib/cadet_web/plug/ensure_roles.ex b/lib/cadet_web/plug/ensure_roles.ex new file mode 100644 index 000000000..8f14f834c --- /dev/null +++ b/lib/cadet_web/plug/ensure_roles.ex @@ -0,0 +1,25 @@ +defmodule CadetWeb.Plug.EnsureRoles do + @moduledoc """ + Ensures that :current_user's role is inside a list provided as option. + If the user is not inside the list, HTTP 403 response will be sent. + """ + + import Plug.Conn + + def init(opts), do: opts + + def call(conn, %{roles: roles}) do + if conn.assigns[:current_user].role in roles do + conn + else + body = + roles + |> Enum.map(&to_string/1) + |> Enum.join("/") + conn + |> put_resp_content_type("text/html") + |> send_resp(:forbidden, "Not #{body}") + |> halt() + end + end +end diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex index 7abd42a22..c4759045f 100644 --- a/lib/cadet_web/router.ex +++ b/lib/cadet_web/router.ex @@ -18,8 +18,8 @@ defmodule CadetWeb.Router do plug(Guardian.Plug.EnsureAuthenticated) end - pipeline :ensure_admin do - plug(CadetWeb.Plug.CheckAdmin) + pipeline :ensure_admin_staff do + plug(CadetWeb.Plug.EnsureRoles, %{roles: [:admin, :staff]}) end # Public Pages @@ -39,7 +39,7 @@ defmodule CadetWeb.Router do # Admin Pages scope "/admin", CadetWeb do - pipe_through([:browser, :auth, :ensure_auth, :ensure_admin]) + pipe_through([:browser, :auth, :ensure_auth, :ensure_admin_staff]) get("/", AdminController, :index) end diff --git a/lib/cadet_web/templates/layout/app.navbar.html.eex b/lib/cadet_web/templates/layout/app.navbar.html.eex index 51d24847b..cf9a5ec87 100644 --- a/lib/cadet_web/templates/layout/app.navbar.html.eex +++ b/lib/cadet_web/templates/layout/app.navbar.html.eex @@ -20,7 +20,7 @@ Playground - <%= if is_admin?(@conn) do %> + <%= if is_roles?(@conn, [:admin, :staff]) do %> <%= button "Admin", to: admin_path(@conn, :index), method: "get", class: "pt-button pt-minimal pt-icon-settings" %> <% end %> diff --git a/lib/cadet_web/views/view_helpers.ex b/lib/cadet_web/views/view_helpers.ex index aa6c5b614..9b653fd19 100644 --- a/lib/cadet_web/views/view_helpers.ex +++ b/lib/cadet_web/views/view_helpers.ex @@ -8,9 +8,9 @@ defmodule CadetWeb.ViewHelpers do conn.assigns[:current_user] != nil end - def is_admin?(conn) do + def is_roles?(conn, roles) do if logged_in?(conn) do - conn.assigns[:current_user].role == :admin + conn.assigns[:current_user].role in roles else false end diff --git a/test/cadet_web/plug/check_admin_test.exs b/test/cadet_web/plug/check_admin_test.exs deleted file mode 100644 index f1f247979..000000000 --- a/test/cadet_web/plug/check_admin_test.exs +++ /dev/null @@ -1,18 +0,0 @@ -defmodule CadetWeb.Plug.CheckAdminTest do - use CadetWeb.ConnCase - - alias CadetWeb.Plug.AssignCurrentUser - alias CadetWeb.Plug.CheckAdmin - - test "init" do - CheckAdmin.init(%{}) - # nothing to test - end - - @tag authenticate: :student - test "logged in as student", %{conn: conn} do - conn = AssignCurrentUser.call(conn, %{}) - conn = CheckAdmin.call(conn, %{}) - assert html_response(conn, 403) =~ "Not admin" - end -end diff --git a/test/cadet_web/plug/ensure_roles_test.exs b/test/cadet_web/plug/ensure_roles_test.exs new file mode 100644 index 000000000..86d3cca9a --- /dev/null +++ b/test/cadet_web/plug/ensure_roles_test.exs @@ -0,0 +1,32 @@ +defmodule CadetWeb.Plug.EnsureRolesTest do + use CadetWeb.ConnCase + + alias CadetWeb.Plug.AssignCurrentUser + alias CadetWeb.Plug.EnsureRoles + + test "init" do + EnsureRoles.init(%{}) + # nothing to test + end + + @tag authenticate: :student + test "logged in as student", %{conn: conn} do + conn = AssignCurrentUser.call(conn, %{}) + conn = EnsureRoles.call(conn, %{roles: [:admin, :staff]}) + assert html_response(conn, 403) =~ "Not admin/staff" + end + + @tag authenticate: :staff + test "logged in as staff", %{conn: conn} do + conn = AssignCurrentUser.call(conn, %{}) + conn = EnsureRoles.call(conn, %{roles: [:admin, :staff]}) + refute conn.status # conn.status is not set yet + end + + @tag authenticate: :admin + test "logged in as admin", %{conn: conn} do + conn = AssignCurrentUser.call(conn, %{}) + conn = EnsureRoles.call(conn, %{roles: [:admin, :staff]}) + refute conn.status # conn.status is not set yet + end +end