diff --git a/app/controllers/api/v1/recordings_controller.rb b/app/controllers/api/v1/recordings_controller.rb
index 7255133ce4e992d8266cacd70465f8ab1077ef59..950f232e818684c790a604b805547d77de2b145c 100644
--- a/app/controllers/api/v1/recordings_controller.rb
+++ b/app/controllers/api/v1/recordings_controller.rb
@@ -19,12 +19,14 @@
module Api
module V1
class RecordingsController < ApiController
+ skip_before_action :ensure_authenticated, only: :recording_url
+
before_action :find_recording, only: %i[update update_visibility recording_url]
before_action only: %i[destroy] do
ensure_authorized('ManageRecordings', record_id: params[:id])
end
before_action only: %i[update update_visibility recording_url] do
- ensure_authorized(%w[ManageRecordings SharedRoom], record_id: params[:id])
+ ensure_authorized(%w[ManageRecordings SharedRoom PublicRecordings], record_id: params[:id])
end
before_action only: %i[index recordings_count] do
ensure_authorized('CreateRoom')
@@ -92,16 +94,29 @@ module Api
def recording_url
record_format = params[:recording_format]
- url = if @recording.visibility == 'Protected'
- recording = BigBlueButtonApi.new(provider: current_provider).get_recording(record_id: @recording.record_id)
- formats = recording[:playback][:format]
-
- record_format.present? ? formats.find { |format| format[:type] == record_format }[:url] : formats.pluck(:url)
- else
- record_format.present? ? @recording.formats.find_by(recording_type: record_format).url : @recording.formats.pluck(:url)
- end
-
- render_data data: url, status: :ok
+ urls = if [Recording::VISIBILITIES[:protected], Recording::VISIBILITIES[:public_protected]].include? @recording.visibility
+ recording = BigBlueButtonApi.new(provider: current_provider).get_recording(record_id: @recording.record_id)
+ formats = recording[:playback][:format]
+ formats = [formats] unless formats.is_a? Array
+
+ if record_format.present?
+ found_format = formats.find { |format| format[:type] == record_format }
+ return render_error status: :not_found unless found_format
+
+ found_format[:url]
+ else
+ formats.pluck(:url)
+ end
+ elsif record_format.present?
+ found_format = @recording.formats.find_by(recording_type: record_format)
+ return render_error status: :not_found unless found_format
+
+ found_format[:url]
+ else
+ @recording.formats.pluck(:url)
+ end
+
+ render_data data: urls, status: :ok
end
private
diff --git a/app/services/permissions_checker.rb b/app/services/permissions_checker.rb
index 39f878330523fcedecd65c76d3864e9a4c68b4c3..380251c6cd246a42d046703c46aeb0038fdc7e34 100644
--- a/app/services/permissions_checker.rb
+++ b/app/services/permissions_checker.rb
@@ -16,6 +16,7 @@
# frozen_string_literal: true
+# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
class PermissionsChecker
def initialize(current_user:, permission_names:, current_provider:, user_id: nil, friendly_id: nil, record_id: nil)
@current_user = current_user
@@ -26,20 +27,29 @@ class PermissionsChecker
@current_provider = current_provider
end
+ # The permission checker checks if:
+ # 1. A current_user exists in the context:
+ # 1.1. Checks if the current_user is a SuperAdmin and allow it without further checks.
+ # 1.2. Ensures that the current_user provider and the target resource provider matches.
+ # 1.3. Ensures that the the current_user is active.
+ # 1.4. Checks if the current_user is allowed to access a whole class of resources AKA admin (has at least one permission enabled for its role).
+ # 2. Checks if the current_user is not an admin but allowed to access that specific target resource.
def call
- # check to see if current_user has SuperAdmin role
- return true if @current_user.role == Role.find_by(name: 'SuperAdmin', provider: 'bn')
-
- # checking if user is trying to access users/rooms/recordings from different provider
- return false unless current_provider_check
- # Make sure the user is not banned or pending
- return false unless @current_user.active?
-
- return true if RolePermission.joins(:permission).exists?(
- role_id: @current_user.role_id,
- permission: { name: @permission_names },
- value: 'true'
- )
+ if @current_user
+ # check to see if current_user has SuperAdmin role
+ return true if @current_user.role == Role.find_by(name: 'SuperAdmin', provider: 'bn')
+
+ # checking if user is trying to access users/rooms/recordings from different provider
+ return false unless current_provider_check
+ # Make sure the user is not banned or pending
+ return false unless @current_user.active?
+
+ return true if RolePermission.joins(:permission).exists?(
+ role_id: @current_user.role_id,
+ permission: { name: @permission_names },
+ value: 'true'
+ )
+ end
# convert to array if only checking for 1 permission (for simplicity)
Array(@permission_names).each do |permission|
@@ -54,6 +64,8 @@ class PermissionsChecker
return true if authorize_manage_recordings
when 'RoomLimit'
return true if authorize_room_limit
+ when 'PublicRecordings'
+ return true if authorize_public_recordings
end
end
@@ -62,31 +74,53 @@ class PermissionsChecker
private
+ # Ensures that the current_user is requesting access to manage its account specifically.
def authorize_manage_user
+ return false if @current_user.blank?
return false if @user_id.blank?
@current_user.id.to_s == @user_id.to_s
end
+ # Ensures that the current_user is requesting access to manage one of its rooms specifically.
def authorize_manage_rooms
+ return false if @current_user.blank?
return false if @friendly_id.blank?
@current_user.rooms.find_by(friendly_id: @friendly_id).present?
end
+ # Ensures that the current_user is requesting access to manage a shared room specifically.
def authorize_shared_room
+ return false if @current_user.blank?
return false if @friendly_id.blank? && @record_id.blank?
@current_user.shared_rooms.exists?(friendly_id: @friendly_id) ||
@current_user.shared_rooms.exists?(id: Recording.find_by(record_id: @record_id)&.room_id)
end
+ # Ensures that the current_user is requesting access to manage its rooms recordings specifically.
def authorize_manage_recordings
+ return false if @current_user.blank?
return false if @record_id.blank?
@current_user.recordings.find_by(record_id: @record_id).present?
end
+ # Ensures that the request is to access and manage a publicly accessible recording.
+ def authorize_public_recordings
+ return false if @record_id.blank?
+ return false if @current_provider.blank?
+
+ recording = Recording.find_by(record_id: @record_id)
+ return false unless recording
+ return false if recording.user.provider != @current_provider
+ return false unless [Recording::VISIBILITIES[:public], Recording::VISIBILITIES[:public_protected]].include? recording.visibility
+
+ true
+ end
+
+ # Checks if the target user has reached its role room limit.
def authorize_room_limit
return false if @user_id.blank?
@@ -99,7 +133,10 @@ class PermissionsChecker
true
end
+ # Checks if the current_user is requsting to access a resource of the same provider.
def current_provider_check
+ return false if @current_user.blank? || @current_user.provider.blank? || @current_provider.blank?
+
# check to see if current user is trying to access another provider
return false if @current_user.provider != @current_provider
@@ -115,3 +152,4 @@ class PermissionsChecker
true
end
end
+# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
diff --git a/spec/controllers/recordings_controller_spec.rb b/spec/controllers/recordings_controller_spec.rb
index a3f4c3830fbf0f8fb6727f6543e6cfca4bc55256..fd8a53c35871cb586ba413d96954ca0a639d5e5a 100644
--- a/spec/controllers/recordings_controller_spec.rb
+++ b/spec/controllers/recordings_controller_spec.rb
@@ -301,40 +301,202 @@ RSpec.describe Api::V1::RecordingsController, type: :controller do
end
context 'format not passed' do
- it 'makes a call to BBB and returns the url returned if the recording is protected' do
- recording = create(:recording, visibility: 'Protected', room:)
+ context 'Protected recording' do
+ let(:recording) do
+ create(:recording, visibility: [
+ Recording::VISIBILITIES[:protected],
+ Recording::VISIBILITIES[:public_protected]
+ ].sample,
+ room:)
+ end
+
+ it 'makes a call to BBB and returns the URLs' do
+ post :recording_url, params: { id: recording.record_id }
+
+ expect(response).to have_http_status(:ok)
+ expect(JSON.parse(response.body)).to match_array ['https://test.com/screenshare', 'https://test.com/video']
+ end
+
+ context 'Single playback format' do
+ before do
+ allow_any_instance_of(BigBlueButtonApi).to receive(:get_recording).and_return(
+ playback: { format: { type: 'screenshare', url: 'https://test.com/screenshare' } }
+ )
+ end
+
+ it 'makes a call to BBB and returns the URL' do
+ post :recording_url, params: { id: recording.record_id }
+
+ expect(response).to have_http_status(:ok)
+ expect(JSON.parse(response.body)).to eq ['https://test.com/screenshare']
+ end
+ end
+ end
- post :recording_url, params: { id: recording.record_id }
+ context 'Unprotected recording' do
+ let(:recording) do
+ create(:recording, visibility: [
+ Recording::VISIBILITIES[:published],
+ Recording::VISIBILITIES[:unpublished],
+ Recording::VISIBILITIES[:public]
+ ].sample,
+ room:)
+ end
+
+ before { create_list(:format, Faker::Number.within(range: 1..3), recording:) }
- expect(JSON.parse(response.body)).to match_array ['https://test.com/screenshare', 'https://test.com/video']
+ it 'returns the recording record formats URLs' do
+ post :recording_url, params: { id: recording.record_id }
+
+ expect(response).to have_http_status(:ok)
+ expect(JSON.parse(response.body)).to match_array recording.formats.pluck(:url)
+ end
end
+ end
+
+ context 'format is passed' do
+ context 'Protected recording' do
+ let(:recording) do
+ create(:recording, visibility: [
+ Recording::VISIBILITIES[:protected],
+ Recording::VISIBILITIES[:public_protected]
+ ].sample,
+ room:)
+ end
+
+ before { create(:format, recording:, recording_type: 'screenshare', url: 'https://invalid.com/screenshare') }
+
+ it 'makes a call to BBB and returns the URL' do
+ post :recording_url, params: { id: recording.record_id, recording_format: 'screenshare' }
+
+ expect(response).to have_http_status(:ok)
+ expect(JSON.parse(response.body)['data']).to eq 'https://test.com/screenshare'
+ end
+
+ context 'Single playback format' do
+ before do
+ allow_any_instance_of(BigBlueButtonApi).to receive(:get_recording).and_return(
+ playback: { format: { type: 'screenshare', url: 'https://test.com/screenshare/solo' } }
+ )
+ end
+
+ it 'makes a call to BBB and returns the URL' do
+ post :recording_url, params: { id: recording.record_id, recording_format: 'screenshare' }
+
+ expect(response).to have_http_status(:ok)
+ expect(JSON.parse(response.body)['data']).to eq 'https://test.com/screenshare/solo'
+ end
+ end
+
+ context 'Inexistent format' do
+ it 'returns :not_found' do
+ post :recording_url, params: { id: recording.record_id, recording_format: '404' }
+
+ expect(response).to have_http_status(:not_found)
+ expect(JSON.parse(response.body)['data']).to be_blank
+ end
+ end
+ end
+
+ context 'Unprotected recording' do
+ let(:recording) do
+ create(:recording, visibility: [
+ Recording::VISIBILITIES[:published],
+ Recording::VISIBILITIES[:unpublished],
+ Recording::VISIBILITIES[:public]
+ ].sample,
+ room:)
+ end
+
+ let(:target_format) { create(:format, recording:, recording_type: 'screenshare') }
+
+ before { create_list(:format, 2, recording:) }
+
+ it 'returns the formats URL' do
+ post :recording_url, params: { id: recording.record_id, recording_format: target_format.recording_type }
+
+ expect(response).to have_http_status(:ok)
+ expect(JSON.parse(response.body)['data']).to eq target_format.url
+ end
+
+ context 'Inexistent format' do
+ it 'returns :not_found' do
+ post :recording_url, params: { id: recording.record_id, recording_format: '404' }
+
+ expect(response).to have_http_status(:not_found)
+ expect(JSON.parse(response.body)['data']).to be_blank
+ end
+ end
+ end
+ end
- it 'returns the formats url' do
- recording = create(:recording, visibility: 'Published', room:)
- create(:format, recording:)
+ context 'Other users' do
+ let(:recording) { create(:recording, room:) }
+ let(:signed_in_user) { create(:user) }
+
+ before { sign_in_user(signed_in_user) }
+ it 'returns :forbidden' do
post :recording_url, params: { id: recording.record_id }
- expect(JSON.parse(response.body)).to match_array recording.formats.pluck(:url)
+ expect(response).to have_http_status(:forbidden)
+ end
+
+ context 'shared room' do
+ before do
+ create(:shared_access, user_id: signed_in_user.id, room_id: room.id)
+ end
+
+ it 'returns :ok' do
+ post :recording_url, params: { id: recording.record_id }
+
+ expect(response).to have_http_status(:ok)
+ end
+ end
+
+ context 'Recordings Manager' do
+ before { sign_in_user(user_with_manage_recordings_permission) }
+
+ it 'returns :ok' do
+ post :recording_url, params: { id: recording.record_id }
+
+ expect(response).to have_http_status(:ok)
+ end
end
end
- context 'format is passed' do
- it 'makes a call to BBB and returns the url returned if the recording is protected' do
- recording = create(:recording, visibility: 'Protected', room:)
+ context 'Unauthenticated' do
+ before { sign_out_user }
+
+ describe 'Public recordings' do
+ let(:recording) { create(:recording, visibility: [Recording::VISIBILITIES[:public], Recording::VISIBILITIES[:public_protected]].sample) }
- post :recording_url, params: { id: recording.record_id, recording_format: 'screenshare' }
+ it 'returns :ok' do
+ post :recording_url, params: { id: recording.record_id }
- expect(JSON.parse(response.body)['data']).to eq 'https://test.com/screenshare'
+ expect(response).to have_http_status(:ok)
+ end
end
- it 'returns the formats url' do
- recording = create(:recording, visibility: 'Published', room:)
- format = create(:format, recording:, recording_type: 'podcast')
+ describe 'Private recordings' do
+ let(:recording) do
+ create(:recording,
+ visibility: [Recording::VISIBILITIES[:protected], Recording::VISIBILITIES[:published], Recording::VISIBILITIES[:unpublished]].sample)
+ end
+
+ it 'returns :forbidden' do
+ post :recording_url, params: { id: recording.record_id }
+
+ expect(response).to have_http_status(:forbidden)
+ end
+ end
+ end
- post :recording_url, params: { id: recording.record_id, recording_format: format.recording_type }
+ context 'Inexistent recording' do
+ it 'returns :not_found' do
+ post :recording_url, params: { id: '404' }
- expect(JSON.parse(response.body)['data']).to eq format.url
+ expect(response).to have_http_status(:not_found)
end
end
end
diff --git a/spec/services/permissions_checker_spec.rb b/spec/services/permissions_checker_spec.rb
index 339f61ebb3a9e52af6b1d9c16ec2bdd29d85cb2e..99a0d079393d57edce6f1070e8a95bb480762d5a 100644
--- a/spec/services/permissions_checker_spec.rb
+++ b/spec/services/permissions_checker_spec.rb
@@ -170,5 +170,143 @@ describe PermissionsChecker, type: :service do
end
end
end
+
+ context 'Public recordings' do
+ let(:public_recording) { create(:recording, visibility: [Recording::VISIBILITIES[:public], Recording::VISIBILITIES[:public_protected]].sample) }
+
+ let(:private_recording) do
+ create(:recording,
+ visibility: [Recording::VISIBILITIES[:published], Recording::VISIBILITIES[:protected], Recording::VISIBILITIES[:unpublished]].sample)
+ end
+
+ it 'returns true if the recording is public' do
+ expect(described_class.new(
+ current_user: nil,
+ permission_names: 'PublicRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: public_recording.record_id,
+ current_provider: public_recording.user.provider
+ ).call).to be(true)
+ end
+
+ it 'returns false if the recording is private' do
+ expect(described_class.new(
+ current_user: nil,
+ permission_names: 'PublicRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: private_recording.record_id,
+ current_provider: private_recording.user.provider
+ ).call).to be(false)
+ end
+
+ describe 'Differnt provider' do
+ it 'returns false' do
+ expect(described_class.new(
+ current_user: nil,
+ permission_names: 'PublicRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: public_recording.record_id,
+ current_provider: "NOT_#{public_recording.user.provider}"
+ ).call).to be(false)
+ end
+ end
+
+ describe 'Inexistent recording' do
+ it 'returns false' do
+ expect(described_class.new(
+ current_user: nil,
+ permission_names: 'PublicRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: '404',
+ current_provider: public_recording.user.provider
+ ).call).to be(false)
+ end
+ end
+ end
+
+ context 'ManageRecordings' do
+ let(:current_user) { create(:user) }
+
+ describe 'Recording owned by current user' do
+ let(:room) { create(:room, user: current_user) }
+ let(:recording) { create(:recording, room:) }
+
+ it 'returns true' do
+ expect(described_class.new(
+ current_user:,
+ permission_names: 'ManageRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: recording.record_id,
+ current_provider: current_user.provider
+ ).call).to be(true)
+ end
+ end
+
+ describe 'Recording not owned by current user' do
+ let(:recording) { create(:recording) }
+
+ it 'returns false' do
+ expect(described_class.new(
+ current_user:,
+ permission_names: 'ManageRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: recording.record_id,
+ current_provider: current_user.provider
+ ).call).to be(false)
+ end
+
+ context 'User with ManageRecordings permission' do
+ let(:current_user) { create(:user, :with_manage_recordings_permission) }
+
+ describe 'Same provider' do
+ it 'returns true' do
+ expect(described_class.new(
+ current_user:,
+ permission_names: 'ManageRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: recording.record_id,
+ current_provider: current_user.provider
+ ).call).to be(true)
+ end
+ end
+
+ describe 'Different provider' do
+ it 'returns true' do
+ expect(described_class.new(
+ current_user:,
+ permission_names: 'ManageRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: recording.record_id,
+ current_provider: "NOT_#{recording.user.provider}"
+ ).call).to be(false)
+ end
+ end
+ end
+ end
+
+ describe 'Unauthenticated user' do
+ let(:current_user) { nil }
+ let(:recording) { create(:recording) }
+
+ it 'returns false' do
+ expect(described_class.new(
+ current_user:,
+ permission_names: 'ManageRecordings',
+ user_id: '',
+ friendly_id: '',
+ record_id: recording.record_id,
+ current_provider: recording.room.user.provider
+ ).call).to be(false)
+ end
+ end
+ end
end
end