From 3378790764d471e5d7a5946d429bff733843aa67 Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 17:04:50 +0200 Subject: [PATCH 01/17] Adds migrations & models for administrators & moderators --- app/models/administrator.rb | 6 ++++++ app/models/moderator.rb | 6 ++++++ .../20150807140235_create_administrators.rb | 7 +++++++ db/migrate/20150807140346_create_moderators.rb | 7 +++++++ db/schema.rb | 16 +++++++++++++++- 5 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 app/models/administrator.rb create mode 100644 app/models/moderator.rb create mode 100644 db/migrate/20150807140235_create_administrators.rb create mode 100644 db/migrate/20150807140346_create_moderators.rb diff --git a/app/models/administrator.rb b/app/models/administrator.rb new file mode 100644 index 000000000..c82c4f728 --- /dev/null +++ b/app/models/administrator.rb @@ -0,0 +1,6 @@ +class Administrator < ActiveRecord::Base + belongs_to :user + delegate :name, :email, to: :user + + validates :user_id, presence: true +end diff --git a/app/models/moderator.rb b/app/models/moderator.rb new file mode 100644 index 000000000..6cfe0c291 --- /dev/null +++ b/app/models/moderator.rb @@ -0,0 +1,6 @@ +class Moderator < ActiveRecord::Base + belongs_to :user + delegate :name, :email, to: :user + + validates :user_id, presence: true +end diff --git a/db/migrate/20150807140235_create_administrators.rb b/db/migrate/20150807140235_create_administrators.rb new file mode 100644 index 000000000..d844531a4 --- /dev/null +++ b/db/migrate/20150807140235_create_administrators.rb @@ -0,0 +1,7 @@ +class CreateAdministrators < ActiveRecord::Migration + def change + create_table :administrators do |t| + t.belongs_to :user, index: true, foreign_key: true + end + end +end diff --git a/db/migrate/20150807140346_create_moderators.rb b/db/migrate/20150807140346_create_moderators.rb new file mode 100644 index 000000000..526a29944 --- /dev/null +++ b/db/migrate/20150807140346_create_moderators.rb @@ -0,0 +1,7 @@ +class CreateModerators < ActiveRecord::Migration + def change + create_table :moderators do |t| + t.belongs_to :user, index: true, foreign_key: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e5f6f9c0a..7833fc34c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,11 +11,17 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150806140048) do +ActiveRecord::Schema.define(version: 20150807140346) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" + create_table "administrators", force: :cascade do |t| + t.integer "user_id" + end + + add_index "administrators", ["user_id"], name: "index_administrators_on_user_id", using: :btree + create_table "comments", force: :cascade do |t| t.integer "commentable_id" t.string "commentable_type" @@ -41,6 +47,12 @@ ActiveRecord::Schema.define(version: 20150806140048) do t.datetime "updated_at", null: false end + create_table "moderators", force: :cascade do |t| + t.integer "user_id" + end + + add_index "moderators", ["user_id"], name: "index_moderators_on_user_id", using: :btree + create_table "taggings", force: :cascade do |t| t.integer "tag_id" t.integer "taggable_id" @@ -103,4 +115,6 @@ ActiveRecord::Schema.define(version: 20150806140048) do add_index "votes", ["votable_id", "votable_type", "vote_scope"], name: "index_votes_on_votable_id_and_votable_type_and_vote_scope", using: :btree add_index "votes", ["voter_id", "voter_type", "vote_scope"], name: "index_votes_on_voter_id_and_voter_type_and_vote_scope", using: :btree + add_foreign_key "administrators", "users" + add_foreign_key "moderators", "users" end From c04d397e66964e2bff23f7b79a622439d6db0f6b Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 17:20:20 +0200 Subject: [PATCH 02/17] Implements User.administrator? & User.moderator? --- app/models/user.rb | 8 ++++++++ spec/factories.rb | 10 +++++++++- spec/models/user_spec.rb | 28 ++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 17573eaa7..4eb832546 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,4 +19,12 @@ class User < ActiveRecord::Base voted = votes.where("votable_type = ? AND votable_id IN (?)", "Debate", debates_ids) voted.each_with_object({}){ |v,_| _[v.votable_id] = v.vote_flag } end + + def administrator? + @is_administrator ||= Administrator.where(user_id: id).exists? + end + + def moderator? + @is_moderator ||= Moderator.where(user_id: id).exists? + end end diff --git a/spec/factories.rb b/spec/factories.rb index 8e1a2ea6c..fc76685d0 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -31,4 +31,12 @@ FactoryGirl.define do debate end -end \ No newline at end of file + factory :administrator do + user + end + + factory :moderator do + user + end + +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0a7665127..24ccc9361 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -7,13 +7,13 @@ describe User do @user = create(:user) end - it "should return {} if no debate" do + it "returns {} if no debate" do expect(@user.votes_on_debates()).to eq({}) expect(@user.votes_on_debates([])).to eq({}) expect(@user.votes_on_debates([nil, nil])).to eq({}) end - it "should return a hash of debates ids and votes" do + it "returns a hash of debates ids and votes" do debate1 = create(:debate) debate2 = create(:debate) debate3 = create(:debate) @@ -73,4 +73,28 @@ describe User do end end + describe "administrator?" do + it "is false when the user is not an admin" do + expect(subject.administrator?).to be false + end + + it "is true when the user is an admin" do + subject.save + create(:administrator, user: subject) + expect(subject.administrator?).to be true + end + end + + describe "moderator?" do + it "is false when the user is not an admin" do + expect(subject.moderator?).to be false + end + + it "is true when the user is an admin" do + subject.save + create(:moderator, user: subject) + expect(subject.moderator?).to be true + end + end + end From 8d83607331dc0958a41624dffec307be184a5acd Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 17:20:28 +0200 Subject: [PATCH 03/17] Implements dashboard controllers & views for admin & moderation --- app/controllers/admin/dashboard_controller.rb | 6 ++++++ app/controllers/moderation/dashboard_controller.rb | 6 ++++++ app/views/admin/dashboard/index.html.erb | 1 + app/views/moderation/dashboard/index.html.erb | 1 + config/locales/en.yml | 8 ++++++++ config/locales/es.yml | 8 ++++++++ config/routes.rb | 9 ++++++++- 7 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 app/controllers/admin/dashboard_controller.rb create mode 100644 app/controllers/moderation/dashboard_controller.rb create mode 100644 app/views/admin/dashboard/index.html.erb create mode 100644 app/views/moderation/dashboard/index.html.erb diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb new file mode 100644 index 000000000..697546889 --- /dev/null +++ b/app/controllers/admin/dashboard_controller.rb @@ -0,0 +1,6 @@ +class Admin::DashboardController < ApplicationController + + def index + end + +end diff --git a/app/controllers/moderation/dashboard_controller.rb b/app/controllers/moderation/dashboard_controller.rb new file mode 100644 index 000000000..50491e4e6 --- /dev/null +++ b/app/controllers/moderation/dashboard_controller.rb @@ -0,0 +1,6 @@ +class Moderation::DashboardController < ApplicationController + + def index + end + +end diff --git a/app/views/admin/dashboard/index.html.erb b/app/views/admin/dashboard/index.html.erb new file mode 100644 index 000000000..ffc8a281a --- /dev/null +++ b/app/views/admin/dashboard/index.html.erb @@ -0,0 +1 @@ +

<%= t("admin.dashboard.index.title") %>

diff --git a/app/views/moderation/dashboard/index.html.erb b/app/views/moderation/dashboard/index.html.erb new file mode 100644 index 000000000..16eb4508f --- /dev/null +++ b/app/views/moderation/dashboard/index.html.erb @@ -0,0 +1 @@ +

<%= t("moderation.dashboard.index.title") %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 7fdf7a214..fbb77d5a4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -11,6 +11,14 @@ en: open_city_slogan: So the citizens can decide what kind of city they want. create_debate: Create a debate my_account_link: My account + admin: + dashboard: + index: + title: Administration + moderation: + dashboard: + index: + title: Moderation debates: index: create_debate: Create a debate diff --git a/config/locales/es.yml b/config/locales/es.yml index 9bbdde6b6..c53d01547 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -11,6 +11,14 @@ es: open_city_slogan: Para que todos los madrileños decidamos que ciudad queremos tener. create_debate: Crea un debate my_account_link: Mi cuenta + admin: + dashboard: + index: + title: Administración + moderation: + dashboard: + index: + title: Moderación debates: index: create_debate: Crea un debate diff --git a/config/routes.rb b/config/routes.rb index 73a77f60e..74179ba2a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -17,11 +17,18 @@ Rails.application.routes.draw do post :vote end end - end resource :account, controller: "account", only: [:show, :update] + namespace :admin do + root to: "dashboard#index" + end + + namespace :moderation do + root to: "dashboard#index" + end + # Example of regular route: # get 'products/:id' => 'catalog#view' From 5cc9c21b8dfb6be99ee534e4de3fdafc9bfe69db Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 17:46:36 +0200 Subject: [PATCH 04/17] fixes typo in user_spec --- spec/models/user_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 24ccc9361..f360e207e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -86,11 +86,11 @@ describe User do end describe "moderator?" do - it "is false when the user is not an admin" do + it "is false when the user is not a moderator" do expect(subject.moderator?).to be false end - it "is true when the user is an admin" do + it "is true when the user is a moderator" do subject.save create(:moderator, user: subject) expect(subject.moderator?).to be true From d04383db249f153d74a1612cb8f46a81ba46c354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Fri, 7 Aug 2015 18:20:20 +0200 Subject: [PATCH 05/17] validates uniqueness of admins & mods --- app/models/administrator.rb | 2 +- app/models/moderator.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/administrator.rb b/app/models/administrator.rb index c82c4f728..15715a328 100644 --- a/app/models/administrator.rb +++ b/app/models/administrator.rb @@ -2,5 +2,5 @@ class Administrator < ActiveRecord::Base belongs_to :user delegate :name, :email, to: :user - validates :user_id, presence: true + validates :user_id, presence: true, uniqueness: true end diff --git a/app/models/moderator.rb b/app/models/moderator.rb index 6cfe0c291..30d4f4394 100644 --- a/app/models/moderator.rb +++ b/app/models/moderator.rb @@ -2,5 +2,5 @@ class Moderator < ActiveRecord::Base belongs_to :user delegate :name, :email, to: :user - validates :user_id, presence: true + validates :user_id, presence: true, uniqueness: true end From 99df1c0bbf8e041cb85ee003c2f207988840dc7b Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 17:49:16 +0200 Subject: [PATCH 06/17] adds cancancan --- Gemfile | 1 + Gemfile.lock | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index d96ea4e03..00e2a98eb 100644 --- a/Gemfile +++ b/Gemfile @@ -37,6 +37,7 @@ gem 'foundation-rails' gem 'acts_as_votable' gem "recaptcha", require: "recaptcha/rails" gem 'ckeditor' +gem 'cancancan' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index bc4dd1e27..67f4a7616 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,6 +53,7 @@ GEM builder (3.2.2) byebug (5.0.0) columnize (= 0.9.0) + cancancan (1.12.0) capybara (2.4.4) mime-types (>= 1.16) nokogiri (>= 1.3.3) @@ -271,6 +272,7 @@ DEPENDENCIES acts_as_commentable_with_threading acts_as_votable byebug + cancancan capybara ckeditor coffee-rails (~> 4.1.0) From 9eae91c76401e4be3d8e1da75003f83fba0aba2c Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 18:52:09 +0200 Subject: [PATCH 07/17] Makes user.moderator? return true for administrators --- app/models/user.rb | 2 +- spec/models/user_spec.rb | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 4eb832546..931aaae60 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,6 +25,6 @@ class User < ActiveRecord::Base end def moderator? - @is_moderator ||= Moderator.where(user_id: id).exists? + @is_moderator ||= Moderator.where(user_id: id).exists? || administrator? end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f360e207e..e53871fe2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -86,7 +86,7 @@ describe User do end describe "moderator?" do - it "is false when the user is not a moderator" do + it "is false when the user is not a moderator nor an administrator" do expect(subject.moderator?).to be false end @@ -95,6 +95,12 @@ describe User do create(:moderator, user: subject) expect(subject.moderator?).to be true end + + it "is true when the user is an administrator" do + subject.save + create(:administrator, user: subject) + expect(subject.moderator?).to be true + end end end From dac5b8d22a17be35888e85edfa8d14af4dd92330 Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 7 Aug 2015 19:15:08 +0200 Subject: [PATCH 08/17] Make /admin and /moderation only accesible to Admins & Moderators --- app/controllers/admin/base_controller.rb | 11 ++++++ app/controllers/admin/dashboard_controller.rb | 2 +- app/controllers/application_controller.rb | 7 ++++ app/controllers/moderation/base_controller.rb | 11 ++++++ .../moderation/dashboard_controller.rb | 2 +- spec/features/admin_spec.rb | 34 +++++++++++++++++++ spec/features/moderation_spec.rb | 34 +++++++++++++++++++ 7 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 app/controllers/admin/base_controller.rb create mode 100644 app/controllers/moderation/base_controller.rb create mode 100644 spec/features/admin_spec.rb create mode 100644 spec/features/moderation_spec.rb diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb new file mode 100644 index 000000000..d2f06ee84 --- /dev/null +++ b/app/controllers/admin/base_controller.rb @@ -0,0 +1,11 @@ +class Admin::BaseController < ApplicationController + + before_filter :verify_administrator + + private + + def verify_administrator + raise CanCan::AccessDenied unless current_user.try(:administrator?) + end + +end diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 697546889..f7aa5c440 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -1,4 +1,4 @@ -class Admin::DashboardController < ApplicationController +class Admin::DashboardController < Admin::BaseController def index end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7551f3f1a..eaf1c94dd 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,6 +11,13 @@ class ApplicationController < ActionController::Base # For APIs, you may want to use :null_session instead. protect_from_forgery with: :exception + rescue_from CanCan::AccessDenied do |exception| + respond_to do |format| + format.json { render nothing: true, status: :forbidden } + format.html { redirect_to main_app.root_url, :alert => exception.message } + end + end + private def set_locale diff --git a/app/controllers/moderation/base_controller.rb b/app/controllers/moderation/base_controller.rb new file mode 100644 index 000000000..2cebe7320 --- /dev/null +++ b/app/controllers/moderation/base_controller.rb @@ -0,0 +1,11 @@ +class Moderation::BaseController < ApplicationController + + before_filter :verify_moderator + + private + + def verify_moderator + raise CanCan::AccessDenied unless current_user.try(:moderator?) + end + +end diff --git a/app/controllers/moderation/dashboard_controller.rb b/app/controllers/moderation/dashboard_controller.rb index 50491e4e6..ceaddd6f4 100644 --- a/app/controllers/moderation/dashboard_controller.rb +++ b/app/controllers/moderation/dashboard_controller.rb @@ -1,4 +1,4 @@ -class Moderation::DashboardController < ApplicationController +class Moderation::DashboardController < Moderation::BaseController def index end diff --git a/spec/features/admin_spec.rb b/spec/features/admin_spec.rb new file mode 100644 index 000000000..a4728623b --- /dev/null +++ b/spec/features/admin_spec.rb @@ -0,0 +1,34 @@ +require 'rails_helper' + +feature 'Admin' do + let(:user) { create(:user) } + + scenario 'Access as regular user is not authorized' do + login_as(user) + visit admin_root_path + + expect(current_path).to eq(root_path) + expect(page).to have_content "not authorized" + end + + scenario 'Access as a moderator is not authorized' do + create(:moderator, user: user) + + login_as(user) + visit admin_root_path + + expect(current_path).to eq(root_path) + expect(page).to have_content "not authorized" + end + + scenario 'Access as an administrator is authorized' do + create(:administrator, user: user) + + login_as(user) + visit admin_root_path + + expect(current_path).to eq(admin_root_path) + expect(page).to_not have_content "not authorized" + end + +end diff --git a/spec/features/moderation_spec.rb b/spec/features/moderation_spec.rb new file mode 100644 index 000000000..cd3052f70 --- /dev/null +++ b/spec/features/moderation_spec.rb @@ -0,0 +1,34 @@ +require 'rails_helper' + +feature 'Admin' do + let(:user) { create(:user) } + + scenario 'Access as regular user is not authorized' do + login_as(user) + visit moderation_root_path + + expect(current_path).to eq(root_path) + expect(page).to have_content "not authorized" + end + + scenario 'Access as a moderator is authorized' do + create(:moderator, user: user) + + login_as(user) + visit moderation_root_path + + expect(current_path).to eq(moderation_root_path) + expect(page).to_not have_content "not authorized" + end + + scenario 'Access as an administrator is authorized' do + create(:administrator, user: user) + + login_as(user) + visit moderation_root_path + + expect(current_path).to eq(moderation_root_path) + expect(page).to_not have_content "not authorized" + end + +end From a949d54a8533b56e63abff27619690098df2e505 Mon Sep 17 00:00:00 2001 From: rgarcia Date: Fri, 7 Aug 2015 21:52:14 +0200 Subject: [PATCH 09/17] removes unused json respond block [#77] --- app/controllers/application_controller.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eaf1c94dd..f896ee1a6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,10 +12,7 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception rescue_from CanCan::AccessDenied do |exception| - respond_to do |format| - format.json { render nothing: true, status: :forbidden } - format.html { redirect_to main_app.root_url, :alert => exception.message } - end + redirect_to main_app.root_url, alert: exception.message end private From 8b53ae6f08ef0b8c33713231f9b7a758b253fd51 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 12:38:22 +0200 Subject: [PATCH 10/17] Adds Abilities for Debates --- app/models/ability.rb | 22 +++++++++++++++ spec/models/ability_spec.rb | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 app/models/ability.rb create mode 100644 spec/models/ability_spec.rb diff --git a/app/models/ability.rb b/app/models/ability.rb new file mode 100644 index 000000000..74c79f472 --- /dev/null +++ b/app/models/ability.rb @@ -0,0 +1,22 @@ +class Ability + include CanCan::Ability + + def initialize(user) + # Not logged in users + can :read, Debate + + if user # logged-in users + can [:read, :create, :vote], Debate + can :edit, Debate do |debate| + debate.editable_by?(user) + end + + if user.moderator? or user.administrator? + + elsif user.administrator? + + end + end + end + +end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb new file mode 100644 index 000000000..70d09f684 --- /dev/null +++ b/spec/models/ability_spec.rb @@ -0,0 +1,55 @@ +require 'rails_helper' +require 'cancan/matchers' + +describe Ability do + subject(:ability) { Ability.new(user) } + let(:debate) { Debate.new } + + describe "Non-logged in users" do + let(:user) { nil } + + it { should be_able_to(:index, Debate) } + it { should be_able_to(:show, debate) } + it { should_not be_able_to(:edit, Debate) } + it { should_not be_able_to(:vote, Debate) } + end + + describe "Citizens" do + let(:user) { create(:user) } + + it { should be_able_to(:index, Debate) } + it { should be_able_to(:show, debate) } + it { should be_able_to(:vote, debate) } + + describe "editing debates" do + let(:own_debate) { create(:debate, author: user) } + let(:own_debate_non_editable) { create(:debate, author: user) } + + before { allow(own_debate_non_editable).to receive(:editable?).and_return(false) } + + it { should be_able_to(:edit, own_debate) } + it { should_not be_able_to(:edit, debate) } # Not his + it { should_not be_able_to(:edit, own_debate_non_editable) } + end + end + + describe "Moderators" do + let(:user) { create(:user) } + before { create(:moderator, user: user) } + + it { should be_able_to(:index, Debate) } + it { should be_able_to(:show, debate) } + it { should be_able_to(:vote, debate) } + + end + + describe "Administrators" do + let(:user) { create(:user) } + before { create(:administrator, user: user) } + + it { should be_able_to(:index, Debate) } + it { should be_able_to(:show, debate) } + it { should be_able_to(:vote, debate) } + + end +end From a9a4f4fb81a50fd0a77417eae89ee442260052c5 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 13:00:26 +0200 Subject: [PATCH 11/17] Undo user.moderator? returning true for admins --- app/controllers/admin/base_controller.rb | 6 +++--- app/controllers/moderation/base_controller.rb | 6 +++--- app/models/user.rb | 2 +- spec/models/user_spec.rb | 8 +------- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb index d2f06ee84..4f54aa4b1 100644 --- a/app/controllers/admin/base_controller.rb +++ b/app/controllers/admin/base_controller.rb @@ -4,8 +4,8 @@ class Admin::BaseController < ApplicationController private - def verify_administrator - raise CanCan::AccessDenied unless current_user.try(:administrator?) - end + def verify_administrator + raise CanCan::AccessDenied unless current_user.try(:administrator?) + end end diff --git a/app/controllers/moderation/base_controller.rb b/app/controllers/moderation/base_controller.rb index 2cebe7320..c8f703225 100644 --- a/app/controllers/moderation/base_controller.rb +++ b/app/controllers/moderation/base_controller.rb @@ -4,8 +4,8 @@ class Moderation::BaseController < ApplicationController private - def verify_moderator - raise CanCan::AccessDenied unless current_user.try(:moderator?) - end + def verify_moderator + raise CanCan::AccessDenied unless current_user.try(:moderator?) || current_user.try(:administrator?) + end end diff --git a/app/models/user.rb b/app/models/user.rb index 931aaae60..4eb832546 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,6 +25,6 @@ class User < ActiveRecord::Base end def moderator? - @is_moderator ||= Moderator.where(user_id: id).exists? || administrator? + @is_moderator ||= Moderator.where(user_id: id).exists? end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9387c2bed..4a8fe8fd7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -100,7 +100,7 @@ describe User do end describe "moderator?" do - it "is false when the user is not a moderator nor an administrator" do + it "is false when the user is not a moderator" do expect(subject.moderator?).to be false end @@ -109,12 +109,6 @@ describe User do create(:moderator, user: subject) expect(subject.moderator?).to be true end - - it "is true when the user is an administrator" do - subject.save - create(:administrator, user: subject) - expect(subject.moderator?).to be true - end end end From 30e738a2fe2aca50b7ca921aff3fd036a4cb9b29 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 14:59:42 +0200 Subject: [PATCH 12/17] Add cancan authorization in all main urls --- app/controllers/admin/base_controller.rb | 1 + app/controllers/application_controller.rb | 3 +++ app/controllers/moderation/base_controller.rb | 1 + app/models/ability.rb | 2 +- 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb index 4f54aa4b1..d8cbcd6fa 100644 --- a/app/controllers/admin/base_controller.rb +++ b/app/controllers/admin/base_controller.rb @@ -1,5 +1,6 @@ class Admin::BaseController < ApplicationController + skip_authorization_check before_filter :verify_administrator private diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f896ee1a6..ad69fd511 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,6 +1,9 @@ require "application_responder" class ApplicationController < ActionController::Base + + check_authorization unless: :devise_controller? + self.responder = ApplicationResponder respond_to :html diff --git a/app/controllers/moderation/base_controller.rb b/app/controllers/moderation/base_controller.rb index c8f703225..d2391abd7 100644 --- a/app/controllers/moderation/base_controller.rb +++ b/app/controllers/moderation/base_controller.rb @@ -1,5 +1,6 @@ class Moderation::BaseController < ApplicationController + skip_authorization_check before_filter :verify_moderator private diff --git a/app/models/ability.rb b/app/models/ability.rb index 74c79f472..736e15da2 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,7 +7,7 @@ class Ability if user # logged-in users can [:read, :create, :vote], Debate - can :edit, Debate do |debate| + can :update, Debate do |debate| debate.editable_by?(user) end From c1c136278082d67248606dfbe4f74b054e5e74f5 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 15:00:14 +0200 Subject: [PATCH 13/17] Adapts debates controller to new permissions system --- app/controllers/debates_controller.rb | 8 +------- spec/features/debates_spec.rb | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index a395f9c5e..e4e55b8a7 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -1,8 +1,6 @@ class DebatesController < ApplicationController include RecaptchaHelper - before_action :set_debate, only: [:show, :edit, :update, :vote] - before_action :authenticate_user!, except: [:index, :show] - before_action :validate_ownership, only: [:edit, :update] + load_and_authorize_resource def index if params[:tag] @@ -56,10 +54,6 @@ class DebatesController < ApplicationController params.require(:debate).permit(:title, :description, :tag_list, :terms_of_service) end - def validate_ownership - raise ActiveRecord::RecordNotFound unless @debate.editable_by?(current_user) - end - def set_voted_values(debates_ids) @voted_values = current_user ? current_user.votes_on_debates(debates_ids) : {} end diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index f8b96e2ed..894b09b19 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -98,9 +98,9 @@ feature 'Debates' do expect(debate).to be_editable login_as(create(:user)) - expect { - visit edit_debate_path(debate) - }.to raise_error ActiveRecord::RecordNotFound + visit edit_debate_path(debate) + expect(current_path).to eq(root_path) + expect(page).to have_content 'not authorized' end scenario 'Update should not be posible if debate is not editable' do @@ -109,17 +109,19 @@ feature 'Debates' do expect(debate).to_not be_editable login_as(debate.author) - expect { - visit edit_debate_path(debate) - }.to raise_error ActiveRecord::RecordNotFound + visit edit_debate_path(debate) + edit_debate_path(debate) + expect(current_path).to eq(root_path) + expect(page).to have_content 'not authorized' end scenario 'Update should be posible for the author of an editable debate' do debate = create(:debate) login_as(debate.author) - visit debate_path(debate) - click_link 'Edit' + visit edit_debate_path(debate) + expect(current_path).to eq(edit_debate_path(debate)) + fill_in 'debate_title', with: "End child poverty" fill_in 'debate_description', with: "Let's..." From ce27a6f2ea5a9d76627e6e4f0cea102998d74a33 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 15:10:47 +0200 Subject: [PATCH 14/17] Adapts AccountController to new permissions system --- app/controllers/account_controller.rb | 2 +- app/models/ability.rb | 2 ++ spec/features/account_spec.rb | 3 ++- spec/models/ability_spec.rb | 17 +++++++++++++---- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index e9b5b6c99..28f61c9b2 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -1,7 +1,7 @@ class AccountController < ApplicationController - before_action :authenticate_user! before_action :set_account + load_and_authorize_resource class: "User" def show end diff --git a/app/models/ability.rb b/app/models/ability.rb index 736e15da2..3946e5f53 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -6,6 +6,8 @@ class Ability can :read, Debate if user # logged-in users + can [:read, :update], User, id: user.id + can [:read, :create, :vote], Debate can :update, Debate do |debate| debate.editable_by?(user) diff --git a/spec/features/account_spec.rb b/spec/features/account_spec.rb index e622c83c1..4e4f7100e 100644 --- a/spec/features/account_spec.rb +++ b/spec/features/account_spec.rb @@ -10,6 +10,7 @@ feature 'Account' do login_as(@user) visit root_path click_link "My account" + expect(current_path).to eq(account_path) expect(page).to have_selector("input[value='Manuela']") expect(page).to have_selector("input[value='Colau']") @@ -34,4 +35,4 @@ feature 'Account' do expect(page).to have_selector("input[id='account_email_on_debate_comment'][value='1']") expect(page).to have_selector("input[id='account_email_on_comment_reply'][value='1']") end -end \ No newline at end of file +end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 70d09f684..220db821d 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -5,7 +5,7 @@ describe Ability do subject(:ability) { Ability.new(user) } let(:debate) { Debate.new } - describe "Non-logged in users" do + describe "Non-logged in user" do let(:user) { nil } it { should be_able_to(:index, Debate) } @@ -14,13 +14,22 @@ describe Ability do it { should_not be_able_to(:vote, Debate) } end - describe "Citizens" do + describe "Citizen" do let(:user) { create(:user) } it { should be_able_to(:index, Debate) } it { should be_able_to(:show, debate) } it { should be_able_to(:vote, debate) } + it { should be_able_to(:show, user) } + it { should be_able_to(:edit, user) } + + describe "other users" do + let(:other_user) { create(:user) } + it { should_not be_able_to(:show, other_user) } + it { should_not be_able_to(:edit, other_user) } + end + describe "editing debates" do let(:own_debate) { create(:debate, author: user) } let(:own_debate_non_editable) { create(:debate, author: user) } @@ -33,7 +42,7 @@ describe Ability do end end - describe "Moderators" do + describe "Moderator" do let(:user) { create(:user) } before { create(:moderator, user: user) } @@ -43,7 +52,7 @@ describe Ability do end - describe "Administrators" do + describe "Administrator" do let(:user) { create(:user) } before { create(:administrator, user: user) } From 84d848df7e11f9e4989e91e190ad70ab6fff16ce Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 15:40:23 +0200 Subject: [PATCH 15/17] Adapts the CommentsController to the new permissions system --- app/controllers/comments_controller.rb | 26 ++++++++++++++------------ app/models/ability.rb | 2 ++ spec/models/ability_spec.rb | 3 +++ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index bbad3ca3b..3854ff07f 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -1,12 +1,11 @@ class CommentsController < ApplicationController - before_action :authenticate_user! - before_action :set_debate, :set_parent, only: :create + before_action :build_comment, only: :create + load_and_authorize_resource respond_to :html, :js def create - @comment = Comment.build(@debate, current_user, params[:comment][:body]) @comment.save! - @comment.move_to_child_of(@parent) if reply? + @comment.move_to_child_of(parent) if reply? Mailer.comment(@comment).deliver_now if email_on_debate_comment? Mailer.reply(@comment).deliver_now if email_on_comment_reply? @@ -15,7 +14,6 @@ class CommentsController < ApplicationController end def vote - @comment = Comment.find(params[:id]) @comment.vote_by(voter: current_user, vote: params[:value]) respond_with @comment end @@ -25,16 +23,20 @@ class CommentsController < ApplicationController params.require(:comments).permit(:commentable_type, :commentable_id, :body) end - def set_debate - @debate = Debate.find(params[:debate_id]) + def build_comment + @comment = Comment.build(debate, current_user, params[:comment][:body]) end - def set_parent - @parent = Comment.find_parent(params[:comment]) + def debate + @debate ||= Debate.find(params[:debate_id]) + end + + def parent + @parent ||= Comment.find_parent(params[:comment]) end def reply? - @parent.class == Comment + parent.class == Comment end def email_on_debate_comment? @@ -42,6 +44,6 @@ class CommentsController < ApplicationController end def email_on_comment_reply? - reply? && @parent.author.email_on_comment_reply? + reply? && parent.author.email_on_comment_reply? end -end \ No newline at end of file +end diff --git a/app/models/ability.rb b/app/models/ability.rb index 3946e5f53..c1fc24ae3 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -13,6 +13,8 @@ class Ability debate.editable_by?(user) end + can [:create, :vote], Comment + if user.moderator? or user.administrator? elsif user.administrator? diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 220db821d..3954ec600 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -24,6 +24,9 @@ describe Ability do it { should be_able_to(:show, user) } it { should be_able_to(:edit, user) } + it { should be_able_to(:create, Comment) } + it { should be_able_to(:vote, Comment) } + describe "other users" do let(:other_user) { create(:user) } it { should_not be_able_to(:show, other_user) } From 51323fa8911eb85b8d5ef39de348a796392c7826 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 16:05:58 +0200 Subject: [PATCH 16/17] Adds back missing authenticate_user! calls --- app/controllers/account_controller.rb | 1 + app/controllers/admin/base_controller.rb | 3 ++- app/controllers/comments_controller.rb | 1 + app/controllers/debates_controller.rb | 1 + app/controllers/moderation/base_controller.rb | 3 ++- 5 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index 28f61c9b2..3bf6c41ba 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -1,5 +1,6 @@ class AccountController < ApplicationController + before_action :authenticate_user! before_action :set_account load_and_authorize_resource class: "User" diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb index d8cbcd6fa..91524b6d3 100644 --- a/app/controllers/admin/base_controller.rb +++ b/app/controllers/admin/base_controller.rb @@ -1,7 +1,8 @@ class Admin::BaseController < ApplicationController + before_action :authenticate_user! skip_authorization_check - before_filter :verify_administrator + before_action :verify_administrator private diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 3854ff07f..abee2974d 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -1,4 +1,5 @@ class CommentsController < ApplicationController + before_action :authenticate_user! before_action :build_comment, only: :create load_and_authorize_resource respond_to :html, :js diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index e4e55b8a7..d72965432 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -1,5 +1,6 @@ class DebatesController < ApplicationController include RecaptchaHelper + before_action :authenticate_user!, except: [:index, :show] load_and_authorize_resource def index diff --git a/app/controllers/moderation/base_controller.rb b/app/controllers/moderation/base_controller.rb index d2391abd7..0ee155e03 100644 --- a/app/controllers/moderation/base_controller.rb +++ b/app/controllers/moderation/base_controller.rb @@ -1,7 +1,8 @@ class Moderation::BaseController < ApplicationController + before_action :authenticate_user! skip_authorization_check - before_filter :verify_moderator + before_action :verify_moderator private From 7f5f5d6b196a641837bfbe8a670d154c736585f7 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 10 Aug 2015 16:24:01 +0200 Subject: [PATCH 17/17] Adds i18n for unauthorized messages --- config/i18n-tasks.yml | 4 ++++ config/locales/en.yml | 4 ++++ config/locales/es.yml | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index f9b28d6a2..d556950f5 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -85,6 +85,8 @@ search: # ignore_missing: # - 'errors.messages.{accepted,blank,invalid,too_short,too_long}' # - '{devise,simple_form}.*' +ignore_missing: + - 'unauthorized.*' ## Consider these keys used: # ignore_unused: @@ -93,6 +95,8 @@ search: # - 'simple_form.{yes,no}' # - 'simple_form.{placeholders,hints,labels}.*' # - 'simple_form.{error_notification,required}.:' +ignore_unused: + - 'unauthorized.*' ## Exclude these keys from the `i18n-tasks eq-base' report: # ignore_eq_base: diff --git a/config/locales/en.yml b/config/locales/en.yml index 79e4e1129..3ab672c01 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -89,3 +89,7 @@ en: subject: Someone has commented on your debate reply: subject: Someone has replied to your comment + unauthorized: + default: "You are not authorized to access this page." + manage: + all: "You are not authorized to %{action} %{subject}." diff --git a/config/locales/es.yml b/config/locales/es.yml index 8daf07d42..5261d14c3 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -89,3 +89,21 @@ es: subject: Alguien ha comentado en tu debate reply: subject: Alguien ha respondido a tu comentario + unauthorized: + default: "No tienes permiso para acceder a esta página." + index: + all: "No tienes permiso para listar %{subject}" + show: + all: "No tienes permiso para ver %{subject}" + edit: + all: "No tienes permiso para editar %{subject}" + update: + all: "No tienes permiso para modificar %{subject}" + create: + all: "No tienes permiso para crear %{subject}" + delete: + all: "No tienes permiso para borrar %{subject}" + manage: + all: "No tienes permiso para realizar la acción '%{action}' sobre %{subject}." + +