Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2022 - E2228. Refactor JavaScript on Expertiza

2022-05-02T01:25:14Z

Mrjain:

== Problem Statement==
Refactor JavaScript on Expertiza for but not limited to the Assignment View

== Background ==
Expertiza is a software project that uses peer review to produce reusable learning items. It also allows for the uploading of practically any document type, including URLs and wiki pages, as well as team projects. Expertiza's primary language for managing views is JavaScript. However, these scripts are not well structured and present a large scope of improvement in terms of code quality and optimization.

== Files to be modified ==
* assignments/edit/_due_dates.html.erb
* assignments/edit/_rubrics.html.erb
* assignments/edit/_general.html.erb
* assignments/edit.html.erb

== Plan of Action ==
* Refactor the existing methods to ensure that it follows DRY, ensure functions are modular and follow Single Responsibility which stipulates that a method or function should only be used for one reason and thereby increases code reuse..
* Use an npm package called erb to inject erb code into JavaScript code.
* Provide descriptive comments for each method as well as make code readable by adding proper indentation.
* Identify and remove functions do not serve any purpose as well as remove unnecessary documentation/methods that have been commented out.
* Change deprecated JavaScript syntax to ES6 syntax.

== Issues and potential fixes ==

===1. Code comment and fix indentation ===

All files under consideration are poorly indented and commented. Unindented HTML code may contain bugs that are difficult to trace and make the code less readable. Additionally, it becomes hard to identify repeating codes and enforce DRY. Uncommented code makes it extremely difficult for new programmers to get hang of the application and its components. A snapshot of the existing code to illustrate this issue is shown below:

[[File:comments.jpeg||frame|center]]

Indent codes and add comments might fix this problem.

===2. Ensure DRY is enforced ===

This requires that repeating code is put in functions. There are many places throughout the code where this principle is being violated. Repeated code reduces the quality of the code, increases the file size and makes the program run slower.A snapshot of the existing code to illustrate this issue is shown below:

[[File:dry.jpeg||frame|center]]

Create a parameterized function that can be called wherever the code is being repeated in order to maintain the DRY Principle. This will help in improving the code quality and make it more clean and readable.

=== 3. Isolate Javascript ===

For Javascript methods that do not use any erb tags, we isolate them and keep them in a separate file. This is done since we do not need embedded ruby in the Javascript function and the logic can be separated out. Performing this fix makes the code more readable and easier to handle as well as understand. A snapshot of the existing code to illustrate this issue is shown below:

[[File:jsq.jpeg||frame|center]]

As can be seen in the code snippet above, all the javascript functions have been included in the .html.erb file. These can be moved to a separate directory, for instance app/assets/javascript/

=== 4. Violating Single Responsibility for each function. ===

Split big functions into smaller functions to ensure Single Responsibility for each function. There are many tasks that this function is performing which can be broken down into smaller functions that perform a single task. This will also help us write test cases for each function which will improve the code quality. A snapshot of the existing code to illustrate this issue is shown below:

[[File:sr1.jpeg||frame|center]]

[[File:sr2.jpeg||frame|center]]

[[File:sr3.jpeg||frame|center]]

[[File:sr4.jpeg||frame|center]]

[[File:sr5.jpeg||frame|center]]

[[File:sr6.jpeg||frame|center]]

Break the function into smaller parts to ensure that the DRY Principle and single responsibility principle is not violated and code quality can be improved.

=== 5. Have css in separate files ===

The CSS code is cluttering erb files. The CSS code should be moved into a different file so that in future if there are any changes to be made in the UI which will need the CSS code to be modified, it will not affect the main erb files. A snapshot of the existing code to illustrate this issue is shown below:

[[File:css1.jpeg||frame|center]]

[[File:css2.jpeg||frame|center]]

We plan on defining classes for different components and moving these to a separate stylesheet.

=== 6. Remove unused and useless code ===

Instances of unused code found in some files. Removing code blocks which have no code in them. A snapshot of the existing code to illustrate this issue is shown below:

[[File:unused.jpeg||frame|center]]

=== 7. Refactor variable, function and class names ===

Variables, classes and functions have been defined using names that do not explain their functionalities in full capacity.A snapshot of the existing code to illustrate this issue is shown below:

[[File:changenames.jpeg||frame|center]]

== Goals Acheived ==

* Pure js functions have been moved to assests/javascripts folder.

* Unused functions that have been declared but not used anywhere have been removed.

* The indentations have been fixed and comments have been added.

* The code containing CSS script has been moved.

== Roadblocks ==

* The JavaScript functions take in embedded ruby content as variables. This ruby content is actually ruby objects which are accessing database content. Copying these into JavaScript is not possible, because the changes made on these JavaScript variables will not reflect on the database, as we desire (since we are working on "edit" action).

* Secondly, Javascript is client side and Ruby is server side. Converting variables from language to another will add to network load, as this requires a client-server communication, and impact the performance.

* Repeating codes could not be converted into callable functions because these repeating code lines have javascript and ruby variables intermingling in the javascript function. Hence it has been left as is.

== Testing Plan ==

This project was a refactoring project where we had to separate JavaScript code from html.erb files. All the JS code was used to render the frontend which is why there is no test cases possible.

== Conclusion ==

We analyzed the entire codebase and identified potential problems with the JavaScript code. We plan on using the above mentioned solutions to fix the same. Subsequently, we plan on adding new fixes to the code as and when we find new issues.

== Useful Links ==

* https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Spring_2022_-_E2228._Refactor_JavaScript_on_Expertiza
* https://github.com/expertiza/expertiza/pull/2404
* https://github.com/KrishnaSatya10/expertiza/tree/beta
* https://go.ncsu.edu/e2228-refactor-javascript-on-expertiza

CSC/ECE 517 Spring 2022 - E2228. Refactor JavaScript on Expertiza

2022-05-02T00:20:16Z

Mrjain:

CSC/ECE 517 Spring 2022 - E2228. Refactor JavaScript on Expertiza

2022-05-02T00:11:09Z

Mrjain:

== Problem Statement==
Refactor JavaScript on Expertiza for but not limited to the Assignment View

== Background ==
Expertiza is a software project that uses peer review to produce reusable learning items. It also allows for the uploading of practically any document type, including URLs and wiki pages, as well as team projects. Expertiza's primary language for managing views is JavaScript. However, these scripts are not well structured and present a large scope of improvement in terms of code quality and optimization.

== Files to be modified ==
* assignments/edit/_due_dates.html.erb
* assignments/edit/_rubrics.html.erb
* assignments/edit/_general.html.erb
* assignments/edit.html.erb

== Plan of Action ==
* Refactor the existing methods to ensure that it follows DRY, ensure functions are modular and follow Single Responsibility which stipulates that a method or function should only be used for one reason and thereby increases code reuse..
* Use an npm package called erb to inject erb code into JavaScript code.
* Provide descriptive comments for each method as well as make code readable by adding proper indentation.
* Identify and remove functions do not serve any purpose as well as remove unnecessary documentation/methods that have been commented out.
* Change deprecated JavaScript syntax to ES6 syntax.

== Issues and potential fixes ==

===1. Code comment and fix indentation ===

All files under consideration are poorly indented and commented. Unindented HTML code may contain bugs that are difficult to trace and make the code less readable. Additionally, it becomes hard to identify repeating codes and enforce DRY. Uncommented code makes it extremely difficult for new programmers to get hang of the application and its components. A snapshot of the existing code to illustrate this issue is shown below:

[[File:comments.jpeg||frame|center]]

Indent codes and add comments might fix this problem.

===2. Ensure DRY is enforced ===

This requires that repeating code is put in functions. There are many places throughout the code where this principle is being violated. Repeated code reduces the quality of the code, increases the file size and makes the program run slower.A snapshot of the existing code to illustrate this issue is shown below:

[[File:dry.jpeg||frame|center]]

Create a parameterized function that can be called wherever the code is being repeated in order to maintain the DRY Principle. This will help in improving the code quality and make it more clean and readable.

=== 3. Isolate Javascript ===

For Javascript methods that do not use any erb tags, we isolate them and keep them in a separate file. This is done since we do not need embedded ruby in the Javascript function and the logic can be separated out. Performing this fix makes the code more readable and easier to handle as well as understand. A snapshot of the existing code to illustrate this issue is shown below:

[[File:jsq.jpeg||frame|center]]

As can be seen in the code snippet above, all the javascript functions have been included in the .html.erb file. These can be moved to a separate directory, for instance app/assets/javascript/

=== 4. Violating Single Responsibility for each function. ===

Split big functions into smaller functions to ensure Single Responsibility for each function. There are many tasks that this function is performing which can be broken down into smaller functions that perform a single task. This will also help us write test cases for each function which will improve the code quality. A snapshot of the existing code to illustrate this issue is shown below:

[[File:sr1.jpeg||frame|center]]

[[File:sr2.jpeg||frame|center]]

[[File:sr3.jpeg||frame|center]]

[[File:sr4.jpeg||frame|center]]

[[File:sr5.jpeg||frame|center]]

[[File:sr6.jpeg||frame|center]]

Break the function into smaller parts to ensure that the DRY Principle and single responsibility principle is not violated and code quality can be improved.

=== 5. Have css in separate files ===

The CSS code is cluttering erb files. The CSS code should be moved into a different file so that in future if there are any changes to be made in the UI which will need the CSS code to be modified, it will not affect the main erb files. A snapshot of the existing code to illustrate this issue is shown below:

[[File:css1.jpeg||frame|center]]

[[File:css2.jpeg||frame|center]]

We plan on defining classes for different components and moving these to a separate stylesheet.

=== 6. Remove unused and useless code ===

Instances of unused code found in some files. Removing code blocks which have no code in them. A snapshot of the existing code to illustrate this issue is shown below:

[[File:unused.jpeg||frame|center]]

=== 7. Refactor variable, function and class names ===

Variables, classes and functions have been defined using names that do not explain their functionalities in full capacity.A snapshot of the existing code to illustrate this issue is shown below:

[[File:changenames.jpeg||frame|center]]

== Goals Acheived ==

* Pure js functions have been moved to assests/javascripts folder.

* Unused functions that have been declared but not used anywhere have been removed.

* The indentations have been fixed and comments have been added.

* The code containing CSS script has been moved.

== Roadblocks ==

* The JavaScript functions take in embedded ruby content as variables. This ruby content is actually ruby objects which are accessing database content. Copying these into JavaScript is not possible, because the changes made on these JavaScript variables will not reflect on the database, as we desire (since we are working on "edit" action).

* Secondly, Javascript is client side and Ruby is server side. Converting variables from language to another will add to network load, as this requires a client-server communication, and impact the performance.

* Repeating codes could not be converted into callable functions because these repeating code lines have javascript and ruby variables intermingling in the javascript function. Hence it has been left as is.

== Conclusion ==

We analyzed the entire codebase and identified potential problems with the JavaScript code. We plan on using the above mentioned solutions to fix the same. Subsequently, we plan on adding new fixes to the code as and when we find new issues.

== Useful Links ==

* https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Spring_2022_-_E2228._Refactor_JavaScript_on_Expertiza
* https://github.com/expertiza/expertiza/pull/2404
* https://github.com/KrishnaSatya10/expertiza/tree/beta

CSC/ECE 517 Spring 2022 - S2222: Refactor impersonate controller

2022-03-28T00:39:50Z

Mrjain:

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

==About Expertiza==

[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(s) under him and 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 other and access their account.
The demonstration for the feature is as shown below.

[[File:img1.jpeg|Figure 1|frame|center]]
[[File:img2.jpeg|Figure 2|frame|center]]
[[File:img3.jpeg|Figure 3|frame|center]]

== What does this controller do? ==
Login: We can only login as an instructor(only instructor login details are available)
username -> instructor6, password -> password

* When logged in as an instructor, under the manage option as shown 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.
* As shown in figure 3, this can be used to revert the impersonation and return to the instructor profile.

==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 follows:

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 this project is to refactor the impersonate controller.

== Major issues in the previous version of code ==

* Long single lines of code throughout the controller.
* Repeated code written multiple times in different methods throughout the controller
* Method names used were not informative and readable.
* Undiscovered bug that was breaking the application(Has been reported in Expertiza).
* Unused method discovered.

==What needs to be done==

* Introduce Single Responsibility Principle - A method/function should only have one purpose to fulfill (Read more about it here). Read more about the ideal length of a function here.
* You’re supposed to figure out different things a method is doing, break that method into smaller methods such that each resulting method is doing only one thing.
* Throughout the file, the user is being initialized but never used. This is bad coding practice as it slows down runtime.
* Code comments are needed for each and every function. Figure out what the function is doing and write at-least 50 word descriptions for each function.
* Your code comments should also include the information about what the parameters are for, and what the method returns (not counted for 50 word description).
* Read more about how to write meaningful code comments here.
* Rename the methods to a more informative name. For example, There can be better names for check_if_user_impersonateable method, do_main_operation, check_if_special_char methods
* Refactor very long lines of code to make it more readable
* There are many unnecessary nesting of if statements. Convert them to elif. For example, Line 36, Line 139.
* Use a guard clause instead of wrapping the code inside a conditional expression. Read more about guard clauses here.
* A generalized exception is being rescued everywhere, which is against the coding principles that are supposed to be followed. Find what exceptions could a piece of code generate and rescue a more specialized exception.
* Test Functions and increase test code coverage.

== Solution ==

=== Original Code ===

* Method to overwrite the session details that are corresponding to the user or one being impersonated
<pre>
def overwrite_session
# If not impersonatable, then original user's session remains
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : 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
elsif !params[:impersonate][:name].empty?
# 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.find_by(name: params[:impersonate][:name])
AuthController.clear_user_info(session, nil)
session[:user] = user
session[:impersonate] = true
session[:original_user] = @original_user
else
# E1991 : check whether instructor is currently in anonymized view
AuthController.clear_user_info(session, nil)
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
</pre>

<pre>
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

</pre>

* Checking if the username provided can be impersonated or not

<pre>
def check_if_user_impersonateable
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : 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
unless params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
overwrite_session
end
end
end
</pre>

* Main operation
<pre>

def do_main_operation(user)
if user
check_if_user_impersonateable
else
display_error_msg
end
end
</pre>

* Main operation, method used to break the functions in impersonate controller and bring out 2 functionalities at same level,
* checking if user impersonate-able, if not throw corresponding error message

<pre>

def impersonate
# Initial check to see if the username exists
display_error_msg
begin
@original_user = session[:super_user] || session[:user]
# Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method
if params[:impersonate].nil?
# Check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
do_main_operation(user)
else
# Impersonate a new account
if !params[:impersonate][:name].empty?
# check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
# 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.find_by(name: params[:impersonate][:name])
do_main_operation(user)
# Revert to original account when currently in the impersonated session
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
display_error_msg
end
end
end
# Navigate to user's home location as the default landing page after impersonating or reverting
AuthController.set_current_role(user.role_id, session)
redirect_to action: AuthHelper.get_home_action(session[:user]),
controller: AuthHelper.get_home_controller(session[:user])
rescue StandardError
flash[:error] = @message
redirect_to :back
end
end
</pre>

=== New Code ===
* This function checks if the logged in user is a student or not. If it is a student, do not allow the impersonate mode.
* If the logged in user has the role or anything other than the student, we allow that user to use the impersonate mode.

<pre>

before_action :check_if_input_is_valid

</pre>

=== Refactored Code ===

* Method to Generate Overwrite Session when the user tries to Impersonate another User
* Function created to generate session whenever needed.

<pre>
def generate_session(user)
AuthController.clear_user_info(session, nil)
session[:original_user] = @original_user
session[:impersonate] = true
session[:user] = user
end
</pre>

* Method to overwrite the session details that are corresponding to the user or one being impersonated
* The first 'if' statement is executed if the logged in user tried to access the impersonate feature from his account.
* The 'elsif' statement is executed if the user is impersonating someone and then tried to impersonate another person.
* Function Refactored to call the generate session method to reduce the length of the code and reduced long lines of code

<pre>
def overwrite_session
if params[:impersonate].nil?
user = get_real_user(params[:user][:name])
session[:super_user] = session[:user] if session[:super_user].nil?
generate_session(user)
elsif !params[:impersonate][:name].empty?
user = get_real_user(params[:impersonate][:name])
generate_session(user)
else
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
</pre>

* Checks if special characters are present in the username provided, only alphanumeric should be used
* warn_for_special_chars is a method in SecurityHelper class.SecurityHelper class has methods to handle this.
* special_chars method-Initialises string with special characters /\?<>|&$# .
* contains_special_chars method-converts it to regex and compares with the string
* warn_for_special_chars takes the output from above method and flashes an error if there are any special characters(/\?<>|&$#) in the string

* Function refactored to use guard clauses instead of if else nesting.

<pre>
def check_if_special_char
redirect_back if params[:user] && warn_for_special_chars(params[:user][:name], 'Username')
redirect_back if params[:impersonate] && warn_for_special_chars(params[:impersonate][:name], 'Username')
end
</pre>

* Function refactored to reduce long lines of code.

<pre>
def check_if_user_impersonateable
if params[:impersonate].nil?
user = get_real_user(params[:user][:name])
if !@original_user.can_impersonate? user
@message = "You cannot impersonate '#{params[:user][:name]}'."
AuthController.clear_user_info(session, nil)
else
overwrite_session
end
else
unless params[:impersonate][:name].empty?
overwrite_session
end
end
end
</pre>

* Function name refactored to a better name

<pre>
def do_impersonate_operation(user)
check_if_user_impersonateable if user
display_error_msg
end
</pre>

* Function refactored to reduce long lines of code

<pre>
def impersonate
# Initial check to see if the username exists
display_error_msg
begin
@original_user = session[:super_user] || session[:user]
# Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method
if params[:impersonate].nil?
# Check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
user = get_real_user(params[:user][:name])
do_impersonate_operation(user)
else
# Impersonate a new account
if !params[:impersonate][:name].empty?
# check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
user = get_real_user(params[:impersonate][:name])
do_impersonate_operation(user)
# Revert to original account when currently in the impersonated session
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
display_error_msg
end
end
end
# Navigate to user's home location as the default landing page after impersonating or reverting
AuthController.set_current_role(user.role_id, session)
redirect_to action: AuthHelper.get_home_action(session[:user]),
controller: AuthHelper.get_home_controller(session[:user])
rescue StandardError
flash[:error] = @message
redirect_to :back
end
end
</pre>

* Function created to reduce long lines of code used throughout the code
<pre>
def get_real_user(name)
if User.anonymized_view?(session[:ip])
user = User.real_user_from_anonymized_name(name)
else
user = User.find_by(name: name)
end
return user
end
end
</pre>

== Testing Plan ==

* instructor should not be able to impersonate a super admin user with their real name

This tests for the case when instructor tries to impersonate super_admin in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'instructor should not be able to impersonate a super admin user with their real name' do
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(instructor).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end
</pre>

* instructor should be able to impersonate a user while already impersonating a user but from nav 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.

<pre>
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

</pre>

* instructor should be able to impersonate a user with their anonymized name

This test verifies the functionality that is available with from the Anonymized view. It checks for the impersonation from both the ends - Through the Manage tab as well as the Navigation Bar.

<pre>
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

</pre>

* instructor should not be able to impersonate an admin user with their real name

This tests for the case when instructor tries to impersonate admin in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'instructor should not be able to impersonate an admin user with their real name' do
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(instructor).to receive(:can_impersonate?).with(admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* instructor should be able to impersonate a teaching assistant user with their real name

This tests for the case when instructor tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>
it 'instructor should be able to impersonate a teaching assistant user with their real name' do
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(instructor).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:super_user]).to eq instructor
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq instructor
expect(session[:impersonate]).to be true
end

</pre>

* teaching assistant should be able to impersonate a student with their real name

This tests for the case when teaching assistant tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>
it 'teaching assistant should be able to impersonate a student with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(teaching_assistant).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:super_user]).to eq teaching_assistant
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq teaching_assistant
expect(session[:impersonate]).to be true
end

</pre>

* teaching assistant should not be able to impersonate an instructor with their real name

This tests for the case when teaching assistant tries to impersonate an instructor in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'teaching assistant should not be able to impersonate an instructor with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(teaching_assistant).to receive(:can_impersonate?).with(instructor).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end
end

</pre>

* teaching assistant should not be able to impersonate an admin with their real name

This tests for the case when teaching assistant tries to impersonate an admin in which case parameter 'impersonate' will be nil in session parameters

<pre>

it 'teaching assistant should not be able to impersonate an admin with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(teaching_assistant).to receive(:can_impersonate?).with(admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* teaching assistant should not be able to impersonate a super admin with their real name

This tests for the case when teaching assistant tries to impersonate a super admin in which case parameter 'impersonate' will be nil in session parameters

<pre>

it 'teaching assistant should not be able to impersonate an super admin with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(teaching_assistant).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* admin should be able to impersonate a student with their real name

This tests for the case when admin tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'admin should be able to impersonate a student with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(admin).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq admin
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq admin
expect(session[:impersonate]).to be true
end

</pre>

* admin should be able to impersonate a teaching assistant with their real name

This tests for the case when admin tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'admin should be able to impersonate a teaching assistant with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(admin).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq admin
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq admin
expect(session[:impersonate]).to be true
end

</pre>

* admin should be able to impersonate an instructor with their real name

This tests for the case when admin tries to impersonate an instructor in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'admin should be able to impersonate an instructor with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(admin).to receive(:can_impersonate?).with(instructor).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq admin
expect(session[:user]).to eq instructor
expect(session[:original_user]).to eq admin
expect(session[:impersonate]).to be true
end

</pre>

* admin should not be able to impersonate a super admin with their real name

This tests for the case when admin tries to impersonate a super admin in which case parameter 'impersonate' will be nil in session parameters

<pre>

it 'admin should not be able to impersonate a super admin with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(admin).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* super admin should be able to impersonate a student with their real name

This tests for the case when super admin tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate a student with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(super_admin).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

* super admin should be able to impersonate a teaching assistant with their real name

This tests for the case when super admin tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate a teaching assistant with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(super_admin).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

* super admin should be able to impersonate an instructor with their real name

This tests for the case when super admin tries to impersonate an instructor in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate an instructor with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(super_admin).to receive(:can_impersonate?).with(instructor).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq instructor
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

* super admin should be able to impersonate an admin with their real name

This tests for the case when super admin tries to impersonate an admin in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate an admin with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(super_admin).to receive(:can_impersonate?).with(admin).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq admin
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

=== Useful Links ===
The following are the links to the useful pages in understanding this project(as of Spring 2022)
* [https://expertiza.csc.ncsu.edu/index.php/Expertiza_documentation] - General wiki describing the Expertiza project.
* [https://github.com/expertiza/expertiza] - GitHub repo that maintains code for Expertiza.
* [https://github.com/moksh98/expertiza/tree/E2222-beta] - GitHub repo for the work done in Spring 2022 for the refactoring of impersonate controller.
* [https://expertiza.csc.ncsu.edu/index.php/CSE/ECE_517_Spring_2021_-_E2108._Impersonate_controller.rb] - Spring 2021's wiki for the refactoring of impersonate controller.
* [https://github.com/expertiza/expertiza/pull/2347] - Pull request for the work done in Spring 2022.
* [https://github.com/expertiza/expertiza/issues/2342] - Issue that has been raised for the undiscovered bug that was breaking the application.
* [http://152.7.98.229:8080/] - Application Link

CSC/ECE 517 Spring 2022 - S2222: Refactor impersonate controller

2022-03-28T00:37:22Z

Mrjain:

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

==About Expertiza==

[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(s) under him and 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 other and access their account.
The demonstration for the feature is as shown below.

[[File:img1.jpeg|Figure 1|frame|center]]
[[File:img2.jpeg|Figure 2|frame|center]]
[[File:img3.jpeg|Figure 3|frame|center]]

== What does this controller do? ==
Login: We can only login as an instructor(only instructor login details are available)
username -> instructor6, password -> password

* When logged in as an instructor, under the manage option as shown 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.
* As shown in figure 3, this can be used to revert the impersonation and return to the instructor profile.

==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 follows:

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 this project is to refactor the impersonate controller.

== Major issues in the previous version of code ==

* Long single lines of code throughout the controller.
* Repeated code written multiple times in different methods throughout the controller
* Method names used were not informative and readable.
* Undiscovered bug that was breaking the application(Has been reported in Expertiza).
* Unused method discovered.

==What needs to be done==

* Introduce Single Responsibility Principle - A method/function should only have one purpose to fulfill (Read more about it here). Read more about the ideal length of a function here.
* You’re supposed to figure out different things a method is doing, break that method into smaller methods such that each resulting method is doing only one thing.
* Throughout the file, the user is being initialized but never used. This is bad coding practice as it slows down runtime.
* Code comments are needed for each and every function. Figure out what the function is doing and write at-least 50 word descriptions for each function.
* Your code comments should also include the information about what the parameters are for, and what the method returns (not counted for 50 word description).
* Read more about how to write meaningful code comments here.
* Rename the methods to a more informative name. For example, There can be better names for check_if_user_impersonateable method, do_main_operation, check_if_special_char methods
* Refactor very long lines of code to make it more readable
* There are many unnecessary nesting of if statements. Convert them to elif. For example, Line 36, Line 139.
* Use a guard clause instead of wrapping the code inside a conditional expression. Read more about guard clauses here.
* A generalized exception is being rescued everywhere, which is against the coding principles that are supposed to be followed. Find what exceptions could a piece of code generate and rescue a more specialized exception.
* Test Functions and increase test code coverage.

== Solution ==

=== Original Code ===

* Method to overwrite the session details that are corresponding to the user or one being impersonated
<pre>
def overwrite_session
# If not impersonatable, then original user's session remains
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : 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
elsif !params[:impersonate][:name].empty?
# 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.find_by(name: params[:impersonate][:name])
AuthController.clear_user_info(session, nil)
session[:user] = user
session[:impersonate] = true
session[:original_user] = @original_user
else
# E1991 : check whether instructor is currently in anonymized view
AuthController.clear_user_info(session, nil)
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
</pre>

<pre>
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

</pre>

* Checking if the username provided can be impersonated or not

<pre>
def check_if_user_impersonateable
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : 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
unless params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
overwrite_session
end
end
end
</pre>

* Main operation
<pre>

def do_main_operation(user)
if user
check_if_user_impersonateable
else
display_error_msg
end
end
</pre>

* Main operation, method used to break the functions in impersonate controller and bring out 2 functionalities at same level,
* checking if user impersonate-able, if not throw corresponding error message

<pre>

def impersonate
# Initial check to see if the username exists
display_error_msg
begin
@original_user = session[:super_user] || session[:user]
# Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method
if params[:impersonate].nil?
# Check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
do_main_operation(user)
else
# Impersonate a new account
if !params[:impersonate][:name].empty?
# check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
# 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.find_by(name: params[:impersonate][:name])
do_main_operation(user)
# Revert to original account when currently in the impersonated session
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
display_error_msg
end
end
end
# Navigate to user's home location as the default landing page after impersonating or reverting
AuthController.set_current_role(user.role_id, session)
redirect_to action: AuthHelper.get_home_action(session[:user]),
controller: AuthHelper.get_home_controller(session[:user])
rescue StandardError
flash[:error] = @message
redirect_to :back
end
end
</pre>

=== New Code ===
* This function checks if the logged in user is a student or not. If it is a student, do not allow the impersonate mode.
* If the logged in user has the role or anything other than the student, we allow that user to use the impersonate mode.

<pre>

before_action :check_if_input_is_valid

</pre>

=== Refactored Code ===

* Method to Generate Overwrite Session when the user tries to Impersonate another User
* Function created to generate session whenever needed.

<pre>
def generate_session(user)
AuthController.clear_user_info(session, nil)
session[:original_user] = @original_user
session[:impersonate] = true
session[:user] = user
end
</pre>

* Method to overwrite the session details that are corresponding to the user or one being impersonated
* The first 'if' statement is executed if the logged in user tried to access the impersonate feature from his account.
* The 'elsif' statement is executed if the user is impersonating someone and then tried to impersonate another person.
* Function Refactored to call the generate session method to reduce the length of the code and reduced long lines of code

<pre>
def overwrite_session
if params[:impersonate].nil?
user = get_real_user(params[:user][:name])
session[:super_user] = session[:user] if session[:super_user].nil?
generate_session(user)
elsif !params[:impersonate][:name].empty?
user = get_real_user(params[:impersonate][:name])
generate_session(user)
else
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
</pre>

* Checks if special characters are present in the username provided, only alphanumeric should be used
* warn_for_special_chars is a method in SecurityHelper class.SecurityHelper class has methods to handle this.
* special_chars method-Initialises string with special characters /\?<>|&$# .
* contains_special_chars method-converts it to regex and compares with the string
* warn_for_special_chars takes the output from above method and flashes an error if there are any special characters(/\?<>|&$#) in the string

* Function refactored to use guard clauses instead of if else nesting.

<pre>
def check_if_special_char
redirect_back if params[:user] && warn_for_special_chars(params[:user][:name], 'Username')
redirect_back if params[:impersonate] && warn_for_special_chars(params[:impersonate][:name], 'Username')
end
</pre>

* Function refactored to reduce long lines of code.

<pre>
def check_if_user_impersonateable
if params[:impersonate].nil?
user = get_real_user(params[:user][:name])
if !@original_user.can_impersonate? user
@message = "You cannot impersonate '#{params[:user][:name]}'."
AuthController.clear_user_info(session, nil)
else
overwrite_session
end
else
unless params[:impersonate][:name].empty?
overwrite_session
end
end
end
</pre>

* Function name refactored to a better name

<pre>
def do_impersonate_operation(user)
check_if_user_impersonateable if user
display_error_msg
end
</pre>

* Function refactored to reduce long lines of code

<pre>
def impersonate
# Initial check to see if the username exists
display_error_msg
begin
@original_user = session[:super_user] || session[:user]
# Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method
if params[:impersonate].nil?
# Check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
user = get_real_user(params[:user][:name])
do_impersonate_operation(user)
else
# Impersonate a new account
if !params[:impersonate][:name].empty?
# check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
user = get_real_user(params[:impersonate][:name])
do_impersonate_operation(user)
# Revert to original account when currently in the impersonated session
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
display_error_msg
end
end
end
# Navigate to user's home location as the default landing page after impersonating or reverting
AuthController.set_current_role(user.role_id, session)
redirect_to action: AuthHelper.get_home_action(session[:user]),
controller: AuthHelper.get_home_controller(session[:user])
rescue StandardError
flash[:error] = @message
redirect_to :back
end
end
</pre>

* Function created to reduce long lines of code used throughout the code
<pre>
def get_real_user(name)
if User.anonymized_view?(session[:ip])
user = User.real_user_from_anonymized_name(name)
else
user = User.find_by(name: name)
end
return user
end
end
</pre>

== Testing Plan ==

* instructor should not be able to impersonate a super admin user with their real name

This tests for the case when instructor tries to impersonate super_admin in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'instructor should not be able to impersonate a super admin user with their real name' do
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(instructor).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end
</pre>

* instructor should be able to impersonate a user while already impersonating a user but from nav 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.

<pre>
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

</pre>

* instructor should be able to impersonate a user with their anonymized name

This test verifies the functionality that is available with from the Anonymized view. It checks for the impersonation from both the ends - Through the Manage tab as well as the Navigation Bar.

<pre>
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

</pre>

* instructor should not be able to impersonate an admin user with their real name

This tests for the case when instructor tries to impersonate admin in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'instructor should not be able to impersonate an admin user with their real name' do
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(instructor).to receive(:can_impersonate?).with(admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* instructor should be able to impersonate a teaching assistant user with their real name

This tests for the case when instructor tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>
it 'instructor should be able to impersonate a teaching assistant user with their real name' do
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(instructor).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:super_user]).to eq instructor
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq instructor
expect(session[:impersonate]).to be true
end

</pre>

* teaching assistant should be able to impersonate a student with their real name

This tests for the case when teaching assistant tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>
it 'teaching assistant should be able to impersonate a student with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(teaching_assistant).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:super_user]).to eq teaching_assistant
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq teaching_assistant
expect(session[:impersonate]).to be true
end

</pre>

* teaching assistant should not be able to impersonate an instructor with their real name

This tests for the case when teaching assistant tries to impersonate an instructor in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'teaching assistant should not be able to impersonate an instructor with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(teaching_assistant).to receive(:can_impersonate?).with(instructor).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end
end

</pre>

* teaching assistant should not be able to impersonate an admin with their real name

This tests for the case when teaching assistant tries to impersonate an admin in which case parameter 'impersonate' will be nil in session parameters

<pre>

it 'teaching assistant should not be able to impersonate an admin with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(teaching_assistant).to receive(:can_impersonate?).with(admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* teaching assistant should not be able to impersonate a super admin with their real name

This tests for the case when teaching assistant tries to impersonate a super admin in which case parameter 'impersonate' will be nil in session parameters

<pre>

it 'teaching assistant should not be able to impersonate an super admin with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(teaching_assistant).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* admin should be able to impersonate a student with their real name

This tests for the case when admin tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'admin should be able to impersonate a student with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(admin).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq admin
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq admin
expect(session[:impersonate]).to be true
end

</pre>

* admin should be able to impersonate a teaching assistant with their real name

This tests for the case when admin tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'admin should be able to impersonate a teaching assistant with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(admin).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq admin
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq admin
expect(session[:impersonate]).to be true
end

</pre>

* admin should be able to impersonate an instructor with their real name

This tests for the case when admin tries to impersonate an instructor in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'admin should be able to impersonate an instructor with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(admin).to receive(:can_impersonate?).with(instructor).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq admin
expect(session[:user]).to eq instructor
expect(session[:original_user]).to eq admin
expect(session[:impersonate]).to be true
end

</pre>

* admin should not be able to impersonate a super admin with their real name

This tests for the case when admin tries to impersonate a super admin in which case parameter 'impersonate' will be nil in session parameters

<pre>

it 'admin should not be able to impersonate a super admin with their real name' do
stub_current_user(admin, admin.role.name, admin.role)
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(admin).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: admin }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* super admin should be able to impersonate a student with their real name

This tests for the case when super admin tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate a student with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(super_admin).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

* super admin should be able to impersonate a teaching assistant with their real name

This tests for the case when super admin tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate a teaching assistant with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(super_admin).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

* super admin should be able to impersonate an instructor with their real name

This tests for the case when super admin tries to impersonate an instructor in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate an instructor with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(super_admin).to receive(:can_impersonate?).with(instructor).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq instructor
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

* super admin should be able to impersonate an admin with their real name

This tests for the case when super admin tries to impersonate an admin in which case parameter 'impersonate' will be true in session parameters

<pre>

it 'super admin should be able to impersonate an admin with their real name' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(super_admin).to receive(:can_impersonate?).with(admin).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: super_admin }
post :impersonate, @params, @session
expect(session[:super_user]).to eq super_admin
expect(session[:user]).to eq admin
expect(session[:original_user]).to eq super_admin
expect(session[:impersonate]).to be true
end

</pre>

=== Useful Links ===
The following are the links to the useful pages in understanding this project(as of Spring 2022)
* [https://expertiza.csc.ncsu.edu/index.php/Expertiza_documentation] - General wiki describing the Expertiza project.
* [https://github.com/expertiza/expertiza] - GitHub repo that maintains code for Expertiza.
* [https://github.com/moksh98/expertiza/tree/E2222-beta] - GitHub repo for the work done in Spring 2022 for the refactoring of impersonate controller.
* [https://expertiza.csc.ncsu.edu/index.php/CSE/ECE_517_Spring_2021_-_E2108._Impersonate_controller.rb] - Spring 2021's wiki for the refactoring of impersonate controller.
* [https://github.com/expertiza/expertiza/pull/2347] - Pull request for the work done in Spring 2022.
* [https://github.com/expertiza/expertiza/issues/2342]- Issue that has been raised for the undiscovered bug that was breaking the application.
* [http://152.7.98.229:8080/] - Application Link

CSC/ECE 517 Spring 2022 - S2222: Refactor impersonate controller

2022-03-28T00:28:28Z

Mrjain:

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

==About Expertiza==

[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(s) under him and 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 other and access their account.
The demonstration for the feature is as shown below.

[[File:img1.jpeg|Figure 1|frame|center]]
[[File:img2.jpeg|Figure 2|frame|center]]
[[File:img3.jpeg|Figure 3|frame|center]]

== What does this controller do? ==
Login: We can only login as an instructor(only instructor login details are available)
username -> instructor6, password -> password

* When logged in as an instructor, under the manage option as shown 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.
* As shown in figure 3, this can be used to revert the impersonation and return to the instructor profile.

==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 follows:

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 this project is to refactor the impersonate controller.

== Major issues in the previous version of code ==

* Long single lines of code throughout the controller.
* Repeated code written multiple times in different methods throughout the controller
* Method names used were not informative and readable.
* Undiscovered bug that was breaking the application(Has been reported in Expertiza).
* Unused method discovered.

==What needs to be done==

* Introduce Single Responsibility Principle - A method/function should only have one purpose to fulfill (Read more about it here). Read more about the ideal length of a function here.
* You’re supposed to figure out different things a method is doing, break that method into smaller methods such that each resulting method is doing only one thing.
* Throughout the file, the user is being initialized but never used. This is bad coding practice as it slows down runtime.
* Code comments are needed for each and every function. Figure out what the function is doing and write at-least 50 word descriptions for each function.
* Your code comments should also include the information about what the parameters are for, and what the method returns (not counted for 50 word description).
* Read more about how to write meaningful code comments here.
* Rename the methods to a more informative name. For example, There can be better names for check_if_user_impersonateable method, do_main_operation, check_if_special_char methods
* Refactor very long lines of code to make it more readable
* There are many unnecessary nesting of if statements. Convert them to elif. For example, Line 36, Line 139.
* Use a guard clause instead of wrapping the code inside a conditional expression. Read more about guard clauses here.
* A generalized exception is being rescued everywhere, which is against the coding principles that are supposed to be followed. Find what exceptions could a piece of code generate and rescue a more specialized exception.
* Test Functions and increase test code coverage.

== Solution ==

=== Original Code ===

* Method to overwrite the session details that are corresponding to the user or one being impersonated
<pre>
def overwrite_session
# If not impersonatable, then original user's session remains
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : 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
elsif !params[:impersonate][:name].empty?
# 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.find_by(name: params[:impersonate][:name])
AuthController.clear_user_info(session, nil)
session[:user] = user
session[:impersonate] = true
session[:original_user] = @original_user
else
# E1991 : check whether instructor is currently in anonymized view
AuthController.clear_user_info(session, nil)
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
</pre>

<pre>
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

</pre>

* Checking if the username provided can be impersonated or not

<pre>
def check_if_user_impersonateable
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : 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
unless params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
overwrite_session
end
end
end
</pre>

* Main operation
<pre>

def do_main_operation(user)
if user
check_if_user_impersonateable
else
display_error_msg
end
end
</pre>

* Main operation, method used to break the functions in impersonate controller and bring out 2 functionalities at same level,
* checking if user impersonate-able, if not throw corresponding error message

<pre>

def impersonate
# Initial check to see if the username exists
display_error_msg
begin
@original_user = session[:super_user] || session[:user]
# Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method
if params[:impersonate].nil?
# Check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
do_main_operation(user)
else
# Impersonate a new account
if !params[:impersonate][:name].empty?
# check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
# 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.find_by(name: params[:impersonate][:name])
do_main_operation(user)
# Revert to original account when currently in the impersonated session
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
display_error_msg
end
end
end
# Navigate to user's home location as the default landing page after impersonating or reverting
AuthController.set_current_role(user.role_id, session)
redirect_to action: AuthHelper.get_home_action(session[:user]),
controller: AuthHelper.get_home_controller(session[:user])
rescue StandardError
flash[:error] = @message
redirect_to :back
end
end
</pre>

=== New Code ===
* This function checks if the logged in user is a student or not. If it is a student, do not allow the impersonate mode.
* If the logged in user has the role or anything other than the student, we allow that user to use the impersonate mode.

<pre>

before_action :check_if_input_is_valid

</pre>

=== Refactored Code ===

* Method to Generate Overwrite Session when the user tries to Impersonate another User
* Function created to generate session whenever needed.

<pre>
def generate_session(user)
AuthController.clear_user_info(session, nil)
session[:original_user] = @original_user
session[:impersonate] = true
session[:user] = user
end
</pre>

* Method to overwrite the session details that are corresponding to the user or one being impersonated
* The first 'if' statement is executed if the logged in user tried to access the impersonate feature from his account.
* The 'elsif' statement is executed if the user is impersonating someone and then tried to impersonate another person.
* Function Refactored to call the generate session method to reduce the length of the code and reduced long lines of code

<pre>
def overwrite_session
if params[:impersonate].nil?
user = get_real_user(params[:user][:name])
session[:super_user] = session[:user] if session[:super_user].nil?
generate_session(user)
elsif !params[:impersonate][:name].empty?
user = get_real_user(params[:impersonate][:name])
generate_session(user)
else
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
</pre>

* Checks if special characters are present in the username provided, only alphanumeric should be used
* warn_for_special_chars is a method in SecurityHelper class.SecurityHelper class has methods to handle this.
* special_chars method-Initialises string with special characters /\?<>|&$# .
* contains_special_chars method-converts it to regex and compares with the string
* warn_for_special_chars takes the output from above method and flashes an error if there are any special characters(/\?<>|&$#) in the string

* Function refactored to use guard clauses instead of if else nesting.

<pre>
def check_if_special_char
redirect_back if params[:user] && warn_for_special_chars(params[:user][:name], 'Username')
redirect_back if params[:impersonate] && warn_for_special_chars(params[:impersonate][:name], 'Username')
end
</pre>

* Function refactored to reduce long lines of code.

<pre>
def check_if_user_impersonateable
if params[:impersonate].nil?
user = get_real_user(params[:user][:name])
if !@original_user.can_impersonate? user
@message = "You cannot impersonate '#{params[:user][:name]}'."
AuthController.clear_user_info(session, nil)
else
overwrite_session
end
else
unless params[:impersonate][:name].empty?
overwrite_session
end
end
end
</pre>

* Function name refactored to a better name

<pre>
def do_impersonate_operation(user)
check_if_user_impersonateable if user
display_error_msg
end
</pre>

* Function refactored to reduce long lines of code

<pre>
def impersonate
# Initial check to see if the username exists
display_error_msg
begin
@original_user = session[:super_user] || session[:user]
# Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method
if params[:impersonate].nil?
# Check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
user = get_real_user(params[:user][:name])
do_impersonate_operation(user)
else
# Impersonate a new account
if !params[:impersonate][:name].empty?
# check if special chars /\?<>|&$# are used to avoid html tags or system command
check_if_special_char
user = get_real_user(params[:impersonate][:name])
do_impersonate_operation(user)
# Revert to original account when currently in the impersonated session
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
display_error_msg
end
end
end
# Navigate to user's home location as the default landing page after impersonating or reverting
AuthController.set_current_role(user.role_id, session)
redirect_to action: AuthHelper.get_home_action(session[:user]),
controller: AuthHelper.get_home_controller(session[:user])
rescue StandardError
flash[:error] = @message
redirect_to :back
end
end
</pre>

* Function created to reduce long lines of code used throughout the code
<pre>
def get_real_user(name)
if User.anonymized_view?(session[:ip])
user = User.real_user_from_anonymized_name(name)
else
user = User.find_by(name: name)
end
return user
end
end
</pre>

== Testing Plan ==

* instructor should not be able to impersonate a super admin user with their real name

This tests for the case when instructor tries to impersonate super_admin in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'instructor should not be able to impersonate a super admin user with their real name' do
allow(User).to receive(:find_by).with(name: super_admin.name).and_return(super_admin)
allow(instructor).to receive(:can_impersonate?).with(super_admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: super_admin.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end
</pre>

* instructor should be able to impersonate a user while already impersonating a user but from nav 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.

<pre>
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

</pre>

* instructor should be able to impersonate a user with their anonymized name

This test verifies the functionality that is available with from the Anonymized view. It checks for the impersonation from both the ends - Through the Manage tab as well as the Navigation Bar.

<pre>
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

</pre>

* instructor should not be able to impersonate an admin user with their real name

This tests for the case when instructor tries to impersonate admin in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'instructor should not be able to impersonate an admin user with their real name' do
allow(User).to receive(:find_by).with(name: admin.name).and_return(admin)
allow(instructor).to receive(:can_impersonate?).with(admin).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: admin.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end

</pre>

* instructor should be able to impersonate a teaching assistant user with their real name

This tests for the case when instructor tries to impersonate a teaching assistant in which case parameter 'impersonate' will be true in session parameters

<pre>
it 'instructor should be able to impersonate a teaching assistant user with their real name' do
allow(User).to receive(:find_by).with(name: teaching_assistant.name).and_return(teaching_assistant)
allow(instructor).to receive(:can_impersonate?).with(teaching_assistant).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: teaching_assistant.name } }
@session = { user: instructor }
post :impersonate, @params, @session
expect(session[:super_user]).to eq instructor
expect(session[:user]).to eq teaching_assistant
expect(session[:original_user]).to eq instructor
expect(session[:impersonate]).to be true
end

</pre>

* teaching assistant should be able to impersonate a student with their real name

This tests for the case when teaching assistant tries to impersonate a student in which case parameter 'impersonate' will be true in session parameters

<pre>
it 'teaching assistant should be able to impersonate a student with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: student1.name).and_return(student1)
allow(teaching_assistant).to receive(:can_impersonate?).with(student1).and_return(true)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: student1.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:super_user]).to eq teaching_assistant
expect(session[:user]).to eq student1
expect(session[:original_user]).to eq teaching_assistant
expect(session[:impersonate]).to be true
end

</pre>

* teaching assistant should not be able to impersonate an instructor with their real name

This tests for the case when teaching assistant tries to impersonate an instructor in which case parameter 'impersonate' will be nil in session parameters

<pre>
it 'teaching assistant should not be able to impersonate an instructor with their real name' do
stub_current_user(teaching_assistant, teaching_assistant.role.name, teaching_assistant.role)
allow(User).to receive(:find_by).with(name: instructor.name).and_return(instructor)
allow(teaching_assistant).to receive(:can_impersonate?).with(instructor).and_return(false)
request.env['HTTP_REFERER'] = 'http://www.example.com'
@params = { user: { name: instructor.name } }
@session = { user: teaching_assistant }
post :impersonate, @params, @session
expect(session[:impersonate]).to be nil
end
end

</pre>

=== Useful Links ===
The following are the links to the useful pages in understanding this project(as of Spring 2022)
* [https://expertiza.csc.ncsu.edu/index.php/Expertiza_documentation] - General wiki describing the Expertiza project.
* [https://github.com/expertiza/expertiza] - GitHub repo that maintains code for Expertiza.
* [https://github.com/moksh98/expertiza/tree/E2222-beta] - GitHub repo for the work done in Spring 2022 for the refactoring of impersonate controller.
* [https://expertiza.csc.ncsu.edu/index.php/CSE/ECE_517_Spring_2021_-_E2108._Impersonate_controller.rb] - Spring 2021's wiki for the refactoring of impersonate controller.
* [https://github.com/expertiza/expertiza/pull/2347] - Pull request for the work done in Spring 2022.
* [https://github.com/expertiza/expertiza/issues/2342]- Issue that has been raised for the undiscovered bug that was breaking the application.
* [http://152.7.98.229:8080/]-