Introduce admin

Gemfile

@@ -31,6 +31,7 @@ gem 'yajl-ruby',            '~> 1.1.0', :require => 'yajl'
 gem 'newrelic_rpm'
 gem 'virtus',               '~> 1.0.1'
 gem 'warden',               '~> 1.2.3'
+gem 'pundit',               '~> 0.2.3'
 
 group :development do
   gem 'quiet_assets',   '~> 1.0.1'

Gemfile.lock

@@ -212,6 +212,8 @@ GEM
       slop (~> 3.4)
     pry-rails (0.3.2)
       pry (>= 0.9.10)
+    pundit (0.2.3)
+      activesupport (>= 3.0.0)
     quiet_assets (1.0.2)
       railties (>= 3.1, < 5.0)
     rack (1.5.2)
@@ -378,6 +380,7 @@ DEPENDENCIES
   pg (~> 0.14)
   poltergeist (~> 1.5.0)
   pry-rails (~> 0.3.2)
+  pundit (~> 0.2.3)
   quiet_assets (~> 1.0.1)
   rails (= 4.0.4)
   rake (~> 10.0.4)

app/controllers/application_controller.rb

@@ -4,16 +4,13 @@ class ApplicationController < ActionController::Base
 
   protect_from_forgery
 
-  class Unauthorized < Exception; end
-  class Forbidden < Exception; end
-
-  rescue_from ActiveRecord::RecordNotFound, :with => :not_found
-  rescue_from Unauthorized, :with => :unauthorized
-  rescue_from Forbidden, :with => :forbidden
+  rescue_from ActiveRecord::RecordNotFound, with: :handle_not_found
+  rescue_from Pundit::NotAuthorizedError, with: :handle_unauthorized
 
   helper_method :decorated_current_user
 
   include WardenAuthentication
+  include Pundit
 
   private
 
@@ -26,7 +23,7 @@ class ApplicationController < ActionController::Base
   end
 
   def ensure_authenticated!
-    raise Unauthorized unless current_user
+    handle_unauthenticated unless current_user
   end
 
   def omniauth_credentials
@@ -51,31 +48,31 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def forbidden
+  def handle_unauthorized
     if request.xhr?
-      render :json => "Forbidden", :status => 403
+      render json: "Unauthorized", status: 403
     else
-      redirect_to root_path, :alert => "This action is forbidden"
+      redirect_to(request.referrer || root_path, alert: "You can't do that.")
     end
   end
 
-  def unauthorized
+  def handle_unauthenticated
     if request.xhr?
-      render :json => "Unauthorized", :status => 401
+      render json: "Unauthenticated", status: 401
     else
       store_location
-      redirect_to login_path, :notice => "Please sign in to proceed"
+      redirect_to login_path, notice: "Please sign in to proceed"
     end
   end
 
-  def not_found
+  def handle_not_found
     respond_to do |format|
       format.any do
-        render :text => 'Requested resource not found', :status => 404
+        render text: 'Requested resource not found', status: 404
       end
 
       format.html do
-        render 'application/not_found', :status => 404, :layout => 'application'
+        render 'application/not_found', status: 404, layout: 'application'
       end
     end
   end

app/controllers/asciicasts_controller.rb

@@ -2,7 +2,6 @@ class AsciicastsController < ApplicationController
 
   before_filter :load_resource, except: [:index]
   before_filter :ensure_authenticated!, only: [:edit, :update, :destroy]
-  before_filter :ensure_owner!, only: [:edit, :update, :destroy]
 
   respond_to :html, :json
 
@@ -35,9 +34,12 @@ class AsciicastsController < ApplicationController
   end
 
   def edit
+    authorize asciicast
   end
 
   def update
+    authorize asciicast
+
     if asciicast.update_attributes(update_params)
       redirect_to asciicast_path(asciicast),
                   :notice => 'Asciicast was updated.'
@@ -47,6 +49,8 @@ class AsciicastsController < ApplicationController
   end
 
   def destroy
+    authorize asciicast
+
     if asciicast.destroy
       redirect_to profile_path(current_user),
                   :notice => 'Asciicast was deleted.'
@@ -63,18 +67,12 @@ class AsciicastsController < ApplicationController
     @asciicast = Asciicast.find(params[:id])
   end
 
-  def ensure_owner!
-    if current_user != asciicast.user
-      redirect_to asciicast_path(asciicast), :alert => "You can't do that."
-    end
-  end
-
   def view_counter
     @view_counter ||= ViewCounter.new
   end
 
   def update_params
-    params.require(:asciicast).permit(:title, :description, :theme_name)
+    params.require(:asciicast).permit(*policy(asciicast).permitted_attributes)
   end
 
 end

app/controllers/users_controller.rb

@@ -27,10 +27,12 @@ class UsersController < ApplicationController
 
   def edit
     @user = current_user
+    authorize @user
   end
 
   def update
     @user = User.find(current_user.id)
+    authorize @user
 
     if @user.update_attributes(update_params)
       redirect_to profile_path(@user), notice: 'Account settings saved.'

app/models/asciicast.rb

@@ -66,14 +66,6 @@ class Asciicast < ActiveRecord::Base
     terminal.release if terminal
   end
 
-  def managable_by?(user)
-    if user
-      user == self.user
-    else
-      false
-    end
-  end
-
   def theme
     theme_name.presence && Theme.for_name(theme_name)
   end

app/models/user.rb

@@ -105,10 +105,6 @@ class User < ActiveRecord::Base
     asciicasts.where('id <> ?', asciicast.id).order('RANDOM()').limit(limit)
   end
 
-  def editable_by?(user)
-    user && user.id == id
-  end
-
   def paged_asciicasts(page, per_page)
     asciicasts.
       includes(:user).
@@ -116,6 +112,10 @@ class User < ActiveRecord::Base
       paginate(page, per_page)
   end
 
+  def admin?
+    CFG.admin_ids.include?(id)
+  end
+
   private
 
   def generate_auth_token

app/policies/application_policy.rb

@@ -0,0 +1,42 @@
+class ApplicationPolicy
+
+  attr_reader :user, :record
+
+  def initialize(user, record)
+    @user = user
+    @record = record
+  end
+
+  def index?
+    false
+  end
+
+  def show?
+    scope.where(:id => record.id).exists?
+  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
+
+end

app/policies/asciicast_policy.rb

@@ -0,0 +1,44 @@
+class AsciicastPolicy < ApplicationPolicy
+
+  class Scope < Struct.new(:user, :scope)
+    def resolve
+      scope
+    end
+  end
+
+  def permitted_attributes
+    if user.admin? || record.user == user
+      attrs = [:title, :description, :theme_name]
+      attrs << :featured if user.admin?
+
+      attrs
+    else
+      []
+    end
+  end
+
+  def update?
+    return false unless user
+
+    user.admin? || record.user == user
+  end
+
+  def destroy?
+    return false unless user
+
+    user.admin? || record.user == user
+  end
+
+  def feature?
+    return false unless user
+
+    user.admin?
+  end
+
+  def unfeature?
+    return false unless user
+
+    user.admin?
+  end
+
+end

app/policies/user_policy.rb

@@ -0,0 +1,13 @@
+class UserPolicy < ApplicationPolicy
+
+  class Scope < Struct.new(:user, :scope)
+    def resolve
+      scope
+    end
+  end
+
+  def update?
+    record == user
+  end
+
+end

app/presenters/asciicast_page_presenter.rb

@@ -1,20 +1,23 @@
 class AsciicastPagePresenter
 
-  attr_reader :asciicast, :current_user, :playback_options
+  attr_reader :asciicast, :current_user, :policy, :playback_options
 
   def self.build(asciicast, current_user, playback_options)
     decorated_asciicast = asciicast.decorate
+    policy = Pundit.policy(current_user, asciicast)
 
     playback_options = {
       'theme' =>  decorated_asciicast.theme_name
     }.merge(playback_options)
 
-    new(decorated_asciicast, current_user, PlaybackOptions.new(playback_options))
+    new(decorated_asciicast, current_user, policy,
+        PlaybackOptions.new(playback_options))
   end
 
-  def initialize(asciicast, current_user, playback_options)
+  def initialize(asciicast, current_user, policy, playback_options)
     @asciicast        = asciicast
     @current_user     = current_user
+    @policy           = policy
     @playback_options = playback_options
   end
 
@@ -46,14 +49,33 @@ class AsciicastPagePresenter
     asciicast.views_count
   end
 
-  def embed_script(h)
-    src = h.asciicast_url(asciicast, format: :js)
+  def embed_script(routes)
+    src = routes.asciicast_url(asciicast, format: :js)
     id = "asciicast-#{asciicast.id}"
     %(<script type="text/javascript" src="#{src}" id="#{id}" async></script>)
   end
 
   def show_admin_dropdown?
-    asciicast.managable_by?(current_user)
+    [show_edit_link?,
+     show_delete_link?,
+     show_set_featured_link?,
+     show_unset_featured_link?].any?
+  end
+
+  def show_edit_link?
+    policy.update?
+  end
+
+  def show_delete_link?
+    policy.destroy?
+  end
+
+  def show_set_featured_link?
+    !asciicast.featured? && policy.feature?
+  end
+
+  def show_unset_featured_link?
+    asciicast.featured? && policy.unfeature?
   end
 
   def show_description?

app/presenters/user_page_presenter.rb

@@ -2,15 +2,17 @@ class UserPagePresenter
 
   PER_PAGE = 15
 
-  attr_reader :user, :current_user, :page, :per_page
+  attr_reader :user, :current_user, :policy, :page, :per_page
 
   def self.build(user, current_user, page = nil, per_page = nil)
-    new(user.decorate, current_user, page || 1, per_page || PER_PAGE)
+    policy = Pundit.policy(current_user, user)
+    new(user.decorate, current_user, policy, page || 1, per_page || PER_PAGE)
   end
 
-  def initialize(user, current_user, page, per_page)
+  def initialize(user, current_user, policy, page, per_page)
     @user         = user
     @current_user = current_user
+    @policy       = policy
     @page         = page
     @per_page     = per_page
   end
@@ -32,7 +34,7 @@ class UserPagePresenter
   end
 
   def show_settings?
-    user.editable_by?(current_user)
+    policy.update?
   end
 
   def asciicast_count_text(h)

app/views/asciicasts/show.html.slim

@@ -36,14 +36,26 @@
                 ' 
                 span.caret
               ul.dropdown-menu
-                li
-                  = link_to edit_asciicast_path(page.asciicast) do
-                    span.glyphicon.glyphicon-edit
-                    '  Edit
-                li
-                  = link_to(asciicast_path(page.asciicast), method: :delete, data: { confirm: 'Really delete this asciicast?' }) do
-                    span.glyphicon.glyphicon-remove
-                    '  Delete
+                - if page.show_edit_link?
+                  li
+                    = link_to edit_asciicast_path(page.asciicast) do
+                      span.glyphicon.glyphicon-edit
+                      '  Edit
+                - if page.show_set_featured_link?
+                  li
+                    = link_to(asciicast_path(page.asciicast, 'asciicast[featured]' => 1), method: :put) do
+                      span.glyphicon.glyphicon-eye-open
+                      '  Make featured
+                - if page.show_unset_featured_link?
+                  li
+                    = link_to(asciicast_path(page.asciicast, 'asciicast[featured]' => 0), method: :put) do
+                      span.glyphicon.glyphicon-eye-close
+                      '  Make not featured
+                - if page.show_delete_link?
+                  li
+                    = link_to(asciicast_path(page.asciicast), method: :delete, data: { confirm: 'Really delete this asciicast?' }) do
+                      span.glyphicon.glyphicon-remove
+                      '  Delete
 
           ul.meta-list.actions-list
             li

config/cfg.rb

@@ -20,6 +20,7 @@ module Asciinema
     attribute :secret_token,                   String, default: '21deaa1a1228e119434aa783ecb4af21be7513ff1f5b8c1d8894241e5fc70ad395db72c8c1b0508a0ebb994ed88a8d73f6c84e44f7a4bc554a40d77f9844d2f4'
     attribute :twitter_consumer_key,           String
     attribute :twitter_consumer_secret,        String
+    attribute :admin_ids,                      Array[Integer]
 
     alias_method :local_persona_js?, :local_persona_js
 

spec/controllers/application_controller_spec.rb

@@ -3,11 +3,11 @@ require 'spec_helper'
 class FakesController < ApplicationController
 
   def foo
-    raise Unauthorized
+    ensure_authenticated!
   end
 
   def bar
-    raise Forbidden
+    raise Pundit::NotAuthorizedError
   end
 
   def store
@@ -41,20 +41,17 @@ describe FakesController do
   end
 
   describe "action raise unauthorized" do
-
     context "when xhr" do
       before { allow(request).to receive(:xhr?).and_return(true) }
 
-      it "response with 401" do
+      it "responds with 401" do
         get :foo
 
         expect(response.status).to eq(401)
       end
-
     end
 
-    context "when typical request" do
-
+    context "when normal request" do
       it "redirects to login_path" do
         expect(@controller).to receive(:store_location)
 
@@ -63,30 +60,27 @@ describe FakesController do
         expect(flash[:notice]).to eq("Please sign in to proceed")
         should redirect_to(login_path)
       end
-
     end
   end
 
-  context "when action raise forbidden" do
+  context "when action raises Pundit::NotAuthorizedError" do
     context "when xhr" do
       before { allow(request).to receive(:xhr?).and_return(true) }
 
-      it "response with 401" do
+      it "responds with 403" do
         get :bar
 
         expect(response.status).to eq(403)
       end
     end
 
-    context "when typical request" do
-
+    context "when normal request" do
       it "redirects to root_path" do
         get :bar
 
-        expect(flash[:alert]).to eq("This action is forbidden")
+        expect(flash[:alert]).to_not be(nil)
         should redirect_to(root_path)
       end
-
     end
   end
 

spec/controllers/asciicasts_controller_spec.rb

@@ -6,7 +6,7 @@ shared_examples_for 'guest user trying to modify' do
 end
 
 shared_examples_for 'non-owner user trying to modify' do
-  it { should redirect_to(asciicast_path(asciicast)) }
+  it { should redirect_to(root_path) }
   specify { expect(flash[:alert]).to match(/can't/) }
 end
 

spec/models/asciicast_spec.rb

@@ -106,37 +106,6 @@ describe Asciicast do
     end
   end
 
-  describe '#managable_by?' do
-    subject { asciicast.managable_by?(user) }
-
-    let(:asciicast) { Asciicast.new }
-    let(:owner) { nil }
-
-    before do
-      asciicast.user = owner
-    end
-
-    context "when user is nil" do
-      let(:user) { nil }
-
-      it { should be(false) }
-    end
-
-    context "when user is owner of the asciicast" do
-      let(:user) { User.new }
-      let(:owner) { user }
-
-      it { should be(true) }
-    end
-
-    context "when user isn't owner of the asciicast" do
-      let(:user) { User.new }
-      let(:owner) { User.new }
-
-      it { should be(false) }
-    end
-  end
-
   describe '#theme' do
     it 'returns proper theme when theme_name is not blank' do
       asciicast = described_class.new(theme_name: 'tango')

spec/models/user_spec.rb

@@ -297,24 +297,6 @@ describe User do
     end
   end
 
-  describe '#editable_by?' do
-    subject { user.editable_by?(other) }
-
-    let(:user) { create(:user) }
-
-    context "when it's the same user" do
-      let(:other) { user }
-
-      it { should be(true) }
-    end
-
-    context "when it's a different user" do
-      let(:other) { create(:user) }
-
-      it { should be(false) }
-    end
-  end
-
   describe '#merge_to' do
     subject { user.merge_to(target_user) }
 

spec/policies/asciicast_policy_spec.rb

@@ -0,0 +1,109 @@
+require 'spec_helper'
+
+describe AsciicastPolicy do
+
+  subject { described_class }
+
+  describe '#permitted_attributes' do
+    subject { Pundit.policy(user, asciicast).permitted_attributes }
+
+    let(:asciicast) { Asciicast.new }
+
+    context "when user is admin" do
+      let(:user) { stub_model(User, admin?: true) }
+
+      it "includes featured" do
+        expect(subject).to eq([:title, :description, :theme_name, :featured])
+      end
+    end
+
+    context "when user isn't admin" do
+      let(:user) { stub_model(User, admin?: false) }
+
+      it "is empty" do
+        expect(subject).to eq([])
+      end
+
+      context "and is creator of the asciicast" do
+        let(:asciicast) { Asciicast.new(user: user) }
+
+        it "doesn't include featured" do
+          expect(subject).to eq([:title, :description, :theme_name])
+        end
+      end
+    end
+  end
+
+  permissions :update? do
+    it "denies access if user is nil" do
+      expect(subject).not_to permit(nil, Asciicast.new)
+    end
+
+    it "grants access if user is admin" do
+      user = stub_model(User, admin?: true)
+      expect(subject).to permit(user, Asciicast.new)
+    end
+
+    it "grants access if user is creator of the asciicast" do
+      user = stub_model(User, admin?: false)
+      expect(subject).to permit(user, Asciicast.new(user: user))
+    end
+
+    it "denies access if user isn't the creator of the asciicast" do
+      expect(subject).not_to permit(User.new, Asciicast.new(user: User.new))
+    end
+  end
+
+  permissions :destroy? do
+    it "denies access if user is nil" do
+      expect(subject).not_to permit(nil, Asciicast.new)
+    end
+
+    it "grants access if user is admin" do
+      user = stub_model(User, admin?: true)
+      expect(subject).to permit(user, Asciicast.new)
+    end
+
+    it "grants access if user is creator of the asciicast" do
+      user = stub_model(User, admin?: false)
+      expect(subject).to permit(user, Asciicast.new(user: user))
+    end
+
+    it "denies access if user isn't the creator of the asciicast" do
+      expect(subject).not_to permit(User.new, Asciicast.new(user: User.new))
+    end
+  end
+
+  permissions :feature? do
+    it "denies access if user is nil" do
+      expect(subject).not_to permit(nil, Asciicast.new)
+    end
+
+    it "grants access if user is admin" do
+      user = stub_model(User, admin?: true)
+      expect(subject).to permit(user, Asciicast.new)
+    end
+
+    it "denies access if user isn't admin" do
+      user = stub_model(User, admin?: false)
+      expect(subject).not_to permit(user, Asciicast.new)
+    end
+  end
+
+  permissions :unfeature? do
+    it "denies access if user is nil" do
+      expect(subject).not_to permit(nil, Asciicast.new)
+    end
+
+    it "grants access if user is admin" do
+      user = stub_model(User, admin?: true)
+      expect(subject).to permit(user, Asciicast.new)
+    end
+
+    it "denies access if user isn't admin" do
+      user = stub_model(User, admin?: false)
+      expect(subject).not_to permit(user, Asciicast.new)
+    end
+  end
+
+end

spec/policies/user_policy_spec.rb

@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe UserPolicy do
+
+  subject { described_class }
+
+  permissions :update? do
+    it "grants access if edited user is current user" do
+      user = User.new
+      expect(subject).to permit(user, user)
+    end
+
+    it "denies access if edited user is not current user" do
+      expect(subject).not_to permit(User.new, User.new)
+    end
+  end
+
+end

spec/presenters/asciicast_page_presenter_spec.rb

@@ -24,9 +24,10 @@ describe AsciicastPagePresenter do
     end
   end
 
-  let(:presenter) { described_class.new(asciicast, current_user, nil) }
+  let(:presenter) { described_class.new(asciicast, current_user, policy, nil) }
   let(:asciicast) { stub_model(Asciicast, user: author) }
   let(:current_user) { User.new }
+  let(:policy) { double('policy') }
   let(:author) { User.new }
 
   let(:view_context) {
@@ -127,27 +128,6 @@ describe AsciicastPagePresenter do
     end
   end
 
-  describe '#show_admin_dropdown?' do
-    subject { presenter.show_admin_dropdown? }
-
-    before do
-      allow(asciicast).to receive(:managable_by?).
-        with(current_user) { managable }
-    end
-
-    context "when asciicast can't be managed by the user" do
-      let(:managable) { false }
-
-      it { should be(false) }
-    end
-
-    context "when asciicast can be managed by the user" do
-      let(:managable) { true }
-
-      it { should be(true) }
-    end
-  end
-
   describe '#show_description?' do
     subject { presenter.show_description? }
 

spec/presenters/user_page_presenter_spec.rb

@@ -44,9 +44,11 @@ describe UserPagePresenter do
     end
   end
 
-  let(:presenter) { described_class.new(user, current_user, page, per_page) }
-  let(:user) { double('user', username: 'cartman') }
-  let(:current_user) { double('current_user') }
+  let(:presenter) { described_class.new(user, current_user, policy, page,
+                                        per_page) }
+  let(:user) { stub_model(User, username: 'cartman') }
+  let(:current_user) { stub_model(User) }
+  let(:policy) { double('policy') }
   let(:page) { 2 }
   let(:per_page) { 5 }
 
@@ -95,11 +97,21 @@ describe UserPagePresenter do
   describe '#show_settings?' do
     subject { presenter.show_settings? }
 
-    before do
-      allow(user).to receive(:editable_by?).with(current_user) { :right }
+    context "when policy allows for update" do
+      before do
+        allow(policy).to receive(:update?) { true }
+      end
+
+      it { should be(true) }
     end
 
-    it { should eq(:right) }
+    context "when policy doesn't allow for update" do
+      before do
+        allow(policy).to receive(:update?) { false }
+      end
+
+      it { should be(false) }
+    end
   end
 
   describe '#asciicast_count_text' do
@@ -152,7 +164,7 @@ describe UserPagePresenter do
     end
 
     context "when current_user is a different user" do
-      let(:current_user) { double('other_user') }
+      let(:current_user) { stub_model(User) }
 
       it { should be(false) }
     end

spec/spec_helper.rb

@@ -18,6 +18,7 @@ require 'rspec/rails'
 require 'capybara/rspec'
 require 'capybara/poltergeist'
 require 'sidekiq/testing'
+require 'pundit/rspec'
 
 Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }
 Dir[Rails.root.join("spec/shared/**/*.rb")].each  { |f| require f }