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

Following the installation and setup of the Expertiza system with the scrubbed database, testing of the project functionality can be done as follows.

A. User Deletion

1. Sign in as a Super-Administrator (see above for sample account)

2. Navigate to Administration>Show>Instructors

i. Select one of the instructor accounts

ii. On the account page, click the delete link at the bottom

iii. Assuming the instructor account is not tied to any assignments or other associations, the account will be gone from the list. If there is an issue, an error will be displayed at the top of the page.

3. Navigate to Administration>Show>Administrators

i. Select one of the administrator accounts

ii. On the account page, click the delete link at the bottom

iii. Assuming the administrator account is not tied to any assignments or other associations, the account will be gone from the list. If there is an issue, an error will be displayed at the top of the page.

4. Errors may appear on the page when an account to be deleted still has relations to other assignments, teams, etc. The content and quality of these error messages is outside the scope of this project; some may show an English error, others may return an SQL error. This is the intended functionality as of the time this project was written - this project only concerns successful deletion in situations where deletion should be working.

B. Role Creation

1. Sign in as a Super-Administrator

2. Navigate to Administration>Setup>Roles

3. Ensure that there is no link on the page to create a new role

4. Ensure that manually navigating to /roles/new results in an error message on the page.

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. The code segments only show the destroy method for the users controller, as the code is nearly identical in the administrator controller for both Instructor and Administrator deletion, with the only changes being to the name of the position being passed and the redirect_to path after the deletion.

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

Role Creation

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

RSpec Test Cases

In order to develop test cases for the functionality added and removed in this project, both an admin_controller_spec.rb and a roles_controller_spec.rb file were created and added to the RSpec test environment.

The admin tests include building and deleting both Administrator and Instructor accounts using Capybara for testing the front-end web functionality.

To test the role creation, Capybara was again used to check that the web page did not have any button for "New Role" and that navigating to the page manually resulted in an error.