CSC/ECE 517 Fall 2021 - E2126. Refactor account request controller.rb

About Expertiza

Expertiza is an open source project based on Ruby on Rails framework that supports submission across different document types, including theURLs and wiki pages. It allows the instructor not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions.

Description about project

This page is a description of Expertiza OSS project E2126 which refactoring the account_request_controller.rb file. This file allows Super Administrators to manage the approval process for new users. Our job was to refactor some of the longer methods in this file, make error messages more intuitive, and add a security captcha when requesting the page to make a new user.

DISCLAIMER:

• Some of the issues handed to us were already fixed when we looked at them from a previous project, so this page is only documenting the ones that we worked on. It can be assumed that the other undocumented issues are already done.
• THIS PROJECT IS NOT ABOUT MAKING A NEW USER. The purpose of the captcha is to verify if the user CAN BE REDIRECTED to the request new user page. The actual act of requesting a new user will error out due to some complications with the internal system's mailing system which is out of the scope of our assignments for this project. If you attempt to create a user, you are testing functionality that we were not assigned to fix.

Team

Connor Smith (cpsmith6)

Abir Majumder (aamajumd)

Joshua Myers (jamyers3)

Files Involved

account_request_controller.rb

Super Admin Login

• Despite the username, this user is not an instructor. The role was changed in the database to give instructor6 super admin privileges.

Username: instructor6 Password: password

Running Tests

  rspec ./spec/controllers/account_request_controller.rb

Relevant Methods

• action_allowed?
• create_approved_user
• create_requested_user_record

action_allowed?

This method validates if the user is allowed to be redirected to the appropriate page. The task for this project is to add a captcha system before the user requests new. This happens when the user hits the "Request account" button on the login page. This project's repo already has the "recaptcha" gem installed. Future developers need to keep in mind that functionality of the captcha depends on the developer updating the site key and secret key of the Google ReCaptcha API. Installation instructions of the gem and integrating the recaptcha API key can be found here.

There was one change to the frontend to display the captcha at _login.html.erb.

Front end change /app/views/_login.html.erb

...
...
<% end %>
  <p>

  </p>
  <br/>
</div>
    <%= form_tag({:controller => 'account_request', :role => "Instructor", :action => 'new'}, :method => "get") do %>
        <div>
          <%= recaptcha_tags %>  <------ Displays reCaptcha widget
        </div>
        <%= submit_tag "Request account", :class => 'btn btn-danger', :style => 'width:100%; margin-left:auto; margin-right:auto;' %>

    <% end %>
<% end %>

action_allowed? changes

    def action_allowed?
    case params[:action]
    when 'list_pending_requested'
      current_user_has_admin_privileges?
    when 'new'
      verify_recaptcha <---- function provided by recaptcha gem; validates frontend recaptcha input
    when 'create_requested_user_record'
      true
    when 'keys'
      current_user_has_student_privileges?
    else
      current_user_has_ta_privileges?
    end
  end

create_approved_user

This method allows a Super Admin to approve or reject new users, and gives them an error message if the incorrect steps are taken during that process.

Login info:

Username: instructor6 Password: password
There has been a database change where the role of instructor6 was bumped up to Super Admin.

Original Method

  def create_approved_user
    if params[:selection] == nil
      flash[:error] = "Please Approve or Reject before submitting"
      redirect_to action: 'list_pending_requested'
      return 
    end
    is_approved = (params[:commit] == "Accept"? "Approved" : "Rejected")
    users = params[:selection]
    users.each do |user|
      requested_user = AccountRequest.find_by(id: user.first)
      requested_user.status = is_approved
      if requested_user.status.nil?
        flash[:error] = "Please Approve or Reject before submitting"
      elsif requested_user.update_attributes(params[:user])
        flash[:success] = "The user \"#{requested_user.name}\" has been successfully updated."
      end
      if requested_user.status == "Approved"
        new_user = User.new
        new_user.name = requested_user.name
        new_user.role_id = requested_user.role_id
        new_user.institution_id = requested_user.institution_id
        new_user.fullname = requested_user.fullname
        new_user.email = requested_user.email
        new_user.parent_id = session[:user].id
        new_user.timezonepref = User.find_by(id: new_user.parent_id).timezonepref
        if new_user.save
          password = new_user.reset_password
          # Mail is sent to the user with a new password
          prepared_mail = MailerHelper.send_mail_to_user(new_user, "Your Expertiza account and password have been created.", "user_welcome", password)
          prepared_mail.deliver_now
          flash[:success] = "A new password has been sent to new user's e-mail address."
          undo_link("The user \"#{requested_user.name}\" has been successfully created. ")
        else
          foreign
        end
      elsif requested_user.status == "Rejected"
        # If the user request has been rejected, a flash message is shown and redirected to review page
        if requested_user.update_columns(status: is_approved)
          flash[:success] = "The user \"#{requested_user.name}\" has been Rejected."
          # redirect_to action: 'list_pending_requested'
          # return
        else
          flash[:error] = "Error processing request."
        end
      end
    end
    redirect_to action: 'list_pending_requested'
  end

Refactored Method

This method was refactored so the creation of the new user is handled in a separate method called "user_new"

  def create_approved_user
    if params[:selection] == nil
      flash[:error] = "Please Approve or Reject before submitting"
      redirect_to action: 'list_pending_requested'
      return 
    end
    is_approved = (params[:commit] == "Accept"? "Approved" : "Rejected")
    users = params[:selection]
    users.each do |user|
      requested_user = AccountRequest.find_by(id: user.first)
      requested_user.status = is_approved
      puts "Here"
      if requested_user.status.nil?
        flash[:error] = "Please Approve or Reject before submitting"
      elsif requested_user.update_attributes(params[:user])
        flash[:success] = "The user \"#{requested_user.name}\" has been successfully updated."
      end
      if requested_user.status == "Approved"
        user_new(requested_user)
      elsif requested_user.status == "Rejected"
        # If the user request has been rejected, a flash message is shown and redirected to review page
        if requested_user.update_columns(status: is_approved)
          flash[:success] = "The user \"#{requested_user.name}\" has been Rejected."
          # redirect_to action: 'list_pending_requested'
          # return
        else
          flash[:error] = "Error processing request."
        end
      end
    end
    redirect_to action: 'list_pending_requested'
  end

  def user_new(requested_user)
    puts requested_user.inspect
    new_user = User.new
    new_user.name = requested_user.name
    new_user.role_id = requested_user.role_id
    new_user.institution_id = requested_user.institution_id
    new_user.fullname = requested_user.fullname
    new_user.email = requested_user.email
    new_user.parent_id = session[:user].id
    new_user.timezonepref = User.find_by(id: new_user.parent_id).timezonepref
    if new_user.save
      password = new_user.reset_password
      # Mail is sent to the user with a new password
      prepared_mail = MailerHelper.send_mail_to_user(new_user, "Your Expertiza account and password have been created.", "user_welcome", password)
      prepared_mail.deliver_now
      flash[:success] = "A new password has been sent to new user's e-mail address."
      undo_link("The user \"#{requested_user.name}\" has been successfully created. ")
    else
      foreign
    end
  end

create_requested_user_record

create_requested_user creates a new account request for a user (if they are not a duplicate user) so that it may be approved by a higher level user.

Original Method

  def create_requested_user_record
  
    requested_user = AccountRequest.new(requested_user_params)
    #An object is created with respect to AccountRequest model inorder to populate the users information when account is requested

    if params[:user][:institution_id].empty?
      institution = Institution.find_or_create_by(name: params[:institution][:name])
      requested_user.institution_id = institution.id
    end
    #If user enters others and adds a new institution, an institution id will be created with respect to the institution model. 
    #This institution_attribute will be added to the AccountRequest model under institution_id attribute!

    #
    requested_user.status = 'Under Review'
    #The status is by default 'Under Review' until the super admin approves or rejects

    user_existed = User.find_by(name: requested_user.name) or User.find_by(name: requested_user.email)
    # default to instructor role
    if requested_user.role_id == nil
      requested_user.role_id = Role.where(:name => "Instructor")[0].id
    end
    requested_user_saved = requested_user.save
    #Stores a boolean value with respect to whether the user data is saved or not

    if !user_existed and requested_user_saved
      super_users = User.joins(:role).where('roles.name = ?', 'Super-Administrator')
      super_users.each do |super_user|
        prepared_mail = MailerHelper.send_mail_to_all_super_users(super_user, requested_user, 'New account Request')
        prepared_mail.deliver
      end
      #Notifying an email to the administrator regarding the new user request!
      ExpertizaLogger.info LoggerMessage.new(controller_name, requested_user.name, 'The account you are requesting has been created successfully.', request)
      flash[:success] = "User signup for \"#{requested_user.name}\" has been successfully requested."
      redirect_to '/instructions/home'
      #Print out the acknowledgement message to the user and redirect to /instructors/home page when successful

      return
    elsif user_existed
      flash[:error] = "The account you are requesting has already existed in Expertiza."
      #If the user account already exists, log error to the user
    else
      flash[:error] = requested_user.errors.full_messages.to_sentence
      #If saving in the AccountRequests model has failed
    end
    ExpertizaLogger.error LoggerMessage.new(controller_name, requested_user.name, flash[:error], request)
    redirect_to controller: 'account_request', action: 'new', role: 'Student'
    #if the first if clause fails, redirect back to the account requests page!
  end

Refactored Method

The new create_requested_user_record now only handles the different cases that can happen when a user requests an account (New user and request goes through, duplicate user and request doesn't go through, or some other error in the saving process. The new method save_requested_user creates the request object, validates it has all needed fields and tries to save it to the database. The new method notify_supers_new_request sends an email to the Super-Administrators that new user account request has been successfully created. Some variable names and error messages have been reworded to improve readability as well.

def create_requested_user_record
    requested_user = AccountRequest.new(requested_user_params)
    #An object is created with respect to AccountRequest model inorder to populate the users information when account is requested
    user_exists = User.find_by(name: requested_user.name) or User.find_by(name: requested_user.email)
    requested_user_saved = save_requested_user(requested_user, params)
    #Stores a boolean value with respect to whether the user data is saved or not
    if !user_exists and requested_user_saved
      notify_supers_new_request(requested_user)
      redirect_to '/instructions/home'
      return
    elsif user_exists
      flash[:error] = "The account you are requesting already exists in Expertiza."
      #If the user account already exists, log error to the user
    else
      flash[:error] = requested_user.errors.full_messages.to_sentence
      #If saving in the AccountRequests model has failed
    end
    ExpertizaLogger.error LoggerMessage.new(controller_name, requested_user.name, flash[:error], request)
    redirect_to controller: 'account_request', action: 'new', role: 'Student'
    #if the first if clause fails, redirect back to the account requests page!
  end

  def save_requested_user(requested_user, params)
    if params[:user][:institution_id].empty?
      institution = Institution.find_or_create_by(name: params[:institution][:name])
      requested_user.institution_id = institution.id
    end
    #If user enters others and adds a new institution, an institution id will be created with respect to the institution model.
    #This institution_attribute will be added to the AccountRequest model under institution_id attribute!
    requested_user.status = 'Under Review'
    #The status is by default 'Under Review' until the super admin approves or rejects
    # default to instructor role
    if requested_user.role_id == nil
      requested_user.role_id = Role.where(:name => "Instructor")[0].id
    end
    return requested_user.save
  end

  def  notify_supers_new_request(requested_user)
    super_users = User.joins(:role).where('roles.name = ?', 'Super-Administrator')
    super_users.each do |super_user|
      prepared_mail = MailerHelper.send_mail_to_all_super_users(super_user, requested_user, 'New account Request')
      prepared_mail.deliver
    end
    #Notifying an email to the administrator regarding the new user request!
    ExpertizaLogger.info LoggerMessage.new(controller_name, requested_user.name, 'The account you are requesting has been created successfully.', request)
    flash[:success] = "User signup for \"#{requested_user.name}\" has been successfully requested."
    #Print out the acknowledgement message to the user and redirect to /instructors/home page when successful
  end

Results

12 out of 12 tests in the account_request_controller_spec.rb test file.

Here is a image of our tests passing:

Relevant Links

Main Expertiza Repository can be found here.

Our forked Repository can be found here.

Our pull request to the beta branch can be found here.