* Added expected scenarios to RecordingsController#recording_url API &...

* Added expected scenarios to RecordingsController#recording_url API & PermissionChecker service specs. (#5235)

* Extended RecordingsController#recording_url API to meet expected scenarios.

* Extended PermissionChecker API to meet expected scenarios.

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'
+        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
 
-                record_format.present? ? formats.find { |format| format[:type] == record_format }[:url] : formats.pluck(:url)
+                 found_format[:url]
                else
-                record_format.present? ? @recording.formats.find_by(recording_type: record_format).url : @recording.formats.pluck(:url)
+                 @recording.formats.pluck(:url)
                end
 
-        render_data data: url, status: :ok
+        render_data data: urls, status: :ok
       end
 
       private

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,7 +27,15 @@ 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
+    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')
 
@@ -40,6 +49,7 @@ class PermissionsChecker
         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

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
 
-      it 'returns the formats url' do
-        recording = create(:recording, visibility: 'Published', room:)
-        create(:format, recording:)
+        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
+
+      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:) }
+
+        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
-      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
 
+        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
 
-      it 'returns the formats url' do
-        recording = create(:recording, visibility: 'Published', room:)
-        format = create(:format, recording:, recording_type: 'podcast')
+        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
 
-        post :recording_url, params: { id: recording.record_id, recording_format: format.recording_type }
+          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(JSON.parse(response.body)['data']).to eq format.url
+            expect(response).to have_http_status(:not_found)
+            expect(JSON.parse(response.body)['data']).to be_blank
+          end
+        end
+      end
+    end
+
+    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(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 'Unauthenticated' do
+      before { sign_out_user }
+
+      describe 'Public recordings' do
+        let(:recording) { create(:recording, visibility: [Recording::VISIBILITIES[:public], Recording::VISIBILITIES[:public_protected]].sample) }
+
+        it 'returns :ok' do
+          post :recording_url, params: { id: recording.record_id }
+
+          expect(response).to have_http_status(:ok)
+        end
+      end
+
+      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
+
+    context 'Inexistent recording' do
+      it 'returns :not_found' do
+        post :recording_url, params: { id: '404' }
+
+        expect(response).to have_http_status(:not_found)
       end
     end
   end

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