Skip to content

Commit e430efb

Browse files
committed
Send Cache-Control: no-store on account and user-edit pages. OP-19674
Firefox serves these pages stale from the HTTP cache / bfcache on reload and history-back; the Turbo meta only covers Turbo's own snapshot. no-store opts out of both, fixing Firefox. Chrome revalidates and is already fresh on reload and forward navigation, so those two specs pass there and act as cross-browser guards.
1 parent 74a0105 commit e430efb

5 files changed

Lines changed: 84 additions & 0 deletions

File tree

app/controllers/application_controller.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ def set_cache_buster
178178
end
179179
end
180180

181+
# Firefox serves these pages stale from the HTTP cache and bfcache on reload and
182+
# history-back (the turbo-cache-control meta only governs Turbo's snapshot).
183+
# no-store opts out of both so a fresh copy is always fetched.
184+
def prevent_response_caching
185+
response.cache_control.merge!(no_store: true)
186+
end
187+
181188
def tag_request
182189
context = { controller: self, request: }
183190
::OpenProject::Appsignal.tag_request(context)

app/controllers/my_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class MyController < ApplicationController
4141
before_action :require_login
4242
before_action :set_current_user
4343
before_action :check_password_confirmation, only: %i[update_account]
44+
before_action :prevent_response_caching, only: :account
4445

4546
no_authorization_required! :account,
4647
:update_account,

app/controllers/users_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class UsersController < ApplicationController
3636
layout "admin"
3737

3838
before_action :authorize_global, except: %i[show deletion_info destroy]
39+
before_action :prevent_response_caching, only: :edit
3940

4041
# rubocop:disable Rails/LexicallyScopedActionFilter
4142
before_action :find_user, only: %i[show
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# frozen_string_literal: true
2+
3+
require "spec_helper"
4+
5+
# OP-19674: after an administrator changes a user's attributes out-of-band, the
6+
# account page must not be served stale. This example covers navigating away and
7+
# back to it through in-app menu links (a forward Turbo visit, not history).
8+
RSpec.describe "My account is fresh after navigating back to it", :js do
9+
shared_let(:admin) { create(:admin) }
10+
shared_let(:section) { create(:user_custom_field_section, name: "Professional info") }
11+
shared_let(:job_title) { create(:user_custom_field, :string, name: "Job title", user_custom_field_section: section) }
12+
shared_let(:alpha) { create(:department, name: "Alpha department", members: [admin]) }
13+
shared_let(:beta) { create(:department, name: "Beta department") }
14+
15+
before do
16+
section.update!(attribute_order: ["department", job_title.column_name])
17+
admin.update!(custom_field_values: { job_title.id => "Old title" })
18+
login_as admin
19+
20+
visit my_account_path
21+
expect(page).to have_select("user[department_id]", disabled: true, selected: "Alpha department")
22+
expect(page).to have_field("Job title", with: "Old title")
23+
24+
# An administrator moves the user and changes the job title out-of-band.
25+
Departments::AddUserService.new(beta, user: admin)
26+
.call(user_id: admin.id, remove_from_previous_department: true)
27+
admin.update!(custom_field_values: { job_title.id => "New title" })
28+
end
29+
30+
it "shows the updated Department and Job title after navigating back via the menu" do
31+
click_on "Notification and email"
32+
expect(page).to have_current_path(my_notifications_path, wait: 10, ignore_query: true)
33+
34+
click_on "Account"
35+
expect(page).to have_current_path(my_account_path, wait: 10, ignore_query: true)
36+
37+
expect(page).to have_select("user[department_id]", disabled: true, selected: "Beta department")
38+
expect(page).to have_field("Job title", with: "New title")
39+
end
40+
end
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# frozen_string_literal: true
2+
3+
require "spec_helper"
4+
5+
# OP-19674: after an administrator changes a user's attributes out-of-band, the
6+
# account page must not be served stale. This example covers a plain reload (Ctrl+R).
7+
RSpec.describe "My account is fresh after a reload", :js do
8+
shared_let(:admin) { create(:admin) }
9+
shared_let(:section) { create(:user_custom_field_section, name: "Professional info") }
10+
shared_let(:job_title) { create(:user_custom_field, :string, name: "Job title", user_custom_field_section: section) }
11+
shared_let(:alpha) { create(:department, name: "Alpha department", members: [admin]) }
12+
shared_let(:beta) { create(:department, name: "Beta department") }
13+
14+
before do
15+
section.update!(attribute_order: ["department", job_title.column_name])
16+
admin.update!(custom_field_values: { job_title.id => "Old title" })
17+
login_as admin
18+
19+
visit my_account_path
20+
expect(page).to have_select("user[department_id]", disabled: true, selected: "Alpha department")
21+
expect(page).to have_field("Job title", with: "Old title")
22+
23+
# An administrator moves the user and changes the job title out-of-band.
24+
Departments::AddUserService.new(beta, user: admin)
25+
.call(user_id: admin.id, remove_from_previous_department: true)
26+
admin.update!(custom_field_values: { job_title.id => "New title" })
27+
end
28+
29+
it "shows the updated Department and Job title after reloading" do
30+
page.refresh
31+
32+
expect(page).to have_select("user[department_id]", disabled: true, selected: "Beta department")
33+
expect(page).to have_field("Job title", with: "New title")
34+
end
35+
end

0 commit comments

Comments
 (0)