From 1b612feb1650de3ec575dc1412ef7d7940dd9db6 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Mon, 19 Sep 2016 09:10:22 -0500 Subject: [PATCH 01/16] add brakeman security scanner to guard --- Gemfile | 2 ++ Gemfile.lock | 6 ++++++ Guardfile | 10 +++++++++- config/brakeman.ignore | 10 ++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 config/brakeman.ignore diff --git a/Gemfile b/Gemfile index 0bed934..f2a509c 100644 --- a/Gemfile +++ b/Gemfile @@ -34,8 +34,10 @@ end group :development, :test do gem 'awesome_print' gem 'binding_of_caller' + gem 'brakeman' gem 'byebug', platform: :mri gem 'guard' + gem 'guard-brakeman' gem 'guard-livereload' gem 'guard-minitest' gem 'guard-rubocop' diff --git a/Gemfile.lock b/Gemfile.lock index 8d3e6f0..e9e37a6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,6 +53,7 @@ GEM bourbon (4.2.7) sass (~> 3.4) thor (~> 0.19) + brakeman (3.4.0) builder (3.2.2) byebug (9.0.5) choice (0.2.0) @@ -86,6 +87,9 @@ GEM pry (>= 0.9.12) shellany (~> 0.0) thor (>= 0.18.1) + guard-brakeman (0.8.3) + brakeman (>= 2.1.1) + guard (>= 2.0.0) guard-compat (1.2.1) guard-livereload (2.5.2) em-websocket (~> 0.5) @@ -287,9 +291,11 @@ DEPENDENCIES better_errors binding_of_caller bourbon + brakeman byebug figaro (~> 1.1.1) guard + guard-brakeman guard-livereload guard-minitest guard-rubocop diff --git a/Guardfile b/Guardfile index 704d52c..9beb07c 100644 --- a/Guardfile +++ b/Guardfile @@ -78,8 +78,16 @@ guard :shell, all_on_start: true do end end -guard :rubocop do +guard :rubocop, cli: %w(-D -S) do + watch(/.rubocop.yml/) watch(/.+\.rb$/) watch(/Rakefile/) watch(%r{(?:.+/)?\.rubocop\.yml$}) { |m| File.dirname(m[0]) } end + +guard 'brakeman', run_on_start: true, quiet: true do + watch(%r{^app/.+\.(erb|haml|rhtml|rb)$}) + watch(%r{^config/.+\.rb$}) + watch(%r{^lib/.+\.rb$}) + watch('Gemfile') +end diff --git a/config/brakeman.ignore b/config/brakeman.ignore new file mode 100644 index 0000000..3348c50 --- /dev/null +++ b/config/brakeman.ignore @@ -0,0 +1,10 @@ +{ + "ignored_warnings": [ + { + "fingerprint": "da17225c940987e6239cc4ecfe27bcb1e5da2db1134435dc3e1025d97927e0ba", + "note": "false positive" + } + ], + "updated": "2016-09-19 09:06:25 -0500", + "brakeman_version": "3.4.0" +} From 3c45527a0458b3ed2b21eb6861fa4144f744a195 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Mon, 19 Sep 2016 12:59:12 -0500 Subject: [PATCH 02/16] add in manager role --- app/helpers/application_helper.rb | 1 + test/fixtures/users.yml | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e6138c5..11f160f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -19,6 +19,7 @@ module ApplicationHelper options_for_select([ %w(Reviewer reviewer), %w(Recruiter recruiter), + %w(Manager manager), %w(Admin admin) ], disabled: "-", selected: (val.blank? ? '' : val)) end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 9c007a3..94e21e3 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -18,6 +18,12 @@ reviewer2: password_digest: <%= BCrypt::Password.create("password", cost: 4) %> role: reviewer +manager: + name: Mary Manager + email: mary.manager@mailinator.com + password_digest: <%= BCrypt::Password.create("password", cost: 4) %> + role: manager + admin: name: Alan Admin email: alan.admin@mailinator.com From 0a69eb578eb8ffeb60b9e5eba68725edb464fb85 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Mon, 19 Sep 2016 14:25:17 -0500 Subject: [PATCH 03/16] current_admin to current_user => prep for pundit --- app/controllers/admin/auth_controller.rb | 8 ++++---- app/controllers/admin/profile_controller.rb | 4 ++-- app/controllers/admin_controller.rb | 13 ++++++------- app/controllers/concerns/.keep | 0 app/models/concerns/.keep | 0 app/views/admin/profile/view.html.erb | 6 +++--- test/controllers/admin/auth_controller_test.rb | 10 ++++------ 7 files changed, 19 insertions(+), 22 deletions(-) delete mode 100644 app/controllers/concerns/.keep delete mode 100644 app/models/concerns/.keep diff --git a/app/controllers/admin/auth_controller.rb b/app/controllers/admin/auth_controller.rb index f063302..6c8f07f 100644 --- a/app/controllers/admin/auth_controller.rb +++ b/app/controllers/admin/auth_controller.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true module Admin class AuthController < AdminController - skip_before_action :authorize_admin + skip_before_action :authorize_user def login end def auth - admin = User.find_by(email: auth_params[:email], role: 'admin') + user = User.find_by(email: auth_params[:email]) - if admin && admin.authenticate(auth_params[:password]) - session[:user] = admin.to_i + if user && user.authenticate(auth_params[:password]) + session[:user] = user.to_i redirect_to admin_path else redirect_to admin_login_path, diff --git a/app/controllers/admin/profile_controller.rb b/app/controllers/admin/profile_controller.rb index 0894a36..27b9761 100644 --- a/app/controllers/admin/profile_controller.rb +++ b/app/controllers/admin/profile_controller.rb @@ -5,11 +5,11 @@ module Admin end def edit - @user = current_admin + @user = current_user end def update - @user = current_admin + @user = current_user if @user.update_attributes(user_params) redirect_to admin_profile_path, diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 27eec24..39b9f06 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,22 +1,21 @@ # frozen_string_literal: true class AdminController < ApplicationController layout 'admin' - before_action :authorize_admin + before_action :authorize_user def dashboard @quizzes = Quiz.includes(:questions).all @users = User.order(:role, :name) end - def current_admin - user_args = { id: session[:user], role: 'admin' } - @current_admin ||= User.find_by(user_args) if session[:user] + def current_user + @current_user ||= User.find_by(id: session[:user]) if session[:user] end - helper_method :current_admin + helper_method :current_user private - def authorize_admin - redirect_to admin_login_path unless current_admin + def authorize_user + redirect_to admin_login_path unless current_user end end diff --git a/app/controllers/concerns/.keep b/app/controllers/concerns/.keep deleted file mode 100644 index e69de29..0000000 diff --git a/app/models/concerns/.keep b/app/models/concerns/.keep deleted file mode 100644 index e69de29..0000000 diff --git a/app/views/admin/profile/view.html.erb b/app/views/admin/profile/view.html.erb index 92ff9b1..1c1af5f 100644 --- a/app/views/admin/profile/view.html.erb +++ b/app/views/admin/profile/view.html.erb @@ -2,7 +2,7 @@ content_for :section_title, "Profile" %> -

Name: <%= current_admin.name %>

-

email: <%= current_admin.email %>

-

Role: <%= current_admin.role %>

+

Name: <%= current_user.name %>

+

email: <%= current_user.email %>

+

Role: <%= current_user.role %>

<%= link_to('Edit', admin_edit_profile_path, { class: 'btn' }) %> diff --git a/test/controllers/admin/auth_controller_test.rb b/test/controllers/admin/auth_controller_test.rb index 671f42a..c3cb40c 100644 --- a/test/controllers/admin/auth_controller_test.rb +++ b/test/controllers/admin/auth_controller_test.rb @@ -26,18 +26,16 @@ module Admin assert_redirected_to admin_url end - test "recruiter should not admin auth" do + test "recruiter should auth to dashboard" do post admin_auth_url, params: { auth: { email: 'pdr.recruiter@mailinator.com', password: 'password' } } - assert_redirected_to admin_login_url - assert_match(/incorrect.*email/, flash[:error]) + assert_redirected_to admin_url end - test "reviewer should not admin auth" do + test "reviewer should auth to dashboard" do post admin_auth_url, params: { auth: { email: 'fed.reviewer@mailinator.com', password: 'password' } } - assert_redirected_to admin_login_url - assert_match(/incorrect.*email/, flash[:error]) + assert_redirected_to admin_url end test "should get reset_request" do From 12c7e9e77c14645b501152d90822af33071ca116 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Mon, 19 Sep 2016 16:40:56 -0500 Subject: [PATCH 04/16] basic admin policy start --- .rubocop.yml | 4 ++++ Gemfile | 2 ++ Gemfile.lock | 7 +++++++ app/controllers/admin_controller.rb | 9 +++++++++ app/policies/admin_policy.rb | 31 +++++++++++++++++++++++++++++ test/policies/admin_policy_test.rb | 10 ++++++++++ test/test_helper.rb | 1 + 7 files changed, 64 insertions(+) create mode 100644 app/policies/admin_policy.rb create mode 100644 test/policies/admin_policy_test.rb diff --git a/.rubocop.yml b/.rubocop.yml index f6f0f20..ddbc53a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -38,6 +38,10 @@ Style/SpaceBeforeFirstArg: Style/StringLiterals: Enabled: false +Style/StructInheritance: + Exclude: + - app/policies/**/* + Metrics/AbcSize: Exclude: - db/migrate/**/* diff --git a/Gemfile b/Gemfile index f2a509c..8a1a259 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,7 @@ gem 'jquery-rails' gem 'json', '~> 2.0.2' gem 'mailjet', '~> 1.3.8' gem 'puma', '~> 3.0' +gem 'pundit' gem 'sass-rails', '~> 5.0' gem 'settingslogic', '~> 2.0.9' gem 'turbolinks', '~> 5' @@ -44,6 +45,7 @@ group :development, :test do gem 'guard-shell' gem 'listen', '~> 3.0' gem 'minitest-reporters' + gem 'policy-assertions' gem 'pry-byebug' gem 'pry-rails' gem 'rails-controller-testing' diff --git a/Gemfile.lock b/Gemfile.lock index e9e37a6..248ef2c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -161,6 +161,9 @@ GEM parser (2.3.1.2) ast (~> 2.2) pkg-config (1.1.7) + policy-assertions (0.0.3) + activesupport (>= 3.0.0) + pundit (>= 1.0.0) powerpack (0.1.1) premailer (1.8.7) css_parser (>= 1.4.5) @@ -178,6 +181,8 @@ GEM pry-rails (0.3.4) pry (>= 0.9.10) puma (3.6.0) + pundit (1.1.0) + activesupport (>= 3.0.0) rack (2.0.1) rack-livereload (0.3.16) rack @@ -309,10 +314,12 @@ DEPENDENCIES minitest-reporters mysql2 (>= 0.3.18, < 0.5) neat + policy-assertions premailer-rails pry-byebug pry-rails puma (~> 3.0) + pundit rack-livereload rails (~> 5.0, >= 5.0.0.1) rails-controller-testing diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 39b9f06..423782b 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,9 +1,13 @@ # frozen_string_literal: true class AdminController < ApplicationController + include Pundit layout 'admin' before_action :authorize_user + rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized + def dashboard + authorize :admin, :dashboard? @quizzes = Quiz.includes(:questions).all @users = User.order(:role, :name) end @@ -18,4 +22,9 @@ class AdminController < ApplicationController def authorize_user redirect_to admin_login_path unless current_user end + + def user_not_authorized + flash[:error] = "You are not authorized to perform this action." + redirect_to(request.referer || root_path) + end end diff --git a/app/policies/admin_policy.rb b/app/policies/admin_policy.rb new file mode 100644 index 0000000..2644868 --- /dev/null +++ b/app/policies/admin_policy.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +class AdminPolicy < Struct.new(:user, :dashboard) + attr_reader :user, :record + + def initialize(user, record) + raise Pundit::NotAuthorizedError, "Must be logged in." unless user + @user = user + @record = record + end + + def dashboard? + true + end + + def scope + Pundit.policy_scope!(user, record.class) + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope + end + end +end diff --git a/test/policies/admin_policy_test.rb b/test/policies/admin_policy_test.rb new file mode 100644 index 0000000..00f8494 --- /dev/null +++ b/test/policies/admin_policy_test.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true +require 'test_helper' + +# TODO: How the heck to you test a headless policy?... +# +# class AdminPolicyTest < PolicyAssertions::Test +# def test_dashboard +# assert_permit users(:recruiter), Admin +# end +# end diff --git a/test/test_helper.rb b/test/test_helper.rb index 2eca63e..01cdb2c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -13,6 +13,7 @@ require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' require "minitest/autorun" require 'minitest/reporters' +require 'policy_assertions' Dir[Rails.root.join("test/test_helpers/**/*.rb")].each { |f| require f } Minitest::Reporters.use! [Minitest::Reporters::DefaultReporter.new(color: true)] From ead9564fe8b8ee310702579d31b7c3ed75e454df Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Tue, 20 Sep 2016 14:22:20 -0500 Subject: [PATCH 05/16] user policies --- app/controllers/admin/user_controller.rb | 7 ++- app/controllers/admin_controller.rb | 6 ++- app/models/user.rb | 6 +++ app/policies/application_policy.rb | 60 ++++++++++++++++++++++++ app/policies/user_policy.rb | 21 +++++++++ test/policies/application_policy_test.rb | 30 ++++++++++++ test/policies/user_policy_test.rb | 31 ++++++++++++ test/test_helper.rb | 1 + 8 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 app/policies/application_policy.rb create mode 100644 app/policies/user_policy.rb create mode 100644 test/policies/application_policy_test.rb create mode 100644 test/policies/user_policy_test.rb diff --git a/app/controllers/admin/user_controller.rb b/app/controllers/admin/user_controller.rb index f344909..3b37e55 100644 --- a/app/controllers/admin/user_controller.rb +++ b/app/controllers/admin/user_controller.rb @@ -2,14 +2,16 @@ module Admin class UserController < AdminController def index - @users = User.order(:name) + @users = policy_scope User.order(:name) end def new @user = User.new + authorize @user end def create + authorize User default_passwd = SecureRandom.urlsafe_base64(12) @user = User.create({ password: default_passwd }.merge(user_params.to_h)) @@ -24,14 +26,17 @@ module Admin def view @user = User.find(params[:user_id]) + authorize @user end def edit @user = User.find(params[:user_id]) + authorize @user end def update @user = User.find(params[:user_id]) + authorize @user if @user.update_attributes(user_params) redirect_to admin_user_path(@user.to_i), diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 423782b..0deebc0 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -4,8 +4,12 @@ class AdminController < ApplicationController layout 'admin' before_action :authorize_user + # TODO: after_action :verify_authorized, except: :index + # TODO: after_action :verify_policy_scoped, only: :index + rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized + # TODO: move to DashboardController#index def dashboard authorize :admin, :dashboard? @quizzes = Quiz.includes(:questions).all @@ -25,6 +29,6 @@ class AdminController < ApplicationController def user_not_authorized flash[:error] = "You are not authorized to perform this action." - redirect_to(request.referer || root_path) + redirect_to(request.referer || admin_login_path) end end diff --git a/app/models/user.rb b/app/models/user.rb index f57a07d..ec706e5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,6 +15,12 @@ class User < ApplicationRecord save end + # TODO: move to mixin: UserRoles + # define remaining helpers + def admin? + role == 'admin' + end + private def gen_reset_token diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb new file mode 100644 index 0000000..5e0c857 --- /dev/null +++ b/app/policies/application_policy.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true +class ApplicationPolicy + attr_reader :user, :record + + def initialize(user, record) + raise Pundit::NotAuthorizedError, "Must be logged in." unless user + @user = user + @record = record + end + + def index? + false + end + + def show? + scope.where(id: record.id).exists? + end + + def view? + show? + end + + def create? + false + end + + def new? + create? + end + + def update? + false + end + + def edit? + update? + end + + def destroy? + false + end + + def scope + Pundit.policy_scope!(user, record.class) + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + # This is a closed system. + raise Pundit::NotAuthorizedError, "No access to resource." + end + end +end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 0000000..ef5c64a --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +class UserPolicy < ApplicationPolicy + def view? + user.admin? && show? + end + + def create? + user.admin? + end + + def update? + user.admin? + end + + class Scope < Scope + def resolve + return scope if user.admin? + raise Pundit::NotAuthorizedError, "No access to resource." + end + end +end diff --git a/test/policies/application_policy_test.rb b/test/policies/application_policy_test.rb new file mode 100644 index 0000000..66337a9 --- /dev/null +++ b/test/policies/application_policy_test.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true +require 'test_helper' + +class ApplicationPolicyTest < PolicyAssertions::Test + # Verify default policies are most restrictive + + test 'should require a user' do + assert_raise Pundit::NotAuthorizedError do + ApplicationPolicy.new(nil, User.new) + end + end + + test 'should not allow collections' do + assert_raise Pundit::NotAuthorizedError do + ApplicationPolicy::Scope.new(users(:admin), User).resolve + end + end + + test 'should not permit by default' do + admin = users(:admin) + refute ApplicationPolicy.new(admin, User.new).view? + refute ApplicationPolicy.new(admin, User.new).show? + refute ApplicationPolicy.new(admin, nil).index? + refute ApplicationPolicy.new(admin, nil).create? + refute ApplicationPolicy.new(admin, nil).new? + refute ApplicationPolicy.new(admin, nil).update? + refute ApplicationPolicy.new(admin, nil).edit? + refute ApplicationPolicy.new(admin, nil).destroy? + end +end diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb new file mode 100644 index 0000000..88d3413 --- /dev/null +++ b/test/policies/user_policy_test.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +require 'test_helper' + +class UserPolicyTest < PolicyAssertions::Test + test 'should allow admin to scope' do + scope = UserPolicy::Scope.new(users(:admin), User).resolve + assert_equal User.count, scope.count + end + + test 'should not allow non_admin' do + assert_raise Pundit::NotAuthorizedError do + UserPolicy::Scope.new(users(:manager), User).resolve + end + end + + test 'should require current_user' do + assert_raise Pundit::NotAuthorizedError do + UserPolicy.new(nil, User.first).view? + end + end + + def test_view + refute_permit users(:manager), User.first + assert_permit users(:admin), User.first + end + + def test_create_and_update + refute_permit users(:manager), User + assert_permit users(:admin), User + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 01cdb2c..7b7fb5b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -7,6 +7,7 @@ SimpleCov.start 'rails' do add_group 'Models', %w(app/models app/validators) add_group 'Services & Workers', %w(app/workers app/services) add_group "Jobs", 'app/jobs' + add_group "Policies", 'app/policies' end require File.expand_path('../../config/environment', __FILE__) From 13610edcd14c819b2000b306ea74285645ec1267 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Tue, 20 Sep 2016 17:19:11 -0500 Subject: [PATCH 06/16] quiz policies --- app/controllers/admin/quiz_controller.rb | 7 +++- app/models/user.rb | 18 ++++++++- app/policies/quiz_policy.rb | 31 ++++++++++++++++ app/policies/user_policy.rb | 4 ++ test/fixtures/quizzes.yml | 5 +++ test/policies/quiz_policy_test.rb | 47 ++++++++++++++++++++++++ test/policies/user_policy_test.rb | 10 ++++- 7 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 app/policies/quiz_policy.rb create mode 100644 test/policies/quiz_policy_test.rb diff --git a/app/controllers/admin/quiz_controller.rb b/app/controllers/admin/quiz_controller.rb index 758516f..4ca5fbb 100644 --- a/app/controllers/admin/quiz_controller.rb +++ b/app/controllers/admin/quiz_controller.rb @@ -2,14 +2,16 @@ module Admin class QuizController < AdminController def index - @quizzes = Quiz.all + @quizzes = policy_scope Quiz.all end def new @quiz = Quiz.new + authorize @quiz end def create + authorize Quiz @quiz = Quiz.create(quiz_params) if @quiz.persisted? @@ -22,14 +24,17 @@ module Admin def view @quiz = Quiz.find(params[:quiz_id]) + authorize @quiz end def edit @quiz = Quiz.find(params[:quiz_id]) + authorize @quiz end def update @quiz = Quiz.find(params[:quiz_id]) + authorize @quiz if @quiz.update_attributes(quiz_params) redirect_to admin_quiz_path(@quiz.to_i), diff --git a/app/models/user.rb b/app/models/user.rb index ec706e5..c20f17e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -16,9 +16,23 @@ class User < ApplicationRecord end # TODO: move to mixin: UserRoles - # define remaining helpers def admin? - role == 'admin' + 'admin' == role + end + + # TODO: move to mixin: UserRoles + def manager? + %w(admin manager).include? role + end + + # TODO: move to mixin: UserRoles + def recruiter? + 'recruiter' == role + end + + # TODO: move to mixin: UserRoles + def reviewer? + 'reviewer' == role end private diff --git a/app/policies/quiz_policy.rb b/app/policies/quiz_policy.rb new file mode 100644 index 0000000..49505e8 --- /dev/null +++ b/app/policies/quiz_policy.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +class QuizPolicy < ApplicationPolicy + # Quiz Access Policy + # + # Only Admins and Managers can create or update a quiz (and its questions) + # Reviewers can view any quiz they are linked to + # Recruiters can only list quiz names (for candidate assignments) + + def view? + return true if user.admin? || user.manager? + user.quizzes.include? record + end + + def create? + user.manager? || user.admin? + end + + def update? + user.manager? || user.admin? + end + + class Scope < Scope + def resolve + if user.reviewer? + scope.joins(:reviewers).where('reviewer_to_quizzes.user_id = ?', user.id) + else + scope + end + end + end +end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index ef5c64a..61be4d4 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true class UserPolicy < ApplicationPolicy + # User Access Policy + # + # Only Admins can view, create, or update, users + def view? user.admin? && show? end diff --git a/test/fixtures/quizzes.yml b/test/fixtures/quizzes.yml index daf1e5d..f4bd5fa 100644 --- a/test/fixtures/quizzes.yml +++ b/test/fixtures/quizzes.yml @@ -4,3 +4,8 @@ fed: name: PDR Standard FED Screening unit: PDR dept: FED + +admin: + name: An extra quiz not assigned to anyone + unit: PDR + dept: NOPE diff --git a/test/policies/quiz_policy_test.rb b/test/policies/quiz_policy_test.rb new file mode 100644 index 0000000..e5eb7be --- /dev/null +++ b/test/policies/quiz_policy_test.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true +require 'test_helper' + +class QuizPolicyTest < PolicyAssertions::Test + test 'should require current_user' do + assert_raise Pundit::NotAuthorizedError do + QuizPolicy.new(nil, Quiz.first).view? + end + end + + test 'should allow admin to scope' do + scope = QuizPolicy::Scope.new(users(:admin), Quiz).resolve + assert_equal Quiz.count, scope.count + end + + test 'should allow manager to scope' do + scope = QuizPolicy::Scope.new(users(:manager), Quiz).resolve + assert_equal Quiz.count, scope.count + end + + test 'should allow reviewer to scope' do + scope = QuizPolicy::Scope.new(users(:reviewer), Quiz).resolve + assert_equal users(:reviewer).quizzes.count, scope.count + end + + test 'should allow recruiter to scope' do + scope = QuizPolicy::Scope.new(users(:recruiter), Quiz).resolve + assert_equal Quiz.count, scope.count + end + + def test_view + assert_permit users(:admin), quizzes(:fed) + assert_permit users(:manager), quizzes(:fed) + assert_permit users(:reviewer), quizzes(:fed) + + refute_permit users(:reviewer), quizzes(:admin) + refute_permit users(:recruiter), quizzes(:fed) + end + + def test_create_and_update + assert_permit users(:admin), Quiz + assert_permit users(:manager), Quiz + + refute_permit users(:recruiter), Quiz + refute_permit users(:reviewer), Quiz + end +end diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index 88d3413..4a9096c 100644 --- a/test/policies/user_policy_test.rb +++ b/test/policies/user_policy_test.rb @@ -20,12 +20,18 @@ class UserPolicyTest < PolicyAssertions::Test end def test_view - refute_permit users(:manager), User.first assert_permit users(:admin), User.first + + refute_permit users(:manager), User.first + refute_permit users(:reviewer), User.first + refute_permit users(:recruiter), User.first end def test_create_and_update - refute_permit users(:manager), User assert_permit users(:admin), User + + refute_permit users(:manager), User + refute_permit users(:reviewer), User + refute_permit users(:recruiter), User end end From 8ad98215c1ba5a10daaf96433daa6c2d039c60b8 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Tue, 20 Sep 2016 18:17:27 -0500 Subject: [PATCH 07/16] question policies --- app/controllers/admin/question_controller.rb | 19 ++++++-- app/controllers/admin/quiz_controller.rb | 2 +- app/policies/question_policy.rb | 38 +++++++++++++++ test/fixtures/questions.yml | 10 ++++ test/policies/question_policy_test.rb | 50 ++++++++++++++++++++ test/policies/user_policy_test.rb | 12 ++--- 6 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 app/policies/question_policy.rb create mode 100644 test/policies/question_policy_test.rb diff --git a/app/controllers/admin/question_controller.rb b/app/controllers/admin/question_controller.rb index c3b74f4..1dfbb84 100644 --- a/app/controllers/admin/question_controller.rb +++ b/app/controllers/admin/question_controller.rb @@ -2,16 +2,20 @@ module Admin class QuestionController < AdminController def index - @questions = Question.includes(:quiz).order("quizzes.name", { active: :desc }, :sort) + @questions = policy_scope Question.includes(:quiz).order("quizzes.name", { active: :desc }, :sort) end def new + authorize Question + @question = Question.new(active: true) - @quizzes = Quiz.all + @quizzes = policy_scope Quiz.all end def create - @quizzes = Quiz.all + authorize Quiz + + @quizzes = policy_scope Quiz.all @question = Question.create(process_question_params) if @question.persisted? @@ -24,16 +28,20 @@ module Admin def view @question = Question.includes(:quiz).find(params[:question_id]) + authorize @question end def edit - @quizzes = Quiz.all + @quizzes = policy_scope Quiz.all @question = Question.includes(:quiz).find(params[:question_id]) + + authorize @question end def update - @quizzes = Quiz.all + @quizzes = policy_scope Quiz.all @question = Question.find(params[:question_id]) + authorize @question if @question.update_attributes(process_question_params) redirect_to admin_question_path(@question.to_i), @@ -46,6 +54,7 @@ module Admin def options @question = params[:question_id].present? ? Question.find(params[:question_id]) : Question.new + authorize @question render layout: false end diff --git a/app/controllers/admin/quiz_controller.rb b/app/controllers/admin/quiz_controller.rb index 4ca5fbb..9d1d5ef 100644 --- a/app/controllers/admin/quiz_controller.rb +++ b/app/controllers/admin/quiz_controller.rb @@ -6,8 +6,8 @@ module Admin end def new + authorize Quiz @quiz = Quiz.new - authorize @quiz end def create diff --git a/app/policies/question_policy.rb b/app/policies/question_policy.rb new file mode 100644 index 0000000..2c64165 --- /dev/null +++ b/app/policies/question_policy.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +class QuestionPolicy < ApplicationPolicy + # Question Access Policy + # + # Only Admins and Managers can create or update a quiz (and its questions) + # Reviewers can view any quiz they are linked to + # Recruiters can NOT list or view questions + + def view? + return false if user.recruiter? + return true if user.admin? || user.manager? + user.quizzes.include? record.quiz + end + + def create? + user.manager? || user.admin? + end + + def update? + user.manager? || user.admin? + end + + def options? + !user.recruiter? + end + + class Scope < Scope + def resolve + raise(Pundit::NotAuthorizedError, 'No Access to resource.') if user.recruiter? + + if user.admin? || user.manager? + scope + else + scope.where(quiz_id: user.quizzes.map(&:id)) + end + end + end +end diff --git a/test/fixtures/questions.yml b/test/fixtures/questions.yml index 6460c5f..b498ff2 100644 --- a/test/fixtures/questions.yml +++ b/test/fixtures/questions.yml @@ -111,3 +111,13 @@ fed10: - "wibbly wobbly, timey wimey" sort: 9 active: true + +admin1: + quiz: admin + question: 'You have a question you want to ask.' + category: Admin + input_type: text + input_options: + sort: 0 + active: true + diff --git a/test/policies/question_policy_test.rb b/test/policies/question_policy_test.rb new file mode 100644 index 0000000..28d63c8 --- /dev/null +++ b/test/policies/question_policy_test.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true +require 'test_helper' + +class QuestionPolicyTest < PolicyAssertions::Test + test 'should require current_user' do + assert_raise Pundit::NotAuthorizedError do + QuestionPolicy.new(nil, Question.first).view? + end + end + + test 'should allow admin to scope' do + scope = QuestionPolicy::Scope.new(users(:admin), Question).resolve + assert_equal Question.count, scope.count + end + + test 'should allow manager to scope' do + scope = QuestionPolicy::Scope.new(users(:manager), Question).resolve + assert_equal Question.count, scope.count + end + + test 'should allow reviewer to scope' do + quiz_ids = users(:reviewer).quizzes.map(&:id) + + scope = QuestionPolicy::Scope.new(users(:reviewer), Question).resolve + assert_equal Question.where(quiz_id: quiz_ids).count, scope.count + end + + test 'should NOT allow recruiter to scope' do + assert_raise Pundit::NotAuthorizedError do + QuestionPolicy::Scope.new(users(:recruiter), Question).resolve + end + end + + def test_view + assert_permit users(:admin), questions(:fed1) + assert_permit users(:manager), questions(:fed1) + assert_permit users(:reviewer), questions(:fed1) + + refute_permit users(:reviewer), questions(:admin1) + refute_permit users(:recruiter), questions(:fed1) + end + + def test_create_and_update + assert_permit users(:admin), Question + assert_permit users(:manager), Question + + refute_permit users(:recruiter), Question + refute_permit users(:reviewer), Question + end +end diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index 4a9096c..e2514d4 100644 --- a/test/policies/user_policy_test.rb +++ b/test/policies/user_policy_test.rb @@ -2,6 +2,12 @@ require 'test_helper' class UserPolicyTest < PolicyAssertions::Test + test 'should require current_user' do + assert_raise Pundit::NotAuthorizedError do + UserPolicy.new(nil, User.first).view? + end + end + test 'should allow admin to scope' do scope = UserPolicy::Scope.new(users(:admin), User).resolve assert_equal User.count, scope.count @@ -13,12 +19,6 @@ class UserPolicyTest < PolicyAssertions::Test end end - test 'should require current_user' do - assert_raise Pundit::NotAuthorizedError do - UserPolicy.new(nil, User.first).view? - end - end - def test_view assert_permit users(:admin), User.first From 75a4fbf71a1a5d4250a8bd251e795a62fc24ce04 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Wed, 21 Sep 2016 11:03:45 -0500 Subject: [PATCH 08/16] user policy to allow profile edits --- app/controllers/admin/profile_controller.rb | 3 + app/controllers/admin_controller.rb | 4 +- app/policies/user_policy.rb | 5 +- test/policies/user_policy_test.rb | 87 +++++++++++++++++---- 4 files changed, 81 insertions(+), 18 deletions(-) diff --git a/app/controllers/admin/profile_controller.rb b/app/controllers/admin/profile_controller.rb index 27b9761..ef118f7 100644 --- a/app/controllers/admin/profile_controller.rb +++ b/app/controllers/admin/profile_controller.rb @@ -2,14 +2,17 @@ module Admin class ProfileController < AdminController def view + authorize current_user end def edit @user = current_user + authorize @user end def update @user = current_user + authorize @user if @user.update_attributes(user_params) redirect_to admin_profile_path, diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 0deebc0..d8a0b69 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -4,8 +4,8 @@ class AdminController < ApplicationController layout 'admin' before_action :authorize_user - # TODO: after_action :verify_authorized, except: :index - # TODO: after_action :verify_policy_scoped, only: :index + # after_action :verify_authorized, except: :index + # after_action :verify_policy_scoped, only: :index rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 61be4d4..dad8c7f 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -3,9 +3,10 @@ class UserPolicy < ApplicationPolicy # User Access Policy # # Only Admins can view, create, or update, users + # All other users can only access themselves (profile interface) def view? - user.admin? && show? + user.admin? || user == record end def create? @@ -13,7 +14,7 @@ class UserPolicy < ApplicationPolicy end def update? - user.admin? + user.admin? || user == record end class Scope < Scope diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index e2514d4..2bd3b54 100644 --- a/test/policies/user_policy_test.rb +++ b/test/policies/user_policy_test.rb @@ -13,25 +13,84 @@ class UserPolicyTest < PolicyAssertions::Test assert_equal User.count, scope.count end - test 'should not allow non_admin' do - assert_raise Pundit::NotAuthorizedError do - UserPolicy::Scope.new(users(:manager), User).resolve + test 'should not allow non_admin to scope' do + %i(manager reviewer recruiter).each do |role| + assert_raise Pundit::NotAuthorizedError, "Failed to raise auth error for #{role}" do + UserPolicy::Scope.new(users(role), User).resolve + end end end - def test_view - assert_permit users(:admin), User.first - - refute_permit users(:manager), User.first - refute_permit users(:reviewer), User.first - refute_permit users(:recruiter), User.first + # view? + test 'admin can view any user role' do + assert_permit users(:admin), users(:admin), 'view?' + assert_permit users(:admin), users(:manager), 'view?' + assert_permit users(:admin), users(:reviewer), 'view?' + assert_permit users(:admin), users(:recruiter), 'view?' end - def test_create_and_update - assert_permit users(:admin), User + test 'manager can only view herself' do + assert_permit users(:manager), users(:manager), 'view?' - refute_permit users(:manager), User - refute_permit users(:reviewer), User - refute_permit users(:recruiter), User + refute_permit users(:manager), users(:admin), 'view?' + refute_permit users(:manager), users(:reviewer), 'view?' + refute_permit users(:manager), users(:recruiter), 'view?' + end + + test 'reviewer can only view herself' do + assert_permit users(:reviewer), users(:reviewer), 'view?' + + refute_permit users(:reviewer), users(:admin), 'view?' + refute_permit users(:reviewer), users(:manager), 'view?' + refute_permit users(:reviewer), users(:recruiter), 'view?' + end + + test 'recruiter can only view herself' do + assert_permit users(:recruiter), users(:recruiter), 'view?' + + refute_permit users(:recruiter), users(:admin), 'view?' + refute_permit users(:recruiter), users(:manager), 'view?' + refute_permit users(:recruiter), users(:reviewer), 'view?' + end + + # update? + test 'admin can update any user role' do + assert_permit users(:admin), users(:admin), 'update?' + assert_permit users(:admin), users(:manager), 'update?' + assert_permit users(:admin), users(:reviewer), 'update?' + assert_permit users(:admin), users(:recruiter), 'update?' + end + + test 'manager can only update herself' do + assert_permit users(:manager), users(:manager), 'update?' + + refute_permit users(:manager), users(:admin), 'update?' + refute_permit users(:manager), users(:reviewer), 'update?' + refute_permit users(:manager), users(:recruiter), 'update?' + end + + test 'reupdateer can only update herself' do + assert_permit users(:reviewer), users(:reviewer), 'update?' + + refute_permit users(:reviewer), users(:admin), 'update?' + refute_permit users(:reviewer), users(:manager), 'update?' + refute_permit users(:reviewer), users(:recruiter), 'update?' + end + + test 'recruiter can only update herself' do + assert_permit users(:recruiter), users(:recruiter), 'update?' + + refute_permit users(:recruiter), users(:admin), 'update?' + refute_permit users(:recruiter), users(:manager), 'update?' + refute_permit users(:recruiter), users(:reviewer), 'update?' + end + + # create + test 'only admin can create users' do + assert_permit users(:admin), User, 'create?' + + refute_permit users(:manager), User, 'create?' + refute_permit users(:reviewer), User, 'create?' + refute_permit users(:recruiter), User, 'create?' end end From d3205a60806c9ecdbe426623127dcf293fbcce6a Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Wed, 21 Sep 2016 15:20:43 -0500 Subject: [PATCH 09/16] auth controller needs no pundit --- app/controllers/admin/auth_controller.rb | 4 ++++ app/controllers/admin_controller.rb | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/auth_controller.rb b/app/controllers/admin/auth_controller.rb index 6c8f07f..afc039f 100644 --- a/app/controllers/admin/auth_controller.rb +++ b/app/controllers/admin/auth_controller.rb @@ -3,6 +3,10 @@ module Admin class AuthController < AdminController skip_before_action :authorize_user + # bypass pundit lockdowns for auth requests. + after_action :skip_policy_scope + after_action :skip_authorization + def login end diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index d8a0b69..b152c8e 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -4,8 +4,8 @@ class AdminController < ApplicationController layout 'admin' before_action :authorize_user - # after_action :verify_authorized, except: :index - # after_action :verify_policy_scoped, only: :index + after_action :verify_authorized, except: :index + after_action :verify_policy_scoped, only: :index rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized From 4a70b795e5feccfa5981567129eaef6507dd5542 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Wed, 21 Sep 2016 15:50:02 -0500 Subject: [PATCH 10/16] policy strong_params --- app/controllers/admin/profile_controller.rb | 2 +- app/controllers/admin/user_controller.rb | 2 +- app/policies/user_policy.rb | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/profile_controller.rb b/app/controllers/admin/profile_controller.rb index ef118f7..b176ac2 100644 --- a/app/controllers/admin/profile_controller.rb +++ b/app/controllers/admin/profile_controller.rb @@ -26,7 +26,7 @@ module Admin private def user_params - params.require(:user).permit(:name, :email, :password, :password_confirmation) + params.require(:user).permit(policy(User).permitted_attributes) end end end diff --git a/app/controllers/admin/user_controller.rb b/app/controllers/admin/user_controller.rb index 3b37e55..d8ab69a 100644 --- a/app/controllers/admin/user_controller.rb +++ b/app/controllers/admin/user_controller.rb @@ -50,7 +50,7 @@ module Admin private def user_params - params.require(:user).permit(:name, :email, :role, :password, quiz_ids: []) + params.require(:user).permit(policy(User).permitted_attributes) end end end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index dad8c7f..5a1140e 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -17,6 +17,11 @@ class UserPolicy < ApplicationPolicy user.admin? || user == record end + def permitted_attributes + return [:name, :email, :role, :password, quiz_ids: []] if user.admin? + [:name, :email, :password, :password_confirmation] + end + class Scope < Scope def resolve return scope if user.admin? From 7774a1e3f27886ddf3466c7d4a21ffae69325daa Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Wed, 21 Sep 2016 17:04:08 -0500 Subject: [PATCH 11/16] dashboard controller --- app/controllers/admin/dashboard_controller.rb | 10 ++++++ app/controllers/admin_controller.rb | 11 ++---- app/models/user.rb | 5 +-- app/policies/admin_policy.rb | 31 ---------------- app/policies/dashboard_policy.rb | 14 ++++++++ app/policies/quiz_policy.rb | 4 +++ app/policies/user_policy.rb | 6 +++- app/views/admin/dashboard.html.erb | 15 -------- app/views/admin/dashboard/show.html.erb | 35 +++++++++++++++++++ config/routes.rb | 2 +- .../admin/dashboard_controller_test.rb | 18 ++++++++++ test/controllers/admin_controller_test.rb | 11 ------ test/policies/user_policy_test.rb | 8 ++--- 13 files changed, 94 insertions(+), 76 deletions(-) create mode 100644 app/controllers/admin/dashboard_controller.rb delete mode 100644 app/policies/admin_policy.rb create mode 100644 app/policies/dashboard_policy.rb delete mode 100644 app/views/admin/dashboard.html.erb create mode 100644 app/views/admin/dashboard/show.html.erb create mode 100644 test/controllers/admin/dashboard_controller_test.rb diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb new file mode 100644 index 0000000..94c7f00 --- /dev/null +++ b/app/controllers/admin/dashboard_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true +module Admin + class DashboardController < AdminController + def show + authorize :dashboard + @quizzes = policy_scope Quiz.includes(:questions).all + @users = policy_scope User.order(:role, :name) + end + end +end diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index b152c8e..2c2daf6 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -4,17 +4,10 @@ class AdminController < ApplicationController layout 'admin' before_action :authorize_user - after_action :verify_authorized, except: :index - after_action :verify_policy_scoped, only: :index - rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized - # TODO: move to DashboardController#index - def dashboard - authorize :admin, :dashboard? - @quizzes = Quiz.includes(:questions).all - @users = User.order(:role, :name) - end + after_action :verify_authorized, except: :index + after_action :verify_policy_scoped, only: :index def current_user @current_user ||= User.find_by(id: session[:user]) if session[:user] diff --git a/app/models/user.rb b/app/models/user.rb index c20f17e..a44ec5b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,22 +15,19 @@ class User < ApplicationRecord save end - # TODO: move to mixin: UserRoles + # Roles def admin? 'admin' == role end - # TODO: move to mixin: UserRoles def manager? %w(admin manager).include? role end - # TODO: move to mixin: UserRoles def recruiter? 'recruiter' == role end - # TODO: move to mixin: UserRoles def reviewer? 'reviewer' == role end diff --git a/app/policies/admin_policy.rb b/app/policies/admin_policy.rb deleted file mode 100644 index 2644868..0000000 --- a/app/policies/admin_policy.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true -class AdminPolicy < Struct.new(:user, :dashboard) - attr_reader :user, :record - - def initialize(user, record) - raise Pundit::NotAuthorizedError, "Must be logged in." unless user - @user = user - @record = record - end - - def dashboard? - true - end - - def scope - Pundit.policy_scope!(user, record.class) - end - - class Scope - attr_reader :user, :scope - - def initialize(user, scope) - @user = user - @scope = scope - end - - def resolve - scope - end - end -end diff --git a/app/policies/dashboard_policy.rb b/app/policies/dashboard_policy.rb new file mode 100644 index 0000000..1ba609e --- /dev/null +++ b/app/policies/dashboard_policy.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class DashboardPolicy < Struct.new(:user, :dashboard) + attr_reader :user, :record + + def initialize(user, record) + raise Pundit::NotAuthorizedError, "Must be logged in." unless user + @user = user + @record = record + end + + def show? + true + end +end diff --git a/app/policies/quiz_policy.rb b/app/policies/quiz_policy.rb index 49505e8..4b2dc56 100644 --- a/app/policies/quiz_policy.rb +++ b/app/policies/quiz_policy.rb @@ -6,6 +6,10 @@ class QuizPolicy < ApplicationPolicy # Reviewers can view any quiz they are linked to # Recruiters can only list quiz names (for candidate assignments) + def index? + true + end + def view? return true if user.admin? || user.manager? user.quizzes.include? record diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 5a1140e..c02a137 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -5,6 +5,10 @@ class UserPolicy < ApplicationPolicy # Only Admins can view, create, or update, users # All other users can only access themselves (profile interface) + def index? + user.admin? + end + def view? user.admin? || user == record end @@ -25,7 +29,7 @@ class UserPolicy < ApplicationPolicy class Scope < Scope def resolve return scope if user.admin? - raise Pundit::NotAuthorizedError, "No access to resource." + scope.where(id: user.id) end end end diff --git a/app/views/admin/dashboard.html.erb b/app/views/admin/dashboard.html.erb deleted file mode 100644 index 9939146..0000000 --- a/app/views/admin/dashboard.html.erb +++ /dev/null @@ -1,15 +0,0 @@ -<% - content_for :section_title, "Admin Dashboard" -%> - -
-

Quizzes

- <%= render partial: 'admin/quiz/table_list', locals: { quizzes: @quizzes } %> - <%= link_to('New Quiz', admin_new_quiz_path, { class: 'btn' }) %> -
- -
-

Users

- <%= render partial: 'admin/user/table_list', locals: { users: @users } %> - <%= link_to('New User', admin_new_user_path, { class: 'btn' }) %> -
diff --git a/app/views/admin/dashboard/show.html.erb b/app/views/admin/dashboard/show.html.erb new file mode 100644 index 0000000..ee1284a --- /dev/null +++ b/app/views/admin/dashboard/show.html.erb @@ -0,0 +1,35 @@ +<% + content_for :section_title, "Admin Dashboard" +%> + +
+## Admin
+Users | Dept/Unit | Quizzes | Candidates | Results | Profile | Logout
+
+## Manager
+Quizzes | Results | Profile | Logout
+
+## Recruiter
+Results | Profile | Logout
+
+## Reviewer
+Candidates | Profile | Logout
+
+ +<% if policy(Quiz).index? %> +
+

Quizzes

+ <%= render partial: 'admin/quiz/table_list', locals: { quizzes: @quizzes } %> + <%= link_to('New Quiz', admin_new_quiz_path, { class: 'btn' }) %> +
+<% end %> + +<% if policy(User).index? %> +
+

Users

+ <%= render partial: 'admin/user/table_list', locals: { users: @users } %> + <%= link_to('New User', admin_new_user_path, { class: 'btn' }) %> +
+<% end %> + + diff --git a/config/routes.rb b/config/routes.rb index 0a9f7e8..41c8002 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,7 +37,7 @@ Rails.application.routes.draw do post "/admin/profile", to: "admin/profile#update", as: :admin_update_profile get "/admin/profile/edit", to: "admin/profile#edit", as: :admin_edit_profile - get "/admin", to: "admin#dashboard", as: :admin + get "/admin", to: "admin/dashboard#show", as: :admin ######################################################################################### diff --git a/test/controllers/admin/dashboard_controller_test.rb b/test/controllers/admin/dashboard_controller_test.rb new file mode 100644 index 0000000..9b36041 --- /dev/null +++ b/test/controllers/admin/dashboard_controller_test.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +require 'test_helper' + +module Admin + class DashboardControllerTest < ActionDispatch::IntegrationTest + test "dashboard should require auth" do + get admin_url + assert_redirected_to admin_login_url + end + + test "should get dashboard" do + post admin_auth_url, params: { auth: + { email: 'alan.admin@mailinator.com', password: 'password' } } + get admin_url + assert_response :success + end + end +end diff --git a/test/controllers/admin_controller_test.rb b/test/controllers/admin_controller_test.rb index 56df4d6..60da372 100644 --- a/test/controllers/admin_controller_test.rb +++ b/test/controllers/admin_controller_test.rb @@ -2,15 +2,4 @@ require 'test_helper' class AdminControllerTest < ActionDispatch::IntegrationTest - test "dashboard should require auth" do - get admin_url - assert_redirected_to admin_login_url - end - - test "should get dashboard" do - post admin_auth_url, params: { auth: - { email: 'alan.admin@mailinator.com', password: 'password' } } - get admin_url - assert_response :success - end end diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index 2bd3b54..98f0dd3 100644 --- a/test/policies/user_policy_test.rb +++ b/test/policies/user_policy_test.rb @@ -13,11 +13,11 @@ class UserPolicyTest < PolicyAssertions::Test assert_equal User.count, scope.count end - test 'should not allow non_admin to scope' do + test 'non admins can only scope themselves' do %i(manager reviewer recruiter).each do |role| - assert_raise Pundit::NotAuthorizedError, "Failed to raise auth error for #{role}" do - UserPolicy::Scope.new(users(role), User).resolve - end + scope = UserPolicy::Scope.new(users(role), User).resolve + assert_equal 1, scope.count, "Scope did not have 1 result for #{role}" + assert_equal users(role), scope.first, "Scope did not contain self for #{role}" end end From 47d7188a2f06b7b335b91dad61ea4e8dad2959bb Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Thu, 22 Sep 2016 09:07:06 -0500 Subject: [PATCH 12/16] stop duplicate minitest runs --- Gemfile | 20 ++++++++++++-------- Gemfile.lock | 2 +- Guardfile | 12 ++++++++---- test/test_helper.rb | 1 - 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/Gemfile b/Gemfile index 8a1a259..e80ee10 100644 --- a/Gemfile +++ b/Gemfile @@ -33,26 +33,30 @@ group :development do end group :development, :test do - gem 'awesome_print' - gem 'binding_of_caller' - gem 'brakeman' - gem 'byebug', platform: :mri + gem 'spring' + gem 'spring-watcher-listen', '~> 2.0.0' + gem 'listen' + gem 'guard' gem 'guard-brakeman' gem 'guard-livereload' gem 'guard-minitest' gem 'guard-rubocop' gem 'guard-shell' - gem 'listen', '~> 3.0' + gem 'minitest-reporters' + gem 'rails-controller-testing' gem 'policy-assertions' + + gem 'byebug', platform: :mri gem 'pry-byebug' gem 'pry-rails' - gem 'rails-controller-testing' + gem 'binding_of_caller' + gem 'awesome_print' + gem 'rubocop', '~> 0.42.0' gem 'simplecov', require: false - gem 'spring' - gem 'spring-watcher-listen', '~> 2.0.0' + gem 'brakeman' end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem diff --git a/Gemfile.lock b/Gemfile.lock index 248ef2c..589a2b6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -309,7 +309,7 @@ DEPENDENCIES jbuilder (~> 2.6) jquery-rails json (~> 2.0.2) - listen (~> 3.0) + listen mailjet (~> 1.3.8) minitest-reporters mysql2 (>= 0.3.18, < 0.5) diff --git a/Guardfile b/Guardfile index 9beb07c..7a847c9 100644 --- a/Guardfile +++ b/Guardfile @@ -72,6 +72,7 @@ end # ESLint guard :shell, all_on_start: true do + # TODO: Annoyingly, all files are linted twice on start/full runs. Why? watch %r{app/assets/javascripts/*/.*} do |file| system %(echo "ESLint:\033[32m #{file[0]}\033[0m") system %(eslint #{file[0]}) @@ -86,8 +87,11 @@ guard :rubocop, cli: %w(-D -S) do end guard 'brakeman', run_on_start: true, quiet: true do - watch(%r{^app/.+\.(erb|haml|rhtml|rb)$}) - watch(%r{^config/.+\.rb$}) - watch(%r{^lib/.+\.rb$}) - watch('Gemfile') + ## Lets not watch files for brakeman, + ## just scan on guard start, and full runs. + # + # watch(%r{^app/.+\.(erb|haml|rhtml|rb)$}) + # watch(%r{^config/.+\.rb$}) + # watch(%r{^lib/.+\.rb$}) + # watch('Gemfile') end diff --git a/test/test_helper.rb b/test/test_helper.rb index 7b7fb5b..65a1a33 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -12,7 +12,6 @@ end require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' -require "minitest/autorun" require 'minitest/reporters' require 'policy_assertions' Dir[Rails.root.join("test/test_helpers/**/*.rb")].each { |f| require f } From 9078c463f4287f2d504695fe6b7773f1513b51bb Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Thu, 22 Sep 2016 13:30:30 -0500 Subject: [PATCH 13/16] move recruiter to admin/candidate --- app/controllers/admin/candidate_controller.rb | 70 ++++++++++++++++ app/controllers/recruiter_controller.rb | 80 ------------------- app/models/user.rb | 16 ++++ app/policies/candidate_policy.rb | 33 ++++++++ app/policies/question_policy.rb | 16 ++-- app/policies/quiz_policy.rb | 6 +- app/policies/user_policy.rb | 12 +-- .../candidate}/_form.html.erb | 0 app/views/admin/candidate/edit.html.erb | 6 ++ .../candidate}/index.html.erb | 6 +- app/views/admin/candidate/new.html.erb | 6 ++ app/views/recruiter/edit.html.erb | 6 -- app/views/recruiter/login.html.erb | 21 ----- app/views/recruiter/new.html.erb | 6 -- config/routes.rb | 54 ++++++------- .../admin/candidate_controller/index_test.rb | 31 +++++++ .../new_candidate_test.rb | 73 +++++++++++++++++ .../update_candidate_test.rb | 36 +++++++++ .../recruiter_controller/index_test.rb | 56 ------------- .../new_candidate_test.rb | 71 ---------------- .../update_candidate_test.rb | 34 -------- test/controllers/review_controller_test.rb | 2 +- test/policies/candidate_policy_test.rb | 48 +++++++++++ test/policies/question_policy_test.rb | 2 +- test/test_helpers/auth_test_helper.rb | 19 ++++- 25 files changed, 383 insertions(+), 327 deletions(-) create mode 100644 app/controllers/admin/candidate_controller.rb delete mode 100644 app/controllers/recruiter_controller.rb create mode 100644 app/policies/candidate_policy.rb rename app/views/{recruiter => admin/candidate}/_form.html.erb (100%) create mode 100644 app/views/admin/candidate/edit.html.erb rename app/views/{recruiter => admin/candidate}/index.html.erb (71%) create mode 100644 app/views/admin/candidate/new.html.erb delete mode 100644 app/views/recruiter/edit.html.erb delete mode 100644 app/views/recruiter/login.html.erb delete mode 100644 app/views/recruiter/new.html.erb create mode 100644 test/controllers/admin/candidate_controller/index_test.rb create mode 100644 test/controllers/admin/candidate_controller/new_candidate_test.rb create mode 100644 test/controllers/admin/candidate_controller/update_candidate_test.rb delete mode 100644 test/controllers/recruiter_controller/index_test.rb delete mode 100644 test/controllers/recruiter_controller/new_candidate_test.rb delete mode 100644 test/controllers/recruiter_controller/update_candidate_test.rb create mode 100644 test/policies/candidate_policy_test.rb diff --git a/app/controllers/admin/candidate_controller.rb b/app/controllers/admin/candidate_controller.rb new file mode 100644 index 0000000..6eaf61f --- /dev/null +++ b/app/controllers/admin/candidate_controller.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true +module Admin + class CandidateController < AdminController + before_action :collect_quizzes, except: [:login, :auth] + + def index + @candidates = policy_scope current_recruiter.candidates + end + + def new + authorize Candidate + @candidate = Candidate.new + render :new + end + + def create + authorize Candidate + @candidate = Candidate.create(candidate_params.merge(recruiter_id: current_recruiter.id)) + + if @candidate.persisted? + send_notifications @candidate + redirect_to admin_candidate_path, + flash: { success: "Sucessfully created candidate #{@candidate.name}" } + else + flash[:error] = "Failed to save candidate." + render :new + end + end + + def edit + authorize Candidate + @candidate = Candidate.find_by(id: params[:id]) + end + + def update + authorize Candidate + @candidate = Candidate.find_by(id: params[:id]) + @candidate.update(candidate_params) + + if @candidate.save + redirect_to admin_candidate_path, flash: { success: "#{@candidate.name} updated!" } + else + flash[:error] = "Failed to save candidate." + render :edit + end + end + + def resend_welcome + authorize Candidate + candidate = Candidate.find_by(id: params[:id]) + CandidateMailer.welcome(candidate).deliver_later + render json: { message: "Email queued!" }.to_json + end + + private + + def candidate_params + params.require(:candidate).permit(:name, :email, :experience, :quiz_id) + end + + def collect_quizzes + @quizzes ||= Quiz.order(:name) + end + + def send_notifications candidate + CandidateMailer.welcome(candidate).deliver_later + RecruiterMailer.candidate_created(candidate).deliver_later + end + end +end diff --git a/app/controllers/recruiter_controller.rb b/app/controllers/recruiter_controller.rb deleted file mode 100644 index a8b32cb..0000000 --- a/app/controllers/recruiter_controller.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true -class RecruiterController < ApplicationController - before_action :authorize_recruiter, except: [:login, :auth] - before_action :collect_quizzes, except: [:login, :auth] - - def index - @candidates = current_recruiter.candidates - end - - def new - @candidate = Candidate.new - render :new - end - - def create - @candidate = Candidate.create(candidate_params.merge(recruiter_id: current_recruiter.id)) - - if @candidate.persisted? - CandidateMailer.welcome(@candidate).deliver_later - RecruiterMailer.candidate_created(@candidate).deliver_later - redirect_to recruiter_path, flash: { success: "Sucessfully created candidate #{@candidate.name}" } - else - flash[:error] = "Failed to save candidate." - render :new - end - end - - def edit - @candidate = Candidate.find_by(id: params[:id]) - end - - def update - @candidate = Candidate.find_by(id: params[:id]) - @candidate.update(candidate_params) - - if @candidate.save - redirect_to recruiter_path, flash: { success: "#{@candidate.name} updated!" } - else - flash[:error] = "Failed to save candidate." - render :edit - end - end - - def login - redirect_to recruiter_path unless current_recruiter.nil? - end - - def auth - recruiter = User.find_by(email: auth_params[:email], role: %w(admin recruiter)) - - if recruiter && recruiter.authenticate(auth_params[:password]) - session[:user] = recruiter.to_i - redirect_to recruiter_path - else - redirect_to recruiter_login_path, - flash: { error: "Sorry, incorrect email or password. Please try again." } - end - end - - def logout - reset_session - redirect_to recruiter_login_path - end - - def resend_welcome - candidate = Candidate.find_by(id: params[:id]) - CandidateMailer.welcome(candidate).deliver_later - render json: { message: "Email queued!" }.to_json - end - - private - - def candidate_params - params.require(:candidate).permit(:name, :email, :experience, :quiz_id) - end - - def collect_quizzes - @quizzes ||= Quiz.order(:name) - end -end diff --git a/app/models/user.rb b/app/models/user.rb index a44ec5b..35ed228 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -20,7 +20,15 @@ class User < ApplicationRecord 'admin' == role end + def acts_as_admin? + 'admin' == role + end + def manager? + 'manager' == role + end + + def acts_as_manager? %w(admin manager).include? role end @@ -28,10 +36,18 @@ class User < ApplicationRecord 'recruiter' == role end + def acts_as_recruiter? + %w(admin recruiter).include? role + end + def reviewer? 'reviewer' == role end + def acts_as_reviewer? + %w(admin reviewer).include? role + end + private def gen_reset_token diff --git a/app/policies/candidate_policy.rb b/app/policies/candidate_policy.rb new file mode 100644 index 0000000..4e6418f --- /dev/null +++ b/app/policies/candidate_policy.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true +class CandidatePolicy < ApplicationPolicy + # Candidate Access Policy + # + # Only Recruiters and Admins can view, create, or update, candidates + + def index? + user.acts_as_recruiter? + end + + def view? + user.acts_as_recruiter? + end + + def create? + user.acts_as_recruiter? + end + + def update? + user.acts_as_recruiter? + end + + def resend_welcome? + user.acts_as_recruiter? + end + + class Scope < Scope + def resolve + return scope if user.acts_as_recruiter? + raise Pundit::NotAuthorizedError, "No Access to Resource" + end + end +end diff --git a/app/policies/question_policy.rb b/app/policies/question_policy.rb index 2c64165..f578844 100644 --- a/app/policies/question_policy.rb +++ b/app/policies/question_policy.rb @@ -8,27 +8,27 @@ class QuestionPolicy < ApplicationPolicy def view? return false if user.recruiter? - return true if user.admin? || user.manager? + return true if user.acts_as_manager? user.quizzes.include? record.quiz end + def options? + view? + end + def create? - user.manager? || user.admin? + user.acts_as_manager? end def update? - user.manager? || user.admin? - end - - def options? - !user.recruiter? + user.acts_as_manager? end class Scope < Scope def resolve raise(Pundit::NotAuthorizedError, 'No Access to resource.') if user.recruiter? - if user.admin? || user.manager? + if user.acts_as_manager? scope else scope.where(quiz_id: user.quizzes.map(&:id)) diff --git a/app/policies/quiz_policy.rb b/app/policies/quiz_policy.rb index 4b2dc56..6f80113 100644 --- a/app/policies/quiz_policy.rb +++ b/app/policies/quiz_policy.rb @@ -11,16 +11,16 @@ class QuizPolicy < ApplicationPolicy end def view? - return true if user.admin? || user.manager? + return true if user.acts_as_manager? user.quizzes.include? record end def create? - user.manager? || user.admin? + user.acts_as_manager? end def update? - user.manager? || user.admin? + user.acts_as_manager? end class Scope < Scope diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index c02a137..00a81f5 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -6,29 +6,29 @@ class UserPolicy < ApplicationPolicy # All other users can only access themselves (profile interface) def index? - user.admin? + user.acts_as_admin? end def view? - user.admin? || user == record + user.acts_as_admin? || user == record end def create? - user.admin? + user.acts_as_admin? end def update? - user.admin? || user == record + user.acts_as_admin? || user == record end def permitted_attributes - return [:name, :email, :role, :password, quiz_ids: []] if user.admin? + return [:name, :email, :role, :password, quiz_ids: []] if user.acts_as_admin? [:name, :email, :password, :password_confirmation] end class Scope < Scope def resolve - return scope if user.admin? + return scope if user.acts_as_admin? scope.where(id: user.id) end end diff --git a/app/views/recruiter/_form.html.erb b/app/views/admin/candidate/_form.html.erb similarity index 100% rename from app/views/recruiter/_form.html.erb rename to app/views/admin/candidate/_form.html.erb diff --git a/app/views/admin/candidate/edit.html.erb b/app/views/admin/candidate/edit.html.erb new file mode 100644 index 0000000..5de8973 --- /dev/null +++ b/app/views/admin/candidate/edit.html.erb @@ -0,0 +1,6 @@ +
+

Edit: <%= @candidate.name %>

+

Test ID: <%= @candidate.test_hash %>

+ + <%= render partial: 'form', locals: { action: admin_update_candidate_path(@candidate.id), candidate: @candidate, quizzes: @quizzes } %> +
diff --git a/app/views/recruiter/index.html.erb b/app/views/admin/candidate/index.html.erb similarity index 71% rename from app/views/recruiter/index.html.erb rename to app/views/admin/candidate/index.html.erb index c7fc407..e6095fc 100644 --- a/app/views/recruiter/index.html.erb +++ b/app/views/admin/candidate/index.html.erb @@ -1,7 +1,7 @@

Candidates

- <%= link_to(new_candidate_path, { class: 'secondary-btn' }) do %> + <%= link_to(admin_new_candidate_path, { class: 'secondary-btn' }) do %> <% end %> @@ -18,12 +18,12 @@ <% @candidates.each do |candidate| %> - <%= link_to candidate.name, edit_candidate_path(candidate.id) %> + <%= link_to candidate.name, admin_edit_candidate_path(candidate.id) %> <%= candidate.test_hash %> <%= mail_to(candidate.email) %>
- <%= link_to "resend welcome email", resend_welcome_path(candidate.id), remote: true, class: '', data: { id: 'ajax-action' } %> + <%= link_to "resend welcome email", admin_resend_welcome_path(candidate.id), remote: true, class: '', data: { id: 'ajax-action' } %> <%= candidate.experience %> years <%= candidate.status %> diff --git a/app/views/admin/candidate/new.html.erb b/app/views/admin/candidate/new.html.erb new file mode 100644 index 0000000..c529467 --- /dev/null +++ b/app/views/admin/candidate/new.html.erb @@ -0,0 +1,6 @@ +
+

New Candidate

+ + <%= render partial: 'form', locals: + { action: admin_create_candidate_path, candidate: @candidate, quizzes: @quizzes } %> +
diff --git a/app/views/recruiter/edit.html.erb b/app/views/recruiter/edit.html.erb deleted file mode 100644 index dc6aff7..0000000 --- a/app/views/recruiter/edit.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -
-

Edit: <%= @candidate.name %>

-

Test ID: <%= @candidate.test_hash %>

- - <%= render partial: 'form', locals: { action: update_candidate_path(@candidate.id), candidate: @candidate, quizzes: @quizzes } %> -
diff --git a/app/views/recruiter/login.html.erb b/app/views/recruiter/login.html.erb deleted file mode 100644 index 9d4098d..0000000 --- a/app/views/recruiter/login.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -
-

Recruiter Login

- - <% if flash[:error].present? %> -
<%= flash[:error] %>
- <% end %> - - <%= form_for :auth, url: recruiter_login_path do |form| %> -
- <%= form.label :email %> - <%= form.email_field :email %> -
- -
- <%= form.label :password %> - <%= form.password_field :password %> -
- - <%= submit_tag "Log in" %> - <% end %> -
diff --git a/app/views/recruiter/new.html.erb b/app/views/recruiter/new.html.erb deleted file mode 100644 index 7892256..0000000 --- a/app/views/recruiter/new.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -
-

New Candidate

- - <%= render partial: 'form', locals: - { action: create_candidate_path, candidate: @candidate, quizzes: @quizzes } %> -
diff --git a/config/routes.rb b/config/routes.rb index 41c8002..1fd272b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true Rails.application.routes.draw do + get "/styleguide", to: "application#styleguide", as: :styleguide + get "/admin/styleguide", to: "application#styleguide" + post "/admin/login", to: "admin/auth#auth", as: :admin_auth get "/admin/login", to: "admin/auth#login", as: :admin_login get "/admin/logout", to: "admin/auth#logout", as: :admin_logout @@ -33,26 +36,16 @@ Rails.application.routes.draw do post "/admin/question/:question_id/edit", to: "admin/question#update", as: :admin_update_question patch "/admin/question/:question_id/edit", to: "admin/question#update" - get "/admin/profile", to: "admin/profile#view", as: :admin_profile - post "/admin/profile", to: "admin/profile#update", as: :admin_update_profile - get "/admin/profile/edit", to: "admin/profile#edit", as: :admin_edit_profile + get "/admin/profile", to: "admin/profile#view", as: :admin_profile + post "/admin/profile", to: "admin/profile#update", as: :admin_update_profile + get "/admin/profile/edit", to: "admin/profile#edit", as: :admin_edit_profile - get "/admin", to: "admin/dashboard#show", as: :admin - - ######################################################################################### - - post "/validate", to: "candidate#validate", as: :validate_candidate - get "/login(/:test_id)", to: "candidate#login", as: :login - get "/welcome", to: "candidate#welcome", as: :welcome - get "/saved", to: "candidate#saved", as: :saved - get "/thankyou", to: "candidate#thankyou", as: :thankyou - - get "/oops", to: "candidate#oops", as: :oops - - post "/question(/:answer_id)", to: "quiz#update_answer", as: :post_answer - get "/question(/:question_id)", to: "quiz#question", as: :question - post "/summary", to: "quiz#submit_summary", as: :post_summary - get "/summary", to: "quiz#summary", as: :summary + get "/admin/candidate", to: "admin/candidate#index", as: :admin_candidate + get "/admin/candidate/new", to: "admin/candidate#new", as: :admin_new_candidate + post "/admin/candidate/new", to: "admin/candidate#create", as: :admin_create_candidate + get "/admin/candidate/:id", to: "admin/candidate#edit", as: :admin_edit_candidate + post "/admin/candidate/:id", to: "admin/candidate#update", as: :admin_update_candidate + get "/admin/candidate/:id/resend", to: "admin/candidate#resend_welcome", as: :admin_resend_welcome get "/review/logout", to: "review#logout", as: :review_logout post "/review/login", to: "review#auth", as: :review_auth @@ -60,18 +53,21 @@ Rails.application.routes.draw do get "/review", to: "review#index", as: :review get "/review/:test_hash", to: "review#view", as: :review_test - get "/resend/welcome/:id", to: "recruiter#resend_welcome", as: :resend_welcome + get "/admin", to: "admin/dashboard#show", as: :admin - get "/recruiter", to: "recruiter#index", as: :recruiter - get "/recruiter/candidate", to: "recruiter#new", as: :new_candidate - post "/recruiter/candidate", to: "recruiter#create", as: :create_candidate - get "/recruiter/candidate/:id", to: "recruiter#edit", as: :edit_candidate - post "/recruiter/candidate/:id", to: "recruiter#update", as: :update_candidate - get "/recruiter/logout", to: "recruiter#logout", as: :recruiter_logout - get "/recruiter/login", to: "recruiter#login", as: :recruiter_login - post "/recruiter/login", to: "recruiter#auth", as: :recruiter_auth + ######################################################################################### - get "/styleguide", to: "application#styleguide", as: :styleguide + post "/validate", to: "candidate#validate", as: :validate_candidate + get "/login(/:test_id)", to: "candidate#login", as: :login + get "/welcome", to: "candidate#welcome", as: :welcome + get "/saved", to: "candidate#saved", as: :saved + get "/thankyou", to: "candidate#thankyou", as: :thankyou + get "/oops", to: "candidate#oops", as: :oops + + post "/question(/:answer_id)", to: "quiz#update_answer", as: :post_answer + get "/question(/:question_id)", to: "quiz#question", as: :question + post "/summary", to: "quiz#submit_summary", as: :post_summary + get "/summary", to: "quiz#summary", as: :summary root to: "candidate#login" diff --git a/test/controllers/admin/candidate_controller/index_test.rb b/test/controllers/admin/candidate_controller/index_test.rb new file mode 100644 index 0000000..d9c2b11 --- /dev/null +++ b/test/controllers/admin/candidate_controller/index_test.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +require 'test_helper' + +module Admin + class CandidateControllerTest < ActionDispatch::IntegrationTest + test "should require auth or redirect" do + get admin_candidate_url + assert_redirected_to admin_login_url + + get admin_new_candidate_url + assert_redirected_to admin_login_url + + post admin_create_candidate_url, params: { candidate: { name: 'foo', email: 'bar', experience: 'baz' } } + assert_redirected_to admin_login_url + end + + test "should get candidate list" do + auth_recruiter + get admin_candidate_url + assert_response :success + assert assigns(:candidates), "@candidates not present" + end + + test 'should have edit links' do + auth_recruiter + get admin_candidate_url + assert_response :success + assert_select "a[href='#{admin_edit_candidate_path(candidates(:martha))}']" + end + end +end diff --git a/test/controllers/admin/candidate_controller/new_candidate_test.rb b/test/controllers/admin/candidate_controller/new_candidate_test.rb new file mode 100644 index 0000000..50fb582 --- /dev/null +++ b/test/controllers/admin/candidate_controller/new_candidate_test.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true +require 'test_helper' + +module Admin + class CandidateControllerTest < ActionDispatch::IntegrationTest + include ActiveJob::TestHelper + + test "should get new" do + auth_recruiter + get admin_new_candidate_url + assert_response :success + assert assigns(:candidate), "@candidate not present" + end + + test "should get create" do + auth_recruiter + get admin_create_candidate_url + assert_response :success + end + + test "should create new candidate" do + auth_recruiter + + assert_enqueued_jobs 2 do + assert_difference("Candidate.count") do + post admin_create_candidate_path, params: { candidate: + { name: 'new name', email: 'test@mailinator.com', experience: '0-3', quiz_id: quizzes(:fed).id } } + end + end + assert_redirected_to admin_candidate_path + assert flash[:success] + end + + test "should fail creation with improper email format" do + auth_recruiter + + assert_enqueued_jobs 0 do + assert_difference("Candidate.count", 0) do + post admin_create_candidate_path, params: { candidate: + { name: 'new name', email: 'test@mailinatorcom', experience: '0-3', quiz_id: quizzes(:fed).id } } + end + end + assert :success + assert assigns(:candidate), "@candidate not present" + assert_match(/failed.*save/i, flash[:error]) + end + + test "should fail creation gracefully with empty email" do + auth_recruiter + + assert_enqueued_jobs 0 do + assert_difference("Candidate.count", 0) do + post admin_create_candidate_path, params: { candidate: + { name: 'new name', email: "", experience: '0-3', quiz_id: quizzes(:fed).id } } + end + end + assert :success + assert assigns(:candidate), "@candidate not present" + assert_match(/failed.*save/i, flash[:error]) + end + + test 'should queue up a welcome email [resend]' do + auth_recruiter + + assert_enqueued_jobs 1 do + get admin_resend_welcome_path(id: candidates(:peggy)), xhr: true + end + assert_response :success + data = JSON.parse(response.body) + assert_match 'queued', data["message"] + end + end +end diff --git a/test/controllers/admin/candidate_controller/update_candidate_test.rb b/test/controllers/admin/candidate_controller/update_candidate_test.rb new file mode 100644 index 0000000..25a6efc --- /dev/null +++ b/test/controllers/admin/candidate_controller/update_candidate_test.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true +require 'test_helper' + +module Admin + class CandidateControllerTest < ActionDispatch::IntegrationTest + test 'should edit candidate' do + auth_recruiter + candidate = candidates(:martha) + + get admin_edit_candidate_path(candidate.id) + assert_response :success + assert_select 'form' + end + + test 'should update candidate, but NOT test_hash' do + auth_recruiter + candidate = candidates(:martha) + post admin_update_candidate_url(id: candidate.id), params: + { candidate: { name: 'new name', email: "mail@martha.me", test_hash: 'SOMENEWSTRING' } } + + refute_equal candidate.name, Candidate.find_by(id: candidate.id).name + assert_equal candidate.test_hash, Candidate.find_by(id: candidate.id).test_hash + assert_redirected_to admin_candidate_url + end + + test 'should redirect to form on fail' do + auth_recruiter + candidate = candidates(:martha) + post admin_update_candidate_url(id: candidate.id), params: + { candidate: { name: 'new name', email: "mail@martha" } } + + assert :success + assert_match(/failed.*save/i, flash[:error]) + end + end +end diff --git a/test/controllers/recruiter_controller/index_test.rb b/test/controllers/recruiter_controller/index_test.rb deleted file mode 100644 index b6b4a7c..0000000 --- a/test/controllers/recruiter_controller/index_test.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true -require 'test_helper' - -class RecruiterControllerTest < ActionDispatch::IntegrationTest - test "should get login" do - get recruiter_login_url - assert_response :success - end - - test 'should logout and reset session' do - auth_recruiter - get recruiter_logout_path - - assert :success - assert session[:user].nil? - end - - test "should require auth or redirect" do - get recruiter_url - assert_redirected_to recruiter_login_path - - get new_candidate_url - assert_redirected_to recruiter_login_path - - post create_candidate_url, params: { candidate: { name: 'foo', email: 'bar', experience: 'baz' } } - assert_redirected_to recruiter_login_path - end - - test "should auth to index" do - auth_recruiter - assert_redirected_to recruiter_path - assert session[:user].present? - end - - test "should fail auth with flash" do - post recruiter_auth_url, params: { auth: - { email: 'pdr.recruiter@mailinator.com', password: 'bad-password' } } - - assert_redirected_to recruiter_login_path - assert flash[:error] - end - - test "should get candidate list" do - auth_recruiter - get recruiter_url - assert_response :success - assert assigns(:candidates), "@candidates not present" - end - - test 'should have edit links' do - auth_recruiter - get recruiter_url - assert_response :success - assert_select "a[href='#{edit_candidate_path(candidates(:martha))}']" - end -end diff --git a/test/controllers/recruiter_controller/new_candidate_test.rb b/test/controllers/recruiter_controller/new_candidate_test.rb deleted file mode 100644 index 458237b..0000000 --- a/test/controllers/recruiter_controller/new_candidate_test.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true -require 'test_helper' - -class RecruiterControllerTest < ActionDispatch::IntegrationTest - include ActiveJob::TestHelper - - test "should get new" do - auth_recruiter - get new_candidate_url - assert_response :success - assert assigns(:candidate), "@candidate not present" - end - - test "should get create" do - auth_recruiter - get create_candidate_url - assert_response :success - end - - test "should create new candidate" do - auth_recruiter - - assert_enqueued_jobs 2 do - assert_difference("Candidate.count") do - post create_candidate_path, params: { candidate: - { name: 'new name', email: 'test@mailinator.com', experience: '0-3', quiz_id: quizzes(:fed).id } } - end - end - assert_redirected_to recruiter_path - assert flash[:success] - end - - test "should fail creation with improper email format" do - auth_recruiter - - assert_enqueued_jobs 0 do - assert_difference("Candidate.count", 0) do - post create_candidate_path, params: { candidate: - { name: 'new name', email: 'test@mailinatorcom', experience: '0-3', quiz_id: quizzes(:fed).id } } - end - end - assert :success - assert assigns(:candidate), "@candidate not present" - assert_match(/failed.*save/i, flash[:error]) - end - - test "should fail creation gracefully with empty email" do - auth_recruiter - - assert_enqueued_jobs 0 do - assert_difference("Candidate.count", 0) do - post create_candidate_path, params: { candidate: - { name: 'new name', email: "", experience: '0-3', quiz_id: quizzes(:fed).id } } - end - end - assert :success - assert assigns(:candidate), "@candidate not present" - assert_match(/failed.*save/i, flash[:error]) - end - - test 'should queue up a welcome email [resend]' do - auth_recruiter - - assert_enqueued_jobs 1 do - get resend_welcome_path(id: candidates(:peggy)), xhr: true - end - assert_response :success - data = JSON.parse(response.body) - assert_match 'queued', data["message"] - end -end diff --git a/test/controllers/recruiter_controller/update_candidate_test.rb b/test/controllers/recruiter_controller/update_candidate_test.rb deleted file mode 100644 index 02a1dbe..0000000 --- a/test/controllers/recruiter_controller/update_candidate_test.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true -require 'test_helper' - -class RecruiterControllerTest < ActionDispatch::IntegrationTest - test 'should edit candidate' do - auth_recruiter - candidate = candidates(:martha) - - get edit_candidate_path(candidate.id) - assert_response :success - assert_select 'form' - end - - test 'should update candidate, but NOT test_hash' do - auth_recruiter - candidate = candidates(:martha) - post update_candidate_url(id: candidate.id), params: - { candidate: { name: 'new name', email: "mail@martha.me", test_hash: 'SOMENEWSTRING' } } - - refute_equal candidate.name, Candidate.find_by(id: candidate.id).name - assert_equal candidate.test_hash, Candidate.find_by(id: candidate.id).test_hash - assert_redirected_to recruiter_url - end - - test 'should redirect to form on fail' do - auth_recruiter - candidate = candidates(:martha) - post update_candidate_url(id: candidate.id), params: - { candidate: { name: 'new name', email: "mail@martha" } } - - assert :success - assert_match(/failed.*save/i, flash[:error]) - end -end diff --git a/test/controllers/review_controller_test.rb b/test/controllers/review_controller_test.rb index 5851678..453a629 100644 --- a/test/controllers/review_controller_test.rb +++ b/test/controllers/review_controller_test.rb @@ -17,7 +17,7 @@ class ReviewControllerTest < ActionDispatch::IntegrationTest test "should auth to index" do auth_reviewer - assert_redirected_to review_path + assert_redirected_to admin_path assert session[:user].present? end diff --git a/test/policies/candidate_policy_test.rb b/test/policies/candidate_policy_test.rb new file mode 100644 index 0000000..5124ec8 --- /dev/null +++ b/test/policies/candidate_policy_test.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true +require 'test_helper' + +class CandidatePolicyTest < PolicyAssertions::Test + test 'should require current_user' do + assert_raise Pundit::NotAuthorizedError do + CandidatePolicy.new(nil, Candidate.first).view? + end + end + + test 'should allow admin to scope' do + scope = CandidatePolicy::Scope.new(users(:admin), Candidate).resolve + assert_equal Candidate.count, scope.count + end + + test 'should allow recruiter to scope' do + scope = CandidatePolicy::Scope.new(users(:recruiter), Candidate).resolve + assert_equal Candidate.count, scope.count + end + + test 'reviewer CAN NOT scope candidates' do + assert_raise Pundit::NotAuthorizedError do + CandidatePolicy::Scope.new(users(:reviewer), Candidate).resolve + end + end + + test 'manager CAN NOT scope candidates' do + assert_raise Pundit::NotAuthorizedError do + CandidatePolicy::Scope.new(users(:manager), Candidate).resolve + end + end + + def test_view_and_update + assert_permit users(:admin), candidates(:roy) + assert_permit users(:recruiter), candidates(:roy) + + refute_permit users(:manager), candidates(:roy) + refute_permit users(:reviewer), candidates(:roy) + end + + def test_create + assert_permit users(:admin), Candidate + assert_permit users(:recruiter), Candidate + + refute_permit users(:manager), Candidate + refute_permit users(:reviewer), Candidate + end +end diff --git a/test/policies/question_policy_test.rb b/test/policies/question_policy_test.rb index 28d63c8..f8173db 100644 --- a/test/policies/question_policy_test.rb +++ b/test/policies/question_policy_test.rb @@ -31,7 +31,7 @@ class QuestionPolicyTest < PolicyAssertions::Test end end - def test_view + def test_view_and_options assert_permit users(:admin), questions(:fed1) assert_permit users(:manager), questions(:fed1) assert_permit users(:reviewer), questions(:fed1) diff --git a/test/test_helpers/auth_test_helper.rb b/test/test_helpers/auth_test_helper.rb index 7bddabe..152a7b6 100644 --- a/test/test_helpers/auth_test_helper.rb +++ b/test/test_helpers/auth_test_helper.rb @@ -4,13 +4,28 @@ module AuthTestHelper post validate_candidate_url, params: { test_id: candidate.test_hash } end + def auth_user user + post admin_auth_url, params: { auth: + { email: user.email, password: 'password' } } + end + + def auth_admin + post admin_auth_url, params: { auth: + { email: 'alan.admin@mailinator.com', password: 'password' } } + end + + def auth_manager + post admin_auth_url, params: { auth: + { email: 'mary.manager@mailinator.com', password: 'password' } } + end + def auth_recruiter - post recruiter_auth_url, params: { auth: + post admin_auth_url, params: { auth: { email: 'pdr.recruiter@mailinator.com', password: 'password' } } end def auth_reviewer - post review_auth_url, params: { auth: + post admin_auth_url, params: { auth: { email: 'fed.reviewer@mailinator.com', password: 'password' } } end end From 0a9bf96e243cb516199f9a61be2ed8ccd4927822 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Thu, 22 Sep 2016 14:19:44 -0500 Subject: [PATCH 14/16] move /review to /admin/results --- app/controllers/admin/result_controller.rb | 21 +++++++ app/controllers/review_controller.rb | 34 ---------- .../{review => admin/result}/index.html.erb | 2 +- .../{review => admin/result}/view.html.erb | 2 +- app/views/review/login.html.erb | 21 ------- .../candidate_submission.html.inky | 4 +- .../candidate_submission.text.erb | 2 +- config/routes.rb | 7 +-- .../admin/result_controller_test.rb | 23 +++++++ test/controllers/review_controller_test.rb | 63 ------------------- test/integration/question_attachments_test.rb | 5 +- 11 files changed, 53 insertions(+), 131 deletions(-) create mode 100644 app/controllers/admin/result_controller.rb delete mode 100644 app/controllers/review_controller.rb rename app/views/{review => admin/result}/index.html.erb (81%) rename app/views/{review => admin/result}/view.html.erb (94%) delete mode 100644 app/views/review/login.html.erb create mode 100644 test/controllers/admin/result_controller_test.rb delete mode 100644 test/controllers/review_controller_test.rb diff --git a/app/controllers/admin/result_controller.rb b/app/controllers/admin/result_controller.rb new file mode 100644 index 0000000..9d14ded --- /dev/null +++ b/app/controllers/admin/result_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +module Admin + class ResultController < AdminController + # + # TODO: change context from Candidate to Quiz + # bypass pundit lockdowns until completed + after_action :skip_policy_scope + after_action :skip_authorization + # + + def index + @candidates = Candidate.where(completed: true).includes(:recruiter) + end + + def view + @candidate = Candidate.find_by(test_hash: params[:test_hash]) + @quiz = @candidate.my_quiz + @status = QuizStatus.new(@candidate) + end + end +end diff --git a/app/controllers/review_controller.rb b/app/controllers/review_controller.rb deleted file mode 100644 index d810bad..0000000 --- a/app/controllers/review_controller.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true -class ReviewController < ApplicationController - before_action :authorize_reviewer, except: [:login, :auth] - - def index - @candidates = Candidate.where(completed: true).includes(:recruiter) - end - - def view - @candidate = Candidate.find_by(test_hash: params[:test_hash]) - @quiz = @candidate.my_quiz - @status = QuizStatus.new(@candidate) - end - - def login - redirect_to review_path unless current_reviewer.nil? - end - - def auth - reviewer = User.find_by(email: auth_params[:email], role: %w(admin reviewer)) - - if reviewer && reviewer.authenticate(auth_params[:password]) - session[:user] = reviewer.to_i - redirect_to review_path - else - redirect_to review_login_path, flash: { error: "Sorry, incorrect email or password. Please try again." } - end - end - - def logout - reset_session - redirect_to review_login_path - end -end diff --git a/app/views/review/index.html.erb b/app/views/admin/result/index.html.erb similarity index 81% rename from app/views/review/index.html.erb rename to app/views/admin/result/index.html.erb index 3601598..158605e 100644 --- a/app/views/review/index.html.erb +++ b/app/views/admin/result/index.html.erb @@ -10,7 +10,7 @@ <% @candidates.each do |candidate| %> - <%= link_to candidate.test_hash, review_test_path(candidate.test_hash) %> + <%= link_to candidate.test_hash, admin_result_path(candidate.test_hash) %> <%= candidate.experience %> years <%= mail_to(candidate.recruiter.email) %> diff --git a/app/views/review/view.html.erb b/app/views/admin/result/view.html.erb similarity index 94% rename from app/views/review/view.html.erb rename to app/views/admin/result/view.html.erb index 497199a..73a0f72 100644 --- a/app/views/review/view.html.erb +++ b/app/views/admin/result/view.html.erb @@ -27,7 +27,7 @@ <% end #form_tag %> <% end #questions loop %> - <%= link_to(review_path, { class: 'secondary-btn' }) do %> + <%= link_to(admin_results_path, { class: 'secondary-btn' }) do %> <% end %>
diff --git a/app/views/review/login.html.erb b/app/views/review/login.html.erb deleted file mode 100644 index 6d84f0f..0000000 --- a/app/views/review/login.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -
-

Reviewer Login

- - <% if flash[:error].present? %> -
<%= flash[:error] %>
- <% end %> - - <%= form_for :auth, url: review_login_path do |form| %> -
- <%= form.label :email %> - <%= form.email_field :email %> -
- -
- <%= form.label :password %> - <%= form.password_field :password %> -
- - <%= submit_tag "Login" %> - <% end %> -
diff --git a/app/views/reviewer_mailer/candidate_submission.html.inky b/app/views/reviewer_mailer/candidate_submission.html.inky index 4f61e26..1c5a20f 100644 --- a/app/views/reviewer_mailer/candidate_submission.html.inky +++ b/app/views/reviewer_mailer/candidate_submission.html.inky @@ -1,6 +1,6 @@ - \ No newline at end of file + diff --git a/app/views/reviewer_mailer/candidate_submission.text.erb b/app/views/reviewer_mailer/candidate_submission.text.erb index 7ca4a38..e81acc2 100644 --- a/app/views/reviewer_mailer/candidate_submission.text.erb +++ b/app/views/reviewer_mailer/candidate_submission.text.erb @@ -2,4 +2,4 @@ PERFICIENT/digital SKILLS ASSESSMENT RESULTS Candidate <%= @candidate.test_hash %> has completed the Skills Assessment Test. -You can view the results here: <%= review_test_url(@candidate.test_hash) %>. +You can view the results here: <%= admin_result_url(@candidate.test_hash) %>. diff --git a/config/routes.rb b/config/routes.rb index 1fd272b..2691c85 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -47,11 +47,8 @@ Rails.application.routes.draw do post "/admin/candidate/:id", to: "admin/candidate#update", as: :admin_update_candidate get "/admin/candidate/:id/resend", to: "admin/candidate#resend_welcome", as: :admin_resend_welcome - get "/review/logout", to: "review#logout", as: :review_logout - post "/review/login", to: "review#auth", as: :review_auth - get "/review/login", to: "review#login", as: :review_login - get "/review", to: "review#index", as: :review - get "/review/:test_hash", to: "review#view", as: :review_test + get "/admin/results", to: "admin/result#index", as: :admin_results + get "/admin/result/:test_hash", to: "admin/result#view", as: :admin_result get "/admin", to: "admin/dashboard#show", as: :admin diff --git a/test/controllers/admin/result_controller_test.rb b/test/controllers/admin/result_controller_test.rb new file mode 100644 index 0000000..699517d --- /dev/null +++ b/test/controllers/admin/result_controller_test.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +require 'test_helper' + +module Admin + class ResultControllerTest < ActionDispatch::IntegrationTest + test "should get results list" do + auth_reviewer + get admin_results_url + assert_response :success + assert assigns(:candidates), '@candidates not present' + end + + test "should get view" do + auth_reviewer + + get admin_result_url(candidates(:richard).test_hash) + assert_response :success + assert assigns(:candidate), "@candidate not present" + assert assigns(:quiz), "@quiz not present" + assert assigns(:status), "@status not present" + end + end +end diff --git a/test/controllers/review_controller_test.rb b/test/controllers/review_controller_test.rb deleted file mode 100644 index 453a629..0000000 --- a/test/controllers/review_controller_test.rb +++ /dev/null @@ -1,63 +0,0 @@ -# frozen_string_literal: true -require 'test_helper' - -class ReviewControllerTest < ActionDispatch::IntegrationTest - test "should get login" do - get review_login_url - assert_response :success - end - - test "should require auth or redirect" do - get review_url - assert_redirected_to review_login_path - - get review_test_url(candidates(:richard).test_hash) - assert_redirected_to review_login_path - end - - test "should auth to index" do - auth_reviewer - assert_redirected_to admin_path - assert session[:user].present? - end - - test "should fail auth with flash" do - post review_auth_url, params: { auth: - { email: 'fed.review@mailinator.com', password: 'bad-password' } } - - assert_redirected_to review_login_path - assert flash[:error] - end - - test "should get review list" do - auth_reviewer - get review_url - assert_response :success - assert assigns(:candidates), '@candidates not present' - end - - test "should get index" do - auth_reviewer - - get review_url - assert_response :success - end - - test "should get view" do - auth_reviewer - - get review_test_url(candidates(:richard).test_hash) - assert_response :success - assert assigns(:candidate), "@candidate not present" - assert assigns(:quiz), "@quiz not present" - assert assigns(:status), "@status not present" - end - - test 'should logout and reset session' do - auth_reviewer - get review_logout_path - - assert :success - assert session[:user].nil? - end -end diff --git a/test/integration/question_attachments_test.rb b/test/integration/question_attachments_test.rb index e10f3e8..84b4215 100644 --- a/test/integration/question_attachments_test.rb +++ b/test/integration/question_attachments_test.rb @@ -20,10 +20,9 @@ class QuestionAttachmentsTest < ActionDispatch::IntegrationTest end test "should show attachments on review" do - user = users :reviewer - post review_auth_url, params: { auth: { email: user.email, password: 'password' } } + auth_reviewer - get review_test_path(candidates(:richard).test_hash) + get admin_result_path(candidates(:richard).test_hash) assert_response :success assert_select "img[src=\"#{questions(:fed6).attachment}\"]" end From aeef75bf8bbc96d3311ae3312abedcc9e99747c2 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Thu, 22 Sep 2016 14:21:34 -0500 Subject: [PATCH 15/16] fix admin/candidate[s] naming --- app/controllers/admin/candidate_controller.rb | 4 ++-- config/routes.rb | 2 +- test/controllers/admin/candidate_controller/index_test.rb | 6 +++--- .../admin/candidate_controller/new_candidate_test.rb | 2 +- .../admin/candidate_controller/update_candidate_test.rb | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/candidate_controller.rb b/app/controllers/admin/candidate_controller.rb index 6eaf61f..85f4d2b 100644 --- a/app/controllers/admin/candidate_controller.rb +++ b/app/controllers/admin/candidate_controller.rb @@ -19,7 +19,7 @@ module Admin if @candidate.persisted? send_notifications @candidate - redirect_to admin_candidate_path, + redirect_to admin_candidates_path, flash: { success: "Sucessfully created candidate #{@candidate.name}" } else flash[:error] = "Failed to save candidate." @@ -38,7 +38,7 @@ module Admin @candidate.update(candidate_params) if @candidate.save - redirect_to admin_candidate_path, flash: { success: "#{@candidate.name} updated!" } + redirect_to admin_candidates_path, flash: { success: "#{@candidate.name} updated!" } else flash[:error] = "Failed to save candidate." render :edit diff --git a/config/routes.rb b/config/routes.rb index 2691c85..c5356dd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -40,7 +40,7 @@ Rails.application.routes.draw do post "/admin/profile", to: "admin/profile#update", as: :admin_update_profile get "/admin/profile/edit", to: "admin/profile#edit", as: :admin_edit_profile - get "/admin/candidate", to: "admin/candidate#index", as: :admin_candidate + get "/admin/candidates", to: "admin/candidate#index", as: :admin_candidates get "/admin/candidate/new", to: "admin/candidate#new", as: :admin_new_candidate post "/admin/candidate/new", to: "admin/candidate#create", as: :admin_create_candidate get "/admin/candidate/:id", to: "admin/candidate#edit", as: :admin_edit_candidate diff --git a/test/controllers/admin/candidate_controller/index_test.rb b/test/controllers/admin/candidate_controller/index_test.rb index d9c2b11..cd6ebe0 100644 --- a/test/controllers/admin/candidate_controller/index_test.rb +++ b/test/controllers/admin/candidate_controller/index_test.rb @@ -4,7 +4,7 @@ require 'test_helper' module Admin class CandidateControllerTest < ActionDispatch::IntegrationTest test "should require auth or redirect" do - get admin_candidate_url + get admin_candidates_url assert_redirected_to admin_login_url get admin_new_candidate_url @@ -16,14 +16,14 @@ module Admin test "should get candidate list" do auth_recruiter - get admin_candidate_url + get admin_candidates_url assert_response :success assert assigns(:candidates), "@candidates not present" end test 'should have edit links' do auth_recruiter - get admin_candidate_url + get admin_candidates_url assert_response :success assert_select "a[href='#{admin_edit_candidate_path(candidates(:martha))}']" end diff --git a/test/controllers/admin/candidate_controller/new_candidate_test.rb b/test/controllers/admin/candidate_controller/new_candidate_test.rb index 50fb582..c59f58c 100644 --- a/test/controllers/admin/candidate_controller/new_candidate_test.rb +++ b/test/controllers/admin/candidate_controller/new_candidate_test.rb @@ -27,7 +27,7 @@ module Admin { name: 'new name', email: 'test@mailinator.com', experience: '0-3', quiz_id: quizzes(:fed).id } } end end - assert_redirected_to admin_candidate_path + assert_redirected_to admin_candidates_path assert flash[:success] end diff --git a/test/controllers/admin/candidate_controller/update_candidate_test.rb b/test/controllers/admin/candidate_controller/update_candidate_test.rb index 25a6efc..e4b2779 100644 --- a/test/controllers/admin/candidate_controller/update_candidate_test.rb +++ b/test/controllers/admin/candidate_controller/update_candidate_test.rb @@ -20,7 +20,7 @@ module Admin refute_equal candidate.name, Candidate.find_by(id: candidate.id).name assert_equal candidate.test_hash, Candidate.find_by(id: candidate.id).test_hash - assert_redirected_to admin_candidate_url + assert_redirected_to admin_candidates_url end test 'should redirect to form on fail' do From 5398a8ea8bc6b50b61695e82c2aa3809f41cb8e8 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Thu, 22 Sep 2016 16:29:19 -0500 Subject: [PATCH 16/16] admin global nav --- app/assets/stylesheets/molecules/_nav.scss | 17 +++++++++ app/assets/stylesheets/templates/_header.scss | 4 +++ app/controllers/admin/candidate_controller.rb | 4 +-- app/controllers/admin/dashboard_controller.rb | 21 +++++++++-- app/controllers/application_controller.rb | 18 ---------- app/views/admin/_nav.html.erb | 9 +++++ app/views/admin/candidate/index.html.erb | 7 ++-- app/views/admin/dashboard/show.html.erb | 35 ------------------- app/views/admin/quiz/_table_list.html.erb | 2 +- app/views/admin/result/index.html.erb | 5 +-- app/views/admin/user/_table_list.html.erb | 2 +- app/views/admin/user/index.html.erb | 2 +- app/views/layouts/admin.html.erb | 3 +- .../admin/dashboard_controller_test.rb | 2 +- 14 files changed, 64 insertions(+), 67 deletions(-) create mode 100644 app/assets/stylesheets/molecules/_nav.scss create mode 100644 app/views/admin/_nav.html.erb delete mode 100644 app/views/admin/dashboard/show.html.erb diff --git a/app/assets/stylesheets/molecules/_nav.scss b/app/assets/stylesheets/molecules/_nav.scss new file mode 100644 index 0000000..f27eb2f --- /dev/null +++ b/app/assets/stylesheets/molecules/_nav.scss @@ -0,0 +1,17 @@ +nav { + margin: 15px 0; + padding: 0; + text-align: right; + + a, + a:visited { + text-decoration: none; + padding: 15px; + margin: 0; + text-transform: uppercase; + + &:hover { + background-color: $gray-lighter; + } + } +} diff --git a/app/assets/stylesheets/templates/_header.scss b/app/assets/stylesheets/templates/_header.scss index b5f6734..c12384f 100644 --- a/app/assets/stylesheets/templates/_header.scss +++ b/app/assets/stylesheets/templates/_header.scss @@ -5,6 +5,10 @@ header { &.no-progressbar { padding-top: 52px; } + + &.no-progressbar.admin { + padding-top: 0; + } } .page-title { @include omega(); diff --git a/app/controllers/admin/candidate_controller.rb b/app/controllers/admin/candidate_controller.rb index 85f4d2b..5a95b68 100644 --- a/app/controllers/admin/candidate_controller.rb +++ b/app/controllers/admin/candidate_controller.rb @@ -4,7 +4,7 @@ module Admin before_action :collect_quizzes, except: [:login, :auth] def index - @candidates = policy_scope current_recruiter.candidates + @candidates = policy_scope Candidate.order(:name) end def new @@ -15,7 +15,7 @@ module Admin def create authorize Candidate - @candidate = Candidate.create(candidate_params.merge(recruiter_id: current_recruiter.id)) + @candidate = Candidate.create(candidate_params.merge(recruiter_id: current_user.id)) if @candidate.persisted? send_notifications @candidate diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 94c7f00..ea5492f 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -3,8 +3,25 @@ module Admin class DashboardController < AdminController def show authorize :dashboard - @quizzes = policy_scope Quiz.includes(:questions).all - @users = policy_scope User.order(:role, :name) + send "redirect_for_#{current_user.role}" end + + private + + def redirect_for_admin + redirect_to admin_users_url + end + + def redirect_for_manager + redirect_to admin_quizzes_url + end + + def redirect_for_reviewer + redirect_to admin_results_url + end + + def redirect_for_recruiter + redirect_to admin_candidates_url + end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 37fe04f..c9b89a9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,16 +4,6 @@ class ApplicationController < ActionController::Base add_flash_types :warning, :success - def current_recruiter - user_parms = { id: session[:user], role: %w(admin recruiter) } - @current_recruiter ||= User.find_by(user_parms) if session[:user] - end - - def current_reviewer - user_parms = { id: session[:user], role: %w(admin reviewer) } - @current_reviewer ||= User.find_by(user_parms) if session[:user] - end - def current_candidate @current_candidate ||= Candidate.find_by(test_hash: session[:test_id]) if session[:test_id] end @@ -29,14 +19,6 @@ class ApplicationController < ActionController::Base params.require(:auth).permit(:email, :password) end - def authorize_recruiter - redirect_to recruiter_login_path unless current_recruiter - end - - def authorize_reviewer - redirect_to review_login_path unless current_reviewer - end - def authorize_candidate redirect_to login_path unless current_candidate end diff --git a/app/views/admin/_nav.html.erb b/app/views/admin/_nav.html.erb new file mode 100644 index 0000000..e81cb18 --- /dev/null +++ b/app/views/admin/_nav.html.erb @@ -0,0 +1,9 @@ + diff --git a/app/views/admin/candidate/index.html.erb b/app/views/admin/candidate/index.html.erb index e6095fc..0c29e2e 100644 --- a/app/views/admin/candidate/index.html.erb +++ b/app/views/admin/candidate/index.html.erb @@ -1,9 +1,10 @@ +<% + content_for :section_title, "Candidates" +%>
-

Candidates

- <%= link_to(admin_new_candidate_path, { class: 'secondary-btn' }) do %> - <% end %> + <% end if policy(User).create? %> diff --git a/app/views/admin/dashboard/show.html.erb b/app/views/admin/dashboard/show.html.erb deleted file mode 100644 index ee1284a..0000000 --- a/app/views/admin/dashboard/show.html.erb +++ /dev/null @@ -1,35 +0,0 @@ -<% - content_for :section_title, "Admin Dashboard" -%> - -
-## Admin
-Users | Dept/Unit | Quizzes | Candidates | Results | Profile | Logout
-
-## Manager
-Quizzes | Results | Profile | Logout
-
-## Recruiter
-Results | Profile | Logout
-
-## Reviewer
-Candidates | Profile | Logout
-
- -<% if policy(Quiz).index? %> -
-

Quizzes

- <%= render partial: 'admin/quiz/table_list', locals: { quizzes: @quizzes } %> - <%= link_to('New Quiz', admin_new_quiz_path, { class: 'btn' }) %> -
-<% end %> - -<% if policy(User).index? %> -
-

Users

- <%= render partial: 'admin/user/table_list', locals: { users: @users } %> - <%= link_to('New User', admin_new_user_path, { class: 'btn' }) %> -
-<% end %> - - diff --git a/app/views/admin/quiz/_table_list.html.erb b/app/views/admin/quiz/_table_list.html.erb index 98fbd79..8ba51bc 100644 --- a/app/views/admin/quiz/_table_list.html.erb +++ b/app/views/admin/quiz/_table_list.html.erb @@ -13,7 +13,7 @@ - + <% end %>
<%= quiz.dept %> <%= quiz.unit %> <%= quiz.questions.count %><%= link_to 'edit', admin_edit_quiz_path(quiz.to_i), { class: 'btn tertiary-btn' } %><%= link_to 'edit', admin_edit_quiz_path(quiz.to_i), { class: 'btn tertiary-btn' } if policy(quiz).edit? %>
diff --git a/app/views/admin/result/index.html.erb b/app/views/admin/result/index.html.erb index 158605e..3c1373c 100644 --- a/app/views/admin/result/index.html.erb +++ b/app/views/admin/result/index.html.erb @@ -1,6 +1,7 @@ +<% + content_for :section_title, "Completed Tests" +%>
-

Completed Tests

- diff --git a/app/views/admin/user/_table_list.html.erb b/app/views/admin/user/_table_list.html.erb index bcce061..9b8ef8f 100644 --- a/app/views/admin/user/_table_list.html.erb +++ b/app/views/admin/user/_table_list.html.erb @@ -11,7 +11,7 @@ - + <% end %>
Test ID<%= link_to user.name, admin_user_path(user.to_i) %> <%= mail_to(user.email) %> <%= user.role %><%= link_to 'edit', admin_edit_user_path(user.to_i), { class: 'btn tertiary-btn' } %><%= link_to 'edit', admin_edit_user_path(user.to_i), { class: 'btn tertiary-btn' } if policy(user).edit? %>
diff --git a/app/views/admin/user/index.html.erb b/app/views/admin/user/index.html.erb index 2598812..303aa62 100644 --- a/app/views/admin/user/index.html.erb +++ b/app/views/admin/user/index.html.erb @@ -4,4 +4,4 @@

Users

<%= render partial: 'admin/user/table_list', locals: { users: @users } %> -<%= link_to('New User', admin_new_user_path, { class: 'btn' }) %> +<%= link_to('New User', admin_new_user_path, { class: 'btn' }) if policy(User).create? %> diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index 79672e3..dae9069 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -20,7 +20,8 @@
-
+
+ <%= render partial: "admin/nav" if current_user %>
<% if content_for?(:section_title) %>
<%= yield(:section_title) %>
diff --git a/test/controllers/admin/dashboard_controller_test.rb b/test/controllers/admin/dashboard_controller_test.rb index 9b36041..10a1474 100644 --- a/test/controllers/admin/dashboard_controller_test.rb +++ b/test/controllers/admin/dashboard_controller_test.rb @@ -11,7 +11,7 @@ module Admin test "should get dashboard" do post admin_auth_url, params: { auth: { email: 'alan.admin@mailinator.com', password: 'password' } } - get admin_url + get admin_users_url assert_response :success end end