GRN2-196: Fixed issues that scrutinizer is complaining about (#765)

* Refactored code to improve scrutinizer score

* Bug fixes

app/assets/javascripts/rename.js

@@ -124,7 +124,7 @@ $(document).on('turbolinks:load', function(){
         submit_update_request({
           setting: "rename_header",
           room_name: element.find('#user-text').text(),
-        }, element.data('path'));
+        }, element.data('path'), "POST");
       }
       else if(element.is('#recording-title')){
         submit_update_request({
@@ -132,16 +132,16 @@ $(document).on('turbolinks:load', function(){
           record_id: element.data('recordid'),
           record_name: element.find('text').text(),
           room_uid: element.data('room-uid'),
-        }, element.data('path'));
+        }, element.data('path'), "PATCH");
       }
     }
 
     // Helper for submitting ajax requests
-    var submit_update_request = function(data, path){
+    var submit_update_request = function(data, path, action){
       // Send ajax request for update
       $.ajax({
         url: path,
-        type: "PATCH",
+        type: action,
         data: data,
       });
     }

app/assets/javascripts/search.js

@@ -26,8 +26,7 @@ $(document).on('turbolinks:load', function(){
       (controller == "admins" && action == "server_recordings")) {
     // Submit search if the user hits enter
     $("#search-input").keypress(function(key) {
-      var keyPressed = key.which
-      if (keyPressed == 13) {
+      if (key.which == 13) {
         searchPage()
       }
     })
@@ -35,8 +34,6 @@ $(document).on('turbolinks:load', function(){
     // Add listeners for sort
     $("th[data-order]").click(function(data){
       var header_elem = $(data.target)
-      var controller = $("body").data('controller');
-      var action = $("body").data('action');
 
       if(header_elem.data('order') === 'asc'){ // asc
         header_elem.data('order', 'desc');
@@ -50,15 +47,10 @@ $(document).on('turbolinks:load', function(){
 
       var search = $("#search-input").val();
 
-      if(controller === "rooms" && action === "show"){
-        window.location.replace(window.location.pathname + "?page=1&search=" + search + 
-          "&column=" + header_elem.data("header") + "&direction="+ header_elem.data('order') + 
-          "#recordings-table");
-      }
-      else{
-        window.location.replace(window.location.pathname + "?page=1&search=" + search + 
-          "&column=" + header_elem.data("header") + "&direction="+ header_elem.data('order'));
-      }
+      var url = window.location.pathname + "?page=1&search=" + search + "&column=" + header_elem.data("header") +
+       "&direction=" + header_elem.data('order')
+
+      window.location.replace(addRecordingTable(url))
     })
 
     if(controller === "rooms" && action === "show"){
@@ -75,42 +67,30 @@ $(document).on('turbolinks:load', function(){
 function searchPage() {
   var search = $("#search-input").val();
 
-  var controller = $("body").data('controller');
-  var action = $("body").data('action');
-
   // Check if the user filtered by role
   var role = new URL(location.href).searchParams.get('role')
 
   var url = window.location.pathname + "?page=1&search=" + search
 
-  if (role) {
-    url += "&role=" + role
-  }  
-
-  if(controller === "rooms" && action === "show"){
-    window.location.replace(url + "#recordings-table");
-  } else{
-    window.location.replace(url);
-  }
+  if (role) { url += "&role=" + role } 
 
+  window.location.replace(addRecordingTable(url));
 }
 
 // Clears the search bar
 function clearSearch() {
-  var controller = $("body").data('controller');
-  var action = $("body").data('action');
-
   var role = new URL(location.href).searchParams.get('role')
 
   var url = window.location.pathname + "?page=1"
 
-  if (role) {
-    url += "&role=" + role
-  }  
+  if (role) { url += "&role=" + role } 
   
-  if(controller === "rooms" && action === "show"){
-    window.location.replace(url + "#recordings-table");
-  } else{
-    window.location.replace(url);
-  }
+  window.location.replace(addRecordingTable(url));
+}
+
+function addRecordingTable(url) {
+  if($("body").data('controller') === "rooms" && $("body").data('action') === "show") { 
+    url += "#recordings-table"
+  }
+  return url
 }

app/controllers/application_controller.rb

@@ -18,16 +18,9 @@
 
 class ApplicationController < ActionController::Base
   include BbbServer
-  include ThemingHelper
 
-  before_action :redirect_to_https
-  before_action :set_user_domain
-  before_action :set_user_settings
-  before_action :maintenance_mode?
-  before_action :migration_error?
-  before_action :user_locale
-  before_action :check_admin_password
-  before_action :check_user_role
+  before_action :redirect_to_https, :set_user_domain, :set_user_settings, :maintenance_mode?, :migration_error?,
+    :user_locale, :check_admin_password, :check_user_role
 
   # Manually handle BigBlueButton errors
   rescue_from BigBlueButton::BigBlueButtonException, with: :handle_bigbluebutton_error
@@ -77,6 +70,23 @@ class ApplicationController < ActionController::Base
     @settings = Setting.find_or_create_by(provider: @user_domain)
   end
 
+  # Redirects the user to a Maintenance page if turned on
+  def maintenance_mode?
+    if ENV["MAINTENANCE_MODE"] == "true"
+      render "errors/greenlight_error", status: 503, formats: :html,
+        locals: {
+          status_code: 503,
+          message: I18n.t("errors.maintenance.message"),
+          help: I18n.t("errors.maintenance.help"),
+        }
+    end
+    if Rails.configuration.maintenance_window.present?
+      unless cookies[:maintenance_window] == Rails.configuration.maintenance_window
+        flash.now[:maintenance] = I18n.t("maintenance.window_alert", date: Rails.configuration.maintenance_window)
+      end
+    end
+  end
+
   # Show an information page when migration fails and there is a version error.
   def migration_error?
     render :migration_error, status: 500 unless ENV["DB_MIGRATE_FAILED"].blank?
@@ -113,23 +123,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  # Redirects the user to a Maintenance page if turned on
-  def maintenance_mode?
-    if ENV["MAINTENANCE_MODE"] == "true"
-      render "errors/greenlight_error", status: 503, formats: :html,
-        locals: {
-          status_code: 503,
-          message: I18n.t("errors.maintenance.message"),
-          help: I18n.t("errors.maintenance.help"),
-        }
-    end
-    if Rails.configuration.maintenance_window.present?
-      unless cookies[:maintenance_window] == Rails.configuration.maintenance_window
-        flash.now[:maintenance] = I18n.t("maintenance.window_alert", date: Rails.configuration.maintenance_window)
-      end
-    end
-  end
-
   # Relative root helper (when deploying to subdirectory).
   def relative_root
     Rails.configuration.relative_url_root || ""

app/controllers/concerns/authenticator.rb

@@ -56,6 +56,10 @@ module Authenticator
     end
   end
 
+  def ensure_unauthenticated_except_twitter
+    redirect_to current_user.main_room if current_user && params[:old_twitter_user_id].nil?
+  end
+
   # Logs current user out of GreenLight.
   def logout
     session.delete(:user_id) if current_user

app/controllers/concerns/joiner.rb

@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+# BigBlueButton open source conferencing system - http://www.bigbluebutton.org/.
+#
+# Copyright (c) 2018 BigBlueButton Inc. and by respective authors (see below).
+#
+# This program is free software; you can redistribute it and/or modify it under the
+# terms of the GNU Lesser General Public License as published by the Free Software
+# Foundation; either version 3.0 of the License, or (at your option) any later
+# version.
+#
+# BigBlueButton is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+# PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with BigBlueButton; if not, see <http://www.gnu.org/licenses/>.
+
+module Joiner
+  extend ActiveSupport::Concern
+
+  # Displays the join room page to the user
+  def show_user_join
+    # Get users name
+    @name = if current_user
+      current_user.name
+    elsif cookies.encrypted[:greenlight_name]
+      cookies.encrypted[:greenlight_name]
+    else
+      ""
+    end
+
+    @search, @order_column, @order_direction, pub_recs =
+      public_recordings(@room.bbb_id, params.permit(:search, :column, :direction), true)
+
+    @pagy, @public_recordings = pagy_array(pub_recs)
+
+    render :join
+  end
+
+  # create or update cookie to track the three most recent rooms a user joined
+  def save_recent_rooms
+    if current_user
+      recently_joined_rooms = cookies.encrypted["#{current_user.uid}_recently_joined_rooms"].to_a
+      cookies.encrypted["#{current_user.uid}_recently_joined_rooms"] =
+        recently_joined_rooms.prepend(@room.id).uniq[0..2]
+    end
+  end
+
+  def join_room(opts)
+    room_settings = JSON.parse(@room[:room_settings])
+
+    if room_running?(@room.bbb_id) || @room.owned_by?(current_user) || room_settings["anyoneCanStart"]
+
+      # Determine if the user needs to join as a moderator.
+      opts[:user_is_moderator] = @room.owned_by?(current_user) || room_settings["joinModerator"]
+
+      opts[:require_moderator_approval] = room_settings["requireModeratorApproval"]
+
+      if current_user
+        redirect_to join_path(@room, current_user.name, opts, current_user.uid)
+      else
+        join_name = params[:join_name] || params[@room.invite_path][:join_name]
+        redirect_to join_path(@room, join_name, opts)
+      end
+    else
+      search_params = params[@room.invite_path] || params
+      @search, @order_column, @order_direction, pub_recs =
+        public_recordings(@room.bbb_id, search_params.permit(:search, :column, :direction), true)
+
+      @pagy, @public_recordings = pagy_array(pub_recs)
+
+      # They need to wait until the meeting begins.
+      render :wait
+    end
+  end
+
+  def incorrect_user_domain
+    Rails.configuration.loadbalanced_configuration && @room.owner.provider != @user_domain
+  end
+
+  # Default, unconfigured meeting options.
+  def default_meeting_options
+    invite_msg = I18n.t("invite_message")
+    {
+      user_is_moderator: false,
+      meeting_logout_url: request.base_url + logout_room_path(@room),
+      meeting_recorded: true,
+      moderator_message: "#{invite_msg}\n\n#{request.base_url + room_path(@room)}",
+      host: request.host,
+      recording_default_visibility: @settings.get_value("Default Recording Visibility") == "public"
+    }
+  end
+end

app/controllers/concerns/registrar.rb

@@ -43,4 +43,33 @@ module Registrar
       { present: false, verified: false }
     end
   end
+
+  # Checks if the user passes the requirements to be invited
+  def passes_invite_reqs
+    # check if user needs to be invited and IS invited
+    invitation = check_user_invited(@user.email, session[:invite_token], @user_domain)
+
+    @user.email_verified = true if invitation[:verified]
+
+    invitation[:present]
+  end
+
+  # Add validation errors to model if they exist
+  def valid_user_or_captcha
+    valid_user = @user.valid?
+    valid_captcha = Rails.configuration.recaptcha_enabled ? verify_recaptcha(model: @user) : true
+
+    logger.error("Support: #{@user.email} creation failed: User params are not valid.") unless valid_user
+
+    valid_user && valid_captcha
+  end
+
+  # Checks if the user trying to sign in with twitter account
+  def check_if_twitter_account(log_out = false)
+    unless params[:old_twitter_user_id].nil? && session[:old_twitter_user_id].nil?
+      logout if log_out
+      flash.now[:alert] = I18n.t("registration.deprecated.new_signin")
+      session[:old_twitter_user_id] = params[:old_twitter_user_id] unless params[:old_twitter_user_id].nil?
+    end
+  end
 end

app/controllers/concerns/themer.rb

@@ -22,22 +22,20 @@ module Themer
   # Lightens a color by 40%
   def color_lighten(color)
     # Uses the built in Sass Engine to lighten the color
-
-    dummy_scss = "h1 { color: $lighten; }"
-    compiled = SassC::Engine.new("$lighten:lighten(#{color}, 40%);" + dummy_scss, syntax: :scss).render
-
-    string_locater = 'color: '
-    color_start = compiled.index(string_locater) + string_locater.length
-
-    compiled[color_start..color_start + 6]
+    generate_sass("lighten", color, "40%")
   end
 
   # Darkens a color by 10%
   def color_darken(color)
     # Uses the built in Sass Engine to darken the color
+    generate_sass("darken", color, "10%")
+  end
 
-    dummy_scss = "h1 { color: $darken; }"
-    compiled = SassC::Engine.new("$darken:darken(#{color}, 10%);" + dummy_scss, syntax: :scss).render
+  private
+
+  def generate_sass(action, color, percentage)
+    dummy_scss = "h1 { color: $#{action}; }"
+    compiled = SassC::Engine.new("$#{action}:#{action}(#{color}, #{percentage});" + dummy_scss, syntax: :scss).render
 
     string_locater = 'color: '
     color_start = compiled.index(string_locater) + string_locater.length

app/controllers/password_resets_controller.rb

@@ -39,7 +39,7 @@ class PasswordResetsController < ApplicationController
       redirect_to root_path
     rescue
       # User doesn't exist
-      redirect_to new_password_reset_path, flash: { alert: I18n.t("no_user_email_exists") }
+      redirect_to root_path, flash: { success: I18n.t("email_sent", email_type: t("reset_password.subtitle")) }
     end
   end
 

app/controllers/recordings_controller.rb

@@ -34,6 +34,13 @@ class RecordingsController < ApplicationController
     redirect_back fallback_location: root_path if res[:updated]
   end
 
+  # PATCH /:meetingID/:record_id
+  def rename
+    update_recording(params[:record_id], "meta_name" => params[:record_name])
+
+    redirect_back fallback_location: room_path(@room)
+  end
+
   # DELETE /:meetingID/:record_id
   def delete
     delete_recording(params[:record_id])

app/controllers/rooms_controller.rb

@@ -19,12 +19,13 @@
 class RoomsController < ApplicationController
   include Pagy::Backend
   include Recorder
+  include Joiner
 
   before_action :validate_accepted_terms, unless: -> { !Rails.configuration.terms }
   before_action :validate_verified_email, except: [:show, :join],
                 unless: -> { !Rails.configuration.enable_email_verification }
   before_action :find_room, except: [:create, :join_specific_room]
-  before_action :verify_room_ownership, except: [:create, :show, :join, :logout, :login, :join_specific_room]
+  before_action :verify_room_ownership, only: [:destroy, :start, :update_settings]
   before_action :verify_room_owner_verified, only: [:show, :join],
                 unless: -> { !Rails.configuration.enable_email_verification }
   before_action :verify_user_not_admin, only: [:show]
@@ -42,19 +43,17 @@ class RoomsController < ApplicationController
     @room.owner = current_user
     @room.room_settings = create_room_settings_string(room_params)
 
-    # Save the room
-    if @room.save
-      logger.info "Support: #{current_user.email} has created a new room #{@room.uid}."
+    # Save the room and redirect if it fails
+    return redirect_to current_user.main_room, flash: { alert: I18n.t("room.create_room_error") } unless @room.save
 
-      # Start the room if auto join was turned on
-      if room_params[:auto_join] == "1"
-        start
-      else
-        redirect_to @room, flash: { success: I18n.t("room.create_room_success") }
-      end
-    else
-      redirect_to current_user.main_room, flash: { alert: I18n.t("room.create_room_error") }
-    end
+    logger.info "Support: #{current_user.email} has created a new room #{@room.uid}."
+
+    # Redirect to room is auto join was not turned on
+    return redirect_to @room,
+      flash: { success: I18n.t("room.create_room_success") } unless room_params[:auto_join] == "1"
+
+    # Start the room if auto join was turned on
+    start
   end
 
   # GET /:room_uid
@@ -76,6 +75,7 @@ class RoomsController < ApplicationController
         render :cant_create_rooms
       end
     else
+<<<<<<< HEAD
       return redirect_to root_path, flash: { alert: I18n.t("room.invalid_provider") } if incorrect_user_domain
 
       # Get users name
@@ -102,9 +102,10 @@ class RoomsController < ApplicationController
       update_room_attributes("name")
     elsif params[:setting] == "rename_recording"
       update_recording(params[:record_id], "meta_name" => params[:record_name])
+=======
+      show_user_join
+>>>>>>> GRN2-196: Fixed issues that scrutinizer is complaining about (#765)
     end
-
-    redirect_back fallback_location: room_path
   end
 
   # POST /:room_uid
@@ -123,19 +124,14 @@ class RoomsController < ApplicationController
         @join_name = params[@room.invite_path][:join_name]
       elsif !params[:join_name]
         # Join name not passed.
-        return
+        return redirect_to root_path
       end
     end
 
     # create or update cookie with join name
     cookies.encrypted[:greenlight_name] = @join_name unless cookies.encrypted[:greenlight_name] == @join_name
 
-    if current_user
-      # create or update cookie to track the three most recent rooms a user joined
-      recently_joined_rooms = cookies.encrypted["#{current_user.uid}_recently_joined_rooms"].to_a
-      cookies.encrypted["#{current_user.uid}_recently_joined_rooms"] =
-        recently_joined_rooms.prepend(@room.id).uniq[0..2]
-    end
+    save_recent_rooms
 
     logger.info "Support: #{current_user.present? ? current_user.email : @join_name} is joining room #{@room.uid}"
     join_room(default_meeting_options)
@@ -195,20 +191,25 @@ class RoomsController < ApplicationController
   def update_settings
     begin
       raise "Room name can't be blank" if room_params[:name].empty?
+      raise "Unauthorized Request" if !@room.owned_by?(current_user) || @room == current_user.main_room
 
-      @room = Room.find_by!(uid: params[:room_uid])
       # Update the rooms settings
-      update_room_attributes("settings")
+      if room_params
+        room_settings_string = create_room_settings_string(room_params)
+        @room.update_attributes(room_settings: room_settings_string)
+      end
+
       # Update the rooms name if it has been changed
-      update_room_attributes("name") if @room.name != room_params[:name]
+      @room.update_attributes(name: params[:room_name] || room_params[:name]) if @room.name != room_params[:name]
       # Update the room's access code if it has changed
-      update_room_attributes("access_code") if @room.access_code != room_params[:access_code]
-    rescue => e
-      logger.error "Error in updating room settings: #{e}"
-      flash[:alert] = I18n.t("room.update_settings_error")
-    else
+      @room.update_attributes(access_code: room_params[:access_code]) if @room.access_code != room_params[:access_code]
+
       flash[:success] = I18n.t("room.update_settings_success")
+    rescue => e
+      logger.error "Support: Error in updating room settings: #{e}"
+      flash[:alert] = I18n.t("room.update_settings_error")
     end
+
     redirect_to room_path
   end
 
@@ -231,19 +232,6 @@ class RoomsController < ApplicationController
 
   private
 
-  def update_room_attributes(update_type)
-    if @room.owned_by?(current_user) && @room != current_user.main_room
-      if update_type.eql? "name"
-        @room.update_attributes(name: params[:room_name] || room_params[:name])
-      elsif update_type.eql? "settings"
-        room_settings_string = create_room_settings_string(room_params)
-        @room.update_attributes(room_settings: room_settings_string)
-      elsif update_type.eql? "access_code"
-        @room.update_attributes(access_code: room_params[:access_code])
-      end
-    end
-  end
-
   def create_room_settings_string(options)
     room_settings = {}
     room_settings["muteOnStart"] = options[:mute_on_join] == "1"
@@ -269,18 +257,7 @@ class RoomsController < ApplicationController
 
   # Ensure the user is logged into the room they are accessing.
   def verify_room_ownership
-    bring_to_room unless @room.owned_by?(current_user)
-  end
-
-  # Redirects a user to their room.
-  def bring_to_room
-    if current_user
-      # Redirect authenticated users to their room.
-      redirect_to room_path(current_user.main_room)
-    else
-      # Redirect unauthenticated users to root.
-      redirect_to root_path
-    end
+    return redirect_to root_path unless @room.owned_by?(current_user)
   end
 
   def validate_accepted_terms
@@ -294,12 +271,7 @@ class RoomsController < ApplicationController
   def verify_room_owner_verified
     unless @room.owner.activated?
       flash[:alert] = t("room.unavailable")
-
-      if current_user && !@room.owned_by?(current_user)
-        redirect_to current_user.main_room
-      else
-        redirect_to root_path
-      end
+      redirect_to root_path
     end
   end
 
@@ -321,49 +293,4 @@ class RoomsController < ApplicationController
     current_user.rooms.length >= limit
   end
   helper_method :room_limit_exceeded
-
-  def join_room(opts)
-    room_settings = JSON.parse(@room[:room_settings])
-
-    if room_running?(@room.bbb_id) || @room.owned_by?(current_user) || room_settings["anyoneCanStart"]
-
-      # Determine if the user needs to join as a moderator.
-      opts[:user_is_moderator] = @room.owned_by?(current_user) || room_settings["joinModerator"]
-
-      opts[:require_moderator_approval] = room_settings["requireModeratorApproval"]
-
-      if current_user
-        redirect_to join_path(@room, current_user.name, opts, current_user.uid)
-      else
-        join_name = params[:join_name] || params[@room.invite_path][:join_name]
-        redirect_to join_path(@room, join_name, opts)
-      end
-    else
-      search_params = params[@room.invite_path] || params
-      @search, @order_column, @order_direction, pub_recs =
-        public_recordings(@room.bbb_id, search_params.permit(:search, :column, :direction), true)
-
-      @pagy, @public_recordings = pagy_array(pub_recs)
-
-      # They need to wait until the meeting begins.
-      render :wait
-    end
-  end
-
-  def incorrect_user_domain
-    Rails.configuration.loadbalanced_configuration && @room.owner.provider != @user_domain
-  end
-  
-  # Default, unconfigured meeting options.
-  def default_meeting_options
-    invite_msg = I18n.t("invite_message")
-    {
-      user_is_moderator: false,
-      meeting_logout_url: request.base_url + logout_room_path(@room),
-      meeting_recorded: true,
-      moderator_message: "#{invite_msg}\n\n#{request.base_url + room_path(@room)}",
-      host: request.host,
-      recording_default_visibility: @settings.get_value("Default Recording Visibility") == "public"
-    }
-  end
 end

app/controllers/sessions_controller.rb

@@ -23,6 +23,42 @@ class SessionsController < ApplicationController
   include LdapAuthenticator
 
   skip_before_action :verify_authenticity_token, only: [:omniauth, :fail]
+  before_action :check_user_signup_allowed, only: [:new]
+  before_action :ensure_unauthenticated_except_twitter, only: [:new, :signin]
+
+  # GET /signin
+  def signin
+    check_if_twitter_account
+
+    providers = configured_providers
+    if one_provider
+      provider_path = if Rails.configuration.omniauth_ldap
+        ldap_signin_path
+      else
+        "#{Rails.configuration.relative_url_root}/auth/#{providers.first}"
+      end
+
+      return redirect_to provider_path
+    end
+  end
+
+  # GET /ldap_signin
+  def ldap_signin
+  end
+
+  # GET /signup
+  def new
+    # Check if the user needs to be invited
+    if invite_registration
+      redirect_to root_path, flash: { alert: I18n.t("registration.invite.no_invite") } unless params[:invite_token]
+
+      session[:invite_token] = params[:invite_token]
+    end
+
+    check_if_twitter_account(true)
+
+    @user = User.new
+  end
 
   # POST /users/login
   def create
@@ -101,10 +137,20 @@ class SessionsController < ApplicationController
 
   private
 
+  # Verify that GreenLight is configured to allow user signup.
+  def check_user_signup_allowed
+    redirect_to root_path unless Rails.configuration.allow_user_signup
+  end
+
   def session_params
     params.require(:session).permit(:email, :password)
   end
 
+  def one_provider
+    (!allow_user_signup? || !allow_greenlight_accounts?) && providers.count == 1 &&
+      !Rails.configuration.loadbalanced_configuration
+  end
+
   def check_user_exists
     provider = @auth['provider'] == "bn_launcher" ? @auth['info']['customer'] : @auth['provider']
     User.exists?(social_uid: @auth['uid'], provider: provider)

app/controllers/themes_controller.rb

@@ -17,7 +17,8 @@
 # with BigBlueButton; if not, see <http://www.gnu.org/licenses/>.
 
 class ThemesController < ApplicationController
-  skip_before_action :maintenance_mode?
+  skip_before_action :redirect_to_https, :maintenance_mode?, :migration_error?, :user_locale,
+    :check_admin_password, :check_user_role
 
   # GET /primary
   def index

app/controllers/users_controller.rb

@@ -25,19 +25,17 @@ class UsersController < ApplicationController
   include Rolify
 
   before_action :find_user, only: [:edit, :change_password, :delete_account, :update, :destroy]
-  before_action :ensure_unauthenticated, only: [:new, :create, :signin]
+  before_action :ensure_unauthenticated_except_twitter, only: [:create]
+  before_action :check_user_signup_allowed, only: [:create]
   before_action :check_admin_of, only: [:edit, :change_password, :delete_account]
 
   # POST /u
   def create
-    # Verify that GreenLight is configured to allow user signup.
-    return unless Rails.configuration.allow_user_signup
-
     @user = User.new(user_params)
     @user.provider = @user_domain
 
     # User or recpatcha is not valid
-    render(:new) && return unless valid_user_or_captcha
+    render("sessions/new") && return unless valid_user_or_captcha
 
     # Redirect to root if user token is either invalid or expired
     return redirect_to root_path, flash: { alert: I18n.t("registration.invite.fail") } unless passes_invite_reqs
@@ -65,50 +63,6 @@ class UsersController < ApplicationController
     redirect_to root_path
   end
 
-  # GET /signin
-  def signin
-    unless params[:old_twitter_user_id].nil? && session[:old_twitter_user_id].nil?
-      flash[:alert] = I18n.t("registration.deprecated.new_signin")
-      session[:old_twitter_user_id] = params[:old_twitter_user_id] unless params[:old_twitter_user_id].nil?
-    end
-
-    providers = configured_providers
-    if (!allow_user_signup? || !allow_greenlight_accounts?) && providers.count == 1 &&
-       !Rails.configuration.loadbalanced_configuration
-      provider_path = if Rails.configuration.omniauth_ldap
-        ldap_signin_path
-      else
-        "#{Rails.configuration.relative_url_root}/auth/#{providers.first}"
-      end
-
-      return redirect_to provider_path
-    end
-  end
-
-  # GET /ldap_signin
-  def ldap_signin
-  end
-
-  # GET /signup
-  def new
-    return redirect_to root_path unless Rails.configuration.allow_user_signup
-
-    # Check if the user needs to be invited
-    if invite_registration
-      redirect_to root_path, flash: { alert: I18n.t("registration.invite.no_invite") } unless params[:invite_token]
-
-      session[:invite_token] = params[:invite_token]
-    end
-
-    unless params[:old_twitter_user_id].nil? && session[:old_twitter_user_id].nil?
-      logout
-      flash.now[:alert] = I18n.t("registration.deprecated.new_signin")
-      session[:old_twitter_user_id] = params[:old_twitter_user_id] unless params[:old_twitter_user_id].nil?
-    end
-
-    @user = User.new
-  end
-
   # GET /u/:user_uid/edit
   def edit
     redirect_to root_path unless current_user
@@ -116,6 +70,7 @@ class UsersController < ApplicationController
 
   # GET /u/:user_uid/change_password
   def change_password
+    redirect_to edit_user_path unless current_user.greenlight_account?
   end
 
   # GET /u/:user_uid/delete_account
@@ -124,11 +79,11 @@ class UsersController < ApplicationController
 
   # PATCH /u/:user_uid/edit
   def update
-    redirect_path = current_user.admin_of?(@user) ? admins_path : edit_user_path(@user)
+    profile = params[:setting] == "password" ? change_password_path(@user) : edit_user_path(@user)
+    redirect_path = current_user.admin_of?(@user) ? admins_path : profile
 
     if params[:setting] == "password"
       # Update the users password.
-      errors = {}
 
       if @user.authenticate(user_params[:password])
         # Verify that the new passwords match.
@@ -136,21 +91,18 @@ class UsersController < ApplicationController
           @user.password = user_params[:new_password]
         else
           # New passwords don't match.
-          errors[:password_confirmation] = "doesn't match"
+          @user.errors.add(:password_confirmation, "doesn't match")
         end
       else
         # Original password is incorrect, can't update.
-        errors[:password] = "is incorrect"
+        @user.errors.add(:password, "is incorrect")
       end
 
-      if errors.empty? && @user.save
-        # Notify the user that their account has been updated.
-        redirect_to redirect_path, flash: { success: I18n.t("info_update_success") }
-      else
-        # Append custom errors.
-        errors.each { |k, v| @user.errors.add(k, v) }
-        render :edit, params: { settings: params[:settings] }
-      end
+      # Notify the user that their account has been updated.
+      return redirect_to redirect_path,
+        flash: { success: I18n.t("info_update_success") } if @user.errors.empty? && @user.save
+
+      render :change_password
     else
       if @user.update_attributes(user_params)
         @user.update_attributes(email_verified: false) if user_params[:email] != @user.email
@@ -164,7 +116,7 @@ class UsersController < ApplicationController
         end
       end
 
-      render :edit, params: { settings: params[:settings] }
+      render :edit
     end
   end
 
@@ -172,20 +124,19 @@ class UsersController < ApplicationController
   def destroy
     logger.info "Support: #{current_user.email} is deleting #{@user.email}."
 
-    if current_user && current_user == @user
-      @user.destroy
-      session.delete(:user_id)
-    elsif current_user.admin_of?(@user)
-      begin
+    self_delete = current_user == @user
+    begin
+      if current_user && (self_delete || current_user.admin_of?(@user))
         @user.destroy
-      rescue => e
-        logger.error "Support: Error in user deletion: #{e}"
-        flash[:alert] = I18n.t(params[:message], default: I18n.t("administrator.flash.delete_fail"))
-      else
-        flash[:success] = I18n.t("administrator.flash.delete")
+        session.delete(:user_id) if self_delete
+
+        return redirect_to admins_path, flash: { success: I18n.t("administrator.flash.delete") } unless self_delete
       end
-      redirect_to(admins_path) && return
+    rescue => e
+      logger.error "Support: Error in user deletion: #{e}"
+      flash[:alert] = I18n.t(params[:message], default: I18n.t("administrator.flash.delete_fail"))
     end
+
     redirect_to root_path
   end
 
@@ -216,8 +167,9 @@ class UsersController < ApplicationController
     @user = User.where(uid: params[:user_uid]).includes(:roles).first
   end
 
-  def ensure_unauthenticated
-    redirect_to current_user.main_room if current_user && params[:old_twitter_user_id].nil?
+  # Verify that GreenLight is configured to allow user signup.
+  def check_user_signup_allowed
+    redirect_to root_path unless Rails.configuration.allow_user_signup
   end
 
   def user_params
@@ -233,26 +185,6 @@ class UsersController < ApplicationController
     end
   end
 
-  # Add validation errors to model if they exist
-  def valid_user_or_captcha
-    valid_user = @user.valid?
-    valid_captcha = Rails.configuration.recaptcha_enabled ? verify_recaptcha(model: @user) : true
-
-    logger.error("Support: #{@user.email} creation failed: User params are not valid.") unless valid_user
-
-    valid_user && valid_captcha
-  end
-
-  # Checks if the user passes the requirements to be invited
-  def passes_invite_reqs
-    # check if user needs to be invited and IS invited
-    invitation = check_user_invited(@user.email, session[:invite_token], @user_domain)
-
-    @user.email_verified = true if invitation[:verified]
-
-    invitation[:present]
-  end
-
   # Checks that the user is allowed to edit this user
   def check_admin_of
     redirect_to current_user.main_room if current_user && @user != current_user && !current_user.admin_of?(@user)

app/helpers/admins_helper.rb

@@ -24,8 +24,9 @@ module AdminsHelper
     Room.find_by(bbb_id: room_id).owner.email
   end
 
-  def invite_registration
-    @settings.get_value("Registration Method") == Rails.configuration.registration_methods[:invite]
+  def admin_invite_registration
+    controller_name == "admins" && action_name == "index" &&
+      @settings.get_value("Registration Method") == Rails.configuration.registration_methods[:invite]
   end
 
   def room_authentication_string

app/models/concerns/auth_values.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+# BigBlueButton open source conferencing system - http://www.bigbluebutton.org/.
+#
+# Copyright (c) 2018 BigBlueButton Inc. and by respective authors (see below).
+#
+# This program is free software; you can redistribute it and/or modify it under the
+# terms of the GNU Lesser General Public License as published by the Free Software
+# Foundation; either version 3.0 of the License, or (at your option) any later
+# version.
+#
+# BigBlueButton is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+# PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with BigBlueButton; if not, see <http://www.gnu.org/licenses/>.
+
+module AuthValues
+  extend ActiveSupport::Concern
+
+  # Provider attributes.
+  def auth_name(auth)
+    case auth['provider']
+    when :office365
+      auth['info']['display_name']
+    else
+      auth['info']['name']
+    end
+  end
+
+  def auth_username(auth)
+    case auth['provider']
+    when :google
+      auth['info']['email'].split('@').first
+    when :bn_launcher
+      auth['info']['username']
+    else
+      auth['info']['nickname']
+    end
+  end
+
+  def auth_email(auth)
+    auth['info']['email']
+  end
+
+  def auth_image(auth)
+    case auth['provider']
+    when :twitter
+      auth['info']['image'].gsub("http", "https").gsub("_normal", "")
+    else
+      auth['info']['image']
+    end
+  end
+
+  def auth_roles(user, auth)
+    unless auth['info']['roles'].nil?
+      roles = auth['info']['roles'].split(',')
+
+      role_provider = auth['provider'] == "bn_launcher" ? auth['info']['customer'] : "greenlight"
+      roles.each do |role_name|
+        role = Role.where(provider: role_provider, name: role_name).first
+        user.roles << role unless role.nil?
+      end
+    end
+  end
+end

app/models/user.rb

@@ -20,8 +20,7 @@ require 'bbb_api'
 
 class User < ApplicationRecord
   attr_accessor :reset_token
-  after_create :assign_default_role
-  after_create :initialize_main_room
+  after_create :setup_user
 
   before_save { email.try(:downcase!) }
 
@@ -49,6 +48,8 @@ class User < ApplicationRecord
   has_secure_password(validations: false)
 
   class << self
+    include AuthValues
+
     # Generates a user from omniauth.
     def from_omniauth(auth)
       # Provider is the customer name if in loadbalanced config mode
@@ -63,54 +64,6 @@ class User < ApplicationRecord
         u.save!
       end
     end
-
-    private
-
-    # Provider attributes.
-    def auth_name(auth)
-      case auth['provider']
-      when :office365
-        auth['info']['display_name']
-      else
-        auth['info']['name']
-      end
-    end
-
-    def auth_username(auth)
-      case auth['provider']
-      when :google
-        auth['info']['email'].split('@').first
-      when :bn_launcher
-        auth['info']['username']
-      else
-        auth['info']['nickname']
-      end
-    end
-
-    def auth_email(auth)
-      auth['info']['email']
-    end
-
-    def auth_image(auth)
-      case auth['provider']
-      when :twitter
-        auth['info']['image'].gsub("http", "https").gsub("_normal", "")
-      else
-        auth['info']['image']
-      end
-    end
-
-    def auth_roles(user, auth)
-      unless auth['info']['roles'].nil?
-        roles = auth['info']['roles'].split(',')
-
-        role_provider = auth['provider'] == "bn_launcher" ? auth['info']['customer'] : "greenlight"
-        roles.each do |role_name|
-          role = Role.where(provider: role_provider, name: role_name).first
-          user.roles << role unless role.nil?
-        end
-      end
-    end
   end
 
   def self.admins_search(string, role)
@@ -149,21 +102,17 @@ class User < ApplicationRecord
 
   # Activates an account and initialize a users main room
   def activate
-    update_attribute(:email_verified, true)
-    update_attribute(:activated_at, Time.zone.now)
-    save
+    update_attributes(email_verified: true, activated_at: Time.zone.now)
   end
 
   def activated?
-    return true unless Rails.configuration.enable_email_verification
-    email_verified
+    Rails.configuration.enable_email_verification ? email_verified : true
   end
 
   # Sets the password reset attributes.
   def create_reset_digest
     self.reset_token = User.new_token
-    update_attribute(:reset_digest,  User.digest(reset_token))
-    update_attribute(:reset_sent_at, Time.zone.now)
+    update_attributes(reset_digest: User.digest(reset_token), reset_sent_at: Time.zone.now)
   end
 
   # Returns true if the given token matches the digest.
@@ -288,8 +237,7 @@ class User < ApplicationRecord
 
   def create_reset_activation_digest(token)
     # Create the digest and persist it.
-    self.activation_digest = User.digest(token)
-    save
+    update_attribute(:activation_digest, User.digest(token))
     token
   end
 
@@ -298,19 +246,17 @@ class User < ApplicationRecord
     rooms.destroy_all
   end
 
-  # Initializes a room for the user and assign a BigBlueButton user id.
-  def initialize_main_room
-    self.uid = "gl-#{(0...12).map { rand(65..90).chr }.join.downcase}"
-    self.main_room = Room.create!(owner: self, name: I18n.t("home_room"))
-    save
-  end
+  def setup_user
+    # Initializes a room for the user and assign a BigBlueButton user id.
+    id = "gl-#{(0...12).map { rand(65..90).chr }.join.downcase}"
+    room = Room.create!(owner: self, name: I18n.t("home_room"))
 
-  # Initialize the user to use the default user role
-  def assign_default_role
+    update_attributes(uid: id, main_room: room)
+
+    # Initialize the user to use the default user role
     role_provider = Rails.configuration.loadbalanced_configuration ? provider : "greenlight"
 
     Role.create_default_roles(role_provider) if Role.where(provider: role_provider).count.zero?
-
     add_role(:user) if roles.blank?
   end
 

app/views/admins/components/_server_recording_row.html.erb

@@ -15,7 +15,7 @@
 
 <tr>
   <td>
-    <div id="recording-title" class="form-inline edit_hover_class" data-recordid="<%= recording[:recordID] %>" data-room-uid="<%= room_uid_from_bbb(recording[:meetingID]) %>" data-path="<%= update_room_path(room_uid: room_uid_from_bbb(recording[:meetingID])) %>">
+    <div id="recording-title" class="form-inline edit_hover_class" data-recordid="<%= recording[:recordID] %>" data-room-uid="<%= room_uid_from_bbb(recording[:meetingID]) %>" data-path="<%= rename_recording_path(meetingID: recording[:meetingID], record_id: recording[:recordID]) %>">
       <text id='recording-text'>
         <% if recording[:metadata][:name] %>
           <%= recording[:metadata][:name] %>

app/views/rooms/show.html.erb

@@ -23,7 +23,7 @@
   <div class="container">
     <div class="row pt-9">
       <div class="col-lg-8 col-sm-12">
-        <div id="room-title" class="display-3 form-inline <%= 'edit_hover_class' if current_user.main_room != @room %>">
+        <div id="room-title" class="display-3 form-inline <%= 'edit_hover_class' if current_user.main_room != @room %>" data-path="<%= update_settings_path(@room) %>">
           <% if current_user.main_room == @room %>
             <h1 contenteditable=false id="user-text" class="display-3 text-left mb-3 font-weight-400"><%= t("home_room") %></h1>
             <a class="disable-click"><i class="fas fa-home align-top home-indicator ml-2"></i></a>

app/views/shared/components/_public_recording_row.html.erb

@@ -15,7 +15,7 @@
 
 <tr>
   <td>
-    <div id="recording-title" class="form-inline edit_hover_class" data-recordid="<%= recording[:recordID] %>" data-room-uid="<%= room_uid_from_bbb(recording[:meetingID]) %>" data-path="<%= update_room_path(room_uid: room_uid_from_bbb(recording[:meetingID])) %>">
+    <div id="recording-title" class="form-inline edit_hover_class" data-recordid="<%= recording[:recordID] %>" data-room-uid="<%= room_uid_from_bbb(recording[:meetingID]) %>" data-path="<%= rename_recording_path(meetingID: recording[:meetingID], record_id: recording[:recordID]) %>">
       <text>
         <% if recording[:metadata][:name] %>
           <%= recording[:metadata][:name] %>

app/views/shared/components/_recording_row.html.erb

@@ -15,7 +15,7 @@
 
 <tr>
   <td>
-    <div id="recording-title" class="form-inline edit_hover_class" data-recordid="<%= recording[:recordID] %>" data-room-uid="<%= room_uid_from_bbb(recording[:meetingID]) %>" data-path="<%= update_room_path(room_uid: room_uid_from_bbb(recording[:meetingID])) %>">
+    <div id="recording-title" class="form-inline edit_hover_class" data-recordid="<%= recording[:recordID] %>" data-room-uid="<%= room_uid_from_bbb(recording[:meetingID]) %>" data-path="<%= rename_recording_path(meetingID: recording[:meetingID], record_id: recording[:recordID]) %>">
       <text id='recording-text'>
         <% if recording[:metadata][:name] %>
           <%= recording[:metadata][:name] %>

app/views/shared/components/_subtitle.html.erb

@@ -20,7 +20,7 @@
     </div>
 
     <div class="col-8">
-      <% if invite_registration %>
+      <% if admin_invite_registration %>
         <div id="invite-user" class="d-inline-block float-right ml-3">
           <%= link_to "#inviteModal", :class => "btn btn-primary", "data-toggle": "modal" do %>
             <%= t("administrator.users.invite") %> <i class="fas fa-paper-plane ml-1"></i>

app/views/users/change_password.html.erb

@@ -18,6 +18,15 @@
   <div class="row">
     <div class="col-lg-3 mb-4">
       <%= render "users/components/menu_buttons" %>
+
+      <% if @user.errors.any? %>
+        <h5 class="mt-8"><%= t("errors.title") %>:</h5>
+        <ul>
+        <% @user.errors.full_messages.each do |err| %>
+          <li class="text-danger"><%= err %>.</li>
+        <% end %>
+        </ul>
+      <% end %>
     </div>
     <div class="col-lg-9">
       <%= render "users/components/setting_view", setting_id: "password", setting_title: t("settings.password.subtitle") %>

app/views/users/components/_menu_buttons.html.erb

@@ -17,8 +17,10 @@
   <%= link_to edit_user_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "edit"}" do %>
     <span class="icon mr-3"><i class="fas fa-user"></i></span><%= t("settings.account.title") %>
   <% end %>
-  <%= link_to change_password_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "change_password"}" do %>
-    <span class="icon mr-3"><i class="fas fa-key"></i></span><%= t("settings.password.title") %>
+  <% if current_user.greenlight_account? %>
+    <%= link_to change_password_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "change_password"}" do %>
+      <span class="icon mr-3"><i class="fas fa-key"></i></span><%= t("settings.password.title") %>
+    <% end %>
   <% end %>
   <%= link_to delete_account_path, class: "list-group-item list-group-item-action dropdown-item #{"active" if active_page == "delete_account"}" do %>
     <span class="icon mr-3"><i class="fas fa-trash-alt"></i></span><%= t("settings.delete.title") %>

app/views/users/components/_password.html.erb

@@ -19,13 +19,13 @@
     <div class="row">
       <div class="col-8">
         <%= f.label :password, t("settings.password.old"), class: "form-label" %>
-        <%= f.password_field :password, class: "form-control #{form_is_invalid?(@user, :password)}" %>
+        <%= f.password_field :password, class: "form-control #{form_is_invalid?(@user, :password)}", autocomplete: :off, required: true %>
         <br>
         <%= f.label :new_password, t("settings.password.new"), class: "form-label" %>
-        <%= f.password_field :new_password, class: "form-control #{form_is_invalid?(@user, :password)}" %>
+        <%= f.password_field :new_password, class: "form-control #{form_is_invalid?(@user, :password)}", autocomplete: :off, required: true %>
         <br>
         <%= f.label :password_confirmation, t("settings.password.confirmation"), class: "form-label" %>
-        <%= f.password_field :password_confirmation, class: "form-control #{form_is_invalid?(@user, :password_confirmation)}" %>
+        <%= f.password_field :password_confirmation, class: "form-control #{form_is_invalid?(@user, :password_confirmation)}", autocomplete: :off, required: true %>
       </div>
     </div>
   </div>

config/routes.rb

@@ -25,10 +25,10 @@ Rails.application.routes.draw do
   match '/500', to: 'errors#internal_error', via: :all, as: :internal_error
 
   # Signin/Signup routes.
-  get '/signin', to: 'users#signin', as: :signin
-  get '/signup', to: 'users#new', as: :signup
+  get '/signin', to: 'sessions#signin', as: :signin
+  get '/signup', to: 'sessions#new', as: :signup
   post '/signup', to: 'users#create', as: :create_user
-  get '/ldap_signin', to: 'users#ldap_signin', as: :ldap_signin
+  get '/ldap_signin', to: 'sessions#ldap_signin', as: :ldap_signin
 
   # Redirect to terms page
   match '/terms', to: 'users#terms', via: [:get, :post]
@@ -119,6 +119,7 @@ Rails.application.routes.draw do
     # Manage recordings
     scope '/:record_id' do
       post '/', to: 'recordings#update', as: :update_recording
+      patch '/', to: 'recordings#rename', as: :rename_recording
       delete '/', to: 'recordings#delete', as: :delete_recording
     end
   end

spec/controllers/password_resets_controller_spec.rb

@@ -50,7 +50,7 @@ describe PasswordResetsController, type: :controller do
         expect(response).to redirect_to(root_path)
       end
 
-      it "reloads the page if no email exists in the database" do
+      it "redirects to root with success flash if email does not exists" do
         params = {
           password_reset: {
             email: nil,
@@ -58,7 +58,8 @@ describe PasswordResetsController, type: :controller do
         }
 
         post :create, params: params
-        expect(response).to redirect_to(new_password_reset_path)
+        expect(flash[:success]).to be_present
+        expect(response).to redirect_to(root_path)
       end
     end
 

spec/controllers/recordings_controller_spec.rb

@@ -44,6 +44,19 @@ describe RecordingsController, type: :controller do
     end
   end
 
+  context "PATCH #rename" do
+    it "properly updates recording name and redirects to current page" do
+      allow_any_instance_of(BbbServer).to receive(:update_recording).and_return(updated: true)
+
+      @request.session[:user_id] = @user.id
+      name = Faker::Games::Pokemon.name
+
+      patch :rename, params: { meetingID: @room.bbb_id, record_id: Faker::IDNumber.valid, record_name: name }
+
+      expect(response).to redirect_to(@room)
+    end
+  end
+
   context "DELETE #delete_recording" do
     it "deletes the recording" do
       allow_any_instance_of(BbbServer).to receive(:delete_recording).and_return(true)

spec/controllers/rooms_controller_spec.rb

@@ -366,7 +366,7 @@ describe RoomsController, type: :controller do
       @request.session[:user_id] = @user.id
       post :start, params: { room_uid: @other_room }
 
-      expect(response).to redirect_to(@user.main_room)
+      expect(response).to redirect_to(root_path)
     end
 
     it "should bring to root if not authenticated" do
@@ -405,37 +405,11 @@ describe RoomsController, type: :controller do
         .from(@secondary_room.room_settings).to(formatted_room_params)
       expect(response).to redirect_to(@secondary_room)
     end
-  end
-
-  describe "PATCH #update" do
-    before do
-      @user = create(:user)
-      @secondary_room = create(:room, owner: @user)
-      @editable_room = create(:room, owner: @user)
-    end
-
-    it "properly updates room name through room block and redirects to current page" do
-      @request.session[:user_id] = @user.id
-
-      patch :update, params: { room_uid: @secondary_room, room_block_uid: @editable_room,
-                               setting: :rename_block, room_name: :name }
-
-      expect(response).to redirect_to(@secondary_room)
-    end
 
     it "properly updates room name through room header and redirects to current page" do
       @request.session[:user_id] = @user.id
 
-      patch :update, params: { room_uid: @secondary_room, setting: :rename_header, room_name: :name }
-
-      expect(response).to redirect_to(@secondary_room)
-    end
-
-    it "properly updates recording name and redirects to current page" do
-      @request.session[:user_id] = @user.id
-
-      patch :update, params: { room_uid: @secondary_room, recordid: :recordid,
-                               setting: :rename_recording, record_name: :name }
+      patch :update_settings, params: { room_uid: @secondary_room, setting: :rename_header, room_name: :name }
 
       expect(response).to redirect_to(@secondary_room)
     end

spec/controllers/sessions_controller_spec.rb

@@ -19,6 +19,50 @@
 require "rails_helper"
 
 describe SessionsController, type: :controller do
+  describe "GET #new" do
+    it "assigns a blank user to the view" do
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+
+      get :new
+      expect(assigns(:user)).to be_a_new(User)
+    end
+
+    it "redirects to root if allow_user_signup is false" do
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(false)
+
+      get :new
+      expect(response).to redirect_to(root_path)
+    end
+
+    it "rejects the user if they are not invited" do
+      allow_any_instance_of(Registrar).to receive(:invite_registration).and_return(true)
+      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
+
+      get :new
+
+      expect(flash[:alert]).to be_present
+      expect(response).to redirect_to(root_path)
+    end
+  end
+
+  describe "GET #signin" do
+    it "redirects to main room if already authenticated" do
+      user = create(:user)
+      @request.session[:user_id] = user.id
+
+      post :signin
+      expect(response).to redirect_to(room_path(user.main_room))
+    end
+  end
+
+  describe 'GET #ldap_signin' do
+    it "should render the ldap signin page" do
+      get :ldap_signin
+
+      expect(response).to render_template(:ldap_signin)
+    end
+  end
+
   describe "GET #destroy" do
     before(:each) do
       user = create(:user, provider: "greenlight")

spec/controllers/users_controller_spec.rb

@@ -46,32 +46,6 @@ describe UsersController, type: :controller do
     }
   end
 
-  describe "GET #new" do
-    it "assigns a blank user to the view" do
-      allow(Rails.configuration).to receive(:allow_user_signup).and_return(true)
-
-      get :new
-      expect(assigns(:user)).to be_a_new(User)
-    end
-
-    it "redirects to root if allow_user_signup is false" do
-      allow(Rails.configuration).to receive(:allow_user_signup).and_return(false)
-
-      get :new
-      expect(response).to redirect_to(root_path)
-    end
-  end
-
-  describe "GET #signin" do
-    it "redirects to main room if already authenticated" do
-      user = create(:user)
-      @request.session[:user_id] = user.id
-
-      post :signin
-      expect(response).to redirect_to(room_path(user.main_room))
-    end
-  end
-
   describe "GET #edit" do
     it "renders the edit template" do
       user = create(:user)
@@ -211,13 +185,6 @@ describe UsersController, type: :controller do
           expect { post :create, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
         end
 
-        it "rejects the user if they are not invited" do
-          get :new
-
-          expect(flash[:alert]).to be_present
-          expect(response).to redirect_to(root_path)
-        end
-
         it "allows the user to signup if they are invited" do
           allow(Rails.configuration).to receive(:enable_email_verification).and_return(false)
 
@@ -493,12 +460,4 @@ describe UsersController, type: :controller do
       expect(response).to redirect_to(root_path)
     end
   end
-
-  context 'GET #ldap_signin' do
-    it "should render the ldap signin page" do
-      get :ldap_signin
-
-      expect(response).to render_template(:ldap_signin)
-    end
-  end
 end