Add pundit policy for several controllers (#476)

* Add pundit policy for several controllers

* Adjust video spec

* Fix tag request specs

* Add proper twilio tokens request specs

* Remove puts statements

app/controllers/tags_controller.rb

@@ -1,20 +1,22 @@
 class TagsController < ApplicationController
   before_action :set_cache_control_headers, only: [:index]
   before_action :authenticate_user!, only: %i(edit update)
+  after_action :verify_authorized
 
   def index
+    skip_authorization
     @tags_index = true
     @tags = Tag.all.order("hotness_score DESC").first(100)
   end
 
   def edit
     @tag = Tag.find_by!(name: params[:tag])
-    check_authorization
+    authorize @tag
   end
 
   def update
     @tag = Tag.find_by!(id: params[:id])
-    check_authorization
+    authorize @tag
     if @tag.errors.messages.blank? && @tag.update(tag_params)
       flash[:success] = "Tag successfully updated! 👍 "
       redirect_to "/t/#{@tag.name}/edit"
@@ -26,11 +28,6 @@ class TagsController < ApplicationController
 
   private
 
-  def check_authorization
-    raise "UNAUTHORIZED" unless current_user.has_role?(:super_admin) ||
-        current_user.has_role?(:tag_moderator, @tag)
-  end
-
   def convert_empty_string_to_nil
     # nil plays nicely with our hex colors, whereas empty string doesn't
     params[:tag][:text_color_hex] = nil if params[:tag][:text_color_hex] == ""

app/controllers/twilio_tokens_controller.rb

@@ -1,14 +1,16 @@
 class TwilioTokensController < ApplicationController
+  after_action :verify_authorized
 
   def show
-    video_channel = params[:id]
-    if (video_channel.start_with?("private-video-channel-") &&
-      ChatChannel.find(video_channel.split("private-video-channel-")[1].to_i)).has_member?(current_user)
-      @twilio_token = TwilioToken.new(current_user, params[:id]).get
-      render json: { status: "success", token: @twilio_token }, status: 200
-    else
-      render json: { status: "failure", message: "User does not have permission" }, status: 401
+    video_channel = params[:id].split("private-video-channel-")[1] if params[:id].start_with?("private-video-channel-")
+    unless video_channel
+      skip_authorization
+      render json: { status: "failure", token: @twilio_token }, status: 404
+      return
     end
+    @chat_channel = ChatChannel.find(video_channel.to_i)
+    authorize @chat_channel #show pundit method for chat_channel_policy works here, should always check though
+    @twilio_token = TwilioToken.new(current_user, params[:id]).get
+    render json: { status: "success", token: @twilio_token }, status: 200
   end
-
-end
+end

app/controllers/video_states_controller.rb

@@ -1,5 +1,7 @@
 class VideoStatesController < ApplicationController
   skip_before_action :verify_authenticity_token
+  # Not authorized using pundit because user this is not accessed via user session
+  # This is purely for responding to AWS video state changes.
 
   def create
     unless valid_user

app/controllers/videos_controller.rb

@@ -1,10 +1,12 @@
 class VideosController < ApplicationController
-  before_action :check_video_permission
+  after_action :verify_authorized
 
   def new
+    authorize :video
   end
 
   def create
+    authorize :video
     @article = Article.new(body_markdown:"---\ntitle: Unpublished Video ~ #{rand(100000).to_s(26)}\npublished: false\ndescription: \ntags: \n---\n\n",processed_html:"")
     @article.user_id = current_user.id
     @article.show_comments = true
@@ -14,7 +16,7 @@ class VideosController < ApplicationController
   end
 
   def assign_video_attributes
-    if params[:article][:video] && current_user.has_role?(:video_permission)
+    if params[:article][:video]
       @article.video = params[:article][:video]
       @article.video_state = "PROGRESSING"
       @article.video_code = @article.video.split("dev-to-input-v0/")[1]
@@ -23,10 +25,4 @@ class VideosController < ApplicationController
 
     end
   end
-
-  private
-
-  def check_video_permission
-    redirect_to "/enter" unless current_user&.has_role?(:video_permission)
-  end
 end

app/policies/tag_policy.rb

@@ -0,0 +1,20 @@
+class TagPolicy < ApplicationPolicy
+  def index?
+    true
+  end
+
+  def edit?
+    has_mod_permission?
+  end
+
+  def update?
+    has_mod_permission?
+  end
+
+  private
+
+  def has_mod_permission?
+    user.has_role?(:super_admin) ||
+      user.has_role?(:tag_moderator, record)
+  end
+end

app/policies/video_policy.rb

@@ -0,0 +1,9 @@
+class VideoPolicy < ApplicationPolicy
+  def new?
+    user.has_role?(:video_permission)
+  end
+
+  def create?
+    user.has_role?(:video_permission)
+  end
+end

app/views/articles/_video_player.html.erb

@@ -3,7 +3,7 @@
     <meta itemprop="uploadDate" content="<%= @article.published_at %>" />
     <meta itemprop="name" content="<%= @article.title %>" />
     <meta itemprop="description" content="<%= @article.description %>" />
-    <meta itemprop="thumbnailUrl" content="<%= article.video_thumbnail_url %>" />
+    <meta itemprop="thumbnailUrl" content="<%= cloudinary(article.video_thumbnail_url, 880) %>" />
     <meta itemprop="contentUrl" content="<%= article.video_source_url %>" />
   <% end %>
   <script src="//content.jwplatform.com/libraries/b1zWy2iv.js" async> </script>

spec/policies/tag_policy_spec.rb

@@ -0,0 +1,20 @@
+require "rails_helper"
+
+RSpec.describe TagPolicy do
+  subject { described_class.new(user, tag) }
+
+  let(:tag) { build(:tag) }
+
+  context "when user is not signed-in" do
+    let(:user) { nil }
+
+    it { within_block_is_expected.to raise_error(Pundit::NotAuthorizedError) }
+  end
+
+  context "when user is not a moderator" do
+    let(:user) { build(:user) }
+
+    it { is_expected.to permit_actions(%i[index]) }
+    it { is_expected.to forbid_actions(%i[edit update]) }
+  end
+end

spec/policies/video_policy_spec.rb

@@ -0,0 +1,22 @@
+require 'rails_helper'
+
+RSpec.describe VideoPolicy do
+  subject { described_class.new(user, nil) }
+  let(:user) { User.new }
+
+  context "when user is not signed-in" do
+    let(:user) { nil }
+    it { within_block_is_expected.to raise_error(Pundit::NotAuthorizedError) }
+  end
+
+  context "when user does not have video permission" do
+    let(:user) { build(:user) }
+    it { is_expected.to forbid_actions(%i[new create]) }
+  end
+
+  context "when user does not have video permission" do
+    let(:user) { build(:user) }
+    before { user.add_role :video_permission }
+    it { is_expected.to permit_actions(%i[new create]) }
+  end
+end

spec/requests/tags_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe "Tags", type: :request do
 
     it "does not allow users who are not tag moderators" do
       sign_in unauthorized_user
-      expect { get "/t/#{tag}/edit" }.to raise_error("UNAUTHORIZED")
+      expect { get "/t/#{tag}/edit" }.to raise_error(Pundit::NotAuthorizedError)
     end
 
     it "allows super admins" do
@@ -43,7 +43,7 @@ RSpec.describe "Tags", type: :request do
       end
 
       it "does not allow moderators of one tag to edit another tag" do
-        expect { get "/t/#{another_tag}/edit" }.to raise_error("UNAUTHORIZED")
+        expect { get "/t/#{another_tag}/edit" }.to raise_error(Pundit::NotAuthorizedError)
       end
     end
   end
@@ -63,7 +63,7 @@ RSpec.describe "Tags", type: :request do
 
     it "does not allow unauthorized users" do
       sign_in unauthorized_user
-      expect { patch "/tag/#{tag.id}" }.to raise_error("UNAUTHORIZED")
+      expect { patch "/tag/#{tag.id}" }.to raise_error(Pundit::NotAuthorizedError)
     end
 
     it "allows super admins" do
@@ -93,7 +93,7 @@ RSpec.describe "Tags", type: :request do
 
       it "does not allow moderators of one tag to edit another tag" do
         expect { patch("/tag/#{another_tag.id}", params: valid_params) }.
-          to raise_error("UNAUTHORIZED")
+          to raise_error(Pundit::NotAuthorizedError)
       end
     end
   end

spec/requests/top_tags_spec.rb

@@ -1,10 +0,0 @@
-require "rails_helper"
-
-RSpec.describe "ArticlesApi", type: :request do
-  describe "GET /tags" do
-    it "returns proper page" do
-      get "/tags"
-      expect(response.body).to include("Top 100 Tags")
-    end
-  end
-end

spec/requests/twilio_tokens_spec.rb

@@ -0,0 +1,35 @@
+require "rails_helper"
+
+RSpec.describe "TwilioTokens", type: :request do
+  let(:user) { create(:user) }
+  let(:chat_channel) { create(:chat_channel) }
+  
+  before do
+    sign_in user
+  end
+
+  describe "GET /twilio_tokens/:id" do
+    it "returns a token for member of a channel" do
+      chat_channel.add_users [user]
+      get "/twilio_tokens/private-video-channel-#{chat_channel.id}"
+      expect(response.status).to eq(200)
+    end
+
+    it "returns not found if unknown ID" do
+      chat_channel.add_users [user]
+      get "/twilio_tokens/sddds3423443efrwdfsd"
+      expect(response.status).to eq(404)
+    end
+
+    it "returns not found if wrong ID prefix" do
+      chat_channel.add_users [user]
+      get "/twilio_tokens/sddds3423443efrwdfsd-#{chat_channel.id}"
+      expect(response.status).to eq(404)
+    end
+
+    it "returns unauthorized if user not member of channel" do
+      expect { get "/twilio_tokens/private-video-channel-#{chat_channel.id}" }.
+        to raise_error(Pundit::NotAuthorizedError)
+    end
+  end
+end

spec/requests/videos_spec.rb

@@ -7,14 +7,12 @@ RSpec.describe "Videos", type: :request do
   describe "GET /videos/new" do
     context "when not authorized" do
       it "redirects non-logged in users" do
-        get "/videos/new"
-        is_expected.to redirect_to("/enter")
+        expect { get "/videos/new"}.to raise_error(Pundit::NotAuthorizedError)
       end
 
       it "redirects logged in users" do
         login_as unauthorized_user
-        get "/videos/new"
-        is_expected.to redirect_to("/enter")
+        expect { get "/videos/new"}.to raise_error(Pundit::NotAuthorizedError)
       end
     end
 
@@ -30,14 +28,12 @@ RSpec.describe "Videos", type: :request do
   describe "POST /videos" do
     context "when not authorized" do
       it "redirects non-logged in users" do
-        post "/videos"
-        is_expected.to redirect_to("/enter")
+        expect { post "/videos"}.to raise_error(Pundit::NotAuthorizedError)
       end
 
       it "redirects logged in users" do
         login_as unauthorized_user
-        post "/videos"
-        is_expected.to redirect_to("/enter")
+        expect { post "/videos"}.to raise_error(Pundit::NotAuthorizedError)
       end
     end