Merge pull request #43737 from code-dot-org/jamescodeorg/disable-cach…

…ing-apis

Prevent api responses that depend on current_user from being cached

dashboard/app/controllers/api/v1/projects/personal_projects_controller.rb

@@ -1,6 +1,7 @@
 class Api::V1::Projects::PersonalProjectsController < ApplicationController
   # GET /api/v1/projects/personal/
   def index
+    prevent_caching
     return head :forbidden unless current_user
     render json: ProjectsList.fetch_personal_projects(current_user.id)
   rescue ArgumentError => e

dashboard/app/controllers/api/v1/sections_controller.rb

@@ -18,6 +18,7 @@ class Api::V1::SectionsController < Api::V1::JsonApiController
   # GET /api/v1/sections
   # Get the set of sections owned by the current user
   def index
+    prevent_caching
     render json: current_user.sections.map(&:summarize)
   end
 

dashboard/app/controllers/api/v1/users_controller.rb

@@ -15,6 +15,7 @@ def load_user
 
   # GET /api/v1/users/current
   def current
+    prevent_caching
     if current_user
       render json: {
         id: current_user.id,

dashboard/app/controllers/api_controller.rb

@@ -163,6 +163,7 @@ def import_google_classroom
   end
 
   def user_menu
+    prevent_caching
     show_pairing_dialog = !!session.delete(:show_pairing_dialog)
     @user_header_options = {}
     @user_header_options[:current_user] = current_user
@@ -207,6 +208,7 @@ def update_lockable_state
 
   # For a given user, gets the lockable state for each student in each of their sections
   def lockable_state
+    prevent_caching
     unless current_user
       render json: {}
       return
@@ -232,6 +234,7 @@ def lockable_state
   use_database_pool section_progress: :persistent
 
   def section_progress
+    prevent_caching
     section = load_section
     script = load_script(section)
 
@@ -312,6 +315,7 @@ def section_progress
   # If not specified, the API will default to a page size of 50, providing the first page
   # of students
   def section_level_progress
+    prevent_caching
     section = load_section
     script = load_script(section)
 
@@ -345,6 +349,7 @@ def section_level_progress
   # GET /api/teacher_panel_progress/:section_id
   # Get complete details of a particular section for the teacher panel progress
   def teacher_panel_progress
+    prevent_caching
     section = load_section
     script = load_script(section)
 
@@ -383,6 +388,7 @@ def teacher_panel_progress
 
   # Get /api/teacher_panel_section
   def teacher_panel_section
+    prevent_caching
     teacher_sections = current_user&.sections
 
     if teacher_sections.blank?
@@ -424,6 +430,7 @@ def script_standards
 
   # Return a JSON summary of the user's progress for params[:script].
   def user_progress
+    prevent_caching
     if current_user
       if params[:user_id].present?
         user = User.find(params[:user_id])
@@ -450,6 +457,7 @@ def user_progress
   # Returns app_options values that are user-specific. This is used on cached
   # levels.
   def user_app_options
+    prevent_caching
     response = {}
 
     script = Script.get_from_cache(params[:script])
@@ -506,7 +514,7 @@ def user_app_options
   # given user. This code is analogous to parts of LevelsHelper#app_options.
   # TODO: Eliminate this logic from LevelsHelper#app_options or refactor methods
   # to share code.
-  def progress_app_options(script, level, user)
+  private def progress_app_options(script, level, user)
     response = {}
 
     user_level = user.last_attempt(level, script)

dashboard/test/controllers/api/v1/projects/personal_projects_controller_test.rb

@@ -21,6 +21,7 @@ class Api::V1::Projects::PersonalProjectsControllerTest < ActionDispatch::Integr
     sign_in(@project_owner)
     get "/api/v1/projects/personal/"
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
     personal_projects_list = JSON.parse(@response.body)
     assert_equal 1, personal_projects_list.size
     project_row = personal_projects_list.first

dashboard/test/controllers/api/v1/users_controller_test.rb

@@ -168,6 +168,7 @@ class Api::V1::UsersControllerTest < ActionController::TestCase
   test "a get request to get current returns signed out user info" do
     get :current
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
     response = JSON.parse(@response.body)
     assert_equal false, response["is_signed_in"]
   end
@@ -177,6 +178,7 @@ class Api::V1::UsersControllerTest < ActionController::TestCase
     sign_in(teacher)
     get :current
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
     response = JSON.parse(@response.body)
     assert_equal true, response["is_signed_in"]
     assert_equal teacher.id, response["id"]

dashboard/test/controllers/api_controller_test.rb

@@ -287,6 +287,7 @@ class ApiControllerTest < ActionController::TestCase
       script_id: script.id
     }
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
     body = JSON.parse(response.body)
 
     assert_equal [@section.id.to_s, @flappy_section.id.to_s, @allthings_section.id.to_s], body.keys, "entry for each section"
@@ -825,6 +826,7 @@ def get_student_response(script, level, lesson, student_number)
 
     get :user_progress, params: {script: @script.name}
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
 
     body = JSON.parse(response.body)
     assert_equal true, body['signedIn']
@@ -887,6 +889,8 @@ def get_student_response(script, level, lesson, student_number)
       level: @level.id
     }
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
+
     body = JSON.parse(response.body)
     assert_equal true, body['signedIn']
     assert_equal false, body['disableSocialShare']
@@ -1146,6 +1150,7 @@ def get_student_response(script, level, lesson, student_number)
       get :section_progress, params: {section_id: @flappy_section.id}
     end
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
 
     data = JSON.parse(@response.body)
     expected = {
@@ -1235,6 +1240,12 @@ def get_student_response(script, level, lesson, student_number)
     assert_equal 1, data['students'].length
   end
 
+  test 'section_level_progress response should not be cached by the browser' do
+    get :section_level_progress, params: {section_id: @section.id, page: 1, per: 2}
+    assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
+  end
+
   test "should get paginated section level progress" do
     get :section_level_progress, params: {section_id: @section.id, page: 1, per: 2}
     assert_response :success
@@ -1375,6 +1386,7 @@ def get_student_response(script, level, lesson, student_number)
     }
 
     assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
 
     response = JSON.parse(@response.body)
 
@@ -1446,8 +1458,9 @@ def get_student_response(script, level, lesson, student_number)
     }
 
     assert_response :success
-    response = JSON.parse(@response.body)
+    assert_match "no-store", response.headers["Cache-Control"]
 
+    response = JSON.parse(@response.body)
     assert_equal @section.id, response["id"]
     assert_equal @teacher.name, response["teacherName"]
     assert_equal 7, response["students"].length
@@ -1530,6 +1543,15 @@ def get_student_response(script, level, lesson, student_number)
     assert_equal expected_response, response
   end
 
+  test 'user_menu response should not be cached by the browser' do
+    sign_in create(:student)
+
+    get :user_menu
+
+    assert_response :success
+    assert_match "no-store", response.headers["Cache-Control"]
+  end
+
   test "user menu should open pairing dialog if asked to in the session" do
     sign_in create(:student)