Multiple security fixes (#6222)

farhatahmad · web-flow · commit cb6301e3d776 · 2026-03-16T11:03:02.000-04:00
* Security fix 1

* security fix 2

* Fix 3

* Fix 4

* Fix 5

* Fix 6

* Fix 7

* Fix 8

* Fix 9

* Rubo
diff --git a/app/controllers/api/v1/admin/invitations_controller.rb b/app/controllers/api/v1/admin/invitations_controller.rb
@@ -62,8 +62,9 @@ def create
         end
 
         def destroy
-          invitation = Invitation.find(params[:id])
-          if invitation.destroy
+          invitation = Invitation.find_by(id: params[:id], provider: current_provider)
+
+          if invitation&.destroy
             render_data status: :ok
           else
             render_error status: :not_found
diff --git a/app/controllers/api/v1/admin/role_permissions_controller.rb b/app/controllers/api/v1/admin/role_permissions_controller.rb
@@ -27,9 +27,9 @@ class RolePermissionsController < ApiController
         # GET /api/v1/admin/role_permissions
         # Returns a hash of all Role Permissions
         def index
-          roles_permissions = RolePermission.joins(:permission)
-                                            .where(role_id: params[:role_id])
-                                            .pluck(:name, :value)
+          roles_permissions = RolePermission.joins(:permission, :role)
+                                            .where(role_id: params[:role_id], roles: { provider: current_provider })
+                                            .pluck('permissions.name', 'role_permissions.value')
                                             .to_h
 
           render_data data: roles_permissions, status: :ok
@@ -38,7 +38,8 @@ def index
         # POST /api/v1/admin/role_permissions
         # Updates the permission for the specified role
         def update
-          role_permission = RolePermission.joins(:permission).find_by(role_id: role_params[:role_id], permission: { name: role_params[:name] })
+          role_permission = RolePermission.joins(:permission, :role).find_by(role_id: role_params[:role_id], roles: { provider: current_provider },
+                                                                             permission: { name: role_params[:name] })
 
           return render_error status: :not_found unless role_permission
           return render_error status: :bad_request unless role_permission.update(value: role_params[:value].to_s)
diff --git a/app/controllers/api/v1/admin/roles_controller.rb b/app/controllers/api/v1/admin/roles_controller.rb
@@ -80,7 +80,7 @@ def role_params
         end
 
         def find_role
-          @role = Role.find params[:id]
+          @role = Role.with_provider(current_provider).find(params[:id])
         end
       end
     end
diff --git a/app/controllers/api/v1/admin/users_controller.rb b/app/controllers/api/v1/admin/users_controller.rb
@@ -27,7 +27,7 @@ class UsersController < ApiController
         # GET /api/v1/admin/users/:id.json
         # Updates the specified user's status
         def update
-          user = User.find(params[:id])
+          user = User.with_provider(current_provider).find(params[:id])
           initial_status = user.status
 
           if user.update(user_params)
diff --git a/app/controllers/api/v1/meetings_controller.rb b/app/controllers/api/v1/meetings_controller.rb
@@ -100,7 +100,7 @@ def running
       private
 
       def find_room
-        @room = Room.find_by!(friendly_id: params[:friendly_id])
+        @room = Room.includes(:user).with_provider(current_provider).find_by!(friendly_id: params[:friendly_id])
       end
 
       def authorized_as_viewer?(viewer_code:)
diff --git a/app/controllers/api/v1/recordings_controller.rb b/app/controllers/api/v1/recordings_controller.rb
@@ -25,7 +25,10 @@ class RecordingsController < ApiController
       before_action only: %i[destroy] do
         ensure_authorized('ManageRecordings', record_id: params[:id])
       end
-      before_action only: %i[update update_visibility recording_url] do
+      before_action only: %i[update update_visibility] do
+        ensure_authorized(%w[ManageRecordings SharedRoom], record_id: params[:id])
+      end
+      before_action only: %i[recording_url] do
         ensure_authorized(%w[ManageRecordings SharedRoom PublicRecordings], record_id: params[:id])
       end
 
diff --git a/app/controllers/api/v1/rooms_controller.rb b/app/controllers/api/v1/rooms_controller.rb
@@ -156,7 +156,7 @@ def recordings_processing
       private
 
       def find_room
-        @room = Room.find_by!(friendly_id: params[:friendly_id])
+        @room = Room.includes(:user).with_provider(current_provider).find_by!(friendly_id: params[:friendly_id])
       end
 
       def room_params
diff --git a/app/controllers/api/v1/sessions_controller.rb b/app/controllers/api/v1/sessions_controller.rb
@@ -47,12 +47,6 @@ def create
         # Will return an error if the user is NOT from the current provider and if the user is NOT a super admin
         return render_error status: :forbidden if !user.super_admin? && (user.provider != current_provider || external_auth?)
 
-        # Password is not set (local user migrated from v2)
-        if user.external_id.blank? && user.password_digest.blank?
-          token = user.generate_reset_token!
-          return render_error data: token, errors: 'PasswordNotSet'
-        end
-
         if user.authenticate(session_params[:password])
           return render_error data: user.id, errors: Rails.configuration.custom_error_msgs[:unverified_user] unless user.verified?
           return render_error errors: Rails.configuration.custom_error_msgs[:pending_user] if user.pending?
diff --git a/config/environments/production.rb b/config/environments/production.rb
@@ -174,10 +174,5 @@
   config.active_record.attributes_for_inspect = [:id]
 
   # Enable DNS rebinding protection and other `Host` header attacks.
-  # config.hosts = [
-  #   "example.com",     # Allow requests from example.com
-  #   /.*\.example\.com/ # Allow requests from subdomains like `www.example.com`
-  # ]
-  # Skip DNS rebinding protection for the default health check endpoint.
-  # config.host_authorization = { exclude: ->(request) { request.path == "/up" } }
+  config.hosts << ENV.fetch('URL_HOST') if ENV['URL_HOST'].present?
 end
diff --git a/spec/controllers/admin/invitations_controller_spec.rb b/spec/controllers/admin/invitations_controller_spec.rb
@@ -117,5 +117,12 @@
       expect { delete :destroy, params: { id: 'invalid-id' } }.not_to change(Invitation, :count)
       expect(response).to have_http_status(:not_found)
     end
+
+    it 'does not delete invitations from another provider' do
+      invitation = create(:invitation, provider: 'other-provider')
+
+      expect { delete :destroy, params: { id: invitation.id } }.not_to change(Invitation, :count)
+      expect(response).to have_http_status(:not_found)
+    end
   end
 end
diff --git a/spec/controllers/admin/role_permissions_controller_spec.rb b/spec/controllers/admin/role_permissions_controller_spec.rb
@@ -38,6 +38,15 @@
       expect(response.parsed_body['data']['ManageRoles']).to eq('true')
     end
 
+    it 'does not return RolePermissions for a role belonging to another provider' do
+      other_role = create(:role, provider: 'other-provider')
+
+      get :index, params: { role_id: other_role.id }
+
+      expect(response).to have_http_status(:ok)
+      expect(response.parsed_body['data']).to be_empty
+    end
+
     context 'user without ManageRoles permission' do
       before do
         sign_in_user(user)
@@ -61,6 +70,17 @@
       expect(role_permission.reload.value).to eq('false')
     end
 
+    it 'does not update RolePermissions for a role belonging to another provider' do
+      other_role = create(:role, provider: 'other-provider')
+      permission = create(:permission)
+      role_permission = create(:role_permission, role: other_role, permission:, value: 'true')
+
+      get :update, params: { role: { name: permission.name, role_id: other_role.id, value: false } }
+
+      expect(response).to have_http_status(:not_found)
+      expect(role_permission.reload.value).to eq('true')
+    end
+
     context 'user without ManageRoles permission' do
       before do
         sign_in_user(user)
diff --git a/spec/controllers/admin/roles_controller_spec.rb b/spec/controllers/admin/roles_controller_spec.rb
@@ -153,6 +153,39 @@
     end
   end
 
+  describe 'roles#show' do
+    it 'returns the role for valid params' do
+      role = create(:role)
+      get :show, params: { id: role.id }
+      expect(response).to have_http_status(:ok)
+      expect(response.parsed_body['data']['id']).to eq(role.id)
+    end
+
+    it 'returns :not_found for unfound roles' do
+      create(:role)
+      get :show, params: { id: 'Invalid' }
+      expect(response).to have_http_status(:not_found)
+    end
+
+    it 'does not return a role belonging to another provider' do
+      role = create(:role, provider: 'other-provider')
+      get :show, params: { id: role.id }
+      expect(response).to have_http_status(:not_found)
+    end
+
+    context 'user without ManageRoles permission' do
+      before do
+        sign_in_user(user)
+      end
+
+      it 'user without ManageRoles permission cannot return the role' do
+        role = create(:role)
+        get :show, params: { id: role.id }
+        expect(response).to have_http_status(:forbidden)
+      end
+    end
+  end
+
   describe 'roles#update' do
     let!(:role) { create(:role) }
 
@@ -177,6 +210,12 @@
       expect(response.parsed_body['errors']).not_to be_empty
     end
 
+    it 'does not update a role belonging to another provider' do
+      other_role = create(:role, provider: 'other-provider')
+      expect { post :update, params: { id: other_role.id, role: { name: 'Tampered' } } }.not_to(change { other_role.reload.name })
+      expect(response).to have_http_status(:not_found)
+    end
+
     context 'user without ManageRoles permission' do
       before do
         sign_in_user(user)
@@ -191,33 +230,6 @@
     end
   end
 
-  describe 'roles#show' do
-    it 'returns the role for valid params' do
-      role = create(:role)
-      get :show, params: { id: role.id }
-      expect(response).to have_http_status(:ok)
-      expect(response.parsed_body['data']['id']).to eq(role.id)
-    end
-
-    it 'returns :not_found for unfound roles' do
-      create(:role)
-      get :show, params: { id: 'Invalid' }
-      expect(response).to have_http_status(:not_found)
-    end
-
-    context 'user without ManageRoles permission' do
-      before do
-        sign_in_user(user)
-      end
-
-      it 'user without ManageRoles permission cannot return the role' do
-        role = create(:role)
-        get :show, params: { id: role.id }
-        expect(response).to have_http_status(:forbidden)
-      end
-    end
-  end
-
   describe 'roles#destroy' do
     it 'removes a given role if no user is dependant' do
       role = create(:role)
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
@@ -164,6 +164,16 @@
       expect(unverified_user.verified).to be(true)
     end
 
+    it 'does not update a user belonging to another provider' do
+      other_role = create(:role, provider: 'other-provider')
+      other_user = create(:user, provider: 'other-provider', role: other_role, status: 'active')
+
+      post :update, params: { id: other_user.id, user: { status: 'banned' } }
+
+      expect(response).to have_http_status(:not_found)
+      expect(other_user.reload.status).to eq('active')
+    end
+
     context 'user without ManageUsers permission' do
       before do
         sign_in_user(user)
diff --git a/spec/controllers/meetings_controller_spec.rb b/spec/controllers/meetings_controller_spec.rb
@@ -416,6 +416,16 @@
       end
     end
 
+    it 'returns not_found for a room belonging to another provider' do
+      other_role = create(:role, provider: 'other-provider')
+      other_user = create(:user, provider: 'other-provider', role: other_role)
+      other_room = create(:room, user: other_user)
+
+      post :status, params: { friendly_id: other_room.friendly_id }
+
+      expect(response).to have_http_status(:not_found)
+    end
+
     it 'allows access to an unauthenticated user' do
       allow(SecureRandom).to receive(:hex).and_return('abc123456789')
 
diff --git a/spec/controllers/recordings_controller_spec.rb b/spec/controllers/recordings_controller_spec.rb
@@ -175,6 +175,17 @@
       expect(response).to have_http_status(:forbidden)
     end
 
+    it 'user cannot update the name of a public recording they do not own' do
+      other_user = create(:user)
+      room = create(:room, user: other_user)
+      recording = create(:recording, room:, visibility: Recording::VISIBILITIES[:public])
+
+      expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings)
+
+      expect { post :update, params: { recording: { name: 'Tampered Name' }, id: recording.record_id } }.not_to(change { recording.reload.name })
+      expect(response).to have_http_status(:forbidden)
+    end
+
     context 'user has ManageRecordings permission' do
       before do
         sign_in_user(user_with_manage_recordings_permission)
@@ -321,6 +332,20 @@
       end
     end
 
+    it 'user cannot update the visibility of a public recording they do not own' do
+      other_user = create(:user)
+      room = create(:room, user: other_user)
+      recording = create(:recording, room:, visibility: Recording::VISIBILITIES[:public])
+
+      expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recording_visibility)
+
+      expect do
+        post :update_visibility, params: { visibility: Recording::VISIBILITIES[:unpublished], id: recording.record_id }
+      end.not_to(change { recording.reload.visibility })
+
+      expect(response).to have_http_status(:forbidden)
+    end
+
     # TODO: samuel - add tests for user_with_manage_recordings_permission
   end
 
diff --git a/spec/controllers/rooms_controller_spec.rb b/spec/controllers/rooms_controller_spec.rb
@@ -143,6 +143,15 @@
       expect(response).to have_http_status(:not_found)
       expect(response.parsed_body['data']).to be_nil
     end
+
+    it 'returns :not_found for a room belonging to another provider' do
+      other_role = create(:role, provider: 'other-provider')
+      other_user = create(:user, provider: 'other-provider', role: other_role)
+      other_room = create(:room, user: other_user)
+
+      get :public_show, params: { friendly_id: other_room.friendly_id }
+      expect(response).to have_http_status(:not_found)
+    end
   end
 
   describe '#destroy' do
@@ -547,5 +556,17 @@ def expect_response_to_match(recordings:)
         expect(response.parsed_body['data']).to be_blank
       end
     end
+
+    context 'room belonging to another provider' do
+      it 'returns :not_found' do
+        other_role = create(:role, provider: 'other-provider')
+        other_user = create(:user, provider: 'other-provider', role: other_role)
+        other_room = create(:room, user: other_user)
+
+        get :public_recordings, params: { friendly_id: other_room.friendly_id }
+
+        expect(response).to have_http_status(:not_found)
+      end
+    end
   end
 end
