E1845 Issues Related to Security

This page documents changes made to the Expertiza system as part of an OSS project on sorting issues related to roles and user accounts.

Peer Testing

In order to test all functionality, a super-administrator account is needed. The following account can be used in a standard Expertiza deployment:

- Super_administrator2 : password

User Deletion

Background

The majority of the project was related to solving issues regarding the deletion of Administrator and Instructor accounts. Administrator and Instructor are both account types that inherit behavior from User but are handled and deleted in different codepaths. Furthermore, properties are used on a User that aren't on an Administrator, such as team ids or course associations.

Changes

The first step towards fixing the deletion functionality was to sort out the routing issues. No listing in the routing table existed for administrator deletion, and no controller method existed for neither administrator nor instructor deletion. With both of those in place, the only issue left was the deletion functionality.

Because of the varying properties on Users and Administrators, and due to some database table migrations that were put into effect earlier, the rails system was unable to find user_id columns in the Users table when trying to delete the Administrator and Instructor accounts. The earlier migration had renamed it to team_id when team functionality was changed, but since Administrator and Instructor don't join teams, that id was invalid. A new migration was created to re-add the user_id column to the table as a blank column just used for SQL matching.

In order to maintain DRY principles and support future code maintenance, a helper method was created for use with deleting Users, Administrator, and Instructor. By using a static class method, both UserController and AdminController could use the same underlying functionality for deleting User objects while customizing the flash message and the redirect url. The changes made to both controllers is visible in the UML diagram below and the before and after code segments.

Before: (users_controller.rb)

  def destroy
    begin
      @user = User.find(params[:id])
      AssignmentParticipant.where(user_id: @user.id).each(&:delete)
      TeamsUser.where(user_id: @user.id).each(&:delete)
      AssignmentQuestionnaire.where(user_id: @user.id).each(&:destroy)
      # Participant.delete(true)
      @user.destroy
      flash[:note] = undo_link("The user \"#{@user.name}\" has been successfully deleted.")
    rescue StandardError
      flash[:error] = $ERROR_INFO
    end
    redirect_to action: 'list'
  end

After: (users_controller.rb)

  def destroy
    begin
    flash[:note] = undo_link(UsersController.destroy_helper(params, 'user'))
    rescue StandardError
      flash[:error] = $ERROR_INFO
    end
    redirect_to action: 'list'
  end

  def self.destroy_helper(params, position)
    begin
      @user = User.find(params[:id])
      AssignmentParticipant.where(user_id: @user.id).each(&:delete)
      TeamsUser.where(user_id: @user.id).each(&:delete)
      AssignmentQuestionnaire.where(user_id: @user.id).each(&:destroy)
      # Participant.delete(true)
      @user.destroy
      "The #{position} \"#{@user.name}\" has been successfully deleted."
    rescue StandardError
      raise
    end
  end

Roles

Background

In Expertiza, each user has a role in the system which dictates the availability of functionality to the user. The current roles in the system are Administrator, Instructor, Student, Super-Administrator, Teaching Assistant, and Unregistered User. The other focus of this project was to remove the ability to add new roles to the system. The functionality was previously available only to Super Administrators, but the functionality was deemed unnecessary and a potential issue to keep around. The solution simply involved removing the New button from the Roles list view and preventing any new functionality in the Role controller.

Changes

Before: (roles_controller.rb)

 def new
   @role = Role.new
   foreign
 end

In the controller, the above was changed to the following in order to prevent the functionality on the GUI side.

After: (roles_controller.rb)

 def new
   flash[:error] = 'New Roles cannot be created.'
   redirect_to roles_path
 end