Fixing all issues with shared access (#4660)

app/controllers/api/v1/recordings_controller.rb

@@ -4,9 +4,12 @@ module Api
   module V1
     class RecordingsController < ApiController
       before_action :find_recording, only: %i[update update_visibility]
-      before_action only: %i[update destroy update_visibility] do
+      before_action only: %i[destroy] do
         ensure_authorized('ManageRecordings', record_id: params[:id])
       end
+      before_action only: %i[update update_visibility] do
+        ensure_authorized(%w[ManageRecordings SharedRoom], record_id: params[:id])
+      end
       before_action only: %i[index recordings_count] do
         ensure_authorized('CreateRoom')
       end

app/controllers/api/v1/room_settings_controller.rb

@@ -6,7 +6,7 @@ module Api
       before_action :find_room, only: %i[show update]
 
       before_action only: %i[show update] do
-        ensure_authorized('ManageRooms', friendly_id: params[:friendly_id])
+        ensure_authorized(%w[ManageRooms SharedRoom], friendly_id: params[:friendly_id])
       end
 
       # GET /api/v1/room_settings/:friendly_id

app/controllers/api/v1/rooms_controller.rb

@@ -13,10 +13,10 @@ module Api
       before_action only: %i[create] do
         ensure_authorized('ManageUsers', user_id: room_params[:user_id])
       end
-      before_action only: %i[show] do
+      before_action only: %i[show update recordings recordings_processing purge_presentation] do
         ensure_authorized(%w[ManageRooms SharedRoom], friendly_id: params[:friendly_id])
       end
-      before_action only: %i[update destroy recordings recordings_processing purge_presentation] do
+      before_action only: %i[destroy] do
         ensure_authorized('ManageRooms', friendly_id: params[:friendly_id])
       end
 

app/controllers/api/v1/shared_accesses_controller.rb

@@ -5,9 +5,12 @@ module Api
     class SharedAccessesController < ApiController
       before_action :find_room
 
-      before_action only: %i[show create destroy shareable_users] do
+      before_action only: %i[create destroy shareable_users] do
         ensure_authorized('ManageRooms', friendly_id: params[:friendly_id])
       end
+      before_action only: %i[show] do
+        ensure_authorized(%w[ManageRooms SharedRoom], friendly_id: params[:friendly_id])
+      end
 
       # POST /api/v1/shared_accesses.json
       # Shares the room with all of the specified users
@@ -38,7 +41,7 @@ module Api
       # GET /api/v1/shared_accesses/friendly_id/shareable_users.json
       # Returns a list of users with whom a room can be shared with (based on role permissions)
       def shareable_users
-        return render_error status: :bad_request unless params[:search].present? && params[:search].length >= 3
+        return render_data data: [], status: :ok unless params[:search].present? && params[:search].length >= 3
 
         # role_id of roles that have SharedList permission set to true
         role_ids = RolePermission.joins(:permission).where(permission: { name: 'SharedList' }).where(value: 'true').pluck(:role_id)

app/javascript/components/recordings/RecordingRow.jsx

@@ -164,7 +164,7 @@ RecordingRow.propTypes = {
       recording_type: PropTypes.string.isRequired,
     })),
     visibility: PropTypes.string.isRequired,
-    protectable: PropTypes.bool.isRequired,
+    protectable: PropTypes.bool,
     created_at: PropTypes.string.isRequired,
     map: PropTypes.func,
   }).isRequired,

app/javascript/components/recordings/RecordingsList.jsx

@@ -86,7 +86,7 @@ RecordingsList.defaultProps = {
 RecordingsList.propTypes = {
   recordings: PropTypes.shape({
     data: PropTypes.arrayOf(PropTypes.shape({
-      id: PropTypes.number,
+      id: PropTypes.string,
       name: PropTypes.string,
       length: PropTypes.number,
       visibility: PropTypes.string,

app/javascript/components/rooms/room/FeatureTabs.jsx

@@ -62,6 +62,10 @@ export default function FeatureTabs({ shared }) {
   );
 }
 
+FeatureTabs.defaultProps = {
+  shared: false,
+};
+
 FeatureTabs.propTypes = {
-  shared: PropTypes.bool.isRequired,
+  shared: PropTypes.bool,
 };

app/javascript/components/rooms/room/join/RoomJoin.jsx

@@ -117,7 +117,7 @@ export default function RoomJoin() {
     return <RequireAuthentication path={path} />;
   }
 
-  if (publicRoom.data.owner_id === currentUser?.id) {
+  if (publicRoom.data.owner_id === currentUser?.id || publicRoom.data.shared_user_ids.includes(currentUser?.id)) {
     return <Navigate to={`/rooms/${publicRoom.data.friendly_id}`} />;
   }
 

app/javascript/components/rooms/room/room_settings/RoomSettings.jsx

@@ -13,6 +13,7 @@ import AccessCodeRow from './AccessCodeRow';
 import useUpdateRoomSetting from '../../../../hooks/mutations/room_settings/useUpdateRoomSetting';
 import { useAuth } from '../../../../contexts/auth/AuthProvider';
 import UpdateRoomNameForm from './forms/UpdateRoomNameForm';
+import useRoom from '../../../../hooks/queries/rooms/useRoom';
 
 export default function RoomSettings() {
   const { t } = useTranslation();
@@ -20,6 +21,7 @@ export default function RoomSettings() {
   const { friendlyId } = useParams();
   const roomSetting = useRoomSettings(friendlyId);
   const { data: roomConfigs } = useRoomConfigs();
+  const { data: room } = useRoom(friendlyId);
 
   const updateMutationWrapper = () => useUpdateRoomSetting(friendlyId);
   const deleteMutationWrapper = (args) => useDeleteRoom({ friendlyId, ...args });
@@ -93,11 +95,19 @@ export default function RoomSettings() {
                 description={t('room.settings.mute_users_on_join')}
               />
               <div className="float-end mt-3">
+                {!room.shared && (
                 <Modal
-                  modalButton={<Button variant="delete" className="mt-1 mx-2 float-end">{ t('room.delete_room') }</Button>}
+                  modalButton={(
+                    <Button
+                      variant="delete"
+                      className="mt-1 mx-2 float-end"
+                    >{t('room.delete_room')}
+                    </Button>
+)}
                   title={t('room.delete_room')}
                   body={<DeleteRoomForm mutation={deleteMutationWrapper} />}
                 />
+                )}
               </div>
             </Col>
           </Row>

app/javascript/components/rooms/room/room_settings/RoomSettingsRow.jsx

@@ -41,13 +41,14 @@ export default function RoomSettingsRow({
 }
 
 RoomSettingsRow.defaultProps = {
+  value: '',
   config: 'false',
 };
 
 RoomSettingsRow.propTypes = {
   settingName: PropTypes.string.isRequired,
   updateMutation: PropTypes.func.isRequired,
-  value: PropTypes.string.isRequired,
+  value: PropTypes.string,
   config: PropTypes.string,
   description: PropTypes.string.isRequired,
 };

app/javascript/components/rooms/room/shared_access/SharedAccess.jsx

@@ -12,6 +12,7 @@ import SearchBar from '../../../shared_components/search/SearchBar';
 import useDeleteSharedAccess from '../../../../hooks/mutations/shared_accesses/useDeleteSharedAccess';
 import useSharedUsers from '../../../../hooks/queries/shared_accesses/useSharedUsers';
 import SharedAccessEmpty from './SharedAccessEmpty';
+import useRoom from '../../../../hooks/queries/rooms/useRoom';
 
 export default function SharedAccess() {
   const { t } = useTranslation();
@@ -19,6 +20,7 @@ export default function SharedAccess() {
   const [searchInput, setSearchInput] = useState();
   const { data: sharedUsers } = useSharedUsers(friendlyId, searchInput);
   const deleteSharedAccess = useDeleteSharedAccess(friendlyId);
+  const { data: room } = useRoom(friendlyId);
 
   if (sharedUsers?.length || searchInput) {
     return (
@@ -27,21 +29,29 @@ export default function SharedAccess() {
           <div>
             <SearchBar searchInput={searchInput} setSearchInput={setSearchInput} />
           </div>
+          { !room.shared && (
             <Modal
-            modalButton={<Button variant="brand-outline" className="ms-auto">{ t('room.shared_access.add_share_access')}</Button>}
+              modalButton={(
+                <Button
+                  variant="brand-outline"
+                  className="ms-auto"
+                >{t('room.shared_access.add_share_access')}
+                </Button>
+)}
               title={t('room.shared_access.share_room_access')}
               body={<SharedAccessForm />}
               size="lg"
               id="shared-access-modal"
             />
+          )}
         </Stack>
         <Card className="border-0 shadow-sm mt-3">
           <Card.Body className="p-0">
             <Table hover responsive className="text-secondary mb-0">
               <thead>
                 <tr className="text-muted small">
-                  <th className="fw-normal w-50">{ t('user.name') }</th>
-                  <th className="fw-normal w-50">{ t('user.email_address') }</th>
+                  <th className="fw-normal w-75">{ t('user.name') }</th>
+                  <th className="fw-normal w-25" aria-label="delete" />
                 </tr>
               </thead>
               <tbody className="border-top-0">
@@ -56,7 +66,7 @@ export default function SharedAccess() {
                           </Stack>
                         </td>
                         <td>
-                          <span className="text-muted"> {user.email} </span>
+                          {!room.shared && (
                           <Button
                             variant="icon"
                             className="float-end pe-2"
@@ -64,6 +74,7 @@ export default function SharedAccess() {
                           >
                             <TrashIcon className="hi-s" />
                           </Button>
+                          )}
                         </td>
                       </tr>
                     ))

app/javascript/components/rooms/room/shared_access/SharedAccessList.jsx

@@ -39,9 +39,6 @@ export default function SharedAccessList({ users }) {
             <Col>
               <span className="text-muted small">{ t('user.name') }</span>
             </Col>
-            <Col>
-              <span className="text-muted small">{ t('user.email_address') }</span>
-            </Col>
           </Row>
           {
             users?.filter((user) => {
@@ -60,7 +57,6 @@ export default function SharedAccessList({ users }) {
                     </Stack>
                   </Col>
                   <Col className="my-auto">
-                    <span className="text-muted"> {user.email} </span>
                     <Button
                       variant="icon"
                       className="float-end pe-2"

app/javascript/components/rooms/room/shared_access/forms/SharedAccessForm.jsx

@@ -29,8 +29,7 @@ export default function SharedAccessForm({ handleClose }) {
           <Table hover responsive className="text-secondary my-3">
             <thead>
               <tr className="text-muted small">
-                <th className="fw-normal w-50">{ t('user.name') }</th>
-                <th className="fw-normal w-50">{ t('user.email_address') }</th>
+                <th className="fw-normal">{ t('user.name') }</th>
               </tr>
             </thead>
             <tbody className="border-top-0">
@@ -39,10 +38,18 @@ export default function SharedAccessForm({ handleClose }) {
                   if (searchInput?.length >= 3 && shareableUsers?.length) {
                     return (
                       shareableUsers.map((user) => (
-                        <tr key={user.id} className="align-middle">
+                        <tr
+                          key={user.id}
+                          className="align-middle"
+                          onClick={() => {
+                            const checkbox = document.getElementById(`${user.id}-checkbox`);
+                            checkbox.checked = !checkbox.checked;
+                          }}
+                        >
                           <td>
                             <Stack direction="horizontal" className="py-2">
                               <Form.Check
+                                id={`${user.id}-checkbox`}
                                 type="checkbox"
                                 value={user.id}
                                 className="pe-3"
@@ -52,9 +59,6 @@ export default function SharedAccessForm({ handleClose }) {
                               <h6 className="text-brand mb-0 ps-3"> {user.name} </h6>
                             </Stack>
                           </td>
-                          <td>
-                            <span className="text-muted"> {user.email} </span>
-                          </td>
                         </tr>
                       )));
                   } if (searchInput?.length >= 3) {

app/serializers/public_room_serializer.rb

@@ -4,7 +4,7 @@ class PublicRoomSerializer < ApplicationSerializer
   include Avatarable
 
   attributes :name, :recording_consent, :require_authentication, :viewer_access_code, :moderator_access_code,
-             :friendly_id, :owner_name, :owner_id, :owner_avatar
+             :friendly_id, :owner_name, :owner_id, :owner_avatar, :shared_user_ids
 
   def recording_consent
     @instance_options[:options][:settings]['record']
@@ -33,4 +33,8 @@ class PublicRoomSerializer < ApplicationSerializer
   def owner_avatar
     user_avatar(object.user)
   end
+
+  def shared_user_ids
+    object.shared_users.pluck(:id)
+  end
 end

app/serializers/shared_access_serializer.rb

@@ -2,7 +2,7 @@
 
 class SharedAccessSerializer < ApplicationSerializer
   include Avatarable
-  attributes :name, :email, :id, :avatar
+  attributes :name, :id, :avatar
 
   def avatar
     user_avatar(object)

app/services/permissions_checker.rb

@@ -59,9 +59,10 @@ class PermissionsChecker
   end
 
   def authorize_shared_room
-    return false if @friendly_id.blank?
+    return false if @friendly_id.blank? && @record_id.blank?
 
-    @current_user.shared_rooms.exists?(friendly_id: @friendly_id)
+    @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
 
   def authorize_manage_recordings

spec/controllers/recordings_controller_spec.rb

@@ -137,6 +137,18 @@ RSpec.describe Api::V1::RecordingsController, type: :controller do
       expect(response).to have_http_status(:not_found)
     end
 
+    it 'allows a shared user to update a recording name' do
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: recording.record_id,
+                                                                                   meta_hash: { meta_name: 'My Awesome Recording!' })
+
+      expect { post :update, params: { recording: { name: 'My Awesome Recording!' }, id: recording.record_id } }.to(change { recording.reload.name })
+      expect(response).to have_http_status(:ok)
+    end
+
     it 'user cannot update another users recordings name' do
       user = create(:user)
       room = create(:room, user:)
@@ -216,6 +228,18 @@ RSpec.describe Api::V1::RecordingsController, type: :controller do
       expect(unpublished_recording.reload.visibility).to eq('Unpublished')
     end
 
+    it 'allows a shared user to update a recording visibility' do
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: published_recording.record_id, publish: false)
+      expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings)
+      expect do
+        post :update_visibility, params: { visibility: 'Unpublished', id: published_recording.record_id }
+      end.to(change { published_recording.reload.visibility })
+    end
+
     # TODO: samuel - add tests for user_with_manage_recordings_permission
   end
 

spec/controllers/room_settings_controller_spec.rb

@@ -29,6 +29,15 @@ RSpec.describe Api::V1::RoomSettingsController, type: :controller do
       expect(JSON.parse(response.body)['data']).to eq({ 'setting' => 'value' })
     end
 
+    it 'returns the value for a shared user' do
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      get :show, params: { friendly_id: room.friendly_id }
+      expect(JSON.parse(response.body)['data']).to eq({ 'setting' => 'value' })
+    end
+
     context 'AuthN' do
       it 'returns :unauthorized response for unauthenticated requests' do
         session[:session_token] = nil
@@ -69,6 +78,19 @@ RSpec.describe Api::V1::RoomSettingsController, type: :controller do
       expect(room.room_meeting_options.take.value).to eq('notOptionalAnymore')
     end
 
+    it 'updates the value for a shared user' do
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      meeting_option = create(:meeting_option, name: 'setting')
+      create(:rooms_configuration, meeting_option:, value: 'optional')
+      create(:room_meeting_option, room:, meeting_option:)
+
+      put :update, params: { room_setting: { settingName: 'setting', settingValue: 'notOptionalAnymore' }, friendly_id: room.friendly_id }
+      expect(room.room_meeting_options.take.value).to eq('notOptionalAnymore')
+    end
+
     context 'Access codes' do
       it 'uses MeetingOption::get_config_value and generates a code if it\'s not disabled and the value is "true"' do
         random_access_code_name = %w[glViewerAccessCode glModeratorAccessCode].sample

spec/controllers/rooms_controller_spec.rb

@@ -237,6 +237,18 @@ RSpec.describe Api::V1::RoomsController, type: :controller do
                       presentation: { presentation: fixture_file_upload(file_fixture('default-avatar.png'), 'image/png') } }
       expect(room.reload.presentation).to be_attached
     end
+
+    it 'allows a shared user to update the room' do
+      room = create(:room, presentation: fixture_file_upload(file_fixture('default-avatar.png'), 'image/png'))
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      patch :update,
+            params: { friendly_id: room.friendly_id,
+                      presentation: { presentation: fixture_file_upload(file_fixture('default-avatar.png'), 'image/png') } }
+      expect(room.reload.presentation).to be_attached
+    end
   end
 
   describe '#purge_presentation' do
@@ -246,6 +258,17 @@ RSpec.describe Api::V1::RoomsController, type: :controller do
       delete :purge_presentation, params: { friendly_id: room.friendly_id }
       expect(room.reload.presentation).not_to be_attached
     end
+
+    it 'allows a shared user to update the room' do
+      room = create(:room, user:, presentation: fixture_file_upload(file_fixture('default-avatar.png'), 'image/png'))
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      expect(room.reload.presentation).to be_attached
+      delete :purge_presentation, params: { friendly_id: room.friendly_id }
+      expect(room.reload.presentation).not_to be_attached
+    end
   end
 
   describe '#recordings' do
@@ -267,5 +290,20 @@ RSpec.describe Api::V1::RoomsController, type: :controller do
       expect(response).to have_http_status(:ok)
       expect(recording_ids).to be_empty
     end
+
+    it 'allows a shared user to view the recordings' do
+      room = create(:room, user:, friendly_id: 'friendly_id_1')
+      shared_user = create(:user)
+      create(:shared_access, user_id: shared_user.id, room_id: room.id)
+      sign_in_user(shared_user)
+
+      recordings = create_list(:recording, 5, room:)
+
+      get :recordings, params: { friendly_id: room.friendly_id }
+
+      recording_ids = JSON.parse(response.body)['data'].map { |recording| recording['id'] }
+      expect(response).to have_http_status(:ok)
+      expect(recording_ids).to match_array(recordings.pluck(:id))
+    end
   end
 end

spec/controllers/shared_accesses_controller_spec.rb

@@ -49,15 +49,27 @@ RSpec.describe Api::V1::SharedAccessesController, type: :controller do
       response_users_ids = JSON.parse(response.body)['data'].map { |user| user['id'] }
       expect(response_users_ids).to match_array(searched_users.pluck(:id))
     end
+
+    it 'allows a shared user to view the shared access list' do
+      shared_user = create(:user)
+      sign_in_user(shared_user)
+
+      shared_users = create_list(:user, 5)
+      room.shared_users = shared_users + [shared_user]
+
+      get :show, params: { friendly_id: room.friendly_id, search: '' }
+      shared_user_ids = JSON.parse(response.body)['data'].map { |user| user['id'] }
+      expect(shared_user_ids).to match_array((shared_users + [shared_user]).pluck(:id))
+    end
   end
 
   describe '#shareable_users' do
-    it 'does not return any users if the search params is empty' do
+    it 'returns an empty list if the search params is empty' do
       shareable_users = create_list(:user, 5, name: 'John Doe')
       shareable_users << user
 
       get :shareable_users, params: { friendly_id: room.friendly_id, search: '' }
-      expect(response).to have_http_status(:bad_request)
+      expect(JSON.parse(response.body)['data']).to be_empty
     end
 
     it 'does not return any users if the search params has less than 3 characters' do
@@ -65,7 +77,7 @@ RSpec.describe Api::V1::SharedAccessesController, type: :controller do
       shareable_users << user
 
       get :shareable_users, params: { friendly_id: room.friendly_id, search: 'Jo' }
-      expect(response).to have_http_status(:bad_request)
+      expect(JSON.parse(response.body)['data']).to be_empty
     end
 
     it 'returns the users that the room can be shared to' do