Merge branch 'master' into SIP

.rubocop.yml

@@ -73,7 +73,7 @@ Metrics/ClassLength:
 # A calculated magnitude based on number of assignments,
 # branches, and conditions.
 Metrics/AbcSize:
-  Max: 90
+  Max: 95
 
 Metrics/ParameterLists:
   CountKeywordArgs: false
@@ -82,10 +82,10 @@ RSpec/AnyInstance:
   Enabled: false
 
 Metrics/CyclomaticComplexity:
-  Max: 20
+  Max: 25
 
 Metrics/PerceivedComplexity:
-  Max: 20
+  Max: 25
 
 Rails/Exit:
   Exclude:

app/controllers/api/v1/users_controller.rb

@@ -35,7 +35,7 @@ def show
         render_data data: user, status: :ok
       end
 
-      # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+      # rubocop:disable Metrics/AbcSize
       # POST /api/v1/users.json
       # Creates and saves a new user record in the database with the provided parameters
       def create
@@ -62,7 +62,7 @@ def create
         create_user_params[:language] = current_user&.language || I18n.default_locale if create_user_params[:language].blank?
 
         # renders an error if the user is signing up with an invalid domain based off site settings
-        return render_error errors: Rails.configuration.custom_error_msgs[:unauthorized], status: :forbidden unless valid_domain?
+        return render_error errors: Rails.configuration.custom_error_msgs[:banned_user], status: :forbidden unless valid_domain?
 
         user = UserCreator.new(user_params: create_user_params.except(:invite_token), provider: current_provider, role: default_role).call
 
@@ -97,7 +97,7 @@ def create
           render_error errors: Rails.configuration.custom_error_msgs[:record_invalid], status: :bad_request
         end
       end
-      # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+      # rubocop:enable Metrics/AbcSize
 
       # PATCH /api/v1/users/:id.json
       # Updates the values of a user
@@ -169,11 +169,7 @@ def create_user_params
       end
 
       def update_user_params
-        @update_user_params ||= if external_auth?
-                                  params.require(:user).permit(:password, :avatar, :language, :role_id, :invite_token)
-                                else
-                                  params.require(:user).permit(:name, :password, :avatar, :language, :role_id, :invite_token)
-                                end
+        @update_user_params ||= params.require(:user).permit(permitted_params)
       end
 
       def change_password_params
@@ -198,6 +194,14 @@ def valid_domain?
         end
         false
       end
+
+      def permitted_params
+        is_admin = PermissionsChecker.new(current_user:, permission_names: 'ManageUsers', current_provider:).call
+
+        return %i[password avatar language role_id invite_token] if external_auth? && !is_admin
+
+        %i[name password avatar language role_id invite_token]
+      end
     end
   end
 end

app/controllers/external_controller.rb

@@ -48,9 +48,10 @@ def create_user
       return redirect_to root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid])
     end
 
-    return render_error status: :forbidden unless valid_domain?(user_info[:email])
+    # Redirect to root if the user doesn't exist and has an invalid domain
+    return redirect_to root_path(error: Rails.configuration.custom_error_msgs[:banned_user]) if new_user && !valid_domain?(user_info[:email])
 
-    # Create the user if they dont exist
+    # Create the user if they don't exist
     if new_user
       user = UserCreator.new(user_params: user_info, provider: current_provider, role: default_role).call
       user.save!
@@ -113,6 +114,8 @@ def recording_ready
     RecordingCreator.new(recording:, first_creation: true).call
 
     render json: {}, status: :ok
+  rescue JWT::DecodeError
+    render json: {}, status: :unauthorized
   end
 
   # GET /meeting_ended

app/javascript/components/admin/site_settings/registration/Registration.jsx

@@ -108,6 +108,7 @@ export default function Registration() {
           <input
             className="form-control"
             placeholder={t('admin.site_settings.registration.enter_allowed_domains_rule')}
+            defaultValue={siteSettings?.AllowedDomains}
           />
           <Button
             variant="brand"

app/javascript/components/home/HomePage.jsx

@@ -59,6 +59,9 @@ export default function HomePage() {
       case 'SignupError':
         toast.error(t('toast.error.users.signup_error'));
         break;
+      case 'BannedUser':
+        toast.error(t('toast.error.users.banned'));
+        break;
       default:
     }
     if (error) { setSearchParams(searchParams.delete('error')); }

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

@@ -43,6 +43,7 @@ import useRoomJoinForm from '../../../../hooks/forms/rooms/useRoomJoinForm';
 import ButtonLink from '../../../shared_components/utilities/ButtonLink';
 import Title from '../../../shared_components/utilities/Title';
 import useRoomConfigValue from '../../../../hooks/queries/rooms/useRoomConfigValue';
+import usePublicRecordings from '../../../../hooks/queries/recordings/usePublicRecordings';
 
 export default function JoinCard() {
   const { t } = useTranslation();
@@ -53,6 +54,7 @@ export default function JoinCard() {
 
   const publicRoom = usePublicRoom(friendlyId);
   const roomStatusAPI = useRoomStatus(friendlyId, joinInterval);
+  const { data: recordings } = usePublicRecordings({ friendlyId });
 
   const { data: env } = useEnv();
   const { data: recordValue } = useRoomConfigValue('record');
@@ -69,7 +71,7 @@ export default function JoinCard() {
 
   useEffect(() => { // set cookie to return to if needed
     const date = new Date();
-    date.setTime(date.getTime() + (60 * 1000)); // expire the cookie in 1min
+    date.setTime(date.getTime() + (60 * 10000)); // expire the cookie in 10min
     document.cookie = `location=${path};path=/;expires=${date.toGMTString()}`;
 
     return () => { // delete redirect location when unmounting
@@ -222,7 +224,7 @@ export default function JoinCard() {
             <h1 className="mt-2">
               {publicRoom?.data.name}
             </h1>
-            { (recordValue !== 'false') && (
+            { (recordValue !== 'false') && recordings?.data?.length > 0 && (
               <ButtonLink
                 variant="brand-outline"
                 className="mt-3 mb-0 cursor-pointer"

app/javascript/components/users/user/forms/UpdateUserForm.jsx

@@ -70,7 +70,7 @@ export default function UpdateUserForm({ user }) {
 
   return (
     <Form methods={methods} onSubmit={updateUserAPI.mutate}>
-      <FormControl field={fields.name} type="text" readOnly={currentUser.external_account} />
+      <FormControl field={fields.name} type="text" readOnly={user.external_account && !PermissionChecker.hasManageUsers(currentUser)} />
       <FormControl field={fields.email} type="email" readOnly />
       <FormSelect field={fields.language} variant="dropdown">
         {
@@ -102,6 +102,7 @@ UpdateUserForm.propTypes = {
     name: PropTypes.string.isRequired,
     email: PropTypes.string.isRequired,
     provider: PropTypes.string.isRequired,
+    external_account: PropTypes.bool.isRequired,
     role: PropTypes.shape({
       id: PropTypes.string.isRequired,
       name: PropTypes.string.isRequired,

app/javascript/hooks/mutations/users/useCreateUser.jsx

@@ -51,6 +51,8 @@ export default function useCreateUser() {
           toast.error(t('toast.error.users.invalid_invite'));
         } else if (err.response.data.errors === 'EmailAlreadyExists') {
           toast.error(t('toast.error.users.email_exists'));
+        } else if (err.response.data.errors === 'BannedUser') {
+          toast.error(t('toast.error.users.banned'));
         } else {
           toast.error(t('toast.error.problem_completing_action'));
         }

app/serializers/current_user_serializer.rb

@@ -17,16 +17,12 @@
 # frozen_string_literal: true
 
 class CurrentUserSerializer < UserSerializer
-  attributes :signed_in, :permissions, :status, :external_account, :super_admin
+  attributes :signed_in, :permissions, :status, :super_admin
 
   def signed_in
     true
   end
 
-  def external_account
-    object.external_id?
-  end
-
   def permissions
     RolePermission.joins(:permission)
                   .where(role_id: object.role_id)

app/serializers/user_serializer.rb

@@ -19,14 +19,18 @@
 class UserSerializer < ApplicationSerializer
   include Avatarable
 
-  attributes :id, :name, :email, :provider, :language, :avatar, :verified, :created_at
+  attributes :id, :name, :email, :provider, :language, :avatar, :verified, :created_at, :external_account
 
   belongs_to :role
 
   def language
     object.language.tr('_', '-')
   end
 
+  def external_account
+    object.external_id?
+  end
+
   def avatar
     user_avatar(object)
   end

bin/start

@@ -26,6 +26,7 @@ fi
 
 rails assets:precompile
 rails db:create
+rails db:migrate
 rails db:migrate:with_data
 
 exec rails s -b 0.0.0.0 -p $PORT

spec/controllers/external_controller_spec.rb

@@ -371,6 +371,15 @@
 
           expect { get :create_user, params: { provider: 'openid_connect' } }.not_to change(User, :count)
         end
+
+        it 'does not affect existing users with different domains' do
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+
+          create(:user, external_id: OmniAuth.config.mock_auth[:openid_connect][:uid])
+
+          get :create_user, params: { provider: 'openid_connect' }
+          expect(response).not_to redirect_to(root_path(error: Rails.configuration.custom_error_msgs[:banned_user]))
+        end
       end
 
       context 'restricted domain set to multiple domain' do

spec/controllers/users_controller_spec.rb

@@ -463,6 +463,21 @@
 
       expect(user.role_id).to eq(updated_params[:role_id])
     end
+
+    it 'allows a user with ManageUser permissions to edit an external users name' do
+      sign_in_user(user_with_manage_users_permission)
+
+      external_user = create(:user, external_id: 'external-id')
+      updated_params = {
+        name: 'New External Name'
+      }
+
+      patch :update, params: { id: external_user.id, user: updated_params }
+
+      external_user.reload
+
+      expect(external_user.name).to eq(updated_params[:name])
+    end
   end
 
   describe '#destroy' do