diff --git a/app/controllers/api/event_preferences_controller.rb b/app/controllers/api/event_preferences_controller.rb index 93077bd5..8f19f9b8 100644 --- a/app/controllers/api/event_preferences_controller.rb +++ b/app/controllers/api/event_preferences_controller.rb @@ -31,6 +31,10 @@ def batch_show return end + # Read the version before the preferences. A write between the two + # reads then gives an old version with new data, which the next version + # check corrects. The other order would label old data as current. + version = PreferenceVersion.for(current_user) record_ids = ids.index_with { |id| meeting_time_record_id(id) } meeting_times = Course::MeetingTime.includes(course: [ :faculties, :term ], rooms: :building) .where(id: record_ids.values.compact) @@ -48,7 +52,7 @@ def batch_show end end - render json: { preferences: preferences, missing: missing } + render json: { preferences: preferences, missing: missing, version: version } end def update diff --git a/app/controllers/api/preference_versions_controller.rb b/app/controllers/api/preference_versions_controller.rb new file mode 100644 index 00000000..321d3c4c --- /dev/null +++ b/app/controllers/api/preference_versions_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Api + class PreferenceVersionsController < ApiController + def show + response.set_header("Cache-Control", "private, no-store") + render json: { version: PreferenceVersion.for(current_user) } + end + end +end diff --git a/app/services/preference_resolver.rb b/app/services/preference_resolver.rb index 3178cf3f..2fd78053 100644 --- a/app/services/preference_resolver.rb +++ b/app/services/preference_resolver.rb @@ -46,6 +46,13 @@ class PreferenceResolver visibility: "default" }.freeze + # Every default constant that resolve_field can return. PreferenceVersion + # hashes these, so a deploy that changes one gives each user a new version. + # Add a new default constant here. + def self.defaults + [ SYSTEM_DEFAULTS, FINAL_EXAM_DEFAULTS, UNI_CAL_DEFAULTS, UNI_CAL_ALL_DAY_REMINDERS, UNI_CAL_TIMED_REMINDERS ] + end + def initialize(user) @user = user @cache = {} diff --git a/app/services/preference_version.rb b/app/services/preference_version.rb new file mode 100644 index 00000000..87b9a67e --- /dev/null +++ b/app/services/preference_version.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "digest" + +# Fingerprints preference inputs without resolving each meeting's templates. +# Include record identities and values so deletions and bulk writes also change +# the token; unrelated users and display-only settings cannot invalidate it. +class PreferenceVersion + def self.for(user) + fields = PreferenceResolver::PREFERENCE_FIELDS + inputs = { + user: user.id, + events: user.event_preferences.order(:id).pluck(:id, :preferenceable_type, :preferenceable_id, *fields), + calendars: user.calendar_preferences.order(:id).pluck(:id, :scope, :event_type, *fields), + colors: UserExtensionConfig.where(user: user).pick(:default_color_lecture, :default_color_lab), + notifications_disabled: user.notifications_disabled?, + defaults: PreferenceResolver.defaults + } + + Digest::SHA256.hexdigest(JSON.generate(inputs)) + end +end diff --git a/config/routes.rb b/config/routes.rb index 86341e5a..3ef3b299 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -153,6 +153,8 @@ collection { post :preview } end + get "user/preferences/version", to: "preference_versions#show" + # Per-event preferences (meeting time or calendar event) post "meeting_times/preferences", to: "event_preferences#batch_show" resources :meeting_times, only: [] do diff --git a/spec/requests/api/event_preferences_batch_spec.rb b/spec/requests/api/event_preferences_batch_spec.rb index 27d930ed..5b7a62ca 100644 --- a/spec/requests/api/event_preferences_batch_spec.rb +++ b/spec/requests/api/event_preferences_batch_spec.rb @@ -86,6 +86,32 @@ def query_count expect(three_count).to eq(one_count) end + it "returns the preference version that the data was read under" do + meeting_time = enroll_in(%w[11111]).first + create(:calendar_preference, user: user, title_template: "Mine: {{title}}") + + post "/api/meeting_times/preferences", + params: { meeting_time_ids: [ meeting_time.public_id ] }, headers: headers, as: :json + + expect(response).to have_http_status(:ok) + expect(json["version"]).to eq(PreferenceVersion.for(user)) + end + + it "reads the version before the preferences, so a write between them is not hidden" do + meeting_time = enroll_in(%w[11111]).first + preference = create(:calendar_preference, user: user, title_template: "Before: {{title}}") + old_version = PreferenceVersion.for(user) + allow(PreferenceVersion).to receive(:for).and_wrap_original do |original, *args| + original.call(*args).tap { preference.update_columns(title_template: "After: {{title}}") } + end + + post "/api/meeting_times/preferences", + params: { meeting_time_ids: [ meeting_time.public_id ] }, headers: headers, as: :json + + expect(json["version"]).to eq(old_version) + expect(json.dig("preferences", meeting_time.public_id, "preview", "title")).to eq("After: Data Structures") + end + it "lists ids that match no meeting time instead of failing" do meeting_time = enroll_in(%w[11111]).first diff --git a/spec/requests/api/preference_versions_spec.rb b/spec/requests/api/preference_versions_spec.rb new file mode 100644 index 00000000..2ebc2415 --- /dev/null +++ b/spec/requests/api/preference_versions_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "GET /api/user/preferences/version", type: :request do + let(:user) { create(:user) } + let(:headers) { auth_headers_for(user) } + + def json = JSON.parse(response.body) + + it "returns only the signed-in user's opaque token and prohibits response caching" do + create(:event_preference, user: user, title_template: "Private title") + expected_version = PreferenceVersion.for(user) + + get "/api/user/preferences/version", headers: headers + + expect(response).to have_http_status(:ok) + expect(response.media_type).to eq("application/json") + expect(json).to eq("version" => expected_version) + expect(json["version"]).to match(/\A[0-9a-f]{64}\z/) + expect(response.headers["Cache-Control"]).to eq("private, no-store") + + get "/api/user/preferences/version", headers: headers + + expect(response).to have_http_status(:ok) + expect(json["version"]).to eq(expected_version) + end + + it "reflects persisted preference changes on the next request" do + preference = create(:calendar_preference, user: user, title_template: "Original") + + get "/api/user/preferences/version", headers: headers + expect(response).to have_http_status(:ok) + version = json.fetch("version") + + preference.update_columns(title_template: "Changed") + + get "/api/user/preferences/version", headers: headers + + expect(response).to have_http_status(:ok) + expect(json.fetch("version")).not_to eq(version) + end + + it "does not accept a different account through request parameters" do + other_user = create(:user) + create(:event_preference, user: other_user, title_template: "Other user's title") + + get "/api/user/preferences/version", params: { user_id: other_user.id }, headers: headers + + expect(response).to have_http_status(:ok) + expect(json).to eq("version" => PreferenceVersion.for(user)) + expect(json["version"]).not_to eq(PreferenceVersion.for(other_user)) + end + + it "requires authentication" do + get "/api/user/preferences/version" + + expect(response).to have_http_status(:unauthorized) + expect(json["code"]).to eq("AUTH_MISSING") + expect(json).not_to have_key("version") + end + + it "rejects a revoked session" do + token = api_token_for(user) + UserSession.find_by!(jti: JsonWebTokenService.decode(token)[:jti]).revoke! + + get "/api/user/preferences/version", headers: { "Authorization" => "Bearer #{token}" } + + expect(response).to have_http_status(:unauthorized) + expect(json["code"]).to eq("AUTH_REVOKED") + expect(json).not_to have_key("version") + end + + it "does not resolve templates or enqueue calendar sync work" do + create(:event_preference, user: user, title_template: "Saved title") + request_headers = headers + expect(PreferenceResolver).not_to receive(:new) + expect(CalendarTemplateRenderer).not_to receive(:new) + expect(GoogleCalendarSyncJob).not_to receive(:perform_later) + + get "/api/user/preferences/version", headers: request_headers + + expect(response).to have_http_status(:ok) + end +end diff --git a/spec/services/preference_version_spec.rb b/spec/services/preference_version_spec.rb new file mode 100644 index 00000000..26196dcc --- /dev/null +++ b/spec/services/preference_version_spec.rb @@ -0,0 +1,163 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe PreferenceVersion do + include ActiveSupport::Testing::TimeHelpers + + let(:user) { create(:user) } + + before { allow(GoogleCalendarSyncJob).to receive(:perform_later) } + + it "returns a stable opaque token for unchanged persisted inputs" do + create(:event_preference, user: user) + create(:calendar_preference, user: user) + + version = described_class.for(user) + + expect(version).to match(/\A[0-9a-f]{64}\z/) + expect(described_class.for(user.reload)).to eq(version) + end + + { + title_template: "Saved title", + description_template: "Saved description", + location_template: "Saved location", + reminder_settings: [ { "time" => "45", "type" => "minutes", "method" => "popup" } ], + color_id: "#abcdef", + visibility: "private" + }.each do |field, value| + [ :event_preference, :calendar_preference ].each do |factory| + it "detects a bulk #{field} edit to a #{factory} without a timestamp change" do + preference = create(factory, user: user) + version = described_class.for(user) + timestamp = preference.updated_at + + preference.class.where(id: preference.id).update_all(field => value) + + expect(preference.reload.updated_at).to eq(timestamp) + expect(described_class.for(user)).not_to eq(version) + end + end + end + + [ :event_preference, :calendar_preference ].each do |factory| + it "detects creation and deletion of a #{factory}" do + empty_version = described_class.for(user) + preference = create(factory, user: user) + populated_version = described_class.for(user) + + expect(populated_version).not_to eq(empty_version) + + preference.class.where(id: preference.id).delete_all + + expect(described_class.for(user)).to eq(empty_version) + end + + it "detects nested reminder changes in a #{factory}" do + preference = create(factory, user: user, + reminder_settings: [ { "time" => "30", "type" => "minutes", "method" => "popup" } ]) + version = described_class.for(user) + + preference.update!(reminder_settings: [ { "time" => "30", "type" => "minutes", "method" => "email" } ]) + + expect(described_class.for(user)).not_to eq(version) + end + + it "distinguishes inherited reminders from disabled reminders in a #{factory}" do + preference = create(factory, user: user, title_template: "Saved title", reminder_settings: nil) + version = described_class.for(user) + + preference.update!(reminder_settings: []) + + expect(described_class.for(user)).not_to eq(version) + end + + it "ignores timestamp-only changes to a #{factory}" do + preference = create(factory, user: user) + version = described_class.for(user) + + preference.update_columns(updated_at: preference.updated_at + 1.hour) + + expect(described_class.for(user)).to eq(version) + end + end + + [ :default_color_lecture, :default_color_lab ].each do |field| + it "detects a change to #{field}" do + version = described_class.for(user) + + user.user_extension_config.update!(field => "#abcdef") + + expect(described_class.for(user)).not_to eq(version) + end + end + + it "ignores display-only settings" do + config = user.user_extension_config + version = described_class.for(user) + + config.update!(military_time: !config.military_time, + advanced_editing: !config.advanced_editing, + show_historic_terms: !config.show_historic_terms) + + expect(described_class.for(user)).to eq(version) + end + + it "detects notification suppression and its expiry without another write" do + travel_to(Time.zone.local(2026, 10, 7, 12, 0)) do + enabled_version = described_class.for(user) + user.update!(notifications_disabled_until: 5.minutes.from_now) + disabled_version = described_class.for(user) + + expect(disabled_version).not_to eq(enabled_version) + + travel 6.minutes + + expect(described_class.for(user)).to eq(enabled_version) + end + end + + it "detects notifications being enabled again" do + user.disable_notifications! + version = described_class.for(user) + + user.enable_notifications! + + expect(described_class.for(user)).not_to eq(version) + end + + it "isolates tokens from another user's preference, color, and notification changes" do + other_user = create(:user) + version = described_class.for(user) + + expect(described_class.for(other_user)).not_to eq(version) + + event_preference = create(:event_preference, user: other_user, title_template: "Other title") + calendar_preference = create(:calendar_preference, user: other_user, title_template: "Other title") + other_user.user_extension_config.update!(default_color_lecture: "#abcdef", default_color_lab: "#123456") + other_user.disable_notifications! + + expect(described_class.for(user)).to eq(version) + + event_preference.update!(title_template: "Other edit") + calendar_preference.update!(reminder_settings: []) + event_preference.destroy! + calendar_preference.destroy! + + expect(described_class.for(user)).to eq(version) + end + + [ :SYSTEM_DEFAULTS, :FINAL_EXAM_DEFAULTS, :UNI_CAL_DEFAULTS, :UNI_CAL_ALL_DAY_REMINDERS, :UNI_CAL_TIMED_REMINDERS ].each do |constant| + it "detects a change to resolver #{constant}" do + version = described_class.for(user) + original = PreferenceResolver.const_get(constant) + replacement = [ { "time" => "45", "type" => "minutes", "method" => "popup" } ] + replacement = original.merge(title_template: "New default title") if original.is_a?(Hash) + + stub_const("PreferenceResolver::#{constant}", replacement) + + expect(described_class.for(user)).not_to eq(version) + end + end +end