E1915 Authorization Utilities: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 29: Line 29:


=====Drawbacks and Solutions=====
=====Drawbacks and Solutions=====
* '''Problem 1''': TODO: add description of problem 1
 
The problems listed below are examples of the three main classes of problems we encountered with Expertiza authorization.  This is not an exhaustive list of problems, but is a good representation of the classes of problems addressed.
 
* '''Problem 1''': Much of the authorization logic is repeated (un-DRY).  For example, multiple controllers contain the following exact code.
<pre>
<pre>
# TODO: add code demonstrating problem 1
  ['Super-Administrator',
  'Administrator',
  'Instructor',
  'Teaching Assistant'].include? current_role_name
</pre>
* '''Solution 1''': Use one of the helper methods from the new authorization_helper.rb to allow TAs *and above* (instructors, admins, super-admins) to perform this work.
<pre>
  # TODO add updated code here
</pre>
 
* '''Problem 2''': Some logic is slightly incorrect.  For example, some places call for a specific user type, when users "above" this type should also be allowed to perform the work.  In the following example (advertise_for_partner_controller.rb), only Students may advertise for partners.  However per Dr. Gehringer, "There are no cases I am aware of where a particular type of user can do something that more-privileged users cannot do".
<pre>
  def action_allowed?
    current_user.role.name.eql?("Student")
  end
</pre>
* '''Solution 2''': Use one of the helper methods from the new authorization_helper.rb to allow Students *and above* (TAs, instructors, admins, super-admins) to perform this work.
<pre>
  # TODO add updated code here
</pre>
 
* '''Problem 3''': Some logic is DRY and correct, but places too much logic in the controller.  This makes the controller more difficult to read, and scatters authorization logic, when it would be easier to understand if it were all in one place.
<pre>
  def action_allowed?
    if %w[edit update list_submissions].include? params[:action]
      assignment = Assignment.find(params[:id])
      (%w[Super-Administrator Administrator].include? current_role_name) ||
      (assignment.instructor_id == current_user.try(:id)) ||
      TaMapping.exists?(ta_id: current_user.try(:id), course_id: assignment.course_id) ||
      (assignment.course_id && Course.find(assignment.course_id).instructor_id == current_user.try(:id))
    else
      ['Super-Administrator',
      'Administrator',
      'Instructor',
      'Teaching Assistant'].include? current_role_name
    end
  end
</pre>
* '''Solution 3''': Establish helper methods in the new authorization_helper.rb to centralize as much authorization logic as possible.  In this way, a developer with questions about authorization knows just where to look to find answers - authorization_helper.rb
<pre>
  # TODO add updated code here
</pre>
</pre>
* '''Solution''': TODO: add description of solution to problem 1


===New Implementation===
===New Implementation===

Revision as of 13:30, 14 March 2019

E1915. Authorization Utilities

This page provides a description of an Expertiza OSS project.



About Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor 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. Expertiza supports submission across various document types, including the URLs and wiki pages.

Problem Statement

The following tasks were accomplished in this project:

  • Centralize user authentication logic to support the DRY principle
  • Improve user authentication logic in cases where it was clearly flawed
  • Support this work with RSpec unit tests

Current Implementation

Functionality
  • Most controllers contain an action_allowed? method which determines which users are allowed to perform which actions
  • This logic is in most cases correct, but is often repeated between controllers (un-DRY)
  • This logic is in some cases slightly incorrect
  • The Roles model provides a helpful method hasAllPrivilegesOf, which could be used to simplify authorization logic
Drawbacks and Solutions

The problems listed below are examples of the three main classes of problems we encountered with Expertiza authorization. This is not an exhaustive list of problems, but is a good representation of the classes of problems addressed.

  • Problem 1: Much of the authorization logic is repeated (un-DRY). For example, multiple controllers contain the following exact code.
  ['Super-Administrator',
  'Administrator',
  'Instructor',
  'Teaching Assistant'].include? current_role_name
  • Solution 1: Use one of the helper methods from the new authorization_helper.rb to allow TAs *and above* (instructors, admins, super-admins) to perform this work.
  # TODO add updated code here
  • Problem 2: Some logic is slightly incorrect. For example, some places call for a specific user type, when users "above" this type should also be allowed to perform the work. In the following example (advertise_for_partner_controller.rb), only Students may advertise for partners. However per Dr. Gehringer, "There are no cases I am aware of where a particular type of user can do something that more-privileged users cannot do".
  def action_allowed?
    current_user.role.name.eql?("Student")
  end
  • Solution 2: Use one of the helper methods from the new authorization_helper.rb to allow Students *and above* (TAs, instructors, admins, super-admins) to perform this work.
  # TODO add updated code here
  • Problem 3: Some logic is DRY and correct, but places too much logic in the controller. This makes the controller more difficult to read, and scatters authorization logic, when it would be easier to understand if it were all in one place.
  def action_allowed?
    if %w[edit update list_submissions].include? params[:action]
      assignment = Assignment.find(params[:id])
      (%w[Super-Administrator Administrator].include? current_role_name) ||
      (assignment.instructor_id == current_user.try(:id)) ||
      TaMapping.exists?(ta_id: current_user.try(:id), course_id: assignment.course_id) ||
      (assignment.course_id && Course.find(assignment.course_id).instructor_id == current_user.try(:id))
    else
      ['Super-Administrator',
       'Administrator',
       'Instructor',
       'Teaching Assistant'].include? current_role_name
    end
  end
  • Solution 3: Establish helper methods in the new authorization_helper.rb to centralize as much authorization logic as possible. In this way, a developer with questions about authorization knows just where to look to find answers - authorization_helper.rb
  # TODO add updated code here

New Implementation

  • TODO: add bulleted list of new implementation stuff
  # TODO: add code demonstrating something about each new implementation bullet point

Code improvements

  • TODO: add bulleted list of code improvements that are not already discussed above

Automated Testing using RSPEC

TODO: describe our strategy for RSPEC testing

  # TODO add terminal command and result for running RSPEC testing on our new code

Testing from UI

  • TODO: add bulleted list of manual test steps for a few key authorizations

References (General)

  1. Expertiza on GitHub
  2. The live Expertiza website
  3. Expertiza project documentation wiki
  4. Rspec Documentation

References (Our Work)

  1. GitHub Project Repository Fork
  2. [TODO: add link to our deployed app]