diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index feedc6a24..6ba71607e 100644 --- a/app/controllers/tags_controller.rb +++ b/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] == "" diff --git a/app/controllers/twilio_tokens_controller.rb b/app/controllers/twilio_tokens_controller.rb index e440cbc39..1c7296d79 100644 --- a/app/controllers/twilio_tokens_controller.rb +++ b/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 \ No newline at end of file +end diff --git a/app/controllers/video_states_controller.rb b/app/controllers/video_states_controller.rb index dd006dcb9..dcdc25ff1 100644 --- a/app/controllers/video_states_controller.rb +++ b/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 diff --git a/app/controllers/videos_controller.rb b/app/controllers/videos_controller.rb index 622ccccc5..d60f10024 100644 --- a/app/controllers/videos_controller.rb +++ b/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 diff --git a/app/policies/tag_policy.rb b/app/policies/tag_policy.rb new file mode 100644 index 000000000..2c9796e9a --- /dev/null +++ b/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 diff --git a/app/policies/video_policy.rb b/app/policies/video_policy.rb new file mode 100644 index 000000000..d5c5b4395 --- /dev/null +++ b/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 diff --git a/app/views/articles/_video_player.html.erb b/app/views/articles/_video_player.html.erb index dd6634a9b..e39009d3b 100644 --- a/app/views/articles/_video_player.html.erb +++ b/app/views/articles/_video_player.html.erb @@ -3,7 +3,7 @@ - + <% end %> diff --git a/spec/policies/tag_policy_spec.rb b/spec/policies/tag_policy_spec.rb new file mode 100644 index 000000000..ed298ba7a --- /dev/null +++ b/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 diff --git a/spec/policies/video_policy_spec.rb b/spec/policies/video_policy_spec.rb new file mode 100644 index 000000000..d0f7c47c6 --- /dev/null +++ b/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 diff --git a/spec/requests/tags_spec.rb b/spec/requests/tags_spec.rb index 760649b33..006f9b726 100644 --- a/spec/requests/tags_spec.rb +++ b/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 diff --git a/spec/requests/top_tags_spec.rb b/spec/requests/top_tags_spec.rb deleted file mode 100644 index 633e01608..000000000 --- a/spec/requests/top_tags_spec.rb +++ /dev/null @@ -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 diff --git a/spec/requests/twilio_tokens_spec.rb b/spec/requests/twilio_tokens_spec.rb new file mode 100644 index 000000000..149c1503a --- /dev/null +++ b/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 diff --git a/spec/requests/videos_spec.rb b/spec/requests/videos_spec.rb index 2cbd2c668..7a874a335 100644 --- a/spec/requests/videos_spec.rb +++ b/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