Main Page/CSC/CSC 517 Spring 2020 Refactor impersonate controller: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
mNo edit summary
(Added test snips to testing plan)
 
(5 intermediate revisions by 2 users not shown)
Line 1: Line 1:
This wiki page describes the work done under E2002 OSS Program for Spring 2020, in the CSC/ECE 517 course.
This wiki page describes the work done under E2002 OSS Program for Spring 2020, in the CSC/ECE 517 course.


===Peer Review Information===
1. Instructor login: username -> instructor6, password -> password


when logged in as an instructor, under the manage option in the ribbon, select impersonate user. Upon redirected to impersonate page, enter the account which needs to be impersonated. If possible it impersonates that user. Now a new button called revert appears on the ribbon, this can be used to revert the impersonation and return to the instructor profile.
 
 


===About Expertiza===
===About Expertiza===
Line 10: Line 9:
[http://expertiza.ncsu.edu/ Expertiza] is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza is a complete instructor-student usage website where the instructor can assign assignments, deadlines, grades, etc that is required for the course. Similarly, the students can use this website to perform the tasks required as part of the course like project or assignments submission, forming groups and collaborating with them, as well as reviewing projects and teammates.
[http://expertiza.ncsu.edu/ Expertiza] is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza is a complete instructor-student usage website where the instructor can assign assignments, deadlines, grades, etc that is required for the course. Similarly, the students can use this website to perform the tasks required as part of the course like project or assignments submission, forming groups and collaborating with them, as well as reviewing projects and teammates.


This project focuses on a specific feature of expertiza which allows administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account.
The demonstration for the feature is as shown below.


[[File:Impersonate guide.jpg|figure 1|frame|center]]
       
[[File:Non impersonate.jpg|figure 2|frame|center]]
       
[[File:Impersonated view.jpg|figure 3|frame|center]]
       
===Problem Statement===
===Problem Statement===
The aim of the project is to refactor the impersonate controller. The pre-existing code had two major issues.
The aim of the project is to refactor the impersonate controller. The pre-existing code had the following major issues.
*All functions related to impersonate controller were present in a single method
*All functions related to impersonate controller were present in a single method ( from figure 4a and 4b)
*Presence of repetitive code
*Presence of repetitive code ( from figure 4a and 4b)
*3 levels of block nesting (from figure 5)
* Too many return statements (from figure 6)
[[File:initial code1.jpg| figure 4a| frame|center]]
[[File:initial code2.jpg| figure 4b| frame|center]]
[[File:code climate3.jpg| figure 5| frame|center]]
[[File:code climate2.jpg| figure 6| frame|center]]
 
This project is focused on resolving the issues mentioned above.
 
 
===About Impersonate Controller===
Expertiza allows the administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. For example, an instructor impersonating a student’s account can view their assignments, stage deadlines, peer reviews and anything else that the student can view.
 
One thing to be noted is that most of these users can only impersonate users for whom they are a parent. For example, instructor6 is a parent of student3841 and not student3836; as a result, instructor6 can impersonate only 3841.
 
===Impersonate functionality navigation===
1. Instructor login: username -> instructor6, password -> password


These two issues have been tackled by
when logged in as an instructor, under the manage option in the ribbon as in Figure 1, select impersonate user.
Creating new methods which were called inside the impersonate controller. The new methods were:
Upon redirected to impersonate page, enter the account which needs to be impersonated. It impersonates that user provided that user can be impersonated. Now a new button called revert appears on the ribbon as in figure 3, this can be used to revert the impersonation and return to the instructor profile.


*checkif_user_impersonateable
===Problem Solution===
The above-mentioned issues have been tackled by refactoring the impersonate controller by splitting into many smaller methods which are later called by the main impersonate controller.
 
The following are the refactored new methods that help in tackling the issue1 apart from each being specifically for some issue rectification:
*check_if_user_impersonateable
*display_error_msg
*display_error_msg
*clear_session
*overwrite_session
*check_if_special_char
*do_main_operartion


===About Impersonate Controller===
 
Expertiza allows the administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. For example, an instructor impersonating a student’s account can view their assignments, stage deadlines, peer reviews and anything else that the student can view. One thing to be noted is that most of these users con only impersonate users for whom they are a parent. For example, instructor6 is a parent of student3841 and not student3836; as a result, instructor6 can impersonate only 3841.
=====check_if_user_impersonateable=====
This method plays the main role in tackling issue3 - 3 levels of block nesting apart from issue1.
 
'''Intial Code'''
<pre>
if user
          unless original_user.can_impersonate? user
            flash[:error] = "You cannot impersonate #{params[:user][:name]}."
            redirect_back
            return
          end
</pre>
''' After recfactoring - Moved to separate method'''
<pre>
    def check_if_user_impersonateable
    if params[:impersonate].nil?
          user = User.find_by(name: params[:user][:name])
          if !@original_user.can_impersonate? user
            @message = "You cannot impersonate '#{params[:user][:name]}'."    
            temp
    AuthController.clear_user_info(session, nil)         
          else
            overwrite_session
  end
    else
          if !params[:impersonate][:name].empty?
            user = User.find_by(name: params[:impersonate][:name])
    overwrite_session
          end
    end
  end
</pre>
 
=====display_error_msg=====
This method is used to tackle issues1, 2 and 4. All the error message related code is moved to this method.
<pre>
    def display_error_msg
    if params[:user]
          @message = "No user exists with the name '#{params[:user][:name]}'."
    elsif params[:impersonate]
          @message = "No user exists with the name '#{params[:impersonate][:name]}'."   
    else
          if params[:impersonate].nil?
            @message = "You cannot impersonate '#{params[:user][:name]}'."
          else
            if !params[:impersonate][:name].empty?
              @message = "You cannot impersonate '#{params[:impersonate][:name]}'."
            else
              @message = "No original account was found. Please close your browser and start a new session."
          end
      end
    end
    rescue Exception => e
      flash[:error] = @message
      redirect_to :back
  end
</pre>
=====overwrite_session=====
This method reduces the number of return statements used in impersonate controller, apart from reducing the size of the controller.
 
====== Initial Code======
<pre>
if params[:impersonate].nil?
        # check if special chars /\?<>|&$# are used to avoid html tags or system command
        if warn_for_special_chars(params[:user][:name], "Username")
          redirect_back
          return
        end
        user = User.find_by(name: params[:user][:name])
        if user
          unless original_user.can_impersonate? user
            flash[:error] = "You cannot impersonate #{params[:user][:name]}."
            redirect_back
            return
          end
          session[:super_user] = session[:user] if session[:super_user].nil?
          AuthController.clear_user_info(session, nil)
          session[:original_user] = original_user
          session[:impersonate] = true
          session[:user] = user
        else
          flash[:error] = message
          redirect_back
          return
        end
      else
        # Impersonate a new account
        if !params[:impersonate][:name].empty?
          # check if special chars /\?<>|&$# are used to avoid html tags or system command
          if warn_for_special_chars(params[:impersonate][:name], "Username")
            redirect_back
            return
          end
          user = User.find_by(name: params[:impersonate][:name])
          if user
            unless original_user.can_impersonate? user
              flash[:error] = "You cannot impersonate #{params[:user][:name]}."
              redirect_back
              return
            end
            AuthController.clear_user_info(session, nil)
            session[:user] = user
            session[:impersonate] =  true
            session[:original_user] = original_user
          else
            flash[:error] = message
            redirect_back
            return
          end
</pre>
====== After Refactoring - Moved to a separate method and accessed through the adapter method do_main_operation======
<pre>
    def overwrite_session
    #if not impersonatable, then original user's session remains
    if params[:impersonate].nil?
          user = User.find_by(name: params[:user][:name])
          session[:super_user] = session[:user] if session[:super_user].nil?
        AuthController.clear_user_info(session, nil)
          session[:original_user] = @original_user
          session[:impersonate] = true
          session[:user] = user
    else
    #if some user is to be impersonated, their session details are overwritten onto the current to impersonate
 
          if !params[:impersonate][:name].empty?
    user = User.find_by(name: params[:impersonate][:name])
    AuthController.clear_user_info(session, nil)
            session[:user] = user
            session[:impersonate] =  true
            session[:original_user] = @original_user
          else
            user = User.find_by(name: params[:user][:name])
    AuthController.clear_user_info(session, nil)
            session[:user] = session[:super_user]
            user = session[:user]
            session[:super_user] = nil
          end
    end
  end
</pre>
=====check_if_special_char=====
This code is used to reduce one functionality performed under the impersonate controller. This method checks to see if the given username is acceptable.
<pre>
  def check_if_special_char
    if warn_for_special_chars(params[:user][:name], "Username")
          redirect_back
          return
    end
  end
</pre>
=====do_main_operation=====
This like an adapter method that is used to interface the impersonate method with display_error_msg and check_if_user_impersonatable. One main purpose to do this is to make the methods flexible for change apart from reducing the number of lines from the impersonate controller.
<pre>
    def do_main_operation(user)
    if user
      check_if_user_impersonateable
    else
      display_error_msg
    end
  end 
</pre>
 
=== Test Plan ===
The project can be tested from the UI as follows.
 
Checking if impersonating a user is working
  1. Input: User that can be impersonated
    - Login as the instructor (username -> instructor6 , password -> password
    - Under "Manage" tab, select "impersonate user" option
    - In the form, give the user ID as "student5890"
  Now you will be able to see that user "student5890" has been impersonated.
 
[[File:imp test 1.jpg| figure 7| frame|center]]
[[File:imp test 2.jpg| figure 8| frame|center]]
 
[[File:imp test 4.jpg| figure 9| frame|center]]
 
  2. Input: Reverting from impersonated account
      We will have the user "student5890"  impersonated.
      - Press the blue revert button at the top of the window
      Now you will have returned to the instructor profile
 
[[File:imp test 4.jpg| figure 10| frame|center]]
[[File:imp test 8.jpg| figure 11| frame|center]]
 
  3. Input: User that cannot be impersonated
    - Login as the instructor (username -> instructor6 , password -> password
    - Under "Manage" tab, select "impersonate user" option
    - In the form, give the user ID as "super_administrator2"
  Now you will be able to see a message being displayed on the screen, that tells that the given user cannot be impersonated.
[[File:imp test 6.jpg| figure 12| frame|center]]
[[File:imp test 5.jpg| figure 13| frame|center]]
 
 
  4. Input: User that does not exist
    - Login as the instructor (username -> instructor6 , password -> password
    - Under "Manage" tab, select "impersonate user" option
    - In the form, give the user ID as "studentstudent"
  Now you will be able to see a message being displayed on the screen, that tells that the given user does not exist.
[[File:imp test 7.jpg| figure 14| frame|center]]
[[File:imp test 3.jpg| figure 15| frame|center]]

Latest revision as of 03:22, 1 April 2020

This wiki page describes the work done under E2002 OSS Program for Spring 2020, in the CSC/ECE 517 course.



About Expertiza

Expertiza is an open-source project based on Ruby on Rails framework. Expertiza is a complete instructor-student usage website where the instructor can assign assignments, deadlines, grades, etc that is required for the course. Similarly, the students can use this website to perform the tasks required as part of the course like project or assignments submission, forming groups and collaborating with them, as well as reviewing projects and teammates.

This project focuses on a specific feature of expertiza which allows administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. The demonstration for the feature is as shown below.

figure 1


figure 2
figure 3

Problem Statement

The aim of the project is to refactor the impersonate controller. The pre-existing code had the following major issues.

  • All functions related to impersonate controller were present in a single method ( from figure 4a and 4b)
  • Presence of repetitive code ( from figure 4a and 4b)
  • 3 levels of block nesting (from figure 5)
  • Too many return statements (from figure 6)
figure 4a
figure 4b
figure 5
figure 6

This project is focused on resolving the issues mentioned above.


About Impersonate Controller

Expertiza allows the administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. For example, an instructor impersonating a student’s account can view their assignments, stage deadlines, peer reviews and anything else that the student can view.

One thing to be noted is that most of these users can only impersonate users for whom they are a parent. For example, instructor6 is a parent of student3841 and not student3836; as a result, instructor6 can impersonate only 3841.

Impersonate functionality navigation

1. Instructor login: username -> instructor6, password -> password

when logged in as an instructor, under the manage option in the ribbon as in Figure 1, select impersonate user. Upon redirected to impersonate page, enter the account which needs to be impersonated. It impersonates that user provided that user can be impersonated. Now a new button called revert appears on the ribbon as in figure 3, this can be used to revert the impersonation and return to the instructor profile.

Problem Solution

The above-mentioned issues have been tackled by refactoring the impersonate controller by splitting into many smaller methods which are later called by the main impersonate controller.

The following are the refactored new methods that help in tackling the issue1 apart from each being specifically for some issue rectification:

  • check_if_user_impersonateable
  • display_error_msg
  • overwrite_session
  • check_if_special_char
  • do_main_operartion


check_if_user_impersonateable

This method plays the main role in tackling issue3 - 3 levels of block nesting apart from issue1.

Intial Code

if user
          unless original_user.can_impersonate? user
            flash[:error] = "You cannot impersonate #{params[:user][:name]}."
            redirect_back
            return
          end

After recfactoring - Moved to separate method

    def check_if_user_impersonateable 
    if params[:impersonate].nil?
          user = User.find_by(name: params[:user][:name])
          if !@original_user.can_impersonate? user
            @message = "You cannot impersonate '#{params[:user][:name]}'."	    
            temp
	    AuthController.clear_user_info(session, nil)          
          else 
            overwrite_session
	  end
    else 
          if !params[:impersonate][:name].empty?
            user = User.find_by(name: params[:impersonate][:name])
	    overwrite_session
          end
    end
  end 
display_error_msg

This method is used to tackle issues1, 2 and 4. All the error message related code is moved to this method.

    def display_error_msg
    if params[:user]
          @message = "No user exists with the name '#{params[:user][:name]}'."
    elsif params[:impersonate]
          @message = "No user exists with the name '#{params[:impersonate][:name]}'."    
    else	 
          if params[:impersonate].nil?
            @message = "You cannot impersonate '#{params[:user][:name]}'."
          else
            if !params[:impersonate][:name].empty?
              @message = "You cannot impersonate '#{params[:impersonate][:name]}'."
            else
              @message = "No original account was found. Please close your browser and start a new session."
           end 
       end
    end
    rescue Exception => e
      flash[:error] = @message
      redirect_to :back
  end 
overwrite_session

This method reduces the number of return statements used in impersonate controller, apart from reducing the size of the controller.

Initial Code
if params[:impersonate].nil?
        # check if special chars /\?<>|&$# are used to avoid html tags or system command
        if warn_for_special_chars(params[:user][:name], "Username")
          redirect_back
          return
        end
        user = User.find_by(name: params[:user][:name])
        if user
          unless original_user.can_impersonate? user
            flash[:error] = "You cannot impersonate #{params[:user][:name]}."
            redirect_back
            return
          end
          session[:super_user] = session[:user] if session[:super_user].nil?
          AuthController.clear_user_info(session, nil)
          session[:original_user] = original_user
          session[:impersonate] = true
          session[:user] = user
        else
          flash[:error] = message
          redirect_back
          return
        end
      else
        # Impersonate a new account
        if !params[:impersonate][:name].empty?
          # check if special chars /\?<>|&$# are used to avoid html tags or system command
          if warn_for_special_chars(params[:impersonate][:name], "Username")
            redirect_back
            return
          end
          user = User.find_by(name: params[:impersonate][:name])
          if user
            unless original_user.can_impersonate? user
              flash[:error] = "You cannot impersonate #{params[:user][:name]}."
              redirect_back
              return
            end
            AuthController.clear_user_info(session, nil)
            session[:user] = user
            session[:impersonate] =  true
            session[:original_user] = original_user
          else
            flash[:error] = message
            redirect_back
            return
          end
After Refactoring - Moved to a separate method and accessed through the adapter method do_main_operation
     def overwrite_session
    #if not impersonatable, then original user's session remains
    if params[:impersonate].nil?
          user = User.find_by(name: params[:user][:name])
          session[:super_user] = session[:user] if session[:super_user].nil?
      	  AuthController.clear_user_info(session, nil)
          session[:original_user] = @original_user
          session[:impersonate] = true
          session[:user] = user
    else
    #if some user is to be impersonated, their session details are overwritten onto the current to impersonate	

          if !params[:impersonate][:name].empty?
	    user = User.find_by(name: params[:impersonate][:name])
	    AuthController.clear_user_info(session, nil)
            session[:user] = user
            session[:impersonate] =  true
            session[:original_user] = @original_user
          else
            user = User.find_by(name: params[:user][:name])
	    AuthController.clear_user_info(session, nil)
            session[:user] = session[:super_user]
            user = session[:user]
            session[:super_user] = nil
          end
    end
   end
check_if_special_char

This code is used to reduce one functionality performed under the impersonate controller. This method checks to see if the given username is acceptable.

  def check_if_special_char
    if warn_for_special_chars(params[:user][:name], "Username")
          redirect_back
          return
    end
  end
do_main_operation

This like an adapter method that is used to interface the impersonate method with display_error_msg and check_if_user_impersonatable. One main purpose to do this is to make the methods flexible for change apart from reducing the number of lines from the impersonate controller.

    def do_main_operation(user)
     if user
      check_if_user_impersonateable
     else
      display_error_msg
     end
  end  

Test Plan

The project can be tested from the UI as follows.

Checking if impersonating a user is working

  1. Input: User that can be impersonated 
    - Login as the instructor (username -> instructor6 , password -> password 
    - Under "Manage" tab, select "impersonate user" option 
    - In the form, give the user ID as "student5890" 
  Now you will be able to see that user "student5890" has been impersonated.
figure 7
figure 8
figure 9
  2. Input: Reverting from impersonated account 

      We will have the user "student5890"  impersonated.
     - Press the blue revert button at the top of the window
     Now you will have returned to the instructor profile
figure 10
figure 11
  3. Input: User that cannot be impersonated
    - Login as the instructor (username -> instructor6 , password -> password 
    - Under "Manage" tab, select "impersonate user" option 
    - In the form, give the user ID as "super_administrator2"
  Now you will be able to see a message being displayed on the screen, that tells that the given user cannot be impersonated. 
figure 12
figure 13


  4. Input: User that does not exist
    - Login as the instructor (username -> instructor6 , password -> password 
    - Under "Manage" tab, select "impersonate user" option 
    - In the form, give the user ID as "studentstudent"
  Now you will be able to see a message being displayed on the screen, that tells that the given user does not exist.
figure 14
figure 15