diff --git a/Gemfile b/Gemfile index 460a9de..025cc9f 100644 --- a/Gemfile +++ b/Gemfile @@ -14,6 +14,7 @@ gem 'font-awesome-rails' gem 'jquery-rails' gem 'pg', '>= 0.18', '< 2.0' gem 'puma', '~> 3.11' +gem 'pundit' gem 'rails', '~> 5.2.0' gem 'react-rails' gem 'sass-rails' diff --git a/Gemfile.lock b/Gemfile.lock index 300f162..79007af 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -186,6 +186,8 @@ GEM pry (>= 0.10.4) public_suffix (3.0.2) puma (3.11.4) + pundit (2.0.1) + activesupport (>= 3.0.0) rack (2.0.5) rack-proxy (0.6.4) rack @@ -337,6 +339,7 @@ DEPENDENCIES pg (>= 0.18, < 2.0) pry-rails puma (~> 3.11) + pundit rails (~> 5.2.0) rails-controller-testing react-rails diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 16045e9..abddad3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,5 @@ class ApplicationController < ActionController::Base + include Pundit before_action :authenticate_user! before_action :configure_permitted_parameters, if: :devise_controller? diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 041f151..e69a7a6 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -4,7 +4,7 @@ class ProjectsController < ApplicationController before_action :load_project, only: %i[show edit update] def index - @projects = Project.all + @projects = policy_scope(Project) end def show @@ -19,6 +19,7 @@ def new def create @project = Project.new(project_params) + @project.users << current_user if @project.save flash[:notice] = 'Project created' @@ -44,7 +45,13 @@ def update private def load_project - @project = Project.find(params[:id]) + begin + @project = policy_scope(Project).find(params[:id]) + rescue ActiveRecord::RecordNotFound + flash[:error] = 'You\'re not allowed to access this project' + return redirect_to projects_path + end + add_breadcrumb 'Projects', :projects_path add_breadcrumb @project.name, project_path(@project) end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb new file mode 100644 index 0000000..eefe976 --- /dev/null +++ b/app/policies/application_policy.rb @@ -0,0 +1,49 @@ +class ApplicationPolicy + attr_reader :user, :record + + def initialize(user, record) + @user = user + @record = record + end + + def index? + false + end + + def show? + false + end + + def create? + false + end + + def new? + create? + end + + def update? + false + end + + def edit? + update? + end + + def destroy? + false + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb new file mode 100644 index 0000000..abc6d76 --- /dev/null +++ b/app/policies/project_policy.rb @@ -0,0 +1,7 @@ +class ProjectPolicy < ApplicationPolicy + class Scope < Scope + def resolve + scope.joins(:users).where('user_id = ?', user.id) + end + end +end diff --git a/spec/features/project_management_spec.rb b/spec/features/project_management_spec.rb index 9c0c239..dd02cc1 100644 --- a/spec/features/project_management_spec.rb +++ b/spec/features/project_management_spec.rb @@ -1,13 +1,14 @@ require 'rails_helper' feature 'Project management' do - let!(:project) { create(:project) } + let(:user) { create(:user) } + let!(:project) { create(:project, users: [user]) } before do - sign_in(create(:user)) + sign_in(user) end - it 'User can view all projects and project details' do + it 'User can view all projects and project details that he belongs to' do visit root_path expect(page).to have_content 'Projects' diff --git a/spec/features/target_management_spec.rb b/spec/features/target_management_spec.rb index 4a16124..a3fd5f1 100644 --- a/spec/features/target_management_spec.rb +++ b/spec/features/target_management_spec.rb @@ -2,13 +2,14 @@ require 'rails_helper' feature 'Target management', js: true do + let(:user) { create(:user) } let(:target_one_name) { 'TEST TARGET ONE' } let(:target_two_name) { 'TEST TARGET TWO' } - let(:project) { create(:project) } + let(:project) { create(:project, users: [user]) } let!(:terrestrial_target_type) { TargetType.create(name: 'Terrestrial Ecosystem') } before do - sign_in(create(:user)) + sign_in(user) end it 'User can view all targets for a Project' do diff --git a/spec/features/task_management_spec.rb b/spec/features/task_management_spec.rb index d082113..75cf929 100644 --- a/spec/features/task_management_spec.rb +++ b/spec/features/task_management_spec.rb @@ -6,8 +6,8 @@ let(:task_three_name) { 'TEST TASK 3' } let(:user_email) { 'blah@blah.com' } let(:project_name) { 'TEST PROJECT' } - let(:project) { build(:project, name: project_name) } let(:user) { create(:user, email: user_email) } + let(:project) { build(:project, name: project_name, users: [user]) } let!(:task_one) { create(:task, name: task_one_name, project: project) } let!(:task_two) { create(:task, name: task_two_name, project: project, user: user) } diff --git a/spec/requests/projects_request_spec.rb b/spec/requests/projects_request_spec.rb index 7add589..f02ac05 100644 --- a/spec/requests/projects_request_spec.rb +++ b/spec/requests/projects_request_spec.rb @@ -1,13 +1,15 @@ require 'rails_helper' describe 'Project Requests', type: :request do - let(:project) { create(:project) } - let(:new_project) { build(:project) } + let(:user) { create(:user) } + let(:new_user) { create(:user) } + let(:project) { create(:project, users: [user]) } + let(:new_project) { create(:project, users:[new_user]) } let!(:task) { create(:task, project: project) } let!(:archived_task) { create(:task, :archived, project: project) } before do - sign_in(create(:user)) + sign_in(user) end describe 'GET index' do @@ -17,20 +19,30 @@ expect(response).to have_http_status 200 expect(response.body).to include('Projects') expect(response.body).to include(project.name) + expect(response.body).to_not include(new_project.name) end end describe 'GET SHOW' do - before do - get "/projects/#{project.id}" - end + context 'when a project is associated to user' do + before do + get "/projects/#{project.id}" + end - it 'excludes archived tasks' do - expect(response.body).to_not include(archived_task.name) + it 'excludes archived tasks' do + expect(response.body).to_not include(archived_task.name) + end + + it 'includes non-archived tasks' do + expect(response.body).to include(task.name) + end end + context 'when a project is not associated to the user' do + subject { get project_path(new_project.id) } - it 'includes non-archived tasks' do - expect(response.body).to include(task.name) + it 'should be redirect_to index' do + expect(subject).to redirect_to(projects_path) + end end end @@ -41,6 +53,7 @@ expect(response).to have_http_status :found expect(response).to redirect_to(project_path(Project.last)) + expect(Project.last.users.count).to eq 1 end end @@ -55,21 +68,32 @@ end describe 'PUT update' do - describe 'success' do - it 'updates the project' do - put "/projects/#{project.id}", params: { project: { name: 'Bender saves the ocean' } } - follow_redirect! + context 'when editing a project that users is associated with' do + describe 'success' do + it 'updates the project' do + put "/projects/#{project.id}", params: { project: { name: 'Bender saves the ocean' } } + follow_redirect! - expect(response.body).to include('Bender saves the ocean') + expect(response.body).to include('Bender saves the ocean') + end end - end - describe 'failure' do - it 'shows errors' do - put "/projects/#{project.id}", params: { project: { name: '' } } + describe 'failure' do + it 'shows errors' do + put "/projects/#{project.id}", params: { project: { name: '' } } - expect(response).to have_http_status 422 - expect(response.body).to include('Name can't be blank') + expect(response).to have_http_status 422 + expect(response.body).to include('Name can't be blank') + end + end + end + context 'if somehow the user try to edit a project that not he\'s not associated with' do + it 'then it should be redirected to index page, and data must\'t be updated' do + put "/projects/#{new_project.id}", params: { project: { name: 'Bender saves the ocean' } } + expect(response).to redirect_to(projects_path) + + follow_redirect! + expect(response.body).to_not include('Bender saves the ocean') end end end