CSE/ECE 517 Spring 2021 - E2108. Impersonate controller.rb
This wiki page describes the work done under E2018 OSS Program for Spring 2021, 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.
Problem Statement
Expertiza allows administrators, instructors and Teaching Assistants to impersonate other users like a student. This allows the impersonator to view assignments, deadlines and submissions of other students. The rules to impersonating a user is, the impersonator has to be an ancestor of the impersonate. The hierarchy of impersonation is as follow: super administrator -> Administrator -> Instructor -> Teaching Assistant -> Student Note: impersonation cannot happen within the same level of hierarchy. For Example, a Super Administrator can impersonate any user apart from other Super Administrators, an Administrator can impersonate Instructors, TA, Students and not other Admins and so on. 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 which is 79 lines long.
- Presence of repetitive code (Around 40 repetitive lines)
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 # E1991 : check whether instructor is currently in anonymized view if User.anonymized_view?(session[:ip]) # get real name when instructor is in anonymized view user = User.real_user_from_anonymized_name(params[:impersonate][:name]) else user = User.find_by(name: params[:impersonate][:name]) end 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 # Revert to original account else if !session[:super_user].nil? AuthController.clear_user_info(session, nil) session[:user] = session[:super_user] user = session[:user] session[:super_user] = nil else flash[:error] = "No original account was found. Please close your browser and start a new session." redirect_back return end end end
- Block nesting
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 # E1991 : check whether instructor is currently in anonymized view if User.anonymized_view?(session[:ip]) # get real name when instructor is in anonymized view user = User.real_user_from_anonymized_name(params[:impersonate][:name]) else user = User.find_by(name: params[:impersonate][:name]) end if user unless original_user.can_impersonate? user flash[:error] = "You cannot impersonate #{params[:user][:name]}." redirect_back return end
- Too many return statements
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
- The impersonation can be done by using an impersonate bar which currently does not allow initial impersonation.
This project is focused on resolving the issues mentioned above.
How Impersonating works?
Basic login details: (Instructor login is only available) 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
A.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
B. 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
C.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
D. 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. Older version consisted only of the following snippet of code.
def check_if_special_char if params[:user] if warn_for_special_chars(params[:user][:name], "Username") redirect_back return end end
In the current code, the following code is added to accomodate the navigation bar funcionality of impersonate.
if params[:impersonate] if warn_for_special_chars(params[:impersonate][:name], "Username") redirect_back return end end end
E. 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
F. Changes made to implement Anonymized View
Initially, while using the anonymized view to impersonate the account: student8597, we received the following error. This occured as the following method was initially splitting the name using
def self.real_user_from_anonymized_name(anonymized_name) user_id = anonymized_name.split(' ’)[1] user = User.find_by(id: user_id) return user end
Thus while doing the above function, what happens is that the student’s name is searched for a space and the ID is obtained as the numerics after the space, which isn’t the case for our user’s names (eg: student8597). Thus we had to modify this method under app/models/user.rb to the following:
def self.real_user_from_anonymized_name(anonymized_name) user = User.find_by(name: anonymized_name) return user end
In the above code snippet, what we are instead doing is, directly finding the user from the anonymized_name which in the errored case as in the figure above was ’’student8597'’. Apart from these changes, we also had to include the following statements in various parts of the impersonate_controller.rb file to ensure the the previously written test for anonymous view doesn’t break anywhere.
# E1991 : check whether instructor is currently in anonymized view user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:impersonate][:name]) : user = user = User.find_by(name: params[:impersonate][:name])
The broken part of this refactoring project, as compared to the previous year’s, was the Navigation bar on the top right part of the screen. As mentioned earlier, in order to impersonate we have two methods: Using the Manage --> Impersonate User Using the Navigation Bar. However, as far as the previous submission goes, the Navigation bar’s functionality wasn’t complete. The aim of this refactoring is to also fix this and enable the impersonate functionality to work from there as well. The previous submission broke while trying to use the Navigation bar for impersonation because of the wrong passage of params between the views and controllers. In order to make the impersonate from the Manage --> Impersonate User work, the form that was rendered was the expertiza/app/views/impersonate/start.html.erb. Here, the field filled was :user. And was then checkd for in the impersonate_controller.rb. However, the Navigation Bar makes use of the :impersonate symbol to pass the impersonated user’s name and other details to the controller, for which no check was included earlier. Thus, there was no way to procure a user from the navigation bar. In the current submission this is fixed by including the following check in the impersonate_controller.rb/check_if_special_char:
The check_if_special_char is a function which checks if the Username entered is valid or not.
def check_if_special_char if params[:user] if warn_for_special_chars(params[:user][:name], “Username”) redirect_back return end end if params[:impersonate] if warn_for_special_chars(params[:impersonate][:name], “Username”) redirect_back return end end end
As you can see, in the above code snippet, there is a check for the :impersonate’s params, which earlier was missing(Refer D. check_if_special_char)
Test Plan
The UI testing of this project can be performed on three fronts:
- The normal impersonate functionality from the Manage tab and Revert functionality.
- The impersonate functionality using the navigation bar.
- The anonymized view's impersonate functionality(both from the Manage bar as well as the navigation bar).
You can use the following details of various users to test the heirarchies and the impersonations between them.
Note: You cannot login as a super_administrator2, as the login details are unknown. However, we have included ways to test the inability for a lower hierarchy memeber to impersonate the super_administrator.
Hierarchy of user | User account to enter |
---|---|
Super Administrator | super_administrator2 |
Instructor | intructor6 |
TA | teaching_assistant6753 |
Student | student5890 |
Student | student7609 |
Student | student7610, 7603 |
To test out the three fronts, follow the steps mentioned below under each sub-heading:
A. Testing Impersonate functionality from the Manage tab and Revert functionality:
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.
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
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.
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.
In order to perform the following testing:
1. Login as instructor using username: instructor6 and password: password. 2. Go on to the navigation bar on top right of the screen and enter student8598 and click on impersonate.
note: The above screenshot was captured after instructor6 impersonated student8597, you can also try to impersonate student8597
C. Testing Anonymized View
1.Login as instructor using username: instructor6 and password: password. 2.Go to Manage --> Anonymized view
3.Check for impersonate functionality using Manage --> Impersonate User with user account: student8597.
4.Check for impersonate functionality using navigation bar on top right, with user account: student8598.
Testing Functionalities from a RSPec end
Instructor should be able to impersonate a user while already impersonating a user but from navigation bar. This test is to ascertain the functionality of the user being able to impersonate another user(obeying hierarchy) through the navigation bar on the top right hand corner. This is to test the rectification of the previous issue, where this functionality was broken. Here, we test the functionality by first logging in as the instructor(id:2) and then first impersonating a student1 using the main menu’s Manage tab. This redirects us to the student1’s page. Now, from the navigation bar, we try to impersonate another student, student2. This test finally checks if the session is true and the parameters are correct.
it 'instructor should be able to impersonate a user while already impersonating a user but from nav bar' do allow(User).to receive(:find_by).with(name: student1.name).and_return(student1) allow(User).to receive(:find_by).with(name: student2.name).and_return(student2) allow(instructor).to receive(:can_impersonate?).with(student1).and_return(true) allow(instructor).to receive(:can_impersonate?).with(student2).and_return(true) request.env["HTTP_REFERER"] = "http://www.example.com" @params = { user: { name: student1.name } } @session = { user: instructor } post :impersonate, @params, @session # nav bar uses the :impersonate as the param name, so let make sure it always works from there too. @params = { impersonate: { name: student2.name } } post :impersonate, @params, @session expect(session[:super_user]).to eq instructor expect(session[:user]).to eq student2 expect(session[:original_user]).to eq instructor expect(session[:impersonate]).to be true end
B. Testing Anonymized User
it ‘instructor should be able to impersonate a user with their anonymized name’ do allow(User).to receive(:find_by).with(name: student1.name).and_return(student1) allow(instructor).to receive(:can_impersonate?).with(student1).and_return(true) allow(User).to receive(:anonymized_view?).and_return(true) allow(User).to receive(:real_user_from_anonymized_name).with(“Student30”).and_return(student1) request.env[“HTTP_REFERER”] = “http://www.example.com” @params = { user: { name: “Student30" } } @session = { user: instructor } post :impersonate, @params, @session expect(session[:super_user]).to eq instructor expect(session[:user]).to eq student1 expect(session[:original_user]).to eq instructor expect(session[:impersonate]).to be true end